From 0b4f7c4cbedd9df10f8888544983db4b4be2d8ee Mon Sep 17 00:00:00 2001 From: "Dirk-Jan C. Binnema" Date: Sun, 9 Jul 2023 23:17:08 +0300 Subject: [PATCH] lib: xapian-db/store: simplify No need for "pimpl" in xapian-db; keep it simple. --- lib/mu-store.cc | 13 ++----- lib/mu-xapian-db.cc | 93 +++++++++++++++------------------------------ lib/mu-xapian-db.hh | 39 +++++-------------- 3 files changed, 44 insertions(+), 101 deletions(-) diff --git a/lib/mu-store.cc b/lib/mu-store.cc index d5571fa3..ddcd2eba 100644 --- a/lib/mu-store.cc +++ b/lib/mu-store.cc @@ -65,8 +65,8 @@ remove_slash(const std::string& str) struct Store::Private { Private(const std::string& path, bool readonly): - xapian_db_{make_db(path, readonly ? XapianDb::Flavor::ReadOnly - : XapianDb::Flavor::Open)}, + xapian_db_{XapianDb(path, readonly ? XapianDb::Flavor::ReadOnly + : XapianDb::Flavor::Open)}, config_{xapian_db_}, contacts_cache_{config_}, root_maildir_{remove_slash(config_.get())} @@ -74,7 +74,7 @@ struct Store::Private { Private(const std::string& path, const std::string& root_maildir, Option conf): - xapian_db_{make_db(path, XapianDb::Flavor::CreateOverwrite)}, + xapian_db_{XapianDb(path, XapianDb::Flavor::CreateOverwrite)}, config_{make_config(xapian_db_, root_maildir, conf)}, contacts_cache_{config_}, root_maildir_{remove_slash(config_.get())} @@ -118,13 +118,6 @@ struct Store::Private { } } - XapianDb make_db(const std::string& path, XapianDb::Flavor flavor) { - if (auto&& res{XapianDb::make(path, flavor)}; res) - return std::move(res.value()); - else - throw res.error(); - } - Config make_config(XapianDb& xapian_db, const std::string& root_maildir, Option conf) { diff --git a/lib/mu-xapian-db.cc b/lib/mu-xapian-db.cc index d2ffc2fa..ccda245b 100644 --- a/lib/mu-xapian-db.cc +++ b/lib/mu-xapian-db.cc @@ -26,38 +26,13 @@ using namespace Mu; -struct XapianDb::Private { - using DbType = std::variant; - - Private(Xapian::Database&& db, const std::string& path): - db_{std::move(db)}, path_{path} {} - Private(Xapian::WritableDatabase&& wdb, const std::string& path): - db_{std::move(wdb)}, path_{path} {} - - DbType db_; - const std::string path_; - mutable std::mutex lock_; -}; - - -XapianDb::XapianDb(Xapian::Database&& db, const std::string& path): - priv_{std::make_unique(std::move(db), path)} -{} -XapianDb::XapianDb(Xapian::WritableDatabase&& wdb, const std::string& path): - priv_{std::make_unique(std::move(wdb), path)} -{} - -XapianDb::XapianDb(XapianDb&& rhs) = default; -XapianDb::~XapianDb() = default; - - const Xapian::Database& XapianDb::db() const { - if (std::holds_alternative(priv_->db_)) - return std::get(priv_->db_); + if (std::holds_alternative(db_)) + return std::get(db_); else - return std::get(priv_->db_); + return std::get(db_); } Xapian::WritableDatabase& @@ -65,25 +40,19 @@ XapianDb::wdb() { if (read_only()) throw std::runtime_error("database is read-only"); - return std::get(priv_->db_); + return std::get(db_); } bool XapianDb::read_only() const { - return !std::holds_alternative(priv_->db_); + return !std::holds_alternative(db_); } const std::string& XapianDb::path() const { - return priv_->path_; -} - -std::mutex& -XapianDb::lock() const -{ - return priv_->lock_; + return path_; } void @@ -92,44 +61,44 @@ XapianDb::set_timestamp(const std::string_view key) wdb().set_metadata(std::string{key}, mu_format("{}", ::time({}))); } -Result -XapianDb::make(const std::string& db_path, Flavor flavor) noexcept try { +using Flavor = XapianDb::Flavor; +static std::string +make_path(const std::string& db_path, Flavor flavor) +{ if (flavor != Flavor::ReadOnly) { /* we do our own flushing, set Xapian's internal one as * the backstop*/ g_setenv("XAPIAN_FLUSH_THRESHOLD", "500000", 1); /* create path if needed */ if (g_mkdir_with_parents(db_path.c_str(), 0700) != 0) - return Err(Error::Code::File, "failed to create database dir {}: {}", - db_path, ::strerror(errno)); + throw Error(Error::Code::File, "failed to create database dir {}: {}", + db_path, ::strerror(errno)); } + return db_path; +} + +static XapianDb::DbType +make_db(const std::string& db_path, Flavor flavor) +{ switch (flavor) { case Flavor::ReadOnly: - return Ok(XapianDb(Xapian::Database(db_path), db_path)); + return Xapian::Database(db_path); case Flavor::Open: - return Ok(XapianDb(Xapian::WritableDatabase( - db_path, Xapian::DB_OPEN), db_path)); - case Flavor::CreateOverwrite: { - auto&& xdb{XapianDb(Xapian::WritableDatabase( - db_path, - Xapian::DB_CREATE_OR_OVERWRITE), db_path)}; - xdb.set_timestamp(MetadataIface::created_key); - return Ok(std::move(xdb)); - } + return Xapian::WritableDatabase(db_path, Xapian::DB_OPEN); + case Flavor::CreateOverwrite: + return Xapian::WritableDatabase(db_path, Xapian::DB_CREATE_OR_OVERWRITE); default: - return Err(Error::Code::Internal, "invalid xapian options"); + throw std::logic_error("unknown flavor"); } - -} catch (const Xapian::DatabaseLockError& xde) { - return Err(Error::Code::StoreLock, "{}", xde.get_msg()); -} catch (const Xapian::DatabaseError& xde) { - return Err(Error::Code::Store, "{}", xde.get_msg()); -} catch (const Mu::Error& me) { - return Err(me); -} catch (...) { - return Err(Error::Code::Internal, - "something went wrong when opening store @ {}", db_path); +} + +XapianDb::XapianDb(const std::string& db_path, Flavor flavor) : + path_(make_path(db_path, flavor)), + db_(make_db(path_,flavor)) { + + if (flavor == Flavor::CreateOverwrite) + set_timestamp(MetadataIface::created_key); } diff --git a/lib/mu-xapian-db.hh b/lib/mu-xapian-db.hh index 2e57f779..7380da14 100644 --- a/lib/mu-xapian-db.hh +++ b/lib/mu-xapian-db.hh @@ -162,7 +162,7 @@ private: * with just the things we need + locking + exception handling */ class XapianDb: public MetadataIface { -#define DB_LOCKED std::unique_lock l{lock()} +#define DB_LOCKED std::unique_lock lock__{lock_}; public: /** * Type of database to create. @@ -174,15 +174,7 @@ public: CreateOverwrite, /**< Create new or overwrite existing */ }; - /** - * Make a new Xapian Database wrapper object - * - * @param db_path path to the database - * @param flavor type of database to created - * - * @return a result; either a XapianDb or an Error. - */ - static Result make(const std::string& db_path, Flavor flavor) noexcept; + XapianDb(const std::string& db_path, Flavor flavor); /** * Is the database read-only? @@ -364,6 +356,11 @@ public: xapian_try([&]{ DB_LOCKED; return wdb().commit_transaction();}); } + using DbType = std::variant; + +private: + void set_timestamp(const std::string_view key); + /** * Get a reference to the underlying database * @@ -378,26 +375,10 @@ public: */ Xapian::WritableDatabase& wdb(); - /** - * DTOR - */ - ~XapianDb(); + mutable std::mutex lock_; + std::string path_; - /** - * Move CTOR - * - * @param rhs - */ - XapianDb(XapianDb&& rhs); - -private: - XapianDb(Xapian::Database&& db, const std::string& path); - XapianDb(Xapian::WritableDatabase&& wdb, const std::string& path); - void set_timestamp(const std::string_view key); - std::mutex& lock() const; - - struct Private; - std::unique_ptr priv_; + DbType db_; }; } // namespace Mu