maildir: improve testing coverage

Remove some dead/unused code. Update docs. Add test cases.
This commit is contained in:
Dirk-Jan C. Binnema 2022-06-29 22:19:26 +03:00
parent fc25bb2866
commit 97459beed9
4 changed files with 91 additions and 75 deletions

View File

@ -257,43 +257,8 @@ Mu::maildir_clear_links(const std::string& path)
return Ok();
}
static size_t
get_file_size(const std::string& path)
{
int rv;
struct stat statbuf;
rv = ::stat(path.c_str(), &statbuf);
if (rv != 0) {
/* g_warning ("error: %s", g_strerror (errno)); */
return -1;
}
return static_cast<size_t>(statbuf.st_size);
}
static Mu::Result<void>
msg_move_check_pre(const std::string& src, const std::string& dst)
{
if (::access(src.c_str(), R_OK) != 0)
return Err(Error{Error::Code::File, "cannot read %s", src.c_str()});
if (::access(dst.c_str(), F_OK) != 0)
return Ok();
/* target exist; we simply overwrite it, unless target has a different
* size. ignore the exceedingly rare case where have duplicate message
* file names with different content yet the same length. (md5 etc. is a
* bit slow) */
if (get_file_size(src) != get_file_size(dst))
return Err(Error{Error::Code::File, "%s already exists", dst.c_str()});
return Ok();
}
static Mu::Result<void>
msg_move_check_post(const std::string& src, const std::string& dst)
msg_move_verify(const std::string& src, const std::string& dst)
{
/* double check -- is the target really there? */
if (::access(dst.c_str(), F_OK) != 0)
@ -322,9 +287,9 @@ msg_move_g_file(const std::string& src, const std::string& dst)
GFile *dstfile{g_file_new_for_path(dst.c_str())};
GError* err{};
auto res = g_file_move(srcfile, dstfile, G_FILE_COPY_NONE,
auto res = g_file_move(srcfile, dstfile,
G_FILE_COPY_OVERWRITE,
NULL, NULL, NULL, &err);
g_clear_object(&srcfile);
g_clear_object(&dstfile);
@ -337,38 +302,41 @@ msg_move_g_file(const std::string& src, const std::string& dst)
}
static Mu::Result<void>
msg_move(const std::string& src, const std::string& dst)
msg_move(const std::string& src, const std::string& dst, bool force_gio)
{
if (auto&& res = msg_move_check_pre(src, dst); !res)
return res;
if (::access(src.c_str(), R_OK) != 0)
return Err(Error{Error::Code::File, "cannot read %s", src.c_str()});
if (::rename(src.c_str(), dst.c_str()) == 0) /* seems it worked; double-check */
return msg_move_check_post(src, dst);
if (!force_gio) { /* for testing */
if (errno != EXDEV) /* some unrecoverable error occurred */
return Err(Error{Error::Code::File, "error moving %s -> %s",
if (::rename(src.c_str(), dst.c_str()) == 0) /* seems it worked; double-check */
return msg_move_verify(src, dst);
if (errno != EXDEV) /* some unrecoverable error occurred */
return Err(Error{Error::Code::File, "error moving %s -> %s",
src.c_str(), dst.c_str()});
}
/* the EXDEV case -- source and target live on different filesystems */
return msg_move_g_file(src, dst);
/* the EXDEV / force-gio case -- source and target live on different
* filesystems */
if (!msg_move_g_file(src, dst))
return Err(Error{Error::Code::File, "error moving %s -> %s",
src.c_str(), dst.c_str()});
else
return msg_move_verify(src, dst);
}
Mu::Result<void>
Mu::maildir_move_message(const std::string& oldpath,
const std::string& newpath,
bool ignore_dups)
const std::string& newpath,
bool force_gio)
{
if (oldpath == newpath) {
if (ignore_dups)
return Ok();
else
return Err(Error{Error::Code::InvalidArgument,
"target equals source"});
}
if (oldpath == newpath)
return Ok(); // nothing to do.
g_debug("moving %s --> %s", oldpath.c_str(), newpath.c_str());
return msg_move(oldpath, newpath);
return msg_move(oldpath, newpath, force_gio);
}
static std::string

View File

@ -75,21 +75,20 @@ Result<void> maildir_link(const std::string& src, const std::string& targetpath,
Result<void> maildir_clear_links(const std::string& dir);
/**
* Move a message file to another maildir. If the target file already 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 absolete full path to the target file
* @param ignore_dups whether to silently ignore the src==target case
* (and return @true)
* @param err receives error information
* @param force_gio force the use of GIO for moving; this is done automatically
* when needed; forcing is mostly useful for tests
*
* @return * @return a valid result (!!result) or an Error
* @return a valid result (!!result) or an Error
*/
Result<void> maildir_move_message(const std::string& oldpath,
const std::string& newpath,
bool ignore_dups);
const std::string& newpath,
bool force_gio = false);
/**
* Determine the target path for a to-be-moved message; i.e. this does not

View File

@ -539,8 +539,7 @@ Store::move_message(Store::Id id,
return Err(target_path.error());
/* 2. let's move it */
if (const auto res = maildir_move_message(
msg->path(), target_path.value(), true/*ignore dups*/); !res)
if (const auto res = maildir_move_message(msg->path(), target_path.value()); !res)
return Err(res.error());
/* 3. file move worked, now update the message with the new info.*/

View File

@ -28,6 +28,7 @@
#include "test-mu-common.hh"
#include "mu-maildir.hh"
#include "utils/mu-result.hh"
#include "utils/mu-util.h"
using namespace Mu;
@ -458,12 +459,10 @@ test_maildir_from_path(void)
static void
test_maildir_link()
{
TempDir tmpdir{false};
TempDir tmpdir;
g_message("%s", tmpdir.path().c_str());
g_assert_true(!!maildir_mkdir(tmpdir.path() + "/foo"));
g_assert_true(!!maildir_mkdir(tmpdir.path() + "/bar"));
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";
@ -482,8 +481,8 @@ test_maildir_link()
stream.close();
}
g_assert_true(!!maildir_link(srcpath1, tmpdir.path() + "/bar", false));
g_assert_true(!!maildir_link(srcpath2, tmpdir.path() + "/bar", false));
assert_valid_result(maildir_link(srcpath1, tmpdir.path() + "/bar", false));
assert_valid_result(maildir_link(srcpath2, tmpdir.path() + "/bar", false));
const auto dstpath1 = tmpdir.path() + "/bar/cur/msg1";
const auto dstpath2 = tmpdir.path() + "/bar/new/msg2";
@ -491,11 +490,59 @@ 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_true(!!maildir_clear_links(tmpdir.path() + "/bar"));
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);
}
static void
test_maildir_move(bool use_gio)
{
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";
{
std::ofstream stream(srcpath1);
stream.write("cur", 3);
g_assert_true(stream.good());
stream.close();
}
{
std::ofstream stream(srcpath2);
stream.write("new", 3);
g_assert_true(stream.good());
stream.close();
}
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));
//g_assert_true(g_access(dstpath.c_str(), F_OK) == 0);
}
static void
test_maildir_move_vanilla()
{
test_maildir_move(false/*!gio*/);
}
static void
test_maildir_move_gio()
{
test_maildir_move(true/*gio*/);
}
int
main(int argc, char* argv[])
{
@ -523,5 +570,8 @@ main(int argc, char* argv[])
g_test_add_func("/mu-maildir/mu-maildir-link", test_maildir_link);
g_test_add_func("/mu-maildir/mu-maildir-move-vanilla", test_maildir_move_vanilla);
g_test_add_func("/mu-maildir/mu-maildir-move-gio", test_maildir_move_gio);
return g_test_run();
}