From ea4baf58b9bc06c0958b5b4c0b80cf41fbd8de8a Mon Sep 17 00:00:00 2001 From: sbiot-aveva Date: Fri, 17 Jan 2025 01:50:44 -0500 Subject: [PATCH] Handle KeyNotFoundException thrown from DictionaryPropertyProxy and DictionaryTypedPropertyProxy (#40) * handle KeyNotFoundException thrown from DictionaryPropertyProxy and DictionaryTypedPropertyProxy * use TryGetValue in DictionaryTypedPropertyProxy * remove try-catch in DictionaryPropertyProxy * revert to stopping patch execution on patch error --- .../DictionaryIntegrationTest.cs | 186 ++++++++++++++---- .../TestObjectModels/NonGenericDictionary.cs | 41 ++++ .../Proxies/DictionaryPropertyProxy.cs | 14 +- .../Proxies/DictionaryTypedPropertyProxy.cs | 12 +- 4 files changed, 212 insertions(+), 41 deletions(-) create mode 100644 SystemTextJsonPatch.Tests/TestObjectModels/NonGenericDictionary.cs diff --git a/SystemTextJsonPatch.Tests/IntegrationTests/DictionaryIntegrationTest.cs b/SystemTextJsonPatch.Tests/IntegrationTests/DictionaryIntegrationTest.cs index 64717f3..cd85b55 100644 --- a/SystemTextJsonPatch.Tests/IntegrationTests/DictionaryIntegrationTest.cs +++ b/SystemTextJsonPatch.Tests/IntegrationTests/DictionaryIntegrationTest.cs @@ -1,7 +1,9 @@ -using System.Collections.Generic; +using System.Collections; +using System.Collections.Generic; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; using SystemTextJsonPatch.Exceptions; +using SystemTextJsonPatch.Operations; using Xunit; namespace SystemTextJsonPatch.IntegrationTests; @@ -130,11 +132,13 @@ private class Address private class IntDictionary { public IDictionary DictionaryOfStringToInteger { get; } = new Dictionary(); + public IDictionary NonGenericDictionary { get; } = new NonGenericDictionary(); } private class CustomerDictionary { - public IDictionary DictionaryOfStringToCustomer { get; } = new Dictionary(); + public IDictionary DictionaryOfIntegerToCustomer { get; } = new Dictionary(); + public IDictionary NonGenericDictionary { get; } = new NonGenericDictionary(); } #if NET7_0_OR_GREATER @@ -165,9 +169,9 @@ public void TestPocoObjectSucceeds() var key1 = 100; var value1 = new Customer() { Name = "James" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key1] = value1; var patchDocument = new JsonPatchDocument(); - patchDocument.Test($"/DictionaryOfStringToCustomer/{key1}/Name", "James"); + patchDocument.Test($"/DictionaryOfIntegerToCustomer/{key1}/Name", "James"); // Act & Assert patchDocument.ApplyTo(model); @@ -180,9 +184,9 @@ public void TestPocoObjectFailsWhenTestValueIsNotEqualToObjectValue() var key1 = 100; var value1 = new Customer() { Name = "James" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key1] = value1; var patchDocument = new JsonPatchDocument(); - patchDocument.Test($"/DictionaryOfStringToCustomer/{key1}/Name", "Mike"); + patchDocument.Test($"/DictionaryOfIntegerToCustomer/{key1}/Name", "Mike"); // Act var exception = Assert.Throws(() => { patchDocument.ApplyTo(model); }); @@ -200,19 +204,19 @@ public void AddPocoObjectSucceeds() var key2 = 200; var value2 = new Customer() { Name = "Mike" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key1] = value1; var patchDocument = new JsonPatchDocument(); - patchDocument.Add($"/DictionaryOfStringToCustomer/{key2}", value2); + patchDocument.Add($"/DictionaryOfIntegerToCustomer/{key2}", value2); // Act patchDocument.ApplyTo(model); // Assert - Assert.Equal(2, model.DictionaryOfStringToCustomer.Count); - var actualValue1 = model.DictionaryOfStringToCustomer[key1]; + Assert.Equal(2, model.DictionaryOfIntegerToCustomer.Count); + var actualValue1 = model.DictionaryOfIntegerToCustomer[key1]; Assert.NotNull(actualValue1); Assert.Equal("James", actualValue1.Name); - var actualValue2 = model.DictionaryOfStringToCustomer[key2]; + var actualValue2 = model.DictionaryOfIntegerToCustomer[key2]; Assert.NotNull(actualValue2); Assert.Equal("Mike", actualValue2.Name); } @@ -226,17 +230,17 @@ public void AddReplacesPocoObjectSucceeds() var key2 = 200; var value2 = new Customer() { Name = "Mike" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; - model.DictionaryOfStringToCustomer[key2] = value2; + model.DictionaryOfIntegerToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key2] = value2; var patchDocument = new JsonPatchDocument(); - patchDocument.Add($"/DictionaryOfStringToCustomer/{key1}/Name", "James"); + patchDocument.Add($"/DictionaryOfIntegerToCustomer/{key1}/Name", "James"); // Act patchDocument.ApplyTo(model); // Assert - Assert.Equal(2, model.DictionaryOfStringToCustomer.Count); - var actualValue1 = model.DictionaryOfStringToCustomer[key1]; + Assert.Equal(2, model.DictionaryOfIntegerToCustomer.Count); + var actualValue1 = model.DictionaryOfIntegerToCustomer[key1]; Assert.NotNull(actualValue1); Assert.Equal("James", actualValue1.Name); } @@ -271,16 +275,16 @@ public void RemovePocoObjectSucceeds() var key2 = 200; var value2 = new Customer() { Name = "Mike" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; - model.DictionaryOfStringToCustomer[key2] = value2; + model.DictionaryOfIntegerToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key2] = value2; var patchDocument = new JsonPatchDocument(); - patchDocument.Remove($"/DictionaryOfStringToCustomer/{key1}/Name"); + patchDocument.Remove($"/DictionaryOfIntegerToCustomer/{key1}/Name"); // Act patchDocument.ApplyTo(model); // Assert - var actualValue1 = model.DictionaryOfStringToCustomer[key1]; + var actualValue1 = model.DictionaryOfIntegerToCustomer[key1]; Assert.Null(actualValue1.Name); } @@ -293,16 +297,16 @@ public void MovePocoObjectSucceeds() var key2 = 200; var value2 = new Customer() { Name = "Mike" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; - model.DictionaryOfStringToCustomer[key2] = value2; + model.DictionaryOfIntegerToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key2] = value2; var patchDocument = new JsonPatchDocument(); - patchDocument.Move($"/DictionaryOfStringToCustomer/{key1}/Name", $"/DictionaryOfStringToCustomer/{key2}/Name"); + patchDocument.Move($"/DictionaryOfIntegerToCustomer/{key1}/Name", $"/DictionaryOfIntegerToCustomer/{key2}/Name"); // Act patchDocument.ApplyTo(model); // Assert - var actualValue2 = model.DictionaryOfStringToCustomer[key2]; + var actualValue2 = model.DictionaryOfIntegerToCustomer[key2]; Assert.NotNull(actualValue2); Assert.Equal("James", actualValue2.Name); } @@ -316,17 +320,17 @@ public void CopyPocoObjectSucceeds() var key2 = 200; var value2 = new Customer() { Name = "Mike" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; - model.DictionaryOfStringToCustomer[key2] = value2; + model.DictionaryOfIntegerToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key2] = value2; var patchDocument = new JsonPatchDocument(); - patchDocument.Copy($"/DictionaryOfStringToCustomer/{key1}/Name", $"/DictionaryOfStringToCustomer/{key2}/Name"); + patchDocument.Copy($"/DictionaryOfIntegerToCustomer/{key1}/Name", $"/DictionaryOfIntegerToCustomer/{key2}/Name"); // Act patchDocument.ApplyTo(model); // Assert - Assert.Equal(2, model.DictionaryOfStringToCustomer.Count); - var actualValue2 = model.DictionaryOfStringToCustomer[key2]; + Assert.Equal(2, model.DictionaryOfIntegerToCustomer.Count); + var actualValue2 = model.DictionaryOfIntegerToCustomer[key2]; Assert.NotNull(actualValue2); Assert.Equal("James", actualValue2.Name); } @@ -340,17 +344,17 @@ public void ReplacePocoObjectSucceeds() var key2 = 200; var value2 = new Customer() { Name = "Mike" }; var model = new CustomerDictionary(); - model.DictionaryOfStringToCustomer[key1] = value1; - model.DictionaryOfStringToCustomer[key2] = value2; + model.DictionaryOfIntegerToCustomer[key1] = value1; + model.DictionaryOfIntegerToCustomer[key2] = value2; var patchDocument = new JsonPatchDocument(); - patchDocument.Replace($"/DictionaryOfStringToCustomer/{key1}/Name", "James"); + patchDocument.Replace($"/DictionaryOfIntegerToCustomer/{key1}/Name", "James"); // Act patchDocument.ApplyTo(model); // Assert - Assert.Equal(2, model.DictionaryOfStringToCustomer.Count); - var actualValue1 = model.DictionaryOfStringToCustomer[key1]; + Assert.Equal(2, model.DictionaryOfIntegerToCustomer.Count); + var actualValue1 = model.DictionaryOfIntegerToCustomer[key1]; Assert.NotNull(actualValue1); Assert.Equal("James", actualValue1.Name); } @@ -379,4 +383,118 @@ public void ReplacePocoObjectWithEscapingSucceeds() Assert.Equal(300, actualValue1); Assert.Equal(200, actualValue2); } + + [Theory] + [InlineData("test", "DictionaryOfStringToInteger")] + [InlineData("move", "DictionaryOfStringToInteger")] + [InlineData("copy", "DictionaryOfStringToInteger")] + [InlineData("test", "NonGenericDictionary")] + [InlineData("move", "NonGenericDictionary")] + [InlineData("copy", "NonGenericDictionary")] + public void ReadIntegerValueOfMissingKeyThrowsJsonPatchExceptionWithDefaultErrorReporter(string op, string dictionaryPropertyName) + { + // Arrange + var model = new IntDictionary(); + var missingKey = "eight"; + var operation = new Operation( + op, + path: $"/{dictionaryPropertyName}/{missingKey}", + from: $"/{dictionaryPropertyName}/{missingKey}", + value: 8); + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(operation); + + // Act + var exception = Assert.Throws(() => { patchDocument.ApplyTo(model); }); + + // Assert + Assert.Equal($"The target location specified by path segment '{missingKey}' was not found.", exception.Message); + } + + [Theory] + [InlineData("test", "DictionaryOfStringToInteger")] + [InlineData("move", "DictionaryOfStringToInteger")] + [InlineData("copy", "DictionaryOfStringToInteger")] + [InlineData("test", "NonGenericDictionary")] + [InlineData("move", "NonGenericDictionary")] + [InlineData("copy", "NonGenericDictionary")] + public void ReadIntegerValueOfMissingKeyDoesNotThrowExceptionWithCustomErrorReporter(string op, string dictionaryPropertyName) + { + // Arrange + var patchErrorLogger = new TestErrorLogger(); + var model = new IntDictionary(); + var missingKey = "eight"; + var operation = new Operation( + op, + path: $"/{dictionaryPropertyName}/{missingKey}", + from: $"/{dictionaryPropertyName}/{missingKey}", + value: 8); + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(operation); + + // Act + patchDocument.ApplyTo(model, patchErrorLogger.LogErrorMessage); + + // Assert + Assert.Equal($"The target location specified by path segment '{missingKey}' was not found.", patchErrorLogger.ErrorMessage); + } + + [Theory] + [InlineData("test", "DictionaryOfIntegerToCustomer")] + [InlineData("move", "DictionaryOfIntegerToCustomer")] + [InlineData("copy", "DictionaryOfIntegerToCustomer")] + [InlineData("test", "NonGenericDictionary")] + [InlineData("move", "NonGenericDictionary")] + [InlineData("copy", "NonGenericDictionary")] + public void ReadPocoObjectValueOfMissingKeyThrowsJsonPatchExceptionWithDefaultErrorReporter(string op, string dictionaryPropertyName) + { + // Arrange + var model = new CustomerDictionary(); + var missingKey = 8; + var operation = new Operation( + op, + path: $"/{dictionaryPropertyName}/{missingKey}/Address/City", + from: $"/{dictionaryPropertyName}/{missingKey}/Address/City", + value: "Nowhere"); + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(operation); + + // Act + var exception = Assert.Throws(() => { patchDocument.ApplyTo(model); }); + + // Assert + Assert.Equal($"The target location specified by path segment '{missingKey}' was not found.", exception.Message); + } + + [Theory] + [InlineData("test", "DictionaryOfIntegerToCustomer")] + [InlineData("move", "DictionaryOfIntegerToCustomer")] + [InlineData("copy", "DictionaryOfIntegerToCustomer")] + [InlineData("test", "NonGenericDictionary")] + [InlineData("move", "NonGenericDictionary")] + [InlineData("copy", "NonGenericDictionary")] + public void ReadPocoObjectValueOfMissingKeyDoesNotThrowExceptionWithCustomErrorReporter(string op, string dictionaryPropertyName) + { + // Arrange + var patchErrorLogger = new TestErrorLogger(); + var model = new CustomerDictionary(); + var missingKey = 8; + var operation = new Operation( + op, + path: $"/{dictionaryPropertyName}/{missingKey}/Address/City", + from: $"/{dictionaryPropertyName}/{missingKey}/Address/City", + value: "Nowhere"); + + var patchDocument = new JsonPatchDocument(); + patchDocument.Operations.Add(operation); + + // Act + patchDocument.ApplyTo(model, patchErrorLogger.LogErrorMessage); + + // Assert + Assert.Equal($"The target location specified by path segment '{missingKey}' was not found.", patchErrorLogger.ErrorMessage); + } } diff --git a/SystemTextJsonPatch.Tests/TestObjectModels/NonGenericDictionary.cs b/SystemTextJsonPatch.Tests/TestObjectModels/NonGenericDictionary.cs new file mode 100644 index 0000000..819114e --- /dev/null +++ b/SystemTextJsonPatch.Tests/TestObjectModels/NonGenericDictionary.cs @@ -0,0 +1,41 @@ +using System; +using System.Collections; +using System.Collections.Generic; + +namespace SystemTextJsonPatch.IntegrationTests; + +public class NonGenericDictionary : IDictionary +{ + private readonly Dictionary _dictionary = new(); + + public object this[object key] { get => _dictionary[key]; set => _dictionary[key] = value; } + + public bool IsFixedSize => false; + + public bool IsReadOnly => false; + + public ICollection Keys => _dictionary.Keys; + + public ICollection Values => _dictionary.Values; + + public int Count => _dictionary.Count; + + public bool IsSynchronized => false; + + public object SyncRoot => null; + + public void Add(object key, object value) => _dictionary.Add(key, value); + + public void Clear() => _dictionary.Clear(); + + public bool Contains(object key) => _dictionary.ContainsKey(key); + + public void CopyTo(Array array, int index) => throw new NotImplementedException(); + + public IDictionaryEnumerator GetEnumerator() => _dictionary.GetEnumerator(); + + public void Remove(object key) => _dictionary.Remove(key); + + IEnumerator IEnumerable.GetEnumerator() => _dictionary.GetEnumerator(); +} + diff --git a/SystemTextJsonPatch/Internal/Proxies/DictionaryPropertyProxy.cs b/SystemTextJsonPatch/Internal/Proxies/DictionaryPropertyProxy.cs index 416fa03..b50b12b 100644 --- a/SystemTextJsonPatch/Internal/Proxies/DictionaryPropertyProxy.cs +++ b/SystemTextJsonPatch/Internal/Proxies/DictionaryPropertyProxy.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using SystemTextJsonPatch.Exceptions; namespace SystemTextJsonPatch.Internal.Proxies { @@ -40,9 +41,14 @@ internal DictionaryPropertyProxy(IDictionary dictionary, string propertyName) public object? GetValue(object target) { - var value = _dictionary[_propertyName]; - - return value; + if (_dictionary.Contains(_propertyName)) + { + return _dictionary[_propertyName]; + } + else + { + throw new JsonPatchException(Resources.FormatTargetLocationAtPathSegmentNotFound(_propertyName), null); + } } public void SetValue(object target, object? convertedValue) @@ -69,7 +75,7 @@ public Type PropertyType { get { - var val = _dictionary[_propertyName]; + var val = _dictionary.Contains(_propertyName) ? _dictionary[_propertyName] : null; if (val == null) { return typeof(object); diff --git a/SystemTextJsonPatch/Internal/Proxies/DictionaryTypedPropertyProxy.cs b/SystemTextJsonPatch/Internal/Proxies/DictionaryTypedPropertyProxy.cs index 5a22393..6a57a17 100644 --- a/SystemTextJsonPatch/Internal/Proxies/DictionaryTypedPropertyProxy.cs +++ b/SystemTextJsonPatch/Internal/Proxies/DictionaryTypedPropertyProxy.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using SystemTextJsonPatch.Exceptions; namespace SystemTextJsonPatch.Internal.Proxies { @@ -16,9 +17,14 @@ internal DictionaryTypedPropertyProxy(IDictionary dictionary, TKe public object? GetValue(object target) { - var value = _dictionary[_propertyName]; - - return value; + if (_dictionary.TryGetValue(_propertyName, out var value)) + { + return value; + } + else + { + throw new JsonPatchException(Resources.FormatTargetLocationAtPathSegmentNotFound(_propertyName), null); + } } public void SetValue(object target, object? convertedValue)