[JMAP] Re: Status of JMAP Metadata, RefPlus and Mail Sharing drafts ahead of IETF 125

[Neil Jenkins <[e-mail]>]

I've read through all of these again.

> https://datatracker.ietf.org/doc/draft-ietf-jmap-mail-sharing/

This looks great, I think it should go to WG last call and be submitted for publication.

> https://datatracker.ietf.org/doc/draft-ietf-jmap-refplus/

My overall view of this is still that the added complexity (and execution performance concerns) of this outweighs the performance benefits of reducing a few extra round trips. I'm not objecting to the document moving forward, but I'd love to see some evidence that someone has a real use case for it. Regardless, some more specific concerns:

**§1.2.1. The value of this property in the JMAP Session "accountCapabilities" property is an empty object.**

I don't think the capability should be included in `accountCapabilities`, just the top-level `capabilities`. These changes are not things within a particular account; they apply at a higher level (like JMAP Core). (Caveat, see comment on /set below.)
*
*
**§2.2.1 The nodelist produced by JSON Path evaluation MUST be resolved according to the expected type of the target property**

I *really* don't like this. Instead of being a pure syntactical transformation, the dispatching server must now understand the full expected structure of the method being called in order to apply the result reference. This is a layering violation, and could be impossible for a generic JMAP proxy that doesn't understand (or need to understand) the actual method calls it's dispatching to JMAP servers downstream.

(Yes, I saw later in §3.1 you say *"The type-specific resolution rules defined in this specification (determining whether a resolved value should be treated as a single value, an array, or a map) are applied by the JMAP method implementation layer, not by the reference resolution layer."* — that's still requiring support at multiple layers.)

Instead, why not add an extra `type` property to the ResultReference object that gives the expected type? The caller should know what they expect this to resolve to, and that means the reference resolver doesn't need to know. This lets everything be done at the Syntactic Resolution Phase (per §3.2).

**§2.3 Usage in /set Methods**

I think we need to explicitly state that should you opt in to the refplus capability you *cannot *set a property within a Foo object that beings with a literal `#`. This is not a problem with any of the data types being used with JMAP so far (as far as I'm aware), but worth noting for the future. (If for some reason a client *does* need to do this in the future, they must not include the refplus capability in the `using` array of the Request object; as per RFC8620, if not explicitly opted in, the server must not apply the capability.)

**§2.3**** If a result reference fails to resolve, the server MUST reject the creation or update of that specific object with a SetError of type "invalidResultReference"**

This is problematic, because it means the resolution has to happen at method execution time, rather than at method dispatch time. I don't think it's necessary: we can resolve the references before dispatch and reject the whole method with `invalidResultReference` if any fail. That's consistent with all the other result reference resolution, and means it can be handled in the same layer.

Otherwise, if we really want to keep the currently specified behaviour, I think we will need to add the refplus capability to the `accountCapabilities` of accounts that support using ResultReferences inside `/set` create/update. Again, in the proxy case it may not be all of them.

> https://datatracker.ietf.org/doc/draft-ietf-jmap-metadata/

I think this is looking fairly solid. A few comments:

**§2.1.2 Servers SHOULD document whether they provide read-only or read-write access to ImapMetadata.**
**§2.1.3 **Servers SHOULD document whether they provide read-only or read-write access to WebDavMetadata.****

Why not put this in the capabilities?

**§***3.1 "alreadyExists" SetError […] MAY include an "existingId" property containing the id of the existing Metadata object.*

I'd change this MAY to a MUST — it's easy to implement, and if it's only a MAY a client can't rely on it, which is a pain. (As a general rule, 

**§4.1 The response to an extended /get method includes the following additional property**

I think we probably need to include `metadataState` in the response as well?
**
**
**§4.2.1 Servers SHOULD NOT generate […] state changes for automatically deleted metadata. The deletion is considered an implicit consequence of destroying the related object. However, servers MAY emit push notifications or update metadata state strings to reflect that metadata has been deleted, if such notifications would otherwise occur for explicitly deleted metadata.**

I'm not quite sure what you're going for here. Surely the metadata state *has* to change if a metadata object is destroyed, regardless of how this happened. Otherwise, the `/changes` updates won't work correctly.

Related, the document doesn't currently specify that changes purely to a Metadata object SHOULD NOT change the state string for the related type, but I think that's the idea. Although…

*Is this the right approach?*

With my client implementer hat on, I was thinking about how I would want to use metadata. Generally, I'd want to be able to say "I'm interested in metadata for this particular type" and then fetch/update it with the instances of that type themselves. I can't think of a circumstance where I'd want metadata without the related object, but I could definitely just have a small subset of the dataset for a particular object (e.g. I have loaded 100 emails for a user with 1,000,000 messages). The current approach is a bit painful:
 • I can fetch the metadata with the Email objects by using the *fetchMetadata* argument. Fine.
 • The Metadata state changes. I call `Metadata/changes` with *filterRelatedType* set to `"Email"`. I can ignore any *updated* metadata objects I don't have, but I have to always fetch the *created* metadata objects, as I have no way of knowing if they're related to the Email objects I currently have in memory or not.
 • I can't search for a Foo object that has some particular metadata AND a regular property (e.g. if I used metadata to store a memo <https://www.fastmail.com/features/memos/> on an Email, and wanted to search for all emails that are in the inbox and have a memo with the text "foo").
A very different approach would be:
 • Data types with metadata support (as per the capabilities) get a mutable `_metadata_: String[*]` property if you opt in to the capability. We could have a `_private_metadata_` property too for private metadata, if supported (per the capability). 
 • The value of this property is a (possibly empty) object (never null so you can always patch). The keys at the top level are registered, or a domain name:
   • `imap`: maps to IMAP metadata
   • `webdav`: maps to WebDAV metadata
   • `example.com`: maps to arbitrary vendor data.
 • The metadata property can be specified in the *properties* argument to `Foo/get`, like any other property. If you don't specify any properties, it will be returned with the default set of properties (normally everything) for that data type. (Or maybe we'd need a way to say "give me all the default properties, but *not* the metadata properties for this object type"?)
 • `Foo/changes` returns objects where the `_metadata_` property has changed in the *updated* argument of the response. We add an *updatedProperties* argument to the response, like in Mailbox/changes <https://www.rfc-editor.org/rfc/rfc8621.html#section-2.2>. If only metadata has changed, the server should use this to indicate that, so you can then only fetch the metadata property. Possibly we add an extra argument to `Foo/changes` to ignore metadata-only changes for data types where you're not interested in the metadata.
 • We define some extensions to the `Foo/query` FilterCondition for any data type that supports metadata:
   • the presence of metadata at a particular path, e.g., `{"metadataExists": "fastmail.com/memo"}` — this matches any object where it has a value defined at that path.
   • text match at a particular path. e.g., `{"metadataText": { path: "fastmail.com/memo", value: "foo"}` — looks for a string in the property at a particular path. Is false if the path doesn't exist or isn't a string type.
 • That's … kind of it? You can use standard `Foo/set` patching to update specific metadata properties without stomping on other vendors’ metadata. You always keep your metadata in sync with the underlying objects. You don't really need to change any code in the client, and all existing sync code will just work.
Thoughts? This seems both simpler and more functional to me, and I don't see any obvious drawbacks at first glance.

Cheers,
Neil.