agrestio / agrest

Server-side Java REST Framework for easy access to data graphs from various backends
https://agrest.io
Apache License 2.0
81 stars 34 forks source link

Support for PATCH #39

Open andrus opened 10 years ago

andrus commented 10 years ago

PATCH is absolutely critical for any generic REST API to scale. We should investigate and start supporting RFC6902 (JSON Patch).

Also a useful link on the topic: http://kingsfleet.blogspot.be/2014/02/transparent-patch-support-in-jax-rs-20.html

andrus commented 9 years ago

As a part of the implementation we'll need to be able to parse JSON Pointers. It appears that Jackson core implements a JSON pointer parser.

I don't (yet) think we'll need a special parser for JSON patch itself, as we'll be applying the operations to the object graph, not to JSON.

andrus commented 9 years ago

Also a related spec is JSON Merge PATCH , which looks very close to to the current update mechanism. It should probably be a topic of a separate task and can probably be reconciled with our current updates with a bit of effort. Per spec merge is tied to a PATCH request, it defines simple and clear object property merge rules, and terminates merge at collections (which may actually be applicable to our model)

atomashpolskiy commented 8 years ago

@andrus, I've started to work on this one recently. So far it's going really smooth, thankfully to recent introduction of EntityUpdates. However there are a few issues, mainly due to some discrepancies between LR and RFC6902. You will help me a lot, if you share your thoughts on the following points:

1) According to RFC, add operation for array members should look like this:

path = _/path/to/array/element_ref_, where element_ref is one of {element_index, "-"}, and value is some object

I'd like to change/extend this a bit:

2) According to RFC, delete operation's target (specified in path) must exist This is kinda vague; I'd propose to interpret this wrt LR as "must be defined in the entity". Then if it's null, delete will just result in no-op. Of course in case target member is not defined in the entity, then delete will fail to execute.

3) According to RFC, delete operation for array members should look like this:

path = _/path/to/array/element_ref_, where element_ref is element's index

I'd like to change this so that:

4) Same rules are applied in RFC for replace operation' path attribute. But now element_ref seems to be absolutely meaningless. In LR it would be logical to allow element_ref to be absent for to-one relationships. For to-many relationships, replace operation itself hardly makes any sense...

5) multi-attribute IDs are probably not going to be supported in delete and replace, at least for now (I've no idea how to).

That's all for now, everything else seems to be fine. Thanks in advance!

andrus commented 8 years ago

Replacing my earlier generic comment with a more detailed description of the mismatch between the 2 RFC's (RFC 6901, RFC 6902) and what we are doing in LinkRest. The difference comes from the fact that the RFCs deal with generic JSON documents, while LR is dealing with JSON documents that represent a persistent entity graph, i.e. objects with global IDs and no preset ordering of relationships.

So the issues:

  1. Addressing objects by ID in collections (you've noticed this one). It is problematic when working with to-many relationships and when patching a root collection.
  2. Scope. We initially hoped to build some kind of mega-controller API that would update any set of records in DB, addressing them by ID, and then patching them. RFC6902 on the other hand is based on JSON pointers relative to the resource root. Meaning we are working in a more narrow scope (a single object resource or a collection resource). Which is actually fine I think. I like having the root context predefined.
  3. Unlink vs. delete. Since RFC6902 deals with JSON documents, not objects, it does not need such distinction. But LR needs a hint on what to do with target object(s) when a relationships is "deleted" or "replaced". Perhaps this semantics should indeed exist outside of LR protocol (e.g. in the Cayenne model, where we define whether a relationship's target is an independent object, or it can only exist when linked to the source). Though this may be too limiting.
  4. Ops that make no sense. "move" and "copy" do not in LR context. "replace" are only partially useful.

So.. it appears that we need to design our own pointer and patch formats, using RFCs as an inspiration, but no more than that. Other inspirations are Cayenne path expressions, and Cayenne graph change tracking API.

For the pointers we have a number of choices:

For operations it would be nice to use Cayenne GraphChangeHandler.java vocabulary (translating it to something more RESTful). Here we can address "unlink" vs. "delete" BTW... So one possible operations vocabulary may look like this: "add" , "delete", "unlink" (only 3 ops, but looks like this may handle the entire spectrum of graph editing)

Anyways, this is open for comments, and further discussion.