Asynchronous processing issue with Firebase Realtime database and need completion handler

Question

I created the following interface:

 private interface AllPicksFirebaseCallback {

    void onSuccess(DataSnapshot dataSnapshot);

    void onStart();

    void onFailure();

}

and created the following readData function:

private void readData(Query query, SurvivorAllPicksActivity.AllPicksFirebaseCallback listener) {

    listener.onStart();

    query.addListenerForSingleValueEvent(new ValueEventListener() {

        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {

            if (dataSnapshot.exists()) {

                listener.onSuccess(dataSnapshot);

            } else { 

                //dataSnapshot doesn't exist

            }

        }

        @Override
        public void onCancelled(DatabaseError databaseError) {

            Log.d(TAG, databaseError.getMessage());

            listener.onFailure();

        }

    });

}

My Activity class has the following:

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);

    playerList = new ArrayList<>();

    playerInPoolReference = mFirebaseDatabase.getReference("POOLPLAYERS");

    playerInPoolQuery = playerInPoolReference.child(currPool).orderByValue().equalTo(true);

    readData(playerInPoolQuery, new SurvivorAllPicksActivity.AllPicksFirebaseCallback() {

        @Override
        public void onSuccess(DataSnapshot dataSnapshot) {

            int counter = 0;

            for (DataSnapshot snapshot : dataSnapshot.getChildren()) {

                String userId = snapshot.getKey();

                setPlayerUsername(userId, counter, dataSnapshot.getChildrenCount());

                counter = counter + 1;

            }

        }

        @Override
        public void onStart() {

            Log.d("ONSTART", "Started");

        }

        @Override
        public void onFailure() {

            Log.d("onFailure", "Failed");

        }

    });

}


public void setPlayerUsername(String userId, final int position, long totalCount) {

    userReference = mFirebaseDatabase.getReference("Users");

    userQuery = userReference.child(userId).child("username");

    // Add handle for listener
    userQuery.addListenerForSingleValueEvent(new ValueEventListener() {

        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {

            if (dataSnapshot.exists()) {

                String username = dataSnapshot.getValue().toString();

                SurvivorAllPicks result = new SurvivorAllPicks(username);
                //
                playerList.add(result);


                survivorPicksReference = mFirebaseDatabase.getReference("SURVIVORPICKS");

                survivorPicksQuery = survivorPicksReference.child(currPool).child(userId);

                // Completion Handler for Player Picks Lookups
                readPlayerSurvivorPicks(survivorPicksQuery, new SurvivorAllPicksActivity.PlayerPicksCallback() {

                    @Override
                    public void onSuccess(DataSnapshot dataSnapshot) {

                        int pickIndex = 0;

                        for (DataSnapshot allPicksSnapshot : dataSnapshot.getChildren()) {
                          
                            // *** NEED COMPLETION HANDLER ***

                            // Set the Players Picks Info here
                            playerList.get(position).setSurvivorAllPicks(allPicksSnapshot);

                            // Call the rest of the lookups based on the playerList info populated just above
                            setGameStatus(playerList, position, pickIndex, totalCount);

                            pickIndex = pickIndex + 1;

                            // *** NEED COMPLETION HANDLER ***

                        }

                    }

                    @Override
                    public void onStart() {

                        Log.d("ONSTART", "Started");

                    }

                    @Override
                    public void onFailure() {

                        Log.d("onFailure", "Failed");

                    }

                });

            } else { 

                // dataSnapshot doesn't exist

            }

        }

        @Override
        public void onCancelled(DatabaseError databaseError) {

            Log.w("SurvivorAllPicksActivity", "onCancelled", databaseError.toException());

        }

    });

}

With the interface for the above code:

private interface PlayerPicksCallback {

    void onSuccess(DataSnapshot dataSnapshot);

    void onStart();

    void onFailure();

}

private void readPlayerSurvivorPicks(Query query, SurvivorAllPicksActivity.PlayerPicksCallback listener) {

    listener.onStart();

    query.addListenerForSingleValueEvent(new ValueEventListener() {

        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {

            if (dataSnapshot.exists()) {

                listener.onSuccess(dataSnapshot);

            } else { 

                // dataSnapshot doesn't exist

            }

        }

        @Override
        public void onCancelled(DatabaseError databaseError) {

            Log.d(TAG, databaseError.getMessage());
            
            listener.onFailure();

        }

    });

}


public void setGameStatus(List<SurvivorAllPicks> playerList, final int position, int index, long totalCount) {

    gameReference = mFirebaseDatabase.getReference("Games");

    gameQuery = gameReference.child(playerList.get(position).getGameId()).child("isPlayed");

    // Add handle for listener
    gameQuery.addListenerForSingleValueEvent(new ValueEventListener() {

        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {

            if (dataSnapshot.exists()) {

                Boolean gameStatus = (Boolean) dataSnapshot.getValue();

                setPickInfo(playerList, gameStatus, position, index, totalCount);

            }

        }

        @Override
        public void onCancelled(DatabaseError databaseError) {

            Log.w("SurvivorAllPicksActivity", "onCancelled", databaseError.toException());

        }

    });

}

public void setPickInfo(List<SurvivorAllPicks> playerList, Boolean gameStatus, final int position, int index, long totalCount) {

    teamReference = mFirebaseDatabase.getReference("Teams");

    teamQuery = teamReference.child(playerList.get(position).getTeamId());

    // Add handle for listener
    teamQuery.addListenerForSingleValueEvent(new ValueEventListener() {

        @Override
        public void onDataChange(DataSnapshot dataSnapshot) {

            if (dataSnapshot.exists()) {

                String teamLogo = dataSnapshot.child("Logo").getValue().toString();

                String teamInit = dataSnapshot.child("Init").getValue().toString();

                // Set the PicksDetails here!!!
                playerList.get(position).setSurvivorAllPicksDetails(teamLogo, teamInit, index, gameStatus, playerList.get(position).getPoolStatus()); 

            }

            if (position == (totalCount - 1)) {

                setContentView(new SurvivorAllPickLayout(getApplicationContext(), playerList));

            }

        }

        @Override
        public void onCancelled(DatabaseError databaseError) {

            Log.w("SurvivorAllPicksActivity", "onCancelled", databaseError.toException());

        }

    });

}

My issue is the need for a completion handler; I marked the code where i think i need it.

What is happening is that the code keeps moving along and by the time the realtime database lookups are executed the lookup for the second iteration is already executing and it replicates the result for the array list.

How can i alter my code to stop that from happening? I need to make the lookup complete first for the first lookup before it moves forward for the second lookup and so on!?

Answer 1

The problem with your current workflow is that you are assuming that the firebase callbacks return in the order you call them, and that you can access data in an array based on that order, but that may not always be the case. As I understand your workflow you have several dependent queries:

1. Get a list of user ids
2. For each user ID, get some data with several separate queries (e.g. user name, teamId, gameId)
3. For each user, make a query to get some dependent data using their userId and their teamId or gameId (so these queries depend on the results of step 2)

I have included an example workflow below using a map of UserId -> Player that works more robustly with many chained queries like this.

To make this sort of thing easier to test, I have included a mock Firebase class that can help test out async workflows by initiating callbacks with random delays and returning some expected data you provide.

Because data is being returned in potentially non-deterministic order, your Player class needs to know when it is completely loaded. I added an isLoaded() method to the dummy Player class I used here that checks if its fields are all non-empty.

Using these components, I set up a workflow that mimics your example using a ViewModel and LiveData:

public class MainViewModel extends ViewModel {

    // Create a LiveData of a list of Players, the activity
    // will observe this to wait for it to be loaded
    private final MutableLiveData<List<Player>> playerList = new MutableLiveData<>();
    LiveData<List<Player>> getPlayers() {
        return playerList;
    }

    private final FirebaseMock firebase = new FirebaseMock();
    private final HashMap<String, Player> playerData = new HashMap<>();
    private int totalPlayers = 0;
    private int loadedPlayers = 0;

    void load() {
        totalPlayers = 0;
        loadedPlayers = 0;
        playerData.clear();

        firebase.runQueryForList(
            Arrays.asList("1", "2", "3", "4"), // userIds
            userIds -> {
                totalPlayers = userIds.size();
                for(String userId : userIds) {
                    System.out.println("TEST: got user id " + userId);
                    playerData.put(userId, new Player());
                    playerData.get(userId).userId = userId;

                    // neither of these queries depend on each other, so you
                    // can start them at the same time
                    getUsername(userId);
                    getTeamAndGameIds(userId);
                }
            }
        );
    }


    private void getUsername(String userId) {
        firebase.runQueryForValue(
            userId+"name", // userName
            data -> {
                System.out.println("TEST: got user name " + data + " for " + userId);
                playerData.get(userId).userName = data;
                checkLoaded(userId); // need to call checkLoaded for any potentially final result (non-dependent)
            }
        );
    }

    private void getTeamAndGameIds(String userId) {
        HashMap<String,String> expected = new HashMap<>();
        expected.put("team", "123"+userId);
        expected.put("game", "abc"+userId);

        firebase.runQueryForMap(
            expected, // team and game IDs
            data -> {
                System.out.println("TEST: got team/game ids " + data + " for " + userId);
                playerData.get(userId).teamId = data.get("team");
                playerData.get(userId).gameId = data.get("game");
                getTeamInfo(userId);
                getGameInfo(userId);

                // do not need to call checkLoaded from here since this
                // launched dependent queries
            }
        );
    }

    private void getTeamInfo(String userId) {
        String teamId = playerData.get(userId).teamId;
        firebase.runQueryForValue(
            "logo"+teamId, // team logo
            data -> {
                System.out.println("TEST: got team logo " + data + " for " + userId);
                playerData.get(userId).teamLogo = data;
                checkLoaded(userId);
            }
        );
    }

    private void getGameInfo(String userId) {
        String gameId = playerData.get(userId).gameId;
        firebase.runQueryForValue(
            "gameStatus"+gameId, // game status
            data -> {
                System.out.println("TEST: got game status " + data + " for " + userId);
                playerData.get(userId).gameStatus = data;
                checkLoaded(userId);
            }
        );
    }

    private void checkLoaded(String userId) {
        if( playerData.get(userId).isLoaded()) {
            ++loadedPlayers;
        }

        if( loadedPlayers == totalPlayers ) {
            // notify the activity that the data is ready
            playerList.postValue(new ArrayList<>(playerData.values()));
        }
    }
}

To robustly handle failures, you will need to set a failure flag on the Player when one of the queries fails and check for that in checkLoaded as well (either count is as being "loaded" or decrement totalPlayers and remove it from the playerData map).

The activity would initiate loading by calling load() on the ViewModel, and observe the list of Players to see when they are all loaded.

@Override
protected void onCreate(Bundle savedInstanceState) {
    super.onCreate(savedInstanceState);
    setContentView(R.layout.activity_main);

    MainViewModel model = new ViewModelProvider(this).get(MainViewModel.class);

    final Observer<List<Player>> userObserver = userList -> {
        // Update the UI
        // - hide the progress bar
        // - show the main views
        // - post values to adapters, update textviews, etc
        System.out.println("TEST: got data " + userList);
        Toast.makeText(this, "ALL DATA LOADA", Toast.LENGTH_LONG).show();
    };

    model.getPlayers().observe(this, userObserver);
    model.load();

    // Set the UI to the "loading" state
    // - show a progress bar
    // - hide the main views
}

The mock firebase class is defined below. Putting your firebase calls behind an interface that you could mock like this can help with testing too.

public class FirebaseMock {
    public interface OnGotList {
        void onComplete(List<String> data);
    }

    public interface OnGotMap {
        void onComplete(Map<String,String> data);
    }

    public interface OnGotValue {
        void onComplete(String data);
    }

    private final Handler handler = new Handler();
    private final Random random = new Random();

    void runQueryForList(List<String> expected, OnGotList callback) {
        int delay = 500 + random.nextInt(4000);
        handler.postDelayed(() -> callback.onComplete(expected), delay);
    }

    void runQueryForMap(Map<String,String> expected, OnGotMap callback) {
        int delay = 500 + random.nextInt(4000);
        handler.postDelayed(() -> callback.onComplete(expected), delay);
    }

    void runQueryForValue(String expected, OnGotValue callback) {
        int delay = 500 + random.nextInt(4000);
        handler.postDelayed(() -> callback.onComplete(expected), delay);
    }
}

And for completeness, the dummy Player class I used for this:

public class Player {
    String userId = "";
    String userName = "";
    String teamId = "";
    String gameId = "";
    String teamLogo = "";
    String gameStatus = "";

    Boolean isLoaded() {
        return !userId.isEmpty() &&
               !userName.isEmpty() &&
               !teamId.isEmpty() &&
               !gameId.isEmpty() &&
               !teamLogo.isEmpty() &&
               !gameStatus.isEmpty();
    }
}