You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2013/12/26 16:54:09 UTC
svn commit: r1553501 - in /subversion/trunk/subversion:
libsvn_ra_serf/commit.c tests/cmdline/lock_tests.py
Author: rhuijben
Date: Thu Dec 26 15:54:09 2013
New Revision: 1553501
URL: http://svn.apache.org/r1553501
Log:
In an attempt to finally resolve issue #3674 "Replace + propset of locked file
fails over DAV", fix the "If" header generation in libsvn_ra_serf and
consistently use that for the DELETE, MKCOL and COPY operations affecting
trees.
* subversion/libsvn_ra_serf/commit.c
(delete_context_t): Make more consistent with other commit batons.
(setup_if_header_recursive,
setup_add_dir_common_headers): New function.
(setup_copy_dir_headers): Call setup_add_dir_common_headers.
(setup_delete_headers): Use setup_if_header_recursive to add all locks to
the header instead of only those on the target itself.
(create_delete_body): Remove unneeded function.
(delete_entry): Update calller. Remove unneeded retry handling for missing
locks.
(add_directory): Hook creation header for 'MKCOL' to add 'If' when necessary.
(svn_ra_serf__get_commit_editor): Store empty lock hash as no hash.
(svn_ra_serf__change_rev_prop): Remove unused variable.
* subversion/tests/cmdline/lock_tests.py
(replace_and_propset_locked_path): Remove XFail marker.
Modified:
subversion/trunk/subversion/libsvn_ra_serf/commit.c
subversion/trunk/subversion/tests/cmdline/lock_tests.py
Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1553501&r1=1553500&r2=1553501&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Thu Dec 26 15:54:09 2013
@@ -99,14 +99,11 @@ typedef struct proppatch_context_t {
} proppatch_context_t;
typedef struct delete_context_t {
- const char *path;
+ const char *relpath;
svn_revnum_t revision;
- const char *lock_token;
- apr_hash_t *lock_token_hash;
- svn_boolean_t keep_locks;
-
+ commit_context_t *commit;
} delete_context_t;
/* Represents a directory. */
@@ -149,7 +146,6 @@ typedef struct dir_context_t {
/* The checked-out working resource for this directory. May be NULL; if so
call checkout_dir() first. */
const char *working_url;
-
} dir_context_t;
/* Represents a file to be committed. */
@@ -1080,6 +1076,93 @@ setup_copy_file_headers(serf_bucket_t *h
}
static svn_error_t *
+setup_if_header_recursive(svn_boolean_t *added,
+ serf_bucket_t *headers,
+ commit_context_t *commit_ctx,
+ const char *relpath,
+ apr_pool_t *pool)
+{
+ svn_stringbuf_t *sb = NULL;
+ apr_hash_index_t *hi;
+ apr_pool_t *iterpool = NULL;
+
+ if (!commit_ctx->lock_tokens)
+ return SVN_NO_ERROR;
+
+ /* We try to create a directory, so within the Subversion world that
+ would imply that there is nothing here, but mod_dav_svn still sees
+ locks on the old nodes here as in DAV it is perfectly legal to lock
+ something that is not there...
+
+ Let's make mod_dav, mod_dav_svn and the DAV RFC happy by providing
+ the locks we know of with the request */
+
+ for (hi = apr_hash_first(pool, commit_ctx->lock_tokens);
+ hi;
+ hi = apr_hash_next(hi))
+ {
+ const char *relpath = svn__apr_hash_index_key(hi);
+ apr_uri_t uri;
+
+ if (!svn_relpath_skip_ancestor(relpath, relpath))
+ continue;
+ else if (svn_hash_gets(commit_ctx->deleted_entries, relpath))
+ {
+ /* When a path is already explicit deleted then its lock
+ will be removed by mod_dav. But mod_dav doesn't remove
+ locks on descendants */
+ continue;
+ }
+
+ if (!iterpool)
+ iterpool = svn_pool_create(pool);
+ else
+ svn_pool_clear(iterpool);
+
+ if (sb == NULL)
+ sb = svn_stringbuf_create("", pool);
+ else
+ svn_stringbuf_appendbyte(sb, ' ');
+
+ uri = commit_ctx->session->session_url;
+ uri.path = (char *)svn_path_url_add_component2(uri.path, relpath,
+ iterpool);
+
+ svn_stringbuf_appendbyte(sb, '<');
+ svn_stringbuf_appendcstr(sb, apr_uri_unparse(iterpool, &uri, 0));
+ svn_stringbuf_appendcstr(sb, "> (<");
+ svn_stringbuf_appendcstr(sb, svn__apr_hash_index_val(hi));
+ svn_stringbuf_appendcstr(sb, ">)");
+ }
+
+ if (iterpool)
+ svn_pool_destroy(iterpool);
+
+ if (sb)
+ {
+ serf_bucket_headers_set(headers, "If", sb->data);
+ *added = TRUE;
+ }
+ else
+ *added = FALSE;
+
+ return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+setup_add_dir_common_headers(serf_bucket_t *headers,
+ void *baton,
+ apr_pool_t *pool)
+{
+ dir_context_t *dir = baton;
+ svn_boolean_t added;
+
+ return svn_error_trace(
+ setup_if_header_recursive(&added, headers, dir->commit, dir->relpath,
+ pool));
+}
+
+static svn_error_t *
setup_copy_dir_headers(serf_bucket_t *headers,
void *baton,
apr_pool_t *pool)
@@ -1111,7 +1194,7 @@ setup_copy_dir_headers(serf_bucket_t *he
/* Implicitly checkout this dir now. */
dir->working_url = apr_pstrdup(dir->pool, uri.path);
- return SVN_NO_ERROR;
+ return svn_error_trace(setup_add_dir_common_headers(headers, baton, pool));
}
static svn_error_t *
@@ -1119,51 +1202,19 @@ setup_delete_headers(serf_bucket_t *head
void *baton,
apr_pool_t *pool)
{
- delete_context_t *ctx = baton;
+ delete_context_t *del = baton;
+ svn_boolean_t added;
serf_bucket_headers_set(headers, SVN_DAV_VERSION_NAME_HEADER,
- apr_ltoa(pool, ctx->revision));
-
- if (ctx->lock_token_hash)
- {
- ctx->lock_token = svn_hash_gets(ctx->lock_token_hash, ctx->path);
-
- if (ctx->lock_token)
- {
- const char *token_header;
-
- token_header = apr_pstrcat(pool, "<", ctx->path, "> (<",
- ctx->lock_token, ">)", SVN_VA_NULL);
-
- serf_bucket_headers_set(headers, "If", token_header);
-
- if (ctx->keep_locks)
- serf_bucket_headers_setn(headers, SVN_DAV_OPTIONS_HEADER,
- SVN_DAV_OPTION_KEEP_LOCKS);
- }
- }
-
- return SVN_NO_ERROR;
-}
-
-/* Implements svn_ra_serf__request_body_delegate_t */
-static svn_error_t *
-create_delete_body(serf_bucket_t **body_bkt,
- void *baton,
- serf_bucket_alloc_t *alloc,
- apr_pool_t *pool)
-{
- delete_context_t *ctx = baton;
- serf_bucket_t *body;
-
- body = serf_bucket_aggregate_create(alloc);
+ apr_ltoa(pool, del->revision));
- svn_ra_serf__add_xml_header_buckets(body, alloc);
+ SVN_ERR(setup_if_header_recursive(&added, headers, del->commit,
+ del->relpath, pool));
- svn_ra_serf__merge_lock_token_list(ctx->lock_token_hash, ctx->path,
- body, alloc, pool);
+ if (added && del->commit->keep_locks)
+ serf_bucket_headers_setn(headers, SVN_DAV_OPTIONS_HEADER,
+ SVN_DAV_OPTION_KEEP_LOCKS);
- *body_bkt = body;
return SVN_NO_ERROR;
}
@@ -1528,7 +1579,6 @@ delete_entry(const char *path,
delete_context_t *delete_ctx;
svn_ra_serf__handler_t *handler;
const char *delete_target;
- svn_error_t *err;
if (USING_HTTPV2_COMMIT_SUPPORT(dir->commit))
{
@@ -1547,10 +1597,9 @@ delete_entry(const char *path,
/* DELETE our entry */
delete_ctx = apr_pcalloc(pool, sizeof(*delete_ctx));
- delete_ctx->path = apr_pstrdup(pool, path);
+ delete_ctx->relpath = apr_pstrdup(pool, path);
delete_ctx->revision = revision;
- delete_ctx->lock_token_hash = dir->commit->lock_tokens;
- delete_ctx->keep_locks = dir->commit->keep_locks;
+ delete_ctx->commit = dir->commit;
handler = apr_pcalloc(pool, sizeof(*handler));
handler->handler_pool = pool;
@@ -1566,24 +1615,7 @@ delete_entry(const char *path,
handler->method = "DELETE";
handler->path = delete_target;
- err = svn_ra_serf__context_run_one(handler, pool);
-
- if (err &&
- (err->apr_err == SVN_ERR_FS_BAD_LOCK_TOKEN ||
- err->apr_err == SVN_ERR_FS_NO_LOCK_TOKEN ||
- err->apr_err == SVN_ERR_FS_LOCK_OWNER_MISMATCH ||
- err->apr_err == SVN_ERR_FS_PATH_ALREADY_LOCKED))
- {
- svn_error_clear(err);
-
- handler->body_delegate = create_delete_body;
- handler->body_delegate_baton = delete_ctx;
- handler->body_type = "text/xml";
-
- SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
- }
- else
- SVN_ERR(err);
+ SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
/* 204 No Content: item successfully deleted */
if (handler->sline.code != 204)
@@ -1652,6 +1684,9 @@ add_directory(const char *path,
{
handler->method = "MKCOL";
handler->path = mkcol_target;
+
+ handler->header_delegate = setup_add_dir_common_headers;
+ handler->header_delegate_baton = dir;
}
else
{
@@ -2315,7 +2350,8 @@ svn_ra_serf__get_commit_editor(svn_ra_se
ctx->callback = callback;
ctx->callback_baton = callback_baton;
- ctx->lock_tokens = lock_tokens;
+ ctx->lock_tokens = (lock_tokens && apr_hash_count(lock_tokens))
+ ? lock_tokens : NULL;
ctx->keep_locks = keep_locks;
ctx->deleted_entries = apr_hash_make(ctx->pool);
@@ -2362,7 +2398,6 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
commit_context_t *commit;
const char *proppatch_target;
const char *ns;
- svn_error_t *err;
const svn_string_t *tmp_old_value;
svn_boolean_t atomic_capable = FALSE;
Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1553501&r1=1553500&r2=1553501&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Thu Dec 26 15:54:09 2013
@@ -1520,7 +1520,6 @@ def verify_path_escaping(sbox):
#----------------------------------------------------------------------
# Issue #3674: Replace + propset of locked file fails over DAV
-@XFail(svntest.main.is_ra_type_dav)
@Issue(3674)
def replace_and_propset_locked_path(sbox):
"test replace + propset of locked file"
@@ -1553,11 +1552,9 @@ def replace_and_propset_locked_path(sbox
# Replace A/D/G and A/D/G/rho, propset on A/D/G/rho.
svntest.actions.run_and_verify_svn(None, None, [],
'rm', G_path)
- # Recreate path for single-db
- if not os.path.exists(G_path):
- os.mkdir(G_path)
+
svntest.actions.run_and_verify_svn(None, None, [],
- 'add', G_path)
+ 'mkdir', G_path)
svntest.main.file_append(rho_path, "This is the new file 'rho'.\n")
svntest.actions.run_and_verify_svn(None, None, [],
'add', rho_path)