firebase / firebase-android-sdk

Firebase Android SDK
https://firebase.google.com
Apache License 2.0
2.25k stars 572 forks source link

Add executor parameter to read requests #1473

Open bolds07 opened 4 years ago

bolds07 commented 4 years ago

What feature would you like to see?

An Executor parameter to the method addListenerForSingleValueEvent on realtime database api to allow the result be processed in a background thread.

How would you use it?

As a standard all firebase apis allow adding Executor to allow the task listener process the results on a background thread. for some reason the event "read" doesn't have same behavior. I know it is very easy to get the results of addListenerForSingleValueEvent and start a new thread to process them... but there ISNT ANY REASON FOR THIS METHOD NOT FOLLOW THE API STRUCTURE... all other methods in all other firebase apis to android return a task which can receive a Executor

example

FirebaseDatabase.getInstance().getReference("blablabla").removeValue().addOnSuccessListener(AsyncTask.THREAD_POOL_EXECUTOR,something);
        FirebaseDatabase.getInstance().getReference("blablabla").setValue(null).addOnSuccessListener(AsyncTask.THREAD_POOL_EXECUTOR,something);
        FirebaseAuth.getInstance().signInAnonymously().addOnSuccessListener(AsyncTask.THREAD_POOL_EXECUTOR,something);
        FirebaseStorage.getInstance().getReference("blablabla").getFile(file).addOnCompleteListener(AsyncTask.THREAD_POOL_EXECUTOR,something);

        FirebaseDatabase.getInstance().getReference("blablabla").addListenerForSingleValueEvent(new ValueEventListener() {
            @Override
            public void onDataChange(@NonNull DataSnapshot dataSnapshot) {
                AsyncTask.THREAD_POOL_EXECUTOR.execute(() -> {
                    something              
                });
            }

            @Override
            public void onCancelled(@NonNull DatabaseError databaseError) {

            }
        });

As it is noticeable in this short snippet the "read" event of firebase database completely breaks the code standard and i dont see one single reason for the realtime database api not having a read method which returns a task that can be processed in the backgrond

google-oss-bot commented 4 years ago

I couldn't figure out how to label this issue, so I've labeled it for a human to triage. Hang tight.

schmidt-sebastian commented 4 years ago

@bolds07 Thank you for your feature request! As you stated, the Realtime Database API currently does not use the Task API and instead relies on a callback pattern that is used throughout the SDK. We have certainly seen demand for first-class support of the Task API and have added this to Firestore already. If there is considerable demand for the RTDB, we can expose this here as well.

Note that addListenerForSingleValueEvent() will currently always return a cached value if one is available and not wait to synchronize this value with the backend. Therefore, it is usually preferable to register a listener via addValueEventListener(), which allows you to receive updates as the backend synchronization completes.

bolds07 commented 4 years ago

Note that addListenerForSingleValueEvent() will currently always return a cached value if one is available and not wait to synchronize this value with the backend

this could so easily be solved with a sync and an unsync method... WOULD EVEN BE CLEARER to the develop when the value he is getting as a return is just a cached one or it is realtime one (BELIEVE ME, so many people get problems with this)

imho you should keep the value event listener for people who need behavior like this, replace the addListenerForSingleValueEvent for a task api return and have a a different method to return cached values