Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Serializing base class with discriminator property results in KeyNotFoundException #79

Closed
CK-Yong opened this issue Jul 19, 2019 · 1 comment
Milestone

Comments

@CK-Yong
Copy link

CK-Yong commented Jul 19, 2019

Hello, awesome package you have here. However, I have found a potential bug when you are serializing a base type while enabling discriminators. I have added a failing test below.

    public class Test
    {
        [Fact]
        public void BreakingTest()
        {
            var input = new[] {new Cat(), new Animal()};

            var serializersettings = new JsonSerializerSettings();
            serializersettings.Converters.Add(
                JsonSubtypesConverterBuilder
                    .Of(typeof(Animal), "Type")
                    .SerializeDiscriminatorProperty()
                    .RegisterSubtype(typeof(Cat), typeof(Cat).Name)
                    .Build());

            var result = String.Empty;
            var exception = Record.Exception(() => result = JsonConvert.SerializeObject(input, serializersettings));
            
            Assert.Null(exception);
            Assert.Equal("[{\"Type\":\"Cat\"},{\"Type\":\"Animal\"}]", result);
        }
    }

public class Cat : Animal{}

public class Animal{}

Expected behavior

When you serialize a base type with discriminators enabled, it'll be serialized with the base type as the discriminator value. See also provided test case.

Actual behavior

I get the following error:

System.Collections.Generic.KeyNotFoundException: The given key '(...).Animal' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at JsonSubTypes.JsonSubtypesConverter.WriteJson(JsonWriter writer, Object value, JsonSerializer serializer)
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeConvertable(JsonWriter writer, JsonConverter converter, Object value, JsonContract contract, JsonContainerContract collectionContract, JsonProperty containerProperty) in /_/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs:line 652
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.SerializeList(JsonWriter writer, IEnumerable values, JsonArrayContract contract, JsonProperty member, JsonContainerContract collectionContract, JsonProperty containerProperty) in /_/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs:line 691
   at Newtonsoft.Json.Serialization.JsonSerializerInternalWriter.Serialize(JsonWriter jsonWriter, Object value, Type objectType) in /_/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs:line 80
(...)
@manuc66
Copy link
Owner

manuc66 commented Jul 25, 2019

Hi @CK-Yong

Thanks for submitting this issue

I don't agree with the expectation that unregistered base class should serialize their class name because the "discriminator's value" may be numeric or have a specific naming convention (with specific prefix/suffix or another)

When using JsonSubtypesConverterBuilder all sub types should be registered:

          var serializersettings = new JsonSerializerSettings();
            serializersettings.Converters.Add(
                JsonSubtypesConverterBuilder
                    .Of(typeof(Animal2), "Type")
                    .SerializeDiscriminatorProperty()
                    .RegisterSubtype(typeof(Animal), typeof(Animal).Name)
                    .RegisterSubtype(typeof(Cat), typeof(Cat).Name)
                    .Build());

However, I agree that the exception KeyNotFoundException is not explicit enough and should be fixed

manuc66 added a commit that referenced this issue Aug 12, 2019
@manuc66 manuc66 added this to the 1.7.0 milestone Mar 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants