From: Austin Clements <amdragon@MIT.EDU>
To: notmuch@notmuchmail.org
Subject: [PATCH 06/10] lib: Add API's to find by filename and remove a
	filename from a message.
Date: Fri, 18 Feb 2011 02:58:56 -0500
Message-Id: <1298015940-31986-7-git-send-email-amdragon@mit.edu>
In-Reply-To: <1298015940-31986-1-git-send-email-amdragon@mit.edu>
References: <1298015940-31986-1-git-send-email-amdragon@mit.edu>
Cc: amdragon@mit.edu
Precedence: list

The two new API functions, notmuch_database_find_message_by_filename
and notmuch_message_remove_filename give library users more control
over the filename removal process.  notmuch_database_remove_message
has been reimplemented in terms of these new functions.

notmuch_message_remove_filename acts much like
notmuch_message_remove_tag in that it does not synchronize with the
database if the message is frozen.  Thus, callers can freeze a message
to remove a filename and perform tag synchronization in one atomic
operation.

This new approach also naturally eliminates an atomicity violation in
the old code.  Previously, notmuch_database_remove_message would first
update the database document to remove the filename and only then
remove the document if it had no more filenames left.  An interruption
between these two steps resulted in a permanently un-removable zombie
message that would produce errors and odd results in searches.  Since
this new approach delegates document deletion to
_notmuch_message_sync, the document will be deleted without first
being updated, eliminating this window.
---
 lib/database.cc |   58 +++++++++++++++++++-----------------------------------
 lib/message.cc  |   21 +++++++++++++++++++
 lib/notmuch.h   |   43 ++++++++++++++++++++++++++++++++++++++-
 3 files changed, 83 insertions(+), 39 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index d88b168..bee1e96 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1689,72 +1689,56 @@ notmuch_status_t
 notmuch_database_remove_message (notmuch_database_t *notmuch,
 				 const char *filename)
 {
-    Xapian::WritableDatabase *db;
+    notmuch_message_t *message;
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+    message = notmuch_database_find_message_by_filename (notmuch, filename);
+    if (message) {
+	status = notmuch_message_remove_filename (message, filename);
+	notmuch_message_destroy (message);
+    }
+    return status;
+}
+
+notmuch_message_t *
+notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
+					   const char *filename)
+{
     void *local;
     const char *prefix = _find_prefix ("file-direntry");
     char *direntry, *term;
     Xapian::PostingIterator i, end;
-    Xapian::Document document;
+    notmuch_message_t *message = NULL;
     notmuch_status_t status;
 
-    status = _notmuch_database_ensure_writable (notmuch);
-    if (status)
-	return status;
-
     local = talloc_new (notmuch);
 
-    db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
-
     try {
 
 	status = _notmuch_database_filename_to_direntry (local, notmuch,
 							 filename, &direntry);
 	if (status)
-	    return status;
+	    return NULL;
 
 	term = talloc_asprintf (local, "%s%s", prefix, direntry);
 
 	find_doc_ids_for_term (notmuch, term, &i, &end);
 
-	for ( ; i != end; i++) {
-	    Xapian::TermIterator j;
-	    notmuch_message_t *message;
+	if (i != end) {
 	    notmuch_private_status_t private_status;
 
-	    message = _notmuch_message_create (local, notmuch,
+	    message = _notmuch_message_create (notmuch, notmuch,
 					       *i, &private_status);
-	    if (message == NULL)
-		return COERCE_STATUS (private_status,
-				      "Inconsistent document ID in datbase.");
-
-	    _notmuch_message_remove_filename (message, filename);
-	    _notmuch_message_sync (message);
-
-	    /* Take care to find document after sync'ing filename removal. */
-	    document = find_document_for_doc_id (notmuch, *i);
-	    j = document.termlist_begin ();
-	    j.skip_to (prefix);
-
-	    /* Was this the last file-direntry in the message? */
-	    if (j == document.termlist_end () ||
-		strncmp ((*j).c_str (), prefix, strlen (prefix)))
-	    {
-		db->delete_document (document.get_docid ());
-		status = NOTMUCH_STATUS_SUCCESS;
-	    } else {
-		status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
-	    }
 	}
     } catch (const Xapian::Error &error) {
-	fprintf (stderr, "Error: A Xapian exception occurred removing message: %s\n",
+	fprintf (stderr, "Error: A Xapian exception occurred finding message by filename: %s\n",
 		 error.get_msg().c_str());
 	notmuch->exception_reported = TRUE;
-	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+	message = NULL;
     }
 
     talloc_free (local);
 
-    return status;
+    return message;
 }
 
 notmuch_tags_t *
diff --git a/lib/message.cc b/lib/message.cc
index 635f5cf..b4adb5c 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1300,6 +1300,27 @@ notmuch_message_remove_all_tags (notmuch_message_t *message)
 }
 
 notmuch_status_t
+notmuch_message_remove_filename (notmuch_message_t *message,
+				 const char *filename)
+{
+    notmuch_status_t status;
+
+    status = _notmuch_database_ensure_writable (message->notmuch);
+    if (status)
+	return status;
+
+    status = _notmuch_message_remove_filename (message, filename);
+    /* Was this the last file-direntry in the message? */
+    if (status == NOTMUCH_STATUS_SUCCESS)
+	message->deleted = TRUE;
+
+    if (! message->frozen)
+	_notmuch_message_sync (message);
+
+    return status;
+}
+
+notmuch_status_t
 notmuch_message_freeze (notmuch_message_t *message)
 {
     notmuch_status_t status;
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e508309..61030cb 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -316,6 +316,22 @@ notmuch_message_t *
 notmuch_database_find_message (notmuch_database_t *database,
 			       const char *message_id);
 
+/* Find a message with the given filename.
+ *
+ * If the database contains a message with the given filename, then a
+ * new notmuch_message_t object is returned.  The caller should call 
+ * notmuch_message_destroy when done with the message.
+ *
+ * This function returns NULL in the following situations:
+ *
+ *	* No message is found with the given filename
+ *	* An out-of-memory situation occurs
+ *	* A Xapian exception occurs
+ */
+notmuch_message_t *
+notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
+					   const char *filename);
+
 /* Return a list of all tags found in the database.
  *
  * This function creates a list of all tags found in the database. The
@@ -979,11 +995,34 @@ notmuch_message_maildir_flags_to_tags (notmuch_message_t *message);
 notmuch_status_t
 notmuch_message_tags_to_maildir_flags (notmuch_message_t *message);
 
+/* Remove a filename from a message.  If this is the last copy of this
+ * message, also delete it from the database.
+ *
+ * Much like notmuch_message_remove_tag, if message is frozen, it will
+ * not be removed from or updated in the database until thawed.
+ *
+ * Return value:
+ *
+ * NOTMUCH_STATUS_SUCCESS: The last filename was removed and the
+ *	message was removed from the database.
+ *
+ * NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID: This filename was removed but
+ *	the message persists in the database with at least one other
+ *	filename.
+ *
+ * NOTMUCH_STATUS_READ_ONLY_DATABASE: Database was opened in read-only
+ *	mode so no message can be removed.
+ */
+notmuch_status_t
+notmuch_message_remove_filename (notmuch_message_t *message,
+				 const char *filename);
+
 /* Freeze the current state of 'message' within the database.
  *
  * This means that changes to the message state, (via
- * notmuch_message_add_tag, notmuch_message_remove_tag, and
- * notmuch_message_remove_all_tags), will not be committed to the
+ * notmuch_message_add_tag, notmuch_message_remove_tag,
+ * notmuch_message_remove_all_tags, and
+ * notmuch_message_remove_filename), will not be committed to the
  * database until the message is thawed with notmuch_message_thaw.
  *
  * Multiple calls to freeze/thaw are valid and these calls will
-- 
1.7.2.3