Review of protocol 04.3

From: Tim Ellison/OTT/OTI (Tim_Ellison@oti.com)
Date: Wed, Apr 26 2000

  • Next message: Chris Kaler: "interim DeltaV design meeting"

    To: ietf-dav-versioning@w3.org
    Message-ID: <OFAA5ECCCB.9D9C59A4-ON852568CC.00626039@ott.oti.com>
    From: "Tim Ellison/OTT/OTI" <Tim_Ellison@oti.com>
    Date: Wed, 26 Apr 2000 11:25:56 -0400
    Subject: Review of protocol 04.3
    
    Review of:
         draft-ietf-deltav-versioning-04.3
         Clemm, Kaler
         April 7, 2000
    
    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.
    
    
    Regards,
    Tim
    ---------------------------------------
    
    1.2 Terms
    Revision Label
    
    Remove second period at end of sentence.
    - - - - - - - -
    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.
    - - - - - - - -
    2.3 Naming a Revisions: ... -> Naming a Revision:...
    - - - - - - - -
    2.4 Accessing a Versioned...
    
    Discussion of default revision is excluding the posibility of working
    resources being selected.
    - - - - - - - -
    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.)
    - - - - - - - -
    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.
    - - - - - - - -
    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."
    - - - - - - - -
    3.2 Versioned Resource...
    Sentence two: The DAV:versioned-resource element may -> MAY
    
    I believe this means 'optionally' rather than 'in some cases'.
    - - - - - - - -
    3.2 Versioned Resource...
    Consider putting the last sentence in a new paragraph.
    - - - - - - - -
    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.
    - - - - - - - -
    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.
    - - - - - - - -
    3.3.4 DAV:predecessor-set
    Sentence one.
    
    Rather than say 'contains a stable URL' should say 'contains zero or more
    stable URLs'.
    - - - - - - - -
    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}
    - - - - - - - -
    3.3.9 DAV:single-checkout
    
    Should state that the default value for a newly created revision is "F".
    - - - - - - - -
    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.'
    - - - - - - - -
    5.1.1 DAV:property report -> DAV:property-report
    - - - - - - - -
    6 Versioning and...
    
    Is it true that all existing methods are invalid for stable URLs?
    - - - - - - - -
    6.1  Automatic versioning...
    
    Consider deleting 'and no Target-Selector header has been specified'  as
    described 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.  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).
    - - - - - - - -
    6.2 Propfind
    
    Why is this a good thing?
    - - - - - - - -
    7.1 Version
    Sentence one.
    
    This sentence is quite a mouthful<g> ... and should say 'to convert it'
    What happens if you say VERSION to a versioned resource?  Maybe then
    results from VERSION should not be cached ...
    Postcondition three: default target -> default revision.
    - - - - - - - -
    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.
    - - - - - - - -
    7.2 Checkout
    postconditions
    
    'A new working resource is created that is a copy of the revision.' sounds
    more natural (to me).
    - - - - - - - -
    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?
    
    postconditions
    
    'The working resource id ...' should additionally end with 'and the
    selected versioned resource.'
    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?
    - - - - - - - -
    Should be a description of what OPTIONS would return if the resource
    supports basic versioning.
    - - - - - - - -
    8.1 Advanced versioning terms
    Activity
    
    Should be described as an unversionable resource.
    
    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?
    - - - - - - - -
    Should be a description of the valid OPTIONS that an advanced versioning
    resource may answer.
    - - - - - - - -
    9.1 Revision Selecion and Workspace
    Consider the following rearrangement:
    
    When a request URL identifies a versioned resource, it is necessary to
    provide additional information to specify which working resource or which
    revision of the versioned resource should be accessed.  Advanced versioning
    introduces a "workspace" resource to capture this information.  A workspace
    selects a set of revisions, and contains a set of working resources.  It is
    initially populated with SET-DEFAULT-REVISION and MERGE requests.
    
    When a versioned resource request specifies a Workspace header and the
    workspace contains both a working resource and a revision of a versioned
    resource, the working resource is the target of that versioned resource.
    If the workspace does not contain either a working resource or a revision
    of a versioned resource, an error is returned.
    
    If a request is made and no workspace is specified, a server determined
    default workspace is used.  This is the workspace used by all
    versioning-unaware clients.  A server can associate different default
    workspaces with different URL's, and thereby expose multiple workspaces to
    versioning unaware clients.
    
    A CHECKOUT request that specifies a Workspace header creates a new working
    resource contained by that workspace.  A subsequent CHECKIN request creates
    a new revision that is then selected by the workspace.
    
    - - - - - - - -
    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?
    
    - - - - - - - -
    9.2 Parallel Development...
    para. 1
    
    talks about 'multiple authors' followed by 'results in two parallel
    revisions'
    Should be consistent.
    - - - - - - - -
    9.3 Baselines
    Sentence two: "When a workspace... created to captures" -> "created to
    capture"
    - - - - - - - -
    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.
    - - - - - - - -
    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!)
    - - - - - - - -
    10.3.2 DAV:revision-set
    
    I'm hoping that this is a typo<g>
    If it really contains all the stable URLs for ALL revisions that it selects
    then that could be potentially a very large number (thousands).  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).
    - - - - - - - -
    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.
    - - - - - - - -
    12 Advanced Versioning...
    Sentence one: for clarification, consider ending the sentence with  (see
    Section 9.4)
    
    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.
    - - - - - - - -
    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?
    
    last para.
    Should state that if the workspace has any working resources then the
    CHECKIN MUST fail.
    - - - - - - - -
    12.3 Set-Default-Revision
    first occurrance of SET-DEFAULT_REVISON -> ...REVISION
    - - - - - - - -
    13.1.1 Example - Merge
    Response should include
         Vary: Workspace
    - - - - - - - -
    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.  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.
    
    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.
    - - - - - - - -
    14.1 DAV:default-workspace-report
    
    What is the metadata dtd line about?
    - - - - - - - -
    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.
    - - - - - - - -
    14.2.1 Example - DAV:workspace-url-report
    and
    14.3.1 Example - DAV:conflict-report
    
    Response should include
         Vary: Workspace
    - - - - - - - -
    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.
    - - - - - - - -
    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.
    - - - - - - - -
    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.
    - - - - - - - -
    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?
    - - - - - - - -
    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.
    - - - - - - - -
    15.4 Report
    Sentence two.
    
    Dynamic with respect to what?  and why can't options handle it?
    - - - - - - - -
    
    - - - - - - - -