Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Delete attribute with patch? #597

Closed
leplatrem opened this issue Nov 26, 2015 · 3 comments
Closed

Delete attribute with patch? #597

leplatrem opened this issue Nov 26, 2015 · 3 comments

Comments

@leplatrem
Copy link
Contributor

Currently, there is no way to remove an attribute of a record with a PATCH request.

I propose that when a None value is provided for an attribute, it is removed from the record.

Thoughts ?

@glasserc
Copy link
Contributor

IMO, this is part of a larger problem, which is that the semantics of our PATCH operation are a little underspecified. RFC 5789 says that a PATCH request includes "instructions" describing how to modify a resource, but leaves it up to implementors to define those instructions and their semantics, and I don't think we do a good job of specifying that. A single PATCH operation can modify both a record's data and its permissions, and they behave differently -- data just sets, but permissions adds values. For anything else, you have to do a PUT, which is a lot more work. This information is available in the Kinto documentation, but I couldn't find it in this project.

Rather than defining our own semantics, we could borrow existing semantics, such as RFC 7396, JSON Merge Patch or (my personal favorite) RFC 6902, JSON Patch, which (as an added bonus) gives us a clear way to remove attributes. But that seems pretty heavyweight, and it might not even matter too much because there's no way to support this in combination with encryption. So maybe we should just build primitives in our client libraries that let us express changes directly.

@leplatrem
Copy link
Contributor Author

data just sets, but permissions adds values.

Could you detail that part please ? I don't think permissions behave differently than data. Every key in the permission is replaced by the value provided in the PATCH payload.
The only misleading difference may be that the current user is always added/kept among the members of the write permission.

Rather than defining our own semantics, we could borrow existing semantics

+1
Especially if the content-type header allows us to keep retro-compatibilty :)

I couldn't study the propositions in detail, but would be happy to work on that together some day !

Related:

@glasserc
Copy link
Contributor

I don't think permissions behave differently than data. Every key in the permission is replaced by the value provided in the PATCH payload.

You're right! I'm sorry, I totally misunderstood what was going on with that.

glasserc pushed a commit that referenced this issue May 20, 2016
Fix crash when a cache expires setting is set for a specific bucket or collection
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants