JSON Patch: Support real extension scenarios without copying heaps of code

[HappyNomad]

Microsoft.AspNetCore.JsonPatch is an important library in my projects. To be useful, however, I must tweak its ApplyTo behavior as described at #2432. Judging from others' issues I've read, what I'm doing isn't unusual. It seems to be a common need. I'm thankful that at least some extension point exists, but the current extensibility mechanism is cumbersome. The API should be improved.

I slightly diverge from the spec in my scenario, but the extensibility improvement would provide benefit beyond my use case.. One may need to patch objects of types not specifically supported by the library. While the patch document adheres to the spec, one may need to apply it to object types that the library couldn't anticipate. Those target object types could belong to a proprietary database engine, for example.

IObjectAdapter was created to facilitate customization from outside the library. This intent is apparent since (1) it's in the public API, and (2) it's not used internally within the library. Judging by the large amount of code duplication necessary to use it, however, it seems it wasn't tested with a real-world customization like #2432. To customize a few lines of code, I had to copy nine files. (They were ClosedGenericMatcher.cs, ConversionResultProvider.cs, ErrorReporter.cs, IAdapter.cs, ListAdapter.cs, ObjectAdapter.cs, ObjectVisitor.cs, PocoAdapter.cs, and Resources.Designer.cs.) This duplication poses the danger that (1) the copies become out-of-date with their original versions, and (2) the customized patch behavior overly diverges from the JSON Patch spec.

Rather than let the existing API stagnate and this problem fester, I propose the following. Solicit additional real-world extension scenarios from the community. Then take all scenarios into consideration and replace or augment the IObjectAdapter extension point with an API that allows us to cleanly extend JsonPatchDocument.

As for what specific API change to make, I offer the following initial suggestion. Replace the IObjectAdapter extension point with a public interface IAdapterFactory. It defines a single method with the signature IAdapter Create( object target, JsonContract jsonContract ). Use IAdapterFactory in ObjectVisitor's private SelectAdapater method.

[HappyNomad]

I just deleted my previous comment about considering the path when choosing the adapter. In fact, the path should not factor into that decision. The path and its segments should only affect individual operations performed on the chosen adapter.

[HappyNomad]

As an alternative to above IAdapterFactory idea, consider making JSON Patch's adapters like Asp.Net's formatters. Like an output formatter has a CanWriteType( Type ) method, add a CanAdaptType( Type ) method to IAdapter. It would indicate the targets for which the adapter is relevant. Like the formatters lists can be modified in Startup's services.AddMvc call, the adapters list could be modified as an option to JsonPatchDocument's ApplyTo call.

[mkArtakMSFT]

Thanks for contacting us, @HappyNomad.
Unfortunately we have no plans to change this in the upcoming future.

[HappyNomad]

@mkArtakMSFT We already went through a lot of discussion on this and, as far as I can tell, the changes have already been made to the code base. Are you planning to roll those changes back?