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

From: [email-masked] (Henry S. Thompson)
Date: Mon, 16 May 2011 10:04:29 +0100

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.

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.

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?

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?

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

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

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.

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.

A similar problem occurs in the response in 6.3
(floor-request-handling)

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:

   <?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:[email-masked]</confUserID>
	  <confObjID>xcon:[email-masked]</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.

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.

Nits: One more proofreading pass over the first three sections would
be a good idea. . .

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

---

From: Robert Sparks <[email-masked]>
Date: Tue, 17 May 2011 09:16:10 -0500

Adding the XCON WG list

---

From: Mary Barnes <[email-masked]>
Date: Mon, 23 May 2011 13:57:50 -0500

Hi Henry,

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

Regards,
Mary.

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

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

>   way.  why not simply have messages which request new conference or
[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]

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

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

> this document is revised or superseded).  (Likewise for DataModel.xsd
[MB] Agreed. [/MB]

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

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

[MB] Yep. [/MB]

> Or, better, the example _and_ the schema should be changed to read as
[MB] Yes, the suggested change below is better. [/MB]

> schema docuemnt given in section 11.
[MB] Okay. [/MB]

> much use---4 characters seems to be the practical limit for
[MB] How about 'ccmp'?

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

---

From: Mary Barnes <[email-masked]>
Date: Mon, 23 May 2011 13:59:25 -0500

I'm resending as Richard Barnes (XCON WG co-chair) email was incorrect and I
want to make sure he's included in any follow-up.

On Mon, May 23, 2011 at 1:57 PM, Mary Barnes <[email-masked]>wrote:

> On Mon, May 16, 2011 at 4:04 AM, Henry S. Thompson <[email-masked]>wrote:
>
>  Since the XML documents carried in the body of CCMP requests/responses
> need to be compliant with the XCON data model..."
>
>  [/MB]
>
>>
> [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
>>
> [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)...
> 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]
>
>
>>              targetNamespace="
>> http://example.com/ccmp-extension-schema.xsd"