Why has dynamic patching support apparently been dropped between 1.0.0 and 1.1.0?

[TAGC]

I've been developing code that relies on dynamic JSON patching. In my ".NET Core library" project everything works as expected, and all tests pass. However, when I package up this project and use it as a dependency within an ASP.NET Core web application (.NETCore App 1.1.0), the exact same operations result in errors like this:

{Microsoft.AspNetCore.JsonPatch.Exceptions.JsonPatchException: The target location specified by path segment 'foo' was not found.
   at Microsoft.AspNetCore.JsonPatch.Internal.ErrorReporter.<>c.<.cctor>b__1_0(JsonPatchError error)
   at Microsoft.AspNetCore.JsonPatch.Adapters.ObjectAdapter.Add(String path, Object value, Object objectToApplyTo, Operation operation)
   at Microsoft.AspNetCore.JsonPatch.Adapters.ObjectAdapter.Add(Operation operation, Object objectToApplyTo)
   at Microsoft.AspNetCore.JsonPatch.JsonPatchDocument`1.ApplyTo(TModel objectToApplyTo, IObjectAdapter adapter)
   at <<Company>>.<<Product>>Web.Utility.JsonRequestProcessor`1.TryParseModifications(String request, T& discoveredModifications, T& discoveredFixedDetails, Exception& requestException)}  System.Exception {Microsoft.AspNetCore.JsonPatch.Exceptions.JsonPatchException}

It had me scratching my head for a while but I believe I've tracked the root of this issue down to the fact that my library uses Microsoft.AspNetCore.JsonPatch v1.0.0 whereas my web application would be using Microsoft.AspNetCore.JsonPatch v1.1.0.

Homing in on Microsoft.AspNetCore.JsonPatch.Adapters.ObjectAdapter.Add(String path, Object value, Object objectToApplyTo, Operation operation) I compared the differences between the two versions.

From v1.0.0, it looks like it's handling dynamic paths:

private void Add(
    string path,
    object value,
    object objectToApplyTo,
    Operation operationToReport)
{
    if (path == null)
    {
        throw new ArgumentNullException(nameof(path));
    }

    if (objectToApplyTo == null)
    {
        throw new ArgumentNullException(nameof(objectToApplyTo));
    }

    if (operationToReport == null)
    {
        throw new ArgumentNullException(nameof(operationToReport));
    }

    // ...

    if (treeAnalysisResult.UseDynamicLogic)
    {
        // ...
    }
    // ...
}

However, from v1.1.0 this method seems to omit all dynamic path support:

private void Add(
    string path,
    object value,
    object objectToApplyTo,
    Operation operation)
{
    if (path == null)
    {
        throw new ArgumentNullException(nameof(path));
    }

    if (objectToApplyTo == null)
    {
        throw new ArgumentNullException(nameof(objectToApplyTo));
    }

    if (operation == null)
    {
        throw new ArgumentNullException(nameof(operation));
    }

    var parsedPath = new ParsedPath(path);
    var visitor = new ObjectVisitor(parsedPath, ContractResolver);

    IAdapter adapter;
    var target = objectToApplyTo;
    string errorMessage;
    if (!visitor.TryVisit(ref target, out adapter, out errorMessage))
    {
        var error = CreatePathNotFoundError(objectToApplyTo, path, operation, errorMessage);
        ErrorReporter(error);
        return;
    }

    if (!adapter.TryAdd(target, parsedPath.LastSegment, ContractResolver, value, out errorMessage))
    {
        var error = CreateOperationFailedError(objectToApplyTo, path, operation, errorMessage);
        ErrorReporter(error);
        return;
    }
}

Why has this been done?

[kichalla]

@TAGC The support has not been removed is handled by the following: https://github.com/aspnet/JsonPatch/blob/dev/src/Microsoft.AspNetCore.JsonPatch/Internal/ObjectVisitor.cs#L58 https://github.com/aspnet/JsonPatch/blob/dev/src/Microsoft.AspNetCore.JsonPatch/Internal/ExpandoObjectAdapter.cs

Also we have bunch of tests for 'dynamic' here: https://github.com/aspnet/JsonPatch/tree/dev/test/Microsoft.AspNetCore.JsonPatch.Test/Dynamic

Could you share your repro code?

[TAGC]

I think I know where the problem is:

if (targetObject is ExpandoObject)
{
    return new ExpandoObjectAdapter();
}

You seem to handle dynamic patching only for instances of ExpandoObject whereas I've created my own class that subclasses DynamicObject directly.

I have an early version of this class here which you can use to reproduce the issue.

[TAGC]

Looks like this is already covered by #38.

[Eilon]

Yes, looks to be a dup of https://github.com/aspnet/JsonPatch/issues/38.