Firebase does not accept messages when a custom JsonSerializerSettings is provided

[sir-boformer]

I'm using FcmSharp in Asp .Net Core. I Created a JsonSerializer based on the default JsonSerializerSettings provided by ASP .Net Core MVC so that things like date formatting and support for custom serializers are applied to notification payloads.

When I sent a message with a simple ApnsConfig (just an alert with title and body), this error occured:

FcmSharp.Exceptions.FcmMessageException: {
  "error": {
    "code": 400,
    "message": "Request contains an invalid argument.",
    "errors": [
      {
        "message": "Request contains an invalid argument.",
        "domain": "global",
        "reason": "badRequest"
      }
    ],
    "status": "INVALID_ARGUMENT"
  }
}

It turned out that the json serializer was including null values in the serialized messages. Firebase does not accept a ApnsConfig with null properties.

When I set NullValueHandling = NullValueHandling.Ignore in the JsonSerializerSettings, everything works as expected.

I think it doesn't make sense to allow the creation of a FcmClient with an incompatible JsonSerializer.

It would be nice to control the serialization of the CustomData dictionary values of the ApnsConfigPayload (e.g. use snake_case, date formatting), but it should not affect the serialization of the rest of the message.

[bytefish]

This is totally correct! And you are right. The user shouldn't be able to pass the parameters in.

What about the following: I make the constructor protected, so you can only obtain an instance using the static Default method. And if one needs additional options, then the JsonSerializer needs to be overriden.

Does that sound OK? One can also make the constructor private, I am not sure what's the best way.

I think the default FcmSharp serializer already uses NullValueHandling.Ignore as default:

[bytefish]

Ah, I think I get the full picture now. You need to customize the serialization of the CustomData, while the rest of the message needs to be serialized with the FcmSharp (Json.NET) Serializer. Well, I think the CustomData in the messages is only serialized with the default serializer, if you pass complex objects into it, which in turn will be serialized with Json.NET again.

If you pass strings in it, then I think Json.NET won't touch them. There is actually a Unit Test, that shows it:

I know it isn't really convenient API-wise, if you have to first serialize the values to a Dictionary<string, string> yourself, but I cannot think of a clean API to provide CustomData-specific serialization / deserialization.

Still the Constructor shouldn't allow constructing globally invalid JsonSerializerSettings and should be made protected / private.

@sir-boformer What do you think?

[sir-boformer]

It's not really a problem if you know it, I just wanted to report it.

I wanted actual json in the payload, so I just converter my objects into a JObject using my own serializer:

var somePayload = new
{
    testNumber = 1,
    testDate = DateTime.Now,
    testString = "Hello"
};

...

CustomData = new Dictionary<string, object>
{
    ["test"] = JObject.FromObject(somePayload, _jsonSerializer)
}

This results in a nice custom payload (for testing purposes I used snake_case):

{
   "validate_only":false,
   "message":{
      "apns":{
         "payload":{
            ...
            "test":{
               "test_number":1,
               "test_date":"2018-10-14T20:45:42.6582191Z",
               "test_string":"Hello"
            }
         }
      },
      "token":"..."
   }
}

I guess for 99% the default serialization settings will be sufficient. Maybe the JObject (or JArray) trick could be mentioned in the README.

Thanks for your awesome work with this library! It's really well done, and really helped saved me a lot of time, especially because all the REST objects are implemented so well. It really deserves more attention. I'm not sure if there is some kind of tutorial how to integrate this into ASP .Net Core. If not, I would love to write one

[bytefish]

Oh I forgot, did you try using the Attributes like JsonProperty on you objects Properties? That should work just as well and maybe you can also attribute the class and set class-specific Json.NET attributes there.

[bytefish]

A tutorial would be great, I am currently having another open issue #34, which is about creating more examples. We could add it to the Examples Folder, where I have already prepared an Android client, too.

[sir-boformer]

Yep, although that doesn't work for more advanced stuff like date formatting or custom serialization. Also I think it's a bad idea to use JsonProperty if you only want to use a different notation like snake_case. It's just another error source.

I think this issue can be closed.