Property specific JsonConverters

[wernst]

Using Saule 1.8, EF 6.1.3

According to the documentation here - http://awesan.com/saule/content/4-customizing-serialization.html custom serialization can be achieved with the use of a JsonConverter. The example shown registers a global converter to the serializer. I have a use case where I would like to use a custom converter specifically on a few properties. Newtonsoft.Json provides a JsonConverterAttribute to achieve this (see https://www.newtonsoft.com/json/help/html/JsonConverterAttributeProperty.htm), but I have not been successful in getting Saule to call the WriteJson method of my custom converter (deserialization/ReadJson actually works fine).

Is this expected and is there a way to customize serialization at the property level? Thanks!

[sergey-litvinov-work]

Hi @joukevandermaas . We are also experiencing the same issue. We have a custom format for date times so we use JsonConverter attribute to handle specific properties in the different way, but it doesn't work as

https://github.com/joukevandermaas/saule/blob/1f8b0a94e77b32126cf1fbacbf8626a209591a59/Saule/Serialization/ResourceSerializer.cs#L295-L299

that does

https://github.com/joukevandermaas/saule/blob/d578a65c6bdc15037bb6ba6437f661f342a7d330/Saule/ObjectExtensions.cs#L45-L49

and ignores them.

We can handle it in that method or in directly ObjectExtensions.GetValueOfProperty by checking for JsonConverterAttribute and using it if it's present and we can make PR with the actual change. But wanted to check with you, would that approach be fine with you and if not, do you have any ideas how that can be handled better? The need for us is to have a bit custom format for specific DateTime property

Thanks!

[joukevandermaas]

I am a little wary of this, because there are tons of JsonX attributes and I don't want to set an expectation that we support them all. It looks like JSON converters run for each property, and you could have a conditional in there. So it should be possible to add support for any custom stuff from within the converter?

Alternatively I would be OK with adding a converter from within the resource, like

var mySpecialConverter = new CustomJsonConverter();
Attribute("MyProperty", jsonConverter: mySpecialConverter);

At least then the relationship is clearly defined, and there is no confusion about which attributes are supported.

Would that work for your use-case?

[sergey-litvinov-work]

It might work in our case, but we still would need to have JsonConverterAttribute over an actual property as we support both regular Json and JsonApi and in both cases we need to have different format. So to make it work for regular Json we would need to have that attribute over property anyway

[joukevandermaas]

Ah, I see why that would be bad. I had one other idea:

What if the JsonApiConfig allowed you to provide a callback that returned the JsonConverter instance? Like this:

new JsonApiConfig
{
    FindJsonConverter = (serializationContext) =>
    {
        // serializaztion context has values like
        // PropertyName, ModelType, ApiResource,
        // etc. so you can support any attribute
        // you need.

        return MyCustomJsonConverter();
    }
}

[DtaggartMB]

Ah, I see why that would be bad. I had one other idea:

What if the JsonApiConfig allowed you to provide a callback that returned the JsonConverter instance? Like this:

new JsonApiConfig
{
    FindJsonConverter = (serializationContext) =>
    {
        // serializaztion context has values like
        // PropertyName, ModelType, ApiResource,
        // etc. so you can support any attribute
        // you need.

        return MyCustomJsonConverter();
    }
}

I like binding it to the resource better then trying to add logic to the configuration to the configuration. Even though you need to add it to both the property and the resource it seems cleaner then trying to handle everything in one spot, I feel like that could get crowded pretty quick on a larger scale.

[edward-rosado]

@joukevandermaas Yea, we are having a similar issue where we need to specify a specific serialization attribute on a property of a model to be backwards compatible with application/json. To avoid coding the same things twice, it would be best to allow the developer to specify the jsonConverter on the model, and just use the value in the model in the resource during binding. That way you wouldnt need to put the same attribute for two objects.

In other words the models jsonConvert attributes get inherited by the resource, during the bind.

[edward-rosado]

@wernst I am interested to hear what your thoughts are on the above solution.

[DtaggartMB]

@wernst @sergey-litvinov-work @edward-rosado @joukevandermaas

I opened a PR with a possible solution, let me know if you have question or concerns. Thanks

[wernst]

@edward-rosado I agree that specifying the serialization rules in a single place would be better. I don't really think the resource class should be the place to define it - for support for JSON and JSON API as @sergey-litvinov-work pointed out and I've always seen the resource class's role as configuring the JSON API serialization details (links, type, etc), independent of the values of the data (@joukevandermaas is that a fair classification?).

@DtaggartMB thank you for the PR - I will take a look.

[joukevandermaas]

I like binding it to the resource better then trying to add logic to the configuration to the configuration.

I get this and it's fair, but I've always resisted attributes on model classes in saule because it has some implications that are quite troublesome. Mostly, I don't like requiring of coupling models to serialization logic. This can be impossible to do if your models are in a different assembly, for example.

The snippet I shared could look at the attribute and load the correct converter, so if you like you can use the JSON.NET attributes that way. It would require a one time implementation but it would be quite trivial to do.

I'm sorry I haven't looked at the pr yet, I am currently traveling. I will take a look when I am back.

[DtaggartMB]

@joukevandermaas its ok, I know we all get busy sometimes. @wernst @sergey-litvinov-work @edward-rosado what did you guys think?

[joukevandermaas]

Fixed with #238