Subject: Re: [Jmap] Review of draft-ietf-jmap-core-05

From: "Chris Newman" <[email-masked]>
Date: Thu, 07 Jun 2018 17:34:17 -0700

On 30 May 2018, at 22:26, Bron Gondwana wrote:
> On Thu, May 31, 2018, at 13:46, Neil Jhaveri wrote:
>> Hi all,
>>
>> Here are some comments from my latest reading of 
>> draft-ietf-jmap-core-
>> 05. Apologies if I’m revisiting anything already discussed!
> (FYI Neil is on leave for a bit)
>
>> *Questions / Needs Discussion*
>> **
>>
>>> 3.2.1.  Example request
>>> {
>>>   "using": [ "*urn:ietf:params:jmap:core*",
>>>   "urn:ietf:params:jmap:mail" ],> Can the JMAP Core capability just 
>>> be assumed? Any good reasons to
>> actually have this listed out?
> This seems reasonable to me - you can't do JMAP without core.

I lean slightly towards keeping the ":core" explicit. If you make 
incompatible changes to the core in the future, it's much cleaner to add 
a urn:ietf:params:jmap:core2 and then advertise one or both depending on 
whether the implementation is backwards compatible with the original 
core. I note we're about to do something like this with IMAP4rev2 so 
this is not theoretical.

		- Chris

>>> 4.2. /changes
>>> …
>>>    o  *changed*: "String[]" An array of ids for records which have
>>>    been>>       *created or modified* but not destroyed since the 
>>> oldState.
>>
>> Could “creations” and “modifications” be separated into 
>> separate array
>> properties?  A common resynchronization case is a small number of
>> creations, without any modifications, and it may be more efficient 
>> for
>> some clients to process creations than modifications.
> I'm OK with this so long as if the server can't tell, it's allowed to
> put things into modified even if they're new.
>> For instance, an iOS app using Core Data could then create new
>> objects and immediately save them off, rather than having to first do
>> an initial fetch to see if it already has an existing record with
>> that ID.>
>> By comparison to IMAP + CONDSTORE, I think you can infer if something
>> is created vs. modified if a fetch response's UID is greater than 
>> your
>> last-cached UIDNEXT. Since IDs in JMAP seem to be opaque, it might be
>> nice to do this separation to afford this possibility again.>
>>
>>> 4.2. /changes
>>> ...
>>>
>>>    o  *newState*: "String" This is the state the client will be in
>>>    after applying the set of changes to the old state.> If an 
>>> offline client is resynchronizing after being disconnected and
>> there are a lot of changes, the way that /changes is set up seems to
>> imply that if the changes don’t all fit in 1 response, the sequence 
>> of
>> responses will be ordered oldest-to-newest. Is that true?
> This isn't enforced.  Good point.
>
>> If so, and assuming most clients are fairly naive and just 
>> immediately
>> dispatch /get’s on IDs that they retrieve (perhaps by 
>> back-reference),
>> that isn’t ideal for a UI that shows objects in newest-to-oldest
>> order, potentially resulting in a user experience of “here are new
>> objects from 5 days ago”, followed by “here are new objects from 
>> 4
>> days ago”, and so on. When really, the user ideally would prefer to
>> see “here are objects from right now” as the first screen update 
>> — at
>> least for e-mail, I think this is the case.>
>> Have we had any discussion around handling pages of changes in a
>> different way? For instance, the Gmail API[1] has a Users.history :
>> list resource, which takes a “startHistoryId” as well as a
>> “pageToken”. The response includes a “nextPageToken”, or null 
>> if there
>> are no additional pages. Clients only update their local 
>> “historyId”
>> after fetching all pages.>
>> (This admittedly comes at the trade-off of a client not being able to
>> checkpoint any changed-ID ingestion until it is fully caught up with
>> all changed IDs).>
>> A client could do a /query to get the absolute most recent page of
>> items, merge that into it’s cache, and then handle /changes 
>> normally
>> in order, but this seems a little less efficient.
> Yeah, it's a bit tricky.  Do you have a proposal that would be sane?
>
>>
>>> 4.3. /set
>>
>>> A *SetError* object has the following properties:
>>> ...
>>> o  *description*: "String|null" A description of the error *to
>>> display **to the user*.> Presumably, this error is not localized. 
>>> The recommendation to display
>> it to the user is not consistent with the document's earlier
>> recommendation in 3.6.2. Method-level errors: "A ‘description'
>> property MAY be present to help debug with an explanation of what the
>> problem was.  This is a non-localised string, and is not intended to
>> be shown directly to end users.">
>> Should the same suggestion be made here?
>>
>>
>>> 4.4. /query
>>> ...
>>> *anchorOffset*: "Number|null" The index of the anchor object 
>>> relative
>>> to the index of the first result to return.  This MAY be negative.
>>> For example, "-1" means the first Foo after the anchor Foo should be
>>> the first result in the results returned (see below for more
>>> details).> FWIW, when reading this, I also made the same scribble 
>>> note that
>> Matthew H. did almost 3 months ago — that the anchorOffset seemed
>> flipped from what my brain was expecting, and I was confused for a 
>> bit
>> about why it was negative instead of positive. Re-reading it and the
>> previous thread, I get it, and either way is fine since it is just
>> stylistic, but I will cast my vote in favor of flipping anchorOffset.
> Suits me fine.  I think we have a majority for flipping.
>
>>
>>>
>>>
>>> 4.5. /queryChanges
>>> ...
>>> The response has the following arguments:
>>> ...
>>>    o  *removed*:
>>>    o  *added*
>> Is there a reason why there is no modified property? For instance, 
>> how
>> would a primarily-online mail client realize that certain threads
>> became flagged, unread, etc., aside from re-get'ing everything in the
>> query window after handling the adds/deletes?
> Modified is a property of the item, not the list.  So you use /changes
> to find items which have changed and /queryChanges to see what's added
> to or removed to the list.
> If you have all data locally and are sorting locally, you don't need 
> to
> use query or queryChanges at all.
>>
>>> 5. Binary data
>>> ...
>>> A blob that is not referenced by a JMAP object (e.g. as a message
>>> attachment), MAY be deleted by the server to free up resources.>> 
>>> …
>>>    o  Except where quota restrictions force early deletion, an
>>>    unreferenced blob SHOULD NOT be deleted for at least 24h from the
>>>    time of upload; if reuploaded, the same blobId MAY be returned,
>>>    but this SHOULD reset the expiry time.> The way I’m 
>>> understanding this garbage-collection policy, it could be
>> alarming to a user if they try to delete some blob data they
>> accidentally put on the server, but it’s still possible to access 
>> it
>> even after deletion because it’s just unreferenced. There is a 
>> trade-
>> off between being able to save users when they accidentally delete
>> important data, and a user being able to actually delete data 
>> they’re
>> trying to delete, but I think those sort of data-recovery policies
>> should not be totally opaque to JMAP.>
>> What would you all think about being a little more explicit here?
>> Perhaps defining a “MUST”-conformance expiration time in the spec 
>> (N
>> minutes)? (The current policy saying SHOULD NOT delete for 24h seems
>> excessively generous for API clients to me?)
> It sounds like maybe what you want is a deleteBlob operation to clean 
> up
> unreferenced blobs?
>>
>>
>>> 6.2.1. PushSubscription/set
>>> Each *session* may only have a single push subscription registered.
>> Can somebody clarify what is meant by session here? Is this meant to
>> be a single “account”?>
>>
>> *Grammatical Nit-picks*
>> **
>>
>>> 1.1 Notational Conventions
>>> …
>>> *Types* signatures are given for all JSON objects in this document.
>> Should this be “Type”, rather than “Types”?
>>
>>
>>> 2. The JMAP Session resource
>>> To communicate with a JMAP *server you* need two things to start:
>> Is a comma needed between “server” and “you”?
>>
>>> 5. Binary data
>>> ...
>>> A blob that is not referenced by a JMAP object (e.g. as a message
>>> *attachment), MAY* be deleted by the server to free up resources.> I 
>>> think the comma before “MAY” is not needed.
>>
>>> 6.1. The StateChange object
>>>    o  *trigger*: "String" What caused this change.  The following
>>>    causes are defined:>>       *  "delivery": The arrival of a new 
>>> message caused the change.>
>> Since JMAP is intended to be the foundation of data types beyond 
>> Mail,
>> perhaps this should say something besides “new message” — maybe 
>> “The
>> cause of the change is delivery from an external system or user”.>
>>
>> *Formatting nit-picks*
>> PDF version - page 19 is very small (list of IDs doesn’t wrap).
>>
>> - Neil
>>
>>
>> _________________________________________________
>> Jmap mailing list
>> [email-masked]
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_jmap&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=K_BObr5Kfkr3rxt1oBPF9KFiEU3xl9LcD2OOJG3TXfI&m=tF2smCiqvAjqML72IJKlObw87fo9maXqqizombif_c0&s=vDk2QMImzzVf1zaS7iK6jI823oNgxqVb_AgiPzIrca8&e=
>
> --
>   Bron Gondwana, CEO, FastMail Pty Ltd
>   [email-masked]
>
>
>
> Links:
>
>   1. 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__developers.google.com_gmail_api_v1_reference_users_history_list&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=K_BObr5Kfkr3rxt1oBPF9KFiEU3xl9LcD2OOJG3TXfI&m=tF2smCiqvAjqML72IJKlObw87fo9maXqqizombif_c0&s=r_HfwTCBhtQHH4oKJajNaR-6KFMdr1HolfIvyqt7CBA&e=

> _______________________________________________
> Jmap mailing list
> [email-masked]
> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ietf.org_mailman_listinfo_jmap&d=DwICAg&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=K_BObr5Kfkr3rxt1oBPF9KFiEU3xl9LcD2OOJG3TXfI&m=tF2smCiqvAjqML72IJKlObw87fo9maXqqizombif_c0&s=vDk2QMImzzVf1zaS7iK6jI823oNgxqVb_AgiPzIrca8&e=

---

From: Neil Jenkins <[email-masked]>
Date: Fri, 08 Jun 2018 19:13:39 -0400

Hi Neil,

>>   "using": [ "*urn:ietf:params:jmap:core*", "urn:ietf:params:jmap:mail" ],
> Can the JMAP Core capability just be assumed? Any good reasons to actually have this listed out?

My original reasoning for having it was as Chris mentioned, to give an update path for a revised version of core. But on the other hand any future specification could always say presence of its URN implies no "urn:ietf:params:jmap:core", it's just a little messier. The downside of course is that you're sending this same data with every request, so it's extra overhead. There are probably better ways to deal with that if we thought it was an issue though (some way of coming up with a token to represent the set of "using" params you want).

Anyone else have an opinion? If not, I'm slightly inclined to leave it as a specific inclusion.

>> 4.2. /changes
> Could “creations” and “modifications” be separated into separate array properties?

This is worth discussing. I see the following advantages:

 * The client receives more information, so has more flexibility to implement strategies such as always fetching new messages in the same request but delaying fetching updates until the next round trip (and thus can omit fetching these at all if it has a partial cache and the changes are for messages it doesn't have cached anyway).
 * It mirrors the /set create, update, destroy distinction, which I think makes it more consistent and easier to understand.
 * Your example of Core Data APIs being more efficient if you don't have to check whether you already know about an id to decide whether it's a create or modify
 * You can use this to easily check for ids you care about for notifications (new message deliveries), and ignore those that are just changed.

and disadvantages:

 * The server must store more state in order to calculate this; for example, if using a modseq system, it now needs to store a created modseq as well as a modified modseq.
 * You need two Email/get back-references if you want to fetch both updates and new messages in the single request. This could be less efficient for the server to process and adds syntax overhead in both directions. The server could certainly make this pretty much as efficient to process by detecting successive /get calls and processing them together (so it keeps caches open rather than closing between method calls etc.), but this does add complexity.

So, I'm not sure. It could be a good change, but I'd like to hear from server authors on whether this is going to create a significant extra burden to implement. The increase in upload packet size from potentially needing an extra backreference /get call may be a concern too?

>> 4.2. /changes
>> ...
>> 
>>    o  *newState*: "String" This is the state the client will be in after applying the set of changes to the old state.
> If an offline client is resynchronizing after being disconnected and there are a lot of changes, the way that /changes is set up seems to imply that if the changes don’t all fit in 1 response, the sequence of responses will be ordered oldest-to-newest. Is that true?

It's technically not required, but it is most likely for ease of implementation, yes.

> If so, and assuming most clients are fairly naive and just immediately dispatch /get’s on IDs that they retrieve (perhaps by back-reference), that isn’t ideal for a UI that shows objects in newest-to-oldest order, potentially resulting in a user experience of “here are new objects from 5 days ago”, followed by “here are new objects from 4 days ago”, and so on. When really, the user ideally would prefer to see “here are objects from right now” as the first screen update — at least for e-mail, I think this is the case.
> 
> Have we had any discussion around handling pages of changes in a different way? For instance, the Gmail API <https://developers.google.com/gmail/api/v1/reference/users/history/list> has a Users.history : list resource, which takes a “startHistoryId” as well as a “pageToken”. The response includes a “nextPageToken”, or null if there are no additional pages. Clients only update their local “historyId” after fetching all pages.

Just to clarify, the Gmail API pages data in reverse chronological order here?

If you do it in reverse order you might run into issues like a message being both modified and then deleted. If you see the deleted in one page then modified in a future one, you need to have kept track that the deleted is actually the most recent (so you must have kept some data around for the deleted item). That's a bit messy. And of course there's also the problem of what happens if the state on the server changes while you are paging in the data. Not a problem if you are going in forward chronological order, you just keep going.

> A client could do a /query to get the absolute most recent page of items, merge that into it’s cache, and then handle /changes normally in order, but this seems a little less efficient.

For a client with partial cache (e.g. online client, probably most mobile clients) you will be using /query  to determine the subset of messages you are interested in. When a change occurs (you get a push notification), the strategy I'm currently using on our FastMail JMAP UI that's in development is the following:
 * Email/queryChanges the current list in view, max-changes 25
 * Fetch the threads/messages in the "added" response of the /queryChanges result using back references.
 * Email/changes
 * Thread/changes
 * Mailbox/changes + fetch changed using back references.
This is all bundled in a single JMAP request. This allows new messages to be shown and deleted ones removed, and mailbox unread counts to be updated all in a single round trip. A second round trip fetches any flag or mailbox changes for messages in view that the /changes response indicated are out of date. This is a much smaller visual change and generally a little less important than new mail, so delaying one round trip is reasonable, and allows you to not fetch changes for messages you don't care about.

If there have been more than 25 changes (the max I specified), the /queryChanges fails and I retry again this time with a much higher max changes, but without the backreferences fetching the messages/threads. Once my list is up to date, I can then fetch exactly the messages I need in the visible section: total time 3 round trips (including the failed initial /queryChanges), but for a rare case (especially because we have push, most updates are small).

If you have a complete cache offline, you would instead probably use a slightly different strategy. To do the initial sync I would start with a single /query with a filter of null, sorted date descending. This is your list of all messages in the account. To do your initial sync, you would fetch the list, probably a page of say 500 ids at a time, and simultaneously start fetching the messages in order (so you get the newest ones first), until you have a complete set. 

When a change occurs (you get a push notification), you can then do:
 * Email/changes (max-changes 25) + back references to fetch the email headers of the added/modified messages
 * Mailbox/changes + fetch changed using back references.
 * Email/query filter=null sort=date-descending position=0 limit=50
This will return all the changes to the emails if fewer than 25 have changed. If there are more changes, you can just keep repeating the /changes call until you have them all. To make sure you present the newest stuff quickly though, the /query fetches the list of the newest 50 Email ids. You can look at these and start fetching from the top any that you don't have before you get to them in the /changes. It's a little more work, but not that much more work (depending on your data store architecture I suppose).

Does this sound reasonable to you? If not, or if it's just suboptimal, can you detail more what you want to be able to do (and maybe what that might look like at the API level)?

>> o  *description*: "String|null" A description of the error *to display **to the user*.
> Presumably, this error is not localized. The recommendation to display it to the user is not consistent with the document's earlier recommendation in 3.6.2. Method-level errors: "A ‘description' property MAY be present to help debug with an explanation of what the problem was.  This is a non-localised string, and is not intended to be shown directly to end users."
> 

Yeh, this should be the same. I'll change this.

> FWIW, when reading this, I also made the same scribble note that Matthew H. did almost 3 months ago — that the anchorOffset seemed flipped from what my brain was expecting, and I was confused for a bit about why it was negative instead of positive. Re-reading it and the previous thread, I get it, and either way is fine since it is just stylistic, but I will cast my vote in favor of flipping anchorOffset.

OK, seems majority are in favour of flipping; I will do so.

> 4.5. /queryChanges
> Is there a reason why there is no modified property? For instance, how would a primarily-online mail client realize that certain threads became flagged, unread, etc., aside from re-get'ing everything in the query window after handling the adds/deletes? 

As Bron mentioned, it uses the Email/changes method, as I described above. This is generally pretty efficient.

> 5. Binary data
>> ...
>> A blob that is not referenced by a JMAP object (e.g. as a message attachment), MAY be deleted by the server to free up resources.
>> …
>>    o  Except where quota restrictions force early deletion, an unreferenced blob SHOULD NOT be deleted for at least 24h from the time of upload; if reuploaded, the same blobId MAY be returned, but this SHOULD reset the expiry time.
> The way I’m understanding this garbage-collection policy, it could be alarming to a user if they try to delete some blob data they accidentally put on the server, but it’s still possible to access it even after deletion because it’s just unreferenced. There is a trade-off between being able to save users when they accidentally delete important data, and a user being able to actually delete data they’re trying to delete, but I think those sort of data-recovery policies should not be totally opaque to JMAP.
> 
> What would you all think about being a little more explicit here? Perhaps defining a “MUST”-conformance expiration time in the spec (N minutes)? (The current policy saying SHOULD NOT delete for 24h seems excessively generous for API clients to me?)

Yeh, I think we can reduce this. Note this only applies to blobs you directly upload (so have never been referenced). If you delete a message for example, the spec only asks that the blob id remain valid to the end of the API requests (so you can delete a draft and then create a new one referencing the previous attachments).

How about we just reduce the 24h to 1h and make it a MUST? So it becomes:

*Except where quota restrictions force early deletion, an unreferenced blob MUST NOT be deleted for at least 1 hour from the time of upload; if reuploaded, the same blobId MAY be returned, but this SHOULD reset the expiry time.*

> Can somebody clarify what is meant by session here? Is this meant to be a single “account”?

Ah, this dates back to when we had authentication as part of the spec. That's gone so this may need rethinking. A session is essentially a set of auth credentials. This presumes that the server requires either Oauth or app passwords to log in. You then were allowed one push subscription, which would last until the credentials were expired or you explicitly cleared it.

Since we don't have this auth requirement in the spec, perhaps we should change this. I think we would need to do something like:
 * Allow multiple PushSubscriptions (how is this limited?)
 * Make them expire after a set time automatically (can the client specify a requested length subject to server limit? Or just server picks?)
 * The client can renew at any point to reset the time out.

Does something like that sound reasonable?

> Should this be “Type”, rather than “Types”?

Yes.

> Is a comma needed between “server” and “you”?

Yes, but I will restructure for simplification and clarity instead:

*You need two things to connect to a JMAP server:*

>> A blob that is not referenced by a JMAP object (e.g. as a message *attachment), MAY* be deleted by the server to free up resources.
> I think the comma before “MAY” is not needed.

Agreed.

>>    o  *trigger*: "String" What caused this change.  The following causes are defined:
>>       *  "delivery": The arrival of a new message caused the change.
> 
> Since JMAP is intended to be the foundation of data types beyond Mail, perhaps this should say something besides “new message” — maybe “The cause of the change is delivery from an external system or user”.

I agree, and I actually removed this whole property recently. Instead, the mail spec now defines the following:

**Push**

*In addition, servers MUST support a psuedo-type called "EmailDelivery" in the push mechanisms. The state string for this MUST change whenever a new Email is added to the store, but SHOULD NOT change upon any other change to the Email objects.*

Cheers,
Neil.

---

From: Benoît TELLIER <[email-masked]>
Date: Mon, 11 Jun 2018 23:57:48 +0000

IMO having the changes events aggregated is a strength. It is easy to implement on the server side as well as provides a full history to the clients (avoids merging creations, updates & deletions together, even if this is trivial).

However, I'm doubtful concerning implementing state tracking only with the MODSEQ mechanism: the MODSEQ is a property relative to a MAILBOX and thus is not adapted in response to a email/get request that will return email from different mailboxes. You could end up with two email in different mailbox having the same MODSEQ. Having a MODSEQ global to the all account seems the only viable way out to achieve this here, but it is, in my opinion, a very strong assertion.

Hence, I'd advocate that additional information needs to be stored anyway. I see no obstacles to enhancing /changes response to allow the client to distinguish additions from changes.

Best regards,

Benoit Tellier

On Jun 9, 2018 6:13 AM, from [email-masked]

4.2. /changes

The client receives more information, so has more flexibility to implement strategies such as always fetching new messages in the same request but delaying fetching updates until the next round trip (and thus can omit fetching these at all if it has a partial cache and the changes are for messages it doesn't have cached anyway).

It mirrors the /set create, update, destroy distinction, which I think makes it more consistent and easier to understand.

Your example of Core Data APIs being more efficient if you don't have to check whether you already know about an id to decide whether it's a create or modify

You can use this to easily check for ids you care about for notifications (new message deliveries), and ignore those that are just changed.

and disadvantages:

The server must store more state in order to calculate this; for example, if using a modseq system, it now needs to store a created modseq as well as a modified modseq.

You need two Email/get back-references if you want to fetch both updates and new messages in the single request. This could be less efficient for the server to process and adds syntax overhead in both directions. The server could certainly make this pretty much as efficient to process by detecting successive /get calls and processing them together (so it keeps caches open rather than closing between method calls etc.), but this does add complexity.

So, I'm not sure. It could be a good change, but I'd like to hear from server authors on whether this is going to create a significant extra burden to implement. The increase in upload packet size from potentially needing an extra backreference /get call may be a concern to

---

From: Bron Gondwana <[email-masked]>
Date: Mon, 18 Jun 2018 09:04:27 +1000

On Tue, Jun 12, 2018, at 09:57, Benoît TELLIER wrote:

> IMO having the changes events aggregated is a strength. It is easy to
> implement on the server side as well as provides a full history to the
> clients (avoids merging creations, updates & deletions together, even
> if this is trivial).> 

> However, I'm doubtful concerning implementing state tracking only with
> the MODSEQ mechanism: the MODSEQ is a property relative to a MAILBOX
> and thus is not adapted in response to a email/get request that will
> return email from different mailboxes. You could end up with two email
> in different mailbox having the same MODSEQ. Having a MODSEQ global to
> the all account seems the only viable way out to achieve this here,
> but it is, in my opinion, a very strong assertion.
That's why we have a global per-account modseq in Cyrus IMAP now :)  It
was quite trivial to do, just jumping the per-mailbox highestmodseq each
time, for example
Mailbox A: HMS:500 EXISTS:20
Mailbox B: HMS:600 EXISTS:5
Global: HMS:700

Append a new message to Mailbox A.  New state is:

Mailbox A: HMS:701 EXISTS:21
Mailbox B: HMS:600 EXISTS:5
Global HMS:701

I've examined all the assertions in the CONDSTORE and QRESYNC standards
and there's nothing that says that modseqs on a particular mailbox need
to be contiguous, so this worked well for us :)
> Hence, I'd advocate that additional information needs to be stored
> anyway. I see no obstacles to enhancing /changes response to allow the
> client to distinguish additions from changes.
Cool - I'll have a look at what's involved in doing it in Cyrus, or get
Robert to when he's back.
Cheers,

Bron.

---

From: Neil Jenkins <[email-masked]>
Date: Wed, 20 Jun 2018 03:07:00 -0400

Since the consensus seems to be for splitting "changed" into "created" and "updated", I've made a pull request with this <https://github.com/jmapio/jmap/pull/218>. Please review and if there are no objections, I'll merge next week.

Neil.