Skip to content
This repository has been archived by the owner on Nov 15, 2018. It is now read-only.

Commit

Permalink
[Fixes #50] JsonPatchDocument.Replace() yields invalid path when [Jso…
Browse files Browse the repository at this point in the history
…nProperty] is used (1.1.0)
  • Loading branch information
kichalla committed Dec 30, 2016
1 parent c8c2dfa commit aa88e75
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 62 deletions.
41 changes: 16 additions & 25 deletions src/Microsoft.AspNetCore.JsonPatch/Internal/ExpressionHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,20 @@

using System;
using System.Globalization;
using System.Linq;
using System.Linq.Expressions;
using System.Reflection;
using Newtonsoft.Json;
using Newtonsoft.Json.Serialization;

namespace Microsoft.AspNetCore.JsonPatch.Internal
{
public static class ExpressionHelpers
{
public static string GetPath<TModel, TProp>(Expression<Func<TModel, TProp>> expr) where TModel : class
public static string GetPath<TModel, TProp>(Expression<Func<TModel, TProp>> expr, IContractResolver contractResolver) where TModel : class
{
return "/" + GetPath(expr.Body, true);
return "/" + GetPath(expr.Body, contractResolver, true);
}

private static string GetPath(Expression expr, bool firstTime)
private static string GetPath(Expression expr, IContractResolver contractResolver, bool firstTime)
{
switch (expr.NodeType)
{
Expand All @@ -25,7 +25,7 @@ private static string GetPath(Expression expr, bool firstTime)

if (ContinueWithSubPath(binaryExpression.Left.NodeType, false))
{
var leftFromBinaryExpression = GetPath(binaryExpression.Left, false);
var leftFromBinaryExpression = GetPath(binaryExpression.Left, contractResolver, false);
return leftFromBinaryExpression + "/" + binaryExpression.Right.ToString();
}
else
Expand All @@ -37,7 +37,7 @@ private static string GetPath(Expression expr, bool firstTime)

if (ContinueWithSubPath(methodCallExpression.Object.NodeType, false))
{
var leftFromMemberCallExpression = GetPath(methodCallExpression.Object, false);
var leftFromMemberCallExpression = GetPath(methodCallExpression.Object, contractResolver, false);
return leftFromMemberCallExpression + "/" +
GetIndexerInvocation(methodCallExpression.Arguments[0]);
}
Expand All @@ -46,20 +46,20 @@ private static string GetPath(Expression expr, bool firstTime)
return GetIndexerInvocation(methodCallExpression.Arguments[0]);
}
case ExpressionType.Convert:
return GetPath(((UnaryExpression)expr).Operand, false);
return GetPath(((UnaryExpression)expr).Operand, contractResolver, false);
case ExpressionType.MemberAccess:
var memberExpression = expr as MemberExpression;

if (ContinueWithSubPath(memberExpression.Expression.NodeType, false))
{
var left = GetPath(memberExpression.Expression, false);
var left = GetPath(memberExpression.Expression, contractResolver, false);
// Get property name, respecting JsonProperty attribute
return left + "/" + GetPropertyNameFromMemberExpression(memberExpression);
return left + "/" + GetPropertyNameFromMemberExpression(memberExpression, contractResolver);
}
else
{
// Get property name, respecting JsonProperty attribute
return GetPropertyNameFromMemberExpression(memberExpression);
return GetPropertyNameFromMemberExpression(memberExpression, contractResolver);
}
case ExpressionType.Parameter:
// Fits "x => x" (the whole document which is "" as JSON pointer)
Expand All @@ -69,21 +69,12 @@ private static string GetPath(Expression expr, bool firstTime)
}
}

private static string GetPropertyNameFromMemberExpression(MemberExpression memberExpression)
private static string GetPropertyNameFromMemberExpression(MemberExpression memberExpression, IContractResolver contractResolver)
{
// 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;
var jsonObjectContract = (JsonObjectContract)contractResolver.ResolveContract(memberExpression.Expression.Type);
return jsonObjectContract.Properties
.First(jsonProperty => jsonProperty.UnderlyingName == memberExpression.Member.Name)
.PropertyName;
}

private static bool ContinueWithSubPath(ExpressionType expressionType, bool firstTime)
Expand Down
66 changes: 33 additions & 33 deletions src/Microsoft.AspNetCore.JsonPatch/JsonPatchDocumentOfT.cs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public JsonPatchDocument<TModel> Add<TProp>(Expression<Func<TModel, TProp>> path

Operations.Add(new Operation<TModel>(
"add",
ExpressionHelpers.GetPath(path).ToLowerInvariant(),
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant(),
from: null,
value: value));

Expand All @@ -93,7 +93,7 @@ public JsonPatchDocument<TModel> Add<TProp>(

Operations.Add(new Operation<TModel>(
"add",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + position,
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + position,
from: null,
value: value));

Expand All @@ -116,7 +116,7 @@ public JsonPatchDocument<TModel> Add<TProp>(Expression<Func<TModel, IList<TProp>

Operations.Add(new Operation<TModel>(
"add",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
from: null,
value: value));

Expand All @@ -136,7 +136,7 @@ public JsonPatchDocument<TModel> Remove<TProp>(Expression<Func<TModel, TProp>> p
throw new ArgumentNullException(nameof(path));
}

Operations.Add(new Operation<TModel>("remove", ExpressionHelpers.GetPath(path).ToLowerInvariant(), from: null));
Operations.Add(new Operation<TModel>("remove", ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant(), from: null));

return this;
}
Expand All @@ -157,7 +157,7 @@ public JsonPatchDocument<TModel> Remove<TProp>(Expression<Func<TModel, IList<TPr

Operations.Add(new Operation<TModel>(
"remove",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + position,
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + position,
from: null));

return this;
Expand All @@ -178,7 +178,7 @@ public JsonPatchDocument<TModel> Remove<TProp>(Expression<Func<TModel, IList<TPr

Operations.Add(new Operation<TModel>(
"remove",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
from: null));

return this;
Expand All @@ -200,7 +200,7 @@ public JsonPatchDocument<TModel> Replace<TProp>(Expression<Func<TModel, TProp>>

Operations.Add(new Operation<TModel>(
"replace",
ExpressionHelpers.GetPath(path).ToLowerInvariant(),
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant(),
from: null,
value: value));

Expand All @@ -225,7 +225,7 @@ public JsonPatchDocument<TModel> Replace<TProp>(Expression<Func<TModel, IList<TP

Operations.Add(new Operation<TModel>(
"replace",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + position,
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + position,
from: null,
value: value));

Expand All @@ -248,7 +248,7 @@ public JsonPatchDocument<TModel> Replace<TProp>(Expression<Func<TModel, IList<TP

Operations.Add(new Operation<TModel>(
"replace",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
from: null,
value: value));

Expand Down Expand Up @@ -278,8 +278,8 @@ public JsonPatchDocument<TModel> Move<TProp>(

Operations.Add(new Operation<TModel>(
"move",
ExpressionHelpers.GetPath(path).ToLowerInvariant(),
ExpressionHelpers.GetPath(from).ToLowerInvariant()));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant(),
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant()));

return this;
}
Expand Down Expand Up @@ -309,8 +309,8 @@ public JsonPatchDocument<TModel> Move<TProp>(

Operations.Add(new Operation<TModel>(
"move",
ExpressionHelpers.GetPath(path).ToLowerInvariant(),
ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant(),
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant() + "/" + positionFrom));

return this;
}
Expand Down Expand Up @@ -340,8 +340,8 @@ public JsonPatchDocument<TModel> Move<TProp>(

Operations.Add(new Operation<TModel>(
"move",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from).ToLowerInvariant()));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant()));

return this;
}
Expand Down Expand Up @@ -373,8 +373,8 @@ public JsonPatchDocument<TModel> Move<TProp>(

Operations.Add(new Operation<TModel>(
"move",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant() + "/" + positionFrom));

return this;
}
Expand Down Expand Up @@ -404,8 +404,8 @@ public JsonPatchDocument<TModel> Move<TProp>(

Operations.Add(new Operation<TModel>(
"move",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant() + "/" + positionFrom));

return this;
}
Expand Down Expand Up @@ -433,8 +433,8 @@ public JsonPatchDocument<TModel> Move<TProp>(

Operations.Add(new Operation<TModel>(
"move",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from).ToLowerInvariant()));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant()));

return this;
}
Expand Down Expand Up @@ -462,8 +462,8 @@ public JsonPatchDocument<TModel> Copy<TProp>(

Operations.Add(new Operation<TModel>(
"copy",
ExpressionHelpers.GetPath(path).ToLowerInvariant()
, ExpressionHelpers.GetPath(from).ToLowerInvariant()));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant()
, ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant()));

return this;
}
Expand Down Expand Up @@ -493,8 +493,8 @@ public JsonPatchDocument<TModel> Copy<TProp>(

Operations.Add(new Operation<TModel>(
"copy",
ExpressionHelpers.GetPath(path).ToLowerInvariant(),
ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant(),
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant() + "/" + positionFrom));

return this;
}
Expand Down Expand Up @@ -524,8 +524,8 @@ public JsonPatchDocument<TModel> Copy<TProp>(

Operations.Add(new Operation<TModel>(
"copy",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from).ToLowerInvariant()));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant()));

return this;
}
Expand Down Expand Up @@ -557,8 +557,8 @@ public JsonPatchDocument<TModel> Copy<TProp>(

Operations.Add(new Operation<TModel>(
"copy",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/" + positionTo,
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant() + "/" + positionFrom));

return this;
}
Expand Down Expand Up @@ -588,8 +588,8 @@ public JsonPatchDocument<TModel> Copy<TProp>(

Operations.Add(new Operation<TModel>(
"copy",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from).ToLowerInvariant() + "/" + positionFrom));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant() + "/" + positionFrom));

return this;
}
Expand Down Expand Up @@ -617,8 +617,8 @@ public JsonPatchDocument<TModel> Copy<TProp>(

Operations.Add(new Operation<TModel>(
"copy",
ExpressionHelpers.GetPath(path).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from).ToLowerInvariant()));
ExpressionHelpers.GetPath(path, ContractResolver).ToLowerInvariant() + "/-",
ExpressionHelpers.GetPath(from, ContractResolver).ToLowerInvariant()));

return this;
}
Expand Down
Loading

0 comments on commit aa88e75

Please sign in to comment.