Skip to content
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

POST and PATCH response should contain item data #36

Closed
hluz opened this issue Apr 8, 2013 · 10 comments
Closed

POST and PATCH response should contain item data #36

hluz opened this issue Apr 8, 2013 · 10 comments

Comments

@hluz
Copy link

hluz commented Apr 8, 2013

The response resulting of a successful POST or PATCH should contain the full content item(s) created or updated, including the etag values (or it could be an optional capability). Currently to get that information it required a GET following the POST or PATCH (or multiple in case of multiple inserts)

@nicolaiarocci
Copy link
Member

Again it's all about minimizing payload size. Currently, POST and PATCH responses only contain fields that the API updates for the client ('updated', 'created', '_id', 'etag', etc.). In case of a POST the client knows the other field values (it posted them). In case of a PATCH, the only issue could be the with the 'etag' being in the header and not in the payload (we discussed this in #29). Since a client must provide the previous 'etag' in order to successfully patch, it is assumed that it knows the item already. An eventual 'COHERENCE_MODE` setting (as discussed in #29) could also fill PATCH/POST responses with all the extra fields I guess.

@hluz
Copy link
Author

hluz commented Apr 8, 2013

Agree. The main issue is that there is no etag at all returned from POST...

@nicolaiarocci
Copy link
Member

Right!

@nicolaiarocci
Copy link
Member

ebe26dc

@nicolaiarocci
Copy link
Member

Thank you @hluz, by addressing your PRs I also got the chance to improve performance, especially with multiple document insertions (but also on PATCH), and refactor some things I didn't like in my code.

@joshvillbrandt
Copy link
Contributor

AngularJS/resource could definitely make use of this "COHERENCE MODE." Although a valid 'If-Match' header / etag means that the client already has the latest data, AngularJS/resource completely overwrites the object with the response. The correct solution is to make AngularJS/resource aware of this, but it sure would be nice just to flick on this mode...

@cuibonobo
Copy link

@nicolaiarocci I'd like you to reconsider adding COHERENCE MODE to Eve. I'm currently working with a framework (Ember.js, though I hear Angular also has this problem) where I have to jump through hoops in order to save my data correctly if the server does not respond with the full object after POST.

Also, though it may seem like a waste of bandwidth because the client should 'know' what the values are, there may arise a situation (because of encoding or some other issue) where the POST is completed successfully but the data on the server is slightly different from the data on the client. Returning the full object allows the client to be absolutely sure that it has the same data as the server.

@joshvillbrandt
Copy link
Contributor

@nicolaiarocci, I'd like to submit a pull request for this feature in milestone 0.4.

@nicolaiarocci
Copy link
Member

@jenmontes @joshvillbrandt would you please open a separate ticket for the COHERENCE_MODE request? I'm willing to give it a spin, especially if Josh is going to PR it 👍

@joshvillbrandt
Copy link
Contributor

BANDWIDTH_SAVER flag has been implemented. When BANDWIDTH_SAVER == False, the full response document is available including embedded documents and media files. I do not think projections are supported with the write verbs. Unit tests for embedded documents, media files, and projections were not added for the write verbs, but unit tests for basic fields were added.

By the way, I noticed that the self HATEOAS field was populated using item_lookup_field for GET and ID_FIELD for POST. I'm not sure why that was different... I'm using ID_FIELD in build_response_document().

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

No branches or pull requests

4 participants