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

From: Stephan Bosch <[email-masked]>
Date: Mon, 3 Sep 2018 02:38:12 +0200

Hi Neil,

Op 29/08/2018 om 07:22 schreef Neil Jenkins:
> Hi Stephan,
>
>> The thing is, for protocols like IMAP that keep the connection open, 
>> the solution is simple: kill all active connections from the server 
>> side, thereby making sure the clients reconnect and (likely) pick up 
>> on the new settings and capabilities. For JMAP, there's no such 
>> option: the loss of an HTTP connection from either side is usually 
>> insignificant. Yes, changed URLs cause visible problems at the 
>> client, but other session changes (adding accounts, changing 
>> capabilities) don't lead to problems quickly. If the client never 
>> re-downloads the session data, it will not pick up on the changes 
>> indefinitely and the server can do nothing about that. If there is 
>> some sort of session TTL, the situation will always resolve itself 
>> within a server-controlled reasonable time frame.
>
> The trouble with a TTL is always what to set the value to. Too long 
> and the client can be out of date for a long time; too short and you 
> have a large number of unnecessary requests. I think a better solution 
> might be to include a state string on the session object itself, and 
> have each API request include the current session state on the 
> Response object. The client can compare them and refetch if different. 
> This piggybacks onto existing requests so has very little overhead, 
> but results in almost immediate updates. The only time this wouldn't 
> work is if e.g. the API url has changed, and so the request fails. But 
> a failing request probably should prompt the client to refetch the 
> session object anyway, so you're still covered.
>
> Sound reasonable?

Yes.

BTW, what happens if there's some HTTP cache in between? Is that even an 
acceptable scenario?

>>>> The spec already defines an |unsupportedFilter| error, which may be 
>>>> returned if ”the filter is syntactically valid, but the server 
>>>> cannot process it“.
>>>
>>> Given that the filter is generally a result of human input, that 
>>> seems reasonable. The human can create something less complex.  The 
>>> general guiding principle of errors is that they should be 
>>> informative and actionable - the receiver of error should both 
>>> understand what they did wrong, and have a path to back to success.  
>>> The issue with just "unsupportedFilter" could be that it's not easy 
>>> for the creator to know if it's because it's too complex, or 
>>> includes an unknown condition.
>>
>> Bron is stating my point in his last sentence.
>
> I think this error is the one you want here, but I'll update the 
> description. If you specified conditions not in the spec, or gave the 
> wrong type or something, that would be an |invalidArguments| error. 
> The |unsupportedFilter| error is really "another server may be able to 
> process this filter but, alas, this one cannot". Clients should then 
> suggest users simplify their criteria to perhaps find something the 
> server can support.

ok

>> I've thought about this a little more. What about turning the 
>> FilterOperator into an array of conditions in which the first element 
>> is the operator? So, something like:
>>
>> "filter": ["AND", {<FilterCondition>}, {<FilterCondition>}, ["OR", 
>> {<FilterCondition>}, {<FilterCondition>], ["NOT", ["OR", 
>> {<FilterCondition>}, {<FilterCondition>}, {<FilterCondition>}]]]
>
> The trouble with this is that it is difficult to represent in a typed 
> language. It's not an array of a single type.. It's not even an array 
> of |(String|FilterCondition)| because the different types are only 
> allowed to appear in certain positions. But it's not a tuple either 
> (like the JMAP method calls), because it's variable length.
>
> I don't think this brings enough benefits to outweigh this pain.

Is specification difficulty the only problem here? You could do it in 
words, much like the methodCalls request field is described in Section 
3.2. It just calls it an "Array[]" (which is actually a bit weird given 
the definitions in Section 1.1) and then specifies what the positions 
mean. So, something like:

A *filter operator* is represented by an array containing two or more 
elements:

       1.  A "String" identifying the operator. This MUST be one of the 
following strings:
           "AND"/"OR"/"NOT":

           +  *AND*: all of the conditions must match for the filter to
              match.
           +  *OR*: at least one of the conditions must match for the
              filter to match.
           +  *NOT*: none of the conditions must match for the filter to
              match.
             2.     The remaining elements of the array represent the 
operator's conditions, each of which is either a nested *filter 
operation* array or a *FilterCondition* object. The operator in the 
first element determines how the results from evaluating the conditions 
against each record are combined into the result of this *filter operator*.

Alternatively, you could define a new syntax. Something like:

"[ String FilterCondition+ ]"

Or whatever.

Regards,

Stephan.

---

From: Neil Jenkins <[email-masked]>
Date: Mon, 03 Sep 2018 03:46:21 -0400

On Mon, 3 Sep 2018, at 10:39 AM, Stephan Bosch wrote: 
>  BTW, what happens if there's some HTTP cache in between? Is that even an acceptable scenario?  

Yes, standard HTTP semantics apply, so the server needs to make sure it sets appropriate cache control headers on the response.
  
>>> I've thought about this a little more. What about turning the FilterOperator into an array of conditions in which the first element is the operator? So, something like: 
>>>  
>>> "filter": ["AND", {<FilterCondition>}, {<FilterCondition>}, ["OR", {<FilterCondition>}, {<FilterCondition>], ["NOT", ["OR", {<FilterCondition>}, {<FilterCondition>}, {<FilterCondition>}]]] 
>>  
>> The trouble with this is that it is difficult to represent in a typed language. It's not an array of a single type.. It's not even an array of `(String|FilterCondition)` because the different types are only allowed to appear in certain positions. But it's not a tuple either (like the JMAP method calls), because it's variable length. 
>>  
> Is specification difficulty the only problem here? 
 
If it's difficult to specify it's also likely to be difficult to implement in strongly typed languages. I just don't see this syntactical change bringing many benefits, and it seems likely to have definite implementation difficulty downsides. 
 
>  Section 3.2. It just calls it an "Array[]" (which is actually a bit weird given the definitions in Section 1.1) 
 
Yes, it is a bit weird, I'll rewrite that.
 
Neil.

---

From: Stephan Bosch <[email-masked]>
Date: Mon, 3 Sep 2018 10:51:18 +0200

Hi Neil,

Op 03/09/2018 om 09:46 schreef Neil Jenkins:
> Yes, standard HTTP semantics apply, so the server needs to make sure 
> it sets appropriate cache control headers on the response.

So, what is appropriate? No-cache, no-store? Since otherwise there's 
still some TTL (age) value to consider.

>>>> FilterOperator into an array of conditions in which the first 
>>>> element is the operator? So, something like:
>>>>
>>> The trouble with this is that it is difficult to represent in a 
>>> typed language. It's not an array of a single type.. It's not even 
>>> an array of |(String|FilterCondition)| because the different types 
>>> are only allowed to appear in certain positions. But it's not a 
>>> tuple either (like the JMAP method calls), because it's variable length.
>>>
> If it's difficult to specify it's also likely to be difficult to 
> implement in strongly typed languages. I just don't see this 
> syntactical change bringing many benefits, and it seems likely to have 
> definite implementation difficulty downsides.

Oh, I see what you mean now by strongly typed languages. That can be a 
valid concern.

Regards,

Stephan.

---

From: Aki Tuomi <[email-masked]>
Date: Mon, 3 Sep 2018 14:27:29 +0300

> still some TTL (age) value to consider.
I guess it depends on the object, for mail objects you can probably use
quite large expiry time, and for things like session-data you can set
must-revalidate?

Perhaps some considerations on how these headers should be set could be
added to the RFC, as there is clearly no single suitable value.

Aki

---

From: Neil Jenkins <[email-masked]>
Date: Tue, 04 Sep 2018 00:02:01 -0400

On Mon, 3 Sep 2018, at 9:27 PM, Aki Tuomi wrote: 
> I guess it depends on the object, for mail objects you can probably use quite large expiry time, and for things like session-data you can set must-revalidate? 
 
If you are downloading a blob (e.g. a raw message), I would expect a very long cache time as blobIds are immutable so the contents cannot change for the URL, so something like `Cache-Control: private, max-age=31536000`. For your session object, `Cache-Control: no-cache, no-store, must-revalidate` probably makes sense. 
 
I don't know whether this really needs a comment in the spec; it already says standard HTTP semantics are presumed. 
 
Neil.

---

From: Aki Tuomi <[email-masked]>
Date: Tue, 4 Sep 2018 08:55:34 +0300

On 04.09.2018 07:02, Neil Jenkins wrote:
>> I guess it depends on the object, for mail objects you can probably
>> use quite large expiry time, and for things like session-data you can
>> set must-revalidate?
>
> If you are downloading a blob (e.g. a raw message), I would expect a
> very long cache time as blobIds are immutable so the contents cannot
> change for the URL, so something like |Cache-Control: private,
> max-age=31536000|. For your session object, |Cache-Control: no-cache,
> no-store, must-revalidate| probably makes sense.
>
> I don't know whether this really needs a comment in the spec; it
> already says standard HTTP semantics are presumed.
>
> Neil.
>

I don't know. Standard HTTP semantics cannot really make any claims over
application specific data. Already those two statements should be in the
RFC, telling which bits you can and can't cache, and how long. Otherwise
we might end up having implementations that do not set any cache control
headers, or set them in confusing ways, and then they go thru some proxy
that sets a cache on anything that doesn't.

Telling *how* to set the cache values is not within scope of this RFC, I
agree. But telling which objects should be and for how long should be
included at least as considerations for implementers?

Aki

---

From: Alexey Melnikov <[email-masked]>
Date: Tue, 4 Sep 2018 10:08:24 +0100

On 04/09/2018 06:55, Aki Tuomi wrote:

>>> use quite large expiry time, and for things like session-data you 
>>> can set must-revalidate?
>>
> I don't know. Standard HTTP semantics cannot really make any claims 
> over application specific data. Already those two statements should be 
> in the RFC, telling which bits you can and can't cache, and how long. 
> Otherwise we might end up having implementations that do not set any 
> cache control headers, or set them in confusing ways, and then they go 
> thru some proxy that sets a cache on anything that doesn't.
>
> Telling *how* to set the cache values is not within scope of this RFC, 
> I agree. But telling which objects should be and for how long should 
> be included at least as considerations for implementers?

I tend to agree. Implementors advice (even if it is informative) would 
be good in regards to caching.

---

From: Neil Jenkins <[email-masked]>
Date: Wed, 05 Sep 2018 03:42:02 -0400

On Tue, 4 Sep 2018, at 7:09 PM, Alexey Melnikov wrote: 
> I tend to agree. Implementors advice (even if it is informative) would be good in regards to caching. 
 
OK, I'll add some advice on caching to the spec; I've created a GitHub issue <https://github.com/jmapio/jmap/issues/258> to remind me. 
 
Neil.