elasticinbox / elasticinbox-java

ElasticInbox - scalable, distributed email store
BSD 3-Clause "New" or "Revised" License
108 stars 26 forks source link

Support custom attributes for labels #39

Closed rstml closed 11 years ago

rstml commented 11 years ago

Allow users to specify custom attributes for labels and mailboxes.

Below are the examples for V2 API

Create Label:

POST /rest/v2/:domain/:user/mailbox/label/

{
   "name" : "MyNewLabel",
   "attributes" : {
      "order" : "3",
      "color" : "red",
      ...
   },
}

Update Label (add or modify custom attributes):

PUT /rest/v2/:domain/:user/mailbox/label/:id/

{
   "name" : "BlueLabel",
   "attributes" : {
      "order" : "6",
      "color" : "blue",
      "icon" : "smile",
      ...
   },
}

To remove custom attributes just set them to empty string or null:

PUT /rest/v2/:domain/:user/mailbox/label/:id/

{
   "attributes" : {
      "color" : "",
      "icon" : null
   }
}

Get attributes:

GET /rest/v2/:domain/:user/mailbox/?metadata=true

{
  "0": {
    "name": "all",
    "size": 2193085,
    "total": 230,
    "unread": 13
  },
  "1232" : {
    "name" : "MyBlueLabel",
    "attributes" : {
      "order" : "6",
      "color" : "blue",
      ...
  },
  ...
}
Danita commented 11 years ago

I like this approach. Inside attributes anything can go, including nested objects?

rstml commented 11 years ago

Ah, no, we didn't plan to support nested objects. Do you think it will be hard to use just key-value pairs?

Also, I was thinking about alternative implementation for Create/Update label:

PUT /rest/v2/:domain/:user/mailbox/label/:id/attribute/

{
   "order" : "6",
   "color" : "blue",
   "icon" : "smile",
   ...
}

Pros - cleaner API. Cons - you will need to do 2 API calls if you want to create new label with custom attributes.

Do you think you could do 2 API calls?

Danita commented 11 years ago

Oh, I see. This implies the attributes are children elements of a label, hence a request to create a label, another request to create its attributes. It really depends wether we consider the attribute to be a property or a separate entity of the label. I'm not sure the latter approach makes much sense, because we never would want to request an attribute if it's not in context of its label. I mean: labels are independent entities, and they have (or should have, right now we only get the full collection) their own resource url; messages too. Labels and messages are related to each other, but in my opinion, attributes don't mean anything by themselves and they are not related to a label -- they are properties of a label, much as I have a name, a last name and a social security number.

I think in the model, attributes should be a simple member of a label object and it should have no further meaning to the system. If the user wants to stuff anything inside them I think is up to him. What do you think?

rstml commented 11 years ago

Agree, that's a good justification. Let's just keep 'attribute' as path param for removing - I couldn't come up with better way for that.

Danita commented 11 years ago

I think removing attributes shouldn't be different from editing a label. I think it's best to expose a uniform interfase: I create a label by sending all it's properties, then I should edit it the same way, that is, posting the entire record. I fail to see why it should be different from editing it's name, for example:

Create label:

POST /rest/v2/:domain/:user/mailbox/label/

{
   "name" : "Work",
   "attributes" : {
      "order" : "3",
      "color" : "red"
      "icon" : "suitcase"
   },
}

Edit name and remove color attribute from the label:

PUT /rest/v2/:domain/:user/mailbox/label/:id/

{
   "name" : "Office work",
   "attributes" : {
      "order" : "3",
      "icon" : "suitcase"
   },
}

It could be even simpler to implement, am I right?

rstml commented 11 years ago

That's not the exact behavior which was proposed above. The idea is that for PUT ElasticInbox will create new attributes if they don't exist and update value if they exist. It will not remove anything. So if you want to update just a color, all you need to do is:

PUT /rest/v2/:domain/:user/mailbox/label/:id/

{
   "attributes" : {
      "color" : "yellow"
   }
}

This approach is better because when you update you don't need to know which values already exist (i.e. order, icon, etc).

In your scenario it will require read-before-write operation. The danger or read-before-write is concurrency. Imagine label with tens of attributes being modified concurrently by 2 users. User1 reads all attributes to modify. But before it modified those User2 removed one of those. Now when User1 does PUT it will override changes made by User2.

Danita commented 11 years ago

I understand, that scenario could happen if the user has the email client open on two devices at a time and in both of them he has edited the same labels, when they both synchronize one device could recreate the label the other device deleted previously.

Wouldn't that kind of race condition happen also with messages? I DELETE on one device, and PUT the same message on the other. Is that prevented?

Anyway, I like the idea and can't wait to see it implemented :+1:

rstml commented 11 years ago

We try to avoid these situations as much as possible - of course it may not be always possible. Great, expect it soon.

Danita commented 11 years ago

:heart: