On 3 March 2015 at 04:06, <rhuij...@apache.org> wrote:
> Author: rhuijben
> Date: Tue Mar 3 01:06:16 2015
> New Revision: 1663500
>
> URL: http://svn.apache.org/r1663500
> Log:
> In ra-serf (for issue #4557) reinstate a bit of code that retries a delete
> with an altered request if the original request fails, because the server
> determined that it is an invalid request, because it has too many bytes
> in the headers.
>
> Before r1553501 we always retried DELETE requests that required lock
> tokens, as the initial request didn't have an If header at all.
>
Hi Bert,
See my review inline.
[...]
> Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1663500&r1=1663499&r2=1663500&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Tue Mar 3 01:06:16
> 2015
[...]
> static svn_error_t *
> delete_entry(const char *path,
> svn_revnum_t revision,
> @@ -1412,6 +1443,7 @@ 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_ctx))
> {
> @@ -1446,7 +1478,23 @@ delete_entry(const char *path,
> handler->method = "DELETE";
> handler->path = delete_target;
>
> - SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
> + err = svn_ra_serf__context_run_one(handler, pool);
> + if (err && err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED
> + && handler->sline.code == 400)
May be it's better set handler->no_fail_on_http_failure_status to TRUE
and check for handler->sline.code? This will remove dependency on
specific error code and simplify code a bit.
> + {
> + svn_error_clear(err);
> +
> + /* Try again with non-standard body to overcome Apache Httpd
> + header limit */
> + delete_ctx->non_recursive_if = TRUE;
> + handler->body_type = "text/xml";
> + handler->body_delegate = create_delete_body;
> + handler->body_delegate_baton = delete_ctx;
> +
> + SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
I'm not sure that we can reuse svn_ra_serf__handler_t instance for
multiple requests.
I propose the following patch. Do you have any objections?
--
Ivan Zhakov
Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c (revision 1736882)
+++ subversion/libsvn_ra_serf/commit.c (working copy)
@@ -1415,7 +1415,6 @@
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_ctx))
{
@@ -1449,16 +1448,26 @@
handler->method = "DELETE";
handler->path = delete_target;
+ handler->no_fail_on_http_failure_status = TRUE;
- err = svn_ra_serf__context_run_one(handler, pool);
- if (err && err->apr_err == SVN_ERR_RA_DAV_REQUEST_FAILED
- && handler->sline.code == 400)
+ SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
+ if (handler->sline.code == 400)
{
- svn_error_clear(err);
-
/* Try again with non-standard body to overcome Apache Httpd
header limit */
delete_ctx->non_recursive_if = TRUE;
+
+ handler = svn_ra_serf__create_handler(dir->commit_ctx->session, pool);
+
+ handler->response_handler = svn_ra_serf__expect_empty_body;
+ handler->response_baton = handler;
+
+ handler->header_delegate = setup_delete_headers;
+ handler->header_delegate_baton = delete_ctx;
+
+ handler->method = "DELETE";
+ handler->path = delete_target;
+
handler->body_type = "text/xml";
handler->body_delegate = create_delete_body;
handler->body_delegate_baton = delete_ctx;
@@ -1465,9 +1474,10 @@
SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
}
- else
- SVN_ERR(err);
+ if (handler->server_error)
+ return svn_ra_serf__server_error_create(handler, pool);
+
/* 204 No Content: item successfully deleted */
if (handler->sline.code != 204)
return svn_error_trace(svn_ra_serf__unexpected_status(handler));
