zfcampus / zf-apigility-doctrine

Doctrine-enabled services for Apigility
BSD 3-Clause "New" or "Revised" License
107 stars 50 forks source link

PUT behaves like PATCH, but it shouldn't #102

Closed Thinkscape closed 8 years ago

Thinkscape commented 10 years ago

By common understanding, the difference between REST PUT and PATCH is that PUT expects full entity information (all fields) that will be injected in place of all existing fields. PATCH on the other hand only touches fields that are provided in the payload, leaving other values intact.

Currently there is no difference in the way PUT (update()) and PATCH (patch()) perform updates on an entity.

See also #100

TomHAnderson commented 10 years ago

This is correct and now the update() and patch() are the same. It feels limiting to me to make users give all values on an update() since the hydrator can change just those fields included in the request.

I am comfortable with how patch() is now. To modify update() to replace all values not specifically specified feels like more of a bad idea than having it work just like patch().

So unless there is a business case for making update() work specifically on an entity by replacing all values with those given in the request I'd rather keep this as-is.

Thinkscape commented 10 years ago

There is no business case b/c apigility is a library so it doesn't need one.

It's not correct per specification:

Current behavior does not reflect what PUT and PATCH methods were designed for.

To modify update() to replace all values not specifically specified feels like more of a bad idea than having it work just like patch().

Actually, sending an incomplete instance could be interpreted as an error.

Quote:

Use PUT when you can update a resource completely through a specific resource. For instance, if you know that an article resides at http://example.org/article/1234, you can PUT a new resource representation of this article directly through a PUT on this URL. - See more at: http://restcookbook.com/HTTP%20Methods/put-vs-post/#sthash.eHSpeh6h.dpuf

TomHAnderson commented 10 years ago

How can we ensure the $data sent to PUT is complete? There's no method on the hydrator to tell us what all the fields to be expected are.

Without knowing what a complete $data array should be for a PUT request I'll leave this open.

adamlundrigan commented 9 years ago

IMO it would be sufficient to rely on the InputFilter to handle that: if the input filter accepts the payload then it's a valid representation of the object. The trouble is then getting around the issue of not knowing what fields the hydrator expects. The only solution I see is replacing the entity currently persisted with the new one, but i'm not sure if Doctrine ORM supports that (possibliy through merge?).

Personally, I'd like to see patch work like described in RFC 6902 (tl;dr version here), but at this stage that's a pipedream at best (as the default behavior anyway, I know it can be achieved through adding a module)

Thinkscape commented 9 years ago

@TomHAnderson It's not zf-apigility-doctrine's responsibility to validate $data completeness. It's either input filter or hydrator, both which are user's responsibility. Then, there's default values in the model - many of my models will just assume values if not provided.

In other words, why would the controller care? The worst that can happen is a failed INSERT statement, which again, is an effect of invalid data being sent (same as any other underflow). For any extra logic one can write listeners, play with the controller or filter/hydrator.

TomHAnderson commented 8 years ago

I don't think there's anything we can do about this. Closing until someone has a solution.