Skip to content

Commit

Permalink
Handle KeyNotFoundException thrown from DictionaryPropertyProxy and D…
Browse files Browse the repository at this point in the history
…ictionaryTypedPropertyProxy (#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
  • Loading branch information
sbiot-aveva authored Jan 17, 2025
1 parent b7cf008 commit ea4baf5
Show file tree
Hide file tree
Showing 4 changed files with 212 additions and 41 deletions.
186 changes: 152 additions & 34 deletions SystemTextJsonPatch.Tests/IntegrationTests/DictionaryIntegrationTest.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -130,11 +132,13 @@ private class Address
private class IntDictionary
{
public IDictionary<string, int> DictionaryOfStringToInteger { get; } = new Dictionary<string, int>();
public IDictionary NonGenericDictionary { get; } = new NonGenericDictionary();
}

private class CustomerDictionary
{
public IDictionary<int, Customer> DictionaryOfStringToCustomer { get; } = new Dictionary<int, Customer>();
public IDictionary<int, Customer> DictionaryOfIntegerToCustomer { get; } = new Dictionary<int, Customer>();
public IDictionary NonGenericDictionary { get; } = new NonGenericDictionary();
}

#if NET7_0_OR_GREATER
Expand Down Expand Up @@ -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);
Expand All @@ -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<JsonPatchTestOperationException>(() => { patchDocument.ApplyTo(model); });
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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<IntDictionary>(
op,
path: $"/{dictionaryPropertyName}/{missingKey}",
from: $"/{dictionaryPropertyName}/{missingKey}",
value: 8);

var patchDocument = new JsonPatchDocument<IntDictionary>();
patchDocument.Operations.Add(operation);

// Act
var exception = Assert.Throws<JsonPatchException>(() => { 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<DictionaryTest>();
var model = new IntDictionary();
var missingKey = "eight";
var operation = new Operation<IntDictionary>(
op,
path: $"/{dictionaryPropertyName}/{missingKey}",
from: $"/{dictionaryPropertyName}/{missingKey}",
value: 8);

var patchDocument = new JsonPatchDocument<IntDictionary>();
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<CustomerDictionary>(
op,
path: $"/{dictionaryPropertyName}/{missingKey}/Address/City",
from: $"/{dictionaryPropertyName}/{missingKey}/Address/City",
value: "Nowhere");

var patchDocument = new JsonPatchDocument<CustomerDictionary>();
patchDocument.Operations.Add(operation);

// Act
var exception = Assert.Throws<JsonPatchException>(() => { 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<DictionaryTest>();
var model = new CustomerDictionary();
var missingKey = 8;
var operation = new Operation<CustomerDictionary>(
op,
path: $"/{dictionaryPropertyName}/{missingKey}/Address/City",
from: $"/{dictionaryPropertyName}/{missingKey}/Address/City",
value: "Nowhere");

var patchDocument = new JsonPatchDocument<CustomerDictionary>();
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);
}
}
41 changes: 41 additions & 0 deletions SystemTextJsonPatch.Tests/TestObjectModels/NonGenericDictionary.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
using System;
using System.Collections;
using System.Collections.Generic;

namespace SystemTextJsonPatch.IntegrationTests;

public class NonGenericDictionary : IDictionary
{
private readonly Dictionary<object, object> _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();
}

14 changes: 10 additions & 4 deletions SystemTextJsonPatch/Internal/Proxies/DictionaryPropertyProxy.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections;
using SystemTextJsonPatch.Exceptions;

namespace SystemTextJsonPatch.Internal.Proxies
{
Expand Down Expand Up @@ -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)
Expand All @@ -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);
Expand Down
Loading

0 comments on commit ea4baf5

Please sign in to comment.