cmiles74 / bishop

A Webmachine-like library for Clojure
Other
95 stars 10 forks source link

The callback function :is-conflict? should be called for POST requests #11

Closed aviflax closed 12 years ago

aviflax commented 12 years ago

I was confused why my resource wasn’t behaving as I expected it to. Finally I looked up resource.clj and saw that the comment on :is-conflict? says “Returns true if the provided PUT request would cause a conflict” — this limitation seems incorrect. There are many cases wherein I might want to return a 409 when handling a POST request.

cmiles74 commented 12 years ago

The handling of POST request by Webmachine is kind of complicated, the chart of decisions lays out how it gets from request to response. You are correct, there's no path for a POST request that will lead it to a node where the :is-conflict? callback is invoked. The only way to get there is with a PUT request; a PUT to a non-existant resource will land the request at P3 while a PUT to an existing resource will move through O14.

I'm sure it's a cold comfort, but this looks like how it's supposed to work. I'm not saying that we can't change it, just that this matches with Basho's HTTP 1.1 Activity Chart. Looking over the HTTP 1.1 specification, they clearly state that they expect PUT requests are the most likely to cause the 409 response but they don't actually state that's the only way it can happen.

10.4.10 409 Conflict

The request could not be completed due to a conflict with the current state of the resource. This code is only allowed in situations where it is expected that the user might be able to resolve the conflict and resubmit the request. The response body SHOULD include enough information for the user to recognize the source of the conflict. Ideally, the response entity would include enough information for the user or user agent to fix the problem; however, that might not be possible and is not required.

Conflicts are most likely to occur in response to a PUT request. For example, if versioning were being used and the entity being PUT included changes to a resource which conflict with those made by an earlier (third-party) request, the server might use the 409 response to indicate that it can't complete the request. In this case, the response entity would likely contain a list of the differences between the two versions in a format defined by the response Content-Type.

(I've added the emphasis.)

I'm not clear on why this is the case. Let me see if I can find a Webmachine developer in IRC or on a mailing list or something and run this past them. They've spent more time thinking about this than I have and they might have a good reason. Or not. :-P

cmiles74 commented 12 years ago

It seems that Webmachine is preferencing PUT for update requests, the handling of POST requests for anything but creation appears to be kind of a catch-all. My suspicion is that PUT is preferenced because you can presume the operation is idempotent, with a POST request anything could be happening.

Is this a place where you could just as easily use a PUT request? I'm hesitant to diverge much from the other Webmachine ports but it's not out of the question.

In the short term, you could always return a 409 from your :process-post handler.

aviflax commented 12 years ago

I’m not going to change the design of my API (changing this operation from POST to PUT) just to work around this. In the short term, I’m already returning a 409 from my :process-post handler. But that’s not satisfactory. I’m pretty opinionated on HTTP, and from my view it’s perfectly valid for a server to respond to a POST request with a 409 status code; therefore a framework structured like Bishop should do the intuitive thing and call the callback which would result in said status code.

I’m sympathetic to Webmachine preferring PUT to POST — it’s hard to argue with — but until HTML forms support PUT, full-fledged and robust POST support is crucial. This is an area where I think it’d be good for Bishop to diverge from Webmachine.

cmiles74 commented 12 years ago

I agree with you that it's perfectly valid for a server to respond to a POST update with a 409 status code. It's also pretty unlikely that web browsers are going to start supporting idempotent PUT with form content anytime soon. This is likely how we ended up on opposite sides of this issue; Webmachine sees it's role as serving a programmatic REST client, perhaps one written in Javascript.

I'm not looking to cripple Bishop's POST support either, but in my day-to-day work I'm also concentrating on web services that support Javascript clients. It's been easy to overlook this issue of supporting a web-browser post directly.

Let me think about this a little bit more before I make any changes. I'm thinking that an additional node could be added around N16, where we do call that :is-conflict? handler. I don't think that should cause any problems, it doesn't seem like that big a deal (i.e., this handler is still only getting called once, etc.)

cmiles74 commented 12 years ago

Okay, release 1.1.4 now calls the :is-conflict? handler before calling the :process-post handler, just like it does when handling a POST to an existing resource.

aviflax commented 12 years ago

Thanks Chris!