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 constructor deserializing with covariant return type read-only properties sets values to null #56999

Closed
krispenner opened this issue Aug 6, 2021 · 4 comments

Comments

@krispenner
Copy link

krispenner commented Aug 6, 2021

Description

I would expect the below to work, however it does not. Maybe because covariant return types are new and not yet handled in System.Text.Json yet. I think the example speaks for itself but let me know if more information is required.

Configuration

  • Which version of .NET is the code running on? .NET 5.0
  • What is the architecture (x64, x86, ARM, ARM64)? x64
  • Do you know whether it is specific to that configuration? It is not.

Non-working Example

Using covariant return type read-only properties passed through derived constructor does not work.

https://dotnetfiddle.net/IsNX3q

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
					
public class Program
{
	public static void Main()
	{
		var json = "{ \"prop\": { \"id\": \"abc\", \"num\": 2 } }";
		var obj = JsonSerializer.Deserialize<DerivedClass>(json);
		Console.WriteLine(obj?.Property?.Id ?? "null");
		Console.WriteLine(obj?.Property?.Number?.ToString() ?? "null");
	}
	
	public class BaseClass
	{
		public BaseClass(BaseProperty property)
		{
			Property = property;
		}

		[JsonPropertyName("prop")]
		public virtual BaseProperty Property { get; }
	}
	
	public class DerivedClass : BaseClass
	{
		public DerivedClass(DerivedProperty property)
			: base(property)
		{
		}

		public override DerivedProperty Property { get; }
	}
	
	public class BaseProperty
	{
		[JsonPropertyName("id")]
		public string Id { get; set; }
	}
	
	public class DerivedProperty : BaseProperty
	{
		[JsonPropertyName("num")]
		public int? Number { get; set; }
	}
}

Output:

null
null

Working Example

Using non-covariant return type read-only properties passed through derived constructor does work:

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
					
public class Program
{
	public static void Main()
	{
		var json = "{ \"prop\": { \"id\": \"abc\", \"num\": 2 } }";
		var obj = JsonSerializer.Deserialize<DerivedClass>(json);
		Console.WriteLine(obj?.Property?.Id ?? "null");
	}
	
	public class BaseClass
	{
		public BaseClass(BaseProperty property)
		{
			Property = property;
		}

		[JsonPropertyName("prop")]
		public virtual BaseProperty Property { get; }
	}
	
	public class DerivedClass : BaseClass
	{
		public DerivedClass(BaseProperty property)
			: base(property)
		{
		}
	}
	
	public class BaseProperty
	{
		[JsonPropertyName("id")]
		public string Id { get; set; }
	}
}

Output:

abc
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Aug 6, 2021
@ghost
Copy link

ghost commented Aug 6, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

I would expect the below to work, however it does not. Maybe because covariant return types are new and not yet handled in System.Text.Json yet. I think the example speaks for itself but let me know if more information is required.

Configuration

  • Which version of .NET is the code running on? .NET 5.0
  • What is the architecture (x64, x86, ARM, ARM64)? x64
  • Do you know whether it is specific to that configuration? It is not.

Non-working Example

Using covariant return type read-only properties passed through derived constructor does not work.

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
					
public class Program
{
	public static void Main()
	{
		var json = "{ \"prop\": { \"id\": \"abc\", \"num\": 2 } }";
		var obj = JsonSerializer.Deserialize<DerivedClass>(json);
		Console.WriteLine(obj?.Property?.Id ?? "null");
		Console.WriteLine(obj?.Property?.Number?.ToString() ?? "null");
	}
	
	public class BaseClass
	{
		public BaseClass(BaseProperty property)
		{
			Property = property;
		}

		[JsonPropertyName("prop")]
		public virtual BaseProperty Property { get; }
	}
	
	public class DerivedClass : BaseClass
	{
		public DerivedClass(DerivedProperty property)
			: base(property)
		{
		}

		public override DerivedProperty Property { get; }
	}
	
	public class BaseProperty
	{
		[JsonPropertyName("id")]
		public string Id { get; set; }
	}
	
	public class DerivedProperty : BaseProperty
	{
		[JsonPropertyName("num")]
		public int? Number { get; set; }
	}
}

Output:

null
null

Working Example

Using non-covariant return type read-only properties passed through derived constructor does work:

using System;
using System.Text.Json;
using System.Text.Json.Serialization;
					
public class Program
{
	public static void Main()
	{
		var json = "{ \"prop\": { \"id\": \"abc\", \"num\": 2 } }";
		var obj = JsonSerializer.Deserialize<DerivedClass>(json);
		Console.WriteLine(obj?.Property?.Id ?? "null");
	}
	
	public class BaseClass
	{
		public BaseClass(BaseProperty property)
		{
			Property = property;
		}

		[JsonPropertyName("prop")]
		public virtual BaseProperty Property { get; }
	}
	
	public class DerivedClass : BaseClass
	{
		public DerivedClass(BasePropertyproperty)
			: base(property)
		{
		}
	}
	
	public class BaseProperty
	{
		[JsonPropertyName("id")]
		public string Id { get; set; }
	}
}

Output:

"abc"
Author: krispenner
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@krispenner krispenner changed the title System.Text.Json constructor deserializing with covariant return type read-only properties System.Text.Json constructor deserializing with covariant return type read-only properties set values to null Aug 6, 2021
@krispenner krispenner changed the title System.Text.Json constructor deserializing with covariant return type read-only properties set values to null System.Text.Json constructor deserializing with covariant return type read-only properties sets values to null Aug 6, 2021
@eiriktsarpalis
Copy link
Member

I can reproduce; when I change the code to specify a non-covariant override then the serializer would throw the exception:

Unhandled exception. System.InvalidOperationException: Members 'Property' and 'Property' on type 'Program+NonCovariantReturnDerivedClass' cannot both bind with parameter 'property' in constructor 'Void .ctor(System.Object)' on deserialization.

I was not able to produce a workaround that didn't involve renaming the base property. cc @layomia

Minimal reproduction:

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

public class Program
{
    public static void Main()
    {
        var json = "{ \"prop\": \"value\" }";

        var obj = JsonSerializer.Deserialize<BaseClass>(json);
        Console.WriteLine(obj.Property is not null); // True

        obj = JsonSerializer.Deserialize<CovariantReturnDerivedClass>(json);
        Console.WriteLine(obj.Property is not null); // False

        // Unhandled exception. System.InvalidOperationException: Members 'Property' and 'Property' on type 'Program+NonCovariantReturnDerivedClass'
        // cannot both bind with parameter 'property' in constructor 'Void .ctor(System.Object)' on deserialization.
        obj = JsonSerializer.Deserialize<DerivedClass>(json);
    }

    public class BaseClass
    {
        public BaseClass(object property)
        {
            Property = property;
        }

        [JsonPropertyName("prop")]
        public virtual object Property { get; }
    }

    public class CovariantReturnDerivedClass : BaseClass
    {
        public CovariantReturnDerivedClass(string property)
            : base(property)
        {
        }

        public override string Property { get; }
    }

    public class DerivedClass : BaseClass
    {
        public DerivedClass(object property)
            : base(property)
        {
        }

        public override object Property { get; }
    }
}

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Aug 9, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Aug 9, 2021
@eiriktsarpalis
Copy link
Member

Related to #46522.

@eiriktsarpalis eiriktsarpalis added the Priority:2 Work that is important, but not critical for the release label Oct 14, 2021
@jeffhandley jeffhandley modified the milestones: 7.0.0, Future Jul 10, 2022
@jeffhandley jeffhandley removed the Priority:2 Work that is important, but not critical for the release label Jul 10, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Aug 1, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: 8.0.0, Future Sep 29, 2022
@layomia
Copy link
Contributor

layomia commented Dec 2, 2022

We should address as part of #44428.

@layomia layomia closed this as completed Dec 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jan 1, 2023
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

4 participants