From 80d84bf6354e0faabc06cb56b72f039492a81313 Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Mon, 9 May 2022 20:58:35 +0300 Subject: [PATCH] store: use Result builder, add auto upgrade Make it a Result type, and add auto-upgrade (not enabled yet) Update dependents. --- lib/mu-store.cc | 42 ++++++++++++++++-- lib/mu-store.hh | 76 ++++++++++++++++++++++++++++---- lib/tests/test-mu-store.cc | 90 ++++++++++++++++++++------------------ lib/tests/test-parser.cc | 7 ++- lib/tests/test-query.cc | 12 ++--- mu/mu-cmd-server.cc | 11 ++--- mu/mu-cmd.cc | 31 +++++++++---- mu/tests/test-mu-query.cc | 17 +++---- 8 files changed, 201 insertions(+), 85 deletions(-) diff --git a/lib/mu-store.cc b/lib/mu-store.cc index baab078c..2a7de6ae 100644 --- a/lib/mu-store.cc +++ b/lib/mu-store.cc @@ -291,15 +291,47 @@ Store::Private::find_message_unlocked(Store::Id docid) const -Store::Store(const std::string& path, bool readonly) - : priv_{std::make_unique(path, readonly)} +Store::Store(const std::string& path, Store::Options opts) + : priv_{std::make_unique(path, none_of(opts & Store::Options::Writable))} { - if (properties().schema_version != ExpectedSchemaVersion) + if (properties().schema_version == ExpectedSchemaVersion) + return; // all is good. + + // Now, it seems the schema versions do not match; shall we automatically + // update? + + if (none_of(opts & Store::Options::AutoUpgrade)) { + // not allowed to auto-upgrade, so we give up. throw Mu::Error(Error::Code::SchemaMismatch, "expected schema-version %s, but got %s; " - "please use 'mu init'", + "cannot auto-upgrade; please use 'mu init'", ExpectedSchemaVersion, properties().schema_version.c_str()); + } + + // Okay, let's attempt an auto-upgrade. + g_info("attempt reinit database from schema %s --> %s", + properties().schema_version.c_str(), ExpectedSchemaVersion); + + Config conf; + conf.batch_size = properties().batch_size; + conf.max_message_size = properties().max_message_size; + + priv_.reset(); + priv_ = std::make_unique(path, + properties().root_maildir, + properties().personal_addresses, + conf); + // Now let's try again. + priv_.reset(); + priv_ = std::make_unique(path, none_of(opts & Store::Options::Writable)); + if (properties().schema_version != ExpectedSchemaVersion) + // Nope, we failed. + throw Mu::Error(Error::Code::SchemaMismatch, + "failed to auto-upgrade from %s to %s; " + "please use 'mu init'", + properties().schema_version.c_str(), + ExpectedSchemaVersion); } Store::Store(const std::string& path, @@ -310,6 +342,8 @@ Store::Store(const std::string& path, { } +Store::Store(Store&&) = default; + Store::~Store() = default; const Store::Properties& diff --git a/lib/mu-store.hh b/lib/mu-store.hh index 262ec32f..7e78eba0 100644 --- a/lib/mu-store.hh +++ b/lib/mu-store.hh @@ -44,12 +44,35 @@ public: static constexpr Id InvalidId = 0; /**< Invalid store id */ /** - * Construct a store for an existing document database + * Configuration options. + * + * @param path + * @param readonly + */ + enum struct Options { + None = 0, /**< No specific options */ + Writable = 1 << 0, /**< Open in writable mode */ + AutoUpgrade = 1 << 1, /**< automatically re-initialize + * versions do not match */ + }; + + /** + * Make a store for an existing document database * * @param path path to the database - * @param readonly whether to open the database in read-only mode + * @param options startup options + * + * A store or an error. */ - Store(const std::string& path, bool readonly = true); + static Result make(const std::string& path, + Options opts=Options::None) noexcept try { + return Ok(Store{path, opts}); + + } catch (const Mu::Error& me) { + return Err(me); + } catch (...) { + return Err(Error::Code::Internal, "failed to create store"); + } struct Config { size_t max_message_size{}; @@ -67,10 +90,24 @@ public: * 'personal' for identifying personal messages. * @param config a configuration object */ - Store(const std::string& path, - const std::string& maildir, - const StringVec& personal_addresses, - const Config& conf); + static Result make_new(const std::string& path, + const std::string& maildir, + const StringVec& personal_addresses, + const Config& conf) noexcept try { + + return Ok(Store(path, maildir, personal_addresses, conf)); + + } catch (const Mu::Error& me) { + return Err(me); + } catch (...) { + return Err(Error::Code::Internal, "failed to create store"); + } + + /** + * Move CTOR + * + */ + Store(Store&&); /** * DTOR @@ -159,7 +196,7 @@ public: * @param expr a xapian search expression * @param xapian if true, show Xapian's internal representation, * otherwise, mu's. - + * * @return the string representation of the query */ std::string parse_query(const std::string& expr, bool xapian) const; @@ -375,10 +412,33 @@ public: const std::unique_ptr& priv() const { return priv_; } private: + /** + * Construct a store for an existing document database + * + * @param path path to the database + * @param options startup options + */ + Store(const std::string& path, Options opts=Options::None); + + /** + * Construct a store for a not-yet-existing document database + * + * @param path path to the database + * @param maildir maildir to use for this store + * @param personal_addresses addresses that should be recognized as + * 'personal' for identifying personal messages. + * @param config a configuration object + */ + Store(const std::string& path, + const std::string& maildir, + const StringVec& personal_addresses, + const Config& conf); std::unique_ptr priv_; }; +MU_ENABLE_BITOPS(Store::Options); + } // namespace Mu #endif /* __MU_STORE_HH__ */ diff --git a/lib/tests/test-mu-store.cc b/lib/tests/test-mu-store.cc index 0c6a952f..b64e5f34 100644 --- a/lib/tests/test-mu-store.cc +++ b/lib/tests/test-mu-store.cc @@ -43,13 +43,14 @@ static void test_store_ctor_dtor() { TempDir tempdir; - Mu::Store store{tempdir.path(), "/tmp", {}, {}}; + auto store{Store::make_new(tempdir.path(), "/tmp", {}, {})}; + assert_valid_result(store); - g_assert_true(store.empty()); - g_assert_cmpuint(0, ==, store.size()); + g_assert_true(store->empty()); + g_assert_cmpuint(0, ==, store->size()); g_assert_cmpstr(MU_STORE_SCHEMA_VERSION, ==, - store.properties().schema_version.c_str()); + store->properties().schema_version.c_str()); } static void @@ -57,37 +58,38 @@ test_store_add_count_remove() { TempDir tempdir{false}; - Mu::Store store{tempdir.path() + "/xapian", MuTestMaildir, {}, {}}; + auto store{Store::make_new(tempdir.path() + "/xapian", MuTestMaildir, {}, {})}; + assert_valid_result(store); const auto msgpath{MuTestMaildir + "/cur/1283599333.1840_11.cthulhu!2,"}; - const auto id1 = store.add_message(msgpath); + const auto id1 = store->add_message(msgpath); assert_valid_result(id1); - store.commit(); + store->commit(); - g_assert_cmpuint(store.size(), ==, 1); - g_assert_true(store.contains_message(msgpath)); + g_assert_cmpuint(store->size(), ==, 1); + g_assert_true(store->contains_message(msgpath)); - g_assert_true(store.contains_message(msgpath)); + g_assert_true(store->contains_message(msgpath)); - const auto id2 = store.add_message(MuTestMaildir2 + "/bar/cur/mail3"); + const auto id2 = store->add_message(MuTestMaildir2 + "/bar/cur/mail3"); g_assert_false(!!id2); // wrong maildir. - store.commit(); + store->commit(); const auto msg3path{MuTestMaildir + "/cur/1252168370_3.14675.cthulhu!2,S"}; - const auto id3 = store.add_message(msg3path); + const auto id3 = store->add_message(msg3path); assert_valid_result(id3); - g_assert_cmpuint(store.size(), ==, 2); - g_assert_true(store.contains_message(msg3path)); + g_assert_cmpuint(store->size(), ==, 2); + g_assert_true(store->contains_message(msg3path)); - store.remove_message(id1.value()); - g_assert_cmpuint(store.size(), ==, 1); + store->remove_message(id1.value()); + g_assert_cmpuint(store->size(), ==, 1); g_assert_false( - store.contains_message(MuTestMaildir + "/cur/1283599333.1840_11.cthulhu!2,")); + store->contains_message(MuTestMaildir + "/cur/1283599333.1840_11.cthulhu!2,")); - store.remove_message(msg3path); - g_assert_true(store.empty()); - g_assert_false(store.contains_message(msg3path)); + store->remove_message(msg3path); + g_assert_true(store->empty()); + g_assert_false(store->contains_message(msg3path)); } @@ -126,23 +128,24 @@ statement you can use something like: goto * instructions[pOp->opcode]; )"; TempDir tempdir; - Mu::Store store{tempdir.path(), "/home/test/Maildir", {}, {}}; + auto store{Store::make_new(tempdir.path(), "/home/test/Maildir", {}, {})}; + assert_valid_result(store); const auto msgpath{"/home/test/Maildir/inbox/cur/1649279256.107710_1.evergrey:2,S"}; auto message{Message::make_from_text(test_message_1, msgpath)}; assert_valid_result(message); - const auto docid = store.add_message(*message); + const auto docid = store->add_message(*message); assert_valid_result(docid); - g_assert_cmpuint(store.size(),==, 1); + g_assert_cmpuint(store->size(),==, 1); - auto msg2{store.find_message(*docid)}; + auto msg2{store->find_message(*docid)}; g_assert_true(!!msg2); assert_equal(message->path(), msg2->path()); - g_assert_true(store.contains_message(message->path())); + g_assert_true(store->contains_message(message->path())); - const auto qr = store.run_query("to:sqlite-dev@sqlite.org"); + const auto qr = store->run_query("to:sqlite-dev@sqlite.org"); g_assert_true(!!qr); g_assert_cmpuint(qr->size(), ==, 1); } @@ -204,22 +207,23 @@ World! )"; TempDir tempdir; - Mu::Store store{tempdir.path(), "/home/test/Maildir", {}, {}}; + auto store{Store::make_new(tempdir.path(), "/home/test/Maildir", {}, {})}; + assert_valid_result(store); auto message{Message::make_from_text( msg_text, "/home/test/Maildir/inbox/cur/1649279256.abcde_1.evergrey:2,S")}; assert_valid_result(message); - const auto docid = store.add_message(*message); + const auto docid = store->add_message(*message); assert_valid_result(docid); - store.commit(); + store->commit(); - auto msg2{store.find_message(*docid)}; + auto msg2{store->find_message(*docid)}; g_assert_true(!!msg2); assert_equal(message->path(), msg2->path()); - g_assert_true(store.contains_message(message->path())); + g_assert_true(store->contains_message(message->path())); // for (auto&& term = msg2->document().xapian_document().termlist_begin(); // term != msg2->document().xapian_document().termlist_end(); ++term) @@ -272,21 +276,23 @@ Yes, that would be excellent. // Index it into a store. TempDir tempdir; - Store store{tempdir.path(), tempdir2.path() + "/Maildir", {}, {}}; - store.indexer().start({}); + auto store{Store::make_new(tempdir.path(), tempdir2.path() + "/Maildir", {}, {})}; + assert_valid_result(store); + + store->indexer().start({}); size_t n{}; - while (store.indexer().is_running()) { + while (store->indexer().is_running()) { std::this_thread::sleep_for(100ms); g_assert_cmpuint(n++,<=,25); } - g_assert_true(!store.indexer().is_running()); - const auto& prog{store.indexer().progress()}; + g_assert_true(!store->indexer().is_running()); + const auto& prog{store->indexer().progress()}; g_assert_cmpuint(prog.updated,==,1); - g_assert_cmpuint(store.size(), ==, 1); - g_assert_false(store.empty()); + g_assert_cmpuint(store->size(), ==, 1); + g_assert_false(store->empty()); // Find the message - auto qr = store.run_query("path:" + tempdir2.path() + "/Maildir/a/new/msg"); + auto qr = store->run_query("path:" + tempdir2.path() + "/Maildir/a/new/msg"); assert_valid_result(qr); g_assert_cmpuint(qr->size(),==,1); @@ -301,7 +307,7 @@ Yes, that would be excellent. // Move the message from new->cur std::this_thread::sleep_for(1s); /* ctime should change */ - const auto msg3 = store.move_message(msg->docid(), {}, Flags::Seen); + const auto msg3 = store->move_message(msg->docid(), {}, Flags::Seen); assert_valid_result(msg3); assert_equal(msg3->maildir(), "/a"); assert_equal(msg3->path(), tempdir2.path() + "/Maildir/a/cur/msg:2,S"); @@ -309,7 +315,7 @@ Yes, that would be excellent. g_assert_false(::access(oldpath.c_str(), R_OK)==0); g_debug("%s", msg3->to_sexp().to_sexp_string().c_str()); - g_assert_cmpuint(store.size(), ==, 1); + g_assert_cmpuint(store->size(), ==, 1); } diff --git a/lib/tests/test-parser.cc b/lib/tests/test-parser.cc index 79d06769..4429040f 100644 --- a/lib/tests/test-parser.cc +++ b/lib/tests/test-parser.cc @@ -26,6 +26,7 @@ #include "test-mu-common.hh" #include "mu-parser.hh" +#include "utils/mu-result.hh" #include "utils/mu-utils.hh" using namespace Mu; @@ -42,10 +43,12 @@ test_cases(const CaseVec& cases) { char* tmpdir = test_mu_common_get_random_tmpdir(); g_assert(tmpdir); - Mu::Store dummy_store{tmpdir, "/tmp", {}, {}}; + auto dummy_store{Store::make_new(tmpdir, "/tmp", {}, {})}; + assert_valid_result(dummy_store); + g_free(tmpdir); - Parser parser{dummy_store, Parser::Flags::UnitTest}; + Parser parser{*dummy_store, Parser::Flags::UnitTest}; for (const auto& casus : cases) { WarningVec warnings; diff --git a/lib/tests/test-query.cc b/lib/tests/test-query.cc index 5f9b7b23..d84fbea3 100644 --- a/lib/tests/test-query.cc +++ b/lib/tests/test-query.cc @@ -28,6 +28,7 @@ #include "mu-store.hh" #include "mu-query.hh" #include "index/mu-indexer.hh" +#include "utils/mu-result.hh" #include "utils/mu-utils.hh" #include "test-mu-common.hh" @@ -40,10 +41,11 @@ test_query() char* tdir; tdir = test_mu_common_get_random_tmpdir(); - Store store{tdir, std::string{MU_TESTMAILDIR}, {}, {}}; + auto store = Store::make_new(tdir, std::string{MU_TESTMAILDIR}, {}, {}); + assert_valid_result(store); g_free(tdir); - auto&& idx{store.indexer()}; + auto&& idx{store->indexer()}; g_assert_true(idx.start(Indexer::Config{})); while (idx.is_running()) { @@ -60,17 +62,17 @@ test_query() item.message_id().value_or("").c_str()); }; - g_assert_cmpuint(store.size(), ==, 19); + g_assert_cmpuint(store->size(), ==, 19); { - const auto res = store.run_query("", {}, QueryFlags::None); + const auto res = store->run_query("", {}, QueryFlags::None); g_assert_true(!!res); g_assert_cmpuint(res->size(), ==, 19); dump_matches(*res); } { - const auto res = store.run_query("", Field::Id::Path, QueryFlags::None, 11); + const auto res = store->run_query("", Field::Id::Path, QueryFlags::None, 11); g_assert_true(!!res); g_assert_cmpuint(res->size(), ==, 11); dump_matches(*res); diff --git a/mu/mu-cmd-server.cc b/mu/mu-cmd-server.cc index 54daea14..453fb674 100644 --- a/mu/mu-cmd-server.cc +++ b/mu/mu-cmd-server.cc @@ -116,16 +116,17 @@ report_error(const Mu::Error& err) noexcept MuError Mu::mu_cmd_server(const MuConfig* opts, GError** err) try { - Store store{mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), false /*writable*/}; - Server server{store, output_sexp_stdout}; + auto store = Store::make(mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), Store::Options::Writable); + if (!store) + throw store.error(); + Server server{*store, output_sexp_stdout}; g_message("created server with store @ %s; maildir @ %s; debug-mode %s", - store.properties().database_path.c_str(), - store.properties().root_maildir.c_str(), + store->properties().database_path.c_str(), + store->properties().root_maildir.c_str(), opts->debug ? "yes" : "no"); tty = ::isatty(::fileno(stdout)); - const auto eval = std::string{opts->commands ? "(help :full t)" : opts->eval ? opts->eval : ""}; diff --git a/mu/mu-cmd.cc b/mu/mu-cmd.cc index f4d30e3e..e900076f 100644 --- a/mu/mu-cmd.cc +++ b/mu/mu-cmd.cc @@ -549,9 +549,13 @@ cmd_init(const MuConfig* opts, GError** err) ++addrs; } - Mu::Store store(mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), opts->maildir, my_addrs, conf); + auto store = Store::make_new(mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), + opts->maildir, my_addrs, conf); + if (!store) + throw store.error(); + if (!opts->quiet) { - cmd_info(store, opts, NULL); + cmd_info(*store, opts, NULL); std::cout << "\nstore created; use the 'index' command to fill/update it.\n"; } @@ -561,9 +565,11 @@ cmd_init(const MuConfig* opts, GError** err) static Result cmd_find(const MuConfig* opts) { - Mu::Store store{mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), true /*readonly*/}; - - return mu_cmd_find(store, opts); + auto store{Store::make(mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB))}; + if (!store) + return Err(store.error()); + else + return mu_cmd_find(*store, opts); } static void @@ -582,15 +588,22 @@ typedef MuError (*writable_store_func)(Mu::Store&, const MuConfig*, GError** err static MuError with_readonly_store(readonly_store_func func, const MuConfig* opts, GError** err) { - const Mu::Store store{mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), true /*readonly*/}; - return func(store, opts, err); + auto store{Store::make(mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB))}; + if (!store) + throw store.error(); + + return func(*store, opts, err); } static MuError with_writable_store(writable_store_func func, const MuConfig* opts, GError** err) { - Mu::Store store{mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), false /*!readonly*/}; - return func(store, opts, err); + auto store{Store::make(mu_runtime_path(MU_RUNTIME_PATH_XAPIANDB), + Store::Options::Writable)}; + if (!store) + throw store.error(); + + return func(*store, opts, err); } static gboolean diff --git a/mu/tests/test-mu-query.cc b/mu/tests/test-mu-query.cc index 81e13692..781d3ce7 100644 --- a/mu/tests/test-mu-query.cc +++ b/mu/tests/test-mu-query.cc @@ -32,6 +32,7 @@ #include "test-mu-common.hh" #include "mu-query.hh" +#include "utils/mu-result.hh" #include "utils/mu-str.h" #include "utils/mu-utils.hh" #include "mu-store.hh" @@ -90,7 +91,8 @@ run_and_count_matches(const std::string& xpath, const std::string& expr, Mu::QueryFlags flags = Mu::QueryFlags::None) { - Mu::Store store{xpath}; + auto store{Store::make(xpath)}; + assert_valid_result(store); // if (g_test_verbose()) { // std::cout << "==> mquery: " << store.parse_query(expr, false) << "\n"; @@ -99,7 +101,7 @@ run_and_count_matches(const std::string& xpath, Mu::allow_warnings(); - auto qres{store.run_query(expr, {}, flags)}; + auto qres{store->run_query(expr, {}, flags)}; g_assert_true(!!qres); assert_no_dups(*qres); @@ -237,11 +239,10 @@ test_mu_query_logic(void) static void test_mu_query_accented_chars_01(void) { - return; + auto store = Store::make(DB_PATH1); + assert_valid_result(store); - Store store{DB_PATH1}; - - auto qres{store.run_query("fünkÿ")}; + auto qres{store->run_query("fünkÿ")}; g_assert_true(!!qres); g_assert_false(qres->empty()); @@ -252,10 +253,6 @@ test_mu_query_accented_chars_01(void) } assert_equal(msg->subject(), "Greetings from Lothlórien"); - const auto summ{to_string_opt_gchar( - mu_str_summarize(msg->body_text().value_or("").c_str(), 5))}; - g_assert_true(!!summ); - assert_equal(*summ, "Let's write some fünkÿ text using umlauts. Foo."); } static void