New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes to DELETE method handling #324

Merged
merged 2 commits into from Jul 15, 2014

Conversation

Projects
None yet
2 participants
@glyphobet
Contributor

glyphobet commented Jun 14, 2014

  • the if-block for deleting an object from a relation was not committing; the tests were still passing because the session was still open and the next API call would pull the object out of the session, not the DB. Put a session.remove() before the API call to check that the removed object is actually gone from the relation, and you'll see that it's still there in the DB.
  • Idempotency only means that subsequent identical requests cannot have
    additional side-effects. Since the response code is not a side effect,
    but rather a return value, DELETE should respond with 204 only if
    an object is deleted, and with 404 when nothing is deleted.
    (It is very confusing if an API returns 204 to a DELETE for a nonexistent
    or inaccessible resource.)

glyphobet added some commits Jun 14, 2014

fix delete from relation
the if-block for deleting an object from a relation was not committing; the tests were still passing because the session was still open and the next API call would pull the object out of the session, not the DB. Put a `session.remove()` before the API call to check that the removed object is actually gone from the relation, and you'll see that it's still there in the DB.
return 404 for DELETEs to nonexistent/inaccessible resources
Idempotency only means that subsequent identical requests cannot have
additional side-effects. Since the response code is not a side effect,
but rather a return value, `DELETE` should respond with `204` only if
an object is deleted, and with `404` when nothing is deleted.

(It is very confusing if an API returns `204` to a DELETE for a nonexistent
or inaccessible resource.)
@jfinkels

This comment has been minimized.

Owner

jfinkels commented Jul 15, 2014

Okay, I misunderstood the RFC :) Again, I am not an expert, so thanks for double checking me on this.

jfinkels added a commit that referenced this pull request Jul 15, 2014

Merge pull request #324 from goodscloud/delete-404
fixes to DELETE method handling

@jfinkels jfinkels merged commit 3f066c2 into jfinkels:master Jul 15, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@glyphobet glyphobet deleted the goodscloud:delete-404 branch Jul 15, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment