GitLiveApp / firebase-kotlin-sdk

A Kotlin-first SDK for Firebase
https://gitliveapp.github.io/firebase-kotlin-sdk/
Apache License 2.0
1.17k stars 155 forks source link

Map null in map to undefined for JS database #493

Closed Daeda88 closed 7 months ago

Daeda88 commented 7 months ago

@nbransby re https://github.com/GitLiveApp/firebase-kotlin-sdk/pull/448#issuecomment-2044330150 If its specific to updateChildren on JS, this should fix it.

Im not sure how JS is supposed to handle null in a map though. Like in EncodersTest.encodeDecodeGenericClass, is it supposed to be "nullableBool": null or "nullableBool": undefined?

Daeda88 commented 7 months ago

I should note that if I go back to the commit before the Serialization changes where merged, I dont see anything that would do this mapping either.

nbransby commented 7 months ago

Yes I had a look myself and couldn't see anything either. All I know is rolling back fixes it, I'll have to investigate it more

Daeda88 commented 7 months ago

Lets see if we can write a test that reproduces it I guess

Daeda88 commented 7 months ago

So far, on master. I havent been able to reproduce using:

@Test
    fun testUpdateChildren() = runTest {
        setupRealtimeData()
        val reference = database
            .reference("FirebaseRealtimeDatabaseTest")
        reference.updateChildren(mapOf("lastActivity" to null))
    }

    private suspend fun setupRealtimeData() {
        ensureDatabaseConnected()
        val firebaseDatabaseTestReference = database
            .reference("FirebaseRealtimeDatabaseTest")

        val firebaseDatabaseChildTest1 = FirebaseDatabaseChildTest("aaa")
        val firebaseDatabaseChildTest2 = FirebaseDatabaseChildTest("bbb")
        val firebaseDatabaseChildTest3 = FirebaseDatabaseChildTest("ccc")

        val values = firebaseDatabaseTestReference.child("values")
        values.child("1").setValue(firebaseDatabaseChildTest1)
        values.child("2").setValue(firebaseDatabaseChildTest2)
        values.child("3").setValue(firebaseDatabaseChildTest3)
        firebaseDatabaseTestReference.child("lastActivity").setValue(1)
    }

given

{
  "rules": {
    ".read": true,
    "FirebaseRealtimeDatabaseTest": {
      "values": {
        ".write": true
      },
      "lastActivity": {
        ".validate": "!newData.exists() || newData.isNumber()",
        ".write": true
      }
    }
  }
}

Whereas reference.updateChildren(mapOf("lastActivity" to "bla")) does fail

Daeda88 commented 7 months ago

I found a bug in OnDisconnect specifically, will make a patch for that once I have testing figured out

Daeda88 commented 7 months ago

Closing cause real fix is here https://github.com/GitLiveApp/firebase-kotlin-sdk/pull/494