Re: [apps-discuss] Applications Directorate Review of draft-desruisseaux-caldav-sched-11

[Cyrus Daboo <[e-mail]>]

Hi Eliot,
Thank your for your detailed review.

--On March 8, 2012 11:40:29 AM +0100 Eliot Lear <[e-mail]> wrote:

> Summary:
>
> This is a marked improvement from the previous version, represents YET
> ANOTHER very valuable contribution from these two authors, and is nearly
> ready for publication.  I would suggest one more respin.  I believe
> this could happen, however, AFTER the IESG discussion.

OK, that makes sense.

>
> Major Issues:
>
>
> A question: can the new status codes defined in this specification leak
> to non-participating implementations in Reply operations?

Strictly speaking the new 1.xx codes are only meaningful when used on the 
SCHEDULE-STATUS parameter and that parameter is required to be removed from 
any iTIP message the client or server sends (as per Section 7.3). So 
leakage should be minimal. That said, RFC5545 does define the overal 
meaning for the 1.xx class of codes. However, neither iCalendar nor iTIP 
explains what clients should do if they get a specific code they do not 
recognize. I don't believe that there is any known interoperability problem 
related to that (I suspect clients either ignore unknown codes outright, or 
base their behavior on the status code class). If something needs to be 
addressed here, it really ought to be in iCalendar or iTIP, and perhaps 
filed as an erratum against one or both of those.

> Minor Issues:
>  In general the document suffers from a lack of conciseness.  What
> follows are some of the more glaring examples.
>
> In Section 1, 2nd paragraph, There is no need to describe so early what
> functions aren't covered.  The way this is traditionally handled is to
> have a "Future Work" section.  That can be done in the intro or it can
> even be put after examples.  I prefer the latter, especially if you mean
> for the text to be non-normative.  If it is meant to be normative, be
> explicit about it.

One reason that is there is that the we had (early on) a lot of 
implementors asking us about those other methods not being covered. So we 
felt it reasonable to add some commentary that they have been deliberately 
left out.

Now I would be willing to move that into a "Potential Future Work" 
appendix. But I could argue that the 4th paragraph of Section 1 should also 
be moved there (the one about not addressing support for "external" users). 
Do you think that should be moved too?

> Similarly, in the 4th paragraph, same section, it is sufficient to say
> (above) that this mechanism works compatibly with iMIP. You're using a
> double negative.  And this problem recurs in the same paragraph.  You
> should consider a search on the word "not".

How about:

   This specification only addresses how scheduling occurs with users on
   a single system (i.e., scheduling between CalDAV servers, or some
   other calendaring and scheduling system, is not defined).  However,
   this specification is compatible with servers being able to send or
   receive scheduling messages with "external" users (e.g., using the
   iCalendar Message-Based Interoperability Protocol iMIP [RFC6047]).


> §1, pg 6:
>
>
>    In iTIP-based scheduling, there is an event "Organizer" whose role is
>     to schedule an event between one or more "Attendees", and this is
>     done by sending out invitations and handling responses from each
>     Attendee.  Often times an Organizer will do a "freebusy" lookup to
>     check on Attendees' availability (free-time).
>
>
> This document needn't and shouldn't repeat or review iTIP.  Suggest
> remove.

OK - done.

>
>    Servers supporting this specification advertise their capabilities
>     and provides new collections for scheduling operations as described
>     in Section 2.
>
>
> This is mundane and not needed in § 1.  Suggest you cut it.

OK - done.

> §1, ppg 6-7:

> Let's be more clear about about the theory of operation specified in this
> document.  Suggest reword along the following lines:
>
> In order to automate the process of scheduling, we define the term
> "scheduling operation" that consists of a client storing an iCal VEVENT
> in a CALDAV store and then the server taking specific actions in
> response.  In each case... (and lop off "as per..").
>
> Also add one sentence here about atomicity (or lack thereof) of the
> operations.

See next item.

> §1, pg. 7:
>
> Suggest active tense reword as follows:

What I have done is reword and compress the four paragraphs that 
"introduce" the key sections. Now I have:

   Section 3 defines the automated "Scheduling Operations", that allow a
   client to store iCalendar data on a CalDAV server, with the server
   taking specific actions in response.  One of three scheduling
   operations can take place: "create", "modify" or "remove", based on
   the HTTP method used for the request, and a comparison between any
   existing and any new iCalendar data.

   Section 4 defines how the server processes scheduling messages sent
   as the result of a scheduling operation.

   Section 5 defines how freebusy requests with an immediate response is
   accomplished.

   Section 6 defines access control privileges for the scheduling
   operations defined in this specification.

>  §1.1, ppg 7-8:
>
> No need to restate definitions from other documents– unless you see a
> difference in the meaning of the term amongst documents.  In particular,
> I see that you chose to reference RFC-3283 instead of RFC-5546 for
> Calendar User.  3283 would be a downref if you referenced it
> normatively.  The definition should be normative.  You want 5546.

I have removed the terminology items repeated from other specs, so now it 
just lists the new ones being defined. As a result the 3283 reference has 
been completely eliminated.

> Also see nit on this.
>
> §1.3 pg 9:
>
> I've asked Mark Nottingham for an additional set of eyes on this
> section.  I have one immediate comment and question:
>
>
>    This document inherits, and sometimes extends, DTD productions from
>     Section 14 of [RFC4918].
>
>
> I would reword- "This document inherits and extends DTD productions..."
>
> If so, should this specification update RFC4918 in rfc-index.txt?

The text used here is an exact copy of what was used in CardDAV (RFC6352) 
which was based on what we used in CalDAV (RFC4791) with specific changes 
and clarifications that Julian had requested. It reflects standard WebDAV 
XML handling behavior.

> §2.1, pg 10, and similar text in §2.2, pg 11:
>
>
>    The following WebDAV properties specified in CalDAV "calendar-access"
>     [RFC4791] MAY also be defined on scheduling...
>
>
>
> Do you mean "Only the following WebDAV..."?  If not, why state these
> properties?

There are some other properties from CalDAV we are not specifying because 
they have no meaning on an Outbox (e.g. calendar-timezone). And there are 
properties on other types of CalDAV resource that are not relevant (e.g. 
calendar-home-set). So I think the working and intent here is fine.

> §2.1.1, pg 11, and similar text in §2.2.1, pg 13, §2,4,1, pg 14, and
> similar:
>
>
>    PROPFIND behavior:  This property SHOULD NOT be returned by a
>        PROPFIND allprop request (as defined in Section 14.2 of
>        [RFC4918]).
>
>
> Why SHOULD NOT and not MUST NOT?  How should the client interpret the
> information, if returned?

As Julian has noted, this is common practice for the definition of WebDAV 
properties. In some cases a server might decide that a live property is 
trivial to compute and worthwhile to return in an allprop response. Since 
allprop is required to return all dead properties, client have to be able 
to handle getting back properties they don't care about.

> §3.2.1.1, pg 18:
>
>
>    When a scheduling object resource is created by the Organizer, the
>     server will inspect each "ATTENDEE" property to determine if a
>     scheduling message ought to be delivered to this Attendee according
>     to the value of the "SCHEDULE-AGENT" property parameter (see
>     Section 7.1) as described in the table below:
>
>
> Please use normative language and active tense.  Suggest rewording as
> follows:
>
>
> When the Organizer creates a scheduling object resource, the server SHALL
>  inspect each "ATTENDEE" property to determine whether to send a
> scheduling
>  message.  Table XXX below indicates the appropriate action, based on
> the value
>  of the "SCHEDULE-AGENT" property.

Change applied, but still uses "The table below ..." rather than a direct 
"Table N" reference.

>
> §3.2.1.1 AND in §3.2.1.2 and §3.2.1.3:
>
> Why are you treating authorization differently between create & modify
> operations and the remove operation?
>
> Cannot the text in §3.2.1.1 and §3.2.1.2 simply be moved into the
> chapeau (e.g., 3.2.1)?

OK - done.

> §3.2.1.2, pg 19 has similar, but more involved issues to §3.2.1.1:
>
>
>    When a scheduling object resource is modified by the Organizer, the
>     server will inspect each "ATTENDEE" property in the new calendar
> data
>     to determine which ones have the "SCHEDULE-AGENT" iCalendar property
>     parameter.  It will then need to compare this with the "ATTENDEE"
>     properties in the existing calendar object resource that is being
>     modified.
>
>
>
>    For each Attendee in the old and new calendar data on a per-instance
>     basis, and taking into account the addition or removal of Attendees,
>     the server will determine whether to deliver a scheduling message to
>     the Attendee.  The following table determines whether the server
>     needs to deliver a scheduling message, and if so which iTIP
>     scheduling method to use.  The values "SERVER", "CLIENT", and
> "NONE"
>     in the top and left titles of the table refer to the
> "SCHEDULE-AGENT"
>     parameter value of the "ATTENDEE" property, and the values
> "<Absent>"
>     and "<Removed>" are used to cover the cases where the "ATTENDEE"
>     property is not present (Old) or is being removed (New).
>
>
>
> So.. active tense again, same sort of idea:
>
>
> When the Organizer modifies a scheduling object resource, the server SHALL
>  inspect each "ATTENDEE" property in both the original and modified event
> instance
>  to determine whether to send a scheduling message.  Table XXX below
> indicates the
>  appropriate action, based on the value of the "SCHEDULE-AGENT" property.
>  The values "SERVER", "CLIENT", and "NONE" in the top and left headings
> refer to the "SCHEDULE-AGENT"  parameter value of the "ATTENDEE"
> property.
>  The the values "<Absent>"  and "<Removed>" are used to cover the cases
> where the
>   "ATTENDEE" property is not present (Old) or is being removed (New).
>
>
> The use of the word ATTENDEE in the upper left-hand corner of the table
> is confusing, and I would remove it.  I might also change the headings
> to read "Original" (going down)" and "Modified".  This allows for
> consistency of language.

OK - done.

> §3.2.1.3, Rinse and repeat this exercise for "Remove":
>
>
>
>    When a scheduling object resource is removed by the Organizer, the
>     server will inspect each "ATTENDEE" property in the scheduling
> object
>     resource being removed to determine which ones have the "SCHEDULE-
>     AGENT" iCalendar property parameter.
>
>    For each Attendee the server will determine whether to attempt to
>     deliver a scheduling message into the Attendee's scheduling Inbox
>     collection, based on the table below:
>
>
> So...
>
>
> When an Organizer removes a scheduling object resource, the server SHALL
>  inspect each "ATTENDEE" property in the scheduling object resource being
>  removed, and act based on the value of the "SCHEDULE-AGENT" property
> value,
>  according to Table XXX below.

OK - done.

> §3.2.2.3, pg 23:
>
>
>    If the Attendee adds an "EXDATE" property value to effectively remove
>     a recurrence instance, the server MUST deliver an iTIP "REPLY"
>     scheduling message to the Organizer to indicate that the Attendee
> has
>     declined the instance.
>
>
> EXDATE isn't the only way an Attendee can decline. a specific event,
> right?  Is this the only operation in which a server MUST act?  Why
> call this one out?

Normally people think that a change in participation status is governed by 
the ATTENDEE;PARTSTAT value. It is not entirely obvious that the act of 
adding an EXDATE to an event means that the attendee has in effect declined 
the event. At least I don't believe there is anything stated to that effect 
in iCalendar or iTIP. So I think it is appropriate to describe the actions 
for both a change to PARTSTAT and EXDATE.

> §3.2.3.2, pg 25, in the COPY table, and 3.2.3.3, pg 26, the MOVE table" :
>
> Remove (1) and replace with "Same as PUT in Table XXX"

I left the note in but reworded as a reference to the section.

> Also, move DELETE above MOVE (removes forward reference) and then remove
> (2) and replace with "Same as DELETE")

OK - done.

> In all of these sections change to both active tense and normative
> language.  So for instance:
>
> Old:
>
>
>  When a MOVE method request is received, the server will execute
>
>  New:
>
>
> When the server receives a MOVE request, it SHALL execute

OK - done.

>  §3.2.9, pg 32:
>
>
>    The 1.xx "REQUEST-STATUS" codes are new.  This specification
> modifies
>     item (2) of Section 3.6 of [RFC5546] by adding the following
>     restriction:
>
>
>
> Should this memo indicate that it updates RFC-5546?

Yes it should. Done.

>
> §3.2.10.1, §3.2.10.1, pg 34:
>
> Change "Clients can" to "Clients MAY"

OK - done.

> Nits:
>
>
> §1, pg. 6:
>
>
>
> This is called a "Scheduling
>     Operation" and fully described in Section 3
>
>  Missing verb "is".

No longer relevant as that has been re-written.

>
> Throughout:
>
>
> Calendar User is not capitalized consistently.

Lowercased everywhere to be consistent with 5546.

> Throughout:
>
>
> Tables should be numbered for reference.

As noted in separate email, this change won't be applied.

I have attached my working copy with the changes described above as well as 
an HTML diff for your convenience.

-- 
Cyrus Daboo