GodotNuts / GodotFirebase

Implementations of Firebase for Godot using GDScript
MIT License
532 stars 76 forks source link

[BUG] Firebase database filter "ORDER_BY" is not working #159

Closed h1p3rcub3 closed 3 years ago

h1p3rcub3 commented 3 years ago

Describe the bug When trying to get a database reference with ORDER_BY filter it returns an error:

Invalid get index 'ORDER_BY' (on base: 'null instance')

in line 133 of reference.gd if _filter_query.has(_parent.ORDER_BY):

To Reproduce I'm creating the listener like this. It works fine without filters, but i was not able to add any.

var events_listener = Firebase.Database.get_database_reference("matches/1/events", {Firebase.Database.ORDER_BY : "action_id", Firebase.Database.EQUAL_TO : "GAME_START"})

Expected behavior It should return the listener with the filters.

Environment:

h1p3rcub3 commented 3 years ago

I just realized it got nothing to do with ORDER_BY query.

Even the Wiki example is not working var db_ref = Firebase.Database.get_database_reference("path_to_position_in_database", { Firebase.Database.LIMIT_TO_LAST: 10 })

WolfgangSenff commented 3 years ago

Do you have indexes created for the fields you're attempting to order_by and equal_to? If not, it will not work. I'm not sure if we have that info in the wiki as I was expecting people that are attempting to use those features would know you have to make indexes, but it also never occurred to me the plugin would get that popular, so it's my fault. If you don't have those indexes created in your rules, try creating them and see if that works.

h1p3rcub3 commented 3 years ago

I created the index in the rules but it's still giving the same error. As I told in a comment after the bug report, it seems to fail with any kind of filter, even the LIMIT_TO_LAST filter, which has nothing to do with indexes.

WolfgangSenff commented 3 years ago

Oh crap, sorry, totally misread your original report. I didn't see the actual error. Let me take a quick look, one moment.

WolfgangSenff commented 3 years ago

Okay, have a fix, the tests are running and should be integrated soon. Just a bad refactor it looks like, someone didn't understand how the filtering worked.

WolfgangSenff commented 3 years ago

Alright, bug fixed. Had to move the constants into the references, just fyi.

h1p3rcub3 commented 3 years ago

Thank you for your time!

It doesn't give any error now, but still not working. The event is not getting trigered neither when adding an item or in the first sync.

This is my code.

var events_listener = Firebase.Database.get_database_reference("matches/1/events", {FirebaseDatabaseReference.ORDER_BY : "action_id", FirebaseDatabaseReference.EQUAL_TO : "GAME_START"})

events_listener.connect("new_data_update", self, "on_received_event") The "matches/1/events" route are pushed elements all with an "action_id" child. "action_id" values ar strings like "GAME_START", "PLAYER_1_MOVE", etc

WolfgangSenff commented 3 years ago

Hm, and you definitely have your indexes now for ORDER_BY? Those are needed for any ORDER_BY query.

h1p3rcub3 commented 3 years ago

It's possible that i'm not doing it properly, its been a long time since I had to create firebase rules. This is my data estructure.

Sin título

{
  "rules": {
    ".read": true,
    ".write": true,
    "matches": {
      "$matchid": {
        "events": {
        ".indexOn": ["action_id"]
        }
      }
    },
  }
}
WolfgangSenff commented 3 years ago

Try adding another level under events for $eventid maybe? In theory I wouldn't expect you to need it, but I'm not sure what's up, and I have to go do my real job now. Let me know over the course of the day if you figure it out.

h1p3rcub3 commented 3 years ago

It works fine with just the ORDER_BY, but when i combine it with the EQUAL_TO filter i dont get any data. I will keep you updated.

h1p3rcub3 commented 3 years ago

I found the issue. It was not adding the scaped quote to the query.

The query should be processed differently depending on the Type you want to filter.

If its a string, then it should add the scaped quote, if it's a bool it should lowercase it, as in Firebase Database "true/false" are lowercase while they are capital in Godot.

Here is how I made it work. I think this covers all the cases.

func _get_filter():
    if !_filter_query:
        return ""
    # At the moment, this means you can't dynamically change your filter; I think it's okay to specify that in the rules.
    if !_cached_filter:
        _cached_filter = ""
        if _filter_query.has(ORDER_BY):
            _cached_filter += ORDER_BY + _equal_tag + _escaped_quote + _filter_query[ORDER_BY] + _escaped_quote
            _filter_query.erase(ORDER_BY)
        else:
            _cached_filter += ORDER_BY + _equal_tag + _escaped_quote + _key_filter_tag + _escaped_quote # Presumptuous, but to get it to work at all...
        for key in _filter_query.keys():
            if _filter_query[key] is String:
                _cached_filter += _filter_tag + key + _equal_tag + _escaped_quote + _filter_query[key] + _escaped_quote
            else: #Is number or bool
                _cached_filter += _filter_tag + key + _equal_tag + str(_filter_query[key]).to_lower()
    return _cached_filter