Subject: [Simple] comments on xcap-patch-ops

From: Jonathan Rosenberg <[email-masked]>
Date: Mon, 07 Mar 2005 12:17:28 -0500

I have two major comments on this doc, and a bunch of minor ones. For 
the two major ones:

MAJOR ISSUE 1:

> Many prevailing systems nowadays utilize XML documents.  Sometimes
>    there's need to do some minor changes to the documents and therefore
>    it is useful to transport only the changes e.g.  over the network.
>    The XCAP [4] protocol defines a model where HTTP transport with
>    methods: PUT and DELETE can be used to apply changes to a single XML
>    document by doing one update at a time.  The patch operations will
>    complement that model by allowing several modifications to be applied
>    to the document during one transaction.  

I am concerned that this draft is aiming for a different target than we 
had originally intended.

As I recall, the issue was that we had two documents - xcap-diff and the 
partial notifications, which both needed a way to describe the changes 
that had been made to the document. So, we had some discussions and 
agreed that we would look for a common diff format to be used across 
these mechanisms.

This introduction, instead, says that the scope of the document is not 
this common diff format in notifications, but to present yet another 
approach for xcap, by just using HTTP with PATCH, and then including 
this format in the PATCH bodies.

While it may be the case that this format is usable within PATCH as an 
xcap alternative, I think that this is outside of the scope of what we 
are trying to do here. Thus, I think this introduction and the draft 
title need to be updated to reflect that.

MAJOR ISSUE 2:

We had started this activity, as I recall, to look at unifying the 
xcap-diff and partial notify specifications. However, the way the pathc 
operations are defined, there will be a lot of work to translate the 
xcap operations that get applied to the patch format to send. For a 
client and a server that are doing xcap, this means that the client has 
to take its xcap operations, compute the final document, perform a 
separate diff against the previous doc to compute the changes, or 
compute the changes by transforming the xcap request URI into a set of 
patch operations, and then send that to the client. The client will 
already have to know how to change a document from one form to the other 
using xcap operations, and now it will also have to know how to do that 
using a different set of semantics - the patch operations.

Thus, there will be a lot of overhead for usage of this within xcap. If 
we want to unify the partial notification and xcap-diff formats, we need 
to work to come up with a format that is readily transformable from xcap 
operations, yet also useful for presence endpoints not doing xcap.

The alternative is to conclude that we should not unify these 
mechnanisms, or to decide that this overhead is not an issue.

Many of my comments below on the document reflect this major issue.

More comments:

> When evaluating XPath location steps, namespace expansion follows
>    XPath 1.0 [2] semantics, i.e.  if the QName does not have a prefix
>    then the namespace URI in the expanded name is null.

as you know, this is different from the xcap semantics, where the 
default namespace is defined by the app usage. This means that it will 
require some work to modify the xpath expression in the xcap PUT to tell 
a watcher about what changed in the document. I would suggest that we do 
something closer to xcap, and have the xcap change document define the 
default namespace for the QNames in the xpath expression.

> As all the namespace definitions relevant to the patch operations are
>    within the XML change document the element names within the optional
>    data content are also fully namespace qualified.  This means that the
>    content of the XML change document is independent of the prefixes of
>    the patched document. 

I'm pretty confused by this. Lets say I want to add a new element to the 
document, and the new element is defined within the namespace urn:foo. 
The xcap change document defines a namespace prefix of p1, and binds it 
to urn:foo. The patched document defines a different prefix, p2, for the 
same namespace. So, when the element to be added is in the xcap-change 
document, does it exist with prefix p1 or p2? I read the text above as 
saying p1, which means that the prefixes have to be rewritten to match 
the local prefix definitions for the name namespace URI. Is that what 
you are proposing?

> When doing these XPath evaluations with a fully XPath 1.0 compatible
>    library it means that if e.g.  "pref:node" location step has to be
>    evaluated and <xmlns:pref="urn:ietf:params:xml:ns"/> is thus defined
>    within the XML change document the location step "pref:node" has to
>    be changed to e.g.
>    "*[namespace-uri()="urn:ietf:params:xml:ns"][local-name()="node"]".
>    However, XPath libraries usually support some sort of "namespace
>    registering" APIs so that location step parsing can be done properly
>    without any changes in the location step string once the prefixes
>    versus URIs has been "registered".

as with the xcap spec, i think this kind of API impelemtnation 
guidelines should not appear in protocol specs.

> They will provide a relevant XML change document context.
>    Furthermore, it is anticipated that they will define <add>, <replace>
>    and <remove> elements from the corresponding types defined in this
>    XML Schema.  The XML document fragments defined in this draft MUST be
>    well formed and valid.

I don't understand why this document doesnt define the add, replace and 
remove elements (as opposed to types), without a target namespace, and 
then those get included into the parent schema. I believe you have 
argued that this means that the parent schema cannot define the root 
element; however, since most schemas have element definitions that are 
global, they won't be doing this anyway.

>  If there are several sibling elements in the new document the 'sel'
>    selector MUST include a positional constraint to fulfill an
>    unambiguous request.  

Why does it have to be positional? It just needs to be unambiguous.

> Once the 'sel'
>    selector identifies a single element, the content of the <add>
>    element inludes all the attributes and child elements and e.g.
>    possible namespace definitions, comment nodes and processing
>    instructions.

I'm confused what <add> is intended to do. Does it add the content as a 
child of the selected element?

> OPEN ISSUE: <xcap-error> format could be extended as it could be
>    trivial to respond that e.g.  operation add[2] failed combined with
>    some text description.  HTTP patch seems to favor multi-status
>    responses (WebDAV style).

again, this is only na issue if you are looking to change xcap, which is 
not in scope for this document I thought.

> 6.  Node Selector

The BNF for this differs in several respects from xcap. This is likely 
to make it complicated for implementations that are using this diff 
format to describe changes in xcap documents made by xcap.

Some of the differences I noted:

* usage of text() as a selector
* addition of last() condition
* the "." selector

Thanks,
Jonathan R.

-- 
Jonathan D. Rosenberg, Ph.D.                   600 Lanidex Plaza
Director, Service Provider VoIP Architecture   Parsippany, NJ 07054-2711
Cisco Systems
[email-masked]                              FAX:   (973) 952-5050
http://www.jdrosen.net                         PHONE: (973) 952-5000
http://www.cisco.com

_______________________________________________
Simple mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/simple

---

From: Jari Urpalainen <[email-masked]>
Date: Wed, 09 Mar 2005 18:21:12 +0200

Inline,
Thanks for the commnets.

ext Jonathan Rosenberg wrote:

> I am concerned that this draft is aiming for a different target than 
> we had originally intended.
>
> As I recall, the issue was that we had two documents - xcap-diff and 
> the partial notifications, which both needed a way to describe the 
> changes that had been made to the document. So, we had some 
> discussions and agreed that we would look for a common diff format to 
> be used across these mechanisms.
>
Partial PUBLISH is also on the scope.

> title need to be updated to reflect that.
Ok. However, it doesn't change the fact that xcap-package and 
partial-diff really try to do patching as well as conference state etc. 
So imo we'll have quite many places where patching is being used but 
each of them using it's own semantics. So my view is that there's a need 
for common xml patching, be it this or something else.

> xcap-diff and partial notify specifications. However, the way the 
> pathc operations are defined, there will be a lot of work to translate 
> the xcap operations that get applied to the patch format to send. For 
> a client and a server that are doing xcap, this means that the client 
> has to take its xcap operations, compute the final document, perform a 
> already have to know how to change a document from one form to the 
> other using xcap operations, and now it will also have to know how to 
> do that using a different set of semantics - the patch operations.
>
> Thus, there will be a lot of overhead for usage of this within xcap. 
> If we want to unify the partial notification and xcap-diff formats, we 
> need to work to come up with a format that is readily transformable 
> from xcap operations, yet also useful for presence endpoints not doing 
> xcap.
>
ok. there are at least two options when implementing e.g. subscription 
handling: first you could just transfer the incoming xcap-request 
(put,delete) onto the corresponding patch operation. While I havn't 
implemented this I really don't see this as a challenging task quite the 
contrary. The second option is to run diff and apply this patch. This is 
something that I have implemented and it was surprisingly easy because 
of the tree like structure of xml.

> require some work to modify the xpath expression in the xcap PUT to 
> tell a watcher about what changed in the document. I would suggest 
> that we do something closer to xcap, and have the xcap change document 
> define the default namespace for the QNames in the xpath expression.

Well my preference is still doing fully XPath 1.0 compatible model, so 
I'll agree that the title is misleading.

> I'm pretty confused by this. Lets say I want to add a new element to 
> the document, and the new element is defined within the namespace 
> urn:foo. The xcap change document defines a namespace prefix of p1, 
> and binds it to urn:foo. The patched document defines a different 
> prefix, p2, for the same namespace. So, when the element to be added 
> is in the xcap-change document, does it exist with prefix p1 or p2? I 
> read the text above as saying p1, which means that the prefixes have 
> to be rewritten to match the local prefix definitions for the name 
> namespace URI. Is that what you are proposing?

practically yes.

> guidelines should not appear in protocol specs.

Does it harm anything ? For people like me (implementators) it is a very 
practical one and just (except the last line) states the behaviour of 
XPath 1.0.

> I don't understand why this document doesnt define the add, replace 
> and remove elements (as opposed to types), without a target namespace, 
> and then those get included into the parent schema. I believe you have 

My preference is an unambiguous schema.

> Why does it have to be positional? It just needs to be unambiguous.

while I found bugs (e.g. whitespace text nodes) after I wrote this, I 
also noted that this requirement was bad.

> I'm confused what <add> is intended to do. Does it add the content as 
> a child of the selected element?
>
> again, this is only na issue if you are looking to change xcap, which 
> is not in scope for this document I thought.

agreed.

> format to describe changes in xcap documents made by xcap.
Actually this bnf is not needed at all as the selector content can be 
described within xml Schema. Anyway, it is sligthly enlarged version of 
xcap, e.g. text() is pretty much a must in order to handle whispace text 
nodes correctly.

> * addition of last() condition
Thanks,
Jari