jasminb / jsonapi-converter

JSONAPI-Converter is a Java/Android library that provides support for working with JSONAPI spec
Apache License 2.0
273 stars 81 forks source link

Updating resources via PATCH #168

Open marceloverdijk opened 6 years ago

marceloverdijk commented 6 years ago

When updating a resource by sending a JSONAPI document via a HTTP PATCH, is it possible to determine which fields were not part of the request body?

E.g. if I don't include the person first-name in the the payload it should not be updated. As JSONAPIDocument<Person> get returns a Person it is not possible to see if first-name was actually send as null (meaning updating it to null) or committed from the payload so it should be updated.

I guess this is a limitation in the library, right?

workmanw commented 6 years ago

@marceloverdijk We also have this problem. What we ended up doing was overriding the Jackson Mapper's treeToValue public function and we store this data on a thread local so it's accessible later. This is a less than ideal solution and we'd really like to get some help to make this better. I'd be happy to take on the work, if we could come up with a sensible solution.

I honestly don't know what would make this better. It feels like there are so many edge cases related to your domain. For example, what if your Person has included models that are also patched. Do you want to track that?

I've been wondering if a solution would be to make this library a little bit more extensible by adding some basic events like Dozer has (http://dozer.sourceforge.net/documentation/events.html). I'm not sure, but I would love to hear what others think.

jasminb commented 6 years ago

One way to tackle this would be to store raw Node that was deserialised into model as an attribute in JSONAPIDocument.

New method could be added than (to JSONAPIDocument that takes T, eg:

T patch(T toPatch);

Or:

T getPatched(T toPatch);

Where toPatch is entity read from data store and result is patched state.

Patching could be achieved by converting toPatch into Node and simply apply incoming data to it where rules are if incoming data has a value for an attribute (even null) overwrite existing attribute.

After patching is done, convert node back to target type.

Edit: Added alternative method name.

marceloverdijk commented 6 years ago

I don’t need a patch at the moment (get only api so far), but I would look into this approach: https://github.com/FasterXML/jackson-databind/issues/578#issuecomment-58443025

It means adding a flag for each property in the Person class (or keep a map of flags).

marceloverdijk commented 6 years ago

Also this SO discussion is quite useful: https://stackoverflow.com/questions/38424383/how-to-distinguish-between-null-and-not-provided-values-for-partial-updates-in-s

workmanw commented 6 years ago

@jasminb That is an interesting idea. You'd have to also keep the included nodes too though, so it may be a less trivial interface.


@marceloverdijk Yea we've considered this approach too. The problem is it requires you to be in control of the model definition that you're mapping and it does add quite a bit of boilerplate.

I was trying to figure out if there was a way to make this work with Lombok and delegates, but could not come up with one.

bensullivan commented 5 years ago

Hi @jasminb

Could you please shed a bit more light on how I could get a handle on the raw Node?

One way to tackle this would be to store raw Node that was deserialised into model as an attribute in JSONAPIDocument.

Many thanks

Ben

jasminb commented 5 years ago

@bensullivan, currently its not available. I could add it to the JSONAPIDocument and make a method available.

bensullivan commented 5 years ago

Hi @jasminb

Is the patch method on the JSONAPIDocument something that's in the pipeline anytime soon? Or even perhaps just making the raw Node available would be really helpful.

Many thanks

Ben

jasminb commented 5 years ago

Hello @bensullivan,

I will make the raw Node data available as a workaround until i get the time to come up with proper solution.

Edit: This change is now available on develop and in SNAPSHOT mvn repo.