search

mozilla-services / cliquet

CLIQUET IS NOW DEPRECATED use kinto.core instead
http://github.com/Kinto/kinto
Other
64 stars 16 forks source link

Delete attribute with patch? #597

Closed leplatrem closed 8 years ago

leplatrem commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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.