]> granicus.if.org Git - apache/commitdiff
forward-port John Vasta's checkin to mod_dav 1.1.x (on Sep 25, 2000). this
authorGreg Stein <gstein@apache.org>
Sat, 7 Oct 2000 00:50:42 +0000 (00:50 +0000)
committerGreg Stein <gstein@apache.org>
Sat, 7 Oct 2000 00:50:42 +0000 (00:50 +0000)
begins some work to upgrade the versioning support to some of the more
recent drafts.

- get_resource hook has new params
- create_collection hook no longer takes a pool
- new dav_auto_version_info structure to group up autoversion
  rollback/commit handling data
- new functions for getting workspace, target-selector, etc
- supportedlock hook now takes the resource in question (since different
  resources may have different locks)
- new resource types; tweaks in props.c to support them
- some tweaks with resource creation, Location header, etc.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@86424 13f79535-47bb-0310-9956-ffa450edef68

modules/dav/fs/lock.c
modules/dav/main/mod_dav.c
modules/dav/main/mod_dav.h
modules/dav/main/props.c
modules/dav/main/util.c

index b79782ab7e7027be8b49755e787974bf6fbaac6c..be7bdc0137300a9d54e59835502913d61f5a16c0 100644 (file)
@@ -777,7 +777,7 @@ static dav_error * dav_fs_resolve(dav_lockdb *lockdb,
 **    properties. I think we save more returning a static string than
 **    constructing it every time, though it might look cleaner.
 */
-static const char *dav_fs_get_supportedlock(void)
+static const char *dav_fs_get_supportedlock(const dav_resource *resource)
 {
     static const char supported[] = DEBUG_CR
        "<D:lockentry>" DEBUG_CR
index 7bdae15d2593b6f4545a4e6209713240269e2672..1dca6b246977b3815e822715928fdd98712d2f4a 100644 (file)
@@ -541,12 +541,8 @@ static int dav_created(request_rec *r, const char *locn, const char *what,
     /* Per HTTP/1.1, S10.2.2: add a Location header to contain the
      * URI that was created. */
 
-    /* ### locn does not contain an absoluteURI. S14.30 states that
-     * ### the Location header requires an absoluteURI. where to get it? */
-    /* ### disable until we get the right value */
-#if 0
-    apr_table_setn(r->headers_out, "Location", locn);
-#endif
+    /* Convert locn to an absolute URI, and return in Location header */
+    apr_table_setn(r->headers_out, "Location", ap_construct_url(r->pool, locn, r));
 
     /* ### insert an ETag header? see HTTP/1.1 S10.2.2 */
 
@@ -604,13 +600,21 @@ static int dav_get_overwrite(request_rec *r)
     return -1;
 }
 
-/* resolve a request URI to a resource descriptor */
-static int dav_get_resource(request_rec *r, const char *target,
-                            dav_resource **res_p)
+/* resolve a request URI to a resource descriptor.
+ * If target_allowed != 0, then allow the request target to be overridden
+ * by either a DAV:version or DAV:label-name element (passed as
+ * the target argument), or any Target-Selector header in the request.
+ */
+static int dav_get_resource(request_rec *r, int target_allowed,
+                            ap_xml_elem *target, dav_resource **res_p)
 {
     void *data;
     dav_dir_conf *conf;
     const dav_hooks_repository *repos_hooks;
+    const char *workspace = NULL;
+    const char *target_selector = NULL;
+    int is_label = 0;
+    int result;
 
     /* go look for the resource if it isn't already present */
     (void) apr_get_userdata(&data, DAV_KEY_RESOURCE, r->pool);
@@ -624,7 +628,21 @@ static int dav_get_resource(request_rec *r, const char *target,
     /* assert: provider != NULL */
     repos_hooks = dav_lookup_repository(conf->provider);
 
-    *res_p = (*repos_hooks->get_resource)(r, conf->dir, target);
+    /* get any workspace header */
+    if ((result = dav_get_workspace(r, &workspace)) != OK)
+        return result;
+
+    /* if the request target can be overridden, get any target selector */
+    if (target_allowed) {
+        if ((result = dav_get_target_selector(r, target,
+                                              &target_selector,
+                                              &is_label)) != OK)
+           return result;
+    }
+
+    /* resolve the resource */
+    *res_p = (*repos_hooks->get_resource)(r, conf->dir, workspace,
+                                          target_selector, is_label);
     if (*res_p == NULL) {
         /* Apache will supply a default error for this. */
         return HTTP_NOT_FOUND;
@@ -632,6 +650,12 @@ static int dav_get_resource(request_rec *r, const char *target,
 
     (void) apr_set_userdata(*res_p, DAV_KEY_RESOURCE, apr_null_cleanup,
                             r->pool);
+
+    /* ### hmm. this doesn't feel like the right place or thing to do */
+    /* if there were any input headers requiring a Vary header in the response,
+     * add it now */
+    dav_add_vary_header(r, r, *res_p);
+
     return OK;
 }
 
@@ -709,10 +733,10 @@ static int available_report(request_rec *r, const dav_resource *resource)
              "<D:available-report/>" DEBUG_CR,
              r);
 
-    for (; reports->namespace != NULL; ++reports) {
+    for (; reports->nmspace != NULL; ++reports) {
         /* Note: we presume reports->namespace is propertly XML/URL quoted */
         ap_rprintf(r, "<%s xmlns=\"%s\"/>" DEBUG_CR,
-                   reports->name, reports->namespace);
+                   reports->name, reports->nmspace);
     }
 
     ap_rputs("</D:report-set>" DEBUG_CR, r);
@@ -731,7 +755,7 @@ static int dav_method_get(request_rec *r)
      * visible to Apache. We will fetch the resource from the repository,
      * then create a subrequest for Apache to handle.
      */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -741,7 +765,9 @@ static int dav_method_get(request_rec *r)
 
     /* Check resource type */
     if (resource->type != DAV_RESOURCE_TYPE_REGULAR &&
-        resource->type != DAV_RESOURCE_TYPE_REVISION) {
+        resource->type != DAV_RESOURCE_TYPE_VERSION &&
+        resource->type != DAV_RESOURCE_TYPE_WORKING)
+    {
         return dav_error_response(r, HTTP_CONFLICT,
                                   "Cannot GET this type of resource.");
     }
@@ -924,7 +950,7 @@ static int dav_method_post(request_rec *r)
     int result;
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK) {
         return result;
     }
@@ -944,15 +970,12 @@ static int dav_method_put(request_rec *r)
 {
     dav_resource *resource;
     int resource_state;
-    dav_resource *resource_parent;
+    dav_auto_version_info av_info;
     const dav_hooks_locks *locks_hooks = DAV_GET_HOOKS_LOCKS(r);
     const char *body;
     dav_error *err;
     dav_error *err2;
     int result;
-    int resource_existed = 0;
-    int resource_was_writable = 0;
-    int parent_was_writable = 0;
     dav_stream_mode mode;
     dav_stream *stream;
     dav_response *multi_response;
@@ -965,7 +988,7 @@ static int dav_method_put(request_rec *r)
     }
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK) {
         return result;
     }
@@ -1007,10 +1030,7 @@ static int dav_method_put(request_rec *r)
     /* make sure the resource can be modified (if versioning repository) */
     if ((err = dav_ensure_resource_writable(r, resource,
                                            0 /* not parent_only */,
-                                           &resource_parent,
-                                           &resource_existed,
-                                           &resource_was_writable,
-                                           &parent_was_writable)) != NULL) {
+                                           &av_info)) != NULL) {
        /* ### add a higher-level description? */
        return dav_handle_err(r, err, NULL);
     }
@@ -1093,11 +1113,8 @@ static int dav_method_put(request_rec *r)
     }
 
     /* restore modifiability of resources back to what they were */
-    err2 = dav_revert_resource_writability(r, resource, resource_parent,
-                                          err != NULL /* undo if error */,
-                                          resource_existed,
-                                          resource_was_writable,
-                                          parent_was_writable);
+    err2 = dav_revert_resource_writability(r, resource, err != NULL /* undo if error */,
+                                           &av_info);
 
     /* check for errors now */
     if (err != NULL) {
@@ -1148,7 +1165,7 @@ static int dav_method_put(request_rec *r)
     /* NOTE: WebDAV spec, S8.7.1 states properties should be unaffected */
 
     /* return an appropriate response (HTTP_CREATED or HTTP_NO_CONTENT) */
-    return dav_created(r, NULL, "Resource", resource_existed);
+    return dav_created(r, NULL, "Resource", !av_info.resource_created);
 }
 
 /* ### move this to dav_util? */
@@ -1173,14 +1190,12 @@ void dav_add_response(dav_walker_ctx *ctx, const char *href, int status,
 static int dav_method_delete(request_rec *r)
 {
     dav_resource *resource;
-    dav_resource *resource_parent = NULL;
+    dav_auto_version_info av_info;
     dav_error *err;
     dav_error *err2;
     dav_response *multi_response;
-    const char *body;
     int result;
     int depth;
-    int parent_was_writable = 0;
 
     /* We don't use the request body right now, so torch it. */
     if ((result = ap_discard_request_body(r)) != OK) {
@@ -1188,7 +1203,7 @@ static int dav_method_delete(request_rec *r)
     }
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -1214,16 +1229,6 @@ static int dav_method_delete(request_rec *r)
        return HTTP_BAD_REQUEST;
     }
 
-    /* Check for valid resource type */
-    /* ### allow DAV_RESOURCE_TYPE_REVISION with All-Bindings header */
-    if (resource->type != DAV_RESOURCE_TYPE_REGULAR &&
-        resource->type != DAV_RESOURCE_TYPE_WORKSPACE) {
-        body = apr_psprintf(r->pool,
-                           "Cannot delete resource %s.",
-                           ap_escape_html(r->pool, r->uri));
-       return dav_error_response(r, HTTP_CONFLICT, body);
-    }
-
     /*
     ** If any resources fail the lock/If: conditions, then we must fail
     ** the delete. Each of the failing resources will be listed within
@@ -1254,9 +1259,7 @@ static int dav_method_delete(request_rec *r)
 
     /* if versioned resource, make sure parent is checked out */
     if ((err = dav_ensure_resource_writable(r, resource, 1 /* parent_only */,
-                                           &resource_parent,
-                                           NULL, NULL,
-                                           &parent_was_writable)) != NULL) {
+                                           &av_info)) != NULL) {
        /* ### add a higher-level description? */
        return dav_handle_err(r, err, NULL);
     }
@@ -1265,9 +1268,8 @@ static int dav_method_delete(request_rec *r)
     err = (*resource->hooks->remove_resource)(resource, &multi_response);
 
     /* restore writability of parent back to what it was */
-    err2 = dav_revert_resource_writability(r, NULL, resource_parent,
-                                          err != NULL /* undo if error */,
-                                          0, 0, parent_was_writable);
+    err2 = dav_revert_resource_writability(r, NULL, err != NULL /* undo if error */,
+                                          &av_info);
 
     /* check for errors now */
     if (err != NULL) {
@@ -1316,7 +1318,7 @@ static int dav_method_options(request_rec *r)
     ap_set_content_length(r, 0);
 
     /* resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
 
@@ -1523,7 +1525,7 @@ static int dav_method_propfind(request_rec *r)
     dav_walker_ctx ctx = { 0 };
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
 
@@ -1787,7 +1789,7 @@ static int dav_method_proppatch(request_rec *r)
     dav_prop_ctx *ctx;
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -1969,13 +1971,12 @@ static int dav_method_mkcol(request_rec *r)
 {
     dav_resource *resource;
     int resource_state;
-    dav_resource *resource_parent;
+    dav_auto_version_info av_info;
     const dav_hooks_locks *locks_hooks = DAV_GET_HOOKS_LOCKS(r);
     dav_error *err;
     dav_error *err2;
     int result;
     dav_dir_conf *conf;
-    int parent_was_writable = 0;
     dav_response *multi_status;
 
     /* handle the request body */
@@ -1988,7 +1989,7 @@ static int dav_method_mkcol(request_rec *r)
                                                 &dav_module);
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
 
@@ -2023,21 +2024,18 @@ static int dav_method_mkcol(request_rec *r)
 
     /* if versioned resource, make sure parent is checked out */
     if ((err = dav_ensure_resource_writable(r, resource, 1 /* parent_only */,
-                                           &resource_parent,
-                                           NULL, NULL,
-                                           &parent_was_writable)) != NULL) {
+                                           &av_info)) != NULL) {
        /* ### add a higher-level description? */
        return dav_handle_err(r, err, NULL);
     }
 
     /* try to create the collection */
     resource->collection = 1;
-    err = (*resource->hooks->create_collection)(r->pool, resource);
+    err = (*resource->hooks->create_collection)(resource);
 
     /* restore modifiability of parent back to what it was */
-    err2 = dav_revert_resource_writability(r, NULL, resource_parent,
-                                         err != NULL /* undo if error */,
-                                         0, 0, parent_was_writable);
+    err2 = dav_revert_resource_writability(r, NULL, err != NULL /* undo if error */,
+                                          &av_info);
 
     /* check for errors now */
     if (err != NULL) {
@@ -2091,9 +2089,9 @@ static int dav_method_mkcol(request_rec *r)
 static int dav_method_copymove(request_rec *r, int is_move)
 {
     dav_resource *resource;
-    dav_resource *resource_parent = NULL;
+    dav_auto_version_info src_av_info;
     dav_resource *resnew;
-    dav_resource *resnew_parent = NULL;
+    dav_auto_version_info dst_av_info;
     const char *body;
     const char *dest;
     dav_error *err;
@@ -2107,12 +2105,10 @@ static int dav_method_copymove(request_rec *r, int is_move)
     int result;
     dav_lockdb *lockdb;
     int replaced;
-    int src_parent_was_writable = 0;
-    int dst_parent_was_writable = 0;
     int resource_state;
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, !is_move /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -2166,7 +2162,7 @@ static int dav_method_copymove(request_rec *r, int is_move)
     }
 
     /* Resolve destination resource */
-    result = dav_get_resource(lookup.rnew, NULL, &resnew);
+    result = dav_get_resource(lookup.rnew, 0 /*target_allowed*/, NULL, &resnew);
     if (result != OK)
         return result;
 
@@ -2332,11 +2328,8 @@ static int dav_method_copymove(request_rec *r, int is_move)
 
     /* if this is a move, then the source parent collection will be modified */
     if (is_move) {
-        if ((err = dav_ensure_resource_writable(r, resource,
-                                               1 /* parent_only */,
-                                               &resource_parent,
-                                               NULL, NULL,
-                                               &src_parent_was_writable)) != NULL) {
+        if ((err = dav_ensure_resource_writable(r, resource, 1 /* parent_only */,
+                                               &src_av_info)) != NULL) {
            if (lockdb != NULL)
                (*lockdb->hooks->close_lockdb)(lockdb);
 
@@ -2347,17 +2340,13 @@ static int dav_method_copymove(request_rec *r, int is_move)
 
     /* prepare the destination collection for modification */
     if ((err = dav_ensure_resource_writable(r, resnew, 1 /* parent_only */,
-                                           &resnew_parent,
-                                           NULL, NULL,
-                                           &dst_parent_was_writable)) != NULL) {
+                                           &dst_av_info)) != NULL) {
         /* could not make destination writable:
         * if move, restore state of source parent
         */
         if (is_move) {
-            (void) dav_revert_resource_writability(r, NULL, resource_parent,
-                                                  1 /* undo */,
-                                                  0, 0,
-                                                  src_parent_was_writable);
+            (void) dav_revert_resource_writability(r, NULL, 1 /* undo */,
+                                                  &src_av_info);
         }
 
        if (lockdb != NULL)
@@ -2369,12 +2358,14 @@ static int dav_method_copymove(request_rec *r, int is_move)
 
     /* If source and destination parents are the same, then
      * use the same object, so status updates to one are reflected
-     * in the other.
+     * in the other, when reverting their writable states.
      */
-    if (resource_parent != NULL
-        && (*resource_parent->hooks->is_same_resource)(resource_parent,
-                                                       resnew_parent))
-        resnew_parent = resource_parent;
+    if (src_av_info.parent_resource != NULL
+        && (*src_av_info.parent_resource->hooks->is_same_resource)
+            (src_av_info.parent_resource, dst_av_info.parent_resource)) {
+
+        dst_av_info.parent_resource = src_av_info.parent_resource;
+    }
 
     /* New resource will be same kind as source */
     resnew->collection = resource->collection;
@@ -2397,14 +2388,12 @@ static int dav_method_copymove(request_rec *r, int is_move)
     }
 
     /* restore parent collection states */
-    err2 = dav_revert_resource_writability(r, NULL, resnew_parent,
-                                          err != NULL /* undo if error */,
-                                          0, 0, dst_parent_was_writable);
+    err2 = dav_revert_resource_writability(r, NULL, err != NULL /* undo if error */,
+                                          &dst_av_info);
 
     if (is_move) {
-        err3 = dav_revert_resource_writability(r, NULL, resource_parent,
-                                              err != NULL /* undo if error */,
-                                              0, 0, src_parent_was_writable);
+        err3 = dav_revert_resource_writability(r, NULL, err != NULL /* undo if error */,
+                                              &src_av_info);
     }
     else
        err3 = NULL;
@@ -2497,8 +2486,12 @@ static int dav_method_lock(request_rec *r)
        return HTTP_BAD_REQUEST;
     }
 
-    /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    /* Ask repository module to resolve the resource.
+     * DeltaV says result of target selector is undefined,
+     * so allow it, and let provider reject the lock attempt
+     * on a version if it wants to.
+     */
+    result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
 
@@ -2680,8 +2673,12 @@ static int dav_method_unlock(request_rec *r)
        return dav_handle_err(r, err, NULL);
     }
 
-    /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    /* Ask repository module to resolve the resource.
+     * DeltaV says result of target selector is undefined,
+     * so allow it, and let provider reject the unlock attempt
+     * on a version if it wants to.
+     */
+    result = dav_get_resource(r, 1 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
 
@@ -2732,12 +2729,12 @@ static int dav_method_vsn_control(request_rec *r)
 static int dav_method_checkout(request_rec *r)
 {
     dav_resource *resource;
+    dav_resource *working_resource;
     const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
     dav_error *err;
     int result;
     ap_xml_doc *doc;
-    const char *target;
-    const char *location;
+    ap_xml_elem *target = NULL;
 
     /* If no versioning provider, decline the request */
     if (vsn_hooks == NULL)
@@ -2755,15 +2752,12 @@ static int dav_method_checkout(request_rec *r)
             return HTTP_BAD_REQUEST;
         }
 
-        target = dav_get_target_selector(r, dav_find_child(doc->root,
-                                                           "version"));
-    }
-    else {
-        target = dav_get_target_selector(r, NULL);
+        if ((target = dav_find_child(doc->root, "version")) == NULL)
+            target = dav_find_child(doc->root, "label-name");
     }
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, target, &resource);
+    result = dav_get_resource(r, 1 /*target_allowed*/, target, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -2792,7 +2786,7 @@ static int dav_method_checkout(request_rec *r)
     /* ### do lock checks, once behavior is defined */
 
     /* Do the checkout */
-    if ((err = (*vsn_hooks->checkout)(resource, &location)) != NULL) {
+    if ((err = (*vsn_hooks->checkout)(resource, &working_resource)) != NULL) {
        err = dav_push_error(r->pool, HTTP_CONFLICT, 0,
                             apr_psprintf(r->pool,
                                         "Could not CHECKOUT resource %s.",
@@ -2801,10 +2795,9 @@ static int dav_method_checkout(request_rec *r)
         return dav_handle_err(r, err, NULL);
     }
 
-    /* set the Location and Cache-Control headers, per the spec */
-    apr_table_setn(r->headers_out, "Location", location);
+    /* set the Cache-Control headers, per the spec */
     apr_table_setn(r->headers_out, "Cache-Control", "no-cache");
-    return dav_created(r, location, "Working resource", 0);
+    return dav_created(r, working_resource->uri, "Working resource", 0);
 }
 
 /* handle the UNCHECKOUT method */
@@ -2824,7 +2817,7 @@ static int dav_method_uncheckout(request_rec *r)
     }
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /*target_allowed*/, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -2873,6 +2866,7 @@ static int dav_method_uncheckout(request_rec *r)
 static int dav_method_checkin(request_rec *r)
 {
     dav_resource *resource;
+    dav_resource *new_version;
     const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
     dav_error *err;
     int result;
@@ -2886,7 +2880,7 @@ static int dav_method_checkin(request_rec *r)
     }
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    result = dav_get_resource(r, 0 /* target_allowed */, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
@@ -2915,7 +2909,7 @@ static int dav_method_checkin(request_rec *r)
     /* ### do lock checks, once behavior is defined */
 
     /* Do the checkin */
-    if ((err = (*vsn_hooks->checkin)(resource)) != NULL) {
+    if ((err = (*vsn_hooks->checkin)(resource, &new_version)) != NULL) {
        err = dav_push_error(r->pool, HTTP_CONFLICT, 0,
                             apr_psprintf(r->pool,
                                         "Could not CHECKIN resource %s.",
@@ -2924,11 +2918,7 @@ static int dav_method_checkin(request_rec *r)
         return dav_handle_err(r, err, NULL);
     }
 
-    /* no body */
-    ap_set_content_length(r, 0);
-    ap_send_http_header(r);
-
-    return DONE;
+    return dav_created(r, new_version->uri, "Version", 0);
 }
 
 static int dav_method_set_target(request_rec *r)
@@ -2948,6 +2938,7 @@ static int dav_method_report(request_rec *r)
     dav_resource *resource;
     const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
     int result;
+    int target_allowed;
     ap_xml_doc *doc;
 
     /* If no versioning provider, decline the request */
@@ -2964,7 +2955,9 @@ static int dav_method_report(request_rec *r)
     }
 
     /* Ask repository module to resolve the resource */
-    result = dav_get_resource(r, NULL, &resource);
+    /* ### need to compute target_allowed based on report type */
+    target_allowed = 0;
+    result = dav_get_resource(r, target_allowed, NULL, &resource);
     if (result != OK)
         return result;
     if (!resource->exists) {
index 6671946693ffe8717252dd7d21f6585eddf62e25..3836fc288bfb3ecd5f41c950a0daa398a4a92e9a 100644 (file)
@@ -246,15 +246,31 @@ typedef dav_hooks_propdb dav_hooks_db;
 ** The base protocol defines only file and collection resources.
 ** The versioning protocol defines several additional resource types
 ** to represent artifacts of a version control system.
+**
+** This enumeration identifies the type of URL used to identify the
+** resource. Since the same resource may have more than one type of
+** URL which can identify it, dav_resource_type cannot be used
+** alone to determine the type of the resource; attributes of the
+** dav_resource object must also be consulted.
 */
 typedef enum {
-    DAV_RESOURCE_TYPE_REGULAR,      /* file or collection, working resource
-                                      or revision */
-    DAV_RESOURCE_TYPE_REVISION,     /* explicit revision-id */
-    DAV_RESOURCE_TYPE_HISTORY,      /* explicit history-id */
-    DAV_RESOURCE_TYPE_WORKSPACE,    /* workspace */
-    DAV_RESOURCE_TYPE_ACTIVITY,     /* activity */
-    DAV_RESOURCE_TYPE_CONFIGURATION /* configuration */
+    DAV_RESOURCE_TYPE_UNKNOWN,
+
+    DAV_RESOURCE_TYPE_REGULAR,      /* file or collection; could be
+                                     * unversioned or version selector */
+
+    DAV_RESOURCE_TYPE_VERSION,      /* version URL */
+
+    DAV_RESOURCE_TYPE_HISTORY,      /* version history URL */
+
+    DAV_RESOURCE_TYPE_WORKING,      /* working resource URL */
+
+    DAV_RESOURCE_TYPE_WORKSPACE,    /* workspace URL */
+
+    DAV_RESOURCE_TYPE_ACTIVITY,     /* activity URL */
+
+    DAV_RESOURCE_TYPE_BASELINE      /* baseline URL */
+
 } dav_resource_type;
 
 /*
@@ -271,14 +287,23 @@ typedef struct dav_resource {
     dav_resource_type type;
 
     int exists;                /* 0 => null resource */
-    int collection;    /* 0 => file (if type == DAV_RESOURCE_TYPE_REGULAR) */
-    int versioned;     /* 0 => unversioned */
-    int working;       /* 0 => revision (if versioned) */
-    int baselined;     /* 0 => not baselined */
+
+    int collection;    /* 0 => file; can be 1 for
+                         * REGULAR, VERSION, and WORKING resources,
+                         * and is always 1 for WORKSPACE */
+
+    int versioned;     /* 0 => unversioned; can be 1 for
+                         * REGULAR and WORKSPACE resources,
+                         * and is always 1 for VERSION, WORKING,
+                         * and BASELINE */
+
+    int working;       /* 0 => not checked out; can be 1 for
+                         * REGULAR, WORKSPACE, and BASELINE,
+                         * and is always 1 for WORKING */
 
     const char *uri;   /* the URI for this resource */
 
-    dav_resource_private *info;
+    dav_resource_private *info;         /* repository provider's private info */
 
     const dav_hooks_repository *hooks; /* hooks used for this resource */
 
@@ -852,9 +877,10 @@ int dav_get_resource_state(request_rec *r, const dav_resource *resource);
  */
 struct dav_hooks_locks
 {
-    /* Return the supportedlock property for this provider */
-    /* ### maybe this should take a resource argument? */
-    const char * (*get_supportedlock)(void);
+    /* Return the supportedlock property for a resource */
+    const char * (*get_supportedlock)(
+        const dav_resource *resource
+    );
 
     /* Parse a lock token URI, returning a lock token object allocated
      * in the given pool.
@@ -1254,8 +1280,11 @@ struct dav_hooks_repository
      *
      * The root_dir is the root of the directory for which this repository
      * is configured.
-     * The workspace is the value of any Target-Selector header, or NULL
+     * The workspace is the value of any Workspace header, or NULL
      * if there is none.
+     * The target is either a label, or a version URI, or NULL. If there
+     * is a target, then is_label specifies whether the target is a label
+     * or a URI.
      *
      * The provider may associate the request storage pool with the resource,
      * to use in other operations on that resource.
@@ -1263,7 +1292,9 @@ struct dav_hooks_repository
     dav_resource * (*get_resource)(
         request_rec *r,
         const char *root_dir,
-        const char *workspace
+        const char *workspace,
+       const char *target,
+        int is_label
     );
 
     /* Get a resource descriptor for the parent of the given resource.
@@ -1387,7 +1418,7 @@ struct dav_hooks_repository
      * is a collection.
      */
     dav_error * (*create_collection)(
-        apr_pool_t *p, dav_resource *resource
+        dav_resource *resource
     );
 
     /* Copy one resource to another. The destination must not exist.
@@ -1453,18 +1484,53 @@ struct dav_hooks_repository
 ** VERSIONING FUNCTIONS
 */
 
-/* dav_get_target_selector:
+
+/* dav_get_workspace:
  *
- * If a DAV:version element is provided, then it is assumed to provide the
- * target version. If no element is provided (version==NULL), then the
+ * Returns the value of any Workspace header in a request
+ * (used by versioning clients)
+ */
+int dav_get_workspace(request_rec *r, const char **workspace);
+
+/*
+ * dav_get_target_selector
+ *
+ * If a DAV:version or DAV:label-name element is provided,
+ * then it is assumed to provide the target version.
+ * If no element is provided (version==NULL), then the
  * request headers are examined for a Target-Selector header.
  *
- * The target version, if any, is then returned.
+ * The target version, if any, is then returned. If the version
+ * was specified by a label, then *is_label is set to 1.
+ * Otherwise, the target is a version URI.
  *
  * (used by versioning clients)
  */
-const char *dav_get_target_selector(request_rec *r,
-                                    const ap_xml_elem *version);
+int dav_get_target_selector(request_rec *r,
+                            const ap_xml_elem *version,
+                            const char **target,
+                            int *is_label);
+
+/* dav_add_vary_header
+ *
+ * If there were any headers in the request which require a Vary header
+ * in the response, add it.
+ */
+void dav_add_vary_header(request_rec *in_req,
+                        request_rec *out_req,
+                        const dav_resource *resource);
+
+/*
+** This structure is used to record what auto-versioning operations
+** were done to make a resource writable, so that they can be undone
+** at the end of a request.
+*/
+typedef struct {
+    int resource_created;               /* 0 => resource existed previously */
+    int resource_checkedout;            /* 0 => resource was checked out */
+    int parent_checkedout;              /* 0 => parent was checked out */
+    dav_resource *parent_resource;      /* parent resource, if it was needed */
+} dav_auto_version_info;
 
 /* Ensure that a resource is writable. If there is no versioning
  * provider, then this is essentially a no-op. Versioning repositories
@@ -1477,21 +1543,14 @@ const char *dav_get_target_selector(request_rec *r,
  * child does not exist, then a new versioned resource is created and
  * checked out.
  *
- * The parent_resource and parent_was_writable arguments are optional
- * (i.e. they may be NULL). If parent_only is set, then the
- * resource_existed and resource_was_writable arguments are ignored.
- *
- * The previous states of the resources are returned, so they can be
- * restored after the operation completes (see
- * dav_revert_resource_writability())
+ * The dav_auto_version_info structure is filled in with enough information
+ * to restore both parent and child resources to the state they were in
+ * before the auto-versioning operations occurred.
  */
 dav_error *dav_ensure_resource_writable(request_rec *r,
                                        dav_resource *resource,
                                         int parent_only,
-                                       dav_resource **parent_resource,
-                                       int *resource_existed,
-                                       int *resource_was_writable,
-                                       int *parent_was_writable);
+                                        dav_auto_version_info *av_info);
 
 /* Revert the writability of resources back to what they were
  * before they were modified. If undo == 0, then the resource
@@ -1499,25 +1558,24 @@ dav_error *dav_ensure_resource_writable(request_rec *r,
  * If undo != 0, then resource modifications are discarded
  * (i.e. they are unchecked out).
  *
- * The resource and parent_resource arguments are optional
- * (i.e. they may be NULL).
+ * The resource argument may be NULL if only the parent resource
+ * was made writable (i.e. the parent_only was != 0 in the
+ * dav_ensure_resource_writable call).
  */
-dav_error *dav_revert_resource_writability(request_rec *r,
-                                          dav_resource *resource,
-                                          dav_resource *parent_resource,
-                                          int undo,
-                                          int resource_existed,
-                                          int resource_was_writable,
-                                          int parent_was_writable);
+dav_error *dav_revert_resource_writability(
+    request_rec *r,
+    dav_resource *resource,
+    int undo,
+    const dav_auto_version_info *av_info);
 
 /*
 ** This structure is used to describe available reports
 **
-** "namespace" should be valid XML and URL-quoted. mod_dav will place
+** "nmspace" should be valid XML and URL-quoted. mod_dav will place
 ** double-quotes around it and use it in an xmlns declaration.
 */
 typedef struct {
-    const char *namespace;      /* namespace of the XML report element */
+    const char *nmspace;        /* namespace of the XML report element */
     const char *name;           /* element name for the XML report */
 } dav_report_elem;
 
@@ -1538,9 +1596,15 @@ struct dav_hooks_vsn
     /* Checkout a resource. If successful, the resource
      * object state is updated appropriately.
      *
-     * The location of the working resource should be returned in *location.
+     * If the working resource has a different URL from the
+     * target resource, a dav_resource descriptor is returned
+     * for the new working resource. Otherwise, the original
+     * resource descriptor will refer to the working resource.
+     * The working_resource argument can be NULL if the caller
+     * is not interested in the working resource.
      */
-    dav_error * (*checkout)(dav_resource *resource, const char **location);
+    dav_error * (*checkout)(dav_resource *resource,
+                            dav_resource **working_resource);
 
     /* Uncheckout a resource. If successful, the resource
      * object state is updated appropriately.
@@ -1548,9 +1612,13 @@ struct dav_hooks_vsn
     dav_error * (*uncheckout)(dav_resource *resource);
 
     /* Checkin a working resource. If successful, the resource
-     * object state is updated appropriately.
+     * object state is updated appropriately, and the
+     * version_resource descriptor will refer to the new version.
+     * The version_resource argument can be NULL if the caller
+     * is not interested in the new version resource.
      */
-    dav_error * (*checkin)(dav_resource *resource);
+    dav_error * (*checkin)(dav_resource *resource,
+                           dav_resource **version_resource);
 
     /* Determine whether a non-versioned (or non-existent) resource
      * is versionable. Returns != 0 if resource can be versioned.
index 8426f981dcb96a74c443866ea74015b875107967..5bca277071428be15d858176f49e5ce40646e41d 100644 (file)
@@ -431,6 +431,8 @@ static dav_error * dav_insert_coreprop(dav_propdb *propdb,
     case DAV_PROPID_CORE_resourcetype:
         switch (propdb->resource->type) {
         case DAV_RESOURCE_TYPE_REGULAR:
+        case DAV_RESOURCE_TYPE_VERSION:
+        case DAV_RESOURCE_TYPE_WORKING:
             if (propdb->resource->collection) {
                value = "<D:collection/>";
             }
@@ -441,22 +443,19 @@ static dav_error * dav_insert_coreprop(dav_propdb *propdb,
            }
             break;
         case DAV_RESOURCE_TYPE_HISTORY:
-           value = "<D:history/>";
+           value = "<D:version-history/>";
             break;
         case DAV_RESOURCE_TYPE_WORKSPACE:
-           value = "<D:workspace/>";
+           value = "<D:collection/>";
             break;
         case DAV_RESOURCE_TYPE_ACTIVITY:
            value = "<D:activity/>";
             break;
-        case DAV_RESOURCE_TYPE_CONFIGURATION:
-           value = "<D:configuration/>";
+        case DAV_RESOURCE_TYPE_BASELINE:
+           value = "<D:baseline/>";
             break;
-       case DAV_RESOURCE_TYPE_REVISION:
-           value = "<D:revision/>";
-           break;
 
-       default:
+        default:
            /* ### bad juju */
            break;
         }
@@ -496,7 +495,7 @@ static dav_error * dav_insert_coreprop(dav_propdb *propdb,
 
     case DAV_PROPID_CORE_supportedlock:
         if (propdb->lockdb != NULL) {
-           value = (*propdb->lockdb->hooks->get_supportedlock)();
+           value = (*propdb->lockdb->hooks->get_supportedlock)(propdb->resource);
         }
        break;
 
index 4ca3b92a0066b09deedfc728949dd62301bf6294..ee56c7684ce8ee7fee423aebb4bfb014cc6b2a18 100644 (file)
@@ -1502,78 +1502,155 @@ static const char *strip_white(const char *s, apr_pool_t *pool)
     return s;
 }
 
+#define DAV_WORKSPACE_HDR "Workspace"
+#define DAV_TARGET_SELECTOR_HDR "Target-Selector"
+
+/* see mod_dav.h for docco */
+int dav_get_workspace(request_rec *r, const char **workspace)
+{
+    const char *ws_uri;
+
+    *workspace = NULL;
+    ws_uri = apr_table_get(r->headers_in, DAV_WORKSPACE_HDR);
+
+    if (ws_uri != NULL) {
+       dav_lookup_result lookup;
+
+       /* make the URI server-relative */
+       lookup = dav_lookup_uri(ws_uri, r);
+       if (lookup.rnew == NULL) {
+           if (lookup.err.status == HTTP_BAD_REQUEST) {
+               /* This supplies additional information for the default message. */
+               ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r,
+                             lookup.err.desc);
+               return HTTP_BAD_REQUEST;
+           }
+
+           return lookup.err.status;
+       }
+
+       *workspace = lookup.rnew->uri;
+    }
+
+    return OK;
+}
+
 /* see mod_dav.h for docco */
-const char *dav_get_target_selector(request_rec *r, const ap_xml_elem *version)
+int dav_get_target_selector(request_rec *r,
+                            const ap_xml_elem *version,
+                            const char **target,
+                            int *is_label)
 {
+    /* Initialize results */
+    *target = NULL;
+    *is_label = 0;
+
     if (version != NULL) {
-        /* DAV:version contains a DAV:href element. find it. */
-        if ((version = dav_find_child(version, "href")) == NULL) {
-            /* ### this should generate an error... fallthru for now */
-        }
-        else {
+        /* Expect either <DAV:version><DAV:href>URI</DAV:href></DAV:version>
+         * or <DAV:label-name>LABEL</DAV:label-name> */
+        if (strcmp(version->name, "version") == 0) {
+            if ((version = dav_find_child(version, "href")) == NULL) {
+               ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r,
+                             "Missing DAV:href in DAV:version element");
+                return HTTP_BAD_REQUEST;
+            }
+
             /* return the contents of the DAV:href element */
             /* ### this presumes no child elements */
-            return strip_white(version->first_cdata.first->text, r->pool);
+            *target = strip_white(version->first_cdata.first->text, r->pool);
+        }
+        else if (strcmp(version->name, "label-name") == 0) {
+            /* return contents of the DAV:label-name element */
+            *target = strip_white(version->first_cdata.first->text, r->pool);
+            *is_label = 1;
         }
+        else {
+           ap_log_rerror(APLOG_MARK, APLOG_ERR | APLOG_NOERRNO, 0, r,
+                         "Unknown version specifier (not DAV:version or DAV:label-name)");
+            return HTTP_BAD_REQUEST;
+        }
+    }
+    else {
+        /* no element. see if a Target-Selector header was provided
+         * (which is always interpreted as a label) */
+        *target = apr_table_get(r->headers_in, DAV_TARGET_SELECTOR_HDR);
+        *is_label = 1;
     }
 
-    /* no element. see if a Target-Selector header was provided. */
-    return apr_table_get(r->headers_in, "Target-Selector");
+    return OK;
+}
+
+/* dav_add_vary_header
+ *
+ * If there were any headers in the request which require a Vary header
+ * in the response, add it.
+ */
+void dav_add_vary_header(request_rec *in_req,
+                        request_rec *out_req,
+                        const dav_resource *resource)
+{
+    const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(in_req);
+
+    /* Only versioning headers require a Vary response header,
+     * so only do this check if there is a versioning provider */
+    if (vsn_hooks != NULL) {
+       const char *workspace = apr_table_get(in_req->headers_in, DAV_WORKSPACE_HDR);
+       const char *target = apr_table_get(in_req->headers_in, DAV_TARGET_SELECTOR_HDR);
+       const char *vary = apr_table_get(out_req->headers_out, "Vary");
+
+        /* If Workspace header specified, add it to Vary header */
+       if (workspace != NULL) {
+           if (vary == NULL)
+               vary = DAV_WORKSPACE_HDR;
+           else
+               vary = apr_pstrcat(out_req->pool, vary, "," DAV_WORKSPACE_HDR, NULL);
+       }
+
+        /* If Target-Selector specified, add it to the Vary header */
+       if (target != NULL) {
+           if (vary == NULL)
+               vary = DAV_TARGET_SELECTOR_HDR;
+           else
+               vary = apr_pstrcat(out_req->pool, vary, "," DAV_TARGET_SELECTOR_HDR, NULL);
+       }
+
+       if (workspace != NULL || target != NULL)
+           apr_table_setn(out_req->headers_out, "Vary", vary);
+    }
 }
 
 /* see mod_dav.h for docco */
 dav_error *dav_ensure_resource_writable(request_rec *r,
-                                         dav_resource *resource,
-                                          int parent_only,
-                                         dav_resource **parent_resource,
-                                         int *resource_existed,
-                                         int *resource_was_writable,
-                                         int *parent_was_writable)
+                                       dav_resource *resource,
+                                        int parent_only,
+                                        dav_auto_version_info *av_info)
 {
     const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
-    dav_resource *parent = NULL;
     const char *body;
-    int auto_version;
     dav_error *err;
-    const char *location;
 
-    if (parent_resource != NULL)
-        *parent_resource = NULL;
+    /* Initialize results */
+    memset(av_info, 0, sizeof(*av_info));
 
     if (!parent_only) {
-        *resource_existed = resource->exists;
-        *resource_was_writable = 0;
+        av_info->resource_created = !resource->exists;
     }
 
-    if (parent_was_writable != NULL)
-        *parent_was_writable = 0;
-
-    /* if a Target-Selector header is present, then the client knows about
-     * versioning, so it should not be relying on implicit versioning
-     */
-    auto_version = (dav_get_target_selector(r, NULL) == NULL);
-
     /* check parent resource if requested or if resource must be created */
     if (!resource->exists || parent_only) {
-       parent = (*resource->hooks->get_parent_resource)(resource);
+       dav_resource *parent = (*resource->hooks->get_parent_resource)(resource);
         if (parent == NULL || !parent->exists) {
            body = apr_psprintf(r->pool,
-                              "Missing one or more intermediate collections. "
-                              "Cannot create resource %s.",
-                              ap_escape_html(r->pool, resource->uri));
+                                "Missing one or more intermediate collections. "
+                                "Cannot create resource %s.",
+                                ap_escape_html(r->pool, resource->uri));
            return dav_new_error(r->pool, HTTP_CONFLICT, 0, body);
         }
 
-        if (parent_resource != NULL)
-           *parent_resource = parent;
+        av_info->parent_resource = parent;
 
        /* if parent not versioned, assume child can be created */
        if (!parent->versioned) {
-            if (!parent_only)
-               *resource_was_writable = 1;
-
-            if (parent_was_writable != NULL)
-               *parent_was_writable = 1;
            return NULL;
        }
 
@@ -1585,118 +1662,134 @@ dav_error *dav_ensure_resource_writable(request_rec *r,
                                 "provider?");
        }
 
-        /* remember whether parent was already writable */
-        if (parent_was_writable != NULL)
-           *parent_was_writable = parent->working;
-
        /* parent must be checked out */
        if (!parent->working) {
-           if ((err = (*vsn_hooks->checkout)(parent, &location)) != NULL) {
+            /* if parent cannot be automatically checked out, fail */
+            if (!(*vsn_hooks->auto_version_enabled)(parent)) {
+               body = apr_psprintf(r->pool,
+                                    "Parent collection must be checked out. "
+                                    "Cannot create resource %s.",
+                                    ap_escape_html(r->pool, resource->uri));
+               return dav_new_error(r->pool, HTTP_CONFLICT, 0, body);
+            }
+
+            /* Try to checkout the parent collection.
+             * Note that auto-versioning can only be applied to a version selector,
+             * so no separate working resource will be created.
+             */
+           if ((err = (*vsn_hooks->checkout)(parent, NULL))
+                != NULL)
+            {
                body = apr_psprintf(r->pool,
-                                  "Unable to checkout parent collection. "
-                                  "Cannot create resource %s.",
-                                  ap_escape_html(r->pool, resource->uri));
+                                    "Unable to checkout parent collection. "
+                                    "Cannot create resource %s.",
+                                    ap_escape_html(r->pool, resource->uri));
                return dav_push_error(r->pool, HTTP_CONFLICT, 0, body, err);
            }
 
-            /* ### what to do with the location? */
+            /* remember that parent was checked out */
+            av_info->parent_checkedout = 1;
        }
 
        /* if not just checking parent, create new child resource */
         if (!parent_only) {
            if ((err = (*vsn_hooks->mkresource)(resource)) != NULL) {
                body = apr_psprintf(r->pool,
-                                  "Unable to create versioned resource %s.",
-                                  ap_escape_html(r->pool, resource->uri));
+                                    "Unable to create versioned resource %s.",
+                                    ap_escape_html(r->pool, resource->uri));
                return dav_push_error(r->pool, HTTP_CONFLICT, 0, body, err);
            }
+
+            /* remember that resource was created */
+            av_info->resource_created = 1;
         }
     }
-    else {
-       /* resource exists: if not versioned, then assume it is writable */
-       if (!resource->versioned) {
-           *resource_was_writable = 1;
-           return NULL;
-       }
-
-       *resource_was_writable = resource->working;
+    else if (!resource->versioned) {
+       /* resource exists and is not versioned; assume it is writable */
+       return NULL;
     }
 
     /* if not just checking parent, make sure child resource is checked out */
     if (!parent_only && !resource->working) {
-       if ((err = (*vsn_hooks->checkout)(resource, &location)) != NULL) {
+        /* Auto-versioning can only be applied to version selectors, so
+         * no separate working resource will be created. */
+       if ((err = (*vsn_hooks->checkout)(resource, NULL))
+            != NULL)
+        {
            body = apr_psprintf(r->pool,
-                              "Unable to checkout resource %s.",
-                              ap_escape_html(r->pool, resource->uri));
+                                "Unable to checkout resource %s.",
+                                ap_escape_html(r->pool, resource->uri));
            return dav_push_error(r->pool, HTTP_CONFLICT, 0, body, err);
        }
 
-        /* ### what to do with the location? */
+        /* remember that resource was checked out */
+        av_info->resource_checkedout = 1;
     }
 
     return NULL;
 }
 
 /* see mod_dav.h for docco */
-dav_error *dav_revert_resource_writability(request_rec *r,
-                                          dav_resource *resource,
-                                          dav_resource *parent_resource,
-                                          int undo,
-                                          int resource_existed,
-                                          int resource_was_writable,
-                                          int parent_was_writable)
+dav_error *dav_revert_resource_writability(
+    request_rec *r,
+    dav_resource *resource,
+    int undo,
+    const dav_auto_version_info *av_info)
 {
     const dav_hooks_vsn *vsn_hooks = DAV_GET_HOOKS_VSN(r);
     const char *body;
     dav_error *err;
 
+    /* If a resource was provided, restore its writable state.
+     * Otherwise, only the parent must have been modified */
     if (resource != NULL) {
-        if (!resource_was_writable
-           && resource->versioned && resource->working) {
+        if (av_info->resource_checkedout) {
 
             if (undo)
                 err = (*vsn_hooks->uncheckout)(resource);
             else
-                err = (*vsn_hooks->checkin)(resource);
+                err = (*vsn_hooks->checkin)(resource, NULL);
 
             if (err != NULL) {
                body = apr_psprintf(r->pool,
-                                  "Unable to %s resource %s.",
-                                   undo ? "uncheckout" : "checkin",
-                                  ap_escape_html(r->pool, resource->uri));
+                                    "Unable to %s resource %s.",
+                                    undo ? "uncheckout" : "checkin",
+                                    ap_escape_html(r->pool, resource->uri));
                 return dav_push_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0,
                                      body, err);
             }
         }
 
-        if (undo && !resource_existed && resource->exists) {
+        /* If undoing because of an error, and the resource was created,
+         * then remove it */
+        if (undo && av_info->resource_created) {
            dav_response *response;
 
            /* ### should we do anything with the response? */
             if ((err = (*resource->hooks->remove_resource)(resource,
                                                           &response)) != NULL) {
                body = apr_psprintf(r->pool,
-                                  "Unable to undo creation of resource %s.",
-                                  ap_escape_html(r->pool, resource->uri));
+                                    "Unable to undo creation of resource %s.",
+                                    ap_escape_html(r->pool, resource->uri));
                 return dav_push_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0,
                                      body, err);
             }
         }
     }
 
-    if (parent_resource != NULL && !parent_was_writable
-       && parent_resource->versioned && parent_resource->working) {
+    /* If parent resource was made writable, restore its state */
+    if (av_info->parent_resource != NULL && av_info->parent_checkedout) {
 
        if (undo)
-           err = (*vsn_hooks->uncheckout)(parent_resource);
+           err = (*vsn_hooks->uncheckout)(av_info->parent_resource);
        else
-           err = (*vsn_hooks->checkin)(parent_resource);
+           err = (*vsn_hooks->checkin)(av_info->parent_resource, NULL);
 
        if (err != NULL) {
            body = apr_psprintf(r->pool,
-                              "Unable to %s parent collection of %s.",
-                              undo ? "uncheckout" : "checkin",
-                              ap_escape_html(r->pool, resource->uri));
+                                "Unable to %s parent collection %s.",
+                                undo ? "uncheckout" : "checkin",
+                                ap_escape_html(r->pool, av_info->parent_resource->uri));
            return dav_push_error(r->pool, HTTP_INTERNAL_SERVER_ERROR, 0,
                                  body, err);
        }