allan-simon / sentence-aligner-rust

rest service + frontend to align sentences , in rust
13 stars 1 forks source link

409 errors should give back the link to the "conflicting with" resource #54

Closed allan-simon closed 6 years ago

allan-simon commented 6 years ago

so that the client know what id he should reference instead.

jean553 commented 6 years ago

What would be the returned content exactly ? JSON or plain text ?

Something like:

{
    "conflicting_with": "[id]"
}

or smthg else ?

allan-simon commented 6 years ago

i let you check the state of the art on this ?

allan-simon commented 6 years ago

(i.e I have no opinion yet on it)

jean553 commented 6 years ago

From this thread (https://stackoverflow.com/questions/3825990/http-response-code-for-post-when-resource-already-exists):

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.

So, I guess this is closely related to the API and the 409 HTTP meaning. For instance, for POST /sentences, the documentation stipulates that a 409 is returned when "The given UUID is already used by another sentence.". Returning the UUID is not required in order to solve the conflict as the client already knows the UUID it has just sent.

On the contrary, PUT /sentences/{sentence_id}/language request body only contains the language. The 409 response says "A sentence with identical language and content than the edited sentence already exists.". In that case, the client doesn't know about the conflicting sentence or the conflicting sentence "content" (it only knows about the language), so returning the UUID that conflicts makes sense here.

Should we implement the 409 responses that way ?

allan-simon commented 6 years ago
So, I guess this is closely related to the API and the 409 HTTP meaning. For instance, for POST /sentences, the documentation stipulates that a 409 is returned when "The given UUID is already used by another sentence.". Returning the UUID is not required in order to solve the conflict as the client already knows the UUID it has just sent.

=> it is also the case if the language, text is already used

so maybe we should return the "conflicting" entity in all cases ?

jean553 commented 6 years ago

Correct. Let's to that. Fixed the doc https://github.com/allan-simon/sentence-aligner-rust/pull/60.

jean553 commented 6 years ago

Should we return only the sentence UUID then ?

allan-simon commented 6 years ago

I think returning the whole entity would make more sense, so that you can reuse the same "entity" in all the 409 cases

jean553 commented 6 years ago

So we agree that for instance for the edit_sentence_text function, we would have two SQL commands, one to update the sentence and another one to get the whole conflicting object (in case of conflict). Right ?

allan-simon commented 6 years ago

Hmmm if that's the only way, yes, I thought by using the RETURNING clause postgres we could avoid that , but two sql requests is fine too.

jean553 commented 6 years ago

Done for every edition/creation API that might trigger a conflict.