From 3554f1cdacf88ba5d3fbe1dc952d87c34d498bb3 Mon Sep 17 00:00:00 2001 From: Havunen Date: Fri, 18 Oct 2024 19:34:21 +0300 Subject: [PATCH] breaking change: Changed property matching to follow more closely System.Text.Json naming policy. Fixes https://github.com/Havunen/SystemTextJsonPatch/issues/36 --- .../AnonymousObjectIntegrationTest.cs | 4 +- .../CustomConverterErrorMessagesTest.cs | 2 + .../IntegrationTests/DecimalComparisonTest.cs | 1 + .../IntegrationTests/ErrorMessageTest.cs | 1 + .../IntegrationTests/ListIntegrationTest.cs | 2 +- .../NestedObjectIntegrationTest.cs | 100 +++++++++++++++++- .../TestOperationIntegrationTests.cs | 7 +- .../PropertyNameCasingTestObject.cs | 21 ++++ SystemTextJsonPatch/Internal/PocoAdapter.cs | 2 +- .../Internal/PropertyProxyCache.cs | 19 ++-- 10 files changed, 145 insertions(+), 14 deletions(-) create mode 100644 SystemTextJsonPatch.Tests/TestObjectModels/PropertyNameCasingTestObject.cs diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/AnonymousObjectIntegrationTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/AnonymousObjectIntegrationTest.cs index 979d23c..562e404 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/AnonymousObjectIntegrationTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/AnonymousObjectIntegrationTest.cs @@ -1,4 +1,5 @@ -using SystemTextJsonPatch.Exceptions; +using System.Text.Json; +using SystemTextJsonPatch.Exceptions; using Xunit; namespace SystemTextJsonPatch.IntegrationTests; @@ -32,6 +33,7 @@ public void AddNewPropertyToNestedAnonymousObjectShouldFail() }; var patchDocument = new JsonPatchDocument(); + patchDocument.Options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; patchDocument.Add("Nested/NewInt", 1); // Act diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/CustomConverterErrorMessagesTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/CustomConverterErrorMessagesTest.cs index e184166..1681acc 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/CustomConverterErrorMessagesTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/CustomConverterErrorMessagesTest.cs @@ -36,6 +36,7 @@ public void JsonExceptionsFromCustomConvertersShouldBeShownAsIs() { var serializerOptions = new JsonSerializerOptions() { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, Converters = { new CustomJsonConverter() @@ -56,6 +57,7 @@ public void JsonExceptionsFromSystemTextJsonSerializerShouldNotBeShown() { var serializerOptions = new JsonSerializerOptions() { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, }; var model = new TestModel(); diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/DecimalComparisonTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/DecimalComparisonTest.cs index fffe6e9..ef4569a 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/DecimalComparisonTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/DecimalComparisonTest.cs @@ -26,6 +26,7 @@ public void TestValuesShouldBeEqualRegardlessOfNumberOfDecimalZeroes() }; var jsonOptions = new JsonSerializerOptions() { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, }; var incomingJson = JsonSerializer.Serialize(incomingOperations, jsonOptions); diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/ErrorMessageTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/ErrorMessageTest.cs index 9bb18a2..c623232 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/ErrorMessageTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/ErrorMessageTest.cs @@ -30,6 +30,7 @@ public void JsonPatchHasErrorNotUsedInOperationDateTime() var jsonOptions = new JsonSerializerOptions() { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, }; var incomingJson = JsonSerializer.Serialize(incomingOperations, jsonOptions); diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/ListIntegrationTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/ListIntegrationTest.cs index f9974d9..d1684dd 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/ListIntegrationTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/ListIntegrationTest.cs @@ -214,7 +214,7 @@ public void ReplaceFullGenericListWithCollection() } }; - var json = "[{\"op\":\"replace\",\"path\":\"/simpleobjectList\",\"value\":[{\"AnotherIntegerValue\": 6}]}]"; + var json = "[{\"op\":\"replace\",\"path\":\"/SimpleObjectList\",\"value\":[{\"AnotherIntegerValue\": 6}]}]"; var docJson = JsonSerializer.Deserialize>(json); diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/NestedObjectIntegrationTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/NestedObjectIntegrationTest.cs index 90e9b40..bfee8da 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/NestedObjectIntegrationTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/NestedObjectIntegrationTest.cs @@ -61,7 +61,7 @@ public void ReplaceNestedObjectWithSerialization() #if NET8_0 [Fact] - public void ReplaceNestedObjectWithPlainStrings() + public void SnakeCaseOpMatchesProperty() { // Arrange var targetObject = new SimpleObjectWithNestedObject() @@ -76,7 +76,7 @@ public void ReplaceNestedObjectWithPlainStrings() var newNested = new NestedObject { StringProperty = "B" }; var patchDocument = new JsonPatchDocument(); - patchDocument.Operations.Add(new Operation("replace", "/nested_object", JsonSerializer.Serialize(newNested, options))); + patchDocument.Operations.Add(new Operation("replace", "/nested_object", null, newNested)); patchDocument.Options = options; var serialized = JsonSerializer.Serialize(patchDocument, options); @@ -88,9 +88,103 @@ public void ReplaceNestedObjectWithPlainStrings() // Assert Assert.Equal("B", targetObject.NestedObject.StringProperty); } - + #endif + [Fact] + public void CamelCaseOpMatchesExactProperty() + { + // Arrange + var targetObject = new NameCasingTestObject() + { + PropertyName = 1, // This is before propertyName and camelCase matches it so it is used. + propertyName = 1 + }; + var options = new JsonSerializerOptions() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase + }; + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(new Operation("replace", "/propertyName", null, 2)); + patchDocument.Options = options; + + var serialized = JsonSerializer.Serialize(patchDocument, options); + var deserialized = JsonSerializer.Deserialize>(serialized, options); + + // Act + deserialized.ApplyTo(targetObject); + + // Assert + Assert.Equal(2, targetObject.PropertyName); + Assert.Equal(1, targetObject.propertyName); + } + + [Fact] + public void MatchesExactProperty() + { + // Arrange + var targetObject = new NameCasingTestObject() + { + PropertyName = 1, + propertyName = 1 // exact match + }; + var options = new JsonSerializerOptions() + { + }; + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(new Operation("replace", "/propertyName", null, 2)); + patchDocument.Options = options; + + var serialized = JsonSerializer.Serialize(patchDocument, options); + var deserialized = JsonSerializer.Deserialize>(serialized, options); + + // Act + deserialized.ApplyTo(targetObject); + + // Assert + Assert.Equal(1, targetObject.PropertyName); + Assert.Equal(2, targetObject.propertyName); + } + + [Fact] + public void MatchesExactPropertyTestCache() + { + CamelCaseOpMatchesExactPropertyAttributeOverride(); + CamelCaseOpMatchesExactProperty(); + MatchesExactProperty(); + } + + [Fact] + public void CamelCaseOpMatchesExactPropertyAttributeOverride() + { + // Arrange + var targetObject = new AttrNameCasingTestObject() + { + PropertyName = 1, + propertyName = 1 + }; + var options = new JsonSerializerOptions() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase + }; + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(new Operation("replace", "/propertyName", null, 2)); + patchDocument.Options = options; + + var serialized = JsonSerializer.Serialize(patchDocument, options); + var deserialized = JsonSerializer.Deserialize>(serialized, options); + + // Act + deserialized.ApplyTo(targetObject); + + // Assert + Assert.Equal(2, targetObject.PropertyName); // attribute takes precedence + Assert.Equal(1, targetObject.propertyName); + } + [Fact] public void TestStringPropertyInNestedObject() { diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/TestOperationIntegrationTests.cs b/SystemTextJsonPatch.Tests/IntegrationTests/TestOperationIntegrationTests.cs index 05b4eb7..d8fe2dc 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/TestOperationIntegrationTests.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/TestOperationIntegrationTests.cs @@ -1,4 +1,5 @@ -using System.Text.Json.Nodes; +using System.Text.Json; +using System.Text.Json.Nodes; using SystemTextJsonPatch.Operations; using Xunit; @@ -10,6 +11,7 @@ public class TestOperationIntegrationTests public void EmptyStringTest_Dto() { var patchDocument = new JsonPatchDocument(); + patchDocument.Options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; patchDocument.Operations.Add(new Operation(op: "test", path: "/string", from: null, value: "")); var node = new TestClass() { String = "" }; @@ -20,6 +22,7 @@ public void EmptyStringTest_Dto() public void NullFieldTest_Dto() { var patchDocument = new JsonPatchDocument(); + patchDocument.Options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; patchDocument.Operations.Add(new Operation(op: "test", path: "/string", from: null, value: null)); var node = new TestClass() { String = null }; @@ -30,6 +33,7 @@ public void NullFieldTest_Dto() public void EmptyStringTest_Json() { var patchDocument = new JsonPatchDocument(); + patchDocument.Options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; patchDocument.Operations.Add(new Operation(op: "test", path: "/string", from: null, value: "")); var node = JsonNode.Parse("{\"string\": \"\"}")!; patchDocument.ApplyTo(node); @@ -39,6 +43,7 @@ public void EmptyStringTest_Json() public void NullFieldTest_Json() { var patchDocument = new JsonPatchDocument(); + patchDocument.Options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase; patchDocument.Operations.Add(new Operation(op: "test", path: "/string", from: null, value: null)); var node = JsonNode.Parse("{\"string\": null}")!; patchDocument.ApplyTo(node); diff --git a/SystemTextJsonPatch.Tests/TestObjectModels/PropertyNameCasingTestObject.cs b/SystemTextJsonPatch.Tests/TestObjectModels/PropertyNameCasingTestObject.cs new file mode 100644 index 0000000..b043283 --- /dev/null +++ b/SystemTextJsonPatch.Tests/TestObjectModels/PropertyNameCasingTestObject.cs @@ -0,0 +1,21 @@ +using System.Collections.Generic; +using System.Text.Json.Serialization; + +namespace SystemTextJsonPatch; + +public class NameCasingTestObject +{ + public int PropertyName { get; set; } + + public int propertyName { get; set; } + +} + +public class AttrNameCasingTestObject +{ + [JsonPropertyName("propertyName")] + public int PropertyName { get; set; } + + public int propertyName { get; set; } + +} \ No newline at end of file diff --git a/SystemTextJsonPatch/Internal/PocoAdapter.cs b/SystemTextJsonPatch/Internal/PocoAdapter.cs index 887adf2..3193de1 100644 --- a/SystemTextJsonPatch/Internal/PocoAdapter.cs +++ b/SystemTextJsonPatch/Internal/PocoAdapter.cs @@ -225,6 +225,6 @@ private static bool TryGetJsonProperty(object target, string segment, JsonSerial } - return PropertyProxyCache.GetPropertyProxy(target.GetType(), propertyName); + return PropertyProxyCache.GetPropertyProxy(target.GetType(), propertyName, options.PropertyNamingPolicy); } } diff --git a/SystemTextJsonPatch/Internal/PropertyProxyCache.cs b/SystemTextJsonPatch/Internal/PropertyProxyCache.cs index 08bedd6..1c2ed11 100644 --- a/SystemTextJsonPatch/Internal/PropertyProxyCache.cs +++ b/SystemTextJsonPatch/Internal/PropertyProxyCache.cs @@ -1,7 +1,9 @@ using System; using System.Collections.Concurrent; using System.Reflection; +using System.Text.Json; using System.Text.Json.Serialization; +using System.Xml.Linq; using SystemTextJsonPatch.Exceptions; using SystemTextJsonPatch.Internal.Proxies; @@ -10,11 +12,12 @@ namespace SystemTextJsonPatch.Internal internal static class PropertyProxyCache { private static readonly ConcurrentDictionary CachedTypeProperties = new(); - private static readonly ConcurrentDictionary<(Type, string), PropertyProxy?> CachedPropertyProxies = new(); + // Naming policy has to be part of the key because it can change the target property + private static readonly ConcurrentDictionary<(Type, string, JsonNamingPolicy?), PropertyProxy?> CachedPropertyProxies = new(); - internal static PropertyProxy? GetPropertyProxy(Type type, string propName) + internal static PropertyProxy? GetPropertyProxy(Type type, string propName, JsonNamingPolicy? namingPolicy) { - var key = (type, propName); + var key = (type, propName, namingPolicy); if (CachedPropertyProxies.TryGetValue(key, out var propertyProxy)) { @@ -27,19 +30,19 @@ internal static class PropertyProxyCache CachedTypeProperties[type] = properties; } - propertyProxy = FindPropertyInfo(properties, propName); + propertyProxy = FindPropertyInfo(properties, propName, namingPolicy); CachedPropertyProxies[key] = propertyProxy; return propertyProxy; } - private static PropertyProxy? FindPropertyInfo(PropertyInfo[] properties, string propName) + private static PropertyProxy? FindPropertyInfo(PropertyInfo[] properties, string propName, JsonNamingPolicy? namingPolicy) { // First check through all properties if property name matches JsonPropertyNameAttribute foreach (var propertyInfo in properties) { var jsonPropertyNameAttr = propertyInfo.GetCustomAttribute(); - if (jsonPropertyNameAttr != null && string.Equals(jsonPropertyNameAttr.Name, propName, StringComparison.OrdinalIgnoreCase)) + if (jsonPropertyNameAttr != null && string.Equals(jsonPropertyNameAttr.Name, propName, StringComparison.Ordinal)) { EnsureAccessToProperty(propertyInfo); return new PropertyProxy(propertyInfo); @@ -49,7 +52,9 @@ internal static class PropertyProxyCache // If it didn't find match by JsonPropertyName then use property name foreach (var propertyInfo in properties) { - if (string.Equals(propertyInfo.Name, propName, StringComparison.OrdinalIgnoreCase)) + var propertyName = namingPolicy != null ? namingPolicy.ConvertName(propertyInfo.Name) : propertyInfo.Name; + + if (string.Equals(propertyName, propName, StringComparison.Ordinal)) { EnsureAccessToProperty(propertyInfo); return new PropertyProxy(propertyInfo);