On Wed, Mar 01 2023, Kevin Boulain wrote: > _notmuch_message_remove_all_properties wasn't syncing the message back > to the database but was still invalidating the metadata, giving the > impression the properties had actually been removed. > > Also move the metadata invalidation to _notmuch_message_remove_terms > to be closer to what's done in _notmuch_message_modify_property and > _notmuch_message_remove_term.
I don't understand much of this change, but at least code changes are simple enough to not look broken (cannot know about flow, I could not find connection). Somehow testkey1 = testvalue1 disappeared from the test code (which is probably expected -- perhaps the commit message of the *change* 1/2 tried to point to that ;D) So, changes looks good, but cannot say if anything works better... > --- > Don't we need to talloc_free the talloc_asprintf'd term_prefix? the `message` is (probably?) talloc_free()d at some point, and at that time the talloc_asprintf'd data (with message as a reference) will be freed. Tomi > > lib/message-property.cc | 4 ++- > lib/message.cc | 2 ++ > test/T610-message-property.sh | 61 ++++++++++++++++------------------- > 3 files changed, 33 insertions(+), 34 deletions(-) > > diff --git a/lib/message-property.cc b/lib/message-property.cc > index d5afa30c..0d658038 100644 > --- a/lib/message-property.cc > +++ b/lib/message-property.cc > @@ -123,7 +123,6 @@ _notmuch_message_remove_all_properties (notmuch_message_t > *message, const char * > if (status) > return status; > > - _notmuch_message_invalidate_metadata (message, "property"); > if (key) > term_prefix = talloc_asprintf (message, "%s%s%s", _find_prefix > ("property"), key, > prefix ? "" : "="); > @@ -133,6 +132,9 @@ _notmuch_message_remove_all_properties (notmuch_message_t > *message, const char * > /* XXX better error reporting ? */ > _notmuch_message_remove_terms (message, term_prefix); > > + if (! _notmuch_message_frozen (message)) > + _notmuch_message_sync (message); > + > return NOTMUCH_STATUS_SUCCESS; > } > > diff --git a/lib/message.cc b/lib/message.cc > index 1b1a071a..53f35dd1 100644 > --- a/lib/message.cc > +++ b/lib/message.cc > @@ -719,6 +719,8 @@ _notmuch_message_remove_terms (notmuch_message_t > *message, const char *prefix) > /* Ignore failure to remove non-existent term. */ > } > } > + > + _notmuch_message_invalidate_metadata (message, "property"); > } > > > diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh > index 7ebddae3..dd397e16 100755 > --- a/test/T610-message-property.sh > +++ b/test/T610-message-property.sh > @@ -89,17 +89,6 @@ testkey2 = NULL > EOF > test_expect_equal_file EXPECTED OUTPUT > > -test_begin_subtest "notmuch_message_remove_all_properties" > -cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} > -EXPECT0(notmuch_message_remove_all_properties (message, NULL)); > -print_properties (message, "", FALSE); > -EOF > -cat <<'EOF' >EXPECTED > -== stdout == > -== stderr == > -EOF > -test_expect_equal_file EXPECTED OUTPUT > - > test_begin_subtest "testing string map binary search (via message > properties)" > cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} > { > @@ -162,6 +151,17 @@ testkey1 = testvalue1 > EOF > test_expect_equal_file EXPECTED OUTPUT > > +test_begin_subtest "notmuch_message_remove_all_properties" > +cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} > +EXPECT0(notmuch_message_remove_all_properties (message, NULL)); > +print_properties (message, "", FALSE); > +EOF > +cat <<'EOF' >EXPECTED > +== stdout == > +== stderr == > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > test_begin_subtest "notmuch_message_properties: multiple values" > cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} > EXPECT0(notmuch_message_add_property (message, "testkey1", "bob")); > @@ -173,7 +173,6 @@ cat <<'EOF' >EXPECTED > == stdout == > testkey1 = alice > testkey1 = bob > -testkey1 = testvalue1 > testkey1 = testvalue2 > == stderr == > EOF > @@ -186,23 +185,10 @@ EXPECT0(notmuch_message_add_property (message, > "testkey3", "testvalue3")); > EXPECT0(notmuch_message_add_property (message, "testkey3", "alice3")); > print_properties (message, "testkey", FALSE); > EOF > -# expected: 4 values for testkey1, 3 values for testkey3 > -# they are not guaranteed to be sorted, so sort them, leaving the first > -# line '== stdout ==' and the end ('== stderr ==' and whatever error > -# may have been printed) alone > -mv OUTPUT unsorted_OUTPUT > -awk ' NR == 1 { print; next } \ > - NR < 6 { print | "sort"; next } \ > - NR == 6 { close("sort") } \ > - NR < 9 { print | "sort"; next } \ > - NR == 9 { close("sort") } \ > - { print }' unsorted_OUTPUT > OUTPUT > -rm unsorted_OUTPUT > cat <<'EOF' >EXPECTED > == stdout == > testkey1 = alice > testkey1 = bob > -testkey1 = testvalue1 > testkey1 = testvalue2 > testkey3 = alice3 > testkey3 = bob3 > @@ -246,9 +232,23 @@ cat <<'EOF' >EXPECTED > EOF > test_expect_equal_file EXPECTED OUTPUT > > +test_begin_subtest "notmuch_message_remove_all_properties_with_prefix" > +cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} > +EXPECT0(notmuch_message_remove_all_properties_with_prefix (message, > "testkey3")); > +print_properties (message, "", FALSE); > +EOF > +cat <<'EOF' >EXPECTED > +== stdout == > +testkey1 = alice > +testkey1 = bob > +testkey1 = testvalue2 > +== stderr == > +EOF > +test_expect_equal_file EXPECTED OUTPUT > + > test_begin_subtest "dump message properties" > cat <<EOF > PROPERTIES > -#= 4efc743a.3060...@april.org > fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20= > testkey1=alice testkey1=bob testkey1=testvalue1 testkey1=testvalue2 > testkey3=alice3 testkey3=bob3 testkey3=testvalue3 > +#= 4efc743a.3060...@april.org > fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20= > testkey1=alice testkey1=bob testkey1=testvalue2 > EOF > cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} > EXPECT0(notmuch_message_add_property (message, "fancy key with áccènts", > "import value with =")); > @@ -259,7 +259,7 @@ test_expect_equal_file PROPERTIES OUTPUT > test_begin_subtest "dump _only_ message properties" > cat <<EOF > EXPECTED > #notmuch-dump batch-tag:3 properties > -#= 4efc743a.3060...@april.org > fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20= > testkey1=alice testkey1=bob testkey1=testvalue1 testkey1=testvalue2 > testkey3=alice3 testkey3=bob3 testkey3=testvalue3 > +#= 4efc743a.3060...@april.org > fancy%20key%20with%20%c3%a1cc%c3%a8nts=import%20value%20with%20= > testkey1=alice testkey1=bob testkey1=testvalue2 > EOF > notmuch dump --include=properties > OUTPUT > test_expect_equal_file EXPECTED OUTPUT > @@ -313,7 +313,7 @@ print("testkey3 = > {0}".format(msg.get_property("testkey3"))) > EOF > cat <<'EOF' > EXPECTED > testkey1 = alice > -testkey3 = alice3 > +testkey3 = None > EOF > test_expect_equal_file EXPECTED OUTPUT > > @@ -328,7 +328,6 @@ EOF > cat <<'EOF' > EXPECTED > testkey1 = alice > testkey1 = bob > -testkey1 = testvalue1 > testkey1 = testvalue2 > EOF > test_expect_equal_file EXPECTED OUTPUT > @@ -344,11 +343,7 @@ EOF > cat <<'EOF' > EXPECTED > testkey1 = alice > testkey1 = bob > -testkey1 = testvalue1 > testkey1 = testvalue2 > -testkey3 = alice3 > -testkey3 = bob3 > -testkey3 = testvalue3 > EOF > test_expect_equal_file EXPECTED OUTPUT > > _______________________________________________ > notmuch mailing list -- notmuch@notmuchmail.org > To unsubscribe send an email to notmuch-le...@notmuchmail.org _______________________________________________ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org