From 7a487a880a6525ced73b2e3d658c8e20b6578afb Mon Sep 17 00:00:00 2001 From: Kiran Challa Date: Mon, 23 Jan 2017 12:00:42 -0800 Subject: [PATCH] [Fixes #48] Regression: List add must support adding items where index is same as count of elements --- .../Internal/ListAdapter.cs | 35 ++++-- .../ListAdapterTest.cs | 110 +++++++++++++++++- 2 files changed, 136 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.AspNetCore.JsonPatch/Internal/ListAdapter.cs b/src/Microsoft.AspNetCore.JsonPatch/Internal/ListAdapter.cs index c3da14fe5a..343712690a 100644 --- a/src/Microsoft.AspNetCore.JsonPatch/Internal/ListAdapter.cs +++ b/src/Microsoft.AspNetCore.JsonPatch/Internal/ListAdapter.cs @@ -27,7 +27,7 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal } PositionInfo positionInfo; - if (!TryGetPositionInfo(list, segment, out positionInfo, out errorMessage)) + if (!TryGetPositionInfo(list, segment, OperationType.Add, out positionInfo, out errorMessage)) { return false; } @@ -68,7 +68,7 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal } PositionInfo positionInfo; - if (!TryGetPositionInfo(list, segment, out positionInfo, out errorMessage)) + if (!TryGetPositionInfo(list, segment, OperationType.Get, out positionInfo, out errorMessage)) { value = null; return false; @@ -102,7 +102,7 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal } PositionInfo positionInfo; - if (!TryGetPositionInfo(list, segment, out positionInfo, out errorMessage)) + if (!TryGetPositionInfo(list, segment, OperationType.Remove, out positionInfo, out errorMessage)) { return false; } @@ -136,7 +136,7 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal } PositionInfo positionInfo; - if (!TryGetPositionInfo(list, segment, out positionInfo, out errorMessage)) + if (!TryGetPositionInfo(list, segment, OperationType.Replace, out positionInfo, out errorMessage)) { return false; } @@ -243,7 +243,12 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal } } - private bool TryGetPositionInfo(IList list, string segment, out PositionInfo positionInfo, out string errorMessage) + private bool TryGetPositionInfo( + IList list, + string segment, + OperationType operationType, + out PositionInfo positionInfo, + out string errorMessage) { if (segment == "-") { @@ -261,16 +266,24 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal errorMessage = null; return true; } + // As per JSON Patch spec, for Add operation the index value representing the number of elements is valid, + // where as for other operations like Remove, Replace, Move and Copy the target index MUST exist. + else if (position == list.Count && operationType == OperationType.Add) + { + positionInfo = new PositionInfo(PositionType.EndOfList, -1); + errorMessage = null; + return true; + } else { - positionInfo = default(PositionInfo); + positionInfo = new PositionInfo(PositionType.OutOfBounds, position); errorMessage = Resources.FormatIndexOutOfBounds(segment); return false; } } else { - positionInfo = default(PositionInfo); + positionInfo = new PositionInfo(PositionType.Invalid, -1); errorMessage = Resources.FormatInvalidIndexValue(segment); return false; } @@ -295,5 +308,13 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal Invalid, // Ex: not an integer OutOfBounds } + + private enum OperationType + { + Add, + Remove, + Get, + Replace + } } } diff --git a/test/Microsoft.AspNetCore.JsonPatch.Test/ListAdapterTest.cs b/test/Microsoft.AspNetCore.JsonPatch.Test/ListAdapterTest.cs index ee331a2b30..799f6e9e89 100644 --- a/test/Microsoft.AspNetCore.JsonPatch.Test/ListAdapterTest.cs +++ b/test/Microsoft.AspNetCore.JsonPatch.Test/ListAdapterTest.cs @@ -55,12 +55,31 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal message); } + [Fact] + public void Add_WithIndexSameAsNumberOfElements_Works() + { + // Arrange + var resolver = new Mock(MockBehavior.Strict); + var targetObject = new List() { "James", "Mike" }; + var listAdapter = new ListAdapter(); + string message = null; + var position = targetObject.Count.ToString(); + + // Act + var addStatus = listAdapter.TryAdd(targetObject, position, resolver.Object, "Rob", out message); + + // Assert + Assert.Null(message); + Assert.True(addStatus); + Assert.Equal(3, targetObject.Count); + Assert.Equal(new List() { "James", "Mike", "Rob" }, targetObject); + } + [Theory] [InlineData("-1")] [InlineData("-2")] - [InlineData("2")] [InlineData("3")] - public void Patch_WithOutOfBoundsIndex_Fails(string position) + public void Add_WithOutOfBoundsIndex_Fails(string position) { // Arrange var resolver = new Mock(MockBehavior.Strict); @@ -225,6 +244,93 @@ namespace Microsoft.AspNetCore.JsonPatch.Internal Assert.Equal(expected, targetObject); } + [Theory] + [InlineData(new int[] { }, "0")] + [InlineData(new[] { 10, 20 }, "-1")] + [InlineData(new[] { 10, 20 }, "2")] + public void Get_IndexOutOfBounds(int[] input, string position) + { + // Arrange + var resolver = new Mock(MockBehavior.Strict); + var targetObject = new List(input); + var listAdapter = new ListAdapter(); + string message = null; + object value = null; + + // Act + var getStatus = listAdapter.TryGet(targetObject, position, resolver.Object, out value, out message); + + // Assert + Assert.False(getStatus); + Assert.Equal( + string.Format("The index value provided by path segment '{0}' is out of bounds of the array size.", position), + message); + } + + [Theory] + [InlineData(new[] { 10, 20 }, "0", 10)] + [InlineData(new[] { 10, 20 }, "1", 20)] + [InlineData(new[] { 10 }, "0", 10)] + public void Get(int[] input, string position, object expected) + { + // Arrange + var resolver = new Mock(MockBehavior.Strict); + var targetObject = new List(input); + var listAdapter = new ListAdapter(); + string message = null; + object value = null; + + // Act + var getStatus = listAdapter.TryGet(targetObject, position, resolver.Object, out value, out message); + + // Assert + Assert.True(getStatus); + Assert.Equal(expected, value); + Assert.Equal(new List(input), targetObject); + } + + [Theory] + [InlineData(new int[] { }, "0")] + [InlineData(new[] { 10, 20 }, "-1")] + [InlineData(new[] { 10, 20 }, "2")] + public void Remove_IndexOutOfBounds(int[] input, string position) + { + // Arrange + var resolver = new Mock(MockBehavior.Strict); + var targetObject = new List(input); + var listAdapter = new ListAdapter(); + string message = null; + + // Act + var removeStatus = listAdapter.TryRemove(targetObject, position, resolver.Object, out message); + + // Assert + Assert.False(removeStatus); + Assert.Equal( + string.Format("The index value provided by path segment '{0}' is out of bounds of the array size.", position), + message); + } + + [Theory] + [InlineData(new[] { 10, 20 }, "0", new[] { 20 })] + [InlineData(new[] { 10, 20 }, "1", new[] { 10 })] + [InlineData(new[] { 10 }, "0", new int[] { })] + public void Remove(int[] input, string position, int[] expected) + { + // Arrange + var resolver = new Mock(MockBehavior.Strict); + var targetObject = new List(input); + var listAdapter = new ListAdapter(); + string message = null; + + // Act + var removeStatus = listAdapter.TryRemove(targetObject, position, resolver.Object, out message); + + // Assert + Assert.True(removeStatus); + Assert.Equal(new List(expected), targetObject); + } + [Fact] public void Replace_NonCompatibleType_Fails() {