healthjoy / async-firebase

The lightweight asynchronous client that makes interaction with Firebase Cloud Messaging oversimplified.
MIT License
37 stars 14 forks source link

Data values force-converted to string #38

Closed coandco closed 1 year ago

coandco commented 1 year ago

Describe the bug All data values are getting converted to string in the payload, even None/null. There's no way to send data={"foo": null} as a payload.

To Reproduce

        android_config: AndroidConfig = fcm_client.build_android_config(
            priority="normal",
            data={"foo": None}
        )

        response: FcmPushResponse = await fcm_client.push(device_token=device, android=android_config)

This results in the following message being sent:

{
  "message": {
    "token": <censored>,
    "android": {
      "priority": "normal",
      "ttl": "604800s",
      "data": {
        "foo": "None"
      }
    }
  },
  "validate_only": false
}

Expected behavior I'd expect the message generated to look like this:

{
  "message": {
    "token": <censored>,
    "android": {
      "priority": "normal",
      "ttl": "604800s",
      "data": {
        "foo": null
      }
    }
  },
  "validate_only": false
}

Screenshots N/A

Additional context It looks like the issue is https://github.com/healthjoy/async-firebase/blob/master/async_firebase/client.py#L218, which explicitly casts every value in the data key to a string. Could this be changed to remove the cast, or at least exempt nulls from it? The whole point of the data key is to be able to pass arbitrary JSON data to your application and let the app interpret it how you wish. Force-converting everything to string makes it impossible to send some valid JSON messages.

akalex commented 1 year ago

Hey, @coandco thanks for posting this issue. I will take a look at it. As for casting everything to string, AFAIK specifically for Android config, there was (I will re-check that) a hard requirement to have every value cast to a string which then could be parsed adequately on the client side (Android device). Any request that I attempted to assemble using an attribute with a type different from the string failed with error like the wrong payload. For the null I am not sure, I will need to check that first, perhaps it's safe to exempt it.

akalex commented 1 year ago

As documentation states, the data should contain the only string, so that requirement is still valid. Also please take a look at the doc string for this attribute https://github.com/healthjoy/async-firebase/blob/master/async_firebase/client.py#L190

ndmytro commented 1 year ago

Will just put here several options that we should consider for the data field keys and values representation if the value is missing:

@akalex we could try the latter two, what do you think?

akalex commented 1 year ago

@ndmytro Thanks for putting a couple of options together, I will need to do a quick test but I guess changing None to "null" should be the best option, in that case on the Firebase side the value will be parsed into null which I think is the expected result.