Setting Singleton property value in Firebase Listener

Question

I'm currently testing out Firebase along with a Singleton model I plan to use to access during the lifecycle of the whole app. I'm now stuck with something that seems really trivial but I can't figure it out for the life of me. I have a sample of the model I use: Bookmarks in firebase.

public class BookSingleton {



private static BookSingleton model;

private ArrayList<BookMark> bookmarks = new ArrayList<BookMark>();


public static BookSingleton getModel()
{
    if (model == null)
    {
        throw new IllegalStateException("The model has not been initialised yet.");
    }

    return model;
}


public ArrayList<Bookmark> theBookmarkList()
{
    return this.bookmarks;
}


public void setBookmarks(ArrayList<Bookmark> bookmarks){
    this.bookmarks = bookmarks;
}


public void loadModelWithDataFromFirebase(){
    Firebase db = new Firebase(//url);
    Firebase bookmarksRef = fb.child(//access correct child);


    final ArrayList<Bookmark> loadedBookmarks = new ArrayList<Bookmark>();
    bookmarksRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
                    //getting all properties from firebase...
                    Bookmark bookmark = new Bookmark(//properties here);
                    loadedBookmarks.add(bookmark);



                }
            }
            //bookmarks still exist here at this point
            setBookmarks(loadedBookmarks);

        }

        @Override
        public void onCancelled(FirebaseError firebaseError) {

        }
    });
    //by now loadedBookmarks is empty
    //this is probably the issue?
    //even without this line bookmarks is still not set in mainactivity
    setBookmarks(loadedBookmarks);
}

Now when I start the mainActivity with the instance of the Singleton set I get a null error because clearly the function I wrote to load the model data from firebase sets nothing.

Something like this: MainActivity

public class MainActivity extends AppCompatActivity {

private BookSingleton theModel;



@Override
protected void onCreate(Bundle savedInstanceState) {

    super.onCreate(savedInstanceState);

    // Load the model
    theModel = BookSingleton.getModel(this);
      //manually setting this works 
      //        ArrayList<Book> bookSamples = new ArrayList<Book>;
      //        bookSamples.add(aBookSample);

    theModel.loadModelWithSampleData(bookSamples);
    //should have set the singleton model property Bookmarks to the results from firebase

    theModel.loadModelWithDataFromFirebase();
    //returns 0
    Log.d(TAG, "" + theModel.theBookmarkList().size());


    setContentView(R.layout.activity_main);

    //......rest of code

How can I make this work?

Obediah

Just don't use Singleton. Avoid it at all. – Brownley

That's really quick, and yeah I'll probably skip on using Singleton, but I still kind of want to know why since I might face the issue again even without using Singleton. – Obediah

Answer 1

Firebase loads and synchronizes data asynchronously. So your loadModelWithDataFromFirebase() doesn't wait for the loading to finish, it just starts loading the data from the database. By the time your loadModelWithDataFromFirebase() function returns, the loading hasn't finished yet.

You can easily test this for yourself with some well-placed log statements:

public void loadModelWithDataFromFirebase(){
    Firebase db = new Firebase(//url);
    Firebase bookmarksRef = fb.child(//access correct child);

    Log.v("Async101", "Start loading bookmarks");
    final ArrayList<Bookmark> loadedBookmarks = new ArrayList<Bookmark>();
    bookmarksRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            Log.v("Async101", "Done loading bookmarks");
            //getting all properties from firebase...
            Bookmark bookmark = new Bookmark(//properties here);
            loadedBookmarks.add(bookmark);
        }

        @Override
        public void onCancelled(FirebaseError error) { throw error.toException(); }
    });
    Log.v("Async101", "Returning loaded bookmarks");
    setBookmarks(loadedBookmarks);
}

Contrary to what you likely expect, the order of the log statements will be:

Start loading bookmarks
Returning loaded bookmarks
Done loading bookmarks

You have two choice for dealing with the asynchronous nature of this loading:

1. squash the asynchronous bug (usually accompanied by muttering of phrases like: "it was a mistake, these people don't know what they're doing")

2. embrace the asynchronous beast (usually accompanied by quite some hours of cursing, but after a while by peace and better behaved applications)

Take the blue pill - make the asynchronous call behave synchronously

If you feel like picking the first option, a well placed synchronization primitive will do the trick:

public void loadModelWithDataFromFirebase() throws InterruptedException {
    Firebase db = new Firebase(//url);
    Firebase bookmarksRef = fb.child(//access correct child);

    Semaphore semaphore = new Semaphore(0);

    final ArrayList<Bookmark> loadedBookmarks = new ArrayList<Bookmark>();
    bookmarksRef.addListenerForSingleValueEvent(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            Bookmark bookmark = new Bookmark(//properties here);
            loadedBookmarks.add(bookmark);
            semaphore.release();
        }

        @Override
        public void onCancelled(FirebaseError error) { throw error.toException(); }
    });
    semaphore.acquire();
    setBookmarks(loadedBookmarks);
}

Update (20160303): when I just tested this on Android, it blocked my app. It works on a regular JVM fine, but Android is more finicky when it comes to threading. Feel free to try and make it work... or

Take the red pill - deal with the asynchronous nature of data synchronization in Firebase

If you instead choose to embrace asynchronous programming, you should rethink your application's logic.

You currently have "First load the bookmarks. Then load the sample data. And then load even more."

With an asynchronous loading model, you should think like "Whenever the bookmarks have loaded, I want to load the sample data. Whenever the sample data has loaded, I want to load even more."

The bonus of thinking this way is that it also works when the data may be constantly changing and thus synchronized multiple times: "Whenever the bookmarks change, I want to also load the sample data. Whenever the sample data changes, I want to load even more."

In code, this leads to nested calls or event chains:

public void synchronizeBookmarks(){
    Firebase db = new Firebase(//url);
    Firebase bookmarksRef = fb.child(//access correct child);

    final ArrayList<Bookmark> loadedBookmarks = new ArrayList<Bookmark>();
    bookmarksRef.addValueEventListener(new ValueEventListener() {
        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {
            Bookmark bookmark = new Bookmark(//properties here);
            loadedBookmarks.add(bookmark);
            setBookmarks(loadedBookmarks);
            loadSampleData();
        }

        @Override
        public void onCancelled(FirebaseError error) { throw error.toException(); }
    });
}

In the above code we don't just wait for a single value event, we instead deal with all of them. This means that whenever the bookmarks are changed, the onDataChange is executed and we (re)load the sample data (or whatever other action fits your application's needs).

To make the code more reusable, you may want to define your own callback interface, instead of calling the precise code in onDataChange. Have a look at this answer for a good example of that.

Aphra

Semaphores looks really hackey to me so I didn't try to implement it. Your explanation, however, really helped me in understanding the asynchronous nature of Firebase, which leads me to think on setting all logic in the event listener instead. Thank you very much :D – Obediah

So does that mean that the concept of layers should not be used in firebase database. For example I wanted to write a DataAccess layer in which I would call every query seperately and then call this method in DataAccess class in my Activity, but I start to wonder if this is correct here. So I should just paste my query code inside every Activity or what? – Terzetto

You can use a CountDownLatch to easily transform into synchronous with just a couple of lines. Sometimes things don't play well together and it becomes necessary. eg the widget doesn't support async. – Titos

@AnthonyWijaya Oh no, by no means hacky! This is one of the main uses of a semaphore (and mutex too)! That said, like Mr. Frank says, can cause blocking / ANR / crash in Android apps... so that would be a valid reason to embrace asynchronous calls. – Magbie

Answer 2

TL;DR: Embrace Firebase Asynchronicity

As I mentioned in another post, you can deal with the asynchronous nature of Firebase using promises. It would be like this:

public Task<List<Data>> synchronizeBookmarks(List<Bookmark> bookmarks) {
     return Tasks.<Void>forResult(null)
        .then(new GetBook())
        .then(new AppendBookmark(bookmarks))
        .then(new LoadData())
}

public void synchronizeBookmarkWithListener() {
     synchronizeBookmarks()
         .addOnSuccessListener(this)
         .addOnFailureListener(this);
}

com.google.android.gms.tasks

Google API for Android provides a task framework (just like Parse did with Bolts), which is similar to JavaScript promises concept.

First you create a Task for downloading the bookmark from Firebase:

class GetBook implements Continuation<Void, Task<Bookmark>> {

    @Override
    public Task<Bookmark> then(Task<Void> task) {
        TaskCompletionSource<Bookmark> tcs = new TaskCompletionSource();

        Firebase db = new Firebase("url");
        Firebase bookmarksRef = db.child("//access correct child");

        bookmarksRef.addValueEventListener(new ValueEventListener() {
            @Override
            public void onDataChange(DataSnapshot dataSnapshot) {
                tcs.setResult(dataSnapshot.getValue(Bookmark.class));
            }
        });

        tcs.getTask();
    }

}

Now that you got the idea, supose that setBookmarks and loadSampleData are also asynchronous. You also can create them as Continuation tasks (just like the previous one) that will run in sequence:

class AppendBookmark(List<Bookmark> bookmarks) implements
    Continuation<List<Bookmark>, Task<Bookmark> {

    final List<Bookmark> bookmarks;

    LoadBookmarks(List<Bookmark> bookmarks) {
        this.bookmark = bookmark;
    }

    @Override
    Task<List<Bookmark>> then(Task<Bookmark> task) {
        TaskCompletionSource<List<Bookmark>> tcs = new TaskCompletionSource();
        bookmarks.add(task.getResult());         
        tcs.setResult(this.bookmarks);
        return tcs.getTask();
    }
}

class LoadSampleData implements Continuation<List<Bookmark>, List<Data>> {
    @Override
    public Task<List<Data>> then(Task<List<Bookmark>> task) {
        // ...
    }
}

Mere

Upvote for diligently using the TaskCompletionSource API... it is a beautiful task based utility... (although it is a glorified repackaged semaphore... :P) – Magbie

Answer 3

You have to initialize your Singleton when the class is loaded. Put this on your code:

private static BookSingleton  model = new BookSingleton();

private BookSingleton() {
}

public static BookSingleton getModel() {     return model == null ? new BookSingleton() : model;
}

Kingwood