aspnet / JsonPatch

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

Extension point #88

Closed dario-hd closed 6 years ago

dario-hd commented 7 years ago

As far as I can see the extension point is the ObjectAdapter. So, basically the idea is to create a wrapper around it which implements the same interface. But ObjectAdapter is not injectable because of the Action<JsonPatchError> constructor parameter.

In addition, I saw that here and here it is explicitly set to null.

Does it make sense to make it optional in order to allow it to be injectable?

That's what I did when I used this version https://github.com/KevinDockx/JsonPatch, but the parameterless constructor is not available anymore.

public interface IPatchAdapter : IObjectAdapter
{
}

public class PatchAdapter : IPatchAdapter
{
    private readonly IObjectAdapter _objectAdapter;

    public PatchAdapter(IObjectAdapter objectAdapter)
    {
        _objectAdapter = Check.NotNull(objectAdapter, nameof(objectAdapter));
    }

    public void Add(Operation operation, object objectToApplyTo)
    {
        // ...
    }

    public void Copy(Operation operation, object objectToApplyTo)
    {
        // ...
    }

    public void Move(Operation operation, object objectToApplyTo)
    {
        // ...
    }

    public void Remove(Operation operation, object objectToApplyTo)
    {
        // ...
    }

    public void Replace(Operation operation, object objectToApplyTo)
    {
        // ...
    }
}
Eilon commented 7 years ago

@dario-hd can you provide more info on the scenario you are trying to accomplish?

dario-hd commented 7 years ago

@Eilon the idea is to enhance the original ObjectAdapter implementation. In the end I managed to do it by registering IObjectAdapter as follow in Autofac:

builder.Register(c => new ObjectAdapter(new DefaultContractResolver(), logErrorAction: null))
       .As<IObjectAdapter>();

I would create a constructor without logErrorAction in order to make it "more injectable".

Eilon commented 7 years ago

@dario-hd thanks for the extra info, we will evaluate this in our next team bug discussion.

Eilon commented 7 years ago

@dario-hd we took a quick look at this and we think we still need a bit more info. Can you provide info on what it is you're trying to inject? Maybe show the code that you'd like to be able to write when setting up your container and services?

aspnet-hello commented 6 years ago

This issue was moved to aspnet/Home#2431