From: Mark Walters <markwalters1009@gmail.com>
To: Austin Clements <aclements@csail.mit.edu>, notmuch@notmuchmail.org
Subject: Re: [PATCH v2 07/12] lib: Internal support for querying and
	creating	ghost messages
In-Reply-To: <1412637438-4821-8-git-send-email-aclements@csail.mit.edu>
References: <1412637438-4821-1-git-send-email-aclements@csail.mit.edu>
	<1412637438-4821-8-git-send-email-aclements@csail.mit.edu>
User-Agent: Notmuch/0.18.1+86~gef5e66a (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Wed, 22 Oct 2014 00:05:05 +0100
Message-ID: <8738ahja72.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


Hi 

I am slowly working my way through this series: only two trivial queries
so far.

On Tue, 07 Oct 2014, Austin Clements <aclements@csail.mit.edu> wrote:
> From: Austin Clements <amdragon@mit.edu>
>
> This updates the message abstraction to support ghost messages: it
> adds a message flag that distinguishes regular messages from ghost
> messages, and an internal function for initializing a newly created
> (blank) message as a ghost message.
> ---
>  lib/message.cc        | 52 +++++++++++++++++++++++++++++++++++++++++++++++++--
>  lib/notmuch-private.h |  4 ++++
>  lib/notmuch.h         |  9 ++++++++-
>  3 files changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/lib/message.cc b/lib/message.cc
> index 55d2ff6..a7a13cc 100644
> --- a/lib/message.cc
> +++ b/lib/message.cc
> @@ -39,6 +39,9 @@ struct visible _notmuch_message {
>      notmuch_message_file_t *message_file;
>      notmuch_message_list_t *replies;
>      unsigned long flags;
> +    /* For flags that are initialized on-demand, lazy_flags indicates
> +     * if each flag has been initialized. */
> +    unsigned long lazy_flags;

I wonder if valid_flags might be better here as, as far as I can see,
the reason for these is so we can invalidate a flag more than an
optimisation (which is what I thought the lazy implied).

>  
>      Xapian::Document doc;
>      Xapian::termcount termpos;
> @@ -99,6 +102,7 @@ _notmuch_message_create_for_document (const void *talloc_owner,
>  
>      message->frozen = 0;
>      message->flags = 0;
> +    message->lazy_flags = 0;
>  
>      /* Each of these will be lazily created as needed. */
>      message->message_id = NULL;
> @@ -192,7 +196,7 @@ _notmuch_message_create (const void *talloc_owner,
>   *
>   *     There is already a document with message ID 'message_id' in the
>   *     database. The returned message can be used to query/modify the
> - *     document.
> + *     document. The message may be a ghost message.
>   *
>   *   NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND:
>   *
> @@ -305,6 +309,7 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message)
>      const char *thread_prefix = _find_prefix ("thread"),
>  	*tag_prefix = _find_prefix ("tag"),
>  	*id_prefix = _find_prefix ("id"),
> +	*type_prefix = _find_prefix ("type"),
>  	*filename_prefix = _find_prefix ("file-direntry"),
>  	*replyto_prefix = _find_prefix ("replyto");
>  
> @@ -337,10 +342,25 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message)
>  	message->message_id =
>  	    _notmuch_message_get_term (message, i, end, id_prefix);
>  
> +    /* Get document type */
> +    assert (strcmp (id_prefix, type_prefix) < 0);
> +    if (! NOTMUCH_TEST_BIT (message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST)) {
> +	i.skip_to (type_prefix);
> +	/* "T" is the prefix "type" fields.  See
> +	 * BOOLEAN_PREFIX_INTERNAL. */
> +	if (*i == "Tmail")
> +	    NOTMUCH_CLEAR_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
> +	else if (*i == "Tghost")
> +	    NOTMUCH_SET_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
> +	else
> +	    INTERNAL_ERROR ("Message without type term");
> +	NOTMUCH_SET_BIT (&message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST);
> +    }
> +
>      /* Get filename list.  Here we get only the terms.  We lazily
>       * expand them to full file names when needed in
>       * _notmuch_message_ensure_filename_list. */
> -    assert (strcmp (id_prefix, filename_prefix) < 0);
> +    assert (strcmp (type_prefix, filename_prefix) < 0);
>      if (!message->filename_term_list && !message->filename_list)
>  	message->filename_term_list =
>  	    _notmuch_database_get_terms_with_prefix (message, i, end,
> @@ -371,6 +391,11 @@ _notmuch_message_invalidate_metadata (notmuch_message_t *message,
>  	message->tag_list = NULL;
>      }
>  
> +    if (strcmp ("type", prefix_name) == 0) {
> +	NOTMUCH_CLEAR_BIT (&message->flags, NOTMUCH_MESSAGE_FLAG_GHOST);
> +	NOTMUCH_CLEAR_BIT (&message->lazy_flags, NOTMUCH_MESSAGE_FLAG_GHOST);
> +    }
> +
>      if (strcmp ("file-direntry", prefix_name) == 0) {
>  	talloc_free (message->filename_term_list);
>  	talloc_free (message->filename_list);
> @@ -869,6 +894,10 @@ notmuch_bool_t
>  notmuch_message_get_flag (notmuch_message_t *message,
>  			  notmuch_message_flag_t flag)
>  {
> +    if (flag == NOTMUCH_MESSAGE_FLAG_GHOST &&
> +	! NOTMUCH_TEST_BIT (message->lazy_flags, flag))
> +	_notmuch_message_ensure_metadata (message);
> +
>      return NOTMUCH_TEST_BIT (message->flags, flag);
>  }
>  
> @@ -880,6 +909,7 @@ notmuch_message_set_flag (notmuch_message_t *message,
>  	NOTMUCH_SET_BIT (&message->flags, flag);
>      else
>  	NOTMUCH_CLEAR_BIT (&message->flags, flag);
> +    NOTMUCH_SET_BIT (&message->lazy_flags, flag);
>  }
>  
>  time_t
> @@ -989,6 +1019,24 @@ _notmuch_message_delete (notmuch_message_t *message)
>      return NOTMUCH_STATUS_SUCCESS;
>  }
>  
> +/* Transform a blank message into a ghost message.  The caller must
> + * _notmuch_message_sync the message. */
> +notmuch_private_status_t
> +_notmuch_message_initialize_ghost (notmuch_message_t *message,
> +				   const char *thread_id)
> +{
> +    notmuch_private_status_t status;
> +
> +    status = _notmuch_message_add_term (message, "type", "ghost");
> +    if (status)
> +	return status;
> +    status = _notmuch_message_add_term (message, "thread", thread_id);
> +    if (status)
> +	return status;
> +
> +    return NOTMUCH_PRIVATE_STATUS_SUCCESS;
> +}
> +
>  /* Ensure that 'message' is not holding any file object open. Future
>   * calls to various functions will still automatically open the
>   * message file as needed.
> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
> index 7250291..2f43c1d 100644
> --- a/lib/notmuch-private.h
> +++ b/lib/notmuch-private.h
> @@ -308,6 +308,10 @@ _notmuch_message_sync (notmuch_message_t *message);
>  notmuch_status_t
>  _notmuch_message_delete (notmuch_message_t *message);
>  
> +notmuch_private_status_t
> +_notmuch_message_initialize_ghost (notmuch_message_t *message,
> +				   const char *thread_id);
> +
>  void
>  _notmuch_message_close (notmuch_message_t *message);
>  
> diff --git a/lib/notmuch.h b/lib/notmuch.h
> index dae0416..92594b9 100644
> --- a/lib/notmuch.h
> +++ b/lib/notmuch.h
> @@ -1221,7 +1221,14 @@ notmuch_message_get_filenames (notmuch_message_t *message);
>   */
>  typedef enum _notmuch_message_flag {
>      NOTMUCH_MESSAGE_FLAG_MATCH,
> -    NOTMUCH_MESSAGE_FLAG_EXCLUDED
> +    NOTMUCH_MESSAGE_FLAG_EXCLUDED,
> +
> +    /* This message is a "ghost message", meaning it has no filenames
> +     * or content, but we know it exists because it was referenced by
> +     * some other message.  A ghost message has only a message ID and
> +     * thread ID.
> +     */

Can I check here: we are not allowing a ghost message to have any tags?

Best wishes

Mark

> +    NOTMUCH_MESSAGE_FLAG_GHOST,
>  } notmuch_message_flag_t;
>  
>  /**
> -- 
> 2.1.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch