firebase / geofire-js

GeoFire for JavaScript - Realtime location queries with Firebase
MIT License
1.45k stars 347 forks source link

possible bug? key_exited & key_entered events fire even when rules prevent change of data. #130

Closed Tobbyte closed 7 years ago

Tobbyte commented 7 years ago

Firebase version: 3.6.1 GeoFire version: 4.1.1

db structure:

db
  |-someData
    |-key
  |-activeUsers
    |- key

My app needs to delete a key in someData if the user closes a project. But only, if the current user was the only user in activeUsers. I therefore use firebase rules to make sure that someData/key is only writable if activeUsers is empty (or the newData is not a .remove()). By that way a user doesn't need to know about the other users and always fires a .remove(someData/key) when he leaves, bc the server handles if it's ok to delete or not. Part of the .write rule for someData: ".write": "!root.child('activeUsers/key').hasChildren() || newData.val() !== null ",

This works fine. But geoFire behaves somehow strangely: The user that leaves and sends the .remove(someData/key) has an active geoQuery.on("key_exited", onKeyExitedRegistration) and geoQuery.on("key_entered", onKeyEnteredRegistration) for new GeoFire(firebase.database().ref("someData")).

Expected behavior is that a key_exited-event is fired ONLY if the .remove() was successful (aka allowed by the firebase rule bc activeUsers.hasChildren() === false) and the data was really removed.

Actual behavior instead is: If the rule doesn't allow a write and nothing is removed from someData/key, geoFire still triggers a key_exited event and immediately after that a key_entered event for the key that could have been removed, but wasn't. So geoFire events fire without the data in firebase is being altered in anyway.

jwngr commented 7 years ago

Hey there, thank you for the great bug report! This may be surprising, but this is actually expected behavior. Let me explain.

The Firebase clients implement what we call latency compensation. In short, this means that they will fire local events immediately, even before the server confirms that writes are successful. For example, let's say you are not using GeoFire at all and have a ref with a "value" listener attached and try to do ref.set(true) even if you don't have permission to do so. The write will cause the "value" event will fire immediately on the local client with the value being written. Once the Firebase servers reject the write, the "value" event will fire again with the previous value. Firebase ensures the final state of every client is consistent (eventually consistent), but does not ensure each client always has the correct data at any point in time. This may seem weird, but makes a lot of sense when you start thinking about offline support. If we wouldn't implement latency compensation, using a Firebase app while offline would result in apps which don't fire any local events since we would have to wait for the Firebase server to confirm them, which is not possible since we are offline.

Similarly, GeoFire events are latency compensated. So, we will ensure your final state is correct (namely, we fire a "key_exited" event followed by the "key_entered" event), but will not ensure you don't get a local event for a write which actually failed to pass the rules. At the end of the day, the final state is the same for your local client and all other clients will not see either of those two events.