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

Commit

Permalink
Feedback addressed
Browse files Browse the repository at this point in the history
  • Loading branch information
@jbagga
jbagga committed Aug 21, 2017
1 parent ff90ebc commit 4531abf
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 39 deletions.
60 changes: 27 additions & 33 deletions src/Microsoft.AspNetCore.JsonPatch/Internal/DynamicObjectAdapter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Dynamic;
using System.Reflection;
using System.Runtime.CompilerServices;
using Microsoft.CSharp.RuntimeBinder;
Expand All @@ -20,9 +19,8 @@ public bool TryAdd(
object value,
out string errorMessage)
{
if(!TrySetDynamicObjectProperty(target, contractResolver, segment, value))
if (!TrySetDynamicObjectProperty(target, contractResolver, segment, value, out errorMessage))
{
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}

Expand All @@ -37,10 +35,9 @@ public bool TryGet(
out object value,
out string errorMessage)
{
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out value))
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out value, out errorMessage))
{
value = null;
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}

Expand All @@ -55,9 +52,8 @@ public bool TryRemove(
out string errorMessage)
{
object property = null;
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out property))
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out property, out errorMessage))
{
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}

Expand All @@ -70,9 +66,8 @@ public bool TryRemove(
value = Activator.CreateInstance(property.GetType());
}

if (!TrySetDynamicObjectProperty(target, contractResolver, segment, value))
if (!TrySetDynamicObjectProperty(target, contractResolver, segment, value, out errorMessage))
{
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}

Expand All @@ -89,9 +84,8 @@ public bool TryReplace(
out string errorMessage)
{
object property = null;
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out property))
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out property, out errorMessage))
{
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}

Expand All @@ -102,9 +96,8 @@ public bool TryReplace(
return false;
}

if (!TrySetDynamicObjectProperty(target, contractResolver, segment, convertedValue))
if (!TrySetDynamicObjectProperty(target, contractResolver, segment, convertedValue, out errorMessage))
{
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}

Expand All @@ -119,19 +112,10 @@ public bool TryTraverse(
out object nextTarget,
out string errorMessage)
{
var dynamicObject = target as IDynamicMetaObjectProvider;
if (dynamicObject == null)
{
nextTarget = null;
errorMessage = null;
return false;
}

object property = null;
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out property))
if (!TryGetDynamicObjectProperty(target, contractResolver, segment, out property, out errorMessage))
{
nextTarget = null;
errorMessage = null;
return false;
}
else
Expand All @@ -146,16 +130,19 @@ private bool TryGetDynamicObjectProperty(
object target,
IContractResolver contractResolver,
string segment,
out object property)
out object property,
out string errorMessage)
{
var jsonDynamicContract = contractResolver.ResolveContract(target.GetType()) as JsonDynamicContract;

var propertyName = jsonDynamicContract?.PropertyNameResolver(segment);

var binder = CSharp.RuntimeBinder.Binder.GetMember(CSharpBinderFlags.None,
propertyName,
var binder = CSharp.RuntimeBinder.Binder.GetMember(
CSharpBinderFlags.None,
propertyName ?? segment,
target.GetType(),
new List<CSharpArgumentInfo>{
new List<CSharpArgumentInfo>
{
CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null),
CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null)
});
Expand All @@ -165,11 +152,13 @@ private bool TryGetDynamicObjectProperty(
try
{
property = callsite.Target(callsite, target);
errorMessage = null;
return true;
}
catch
catch (RuntimeBinderException)
{
property = null;
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}
}
Expand All @@ -178,16 +167,19 @@ private bool TrySetDynamicObjectProperty(
object target,
IContractResolver contractResolver,
string segment,
object value)
object value,
out string errorMessage)
{
var jsonDynamicContract = contractResolver.ResolveContract(target.GetType()) as JsonDynamicContract;

var propertyName = jsonDynamicContract?.PropertyNameResolver(segment);

var binder = CSharp.RuntimeBinder.Binder.SetMember(CSharpBinderFlags.None,
propertyName,
var binder = CSharp.RuntimeBinder.Binder.SetMember(
CSharpBinderFlags.None,
propertyName ?? segment,
target.GetType(),
new List<CSharpArgumentInfo>{
new List<CSharpArgumentInfo>
{
CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null),
CSharpArgumentInfo.Create(CSharpArgumentInfoFlags.None, null)
});
Expand All @@ -197,10 +189,12 @@ private bool TrySetDynamicObjectProperty(
try
{
callsite.Target(callsite, target, value);
errorMessage = null;
return true;
}
catch
catch (RuntimeBinderException)
{
errorMessage = Resources.FormatTargetLocationAtPathSegmentNotFound(segment);
return false;
}
}
Expand Down
10 changes: 4 additions & 6 deletions test/Microsoft.AspNetCore.JsonPatch.Test/Dynamic/DynamicObjectAdapterTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,8 @@ public void ShouldNotBeAbleToAddToNonExistingPropertyThatIsNotTheRoot()
});
Assert.Equal(
string.Format(
"For operation '{0}', the target location specified by path '{1}' was not found.",
"add",
"/DynamicProperty/OtherProperty/IntProperty"),
"The target location specified by path segment '{0}' was not found.",
"OtherProperty"),
exception.Message);
}

Expand Down Expand Up @@ -889,9 +888,8 @@ public void NestedRemoveMixedCaseThrowsPathNotFoundException()
});

Assert.Equal(
string.Format("For operation '{0}', the target location specified by path '{1}' was not found.",
"remove",
"/Simpledto/stringProperty"),
string.Format("The target location specified by path segment '{0}' was not found.",
"Simpledto"),
exception.Message);
}

Expand Down
18 changes: 18 additions & 0 deletions test/Microsoft.AspNetCore.JsonPatch.Test/ObjectVisitorTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,5 +226,23 @@ public void Visit_DoesNotValidate_FinalPathSegment()
Assert.True(string.IsNullOrEmpty(message), "Expected no error message");
Assert.IsType<PocoAdapter>(adapter);
}

[Fact]
public void Visit_NullTarget_ReturnsNullAdapter()
{
// Arrange
var visitor = new ObjectVisitor(new ParsedPath("test"), new DefaultContractResolver());
IAdapter adapter = null;
string message = null;

// Act
object target = null;
var visitStatus = visitor.TryVisit(ref target, out adapter, out message);

// Assert
Assert.False(visitStatus);
Assert.Null(adapter);
Assert.Null(message);
}
}
}

0 comments on commit 4531abf

Please sign in to comment.