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

From: Eliot Lear <[email-masked]>
Date: Thu, 08 Mar 2012 11:40:29 +0100

I have been selected as the Applications Area Directorate reviewer for
this draft (for background on appsdir, please see
 http://trac.tools.ietf.org/area/app/trac/wiki/ApplicationsAreaDirectorate).

Please resolve these comments along with any other Last Call comments
you may receive. Please wait for direction from your document shepherd
or AD before posting a new version of the draft.

Document:  draft-desruisseaux-caldav-sched-11
Title: CalDAV Scheduling Extensions to WebDAV
Reviewer: Eliot Lear
Review Date: 8 Mar. 2012
IESG Telechat Date: 15 Mar 2012

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.

Major Issues:

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

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.

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".

§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.

>    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.

§1, ppg 6-7:

>    Scheduling works by having the client store iCalendar data with
>    appropriate scheduling properties included ("ORGANIZER" and
>    "ATTENDEE" iCalendar properties).  This is called a "Scheduling
>    Operation" and fully described in Section 3.  The server detects that
>    the data represents a scheduling operation (as per Section 3.1) and
>    identifies the operation as one initiated by the Organizer or
>    Attendee.  Processing of the scheduling operation, then depends on
>    who is initiating the scheduling operation (as per Section 3.2.1 or
>    Section 3.2.2 respectively).  In each case, one of three scheduling
>    operations can take place: "create", "modify" or "remove".  Which
>    operation occurs is based on the HTTP method used for the request, as
>    well as a comparison between any existing iCalendar data and any new
>    iCalendar data in the request (as per Section 3.2.3).

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.

§1, pg. 7:
>    Control over who is allowed to send scheduling messages, or from whom
>    scheduling messages will be received or freebusy results returned to,
>    is controlled via a set of access control privileges dedicated to the
>    various scheduling operations that can occur as per Section 6.

Suggest active tense reword as follows:
> Section 6 describes access authorization mechanisms for the various
> scheduling operations that this memo specifies.
§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.

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?

§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?

§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?

§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.

§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)?

Also see nit below on tables.

§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.

§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.

§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?

§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"

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

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
§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?

§3.2.10.1, §3.2.10.1, pg 34:

Change "Clients can" to "Clients MAY"

Nits:

§1, pg. 6:

> This is called a "Scheduling
>    Operation" and fully described in Section 3
Missing verb "is".

Throughout:

Calendar User is not capitalized consistently.

Throughout:

Tables should be numbered for reference.

---

From: Julian Reschke <[email-masked]>
Date: Thu, 08 Mar 2012 13:35:06 +0100

On 2012-03-08 11:40, Eliot Lear wrote:
> ...

This is similar to other specs building on WebDAV, and we haven't done 
that (saying "updates") before.

> information, if returned?

SHOULD NOT is right; returning it does no harm. Also, it's consistent 
with other WebDAV specs. Having SHOULD NOT in one place and MUST NOT 
somewhere else would be awkward.

> Tables should be numbered for reference.

If this is supposed to be a general rule, I would object to it. It's a 
matter of style, and thus should be discussed in the RFC style manual, 
if it ever gets updated.

Best regards, Julian

---

From: Eliot Lear <[email-masked]>
Date: Thu, 08 Mar 2012 14:18:39 +0100

> somewhere else would be awkward.

Great.  What's the client behavior if it sees it?

> matter of style, and thus should be discussed in the RFC style manual,

No it's not.  It's for internal referencing.

Eliot

---

From: Julian Reschke <[email-masked]>
Date: Thu, 08 Mar 2012 14:25:27 +0100

> Great.  What's the client behavior if it sees it?

Essentially, the client didn't ask for the property but the server has 
sent it anyway. The client can throw it away, or keep it for later use 
(the latter case would be an edge case, because if the client had been 
interested, it would have asked for the property right away).

> No it's not.  It's for internal referencing.

I just checked the spec and couldn't see any case where the tables being 
numbered would have made referencing them easier.

IMHO, "the table below" is more helpful as "table NN" and then having 
the tables being numbered. Things would be different if the references 
were more complex, or in a more complex presentation where tables might 
be moved around (which we do not do in RFCs).

Best regards, Julian

-- 
<green/>bytes GmbH, Hafenweg 16, D-48155 Münster, Germany
Amtsgericht Münster: HRB5782

---

From: Eliot Lear <[email-masked]>
Date: Thu, 08 Mar 2012 14:52:42 +0100

Julian,

Please read my review where I suggest text that references tables.

Eliot

On 3/8/12 2:25 PM, Julian Reschke wrote:
> I just checked the spec and couldn't see any case where the tables
> being numbered would have made referencing them easier.
>
> were more complex, or in a more complex presentation where tables
> might be moved around (which we do not do in RFCs).
>
> Best regards, Julian
>
>

---

From: Cyrus Daboo <[email-masked]>
Date: Thu, 08 Mar 2012 10:51:47 -0500

Hi Eliot,

> Please read my review where I suggest text that references tables.

Just on this one point:

1) There is only ever one table per section in the draft, so a reference to 
a table can be reasonably provided as "the table in Section XXX".

2) Most the the tables are defined using xml2rfc <texttable> elements. 
However, some tables are too complex for xml2rfc's table layout, and hence 
are implemented as ascii-art in a <figure>. That poses a problem when using 
xml2rfc's built-in references because <texttable> references appear as 
"Table 1", whereas <figure> references appear as "Figure 1". That would 
mean we had a mixture of different reference labels. Now, it is possible to 
change all the current <texttable> elements to be ascii art instead, but 
given point #1 above, I would rather not have to do that.

So bottom line, my preference is to use a reference of the form described 
in #1 above whenever needed.

-- 
Cyrus Daboo

---

From: Eliot Lear <[email-masked]>
Date: Thu, 08 Mar 2012 19:15:58 +0100

Hi Cyrus,

On 3/8/12 4:51 PM, Cyrus Daboo wrote:
> 1) There is only ever one table per section in the draft, so a
> reference to a table can be reasonably provided as "the table in
> Section XXX".
>
> However, some tables are too complex for xml2rfc's table layout, and
> hence are implemented as ascii-art in a <figure>. That poses a problem
> when using xml2rfc's built-in references because <texttable>
> references appear as "Table 1", whereas <figure> references appear as
> "Figure 1". That would mean we had a mixture of different reference
> labels. Now, it is possible to change all the current <texttable>
> elements to be ascii art instead, but given point #1 above, I would
> rather not have to do that.

>
> So bottom line, my preference is to use a reference of the form
> described in #1 above whenever needed.
>

Ok.  I feel your pain on this point.

Eliot

---

From: Cyrus Daboo <[email-masked]>
Date: Thu, 08 Mar 2012 17:18:59 -0500

Hi Eliot,
Thank your for your detailed review.

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

> ready for publication.  I would suggest one more respin.  I believe
> this could happen, however, AFTER the IESG discussion.

OK, that makes sense.

> 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.

> for the text to be non-normative.  If it is meant to be normative, be

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?

> 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]).

> This document needn't and shouldn't repeat or review iTIP.  Suggest

OK - done.

> This is mundane and not needed in § 1.  Suggest you cut it.

OK - done.

> Let's be more clear about about the theory of operation specified in this
> document.  Suggest reword along the following lines:
>

See next item.

> 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.

> 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.

> 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.

> 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.

> 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.

>  of the "SCHEDULE-AGENT" property.

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

> Cannot the text in §3.2.1.1 and §3.2.1.2 simply be moved into the

OK - done.

>  inspect each "ATTENDEE" property in both the original and modified event
> instance

OK - done.

>  according to Table XXX below.

OK - done.

> 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.

> Remove (1) and replace with "Same as PUT in Table XXX"

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

> (2) and replace with "Same as DELETE")

OK - done.

> When the server receives a MOVE request, it SHALL execute

OK - done.

> Should this memo indicate that it updates RFC-5546?

Yes it should. Done.

> Change "Clients can" to "Clients MAY"

OK - done.

>     Operation" and fully described in Section 3

No longer relevant as that has been re-written.

> Calendar User is not capitalized consistently.

Lowercased everywhere to be consistent with 5546.

> 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

---

From: Eliot Lear <[email-masked]>
Date: Fri, 09 Mar 2012 07:19:57 +0100

Cyrus,

The changes you've incorporate address my concerns.

Eliot

On 3/8/12 11:18 PM, Cyrus Daboo wrote:
> 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:
> 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.
>
> 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?
>
>> this
> 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:
>>
> 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.
>
> 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.
>
> 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:
>>
>>
> Change applied, but still uses "The table below ..." rather than a
> direct "Table N" reference.
>
>>
>> event
> 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.
>
> I have attached my working copy with the changes described above as
> well as an HTML diff for your convenience.
>