JsonInheritanceConverter thread safety

[neotek]

Hi

Thanks for providing such a nice Swagger/JsonSchema implementation including discriminator support.
I tried to implemented this in a Project. Unfortunately the JsonInheritanceConverter seems not to be thread safe. In Unit Tests I get random exceptions as the discriminator field is not present in the written Json.
Setting the contract.Converter = null; in WriteJson() seems to cause that issue.

Has anyone solved this?

Thanks
Urs

[RicoSuter]

This is bad, I could not find another way to implement the converter...

[RicoSuter]

We need a fix for that. There are unit tests to check its functionality, so it should be simple to experiment...

[RicoSuter]

Why is this running in parallel?

[RicoSuter]

Is the serializer reused?

[neotek]

It runs in parallel as it speeds up unit and integration tests. But this could happen in other parallel scenarios as well.
I'm using it like this: JsonConvert.SerializeObject(entity, Newtonsoft.Json.Formatting.Indented, GetJsonSerializerSettings())
So the JsonSerializerSettings are not reused.

[RicoSuter]

So if you call JsonConvert.SerializeObject then you see these problems? I thought that SerializeObject creates a new serializer to prevent these problems... But it seems that it reuses the serializer internally...

My problem: I could not find a way to implement the converter without setting the Converter to null...

[RicoSuter]

I think this is the problem:

http://stackoverflow.com/questions/33557737/does-json-net-cache-types-serialization-information

But creating a new resolver each time is not an option because it would degrade the performance dramatically...

[RicoSuter]

Ill try something with thread local... But i have to find a solution to reproduce your problem in a unit test first...

[neotek]

I see. Yes, I also was not able to find a solution that works.
This implementation seems to have the same Problem:
https://github.com/Azure/autorest/blob/45ddc1a84c58f5732a2f77418079c9439f39e892/src/client/Microsoft.Rest.ClientRuntime/Serialization/PolymorphicSerializeJsonConverter.cs

I tried to implement a workaround by defining a property on the base class with the name of the discriminator:

    [JsonProperty]
    public string Kind
    {
        get
        {
            if (string.IsNullOrEmpty(_kind))
            {
                Type type = this.GetType().UnderlyingSystemType;
                _kind = type.Name;

                return type.Name;
            }

            return _kind;
        }
    }

This works for Serialize/Deserialize the data, but when i try to build the swagger.json I get an exception as the field is defined multiple times.
https://github.com/NJsonSchema/NJsonSchema/blob/baaa6a3afd2f2e632ad2ca2cd1a2dd4edc509d34/src/NJsonSchema/Generation/JsonSchemaGenerator.cs#L304

Thanks for your help.

[RicoSuter]

The "defined multiple times" problem can easily be fixed...
But I don't like that you have to implement the property yourself...

[RicoSuter]

JamesNK/Newtonsoft.Json#386

[RicoSuter]

I think I have found a solution...

[RicoSuter]

62850de

[RicoSuter]

Also updated in NSwag: RicoSuter/NSwag@a47fca7

[RicoSuter]

Check out NJsonSchema v4.18. Is this working for you?

[neotek]

Hey, thank you.
For me this solution still does not work as I use nested structures with additional classes that have discriminators.

I extended the Tests here:
https://github.com/neotek/NJsonSchema/commit/6246910f651da348d34d989be33af13feaa880ac

With the new approach all child elements are deserialized to the base class. The JsonInheritanceConverter seems to gets called recursively and is disabled on the child elements.

[RicoSuter]

Ok, next workaround :-), now it should work as expected:

97228f2

[RicoSuter]

v4.19

[neotek]

That helped.
No failing unit tests anymore.
Thanks a lot.