Support for filtering and predicates

[asciimike]

See firebase/FirebaseUI-iOS#12

[asciimike]

@zachariah25 This is all yours :)

[puf]

It might be a good approach to add this as a Filterable implementation on Android.
See https://groups.google.com/forum/#!topic/firebase-talk/O1kJ4VLUg10

[frangulyan]

I tried a very simple (and maybe not the most efficient) client-side filtering solution here (not yet fully tested). Any caveats that I don't see yet?

[chalber]

@frangulyan how does your solution suppose to be implemented?

[ghost]

Any forks for this?

[puf]

Not that I know off. Feel free to fork and create something yourself though.

On Thu, Sep 8, 2016 at 5:50 AM Ab-Appletic [email protected] wrote:

Any forks for this?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#15 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA3w35ElIWn00lWU6iEe6hgl9mq_mm3Lks5qoASigaJpZM4GGIjj
.

[ghost]

Maybe some other time

[ghost]

@puf We've been begging for this feature for over a year. How come you still haven't added this feature? No one gives a crap about the fact that Android has to be in sync with iOS. Don't give your users nothing just for a stupid reason. Give some users what they want, then little by little it will be completed. You guys can't sit and do nothing.

All I want it to be able to search a RecyclerView. Too hard to implement? You're Firebase engineers and you have that title for a reason. We've been waiting for this feature for a year and not even a seed of progress has been made. At least give a workaround! I've never seen such a lacking library. If I've known there were better alternatives than Parse, I'd never move to Firebase. But now I'm stuck. Terrible service. It's a simple feature. No wonder I couldn't agree to upgrade my plan, because you don't deserve the money. It's a damn simple feature, instead of giving more big features, finish off the mandatory small ones first.

[SUPERCILEX]

@Ab-Appletic How complex is your filtering? If it isn't too crazy, you can use Firebase's built in filtering and ordering support. For example, I'm using:

BaseHelper.getDatabase()
    .child(Constants.FIREBASE_SCOUT_INDICES)
    .child(BaseHelper.getUid())
    .orderByValue() // Since I'm using the push() method and a number in equalTo(), everything will be ordered by time
    .equalTo(Long.parseLong(number)) // I'm filtering out unwanted team numbers

Firebase provides and in-depth explanation of how everything is ordered: https://firebase.google.com/docs/database/android/lists-of-data#data-order.

[puf]

Allowing client-side reversing of items, re-ordering of items, and filtering of items have been features that we considered before even committing the first code of FirebaseUI for Android and iOS. For simple use-cases it's indeed not the hardest feature to implement, although it definitely has some interesting implications.

One of the reasons we haven't added this feature yet is that we want feature parity between the Android and the iOS libraries. While that may not be important to you, it is important to us. It's important because Firebase is cross-platform product and we need to ensure that all our users have a consistently good experience.

Another reason for not adding the feature is that I'm not certain it's the right direction to take. The reward of client-side filtering is clear: your users get snappy responses to searches. But we'd first have to first load the entire set of potential results and then filter out items client-side. If the list is non-trivial (extends more than a few screenfulls), that additional download is likely to be significant.

This essentially boils down to the fact that a client-side filtering model doesn't fit nicely together with Firebase's server-side query model. By adding client-side filtering as a feature, we increase the amount of data that is downloaded needlessly, and that's something we simply haven't been willing to do yet.

At any moment, anyone who wants this feature can fork the repo and implement it. As you said: it is not that difficult to add. The great thing about FirebaseUI being open-source, is that the fork may prove to be extremely popular and make us reconsider the choices we've made for it initially.

The best way to move this feature forward within the FirebaseUI project is to come up with an implementation that minimizes these disadvantages. We haven't rejected the feature, we just haven't gotten to a satisfactory implementation. Once have such an implementation on both iOS and Android, it will be added to both.

Thanks for providing the incentive to write down my thoughts on this feature. I should've done that a while ago.

[SUPERCILEX]

Hey @puf, I've been thinking about this given what you wrote above, and I believe the underlying issue is that people can't choose what goes into their adpater. If they had a say in it, there wouldn't really be any need for FirebaseUI to support filtering out of the box because people could simple use the getItem() method and decide on their own what goes into the adapter. What I'm trying to say is, why isn't the FirebaseArray.OnChangedListener public api? I don't see any cons to letting people override the onChanged method to do their own processing and decide what they want to display. It also makes FirebaseUI much more flexible and will let firebase/FirebaseUI-iOS#186 finally be merged. What do you think @puf? If you agree with me, I'll submit a quick little PR adding the ability to override the onChanged method and if not, I'd love to hear why.

[puf]

Try filtering out some items using the approach you describe and then change the order of some of the remaining items.

[SUPERCILEX]

@puf Challenge accepted! 😄 I just finished ironing out some bugs and I now have a working proof of concept. The code is gross because I only went as far as getting it working, but you can have a look at it below or build it yourself from my branch. If you think this would work, I have a PR ready to make onChanged public API.

Requirements:
Filter: my PoC filters out all non numerical values
Sort: the PoC sorts the list item in ascending order.

Test cases:

1. Only numbers [add, delete, change value]
2. Only letters [add, delete, change value]
3. Mix of invisible letter chats and visible number chats [add, delete, change value]

@puf Did I miss any?

Yucky code that no one should copy, but can serve as an example of how it could be done with terrible performance and confusion 😊:
Chat:

public class Chat implements Comparable<Chat> {
    private String mName;
    private String mMessage;
    private String mUid;

    public Chat() {
        // Needed for Firebase
    }

    public Chat(String name, String message, String uid) {
        mName = name;
        mMessage = message;
        mUid = uid;
    }

    public String getName() {
        return mName;
    }

    public void setName(String name) {
        mName = name;
    }

    public String getMessage() {
        return mMessage;
    }

    public void setMessage(String message) {
        mMessage = message;
    }

    public String getUid() {
        return mUid;
    }

    public void setUid(String uid) {
        mUid = uid;
    }

    @Override
    public int compareTo(Chat chat) {
        long message1 = Long.parseLong(getMessage());
        long message2 = Long.parseLong(chat.getMessage());
        if (message1 > message2) {
            return 1;
        } else if (message1 == message2) {
            return 0;
        } else {
            return -1;
        }
    }
}

Adapter:

mRecyclerViewAdapter = new FirebaseRecyclerAdapter<Chat, ChatHolder>(
        Chat.class, R.layout.message, ChatHolder.class, lastFifty) {
    List<Chat> mChats = new ArrayList<>();

    @Override
    public void populateViewHolder(ChatHolder chatView, Chat chat, int position) {
        chatView.setName(chat.getName());
        chatView.setText(chat.getMessage());

        FirebaseUser currentUser = mAuth.getCurrentUser();
        if (currentUser != null && chat.getUid().equals(currentUser.getUid())) {
            chatView.setIsSender(true);
        } else {
            chatView.setIsSender(false);
        }
    }

    @Override
    public Chat getItem(int position) {
        return mChats.get(position);
    }

    @Override
    public int getItemCount() {
        return mChats.size();
    }

    @Override
    public void onChanged(EventType type, int index, int unused) {
        List<Chat> filteredChats = new ArrayList<>();
        for (Chat chat : getCopyOfDatabaseSnapshots()) {
            if (isNumber(chat.getMessage())) {
                filteredChats.add(chat);
            }
        }

        if (type == EventType.ADDED) {
            if (filteredChats.size() > mChats.size()) {
                Collections.sort(filteredChats);
                for (int i = 0; i < mChats.size(); i++) {
                    if (!contains(mChats, filteredChats.get(i))) {
                        mChats.add(i, filteredChats.get(i));
                        notifyItemInserted(i);
                        return;
                    }
                }
                mChats.add(filteredChats.get(filteredChats.size() - 1));
                notifyItemInserted(mChats.size());
            }
        } else if (type == EventType.CHANGED) {
            Collections.sort(filteredChats);

            for (int i = 0; i < mChats.size(); i++) {
                boolean contains = false;
                for (int j = 0; j < filteredChats.size(); j++) {
                    Chat oldChat = mChats.get(i);
                    Chat currentChat = filteredChats.get(j);
                    if (oldChat.getMessage().equals(currentChat.getMessage())
                            && oldChat.getName().equals(currentChat.getName())
                            && oldChat.getUid().equals(currentChat.getUid())) {
                        contains = true;
                        break;
                    }
                }

                if (!contains) { // We found the previous index of our chat
                    for (int j = 0; j < filteredChats.size(); j++) {
                        Chat newChat = filteredChats.get(j);
                        if (!contains(mChats,
                                      newChat)) { // We found the index of the new chat
                            Chat oldChat = mChats.get(j);
                            mChats.set(i, oldChat);
                            mChats.set(j, newChat);
                            Collections.sort(mChats);
                            notifyDataSetChanged(); // BAD! I didn't have time to find where the items end up
                            return;
                        }
                    }
                }
            }
        } else if (type == EventType.REMOVED) {
            if (filteredChats.size() < mChats.size()) {
                for (int i = 0; i < filteredChats.size(); i++) {
                    if (!contains(filteredChats, mChats.get(i))) {
                        mChats.remove(i);
                        notifyItemRemoved(i);
                        return;
                    }
                }
                mChats.remove(filteredChats.size());
                notifyItemRemoved(filteredChats.size());
            }
        } else if (type == EventType.MOVED) {
            // We're filtering stuff ourselves so we don't care about MOVED
        } else {
            throw new IllegalStateException("Incomplete case statement");
        }
    }

    /**
     * I can't get List#contains to work
     *
     * @param chats The list to look through
     * @param chat The chat to find
     * @return True if {@code chat} is contained in {@code chat}
     */
    private boolean contains(List<Chat> chats, Chat chat) {
        // Neither contains nor equals is working for some reason
        for (Chat chat1 : chats) {
            if (chat.getMessage().equals(chat1.getMessage())
                    && chat.getName().equals(chat1.getName())
                    && chat.getUid().equals(chat1.getUid())) {
                return true;
            }
        }
        return false;
    }

    /**
     * @return A copy of mSnapshots
     */
    private List<Chat> getCopyOfDatabaseSnapshots() {
        List<Chat> chats = new ArrayList<>();
        for (int i = 0; i < super.getItemCount(); i++) {
            chats.add(super.getItem(i));
        }
        return chats;
    }

    /**
     * @param s The string to be tested
     * @return True if {@code s} is of type long
     */
    private boolean isNumber(String s) {
        try {
            Long.parseLong(s);
            return true;
        } catch (NumberFormatException e) {
            return false;
        }
    }
};

[morganchen12]

@SUPERCILEX the iOS side has filtering as a deliberate non-goal, since we don't want to encourage greedily pulling data that may never be displayed. Third-party devs can still extend the collection type to filter on their own, but by not providing the functionality out of the box those devs will have a better understanding of the limitations of filtering.

As far as feature parity goes, iOS's pull request has added these things:

• An interface to decouple Firebase collections from their data sources
• Opened up FirebaseArray to be more friendly toward subclassing
• Added a subclass of FirebaseArray that maintains its contents in a sorted order

I think this is a better direction for this feature, since it avoids many of the issues @puf describes above while still allowing for greater customizability.

[puf]

I like the idea of splitting sorting from filtering, since indeed the sorting is less contentious. If we can find a way to separate the allowing of adding such (resorting) behavior from the implementation of it (similar to a dataset observer), that would keep the impact on the core FirebaseArray event smaller.

[SUPERCILEX]

@puf So do you want to proceed with my api change or open FirebaseArray? Also, if we were to support sorting out of the box, how would we do that? Subclassing or constructor overload?

[morganchen12]

@puf gentle bump

[abdullah2993]

Not so gentle bump

[morganchen12]

Some background on this since it seems this thread has grown unwieldy:

FirebaseUI Database won't support filtering out-of-the-box. The last series of exchanges and PRs related to this opened up the database classes to customization by users so users could, for example, write a data source that automatically sorts or filters content. We don't support filtering directly because there's no good way to apply an arbitrary filter to a Firebase query without downloading more data than will be displayed, which generally leads to an awful experience on mobile. puf's comments above provide some more detail.

However, if you really want to, you can implement filtering on your own through the extensible data source classes.

[phidias51]

The only way to support this would be to have server-side predicates. The predicates would be evaluated on the server and only those nodes that pass the predicates logic would be returned by the server. I'm not sure what language the server side is written in, but assuming it's Java, you could express the predicates in JavaScript and evaluate it using Java's script engine.

[sarbogast]

And what do you do when you have a complex filter, that combines several criteria, including geo-distance queries (all items whose distance to a given point is less than a given distance). Those things are simply not supported by Firebase's query engine. You shouldn't impose your opinion on developers when features are so limited otherwise.