Open rikvanmechelen opened 10 years ago
:+1:
I could answer of why it might also not be good myself.
Sometimes you post a resource (e.g. an order) server side you calculate the vat etc.. you might want to return these calculated values to the client side, to ensure there are no errors on the client side calculations.
However, i still think that in a lot of the cases a 204 could be a good response.
Not sure if PATCH
should return any data. DELETE
certainly not.
@rikvanmechelen I think, as you indicated, that both options can certainly have pluses and minuses. We opted for the case that was more consistent overall, in the hopes that it makes it easier to learn and reason about if you can always assume you'll get the serialization back. It is extra data over the wire in some cases, but I think as a default it makes sense in order to make it more approachable. I could definitely see adding advanced options which could allow you to specify that you didn't care about the response as an optimization, but I think keeping it simple/consistent in this way is a good starting point.
I'm going to put my :thumbsup: for 204 on a DELETE
. An API should describe the resource you've given it at all times - one good reason for doing this is to help clients synchronise their models to the new dataset. A DELETE
has no dataset, hence a 204 No Content
is the correct status and description of the resource.
@keithamus I guess I would make a small distinction from your comment. I'd say that the API should describe the resource you are referring to (rather than what you've given). If you start from that, I think it seems reasonable that delete would also give you a copy of the state of things. I would agree that in many cases this info is not needed (ie the synchronize case should be pretty trivial on successful delete). That said, I suspect there are cases where getting the resource back would be useful (though I can't name one off hand) and this allows it to be more consistent with other actions. It is a bit of extra data over the wire, but in most systems I do not expect delete to be so frequent as to be the bottleneck. ie I can see how it might be useful and it is more consistent, with relatively little downside, so to me consistency/convenience wins the day. What do you think?
@geemus Maybe I was unclear. I believe an API should describe the current (aka new) state of the object which includes the operation. In other words the response is saying "That was fine, here's what the new representation of this looks like". I would expect that after a PATCH
setting foo
to true
, the response back would have foo
as true
. Because of this, after a DELETE
, I would expect to not get any data back, because no data exists - or "That was fine, here's what the new representation of this data looks like: nothing"
I'd be interested to see what upsides there are to including it back down the pipe. Anyone?
@keithamus thanks, that is clearer and (to me anyway) much more compelling. I hadn't really considered it exactly that way, for whatever reason. A lot of my motivation had been around trying to find a single rule that could be consistently applied (ie return the resource), but as you describe it I think there can still be a consistent rule, it just is "return the resulting resource". I had also thought there might be cases where you would want to know the last-state of something and this would avoid having to do GET+DELETE in succession, but having had that thought I was never successful in coming up with a concrete use case. So perhaps there is not one. What do you think? Seems like we are at least nearing the same page. How would you reword the recommendation?
@geemus I've got a PR over at #41 which is my attempt at rewording this.
Great discussion over here! Thanks for getting the ball rolling on it.
I'm -1 on this change. We added the 200 with a resource representation for a couple reasons:
null
ed?), and not strong enough to overcome the practical advantages of just returning the body.Practical utility: Having a resource representation here is useful for consumers. For example, if I'm designing a client that allows resources to be removed by an arbitrary ID, but where I want to present a consistent message back, having a body handy makes this easy and collapses the number of API calls that I want to make:
heroku destroy -a 1234
App "cryptic-brushlands-7772" has been destroyed.
heroku destroy -a cryptic-brushlands-7772
App "cryptic-brushlands-7772" has been destroyed.
These are the same principles that were used to guide the development of these guidelines overall: if a decision needs to be made between practical usefulness and academic RESTful correctness, take the former.
I'm also a fan of having a JSON API (especially one where you request it via the Accept
header as you version) always return JSON. For example with the Heroku API, you can JSON parse any response, and it's always going to be safe to do so.
Lastly, I think there's value in preaching what we (Heroku) do here. It might be better not to tell other people to return 204's when we don't do it ourselves.
Once again, thanks for the contributions guys! Keep 'em coming.
@brandur - I also like consistency, but I think the argument for the resulting object (rather than the object when the action started) is pretty compelling. I guess this is one of the key points.
I agree with your messaging case, it may depend in part on how broadly applicable we think the friendly-id concept should be applied. We don't outline it in the guide (yet), and without that it makes this argument a bit harder to make. I think it could be pretty broadly applicable (uuids are not user friendly particularly). Maybe expanding on that would give us firmer footing here.
I also agree that I prefer usability to artificial measures of correctness.
JSON everywhere is commendable also. That said, we have a small selection of 204s in the Heroku API already which wouldn't be parseable (I think just around dyno actions maybe). Perhaps that is an exceptional enough case to deserve exceptions, but I'm wary of using sunk-cost as a point in favor or against (there are many things I'd love to change in our implementation).
Thoughts?
With my projects I use deleted_at
timestamps on all models. Permanently deleting a resource is a two step process. The initial delete
actually updates a deleted_at
timestamp on the model and returns the resource with it's deleted at time. Deleted models are then accessible in the collection's trash which returns all of models that have a deleted_at
value. They are permanently deleted once a second delete
request is performed on the trash.
From an end-user prospective it wouldn't make sense to send a PUT
request to trigger the initial delete. This is a use-case of how returning JSON with delete can be useful, IMO.
I wondered why one should not return a 204: No content in response of a successful Patch or Delete request.
Since when you delete a resource, you eighter already have the full resource, or could easilly get it before deleting. The same goes for a patch request.
By doing this, the response can easily be a few kb lighter, which is always good, no?