Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support PATCH updates #164

Open
sdeleuze opened this issue Dec 14, 2012 · 14 comments
Open

Support PATCH updates #164

sdeleuze opened this issue Dec 14, 2012 · 14 comments
Milestone

Comments

@sdeleuze
Copy link
Member

No description provided.

@bclozel
Copy link
Contributor

bclozel commented Dec 14, 2012

See this issue: resthub/springmvc-router#8

I'm wondering how to handle partial updates, though...

@sdeleuze
Copy link
Member Author

modelmapper with a custom strategy ?

@manosbatsis
Copy link

I was thinking of just duplicating ServiceBasedRestController#update to ServiceBasedRestController#patch, then load the persisted entity and update from all non-null members of the incoming resource. However this wouldn't handle updates to null. Would a model mapper allow easy type conversion to a map instead of a resource class instance? That would allow passing member names as keys with null values in the map, to signify updating to null.
I might work on a PR

@manosbatsis
Copy link

Is it possible to assign an object mapper or other impl per resource class and HTTP method to deserialize to a map and pass that to the controller method?

@bmeurant
Copy link
Member

It is a complex subject because we will have to deal with relations 1-n, n-1, n-n, null members (and especially null relations).

Maybe modelmapper could help but I don't think it is sufficient.

I think we must support a standard patch format : cf. http://tools.ietf.org/html/rfc6902

PATCH support is strongly related to a larger subject : #237

@bmeurant
Copy link
Member

Must take a look to converters : https://github.com/resthub/resthub-spring-stack/tree/master/resthub-web/resthub-web-server/src/main/java/org/resthub/web/converter that are responsible for serialize and deserialize json/xml from/to Java Objects.

related to #239 and Param converters listed in #237

@manosbatsis
Copy link

For me n-1 would simply work because it is never the "inverse" part of the relationship per JPA terms. I wouldn't mind PATCH offloading the responsibility of relationship ownership to the persistence code by default. Alternatively, for JPA at least, With that said, it wouldn't hurt if i was able to implement 1-n, n-n or inverse n-1 using a custom objectmapper for PATCH. For example it should be easy to lazy-load with jackson-datatype-hibernate [1] whille deserializing in my case when needed?

[1] https://github.com/FasterXML/jackson-datatype-hibernate

@manosbatsis
Copy link

Overall I don't think that a ServiceBasedRestController#patch needs to be more responsible on relationships any more than ServiceBasedRestController#update is. They both update a resource and more specific requirements in deserialization and persistence should be handled somewhere else as appropriate.

@bmeurant
Copy link
Member

n-1 subject is strongly linked to our global thinking about API and usage in #237 : how do we want to manage relashionships ?

JPA part could possibly be addressed by modelmapper and some libs like jackson-datatype-hibernate but for now we are not coupled neither to Hibernate nor to JPA (cf MongoDB) and must manage everything in a generic way.

We maybe can take a look to spring-data-rest and spring-data-commons ways to handle these points.

We also must thinking about param converters to keep our Java API clean.

And you are right, we must find the proper place to implement these features that could not possibly be managed by controllers. Serialization/Deserialization could be manages in converters ...

I think I have to think more about that and make some POC tests

@manosbatsis
Copy link

I suppose data-mongodb simply passes the incoming resource document as-is for an update? Would a subset overwrite the whole document or just the relevant parts? Similarly to JPA, the n-1 might also be close for mongodb, at least as far as the default behaviour of controllers looks relevant

@manosbatsis
Copy link

I would also suggest it is easier to support PATCHing a subset of a JSON model initially and implement rfc6902 for accept header "application/json-patch+json" down the road. This is convinient for clients as well as they would be able to use the first case with little effort and keep the RFC option open

@bmeurant
Copy link
Member

Right. Obviously web will start with a pragmatic subset of RFC. I dont think that we will support full RFC one day ...

But we will target this format

@bmeurant
Copy link
Member

I have to check about mongo and I have to digg into spring-data-commons ans spring-data-rest to see possibilities

@manosbatsis
Copy link

My understanding of spring-data-commons is that it only interests repository iimplementations.

spring-data-rest might be the right path to many features and a nice option to have. Maybe it can be great for wiring up generic converters to handle PATCH deserialization. But i wouldn’t want to make it my only choice in resthub. I think it can coexist with how things are and everything will find it's place down the road.

@bmeurant bmeurant modified the milestones: 2.2.1, 2.2.0 Oct 7, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants