resthub / resthub-spring-stack

RESThub Spring stack
http://resthub.org/spring-stack.html
Other
121 stars 66 forks source link

Support PATCH updates #164

Open sdeleuze opened 11 years ago

bclozel commented 11 years ago

See this issue: https://github.com/resthub/springmvc-router/issues/8

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

sdeleuze commented 11 years ago

modelmapper with a custom strategy ?

manosbatsis commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

manosbatsis commented 10 years ago

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.