-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Fixed serialization of hidden base class members #32107
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
05639b9
Fixed serialization of hidden base class members
YohDeadfall a01bbe8
Added tests to check non-public props are ignored
YohDeadfall 827df7e
Simplified cache construction
YohDeadfall ed8ca2b
Addressed review issues
YohDeadfall c3d7228
Added requested tests
YohDeadfall 8f9653b
Fixed tests
YohDeadfall 335c9d0
Fixed handling of public hidden prop by new slot
YohDeadfall efd6ab9
Canged test naming style
YohDeadfall 9e42d7c
Covered by new slot hidden member serialization
YohDeadfall cd56b45
Fixed failing test
YohDeadfall 87b0c43
Added comment and improved test
YohDeadfall 33cd6d6
Fixed rebase issues
YohDeadfall 07fd692
Added more tests
YohDeadfall 304fb56
Fixed tests
YohDeadfall dc20062
Fixed tests
YohDeadfall d600e91
Fixed rebase issues
YohDeadfall File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Added requested tests
- Loading branch information
commit c3d7228d560f02c99e6da530373348ca692a9845
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -167,6 +167,90 @@ public static void Ignore_public_property_on_conflict_with_private_due_to_policy | |
| Assert.Equal("ConflictingValue", obj.myString); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void Throw_when_public_properties_conflict_due_to_attributes() | ||
| { | ||
| // Serialize | ||
| var obj = new ClassWithPropertyNamingConflictWhichThrows(); | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Serialize(obj)); | ||
|
|
||
| // Deserialize | ||
| string json = @"{""MyString"":""NewValue""}"; | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Deserialize<ClassWithPropertyNamingConflictWhichThrows>(json)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void Throw_when_public_properties_conflict_due_to_attributes_and_inheritance() | ||
| { | ||
| // Serialize | ||
| var obj = new ClassInheritedWithPropertyNamingConflictWhichThrows(); | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Serialize(obj)); | ||
|
|
||
| // Deserialize | ||
| string json = @"{""MyString"":""NewValue""}"; | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Deserialize<ClassInheritedWithPropertyNamingConflictWhichThrows>(json)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void Throw_when_public_properties_conflict_due_to_attributes_and_double_inheritance() | ||
| { | ||
| // Serialize | ||
| var obj = new ClassTwiceInheritedWithPropertyNamingConflictWhichThrows(); | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Serialize(obj)); | ||
|
|
||
| // Deserialize | ||
| string json = @"{""MyString"":""NewValue""}"; | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Deserialize<ClassTwiceInheritedWithPropertyNamingConflictWhichThrows>(json)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void Throw_when_public_properties_conflict_due_to_policy() | ||
| { | ||
| // Serialize | ||
| var obj = new ClassWithPropertyPolicyConflictWhichThrows(); | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Serialize(obj)); | ||
|
|
||
| // Deserialize | ||
| string json = @"{""MyString"":""NewValue""}"; | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Deserialize<ClassWithPropertyPolicyConflictWhichThrows>(json)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void Throw_when_public_properties_conflict_due_to_policy_and_inheritance() | ||
| { | ||
| // Serialize | ||
| var obj = new ClassInheritedWithPropertyPolicyConflictWhichThrows(); | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Serialize(obj)); | ||
|
|
||
| // Deserialize | ||
| string json = @"{""MyString"":""NewValue""}"; | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Deserialize<ClassInheritedWithPropertyPolicyConflictWhichThrows>(json)); | ||
| } | ||
|
|
||
| [Fact] | ||
| public static void Throw_when_public_properties_conflict_due_to_policy_and_double_inheritance() | ||
| { | ||
| // Serialize | ||
| var obj = new ClassTwiceInheritedWithPropertyPolicyConflictWhichThrows(); | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Serialize(obj)); | ||
|
|
||
| // Deserialize | ||
| string json = @"{""MyString"":""NewValue""}"; | ||
| Assert.Throws<InvalidOperationException>( | ||
| () => JsonSerializer.Deserialize<ClassTwiceInheritedWithPropertyPolicyConflictWhichThrows>(json)); | ||
| } | ||
|
|
||
| public class ClassWithInternalProperty | ||
| { | ||
| internal string MyString { get; set; } = "DefaultValue"; | ||
|
|
@@ -195,13 +279,59 @@ public class ClassWithPropertyNamingConflict | |
| internal string ConflictingString { get; set; } = "ConflictingValue"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need a class variant where this property is public; and a test variant where the conflict spans across two inheritance levels. |
||
| } | ||
|
|
||
| public class ClassWithPropertyNamingConflictWhichThrows | ||
| { | ||
| public string MyString { get; set; } = "DefaultValue"; | ||
|
|
||
| [JsonPropertyName(nameof(MyString))] | ||
| public string ConflictingString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassInheritedWithPropertyNamingConflictWhichThrows : ClassWithPublicProperty | ||
| { | ||
| [JsonPropertyName(nameof(MyString))] | ||
| public string ConflictingString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassTwiceInheritedWithPropertyNamingConflictWhichThrowsDummy : ClassWithPublicProperty | ||
| { | ||
|
|
||
| } | ||
YohDeadfall marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| public class ClassTwiceInheritedWithPropertyNamingConflictWhichThrows : ClassTwiceInheritedWithPropertyNamingConflictWhichThrowsDummy | ||
| { | ||
| [JsonPropertyName(nameof(MyString))] | ||
| public string ConflictingString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassWithPropertyPolicyConflict | ||
| { | ||
| public string MyString { get; set; } = "DefaultValue"; | ||
|
|
||
| internal string myString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassWithPropertyPolicyConflictWhichThrows | ||
| { | ||
| public string MyString { get; set; } = "DefaultValue"; | ||
|
|
||
| public string myString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassInheritedWithPropertyPolicyConflictWhichThrows : ClassWithPublicProperty | ||
| { | ||
| public string myString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassInheritedWithPropertyPolicyConflictWhichThrowsDummy : ClassWithPublicProperty | ||
| { | ||
| } | ||
YohDeadfall marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| public class ClassTwiceInheritedWithPropertyPolicyConflictWhichThrows : ClassInheritedWithPropertyPolicyConflictWhichThrowsDummy | ||
| { | ||
| public string myString { get; set; } = "ConflictingValue"; | ||
| } | ||
|
|
||
| public class ClassWithIgnoredNewSlotProperty : ClassWithInternalProperty | ||
| { | ||
| [JsonIgnore] | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.