final(?) batch of api cleanup for exception handling

final(?) batch of api cleanup for exception handling

[David Bremner]

Hopefully this covers completely the current API for error handling on
closed databases. For the most part, it's not that I care about
supporting operations on closed databases, but rather it provides a
good opportunity to make sure exceptions are being caught at the the
boundary of libnotmuch.

The not-completely-trivial changes are

[PATCH 05/13] lib: catch exceptions in  n_directory_get_child_directories
[PATCH 07/13] lib: catch exceptions in n_directory_get_child_files
[PATCH 13/13] lib: return NULL from n_d_get_default_indexopts on

The last one is a repost to keep the series together. 

[PATCH 01/13] test: add regression test for n_messages_collect_tags

[David Bremner]

Also test n_messages_destroy.
---
 test/T568-lib-thread.sh | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/test/T568-lib-thread.sh b/test/T568-lib-thread.sh
index 66066854..ac13d986 100755
--- a/test/T568-lib-thread.sh
+++ b/test/T568-lib-thread.sh
@@ -285,6 +285,37 @@ unread
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "collect tags with closed database"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+      notmuch_messages_t *messages = notmuch_thread_get_messages (thread);
+
+      notmuch_tags_t *tags = notmuch_messages_collect_tags (messages);
+
+      const char *tag;
+      for (tags = notmuch_thread_get_tags (thread);
+           notmuch_tags_valid (tags);
+           notmuch_tags_move_to_next (tags))
+        {
+          tag = notmuch_tags_get (tags);
+          printf ("%s\n", tag);
+        }
+      notmuch_tags_destroy (tags);
+      notmuch_messages_destroy (messages);
+
+      printf("SUCCESS\n");
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+inbox
+signed
+unread
+SUCCESS
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "destroy thread with closed database"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.27.0

[PATCH 02/13] test: split header for lib-message tests.

[David Bremner]

This allows finer control over when to close the database.
---
 test/T566-lib-message.sh | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 4c9fc0df..7e8f8959 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -18,7 +18,7 @@ cat <<'EOF' > c_tail
 }
 EOF
 
-cat <<EOF > c_head
+cat <<EOF > c_head0
 #include <stdio.h>
 #include <notmuch.h>
 #include <notmuch-test.h>
@@ -36,9 +36,11 @@ int main (int argc, char** argv)
      exit (1);
    }
    EXPECT0(notmuch_database_find_message (db, id, &message));
-   EXPECT0(notmuch_database_close (db));
 EOF
 
+cp c_head0 c_head
+echo "   EXPECT0(notmuch_database_close (db));" >> c_head
+
 test_begin_subtest "Handle getting message-id from closed database"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.27.0

[PATCH 03/13] test: regression test for n_m_get_filenames

[David Bremner]

Closing the database after the iterator is created is not a problem.
---
 test/T566-lib-message.sh | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/test/T566-lib-message.sh b/test/T566-lib-message.sh
index 7e8f8959..d3765220 100755
--- a/test/T566-lib-message.sh
+++ b/test/T566-lib-message.sh
@@ -149,6 +149,29 @@ A Xapian exception occurred at lib/message.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "iterate over all message filenames from closed database"
+cat c_head0 - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_filenames_t *filenames;
+        filenames = notmuch_message_get_filenames (message);
+        EXPECT0(notmuch_database_close (db));
+        for (; notmuch_filenames_valid (filenames);
+               notmuch_filenames_move_to_next (filenames)) {
+            const char *filename = notmuch_filenames_get (filenames);
+            printf("%s\n", filename);
+        }
+        notmuch_filenames_destroy (filenames);
+        printf("SUCCESS\n");
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+MAIL_DIR/01:2,
+SUCCESS
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Handle getting ghost flag from closed database"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
-- 
2.27.0

[PATCH 04/13] test: add known broken test for n_directory_get_child_directories

[David Bremner]

The error message here may need adjusting.
---
 test/T563-lib-directory.sh | 61 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 61 insertions(+)
 create mode 100755 test/T563-lib-directory.sh

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
new file mode 100755
index 00000000..e9e4cd0f
--- /dev/null
+++ b/test/T563-lib-directory.sh
@@ -0,0 +1,61 @@
+#!/usr/bin/env bash
+test_description="notmuch_database_* API"
+
+. $(dirname "$0")/test-lib.sh || exit 1
+
+add_email_corpus
+
+test_begin_subtest "building database"
+test_expect_success "NOTMUCH_NEW"
+
+cat <<EOF > c_head
+#include <stdio.h>
+#include <notmuch.h>
+#include <notmuch-test.h>
+#include <talloc.h>
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   notmuch_directory_t *dir;
+   notmuch_status_t stat = NOTMUCH_STATUS_SUCCESS;
+   char *msg = NULL;
+
+   stat = notmuch_database_open_verbose (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db, &msg);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database: %d %s\n", stat, msg ? msg : "");
+     exit (1);
+   }
+
+   EXPECT0(notmuch_database_get_directory (db, "", &dir));
+   EXPECT0(notmuch_database_close (db));
+EOF
+
+cat <<'EOF' > c_tail
+   if (stat) {
+       const char *stat_str = notmuch_database_status_string (db);
+       if (stat_str)
+           fputs (stat_str, stderr);
+    }
+
+}
+EOF
+
+test_begin_subtest "get child directories for a closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_filenames_t *children;
+        children = notmuch_directory_get_child_directories (dir);
+        printf ("%d\n", children == NULL);
+        stat = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.27.0

[PATCH 05/13] lib: catch exceptions in n_directory_get_child_directories

[David Bremner]

Also clarify API in error case.
---
 lib/directory.cc           | 23 ++++++++++++++++++++---
 lib/notmuch.h              |  2 ++
 test/T563-lib-directory.sh |  1 -
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/lib/directory.cc b/lib/directory.cc
index 09b49245..eb59d24d 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -49,6 +49,19 @@ struct _notmuch_directory {
     time_t mtime;
 };
 
+#define LOG_XAPIAN_EXCEPTION(directory, error) _log_xapian_exception (__location__, directory, error)
+
+static void
+_log_xapian_exception (const char *where, notmuch_directory_t *dir,  const Xapian::Error error) {
+    notmuch_database_t *notmuch = dir->notmuch;
+    _notmuch_database_log (notmuch,
+			   "A Xapian exception occurred at %s: %s\n",
+			   where,
+			   error.get_msg ().c_str ());
+    notmuch->exception_reported = true;
+}
+
+
 /* We end up having to call the destructor explicitly because we had
  * to use "placement new" in order to initialize C++ objects within a
  * block that we allocated with talloc. So C++ is making talloc
@@ -270,14 +283,18 @@ notmuch_filenames_t *
 notmuch_directory_get_child_directories (notmuch_directory_t *directory)
 {
     char *term;
-    notmuch_filenames_t *child_directories;
+    notmuch_filenames_t *child_directories = NULL;
 
     term = talloc_asprintf (directory, "%s%u:",
 			    _find_prefix ("directory-direntry"),
 			    directory->document_id);
 
-    child_directories = _create_filenames_for_terms_with_prefix (directory,
-								 directory->notmuch, term);
+    try {
+	child_directories = _create_filenames_for_terms_with_prefix (directory,
+								     directory->notmuch, term);
+    } catch (Xapian::Error &error) {
+	LOG_XAPIAN_EXCEPTION (directory, error);
+    }
 
     talloc_free (term);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f9e9cc41..03db3b24 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2158,6 +2158,8 @@ notmuch_directory_get_child_files (notmuch_directory_t *directory);
  *
  * The returned filenames will be the basename-entries only (not
  * complete paths).
+ *
+ * Returns NULL if it triggers a Xapian exception
  */
 notmuch_filenames_t *
 notmuch_directory_get_child_directories (notmuch_directory_t *directory);
diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index e9e4cd0f..4d8b7e82 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -41,7 +41,6 @@ cat <<'EOF' > c_tail
 EOF
 
 test_begin_subtest "get child directories for a closed db"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_filenames_t *children;
-- 
2.27.0

[PATCH 06/13] test: add known broken test for n_directory_get_child_files

[David Bremner]

This is a clone of the one for get_child_directories
---
 test/T563-lib-directory.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 4d8b7e82..bbd12e12 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -57,4 +57,22 @@ A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "get child filenames for a closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_filenames_t *children;
+        children = notmuch_directory_get_child_files (dir);
+        printf ("%d\n", children == NULL);
+        stat = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

[PATCH 07/13] lib: catch exceptions in n_directory_get_child_files

[David Bremner]

Also clarify API in error case.
---
 lib/directory.cc           | 12 ++++++++----
 lib/notmuch.h              |  2 ++
 test/T563-lib-directory.sh |  1 -
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/directory.cc b/lib/directory.cc
index eb59d24d..044cd680 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -264,15 +264,19 @@ notmuch_filenames_t *
 notmuch_directory_get_child_files (notmuch_directory_t *directory)
 {
     char *term;
-    notmuch_filenames_t *child_files;
+    notmuch_filenames_t *child_files = NULL;
 
     term = talloc_asprintf (directory, "%s%u:",
 			    _find_prefix ("file-direntry"),
 			    directory->document_id);
 
-    child_files = _create_filenames_for_terms_with_prefix (directory,
-							   directory->notmuch,
-							   term);
+    try {
+	child_files = _create_filenames_for_terms_with_prefix (directory,
+							       directory->notmuch,
+							       term);
+    } catch (Xapian::Error &error) {
+	LOG_XAPIAN_EXCEPTION (directory, error);
+    }
 
     talloc_free (term);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 03db3b24..d52fa976 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2148,6 +2148,8 @@ notmuch_directory_get_mtime (notmuch_directory_t *directory);
  *
  * The returned filenames will be the basename-entries only (not
  * complete paths).
+ *
+ * Returns NULL if it triggers a Xapian exception
  */
 notmuch_filenames_t *
 notmuch_directory_get_child_files (notmuch_directory_t *directory);
diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index bbd12e12..739469a6 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -58,7 +58,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get child filenames for a closed db"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_filenames_t *children;
-- 
2.27.0

[PATCH 08/13] test: regression test for n_directory_delete with closed db.

[David Bremner]

To the best of my current understanding, it's a bug in Xapian that no
exception is thrown here. The test should pass in either case.
---
 test/T563-lib-directory.sh | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 739469a6..4de31078 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -26,7 +26,7 @@ int main (int argc, char** argv)
      exit (1);
    }
 
-   EXPECT0(notmuch_database_get_directory (db, "", &dir));
+   EXPECT0(notmuch_database_get_directory (db, "bar", &dir));
    EXPECT0(notmuch_database_close (db));
 EOF
 
@@ -74,4 +74,21 @@ A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+backup_database
+test_begin_subtest "delete directory document for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        stat = notmuch_directory_delete (dir);
+        printf ("%d\n", stat == NOTMUCH_STATUS_SUCCESS || stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+        stat = NOTMUCH_STATUS_SUCCESS;
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.27.0

[PATCH 09/13] test: regression test for n_directory_{get,set}_mtime

[David Bremner]

The mtime is cached, so closing the db is not a problem. Writing the
mtime throws an exception, which is caught.
---
 test/T563-lib-directory.sh | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 4de31078..c45bc5f8 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -91,4 +91,22 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
 
+backup_database
+test_begin_subtest "get/set mtime of directory for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        time_t stamp = notmuch_directory_get_mtime (dir);
+        stat = notmuch_directory_set_mtime (dir, stamp);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred setting directory mtime: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.27.0

[PATCH 10/13] test: regression test for n_d_get_config_list on closed db.

[David Bremner]

Exception is caught.
---
 test/T590-libconfig.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 46f3a76d..602fad0b 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -61,6 +61,21 @@ valid = 0
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_database_get_config_list: closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   notmuch_config_list_t *list;
+   EXPECT0(notmuch_database_close (db));
+   stat = notmuch_database_get_config_list (db, "nonexistent", &list);
+   printf("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_database_get_config_list: all pairs"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
-- 
2.27.0

[PATCH 11/13] test: regression test for traversing config list with closed db

[David Bremner]

Also mention error return in API docs
---
 lib/notmuch.h          |  1 +
 test/T590-libconfig.sh | 22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/lib/notmuch.h b/lib/notmuch.h
index d52fa976..1fe62914 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2291,6 +2291,7 @@ notmuch_config_list_key (notmuch_config_list_t *config_list);
  * next call to notmuch_config_list_value or notmuch config_list_destroy
  *
  * @since libnotmuch 4.4 (notmuch 0.23)
+ * @retval NULL for errors
  */
 const char *
 notmuch_config_list_value (notmuch_config_list_t *config_list);
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 602fad0b..360e45b0 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -100,6 +100,28 @@ zzzafter afterval
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_database_get_config_list: all pairs (closed db)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   notmuch_config_list_t *list;
+   EXPECT0(notmuch_database_get_config_list (db, "", &list));
+   EXPECT0(notmuch_database_close (db));
+   for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
+      printf("%s %d\n", notmuch_config_list_key (list), NULL == notmuch_config_list_value(list));
+   }
+   notmuch_config_list_destroy (list);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+aaabefore 1
+testkey1 1
+testkey2 1
+zzzafter 1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "notmuch_database_get_config_list: one prefix"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
-- 
2.27.0

[PATCH 12/13] test: regression tests for n_indexopts_{get,set}_decrypt_policy

[David Bremner]

The main criteria is that they don't crash. Working with a closed
database is a bonus.
---
 test/T562-lib-database.sh | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 32fda72e..815c9856 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -389,4 +389,43 @@ cat <<EOF > EXPECTED
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "get decryption policy from closed database"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_indexopts_t *result;
+        result = notmuch_database_get_default_indexopts (db);
+        EXPECT0(notmuch_database_close (db));
+        notmuch_decryption_policy_t policy = notmuch_indexopts_get_decrypt_policy (result);
+        printf ("%d\n",  policy == NOTMUCH_DECRYPT_AUTO);
+        notmuch_indexopts_destroy (result);
+        printf ("SUCCESS\n");
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+SUCCESS
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "set decryption policy with closed database"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        notmuch_indexopts_t *result;
+        result = notmuch_database_get_default_indexopts (db);
+        EXPECT0(notmuch_database_close (db));
+        notmuch_decryption_policy_t policy = notmuch_indexopts_get_decrypt_policy (result);
+        stat = notmuch_indexopts_set_decrypt_policy (result, policy);
+        printf("%d\n%d\n",  policy == NOTMUCH_DECRYPT_AUTO, stat == NOTMUCH_STATUS_SUCCESS);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.27.0

[PATCH 13/13] lib: return NULL from n_d_get_default_indexopts on error

[David Bremner]

This is a rare and probably serious programming error, so better not
to silently return a default value.
---
 lib/indexopts.c           | 2 +-
 lib/notmuch.h             | 1 +
 test/T562-lib-database.sh | 1 -
 3 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/indexopts.c b/lib/indexopts.c
index 1e8d180e..82a0026f 100644
--- a/lib/indexopts.c
+++ b/lib/indexopts.c
@@ -32,7 +32,7 @@ notmuch_database_get_default_indexopts (notmuch_database_t *db)
     char *decrypt_policy;
     notmuch_status_t err = notmuch_database_get_config (db, "index.decrypt", &decrypt_policy);
     if (err)
-	return ret;
+	return NULL;
 
     if (decrypt_policy) {
 	if ((! (strcasecmp (decrypt_policy, "true"))) ||
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 1fe62914..f3cb0fe2 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2325,6 +2325,7 @@ notmuch_config_list_destroy (notmuch_config_list_t *config_list);
  * added to the index.  At the moment it is a featureless stub.
  *
  * @since libnotmuch 5.1 (notmuch 0.26)
+ * @retval NULL in case of error
  */
 notmuch_indexopts_t *
 notmuch_database_get_default_indexopts (notmuch_database_t *db);
diff --git a/test/T562-lib-database.sh b/test/T562-lib-database.sh
index 815c9856..d6097418 100755
--- a/test/T562-lib-database.sh
+++ b/test/T562-lib-database.sh
@@ -373,7 +373,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "get indexopts from closed database"
-test_subtest_known_broken
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
     {
         notmuch_indexopts_t *result;
-- 
2.27.0

[PATCH 1/2] test: known broken test for n_directory_delete with closed db.

[David Bremner]

There is a return value bug in notmuch_directory_delete that is hiding
the exception.
---
 Well that was embarrassing. Silly bug in notmuch code, nothing to do with Xapian.
 
 test/T563-lib-directory.sh | 38 +++++++++++++++++++++++++++++++++++++-
 1 file changed, 37 insertions(+), 1 deletion(-)

diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
index 739469a6..8777f62b 100755
--- a/test/T563-lib-directory.sh
+++ b/test/T563-lib-directory.sh
@@ -26,7 +26,7 @@ int main (int argc, char** argv)
      exit (1);
    }
 
-   EXPECT0(notmuch_database_get_directory (db, "", &dir));
+   EXPECT0(notmuch_database_get_directory (db, "bar", &dir));
    EXPECT0(notmuch_database_close (db));
 EOF
 
@@ -74,4 +74,40 @@ A Xapian exception occurred at lib/directory.cc:XXX: Database has been closed
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+backup_database
+test_begin_subtest "delete directory document for a closed db"
+test_subtest_known_broken
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        stat = notmuch_directory_delete (dir);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred deleting directory entry: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
+backup_database
+test_begin_subtest "get/set mtime of directory for a closed db"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+    {
+        time_t stamp = notmuch_directory_get_mtime (dir);
+        stat = notmuch_directory_set_mtime (dir, stamp);
+        printf ("%d\n", stat == NOTMUCH_STATUS_XAPIAN_EXCEPTION);
+    }
+EOF
+cat <<EOF > EXPECTED
+== stdout ==
+1
+== stderr ==
+A Xapian exception occurred setting directory mtime: Database has been closed.
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.27.0

[PATCH 2/2] lib: fix return value for n_directory_delete

[David Bremner]

Falling out of the catch meant the error return was lost
---
 lib/directory.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/directory.cc b/lib/directory.cc
index 044cd680..37973c09 100644
--- a/lib/directory.cc
+++ b/lib/directory.cc
@@ -308,7 +308,7 @@ notmuch_directory_get_child_directories (notmuch_directory_t *directory)
 notmuch_status_t
 notmuch_directory_delete (notmuch_directory_t *directory)
 {
-    notmuch_status_t status;
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     Xapian::WritableDatabase *db;
 
     status = _notmuch_database_ensure_writable (directory->notmuch);
@@ -327,7 +327,7 @@ notmuch_directory_delete (notmuch_directory_t *directory)
     }
     notmuch_directory_destroy (directory);
 
-    return NOTMUCH_STATUS_SUCCESS;
+    return status;
 }
 
 void
-- 
2.27.0

Re: [PATCH 08/13] test: regression test for n_directory_delete with closed db.

[David Bremner]

David Bremner <david@tethera.net> writes:

> To the best of my current understanding, it's a bug in Xapian that no
> exception is thrown here. The test should pass in either case.
> ---
>  test/T563-lib-directory.sh | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/test/T563-lib-directory.sh b/test/T563-lib-directory.sh
> index 739469a6..4de31078 100755
> --- a/test/T563-lib-directory.sh
> +++ b/test/T563-lib-directory.sh
> @@ -26,7 +26,7 @@ int main (int argc, char** argv)
>       exit (1);
>     }
>  
> -   EXPECT0(notmuch_database_get_directory (db, "", &dir));
> +   EXPECT0(notmuch_database_get_directory (db, "bar", &dir));
>     EXPECT0(notmuch_database_close (db));
>  EOF
note to self. That change should be in the earlier commit that
introduces this particular bit of test scaffolding.