To: ietf-dav-versioning@w3.org Message-ID: <OFAA5ECCCB.9D9C59A4-ON852568CC.00626039@ott.oti.com> From: "Tim Ellison/OTT/OTI" <Tim_Ellison@oti.com> Date: Wed, 26 Apr 2000 11:25:56 -0400 Subject: Review of protocol 04.3 Review of: draft-ietf-deltav-versioning-04.3 Clemm, Kaler April 7, 2000 Here's a few comments from reading the protocol doc. This is by far the most coherent and readable version! The comments are mostly nits. Regards, Tim --------------------------------------- 1.2 Terms Revision Label Remove second period at end of sentence. - - - - - - - - 2.1 Creating and Modifying... para 4. The description of checkin/out and locking is making a misleading lack og distinction between client and workspace. There are many models without a 1:1 client and workspace, which makes this description invalid. - - - - - - - - 2.3 Naming a Revisions: ... -> Naming a Revision:... - - - - - - - - 2.4 Accessing a Versioned... Discussion of default revision is excluding the posibility of working resources being selected. - - - - - - - - 3 Versioning Properties... Sentence two: "When a property cannot.." Duplication of description in section 1.3 -- (but you may be inclined to leave it in to reinforce the point.) - - - - - - - - 3.1.2 id Syntax Sentence two: "The id characters..." Should read something like. "An id is a sequence of characters. When an id is marshaled in the header of a HTTP request the characters are encoded using the UTF-8 encoding scheme." Since when ids are in the body they should use the encoding defined by the enclosing XML doc. - - - - - - - - 3.1.5 stable URL Sentence two: "A MOVE request..." This sentence is out of place here. This should be moved to the description of the MOVE method, and replaced by something like "A stable URL is represented by an href element as defined in section 12.3 of [RFC2518]. " The terms section (1.2) should include "Stable URL" along the lines of: "A stable URL is a server-defined URL that identifies either a versioned resource or a revision of a versioned resource. Clients cannot control the target selection for a stable URL using the Target-Selector header." - - - - - - - - 3.2 Versioned Resource... Sentence two: The DAV:versioned-resource element may -> MAY I believe this means 'optionally' rather than 'in some cases'. - - - - - - - - 3.2 Versioned Resource... Consider putting the last sentence in a new paragraph. - - - - - - - - 3.2.4 DAV:auto-version Sentence one. I know that I've lobbied for this unsuccessfully before, but... I would strike out the words 'without a Target-Selector header' since I don't see that it is relevant. It should just work. - - - - - - - - 3.3 Revision Properties Sentence two. The sentence is unnecessary, however, if you choose to keep it I recommend using 'protected' in place of 'immutable' to be consistent. - - - - - - - - 3.3.4 DAV:predecessor-set Sentence one. Rather than say 'contains a stable URL' should say 'contains zero or more stable URLs'. - - - - - - - - 3.3.8 DAV:working-resource-id-set Why do we need this? The id's themselves are not interesting unless you also have the workspace context, e.g. {0, 0, 1, 45, 45} - - - - - - - - 3.3.9 DAV:single-checkout Should state that the default value for a newly created revision is "F". - - - - - - - - 4.1 Target-Selector Para. 4: A Target-Selector header... Consider changing the second sentence to simply say 'In particular, it has no effect on the target of a stable URL.' - - - - - - - - 5.1.1 DAV:property report -> DAV:property-report - - - - - - - - 6 Versioning and... Is it true that all existing methods are invalid for stable URLs? - - - - - - - - 6.1 Automatic versioning... Consider deleting 'and no Target-Selector header has been specified' as described above. What does 'performed atomically' mean? If the update fails, is the server required to act as though the checkout never happened? i.e. if the PROPPATCH fails you would get the 207 back and there would be no extra revision in the history. I think this is desirable, but may be awkward to specify since what is the definition of update success (the multistat example shows that saying 'a 200 series response' is insufficient). - - - - - - - - 6.2 Propfind Why is this a good thing? - - - - - - - - 7.1 Version Sentence one. This sentence is quite a mouthful<g> ... and should say 'to convert it' What happens if you say VERSION to a versioned resource? Maybe then results from VERSION should not be cached ... Postcondition three: default target -> default revision. - - - - - - - - 7.1.1 Example - Version The status code in the example is 201 Created, in the description it was 200 OK. I think 200 ok is more appropriate, but whatever. - - - - - - - - 7.2 Checkout postconditions 'A new working resource is created that is a copy of the revision.' sounds more natural (to me). - - - - - - - - 7.3 Checkin response codes Consider precondition failed if the checkin policy is overwrite and the predecessor set has multiple entries. 405 seems inappropriate if the check-in policy is not supported? Unsupported media type (c.f. MKCOL),yuk? postconditions 'The working resource id ...' should additionally end with 'and the selected versioned resource.' It sure would be helpful if there was some way to indicate that the newly checked in revision should NOT become the default revision. Maybe part of the check-in policy? - - - - - - - - Should be a description of what OPTIONS would return if the resource supports basic versioning. - - - - - - - - 8.1 Advanced versioning terms Activity Should be described as an unversionable resource. I wonder why the ascii-art implies that activities are associated with the "arcs" between revisions, whereas the text describes activities as references to the revisions themselves. Am I reading too much into the diagram? - - - - - - - - Should be a description of the valid OPTIONS that an advanced versioning resource may answer. - - - - - - - - 9.1 Revision Selecion and Workspace Consider the following rearrangement: When a request URL identifies a versioned resource, it is necessary to provide additional information to specify which working resource or which revision of the versioned resource should be accessed. Advanced versioning introduces a "workspace" resource to capture this information. A workspace selects a set of revisions, and contains a set of working resources. It is initially populated with SET-DEFAULT-REVISION and MERGE requests. When a versioned resource request specifies a Workspace header and the workspace contains both a working resource and a revision of a versioned resource, the working resource is the target of that versioned resource. If the workspace does not contain either a working resource or a revision of a versioned resource, an error is returned. If a request is made and no workspace is specified, a server determined default workspace is used. This is the workspace used by all versioning-unaware clients. A server can associate different default workspaces with different URL's, and thereby expose multiple workspaces to versioning unaware clients. A CHECKOUT request that specifies a Workspace header creates a new working resource contained by that workspace. A subsequent CHECKIN request creates a new revision that is then selected by the workspace. - - - - - - - - 9.1 Revision selection... What does this mean? " a lock on a mutable revision controls an override CHECKIN from any workspace" what is an override checkin? - - - - - - - - 9.2 Parallel Development... para. 1 talks about 'multiple authors' followed by 'results in two parallel revisions' Should be consistent. - - - - - - - - 9.3 Baselines Sentence two: "When a workspace... created to captures" -> "created to capture" - - - - - - - - 9.4 Versioned Collections para. 1 There seems to be an unwritten subtext here<g>. Disallowing unversioned resources in a versioned collection is primarily for an (important) implementation optimization. Allowing any resource would break the semantics of the versioning system per se. I would be happier if this was more explicit. - - - - - - - - 10.3.1 DAV:working-resource-set Why does this contain stable Href's and not workspace id's since the workspace context is known (this is particularly ironic given that the versioned resource has ids where the workspace is unknown!) - - - - - - - - 10.3.2 DAV:revision-set I'm hoping that this is a typo<g> If it really contains all the stable URLs for ALL revisions that it selects then that could be potentially a very large number (thousands). Imagine merging a baseline and getting all the members of the baseline in this set. Consider re-writing this to be the stable URLs of the default revisions set in the workspace. We would then say (elsewhere) that when a clients sets a default revision that is a container (i.e., baseline, activity, etc.) the effect on target selection is as though each member of the container had been set individually in the workspace (though servers are not expected to implement it that way). - - - - - - - - 11.1 Workspace There is something about having "Workspace:" be the metadata area that doesn't sit right with me -- but I can't put my finger on it. I think I would expect that to indicate the server's default workspace. I propose that we (revert to?) have "Workspace: metadata" instead. - - - - - - - - 12 Advanced Versioning... Sentence one: for clarification, consider ending the sentence with (see Section 9.4) Last para: remove the condition "...and no Workspace header has been specified" I guess that I'd like to use this, even as a versioning aware client, as a short-cut when auto-version is set. - - - - - - - - 12.2 Checkin para. two I read this several times, and still don't understand what it means. How can a DAV:working-resource-id be a workspace, it's just an id? last para. Should state that if the workspace has any working resources then the CHECKIN MUST fail. - - - - - - - - 12.3 Set-Default-Revision first occurrance of SET-DEFAULT_REVISON -> ...REVISION - - - - - - - - 13.1.1 Example - Merge Response should include Vary: Workspace - - - - - - - - 13.3 Merge Since merge is such a powerful operation, and certainly one that would not be undertaken lightly by clients, I'd like to see it contain some options in the body that allow clients some control over the server behavior. The semantic description of the merge creating a working resource for all conflicts should be optional--the alternative is to fail the merge. If it always creates working resources then backing out of the effects of the merge will be potentially very expensive/troublesome. And of course there is no guarantee that requesting a report first is going to be sufficient. The description that states that if a working resource exists a request revision is added as a predecessor is also dubious, mostly because it is going to be very difficult to detect that this condition happened, but also it may just not make sense in terms of the resource history. - - - - - - - - 14.1 DAV:default-workspace-report What is the metadata dtd line about? - - - - - - - - 14.2 DAV:workspace-url-report This seems odd. As a client, I think it's great. As a server I'm unsure how I'd implement this. - - - - - - - - 14.2.1 Example - DAV:workspace-url-report and 14.3.1 Example - DAV:conflict-report Response should include Vary: Workspace - - - - - - - - 14.4 DAV:compare-report Again, I would assume that the compare reports differences in terms of the containers that were merged, rather than the individual revisions that are selected by the entire workspace. The alternative is a very expensive transitive walk of the entire namespace. - - - - - - - - As mentioned before on the list, the different reports should probably be different methods, so they can be discovered through options and selectively implemented and administered. - - - - - - - - 14.5 DAV:repository-report Last para. states that the reponse contains the URL of a collection in which the resource can be created. What if there are multiple collections in which it can be created...are they enumerated? probably not, since if you ask about <DAV:collection/> you may get lots of answers -- so how about the answer means, 'or any collection reachable from there' -- probably not that either since some servers may need to be more restrictive. - - - - - - - - 15 Non-versioning WebDAV... "are not specific versioning" -> "are not specific to versioning" What is meant by: they are not specific to versioning but are needed by versioning extensions? - - - - - - - - 15.3 Mkresource Strike: "other than standard data containers and collections" since later text illustrates how to create a standard data container, and collections should be allowed too. - - - - - - - - 15.4 Report Sentence two. Dynamic with respect to what? and why can't options handle it? - - - - - - - - - - - - - - - -