Newtonsoft.json: Does not deserialize getter-only auto-properties

[azzlack]

Getter-only properties in C# are readonly, but they should still be deserialized.

I.e. this does not work (but should) (Tested in v6.0.8):

public class MyClass
{
    public string MyProp { get; }
}

...

var r = JsonConvert.DeserializeObject<MyClass>("{\"MyProp\":\"awesome\"}");

Assert.AreEqual("awesome", r.MyProp);

However, changing the class to this makes it all work:

public class MyClass
{
    public string MyProp { get; private set; }
}

These two classes should work the same.

[eluchsinger]

Isn't there a difference between the two properties? The first one does not have any Setters. Hence you can't Access it via reflection. The second one has a private setter, which you should be able to Access using reflection. This is just a Supposition.

[JamesNK]

There is no way to know where the value for a setter only property comes from. The second class provides a way to set the the property that just happens to be private.

[michaelmairegger]

@JamesNK: I use following code to write a value to a readonly property:

public class T
{
    public string Test { get; }
}

[Test]
public void TestSetReadonlyProperty()
{
    T testObject = new T();

    var property = testObject .GetType().GetProperty("Test");
    if (!property.CanWrite)
    {
        var field = testObject .GetType().GetField($"<{p.Name}>k__BackingField",
                                                   BindingFlags.NonPublic | BindingFlags.Instance);
        field?.SetValue(testObject , "ahah");
    }

    Assert.That(t.Test, Is.EqualTo("ahah"));
}

[khellang]

There is no way to know where the value for a setter only property comes from.

Fortunately, OP isn't talking about setter-only properties...

This is about _getter-only auto properties_, which means there _is_ a way to know where the value comes from: because it's an auto property, it has a compiler-generated backing field :sparkles:

The only difference between a getter-only auto property and one with a setter is that the backing field is readonly and thus can't be assigned through the property outside the constructor.

The second class provides a way to set the the property that just happens to be private.

So does the first class. It just happens to be through a compiler-generated, private field :wink:

Just because the property itself can't be assigned, it doesn't mean that Newtonsoft.Json couldn't assign the backing field directly, like @xxMUROxx's solution.

[NickCraver]

I agree this should be supported - we've added the same functionality to Protobuf and Dapper for all of the similar scenarios people run into.

[TylerBrinkley]

I disagree that this should be supported. If you have a getter-only auto property it should be assigned in the constructor or else add a private setter to the property. That way you're not relying on C# auto property generation implementation details that may change from version to version.

[ekevoo]

Does any other serialization solution support this scenario?

[NickCraver]

@TylerBrinkley How do you propose we do the constructor route (in general) when just deserializing any object? For most users we run into, they expect a getter-only property to be populated when you serialize/deserialize it. That's why we've added such functionality to Dapper and protobuf (@ekevoo's question) .

Just adding a private setter isn't so straightforward. That can easily be a performance impact since the JIT can no longer treat it as a readonly which enables a wide range of assumptions and optimizations.

I'm not arguing for this to be a default behavior, I'm simply saying it should be an easy-to-accomplish task. Yes, for the moment it is implementation dependent, so it'd be better if there was one copy of that implementation match in NuGet to update rather than every app doing the contracts themselves.

[mebjas]

I guess this have been implemented as of now, right?

[robgha01]

I found this today ben working on solving a bugg and it is exactly this issue the 3rd party model that i try to decompile has only one constructor parameter of type JToken token and the class handles the properties by itself "its auto get-only" properties however when i do
object value = token.ToObject(prop.PropertyType, serializer);.

the serializer is not smart and ignores that it has only a constructor parameter of JToken here it would be nice if it understands that hey the properties is not writable but we set the value anyway, passing in the token or doing the above mentioned stuff ?

what is the status of this ?

[HelloKitty]

So was this rejected? Or implemented? I don't see a commit referenced so I'm guessing deserialization to getter only auto-props is not supported?

[JamesNK]

Not supported

[bugproof]

Why is this closed?

[JamesNK]

For the reason I said?

[bugproof]

@JamesNK I saw some workaround with using [JsonConstructor] https://www.michalkomorowski.com/2017/08/jsonnet-also-tricked-me.html

So... how exactly should this problem be approached? every other solution uses custom IContractResolver which doesn't seem ideal to me.

Some interesting read https://www.productiverage.com/trying-to-set-a-readonly-autoproperty-value-externally-plus-a-little-benchmarkdotnet

[TylerBrinkley]

To me, if a property is not assignable purely within code then why should it be assignable through a deserializer? I mean, Visual Studio should give you warnings that a getter only auto property is never assigned to as it can only be assigned within a constructor without resorting to hacking it with reflection.

[Salgat]

It should be supported as an optional (but default off) option, that way a user that knows what they are doing can use it if need be.

[kbrownhbs]

I agree with Salgat.

If I use a JsonSerializer to serialize an object, I expect using the same JsonSerializer to deserializer the resultant json to return the original object.
_(so long as the original object had no ignore properties)_

[TylerBrinkley]

In that scenario, how are you initializing that object to serialize in the first place if not with constructor parameters?

[Salgat]

With the non-default constructor. The idea is that once it's constructed, you can serialize and preserve its state for retrieval elsewhere.

[kbrownhbs]

Because I like working with immutable object, I am initializing using a
constructor that has parameters for each object property.
See below

public sealed class CreateUserCommand
{
    public UserId UserId { get; }
    public EmailAddress Email { get; }
    public UserRole UserRole { get; }

    public CreateUserCommand(UserId userId, EmailAddress email,

UserRole userRole)
{
UserId = userId;
....
}
}

Annotating the constructor of every object in the hierarchy with
[JsonConstructor] works, but I would prefer to avoid polluting my POCO with
annotations if possible.

On Sun, Nov 18, 2018 at 10:58 PM Tyler Brinkley [e-mail]
wrote:

In that scenario, how are you initializing that object to serialize in the
first place if not as a constructor parameter?

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/JamesNK/Newtonsoft.Json/issues/703#issuecomment-439764805,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADX8A2uoaqcrk328YPCKA51pD1wWQ7jyks5uwixegaJpZM4GY74S
.

[JamesNK]

[JsonConstructor] is only required when there are multiple constructors. If you want multiple constructors and you don't want attributes then you can write a contract resolver that chooses the desired constructor.

[kbrownhbs]

Thanks!

Turns out I did have a private default constructor to support Protobuf, which forced me to add [JsonConstructor] primary constructor.

[kbrownhbs]

I was actually able to configure Protobuf to not call the constructor so I'm able to remove it.

Unfortunately Json.Net does not work by default with private constructor that has parameters. To make it work I had to resort back to adding [JsonConstructor]

[viceice]

C# 7.3 has a new backing field attributes feature.

Shouldn't it be possible to deserialize the auto-property with adding JsonPropertyAttribute to the backing field?

c# class Test { [field: JsonProperty("Num")] public int Num { get; } }

[TylerBrinkley]

@ViceIce I thought that might work with adding a JsonIgnoreAttribute to the property but it still doesn't because the backing field is readonly and it appears they aren't supported either.

[v-kydela]

Newtonsoft seems to be incredibly clever about finding ways to populate read-only members. Consider the following scenario:

```c#
public class MyClass
{
public MyClass(string prop1, string someName)
{
Prop1 = prop1;
Prop2 = someName;
}

public string Prop1 { get; }
public string Prop2 { get; }

}

When deserializing this class, Newtonsoft will be able to populate `Prop1` because it sees a parameter named `prop1` in the constructor:

```c#
Console.WriteLine(
    JsonConvert.SerializeObject(
        JsonConvert.DeserializeObject<MyClass>(
            "{\"Prop1\":\"foo\",\"Prop2\":\"bar\"}")));

// {"Prop1":"foo","Prop2":null}

Unfortunately this makes Newtonsoft easy to fool. You could do a little switcheroo in the constructor to have Newtonsoft deserialize the class incorrectly:

c# public MyClass(string prop1, string prop2) { Prop2 = prop1; Prop1 = prop2; }

It seems to me like it would be both easier and more accurate to have Newtonsoft just deserialize read-only members using reflection, but I understand that some developers would prefer to avoid reflection in their libraries to adhere to the principle of least surprise.

[TonyValenti]

Is there a way to make this happen? I have a hard time believing that Newtonsoft can't deserialize readonly properties. The cost on reflection should essentially be a one-time-per-app extremely minimal overhead:

````
using System;
using System.Collections.Generic;
using System.Linq.Expressions;
using System.Reflection;

namespace JsonTests {
class Program {
static void Main(string[] args) {

        var C = new Foo2();
        Setter<Foo2>.Set(C, "Name", "Hello");
        Setter<Foo2>.Set(C, "Age", 21);

        Console.WriteLine(C.Name);
        Console.WriteLine(C.Age);

    }

}


public static class Setter<T> {

    static Dictionary<string, Action<T, object>> Cache = new Dictionary<string, Action<T, object>>(StringComparer.InvariantCultureIgnoreCase);

    static Setter() {

        //var Props = typeof(T).GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.GetProperty | BindingFlags.SetProperty);
        var Props = typeof(T).GetProperties();

        foreach (var item in Props) {
            Cache[item.Name] = CreateSetter(item);
        }
    }

    public static void Set<V>(T Target, string PropName, V Value) {
        if (Cache.TryGetValue(PropName, out var Setter)) {
            Setter(Target, Value);
        }
    }

    public static Action<T, object> CreateSetter(PropertyInfo P) {
        var ret = default(Action<T, object>);

        ParameterExpression oParam = Expression.Parameter(typeof(T), "obj");
        ParameterExpression vParam = Expression.Parameter(typeof(object), "val");
        MethodCallExpression mce = Expression.Call(oParam, P.SetMethod, Expression.Convert(vParam, P.PropertyType));

        var action = Expression.Lambda<Action<T, object>>(mce, oParam, vParam);

        ret = action.Compile();

        return ret;
    }


    public static Action<T, TValue> CreateSetter<TValue>(Expression<Func<T, TValue>> memberLamda) {
        var ret = default(Action<T, TValue>);

        var memberSelectorExpression = memberLamda.Body as MemberExpression;
        if (memberSelectorExpression != null) {
            var property = memberSelectorExpression.Member as PropertyInfo;
            if (property != null) {

                ParameterExpression oParam = Expression.Parameter(typeof(T), "obj");
                ParameterExpression vParam = Expression.Parameter(typeof(TValue), "val");
                MethodCallExpression mce = Expression.Call(oParam, property.SetMethod, vParam);
                Expression<Action<T, TValue>> action = Expression.Lambda<Action<T, TValue>>(mce, oParam, vParam);

                ret = action.Compile();


            }
        }

        return ret;
    }
}


public class Foo2 {
    public string Name { get; set; }
    public int Age { get; private set; }
}

}

````

[MichaelPuckett2]

Can this please be re-opened if it's not already implemented. Having immutable DTO's is more ideal than even private setters and this can be done via reflection.

[cesarfp]

I'm having the same issue, I can't deserialize this class

public class SecretItem
{
public SecretItem() { }
public SecretIdentifier Identifier { get; }
}

[BrianOstrander]

Not trying to revive this from the dead, but since it's the first result on Google for "get only auto properties newtonsoft" I thought I'd leave a writeup I did the help any souls who totally nuked their codebase with { get; } and provide them with a possible -- albeit aggravating -- solution. Hope this helps.

[v-kydela]

@BrianOstrander - Did you see my comment? Newtonsoft deserializes getter-only properties using constructor parameter names.

[BrianOstrander]

@v-kydela Yes, your answer helped me fully explore the scope of the problem I had and the solutions available to me, however it was not a realistic solution for me. I felt I misused the get only auto property, and that requiring a constructor to be kept up to date was not ideal, for some of the very reasons you mentioned in your comment -- it's easy to fool Newtonsoft. Thank you though!

[zahirtezcan-bugs]

```

However, changing the class to this makes it all work:

public class MyClass
{
public string MyProp { get; private set; }
}

It does not work if MyProp returns a custom value-type. For such case, MyProp explicitly needs a public set operation.

[sake402]

I don't think getter only property should be deserialized.

While one may be able to get away with in by using the backing field, it will soon break if the compiler change the name of the backing field in the future.

Also how could we expect to deserialize when there is no backing field as NewtonSoft will have no way of differentiating

public class Foo{
    public string Name{ get; }
}

from

public class Foo{
    public string Name=>"MyName";
}

[MichaelPuckett2]

First, I've been doing this for years now so if it changes you have to update but it works. Second, your first example is a readonly property and it's not the same as your second example using expression bodied properties.
public Count { get;}
is not the same at all as
public Count => count; for example and the same applies if you're assigning a constant
public Count => 5; for example. They work completely different.
Finally, C# 9 is solving this with records anyway so all the naysays out there that don't understand true immutability and the need for it won't have to argue about it anymore.
Making a class deserializable and not immutable is the same as making a string, which is immutable, a List of char just to deserializer it, as opposed to the immutable string. We should be allowed to deserialize true immutable types just like we do any other types and therefore not require the class to be designed to work differently just because we plan on using Newtonsoft to do the work. It's ok though, Newton won't do it then the community has and will continue to do it ourselves.

[sake402]

Yes they are not the same, internally. But from the external perspective of the deserializer or the property caller, they are. Being bodied expression or auto property is encapsulated within the class and I as the class user won't know the difference.

My point is that an attempt by the deserializer to deserialize to public Count => 5 will certainly fail while public Count { get; } will not, even though they look alike from outside the class.

[MichaelPuckett2]

That's not fair. It's the job of the deserializer now to determine which one it is and how to handle it. You can get the difference via reflection.