JamesNK / Newtonsoft.Json

Json.NET is a popular high-performance JSON framework for .NET
https://www.newtonsoft.com/json
MIT License
10.64k stars 3.24k forks source link

Object duplication when deserializing class with initial static value assigned. #2932

Closed pipoon21 closed 5 months ago

pipoon21 commented 5 months ago

Possibly related to https://github.com/JamesNK/Newtonsoft.Json/issues/2451 The problem disappear if ObjectCreationHandling = ObjectCreationHandling.Replace is set.

Source/destination types

    public static class StaticClass {
        public static readonly SomeClass StaticInstance = new();
    }

    public class OuterClass {
        /// <remarks>
        /// This assignment cause the bug, result in duplication of <see cref="SomeClass.NestedClasses"/> for some reason.
        /// </remarks>
        public SomeClass Instance { get; set; } = StaticClass.StaticInstance;

        public required List<SomeClass> Collection { get; set; }
    }

    public class SomeClass {
        public List<NestedClass> NestedClasses { get; set; } = new();
    }

    public class NestedClass;

Expected behavior

First and second time deserializing from the same json string should return the same result.

Actual behavior

The second time OuterClass was deserialized (from the exact same json), multiple NestedClass will appear in NestedClasses property.

Steps to reproduce


  [Fact]
    public void WeiredNewtonsoftJsonBugReproduced_ShouldFailedIfNotFixed() {
        RuntimeHelpers.RunClassConstructor(typeof(StaticClass).TypeHandle);

        SomeClass someClass = new() {
            NestedClasses = new List<NestedClass>() {
                new()
            },
        };
        OuterClass outerClass = new() {
            Instance = someClass,
            Collection = new List<SomeClass>() {
                someClass,
            }
        };

        string jsonStr = JsonConvert.SerializeObject(outerClass,
            new JsonSerializerSettings() {
                TypeNameHandling = TypeNameHandling.All,
                PreserveReferencesHandling = PreserveReferencesHandling.All,
                Formatting = Formatting.Indented,
            });

        OuterClass? d1 =
            JsonConvert.DeserializeObject<OuterClass>(
                jsonStr, new JsonSerializerSettings() {
                    TypeNameHandling = TypeNameHandling.All,
                    PreserveReferencesHandling = PreserveReferencesHandling.All,
                });

        Assert.NotNull(d1);
        Assert.Single(d1.Instance.NestedClasses);

        OuterClass? d2 =
            JsonConvert.DeserializeObject<OuterClass>(
                jsonStr, new JsonSerializerSettings() {
                    TypeNameHandling = TypeNameHandling.All,
                    PreserveReferencesHandling = PreserveReferencesHandling.All,
                });

        Assert.NotNull(d2);
        Assert.Single(d2.Instance.NestedClasses);

    }
elgonzo commented 5 months ago

Not a bug.

As you already yourself mentioned, to get the desired behavior the ObjectCreationHandling setting has to be set to ObjectCreationHandling.Replace.

It's a bit unfortunate that the default setting for ObjectCreationHandling is to reuse objects and add items to existing collections (as documented) instead of replacing them. If you look around in the issue tracker here, you will find quite a number of issue reports of users being surprised by this default behavior like you, but it is what it is, sadly. This default behavior has been the default since a long time, and changing it would constitute a major breaking change that quite likely is not going to happen because of this reason. (Additionally, over the recent years, development activities regarding this library have been minor now that STJ has become the modern go-to json serializer in .NET)