[Fixes #50] JsonPatchDocument.Replace() yields invalid path when [JsonProperty] is used (1.1.0)

src/Microsoft.AspNetCore.JsonPatch/Internal/ExpressionHelpers.cs

@@ -1,120 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved.
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-using System;
-using System.Globalization;
-using System.Linq.Expressions;
-using System.Reflection;
-using Newtonsoft.Json;
-
-namespace Microsoft.AspNetCore.JsonPatch.Internal
-{
-    public static class ExpressionHelpers
-    {
-        public static string GetPath<TModel, TProp>(Expression<Func<TModel, TProp>> expr) where TModel : class
-        {
-            return "/" + GetPath(expr.Body, true);
-        }
-
-        private static string GetPath(Expression expr, bool firstTime)
-        {
-            switch (expr.NodeType)
-            {
-                case ExpressionType.ArrayIndex:
-                    var binaryExpression = (BinaryExpression)expr;
-
-                    if (ContinueWithSubPath(binaryExpression.Left.NodeType, false))
-                    {
-                        var leftFromBinaryExpression = GetPath(binaryExpression.Left, false);
-                        return leftFromBinaryExpression + "/" + binaryExpression.Right.ToString();
-                    }
-                    else
-                    {
-                        return binaryExpression.Right.ToString();
-                    }
-                case ExpressionType.Call:
-                    var methodCallExpression = (MethodCallExpression)expr;
-
-                    if (ContinueWithSubPath(methodCallExpression.Object.NodeType, false))
-                    {
-                        var leftFromMemberCallExpression = GetPath(methodCallExpression.Object, false);
-                        return leftFromMemberCallExpression + "/" +
-                            GetIndexerInvocation(methodCallExpression.Arguments[0]);
-                    }
-                    else
-                    {
-                        return GetIndexerInvocation(methodCallExpression.Arguments[0]);
-                    }
-                case ExpressionType.Convert:
-                    return GetPath(((UnaryExpression)expr).Operand, false);
-                case ExpressionType.MemberAccess:
-                    var memberExpression = expr as MemberExpression;
-
-                    if (ContinueWithSubPath(memberExpression.Expression.NodeType, false))
-                    {
-                        var left = GetPath(memberExpression.Expression, false);
-                        // Get property name, respecting JsonProperty attribute
-                        return left + "/" + GetPropertyNameFromMemberExpression(memberExpression);
-                    }
-                    else
-                    {
-                        // Get property name, respecting JsonProperty attribute
-                        return GetPropertyNameFromMemberExpression(memberExpression);
-                    }
-                case ExpressionType.Parameter:
-                    // Fits "x => x" (the whole document which is "" as JSON pointer)
-                    return firstTime ? string.Empty : null;
-                default:
-                    return string.Empty;
-            }
-        }
-
-        private static string GetPropertyNameFromMemberExpression(MemberExpression memberExpression)
-        {
-            // if there's a JsonProperty attribute, we must return the PropertyName
-            // from the attribute rather than the member name
-            var jsonPropertyAttribute =
-                memberExpression.Member.GetCustomAttribute(
-                typeof(JsonPropertyAttribute), true);
-
-            if (jsonPropertyAttribute == null)
-            {
-                return memberExpression.Member.Name;
-            }
-            // get value
-            var castedAttribute = (JsonPropertyAttribute)jsonPropertyAttribute;
-            return castedAttribute.PropertyName;
-        }
-
-        private static bool ContinueWithSubPath(ExpressionType expressionType, bool firstTime)
-        {
-            if (firstTime)
-            {
-                return (expressionType == ExpressionType.ArrayIndex
-                       || expressionType == ExpressionType.Call
-                       || expressionType == ExpressionType.Convert
-                       || expressionType == ExpressionType.MemberAccess
-                       || expressionType == ExpressionType.Parameter);
-            }
-            else
-            {
-                return (expressionType == ExpressionType.ArrayIndex
-                    || expressionType == ExpressionType.Call
-                    || expressionType == ExpressionType.Convert
-                    || expressionType == ExpressionType.MemberAccess);
-            }
-        }
-
-        private static string GetIndexerInvocation(Expression expression)
-        {
-            var converted = Expression.Convert(expression, typeof(object));
-            var fakeParameter = Expression.Parameter(typeof(object), null);
-            var lambda = Expression.Lambda<Func<object, object>>(converted, fakeParameter);
-            Func<object, object> func;
-
-            func = lambda.Compile();
-
-            return Convert.ToString(func(null), CultureInfo.InvariantCulture);
-        }
-    }
-}

src/Microsoft.AspNetCore.JsonPatch/JsonPatchDocumentOfT.cs

@@ -2,7 +2,9 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
+using System.Linq;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq.Expressions;
 using Microsoft.AspNetCore.JsonPatch.Adapters;
 using Microsoft.AspNetCore.JsonPatch.Converters;
@@ -66,7 +68,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "add",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant(),
+                GetPath(path),
                 from: null,
                 value: value));
 
@@ -93,7 +95,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "add",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + position,
+                GetPath(path) + "/" + position,
                 from: null,
                 value: value));
 
@@ -116,7 +118,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "add",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
                 from: null,
                 value: value));
 
@@ -136,7 +138,7 @@ namespace Microsoft.AspNetCore.JsonPatch
                 throw new ArgumentNullException(nameof(path));
             }
 
-            Operations.Add(new Operation<TModel>("remove", ExpressionHelpers.GetPath(path).ToLowerInvariant(), from: null));
+            Operations.Add(new Operation<TModel>("remove", GetPath(path), from: null));
 
             return this;
         }
@@ -157,7 +159,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "remove",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + position,
+                GetPath(path) + "/" + position,
                 from: null));
 
             return this;
@@ -178,7 +180,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "remove",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
                 from: null));
 
             return this;
@@ -200,7 +202,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "replace",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant(),
+                GetPath(path),
                 from: null,
                 value: value));
 
@@ -225,7 +227,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "replace",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + position,
+                GetPath(path) + "/" + position,
                 from: null,
                 value: value));
 
@@ -248,7 +250,7 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "replace",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
                 from: null,
                 value: value));
 
@@ -278,8 +280,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "move",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant(),
+                GetPath(path),
-                ExpressionHelpers.GetPath(from).ToLowerInvariant()));
+                GetPath(from)));
 
             return this;
         }
@@ -309,8 +311,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "move",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant(),
+                GetPath(path),
-                ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
+                GetPath(from) + "/" + positionFrom));
 
             return this;
         }
@@ -340,8 +342,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "move",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
+                GetPath(path) + "/" + positionTo,
-                ExpressionHelpers.GetPath(from).ToLowerInvariant()));
+                GetPath(from)));
 
             return this;
         }
@@ -373,8 +375,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "move",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
+                GetPath(path) + "/" + positionTo,
-                ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
+                GetPath(from) + "/" + positionFrom));
 
             return this;
         }
@@ -404,8 +406,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "move",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
-                ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
+                GetPath(from) + "/" + positionFrom));
 
             return this;
         }
@@ -433,8 +435,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "move",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
-                ExpressionHelpers.GetPath(from).ToLowerInvariant()));
+                GetPath(from)));
 
             return this;
         }
@@ -462,8 +464,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "copy",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant()
+                GetPath(path)
-              , ExpressionHelpers.GetPath(from).ToLowerInvariant()));
+              , GetPath(from)));
 
             return this;
         }
@@ -493,8 +495,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "copy",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant(),
+                GetPath(path),
-                ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
+                GetPath(from) + "/" + positionFrom));
 
             return this;
         }
@@ -524,8 +526,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "copy",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
+                GetPath(path) + "/" + positionTo,
-                ExpressionHelpers.GetPath(from).ToLowerInvariant()));
+                GetPath(from)));
 
             return this;
         }
@@ -557,8 +559,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "copy",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
+                GetPath(path) + "/" + positionTo,
-                ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
+                GetPath(from) + "/" + positionFrom));
 
             return this;
         }
@@ -588,8 +590,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "copy",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
-                ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
+                GetPath(from) + "/" + positionFrom));
 
             return this;
         }
@@ -617,8 +619,8 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             Operations.Add(new Operation<TModel>(
                 "copy",
-                ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
+                GetPath(path) + "/-",
-                ExpressionHelpers.GetPath(from).ToLowerInvariant()));
+                GetPath(from)));
 
             return this;
         }
@@ -712,5 +714,107 @@ namespace Microsoft.AspNetCore.JsonPatch
 
             return allOps;
         }
+
+        private string GetPath<TProp>(Expression<Func<TModel, TProp>> expr)
+        {
+            return "/" + GetPath(expr.Body, true).ToLowerInvariant();
+        }
+
+        private string GetPath(Expression expr, bool firstTime)
+        {
+            switch (expr.NodeType)
+            {
+                case ExpressionType.ArrayIndex:
+                    var binaryExpression = (BinaryExpression)expr;
+
+                    if (ContinueWithSubPath(binaryExpression.Left.NodeType, false))
+                    {
+                        var leftFromBinaryExpression = GetPath(binaryExpression.Left, false);
+                        return leftFromBinaryExpression + "/" + binaryExpression.Right.ToString();
+                    }
+                    else
+                    {
+                        return binaryExpression.Right.ToString();
+                    }
+                case ExpressionType.Call:
+                    var methodCallExpression = (MethodCallExpression)expr;
+
+                    if (ContinueWithSubPath(methodCallExpression.Object.NodeType, false))
+                    {
+                        var leftFromMemberCallExpression = GetPath(methodCallExpression.Object, false);
+                        return leftFromMemberCallExpression + "/" +
+                            GetIndexerInvocation(methodCallExpression.Arguments[0]);
+                    }
+                    else
+                    {
+                        return GetIndexerInvocation(methodCallExpression.Arguments[0]);
+                    }
+                case ExpressionType.Convert:
+                    return GetPath(((UnaryExpression)expr).Operand, false);
+                case ExpressionType.MemberAccess:
+                    var memberExpression = expr as MemberExpression;
+
+                    if (ContinueWithSubPath(memberExpression.Expression.NodeType, false))
+                    {
+                        var left = GetPath(memberExpression.Expression, false);
+                        // Get property name, respecting JsonProperty attribute
+                        return left + "/" + GetPropertyNameFromMemberExpression(memberExpression);
+                    }
+                    else
+                    {
+                        // Get property name, respecting JsonProperty attribute
+                        return GetPropertyNameFromMemberExpression(memberExpression);
+                    }
+                case ExpressionType.Parameter:
+                    // Fits "x => x" (the whole document which is "" as JSON pointer)
+                    return firstTime ? string.Empty : null;
+                default:
+                    return string.Empty;
+            }
+        }
+
+        private string GetPropertyNameFromMemberExpression(MemberExpression memberExpression)
+        {
+            var jsonObjectContract = ContractResolver.ResolveContract(memberExpression.Expression.Type) as JsonObjectContract;
+            if (jsonObjectContract != null)
+            {
+                return jsonObjectContract.Properties
+                    .First(jsonProperty => jsonProperty.UnderlyingName == memberExpression.Member.Name)
+                    .PropertyName;
+            }
+
+            return null;
+        }
+
+        private static bool ContinueWithSubPath(ExpressionType expressionType, bool firstTime)
+        {
+            if (firstTime)
+            {
+                return (expressionType == ExpressionType.ArrayIndex
+                       || expressionType == ExpressionType.Call
+                       || expressionType == ExpressionType.Convert
+                       || expressionType == ExpressionType.MemberAccess
+                       || expressionType == ExpressionType.Parameter);
+            }
+            else
+            {
+                return (expressionType == ExpressionType.ArrayIndex
+                    || expressionType == ExpressionType.Call
+                    || expressionType == ExpressionType.Convert
+                    || expressionType == ExpressionType.MemberAccess);
+            }
+        }
+
+        private static string GetIndexerInvocation(Expression expression)
+        {
+            var converted = Expression.Convert(expression, typeof(object));
+            var fakeParameter = Expression.Parameter(typeof(object), null);
+            var lambda = Expression.Lambda<Func<object, object>>(converted, fakeParameter);
+            Func<object, object> func;
+
+            func = lambda.Compile();
+
+            return Convert.ToString(func(null), CultureInfo.InvariantCulture);
+        }
     }
 }

test/Microsoft.AspNetCore.JsonPatch.Test/JsonPatchDocumentJsonPropertyAttributeTest.cs

@@ -1,8 +1,10 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using Newtonsoft.Json;
 using System.Linq;
+using System.Reflection;
+using Newtonsoft.Json;
+using Newtonsoft.Json.Serialization;
 using Xunit;
 
 namespace Microsoft.AspNetCore.JsonPatch
@@ -26,13 +28,47 @@ namespace Microsoft.AspNetCore.JsonPatch
             Assert.Equal(pathToCheck, "/anothername");
         }
 
+        [Fact]
+        public void Add_WithExpressionOnStringProperty_FallsbackToPropertyName_WhenJsonPropertyName_IsEmpty()
+        {
+            // Arrange
+            var patchDoc = new JsonPatchDocument<JsonPropertyWithNoPropertyName>();
+            patchDoc.Add(m => m.StringProperty, "Test");
+            var serialized = JsonConvert.SerializeObject(patchDoc);
+
+            // Act
+            var deserialized =
+                JsonConvert.DeserializeObject<JsonPatchDocument<JsonPropertyWithNoPropertyName>>(serialized);
+
+            // Assert
+            var pathToCheck = deserialized.Operations.First().path;
+            Assert.Equal("/stringproperty", pathToCheck);
+        }
+
+        [Fact]
+        public void Add_WithExpressionOnArrayProperty_FallsbackToPropertyName_WhenJsonPropertyName_IsEmpty()
+        {
+            // Arrange
+            var patchDoc = new JsonPatchDocument<JsonPropertyWithNoPropertyName>();
+            patchDoc.Add(m => m.ArrayProperty, "James");
+            var serialized = JsonConvert.SerializeObject(patchDoc);
+
+            // Act
+            var deserialized =
+                JsonConvert.DeserializeObject<JsonPatchDocument<JsonPropertyWithNoPropertyName>>(serialized);
+
+            // Assert
+            var pathToCheck = deserialized.Operations.First().path;
+            Assert.Equal("/arrayproperty/-", pathToCheck);
+        }
+
         [Fact]
         public void Add_WithExpression_RespectsJsonPropertyName_WhenApplyingToDifferentlyTypedClassWithPropertyMatchingJsonPropertyName()
         {
             var patchDocToSerialize = new JsonPatchDocument<JsonPropertyDTO>();
             patchDocToSerialize.Add(p => p.Name, "John");
 
-            // the patchdoc will deserialize to "anothername".  We should thus be able to apply 
+            // the patchdoc will deserialize to "anothername".  We should thus be able to apply
             // it to a class that HAS that other property name.
             var doc = new JsonPropertyWithAnotherNameDTO()
             {
@@ -81,7 +117,7 @@ namespace Microsoft.AspNetCore.JsonPatch
             {
                 Name = "InitialValue"
             };
-            
+
             // serialization should serialize to "AnotherName"
             var serialized = "[{\"value\":\"Kevin\",\"path\":\"/AnotherName\",\"op\":\"add\"}]";
             var deserialized =
@@ -151,9 +187,9 @@ namespace Microsoft.AspNetCore.JsonPatch
             var doc = new JsonPropertyComplexNameDTO()
             {
                 FooSlashBars = "InitialName",
-                FooSlashTilde = new  SimpleDTO
+                FooSlashTilde = new SimpleDTO
                 {
-                    StringProperty  = "Initial Value"
+                    StringProperty = "Initial Value"
                 }
             };
 
@@ -167,5 +203,72 @@ namespace Microsoft.AspNetCore.JsonPatch
             Assert.Equal("Kevin", doc.FooSlashBars);
             Assert.Equal("Final Value", doc.FooSlashTilde.StringProperty);
         }
+
+        [Fact]
+        public void Move_WithExpression_FallsbackToPropertyName_WhenJsonPropertyName_IsEmpty()
+        {
+            // Arrange
+            var patchDoc = new JsonPatchDocument<JsonPropertyWithNoPropertyName>();
+            patchDoc.Move(m => m.StringProperty, m => m.StringProperty2);
+            var serialized = JsonConvert.SerializeObject(patchDoc);
+
+            // Act
+            var deserialized =
+                JsonConvert.DeserializeObject<JsonPatchDocument<JsonPropertyWithNoPropertyName>>(serialized);
+
+            // Assert
+            var fromPath = deserialized.Operations.First().from;
+            Assert.Equal("/stringproperty", fromPath);
+            var toPath = deserialized.Operations.First().path;
+            Assert.Equal("/stringproperty2", toPath);
+        }
+
+        [Fact]
+        public void Add_WithExpression_AndCustomContractResolver_UsesPropertyName_SetByContractResolver()
+        {
+            // Arrange
+            var patchDoc = new JsonPatchDocument<JsonPropertyWithNoPropertyName>();
+            patchDoc.ContractResolver = new CustomContractResolver();
+            patchDoc.Add(m => m.SSN, "123-45-6789");
+            var serialized = JsonConvert.SerializeObject(patchDoc);
+
+            // Act
+            var deserialized =
+                JsonConvert.DeserializeObject<JsonPatchDocument<JsonPropertyWithNoPropertyName>>(serialized);
+
+            // Assert
+            var path = deserialized.Operations.First().path;
+            Assert.Equal("/socialsecuritynumber", path);
+        }
+
+        private class JsonPropertyWithNoPropertyName
+        {
+            [JsonProperty]
+            public string StringProperty { get; set; }
+
+            [JsonProperty]
+            public string[] ArrayProperty { get; set; }
+
+            [JsonProperty]
+            public string StringProperty2 { get; set; }
+
+            [JsonProperty]
+            public string SSN { get; set; }
+        }
+
+        private class CustomContractResolver : DefaultContractResolver
+        {
+            protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
+            {
+                var jsonProperty = base.CreateProperty(member, memberSerialization);
+
+                if (jsonProperty.PropertyName == "SSN")
+                {
+                    jsonProperty.PropertyName = "SocialSecurityNumber";
+                }
+
+                return jsonProperty;
+            }
+        }
     }
 }