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

From: Neil Jhaveri <[email-masked]>
Date: Wed, 30 May 2018 20:46:17 -0700

Hi all,

Here are some comments from my latest reading of draft-ietf-jmap-core-05. Apologies if I’m revisiting anything already discussed! 

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?

> 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. 

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? 

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.

(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.

> 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.

> 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? 

> 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?)

> 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

---

From: Bron Gondwana <[email-masked]>
Date: Thu, 31 May 2018 15:26:36 +1000

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.

> 
>> 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?

>> 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."> 
>> *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.

> 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?
>> 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*
> **
> 
>> *Types* signatures are given for all JSON objects in this document.
>> To communicate with a JMAP *server you* need two things to start:
>> 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.
> 
>>    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*
> _________________________________________________
> Jmap mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/jmap

--
  Bron Gondwana, CEO, FastMail Pty Ltd
  [email-masked]

Links:

  1. https://developers.google.com/gmail/api/v1/reference/users/history/list