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 IgnoreReadOnlyProperties not working for enumerables #80113

Open
npp127 opened this issue Jan 3, 2023 · 7 comments
Open

System.Text.Json IgnoreReadOnlyProperties not working for enumerables #80113

npp127 opened this issue Jan 3, 2023 · 7 comments
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@npp127
Copy link

npp127 commented Jan 3, 2023

Description

Given a class

    public class Test
    {
        public int MyProperty1 { get; set; } = 69;

        public int MyProperty2 { get; } = 420;

        public IEnumerable<int> Collection1 { get; set; } = new int[] { 69, 420 };

        public IEnumerable<int> Collection2 { get; } = new int[] { 69, 420 };
    };

by setting IgnoreReadOnlyProperties to true, one would expect that only "MyProperty1" and "Collection1" would be serialized.
Collection2 Gets also serialized for whatever reason.

Reproduction Steps

Test class

namespace T
{
    public class Test
    {
        public int MyProperty1 { get; set; } = 69;
        public int MyProperty2 { get; } = 420;

        public IEnumerable<int> Collection1 { get; set; } = new int[] { 69, 420 };

        public IEnumerable<int> Collection2 { get; } = new int[] { 69, 420 };
    };
}

Program.cs

using System.Text.Json;
using T;

var opt = new JsonSerializerOptions()
{
    IgnoreReadOnlyProperties = true,
    WriteIndented = true,
};


var obj = new Test();

var json =  JsonSerializer.Serialize(obj, opt);

Console.WriteLine(json);

Csproj

<Project Sdk="Microsoft.NET.Sdk">
	<PropertyGroup>
		<OutputType>Exe</OutputType>
		<TargetFramework>net7.0</TargetFramework>
		<ImplicitUsings>enable</ImplicitUsings>
		<Nullable>enable</Nullable>
		<LangVersion>latest</LangVersion>
	</PropertyGroup>
</Project>

Expected behavior

{
  "MyProperty1": 69,
  "Collection1": [
    69,
    420
  ]
}

Actual behavior

{
  "MyProperty1": 69,
  "Collection1": [
    69,
    420
  ],
  "Collection2": [
    69,
    420
  ]
}

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 3, 2023
@ghost
Copy link

ghost commented Jan 3, 2023

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

Issue Details

Description

Given a class

    public class Test
    {
        public int MyProperty1 { get; set; } = 69;

        public int MyProperty2 { get; } = 420;

        public IEnumerable<int> Collection1 { get; set; } = new int[] { 69, 420 };

        public IEnumerable<int> Collection2 { get; } = new int[] { 69, 420 };
    };

by setting IgnoreReadOnlyProperties to true, one would expect that only "MyProperty1" and "Collection1" would be serialized.
Collection2 Gets also serialized for whatever reason.

Reproduction Steps

Test class

namespace T
{
    public class Test
    {
        public int MyProperty1 { get; set; } = 69;
        public int MyProperty2 { get; } = 420;

        public IEnumerable<int> Collection1 { get; set; } = new int[] { 69, 420 };

        public IEnumerable<int> Collection2 { get; } = new int[] { 69, 420 };
    };
}

Program.cs

using System.Text.Json;
using T;

var opt = new JsonSerializerOptions()
{
    IgnoreReadOnlyProperties = true,
    WriteIndented = true,
};


var obj = new Test();

var json =  JsonSerializer.Serialize(obj, opt);

Console.WriteLine(json);

Expected behavior

{
  "MyProperty1": 69,
  "Collection1": [
    69,
    420
  ]
}

Actual behavior

{
  "MyProperty1": 69,
  "Collection1": [
    69,
    420
  ],
  "Collection2": [
    69,
    420
  ]
}

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: npp127
Assignees: -
Labels:

area-System.Text.Json

Milestone: -

@npp127
Copy link
Author

npp127 commented Jan 3, 2023

it's like that by design for enumerables and dictionaries

if ((EffectiveConverter.ConverterStrategy & (ConverterStrategy.Enumerable | ConverterStrategy.Dictionary)) != 0)
{
// Properties of collections types that only have setters are not supported.
if (Get == null && Set != null && !_isUserSpecifiedSetter)
{
CanDeserialize = false;
}
}
else
{
// For read-only properties of non-collection types, apply IgnoreReadOnlyProperties/Fields policy,
// unless a `ShouldSerialize` predicate has been explicitly applied by the user (null or non-null).
if (Get != null && Set == null && IgnoreReadOnlyMember && !_isUserSpecifiedShouldSerialize)
{
CanSerialize = false;
}
}

Properties of collections types that only have setters are not supported.

But should they?

I'm missing whats the point of the reasoning for not supporting that.
Something like .IgnoreReadOnlyEnumerables with false as a default would allow users to avoid this behaviour

@npp127 npp127 closed this as completed Jan 3, 2023
@npp127 npp127 reopened this Jan 3, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 3, 2023
@eiriktsarpalis eiriktsarpalis added the untriaged New issue has not been triaged by the area owner label Jan 10, 2023
@krwq krwq added this to the 8.0.0 milestone Jan 17, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 17, 2023
@krwq krwq added bug untriaged New issue has not been triaged by the area owner and removed untriaged New issue has not been triaged by the area owner labels Jan 17, 2023
@krwq
Copy link
Member

krwq commented Jan 17, 2023

I'm marking this as 8.0 for now because we should define behavior for this in #78556 and they should be looked at hollistically

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 17, 2023

Duplicate of #67514 and #37599. IIUC this is intentional behavior and collections are treated specially w.r.t. this flag. Changing this now would be a breaking change. We might consider solving this using a different flag that supersedes IgnoreReadOnlyProperties.

@eiriktsarpalis eiriktsarpalis added enhancement Product code improvement that does NOT require public API changes/additions and removed bug labels Jan 17, 2023
@npp127
Copy link
Author

npp127 commented Jan 17, 2023

Duplicate of #67514 and #37599. IIUC this is intentional behavior and collections are treated specially w.r.t. this flag. Changing this now would be a breaking change. We might consider solving this using a different flag that supersedes IgnoreReadOnlyProperties.

Shouldn't this behaviour be made configurable from the options?

@eiriktsarpalis eiriktsarpalis modified the milestones: 8.0.0, Future Jun 16, 2023
@eiriktsarpalis
Copy link
Member

Moving to Future.

@wiz0u
Copy link

wiz0u commented Dec 16, 2023

Same for getter-only arrays. They get serialized, even though they are obviously not deserializable (can't set or resize the array)

My test program:

using System.Text.Json;
 
var test = new Test();
Console.WriteLine(JsonSerializer.Serialize(test, new JsonSerializerOptions { IgnoreReadOnlyProperties = true }));

public partial class Test
{
	public int Id => 123;
	public int[] SomeArrayProp => new[] { 456 };
	public IEnumerable<int> SomeIEnumProp { get { yield return 789; } }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants