webmachine-ruby icon indicating copy to clipboard operation
webmachine-ruby copied to clipboard

Support 'Precondition Required'

Open SebastianEdwards opened this issue 11 years ago • 12 comments

Is there any interest in supporting the Precondition Required response in the decision tree? The status code definition can be found at http://tools.ietf.org/html/rfc6585#section-3.

The change would add the following:

  • A new callback on resource called precondition_required? which would default to false. This would commonly be used to check for the presence of an If-Match header.
  • A new decision node between G7 and G8 which will return code 428 if the precondition_required? callback returns true.

Very happy to prepare a pull request if there is interest in this.

SebastianEdwards avatar May 07 '13 09:05 SebastianEdwards

Hmm, on second thought, the proposed semantics aren't quite right. It really requires a pair of callbacks precondition_required? AND precondition_present?

precondition_present? could helpfully default to checking the If-Match header as this is the common use case.

SebastianEdwards avatar May 07 '13 21:05 SebastianEdwards

First run at this: https://github.com/SebastianEdwards/webmachine-ruby/commit/4faa45bd855d5a6463bff90f082277e8c9febe47

Happy to take a stab at amending the HTTP flowchart if we have consensus on decision node placement.

Thoughts?

SebastianEdwards avatar May 07 '13 23:05 SebastianEdwards

Summoning @justinsheehy as he also might have some thoughts regarding the decision tree

ghost avatar May 07 '13 23:05 ghost

My initial thought would be to put it in the leftmost column with other simple tests, as it seems that it can be determined statically without depending on anything else in the decision flow.

A quick reading of the RFC section indicates to me that only the "required" callback is needed. The presence of any conditional request header ("If-*") would satisfy the requirement.

I would also support adding this to the original Webmachine if a PR happens to appear there.

justinsheehy avatar May 08 '13 01:05 justinsheehy

I concur with @justinsheehy, it seems independent of other decisions, so the earlier the better.

seancribbs avatar May 08 '13 02:05 seancribbs

Thanks for the input @justinsheehy.

It had occurred to me to use the leftmost column. However, since conditional headers check against existing resources, I thought it made more sense to insert it after it's ascertained that the resource exists.

The reasoning for a precondition_present? callback is that in practice most preconditions are not strong enough to guarantee a conflict will not occur. For example: an If-None-Match header, while useful for caching, is completely useless for conflict avoidance. Users usually wish to define a subset of acceptable preconditions depending on their use case.

SebastianEdwards avatar May 08 '13 04:05 SebastianEdwards

@SebastianEdwards Somewhat off-topic, but can you explain why If-None-Match is useless for conflict avoidance? It seems to me that if two clients are trying to update a resource, checking ETags would ensure nobody writes what they haven't read.

samwgoldman avatar May 12 '13 14:05 samwgoldman

@samwgoldman an If-None-Match header would allow the write to occur if the client provides any ETag which DOES NOT match the current one. On the other hand, an If-Match header checks that the ETag DOES match the current one, ensuring the client has at least read the most current version of the resource.

SebastianEdwards avatar May 12 '13 20:05 SebastianEdwards

For what it's worth, If-None-Match is generally only used with "*" as the value, preventing blind overwites of resources that have already been created. It's a similar problem to doing a PUT with If-Modified-Since. Who clobbers things intentionally?

seancribbs avatar May 12 '13 21:05 seancribbs

Sorry, the other usage of If-None-Match is obviously conditional GET, where you want a 304 Not Modified if the ETag(s) still match.

seancribbs avatar May 12 '13 21:05 seancribbs

Ah, I haven't seen the use of If-None-Match: "*" in the wild before. Makes sense though.

SebastianEdwards avatar May 12 '13 22:05 SebastianEdwards

@SebastianEdwards Have you made any headway on code/tests for this feature?

seancribbs avatar Jul 18 '13 13:07 seancribbs