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

The 'new' modifier doesn't always work with [JsonIgnore] #59675

Open
Tracked by #73255
steveharter opened this issue Sep 27, 2021 · 1 comment
Open
Tracked by #73255

The 'new' modifier doesn't always work with [JsonIgnore] #59675

steveharter opened this issue Sep 27, 2021 · 1 comment

Comments

@steveharter
Copy link
Member

steveharter commented Sep 27, 2021

For polymorphic scenarios, the new modifier does not always work with JsonIgnore. This affects both the reflection-based serializer and the source-gen serializer. See also #58985 (comment)

The semantics of the new modifier need to be documented and deterministic. For example, when [JsonIgnore] is applied to a derived type's property that has new and [JsonIgnore], does the base class property (which is a different property / has its own vtable slot) get serialized and deserialized?

Also verification is need when [JsonIgnore] and new are used along other attributes including [JsonPropertyOrder] on a derived type's property. The serializer should used the [JsonPropertyOrder] on the property that is actually serialized, and not the one that is ignored.

Test case failure

  string json = JsonSerializer.Serialize(new ConcreteDerivedClass());
  // Throws System.ArgumentException: 'An item with the same key has already been added. Key: Ignored_New_Property'  

  public class ConcreteDerivedClass : AbstractBaseClass
  {
      [JsonIgnore]
      public new int Ignored_New_Property { get; set; } = 1; // Also test with making this new slot method virtual
  }

  public abstract class AbstractBaseClass
  {
      [JsonIgnore]
      public int Ignored_New_Property { get; set; } = 2;
  }

Test case success 1 (serialize)

  string json = JsonSerializer.Serialize(new ConcreteDerivedClass());
  // Correctly returns "{\"Ignored_New_Property\":1}"
  
  public class ConcreteDerivedClass : AbstractBaseClass
  {
      public new int Ignored_New_Property { get; set; } = 1;
  }
  
  public abstract class AbstractBaseClass
  {
      [JsonIgnore]
      public int Ignored_New_Property { get; set; } = 2;
  }

Test case success 2 (serialize)

  string json = JsonSerializer.Serialize(new ConcreteDerivedClass());
  // Correctly returns "{\"Ignored_New_Property\":2}"
  
  public class ConcreteDerivedClass : AbstractBaseClass
  {
      [JsonIgnore]
      public new int Ignored_New_Property { get; set; } = 1;
  }
  
  public abstract class AbstractBaseClass
  {
      public int Ignored_New_Property { get; set; } = 2;
  }

Test case success 3 (deserialize)

  ConcreteDerivedClass obj = JsonSerializer.Deserialize<ConcreteDerivedClass>("{\"Ignored_New_Property\":42}");
  Debug.Assert(obj.Ignored_New_Property == 1);
  Debug.Assert(((AbstractBaseClass)obj).Ignored_New_Property == 42);

  public abstract class AbstractBaseClass
  {
      public int Ignored_New_Property { get; set; } = 2;
  }

  public class ConcreteDerivedClass : AbstractBaseClass
  {
      [JsonIgnore]
      public new virtual int Ignored_New_Property { get; set; } = 1;
  }

Test case success 4 (deserialize)

  ConcreteDerivedClass obj = JsonSerializer.Deserialize<ConcreteDerivedClass>("{\"Ignored_New_Property\":42}");
  Debug.Assert(obj.Ignored_New_Property == 42);
  Debug.Assert(((AbstractBaseClass)obj).Ignored_New_Property == 2);

  public abstract class AbstractBaseClass
  {
      [JsonIgnore]
      public int Ignored_New_Property { get; set; } = 2;
  }

  public class ConcreteDerivedClass : AbstractBaseClass
  {
      public new virtual int Ignored_New_Property { get; set; } = 1;
  }
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 27, 2021
@ghost
Copy link

ghost commented Sep 27, 2021

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

Issue Details

Description

For polymorphic scenarios, the new modifier does not always work with JsonIgnore. This affects both the reflection-based serializer and the source-gen serializer.

See also #58985 (comment)

Test case failure

  string json = JsonSerializer.Serialize(new ConcreteDerivedClass());
  // Throws System.ArgumentException: 'An item with the same key has already been added. Key: Ignored_New_Property'  

  public class ConcreteDerivedClass : AbstractBaseClass
  {
      [JsonIgnore]
      public new int Ignored_New_Property { get; set; } = 1;
  }

  public abstract class AbstractBaseClass
  {
      [JsonIgnore]
      public int Ignored_New_Property { get; set; } = 2;
  }

Test case success 1

  string json = JsonSerializer.Serialize(new ConcreteDerivedClass());
  // Correctly returns "{\"Ignored_New_Property\":1}"
  
  public class ConcreteDerivedClass : AbstractBaseClass
  {
      public new int Ignored_New_Property { get; set; } = 1;
  }
  
  public abstract class AbstractBaseClass
  {
      [JsonIgnore]
      public int Ignored_New_Property { get; set; } = 2;
  }

Test case success 2

  string json = JsonSerializer.Serialize(new ConcreteDerivedClass());
  // Correctly returns "{\"Ignored_New_Property\":2}"
  
  public class ConcreteDerivedClass : AbstractBaseClass
  {
      [JsonIgnore]
      public new int Ignored_New_Property { get; set; } = 1;
  }
  
  public abstract class AbstractBaseClass
  {
      public int Ignored_New_Property { get; set; } = 2;
  }
Author: steveharter
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

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

3 participants