Skip to content

Loading…

Uniqueness Middleware should work on HTTP respons status codes #21

Closed
bcardarella opened this Issue · 8 comments

2 participants

@bcardarella

Using response status codes is better than parsing 'true' and 'false'

@jhchabran

Hello, can you please explain us why you went for http statuses instead of response bodies ?

@bcardarella

Response codes are just as easily detectable in AJAX and better articulate the intention. The uniqueness validator is seeing if the resource already exists in some form. If it doesn't a RESTful system should show that through a 404 Not Found Status Code instead of a 200.

I'm open to counter arguments if there are any. :)

@jhchabran

Thanks for your answer :)

We've been discussing this with friends and so far we come up to theses points so far :

  • HTTP Statuses are meant for transport whereas a validation comes at the applicative level : this mean your solution works perfectly in common conditions, but what will happen if its hosted behind a varnish which might cache the 404 ? We may freeze this validation answer until varnish empty its cache

  • A 404 is basically an error code, it usually trigger alerts when you're supervising your app, so we'd be triggering them with false positive there.

  • A GET intends to just grab a resource without doing any destructive action and should not be parameterized. Obviously a validation isn't destructive but it still poses an issue as the resource still depends on a parameter to be retrieved, ie the value we're going to validate as a unique one.

Considering all of these points we came to the conclusion that a POST request with a body (true or false) would be the simplest way to express validity. A validation sounds like an applicative thing and should be responded in that way, meaning just true or false.

We also thought about the 422 but it means our input data is invalid semantically where it currently is here, as any value is understandable by the uniqueness validator is correct even if does not validate.

We're also open to discussion there, it's quite an interesting problem (generating trolls in our office haha) :D

@bcardarella

Wow, thanks for the great thoughts. You guys definitely put some good thought into this.

So a little history, originally this method did a GET and would return 200 OK with 'true' or 'false' in the body. (this is why the path still has '.json' on the end) For me this seemed like unnecessary work as the status codes should be expressing the nature of the resource. Unfortunately there was no status code that I saw that was a logical fit, so I chose the generic 404 as it 'kind of' implied what I was after and called it a day.

  • You're right about the caching issue. That alone is enough to convince me to change it. Makes me wonder though, Rails raises 404 when ActiveRecord::RecordNotFound is raised. It seems this would create the same issue with Varnish.

  • The 400s all imply an error on the client of some type. 'Error' is being loosely used in the 400 block and can mean anything from malformed headers to incorrect information being sent (in the case of bad username/password) Wikipedia has a somewhat comprehensive list here: http://en.wikipedia.org/wiki/List_of_HTTP_status_codes#4xx_Client_Error
    but none fit this use case. Maybe it makes sense to define my own 400 value, I could just pick '480' out of a hat, but it would be nice to know that the status code isn't reserved at all.

  • I'm not convinced that a POST should be used. To me when I see POST it implies destruction or update of some kind before anything else. GET implies data retrieval. I'll even go one further, if this were to rely on HTTP status codes alone then it should actually be using HEAD. But I don't know how that would play out with jQuery and the number of browsers this should support.

I'm open to going back to the previous form of using the body to denote validity, I just don't want to do so if it isn't necessary. What is also important here is that the request is not checking for validity of the data. It is checking if the data exists. The validation check still occurs on the client based upon the result of request.

@bcardarella

According to this diagram: http://webmachine.basho.com/diagram.html

412 might be the best fit

@jhchabran
  • About Rails raising a 404, if we cache a 404 Varnish would not invalidate it until it sees a POST against it, meaning the resource had been created again. That won't happen if we're using only GETs.

  • Given how you're considering the request as a presence check instead of a validity check that 404 is semantically valid

  • If you consider it as just a presence check a POST doesn't make sense, but it would solve all issues as it's never cached, which may push us to consider it as a validity check making it semantically valid and praticable in any cases.

Anyway we're using your gem as it is and it works fine :D that generated some interesting thoughts in the process.

@bcardarella

So all of the ClientSideValidation middleware is namespaced under /validators/ url. It would probably make sense to ignore that route for Varnish caching.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.