Re: [Jmap] new JMAP server for prototyping

[Jamey Sharp <[e-mail]>]

On Mon, May 24, 2021 at 02:59:02PM +1000, Neil Jenkins wrote:
>On Sat, 22 May 2021, at 06:20, Jamey Sharp wrote:
>>Looking at a different bit of the text:
>>    In the case of records with references to the same type, the server 
>>    MUST order the creates and updates within a single method call so 
>>    that creates happen before their creation ids are referenced by 
>>    another create/update/destroy in the same call.
>>I think this wording is a little strange because I don't think 
>>destroys can reference creation IDs at all, right?
>
>Technically it could destroy a creation in the same request (it's a 
>weird edge case, but it was worded that way deliberately); I know we 
>actually came across this scenario at some point, although I can't 
>remember why just now.

On Mon, May 24, 2021 at 05:54:42PM +0200, Arnt Gulbrandsen wrote:
>It seems like a reasonable enough combination when a mobile device 
>leaves airplane mode and a mobile client is playing back stored 
>actions.

I suppose it's not unreasonable, but I can't find any hint in the 
specification that the keys of `update` or the entries of `destroy` can 
be creation id references. The most relevant text I can find is this:

    Some records may hold references to other records (foreign keys). 
    That reference may be set (via create or update) in the same request 
    as the referenced record is created.  To do this, the client refers 
    to the new record using its creation id prefixed with a "#".

My reading is that creation id references can only appear within a 
record, either inside the `Foo` in `create`, or inside the `PatchObject` 
in `update`. Am I missing something?

On Mon, May 24, 2021 at 02:59:02PM +1000, Neil Jenkins wrote:
>Hmm, I mean, sure you can do whatever you want for prototyping, but 
>this is definitely not spec compliant.

Yeah, fair enough; I'll think about how to make my validation step 
context-sensitive.

>>>Close. In this situation you have to invalidate any results in your 
>>>sparse query after the first gap, but you can still keep the ones 
>>>before.
>>
>>Ohhhh. That makes sense, thanks. The spec is wrong here though, I think:
>>[...]
>>Here, "id31" is shown as removed, which wasn't in fooIds, but "id3" and 
>>"id4" come after a gap and weren't removed.
>
>Hey damn, you're right! I think my brain glossed over this when 
>proofreading because the line below starts and truncates, and in my 
>code the flag that gets set when you have an unknown destroyed id is  
>called truncateAtFirstGap. But the spec is actually just description 
>the final step of adjusting the query results length, not removing the 
>ids after the gap. Would you like to submit the errata? Or I'm happy to 
>do it.

As much as I enjoy the idea of having my name on a contribution to IETF 
work for the first time, I'm not sure how to word the replacement text, 
so I think probably you should do it.

>>I can't quite figure out what client UX the upToId feature supports.
>
>It's purely an optimisation in both what's sent over the wire and (for 
>certain implementations) how much work the server does. In most 
>scenarios, a client will have just the start of a list cached. Extreme 
>scenario, we have the first 3 ids cached from a list of 500 results:
>
>[ id1, id2, id3, null4 … null500 ]
>
>The goal of queryChanges is to ensure that the state of the client 
>after applying the changes is an accurate representation of the new 
>state on the server, while sending a minimal delta. Suppose the last 
>100 items are removed, and a further 50 items are inserted at the end 
>instead. Simply truncating the client's sparse array to the new length 
>(450) means it is now a valid representation of the new state: every id 
>that's there is in the correct position. This is what upToId does: 
>instead of sending 50 "added" and 100 "removed" ids you can just send 
>the new total instead.

That's a great example for me to keep in mind but it didn't make me less 
confused. I've figured out what's bothering me though.

Let's say the client has cached a prefix of the results, and then some 
of those items are destroyed.

old: ["A", "B", "C", "D", ...]
new: ["B", "D", ...]

Say upToId is "D"; you said I should look for it in the new result, 
where it is at index 1. Then 'any ids that were removed but have a 
higher index than "upToId" SHOULD be omitted.' Which means the removed 
list is permitted to contain only "A", right?

If so, now the client's cache says ["B", "C", "D"], which is wrong.

I imagine current implementations just report all destroyed records as 
removed whether they were in the old query or not, which would obscure 
this issue. It would also come up for sorts or filters on mutable 
properties, but as we've discussed, nobody does that right now.

What my intuition was prodding me about is that using the same index in 
the text of both "added" and "removed" can't be right; which led to me 
trying to guess client-side UX reasons why somebody might want that, 
instead of the minimum delta needed for correct operation.

Here's what I think are the minimum necessary rules for upToId; I'd 
appreciate any comments on whether I've got this right:

     If an "upToId" is supplied and existed in the old results, any ids 
     that were removed but had a higher index than "upToId" previously 
     did SHOULD be omitted. All others MUST NOT be omitted.

     If an "upToId" is supplied and existed in the old results, the last 
     id which was not removed, and which previously had an index no 
     higher than "upToId" did, is the client's last cached id. Any ids 
     which were added but now have a higher index than the client's last 
     cached id does SHOULD be omitted. All others MUST NOT be omitted.

This text might be more clear if written from the point of view of the 
client's requirements rather than restrictions on the server:

     If an "upToId" is supplied, any ids which the server is certain did 
     not appear at or before "upToId" in the old results SHOULD be 
     omitted.

     If an "upToId" is supplied, any ids which are not necessary to 
     return the client's cache to a valid prefix of the new results, 
     after removing the ids given in "removed", SHOULD be omitted.

I think any mention of sort/filter mutability here is an unnecessary 
constraint on server behavior, as is requiring upToId to be present in 
the new results.

But I believe these rules still allow every implementation the current 
spec allows, aside from prohibiting the situation I described above, 
since a server is always allowed to ignore upToId.

I hope that made sense this time...
Jamey