Add a callback in FirebaseIndexArray when a Key doesn't exist in a ref #448

SUPERCILEX · 2016-12-09T04:50:03Z

@puf Please review.

The Big Kahuna of firebase-ui-database #544

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

puf · 2016-12-13T17:43:46Z

database/src/main/java/com/firebase/ui/database/OnIndexMismatchListener.java

+
+import com.google.firebase.database.DataSnapshot;
+
+interface OnIndexMismatchListener {


Remove the On prefix, to align with existing Firebase listeners: ValueEventListener, ChildEventListener.

I'm not really convinced of this class name. It's very specific to this use-case, while we might uncover other use-cases in the future that could use this same interface. If the snapshot is from the originating/key collection, I can see this interface defining a "join this data" contract.

Could we change the behavior from an error to an "override point". So instead of this being the error condition, make it the regular handler for joins? interface JoinResolver, that developers can then implement/override to have their own join/error handling? Thoughts? @SUPERCILEX @samtstern @morganchen12

@puf, so were you thinking of something like this:

@Override public void onChildAdded(DataSnapshot keySnapshot, String previousChildKey) { // ... Query ref = mJoinResolver.onJoin(keySnapshot); // Get the data ref mRefs.put(ref, ref.addValueEventListener(new DataRefListener())); } @Override public void onChildRemoved(DataSnapshot keySnapshot) { Query removeQuery = mJoinResolver.onDisjoin(keySnapshot); // Get the remove ref removeQuery.removeEventListener(mRefs.remove(removeQuery)); // ... }

private class DataRefListener implements ValueEventListener { @Override public void onDataChange(DataSnapshot snapshot) { if (snapshot.getValue() != null) { // ... } else { if (isMatch(index, key)) { // ... } else { mJoinResolver.onJoinFailed(index, snapshot); // notify failure } } } }

Yup. That first snippet looks like it. Not sure about the second snippet though. What would the mJoinResolver consist of there?

@puf the second snippet is occurs when dataRef returns a null snapshot. The original purpose of this PR was to change that piece of code from a log (Log.w(TAG, "Key not found at ref: " + snapshot.getRef());) to a callback. Is it still okay to follow through with mJoinResolver.onJoinFailed(index, snapshot);?

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

…lback

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

SUPERCILEX

@puf It turned out to be more complicated than I thought so this PR is pretty big now, sorry.

I found out today that you can't pass in this while initializing a super constructor. Because of this I had to follow the setter pattern for JoinResolver, but this makes future open sourcing of FirebaseIndexArray and any current implementations unacceptable because people will think that setting a JoinResolver is optional and we would not be able to get the join query. Also, how do we know an onChildAdded event won't be called between the time when we start listening for event and when the JoinResolver is set? To solve this problem, I pretty significantly changed FirebaseArray and FirebaseIndexArray's apis (thank the api god we didn't open source FirebaseArray yet! 😄). Both FirebaseArray and FirebaseIndexArray now follow a start stop pattern like the activity lifecycle: you create stuff and then tear it down. In addition, listeners are no longer optional because a FirebaseArray that never reports back its results is pointless. I made what I believe to be a really nice api (@puf I'd love your thoughts on this!) where you can setup your listeners and then start listening for events once your ready. If you so choose, you may also stop listening for events in effect resetting FirebaseArray. However, the listeners and queries are still present which enables resuming listening at a later time. To prevent errors, all of this is cross checked.

I think my changes make FirebaseArray and FirebaseIndexArray much more ready for open sourcing with nicer apis, @puf what are your thoughts on these changes?

SUPERCILEX · 2016-12-15T02:27:54Z

app/src/main/java/com/firebase/uidemo/database/Chat.java

@@ -0,0 +1,41 @@
+package com.firebase.uidemo.database;
+
+public class Chat {


I extracted Chat and ChatHolder to simplify ChatActivity.

SUPERCILEX · 2016-12-15T02:28:55Z

app/src/main/java/com/firebase/uidemo/database/ChatActivity.java

-                chatView.setName(chat.getName());
-                chatView.setText(chat.getMessage());
+        mRecyclerViewAdapter =
+                new FirebaseIndexRecyclerAdapter<Chat, ChatHolder>(


Because FirebaseIndexRecyclerAdapter is slightly more complicated and less documented, I think we should demo that in the sample.

SUPERCILEX · 2016-12-15T02:29:45Z

app/src/main/java/com/firebase/uidemo/database/ChatActivity.java

+                        attachRecyclerViewAdapter();
+                    }
+                })
+                .addOnCompleteListener(new OnCompleteListener<AuthResult>() {


Making the Toast survive activity rotation.

SUPERCILEX · 2016-12-15T02:36:26Z

database/src/main/java/com/firebase/ui/database/FirebaseArray.java

 */
 class FirebaseArray implements ChildEventListener {
-    public interface OnChangedListener {
+    public interface ChangeEventListener {


Changed the name to follow ChildEventListener.

SUPERCILEX · 2016-12-15T02:37:36Z

database/src/main/java/com/firebase/ui/database/FirebaseArray.java

    }

-    public int getCount() {
+    public int size() {


Changed getCount and getItem to follow array naming conventions.

These look like pretty breaking changes, but I'm only just someone really curious about this project.
Also, was a consensus ever reached on using enums in the code? Shouldn't we be doing something like this:

public interface ChangeListener { public static class EventType { public static final int ADDED = 1; public static final int CHANGED = 2; // ... } }

@TheCraftKid Nope, these aren't breaking api changes. 🎉 We're really lucky @puf wanted to hold out on open sourcing FirebaseArray and FirebaseIndexArray or these would have been (You're thinking of FirebaseRecyclerViewAdapter#getItemCount(), but I didn't change that.)

As for enums, @puf decided against it and I'm fine with that because I found out proguard usually converts enums to ints anyway: http://stackoverflow.com/a/32236950/4548500. Cheers!

Well, then.
Wait, FirebaseArray is being open-sourced? How will that work? Will I finally be able to implement a proper FirebaseSpinnerAdapter?

Woah woah woah! 😄 I was just saying that eventually @puf will have to open source it (technically he doesn't, but come on!). I think he was waiting for some additional refinement and this PR + another one I have ready will clean things up a bit, making it more likely that FirebaseArray will be open sourced. Then yes, you will finally be able to implement a proper FirebaseSpinnerAdapter!

SUPERCILEX · 2016-12-15T02:39:48Z

database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

+        super.setChangeEventListener(mListenerCopy);

-        Query ref = mQuery.getRef().child(keySnapshot.getKey());
+        Query ref = mJoinResolver.onJoin(keySnapshot, previousChildKey);


Using the new JoinResolver.

SUPERCILEX · 2016-12-15T02:54:54Z

database/src/main/java/com/firebase/ui/database/FirebaseIndexRecyclerAdapter.java

                                        Query keyRef,
                                        Query dataRef) {
-        super(modelClass, modelLayout, viewHolderClass, new FirebaseIndexArray(keyRef, dataRef));
+        super(modelClass, modelLayout, viewHolderClass, new FirebaseIndexArray(keyRef), false);


Sadly, I can't just pass in new FirebaseIndexArray(keyRef, this).

SUPERCILEX · 2016-12-15T03:05:01Z

database/src/main/java/com/firebase/ui/database/JoinResolver.java

+import com.google.firebase.database.DataSnapshot;
+import com.google.firebase.database.Query;
+
+public interface JoinResolver {


Thoughts on the documentation?

…lback

puf · 2016-12-19T23:16:24Z

Uhm.... I'm confused. FirebaseArray is just as open-source as the rest of FirebaseUI-Android. If you're interested in how it works, you can admire all 12 lines of it here: https://github.com/firebase/FirebaseUI-Android/blob/master/database/src/main/java/com/firebase/ui/database/FirebaseArray.java .

…

On Sun, Dec 18, 2016 at 9:00 PM Alex Saveau ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In database/src/main/java/com/firebase/ui/database/FirebaseArray.java <#448>: > } - public int getCount() { + public int size() { Woah woah woah! 😄 I was just saying that eventually @puf <https://github.com/puf> will have to open source it (technically he doesn't, but come on!). I think he was waiting for some additional refinement and this PR + another one I have ready will clean things up a bit, making it more likely that FirebaseArray will be open sourced. Then yes, you will finally be able to implement a proper FirebaseSpinnerAdapter ! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#448>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA3w31wWJpFHYOD3A8Z6nIRWgU18I5v5ks5rJg-IgaJpZM4LIlIo> .

SUPERCILEX · 2016-12-19T23:19:28Z

@puf I meant making FirebaseArray a public class so people can use it without having to copy it.

…lback

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

…lback

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

…lback

…lback # Conflicts: # app/src/main/java/com/firebase/uidemo/database/ChatActivity.java # database/src/main/java/com/firebase/ui/database/FirebaseArray.java # database/src/main/java/com/firebase/ui/database/FirebaseListAdapter.java # database/src/main/java/com/firebase/ui/database/FirebaseRecyclerAdapter.java

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

# Conflicts: # database/src/androidTest/java/com/firebase/ui/database/TestUtils.java

…aseUI-Android into db-callback

…lback # Conflicts: # app/src/main/java/com/firebase/uidemo/database/ChatActivity.java # database/src/androidTest/java/com/firebase/ui/database/TestUtils.java # database/src/main/java/com/firebase/ui/database/FirebaseArray.java # database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java # database/src/main/java/com/firebase/ui/database/FirebaseListAdapter.java # database/src/main/java/com/firebase/ui/database/FirebaseRecyclerAdapter.java

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

…lback # Conflicts: # app/src/main/java/com/firebase/uidemo/database/ChatActivity.java # database/src/androidTest/java/com/firebase/ui/database/FirebaseArrayOfObjectsTest.java # database/src/androidTest/java/com/firebase/ui/database/FirebaseArrayTest.java # database/src/androidTest/java/com/firebase/ui/database/FirebaseIndexArrayOfObjectsTest.java # database/src/androidTest/java/com/firebase/ui/database/FirebaseIndexArrayTest.java # database/src/androidTest/java/com/firebase/ui/database/TestUtils.java # database/src/main/java/com/firebase/ui/database/FirebaseArray.java # database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java # database/src/main/java/com/firebase/ui/database/FirebaseIndexRecyclerAdapter.java # database/src/main/java/com/firebase/ui/database/FirebaseListAdapter.java # database/src/main/java/com/firebase/ui/database/FirebaseRecyclerAdapter.java

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

googlebot · 2017-03-06T20:32:11Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

googlebot · 2017-03-06T20:53:39Z

CLAs look good, thanks!

SUPERCILEX · 2017-03-06T20:56:01Z

@samtstern Should we add extra constructors in the index adapters? I feel like this is a niche feature and people who use it will know how to pass in a FirebaseIndexArray to the FirebaseRecyclerAdapter. What do you think?

…lback

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

…lback

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

…lback # Conflicts: # database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

…lback

…lback # Conflicts: # database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

SUPERCILEX · 2017-04-26T00:51:08Z

@samtstern While we're on the merge rollercoaster, I think this one is the next PR to take a look at. 😄

samtstern · 2017-04-26T00:54:16Z

@SUPERCILEX I agree, this one is definitely next. I will take a look tomorrow. A lot has changed in how we handle the IndexedArray (and really everything) since this was first sent out.

Btw we really appreciate that you always keep these PRs up to date and you're ready to discuss when we find the time. It's above-and-beyond as a contributor and arguably we have not earned it since we go radio silent too often.

SUPERCILEX · 2017-04-26T03:06:14Z

@samtstern No problem, I'm just happy whenever the ball moves forward a bit! 😄

Yeah, this was actually the PR that got me started on the whole "Let's redo firebase-ui-database" thing. The funniest part is that it's not even a feature I use in my app. 😆

…lback

SUPERCILEX · 2017-07-30T07:10:24Z

Going to close this PR since there is very little demand and it isn't on the FirebaseUI roadmap.

SUPERCILEX added 2 commits December 8, 2016 19:48

Add a callback in FirebaseIndexArray when a Key doesn't exist in a ref

28c366c

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Add documentation

641297c

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

samtstern assigned puf Dec 13, 2016

puf reviewed Dec 13, 2016

View reviewed changes

SUPERCILEX added 7 commits December 13, 2016 17:32

Rename listeners

a62a025

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Merge remote-tracking branch 'firebase/version-1.1.0-dev' into db-cal…

703f85d

…lback

Add JoinResolver.java interface

f1e702b

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Cleanup

af39a1e

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Rename ChangeListener to ChangeEventListener

25fc4df

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Cleanup

5486e8c

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Make JoinResolver.java public

23806d7

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

SUPERCILEX commented Dec 15, 2016

View reviewed changes

Merge remote-tracking branch 'firebase/version-1.1.0-dev' into db-cal…

06fb7d0

…lback

SUPERCILEX added 10 commits December 22, 2016 12:30

Merge remote-tracking branch 'firebase/version-1.1.0-dev' into db-cal…

0b56e99

…lback

Rename

331b4bf

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Merge remote-tracking branch 'firebase/version-1.1.0-dev' into db-cal…

f396fec

…lback

Fix compile error

cfdb305

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Merge branch 'version-1.1.0-dev' into db-callback

cc3729c

Merge remote-tracking branch 'firebase/version-1.1.0-dev' into db-cal…

f5d06b2

…lback

Fix merge mistakes

97d9cb0

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Merge remote-tracking branch 'firebase/master' into db-callback

e7297d4

# Conflicts: # database/src/androidTest/java/com/firebase/ui/database/TestUtils.java

Merge remote-tracking branch 'firebase/master' into db-callback

9a03342

SUPERCILEX changed the base branch from version-1.1.0-dev to master January 17, 2017 20:43

Merge branch 'version-1.2.0-dev' of https://github.com/firebase/Fireb…

8f87492

…aseUI-Android into db-callback

SUPERCILEX changed the base branch from master to version-1.2.0-dev January 24, 2017 00:15

SUPERCILEX added 2 commits January 24, 2017 17:34

Fix a ton of merge mistakes

48ea75a

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

SUPERCILEX added 2 commits March 5, 2017 19:45

Fix merge mistakes

03c14d2

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

SUPERCILEX changed the base branch from version-1.2.0-dev to version-2.0.0-dev March 6, 2017 20:34

Fixups

63d5ca5

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

SUPERCILEX added 8 commits March 6, 2017 13:05

Merge remote-tracking branch 'firebase/version-2.0.0-dev' into db-cal…

14e3443

…lback

Cleanup

ab613bd

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Merge remote-tracking branch 'firebase/version-2.0.0-dev' into db-cal…

64bfb37

…lback

Merge remote-tracking branch 'firebase/version-2.0.0-dev' into db-cal…

ae83d83

…lback

Cleanup

371c211

Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>

Merge remote-tracking branch 'firebase/version-2.0.0-dev' into db-cal…

dbc1a43

…lback # Conflicts: # database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

0a1d2b5

…lback

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

53920a2

…lback # Conflicts: # database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

SUPERCILEX added 6 commits April 25, 2017 20:08

Flip onJoinFailed params to make more sense

b3c137f

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

c23eec1

…lback

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

d2962fa

…lback

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

52b7c10

…lback

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

f236047

…lback

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

2933598

…lback

samtstern mentioned this pull request Jun 9, 2017

Merge version-2.0.0-dev into master #748

Merged

SUPERCILEX added 2 commits June 9, 2017 14:01

Merge remote-tracking branch 'upstream/version-2.0.0-dev' into db-cal…

a2909e4

…lback

Merge remote-tracking branch 'upstream/master' into db-callback

a11975c

SUPERCILEX changed the base branch from version-2.0.0-dev to version-2.1.0-dev June 13, 2017 02:05

SUPERCILEX closed this Jul 30, 2017


		import com.google.firebase.database.DataSnapshot;

		interface OnIndexMismatchListener {

		@@ -0,0 +1,41 @@
		package com.firebase.uidemo.database;

		public class Chat {

Add a callback in FirebaseIndexArray when a Key doesn't exist in a ref #448

Add a callback in FirebaseIndexArray when a Key doesn't exist in a ref #448

Uh oh!

Conversation

SUPERCILEX commented Dec 9, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SUPERCILEX left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

puf commented Dec 19, 2016 via email

Uh oh!

SUPERCILEX commented Dec 19, 2016

Uh oh!

googlebot commented Mar 6, 2017

Uh oh!

googlebot commented Mar 6, 2017

Uh oh!

SUPERCILEX commented Mar 6, 2017

Uh oh!

SUPERCILEX commented Apr 26, 2017

Uh oh!

samtstern commented Apr 26, 2017

Uh oh!

SUPERCILEX commented Apr 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SUPERCILEX commented Jul 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SUPERCILEX commented Dec 9, 2016 •

edited

Loading

SUPERCILEX commented Apr 26, 2017 •

edited

Loading