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

Wrong Exception Text for duplicate JsonConstructorAttribute #55321

Closed
Tornhoof opened this issue Jul 8, 2021 · 1 comment · Fixed by #55324
Closed

Wrong Exception Text for duplicate JsonConstructorAttribute #55321

Tornhoof opened this issue Jul 8, 2021 · 1 comment · Fixed by #55324
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@Tornhoof
Copy link
Contributor

Tornhoof commented Jul 8, 2021

Description

If you apply the [JsonConstructorAttribute] to more than one constructor, the runtime exception is:

System.InvalidOperationException
  HResult=0x80131509
  Message=The type 'ConsoleApp11.Program+Invoice' cannot have more than one property that has the attribute 'System.Attribute'.
  Source=System.Text.Json
  StackTrace:
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute[TAttribute](Type classType)
   at System.Text.Json.Serialization.Converters.ObjectConverterFactory.GetDeserializationConstructor(Type type)
   at System.Text.Json.Serialization.Converters.ObjectConverterFactory.CreateConverter(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverterFactory.GetConverterInternal(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetConverterInternal(Type typeToConvert)
   at System.Text.Json.JsonSerializerOptions.DetermineConverter(Type parentClassType, Type runtimePropertyType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.GetConverter(Type type, Type parentClassType, MemberInfo memberInfo, Type& runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<RootBuiltInConvertersAndTypeInfoCreator>g__CreateJsonTypeInfo|107_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClass(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClassForRootType(Type type)
   at System.Text.Json.JsonSerializer.GetTypeInfo(Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Write[TValue](TValue& value, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)

The Exception text is wrong:

  • The attribute is not applied to properties at all
  • The attribute type should reference the concrete type, not the base Attribute type.

The error message template should probably just be changed to say member instead of property.
The Throwhelper:

public static void ThrowInvalidOperationException_SerializationDuplicateTypeAttribute<TAttribute>(Type classType)
{
throw new InvalidOperationException(SR.Format(SR.SerializationDuplicateTypeAttribute, classType, typeof(Attribute)));
}

should be changed to use typeof(TAttribute) instead of typeof(Attribute) (I guess that was a typo).

This was found while looking at: #55318

Regression?

No, appears to happen in 5.0 too.

Other information

Repro:

	class Program
	{
		static void Main(string[] args)
		{
			var invoice = new Invoice(new List<InvoiceItem>());
			var serialized = JsonSerializer.Serialize(invoice);
			var deserialized = JsonSerializer.Deserialize<Invoice>(serialized);
		}

		public class Invoice
		{
			private List<InvoiceItem> items;

			public IReadOnlyCollection<InvoiceItem> Items => items;

			[JsonConstructor]
			public Invoice(List<InvoiceItem> items)
			{
				this.items = items;
			}

			[JsonConstructor]
			[Obsolete]
			public Invoice(IReadOnlyCollection<InvoiceItem> items)
			{
				this.items = items as List<InvoiceItem> ?? this.items.ToList();
			}


			public void AddItem(InvoiceItem invoiceItem)
			{
				//Some domain logic validation
				items.Add(invoiceItem);
			}
		}

		public class InvoiceItem
		{ }
    }
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Jul 8, 2021
@ghost
Copy link

ghost commented Jul 8, 2021

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

Issue Details

Description

If you apply the [JsonConstructorAttribute] to more than one constructor, the runtime exception is:

System.InvalidOperationException
  HResult=0x80131509
  Message=The type 'ConsoleApp11.Program+Invoice' cannot have more than one property that has the attribute 'System.Attribute'.
  Source=System.Text.Json
  StackTrace:
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_SerializationDuplicateTypeAttribute[TAttribute](Type classType)
   at System.Text.Json.Serialization.Converters.ObjectConverterFactory.GetDeserializationConstructor(Type type)
   at System.Text.Json.Serialization.Converters.ObjectConverterFactory.CreateConverter(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverterFactory.GetConverterInternal(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetConverterInternal(Type typeToConvert)
   at System.Text.Json.JsonSerializerOptions.DetermineConverter(Type parentClassType, Type runtimePropertyType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.GetConverter(Type type, Type parentClassType, MemberInfo memberInfo, Type& runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<RootBuiltInConvertersAndTypeInfoCreator>g__CreateJsonTypeInfo|107_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClass(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClassForRootType(Type type)
   at System.Text.Json.JsonSerializer.GetTypeInfo(Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Write[TValue](TValue& value, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)

The Exception text is wrong:

  • The attribute is not applied to properties at all
  • The attribute type should reference the concrete type, not the base Attribute type.

The error message template should probably just be changed to say member instead of property.
The Throwhelper:

public static void ThrowInvalidOperationException_SerializationDuplicateTypeAttribute<TAttribute>(Type classType)
{
throw new InvalidOperationException(SR.Format(SR.SerializationDuplicateTypeAttribute, classType, typeof(Attribute)));
}

should be changed to use typeof(TAttribute) instead of typeof(Attribute) (I guess that was a typo).

This was found while looking at: #55318

Regression?

No, appears to happen in 5.0 too.

Other information

Repro:

	class Program
	{
		static void Main(string[] args)
		{
			var invoice = new Invoice(new List<InvoiceItem>());
			var serialized = JsonSerializer.Serialize(invoice);
			var deserialized = JsonSerializer.Deserialize<Invoice>(serialized);
		}

		public class Invoice
		{
			private List<InvoiceItem> items;

			public IReadOnlyCollection<InvoiceItem> Items => items;

			[JsonConstructor]
			public Invoice(List<InvoiceItem> items)
			{
				this.items = items;
			}

			[JsonConstructor]
			[Obsolete]
			public Invoice(IReadOnlyCollection<InvoiceItem> items)
			{
				this.items = items as List<InvoiceItem> ?? this.items.ToList();
			}


			public void AddItem(InvoiceItem invoiceItem)
			{
				//Some domain logic validation
				items.Add(invoiceItem);
			}
		}

		public class InvoiceItem
		{ }
    }
Author: Tornhoof
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 8, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 13, 2021
@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed untriaged New issue has not been triaged by the area owner labels Jul 13, 2021
@eiriktsarpalis eiriktsarpalis added this to the 6.0.0 milestone Jul 13, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants