From: jamsden@us.ibm.com To: ietf-dav-versioning@w3.org Message-ID: <852568AF.007D7FA5.00@d54mta03.raleigh.ibm.com> Date: Mon, 27 Mar 2000 17:01:15 -0500 Subject: versioning-04 review Here's my review of draft-ietf-deltav-versioning-04: - Is the history report covered by the properties report? If so, the spec should show it as an example. If not, how is the history returned? - There seems to be some confusion with respect to default revision selection. The spec says in various places that the default revision is a property of a versioned resource, a versioned collection for picking default revisions of members of that collect, and by using a server default workspace. Having a versioned resource have a default revision is good for basic versioning, but it isn't scalable as a client would have to set the default revision individually for every resource on the server. It also introduces two mechanisms for specifying default revisions, one for basic, and the default workspace for advanced. This needs some thought. See a suggested solution below in comments on section 2.4. - Also some confusion on the use of workspaces as a "checkout token" in basic versioning. Tim's scenarios indicate the client has to create a workspace to be used as a checkout token (or working resource identifier). Then this workspace is used to identify all checked out working resources. The spec says the server returns a workspace for each checkout that identifies the associated working resource, one per checkout. I think a compromise is appropriate. If a workspace is not specified on a checkout, then the server creates one and returns it along with the working resource. If the client does specify a workspace on the checkout, the server uses that one to identify the working resource. If that workspace already has a working resource for the same versioned resource, then an error is returned (since the same workspace cannot identify more than one working resource with the same versioned resource URL). Then clients can choose what they want to do. A client could create workspaces as identifiers for related work, and use them to check out revisions. Or it could let the server create the workspace, and then just hand it back on accesses in order to identify the working resource. Comments below are based on this working assumption. - 1.2 Workspace: A "workspace" is a string used to identify a particular working resource of a versioned resource. It MAY be generated by the server or created by a client. - 1.2 Target: Workspace and Version-Selector should be Revision-Selector. - 2.1 end of 2nd paragraph: add: See below for details describing how workspaces are used to identify working resources. (needs a forward reference). - 2.2 1st paragraph 2nd sentence: A working resource of a versioned resource is given a server assigned workspace when it is created if the client does not specify a working resource on checkout. Alternatively the client may choose to create a workspace and use it on subsequent checkouts. - 2.4 last sentence: need to determine how default revisions are selected. This section describes default revisions as a property of a versioned resource. Section 8.1 and 9.1 describe default revision selection using the default workspace. Section 8.1 associates the default workspace with a collection. 4th paragraph in section 9.1 describes the default workspace associated with the server. Let's discuss this one. Perhaps the best solution is to provide an optional default revision associated with a versioned resource, and an optional default workspace associated with the server. If neither are specified, no revision is selected. If both are specified, the default revision associated with the versioned resource takes precedence. - 3.1.2 then when appropriate escaped -> then appropriately escaped... - 3.3 Use DAV:versioned-resource instead of DAV:versioned-resource-resourcetype. Its too long, not consistent with DAV:collection, and redundant with <DAV:resourcetype>DAV:versioned-resource-resourcetype</DAV:resourcetype>. Similar comments for all other resource type identifiers. - 3.3.1 Use DAV:revisions instead of DAV:revision-set. This is better English and is consistent with our common use of the terms. It doesn't matter that its a set, only that there are many members. - 3.3.3 This is pretty ugly. First its a mouthful of special cases. Second, it allows a down-level client, unaware of versioning, to change the default revision on a resource other clients might be using without even knowing about it. I prefer using the default workspace for this, and allowing servers to have DAV:latest as the only entry in the revision selection rule (effectively). Gets the same result, but cleaner. This doesn't mean servers have to support extended workspaces. They can special case the default workspace implementation. - 3.3.4 Indicate checkout, operation, checkin must be atomic. - 3.4.2 DAV:revision brings up a common problem with stable URLs. The client is generally using versioned resource URLs and revision selectors like revision labels, workspaces, activities, etc. But many methods return stable URLs representing functors the server uses to access specific revisions. Given one of these stable URLs, how can the client determine the versioned resource and labels that should be reported to the user in order to provide more meaningful names? I suppose the client can follow the stable href to the versioned resource to get its URL and to the revision to get its revision id or labels, but this is a lot of requests to get information the client will almost always need. Perhaps we should consider returning a versioned resource/revision id pair to reduce the problem. - 3.4.3 DAV:predecessors instead of DAV:predecessor-set. - 3.4.4 Same for successors. - 3.4.5 Same for labels. - 3.4.5 2nd paragraph: What is this trying to say? That revision labels are URL segments and can be URL encoded for transmission? - 3.4.7 DAV:workspaces instead of DAV:workspace-set. - 3.5.2 Same for DAV:predecessors. - 4.2 needs specific status codes assigned. Just pick the next available numbers. - 5.1 Postconditions is worded kind of funny. "If the response status code is 201..." On successful execution, the resource is created with an empty body and its properties initialized as given in the request entity body. Last paragraph: If execution fails, no new resource is created and any resource that may have existed at the request-URL is unaffected. - 5.2.2 Remove status from the element definition. (reports, conflict, or compare) should be (property, conflict,... There's no reports report. There is an available reports report though. - 5.3.3 <D:available-report-request/> in the <D:report-request> - 5.4 Needs motivation. What would anyone do with a property report? Is it used to get the revision history of a revision or versioned resource? If so, how? - 6.1, 3rd postcondition: what is DAV:resourceid? (I don't have the binding spec with me). - 6.1, last postcondition, what does it mean to have a versionable resource replaced in its parent collection by the new versioned resource? Is this just a new binding? If so, this is what it should say. - 6.1 Need a status code for already versioned? - 6.2 The client should be able to specify a workspace too. - 6.2 2nd request marshalling: is a little confusing. The precondition indicates the request-URL has to be a stable href to a revision, or a versioned resource with either a workspace header, revision-selector header, neither (in which case the default revision is selected), or both. - 6.3 2nd request marshalling: The workspace header specifies the workspace that identifies the working resource. - 6.4 Last sentence in the intro: "If the server supports mutable revisions and there is a single predecessor revision, then CHECKIN can be used to overwrite the value of the predecessor revision." Here is an excellent example of why we need to distinguish between predecessors created with a checkout, and merge predecessors. Another is that the MERGE method should not be allowed to remove a predecessor created from a checkout, only predecessors created by a merge. Otherwise the line-of-descent and revision history wouldn't be meaningful. - 6.4 Response marshalling: 207 multi status might be returned too in order to indicate which checkin policies couldn't be satisfied. And I thought these policies were applied on checkout too so that a client wouldn't be able to checkout a revision with an expectation of how it could be checked in if that expectation could not be met. - 7.1 If the Workspace-URL is omitted from the Workspace header, the versioned resource itself is the target of the request. This is confusing and irregular. Consider using the Revision-Selector as described below. - 7.2 add | "Revision-Selector" ":" "Versioned-Resource" to indicate the versioned resource is selected. Target-Selector is better in this case. Note the extra space in the " Revision-Selector: for label. Should be removed. - 8.1 For the new workspace semantics, distinguish static and dynamic workspaces, and that dynamic workspaces are optional. Dynamic workspaces keep the revisions in the workspace consistent with the workspace revision selection rule at all times and under all circumstances. Static workspaces require the client to refresh the workspace to see revision selection changes. - 8.1 A default workspace should not be associated with a collection, it should be associated with the server. That's probably complicated enough. - 8.1 Conflict Report: perhaps we should eliminate DAV:merge operators in the revision selection rule, and have the conflict report be the entity response body from a MERGE request. It should be possible to merge two resource to set the successor/predecessor relationship, and to merge two workspaces to include changes made in one workspace into another workspace. Or merge an activity into a workspace to pickup the changes made in that activity. For example, merge a user's working area workspace into the team integration workspace. The conflict report would be generated by the merge request. Just keep merging until there are no more conflicts. Maybe the target of the merge should always be a workspace, and the source a revision, activity, configuration, or workspace. - Remove baseline, its redundant with a configuration containing a single collection. There's no reason to have a special place for these configurations (baseline property of a versioned collection). Users should be free to put them where it makes the most sense for them. - Remove repository. There are no methods that do anything with it but setting properties. The properties are never used. More comments in the appropriate sections below. - 9.1 consider having only DAV:rsr-or (or no operator since that would leave only one) operator in the revision selection rule. Generate the merge conflict report as the result of a merge operation. - 9.4 Remove the second paragraph referencing deep revisions and baselines. - Remove 10.2.1 and 10.2.2, DAV:baselines and DAV:repository. - 10.5.1 Perhaps there should be a way to list all the members of a workspace, not just the working resources. - 10.5.2 consider removing DAV:rsr-merge and get the conflicts from the merge method. - 10.5.2 Here's another case where the stable URLs in the revision selection rule won't be very meaningful to a client. How could the client efficiently get back more meaningful versioned resource URLs and labels? - 10.5.2, Consider removing the last paragraph: DAV:rsr-merge. - Remove 10.5.4 DAV:current-label and 10.5.5 DAV:current-activity. They should no be necessary to keep the checked in revision visible in the workspace. Specify the activity in an Activity header on CHECKIN. - 10.7 DAV:configuration not DAV:configuration-resourcetype. - Remove 10.7.1 DAV:baseline-root. - Remove 10.8.1 DAV:versioned-resource-collection. This looks like an implementation detail that clients never need to know. Its never referenced in any method. - Remove 10.8.2 DAV:repository-root, another implementation detail clients never need to know. Its also never referenced in any method. - Remove 10.8.3 DAV:activity-collection. This is contrary to namespace management. A client should be free to create any resource in any namespace he creates in order to organize them in the way that makes the most sense for their project and/or development process. Servers may have restrictions, and may refuse to create collections or bind members of certain types in other collections. But this is nothing new. This property is also never used in any method. - Same for 10.8.4 DAV:configuration-collection. - 11 Why can't a versioned collection contain a member denoting a binding to an unversioned resource? Its the collection that doesn't change, not the resource the collection member refers to. Does this result from collection baselines? If baselines are removed, does the restriction go away? Need motivation in any case. - 11.1 this should refer to dynamic revision selection by the workspace if supported by the server. It has no effect for static revision selection. - 11.2 1st two paragraphs should apply to basic versioning too in order to support "checkout token" reuse. Last paragraph should be removed. Activities should be specified as part of checkin. So this section isn't needed, advanced versioning doesn't add anything. - 11.3 Can remove this section. Advanced versioning doesn't add anything here either. - 12.1 Conflict report should be the output of a MERGE operation, not a derived property of a workspace. That way conflicts can be determined for either static or dynamic workspaces and the DAV:rsr-merge operator in the revision selection rule isn't needed. - 12.2.3 The href's in the D:added elements could be combined into one D:added element (<!ELEMENT (href+, ANY*)>) to simplify the report results. - 13.1 Shouldn't be able to delete a predecessor that was created with checkin. Shouldn't be able to delete all predecessors and leave the revision dangling. - Need a way to merge workspaces, activities, and configurations and generate a conflict report. The use case is for a user to have a working area workspace, and for there to be a team integration workspace. The user wants to catch up with changes made in the team integration workspace by merging the team workspace into their working are workspace. Conversely, the user wants to release his changes into the team workspace by merging them into the team workspace. Similar for activity and configuration. So there are two merge operations. On that detects attempts to refresh a workspace and detect conflicts, and another that resolves conflicts for individual revisions. - 13.2 Here's another case where stable URLs will need to be translated by clients into at least versioned resource URLs and revision ids. Should also have a way to create a configuration from workspace in a single method. - Need to specify the members of a workspace, not just the revision selection rule. That is, the revision selection rule may select revisions of resources the client is not interested in. The workspace needs a scope defined by its members where the revision selection rule specifies what revision of the members will be (or was on the last refresh) selected. - 13.3 Remove this section, just use a configuration with one member and a depth header on add. If not removed, first precondition is confusing. The DAV:baselines property, being plural, implies there may be many baselines for a versioned collection representing different configurations of that versioned collection. But the precondition indicates the DAV:baselines property must not exist and that the BASELINE method creates it with a single baseline. How are multiple baselines supported? Does the client have to remove the DAV:baselines property before creating a new baseline? Should indicate a baseline is a configuration with one collection member and depth infinity. This is implied in the postconditions, but is not stated in section 9.4. - 14.1 as described is correct for dynamic workspaces. Static workspaces select the member that was added to the workspace on the last refresh. Note also that a workspace should have a scope specifying what versioned resources are included in the workspace as well as the revision selection rule. - 25 Open Issues: authority and host header in examples seems to be done. activity as CHECKIN parameter should be the only way activities are associated with a revision. don't allow CHECKOUT to apply to a versionable resource. Require VERSION first. don't use MKRESOURCE to create a baseline. If baselines are retained, use the BASELINE method as described. remove references to repository. The WebDAV working group decided MKRESOURCE was preferable over PROPPATCH on a null resource at the December IETF meeting. A configuration can't contain working resources, so a configuration containing a collection revision depth infinity could not attempt to include any working resources. There is no need for a options call to enumerate the repositories and workspaces. Repositories are unnecessary, and clients are expected to know the meaning of namespaces they create.