diff --git a/lib/mu-maildir.cc b/lib/mu-maildir.cc index f877c1a4..d4392650 100644 --- a/lib/mu-maildir.cc +++ b/lib/mu-maildir.cc @@ -205,18 +205,22 @@ clear_links(const std::string& path, DIR* dir) switch(d_type) { case DT_LNK: if (::unlink(fullpath.c_str()) != 0) { + /* LCOV_EXCL_START*/ mu_warning("error unlinking {}: {}", fullpath, g_strerror(errno)); res = false; + /* LCOV_EXCL_STOP*/ } break; case DT_DIR: { DIR* subdir{::opendir(fullpath.c_str())}; + /* LCOV_EXCL_START*/ if (!subdir) { mu_warning("error opening dir {}: {}", fullpath, g_strerror(errno)); res = false; } if (!clear_links(fullpath, subdir)) res = false; + /* LCOV_EXCL_STOP*/ ::closedir(subdir); } break; default: @@ -299,20 +303,27 @@ msg_move_mv_file(const std::string& src, const std::string& dst) return Ok(); } -static Mu::Result -msg_move(const std::string& src, const std::string& dst, bool assume_remote) +Mu::Result +Mu::maildir_move_message(const std::string& oldpath, + const std::string& newpath, + bool assume_remote) { - if (::access(src.c_str(), R_OK) != 0) - return Err(Error{Error::Code::File, "cannot read {}", src}); + mu_debug("moving {} --> {} (assume-remote:{})", oldpath, newpath, assume_remote); + + if (::access(oldpath.c_str(), R_OK) != 0) + return Err(Error{Error::Code::File, "cannot read {}", oldpath}); + + if (oldpath == newpath) + return Ok(); // nothing to do. if (!assume_remote) { /* for testing */ - if (::rename(src.c_str(), dst.c_str()) == 0) /* seems it worked; double-check */ - return msg_move_verify(src, dst); + if (::rename(oldpath.c_str(), newpath.c_str()) == 0) /* seems it worked; double-check */ + return msg_move_verify(oldpath, newpath); /* LCOV_EXCL_START*/ if (errno != EXDEV) /* some unrecoverable error occurred */ return Err(Error{Error::Code::File, "error moving {} -> {}: {}", - src, dst, strerror(errno)}); + oldpath, newpath, strerror(errno)}); /* LCOV_EXCL_STOP*/ } @@ -323,32 +334,17 @@ msg_move(const std::string& src, const std::string& dst, bool assume_remote) * we use the latter for now, since the former gives some (false) * valgrind alarms. * */ - if (auto&& res{msg_move_mv_file(src, dst)}; !res) + if (auto&& res{msg_move_mv_file(oldpath, newpath)}; !res) return res; else - return msg_move_verify(src, dst); -} - -Mu::Result -Mu::maildir_move_message(const std::string& oldpath, - const std::string& newpath, - bool force_gio) -{ - if (oldpath == newpath) - return Ok(); // nothing to do. - - mu_debug("moving {} --> {} (force-gio:{})", oldpath, newpath, force_gio ? "yes": "no"); - return msg_move(oldpath, newpath, force_gio); + return msg_move_verify(oldpath, newpath); } static std::string reinvent_filename_base() { - return mu_format("{}.{:08x}{:08x}.{}", - ::time({}), - g_random_int(), - g_get_monotonic_time(), - g_get_host_name()); + return mu_format("{}.{:08x}{:08x}.{}", ::time({}), + g_random_int(), g_get_monotonic_time(), g_get_host_name()); } /** diff --git a/lib/mu-maildir.hh b/lib/mu-maildir.hh index 93f12481..6c3153b5 100644 --- a/lib/mu-maildir.hh +++ b/lib/mu-maildir.hh @@ -75,14 +75,13 @@ Result maildir_link(const std::string& src, const std::string& targetpath, Result maildir_clear_links(const std::string& dir); /** - * Move a message file to another maildir. If the target exists, it is - * overwritten. + * Move a message file to another maildir. If the target exists, it is overwritten. * * @param oldpath an absolute file system path to an existing message in an * actual maildir * @param newpath the absolute full path to the target file * @param assume_remote assume the target is on a different file-system, - * and hence rename() won't work and we need another method (this is for testing) + * and hence rename() won't work and we need another method * * @return a valid result or an Error */ diff --git a/lib/tests/test-mu-maildir.cc b/lib/tests/test-mu-maildir.cc index 639a1342..62cd3215 100644 --- a/lib/tests/test-mu-maildir.cc +++ b/lib/tests/test-mu-maildir.cc @@ -469,6 +469,8 @@ test_maildir_link() g_assert_true(g_access(dstpath1.c_str(), F_OK) == 0); g_assert_true(g_access(dstpath2.c_str(), F_OK) == 0); + g_assert_false(!!maildir_clear_links("/nonexistent/bla/foo/xuux")); + assert_valid_result(maildir_clear_links(tmpdir.path() + "/bar")); g_assert_false(g_access(dstpath1.c_str(), F_OK) == 0); g_assert_false(g_access(dstpath2.c_str(), F_OK) == 0); @@ -476,15 +478,15 @@ test_maildir_link() static void -test_maildir_move(bool use_gio) +test_maildir_move(bool assume_remote) { TempDir tmpdir; assert_valid_result(maildir_mkdir(tmpdir.path() + "/foo")); assert_valid_result(maildir_mkdir(tmpdir.path() + "/bar")); - const auto srcpath1 = tmpdir.path() + "/foo/cur/msg1"; - const auto srcpath2 = tmpdir.path() + "/foo/new/msg2"; + const auto srcpath1{join_paths(tmpdir.path(), "/foo/cur/msg1")}; + const auto srcpath2{join_paths(tmpdir.path(), "/foo/new/msg2")}; { std::ofstream stream(srcpath1); @@ -502,23 +504,22 @@ test_maildir_move(bool use_gio) const auto dstpath = tmpdir.path() + "/test1"; - assert_valid_result(maildir_move_message(srcpath1, dstpath, use_gio)); - assert_valid_result(maildir_move_message(srcpath2, dstpath, use_gio)); + assert_valid_result(maildir_move_message(srcpath1, dstpath, assume_remote)); + assert_valid_result(maildir_move_message(srcpath2, dstpath, assume_remote)); - - //g_assert_true(g_access(dstpath.c_str(), F_OK) == 0); + assert_valid_result(maildir_move_message(dstpath, dstpath)); // self-move is okay. } static void test_maildir_move_vanilla() { - test_maildir_move(false/*!gio*/); + test_maildir_move(false/*!assume_remote*/); } static void test_maildir_move_gio() { - test_maildir_move(true/*gio*/); + test_maildir_move(true/*assume_remote*/); } diff --git a/lib/tests/test-mu-store.cc b/lib/tests/test-mu-store.cc index 0a525cbd..0249c3a0 100644 --- a/lib/tests/test-mu-store.cc +++ b/lib/tests/test-mu-store.cc @@ -524,7 +524,32 @@ test_store_maildirs() } +static void +test_store_parse() +{ + allow_warnings(); + TempDir tdir; + auto store = Store::make_new(tdir.path(), MU_TESTMAILDIR2); + assert_valid_result(store); + g_assert_true(store->empty()); + + // Xapian internal format (get_description()) is _not_ guaranteed + // to be the same between versions + const auto&& pq1{store->parse_query("subject:\"hello world\"", false)}; + const auto&& pq2{store->parse_query("subject:\"hello world\"", true)}; + + assert_equal(pq1, "(or (subject \"hello world\") (subject (phrase \"hello world\")))"); + + /* LCOV_EXCL_START*/ + if (pq2 != "Query((Shello world OR (Shello PHRASE 2 Sworld)))") { + g_test_skip("incompatible xapian descriptions"); + return; + } + /* LCOV_EXCL_STOP*/ + + assert_equal(pq2, "Query((Shello world OR (Shello PHRASE 2 Sworld)))"); +} static void test_store_fail() @@ -557,6 +582,7 @@ main(int argc, char* argv[]) g_test_add_func("/store/move-dups", test_store_move_dups); g_test_add_func("/store/maildirs", test_store_maildirs); + g_test_add_func("/store/parse", test_store_parse); g_test_add_func("/store/index/index-move", test_index_move); g_test_add_func("/store/index/circular-symlink", test_store_circular_symlink);