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

System.Text.Json should support deserializing into mutable collections exposed via readonly members #38705

Closed
Symbai opened this issue Jul 2, 2020 · 8 comments

Comments

@Symbai
Copy link

Symbai commented Jul 2, 2020

Description

The pull request #34675 added support for private setters. And the default value for IgnoreReadOnlyProperties is false. So I would have assumed something like: [JsonInclude] public ObservableCollection<int> MyInt { get; } = new ObservableCollection<int>(); would work but it does not. It still requires the private set; to explicit written.

Configuration

  • .NET Core 5.0 (5.0.100-preview.5.20279.10)

Other information

    class Program
    {
        public static JsonSerializerOptions Options =>  new JsonSerializerOptions {IgnoreReadOnlyProperties = false};
        static void Main(string[] args)
        {
            var readonlySetter = new MyClass_Readonly();
            readonlySetter.MyInt.Add(1);
            var readonlyJson = JsonSerializer.Serialize(readonlySetter);
            Console.WriteLine("Readonly JSON: " + readonlyJson);
            var newReadonlySetter = JsonSerializer.Deserialize<MyClass_Readonly>(readonlyJson);
            Console.WriteLine("Readonly List Count: " + newReadonlySetter.MyInt.Count);

            var privateSetter = new MyClass_PrivateSetter();
            privateSetter.MyInt.Add(1);
            var privateJson = JsonSerializer.Serialize(privateSetter);
            Console.WriteLine("Private JSON: " + privateJson);
            var newPrivateSetter = JsonSerializer.Deserialize<MyClass_PrivateSetter>(privateJson);
            Console.WriteLine("Private List Count: " + newPrivateSetter.MyInt.Count);
        }
    }
    class MyClass_Readonly
    {
        [JsonInclude] public ObservableCollection<int> MyInt { get;  } = new ObservableCollection<int>();
    }
    class MyClass_PrivateSetter
    {
        [JsonInclude] public ObservableCollection<int> MyInt { get; private set; } = new ObservableCollection<int>();
    }

Output:

Readonly JSON: {"MyInt":[1]}
Readonly List Count: 0
Private JSON: {"MyInt":[1]}
Private List Count: 1
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Jul 2, 2020
@GrabYourPitchforks
Copy link
Member

I would think this behavior would be by design. A property defined as "{ get; }" doesn't have a private setter. It has literally no setter at all. There's nothing that can be reflection invoked.

@Symbai
Copy link
Author

Symbai commented Jul 3, 2020

Yes it would be a read-only property then. But guess what, IgnoreReadOnlyProperties is set to false. So I would think it would include read-only properties then, right? Might be a bug with private setter or with readonly properties. Both are now supported but in my example it just don't work.

FYI: In Newtonsoft JSON it works out of the box. Just saying in case you would think it would be technically impossible, it is not

@MichalStrehovsky
Copy link
Member

FYI: In Newtonsoft JSON it works out of the box. Just saying in case you would think it would be technically impossible, it is not

It doesn't, according to this: JamesNK/Newtonsoft.Json#703.

@Symbai
Copy link
Author

Symbai commented Jul 3, 2020

FYI: In Newtonsoft JSON it works out of the box. Just saying in case you would think it would be technically impossible, it is not

It doesn't, according to this: JamesNK/Newtonsoft.Json#703.

If you would have taken my example and ran it with Newtonsoft you would have noticed it does

    class Program
    {
        static void Main(string[] args)
        {
            var readonlySetter = new MyClass_Readonly();
            readonlySetter.MyInt.Add(1);
            var readonlyJson = JsonConvert.SerializeObject(readonlySetter);
            Console.WriteLine("Readonly JSON: " + readonlyJson);
            var newReadonlySetter = JsonConvert.DeserializeObject<MyClass_Readonly>(readonlyJson);
            Console.WriteLine("Readonly List Count: " + newReadonlySetter.MyInt.Count);

            var privateSetter = new MyClass_PrivateSetter();
            privateSetter.MyInt.Add(1);
            var privateJson = JsonConvert.SerializeObject(privateSetter);
            Console.WriteLine("Private JSON: " + privateJson);
            var newPrivateSetter = JsonConvert.DeserializeObject<MyClass_PrivateSetter>(privateJson);
            Console.WriteLine("Private List Count: " + newPrivateSetter.MyInt.Count);
        }
    }
    class MyClass_Readonly
    {
        [JsonProperty("a", DefaultValueHandling = DefaultValueHandling.Include)] public ObservableCollection<int> MyInt { get;  } = new ObservableCollection<int>();
    }
    class MyClass_PrivateSetter
    {
        [JsonProperty("b", DefaultValueHandling = DefaultValueHandling.Include)] public ObservableCollection<int> MyInt { get; private set; } = new ObservableCollection<int>();
    }

=== Output ===

Readonly JSON: {"a":[1]}
Readonly List Count: 1
Private JSON: {"b":[1]}
Private List Count: 1

Also this side note does not change my actual issue about both readonly and private setters are officially supported but practically it doesn't work. So let's keep focusing on System.Text.Json instead of Newtonsoft for now.

@MichalStrehovsky
Copy link
Member

I see - you don't want to be able to actually set readonly properties, but to populate existing collections with deserialized data. (I assume Newtonsoft doesn't actually set the readonly property in your sample, but clears the collection and adds new items to it to sidestep problems discussed at length in the Newtonsoft issue.)

@Symbai
Copy link
Author

Symbai commented Jul 3, 2020

Yes exactly! 👍 I want the property to be a new collection with all the data from the deserialization (and if there is no data, it leaves the default behavior) I'm sorry, if I was misleading about this.

@GrabYourPitchforks GrabYourPitchforks changed the title System.Text.Json.Serialization Support for private setters not working with readonly properties System.Text.Json should support deserializing into mutable collections exposed via readonly members Jul 3, 2020
@GrabYourPitchforks
Copy link
Member

I've updated the issue title to more accurately reflect what's being requested.

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Jul 6, 2020
@layomia layomia added this to the 5.0.0 milestone Jul 6, 2020
@layomia
Copy link
Contributor

layomia commented Jul 6, 2020

This is a duplicate of #30258.

@layomia layomia closed this as completed Jul 6, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants