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: DefaultJsonTypeInfoResolver throws on CreateJsonTypeInfo for interfaces with duplicate properties #101673

Closed
angelaki opened this issue Apr 29, 2024 · 6 comments

Comments

@angelaki
Copy link

angelaki commented Apr 29, 2024

Description

In my scenario I need to deserialize interface types. I use some kind of mapping what default type gets used for what interface. Now I get an exception, if several interfaces define the same property. Imho this should be fine, as long as types etc. match.

Even if not, some kind of handler (via override) would be helpful. Right now I'd need to implement a whole new IJsonTypeInfoResolver.

Reproduction Steps

using System.Text.Json;
using System.Text.Json.Serialization.Metadata;

var options = new JsonSerializerOptions();

//Not needed for exception, just to clarify interface deserialization
//options.TypeInfoResolver = new InterfaceJsonTypeInfoResolver();

var result = JsonSerializer.Serialize(new CA(), options);
var ca = JsonSerializer.Deserialize<IA>(result, options);

ca.ToString();

public interface IA1 { string A { get; set; } }
public interface IA2 { string A { get; set; } }
public interface IA : IA1, IA2 { }
public class CA : IA { public string A { get; set; } = "!"; }

public class InterfaceJsonTypeInfoResolver : DefaultJsonTypeInfoResolver
{
    public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
    {
        var result = base.GetTypeInfo(type, options);
        if (type.IsInterface) { result.CreateObject = () => new CA(); }
        return result;
    }
}

Expected behavior

Make it work on some type. Or allow some kind of error handling. (Btw. this is working with Json.Net out of the box. But not sure how this case exactly is handled internal).

Actual behavior

Throws an ThrowInvalidOperationException_SerializerPropertyNameConflict.

Regression?

No response

Known Workarounds

Repeat the property on the top-most interface:
public interface IA : IA1, IA2 { new string A { get; set; } }

Configuration

No response

Other information

No response

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Apr 29, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented May 13, 2024

The contract customization angle is a red herring in this case, the error surfaces from the fact that IA contains conflicting propety names emanating from IA1 and IA2. The same error surfaces if you try to serialize for the same type:

JsonSerializer.Serialize<IA>(new CA());

public interface IA1 { string A { get; set; } }
public interface IA2 { string A { get; set; } }
public interface IA : IA1, IA2 { }
public class CA : IA { public string A { get; set; } = "!"; }

STJ requires that you disambiguate between the two properties, either by changing the name for one of them, changing it via a JsonPropertyNameAttrbute annotation or ignoring it altogether via the JsonIgnoreAttribute.

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label May 13, 2024
@angelaki
Copy link
Author

But if name and type matches, why not just allow that definition? Imho that'd be totally fine. If they don't it could still throw that exception.

@eiriktsarpalis
Copy link
Member

Because they could be pointing to two separate implementations. This is valid for instance:

public class CA : IA
{
    string IA1.A { get; set; } = "foo";
    string IA2.A { get; set; } = "bar";
}

Should the resultant JSON property have value "foo" or "bar" in this case?

@angelaki
Copy link
Author

Ok, valid argument. You even needed to compare the value making things way too complicated / untransparent. But why is my work-around with a public interface IA : IA1, IA2 { new string A { get; set; } } valid? Now I could even have three values! Or have you just decided, that the new keyword is proof enough that the author wanted this property to be used (by default)?

@eiriktsarpalis
Copy link
Member

But why is my work-around with a public interface IA : IA1, IA2 { new string A { get; set; } } valid?

Because in this case the new A shadows both A's defined in IA1 and IA2 so you now only have one property in the contract of IA.

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