You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2012/05/06 19:53:15 UTC

svn commit: r1334720 - in /subversion/trunk/subversion/libsvn_ra_serf: commit.c ra_serf.h util.c

Author: gstein
Date: Sun May  6 17:53:15 2012
New Revision: 1334720

URL: http://svn.apache.org/viewvc?rev=1334720&view=rev
Log:
Switch the multistatus processing over to the new handler-based DONE
and SERVER_ERROR members. This is only used by commit's PROPPATCH.

* subversion/libsvn_ra_serf/ra_serf.h:
  (svn_ra_serf__response_handler_t): clarify the parameter is a
    scratch pool.
  (svn_ra_serf__handle_server_error): this is NOT a response_handler_t
  (svn_ra_serf__handle_multistatus_only): update the BATON type.
    rename to SCRATCH_POOL. rework doc about DONE flag.

* subversion/libsvn_ra_serf/util.c:
  (svn_ra_serf__handle_multistatus_only): rename param to SCRATCH_POOL.
    switch baton type to handler_t. switch error processing over
    HANDLER->SERVER_ERROR. tell the XML parser about the correct DONE
    flag.
  (handle_response_cb): clarify the param by renaming to SCRATCH_POOL

* subversion/libsvn_ra_serf/commit.c:
  (proppatch_resource): adjust response handler baton. switch to
    context_run_one(). examine the correct server error.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/commit.c
    subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
    subversion/trunk/subversion/libsvn_ra_serf/util.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1334720&r1=1334719&r2=1334720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Sun May  6 17:53:15 2012
@@ -950,21 +950,21 @@ proppatch_resource(proppatch_context_t *
   handler->body_delegate_baton = &pbb;
 
   handler->response_handler = svn_ra_serf__handle_multistatus_only;
-  handler->response_baton = &proppatch->progress;
+  handler->response_baton = handler;
 
-  svn_ra_serf__request_create(handler);
-
-  /* If we don't wait for the response, our pool will be gone! */
-  SVN_ERR(svn_ra_serf__context_run_wait(&proppatch->progress.done,
-                                        commit->session, pool));
+  SVN_ERR(svn_ra_serf__context_run_one(handler, pool));
 
   if (handler->sline.code != 207
-      || proppatch->progress.server_error.error)
+      || (handler->server_error != NULL
+          && handler->server_error->error != NULL))
     {
       return svn_error_create(
                SVN_ERR_RA_DAV_PROPPATCH_FAILED,
                return_response_err(handler,
-                                   proppatch->progress.server_error.error),
+                                   handler->server_error != NULL
+                                     ? handler->server_error->error
+                                     : SVN_NO_ERROR
+                                   ),
                _("At least one property change failed; repository"
                  " is unchanged"));
     }

Modified: subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h?rev=1334720&r1=1334719&r2=1334720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Sun May  6 17:53:15 2012
@@ -362,7 +362,7 @@ typedef svn_error_t *
 (*svn_ra_serf__response_handler_t)(serf_request_t *request,
                                    serf_bucket_t *response,
                                    void *handler_baton,
-                                   apr_pool_t *pool);
+                                   apr_pool_t *scratch_pool);
 
 /* Callback for when a request body is needed. */
 /* ### should pass a scratch_pool  */
@@ -701,7 +701,6 @@ svn_ra_serf__handle_discard_body(serf_re
 /*
  * Handler that retrieves the embedded XML error response from the
  * the @a response body associated with a @a request.
- * Implements svn_ra_serf__response_handler_t.
  *
  * All temporary allocations will be made in a @a pool.
  */
@@ -712,18 +711,20 @@ svn_ra_serf__handle_server_error(serf_re
 
 /*
  * Handler that retrieves the embedded XML multistatus response from the
- * the @a RESPONSE body associated with a @a REQUEST. *DONE is set to TRUE.
+ * the @a RESPONSE body associated with a @a REQUEST.
+ *
  * Implements svn_ra_serf__response_handler_t.
  *
- * The @a BATON should be of type svn_ra_serf__simple_request_context_t.
+ * The @a BATON should be of type svn_ra_serf__handler_t. When the request
+ * is complete, the handler's DONE flag will be set to TRUE.
  *
- * All temporary allocations will be made in a @a pool.
+ * All temporary allocations will be made in a @a scratch_pool.
  */
 svn_error_t *
 svn_ra_serf__handle_multistatus_only(serf_request_t *request,
                                      serf_bucket_t *response,
                                      void *baton,
-                                     apr_pool_t *pool);
+                                     apr_pool_t *scratch_pool);
 
 /*
  * This function will feed the RESPONSE body into XMLP.  When parsing is

Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1334720&r1=1334719&r2=1334720&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Sun May  6 17:53:15 2012
@@ -1105,21 +1105,26 @@ svn_error_t *
 svn_ra_serf__handle_multistatus_only(serf_request_t *request,
                                      serf_bucket_t *response,
                                      void *baton,
-                                     apr_pool_t *pool)
+                                     apr_pool_t *scratch_pool)
 {
-  svn_error_t *err;
-  svn_ra_serf__simple_request_context_t *ctx = baton;
-  svn_ra_serf__server_error_t *server_err = &ctx->server_error;
+  svn_ra_serf__handler_t *handler = baton;
 
-  SVN_ERR_ASSERT(ctx->pool);
+  /* This response handler requires a pool for the server error.  */
+  SVN_ERR_ASSERT(handler->handler_pool);
 
   /* If necessary, initialize our XML parser. */
-  if (!server_err->init)
+  if (handler->server_error == NULL)
     {
+      svn_ra_serf__server_error_t *server_err;
       serf_bucket_t *hdrs;
       const char *val;
 
+      /* ### it would be nice to avoid allocating this every time. we
+         ### could potentially have a flag indicating we have examined
+         ### the Content-Type header already.  */
+      server_err = apr_pcalloc(handler->handler_pool, sizeof(*server_err));
       server_err->init = TRUE;
+
       hdrs = serf_bucket_response_get_headers(response);
       val = serf_bucket_headers_get(hdrs, "Content-Type");
       if (val && strncasecmp(val, "text/xml", sizeof("text/xml") - 1) == 0)
@@ -1137,22 +1142,31 @@ svn_ra_serf__handle_multistatus_only(ser
           server_err->parser.ignore_errors = TRUE;
 
           /* Get the parser to set our DONE flag.  */
-          server_err->parser.done = &ctx->done;
+          server_err->parser.done = &handler->done;
         }
       else
         {
-          ctx->done = TRUE;
+          /* ### hmm. this is a bit early. we have not seen EOF. if the
+             ### caller thinks we are "done", then it may never call into
+             ### serf_context_run() again to flush the response.  */
+          handler->done = TRUE;
           server_err->error = SVN_NO_ERROR;
         }
+
+      handler->server_error = server_err;
     }
 
   /* If server_err->error still contains APR_SUCCESS, it means that we
      have not successfully parsed the XML yet. */
-  if (server_err->error
-      && server_err->error->apr_err == APR_SUCCESS)
+  if (handler->server_error
+      && handler->server_error->error
+      && handler->server_error->error->apr_err == APR_SUCCESS)
     {
+      svn_error_t *err;
+
       err = svn_ra_serf__handle_xml_parser(request, response,
-                                           &server_err->parser, pool);
+                                           &handler->server_error->parser,
+                                           scratch_pool);
 
       /* APR_EOF will be returned when parsing is complete.  If we see
          any other error, return it immediately.  In practice the only
@@ -1163,17 +1177,26 @@ svn_ra_serf__handle_multistatus_only(ser
         {
           return svn_error_trace(err);
         }
-      else if (ctx->done && server_err->error->apr_err == APR_SUCCESS)
+      else if (handler->done
+               && handler->server_error->error->apr_err == APR_SUCCESS)
         {
-          svn_error_clear(server_err->error);
-          server_err->error = SVN_NO_ERROR;
+          svn_error_clear(handler->server_error->error);
+          handler->server_error->error = SVN_NO_ERROR;
+
+          /* ### it would be nice to do this, but if we enter this response
+             ### handler again, it would be re-created. this throws back to
+             ### the idea of a flag determining whether we haved looked for
+             ### a server error.  */
+#if 0
+          handler->server_error = NULL;
+#endif
         }
 
       svn_error_clear(err);
     }
 
   return svn_error_trace(svn_ra_serf__handle_discard_body(
-                           request, response, NULL, pool));
+                           request, response, NULL, scratch_pool));
 }
 
 
@@ -1794,7 +1817,7 @@ static apr_status_t
 handle_response_cb(serf_request_t *request,
                    serf_bucket_t *response,
                    void *baton,
-                   apr_pool_t *pool)
+                   apr_pool_t *scratch_pool)
 {
   svn_ra_serf__handler_t *handler = baton;
   svn_ra_serf__session_t *session = handler->session;
@@ -1803,7 +1826,8 @@ handle_response_cb(serf_request_t *reque
   apr_status_t outer_status;
 
   err = svn_error_trace(handle_response(request, response,
-                                        handler, &inner_status, pool));
+                                        handler, &inner_status,
+                                        scratch_pool));
 
   outer_status = save_error(session, err);
   return outer_status ? outer_status : inner_status;