Re: [apps-discuss] apps-team review of draft-ietf-xcon-ccmp-13

[[e-mail] (Henry S. Thompson)]

Mary Barnes writes:

> Thank you for your detailed review.  Responses are inline below [MB].
>
> On Mon, May 16, 2011 at 4:04 AM, Henry S. Thompson <[e-mail]>wrote:
>
>> I have been selected as the Applications Area Review Team reviewer for
>> this draft (for background on apps-review, please see
>> http://www.apps.ietf.org/content/applications-area-review-team).
>>
>> 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-ietf-xcon-ccmp-13
>>
>> Title: Centralized Conferencing Manipulation Protocol
>>
>> Reviewer: Henry S. Thompson
>>
>> Review Date: 2011-05-13
>>
>> Summary: This draft is almost ready for publication as a Proposed
>> Standard but has a few issues that should be fixed before publication
>>
>> Major Issues:
>>
>> 4.2 Data Management
>>
>> 1) The approach to detecting competing updates and their consequences
>>   specified here seems unnecessarily complex.  Was the alternative of
>>   including version numbers in _update_ messages (so that the server
>>   could reject any update whose target version had been superseded)
>>   considered and rejected?  If so, perhaps a brief explanation of why
>>   it was rejected might be helful at this point.
>>
> [MB]   The alternative was considered, however, with the XCON model, the
> server is the only entity that "centralizes" information (in this case, the
> available conference objects). A client might not have the very last version
> of a conference object, due to the potential for concurrent operations
> amongst independent client participants. However, an operation can still
> succeed if there is no overlap amongst the data that is manipulated.  Thus,
> the server-side mechanism is what guarantees coherence in the data stored.
>  We can add some more text around that.  Perhaps, prefacing the paragraph
> with a statement about the model. [/MB]

That sounds like a helpful addition.

>> 2) In a related point, the statement at the end of this section that
>>   "a client subscribed to . . . notifications . . . will always have
>>   the most up-to-date version" is clearly false, in-so-far as it
>>   implies that such a client is guaranteed success for any update, as
>>   there is clearly a race condition here.
>>
> [MB] Yes, I can see that it's really not properly stated. The statement is
> intending to say that the client will "get" the most up-to-date version with
> the next notification - i.e., there is a way to recover in the case that the
> update was either on an earlier version or no response received.  [/MB]

Right, something to that effect. . .

>> 4.3 Data Model Compliance
>>
>> 1) Again this approach seems unnecessarily complex -- why does the
>>   data model have to constrain the initiation of a conference in this
>>   way.  why not simply have messages which request new conference or
>>   new user IDs?
>>
> [MB] We do have messages for a new conference and new user IDs. The intent
> here, was to eliminate the step of requesting those and thus have multiple
> operations as a result of one request.  We can add text around that. [/MB]

Seems like a false economy to me, but yes, at least add a note.

>> 2) I'm also confused by the fact that _elements_ described here as
>>   "mandatory" are not required by the schema.  Specifically in 5.1 we
>>   will see that the 'confUserID' and 'confObjID' elements, which
>>   correspond precisely to XCON-USERID and XCON-URI which are
>>   described here as mandatory, appear in message type definitions as
>>   minOccurs="0", i.e. as optional.  If they are optional, why is the
>>   above gensym complexity needed?  If they are not optional, why
>>   doesn't the schema say so?
>>
> [MB] This is a common practice from what I have seen. This is because the
> base request type is being reused, thus the elements are not mandatory for
> each operation that uses the request type.  So, the expectation is that it's
> mandatory for the protocol for specific operations, but not mandatory in the
> schema for all operations.

In that case, XML Schema provides ways for the derived types to make
inherited optional fields mandatory. . .

> There may also be confusion in that the mandatory elements being referenced
> are those in the *body* of the CCMP messages (the elements as defined in the
> data model document) as opposed to the IDs in the *header* of the CCMP
> messages defined in this document (confUserID and confObjID).

> Perhaps this clarification would help?
> OLD:
>  Since the XML documents carried in the CCMP need to be compliant with the
> XCON data model...
> NEW:
>  Since the XML documents carried in the body of CCMP requests/responses
> need to be compliant with the XCON data model..."

Yes. . .

>>
>> 3) It is unusual to refer to aspects of a data model with words such
>>   as 'element' and 'attribute', which are better reserved for use
>>   with respect to _XML serializations_ of data model instances.  Ah,
>>   I see by looking at draft-ietf-xcon-common-data-model that the XCON
>>   data model is defined as an XML document.  It's undoubtedly too
>>   late to do anything about that, but confounding data models and XML
>>   serializations is usually considered to be a mistake. . .
>>
> [MB] Can you characterize in what sense it is a mistake and what problems
> it might cause?  Or is it purely a nomenclature  issue that is inaccurate?
>  I agree that resolving this concern would impact the data model. [/MB]

Data is not inherently tree-structured.  Thinking about data models as
if they were often leads to awkward-at-best compromises.  But in the
end this was just a comment to explain to myself why what I think of
as XML (serialisation) terminology was being applied to the data
model.  It might be worth including a note early on explaining that
the data model is specified _as_ an XML infoset. . .

>> 12.5 CCMP Protocol Registry
>>
>> Why are these registries needed?  No role is specified for them
>> anywhere in the body of the document. Registries are not free, and if
>> all the information in the registry is also in the published schemas
>> it's not at all clear what purpose they will serve.
>>
> [MB] We define the registries as the protocol is extensible and does
> require specification.  The registries also allow a reference to the
> appropriate document that defines the normative behavior for the new
> operations and new error codes.  This is a standard procedure for RAI area
> protocols - c.f., HELD (RFC 5985), SIP (RFC 3261), Registry for SIP header
> field parameters (RFC 3698)...
> I will note that we don't typically add a reference in the document that we
> are defining the IANA registries.  Is there a particular place you suggest
> we add such a reference?

Same place you end up putting the URIs for the XML Schema documents, maybe?

>
>> Minor Issues:
>>
>>
>> 6.2. Alice gets detailed information about a specific blueprint
>>
>> The blueprintResponse message is not schema-valid per ccmp.xsd.  On
>> lines 32 and 33 of the example read
>>                    <xcon:floor-request-handling>confirm
>>                      </xcon:floor-request-handling>
>> The problem really lies in DataModel.xsd -- whereas (correctly)
>> ccmp.xsd uses xs:token as the base type for enumerated types,
>> DataModel.xsd (in draft-ietf-xcon-common-data-model) uses xs:string,
>> and the string value of the above element is "confirm
>>                      ", which is not one of the allowed values.  The
>> example should be corrected, or, for preference, the schema in
>> draft-ietf-xcon-common-data-model should be changed to use xs:token as
>> the base type for join-handling-type and all other enumerated types.
>>
>> [MB]
>  Agree that the schema in the data model draft could be improved, by
> changing definitions of the enumberated types.  Did you send the authors of
> that document this comment?  Or has that been covered by another apps-team
> review?

No, would you be so kind?

> "confirm" is one of the allowed tokens in the <xcon:floor-request-handling>
> element (at least in the latest version -- 27 -- of the  data model draft.):
>
> <xs:simpleType name="floor-request-handling-type">
>       <xs:restriction base="xs:string">
>         <xs:pattern value="block"/>
>         <xs:pattern value="confirm"/>
>         <xs:pattern value=".+"/>
>       </xs:restriction>
> </xs:simpleType>
>
> The "newline" character was introduced for proper formatting of the document, but we agree that is  not allowed per the definition above (<xs:pattern value=".+"/>). So, we should remove the newline. This change likely needs to be done elsewhere in the document.

I'm all in favour of readability -- no change will be needed if the
schema is changed as suggested above. . .

>> 12.3 Media Type Registration
>>
>> It seems unlikely that the proposed extension of 'ccmpxml' will see
>> much use---4 characters seems to be the practical limit for
>> extensions.
>>
> [MB] How about 'ccmp'?

Yes.

ht
-- 
       Henry S. Thompson, School of Informatics, University of Edinburgh
      10 Crichton Street, Edinburgh EH8 9AB, SCOTLAND -- (44) 131 650-4440
                Fax: [phone], e-mail: [e-mail]
                       URL: http://www.ltg.ed.ac.uk/~ht/
 [mail from me _always_ has a .sig like this -- mail without it is forged spam]