zalando-stups / lizzy

REST Service to deploy AWS CloudFormation templates using Senza
Apache License 2.0
21 stars 17 forks source link

Using request body (json payload) for DELETE http requests #151

Open kaygorodov opened 8 years ago

kaygorodov commented 8 years ago

DELETE /api/stacks/{ID} uses request body payload.

It is a little bit unusual to utilize a request body for DELETE http requests. Many HTTP client libraries don't support it, so it forces a developer to construct requests from low level. Another thing, I guess that some popular web servers if use them as proxy, cut body for DELETE requests as they do for GET by default, so it requires additional configuration for them.

rafaelcaricio commented 8 years ago

Adding reference: https://github.com/zalando/lizzy/blob/release2.0/lizzy/swagger/lizzy.yaml#L182-L186

Good point. We will have to review that.

jmcs commented 8 years ago

From rfc2616:

The presence of a message-body in a request is signaled by the inclusion of a Content-Length or Transfer-Encoding header field in the request's message-headers. A message-body MUST NOT be included in a request if the specification of the request method (section 5.1.1) does not allow sending an entity-body in requests. A server SHOULD read and forward a message-body on any request; if the request method does not include defined semantics for an entity-body, then the message-body SHOULD be ignored when handling the request.

So unless the body is explicitly forbidden (and it isn't for DELETE), then it's allowed. If an HTTP client library or proxy don't support the body in DELETE requests then it's a bug on their side.

kaygorodov commented 8 years ago

From the HTTP wiki page:

In June 2014, the WG released an updated six-part specification obsoleting RFC 2616: ...

(HTTP/1.1): Semantics and Content, https://tools.ietf.org/html/rfc7231

A payload within a DELETE request message has no defined semantics; sending a payload body on a DELETE request might cause some existing implementations to reject the request

But, it is kind of your API design decision, and you of course can use payload for DELETE requests. I just pointed out that it is unusual.

lasomethingsomething commented 7 years ago

Hey @jmcs @rafaelcaricio, anything more to do here? Otherwise, let's close? cc @kaygorodov

DRAirey1 commented 5 years ago

I just want to point out that HTTP DELETE without a payload is an incomplete design. Every multi-user web service needs a concept of Optimistic Concurrency if the users share resources. The only way to prevent one user from overwriting the changes of another user is with Optimistic Concurrency. The only way to get Optimistic Concurrency is by passing a row version or time stamp in the DELETE request. Further, atomic deletes are a requirement for most professional systems. If a POST can atomically add some set of resources (let's say stocks in a request to BUY), then you MUST support the ability to atomically delete that same resource (you can't possibly delete some of the stocks in a BUY and leave open the possibility that they aren't all deleted). The REST API spec is incomplete/brain damaged without the explicit ability to provide a payload in a DELETE call.

IMSoP commented 5 years ago

The only way to get Optimistic Concurrency is by passing a row version or time stamp in the DELETE request.

That sounds like metadata which would be more appropriately indicated using request headers rather than a payload. Consider the parallel case of PUT, where the payload should indicate the desired state of the resource, not pre-conditions for the action being taken.

DonaldAirey commented 5 years ago

That sounds like metadata which would be more appropriately indicated using request headers rather than a payload. Consider the parallel case of PUT, where the payload should indicate the desired state of the resource, not pre-conditions for the action being taken.

It's not metadata. In a larger sense, the version is part of the identity of the resource.

adjenks commented 5 years ago

I would put them in the URL parameters. Time stamp DELETE /resource/1234?timestamp="2019-11-13T22:12:19.435Z" or row version DELETE /resource/1234?row_version=2

DRAirey1 commented 5 years ago

We all know how to use the Query parameter. My understanding is that this topic has to do with the proper interpretation of REST and, to my point, is REST a complete specification for realistic, LOB APIs without the ability to delete the same resources that have a row version?

RovoMe commented 4 years ago

@DRAirey1

I just want to point out that HTTP DELETE without a payload is an incomplete design ... Further, atomic deletes are a requirement for most professional systems. If a POST can atomically add some set of resources (let's say stocks in a request to BUY), then you MUST support the ability to atomically delete that same resource (you can't possibly delete some of the stocks in a BUY and leave open the possibility that they aren't all deleted).

You've got a completely messed up view of what HTTP is and what it should be used for. As Jim Webber pointed correclty out HTTP is a protocol whose application domain is the transfer of documents over the Web. It is NOT your application protocol! Some side effects of the document management causes some business activities to occur, but at it's core HTTP still just sends a document per request around and in that regard HTTP's operation are well-fitting. You place a document somewhere, you get a unique identifier in form of a resource locator (URL/URI) back and can then perform further document management operations on that resource, such as removing it.

If you need to remove something within that document you either can send a new version of that document to the server and ask it to replace the document with the new one (-> PUT), send some instructions the server should perform to modify the resource to a desired state (-> PATCH) or send a document the server knows how to act upon accordingly (-> POST). DELETE is for sure the wrong operation as this basically asks the server to remove the link between the URI and the resource. It doesn't even guarantee that the actual document is removed from its storage location to start with, just that a further GET operation triggered on the same URI doesn't return the previously stored document.

The POST operation is defined to process the payload according to the server's own semantics. At no point does the HTTP spec state that it has to process the request atomically! PATCH does, but not POST. In addition to that, HTTP works best on single resources, not on batches! You might create multiple resources at once but how do you inform a client about that? Send multiple Location headers within the response? Use one Location header with multiple URIs in it? Both of these attempts are also not standard conform and thus lack the support for interoperability.

Every multi-user web service needs a concept of Optimistic Concurrency if the users share resources

HTTP has something similar to optimistic locking present. Please read through RFC 7232

So, please, before stating that HTTP is poorly defined, read through the spec, grasp what its purpose is and then reevaluate and formulate your statement as currently you seem to lack proper knowledge of the protocol you complain about.

My understanding is that this topic has to do with the proper interpretation of REST and, ..., is REST a complete specification for realistic, LOB APIs without the ability to delete the same resources that have a row version?

REST isn't a specification, REST is a thesis of an architectural style which provides some long-term benefits over RPC-based solutions such as the freedom to evolve the service over time without having to fear breaking clients due to changes done to the server side. The ultimate goal in a REST architecture is that a service is able to serve a plethora of clients, especially those not under your direct control, and a single client is able to operate with plenty of different APIs without having to constantly being modified. This is basically only possible if the whole architecture, including APIs, intermediaries and clients, is carefully designed and implemented. It is to easy to introduce coupling and therefore a lot of discipline is required before the benefits a REST architecture advocates are visible.

All REST therefore does is define a set of constraints that if followed constantly will path the way to a system that is highly decoupled, failure tollerant and evolvable. The details of the actual communication are still kept at the transport level, i.e. HTTP in almost all cases. And as mentioned before, HTTP basically ships just documents around but does not trigger or invoke business processes directly. We put some business level semantics onto some of the document management processes and trigger certain business level rules and/or actions based on such events. Hence, neither REST nor HTTP can and should be blamed here.