danielsh_at_apache.org writes:
> Author: danielsh
> Date: Wed Jun 16 06:05:17 2010
> New Revision: 955136
>
> URL: http://svn.apache.org/viewvc?rev=955136&view=rev
> Log:
> Revv the FS change_rev_prop() interface towards more atomicity.
>
> Suggested by: philip
>
>
> * subversion/include/svn_fs.h
> (svn_fs_change_rev_prop2): New, takes OLD_VALUE_P parameter.
> (svn_fs_change_rev_prop): Deprecate.
I don't think the old interface should be deprecated.
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_fs.h (original)
> +++ subversion/trunk/subversion/include/svn_fs.h Wed Jun 16 06:05:17 2010
> @@ -1894,6 +1894,10 @@ svn_fs_revision_proplist(apr_hash_t **ta
> * - @a fs is a filesystem, and @a rev is the revision in that filesystem
> * whose property should change.
> * - @a name is the name of the property to change.
> + * - if @a old_value_p is not @c NULL, then @a *old_value_p is the expected old
> + * value of the property, and changing the value will fail with error
> + * #SVN_ERR_BAD_PROPERTY_VALUE if the present value of the property is not @a
> + * *old_value_p.
> * - @a value is the new value of the property, or zero if the property should
How are you going to check adds? During an add the old value is NULL
but must still be checked.
> * be removed altogether.
> *
> @@ -1902,7 +1906,25 @@ svn_fs_revision_proplist(apr_hash_t **ta
> * via transactions.
> *
> * Do any necessary temporary allocation in @a pool.
> + *
> + * @since New in 1.7.
> + */
> +svn_error_t *
> +svn_fs_change_rev_prop2(svn_fs_t *fs,
> + svn_revnum_t rev,
> + const char *name,
> + const svn_string_t **old_value_p,
> + const svn_string_t *value,
> + apr_pool_t *pool);
> +
> +
> svn_fs_base__set_rev_prop(svn_fs_t *fs,
> svn_revnum_t rev,
> const char *name,
> + const svn_string_t **old_value_p,
> const svn_string_t *value,
> trail_t *trail,
> apr_pool_t *pool)
> @@ -259,6 +260,23 @@ svn_fs_base__set_rev_prop(svn_fs_t *fs,
> txn->proplist = apr_hash_make(pool);
>
> /* Set the property. */
> + if (old_value_p)
> + {
That's not going to work for adds because it will skip the validation.
> + const svn_string_t *wanted_value = *old_value_p;
> + const svn_string_t *present_value = apr_hash_get(txn->proplist, name,
> + APR_HASH_KEY_STRING);
> + if ((!wanted_value != !present_value)
> + || (wanted_value && present_value
> + && !svn_string_compare(wanted_value, present_value)))
> + {
> + /* What we expected isn't what we found. */
> + return svn_error_createf(SVN_ERR_BAD_PROPERTY_VALUE, NULL,
> + _("revprop '%s' has unexpected value in "
> + "filesystem"),
> + name);
> + }
> + /* Fall through. */
> + }
> apr_hash_set(txn->proplist, name, APR_HASH_KEY_STRING, value);
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=955136&r1=955135&r2=955136&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Wed Jun 16 06:05:17 2010
> @@ -7167,6 +7167,7 @@ struct change_rev_prop_baton {
> svn_fs_t *fs;
> svn_revnum_t rev;
> const char *name;
> + const svn_string_t **old_value_p;
> const svn_string_t *value;
> };
>
> @@ -7182,6 +7183,23 @@ change_rev_prop_body(void *baton, apr_po
>
> SVN_ERR(svn_fs_fs__revision_proplist(&table, cb->fs, cb->rev, pool));
>
> + if (cb->old_value_p)
> + {
Again, that's not going to work for adds.
> + const svn_string_t *wanted_value = *cb->old_value_p;
> + const svn_string_t *present_value = apr_hash_get(table, cb->name,
> + APR_HASH_KEY_STRING);
> + if ((!wanted_value != !present_value)
> + || (wanted_value && present_value
> + && !svn_string_compare(wanted_value, present_value)))
> + {
> + /* What we expected isn't what we found. */
> + return svn_error_createf(SVN_ERR_BAD_PROPERTY_VALUE, NULL,
> + _("revprop '%s' has unexpected value in "
> + "filesystem"),
> + cb->name);
> + }
> + /* Fall through. */
> + }
> --- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
> +++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Wed Jun 16 06:05:17 2010
> @@ -593,8 +593,11 @@ revision_props(const svn_test_opts_t *op
> svn_fs_t *fs;
> apr_hash_t *proplist;
> svn_string_t *value;
> + svn_error_t *err;
> int i;
> svn_string_t s1;
> + const svn_string_t s2 = { "wrong value", 11 };
> + const svn_string_t *s2_p = &s2;
>
> const char *initial_props[4][2] = {
> { "color", "red" },
> @@ -638,6 +641,14 @@ revision_props(const svn_test_opts_t *op
> s1.len = value->len;
> SVN_ERR(svn_fs_change_rev_prop(fs, 0, "flower", &s1, pool));
>
> + err = svn_fs_change_rev_prop2(fs, 0, "flower", &s2_p, &s1, pool);
> + if (!err || err->apr_err != SVN_ERR_BAD_PROPERTY_VALUE)
> + return svn_error_create(SVN_ERR_TEST_FAILED, err,
> + "svn_fs_change_rev_prop2() failed to "
> + "detect unexpected old value");
> + else
> + svn_error_clear(err);
I'd like to see adds, modifications and deletes all tested to pass and
fail.
> +
> /* Obtain a list of all current properties, and make sure it matches
> the expected values. */
> SVN_ERR(svn_fs_revision_proplist(&proplist, fs, 0, pool));
>
>
--
Philip
Received on 2010-06-16 10:18:38 CEST