fix thread breakage via ghost-on-removal

implement ghost-on-removal, the solution to T590-thread-breakage.sh
that just adds a ghost message after removing each message.

It leaks information about whether we've ever seen a given message id,
but it's a fairly simple implementation.

Note that _resolve_message_id_to_thread_id already introduces new
message_ids to the database, so i think just searching for a given
message ID may introduce the same metadata leakage.
This commit is contained in:
Daniel Kahn Gillmor 2016-04-08 22:54:48 -03:00 committed by David Bremner
parent 92559ee347
commit 604d1e0977
2 changed files with 39 additions and 16 deletions

View file

@ -1037,20 +1037,44 @@ _notmuch_message_sync (notmuch_message_t *message)
message->modified = FALSE; message->modified = FALSE;
} }
/* Delete a message document from the database. */ /* Delete a message document from the database, leaving a ghost
* message in its place */
notmuch_status_t notmuch_status_t
_notmuch_message_delete (notmuch_message_t *message) _notmuch_message_delete (notmuch_message_t *message)
{ {
notmuch_status_t status; notmuch_status_t status;
Xapian::WritableDatabase *db; Xapian::WritableDatabase *db;
const char *mid, *tid;
notmuch_message_t *ghost;
notmuch_private_status_t private_status;
notmuch_database_t *notmuch;
mid = notmuch_message_get_message_id (message);
tid = notmuch_message_get_thread_id (message);
notmuch = message->notmuch;
status = _notmuch_database_ensure_writable (message->notmuch); status = _notmuch_database_ensure_writable (message->notmuch);
if (status) if (status)
return status; return status;
db = static_cast <Xapian::WritableDatabase *> (message->notmuch->xapian_db); db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
db->delete_document (message->doc_id); db->delete_document (message->doc_id);
return NOTMUCH_STATUS_SUCCESS;
/* and reintroduce a ghost in its place */
ghost = _notmuch_message_create_for_message_id (notmuch, mid, &private_status);
if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {
private_status = _notmuch_message_initialize_ghost (ghost, tid);
if (! private_status)
_notmuch_message_sync (ghost);
} else if (private_status == NOTMUCH_PRIVATE_STATUS_SUCCESS) {
/* this is deeply weird, and we should not have gotten into
this state. is there a better error message to return
here? */
return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
}
notmuch_message_destroy (ghost);
return COERCE_STATUS (private_status, "Error converting to ghost message");
} }
/* Transform a blank message into a ghost message. The caller must /* Transform a blank message into a ghost message. The caller must

View file

@ -94,20 +94,11 @@ notmuch new >/dev/null
test_thread_count 1 'First message removed: still only one thread' test_thread_count 1 'First message removed: still only one thread'
test_content_count apple 0 test_content_count apple 0
test_content_count banana 1 test_content_count banana 1
test_begin_subtest 'should be one ghost after first message removed' test_ghost_count 1 'should be one ghost after first message removed'
test_subtest_known_broken
ghosts=$(../ghost-report ${MAIL_DIR}/.notmuch/xapian)
test_expect_equal "$ghosts" "1"
message_a message_a
notmuch new >/dev/null notmuch new >/dev/null
# this is known to fail (it shows 2 threads) because no "ghost test_thread_count 1 'First message reappears: should return to the same thread'
# message" was created for message A when it was removed from the
# index, despite message B still pointing to it.
test_begin_subtest 'First message reappears: should return to the same thread'
test_subtest_known_broken
count=$(notmuch count --output=threads)
test_expect_equal "$count" "1"
test_content_count apple 1 test_content_count apple 1
test_content_count banana 1 test_content_count banana 1
test_ghost_count 0 test_ghost_count 0
@ -117,13 +108,21 @@ notmuch new >/dev/null
test_thread_count 1 'Removing second message: still only one thread' test_thread_count 1 'Removing second message: still only one thread'
test_content_count apple 1 test_content_count apple 1
test_content_count banana 0 test_content_count banana 0
test_ghost_count 0 'No ghosts should remain after deletion of second message' test_begin_subtest 'No ghosts should remain after deletion of second message'
# this is known to fail; we are leaking ghost messages deliberately
test_subtest_known_broken
ghosts=$(../ghost-report ${MAIL_DIR}/.notmuch/xapian)
test_expect_equal "$ghosts" "0"
rm -f ${MAIL_DIR}/cur/a rm -f ${MAIL_DIR}/cur/a
notmuch new >/dev/null notmuch new >/dev/null
test_thread_count 0 'All messages gone: no threads' test_thread_count 0 'All messages gone: no threads'
test_content_count apple 0 test_content_count apple 0
test_content_count banana 0 test_content_count banana 0
test_ghost_count 0 test_begin_subtest 'No ghosts should remain after full thread deletion'
# this is known to fail; we are leaking ghost messages deliberately
test_subtest_known_broken
ghosts=$(../ghost-report ${MAIL_DIR}/.notmuch/xapian)
test_expect_equal "$ghosts" "0"
test_done test_done