Re: [Jmap] I-D Action: draft-ietf-jmap-sieve-01.txt

[Neil Jenkins <[e-mail]>]

Some comments on this draft:

>    o  Do we need/want both "content" and "blobId" in the SieveScript
      object?  It may be simpler to have just one way of specifying
      content and "blobId" is more versatile and doesn't require JSON-
      encoding of the content.  Furthermore, use of the forthcoming(?)
      Blob/set method would avoid the extra roundtrip of having to
      upload the blob first.

Yes, I think just blobId will probably be better for this, especially if we have methods for manipulating textual blobs in band.

General comments:
 1. Is the contents of the script meant to be immutable or not? The "content" property is apparently mutable, but the "blobId" property is not. This makes no sense.
 2. I find the reference to JSON in the "content" property very confusing. JSON is just the serialisation format and should not be relevant here. For that matter, the discussion of Sieve encoding is also confusing; since this is defined as just the raw octets of the script, why is there extra encoding involved? Shouldn't it just be the script data? I *think* you are just trying to remind people of how strings are written in Sieve, but that seems needless and confusing here to me.
 3. The spec says "When creating or updating a script, a client MUST include either a  _content_ or a _blobId_ property." – this means you can't just update the name of the script. But the name does look like it's meant to be mutable right?
 4. "The id of the SieveScript to activate if the create/update/destroy succeeds." <- Which create/update/destroy? All of them? Or just the one referenced by onSuccessActivateScript? I'm guessing now you mean the latter, but this was not clear to me initially (especially because "destroy" is in there, which doesn't really make sense).
 5. The "replaceOnCreate" arg feels a little ugly to me, but I get why it's there and I don't have a better suggestion.
 6. I think the semantics of the "alreadyExists" SetError is appropriate for the duplicate name issue, so no need to define a new "scriptNameExists" error.
 7. Similarly, I think the "tooManyScripts" error should be just a standard "overQuota" error.
 8. In SieveScript/validate, you specify an "invalidProperties" SetError if no content/blobId or both. This should instead be a (method-level) invalidArguments error.
 9. lastVacationResponse should be of type UTCDate, not Date. (Will need a reference for the data type too).
 10. The /test example request doesn't include an accountId.
Cheers,
Neil.