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

[JsonSerializer] Add test for JsonIncludeAttribute + parameterized ctor deserialization #47855

Closed
layomia opened this issue Feb 4, 2021 · 2 comments
Labels
area-System.Text.Json good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors test-enhancement Improvements of test source code
Milestone

Comments

@layomia
Copy link
Contributor

layomia commented Feb 4, 2021

We should add a test for what happens when a constructor parameter is supposed to bind with a property that would normally not be included for deserialization, but is included via [JsonInclude]. Deserialization should proceed as expected.

public class MyClass
{
    [JsonInclude]
    public int MyInt { get; private set; }

    public MyClass(int myInt) => MyInt = myInt;
}

We should also test that JsonNumberHandlingAttribute is honored.

JsonConverterAttribute, JsonIgnoreAttribute, and JsonPropertyNameAttribute are tested and known to be honored in this scenario.


Edit: the above test should work today with and without the attribute. I believe it becomes more applicable when we have support for non-public members #31511:

public class MyClass
{
    [JsonInclude]
    private int MyInt { get; set; }

    public MyClass(int myInt) => MyInt = myInt;
}
@layomia layomia added area-System.Text.Json good first issue Issue should be easy to implement, good for first-time contributors test-enhancement Improvements of test source code help wanted [up-for-grabs] Good issue for external contributors labels Feb 4, 2021
@layomia layomia added this to the 6.0.0 milestone Feb 4, 2021
@ghost
Copy link

ghost commented Feb 4, 2021

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

Issue Details

We should add a test for what happens when a constructor parameter is supposed to bind with a property that would normally not be included for deserialization, but is included via [JsonInclude]. Deserialization should proceed as expected.

public class MyClass
{
    [JsonInclude]
    public int MyInt { get; private set; }

    public MyClass(int myInt) => MyInt = myInt;
}

We should also test that JsonNumberHandlingAttribute is honored.

JsonIgnoreAttribute and JsonPropertyNameAttribute are tested and known to be honored in this scenario.

Author: layomia
Assignees: -
Labels:

area-System.Text.Json, easy, test enhancement, up-for-grabs

Milestone: 6.0.0

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Feb 4, 2021
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Feb 4, 2021
SkiFoD added a commit to SkiFoD/runtime that referenced this issue Feb 5, 2021
ArgumentDeserialization_Honors_JsonInclude test is added dotnet#47855
SkiFoD added a commit to SkiFoD/runtime that referenced this issue Feb 5, 2021
…zation

ArgumentDeserialization_Honors_JsonNumberHandling test is added dotnet#47855
layomia pushed a commit that referenced this issue Mar 3, 2021
…re honored during deserialization (#47904)

* New test that JsonIncludeAttribute is honored during deserialization

ArgumentDeserialization_Honors_JsonInclude test is added #47855

* New test that JsonNumberHandlingAttribute is honored during deserialization

ArgumentDeserialization_Honors_JsonNumberHandling test is added #47855

* Extended Point_MembersHave_JsonInclude class for variety of test cases

Added new lines
@layomia
Copy link
Contributor Author

layomia commented Mar 4, 2021

Fixed by #47904.

@layomia layomia closed this as completed Mar 4, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors test-enhancement Improvements of test source code
Projects
None yet
Development

No branches or pull requests

1 participant