danielsh_at_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.)
Received on 2010-08-07 19:57:05 CEST