Comments on draft-ietf-deltav-versioning

Tim Ellison OTT (Tim_Ellison@oti.com)
Mon, 15 Nov 1999 10:56:17 -0500


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.