Mvc partial json fields

[robin-thoni]

This change adds an option to ignore fields parse errors and a service to store and access parsed fields via dependency injection so they can be used to select from a database, be forwarded to another API, etc

[dotarj]

Hi, thanks for your PR. I like the idea that information about the fields parameter can be obtained through a provider. I've added some comments to the PR suggesting to add an interface, which makes it possible to add a custom fields provider. I've also added some naming suggestions trying to be consistent with the ASP.NET MVC framework itself. The build has failed due to a problem with the build, I'll fix that.

[robin-thoni]

Sorry for not replying, I have some higher priority stuff to handle ASAP. Will come back here after.

[zihotki]

@robin-thoni hey, will you be able to finish the PR?

[robin-thoni]

Yeah, sorry for this HUGE delay, I was thinking about it a week ago, but i was considering another approach for the service, cause I'm using (well, I'm starting to use) partial fields as partial output (response) AND input: the goal is to be able to insert objects without specifying all fields, if they have some default values. This also allows to update one object field without sending the whole object: it uses less data and is can avoid user2 replacing edits from user1 if they edited different fields.

Although it's not directly related to the MVC package, I made a PR on Automapper (https://github.com/AutoMapper/AutoMapper/pull/2892) to allow partial copy using partial fields. I might do a package to support OOTB interaction between PartialResponse and Automapper.

Also, I was thinking to implement a negative filter in the PartialResponse core package: sometimes, you might want to get all the fields except some, without having to maintain the field list in the frontend, it could be something like *,-someBigFieldIdontWant. which would translate to all fields except "someBigFieldIdontWant".

[zihotki]

Yeah, sorry for this HUGE delay, I was thinking about it a week ago, but i was considering another approach for the service, cause I'm using (well, I'm starting to use) partial fields as partial output (response) AND input: the goal is to be able to insert objects without specifying all fields, if they have some default values. This also allows to update one object field without sending the whole object: it uses less data and is can avoid user2 replacing edits from user1 if they edited different fields.

In regards with that you may consider looking at Json Patch, it was created especially for that purpose - to be able to update only specific fields, and it's supported out of the box in Asp.Net Core. Also I don't see any reason why it can't fit also for creating objects, just create one with default data and apply json patch on top of it.

And Negative Filter seems to be a good idea too, although it'd be better to do it as a separate PR otherwise there would be too many different changes.

[robin-thoni]

Sure, it will be in another PR. Thanks for Json Patch, I'll take a look at it!

[robin-thoni]

@dotarj What do you think now?

[dotarj]

Hello @robin-thoni, thank you for the updated pull request, but did you see the comments I added in the previous iteration? It seems most (if not all) of the comments are not addressed.

Other than that, I really doubt if the MvcPartialJsonOptions.FieldsParamName property is a good addition. Did you have the need to use a different name, or did you have another reason to add the property? Another approach would be to make it a const field in DefaultFieldsResultProvider (MvcPartialJsonFields) and removing the paramName parameter from GetFieldsResult. This would make GetFieldsResult significantly less complex (due to the caching per paramName).

Again, thanks for the involvement. Maybe we can work together on this pull request. With GitHub, repo owners are enabled to update branches of contributers. Let me know if I can help.

[robin-thoni]

hmm, I don't see any comment :)

The base idea is to be able to parse different query parameters, as I said in my previous comment, to be able to also use it as a partial input (@zihotki gave a possible alternative with JSON Patch, but now it's almost done), or to be able to have multiple partial response filters (I don't have have any use case for this, but could be useful).

Yes, sure, go for it, if you think it can be improved!

[dotarj]

Ah, my bad. I assumed that the comments were already visible for you. I completed the review now, so I guess you can see the comments now. I will make some modifications to the branch and get back to you.

[robin-thoni]

I don't really understand what you have against the MvcPartialJsonOptions.FieldsParamName field. Can you elaborate on this? The way I did it was keeping the current behavior by default and was allowing further customization if needed.

[dotarj]

Hi @robin-thoni, the reason I removed the FieldsParamName property is because the same (and more) can be achieved by creating a custom implementation of the IFieldsParser. I would rather keep the public API as small as possible to minimize the chance of making breaking changes in the future. You suggested the make it possible to use another name for the fields parameter, someone else might want to pass the fields parameter using a header. Instead of then adding another configuration property, both scenario's can be achieved using IFieldsParser, thus minimizing the public API. Could you please take another look at the PR? I think it can be merged by now.

[robin-thoni]

I understand you point of view, that makes sense to me. I'll have a look at your changes before the end of the week.

[robin-thoni]

This allows what I wanted, LGTM!

[dotarj]

Hi @robin-thoni, these features are now available in version 5.0.0.