You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/08/07 19:45:39 UTC

svn commit: r983269 - in /subversion/branches/atomic-revprop: ./ notes/http-and-webdav/ subversion/include/private/ subversion/libsvn_ra_neon/ subversion/libsvn_ra_serf/ subversion/mod_dav_svn/

Author: danielsh
Date: Sat Aug  7 17:45:38 2010
New Revision: 983269

URL: http://svn.apache.org/viewvc?rev=983269&view=rev
Log:
On the 'atomic-revprop' branch:

Implement the API over ra_dav.


First, some general infrastructure:

* notes/http-and-webdav/webdav-protocol
  (PROPPATCH):  Document the protocol extension.

* subversion/include/private/svn_dav_protocol.h
  (SVN_DAV__OLD_VALUE, SVN_DAV__OLD_VALUE__ABSENT):  New macros.
  (svn_dav__two_props_t):  New helper typedef.


Then, mod_dav_svn support:

* subversion/mod_dav_svn/deadprops.c
  (save_value):
    Grow and use an OLD_VALUE_P parameter, passing it
    to svn_repos_fs_change_rev_prop4().
  (decode_property_value):
    Grow an ABSENT out parameter, and parse the (new) 'V:absent' attribute.
  (db_store):
    Parse <V:old-value/> and 'V:absent' and pass OLD_VALUE_P to save_value().


ra_serf support:

* subversion/libsvn_ra_serf/ra_serf.h
  (svn_ra_serf__walk_all_props, svn_ra_serf__set_ver_prop):
    Remove "not implemented" markers from docstrings.

* subversion/libsvn_ra_serf/property.c
  (svn_ra_serf__set_ver_prop):
    Store an svn_dav__two_props_t in the hash if an OLD_VALUE_P was provided.
  (svn_ra_serf__walk_all_props):
    Support the VALUES_ARE_PROPPAIRS parameter when calling the walker.

* subversion/libsvn_ra_serf/commit.c
  (proppatch_context_t):  Add 'atomic_props' member.
  (get_encoding_and_cdata):
    Refactor to return a string rather than a bucket,
    and to support a NULL value.
  (proppatch_walker):
    Track get_encoding_and_cdata() signature change.
    Set 'V:absent' and <V:old-value/> as appropriate.
  (create_proppatch_body):
    Walk/Iterate CTX->ATOMIC_PROPS too.
  (svn_ra_serf__change_rev_prop):
    Remove 'not implemented' sentinel.  Initialize CTX->ATOMIC_PROPS.
    Pass OLD_VALUE_P to svn_ra_serf__set_prop() (which passes it
    to svn_ra_serf__set_ver_prop()).


ra_neon support:

* subversion/libsvn_ra_neon/ra_neon.h
  (svn_ra_neon__do_proppatch):
    Add PROP_OLD_VALUES parameter.

* subversion/libsvn_ra_neon/fetch.c
  (svn_ra_neon__change_rev_prop):
    Remove 'not implemented' sentinel.
    Pass OLD_VALUE_P to svn_ra_neon__do_proppatch() via PROP_OLD_VALUES.

* subversion/libsvn_ra_neon/props.c
  (append_setprop):
    Grow OLD_VALUE_P parameter.  Use it to 
    generate 'V:absent' and <V:old-value/> as appropriate.
  (svn_ra_neon__do_proppatch):
    Grow PROP_OLD_VALUES parameter, and use it.
  

* subversion/libsvn_ra_neon/commit.c
  (do_proppatch, apply_revprops):
    Track signature change of svn_ra_neon__do_proppatch().


Finally:

* BRANCH-README:  Mark this as done.

Modified:
    subversion/branches/atomic-revprop/BRANCH-README
    subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol
    subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h
    subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c
    subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c
    subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c
    subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h
    subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c
    subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c
    subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h
    subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c

Modified: subversion/branches/atomic-revprop/BRANCH-README
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/BRANCH-README?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/BRANCH-README (original)
+++ subversion/branches/atomic-revprop/BRANCH-README Sat Aug  7 17:45:38 2010
@@ -21,11 +21,7 @@ Planned work
       achieved with older servers, and because of planned protocol extensions)
   - [DONE] ra_local: Trivial.
   - [DONE] ra_svn: Add a 'change-rev-prop2' verb.
-  - ra_dav
-      Extend PROPPATCH syntax to provide the old value: add a <S:oldvalue/>
-      tag inside the <${propname}> tag.
-        Q: where to document this protocol extension?
-        A: notes/http-and-webdav/webdav-protocol
+  - [DONE] ra_dav: Extend PROPPATCH: see notes/http-and-webdav/webdav-protocol
   - [DONE] unit test
       prop_tests.py 34: atomic_over_ra()
   - TODO: extend it to test props set to the empty string and unset props

Modified: subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol (original)
+++ subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol Sat Aug  7 17:45:38 2010
@@ -177,6 +177,30 @@ Response:
   
   ...svn-svndiff stream that can be passed to svn_txdelta_parse_svndiff...
 
+PROPPATCH
+=========
+
+We extend PROPPATCH as follows.  To pass OLD_VALUE_P (as in
+svn_ra_change_rev_prop2()), any propchange which is accompanied by a non-NULL
+OLD_VALUE_P goes within the <D:set><D:prop> tag (and never within the
+<D:remove><D:prop> tag --- even if it is a propdel).  Consequently, in
+mod_dav_svn it would land in db_store() and not db_remove().
+
+The property tag (in the C: or S: namespace) always contains the propval in its
+cdata (potentially base64-encoded).  The extension is as follows:
+
+* The property tag grows a V:absent attribute, to represent that the property
+  is being removed (i.e., a propdel routed to <D:set><D:prop>).
+
+* A <V:old-value> tag may be nested within the property tag.  The nested tag
+  supports the same V:absent and V:encoding attributed as the parent (property)
+  tag.
+
+Historical note: we route propdels via <D:set>/db_store() because the mod_dav
+API for db_remove() was insufficient.  See this thread:
+http://mid.gmane.org/4C531CFB.2010202@collab.net
+
+
 Custom REPORTs
 ==============
 

Modified: subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h (original)
+++ subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h Sat Aug  7 17:45:38 2010
@@ -46,6 +46,17 @@ extern "C" {
 #define SVN_DAV__INCLUDE_DESCENDANTS "include-descendants"
 #define SVN_DAV__VERSION_NAME "version-name"
 
+/** Names of XML elements attributes and tags for svn_ra_change_rev_prop2()'s
+    extension of PROPPATCH.  */
+#define SVN_DAV__OLD_VALUE "old-value"
+#define SVN_DAV__OLD_VALUE__ABSENT "absent"
+
+/** Helper typedef for svn_ra_change_rev_prop2() implementation. */
+typedef struct svn_dav__two_props_t {
+  const svn_string_t *const *old_value_p;
+  const svn_string_t *new_value;
+} svn_dav__two_props_t;
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c Sat Aug  7 17:45:38 2010
@@ -616,7 +616,8 @@ static svn_error_t * do_proppatch(svn_ra
     }
 
   return svn_ra_neon__do_proppatch(ras, url, rb->prop_changes,
-                                   rb->prop_deletes, extra_headers, pool);
+                                   rb->prop_deletes, NULL, extra_headers,
+                                   pool);
 }
 
 
@@ -1417,7 +1418,7 @@ static svn_error_t * apply_revprops(comm
     return err;
 
   return svn_ra_neon__do_proppatch(cc->ras, vcc_rsrc.wr_url, revprop_table,
-                                   NULL, NULL, pool);
+                                   NULL, NULL, NULL, pool);
 }
 
 svn_error_t * svn_ra_neon__get_commit_editor(svn_ra_session_t *session,

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c Sat Aug  7 17:45:38 2010
@@ -1131,6 +1131,7 @@ svn_error_t *svn_ra_neon__change_rev_pro
   svn_error_t *err;
   apr_hash_t *prop_changes = NULL;
   apr_array_header_t *prop_deletes = NULL;
+  apr_hash_t *prop_old_values = NULL;
   static const ne_propname wanted_props[] =
     {
       { "DAV:", "auto-version" },
@@ -1146,9 +1147,6 @@ svn_error_t *svn_ra_neon__change_rev_pro
 
       /* How did you get past the same check in svn_ra_change_rev_prop2()? */
       SVN_ERR_ASSERT(capable);
-
-      /* ### server-side support hasn't been implemented yet */
-      SVN__NOT_IMPLEMENTED();
     }
 
   /* Main objective: do a PROPPATCH (allprops) on a baseline object */
@@ -1180,19 +1178,33 @@ svn_error_t *svn_ra_neon__change_rev_pro
          to attempt the PROPPATCH if the deltaV server is going to do
          auto-versioning and create a new baseline! */
 
-  if (value)
+  if (old_value_p)
     {
-      prop_changes = apr_hash_make(pool);
-      apr_hash_set(prop_changes, name, APR_HASH_KEY_STRING, value);
+      svn_dav__two_props_t *both_values;
+
+      both_values = apr_palloc(pool, sizeof(*both_values));
+      both_values->old_value_p = old_value_p;
+      both_values->new_value = value;
+
+      prop_old_values = apr_hash_make(pool);
+      apr_hash_set(prop_old_values, name, APR_HASH_KEY_STRING, both_values);
     }
   else
     {
-      prop_deletes = apr_array_make(pool, 1, sizeof(const char *));
-      APR_ARRAY_PUSH(prop_deletes, const char *) = name;
+      if (value)
+        {
+          prop_changes = apr_hash_make(pool);
+          apr_hash_set(prop_changes, name, APR_HASH_KEY_STRING, value);
+        }
+      else
+        {
+          prop_deletes = apr_array_make(pool, 1, sizeof(const char *));
+          APR_ARRAY_PUSH(prop_deletes, const char *) = name;
+        }
     }
 
   err = svn_ra_neon__do_proppatch(ras, baseline->url, prop_changes,
-                                  prop_deletes, NULL, pool);
+                                  prop_deletes, prop_old_values, NULL, pool);
   if (err)
     return
       svn_error_create

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c Sat Aug  7 17:45:38 2010
@@ -1075,12 +1075,14 @@ get_encoding_and_cdata(const char **enco
 static svn_error_t *
 append_setprop(svn_stringbuf_t *body,
                const char *name,
+               const svn_string_t *const *old_value_p,
                const svn_string_t *value,
                apr_pool_t *pool)
 {
   const char *encoding;
   const char *xml_safe;
   const char *xml_tag_name;
+  const char *old_value_tag;
 
   /* Map property names to namespaces */
 #define NSLEN (sizeof(SVN_PROP_PREFIX) - 1)
@@ -1094,11 +1096,45 @@ append_setprop(svn_stringbuf_t *body,
       xml_tag_name = apr_pstrcat(pool, "C:", name, NULL);
     }
 
-  SVN_ERR(get_encoding_and_cdata(&encoding, &xml_safe, value, pool));
+  if (old_value_p)
+    {
+      if (*old_value_p)
+        {
+          const char *encoding2;
+          const char *xml_safe2;
+          SVN_ERR(get_encoding_and_cdata(&encoding2, &xml_safe2,
+                                         *old_value_p, pool));
+          old_value_tag = apr_psprintf(pool, "<%s %s>%s</%s>",
+                                       "V:" SVN_DAV__OLD_VALUE, encoding2,
+                                       xml_safe2, "V:" SVN_DAV__OLD_VALUE);
+        }
+      else
+        {
+#define OLD_VALUE_ABSENT_TAG \
+          "<" "V:" SVN_DAV__OLD_VALUE \
+              " V:" SVN_DAV__OLD_VALUE__ABSENT "=\"1\" " \
+          "/>"
+          old_value_tag = OLD_VALUE_ABSENT_TAG;
+        }
+    }
+  else
+    {
+      old_value_tag = "";
+    }
+
+  if (old_value_p && !value)
+    {
+      encoding = "V:" SVN_DAV__OLD_VALUE__ABSENT "=\"1\"" ;
+      xml_safe = "";
+    }
+  else
+    {
+      SVN_ERR(get_encoding_and_cdata(&encoding, &xml_safe, value, pool));
+    }
 
   svn_stringbuf_appendcstr(body,
-                           apr_psprintf(pool,"<%s %s>%s</%s>",
-                                        xml_tag_name, encoding,
+                           apr_psprintf(pool,"<%s %s>%s%s</%s>",
+                                        xml_tag_name, encoding, old_value_tag,
                                         xml_safe, xml_tag_name));
   return SVN_NO_ERROR;
 }
@@ -1109,6 +1145,7 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
                           const char *url,
                           apr_hash_t *prop_changes,
                           const apr_array_header_t *prop_deletes,
+                          apr_hash_t *prop_old_values,
                           apr_hash_t *extra_headers,
                           apr_pool_t *pool)
 {
@@ -1118,7 +1155,8 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
 
   /* just punt if there are no changes to make. */
   if ((prop_changes == NULL || (! apr_hash_count(prop_changes)))
-      && (prop_deletes == NULL || prop_deletes->nelts == 0))
+      && (prop_deletes == NULL || prop_deletes->nelts == 0)
+      && (prop_old_values == NULL || (! apr_hash_count(prop_old_values))))
     return SVN_NO_ERROR;
 
   /* easier to roll our own PROPPATCH here than use ne_proppatch(), which
@@ -1130,6 +1168,22 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
      SVN_DAV_PROP_NS_CUSTOM "\" xmlns:S=\""
      SVN_DAV_PROP_NS_SVN "\">" DEBUG_CR, pool);
 
+  /* Handle property changes/deletions with expected old values. */
+  if (prop_old_values)
+    {
+      apr_hash_index_t *hi;
+      svn_stringbuf_appendcstr(body, "<D:set><D:prop>");
+      for (hi = apr_hash_first(pool, prop_old_values); hi; hi = apr_hash_next(hi))
+        {
+          const char *name = svn__apr_hash_index_key(hi);
+          svn_dav__two_props_t *both_values = svn__apr_hash_index_val(hi);
+          svn_pool_clear(subpool);
+          SVN_ERR(append_setprop(body, name, both_values->old_value_p,
+                                 both_values->new_value, subpool));
+        }
+      svn_stringbuf_appendcstr(body, "</D:prop></D:set>");
+    }
+
   /* Handle property changes. */
   if (prop_changes)
     {
@@ -1141,7 +1195,7 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
           void *val;
           svn_pool_clear(subpool);
           apr_hash_this(hi, &key, NULL, &val);
-          SVN_ERR(append_setprop(body, key, val, subpool));
+          SVN_ERR(append_setprop(body, key, NULL, val, subpool));
         }
       svn_stringbuf_appendcstr(body, "</D:prop></D:set>");
     }
@@ -1155,7 +1209,7 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
         {
           const char *name = APR_ARRAY_IDX(prop_deletes, n, const char *);
           svn_pool_clear(subpool);
-          SVN_ERR(append_setprop(body, name, NULL, subpool));
+          SVN_ERR(append_setprop(body, name, NULL, NULL, subpool));
         }
       svn_stringbuf_appendcstr(body, "</D:prop></D:remove>");
     }

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h Sat Aug  7 17:45:38 2010
@@ -573,12 +573,15 @@ svn_error_t *svn_ra_neon__get_vcc(const 
 /* Issue a PROPPATCH request on URL, transmitting PROP_CHANGES (a hash
    of const svn_string_t * values keyed on Subversion user-visible
    property names) and PROP_DELETES (an array of property names to
-   delete).  Send any extra request headers in EXTRA_HEADERS. Use POOL
-   for all allocations.  */
+   delete). PROP_OLD_VALUES is a hash of Subversion user-visible property
+   names mapped to svn_dav__two_props_t * values. Send any extra
+   request headers in EXTRA_HEADERS. Use POOL for all allocations.
+ */
 svn_error_t *svn_ra_neon__do_proppatch(svn_ra_neon__session_t *ras,
                                        const char *url,
                                        apr_hash_t *prop_changes,
                                        const apr_array_header_t *prop_deletes,
+                                       apr_hash_t *prop_old_values,
                                        apr_hash_t *extra_headers,
                                        apr_pool_t *pool);
 

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c Sat Aug  7 17:45:38 2010
@@ -108,6 +108,7 @@ typedef struct {
   /* Changed and removed properties. */
   apr_hash_t *changed_props;
   apr_hash_t *removed_props;
+  apr_hash_t *atomic_props;
 
   /* In HTTP v2, this is the file/directory version we think we're changing. */
   svn_revnum_t base_revision;
@@ -639,16 +640,17 @@ checkout_file(file_context_t *file)
 /* Helper function for proppatch_walker() below. */
 static svn_error_t *
 get_encoding_and_cdata(const char **encoding_p,
-                       serf_bucket_t **cdata_bkt_p,
+                       const svn_string_t **encoded_value_p,
                        serf_bucket_alloc_t *alloc,
                        const svn_string_t *value,
                        apr_pool_t *pool)
 {
-  const char *encoding;
-  const char *cdata;
-  apr_size_t len; /* of cdata */
-
-  SVN_ERR_ASSERT(value);
+  if (value == NULL)
+    {
+      *encoding_p = NULL;
+      *encoded_value_p = NULL;
+      return SVN_NO_ERROR;
+    }
 
   /* If a property is XML-safe, XML-encode it.  Else, base64-encode
      it. */
@@ -656,23 +658,15 @@ get_encoding_and_cdata(const char **enco
     {
       svn_stringbuf_t *xml_esc = NULL;
       svn_xml_escape_cdata_string(&xml_esc, value, pool);
-      encoding = NULL;
-      cdata = xml_esc->data;
-      len = xml_esc->len;
+      *encoding_p = NULL;
+      *encoded_value_p = svn_string_create_from_buf(xml_esc, pool);
     }
   else
     {
-      const svn_string_t *base64ed = svn_base64_encode_string2(value, TRUE,
-                                                               pool);
-      encoding = "base64";
-      cdata = base64ed->data;
-      len = base64ed->len;
+      *encoding_p = "base64";
+      *encoded_value_p = svn_base64_encode_string2(value, TRUE, pool);
     }
 
-  /* ENCODING, CDATA, and LEN are now set. */
-
-  *encoding_p = encoding;
-  *cdata_bkt_p = SERF_BUCKET_SIMPLE_STRING_LEN(cdata, len, alloc);
   return SVN_NO_ERROR;
 }
 
@@ -680,13 +674,14 @@ static svn_error_t *
 proppatch_walker(void *baton,
                  const char *ns, apr_ssize_t ns_len,
                  const char *name, apr_ssize_t name_len,
-                 const svn_string_t *const *old_val_p, /* ### */
+                 const svn_string_t *const *old_val_p,
                  const svn_string_t *val,
                  apr_pool_t *pool)
 {
   serf_bucket_t *body_bkt = baton;
   serf_bucket_t *cdata_bkt;
   serf_bucket_alloc_t *alloc;
+  const svn_string_t *encoded_value;
   const char *encoding;
   char *prop_name;
 
@@ -700,12 +695,66 @@ proppatch_walker(void *baton,
 
   alloc = body_bkt->allocator;
 
-  SVN_ERR(get_encoding_and_cdata(&encoding, &cdata_bkt, alloc, val, pool));
+  SVN_ERR(get_encoding_and_cdata(&encoding, &encoded_value, alloc, val, pool));
+  if (encoded_value)
+    {
+      cdata_bkt = SERF_BUCKET_SIMPLE_STRING_LEN(encoded_value->data,
+                                                encoded_value->len,
+                                                alloc);
+    }
+  else
+    {
+      cdata_bkt = NULL;
+    }
+
+  if (cdata_bkt)
+    svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, prop_name,
+                                      "V:encoding", encoding,
+                                      NULL);
+  else
+    svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, prop_name,
+                                      "V:" SVN_DAV__OLD_VALUE__ABSENT, "1",
+                                      NULL);
 
-  svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, prop_name,
-                                    "V:encoding", encoding,
-                                    NULL);
-  serf_bucket_aggregate_append(body_bkt, cdata_bkt);
+  if (old_val_p)
+    {
+      const char *encoding2;
+      const svn_string_t *encoded_value2;
+      serf_bucket_t *cdata_bkt2;
+
+      SVN_ERR(get_encoding_and_cdata(&encoding2, &encoded_value2,
+                                     alloc, *old_val_p, pool));
+
+      if (encoded_value2)
+        {
+          cdata_bkt2 = SERF_BUCKET_SIMPLE_STRING_LEN(encoded_value2->data,
+                                                     encoded_value2->len,
+                                                     alloc);
+        }
+      else
+        {
+          cdata_bkt2 = NULL;
+        }
+
+      if (cdata_bkt2)
+        svn_ra_serf__add_open_tag_buckets(body_bkt, alloc,
+                                          SVN_DAV__OLD_VALUE,
+                                          "V:encoding", encoding2,
+                                          NULL);
+      else
+        svn_ra_serf__add_open_tag_buckets(body_bkt, alloc,
+                                          SVN_DAV__OLD_VALUE,
+                                          "V:" SVN_DAV__OLD_VALUE__ABSENT, "1",
+                                          NULL);
+
+      if (cdata_bkt2)
+        serf_bucket_aggregate_append(body_bkt, cdata_bkt2);
+
+      svn_ra_serf__add_close_tag_buckets(body_bkt, alloc,
+                                         SVN_DAV__OLD_VALUE);
+    }
+  if (cdata_bkt)
+    serf_bucket_aggregate_append(body_bkt, cdata_bkt);
   svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, prop_name);
 
   return SVN_NO_ERROR;
@@ -766,6 +815,19 @@ create_proppatch_body(serf_bucket_t **bk
                                     "xmlns:S", SVN_DAV_PROP_NS_SVN,
                                     NULL);
 
+  if (ctx->atomic_props && apr_hash_count(ctx->atomic_props) > 0)
+    {
+      svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set", NULL);
+      svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop", NULL);
+
+      svn_ra_serf__walk_all_props(ctx->atomic_props, ctx->path,
+                                  SVN_INVALID_REVNUM, TRUE,
+                                  proppatch_walker, body_bkt, pool);
+
+      svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
+      svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
+    }
+
   if (apr_hash_count(ctx->changed_props) > 0)
     {
       svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set", NULL);
@@ -2216,9 +2278,6 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
 
       /* How did you get past the same check in svn_ra_change_rev_prop2()? */
       SVN_ERR_ASSERT(capable);
-
-      /* ### server-side support hasn't been implemented yet */
-      SVN__NOT_IMPLEMENTED();
     }
 
   commit = apr_pcalloc(pool, sizeof(*commit));
@@ -2267,19 +2326,25 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
   proppatch_ctx->path = proppatch_target;
   proppatch_ctx->changed_props = apr_hash_make(proppatch_ctx->pool);
   proppatch_ctx->removed_props = apr_hash_make(proppatch_ctx->pool);
+  proppatch_ctx->atomic_props = apr_hash_make(proppatch_ctx->pool);
   proppatch_ctx->base_revision = SVN_INVALID_REVNUM;
 
-  if (value)
+  if (old_value_p)
+    {
+      svn_ra_serf__set_prop(proppatch_ctx->atomic_props, proppatch_ctx->path,
+                            ns, name, old_value_p, value, proppatch_ctx->pool);
+    }
+  else if (value)
     {
       svn_ra_serf__set_prop(proppatch_ctx->changed_props, proppatch_ctx->path,
-                            ns, name, NULL, value, proppatch_ctx->pool);
+                            ns, name, old_value_p, value, proppatch_ctx->pool);
     }
   else
     {
       value = svn_string_create("", proppatch_ctx->pool);
 
       svn_ra_serf__set_prop(proppatch_ctx->removed_props, proppatch_ctx->path,
-                            ns, name, NULL, value, proppatch_ctx->pool);
+                            ns, name, old_value_p, value, proppatch_ctx->pool);
     }
 
   err = proppatch_resource(proppatch_ctx, commit, proppatch_ctx->pool);

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c Sat Aug  7 17:45:38 2010
@@ -209,7 +209,19 @@ svn_ra_serf__set_ver_prop(apr_hash_t *pr
       apr_hash_set(path_props, ns, APR_HASH_KEY_STRING, ns_props);
     }
 
-  apr_hash_set(ns_props, name, APR_HASH_KEY_STRING, val);
+  if (old_value_p)
+    {
+      /* This must be PROPPATCH_CTX->ATOMIC_PROPS. */
+      svn_dav__two_props_t *both_values;
+      both_values = apr_palloc(pool, sizeof(*both_values));
+      both_values->old_value_p = old_value_p;
+      both_values->new_value = val;
+      apr_hash_set(ns_props, name, APR_HASH_KEY_STRING, both_values);
+    }
+  else
+    {
+      apr_hash_set(ns_props, name, APR_HASH_KEY_STRING, val);
+    }
 }
 
 void
@@ -784,8 +796,18 @@ svn_ra_serf__walk_all_props(apr_hash_t *
 
           apr_hash_this(name_hi, &prop_name, &prop_len, &prop_val);
           /* use a subpool? */
-          SVN_ERR(walker(baton, ns_name, ns_len, prop_name, prop_len,
-                         NULL /* ### */, prop_val, pool));
+          if (values_are_proppairs)
+            {
+              svn_dav__two_props_t *both_values = prop_val;
+              SVN_ERR(walker(baton, ns_name, ns_len, prop_name, prop_len,
+                             both_values->old_value_p, both_values->new_value,
+                             pool));
+            }
+          else
+            {
+              SVN_ERR(walker(baton, ns_name, ns_len, prop_name, prop_len,
+                             NULL, prop_val, pool));
+            }
         }
     }
 

Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h (original)
+++ subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h Sat Aug  7 17:45:38 2010
@@ -961,8 +961,8 @@ svn_ra_serf__retrieve_props(apr_hash_t *
 
 /* Set PROPS for PATH at REV revision with a NS:NAME VAL.
  * 
- * ### If OLD_VALUE_P is not NULL, it must equal the current value of the
- * ### revprop.
+ * If OLD_VALUE_P is not NULL, it must equal the current value of the
+ * revprop.
  *
  * The POOL governs allocation.
  */
@@ -984,7 +984,6 @@ typedef svn_error_t *
                                  const svn_string_t *val,
                                  apr_pool_t *pool);
 
-/* ### VALUES_ARE_PROPPAIRS is not implemented */
 svn_error_t *
 svn_ra_serf__walk_all_props(apr_hash_t *props,
                             const char *name,

Modified: subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c
URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c?rev=983269&r1=983268&r2=983269&view=diff
==============================================================================
--- subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c (original)
+++ subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c Sat Aug  7 17:45:38 2010
@@ -161,7 +161,9 @@ get_value(dav_db *db, const dav_prop_nam
 
 
 static dav_error *
-save_value(dav_db *db, const dav_prop_name *name, const svn_string_t *value)
+save_value(dav_db *db, const dav_prop_name *name,
+           const svn_string_t *const *old_value_p,
+           const svn_string_t *value)
 {
   const char *propname;
   svn_error_t *serr;
@@ -210,10 +212,11 @@ save_value(dav_db *db, const dav_prop_na
         }
       else
         {
-          serr = svn_repos_fs_change_rev_prop3(resource->info->repos->repos,
+          serr = svn_repos_fs_change_rev_prop4(resource->info->repos->repos,
                                                resource->info->root.rev,
                                                resource->info->repos->username,
-                                               propname, value, TRUE, TRUE,
+                                               propname, old_value_p, value,
+                                               TRUE, TRUE,
                                                db->authz_read_func,
                                                db->authz_read_baton,
                                                resource->pool);
@@ -425,6 +428,7 @@ db_map_namespaces(dav_db *db,
 
 static dav_error *
 decode_property_value(const svn_string_t **out_propval_p,
+                      svn_boolean_t *absent,
                       const svn_string_t *maybe_encoded_propval,
                       const apr_xml_elem *elem,
                       apr_pool_t *pool)
@@ -432,6 +436,7 @@ decode_property_value(const svn_string_t
   apr_xml_attr *attr = elem->attr;
 
   /* Default: no "encoding" attribute. */
+  *absent = FALSE;
   *out_propval_p = maybe_encoded_propval;
 
   /* Check for special encodings of the property value. */
@@ -443,12 +448,21 @@ decode_property_value(const svn_string_t
 
           /* Handle known encodings here. */
           if (enc_type && (strcmp(enc_type, "base64") == 0))
-            *out_propval_p = svn_base64_decode_string(maybe_encoded_propval, pool);
+            *out_propval_p = svn_base64_decode_string(maybe_encoded_propval,
+                                                      pool);
           else
             return dav_new_error(pool, HTTP_INTERNAL_SERVER_ERROR, 0,
                                  "Unknown property encoding");
           break;
         }
+
+      if (strcmp(attr->name, SVN_DAV__OLD_VALUE__ABSENT) == 0)
+        {
+          /* ### parse attr->value */
+          *absent = TRUE;
+          *out_propval_p = NULL;
+        }
+
       /* Next attribute, please. */
       attr = attr->next;
     }
@@ -462,7 +476,10 @@ db_store(dav_db *db,
          const apr_xml_elem *elem,
          dav_namespace_map *mapping)
 {
+  const svn_string_t *const *old_propval_p;
+  const svn_string_t *old_propval;
   const svn_string_t *propval;
+  svn_boolean_t absent;
   apr_pool_t *pool = db->p;
   dav_error *derr;
 
@@ -475,11 +492,41 @@ db_store(dav_db *db,
   propval = svn_string_create
     (dav_xml_get_cdata(elem, pool, 0 /* strip_white */), pool);
 
-  derr = decode_property_value(&propval, propval, elem, pool);
+  derr = decode_property_value(&propval, &absent, propval, elem, pool);
   if (derr)
     return derr;
 
-  return save_value(db, name, propval);
+  if (absent && ! elem->first_child)
+    /* ### better error check */
+    return dav_new_error(pool, HTTP_INTERNAL_SERVER_ERROR, 0,
+                         apr_psprintf(pool, 
+                                      "'%s' cannot be specified on the value "
+                                      "without specifying an expectation",
+                                      SVN_DAV__OLD_VALUE__ABSENT));
+
+  /* ### namespace check? */
+  if (elem->first_child && !strcmp(elem->first_child->name, SVN_DAV__OLD_VALUE))
+    {
+      const char *propname;
+
+      get_repos_propname(db, name, &propname);
+
+      /* Parse OLD_PROPVAL. */
+      old_propval = svn_string_create(dav_xml_get_cdata(elem->first_child, pool,
+                                                        0 /* strip_white */),
+                                      pool);
+      derr = decode_property_value(&old_propval, &absent,
+                                   old_propval, elem->first_child, pool);
+      if (derr)
+        return derr;
+
+      old_propval_p = (const svn_string_t *const *) &old_propval;
+    }
+  else
+    old_propval_p = NULL;
+
+
+  return save_value(db, name, old_propval_p, propval);
 }
 
 



Re: svn commit: r983269 - in /subversion/branches/atomic-revprop: ./ notes/http-and-webdav/ subversion/include/private/ subversion/libsvn_ra_neon/ subversion/libsvn_ra_serf/ subversion/mod_dav_svn/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
danielsh@apache.org wrote on Sat, Aug 07, 2010 at 17:45:39 -0000:
> Author: danielsh
> Date: Sat Aug  7 17:45:38 2010
> New Revision: 983269
> 
> URL: http://svn.apache.org/viewvc?rev=983269&view=rev
> Log:
> On the 'atomic-revprop' branch:
> 
> Implement the API over ra_dav.
> 

I'll appreciate some extra eyes on this revision.

Thanks.

Daniel
(reasons for this request: at the end)

> 
> First, some general infrastructure:
> 
> * notes/http-and-webdav/webdav-protocol
>   (PROPPATCH):  Document the protocol extension.
> 
> * subversion/include/private/svn_dav_protocol.h
>   (SVN_DAV__OLD_VALUE, SVN_DAV__OLD_VALUE__ABSENT):  New macros.
>   (svn_dav__two_props_t):  New helper typedef.
> 
> 
> Then, mod_dav_svn support:
> 
> * subversion/mod_dav_svn/deadprops.c
>   (save_value):
>     Grow and use an OLD_VALUE_P parameter, passing it
>     to svn_repos_fs_change_rev_prop4().
>   (decode_property_value):
>     Grow an ABSENT out parameter, and parse the (new) 'V:absent' attribute.
>   (db_store):
>     Parse <V:old-value/> and 'V:absent' and pass OLD_VALUE_P to save_value().
> 
> 
> ra_serf support:
> 
> * subversion/libsvn_ra_serf/ra_serf.h
>   (svn_ra_serf__walk_all_props, svn_ra_serf__set_ver_prop):
>     Remove "not implemented" markers from docstrings.
> 
> * subversion/libsvn_ra_serf/property.c
>   (svn_ra_serf__set_ver_prop):
>     Store an svn_dav__two_props_t in the hash if an OLD_VALUE_P was provided.
>   (svn_ra_serf__walk_all_props):
>     Support the VALUES_ARE_PROPPAIRS parameter when calling the walker.
> 
> * subversion/libsvn_ra_serf/commit.c
>   (proppatch_context_t):  Add 'atomic_props' member.
>   (get_encoding_and_cdata):
>     Refactor to return a string rather than a bucket,
>     and to support a NULL value.
>   (proppatch_walker):
>     Track get_encoding_and_cdata() signature change.
>     Set 'V:absent' and <V:old-value/> as appropriate.
>   (create_proppatch_body):
>     Walk/Iterate CTX->ATOMIC_PROPS too.
>   (svn_ra_serf__change_rev_prop):
>     Remove 'not implemented' sentinel.  Initialize CTX->ATOMIC_PROPS.
>     Pass OLD_VALUE_P to svn_ra_serf__set_prop() (which passes it
>     to svn_ra_serf__set_ver_prop()).
> 
> 
> ra_neon support:
> 
> * subversion/libsvn_ra_neon/ra_neon.h
>   (svn_ra_neon__do_proppatch):
>     Add PROP_OLD_VALUES parameter.
> 
> * subversion/libsvn_ra_neon/fetch.c
>   (svn_ra_neon__change_rev_prop):
>     Remove 'not implemented' sentinel.
>     Pass OLD_VALUE_P to svn_ra_neon__do_proppatch() via PROP_OLD_VALUES.
> 
> * subversion/libsvn_ra_neon/props.c
>   (append_setprop):
>     Grow OLD_VALUE_P parameter.  Use it to 
>     generate 'V:absent' and <V:old-value/> as appropriate.
>   (svn_ra_neon__do_proppatch):
>     Grow PROP_OLD_VALUES parameter, and use it.
>   
> 
> * subversion/libsvn_ra_neon/commit.c
>   (do_proppatch, apply_revprops):
>     Track signature change of svn_ra_neon__do_proppatch().
> 
> 
> Finally:
> 
> * BRANCH-README:  Mark this as done.
> 
> Modified:
>     subversion/branches/atomic-revprop/BRANCH-README
>     subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol
>     subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c
>     subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h
>     subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c
> 
> Modified: subversion/branches/atomic-revprop/BRANCH-README
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/BRANCH-README?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/BRANCH-README (original)
> +++ subversion/branches/atomic-revprop/BRANCH-README Sat Aug  7 17:45:38 2010
> @@ -21,11 +21,7 @@ Planned work
>        achieved with older servers, and because of planned protocol extensions)
>    - [DONE] ra_local: Trivial.
>    - [DONE] ra_svn: Add a 'change-rev-prop2' verb.
> -  - ra_dav
> -      Extend PROPPATCH syntax to provide the old value: add a <S:oldvalue/>
> -      tag inside the <${propname}> tag.
> -        Q: where to document this protocol extension?
> -        A: notes/http-and-webdav/webdav-protocol
> +  - [DONE] ra_dav: Extend PROPPATCH: see notes/http-and-webdav/webdav-protocol
>    - [DONE] unit test
>        prop_tests.py 34: atomic_over_ra()
>    - TODO: extend it to test props set to the empty string and unset props
> 
> Modified: subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol (original)
> +++ subversion/branches/atomic-revprop/notes/http-and-webdav/webdav-protocol Sat Aug  7 17:45:38 2010
> @@ -177,6 +177,30 @@ Response:
>    
>    ...svn-svndiff stream that can be passed to svn_txdelta_parse_svndiff...
>  
> +PROPPATCH
> +=========
> +
> +We extend PROPPATCH as follows.  To pass OLD_VALUE_P (as in
> +svn_ra_change_rev_prop2()), any propchange which is accompanied by a non-NULL
> +OLD_VALUE_P goes within the <D:set><D:prop> tag (and never within the
> +<D:remove><D:prop> tag --- even if it is a propdel).  Consequently, in
> +mod_dav_svn it would land in db_store() and not db_remove().
> +
> +The property tag (in the C: or S: namespace) always contains the propval in its
> +cdata (potentially base64-encoded).  The extension is as follows:
> +
> +* The property tag grows a V:absent attribute, to represent that the property
> +  is being removed (i.e., a propdel routed to <D:set><D:prop>).
> +
> +* A <V:old-value> tag may be nested within the property tag.  The nested tag
> +  supports the same V:absent and V:encoding attributed as the parent (property)
> +  tag.
> +
> +Historical note: we route propdels via <D:set>/db_store() because the mod_dav
> +API for db_remove() was insufficient.  See this thread:
> +http://mid.gmane.org/4C531CFB.2010202@collab.net
> +
> +
>  Custom REPORTs
>  ==============
>  
> 
> Modified: subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h (original)
> +++ subversion/branches/atomic-revprop/subversion/include/private/svn_dav_protocol.h Sat Aug  7 17:45:38 2010
> @@ -46,6 +46,17 @@ extern "C" {
>  #define SVN_DAV__INCLUDE_DESCENDANTS "include-descendants"
>  #define SVN_DAV__VERSION_NAME "version-name"
>  
> +/** Names of XML elements attributes and tags for svn_ra_change_rev_prop2()'s
> +    extension of PROPPATCH.  */
> +#define SVN_DAV__OLD_VALUE "old-value"
> +#define SVN_DAV__OLD_VALUE__ABSENT "absent"
> +
> +/** Helper typedef for svn_ra_change_rev_prop2() implementation. */
> +typedef struct svn_dav__two_props_t {
> +  const svn_string_t *const *old_value_p;
> +  const svn_string_t *new_value;
> +} svn_dav__two_props_t;
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/commit.c Sat Aug  7 17:45:38 2010
> @@ -616,7 +616,8 @@ static svn_error_t * do_proppatch(svn_ra
>      }
>  
>    return svn_ra_neon__do_proppatch(ras, url, rb->prop_changes,
> -                                   rb->prop_deletes, extra_headers, pool);
> +                                   rb->prop_deletes, NULL, extra_headers,
> +                                   pool);
>  }
>  
>  
> @@ -1417,7 +1418,7 @@ static svn_error_t * apply_revprops(comm
>      return err;
>  
>    return svn_ra_neon__do_proppatch(cc->ras, vcc_rsrc.wr_url, revprop_table,
> -                                   NULL, NULL, pool);
> +                                   NULL, NULL, NULL, pool);
>  }
>  
>  svn_error_t * svn_ra_neon__get_commit_editor(svn_ra_session_t *session,
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/fetch.c Sat Aug  7 17:45:38 2010
> @@ -1131,6 +1131,7 @@ svn_error_t *svn_ra_neon__change_rev_pro
>    svn_error_t *err;
>    apr_hash_t *prop_changes = NULL;
>    apr_array_header_t *prop_deletes = NULL;
> +  apr_hash_t *prop_old_values = NULL;
>    static const ne_propname wanted_props[] =
>      {
>        { "DAV:", "auto-version" },
> @@ -1146,9 +1147,6 @@ svn_error_t *svn_ra_neon__change_rev_pro
>  
>        /* How did you get past the same check in svn_ra_change_rev_prop2()? */
>        SVN_ERR_ASSERT(capable);
> -
> -      /* ### server-side support hasn't been implemented yet */
> -      SVN__NOT_IMPLEMENTED();
>      }
>  
>    /* Main objective: do a PROPPATCH (allprops) on a baseline object */
> @@ -1180,19 +1178,33 @@ svn_error_t *svn_ra_neon__change_rev_pro
>           to attempt the PROPPATCH if the deltaV server is going to do
>           auto-versioning and create a new baseline! */
>  
> -  if (value)
> +  if (old_value_p)
>      {
> -      prop_changes = apr_hash_make(pool);
> -      apr_hash_set(prop_changes, name, APR_HASH_KEY_STRING, value);
> +      svn_dav__two_props_t *both_values;
> +
> +      both_values = apr_palloc(pool, sizeof(*both_values));
> +      both_values->old_value_p = old_value_p;
> +      both_values->new_value = value;
> +
> +      prop_old_values = apr_hash_make(pool);
> +      apr_hash_set(prop_old_values, name, APR_HASH_KEY_STRING, both_values);
>      }
>    else
>      {
> -      prop_deletes = apr_array_make(pool, 1, sizeof(const char *));
> -      APR_ARRAY_PUSH(prop_deletes, const char *) = name;
> +      if (value)
> +        {
> +          prop_changes = apr_hash_make(pool);
> +          apr_hash_set(prop_changes, name, APR_HASH_KEY_STRING, value);
> +        }
> +      else
> +        {
> +          prop_deletes = apr_array_make(pool, 1, sizeof(const char *));
> +          APR_ARRAY_PUSH(prop_deletes, const char *) = name;
> +        }
>      }
>  
>    err = svn_ra_neon__do_proppatch(ras, baseline->url, prop_changes,
> -                                  prop_deletes, NULL, pool);
> +                                  prop_deletes, prop_old_values, NULL, pool);
>    if (err)
>      return
>        svn_error_create
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/props.c Sat Aug  7 17:45:38 2010
> @@ -1075,12 +1075,14 @@ get_encoding_and_cdata(const char **enco
>  static svn_error_t *
>  append_setprop(svn_stringbuf_t *body,
>                 const char *name,
> +               const svn_string_t *const *old_value_p,
>                 const svn_string_t *value,
>                 apr_pool_t *pool)
>  {
>    const char *encoding;
>    const char *xml_safe;
>    const char *xml_tag_name;
> +  const char *old_value_tag;
>  
>    /* Map property names to namespaces */
>  #define NSLEN (sizeof(SVN_PROP_PREFIX) - 1)
> @@ -1094,11 +1096,45 @@ append_setprop(svn_stringbuf_t *body,
>        xml_tag_name = apr_pstrcat(pool, "C:", name, NULL);
>      }
>  
> -  SVN_ERR(get_encoding_and_cdata(&encoding, &xml_safe, value, pool));
> +  if (old_value_p)
> +    {
> +      if (*old_value_p)
> +        {
> +          const char *encoding2;
> +          const char *xml_safe2;
> +          SVN_ERR(get_encoding_and_cdata(&encoding2, &xml_safe2,
> +                                         *old_value_p, pool));
> +          old_value_tag = apr_psprintf(pool, "<%s %s>%s</%s>",
> +                                       "V:" SVN_DAV__OLD_VALUE, encoding2,
> +                                       xml_safe2, "V:" SVN_DAV__OLD_VALUE);
> +        }
> +      else
> +        {
> +#define OLD_VALUE_ABSENT_TAG \
> +          "<" "V:" SVN_DAV__OLD_VALUE \
> +              " V:" SVN_DAV__OLD_VALUE__ABSENT "=\"1\" " \
> +          "/>"
> +          old_value_tag = OLD_VALUE_ABSENT_TAG;
> +        }
> +    }
> +  else
> +    {
> +      old_value_tag = "";
> +    }
> +
> +  if (old_value_p && !value)
> +    {
> +      encoding = "V:" SVN_DAV__OLD_VALUE__ABSENT "=\"1\"" ;
> +      xml_safe = "";
> +    }
> +  else
> +    {
> +      SVN_ERR(get_encoding_and_cdata(&encoding, &xml_safe, value, pool));
> +    }
>  
>    svn_stringbuf_appendcstr(body,
> -                           apr_psprintf(pool,"<%s %s>%s</%s>",
> -                                        xml_tag_name, encoding,
> +                           apr_psprintf(pool,"<%s %s>%s%s</%s>",
> +                                        xml_tag_name, encoding, old_value_tag,
>                                          xml_safe, xml_tag_name));
>    return SVN_NO_ERROR;
>  }
> @@ -1109,6 +1145,7 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
>                            const char *url,
>                            apr_hash_t *prop_changes,
>                            const apr_array_header_t *prop_deletes,
> +                          apr_hash_t *prop_old_values,
>                            apr_hash_t *extra_headers,
>                            apr_pool_t *pool)
>  {
> @@ -1118,7 +1155,8 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
>  
>    /* just punt if there are no changes to make. */
>    if ((prop_changes == NULL || (! apr_hash_count(prop_changes)))
> -      && (prop_deletes == NULL || prop_deletes->nelts == 0))
> +      && (prop_deletes == NULL || prop_deletes->nelts == 0)
> +      && (prop_old_values == NULL || (! apr_hash_count(prop_old_values))))
>      return SVN_NO_ERROR;
>  
>    /* easier to roll our own PROPPATCH here than use ne_proppatch(), which
> @@ -1130,6 +1168,22 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
>       SVN_DAV_PROP_NS_CUSTOM "\" xmlns:S=\""
>       SVN_DAV_PROP_NS_SVN "\">" DEBUG_CR, pool);
>  
> +  /* Handle property changes/deletions with expected old values. */
> +  if (prop_old_values)
> +    {
> +      apr_hash_index_t *hi;
> +      svn_stringbuf_appendcstr(body, "<D:set><D:prop>");
> +      for (hi = apr_hash_first(pool, prop_old_values); hi; hi = apr_hash_next(hi))
> +        {
> +          const char *name = svn__apr_hash_index_key(hi);
> +          svn_dav__two_props_t *both_values = svn__apr_hash_index_val(hi);
> +          svn_pool_clear(subpool);
> +          SVN_ERR(append_setprop(body, name, both_values->old_value_p,
> +                                 both_values->new_value, subpool));
> +        }
> +      svn_stringbuf_appendcstr(body, "</D:prop></D:set>");
> +    }
> +
>    /* Handle property changes. */
>    if (prop_changes)
>      {
> @@ -1141,7 +1195,7 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
>            void *val;
>            svn_pool_clear(subpool);
>            apr_hash_this(hi, &key, NULL, &val);
> -          SVN_ERR(append_setprop(body, key, val, subpool));
> +          SVN_ERR(append_setprop(body, key, NULL, val, subpool));
>          }
>        svn_stringbuf_appendcstr(body, "</D:prop></D:set>");
>      }
> @@ -1155,7 +1209,7 @@ svn_ra_neon__do_proppatch(svn_ra_neon__s
>          {
>            const char *name = APR_ARRAY_IDX(prop_deletes, n, const char *);
>            svn_pool_clear(subpool);
> -          SVN_ERR(append_setprop(body, name, NULL, subpool));
> +          SVN_ERR(append_setprop(body, name, NULL, NULL, subpool));
>          }
>        svn_stringbuf_appendcstr(body, "</D:prop></D:remove>");
>      }
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_neon/ra_neon.h Sat Aug  7 17:45:38 2010
> @@ -573,12 +573,15 @@ svn_error_t *svn_ra_neon__get_vcc(const 
>  /* Issue a PROPPATCH request on URL, transmitting PROP_CHANGES (a hash
>     of const svn_string_t * values keyed on Subversion user-visible
>     property names) and PROP_DELETES (an array of property names to
> -   delete).  Send any extra request headers in EXTRA_HEADERS. Use POOL
> -   for all allocations.  */
> +   delete). PROP_OLD_VALUES is a hash of Subversion user-visible property
> +   names mapped to svn_dav__two_props_t * values. Send any extra
> +   request headers in EXTRA_HEADERS. Use POOL for all allocations.
> + */
>  svn_error_t *svn_ra_neon__do_proppatch(svn_ra_neon__session_t *ras,
>                                         const char *url,
>                                         apr_hash_t *prop_changes,
>                                         const apr_array_header_t *prop_deletes,
> +                                       apr_hash_t *prop_old_values,
>                                         apr_hash_t *extra_headers,
>                                         apr_pool_t *pool);
>  
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/commit.c Sat Aug  7 17:45:38 2010
> @@ -108,6 +108,7 @@ typedef struct {
>    /* Changed and removed properties. */
>    apr_hash_t *changed_props;
>    apr_hash_t *removed_props;
> +  apr_hash_t *atomic_props;
>  
>    /* In HTTP v2, this is the file/directory version we think we're changing. */
>    svn_revnum_t base_revision;
> @@ -639,16 +640,17 @@ checkout_file(file_context_t *file)
>  /* Helper function for proppatch_walker() below. */
>  static svn_error_t *
>  get_encoding_and_cdata(const char **encoding_p,
> -                       serf_bucket_t **cdata_bkt_p,
> +                       const svn_string_t **encoded_value_p,
>                         serf_bucket_alloc_t *alloc,
>                         const svn_string_t *value,
>                         apr_pool_t *pool)
>  {
> -  const char *encoding;
> -  const char *cdata;
> -  apr_size_t len; /* of cdata */
> -
> -  SVN_ERR_ASSERT(value);
> +  if (value == NULL)
> +    {
> +      *encoding_p = NULL;
> +      *encoded_value_p = NULL;
> +      return SVN_NO_ERROR;
> +    }
>  
>    /* If a property is XML-safe, XML-encode it.  Else, base64-encode
>       it. */
> @@ -656,23 +658,15 @@ get_encoding_and_cdata(const char **enco
>      {
>        svn_stringbuf_t *xml_esc = NULL;
>        svn_xml_escape_cdata_string(&xml_esc, value, pool);
> -      encoding = NULL;
> -      cdata = xml_esc->data;
> -      len = xml_esc->len;
> +      *encoding_p = NULL;
> +      *encoded_value_p = svn_string_create_from_buf(xml_esc, pool);
>      }
>    else
>      {
> -      const svn_string_t *base64ed = svn_base64_encode_string2(value, TRUE,
> -                                                               pool);
> -      encoding = "base64";
> -      cdata = base64ed->data;
> -      len = base64ed->len;
> +      *encoding_p = "base64";
> +      *encoded_value_p = svn_base64_encode_string2(value, TRUE, pool);
>      }
>  
> -  /* ENCODING, CDATA, and LEN are now set. */
> -
> -  *encoding_p = encoding;
> -  *cdata_bkt_p = SERF_BUCKET_SIMPLE_STRING_LEN(cdata, len, alloc);
>    return SVN_NO_ERROR;
>  }
>  
> @@ -680,13 +674,14 @@ static svn_error_t *
>  proppatch_walker(void *baton,
>                   const char *ns, apr_ssize_t ns_len,
>                   const char *name, apr_ssize_t name_len,
> -                 const svn_string_t *const *old_val_p, /* ### */
> +                 const svn_string_t *const *old_val_p,
>                   const svn_string_t *val,
>                   apr_pool_t *pool)
>  {
>    serf_bucket_t *body_bkt = baton;
>    serf_bucket_t *cdata_bkt;
>    serf_bucket_alloc_t *alloc;
> +  const svn_string_t *encoded_value;
>    const char *encoding;
>    char *prop_name;
>  
> @@ -700,12 +695,66 @@ proppatch_walker(void *baton,
>  
>    alloc = body_bkt->allocator;
>  
> -  SVN_ERR(get_encoding_and_cdata(&encoding, &cdata_bkt, alloc, val, pool));
> +  SVN_ERR(get_encoding_and_cdata(&encoding, &encoded_value, alloc, val, pool));
> +  if (encoded_value)
> +    {
> +      cdata_bkt = SERF_BUCKET_SIMPLE_STRING_LEN(encoded_value->data,
> +                                                encoded_value->len,
> +                                                alloc);
> +    }
> +  else
> +    {
> +      cdata_bkt = NULL;
> +    }
> +
> +  if (cdata_bkt)
> +    svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, prop_name,
> +                                      "V:encoding", encoding,
> +                                      NULL);
> +  else
> +    svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, prop_name,
> +                                      "V:" SVN_DAV__OLD_VALUE__ABSENT, "1",
> +                                      NULL);
>  
> -  svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, prop_name,
> -                                    "V:encoding", encoding,
> -                                    NULL);
> -  serf_bucket_aggregate_append(body_bkt, cdata_bkt);
> +  if (old_val_p)
> +    {
> +      const char *encoding2;
> +      const svn_string_t *encoded_value2;
> +      serf_bucket_t *cdata_bkt2;
> +
> +      SVN_ERR(get_encoding_and_cdata(&encoding2, &encoded_value2,
> +                                     alloc, *old_val_p, pool));
> +
> +      if (encoded_value2)
> +        {
> +          cdata_bkt2 = SERF_BUCKET_SIMPLE_STRING_LEN(encoded_value2->data,
> +                                                     encoded_value2->len,
> +                                                     alloc);
> +        }
> +      else
> +        {
> +          cdata_bkt2 = NULL;
> +        }
> +
> +      if (cdata_bkt2)
> +        svn_ra_serf__add_open_tag_buckets(body_bkt, alloc,
> +                                          SVN_DAV__OLD_VALUE,
> +                                          "V:encoding", encoding2,
> +                                          NULL);
> +      else
> +        svn_ra_serf__add_open_tag_buckets(body_bkt, alloc,
> +                                          SVN_DAV__OLD_VALUE,
> +                                          "V:" SVN_DAV__OLD_VALUE__ABSENT, "1",
> +                                          NULL);
> +
> +      if (cdata_bkt2)
> +        serf_bucket_aggregate_append(body_bkt, cdata_bkt2);
> +
> +      svn_ra_serf__add_close_tag_buckets(body_bkt, alloc,
> +                                         SVN_DAV__OLD_VALUE);
> +    }
> +  if (cdata_bkt)
> +    serf_bucket_aggregate_append(body_bkt, cdata_bkt);
>    svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, prop_name);
>  
>    return SVN_NO_ERROR;
> @@ -766,6 +815,19 @@ create_proppatch_body(serf_bucket_t **bk
>                                      "xmlns:S", SVN_DAV_PROP_NS_SVN,
>                                      NULL);
>  
> +  if (ctx->atomic_props && apr_hash_count(ctx->atomic_props) > 0)
> +    {
> +      svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set", NULL);
> +      svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:prop", NULL);
> +
> +      svn_ra_serf__walk_all_props(ctx->atomic_props, ctx->path,
> +                                  SVN_INVALID_REVNUM, TRUE,
> +                                  proppatch_walker, body_bkt, pool);
> +
> +      svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:prop");
> +      svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "D:set");
> +    }
> +
>    if (apr_hash_count(ctx->changed_props) > 0)
>      {
>        svn_ra_serf__add_open_tag_buckets(body_bkt, alloc, "D:set", NULL);
> @@ -2216,9 +2278,6 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
>  
>        /* How did you get past the same check in svn_ra_change_rev_prop2()? */
>        SVN_ERR_ASSERT(capable);
> -
> -      /* ### server-side support hasn't been implemented yet */
> -      SVN__NOT_IMPLEMENTED();
>      }
>  
>    commit = apr_pcalloc(pool, sizeof(*commit));
> @@ -2267,19 +2326,25 @@ svn_ra_serf__change_rev_prop(svn_ra_sess
>    proppatch_ctx->path = proppatch_target;
>    proppatch_ctx->changed_props = apr_hash_make(proppatch_ctx->pool);
>    proppatch_ctx->removed_props = apr_hash_make(proppatch_ctx->pool);
> +  proppatch_ctx->atomic_props = apr_hash_make(proppatch_ctx->pool);
>    proppatch_ctx->base_revision = SVN_INVALID_REVNUM;
>  
> -  if (value)
> +  if (old_value_p)
> +    {
> +      svn_ra_serf__set_prop(proppatch_ctx->atomic_props, proppatch_ctx->path,
> +                            ns, name, old_value_p, value, proppatch_ctx->pool);
> +    }
> +  else if (value)
>      {
>        svn_ra_serf__set_prop(proppatch_ctx->changed_props, proppatch_ctx->path,
> -                            ns, name, NULL, value, proppatch_ctx->pool);
> +                            ns, name, old_value_p, value, proppatch_ctx->pool);
>      }
>    else
>      {
>        value = svn_string_create("", proppatch_ctx->pool);
>  
>        svn_ra_serf__set_prop(proppatch_ctx->removed_props, proppatch_ctx->path,
> -                            ns, name, NULL, value, proppatch_ctx->pool);
> +                            ns, name, old_value_p, value, proppatch_ctx->pool);
>      }
>  
>    err = proppatch_resource(proppatch_ctx, commit, proppatch_ctx->pool);
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/property.c Sat Aug  7 17:45:38 2010
> @@ -209,7 +209,19 @@ svn_ra_serf__set_ver_prop(apr_hash_t *pr
>        apr_hash_set(path_props, ns, APR_HASH_KEY_STRING, ns_props);
>      }
>  
> -  apr_hash_set(ns_props, name, APR_HASH_KEY_STRING, val);
> +  if (old_value_p)
> +    {
> +      /* This must be PROPPATCH_CTX->ATOMIC_PROPS. */
> +      svn_dav__two_props_t *both_values;
> +      both_values = apr_palloc(pool, sizeof(*both_values));
> +      both_values->old_value_p = old_value_p;
> +      both_values->new_value = val;
> +      apr_hash_set(ns_props, name, APR_HASH_KEY_STRING, both_values);
> +    }
> +  else
> +    {
> +      apr_hash_set(ns_props, name, APR_HASH_KEY_STRING, val);
> +    }
>  }
>  
>  void
> @@ -784,8 +796,18 @@ svn_ra_serf__walk_all_props(apr_hash_t *
>  
>            apr_hash_this(name_hi, &prop_name, &prop_len, &prop_val);
>            /* use a subpool? */
> -          SVN_ERR(walker(baton, ns_name, ns_len, prop_name, prop_len,
> -                         NULL /* ### */, prop_val, pool));
> +          if (values_are_proppairs)
> +            {
> +              svn_dav__two_props_t *both_values = prop_val;
> +              SVN_ERR(walker(baton, ns_name, ns_len, prop_name, prop_len,
> +                             both_values->old_value_p, both_values->new_value,
> +                             pool));
> +            }
> +          else
> +            {
> +              SVN_ERR(walker(baton, ns_name, ns_len, prop_name, prop_len,
> +                             NULL, prop_val, pool));
> +            }
>          }
>      }
>  
> 
> Modified: subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h (original)
> +++ subversion/branches/atomic-revprop/subversion/libsvn_ra_serf/ra_serf.h Sat Aug  7 17:45:38 2010
> @@ -961,8 +961,8 @@ svn_ra_serf__retrieve_props(apr_hash_t *
>  
>  /* Set PROPS for PATH at REV revision with a NS:NAME VAL.
>   * 
> - * ### If OLD_VALUE_P is not NULL, it must equal the current value of the
> - * ### revprop.
> + * If OLD_VALUE_P is not NULL, it must equal the current value of the
> + * revprop.
>   *
>   * The POOL governs allocation.
>   */
> @@ -984,7 +984,6 @@ typedef svn_error_t *
>                                   const svn_string_t *val,
>                                   apr_pool_t *pool);
>  
> -/* ### VALUES_ARE_PROPPAIRS is not implemented */
>  svn_error_t *
>  svn_ra_serf__walk_all_props(apr_hash_t *props,
>                              const char *name,
> 
> Modified: subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c
> URL: http://svn.apache.org/viewvc/subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c?rev=983269&r1=983268&r2=983269&view=diff
> ==============================================================================
> --- subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c (original)
> +++ subversion/branches/atomic-revprop/subversion/mod_dav_svn/deadprops.c Sat Aug  7 17:45:38 2010
> @@ -161,7 +161,9 @@ get_value(dav_db *db, const dav_prop_nam
>  
>  
>  static dav_error *
> -save_value(dav_db *db, const dav_prop_name *name, const svn_string_t *value)
> +save_value(dav_db *db, const dav_prop_name *name,
> +           const svn_string_t *const *old_value_p,
> +           const svn_string_t *value)
>  {
>    const char *propname;
>    svn_error_t *serr;
> @@ -210,10 +212,11 @@ save_value(dav_db *db, const dav_prop_na
>          }
>        else
>          {
> -          serr = svn_repos_fs_change_rev_prop3(resource->info->repos->repos,
> +          serr = svn_repos_fs_change_rev_prop4(resource->info->repos->repos,
>                                                 resource->info->root.rev,
>                                                 resource->info->repos->username,
> -                                               propname, value, TRUE, TRUE,
> +                                               propname, old_value_p, value,
> +                                               TRUE, TRUE,
>                                                 db->authz_read_func,
>                                                 db->authz_read_baton,
>                                                 resource->pool);
> @@ -425,6 +428,7 @@ db_map_namespaces(dav_db *db,
>  
>  static dav_error *
>  decode_property_value(const svn_string_t **out_propval_p,
> +                      svn_boolean_t *absent,
>                        const svn_string_t *maybe_encoded_propval,
>                        const apr_xml_elem *elem,
>                        apr_pool_t *pool)
> @@ -432,6 +436,7 @@ decode_property_value(const svn_string_t
>    apr_xml_attr *attr = elem->attr;
>  
>    /* Default: no "encoding" attribute. */
> +  *absent = FALSE;
>    *out_propval_p = maybe_encoded_propval;
>  
>    /* Check for special encodings of the property value. */
> @@ -443,12 +448,21 @@ decode_property_value(const svn_string_t
>  
>            /* Handle known encodings here. */
>            if (enc_type && (strcmp(enc_type, "base64") == 0))
> -            *out_propval_p = svn_base64_decode_string(maybe_encoded_propval, pool);
> +            *out_propval_p = svn_base64_decode_string(maybe_encoded_propval,
> +                                                      pool);
>            else
>              return dav_new_error(pool, HTTP_INTERNAL_SERVER_ERROR, 0,
>                                   "Unknown property encoding");
>            break;
>          }
> +
> +      if (strcmp(attr->name, SVN_DAV__OLD_VALUE__ABSENT) == 0)
> +        {
> +          /* ### parse attr->value */
> +          *absent = TRUE;
> +          *out_propval_p = NULL;
> +        }
> +
>        /* Next attribute, please. */
>        attr = attr->next;
>      }
> @@ -462,7 +476,10 @@ db_store(dav_db *db,
>           const apr_xml_elem *elem,
>           dav_namespace_map *mapping)
>  {
> +  const svn_string_t *const *old_propval_p;
> +  const svn_string_t *old_propval;
>    const svn_string_t *propval;
> +  svn_boolean_t absent;
>    apr_pool_t *pool = db->p;
>    dav_error *derr;
>  
> @@ -475,11 +492,41 @@ db_store(dav_db *db,
>    propval = svn_string_create
>      (dav_xml_get_cdata(elem, pool, 0 /* strip_white */), pool);
>  
> -  derr = decode_property_value(&propval, propval, elem, pool);
> +  derr = decode_property_value(&propval, &absent, propval, elem, pool);
>    if (derr)
>      return derr;
>  
> -  return save_value(db, name, propval);
> +  if (absent && ! elem->first_child)
> +    /* ### better error check */
> +    return dav_new_error(pool, HTTP_INTERNAL_SERVER_ERROR, 0,
> +                         apr_psprintf(pool, 
> +                                      "'%s' cannot be specified on the value "
> +                                      "without specifying an expectation",
> +                                      SVN_DAV__OLD_VALUE__ABSENT));
> +
> +  /* ### namespace check? */
> +  if (elem->first_child && !strcmp(elem->first_child->name, SVN_DAV__OLD_VALUE))
> +    {
> +      const char *propname;
> +
> +      get_repos_propname(db, name, &propname);
> +
> +      /* Parse OLD_PROPVAL. */
> +      old_propval = svn_string_create(dav_xml_get_cdata(elem->first_child, pool,
> +                                                        0 /* strip_white */),
> +                                      pool);
> +      derr = decode_property_value(&old_propval, &absent,
> +                                   old_propval, elem->first_child, pool);
> +      if (derr)
> +        return derr;
> +
> +      old_propval_p = (const svn_string_t *const *) &old_propval;
> +    }
> +  else
> +    old_propval_p = NULL;
> +
> +
> +  return save_value(db, name, old_propval_p, propval);
>  }
>  
>  
> 
> 

(Why?  It's the RA layer I'm least familiar with, it required the least
straightforward implementation (both client- and server- side), and after
spending quite some time inside a 700-line patch I no longer have the
perspective or patience to review all the small details carefully again.)