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

Add custom (meta) data on buckets and collections #158

Merged
merged 1 commit into from Sep 4, 2015

Conversation

Natim
Copy link
Member

@Natim Natim commented Aug 28, 2015

For some use-cases, it might become useful to be able to store some custom attributes in buckets or collections (e.g. metadata like application version, contact email or whatever).

Currently both Collection and Bucket resources do not define extra fields in their schema, and Cliquet drops unknown fields if not explicitly allowed.

We can either:

  • Allow unknown fields in collection and buckets schemas
  • Add a specific root level field (along data and permissions)
  • Add a specific field (called meta for example) in the schema that could receive anything.

The advantage of the latter is that custom fields do not interfere with anything in the protocol, and are trivial to implement. The inconvenient is having to put {data: {metadata: {email: "toto@yo.com"}} in the payload.

Thoughts ?

@leplatrem
Copy link
Contributor Author

Taken from #161:

When considering this proposal, @QuentinRoy was suggesting that we use "metadata" as a field name for last_modified and other internal properties, and store the collection data in "data".

@QuentinRoy
Copy link

I am not sure I got what is the inconvenient you talked about. But I don't think the metadataobject/container/property have to be inside data. I would have put them in 2 separate places: {data: {}, metadata: {email: "ilove@pancakes.com"}. metadata would be a read-only system-managed property container.

@Natim
Copy link
Member

Natim commented Aug 24, 2015

I would have put them in 2 separate places: {data: {}, metadata: {email: "ilove@pancakes.com"}. metadata would be a read-only system-managed property container.

I agree that this should be the path forward for that.

I would also let people set metadata in their payload for validation.

i.e, the system-side generated value should be equal to the provided one for the server to accept the payload. (email/signing)

@almet
Copy link
Member

almet commented Aug 24, 2015

@Natim why not but this is orthogonal to what we're proposing this IMO.

Doing the rename of the current "data" in "metadata" will break the protocol, though, so we should issue a new version of it. Since we don't have anything in production just yet that relies on the current version of the protocol, it might be the right timing to change it.

@Natim
Copy link
Member

Natim commented Aug 24, 2015

Yes I think it is the good time to break the protocol.

The validation is orthogonal it was just me having some ideas about the read-only proposal.

The question is what about the id?

On a PUT should it be on both data and metadata? Or should we add it back on data on server side if we need it (let say for signing for instance)

@Natim
Copy link
Member

Natim commented Aug 24, 2015

I just realised that this post is about allowing data on buckets and collections I thought this was already the case.

@Natim
Copy link
Member

Natim commented Aug 24, 2015

$ echo '{"data": {"signing": "hash"}}' | http PUT https://kinto.dev.mozaws.net/v1/buckets/default/collections/tasks --auth user:pass
HTTP/1.1 200 OK
Access-Control-Expose-Headers: Backoff, Retry-After, Alert, Content-Length
Connection: keep-alive
Content-Length: 156
Content-Type: application/json; charset=UTF-8
Date: Mon, 24 Aug 2015 13:18:31 GMT
ETag: "1440422311022"
Last-Modified: Mon, 24 Aug 2015 13:18:31 GMT
Server: nginx/1.4.6 (Ubuntu)

{
    "data": {
        "id": "tasks", 
        "last_modified": 1440422311022
    }, 
    "permissions": {
        "write": [
            "basicauth:631c2d625ee5726172cf67c6750de10a3e1a04bcd603bc9ad6d6b196fa8257a6"
        ]
    }
}

Signing is lost during the operation.

@QuentinRoy
Copy link

IMHO the server should never put, alter, use or rely on any data stored in the data container (if we retain the data/metadata scheme we were talking about). It should be entirely managed by the user and should not cause any action from the server (except recording, pull events and sync stuffs of course).
If the server needs an id, even provided by the user at some point, it should be in metadata. If the user wants to use its own id and doesn't want to rely on the server-managed one (in metadata where he may not be able to set it or change it), he can do whatever he wants inside data without any risk of side effects from the server.

@almet
Copy link
Member

almet commented Aug 24, 2015

That's exactly the scope of the data/metadata thing: metadata is data kinto relies on internally, whereas data is meant to be used by the client.

@Natim
Copy link
Member

Natim commented Aug 24, 2015

Yes 👍

@Natim
Copy link
Member

Natim commented Aug 28, 2015

r? @ametaireau

@Natim
Copy link
Member

Natim commented Aug 28, 2015

$ echo '{"data": {"signing": "hash"}}' | http PUT http://localhost:8888/v1/buckets/default/collections/tasks --auth user:pass -v
PUT /v1/buckets/default/collections/tasks HTTP/1.1
Accept: application/json
Accept-Encoding: gzip, deflate
Authorization: Basic dXNlcjpwYXNz
Connection: keep-alive
Content-Length: 30
Content-Type: application/json
Host: localhost:8888
User-Agent: HTTPie/0.9.2

{
    "data": {
        "signing": "hash"
    }
}

HTTP/1.1 200 OK
Access-Control-Expose-Headers: Backoff, Retry-After, Alert, Content-Length
Content-Length: 173
Content-Type: application/json; charset=UTF-8
Date: Fri, 28 Aug 2015 12:52:35 GMT
Etag: "1440766355961"
Last-Modified: Fri, 28 Aug 2015 12:52:35 GMT
Server: waitress

{
    "data": {
        "id": "tasks", 
        "last_modified": 1440766355961, 
        "signing": "hash"
    }, 
    "permissions": {
        "write": [
            "basicauth:631c2d625ee5726172cf67c6750de10a3e1a04bcd603bc9ad6d6b196fa8257a6"
        ]
    }
}

Natim added a commit that referenced this pull request Sep 4, 2015
…tions

Add custom (meta) data on buckets and collections
@Natim Natim merged commit c88b244 into master Sep 4, 2015
@Natim Natim deleted the 158-allow-arbitrary-data-on-collections branch September 4, 2015 08:53
@leplatrem
Copy link
Contributor Author

Note: This was closed, however it was not implemented on buckets.

Intentional ?

@almet
Copy link
Member

almet commented Sep 7, 2015

We hadn't identified the need to store data on buckets so far, but we can add it if you have a use case for it?

@Natim
Copy link
Member

Natim commented Sep 7, 2015

We hadn't identified the need to store data on buckets so far, but we can add it if you have a use case for it?

Agreed on my side, but good catch.

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

Successfully merging this pull request may close these issues.

None yet

4 participants