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/06/01 22:30:24 UTC

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

Author: gstein
Date: Fri Jun  1 20:30:24 2012
New Revision: 1345338

URL: http://svn.apache.org/viewvc?rev=1345338&view=rev
Log:
Move towards getting rid of svn_ra_serf__handle_discard_body(). We
don't need a special response handler since the core handler supports
dicarding bodies (with the HANDLER->DISCARD_BODY flag).

Two callers are in util.c: only one caller used the server_error
stuff, so move the logic there. The other caller used just the simple
drain_bucket() call, so it now calls it directly.

The remaining caller in update.c can likely switch over easily, but is
not considered in this revision.

* subversion/libsvn_ra_serf/ra_serf.h:
  (svn_ra_serf__server_error_t): remove INIT and HAS_XML_RESPONSE
    fields, as they are no longer needed.

* subversion/libsvn_ra_serf/util.c:
  (begin_error_parsing): don't worry about initializing INIT and
    HAS_XML_RESPONSE. they are a given now, and have been removed.
  (svn_ra_serf__handle_discard_body): remove the entire section
    dealing with a SERVER_ERR in the baton. only one caller ever did
    that, and it now handles the server error on its own.
  (handle_server_error): lift in all the SERVER_ERR processing from
    the above handle_discard_body() function. rejigger for a local
    variable and to adjust some of the conditional blocks for clarity.
    this function is still conceptually wrong, but this helps to
    isolate the wrongness.
  (svn_ra_serf__handle_xml_parser): just call drain_bucket() rather
    than the response handler which wraps that call.

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

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=1345338&r1=1345337&r2=1345338&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/ra_serf.h Fri Jun  1 20:30:24 2012
@@ -769,12 +769,6 @@ struct svn_ra_serf__server_error_t {
   /* Our local representation of the error. */
   svn_error_t *error;
 
-  /* Have we checked to see if there's an XML error in this response? */
-  svn_boolean_t init;
-
-  /* Was there an XML error response? */
-  svn_boolean_t has_xml_response;
-
   /* Are we done with the response? */
   svn_boolean_t done;
 

Modified: subversion/trunk/subversion/libsvn_ra_serf/util.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/util.c?rev=1345338&r1=1345337&r2=1345338&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/util.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/util.c Fri Jun  1 20:30:24 2012
@@ -902,9 +902,7 @@ begin_error_parsing(svn_ra_serf__xml_sta
   svn_ra_serf__server_error_t *server_err;
 
   server_err = apr_pcalloc(result_pool, sizeof(*server_err));
-  server_err->init = TRUE;
   server_err->error = svn_error_create(APR_SUCCESS, NULL, NULL);
-  server_err->has_xml_response = TRUE;
   server_err->contains_precondition_error = FALSE;
   server_err->cdata = svn_stringbuf_create_empty(server_err->error->pool);
   server_err->collect_cdata = FALSE;
@@ -926,59 +924,6 @@ svn_ra_serf__handle_discard_body(serf_re
                                  apr_pool_t *pool)
 {
   apr_status_t status;
-  svn_ra_serf__server_error_t *server_err = baton;
-
-  if (server_err)
-    {
-      if (!server_err->init)
-        {
-          serf_bucket_t *hdrs;
-          const char *val;
-
-          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)
-            {
-              /* ### we should figure out how to reuse begin_error_parsing  */
-
-              server_err->error = svn_error_create(APR_SUCCESS, NULL, NULL);
-              server_err->has_xml_response = TRUE;
-              server_err->contains_precondition_error = FALSE;
-              server_err->cdata = svn_stringbuf_create_empty(pool);
-              server_err->collect_cdata = FALSE;
-              server_err->parser.pool = server_err->error->pool;
-              server_err->parser.user_data = server_err;
-              server_err->parser.start = start_error;
-              server_err->parser.end = end_error;
-              server_err->parser.cdata = cdata_error;
-              server_err->parser.done = &server_err->done;
-              server_err->parser.ignore_errors = TRUE;
-            }
-          else
-            {
-              server_err->error = SVN_NO_ERROR;
-            }
-        }
-
-      if (server_err->has_xml_response)
-        {
-          svn_error_t *err = svn_ra_serf__handle_xml_parser(
-                                                        request,
-                                                        response,
-                                                        &server_err->parser,
-                                                        pool);
-
-          if (server_err->done && server_err->error->apr_err == APR_SUCCESS)
-            {
-              svn_error_clear(server_err->error);
-              server_err->error = SVN_NO_ERROR;
-            }
-
-          return svn_error_trace(err);
-        }
-
-    }
 
   status = drain_bucket(response);
   if (status)
@@ -1457,11 +1402,51 @@ handle_server_error(serf_request_t *requ
                     apr_pool_t *scratch_pool)
 {
   svn_ra_serf__server_error_t server_err = { 0 };
+  serf_bucket_t *hdrs;
+  const char *val;
 
-  svn_error_clear(svn_ra_serf__handle_discard_body(request, response,
-                                                   &server_err, scratch_pool));
+  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)
+    {
+      /* ### we should figure out how to reuse begin_error_parsing  */
 
-  return server_err.error;
+      server_err.error = svn_error_create(APR_SUCCESS, NULL, NULL);
+      server_err.contains_precondition_error = FALSE;
+      server_err.cdata = svn_stringbuf_create_empty(scratch_pool);
+      server_err.collect_cdata = FALSE;
+      server_err.parser.pool = server_err.error->pool;
+      server_err.parser.user_data = &server_err;
+      server_err.parser.start = start_error;
+      server_err.parser.end = end_error;
+      server_err.parser.cdata = cdata_error;
+      server_err.parser.done = &server_err.done;
+      server_err.parser.ignore_errors = TRUE;
+
+      /* We don't care about any errors except for SERVER_ERR.ERROR  */
+      svn_error_clear(svn_ra_serf__handle_xml_parser(request,
+                                                     response,
+                                                     &server_err.parser,
+                                                     scratch_pool));
+
+      /* ### checking DONE is silly. the above only parses whatever has
+         ### been received at the network interface. totally wrong. but
+         ### it is what we have for now (maintaining historical code),
+         ### until we fully migrate.  */
+      if (server_err.done && server_err.error->apr_err == APR_SUCCESS)
+        {
+          svn_error_clear(server_err.error);
+          server_err.error = SVN_NO_ERROR;
+        }
+
+      return svn_error_trace(server_err.error);
+    }
+
+  /* The only error that we will return is from the XML response body.
+     Otherwise, ignore the entire body and return success.  */
+  (void) drain_bucket(response);
+
+  return SVN_NO_ERROR;
 }
 
 
@@ -1491,10 +1476,14 @@ svn_ra_serf__handle_xml_parser(serf_requ
 
       err = handle_server_error(request, response, pool);
 
-      SVN_ERR(svn_error_compose_create(
-        svn_ra_serf__handle_discard_body(request, response, NULL, pool),
-        err));
-      return SVN_NO_ERROR;
+      /* ### the above call should have drained the entire response. this
+         ### call is historical, and probably not required. but during
+         ### the rework of this core handling... let's keep it for now.  */
+      status = drain_bucket(response);
+      if (status)
+        err = svn_error_compose_create(svn_error_wrap_apr(status, NULL),
+                                       err);
+      return svn_error_trace(err);
     }
 
   if (ctx->headers_baton == NULL)