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

Mary Barnes <mary.ietf.barnes@gmail.com> Mon, 23 May 2011 18:59 UTC

Return-Path: <mary.ietf.barnes@gmail.com>
X-Original-To: apps-discuss@ietfa.amsl.com
Delivered-To: apps-discuss@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id D652DE06D1; Mon, 23 May 2011 11:59:28 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -102.369
X-Spam-Level:
X-Spam-Status: No, score=-102.369 tagged_above=-999 required=5 tests=[AWL=-1.171, BAYES_00=-2.599, HTML_MESSAGE=0.001, J_CHICKENPOX_25=0.6, J_CHICKENPOX_26=0.6, J_CHICKENPOX_43=0.6, J_CHICKENPOX_93=0.6, RCVD_IN_DNSWL_LOW=-1, USER_IN_WHITELIST=-100]
Received: from mail.ietf.org ([64.170.98.30]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id gjNAxPAWpBWu; Mon, 23 May 2011 11:59:27 -0700 (PDT)
Received: from mail-vx0-f172.google.com (mail-vx0-f172.google.com [209.85.220.172]) by ietfa.amsl.com (Postfix) with ESMTP id 9ADF0E06AB; Mon, 23 May 2011 11:59:26 -0700 (PDT)
Received: by vxg33 with SMTP id 33so5196316vxg.31 for <multiple recipients>; Mon, 23 May 2011 11:59:26 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=8CnjjX8xsrxwUJ871/ag5b79cO1bkpe0czS6d+sndN8=; b=QGa+pr4hiTeXsBCoo4n+ScBsViwtRPbBjgXhHE4BmuG+jZFeR2hVFEkAIEFx24hS06 IchX7p+A6gnpP0Pludq9aDe4ew9lTv1HoMC/vkHK8o643PskGKWsovj1M4Odldb/dp00 Gnp8Z86Xai/Um6y7ZEqSUE7SGNU1MhNsLUoxc=
DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=DX8ENW8BvePIWbgGO3B4WC+kAj8TNzT/V60/N1k+QyWJ3beTOsz/0b/2k7vQu3/alx fHwNo+5+p5pXWnKQ4Bug4xLc8+3IGMK3Tzwaz26RIkzK1iyE3X7VmSK6HSmE+lU3boa4 TFFRIVVul/Ijhix9ji7EYK8k/MZH2hMVm8PI8=
MIME-Version: 1.0
Received: by 10.52.97.10 with SMTP id dw10mr276529vdb.23.1306177165912; Mon, 23 May 2011 11:59:25 -0700 (PDT)
Received: by 10.52.160.132 with HTTP; Mon, 23 May 2011 11:59:25 -0700 (PDT)
In-Reply-To: <BANLkTi=BKTKVTEDS7TD48JMS+9Dgp_Fnvg@mail.gmail.com>
References: <f5br57zt3s2.fsf@calexico.inf.ed.ac.uk> <BANLkTi=BKTKVTEDS7TD48JMS+9Dgp_Fnvg@mail.gmail.com>
Date: Mon, 23 May 2011 13:59:25 -0500
Message-ID: <BANLkTinpLY8FFoUBPCXz8+3k4uos+ftTeg@mail.gmail.com>
From: Mary Barnes <mary.ietf.barnes@gmail.com>
To: "Henry S. Thompson" <ht@inf.ed.ac.uk>
Content-Type: multipart/alternative; boundary="20cf307d01eefceb6c04a3f6119f"
Cc: Alan Johnston <alan.b.johnston@gmail.com>, apps-discuss@ietf.org, Richard Barnes <rbarnes@bbn.com>, Simon Pietro Romano <spromano@unina.it>, Chris Boulton <chris@ns-technologies.com>, xcon@ietf.org, iesg@ietf.org, Henning Schulzrinne <hgs+xcon@cs.columbia.edu>
Subject: Re: [apps-discuss] apps-team review of draft-ietf-xcon-ccmp-13
X-BeenThere: apps-discuss@ietf.org
X-Mailman-Version: 2.1.12
Precedence: list
List-Id: General discussion of application-layer protocols <apps-discuss.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=unsubscribe>
List-Archive: <http://www.ietf.org/mail-archive/web/apps-discuss>
List-Post: <mailto:apps-discuss@ietf.org>
List-Help: <mailto:apps-discuss-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/apps-discuss>, <mailto:apps-discuss-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 23 May 2011 18:59:29 -0000

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 <mary.ietf.barnes@gmail.com>wrote:

> 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 <ht@inf.ed.ac.uk>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:Alice@example.com</confUserID>
>>          <confObjID>xcon:8977794@example.com</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: (44) 131 651-1426, e-mail: ht@inf.ed.ac.uk
>>                       URL: http://www.ltg.ed.ac.uk/~ht/
>>  [mail from me _always_ has a .sig like this -- mail without it is forged
>> spam]
>>
>
>