You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by cm...@apache.org on 2012/08/30 15:29:37 UTC

svn commit: r1378927 - in /subversion/trunk/subversion: libsvn_ra_serf/update.c mod_dav_svn/reports/update.c

Author: cmpilato
Date: Thu Aug 30 13:29:37 2012
New Revision: 1378927

URL: http://svn.apache.org/viewvc?rev=1378927&view=rev
Log:
Allow DAV clients to request that the server transmit all property
changes inline even when not in send-all mode, and teach ra_serf to do
so.

* subversion/mod_dav_svn/reports/update.c
  (update_ctx_t): Add 'include_props' member.
  (item_baton_t): Add 'fetch_props' member.
  (close_helper): Add 'pool' parameter, and do property name
    XML-quoting here.  If the item's 'fetch_props' flag is set,
    transmit this fact to the client.
  (maybe_start_update_report): Include "inline-prop=true" attribute in
    the report response tag if the client so requested that
    functionality.
  (send_propchange): New helper function.
  (upd_change_xxx_prop): Don't XML-quote property names here.  Rework
    logic for comprehensibility, making use of send_propchange().
  (upd_close_directory, upd_close_file): Update calls to close_helper().
  (dav_svn__update_report): Set 'include_props' when in send-all mode,
    and also when the client explicitly requests that props be included
    inline.

* subversion/libsvn_ra_serf/update.c
  (report_context_t): New 'add_props_included' member.
  (start_report): Look for "inline-props=true" on the report response
    tag, setting add_props_included to TRUE if found.  Only set
    fetch_props for added files and directories if add_props_included
    isn't set.
  (make_update_reporter): Add "<S:include-props>yes</S:include-props>"
    XML tag to the REPORT request.

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/update.c
    subversion/trunk/subversion/mod_dav_svn/reports/update.c

Modified: subversion/trunk/subversion/libsvn_ra_serf/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/update.c?rev=1378927&r1=1378926&r2=1378927&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/update.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/update.c Thu Aug 30 13:29:37 2012
@@ -313,6 +313,10 @@ struct report_context_t {
   /* Do we want the server to send copyfrom args or not? */
   svn_boolean_t send_copyfrom_args;
 
+  /* Is the server including properties inline for newly added
+     files/dirs? */
+  svn_boolean_t add_props_included;
+
   /* Path -> lock token mapping. */
   apr_hash_t *lock_path_tokens;
 
@@ -1391,7 +1395,13 @@ start_report(svn_ra_serf__xml_parser_t *
 
   state = parser->state->current_state;
 
-  if (state == NONE && strcmp(name.name, "target-revision") == 0)
+  if (state == NONE && strcmp(name.name, "update-report") == 0)
+    {
+      const char *val = svn_xml_get_attr_value("inline-props", attrs);
+      if (val && (strcmp(val, "true") == 0))
+        ctx->add_props_included = TRUE;
+    }
+  else if (state == NONE && strcmp(name.name, "target-revision") == 0)
     {
       const char *rev;
 
@@ -1531,7 +1541,11 @@ start_report(svn_ra_serf__xml_parser_t *
       /* Mark that we don't have a base. */
       info->base_rev = SVN_INVALID_REVNUM;
       dir->base_rev = info->base_rev;
-      dir->fetch_props = TRUE;
+
+      /* If the server isn't included properties for added items,
+         we'll need to fetch them ourselves. */
+      if (! ctx->add_props_included)
+        dir->fetch_props = TRUE;
 
       dir->repos_relpath = svn_relpath_join(dir->parent_dir->repos_relpath,
                                             dir->base_name, dir->pool);
@@ -1588,9 +1602,13 @@ start_report(svn_ra_serf__xml_parser_t *
       info = push_state(parser, ctx, ADD_FILE);
 
       info->base_rev = SVN_INVALID_REVNUM;
-      info->fetch_props = TRUE;
       info->fetch_file = TRUE;
 
+      /* If the server isn't included properties for added items,
+         we'll need to fetch them ourselves. */
+      if (! ctx->add_props_included)
+        info->fetch_props = TRUE;
+
       info->base_name = apr_pstrdup(info->pool, file_name);
       info->name = NULL;
 
@@ -1906,7 +1924,7 @@ end_report(svn_ra_serf__xml_parser_t *pa
       /* At this point, we should have the checked-in href.
        * If needed, create the PROPFIND to retrieve the dir's properties.
        */
-      if (!SVN_IS_VALID_REVNUM(info->dir->base_rev) || info->dir->fetch_props)
+      if (info->dir->fetch_props)
         {
           /* Unconditionally set fetch_props now. */
           info->dir->fetch_props = TRUE;
@@ -2785,6 +2803,10 @@ make_update_reporter(svn_ra_session_t *r
       make_simple_xml_tag(&buf, "S:recursive", "no", scratch_pool);
     }
 
+  /* Subversion 1.8+ servers can be told to send properties for newly
+     added items inline even when doing a skelta response. */
+  make_simple_xml_tag(&buf, "S:include-props", "yes", scratch_pool);
+
   make_simple_xml_tag(&buf, "S:depth", svn_depth_to_word(depth), scratch_pool);
 
   SVN_ERR(svn_io_file_write_full(report->body_file, buf->data, buf->len,

Modified: subversion/trunk/subversion/mod_dav_svn/reports/update.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/reports/update.c?rev=1378927&r1=1378926&r2=1378927&view=diff
==============================================================================
--- subversion/trunk/subversion/mod_dav_svn/reports/update.c (original)
+++ subversion/trunk/subversion/mod_dav_svn/reports/update.c Thu Aug 30 13:29:37 2012
@@ -81,6 +81,10 @@ typedef struct update_ctx_t {
   /* True iff client requested all data inline in the report. */
   svn_boolean_t send_all;
 
+  /* True iff client requested that properties be transmitted
+     inline.  (This is implied when "send_all" is set.)  */
+  svn_boolean_t include_props;
+
   /* SVNDIFF version to send to client.  */
   int svndiff_version;
 
@@ -119,6 +123,10 @@ typedef struct item_baton_t {
   /* File/dir copied? */
   svn_boolean_t copyfrom;
 
+  /* Does the client need to fetch additional properties for this
+     item? */
+  svn_boolean_t fetch_props;
+
   /* Array of const char * names of removed properties.  (Used only
      for copied files/dirs in skelta mode.)  */
   apr_array_header_t *removed_props;
@@ -432,7 +440,7 @@ open_helper(svn_boolean_t is_dir,
 
 
 static svn_error_t *
-close_helper(svn_boolean_t is_dir, item_baton_t *baton)
+close_helper(svn_boolean_t is_dir, item_baton_t *baton, apr_pool_t *pool)
 {
   if (baton->uc->resource_walk)
     return SVN_NO_ERROR;
@@ -446,14 +454,21 @@ close_helper(svn_boolean_t is_dir, item_
 
       for (i = 0; i < baton->removed_props->nelts; i++)
         {
-          /* We already XML-escaped the property name in change_xxx_prop. */
           qname = APR_ARRAY_IDX(baton->removed_props, i, const char *);
+          qname = apr_xml_quote_string(pool, qname, 1);
           SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output,
                                           "<S:remove-prop name=\"%s\"/>"
                                           DEBUG_CR, qname));
         }
     }
 
+  /* If our client need to fetch properties, let it know. */
+  if (baton->fetch_props)
+    SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output,
+                                    "<S:fetch-props/>" DEBUG_CR));
+    
+
+  /* Let's tie it off, nurse. */
   if (baton->added)
     SVN_ERR(dav_svn__brigade_printf(baton->uc->bb, baton->uc->output,
                                     "</S:add-%s>" DEBUG_CR,
@@ -473,13 +488,13 @@ maybe_start_update_report(update_ctx_t *
 {
   if ((! uc->resource_walk) && (! uc->started_update))
     {
-      SVN_ERR(dav_svn__brigade_printf(uc->bb, uc->output,
-                                      DAV_XML_HEADER DEBUG_CR
-                                      "<S:update-report xmlns:S=\""
-                                      SVN_XML_NAMESPACE "\" "
-                                      "xmlns:V=\"" SVN_DAV_PROP_NS_DAV "\" "
-                                      "xmlns:D=\"DAV:\" %s>" DEBUG_CR,
-                                      uc->send_all ? "send-all=\"true\"" : ""));
+      SVN_ERR(dav_svn__brigade_printf(
+                  uc->bb, uc->output,
+                  DAV_XML_HEADER DEBUG_CR "<S:update-report xmlns:S=\""
+                  SVN_XML_NAMESPACE "\" xmlns:V=\"" SVN_DAV_PROP_NS_DAV "\" "
+                  "xmlns:D=\"DAV:\" %s %s>" DEBUG_CR,
+                  uc->send_all ? "send-all=\"true\"" : "",
+                  uc->include_props ? "inline-props=\"true\"" : ""));
 
       uc->started_update = TRUE;
     }
@@ -593,87 +608,117 @@ upd_open_directory(const char *path,
 
 
 static svn_error_t *
+send_propchange(item_baton_t *b,
+                const char *name,
+                const svn_string_t *value,
+                apr_pool_t *pool)
+{
+  const char *qname;
+
+  /* Ensure that the property name is XML-safe.
+     apr_xml_quote_string() doesn't realloc if there is nothing to
+     quote, so dup the name, but only if necessary. */
+  qname = apr_xml_quote_string(b->pool, name, 1);
+  if (qname == name)
+    qname = apr_pstrdup(b->pool, name);
+
+  if (value)
+    {
+      const char *qval;
+
+      if (svn_xml_is_xml_safe(value->data, value->len))
+        {
+          svn_stringbuf_t *tmp = NULL;
+          svn_xml_escape_cdata_string(&tmp, value, pool);
+          qval = tmp->data;
+          SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output,
+                                          "<S:set-prop name=\"%s\">",
+                                          qname));
+        }
+      else
+        {
+          qval = svn_base64_encode_string2(value, TRUE, pool)->data;
+          SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output,
+                                          "<S:set-prop name=\"%s\" "
+                                          "encoding=\"base64\">" DEBUG_CR,
+                                          qname));
+        }
+
+      SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output, qval));
+      SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output,
+                                    "</S:set-prop>" DEBUG_CR));
+    }
+  else  /* value is null, so this is a prop removal */
+    {
+      SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output,
+                                      "<S:remove-prop name=\"%s\"/>"
+                                      DEBUG_CR,
+                                      qname));
+    }
+  
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
 upd_change_xxx_prop(void *baton,
                     const char *name,
                     const svn_string_t *value,
                     apr_pool_t *pool)
 {
   item_baton_t *b = baton;
-  const char *qname;
 
   /* Resource walks say nothing about props. */
   if (b->uc->resource_walk)
     return SVN_NO_ERROR;
 
-  /* Else this not a resource walk, so either send props or cache them
-     to send later, depending on whether this is a modern report
-     response or not. */
-
-  qname = apr_xml_quote_string(b->pool, name, 1);
-
-  /* apr_xml_quote_string doesn't realloc if there is nothing to
-     quote, so dup the name, but only if necessary. */
-  if (qname == name)
-    qname = apr_pstrdup(b->pool, name);
+  /* If we get here, this not a resource walk, so either send props or
+     cache them to send later, depending on whether this is a modern
+     report response or not. */
 
   /* Even if we are not in send-all mode we have the prop changes already,
      so send them to the client now instead of telling the client to fetch
      them later. */
-  if (b->uc->send_all || !b->added)
+  if (b->uc->send_all)
     {
-      if (value)
+      SVN_ERR(send_propchange(b, name, value, pool));
+    }
+  else
+    {
+      if (b->added)
         {
-          const char *qval;
-
-          if (svn_xml_is_xml_safe(value->data, value->len))
+          /* This is an addition in "skelta" (that is, "not send-all")
+             mode so there is no strict need for an inline response.
+             Clients will assume that added objects need all to have
+             all their properties explicitly fetched from the
+             server. */
+
+          /* That said, beginning in Subversion 1.8, clients might
+             request even in skelta mode that we transmit properties
+             on newly added files explicitly. */
+          if ((! b->copyfrom) && value && b->uc->include_props)
             {
-              svn_stringbuf_t *tmp = NULL;
-              svn_xml_escape_cdata_string(&tmp, value, pool);
-              qval = tmp->data;
-              SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output,
-                                              "<S:set-prop name=\"%s\">",
-                                              qname));
+              SVN_ERR(send_propchange(b, name, value, pool));
             }
-          else
+
+          /* Now, if the object is actually a copy and this is a
+             property removal, we'll still need to cache (and later
+             transmit) property removals, because fetching the
+             object's current property set alone isn't sufficient to
+             communicate the fact that additional properties were, in
+             fact, removed from the copied base object in order to
+             arrive at that set. */
+          if (b->copyfrom && (! value))
             {
-              qval = svn_base64_encode_string2(value, TRUE, pool)->data;
-              SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output,
-                                              "<S:set-prop name=\"%s\" "
-                                              "encoding=\"base64\">" DEBUG_CR,
-                                              qname));
+              if (! b->removed_props)
+                b->removed_props = apr_array_make(b->pool, 1, sizeof(name));
+              
+              APR_ARRAY_PUSH(b->removed_props, const char *) = name;
             }
-
-          SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output, qval));
-          SVN_ERR(dav_svn__brigade_puts(b->uc->bb, b->uc->output,
-                                        "</S:set-prop>" DEBUG_CR));
         }
-      else  /* value is null, so this is a prop removal */
-        {
-          SVN_ERR(dav_svn__brigade_printf(b->uc->bb, b->uc->output,
-                                          "<S:remove-prop name=\"%s\"/>"
-                                          DEBUG_CR,
-                                          qname));
-        }
-    }
-  else if (!value)
-    {
-      /* This is an addition in "skelta" (that is, "not send-all")
-         mode so there is no strict need for an inline response.
-         Clients will assume that added objects need all to have all
-         their properties explicitly fetched from the server. */
-
-      /* Now, if the object is actually a copy, we'll still need to
-         cache (and later transmit) property removals, because
-         fetching the object's current property set alone isn't
-         sufficient to communicate the fact that additional properties
-         were, in fact, removed from the copied base object in order
-         to arrive at that set. */
-      if (b->copyfrom)
+      else
         {
-          if (! b->removed_props)
-            b->removed_props = apr_array_make(b->pool, 1, sizeof(name));
-
-          APR_ARRAY_PUSH(b->removed_props, const char *) = qname;
+          /* "skelta" mode non-addition.  Just send the change. */
+          SVN_ERR(send_propchange(b, name, value, pool));
         }
     }
 
@@ -684,7 +729,7 @@ upd_change_xxx_prop(void *baton,
 static svn_error_t *
 upd_close_directory(void *dir_baton, apr_pool_t *pool)
 {
-  return close_helper(TRUE /* is_dir */, dir_baton);
+  return close_helper(TRUE /* is_dir */, dir_baton, pool);
 }
 
 
@@ -845,7 +890,7 @@ upd_close_file(void *file_baton, const c
                                       text_checksum));
     }
 
-  return close_helper(FALSE /* is_dir */, file);
+  return close_helper(FALSE /* is_dir */, file, pool);
 }
 
 
@@ -944,6 +989,7 @@ dav_svn__update_report(const dav_resourc
               && (strcmp(this_attr->value, "true") == 0))
             {
               uc.send_all = TRUE;
+              uc.include_props = TRUE;
               break;
             }
         }
@@ -1066,6 +1112,14 @@ dav_svn__update_report(const dav_resourc
           if (strcmp(cdata, "no") == 0)
             text_deltas = FALSE;
         }
+      if (child->ns == ns && strcmp(child->name, "include-props") == 0)
+        {
+          cdata = dav_xml_get_cdata(child, resource->pool, 1);
+          if (! *cdata)
+            return malformed_element_error(child->name, resource->pool);
+          if (strcmp(cdata, "no") != 0)
+            uc.include_props = TRUE;
+        }
     }
 
   if (!saw_depth && !saw_recursive && (requested_depth == svn_depth_unknown))