Date: Fri, 22 Sep 2000 18:13:28 -0400 (EDT) Message-Id: <200009222213.SAA20429@tantalum.atria.com> From: "Geoffrey M. Clemm" <geoffrey.clemm@rational.com> To: ietf-dav-versioning@w3.org Subject: Re: comments on deltav-08.2 From: "Boris Bokowski/OTT/OTI" <Boris_Bokowski@oti.com> I'm passing on the following comments against 08.2 from a colleague. So Boris not only reviews it himself, but gets a colleague to review it as well! That pretty much sets the bar for others to aim at (:-). There is only one major issue - we believe that the result of the OPTIONS request is too fine-grained for clients that try to interoperate with not just one particular server implementation. Boris raised this issue in the conference call, and received general agreement that we should try to replace this with a few reasonable subsets. Everyone is encouraged to send in a subset that they think makes sense (or just indicate that they plan on just implementing everything). Whatever gets to me by the end of the month will make it into the last call version of the protocol. p.7 The terms should include version name, and contrast it to version label. "A "version name" is a string chosen by the server to identify a particular version of a version history." How about if I just define it using the preceding sentence, and save the contrast with a label for the later semantics section? This avoids saying it twice. "A "version label" is a string chosen by the client to identify a particular version of a version history. Version labels are optional; clients may add, remove, and reassign version labels at any time." p.10 section 2.3 Labeling a Version "For certain methods, if the request-URL identifies a version selector, a label can be specified in a Target-selector request header to cause the method to be applied to the version selected by that label." - Should be brought into line with the new "version selector is a copy of a version" semantics. How about: "... to the version selected by that label from the version history of the target of that version selector." ? p.12 5.2.1 DAV:resourcetype - This property of versions should be protected. Done. p.13 5.3.1 DAV:resourcetype - This property of version selectors should be protected. Suppose a server allowed you to change the resource type of a resource. I believe the version selector should then allow you to do it as well. p.16 8.4 PUT "If the request-URL identifies a version selector, the PUT MUST fail unless DAV:auto-version is set for that version selector." - This condition is too strong; it would preclude modifying a checked-out version selector. Fixed. p.17 8.6 PROPPATCH "If the request-URL identifies a version selector, and attempt to modify a dead property MUST fail unless DAV:auto-version is set for that version selector." - This condition is too strong; it would preclude modifying a checked-out version selector. Fixed. p.17 8.7 DELETE - It would appear that the client may delete a working resource. Yes. What is the full import of this? Must the server really get rid of the working resource and snip it out of the check-out report? Yes. (Since 10.4 UNCHECKOUT does not apply to working resources, DELETE is the only way to get rid of a working resource.) Yes. p.18 8.9 MOVE - It would appear that the client may move working resources, which live at server-assigned URLs. Is this desireable? Good point. Unless anyone objects, I'll add a constraint that a MOVE on a working resource MUST fail. This is another positive result from separating checked-out version selectors (which you *can* MOVE) from working resources (which you *cannot*). p.19 10.1 VERSION-CONTROL "If the request-URL identified a version selector at the time of the request, the VERSION-CONTROL request MUST NOT change the state of that version selector." The semantics of this method does not cover the case where the request-URL identifies a working resource. Should this be an error? A working resource is not a versionable resource, so it would fail with a DAV:must-be-versionable error. p.20 10.1.2 Example "Example - VERSION-CONTROL (using an existing version history)" - Since version history URLs are not exposed in core-versioning, a less misleading title might be: "Example - VERSION-CONTROL (using an existing version)" Fixed. - <DAV:version> </DAV:version> tags are missing from example request Fixed. p.23 10.3 CHECKIN In the postconditions, there should be a clause for version name. (According to 5.2.1, there needs to be one): "The DAV:version-name of the new version is set to a server-assigned string distinct from the version names of all other versions in the same version history." Added. Also, unless there is some reason to allow servers to assign version labels on their own, there probably should be a clause for label name set: "The DAV:label-name-set of the new version is set to the empty list." Some servers may have some pre-defined labels (e.g. "LATEST"), so we probably shouldn't put on this constraint. p.25 10.5 SET-TARGET Either there should be a precondition that prohibits SET-TARGET on a checked-out version selector, or the semantics of it need to be spelled out (e.g., automatic UNCHECKOUT). I think "prohibited" is what we want. I'll fix that. p.30 11.5.1 Example - DAV:version-tree-report The example response is missing <D:version-name> </D:version-name> tags in 3 places. Added. Why does example response not include checkin-date? This property would always be present unless the server didn't know the time. This field is optional, so apparently the server in the example doesn't store them (:-). Why does the example response not include label-name-set and checkout-set properties? Is it because these sets are empty? Yup. p.41 16 Advanced Versioning and Existing Methods "For any request that includes a Workspace header, the request-URL and every request header URL must be treated as if it were prefixed with the workspace URL specified in the Workspace header." - Saying that every request header URL gets prefixed is overkill, and may result in prefixing request headers that have nothing to do with workspaces (e.g., a header containing the URL of a proxy server). The spec should mandate prefixing of the MOVE/COPY target request headers by name. Fair enough. I'll make that change. Anyone have any other headers (beyond Destination) that we should specify? p.42 16.1 OPTIONS 1. This section is not very helpful because it does not spell out which strings apply to which features. 2. The options are ridiculously fine-grained. For instance, it suggests that workspaces might be supported but workspace headers might not be. This is not helpful to clients who need to know which level of service they can count on. Everyone please get their favorite "option set" to me. p.44 16.8 VERSION-CONTROL "If the collection containing the request-URL is a collection version selector, the request MUST fail unless DAV:auto-version is set for that collection version selector." - This operation should be allowed if the containing collection version selector is checked out: "If the collection containing the request-URL is a collection version selector, the request MUST fail unless DAV:auto-version is set for that collection version selector or that collection version selector is checked out." Fixed. p.46 16.10 CHECKIN - Precondition should say that the URLs in the predecessor-set MUST be version URLs of versions in same version history. Definitely! Fixed. p.47 SET-TARGET "If the request-URL identifies a baseline selector for a collection, then the target of each version selector that is a member of that collection (...) is modified to be the version selected by that baseline." - Needs to also specify what happens if that baseline does not select any version. Good point. (Those version selectors should be deleted). I'll add this. "If a new binding identifies a version selector that was not previously a member of that workspace, then a new version selector is created whose DAV:target is the initial version of that version history." - This is the fallback position only when there is no version selection; when a baseline is involved, there may well be a version selection for this new member. Another good point. I'll rewrite this sentence to make that point. And thanks for the great review! Cheers, Geoff