Next message: Tim Ellison/OTT/OTI: "DAV:auto-version"
Date: Thu, 27 Apr 2000 17:30:25 -0400 (EDT)
Message-Id: <200004272130.RAA05181@tantalum.atria.com>
From: "Geoffrey M. Clemm" <geoffrey.clemm@rational.com>
To: ietf-dav-versioning@w3.org
Subject: Re: Review of protocol 04.3
From: "Tim Ellison/OTT/OTI" <Tim_Ellison@oti.com>
Review of:
draft-ietf-deltav-versioning-04.3
Clemm, Kaler
April 7, 2000
Thanks for the detailed review, Tim!
Note: Tim was actually reviewing an "04.25" draft that I had mailed
him for his scenarios work, which is different from the 04.3 draft
that I uploaded a couple of days ago. I'll upload an 04.4 draft with
Tim's suggested changes, to minimize confusion.
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.
Great!
---------------------------------------
1.2 Terms
Revision Label
Remove second period at end of sentence.
Done.
- - - - - - - -
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.
Got rid of it.
- - - - - - - -
2.3 Naming a Revisions: ... -> Naming a Revision:...
Done.
- - - - - - - -
2.4 Accessing a Versioned...
Discussion of default revision is excluding the posibility of working
resources being selected.
Yes, the default target is always a revision in core versioning.
- - - - - - - -
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.)
Yes, I had questions about what "protected" meant when it was only defined
in one place.
- - - - - - - -
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.
Done.
- - - - - - - -
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."
Added it to the terms section, but used somewhat different language.
Let me know if the new language is OK.
- - - - - - - -
3.2 Versioned Resource...
Sentence two: The DAV:versioned-resource element may -> MAY
I believe this means 'optionally' rather than 'in some cases'.
Done.
- - - - - - - -
3.2 Versioned Resource...
Consider putting the last sentence in a new paragraph.
Done.
- - - - - - - -
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.
When a client uses a Target-Selector header, it means that they are
aware of versioning, which means they should know that a versioned
resource should be checked out before it is modified. So I think that
it would be better to make this an error.
- - - - - - - -
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.
An immutable property can only be changed by checking out the versioned
resource, while a protected property is one that cannot be changed with
a PROPPATCH. I rewrote this paragraph. Let me know if it still
is unclear.
- - - - - - - -
3.3.4 DAV:predecessor-set
Sentence one.
Rather than say 'contains a stable URL' should say 'contains zero or more
stable URLs'.
That doesn't quite work with the rest of the sentence, i.e. "for the
revision that was checked out ...". Let me know how you'd like the
whole sentence to read.
- - - - - - - -
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}
The working resource id is all you need (in addition to the versioned
resource URL) to locate a paraticular working resource. An advanced
versioning server will use workspace URL's as working resource id's,
but in either case, it is the working resource id that identifies the
working resource.
- - - - - - - -
3.3.9 DAV:single-checkout
Should state that the default value for a newly created revision is "F".
Done.
- - - - - - - -
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.'
I rewrote this paragraph ... let me know if the rewrite is OK.
- - - - - - - -
5.1.1 DAV:property report -> DAV:property-report
Done.
- - - - - - - -
6 Versioning and...
Is it true that all existing methods are invalid for stable URLs?
OPTIONS, PROPFIND, and PROPPATCH work, but most of the rest will fail
due to the immutability of revisions.
- - - - - - - -
6.1 Automatic versioning...
Consider deleting 'and no Target-Selector header has been specified' as
described above.
Rationale for not doing so given 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.
Yes.
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).
I'm not sure I follow you here. It would be the same kind of failure you
would get if ACL's didn't let you do the update. Perhaps I'm missing the
point you are making?
- - - - - - - -
6.2 Propfind
Why is this a good thing?
A versioning unaware server will expect that a PUT to a resource would
not change its DAV:urn. I believe it is important that we satisfy that
expectation.
- - - - - - - -
7.1 Version
Sentence one.
This sentence is quite a mouthful<g> ... and should say 'to convert it'
Done.
What happens if you say VERSION to a versioned resource?
It's not a versionable resource, so it fails.
Maybe then
results from VERSION should not be cached ...
Added this requirement.
Postcondition three: default target -> default revision.
I agree we should use the phrase consistently, but I'd make the
arrow go the other way, since default workspaces allow the default
target to be a working resource.
- - - - - - - -
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.
I agree. Done.
- - - - - - - -
7.2 Checkout
postconditions
'A new working resource is created that is a copy of the revision.' sounds
more natural (to me).
Done.
- - - - - - - -
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?
Response codes ... always a fun topic (:-). I'll defer a final "response
code consistency pass" until we're done with the semantics.
postconditions
'The working resource id ...' should additionally end with 'and the
selected versioned resource.'
Done.
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?
What use case would motivate this?
- - - - - - - -
Should be a description of what OPTIONS would return if the resource
supports basic versioning.
Done.
- - - - - - - -
8.1 Advanced versioning terms
Activity
Should be described as an unversionable resource.
Done.
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?
Well, they semantically do correspond to the arcs, so the diagram is
correct, but our semantics aren't really affected much by this.
- - - - - - - -
Should be a description of the valid OPTIONS that an advanced versioning
resource may answer.
Done.
- - - - - - - -
9.1 Revision Selecion and Workspace
Consider the following rearrangement:
...
I rewrote this whole section. Let me know if the result is acceptable.
- - - - - - - -
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?
This was just confused ... I deleted it.
- - - - - - - -
9.2 Parallel Development...
para. 1
talks about 'multiple authors' followed by 'results in two parallel
revisions'
Should be consistent.
Fixed.
- - - - - - - -
9.3 Baselines
Sentence two: "When a workspace... created to captures" -> "created to
capture"
Done.
- - - - - - - -
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.
I added the statement that "this is restriction is required to allow
essential implementation optimizations". Probably don't want to spend
too much space in the protocol on this issue though. Folks that are
implementing them will appreciate the restriction, and folks that aren't
probably don't care (:-).
- - - - - - - -
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!)
The "stable" qualifier was an error. I rewrote this paragraph ...
let me know if it is now clearer.
- - - - - - - -
10.3.2 DAV:revision-set
I'm hoping that this is a typo<g>
Nope ... most implementations will compute this property rather
than store it as flat text (:-).
If it really contains all the stable URLs for ALL revisions that it selects
then that could be potentially a very large number (thousands).
Yup. This means that only small workspaces should ever be asked for their
DAV:revisions.
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).
This doesn't work well in the context of merging, since parts of each of
the various entities merged in will be selected.
But for large workspaces, I agree that the baselines and activities that
have gone into the workspace are what the client is more likely to ask
for, but that information is available in the DAV:predecessor-set and
DAV:activity-set properties of a workspace.
- - - - - - - -
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.
I'm inclined to use Jim Amsden's suggestion, and use:
Target-Selector: versioned-resorce
instead of "metadata".
- - - - - - - -
12 Advanced Versioning...
Sentence one: for clarification, consider ending the sentence with (see
Section 9.4)
Done.
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.
OK, here I agree with you, because a core versioning aware client
(who therefore is using Target-Selector headers) would not be aware
of collection versioning, and therefore would want auto-versioning
behavior for collections. So I agree that this should be changed
for versioned collections.
- - - - - - - -
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?
Fixed this. It should have said "Workspace URL".
last para.
Should state that if the workspace has any working resources then the
CHECKIN MUST fail.
Done.
- - - - - - - -
12.3 Set-Default-Revision
first occurrance of SET-DEFAULT_REVISON -> ...REVISION
Done.
- - - - - - - -
13.1.1 Example - Merge
Response should include
Vary: Workspace
Fixed this by stating that the results of MERE cannot be cached,
which means that a Vary header is no longer needed by MERGE.
- - - - - - - -
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.
That's what the DAV:conflict-report is for. If you want to "scope out"
the number of merges that will be required, the DAV:conflict-report will
get that for you. You'd only run the MERGE after you were sure that the
MERGE would succeed.
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.
For immutable merge arguments (such as revisions and baselines), the
DAV:conflicts-report will tell you exactly what you need to know.
The activity might change a bit between the report and the MERGE,
but I don't think that is a problem. In either case, you can just scan
the working resources of the workspace to see all the merges.
When you have activities, it's even easier, since you can create a
"merge" activity, and all new working resources created by the MERGE
will be associated with that activity.
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.
I agree. I changed it so this is an error.
- - - - - - - -
14.1 DAV:default-workspace-report
What is the metadata dtd line about?
Got rid of it.
- - - - - - - -
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.
Note: this is now called the "versioned-resource-URL-report", and
gets you the versioned resource URL for a stable revision URL.
I believe all servers will be able to do this.
- - - - - - - -
14.2.1 Example - DAV:workspace-url-report
and
14.3.1 Example - DAV:conflict-report
Response should include
Vary: Workspace
Done.
- - - - - - - -
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.
A server can decide how to structure the response, i.e. in terms of
revisions, baselines, or even activities. At some point we could define
specific flavors of the DAV:compare-report, but we can probably hold off
on that for now.
- - - - - - - -
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.
Chris was the strongest proponent of an extensible REPORT method (because
he wants to define a variety of specialized reports that his server
will support), so I'll let him answer this one.
- - - - - - - -
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.
I think it is sufficient for the server to return one place where
these kinds of resources can be put. If there are others, that's fine,
but I don't think we need to enumerate all of them.
- - - - - - - -
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?
Got rid of this section, and moved the various methods into the core
versioning section.
- - - - - - - -
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.
Replaced this with MKACTIVITY and MKWORKSPACE, so the issue no longer
arises.
- - - - - - - -
15.4 Report
Sentence two.
Dynamic with respect to what? and why can't options handle it?
I agree that "dynamic" was not the point, so I rewrote this section.
OPTIONS can't handle it because OPTIONS is required to be about
communication issues, not about random information such as doing
a "compare".
And again, many thanks for the detailed review, Tim!
Cheers,
Geoff