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

Deserializing Existing Json with Basic Example Results in Exception #32

Open
jonmotos opened this issue May 5, 2023 · 3 comments
Open
Assignees
Labels
bug Something isn't working

Comments

@jonmotos
Copy link

jonmotos commented May 5, 2023

Describe the bug
When using JsonKnownTypes on existing json content, adding a new derived type and then decorating the base class with the basic [JsonConverter...] attribute results in an exception:

To Reproduce
Steps to reproduce the behavior:

  1. Create a base class
  2. Serialize this class using standard Newtonsoft behaviors, no type handling
  3. Implement a new derived class
  4. Decorate the base class with the JsonConverter for JsonKnownTypes
  5. Deserialize

Expected behavior
I would expect an instance without a discriminator to automatically deserialize to the base class

Screenshots
If applicable, add screenshots to help explain your problem.

Additional context
Exception content:
JsonKnownTypes.Exceptions.JsonKnownTypesException: discriminator is not registered for T type

I then tried adding [JsonKnownTypeFallback(typeof(Base))]
This results in another exception:
System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.InvalidCastException: Unable to cast object of type 'Base' to type 'Derived'.

Two unexpected behaviors here!

I would be very happy to help implement a solution here!

@jonmotos jonmotos added the bug Something isn't working label May 5, 2023
@dmitry-bym
Copy link
Owner

dmitry-bym commented May 17, 2023

What about that exception

Unable to cast object of type 'Base' to type 'Derived'.

can you pls provide exact code(just past all code here), cause its a bit weird why it cast Base to Derived.

@jonmotos
Copy link
Author

jonmotos commented May 1, 2024

Some more context: I have old data that was deserialized as one type. A derived type was later added.
After setting up testing, I'm not entirely sure I agree with myself about scenario 2. Perhaps the shown behavior is OK. Deserializing as the base type works fine.
I do think scenario 1 is valid, though!

using JsonKnownTypes;
using Newtonsoft.Json;
using Newtonsoft.Json.Linq;

namespace JsonKnownTypesRepro
{
	[TestClass]
	public class KnownTypesDerivedExceptionRepro
	{
		[JsonConverter(typeof(JsonKnownTypesConverter<Base>))]
        public class Base
        {
			public string BaseProp { get; set; }
		}

        public class Derived : Base
        {
			public string DerivedProp { get; set; }
		}

        [TestMethod]
        public void TestLegacyBaseClassDeserialization()
        {
			//in the scenario where a base type was serialized in old code, before the jsonknowntype was associated with it,
			//adding the jsonknowntypes alone conversion results in an exception
			//expected: the conversion to succeed, deserializing the base class

			var @base = new Base { BaseProp = "base" };

			var json = JsonConvert.SerializeObject(@base, Formatting.Indented);

			Console.WriteLine(json);

			//remove the $type discriminator to simulate the scenario
			var token = JToken.Parse(json);
            token["$type"].Parent.Remove();

			Console.WriteLine(token.ToString(Formatting.Indented));

            var deserialized = JsonConvert.DeserializeObject<Base>(token.ToString());
		}

        [JsonConverter(typeof(JsonKnownTypesConverter<Base2>))]
        [JsonKnownTypeFallback(typeof(Base2))]
        public class Base2
        {
            public string BaseProp { get; set; }
        }

        public class Derived2 : Base2
        {
            public string DerivedProp { get; set; }
        }

        [TestMethod]
        public void TestLegacyBaseClassDeserialization2()
        {
            //in the scenario where a base type was serialized in old code, before the jsonknowntype was associated with it,
            //adding the jsonknowntypes conversion and a fallback
            //attempting to deserialize as the derived type results in an exception
            //expected: i'm not sure - perhaps it's perfectly valid to fail
            //also, this would be fixed by JsonDiscriminator.UseBaseTypeForCanConvert 

            var @base = new Base2 { BaseProp = "base2" };

            var json = JsonConvert.SerializeObject(@base, Formatting.Indented);

            Console.WriteLine(json);

            //no $type discriminator is emitted

            var deserialized2 = JsonConvert.DeserializeObject<Derived2>(json);
        }
	}
}

@dmitry-bym
Copy link
Owner

For now don't have time to figure this out, if u can figure this out and fix if needed ill merge and publish that quickly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants