message properties patches, v1.0

message properties patches, v1.0

[David Bremner]

There have been two rounds of RFC/WIP

      id:1463927339-5441-1-git-send-email-david@tethera.net
      id:1464608999-14774-1-git-send-email-david@tethera.net

This is the first feature complete version, and (compared to the
previous one) includes docs, more tests, and dump/restore support.

There is one notable API change: I added a convenience function to
remove all tags. More importantly I adjusted the memory semantics of
the iterators so that the underlying map objects are kept alive. This
is the same issue alread present in the tag iterator code; when the
message metadata cache is invalidated, we don't want the iterator to
start segfaulting, otherwise the iterators are effectively read-only.

[PATCH 1/8] lib: read "property" terms from messages.

[David Bremner]

This is a first step towards providing an API to attach
arbitrary (key,value) pairs to messages and retrieve all of the values
for a given key.
---
 lib/database.cc       |  1 +
 lib/message.cc        | 29 ++++++++++++++++++++++++++++-
 lib/notmuch-private.h |  3 +++
 3 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/lib/database.cc b/lib/database.cc
index afafe88..74f31f5 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -245,6 +245,7 @@ static prefix_t BOOLEAN_PREFIX_INTERNAL[] = {
     { "directory",		"XDIRECTORY" },
     { "file-direntry",		"XFDIRENTRY" },
     { "directory-direntry",	"XDDIRENTRY" },
+    { "property",               "XPROPERTY"  },
 };
 
 static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {
diff --git a/lib/message.cc b/lib/message.cc
index 24e698a..63a8da5 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -37,6 +37,8 @@ struct visible _notmuch_message {
     notmuch_string_list_t *filename_list;
     char *author;
     notmuch_message_file_t *message_file;
+    notmuch_string_list_t *property_term_list;
+    notmuch_string_map_t *property_map;
     notmuch_message_list_t *replies;
     unsigned long flags;
     /* For flags that are initialized on-demand, lazy_flags indicates
@@ -116,6 +118,8 @@ _notmuch_message_create_for_document (const void *talloc_owner,
     message->filename_list = NULL;
     message->message_file = NULL;
     message->author = NULL;
+    message->property_term_list = NULL;
+    message->property_map = NULL;
 
     message->replies = _notmuch_message_list_create (message);
     if (unlikely (message->replies == NULL)) {
@@ -314,6 +318,7 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message)
 	*id_prefix = _find_prefix ("id"),
 	*type_prefix = _find_prefix ("type"),
 	*filename_prefix = _find_prefix ("file-direntry"),
+	*property_prefix = _find_prefix ("property"),
 	*replyto_prefix = _find_prefix ("replyto");
 
     /* We do this all in a single pass because Xapian decompresses the
@@ -369,11 +374,21 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message)
 	    _notmuch_database_get_terms_with_prefix (message, i, end,
 						     filename_prefix);
 
+
+    /* Get property terms. Mimic the setup with filenames above */
+    assert (strcmp (filename_prefix, property_prefix) < 0);
+    if (!message->property_map && !message->property_term_list)
+	message->property_term_list =
+	    _notmuch_database_get_terms_with_prefix (message, i, end,
+						     property_prefix);
+
     /* Get reply to */
-    assert (strcmp (filename_prefix, replyto_prefix) < 0);
+    assert (strcmp (property_prefix, replyto_prefix) < 0);
     if (!message->in_reply_to)
 	message->in_reply_to =
 	    _notmuch_message_get_term (message, i, end, replyto_prefix);
+
+
     /* It's perfectly valid for a message to have no In-Reply-To
      * header. For these cases, we return an empty string. */
     if (!message->in_reply_to)
@@ -405,6 +420,18 @@ _notmuch_message_invalidate_metadata (notmuch_message_t *message,
 	message->filename_term_list = message->filename_list = NULL;
     }
 
+    if (strcmp ("property", prefix_name) == 0) {
+
+	if (message->property_term_list)
+	    talloc_free (message->property_term_list);
+	message->property_term_list = NULL;
+
+	if (message->property_map)
+	    talloc_unlink (message, message->property_map);
+
+	message->property_map = NULL;
+    }
+
     if (strcmp ("replyto", prefix_name) == 0) {
 	talloc_free (message->in_reply_to);
 	message->in_reply_to = NULL;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 3721431..df63905 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -537,6 +537,9 @@ _notmuch_string_list_append (notmuch_string_list_t *list,
 void
 _notmuch_string_list_sort (notmuch_string_list_t *list);
 
+/* string-map.c */
+typedef struct _notmuch_string_map  notmuch_string_map_t;
+
 /* tags.c */
 
 notmuch_tags_t *
-- 
2.8.1

[PATCH 2/8] lib: private string map (associative array) API

[David Bremner]

The choice of array implementation is deliberate, for future iterator support
---
 lib/Makefile.local    |   1 +
 lib/notmuch-private.h |  11 ++++
 lib/string-map.c      | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 165 insertions(+)
 create mode 100644 lib/string-map.c

diff --git a/lib/Makefile.local b/lib/Makefile.local
index beb9635..9280880 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -40,6 +40,7 @@ libnotmuch_c_srcs =		\
 	$(dir)/messages.c	\
 	$(dir)/sha1.c		\
 	$(dir)/built-with.c	\
+	$(dir)/string-map.c    \
 	$(dir)/tags.c
 
 libnotmuch_cxx_srcs =		\
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index df63905..5abde88 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -540,6 +540,17 @@ _notmuch_string_list_sort (notmuch_string_list_t *list);
 /* string-map.c */
 typedef struct _notmuch_string_map  notmuch_string_map_t;
 
+notmuch_string_map_t *
+_notmuch_string_map_create (const void *ctx);
+
+void
+_notmuch_string_map_append (notmuch_string_map_t *map,
+			    const char *key,
+			    const char *value);
+
+const char *
+_notmuch_string_map_get (notmuch_string_map_t *map, const char *key);
+
 /* tags.c */
 
 notmuch_tags_t *
diff --git a/lib/string-map.c b/lib/string-map.c
new file mode 100644
index 0000000..0491a10
--- /dev/null
+++ b/lib/string-map.c
@@ -0,0 +1,153 @@
+/* string-map.c - associative arrays of strings
+ *
+ *
+ * Copyright © 2016 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner <david@tethera.net>
+ */
+
+#include "notmuch-private.h"
+
+/* Create a new notmuch_string_map_t object, with 'ctx' as its
+ * talloc owner.
+ *
+ * This function can return NULL in case of out-of-memory.
+ */
+
+typedef struct _notmuch_string_pair_t {
+    char *key;
+    char *value;
+} notmuch_string_pair_t;
+
+struct _notmuch_string_map {
+    notmuch_bool_t sorted;
+    size_t length;
+    notmuch_string_pair_t *pairs;
+};
+
+notmuch_string_map_t *
+_notmuch_string_map_create (const void *ctx)
+{
+    notmuch_string_map_t *map;
+
+    map = talloc (ctx, notmuch_string_map_t);
+    if (unlikely (map == NULL))
+	return NULL;
+
+    map->length = 0;
+    map->pairs = NULL;
+    map->sorted = TRUE;
+
+    return map;
+}
+
+void
+_notmuch_string_map_append (notmuch_string_map_t *map,
+			    const char *key,
+			    const char *value)
+{
+
+    map->length++;
+    map->sorted = FALSE;
+
+    if (map->pairs)
+	map->pairs = talloc_realloc (map, map->pairs, notmuch_string_pair_t, map->length + 1);
+    else
+	map->pairs = talloc_array (map, notmuch_string_pair_t, map->length + 1);
+
+    map->pairs[map->length - 1].key = talloc_strdup (map, key);
+    map->pairs[map->length - 1].value = talloc_strdup (map, value);
+
+    /* Add sentinel */
+    map->pairs[map->length].key = NULL;
+    map->pairs[map->length].value = NULL;
+
+}
+
+static int
+cmppair (const void *pa, const void *pb)
+{
+    notmuch_string_pair_t *a = (notmuch_string_pair_t *) pa;
+    notmuch_string_pair_t *b = (notmuch_string_pair_t *) pb;
+
+    return strcmp (a->key, b->key);
+}
+
+static void
+_notmuch_string_map_sort (notmuch_string_map_t *map)
+{
+    if (map->length == 0)
+	return;
+
+    if (map->sorted)
+	return;
+
+    qsort (map->pairs, map->length, sizeof (notmuch_string_pair_t), cmppair);
+
+    map->sorted = TRUE;
+}
+
+static notmuch_bool_t
+string_cmp (const char *a, const char *b, notmuch_bool_t exact)
+{
+    if (exact)
+	return (strcmp (a, b));
+    else
+	return (strncmp (a, b, strlen (a)));
+}
+
+static notmuch_string_pair_t *
+bsearch_first (notmuch_string_pair_t *array, size_t len, const char *key, notmuch_bool_t exact)
+{
+    size_t first = 0;
+    size_t last = len - 1;
+    size_t mid;
+
+    if (len <= 0)
+	return NULL;
+
+    while (last > first) {
+	mid = (first + last) / 2;
+	int sign = string_cmp (key, array[mid].key, exact);
+
+	if (sign <= 0)
+	    last = mid;
+	else
+	    first = mid + 1;
+    }
+
+
+    if (string_cmp (key, array[first].key, exact) == 0)
+	return array + first;
+    else
+	return NULL;
+
+}
+
+const char *
+_notmuch_string_map_get (notmuch_string_map_t *map, const char *key)
+{
+    notmuch_string_pair_t *pair;
+
+    /* this means that calling append invalidates iterators */
+    _notmuch_string_map_sort (map);
+
+    pair = bsearch_first (map->pairs, map->length, key, TRUE);
+    if (! pair)
+	return NULL;
+
+    return pair->value;
+}
-- 
2.8.1

Re: [PATCH 2/8] lib: private string map (associative array) API

[Tomi Ollila]

On Mon, Jun 13 2016, David Bremner <david@tethera.net> wrote:

> The choice of array implementation is deliberate, for future iterator support
> ---
>  lib/Makefile.local    |   1 +
>  lib/notmuch-private.h |  11 ++++
>  lib/string-map.c      | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 165 insertions(+)
>  create mode 100644 lib/string-map.c
>
> diff --git a/lib/Makefile.local b/lib/Makefile.local
> index beb9635..9280880 100644
> --- a/lib/Makefile.local
> +++ b/lib/Makefile.local
> @@ -40,6 +40,7 @@ libnotmuch_c_srcs =		\
>  	$(dir)/messages.c	\
>  	$(dir)/sha1.c		\
>  	$(dir)/built-with.c	\
> +	$(dir)/string-map.c    \
>  	$(dir)/tags.c

I suggest everyone to install editor which supports showing tabs & spaces
e.g. in different color and also visualizes trailing whitespace & trailing
empty lines...

... personally I've been pretty happy with ethan-wspace in emacs to do
that (I did M-x ethan-wspace-highlight-tabs-mode (in notmuch-show mode
buffer) to recognize the above).

If there is no need to further changes then the above can IMO go in --
the same "dirtiness" can be observed in $(dir)/query-fp.cc 10 lines below...

HTH.

Tomi

Re: [PATCH 2/8] lib: private string map (associative array) API

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Mon, Jun 13 2016, David Bremner <david@tethera.net> wrote:
>
>> The choice of array implementation is deliberate, for future iterator support
>> ---
>>  lib/Makefile.local    |   1 +
>>  lib/notmuch-private.h |  11 ++++
>>  lib/string-map.c      | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 165 insertions(+)
>>  create mode 100644 lib/string-map.c
>>
>> diff --git a/lib/Makefile.local b/lib/Makefile.local
>> index beb9635..9280880 100644
>> --- a/lib/Makefile.local
>> +++ b/lib/Makefile.local
>> @@ -40,6 +40,7 @@ libnotmuch_c_srcs =		\
>>  	$(dir)/messages.c	\
>>  	$(dir)/sha1.c		\
>>  	$(dir)/built-with.c	\
>> +	$(dir)/string-map.c    \
>>  	$(dir)/tags.c
>
> I suggest everyone to install editor which supports showing tabs & spaces
> e.g. in different color and also visualizes trailing whitespace & trailing
> empty lines...
>

Heh. If only such a thing existed. What about a pre-commit hook?
The default (that I have) uses git diff --index --check --cached, but
that seems not to be enough.

d

Re: [PATCH 2/8] lib: private string map (associative array) API

[Bijan Chokoufe Nejad]

On 16-06-13, David Bremner wrote:
> Tomi Ollila <tomi.ollila@iki.fi> writes:
> 
> > On Mon, Jun 13 2016, David Bremner <david@tethera.net> wrote:
> >
> >> The choice of array implementation is deliberate, for future iterator support
> >> ---
> >>  lib/Makefile.local    |   1 +
> >>  lib/notmuch-private.h |  11 ++++
> >>  lib/string-map.c      | 153 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  3 files changed, 165 insertions(+)
> >>  create mode 100644 lib/string-map.c
> >>
> >> diff --git a/lib/Makefile.local b/lib/Makefile.local
> >> index beb9635..9280880 100644
> >> --- a/lib/Makefile.local
> >> +++ b/lib/Makefile.local
> >> @@ -40,6 +40,7 @@ libnotmuch_c_srcs =		\
> >>  	$(dir)/messages.c	\
> >>  	$(dir)/sha1.c		\
> >>  	$(dir)/built-with.c	\
> >> +	$(dir)/string-map.c    \
> >>  	$(dir)/tags.c
> >
> > I suggest everyone to install editor which supports showing tabs & spaces
> > e.g. in different color and also visualizes trailing whitespace & trailing
> > empty lines...
> >
> 
> Heh. If only such a thing existed. What about a pre-commit hook?
> The default (that I have) uses git diff --index --check --cached, but
> that seems not to be enough.
> 
> d
> _______________________________________________

I know you guys are more in the emacs camp but just for reference/inspiration:
in VIM I have a mapping to strip trailing spaces from a file

  noremap <leader>st :%s/\s\+$/<CR>

This could of course go as a sed/grep command in the pre-commit hook. Concerning
the tabs, I have

  " Tabs. Note: Use :retab to clean up mixed indentation
  set expandtab           " Always uses spaces instead of tab characters
  set tabstop=2           " Size of insterted spaces if tab is pressed
  set list                " Highlight tab characters in files
  set listchars=tab:▸\ ,extends:#,nbsp:.,trail:⋅

For the pre-commit hook one could `grep -P '\t' *`. Maybe even better would be
the use of a linter but I am not sure what is most flexible for C code.

Cheers,
Bijan

[PATCH 3/8] lib: basic message-property API

[David Bremner]

Initially, support get, set and removal of single key/value pair, as
well as removing all properties.
---
 lib/Makefile.local            |   1 +
 lib/message-private.h         |  16 +++++++
 lib/message-property.cc       | 101 ++++++++++++++++++++++++++++++++++++++++++
 lib/message.cc                |  52 +++++++++++++++++++++-
 lib/notmuch.h                 |  69 +++++++++++++++++++++++++++++
 test/T610-message-property.sh |  84 +++++++++++++++++++++++++++++++++++
 6 files changed, 321 insertions(+), 2 deletions(-)
 create mode 100644 lib/message-private.h
 create mode 100644 lib/message-property.cc
 create mode 100755 test/T610-message-property.sh

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 9280880..c012ed1 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -49,6 +49,7 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/directory.cc	\
 	$(dir)/index.cc		\
 	$(dir)/message.cc	\
+	$(dir)/message-property.cc \
 	$(dir)/query.cc		\
 	$(dir)/query-fp.cc      \
 	$(dir)/config.cc	\
diff --git a/lib/message-private.h b/lib/message-private.h
new file mode 100644
index 0000000..7419925
--- /dev/null
+++ b/lib/message-private.h
@@ -0,0 +1,16 @@
+#ifndef MESSAGE_PRIVATE_H
+#define MESSAGE_PRIVATE_H
+
+notmuch_string_map_t *
+_notmuch_message_property_map (notmuch_message_t *message);
+
+notmuch_bool_t
+_notmuch_message_frozen (notmuch_message_t *message);
+
+void
+_notmuch_message_remove_terms (notmuch_message_t *message, const char *prefix);
+
+void
+_notmuch_message_invalidate_metadata (notmuch_message_t *message,  const char *prefix_name);
+
+#endif
diff --git a/lib/message-property.cc b/lib/message-property.cc
new file mode 100644
index 0000000..ad2250f
--- /dev/null
+++ b/lib/message-property.cc
@@ -0,0 +1,101 @@
+/* message-property.cc - Properties are like tags, but (key,value) pairs.
+ * keys are allowed to repeat.
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2016 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner <david@tethera.net>
+ */
+
+#include "notmuch-private.h"
+#include "database-private.h"
+#include "message-private.h"
+
+notmuch_status_t
+notmuch_message_get_property (notmuch_message_t *message, const char *key, const char **value)
+{
+    if (! value)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    *value = _notmuch_string_map_get (_notmuch_message_property_map (message), key);
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+static notmuch_status_t
+_notmuch_message_modify_property (notmuch_message_t *message, const char *key, const char *value,
+				  notmuch_bool_t delete_it)
+{
+    notmuch_private_status_t private_status;
+    notmuch_status_t status;
+    char *term = NULL;
+
+    status = _notmuch_database_ensure_writable (_notmuch_message_database (message));
+    if (status)
+	return status;
+
+    if (key == NULL || value == NULL)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    if (index (key, '='))
+	return NOTMUCH_STATUS_ILLEGAL_ARGUMENT;
+
+    term = talloc_asprintf (message, "%s=%s", key, value);
+
+    if (delete_it)
+	private_status = _notmuch_message_remove_term (message, "property", term);
+    else
+	private_status = _notmuch_message_add_term (message, "property", term);
+
+    if (private_status)
+	return COERCE_STATUS (private_status,
+			      "Unhandled error modifying message property");
+    if (! _notmuch_message_frozen (message))
+	_notmuch_message_sync (message);
+
+    if (term)
+	talloc_free (term);
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+notmuch_status_t
+notmuch_message_add_property (notmuch_message_t *message, const char *key, const char *value)
+{
+    return _notmuch_message_modify_property (message, key, value, FALSE);
+}
+
+notmuch_status_t
+notmuch_message_remove_property (notmuch_message_t *message, const char *key, const char *value)
+{
+    return _notmuch_message_modify_property (message, key, value, TRUE);
+}
+
+notmuch_status_t
+notmuch_message_remove_all_properties (notmuch_message_t *message)
+{
+    notmuch_status_t status;
+    status = _notmuch_database_ensure_writable (_notmuch_message_database (message));
+    if (status)
+	return status;
+
+    _notmuch_message_invalidate_metadata (message, "property");
+    /* XXX better error reporting ? */
+    _notmuch_message_remove_terms (message, _find_prefix ("property"));
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
diff --git a/lib/message.cc b/lib/message.cc
index 63a8da5..9d3e807 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -20,6 +20,7 @@
 
 #include "notmuch-private.h"
 #include "database-private.h"
+#include "message-private.h"
 
 #include <stdint.h>
 
@@ -395,7 +396,7 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message)
 	message->in_reply_to = talloc_strdup (message, "");
 }
 
-static void
+void
 _notmuch_message_invalidate_metadata (notmuch_message_t *message,
 				      const char *prefix_name)
 {
@@ -552,7 +553,7 @@ notmuch_message_get_replies (notmuch_message_t *message)
     return _notmuch_messages_create (message->replies);
 }
 
-static void
+void
 _notmuch_message_remove_terms (notmuch_message_t *message, const char *prefix)
 {
     Xapian::TermIterator i;
@@ -1799,3 +1800,50 @@ _notmuch_message_database (notmuch_message_t *message)
 {
     return message->notmuch;
 }
+
+void
+_notmuch_message_ensure_property_map (notmuch_message_t *message)
+{
+    notmuch_string_node_t *node;
+
+    if (message->property_map)
+	return;
+
+    if (!message->property_term_list)
+	_notmuch_message_ensure_metadata (message);
+
+    message->property_map = _notmuch_string_map_create (message);
+
+    for (node = message->property_term_list->head; node; node = node->next) {
+	const char *key;
+	char *value;
+
+	value = index(node->string, '=');
+	if (!value)
+	    INTERNAL_ERROR ("malformed property term");
+
+	*value = '\0';
+	value++;
+	key = node->string;
+
+	_notmuch_string_map_append (message->property_map, key, value);
+
+    }
+
+    talloc_free (message->property_term_list);
+    message->property_term_list = NULL;
+}
+
+notmuch_string_map_t *
+_notmuch_message_property_map (notmuch_message_t *message)
+{
+    _notmuch_message_ensure_property_map (message);
+
+    return message->property_map;
+}
+
+notmuch_bool_t
+_notmuch_message_frozen (notmuch_message_t *message)
+{
+    return message->frozen;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index d4a97cb..afc48c6 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -180,6 +180,11 @@ typedef enum _notmuch_status {
      */
     NOTMUCH_STATUS_PATH_ERROR,
     /**
+     * One of the arguments violates the preconditions for the
+     * function, in a way not covered by a more specific argument.
+     */
+    NOTMUCH_STATUS_ILLEGAL_ARGUMENT,
+    /**
      * Not an actual status value. Just a way to find out how many
      * valid status values there are.
      */
@@ -1652,6 +1657,70 @@ void
 notmuch_message_destroy (notmuch_message_t *message);
 
 /**
+ * @name Message Properties
+ *
+ * This interface provides the ability to attach arbitrary (key,value)
+ * string pairs to a message, to remove such pairs, and to iterate
+ * over them.  The caller should take some care as to what keys they
+ * add or delete values for, as other subsystems or extensions may
+ * depend on these properties.
+ *
+ */
+/**@{*/
+/**
+ * Retrieve the value for a single property key
+ *
+ * *value* is set to a string owned by the message or NULL if there is
+ * no such key. In the case of multiple values for the given key, the
+ * first one is retrieved.
+ *
+ * @returns
+ * - NOTMUCH_STATUS_NULL_POINTER: *value* may not be NULL.
+ * - NOTMUCH_STATUS_SUCCESS: No error occured.
+
+ */
+notmuch_status_t
+notmuch_message_get_property (notmuch_message_t *message, const char *key, const char **value);
+
+/**
+ * Add a (key,value) pair to a message
+ *
+ * @returns
+ * - NOTMUCH_STATUS_ILLEGAL_ARGUMENT: *key* may not contain an '=' character.
+ * - NOTMUCH_STATUS_NULL_POINTER: Neither *key* nor *value* may be NULL.
+ * - NOTMUCH_STATUS_SUCCESS: No error occured.
+ */
+notmuch_status_t
+notmuch_message_add_property (notmuch_message_t *message, const char *key, const char *value);
+
+/**
+ * Remove a (key,value) pair from a message.
+ *
+ * It is not an error to remove a non-existant (key,value) pair
+ *
+ * @returns
+ * - NOTMUCH_STATUS_ILLEGAL_ARGUMENT: *key* may not contain an '=' character.
+ * - NOTMUCH_STATUS_NULL_POINTER: Neither *key* nor *value* may be NULL.
+ * - NOTMUCH_STATUS_SUCCESS: No error occured.
+ */
+notmuch_status_t
+notmuch_message_remove_property (notmuch_message_t *message, const char *key, const char *value);
+
+/**
+ * Remove all (key,value) pairs from the given message.
+ *
+ * @returns
+ * - NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in
+ *   read-only mode so message cannot be modified.
+ * - NOTMUCH_STATUS_SUCCESS: No error occured.
+ *
+ */
+notmuch_status_t
+notmuch_message_remove_all_properties (notmuch_message_t *message);
+
+/**@}*/
+
+/**
  * Is the given 'tags' iterator pointing at a valid tag.
  *
  * When this function returns TRUE, notmuch_tags_get will return a
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
new file mode 100755
index 0000000..0217950
--- /dev/null
+++ b/test/T610-message-property.sh
@@ -0,0 +1,84 @@
+#!/usr/bin/env bash
+test_description="message property API"
+
+. ./test-lib.sh || exit 1
+
+add_email_corpus
+
+cat <<EOF > c_head
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <notmuch-test.h>
+
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_message_t *message = NULL;
+   const char *val;
+   notmuch_status_t stat;
+
+   EXPECT0(notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db));
+   EXPECT0(notmuch_database_find_message(db, "4EFC743A.3060609@april.org", &message));
+   if (message == NULL) {
+	fprintf (stderr, "unable to find message");
+	exit (1);
+   }
+EOF
+
+cat <<EOF > c_tail
+   EXPECT0(notmuch_database_destroy(db));
+}
+EOF
+
+test_begin_subtest "notmuch_message_{add,get,remove}_property"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   EXPECT0(notmuch_message_add_property (message, "testkey1", "testvalue1"));
+   EXPECT0(notmuch_message_get_property (message, "testkey1", &val));
+   printf("testkey1[1] = %s\n", val);
+   EXPECT0(notmuch_message_add_property (message, "testkey2", "this value has spaces and = sign"));
+   EXPECT0(notmuch_message_get_property (message, "testkey1", &val));
+   printf("testkey1[2] = %s\n", val);
+   EXPECT0(notmuch_message_get_property (message, "testkey1", &val));
+
+   EXPECT0(notmuch_message_get_property (message, "testkey2", &val));
+   printf("testkey2 = %s\n", val);
+
+   /* Add second value for key */
+   EXPECT0(notmuch_message_add_property (message, "testkey2", "zztestvalue3"));
+   EXPECT0(notmuch_message_get_property (message, "testkey2", &val));
+   printf("testkey2 = %s\n", val);
+
+   /* remove first value for key */
+   EXPECT0(notmuch_message_remove_property (message, "testkey2", "this value has spaces and = sign"));
+   EXPECT0(notmuch_message_get_property (message, "testkey2", &val));
+   printf("testkey2 = %s\n", val);
+
+   /* remove non-existant value for key */
+   EXPECT0(notmuch_message_remove_property (message, "testkey2", "this value has spaces and = sign"));
+   EXPECT0(notmuch_message_get_property (message, "testkey2", &val));
+   printf("testkey2 = %s\n", val);
+
+   /* remove only value for key */
+   EXPECT0(notmuch_message_remove_property (message, "testkey2", "zztestvalue3"));
+   EXPECT0(notmuch_message_get_property (message, "testkey2", &val));
+   printf("testkey2 = %s\n", val == NULL ? "NULL" : val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+testkey1[1] = testvalue1
+testkey1[2] = testvalue1
+testkey2 = this value has spaces and = sign
+testkey2 = this value has spaces and = sign
+testkey2 = zztestvalue3
+testkey2 = zztestvalue3
+testkey2 = NULL
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+
+
+test_done
-- 
2.8.1

[PATCH] n_m_remove_property(msg, key, NULL) should removes all properties with key

[Daniel Kahn Gillmor]

We should not require value to be non-NULL when there is a perfectly
sensible semantic:

    notmuch_message_remove_property(msg, key, NULL)

should mean "remove all properties with the given key from msg"
---
 lib/message-property.cc | 28 ++++++++++++++++++++--------
 lib/notmuch.h           |  5 +++--
 2 files changed, 23 insertions(+), 10 deletions(-)

This should apply on top of bremner's "properties" series, making the
semantics slightly more useful.

diff --git a/lib/message-property.cc b/lib/message-property.cc
index 4fa6cac..338f3fb 100644
--- a/lib/message-property.cc
+++ b/lib/message-property.cc
@@ -48,22 +48,34 @@ _notmuch_message_modify_property (notmuch_message_t *message, const char *key, c
     if (status)
 	return status;
 
-    if (key == NULL || value == NULL)
+    if (key == NULL)
 	return NOTMUCH_STATUS_NULL_POINTER;
 
     if (index (key, '='))
 	return NOTMUCH_STATUS_ILLEGAL_ARGUMENT;
 
-    term = talloc_asprintf (message, "%s=%s", key, value);
+    if (value == NULL)
+    {
+	if (!delete_it)
+	    return NOTMUCH_STATUS_NULL_POINTER;
 
-    if (delete_it)
-	private_status = _notmuch_message_remove_term (message, "property", term);
+	term = talloc_asprintf (message, "%s%s=", _find_prefix ("property"), key);
+	_notmuch_message_remove_terms (message, term);
+    }
     else
-	private_status = _notmuch_message_add_term (message, "property", term);
+    {
+	term = talloc_asprintf (message, "%s=%s", key, value);
+
+	if (delete_it)
+	    private_status = _notmuch_message_remove_term (message, "property", term);
+	else
+	    private_status = _notmuch_message_add_term (message, "property", term);
+
+	if (private_status)
+	    return COERCE_STATUS (private_status,
+				  "Unhandled error modifying message property");
+    }
 
-    if (private_status)
-	return COERCE_STATUS (private_status,
-			      "Unhandled error modifying message property");
     if (! _notmuch_message_frozen (message))
 	_notmuch_message_sync (message);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f6bad67..0d667f5 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1698,11 +1698,12 @@ notmuch_message_add_property (notmuch_message_t *message, const char *key, const
 /**
  * Remove a (key,value) pair from a message.
  *
- * It is not an error to remove a non-existant (key,value) pair
+ * It is not an error to remove a non-existant (key,value) pair.  If
+ * *value* is NULL, remove all properties with the given key.
  *
  * @returns
  * - NOTMUCH_STATUS_ILLEGAL_ARGUMENT: *key* may not contain an '=' character.
- * - NOTMUCH_STATUS_NULL_POINTER: Neither *key* nor *value* may be NULL.
+ * - NOTMUCH_STATUS_NULL_POINTER: *key* may not be NULL.
  * - NOTMUCH_STATUS_SUCCESS: No error occured.
  */
 notmuch_status_t
-- 
2.8.1

[PATCH] RFC: all deleting all properties with a given key

[David Bremner]

---

I think I somewhat prefer this way of providing the same
functionality, because the control flow is simpler. If it seems
useful, we could forward remove_property with a NULL value to
remove_all_properties

 lib/message-property.cc | 11 +++++++++--
 lib/notmuch.h           |  5 ++++-
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/lib/message-property.cc b/lib/message-property.cc
index edc6f2f..c6cff33 100644
--- a/lib/message-property.cc
+++ b/lib/message-property.cc
@@ -98,16 +98,23 @@ notmuch_message_remove_property (notmuch_message_t *message, const char *key, co
 }
 
 notmuch_status_t
-notmuch_message_remove_all_properties (notmuch_message_t *message)
+notmuch_message_remove_all_properties (notmuch_message_t *message, const char *key)
 {
     notmuch_status_t status;
+    const char * term_prefix;
+
     status = _notmuch_database_ensure_writable (_notmuch_message_database (message));
     if (status)
 	return status;
 
     _notmuch_message_invalidate_metadata (message, "property");
+    if (key)
+	term_prefix = talloc_asprintf (message, "%s%s=", _find_prefix ("property"), key);
+    else
+	term_prefix = _find_prefix ("property");
+
     /* XXX better error reporting ? */
-    _notmuch_message_remove_terms (message, _find_prefix ("property"));
+    _notmuch_message_remove_terms (message, term_prefix);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 41aee3c..cf5de3e 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1712,6 +1712,9 @@ notmuch_message_remove_property (notmuch_message_t *message, const char *key, co
 /**
  * Remove all (key,value) pairs from the given message.
  *
+ * @param[in,out] message  message to operate on.
+ * @param[in]     key      key to delete properties for. If NULL, delete
+ *			   properties for all keys
  * @returns
  * - NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in
  *   read-only mode so message cannot be modified.
@@ -1719,7 +1722,7 @@ notmuch_message_remove_property (notmuch_message_t *message, const char *key, co
  *
  */
 notmuch_status_t
-notmuch_message_remove_all_properties (notmuch_message_t *message);
+notmuch_message_remove_all_properties (notmuch_message_t *message, const char *key);
 
 /**@}*/
 
-- 
2.8.1

Re: [PATCH] RFC: all deleting all properties with a given key

[Daniel Kahn Gillmor]

On Sat 2016-07-16 12:32:54 +0200, David Bremner wrote:

> I think I somewhat prefer this way of providing the same
> functionality, because the control flow is simpler. If it seems
> useful, we could forward remove_property with a NULL value to
> remove_all_properties

this looks good to me.

     --dkg

[PATCH 4/8] lib: extend private string map API with iterators

[David Bremner]

Support for prefix based iterators is perhaps overengineering, but I
wanted to mimic the existing database_config API.
---
 lib/notmuch-private.h | 21 ++++++++++++++-
 lib/string-map.c      | 72 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+), 1 deletion(-)

diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 5abde88..65f7ead 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -539,7 +539,7 @@ _notmuch_string_list_sort (notmuch_string_list_t *list);
 
 /* string-map.c */
 typedef struct _notmuch_string_map  notmuch_string_map_t;
-
+typedef struct _notmuch_string_map_iterator notmuch_string_map_iterator_t;
 notmuch_string_map_t *
 _notmuch_string_map_create (const void *ctx);
 
@@ -551,6 +551,25 @@ _notmuch_string_map_append (notmuch_string_map_t *map,
 const char *
 _notmuch_string_map_get (notmuch_string_map_t *map, const char *key);
 
+notmuch_string_map_iterator_t *
+_notmuch_string_map_iterator_create (notmuch_string_map_t *map, const char *key,
+				     notmuch_bool_t exact);
+
+notmuch_bool_t
+_notmuch_string_map_iterator_valid (notmuch_string_map_iterator_t *iter);
+
+void
+_notmuch_string_map_iterator_move_to_next (notmuch_string_map_iterator_t *iter);
+
+const char *
+_notmuch_string_map_iterator_key (notmuch_string_map_iterator_t *iterator);
+
+const char *
+_notmuch_string_map_iterator_value (notmuch_string_map_iterator_t *iterator);
+
+void
+_notmuch_string_map_iterator_destroy (notmuch_string_map_iterator_t *iterator);
+
 /* tags.c */
 
 notmuch_tags_t *
diff --git a/lib/string-map.c b/lib/string-map.c
index 0491a10..591ff6d 100644
--- a/lib/string-map.c
+++ b/lib/string-map.c
@@ -38,6 +38,12 @@ struct _notmuch_string_map {
     notmuch_string_pair_t *pairs;
 };
 
+struct _notmuch_string_map_iterator {
+    notmuch_string_pair_t *current;
+    notmuch_bool_t exact;
+    const char *key;
+};
+
 notmuch_string_map_t *
 _notmuch_string_map_create (const void *ctx)
 {
@@ -151,3 +157,69 @@ _notmuch_string_map_get (notmuch_string_map_t *map, const char *key)
 
     return pair->value;
 }
+
+notmuch_string_map_iterator_t *
+_notmuch_string_map_iterator_create (notmuch_string_map_t *map, const char *key,
+				     notmuch_bool_t exact)
+{
+    notmuch_string_map_iterator_t *iter;
+
+    _notmuch_string_map_sort (map);
+
+    iter = talloc (map, notmuch_string_map_iterator_t);
+    if (unlikely (iter == NULL))
+	return NULL;
+
+    iter->key = talloc_strdup (iter, key);
+    iter->exact = exact;
+    iter->current = bsearch_first (map->pairs, map->length, key, exact);
+    return iter;
+}
+
+notmuch_bool_t
+_notmuch_string_map_iterator_valid (notmuch_string_map_iterator_t *iterator)
+{
+    if (iterator->current == NULL)
+	return FALSE;
+
+    /* sentinel */
+    if (iterator->current->key == NULL)
+	return FALSE;
+
+    return (0 == string_cmp (iterator->key, iterator->current->key, iterator->exact));
+
+}
+
+void
+_notmuch_string_map_iterator_move_to_next (notmuch_string_map_iterator_t *iterator)
+{
+
+    if (! _notmuch_string_map_iterator_valid (iterator))
+	return;
+
+    (iterator->current)++;
+}
+
+const char *
+_notmuch_string_map_iterator_key (notmuch_string_map_iterator_t *iterator)
+{
+    if (! _notmuch_string_map_iterator_valid (iterator))
+	return NULL;
+
+    return iterator->current->key;
+}
+
+const char *
+_notmuch_string_map_iterator_value (notmuch_string_map_iterator_t *iterator)
+{
+    if (! _notmuch_string_map_iterator_valid (iterator))
+	return NULL;
+
+    return iterator->current->value;
+}
+
+void
+_notmuch_string_map_iterator_destroy (notmuch_string_map_iterator_t *iterator)
+{
+    talloc_free (iterator);
+}
-- 
2.8.1

[PATCH 5/8] lib: iterator API for message properties

[David Bremner]

This is a thin wrapper around the string map iterator API just introduced.
---
 lib/message-property.cc       |  38 +++++++++++++++
 lib/notmuch.h                 |  95 +++++++++++++++++++++++++++++++++++++
 test/T610-message-property.sh | 107 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 240 insertions(+)

diff --git a/lib/message-property.cc b/lib/message-property.cc
index ad2250f..4fa6cac 100644
--- a/lib/message-property.cc
+++ b/lib/message-property.cc
@@ -99,3 +99,41 @@ notmuch_message_remove_all_properties (notmuch_message_t *message)
 
     return NOTMUCH_STATUS_SUCCESS;
 }
+
+notmuch_message_properties_t *
+notmuch_message_get_properties (notmuch_message_t *message, const char *key, notmuch_bool_t exact)
+{
+    notmuch_string_map_t *map;
+    map = _notmuch_message_property_map (message);
+    return _notmuch_string_map_iterator_create (map, key, exact);
+}
+
+notmuch_bool_t
+notmuch_message_properties_valid (notmuch_message_properties_t *properties)
+{
+    return _notmuch_string_map_iterator_valid (properties);
+}
+
+void
+notmuch_message_properties_move_to_next (notmuch_message_properties_t *properties)
+{
+    return _notmuch_string_map_iterator_move_to_next (properties);
+}
+
+const char *
+notmuch_message_properties_key (notmuch_message_properties_t *properties)
+{
+    return _notmuch_string_map_iterator_key (properties);
+}
+
+const char *
+notmuch_message_properties_value (notmuch_message_properties_t *properties)
+{
+    return _notmuch_string_map_iterator_value (properties);
+}
+
+void
+notmuch_message_properties_destroy (notmuch_message_properties_t *properties)
+{
+    _notmuch_string_map_iterator_destroy (properties);
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index afc48c6..cdf94fd 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1718,6 +1718,101 @@ notmuch_message_remove_property (notmuch_message_t *message, const char *key, co
 notmuch_status_t
 notmuch_message_remove_all_properties (notmuch_message_t *message);
 
+/**
+ * Opaque message property iterator
+ */
+typedef struct _notmuch_string_map_iterator notmuch_message_properties_t;
+
+/**
+ * Get the properties for *message*, returning a
+ * notmuch_message_properties_t object which can be used to iterate
+ * over all properties.
+ *
+ * The notmuch_message_properties_t object is owned by the message and
+ * as such, will only be valid for as long as the message is valid,
+ * (which is until the query from which it derived is destroyed).
+ *
+ * @param[in] message  The message to examine
+ * @param[in] key      key or key prefix
+ * @param[in] exact    if TRUE, require exact match with key. Otherwise
+ *		       treat as prefix.
+ *
+ * Typical usage might be:
+ *
+ *     notmuch_message_properties_t *list;
+ *
+ *     for (list = notmuch_message_get_properties (message, "testkey1", TRUE);
+ *          notmuch_message_properties_valid (list); notmuch_message_properties_move_to_next (list)) {
+ *        printf("%s\n", notmuch_message_properties_value(list));
+ *     }
+ *
+ *     notmuch_message_properties_destroy (list);
+ *
+ * Note that there's no explicit destructor needed for the
+ * notmuch_message_properties_t object. (For consistency, we do
+ * provide a notmuch_message_properities_destroy function, but there's
+ * no good reason to call it if the message is about to be destroyed).
+ */
+notmuch_message_properties_t *
+notmuch_message_get_properties (notmuch_message_t *message, const char *key, notmuch_bool_t exact);
+
+/**
+ * Is the given *properties* iterator pointing at a valid (key,value)
+ * pair.
+ *
+ * When this function returns TRUE,
+ * notmuch_message_properties_{key,value} will return a valid string,
+ * and notmuch_message_properties_move_to_next will do what it
+ * says. Whereas when this function returns FALSE, calling any of
+ * these functions results in undefined behaviour.
+ *
+ * See the documentation of notmuch_message_properties_get for example
+ * code showing how to iterate over a notmuch_message_properties_t
+ * object.
+ */
+notmuch_bool_t
+notmuch_message_properties_valid (notmuch_message_properties_t *properties);
+
+/**
+ * Move the *properties* iterator to the next (key,value) pair
+ *
+ * If *properties* is already pointing at the last pair then the iterator
+ * will be moved to a point just beyond that last pair, (where
+ * notmuch_message_properties_valid will return FALSE).
+ *
+ * See the documentation of notmuch_message_get_properties for example
+ * code showing how to iterate over a notmuch_message_properties_t object.
+ */
+void
+notmuch_message_properties_move_to_next (notmuch_message_properties_t *properties);
+
+/**
+ * Return the key from the current (key,value) pair.
+ *
+ * this could be useful if iterating for a prefix
+ */
+const char *
+notmuch_message_properties_key (notmuch_message_properties_t *properties);
+
+/**
+ * Return the key from the current (key,value) pair.
+ *
+ * This could be useful if iterating for a prefix.
+ */
+const char *
+notmuch_message_properties_value (notmuch_message_properties_t *properties);
+
+
+/**
+ * Destroy a notmuch_message_properties_t object.
+ *
+ * It's not strictly necessary to call this function. All memory from
+ * the notmuch_message_properties_t object will be reclaimed when the
+ * containing message object is destroyed.
+ */
+void
+notmuch_message_properties_destroy (notmuch_message_properties_t *properties);
+
 /**@}*/
 
 /**
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index 0217950..b5ddb7a 100755
--- a/test/T610-message-property.sh
+++ b/test/T610-message-property.sh
@@ -9,8 +9,18 @@ cat <<EOF > c_head
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
+#include <talloc.h>
 #include <notmuch-test.h>
 
+void print_properties (notmuch_message_t *message, const char *prefix, notmuch_bool_t exact) {
+    notmuch_message_properties_t *list;
+    for (list = notmuch_message_get_properties (message, prefix, exact);
+         notmuch_message_properties_valid (list); notmuch_message_properties_move_to_next (list)) {
+       printf("%s\n", notmuch_message_properties_value(list));
+    }
+    notmuch_message_properties_destroy (list);
+}
+
 int main (int argc, char** argv)
 {
    notmuch_database_t *db;
@@ -79,6 +89,103 @@ testkey2 = NULL
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_message_get_properties: empty list"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   notmuch_message_properties_t *list;
+   list = notmuch_message_get_properties (message, "nonexistent", TRUE);
+   printf("valid = %d\n", notmuch_message_properties_valid (list));
+   notmuch_message_properties_destroy (list);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+valid = 0
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_message_properties: one value"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+print_properties (message, "testkey1", TRUE);
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+testvalue1
+== 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"));
+EXPECT0(notmuch_message_add_property (message, "testkey1", "testvalue2"));
+EXPECT0(notmuch_message_add_property (message, "testkey1", "alice"));
+print_properties (message, "testkey1", TRUE);
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+alice
+bob
+testvalue1
+testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "notmuch_message_properties: prefix"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+EXPECT0(notmuch_message_add_property (message, "testkey3", "bob3"));
+EXPECT0(notmuch_message_add_property (message, "testkey3", "testvalue3"));
+EXPECT0(notmuch_message_add_property (message, "testkey3", "alice3"));
+print_properties (message, "testkey", FALSE);
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+alice
+bob
+testvalue1
+testvalue2
+alice3
+bob3
+testvalue3
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "notmuch_message_properties: modify during iteration"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+    const char *keys[1000] = {NULL};
+    const char *vals[1000] = {NULL};
+    notmuch_message_properties_t *properties;
+    int i;
+
+    for (properties = notmuch_message_get_properties (message, "", FALSE), i=0;
+	 notmuch_message_properties_valid (properties);
+	 notmuch_message_properties_move_to_next (properties), i++)
+    {
+	const char *key, *value;
+
+	keys[i]=talloc_strdup(message,
+		    notmuch_message_properties_key (properties));
+        vals[i]=talloc_strdup(message,
+		    notmuch_message_properties_value (properties));
+
+	EXPECT0(notmuch_message_remove_property (message, keys[i], vals[i]));
+    }
+
+    print_properties (message, "", FALSE);
+
+    for (i = 0; keys[i] && vals[i]; i++) {
+        EXPECT0(notmuch_message_add_property (message, keys[i], vals[i]));
+    }
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
 
 test_done
-- 
2.8.1

[PATCH 6/8] CLI: refactor dumping of tags.

[David Bremner]

This is mainly code movement, to make room in the loop over messages for
dumping properties.
---
 notmuch-dump.c | 127 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 69 insertions(+), 58 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index cae1db8..d80ed8b8 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -78,13 +78,78 @@ print_dump_header (gzFile output, int output_format, int include)
 }
 
 static int
+dump_tags_message (void *ctx,
+		   notmuch_message_t *message, int output_format,
+		   gzFile output,
+		   char **buffer_p, size_t *size_p)
+{
+    int first = 1;
+    const char *message_id;
+
+    message_id = notmuch_message_get_message_id (message);
+
+    if (output_format == DUMP_FORMAT_BATCH_TAG &&
+	strchr (message_id, '\n')) {
+	/* This will produce a line break in the output, which
+	 * would be difficult to handle in tools.  However, it's
+	 * also impossible to produce an email containing a line
+	 * break in a message ID because of unfolding, so we can
+	 * safely disallow it. */
+	fprintf (stderr, "Warning: skipping message id containing line break: \"%s\"\n", message_id);
+	return EXIT_SUCCESS;
+    }
+
+    if (output_format == DUMP_FORMAT_SUP) {
+	gzprintf (output, "%s (", message_id);
+    }
+
+    for (notmuch_tags_t *tags = notmuch_message_get_tags (message);
+	 notmuch_tags_valid (tags);
+	 notmuch_tags_move_to_next (tags)) {
+	const char *tag_str = notmuch_tags_get (tags);
+
+	if (! first)
+	    gzputs (output, " ");
+
+	first = 0;
+
+	if (output_format == DUMP_FORMAT_SUP) {
+	    gzputs (output, tag_str);
+	} else {
+	    if (hex_encode (ctx, tag_str,
+			    buffer_p, size_p) != HEX_SUCCESS) {
+		fprintf (stderr, "Error: failed to hex-encode tag %s\n",
+			 tag_str);
+		return EXIT_FAILURE;
+	    }
+	    gzprintf (output, "+%s", *buffer_p);
+	}
+    }
+
+    if (output_format == DUMP_FORMAT_SUP) {
+	gzputs (output, ")\n");
+    } else {
+	if (make_boolean_term (ctx, "id", message_id,
+			       buffer_p, size_p)) {
+	    fprintf (stderr, "Error quoting message id %s: %s\n",
+		     message_id, strerror (errno));
+	    return EXIT_FAILURE;
+	}
+	gzprintf (output, " -- %s\n", *buffer_p);
+    }
+    return EXIT_SUCCESS;
+}
+
+static int
 database_dump_file (notmuch_database_t *notmuch, gzFile output,
 		    const char *query_str, int output_format, int include)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
-    notmuch_tags_t *tags;
+    notmuch_status_t status;
+    char *buffer = NULL;
+    size_t buffer_size = 0;
 
     print_dump_header (output, output_format, include);
 
@@ -110,10 +175,6 @@ database_dump_file (notmuch_database_t *notmuch, gzFile output,
      */
     notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
 
-    char *buffer = NULL;
-    size_t buffer_size = 0;
-    notmuch_status_t status;
-
     status = notmuch_query_search_messages_st (query, &messages);
     if (print_status_query ("notmuch dump", query, status))
 	return EXIT_FAILURE;
@@ -121,62 +182,12 @@ database_dump_file (notmuch_database_t *notmuch, gzFile output,
     for (;
 	 notmuch_messages_valid (messages);
 	 notmuch_messages_move_to_next (messages)) {
-	int first = 1;
-	const char *message_id;
 
 	message = notmuch_messages_get (messages);
-	message_id = notmuch_message_get_message_id (message);
-
-	if (output_format == DUMP_FORMAT_BATCH_TAG &&
-	    strchr (message_id, '\n')) {
-	    /* This will produce a line break in the output, which
-	     * would be difficult to handle in tools.  However, it's
-	     * also impossible to produce an email containing a line
-	     * break in a message ID because of unfolding, so we can
-	     * safely disallow it. */
-	    fprintf (stderr, "Warning: skipping message id containing line break: \"%s\"\n", message_id);
-	    notmuch_message_destroy (message);
-	    continue;
-	}
 
-	if (output_format == DUMP_FORMAT_SUP) {
-	    gzprintf (output, "%s (", message_id);
-	}
-
-	for (tags = notmuch_message_get_tags (message);
-	     notmuch_tags_valid (tags);
-	     notmuch_tags_move_to_next (tags)) {
-	    const char *tag_str = notmuch_tags_get (tags);
-
-	    if (! first)
-		gzputs (output, " ");
-
-	    first = 0;
-
-	    if (output_format == DUMP_FORMAT_SUP) {
-		gzputs (output, tag_str);
-	    } else {
-		if (hex_encode (notmuch, tag_str,
-				&buffer, &buffer_size) != HEX_SUCCESS) {
-		    fprintf (stderr, "Error: failed to hex-encode tag %s\n",
-			     tag_str);
-		    return EXIT_FAILURE;
-		}
-		gzprintf (output, "+%s", buffer);
-	    }
-	}
-
-	if (output_format == DUMP_FORMAT_SUP) {
-	    gzputs (output, ")\n");
-	} else {
-	    if (make_boolean_term (notmuch, "id", message_id,
-				   &buffer, &buffer_size)) {
-		    fprintf (stderr, "Error quoting message id %s: %s\n",
-			     message_id, strerror (errno));
-		    return EXIT_FAILURE;
-	    }
-	    gzprintf (output, " -- %s\n", buffer);
-	}
+	if (dump_tags_message (notmuch, message, output_format, output,
+			       &buffer, &buffer_size))
+	    return EXIT_FAILURE;
 
 	notmuch_message_destroy (message);
     }
-- 
2.8.1

[PATCH 7/8] CLI: add properties to dump output

[David Bremner]

Part of providing extensibility via properties is to make sure that user
data is not lost. Thus we need to be able to dump and restore
properties.
---
 doc/man1/notmuch-dump.rst     | 18 ++++++---
 notmuch-client.h              |  3 ++
 notmuch-dump.c                | 85 +++++++++++++++++++++++++++++++++++++++----
 notmuch-new.c                 |  2 +-
 test/T610-message-property.sh | 21 +++++++++++
 5 files changed, 116 insertions(+), 13 deletions(-)

diff --git a/doc/man1/notmuch-dump.rst b/doc/man1/notmuch-dump.rst
index 94986a8..751850b 100644
--- a/doc/man1/notmuch-dump.rst
+++ b/doc/man1/notmuch-dump.rst
@@ -71,7 +71,7 @@ Supported options for **dump** include
             characters. Note also that tags with spaces will not be
             correctly restored with this format.
 
-    ``--include=(config|tags)``
+    ``--include=(config|properties|tags)``
 
     Control what kind of metadata is included in the output.
 
@@ -81,14 +81,22 @@ Supported options for **dump** include
 	starts with "#@ ", followed by a space seperated key-value
 	pair.  Both key and value are hex encoded if needed.
 
+      **properties**
+
+	Output per-message (key,value) metadata.  Each line starts
+	with "#= ", followed by a message id, and a space seperated
+	list of key=value pairs.  pair.  Ids, keys and values are hex
+	encoded if needed.
+
       **tags**
 
-	Output per-message metadata, namely tags. See *format* above
+	Output per-message boolean metadata, namely tags. See *format* above
 	for description of the output.
 
-      The default is to include both tags and configuration
-      information. As of version 2 of the dump format, there is a
-      header line of the following form
+      The default is to include all available types of data.  The
+      option can be specified multiple times to select some subset. As
+      of version 2 of the dump format, there is a header line of the
+      following form
 
       |
       |  #notmuch-dump <*format*>:<*version*> <*included*>
diff --git a/notmuch-client.h b/notmuch-client.h
index ebc092b..9ce2aef 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -449,8 +449,11 @@ typedef enum dump_formats {
 typedef enum dump_includes {
     DUMP_INCLUDE_TAGS = 1,
     DUMP_INCLUDE_CONFIG = 2,
+    DUMP_INCLUDE_PROPERTIES = 4
 } dump_include_t;
 
+#define DUMP_INCLUDE_DEFAULT (DUMP_INCLUDE_TAGS | DUMP_INCLUDE_CONFIG | DUMP_INCLUDE_PROPERTIES)
+
 #define NOTMUCH_DUMP_VERSION 2
 
 int
diff --git a/notmuch-dump.c b/notmuch-dump.c
index d80ed8b8..ec82660 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -69,12 +69,77 @@ database_dump_config (notmuch_database_t *notmuch, gzFile output)
 static void
 print_dump_header (gzFile output, int output_format, int include)
 {
-    gzprintf (output, "#notmuch-dump %s:%d %s%s%s\n",
+    const char *sep = "";
+
+    gzprintf (output, "#notmuch-dump %s:%d ",
 	      (output_format == DUMP_FORMAT_SUP) ? "sup" : "batch-tag",
-	      NOTMUCH_DUMP_VERSION,
-	      (include & DUMP_INCLUDE_CONFIG) ? "config" : "",
-	      (include & DUMP_INCLUDE_TAGS) && (include & DUMP_INCLUDE_CONFIG) ? "," : "",
-	      (include & DUMP_INCLUDE_TAGS) ? "tags" : "");
+	      NOTMUCH_DUMP_VERSION);
+
+    if (include & DUMP_INCLUDE_CONFIG) {
+	gzputs (output, "config");
+	sep = ",";
+    }
+    if (include & DUMP_INCLUDE_PROPERTIES) {
+	gzprintf (output, "%sproperties",sep);
+	sep = ",";
+    }
+    if (include & DUMP_INCLUDE_TAGS) {
+	gzprintf (output, "%sproperties",sep);
+    }
+    gzputs (output, "\n");
+}
+
+static int
+dump_properties_message (void *ctx,
+			 notmuch_message_t *message,
+			 gzFile output,
+			 char **buffer_p, size_t *size_p)
+{
+    const char *message_id;
+    notmuch_message_properties_t *list;
+    notmuch_bool_t first = TRUE;
+
+    message_id = notmuch_message_get_message_id (message);
+
+    if (strchr (message_id, '\n')) {
+	fprintf (stderr, "Warning: skipping message id containing line break: \"%s\"\n", message_id);
+	return 0;
+    }
+
+   for (list = notmuch_message_get_properties (message, "", FALSE);
+	notmuch_message_properties_valid (list); notmuch_message_properties_move_to_next (list)) {
+       const char *key, *val;
+
+       if (first) {
+	   if (hex_encode (ctx, message_id, buffer_p, size_p) != HEX_SUCCESS) {
+	       fprintf (stderr, "Error: failed to hex-encode message-id %s\n", message_id);
+	       return 1;
+	   }
+	   gzprintf (output, "#= %s", *buffer_p);
+	   first = FALSE;
+       }
+
+       key = notmuch_message_properties_key (list);
+       val = notmuch_message_properties_value (list);
+
+       if (hex_encode (ctx, key, buffer_p, size_p) != HEX_SUCCESS) {
+	   fprintf (stderr, "Error: failed to hex-encode key %s\n", key);
+	   return 1;
+       }
+       gzprintf (output, " %s", *buffer_p);
+
+       if (hex_encode (ctx, val, buffer_p, size_p) != HEX_SUCCESS) {
+	   fprintf (stderr, "Error: failed to hex-encode value %s\n", val);
+	   return 1;
+       }
+       gzprintf (output, "=%s", *buffer_p);
+}
+   notmuch_message_properties_destroy (list);
+
+   if (!first)
+       gzprintf (output, "\n", *buffer_p);
+
+    return 0;
 }
 
 static int
@@ -159,7 +224,7 @@ database_dump_file (notmuch_database_t *notmuch, gzFile output,
 	    return EXIT_FAILURE;
     }
 
-    if (! (include & DUMP_INCLUDE_TAGS))
+    if (! (include & (DUMP_INCLUDE_TAGS | DUMP_INCLUDE_PROPERTIES)))
 	return EXIT_SUCCESS;
 
     if (! query_str)
@@ -189,6 +254,11 @@ database_dump_file (notmuch_database_t *notmuch, gzFile output,
 			       &buffer, &buffer_size))
 	    return EXIT_FAILURE;
 
+	if ((include & DUMP_INCLUDE_PROPERTIES) &&
+	    dump_properties_message (notmuch, message, output,
+				     &buffer, &buffer_size))
+	    return EXIT_FAILURE;
+
 	notmuch_message_destroy (message);
     }
 
@@ -312,6 +382,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD_FLAGS, &include, "include", 'I',
 	  (notmuch_keyword_t []){ { "config", DUMP_INCLUDE_CONFIG },
+				  { "properties", DUMP_INCLUDE_PROPERTIES },
 				  { "tags", DUMP_INCLUDE_TAGS} } },
 	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
 	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
@@ -326,7 +397,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_process_shared_options (argv[0]);
 
     if (include == 0)
-	include = DUMP_INCLUDE_CONFIG | DUMP_INCLUDE_TAGS;
+	include = DUMP_INCLUDE_CONFIG | DUMP_INCLUDE_TAGS | DUMP_INCLUDE_PROPERTIES;
 
     if (opt_index < argc) {
 	query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
diff --git a/notmuch-new.c b/notmuch-new.c
index 799fec2..c55dea7 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1042,7 +1042,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	    }
 
 	    if (notmuch_database_dump (notmuch, backup_name, "",
-				       DUMP_FORMAT_BATCH_TAG, DUMP_INCLUDE_CONFIG | DUMP_INCLUDE_TAGS, TRUE)) {
+				       DUMP_FORMAT_BATCH_TAG, DUMP_INCLUDE_DEFAULT, TRUE)) {
 		fprintf (stderr, "Backup failed. Aborting upgrade.");
 		return EXIT_FAILURE;
 	    }
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index b5ddb7a..e594979 100755
--- a/test/T610-message-property.sh
+++ b/test/T610-message-property.sh
@@ -89,6 +89,17 @@ 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));
+print_properties (message, "", FALSE);
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "notmuch_message_get_properties: empty list"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
@@ -188,4 +199,14 @@ cat <<'EOF' >EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "dump message properties"
+cat <<EOF > PROPERTIES
+#= 4EFC743A.3060609@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
+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 ="));
+EOF
+notmuch dump | grep '^#=' > OUTPUT
+test_expect_equal_file PROPERTIES OUTPUT
+
 test_done
-- 
2.8.1

[PATCH 8/8] cli: optionally restore message properties from dump file

[David Bremner]

This somewhat mimics the config line parsing, except there can be
arbitrarily many key value pairs, so one more level of looping is
required.
---
 doc/man1/notmuch-restore.rst  | 13 +++++--
 notmuch-restore.c             | 83 +++++++++++++++++++++++++++++++++++++++++--
 test/T610-message-property.sh | 28 +++++++++++++++
 3 files changed, 119 insertions(+), 5 deletions(-)

diff --git a/doc/man1/notmuch-restore.rst b/doc/man1/notmuch-restore.rst
index 87fa22e..a0c1b3c 100644
--- a/doc/man1/notmuch-restore.rst
+++ b/doc/man1/notmuch-restore.rst
@@ -50,7 +50,7 @@ Supported options for **restore** include
             format, this heuristic, based the fact that batch-tag format
             contains no parentheses, should be accurate.
 
-    ``--include=(config|tags)``
+    ``--include=(config|properties|tags)``
 
       Control what kind of metadata is restored.
 
@@ -60,13 +60,20 @@ Supported options for **restore** include
 	  with "#@ ", followed by a space seperated key-value pair.
 	  Both key and value are hex encoded if needed.
 
+	**properties**
+
+	  Output per-message (key,value) metadata.  Each line starts
+	  with "#= ", followed by a message id, and a space seperated
+	  list of key=value pairs.  pair.  Ids, keys and values are
+	  hex encoded if needed.
+
 	**tags**
 
 	  Output per-message metadata, namely tags. See *format* above
 	  for more details.
 
-      The default is to restore both tags and configuration
-      information
+      The default is to restore all available types of data.  The
+      option can be specified multiple times to select some subset.
 
     ``--input=``\ <filename>
         Read input from given file instead of stdin.
diff --git a/notmuch-restore.c b/notmuch-restore.c
index 371237c..d2ada61 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -57,6 +57,72 @@ process_config_line (notmuch_database_t *notmuch, const char* line)
     return ret;
 }
 
+static int
+process_properties_line (notmuch_database_t *notmuch, const char* line)
+
+{
+    const char *id_p, *tok;
+    size_t id_len = 0, tok_len = 0;
+    char *id;
+
+    notmuch_message_t *message = NULL;
+    const char *delim = " \t\n";
+    int ret = EXIT_FAILURE;
+
+    void *local = talloc_new(NULL);
+
+    id_p = strtok_len_c (line, delim, &id_len);
+    id = talloc_strndup (local, id_p, id_len);
+    if (hex_decode_inplace (id) != HEX_SUCCESS) {
+	fprintf (stderr, "hex decoding failure on line %s\n", line);
+	goto DONE;
+    }
+
+    if (print_status_database ("notmuch restore", notmuch,
+			       notmuch_database_find_message (notmuch, id, &message)))
+	goto DONE;
+
+    if (print_status_database ("notmuch restore", notmuch,
+			       notmuch_message_remove_all_properties (message)))
+	goto DONE;
+
+    tok = id_p + id_len;
+
+    while ((tok = strtok_len_c (tok + tok_len, delim, &tok_len)) != NULL) {
+	char *key, *value;
+	size_t off = strcspn (tok, "=");
+	if (off > tok_len) {
+	    fprintf (stderr, "unparsable token %s\n", tok);
+	    goto DONE;
+	}
+
+	key = talloc_strndup (local, tok, off);
+	value = talloc_strndup (local, tok+off+1, tok_len - off - 1);
+
+	if (hex_decode_inplace (key) != HEX_SUCCESS) {
+	    fprintf (stderr, "hex decoding failure on key %s\n", key);
+	    goto DONE;
+	}
+
+	if (hex_decode_inplace (value) != HEX_SUCCESS) {
+	    fprintf (stderr, "hex decoding failure on value %s\n", value);
+	    goto DONE;
+	}
+
+	if (print_status_database ("notmuch restore", notmuch,
+				   notmuch_message_add_property (message, key, value)))
+	goto DONE;
+
+    }
+
+    ret = EXIT_SUCCESS;
+
+ DONE:
+    talloc_free (local);
+    return ret;
+}
+
+
 static regex_t regex;
 
 /* Non-zero return indicates an error in retrieving the message,
@@ -188,6 +254,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
 				  { 0, 0 } } },
 	{ NOTMUCH_OPT_KEYWORD_FLAGS, &include, "include", 'I',
 	  (notmuch_keyword_t []){ { "config", DUMP_INCLUDE_CONFIG },
+				  { "properties", DUMP_INCLUDE_PROPERTIES },
 				  { "tags", DUMP_INCLUDE_TAGS} } },
 
 	{ NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 },
@@ -206,7 +273,7 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_exit_if_unmatched_db_uuid (notmuch);
 
     if (include == 0) {
-	include = DUMP_INCLUDE_CONFIG | DUMP_INCLUDE_TAGS;
+	include = DUMP_INCLUDE_CONFIG | DUMP_INCLUDE_PROPERTIES | DUMP_INCLUDE_TAGS;
     }
 
     name_for_error = input_file_name ? input_file_name : "stdin";
@@ -273,13 +340,18 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
 	    if (ret)
 		goto DONE;
 	}
+	if ((include & DUMP_INCLUDE_PROPERTIES) && line_len >= 2 && line[0] == '#' && line[1] == '=') {
+	    ret = process_properties_line(notmuch, line+2);
+	    if (ret)
+		goto DONE;
+	}
 
     } while ((line_len == 0) ||
 	     (line[0] == '#') ||
 	     /* the cast is safe because we checked about for line_len < 0 */
 	     (strspn (line, " \t\n") == (unsigned)line_len));
 
-    if (! (include & DUMP_INCLUDE_TAGS)) {
+    if (! ((include & DUMP_INCLUDE_TAGS) || (include & DUMP_INCLUDE_PROPERTIES))) {
 	ret = EXIT_SUCCESS;
 	goto DONE;
     }
@@ -306,6 +378,13 @@ notmuch_restore_command (notmuch_config_t *config, int argc, char *argv[])
 	    talloc_free (line_ctx);
 
 	line_ctx = talloc_new (config);
+
+	if ((include & DUMP_INCLUDE_PROPERTIES) && line_len >= 2 && line[0] == '#' && line[1] == '=') {
+	    ret = process_properties_line(notmuch, line+2);
+	    if (ret)
+		goto DONE;
+	}
+
 	if (input_format == DUMP_FORMAT_SUP) {
 	    ret = parse_sup_line (line_ctx, line, &query_string, tag_ops);
 	} else {
diff --git a/test/T610-message-property.sh b/test/T610-message-property.sh
index e594979..8952eb7 100755
--- a/test/T610-message-property.sh
+++ b/test/T610-message-property.sh
@@ -209,4 +209,32 @@ EOF
 notmuch dump | grep '^#=' > OUTPUT
 test_expect_equal_file PROPERTIES OUTPUT
 
+
+test_begin_subtest "restore missing message property (single line)"
+notmuch dump | grep '^#=' > BEFORE1
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+EXPECT0(notmuch_message_remove_property (message, "testkey1", "bob"));
+EOF
+notmuch restore < BEFORE1
+notmuch dump | grep '^#=' > OUTPUT
+test_expect_equal_file PROPERTIES OUTPUT
+
+
+test_begin_subtest "restore missing message property (full dump)"
+notmuch dump > BEFORE2
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+EXPECT0(notmuch_message_remove_property (message, "testkey1", "bob"));
+EOF
+notmuch restore < BEFORE2
+notmuch dump | grep '^#=' > OUTPUT
+test_expect_equal_file PROPERTIES OUTPUT
+
+test_begin_subtest "restore clear extra message property"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+EXPECT0(notmuch_message_add_property (message, "testkey1", "charles"));
+EOF
+notmuch restore < BEFORE2
+notmuch dump | grep '^#=' > OUTPUT
+test_expect_equal_file PROPERTIES OUTPUT
+
 test_done
-- 
2.8.1

[PATCH] add has: query prefix to search for specific properties

[Daniel Kahn Gillmor]

We want to be able to query the properties directly, like:

   notmuch count has:foo=bar

which should return a count of messages where the property with key
"foo" has value equal to "bar".

This patch could be improved:

If no = sign is present (e.g. "has:foo"), it'd be nice to just match
 on every message that has property "foo", regardless of value.

It would also be good to include some tests.
---
 lib/database.cc | 4 ++++
 1 file changed, 4 insertions(+)

This applies atop bermner's "message properties" series, and makes it
possible to query for messages with specific properties.

diff --git a/lib/database.cc b/lib/database.cc
index 3a741f0..3bdbd07 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -97,6 +97,9 @@ typedef struct {
  *		        STRING is the name of a file within that
  *		        directory for this mail message.
  *
+ *      has:       Has a property with key=value
+ *                 FIXME: if no = is present, should match on any value
+ *
  *    A mail document also has four values:
  *
  *	TIMESTAMP:	The time_t value corresponding to the message's
@@ -260,6 +263,7 @@ static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {
     { "is",			"K" },
     { "id",			"Q" },
     { "path",			"P" },
+    { "has",			"XPROPERTY" },
     /*
      * Without the ":", since this is a multi-letter prefix, Xapian
      * will add a colon itself if the first letter of the path is
-- 
2.8.1

Re: [PATCH] add has: query prefix to search for specific properties

[David Bremner]

Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:

> We want to be able to query the properties directly, like:
>
>    notmuch count has:foo=bar
>
> which should return a count of messages where the property with key
> "foo" has value equal to "bar".
>
> This patch could be improved:
>
> If no = sign is present (e.g. "has:foo"), it'd be nice to just match
>  on every message that has property "foo", regardless of value.

At the moment I can't think of a nice easy way to do this.  It could do
something like that proposed for "tag:*", namely expand it internally
into an or query, by traversing the
db->allterms_begin("XPROPERTY"). 

> It would also be good to include some tests.

and update notmuch-search-terms(7) ?

>   *	TIMESTAMP:	The time_t value corresponding to the message's
> @@ -260,6 +263,7 @@ static prefix_t BOOLEAN_PREFIX_EXTERNAL[] = {
>      { "is",			"K" },
>      { "id",			"Q" },
>      { "path",			"P" },
> +    { "has",			"XPROPERTY" },
>      /*
>       * Without the ":", since this is a multi-letter prefix, Xapian
>       * will add a colon itself if the first letter of the path is

Do we want to have the same prefix listed in BOOLEAN_PREFIX_INTERNAL and
BOOLEAN_PREFIX_EXTERNAL? this seems quite fragile. If we don't though,
we have to use the same prefix name for queries as in the code (which is
already sprinkled with _find_prefix ("property") ).

Re: [PATCH] add has: query prefix to search for specific properties

[David Bremner]

David Bremner <david@tethera.net> writes:

> Do we want to have the same prefix listed in BOOLEAN_PREFIX_INTERNAL and
> BOOLEAN_PREFIX_EXTERNAL? this seems quite fragile. If we don't though,
> we have to use the same prefix name for queries as in the code (which is
> already sprinkled with _find_prefix ("property") ).

Thinking about it a bit more, there's really no problem here. The
invariant to maintain is the names are unique across all 3 tables used
by _find_prefix, not the necessarily the prefixes. Indeed there's
already the example of "is" and "tag".

d

Re: [PATCH] add has: query prefix to search for specific properties

[Daniel Kahn Gillmor]

On Sun 2016-07-17 02:39:53 +0200, David Bremner wrote:
> Daniel Kahn Gillmor <dkg@fifthhorseman.net> writes:
>
>> We want to be able to query the properties directly, like:
>>
>>    notmuch count has:foo=bar
>>
>> which should return a count of messages where the property with key
>> "foo" has value equal to "bar".
>>
>> This patch could be improved:
>>
>> If no = sign is present (e.g. "has:foo"), it'd be nice to just match
>>  on every message that has property "foo", regardless of value.
>
> At the moment I can't think of a nice easy way to do this.  It could do
> something like that proposed for "tag:*", namely expand it internally
> into an or query, by traversing the
> db->allterms_begin("XPROPERTY"). 
>
>> It would also be good to include some tests.
>
> and update notmuch-search-terms(7) ?

yes, please -- patches welcome ;)

an attempt to describe the distinction between "tags" and "properties":

Tags are user-settable and user-fetchable, and automated systems are
discouraged from adding them (we have some legacy exceptions like
"inbox" and "unread" and "signed" and "encrypted").  Tags are generally
expected to be simple textual labels.

Properties are user-fetchable, but should only be set by automated
systems.  Properties are key=value pairs (though a given message can
have any number of properties with the same key), and while the key is
expected to be a textual string (printable characters except for "=")
and the value can potentially be arbitrary binary data.

I welcome edits on this text if it doesn't capture your current sense of
the distinction.

     --dkg