Support patch / Delta<T> in MVC

[filipw]

+10000. I can't remember the last time I built anything in Web API that didn't use PATCH and some sort of a Delta<T> shim.

Originally, you could get the OData one to work with JSON (at least partially) but after going through a lot of drama. I think this would be a terrific addition.

[ashmind]

@yishaigalatzer which Delta<T>?

I've just googled and read http://www.strathweb.com/2013/01/easy-asp-net-web-api-resource-updates-with-delta/. It seems I did a very similar thing independently. However I think I used a different approach (haven't looked at OData code in detail yet).

For JSON.NET, I mostly delegate JsonContract of Patch<T> to JsonContract of T so that logic is similar to deserializing original object (all property types should be handled as they would normally be handled by JSON.NET). However I replace the property setter with my own property setter, which records an attempt to set the property instead of actually setting it.

Here is my current design in full: https://gist.github.com/ashmind/20b234e2fd69db542beb

Note that it is very raw at the moment and can have bugs, but it works for me in main scenarios (including Guids as JSON.NET is able to rely on original object property types).

That wouldn't work with default XmlSerializer -- but that serializer is terrible anyway, so I am using NFormats.Xml which is easier to extend (I haven't added patch support it yet though).

[yishaigalatzer]

It's the odata delta but the json.net implementation can be found in aspnet.codeplex.com in the webapi samples.

Nothing is set in stone, we haven't started looking into the new design at all

-----Original Message----- From: "Andrey Shchekin" notifications@github.com Sent: ‎1/‎29/‎2015 6:13 PM To: "aspnet/Mvc" Mvc@noreply.github.com Cc: "Yishai Galatzer" yishai_galatzer@yahoo.com Subject: Re: [Mvc] Support patch / Delta in MVC (#650)

@yishaigalatzer which Delta? I've just googled and read http://www.strathweb.com/2013/01/easy-asp-net-web-api-resource-updates-with-delta/. It seems I did a very similar thing independently. However I think I used a different approach (haven't looked at OData code in detail yet). For JSON.NET, I mostly delegate JsonContract of Patch to JsonContract of T so that logic is similar to deserializing original object (all property types should be handled as they would normally be handled by JSON.NET). However I replace the property setter with my own property setter, which records an attempt to set the property instead of actually setting it. Here is my current design in full: https://gist.github.com/ashmind/20b234e2fd69db542beb Note that it is very raw at the moment and can have bugs, but it works for me in main scenarios (including Guids as JSON.NET is able to rely on original object property types). That wouldn't work with default XmlSerializer -- but that serializer is terrible anyway, so I am using NFormats.Xml which is easier to extend (I haven't added patch support it yet though). — Reply to this email directly or view it on GitHub.=

[yishaigalatzer]

@ashmind just took a quick look at NFormatters.xml, the project seems abandoned :(

[ashmind]

@yishaigalatzer That is true, yet even in its current state I found it to be very stable and way more extensible than standard XmlSerializer. I can't advocate a project in that state for WebApi obviously, but I use it in my projects -- with custom converters it can handle anything, while XmlSerializer just ignored e.g. dynamic objects.

Anyway to me it makes sense to design the Patch<>/Delta<> support for a good customizable serializer (Json.Net etc), and discuss question of improving WebApi XML serializer separately.

[lukas-shawford]

Personally, I'd love to see ASP.NET add native support for the JSON Patch spec for supporting partial updates:

http://jsonpatch.com/ https://tools.ietf.org/html/rfc6902

There are a couple projects out there that add partial support for JSON Patch within Web API, but they're both kind of "alpha" at this stage (neither implement the spec fully):

myquay/JsonPatch Blog Post: http://michael-mckenna.com/Blog/how-to-add-json-patch-support-to-asp-net-web-api Github: https://github.com/myquay/JsonPatch NuGet: https://www.nuget.org/packages/JsonPatch/1.0.0

KevinDockx/JsonPatch GitHub: https://github.com/KevinDockx/JsonPatch NuGet: https://www.nuget.org/packages/Marvin.JsonPatch/0.3.0

Just wanted to throw this out there - not sure if this has been considered already or not.

[yishaigalatzer]

@sergkr thanks for the feedback. We will take a look before proceeding with this.

[yishaigalatzer]

@sergkr while we try to make up our mind, I would be glad to assist in porting the existing libraries (particularly I noticed you contribute to myquay/jsonPatch) to MVC 6. Though they do seem pretty straightforward. It will make it a lot easier to consider as a PR as well as it matures.

Note I'm not making any promises about taking it in or not, will need to discuss with the rest of the team first.

[lukas-shawford]

Understood, thank for considering this. I've put in an issue at myquay/JsonPatch to add support for MVC 6. I myself am not familiar yet with what exactly would have to go into this, but feel free to comment on that issue.

[yishaigalatzer]

From the looks of it, you will have to create an InputFormatter and OutputFormatter, the rest doesn't seem to require changes. You can look at StringOutputFormatter or StreamOutputFormatter as an example for a greedy formatter. There is no sample for an input counterpart, but its rather simple.

Unless I'm missing something I also noticed that the implementation there doesn't go and create items at arbitrary index (which is a good thing). I would suggest to open a follow up bug for json patch as a separate issue.

Also opened a separate issue to track this suggestion: #1976

[kevinchalet]

@yishaigalatzer does that mean that Delta won't be ported to MVC 6?

I was considering removing IDelta/Delta from Web API OData vNext, but if there's no global/built-in support in the core, that won't be necessary :smile:

[yishaigalatzer]

Not sure what is webapi odata vnext? None of the work on that has started yet. And 'delta' was always an odata feature.

[kevinchalet]

Meh: http://blogs.msdn.com/b/odatateam/archive/2015/03/12/early-investigation-into-supporting-the-odata-libraries-in-asp-net-5-mvc-6.aspx :smile:

The progress is tracked by these two tickets: https://github.com/OData/WebApi/issues/229 https://github.com/OData/odata.net/issues/97

I know that delta/patch is/was OData-specific, but MVC 6 could be the opportunity to support it at the core level. Currently, because it's implemented at the formatter level, it doesn't support the cool BindAttribute and its property filter stuff, so I suppose we'll have to implement a custom model binder to use it with Delta<>.

Maybe that a generic IDelta/IPatch interface could be added to MVC to tell the pipeline (and specially the model binding part): "hey, you received a JsonPatchDocument<Entity> or a Delta<Entity> but treat it like an Entity?

[yishaigalatzer]

There is no such notion in the pipeline and I doubt there will ever be one. On the odata port, I think a straight port is an interesting exercise. I was hoping moving data to mvc6 would not be a straight port but a point in time to rethink the architecture and morph the design with a longer term viability in mind.

Delta can be placed in a 3rd package that is separate than mvc and odata (like it should have been from day 1). Many people are reluctant to depend on it because its odata specific.

[kevinchalet]

There is no such notion in the pipeline and I doubt there will ever be one. On the odata port, I think a straight port is an interesting exercise. I was hoping moving data to mvc6 would not be a straight port but a point in time to rethink the architecture and morph the design with a longer term viability in mind.

The straight port is only the first part of the plan (starting from scratch would be a bit hard for a single man :man:). And actually, it's not really a straight port, because a few things have already changed (e.g I removed the routing part to re-unify ODataRoute and the MVC routing attributes). Of course, there are tons of things I'd like to change to make Web API OData better (allowing pluggable conventions for the model builder, separating the attributes from the core package, making querying more restrictive by default), but we're not there yet.

If you have particular suggestions, don't hesitate to share them.

Delta can be placed in a 3rd package that is separate than mvc and odata (like it should have been from day 1). Many people are reluctant to depend on it because its odata specific.

Hence my remark on an IPatch<TEntity> that would be integrated to MVC and that could be implemented by third packages to support JSON document patches, OData patches or totally custom formats.

[yishaigalatzer]

I would advice reading through the contribution guide. It's pretty neat that you are taking initiative and it might even be in the right direction, but I wouldn't like you to be disappointed if the guidance will be totally different once we get to a point where we actually sit down and start investing in odata. And that you realize that this work is a prototype and can be scrapped at any point and that ultimately the team is responsible to come up with the guidance and basic design.

'Ipatch' of entity imho ends up being nothing more than having an apply method. And each patch mechanism will have his own error handling exceptions being thrown, etc. So I'm very hesitant to agree with this suggestion. It's better to avoid unnecessary abstractions, that can always be added later when a read need arise. I can't think of a strong need at the moment.

[kevinchalet]

I would advice reading through the contribution guide. It's pretty neat that you are taking initiative and it might even be in the right direction, but I wouldn't like you to be disappointed if the guidance will be totally different once we get to a point where we actually sit down and start investing in odata. And that you realize that this work is a prototype and can be scrapped at any point and that ultimately the team is responsible to come up with the guidance and basic design.

Don't worry, I know exactly what the "open source by MSFT" sauce is :smile:

Honestly, as long as someone (MSFT or the community) is working on the Web API OData port, I'm totally fine. Of course, I realize that my contribution is just a prototype - hence the "prototype" flag on the issue BTW - and that it could be finally rejected in favor of a totally different approach, but you imagine that I wouldn't waste my time if the OData team wasn't interested by my contribution (I had a phone call 2 weeks ago with @congysu and his coworkers). And on the other hand, I'm also totally free to fork it and release it with a new name if I'm not satisfied with the official package.

I'm definitely not looking for glory or personal satisfaction, and my remarks have only one objective: helping MSFT to make people realize that OData (the whole ecosystem, not particularly Web API) is a serious option for both personal and business applications and not an obscure way to expose your database (don't laugh, that's probably the most recurrent argument OData bashers have and I read it a few times on #aspnetvnext :smile:).

Out of curiosity, is the ASP.NET team still involved in the development of Web API OData? (I would say 'no' according to the recent commits list: https://github.com/OData/WebApi/commits/master).

'Ipatch' of entity imho ends up being nothing more than having an apply method. And each patch mechanism will have his own error handling exceptions being thrown, etc. So I'm very hesitant to agree with this suggestion. It's better to avoid unnecessary abstractions, that can always be added later when a read need arise. I can't think of a strong need at the moment.

Understood (even if a IPatch<> should have, of course, far more methods: GetChangedPropertyNames, GetUnchangedPropertyNames, methods to remove changes made to particular property or to apply the patch on specific properties only and methods to validate the changes).

[yishaigalatzer]

The odata team works hand in hand with the ASP.NET team. When it comes to mvc6 we are attempting not to just move over the code and make it work, but reflect on the limitations, and pitfalls we had and redesign appropriately. I think the exercise you are doing is useful, in my experience with mvc6 work so far, these types of attempts (including some of mine) typically end up as thought exercises more than actual shippable code.

[kevinchalet]

The odata team works hand in hand with the ASP.NET team.

But you ignored that the OData team was considering porting Web API OData to vNext. Meh.

When it comes to mvc6 we are attempting not to just move over the code and make it work, but reflect on the limitations, and pitfalls we had and redesign appropriately. I think the exercise you are doing is useful, in my experience with mvc6 work so far, these types of attempts (including some of mine) typically end up as thought exercises more than actual shippable code.

I'm convinced, sir: I'll wisely wait for you to start working on a real OData vNext version and stop wasting my time on these "exercises".

Thanks :bow:

[yishaigalatzer]

I wasn't ignoring it. Been on regular calls with Congyong. They are going to do the porting. We are well coordinated, and the plan was to do a design before starting the work in earnest.

I'm not trying to dissuade you from doing the work. It is useful, I've seen too much disappointment in the past when stuff gets scrapped or heavily modified. I applaud you for the care and effort. I'd hate to see you drop it.

[abpatel]

Any updates on this item? We are using Delta in regular ApiController with Webapi 1.1 We use PATCH extensively (No ODataControllers) but since in WebAPI 2 it appears that Delta does not work with JSON serializer as expected. Will this ever be natively supported?

[niemyjski]

Any updates