danie...@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.2010...@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.)
