From: Tim_Ellison@oti.com (Tim Ellison OTT) To: ietf-dav-versioning@w3.org (ietf-dav-versioning) Message-ID: <1999Nov15.105439.1250.1384867@otismtp.ott.oti.com> Date: Mon, 15 Nov 1999 10:56:17 -0500 Subject: Comments on draft-ietf-deltav-versioning Here are a number of comments on the protocol document. I have to say that the majority are nits, and this version of the document provides a coherent and understandable story. If you want to enter into debate about any of these points, I'd ask that you extract the relevant quotes to keep the postings short. Regards, Tim -------------------- draft-ietf-deltav-versioning-01.1 October 22, 1999 Clemm & Kaler Section 1.2 Terms Revision Label "The same revision label may be assigned to many different versioned resources." Being pedantic, the label is assigned to a revision of the versioned resource. Section 1.2 Terms Default Workspace "workspace may be a modifiable" should read "workspace may be modifiable" Section 1.2 Terms Baseline 1) "the revision of every members of" should read "the revision of every member of" 2) So this implies that all reachable resources must be revisions, not non-versioned resources, not working resources. 3) Should state that only the top-level namespace is captured in a baseline. Section 2.1 Creating versioned resources "When a resource is created in a versioned collection ... it is created as a versioned resource." Why? Section 2.2 Modifying a Versioned Resource 1) I think the comparison drawn in paragraph 2 is unhelpful, since lock and checkout semantics are so different. It doesn't help to explain anything about checkout. 2) para 2. last sentence. "an update to a checked out resource is only visible to" should say that it may be visible to other clients, i.e. it is not necessarily visible since it depends upon their RSR's. Section 3.4 Versioned Resource Properties 1) In general it would be useful to declare the default values, where they exist, throughout this section (i.e. single-checkout, auto-version, etc.) 2) "the following additional properties" additional to what? guess=resource Section 3.4.5 DAV:single-checkout "when the ... property ... is set" should read "when the ... property ... is true" 3.4.7 DAV:baselines Would be helpful to define the members of the collection. 3.5 Revision Properties Additional properties to what? guess = versioned resource 3.5.2 DAV:predecessor 3.5.3 DAV:successors Would be helpful to state that the href value is workspace independent. 3.5.8 DAV:activity "href)" should read "(href)" 3.6 Working Resource Properties Additional properties to what? guess = revision 3.6.2 DAV:checkin-policy 1) "if the resource is identical to" How is identical going to be defined? That there has been no PUT/PROPPATCH's successfully applied to the resource, or that props and content are bytewise indistinguishable ... or server defined? 2) The DTD allows for ANY element as a checkin-policy. The spec should state what a server should do if it does not support the checkin policy, and which are mandatory. 3.7 Workspace Properties Additional properties to what? guess = resource 3..2 DAV:revision-selection-rule 1) para. 1 first sentence "versioned resource is selected" should read "versioned resource that is selected" 2) para 2 "If it selects a particular revision, it may also detect a conflict" IMHO an RSR that is in conflict should not select any revision. 3) last para. but 3 re: DAV:rsr-latest. Here latest means chronologically latest, in section 1.2 Terms Activity latest means last in the succ/pred lineage. It should be consistent (and IMHO it should be succ/pred lineage). 3.8 Activity Properties 3.9 Baseline Properties Additional properties to what? guess = resource 3.9.2 DAV:predecessor "baseline is the baseline that selected by" ?? does not make sense. Why are all these baseline properties maintained anyway? 3.10 Repository Properties Additional properties to what? guess = resource 3.10.4 DAV:configurations "creation of new activities" should read "creation of new configurations" 3.10.6 DAV:default-workspace Should be the default for all versioned resources that do not specify their own default workspace (ref. 3.4.2) i.e. the default default. 4.1 GET Should define the effect of GET on new types introduced by versioning, if only to say that it is unspecified. 4.2 PUT para. 2 "PUT MUST fail unless the versioned resource has a DAV:auto-version property and no Target-Selector has been specified." Why is it conditional on no Target-Selector being specified? this seems strange. 4.2 PUT para 3. "... whose initial revision ..." Why would the initial PUT cause a revision to be made, and not a working resource? In principle, there is nothing special about the first PUT that means it should be preserved in version history. 4.2 PUT para 4. "... and no Workspace or Revision-Selection header ..." 1) We should agree on the terms for the header, be it 'Target-Selector' or 'Revision-Selection' (of course this will likely be split into separate headers soon.) 2) Why must the PUT fail if the workspace and revision-selection have not been specified? 4.3 PROPFIND I found this section confusing. I didn't understand what it was trying to say. 4.5 COPY para 2. Why can't we allow COPY to succeed for all but versioned resources? 4.6 DELETE para. 2 Should specify what happens if the DELETE does not specify 'all bindings' --> fail? 4.9 LOCK "A write lock on a versioned resource checks out the target ... into the default workspace" No -- this cannot be true. 4.10 UNLOCK "An UNLOCK ... checks in ..." No, please, no! Lock/unlock, checkin/checkout should be orthogonal. 5.1 MKRESOURCE 1) para. 1 "... standard data containers ..." Can we give these a friendlier name, I suggest 'ordinary resource'. 2) precondition #2, "a DELETE request ... MUST succeed" there is no explicit DELETE request in this case. Same comments for the semantics section. 3) postconditions. The properties on the resource will be those specified AND default values. 4) response marshaling. Change 412 (precondition failed) to 405 (method not allowed) so that it is the same as MKCOL. 5.1.1 New DAV:resourcetype Values 1) should state that the values MUST be (i.e. caps) 2) why do the values all end in '-resourcetype' this seems redundant? 3) what is the new type "DAV:versioned-resource-resourcetype" refer to? Does a resource change type when it is put under version control? Can we no longer distinguish collections and ordinary resources? 5.1.2 Example - MKRESOURCE 1) Given that the resource types are elements line 10 should read <D:resourcetype><D:workspace-resourcetype/></D:resourcetype> 2) The Request-URI was /project1, the response href is bar.html ;-| 3) The response description is strange. The response is a conflict on the property setting, and the description says that the existing resource could not be deleted. This description would cause the entire response to return 412 (or 405 proposed). 5.2.1 DAV:report-request 1) The DTD is wrong, it should be as follows: <!ELEMENT report-request (available-reports-request | ... )> <!ELEMENT available-reports-request EMPTY> etc. 2) The document is inconsistent about the name of the element 'available-reports-request' often calling it ''...-report-...' (singular). All references need to be fixed up. 5.3 Available REPORT Heading level too high within the document, since 5.1 is MKRESOURCE and 5.2 is REPORT, this section should be 5.2.x 5.3.2 DAV:available-report-response 1) The available reports are not a comma separated list 2) The DTD should be re-written as shown above for 5.2.1 3) The available reports DTD should have a '*' on the end since a server may support multiple report types. 5.4.2 DAV:conflict-report-response The DTD calls it conflict(s) plural. All references in the document need to be fixed up. 5.4.3 to 5.4.5 are logical subparts of 5.4.2 so should be subordinate heading levels. 5.4.4 DAV:common-ancestor Should spec what is returned if there is no common ancestor. 5.5.1 DAV:compare-request First line ends with two periods. 5.5.3 DAV:added 5.5.4 DAV:deleted 5.5.5 DAV:changed What is the ANY directive for? If it is for expansion maybe should be multiple ANYs. 5.5.6 Example - Compare REPORT The response indicates that the target 'myOtherCollection' was both added and deleted. What does that mean (for a target)? 6.1 CHECKOUT 1) para. 1 line 3. should not imply that the server may select a random workspace, but explicitly state that the server will choose the default workspace of the versioned resource, or if none, the default workspace of the repository. 2) Precondition #4. Why do we have this precondition? 3) Precondition #5 How can this be true? see 3.8.3 for a contradiction. 4) Request marshaling should mention body marshaling? 5) Semantics -- now the server is allocating a workspace for the new working resource ... ? 6) Postcondition #4 There is no such property defined (DAV:checked-out). 7) Response marshaling, should not be DAV:href (just href) 8) Consider changing the 201 created response to 200 ok? 9) Does the entire CHECKOUT fail if a property update (in the request body) fails? 6.1.1 Example - CHECKOUT 1) Groovy example, with the resource on a different server to the workspace. I thought this was out of scope for this spec. Otherwise we need new and interesting failure conditions. 2) Response shows <d:workspace> property ... there is no such property on a working resource. Should it be workspaces (plural) in which case that is a collection. 6.2 CHECKIN 1) Precondition #1. Why? why not allow "default", c.f. checkout or explicit working resource URL 2) Precondition #3. should read "if DAV:identical-abort is the DAV:checkin-policy" 3) Semantics para. 3 should read "... if the selected working resource is not a copy ..." 4) Postconditions #3. "... a selected activity ..." should read "... a current activity ..." 5) Postconditions should mention current label. 6) Consider changing status code 201 created to 200 ok? 6.2.1 Example - CHECKIN Maybe the response should return the revision URL or at least the revision ID? 7.1 Target-Selector para. 3 should acknowledge that there may be no revision selected.