aspnet / JsonPatch

[Archived] JSON PATCH library. Project moved to https://github.com/aspnet/AspNetCore
Apache License 2.0
103 stars 48 forks source link

Support for patching dynamic types should go beyond ExpandoObject #38

Closed kichalla closed 7 years ago

kichalla commented 8 years ago

Currently we handle patching of dynamic types like ExpandoObject, but there are also other dynamic types like DynamicObject which should be supported.

All dynamic types are supposed to implement IDynamicMetaObjectProvider which we can look for while trying to support this behavior.

@rynowak @dougbu

Eilon commented 7 years ago

Putting on backlog, awaiting customer feedback.

TAGC commented 7 years ago

Has this been resolved or is anywhere near close to being resolved? .NET Core Preview 2 SDK seems to treat downgrades as errors rather than warnings, and this is the only dependency I'm forced to downgrade (to v1.0.0) in my projects in order to workaround the problems discussed in #59.

jbagga commented 7 years ago

Not yet. This is planned for 2.1

TAGC commented 7 years ago

After updating the Microsoft.* NuGet packages in my projects to the recently released 2.0.0 versions, I'm no longer able to use the downgraded 1.0.0 version of Microsoft.AspNetCore.JsonPatch. I get an exception within the Startup.Configure() method of my web service:

Autofac.Core.DependencyResolutionException occurred
  HResult=0x80131500
  Message=An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = MvcRouteHandler (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.Internal.MvcRouteHandler], Lifetime = Autofac.Core.Lifetime.RootScopeLifetime, Sharing = Shared, Ownership = OwnedByLifetimeScope
  Source=Autofac
  StackTrace:
   at Autofac.Core.Resolving.InstanceLookup.Activate(IEnumerable`1 parameters)
   at Autofac.Core.Lifetime.LifetimeScope.GetOrCreateAndShare(Guid id, Func`1 creator)
   at Autofac.Core.Resolving.InstanceLookup.Execute()
   at Autofac.Core.Resolving.ResolveOperation.GetOrCreateInstance(ISharingLifetimeScope currentOperationScope, IComponentRegistration registration, IEnumerable`1 parameters)
   at Autofac.Core.Resolving.ResolveOperation.Execute(IComponentRegistration registration, IEnumerable`1 parameters)
   at Autofac.ResolutionExtensions.TryResolveService(IComponentContext context, Service service, IEnumerable`1 parameters, Object& instance)
   at Autofac.ResolutionExtensions.ResolveService(IComponentContext context, Service service, IEnumerable`1 parameters)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService(IServiceProvider provider, Type serviceType)
   at Microsoft.Extensions.DependencyInjection.ServiceProviderServiceExtensions.GetRequiredService[T](IServiceProvider provider)
   at Microsoft.AspNetCore.Builder.MvcApplicationBuilderExtensions.UseMvc(IApplicationBuilder app, Action`1 configureRoutes)
   at Microsoft.AspNetCore.Builder.MvcApplicationBuilderExtensions.UseMvc(IApplicationBuilder app)
   at AtpGroup.TestCubeWeb.ServiceAdapters.WebSockets.Startup.Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory, IApplicationLifetime appLifetime) in C:\Users\davidfallah\Documents\Visual Studio 2017\Projects\testcube-web-service-core\src\AtpGroup.TestCubeWeb.ServiceAdapters.WebSockets\Startup.cs:line 152

Inner Exception 1:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = ActionInvokerFactory (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.Infrastructure.IActionInvokerFactory], Lifetime = Autofac.Core.Lifetime.RootScopeLifetime, Sharing = Shared, Ownership = OwnedByLifetimeScope

Inner Exception 2:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = IActionInvokerProvider[] (DelegateActivator), Services = [System.Collections.Generic.IEnumerable`1[[Microsoft.AspNetCore.Mvc.Abstractions.IActionInvokerProvider, Microsoft.AspNetCore.Mvc.Abstractions, Version=2.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60]]], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = ExternallyOwned

Inner Exception 3:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = ControllerActionInvokerProvider (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.Abstractions.IActionInvokerProvider], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = OwnedByLifetimeScope

Inner Exception 4:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = ControllerActionInvokerCache (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvokerCache], Lifetime = Autofac.Core.Lifetime.RootScopeLifetime, Sharing = Shared, Ownership = OwnedByLifetimeScope

Inner Exception 5:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = ActionDescriptorCollectionProvider (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.Infrastructure.IActionDescriptorCollectionProvider], Lifetime = Autofac.Core.Lifetime.RootScopeLifetime, Sharing = Shared, Ownership = OwnedByLifetimeScope

Inner Exception 6:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = IActionDescriptorProvider[] (DelegateActivator), Services = [System.Collections.Generic.IEnumerable`1[[Microsoft.AspNetCore.Mvc.Abstractions.IActionDescriptorProvider, Microsoft.AspNetCore.Mvc.Abstractions, Version=2.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60]]], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = ExternallyOwned

Inner Exception 7:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = ControllerActionDescriptorProvider (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.Abstractions.IActionDescriptorProvider], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = OwnedByLifetimeScope

Inner Exception 8:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = IApplicationModelProvider[] (DelegateActivator), Services = [System.Collections.Generic.IEnumerable`1[[Microsoft.AspNetCore.Mvc.ApplicationModels.IApplicationModelProvider, Microsoft.AspNetCore.Mvc.Core, Version=2.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60]]], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = ExternallyOwned

Inner Exception 9:
DependencyResolutionException: An error occurred during the activation of a particular registration. See the inner exception for details. Registration: Activator = DefaultApplicationModelProvider (ReflectionActivator), Services = [Microsoft.AspNetCore.Mvc.ApplicationModels.IApplicationModelProvider], Lifetime = Autofac.Core.Lifetime.CurrentScopeLifetime, Sharing = None, Ownership = OwnedByLifetimeScope

Inner Exception 10:
DependencyResolutionException: An exception was thrown while invoking the constructor 'Void .ctor(Microsoft.Extensions.Options.IOptions`1[Microsoft.AspNetCore.Mvc.MvcOptions])' on type 'DefaultApplicationModelProvider'.

Inner Exception 11:
FileLoadException: Could not load file or assembly 'Microsoft.AspNetCore.JsonPatch, Version=2.0.0.0, Culture=neutral, PublicKeyToken=adb9793829ddae60'. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)

This issue is now a major blocking one in my case, preventing me from targeting .NET Core 2.0 RTM.

TAGC commented 7 years ago

I've noticed that you've started work on this, though. If it's at all feasible, would it be possible for you to release private or pre-release NuGet packages with these additions? I have my own tests that I can use to validate the implementation of this feature in addition to your own, so it can be beneficial for us both.

Eilon commented 7 years ago

@TAGC once the changes are committed, the builds will be on the aspnetcore-dev MyGet feed. You can find more info here: https://github.com/aspnet/Home#builds

TAGC commented 7 years ago

Hey @jbagga, thanks for the work you put in on this. However, I'm still facing issues with the latest code.

Using Microsoft.AspNetCore.JsonPatch v2.1.0-preview1-26831, I'm facing errors like this:

System.InvalidCastException occurred
  HResult=0x80004002
  Message=Unable to cast object of type 'Newtonsoft.Json.Serialization.JsonDictionaryContract' to type 'Newtonsoft.Json.Serialization.JsonDynamicContract'.
  Source=<Cannot evaluate the exception source>
  StackTrace:
   at Microsoft.AspNetCore.JsonPatch.Internal.DynamicObjectAdapter.TrySetDynamicObjectProperty(Object target, IContractResolver contractResolver, String segment, Object value, String& errorMessage)
   at Microsoft.AspNetCore.JsonPatch.Internal.DynamicObjectAdapter.TryAdd(Object target, String segment, IContractResolver contractResolver, Object value, String& errorMessage)
   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 MyCompany.MyProduct.Utility.Tests.DynamicDeserialisationStoreTests.Store_Should_Handle_Adding_Keyed_Model() in C:\Users\me\...\DynamicDeserialisationStoreTests.cs:line 35

In these unit tests, I'm testing deserialisation against this model:

private class FooModel
{
    [JsonProperty(PropertyName = "number")]
    public int Number { get; set; }

    [JsonProperty(PropertyName = "bazzed")]
    public bool IsBazzed { get; set; }
}

private class FooSystemModel
{
    private readonly IDictionary<string, FooModel> foos;

    public FooSystemModel()
    {
        this.foos = new Dictionary<string, FooModel>();
        this.Foos = new DynamicDeserialisationStore<FooModel>(
            storeValue: (name, foo) => this.foos[name] = foo,
            removeValue: name => this.foos.Remove(name),
            retrieveValue: name => this.foos[name],
            retrieveKeys: () => this.foos.Keys);
    }

    [JsonProperty(PropertyName = "foos")]
    public IDictionary<string, object> Foos { get; }
}

Here's an example of one of the tests (the exception I pasted above is from this):

[Fact]
public void Store_Should_Handle_Adding_Keyed_Model()
{
    // GIVEN the foo system currently contains no foos.
    this.fooSystem.Foos.ShouldBeEmpty();

    // GIVEN a patch document to store a foo called "test".
    var request = "{\"op\":\"add\",\"path\":\"/foos/test\",\"value\":{\"number\":3,\"bazzed\":true}}";
    var operation = JsonConvert.DeserializeObject<Operation<FooSystemModel>>(request);
    var patchDocument = new JsonPatchDocument<FooSystemModel>(
        new[] { operation }.ToList(),
        new CamelCasePropertyNamesContractResolver());

    // WHEN we apply this patch document to the foo system model.
    patchDocument.ApplyTo(this.fooSystem);

    // THEN the system model should now contain a new foo called "test" with the expected properties.
    this.fooSystem.Foos.ShouldHaveSingleItem();
    FooModel foo = this.fooSystem.Foos["test"] as FooModel;
    foo.Number.ShouldBe(3);
    foo.IsBazzed.ShouldBeTrue();
}

The relevant source and test code:

This code all works with v1.0.0 of the JsonPatch library.

Eilon commented 7 years ago

Re-opening so @jbagga can investigate.

jbagga commented 7 years ago

@TAGC Thank you for sharing your code!

Since your class also implements IDictionary<string, object> JSON.NET resolves it as JsonDictionaryContract instead of the expected JsonDynamicContract.

I am working on changing how the adapters are selected (based on what contract is returned) but your specific implementation will still return JsonDictionaryContract which is not compatible with the DynamicObjectAdapter. So the DictionaryAdapter will be selected and the exception will be something related to not being able to find the keys in the dictionary (they do not exist) which you expected to be added dynamically. Can you try to model your class after this test class?

The behavior in 1.0 was not consistent with how JSON.NET behaves. We are attempting to fix that in 2.1.0.

cc @rynowak

TAGC commented 7 years ago

Thanks @jbagga, I've got a solution now that I think works (unit tests are passing for DynamicDeserialisationStore, and unit/integration tests are for the most part working in a higher-level library that uses it - only issue is a heisenbug that intermittently causes a couple tests to fail, and I haven't yet determined if that's related to these changes).

I'll need to migrate more of my codebase to .NET Core 2 when I have a chance to be sure everything's working. I'm happy if this issue is closed for the time being - I'll flag up any issues if I find them.

jbagga commented 7 years ago

Okay, thanks!

@Eilon Closing for now.