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

[Mary Barnes <[e-mail]>]

Hi Henry,

Thank you for your detailed review.  Responses are inline below [MB].

Regards,
Mary.

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]

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

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

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

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

 [/MB]

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

>
> 11. XML Schema
>
> An http URI should be provided where this schema document can be found
> on its own, and an update policy for it (or, preferably, _two_ URIs,
> one for exactly this schema document, and one which will be updated if
> this document is revised or superseded).  (Likewise for DataModel.xsd
> and rfc4575.xsd.)
>
[MB] Agreed. [/MB]

>
> 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?
[/MB]

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

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

[/MB]


A similar problem occurs in the response in 6.3
(floor-request-handling)
[MB] Yep. [/MB]

>
> 6.9 Alice exploits a CCMP server extension
>
> For compatibility with the actual response given, the extension schema
> document should have a target namespace, as follows:
>
>   <?xml version="1.0" encoding="UTF-8"?>
>   <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
>              targetNamespace="http://example.com/ccmp-extension-schema.xsd
> "
>              xmlns="http://example.com/ccmp-extension-schema.xsd">
>
>     <xs:element name="confSummary" type="conf-summary-type"/>
>
>     <xs:complexType name="conf-summary-type">
>       <xs:sequence>
>         <xs:element name="title" type="xs:string"/>
>         <xs:element name="status" type="xs:string"/>
>         <xs:element name="public" type="xs:boolean"/>
>         <xs:element name="media" type="xs:string"/>
>       </xs:sequence>
>     </xs:complexType>
>
>   </xs:schema>
>
> Or, better, the example _and_ the schema should be changed to read as
> follows:
>
[MB] Yes, the suggested change below is better. [/MB]

>
>   <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
>    <ccmp:ccmpResponse xmlns:info="urn:ietf:params:xml:ns:conference-info"
>           xmlns:ccmp="urn:ietf:params:xml:ns:xcon:ccmp"
>           xmlns:xcon="urn:ietf:params:xml:ns:xcon-conference-info"
>           xmlns:example="http://example.com/ccmp-extension">
>      <ccmpResponse xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
>          xsi:type="ccmp:ccmp-extended-response-message-type">
>          <confUserID>xcon-userid:[e-mail]</confUserID>
>          <confObjID>xcon:[e-mail]</confObjID>
>          <operation>retrieve</operation>
>
>
>          <response-code>200</response-code>
>          <response-string>success</response-string>
>          <ccmp:extendedResponse>
>             <extensionName>confSummaryRequest</extensionName>
>             <example:confSummary>
>                 <title> Alice's conference </title>
>                 <status> active </status>
>                 <public> true </public>
>                 <media> audio </media>
>             </example:confSummary>
>          </ccmp:extendedResponse>
>      </ccmpResponse>
>    </ccmp:ccmpResponse>
>
>    <?xml version="1.0" encoding="UTF-8"?>
>    <xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"
>               targetNamespace="http://example.com/ccmp-extension"
>               xmlns="http://example.com/ccmp-extension">
>
>      <xs:element name="confSummary" type="conf-summary-type"/>
>
>      <xs:complexType name="conf-summary-type">
>        <xs:sequence>
>          <xs:element name="title" type="xs:string"/>
>          <xs:element name="status" type="xs:string"/>
>          <xs:element name="public" type="xs:boolean"/>
>          <xs:element name="media" type="xs:string"/>
>        </xs:sequence>
>      </xs:complexType>
>
>    </xs:schema>
>
> Otherwise I've checked all the schemas for conformance and the
> examples for schema-validity.
>
> 12.2 XML Schema Registration
>
> Should include pointers to the RFCs which include the text of the
> schema documents named as "DataModel.xsd" and "rfc4575.xsd" in the
> schema docuemnt given in section 11.
>
[MB] Okay. [/MB]

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

>
> Nits: One more proofreading pass over the first three sections would
> be a good idea. . .
>
[MB] I'll give it a go, but it appears that it's section 3 that needs some
tidying. [/MB]

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