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

[DenisBronx]

In my case when the dataRef (The Firebase location to watch for data changes) gives a null snapshot value I wanted to trigger the parseSnapshot method in order to create the missing data instead of not displaying the row of the adapter.

Currently it only prints :

Log.w(TAG, "Key not found at ref: " + snapshot.getRef());

[samtstern]

@morganchen12 just curious what do we do in iOS? Print a warning or callback?

[DenisBronx]

Also it would be useful if we can get the DataSnapshots of the keyRef Query in case that we don't have only a set of keys in that location.
In this way we could merge two DataSnapshots in a single object.

[morganchen12]

iOS has a callback for handling errors per reference.

[SUPERCILEX]

@samtstern I noticed this issue and the PR are in the "Needs decision" category, what's holding you back?

[samtstern]

@SUPERCILEX thanks for reminding me about this one. I really have no objection to the design of your PR, but I do have a higher-level objection.

One of the goals of all the FirebaseArray changes we made is to make it easier for developers to support complex use cases not easily supported by FirebaseRecyclerAdapter and other simple adapters. In fact if we had done this a long time ago, we probably would have never added the indexed adapters to the core library at all. Instead, you would have used FirebaseArray to create them in your own project or in a fork.

So following that same logic, I think it's ideal to stop expanding the surface of the adapters and encourage people to instead implement their own custom adapters for their use case.

In the case of this JoinResolver PR (which again, is a nice design) could we instead add a single overrideable method to the adapter like onJoinFailed() and then let developers take it from there?

[SUPERCILEX]

@samtstern Ok, that makes sense. Let me look into updating my PR.

[SUPERCILEX]

@samtstern Ok, there are a few issues that are blocking me. First, we need some sort of interface to get the error since it occurs in the index array. So that means we need to keep the JoinResolver interface anyway and we'll need to pass it into the FirebaseIndexArray constructor. The real issue is that we get back into the circular constructor dependency issue like what happened with the snapshot parser. We'll have to pass in this here which doesn't work cuz circluar depencencies. 😢 What do you think we should do?

[samtstern]

@SUPERCILEX those are all good points. I am going to punt this one to version 2.1 (or whatever is next) since we can do it in a non-breaking way in the future. I really want to get 2.0 out the door ASAP, phone auth was the big blocker and now we're unblocked.

[SUPERCILEX]

@samtstern SGTM! 😄

[SUPERCILEX]

@samtstern I really don't like our original assumption that values can't be null, it's caused so many bugs, performance problems, etc.

What if for v3.0 we just added DataSnapshots with null values anyway? Then devs would simply have to deal with it in parseSnapshot or when binding their views to the model. They could choose whether or not to always create empty objects in their parseSnapshot method or handle the null models at binding time. With a bit of documentation, I totally think it could work. What do you think?