diff --git a/include/multipass/file_ops.h b/include/multipass/file_ops.h index 3d5d23977c2..9492eefc39a 100644 --- a/include/multipass/file_ops.h +++ b/include/multipass/file_ops.h @@ -57,7 +57,11 @@ class FileOps : public Singleton // High-level operations virtual void write_transactionally(const QString& file_name, const QByteArrayView& data) const; virtual void write_transactionally(const fs::path& file_name, std::string_view data) const; + virtual void write_file(const fs::path& file_name, + const std::string& content, + bool overwrite = false); virtual std::optional try_read_file(const fs::path& filename) const; + virtual std::string read_file(const fs::path& filename) const; // QDir operations virtual bool exists(const QDir& dir) const; @@ -130,6 +134,7 @@ class FileOps : public Singleton virtual bool is_directory(const fs::path& path, std::error_code& err) const; virtual bool create_directory(const fs::path& path, std::error_code& err) const; virtual bool create_directories(const fs::path& path, std::error_code& err) const; + virtual bool remove(const fs::path& path) const; virtual bool remove(const fs::path& path, std::error_code& err) const; virtual void create_symlink(const fs::path& to, const fs::path& path, diff --git a/include/multipass/utils.h b/include/multipass/utils.h index b01c8c437da..8aeda1cdb23 100644 --- a/include/multipass/utils.h +++ b/include/multipass/utils.h @@ -70,7 +70,6 @@ enum class TimeoutAction QDir base_dir(const QString& path); bool is_dir(const std::filesystem::path& path); QString backend_directory_path(const Path& path, const QString& subdirectory); -std::string contents_of(const multipass::Path& file_path); // path normalization: returns the lexically-normal form of the path with any trailing directory // separator removed. The string overloads additionally convert directory separators to generic @@ -224,10 +223,6 @@ class Utils : public Singleton virtual qint64 filesystem_bytes_available(const QString& data_directory) const; virtual void exit(int code) const; - virtual std::string contents_of(const multipass::Path& file_path) const; - virtual void make_file_with_content(const std::string& file_name, - const std::string& content, - const bool& overwrite = false); virtual Path make_dir(const QDir& a_dir, const QString& name, std::filesystem::perms permissions = std::filesystem::perms::none) const; diff --git a/src/cert/client_cert_store.cpp b/src/cert/client_cert_store.cpp index ce02a17d46a..0b1357bc85c 100644 --- a/src/cert/client_cert_store.cpp +++ b/src/cert/client_cert_store.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -92,7 +93,7 @@ std::string mp::ClientCertStore::PEM_cert_chain() const { auto path = cert_dir.filePath(chain_name); if (QFile::exists(path)) - return mp::utils::contents_of(path); + return MP_FILEOPS.read_file(MP_PLATFORM.qstr_to_path(path)); return {}; } diff --git a/src/cert/ssl_cert_provider.cpp b/src/cert/ssl_cert_provider.cpp index 5e11a7b9d0b..8a3626e925d 100644 --- a/src/cert/ssl_cert_provider.cpp +++ b/src/cert/ssl_cert_provider.cpp @@ -15,6 +15,7 @@ * */ +#include #include #include #include @@ -71,7 +72,7 @@ void openssl_check(T result, const std::string& errorMessage) class WritableFile { public: - explicit WritableFile(const QString& file_path) : fp{open_file(file_path)} + explicit WritableFile(const std::filesystem::path& file_path) : fp{open_file(file_path)} { } @@ -84,13 +85,12 @@ class WritableFile // decltype(&fclose) does not preserve these some extra function attributes of fclose, leads to // warning and compilation error using FilePtr = std::unique_ptr; - [[nodiscard]] static FilePtr open_file(const QString& file_path) + [[nodiscard]] static FilePtr open_file(const std::filesystem::path& file_path) { - const std::filesystem::path file_path_std{file_path.toStdString()}; - std::filesystem::create_directories(file_path_std.parent_path()); + std::filesystem::create_directories(file_path.parent_path()); // make sure the parent directory exist - const auto raw_fp = fopen(file_path_std.string().c_str(), "wb"); + const auto raw_fp = fopen(file_path.string().c_str(), "wb"); openssl_check( raw_fp, fmt::format("failed to open file '{}': {}({})", file_path, strerror(errno), errno)); @@ -117,13 +117,12 @@ class EVPKey return mem.as_string(); } - void write(const QString& key_path) const + void write(const std::filesystem::path& key_path) const { - const std::filesystem::path key_path_std = key_path.toStdU16String(); - if (std::filesystem::exists(key_path_std)) + if (std::filesystem::exists(key_path)) { // enable fopen in WritableFile with wb mode - MP_PLATFORM.set_permissions(key_path_std, + MP_PLATFORM.set_permissions(key_path, std::filesystem::perms::owner_read | std::filesystem::perms::owner_write); } @@ -133,7 +132,7 @@ class EVPKey PEM_write_PrivateKey(file.get(), key.get(), nullptr, nullptr, 0, nullptr, nullptr), fmt::format("Failed writing certificate private key to file '{}'", key_path)); - MP_PLATFORM.set_permissions(key_path_std, std::filesystem::perms::owner_read); + MP_PLATFORM.set_permissions(key_path, std::filesystem::perms::owner_read); } EVP_PKEY* get() const @@ -398,14 +397,13 @@ class X509Cert return mem.as_string(); } - void write(const QString& cert_path) const + void write(const std::filesystem::path& cert_path) const { WritableFile file{cert_path}; openssl_check(PEM_write_X509(file.get(), cert.get()), fmt::format("Failed writing certificate to file '{}'", cert_path)); - const std::filesystem::path cert_path_std = cert_path.toStdU16String(); - MP_PLATFORM.set_permissions(cert_path_std, + MP_PLATFORM.set_permissions(cert_path, std::filesystem::perms::owner_all | std::filesystem::perms::group_read | std::filesystem::perms::others_read); @@ -425,23 +423,22 @@ class X509Cert std::unique_ptr cert{X509_new(), X509_free}; }; -std::unique_ptr load_cert_from_file(const std::string& path) +std::unique_ptr load_cert_from_file(const std::filesystem::path& path) { - std::unique_ptr file{fopen(path.c_str(), "r"), &fclose}; + std::unique_ptr file{fopen(path.string().c_str(), "r"), &fclose}; if (!file) return {nullptr, X509_free}; return {PEM_read_X509(file.get(), nullptr, nullptr, nullptr), X509_free}; } -mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, +mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const std::filesystem::path& cert_dir, const std::string& server_name) { - const QString prefix = - server_name.empty() ? "multipass_cert" : QString::fromStdString(server_name); + const std::string prefix = server_name.empty() ? "multipass_cert" : server_name; - const auto priv_key_path = cert_dir.filePath(prefix + "_key.pem"); - const auto cert_path = cert_dir.filePath(prefix + ".pem"); + const auto priv_key_path = cert_dir / (prefix + "_key.pem"); + const auto cert_path = cert_dir / (prefix + ".pem"); if (!server_name.empty()) { @@ -450,8 +447,8 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, QFile::exists(cert_path)) { // Ensure that we can load both certificates - const auto root_cert = load_cert_from_file(root_cert_path.string()); - const auto cert = load_cert_from_file(cert_path.toStdString()); + const auto root_cert = load_cert_from_file(root_cert_path); + const auto cert = load_cert_from_file(cert_path); if (root_cert && cert) { @@ -459,7 +456,7 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, "Certificates for the gRPC server (root: {}, subordinate: {}) are valid " "X.509 files", root_cert_path, - cert_path.toStdString()); + cert_path); // TODO: Remove in Multipass 1.18 if (!cert_has_eku_nid(*cert.get(), NID_server_auth)) @@ -467,7 +464,7 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, mpl::warn(log_category, "Existing gRPC server certificate (`{}`) does not contain the " "correct extensions", - cert_path.toStdString()); + cert_path); } else if (!is_issuer_of(*root_cert.get(), *cert.get())) { @@ -475,14 +472,14 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, "Existing root certificate (`{}`) is not the signer of the gRPC " "server certificate (`{}`)", root_cert_path, - cert_path.toStdString()); + cert_path); } else if (is_expired(*cert.get())) { mpl::warn( log_category, "Existing gRPC server certificate (`{}`) validity period is not valid", - cert_path.toStdString()); + cert_path); } else { @@ -494,8 +491,7 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, std::filesystem::perms::owner_all | std::filesystem::perms::group_read | std::filesystem::perms::others_read); - return {mp::utils::contents_of(cert_path), - mp::utils::contents_of(priv_key_path)}; + return {MP_FILEOPS.read_file(cert_path), MP_FILEOPS.read_file(priv_key_path)}; } } else @@ -504,12 +500,12 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, "Could not load either of the root (`{}`) or subordinate (`{}`) " "certificates for the gRPC server", root_cert_path, - cert_path.toStdString()); + cert_path); } } mpl::info(log_category, "Regenerating certificates for the gRPC server"); - const auto priv_root_key_path = cert_dir.filePath(prefix + "_root_key.pem"); + const auto priv_root_key_path = cert_dir / (prefix + "_root_key.pem"); EVPKey root_cert_key{}; X509Cert root_cert{root_cert_key, X509Cert::CertType::Root}; @@ -534,7 +530,7 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, // even on `multipass list` // Re-enable it after fixing. // mpl::trace(kLogCategory, "Re-using existing certificates for the gRPC client"); - return {mp::utils::contents_of(cert_path), mp::utils::contents_of(priv_key_path)}; + return {MP_FILEOPS.read_file(cert_path), MP_FILEOPS.read_file(priv_key_path)}; } // mpl::trace(kLogCategory, "Regenerating certificates for the gRPC client"); @@ -543,7 +539,7 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, client_cert_key.write(priv_key_path); client_cert.write(cert_path); - MP_PLATFORM.set_permissions(priv_key_path.toStdU16String(), + MP_PLATFORM.set_permissions(priv_key_path, std::filesystem::perms::owner_all | std::filesystem::perms::group_read | std::filesystem::perms::others_read); @@ -555,7 +551,7 @@ mp::SSLCertProvider::KeyCertificatePair make_cert_key_pair(const QDir& cert_dir, mp::SSLCertProvider::SSLCertProvider(const multipass::Path& cert_dir, const std::string& server_name) - : key_cert_pair{make_cert_key_pair(cert_dir, server_name)} + : key_cert_pair{make_cert_key_pair(MP_PLATFORM.qstr_to_path(cert_dir), server_name)} { } diff --git a/src/client/common/client_common.cpp b/src/client/common/client_common.cpp index d1f6d43bc78..cd7ebb3fc89 100644 --- a/src/client/common/client_common.cpp +++ b/src/client/common/client_common.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -75,7 +76,7 @@ grpc::SslCredentialsOptions get_ssl_credentials_opts_from(const mp::CertProvider { auto opts = grpc::SslCredentialsOptions(); - opts.pem_root_certs = MP_UTILS.contents_of(MP_PLATFORM.get_root_cert_path().u8string().c_str()); + opts.pem_root_certs = MP_FILEOPS.read_file(MP_PLATFORM.get_root_cert_path()); opts.pem_cert_chain = cert_provider.PEM_certificate(); opts.pem_private_key = cert_provider.PEM_signing_key(); diff --git a/src/client/gui/ffi/dart_ffi.cpp b/src/client/gui/ffi/dart_ffi.cpp index 7050de1ae0b..6528ab2b914 100644 --- a/src/client/gui/ffi/dart_ffi.cpp +++ b/src/client/gui/ffi/dart_ffi.cpp @@ -1,5 +1,6 @@ #include "multipass/dart_ffi.h" #include "multipass/cli/client_common.h" +#include "multipass/file_ops.h" #include "multipass/logging/log.h" #include "multipass/memory_size.h" #include "multipass/name_generator.h" @@ -101,7 +102,7 @@ char* get_root_cert() try { const auto cert_path = MP_PLATFORM.get_root_cert_path(); - const auto cert = MP_UTILS.contents_of(QString::fromStdU16String(cert_path.u16string())); + const auto cert = MP_FILEOPS.read_file(cert_path); return strdup(cert.c_str()); } catch (const std::exception& e) diff --git a/src/platform/backends/shared/base_virtual_machine.cpp b/src/platform/backends/shared/base_virtual_machine.cpp index 9f66d83869b..fcacafa5f68 100644 --- a/src/platform/backends/shared/base_virtual_machine.cpp +++ b/src/platform/backends/shared/base_virtual_machine.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -78,7 +79,7 @@ void update_parents_rollback_helper(const std::shared_ptr& deleted std::string trimmed_contents_of(const QString& file_path) { - return mpu::trim(mpu::contents_of(file_path)); + return mpu::trim(MP_FILEOPS.read_file(MP_PLATFORM.qstr_to_path(file_path))); } template @@ -484,9 +485,9 @@ void mp::BaseVirtualMachine::deleted_head_rollback_helper(const Path& head_path, head_snapshot = std::move(old_head); if (wrote_head) top_catch_all(vm_name, [this, &head_path] { - MP_UTILS.make_file_with_content(head_path.toStdString(), - std::to_string(head_snapshot->get_index()) + "\n", - yes_overwrite); + MP_FILEOPS.write_file(head_path.toStdString(), + std::to_string(head_snapshot->get_index()) + "\n", + yes_overwrite); }); } } @@ -669,7 +670,7 @@ auto mp::BaseVirtualMachine::make_common_file_rollback(const Path& file_path, const std::string& old_contents) const { return sg::make_scope_guard( - [this, &file_path, &file, old_contents, existed = file.exists()]() noexcept { + [this, &file_path, &file, old_contents, existed = MP_FILEOPS.exists(file)]() noexcept { common_file_rollback_helper(file_path, file, old_contents, existed); }); } @@ -681,10 +682,10 @@ void mp::BaseVirtualMachine::common_file_rollback_helper(const Path& file_path, { // best effort, ignore returns if (!existed) - file.remove(); + MP_FILEOPS.remove(file); else top_catch_all(vm_name, [&file_path, &old_contents] { - MP_UTILS.make_file_with_content(file_path.toStdString(), old_contents, yes_overwrite); + MP_FILEOPS.write_file(file_path.toStdString(), old_contents, yes_overwrite); }); } @@ -706,9 +707,9 @@ void mp::BaseVirtualMachine::persist_generic_snapshot_info() const auto count_file_rollback = make_common_file_rollback(count_path, count_file, std::to_string(snapshot_count - 1) + "\n"); - MP_UTILS.make_file_with_content(count_path.toStdString(), - std::to_string(snapshot_count) + "\n", - yes_overwrite); + MP_FILEOPS.write_file(count_path.toStdString(), + std::to_string(snapshot_count) + "\n", + yes_overwrite); count_file_rollback.dismiss(); head_file_rollback.dismiss(); @@ -717,9 +718,9 @@ void mp::BaseVirtualMachine::persist_generic_snapshot_info() const void mp::BaseVirtualMachine::persist_head_snapshot_index(const Path& head_path) const { auto head_index = head_snapshot ? head_snapshot->get_index() : 0; - MP_UTILS.make_file_with_content(head_path.toStdString(), - std::to_string(head_index) + "\n", - yes_overwrite); + MP_FILEOPS.write_file(head_path.toStdString(), + std::to_string(head_index) + "\n", + yes_overwrite); } std::string mp::BaseVirtualMachine::generate_snapshot_name() const diff --git a/src/platform/backends/shared/windows/smb_mount_handler.cpp b/src/platform/backends/shared/windows/smb_mount_handler.cpp index 0fe1d7b7eaf..bb004795faa 100644 --- a/src/platform/backends/shared/windows/smb_mount_handler.cpp +++ b/src/platform/backends/shared/windows/smb_mount_handler.cpp @@ -144,10 +144,9 @@ try const auto iv = MP_UTILS.random_bytes(MP_AES.aes_256_block_size()); const auto encrypted_data = MP_AES.encrypt(enc_key, iv, data); - MP_UTILS.make_file_with_content(cred_dir.filePath(iv_filename).toStdString(), - {iv.begin(), iv.end()}); - MP_UTILS.make_file_with_content(cred_dir.filePath(cred_filename).toStdString(), - {encrypted_data.begin(), encrypted_data.end()}); + MP_FILEOPS.write_file(cred_dir.filePath(iv_filename).toStdString(), {iv.begin(), iv.end()}); + MP_FILEOPS.write_file(cred_dir.filePath(cred_filename).toStdString(), + {encrypted_data.begin(), encrypted_data.end()}); mpl::info(category, "Successfully encrypted credentials"); } @@ -160,8 +159,10 @@ std::string SmbMountHandler::decrypt_credentials_from_file(const QString& cred_f const QString& iv_filename) try { - const auto encrypted_data = MP_UTILS.contents_of(cred_dir.filePath(cred_filename)); - const auto iv_str = MP_UTILS.contents_of(cred_dir.filePath(iv_filename)); + const auto encrypted_data = + MP_FILEOPS.read_file(MP_PLATFORM.qstr_to_path(cred_dir.filePath(cred_filename))); + const auto iv_str = + MP_FILEOPS.read_file(MP_PLATFORM.qstr_to_path(cred_dir.filePath(iv_filename))); std::vector iv{iv_str.begin(), iv_str.end()}; iv.resize(MP_AES.aes_256_block_size()); @@ -198,18 +199,18 @@ SmbMountHandler::SmbMountHandler(VirtualMachine* vm, auto data_location{MP_PLATFORM.multipass_storage_location() + "\\data"}; auto enc_key_dir_path{MP_UTILS.make_dir(data_location, "enc-keys")}; - auto key_file = QDir{enc_key_dir_path}.filePath("aes.key"); + auto key_file = MP_PLATFORM.qstr_to_path(QDir{enc_key_dir_path}.filePath("aes.key")); - if (MP_FILEOPS.exists(QFile{key_file})) + if (MP_FILEOPS.exists(key_file)) { - const auto key_str = MP_UTILS.contents_of(key_file); + const auto key_str = MP_FILEOPS.readfile(key_file); enc_key.assign(key_str.begin(), key_str.end()); enc_key.resize(MP_AES.aes_256_key_size()); } else { enc_key = MP_UTILS.random_bytes(MP_AES.aes_256_key_size()); - MP_UTILS.make_file_with_content(key_file.toStdString(), {enc_key.begin(), enc_key.end()}); + MP_FILEOPS.write_file(key_file, {enc_key.begin(), enc_key.end()}); mpl::info(category, "Successfully generated new encryption key"); } } diff --git a/src/platform/platform_linux.cpp b/src/platform/platform_linux.cpp index 14157709eb6..3fe51276068 100644 --- a/src/platform/platform_linux.cpp +++ b/src/platform/platform_linux.cpp @@ -177,11 +177,11 @@ void update_bridges(std::map& networks) } } -std::string get_alias_script_path(const std::string& alias) +std::filesystem::path get_alias_script_path(const std::string& alias) { - auto aliases_folder = MP_PLATFORM.get_alias_scripts_folder(); + auto aliases_folder = MP_PLATFORM.qstr_to_path(MP_PLATFORM.get_alias_scripts_folder().path()); - return aliases_folder.absoluteFilePath(QString::fromStdString(alias)).toStdString(); + return absolute(aliases_folder / alias); } } // namespace @@ -301,7 +301,7 @@ void mp::platform::Platform::create_alias_script(const std::string& alias, std::string script = "#!/bin/sh\n\n" + multipass_exec + " " + alias + " -- \"${@}\"\n"; - MP_UTILS.make_file_with_content(file_path, script, true); + MP_FILEOPS.write_file(file_path, script, true); auto permissions = MP_FILEOPS.get_permissions(file_path) | fs::perms::owner_exec | fs::perms::group_exec | fs::perms::others_exec; diff --git a/src/platform/platform_osx.cpp b/src/platform/platform_osx.cpp index db54f4af882..302f7b3dca0 100644 --- a/src/platform/platform_osx.cpp +++ b/src/platform/platform_osx.cpp @@ -178,11 +178,11 @@ std::optional get_net_info(const QString& nsetup_entry return std::nullopt; } -std::string get_alias_script_path(const std::string& alias) +std::filesystem::path get_alias_script_path(const std::string& alias) { - QDir aliases_folder = MP_PLATFORM.get_alias_scripts_folder(); + auto aliases_folder = MP_PLATFORM.qstr_to_path(MP_PLATFORM.get_alias_scripts_folder().path()); - return aliases_folder.absoluteFilePath(QString::fromStdString(alias)).toStdString(); + return absolute(aliases_folder / alias); } } // namespace @@ -368,7 +368,7 @@ void mp::platform::Platform::create_alias_script(const std::string& alias, std::string script = "#!/bin/sh\n\n\"" + multipass_exec + "\" " + alias + " -- " + "\"${@}\"\n"; - MP_UTILS.make_file_with_content(file_path, script, true); + MP_FILEOPS.write_file(file_path, script, true); auto permissions = MP_FILEOPS.get_permissions(file_path) | fs::perms::owner_exec | fs::perms::group_exec | fs::perms::others_exec; diff --git a/src/platform/platform_win.cpp b/src/platform/platform_win.cpp index 941c5e69e60..f41b1a08dbb 100644 --- a/src/platform/platform_win.cpp +++ b/src/platform/platform_win.cpp @@ -319,11 +319,11 @@ std::string interpret_net_type(const QString& media_type, const QString& physica return physical_media_type.toLower().toStdString(); } -QString get_alias_script_path(const std::string& alias) +std::filesystem::path get_alias_script_path(const std::string& alias) { - auto aliases_folder = MP_PLATFORM.get_alias_scripts_folder(); + auto aliases_folder = MP_PLATFORM.qstr_to_path(MP_PLATFORM.get_alias_scripts_folder().path()); - return aliases_folder.absoluteFilePath(QString::fromStdString(alias)) + ".bat"; + return absolute(aliases_folder / (alias + ".bat")); } QString program_data_multipass_path() @@ -922,14 +922,14 @@ void mp::platform::Platform::create_alias_script(const std::string& alias, std::string script = "@\"" + multipass_exec + "\" " + alias + " -- %*\n"; - MP_UTILS.make_file_with_content(file_path.toStdString(), script, true); + MP_FILEOPS.write_file(file_path, script, true); } void mp::platform::Platform::remove_alias_script(const std::string& alias) const { auto file_path = get_alias_script_path(alias); - if (!QFile::remove(file_path)) + if (!MP_FILEOPS.remove(file_path)) throw std::runtime_error("error removing alias script"); } diff --git a/src/utils/file_ops.cpp b/src/utils/file_ops.cpp index 1abae6b4b77..985fa30e0b7 100644 --- a/src/utils/file_ops.cpp +++ b/src/utils/file_ops.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -155,11 +156,41 @@ void mp::FileOps::write_transactionally(const QString& file_name, const QByteArr void mp::FileOps::write_transactionally(const fs::path& file_name, std::string_view data) const { - write_transactionally(QString::fromStdString(file_name.string()), data); + write_transactionally(MP_PLATFORM.path_to_qstr(file_name), data); } // LCOV_EXCL_START +void mp::FileOps::write_file(const std::filesystem::path& file_name, + const std::string& content, + bool overwrite) +{ + QFile file(MP_PLATFORM.path_to_qstr(file_name)); + if (!overwrite && MP_FILEOPS.exists(file)) + throw std::runtime_error(fmt::format("file '{}' already exists", file_name)); + + QDir parent_dir{QFileInfo{file}.absoluteDir()}; + if (!MP_FILEOPS.mkpath(parent_dir, ".")) + throw std::runtime_error(fmt::format("failed to create dir '{}'", parent_dir.path())); + + if (!MP_FILEOPS.open(file, QFile::WriteOnly)) + throw std::runtime_error( + fmt::format("failed to open file '{}' for writing: {}", file_name, file.errorString())); + + // TODO use a QTextStream instead. Theoretically, this may fail to write it all in one go but + // still succeed. In practice, that seems unlikely. See https://stackoverflow.com/a/70933650 for + // more. + if (MP_FILEOPS.write(file, content.c_str(), content.size()) != (qint64)content.size()) + throw std::runtime_error( + fmt::format("failed to write to file '{}': {}", file_name, file.errorString())); + + if (!MP_FILEOPS.flush(file)) // flush manually to check return (which QFile::close ignores) + throw std::runtime_error( + fmt::format("failed to flush file '{}': {}", file_name, file.errorString())); + + return; // file closed, flush called again with errors ignored +} + bool mp::FileOps::exists(const QDir& dir) const { return dir.exists(); @@ -349,6 +380,7 @@ std::optional mp::FileOps::try_read_file(const fs::path& filename) else if (err) throw fs::filesystem_error( fmt::format("error reading file {}: {}", filename, err.message()), + filename, err); const auto file = MP_FILEOPS.open_read(filename); @@ -356,6 +388,15 @@ std::optional mp::FileOps::try_read_file(const fs::path& filename) return std::string{std::istreambuf_iterator{*file}, {}}; } +std::string mp::FileOps::read_file(const fs::path& filename) const +{ + if (auto contents = try_read_file(filename)) + return *contents; + throw fs::filesystem_error(fmt::format("file {} does not exist", filename), + filename, + std::make_error_code(std::errc::no_such_file_or_directory)); +} + std::unique_ptr mp::FileOps::open_write(const fs::path& path, std::ios_base::openmode mode) const { @@ -408,6 +449,11 @@ bool mp::FileOps::create_directories(const fs::path& path, std::error_code& err) return fs::create_directories(path, err); } +bool mp::FileOps::remove(const fs::path& path) const +{ + return fs::remove(path); +} + bool mp::FileOps::remove(const fs::path& path, std::error_code& err) const { return fs::remove(path, err); diff --git a/src/utils/utils.cpp b/src/utils/utils.cpp index be9fe5c3a55..fd0279bec0b 100644 --- a/src/utils/utils.cpp +++ b/src/utils/utils.cpp @@ -102,36 +102,6 @@ bool mp::Utils::run_cmd_for_status(const QString& cmd, return proc.exitStatus() == QProcess::NormalExit && proc.exitCode() == 0; } -void mp::Utils::make_file_with_content(const std::string& file_name, - const std::string& content, - const bool& overwrite) -{ - QFile file(QString::fromStdString(file_name)); - if (!overwrite && MP_FILEOPS.exists(file)) - throw std::runtime_error(fmt::format("file '{}' already exists", file_name)); - - QDir parent_dir{QFileInfo{file}.absoluteDir()}; - if (!MP_FILEOPS.mkpath(parent_dir, ".")) - throw std::runtime_error(fmt::format("failed to create dir '{}'", parent_dir.path())); - - if (!MP_FILEOPS.open(file, QFile::WriteOnly)) - throw std::runtime_error( - fmt::format("failed to open file '{}' for writing: {}", file_name, file.errorString())); - - // TODO use a QTextStream instead. Theoretically, this may fail to write it all in one go but - // still succeed. In practice, that seems unlikely. See https://stackoverflow.com/a/70933650 for - // more. - if (MP_FILEOPS.write(file, content.c_str(), content.size()) != (qint64)content.size()) - throw std::runtime_error( - fmt::format("failed to write to file '{}': {}", file_name, file.errorString())); - - if (!MP_FILEOPS.flush(file)) // flush manually to check return (which QFile::close ignores) - throw std::runtime_error( - fmt::format("failed to flush file '{}': {}", file_name, file.errorString())); - - return; // file closed, flush called again with errors ignored -} - std::string mp::Utils::get_kernel_version() const { return QSysInfo::kernelVersion().toStdString(); @@ -374,24 +344,6 @@ QString mp::utils::make_uuid(const std::optional& seed) return uuid.toString(QUuid::WithoutBraces); } -std::string mp::utils::contents_of(const multipass::Path& file_path) -{ - // TODO this should protect against long contents - const std::string name{file_path.toStdString()}; - std::ifstream in(name, std::ios::in | std::ios::binary); - if (!in) - throw FileOpenFailedException(name); - - std::stringstream stream; - stream << in.rdbuf(); - return stream.str(); -} - -std::string mp::Utils::contents_of(const multipass::Path& file_path) const -{ - return mp::utils::contents_of(file_path); -} - std::vector mp::Utils::random_bytes(size_t len) { std::vector bytes(len, 0); diff --git a/tests/unit/fake_alias_config.h b/tests/unit/fake_alias_config.h index d62911162bb..492c5eb00bb 100644 --- a/tests/unit/fake_alias_config.h +++ b/tests/unit/fake_alias_config.h @@ -19,6 +19,7 @@ #include #include +#include #include "common.h" #include "mock_standard_paths.h" @@ -39,12 +40,12 @@ struct FakeAliasConfig .WillRepeatedly(Return(fake_alias_dir.path())); } - std::string db_filename() + std::filesystem::path db_filename() { const auto file_name = QStringLiteral("%1/%1_aliases.json").arg(mp::client_name); - return fake_alias_dir.filePath(file_name).toStdString(); + return MP_PLATFORM.qstr_to_path(fake_alias_dir.filePath(file_name)); } void populate_db_file(const std::vector>& aliases) diff --git a/tests/unit/file_operations.cpp b/tests/unit/file_operations.cpp index fbcb809a6c5..f4febcab448 100644 --- a/tests/unit/file_operations.cpp +++ b/tests/unit/file_operations.cpp @@ -20,8 +20,8 @@ #include "file_operations.h" #include "path.h" +#include #include -#include #include #include @@ -49,6 +49,6 @@ QByteArray mpt::load_test_file(const char* file_name) void mpt::make_file_with_content(const QString& file_name, const std::string& content) { - MP_UTILS.Utils::make_file_with_content(file_name.toStdString(), - content); // call the base impl even if it is a mock + MP_FILEOPS.FileOps::write_file(file_name.toStdString(), + content); // call the base impl even if it is a mock } diff --git a/tests/unit/linux/test_platform_linux.cpp b/tests/unit/linux/test_platform_linux.cpp index 46297677635..e5011caedf5 100644 --- a/tests/unit/linux/test_platform_linux.cpp +++ b/tests/unit/linux/test_platform_linux.cpp @@ -577,11 +577,10 @@ TEST_F(PlatformLinux, createAliasScriptWorksConfined) TEST_F(PlatformLinux, createAliasScriptOverwrites) { - auto [mock_utils, guard1] = mpt::MockUtils::inject(); auto [mock_file_ops, guard2] = mpt::MockFileOps::inject(); auto [mock_platform, guard3] = mpt::MockPlatform::inject(); - EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); + EXPECT_CALL(*mock_file_ops, write_file(_, _, true)).Times(1); EXPECT_CALL(*mock_file_ops, get_permissions(_)) .WillOnce(Return(mp::fs::perms::owner_read | mp::fs::perms::owner_write)); EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillOnce(Return(true)); @@ -592,41 +591,26 @@ TEST_F(PlatformLinux, createAliasScriptOverwrites) mp::AliasDefinition{"instance", "other_command", "map"})); } -TEST_F(PlatformLinux, createAliasScriptThrowsIfCannotCreatePath) -{ - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); - - MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", - mp::AliasDefinition{"instance", "command", "map"}), - std::runtime_error, - mpt::match_what(HasSubstr("failed to create dir '"))); -} - TEST_F(PlatformLinux, createAliasScriptThrowsIfCannotWriteScript) { auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)).WillOnce(Return(747)); + EXPECT_CALL(*mock_file_ops, write_file(_, _, true)) + .WillOnce(Throw(std::runtime_error{"intentional"})); MP_EXPECT_THROW_THAT( MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command", "map"}), std::runtime_error, - mpt::match_what(HasSubstr("failed to write to file '"))); + mpt::match_what(StrEq("intentional"))); } TEST_F(PlatformLinux, createAliasScriptThrowsIfCannotSetPermissions) { - auto [mock_utils, guard1] = mpt::MockUtils::inject(); auto [mock_file_ops, guard2] = mpt::MockFileOps::inject(); auto [mock_platform, guard3] = mpt::MockPlatform::inject(); - EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); + EXPECT_CALL(*mock_file_ops, write_file(_, _, true)).Times(1); EXPECT_CALL(*mock_file_ops, get_permissions(_)) .WillOnce(Return(mp::fs::perms::owner_read | mp::fs::perms::owner_write)); EXPECT_CALL(*mock_platform, set_permissions(_, _, _)).WillOnce(Return(false)); @@ -647,7 +631,7 @@ TEST_F(PlatformLinux, removeAliasScriptWorks) writableLocation(mp::StandardPaths::AppLocalDataLocation)) .WillOnce(Return(tmp_dir.path())); - MP_UTILS.make_file_with_content(script_file.fileName().toStdString(), "script content\n"); + MP_FILEOPS.write_file(script_file.fileName().toStdString(), "script content\n"); EXPECT_NO_THROW(MP_PLATFORM.remove_alias_script("alias_name")); diff --git a/tests/unit/macos/test_platform_osx.cpp b/tests/unit/macos/test_platform_osx.cpp index 06397842236..c07eb052ee8 100644 --- a/tests/unit/macos/test_platform_osx.cpp +++ b/tests/unit/macos/test_platform_osx.cpp @@ -340,30 +340,17 @@ TEST(PlatformOSX, createAliasScriptOverwrites) mp::AliasDefinition{"instance", "other_command"})); } -TEST(PlatformOSX, createAliasScriptThrowsIfCannotCreatePath) -{ - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); - - MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), - std::runtime_error, - mpt::match_what(HasSubstr("failed to create dir '"))); -} - TEST(PlatformOSX, createAliasScriptThrowsIfCannotWriteScript) { auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)).WillOnce(Return(747)); + EXPECT_CALL(*mock_file_ops, write_file(_, _, true)) + .WillOnce(Throw(std::runtime_error{"intentional"})); MP_EXPECT_THROW_THAT( MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), std::runtime_error, - mpt::match_what(HasSubstr("failed to write to file '"))); + mpt::match_what(StrEq("intentional"))); } TEST(PlatformOSX, removeAliasScriptWorks) @@ -375,7 +362,7 @@ TEST(PlatformOSX, removeAliasScriptWorks) writableLocation(mp::StandardPaths::AppLocalDataLocation)) .WillOnce(Return(tmp_dir.path())); - MP_UTILS.make_file_with_content(script_file.fileName().toStdString(), "script content\n"); + MP_FILEOPS.write_file(script_file.fileName().toStdString(), "script content\n"); EXPECT_NO_THROW(MP_PLATFORM.remove_alias_script("alias_name")); diff --git a/tests/unit/mock_file_ops.h b/tests/unit/mock_file_ops.h index c2b67fb00cd..9637dc4eab2 100644 --- a/tests/unit/mock_file_ops.h +++ b/tests/unit/mock_file_ops.h @@ -39,7 +39,13 @@ class MockFileOps : public FileOps write_transactionally, (const QString& file_name, const QByteArrayView& data), (const, override)); + MOCK_METHOD(void, write_file, (const std::filesystem::path&, const std::string&), ()); + MOCK_METHOD(void, + write_file, + (const std::filesystem::path&, const std::string&, bool), + (override)); MOCK_METHOD(std::optional, try_read_file, (const fs::path&), (const, override)); + MOCK_METHOD(std::string, read_file, (const fs::path&), (const, override)); // QDir mock methods MOCK_METHOD(QDir, current, (), (const)); diff --git a/tests/unit/mock_utils.h b/tests/unit/mock_utils.h index 338a9341d1b..b12fe359002 100644 --- a/tests/unit/mock_utils.h +++ b/tests/unit/mock_utils.h @@ -40,12 +40,6 @@ class MockUtils : public Utils run_cmd_for_status, (const QString&, const QStringList&, const int), (const, override)); - MOCK_METHOD(std::string, contents_of, (const multipass::Path&), (const, override)); - MOCK_METHOD(void, make_file_with_content, (const std::string&, const std::string&), ()); - MOCK_METHOD(void, - make_file_with_content, - (const std::string&, const std::string&, const bool&), - (override)); MOCK_METHOD(Path, make_dir, (const QDir&, const QString&, std::filesystem::perms), diff --git a/tests/unit/test_alias_dict.cpp b/tests/unit/test_alias_dict.cpp index 28d9832a36d..d6f24c15549 100644 --- a/tests/unit/test_alias_dict.cpp +++ b/tests/unit/test_alias_dict.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include "common.h" #include "daemon_test_fixture.h" @@ -32,7 +33,6 @@ #include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_settings.h" -#include "mock_utils.h" #include "mock_vm_image_vault.h" #include "stub_terminal.h" @@ -471,8 +471,8 @@ TEST_P(DaemonAliasTestsuite, purgeRemovesPurgedInstanceAliasesAndScripts) EXPECT_CALL(*mock_image_vault, prune_expired_images()).WillRepeatedly(Return()); EXPECT_CALL(*mock_image_vault, has_record_for(_)).WillRepeatedly(Return(true)); - const auto [mock_utils, guard] = mpt::MockUtils::inject(); - EXPECT_CALL(*mock_utils, contents_of(_)).WillRepeatedly(Return(mpt::root_cert)); + const auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops, read_file(_)).WillRepeatedly(Return(mpt::root_cert)); config_builder.vault = std::move(mock_image_vault); auto mock_factory = use_a_mock_vm_factory(); diff --git a/tests/unit/test_base_virtual_machine.cpp b/tests/unit/test_base_virtual_machine.cpp index 6dd9a4f1cf8..76489558c05 100644 --- a/tests/unit/test_base_virtual_machine.cpp +++ b/tests/unit/test_base_virtual_machine.cpp @@ -19,6 +19,7 @@ #include "dummy_ssh_key_provider.h" #include "file_operations.h" #include "mock_cloud_init_file_ops.h" +#include "mock_file_ops.h" #include "mock_logger.h" #include "mock_snapshot.h" #include "mock_ssh_test_fixture.h" @@ -1136,59 +1137,32 @@ TEST_F(BaseVM, persistsGenericSnapshotInfoWhenTakingSnapshot) TEST_F(BaseVM, removesGenericSnapshotInfoFilesOnFirstFailure) { - auto [mock_utils_ptr, guard] = mpt::MockUtils::inject(); - auto& mock_utils = *mock_utils_ptr; + auto [mock_file_ops_ptr, guard] = mpt::MockFileOps::inject(); + auto& mock_file_ops = *mock_file_ops_ptr; mock_snapshotting(); - ASSERT_FALSE(QFileInfo{head_path}.exists()); - ASSERT_FALSE(QFileInfo{count_path}.exists()); - - MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock_utils, - make_file_with_content, - mp::Utils, - (EndsWith(head_filename), _, Eq(true))); - EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(head_filename), _, Eq(true))); - EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(count_filename), _, Eq(true))) + EXPECT_CALL(mock_file_ops, exists(A())).WillRepeatedly(Return(false)); + EXPECT_CALL(mock_file_ops, write_file(EndsWith(head_filename), _, Eq(true))); + EXPECT_CALL(mock_file_ops, write_file(EndsWith(count_filename), _, Eq(true))) .WillOnce(Throw(std::runtime_error{"intentional"})); + EXPECT_CALL(mock_file_ops, remove(A())).Times(2); EXPECT_ANY_THROW(vm.take_snapshot({}, "", "")); - - EXPECT_FALSE(QFileInfo{head_path}.exists()); - EXPECT_FALSE(QFileInfo{count_path}.exists()); } TEST_F(BaseVM, restoresGenericSnapshotInfoFileContents) { + auto [mock_file_ops_ptr, guard] = mpt::MockFileOps::inject(); + auto& mock_file_ops = *mock_file_ops_ptr; mock_snapshotting(); - mp::VMSpecs specs{}; - vm.take_snapshot(specs, "", ""); - - ASSERT_TRUE(QFileInfo{head_path}.exists()); - ASSERT_TRUE(QFileInfo{count_path}.exists()); - - auto regex_matcher = make_index_file_contents_matcher(1); - EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); - EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); - - auto [mock_utils_ptr, guard] = mpt::MockUtils::inject(); - auto& mock_utils = *mock_utils_ptr; - - MP_DELEGATE_MOCK_CALLS_ON_BASE_WITH_MATCHERS(mock_utils, - make_file_with_content, - mp::Utils, - (_, _, Eq(true))); - EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(head_filename), _, Eq(true))).Times(2); - EXPECT_CALL(mock_utils, make_file_with_content(EndsWith(count_filename), _, Eq(true))) + EXPECT_CALL(mock_file_ops, exists(A())).WillRepeatedly(Return(true)); + EXPECT_CALL(mock_file_ops, write_file(EndsWith(head_filename), _, Eq(true))).Times(2); + EXPECT_CALL(mock_file_ops, write_file(EndsWith(count_filename), _, Eq(true))) .WillOnce(Throw(std::runtime_error{"intentional"})) .WillOnce(DoDefault()); EXPECT_ANY_THROW(vm.take_snapshot({}, "", "")); - - EXPECT_TRUE(QFileInfo{head_path}.exists()); - EXPECT_TRUE(QFileInfo{count_path}.exists()); - EXPECT_THAT(mpt::load(head_path).toStdString(), regex_matcher); - EXPECT_THAT(mpt::load(count_path).toStdString(), regex_matcher); } TEST_F(BaseVM, persistsHeadIndexOnRestore) @@ -1255,8 +1229,8 @@ TEST_F(BaseVM, rollsbackFailedRestore) EXPECT_CALL(target_snapshot, get_mounts).WillRepeatedly(ReturnRef(original_specs.mounts)); EXPECT_CALL(target_snapshot, get_metadata).WillRepeatedly(ReturnRef(original_specs.metadata)); - auto [mock_utils_ptr, guard] = mpt::MockUtils::inject(); - EXPECT_CALL(*mock_utils_ptr, make_file_with_content(_, _, _)) + auto [mock_file_ops_ptr, guard] = mpt::MockFileOps::inject(); + EXPECT_CALL(*mock_file_ops_ptr, write_file(_, _, _)) .WillOnce(Throw(std::runtime_error{"intentional"})) .WillRepeatedly(DoDefault()); diff --git a/tests/unit/test_cli_client.cpp b/tests/unit/test_cli_client.cpp index 571c9e84616..b152e90fd5c 100644 --- a/tests/unit/test_cli_client.cpp +++ b/tests/unit/test_cli_client.cpp @@ -197,7 +197,7 @@ struct Client : public Test EXPECT_CALL(mock_settings, get(Eq(mp::mounts_key))).WillRepeatedly(Return("true")); EXPECT_CALL(mock_settings, register_handler(_)).WillRepeatedly(Return(nullptr)); EXPECT_CALL(mock_settings, unregister_handler).Times(AnyNumber()); - EXPECT_CALL(*mock_utils, contents_of(_)).WillRepeatedly(Return(mpt::root_cert)); + EXPECT_CALL(*mock_file_ops, read_file(_)).WillRepeatedly(Return(mpt::root_cert)); EXPECT_CALL(mpt::MockStandardPaths::mock_instance(), locate(_, _, _)) .Times(AnyNumber()); // needed to allow general calls once we have added the specific @@ -438,6 +438,8 @@ struct Client : public Test const mpt::MockPlatform* mock_platform = platform_attr.first; const mpt::MockUtils::GuardedMock utils_attr{mpt::MockUtils::inject()}; const mpt::MockUtils* mock_utils = utils_attr.first; + const mpt::MockFileOps::GuardedMock file_ops_attr{mpt::MockFileOps::inject()}; + const mpt::MockFileOps* mock_file_ops = file_ops_attr.first; mpt::StubCertStore cert_store; StrictMock mock_daemon{ @@ -641,12 +643,11 @@ TEST_F(Client, transferCmdInstanceSourceLocalTarget) TEST_F(Client, transferCmdInstanceSourcesLocalTargetNotDir) { - auto [mocked_file_ops, mocked_file_ops_guard] = mpt::MockFileOps::inject(); auto [mocked_sftp_utils, mocked_sftp_utils_guard] = mpt::MockSFTPUtils::inject(); EXPECT_CALL(*mocked_sftp_utils, make_SFTPClient) .WillOnce(Return(std::make_unique())); - EXPECT_CALL(*mocked_file_ops, is_directory).WillOnce(Return(false)); + EXPECT_CALL(*mock_file_ops, is_directory).WillOnce(Return(false)); EXPECT_CALL(mock_daemon, ssh_info) .WillOnce([](auto, grpc::ServerReaderWriter* server) { mp::SSHInfoReply reply; @@ -663,13 +664,12 @@ TEST_F(Client, transferCmdInstanceSourcesLocalTargetNotDir) TEST_F(Client, transferCmdInstanceSourcesLocalTargetCannotAccess) { - auto [mocked_file_ops, mocked_file_ops_guard] = mpt::MockFileOps::inject(); auto [mocked_sftp_utils, mocked_sftp_utils_guard] = mpt::MockSFTPUtils::inject(); EXPECT_CALL(*mocked_sftp_utils, make_SFTPClient) .WillOnce(Return(std::make_unique())); auto err = std::make_error_code(std::errc::permission_denied); - EXPECT_CALL(*mocked_file_ops, is_directory).WillOnce([&](auto, std::error_code& e) { + EXPECT_CALL(*mock_file_ops, is_directory).WillOnce([&](auto, std::error_code& e) { e = err; return false; }); @@ -1421,14 +1421,13 @@ TEST_F(Client, launchCmdMountOption) TEST_F(Client, launchCmdMountOptionFailsOnInvalidDir) { - auto [mocked_file_ops, mocked_file_ops_guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mocked_file_ops, exists(A())) + EXPECT_CALL(*mock_file_ops, exists(A())) .WillOnce(Return(false)) .WillRepeatedly(Return(true)); - EXPECT_CALL(*mocked_file_ops, isDir(A())) + EXPECT_CALL(*mock_file_ops, isDir(A())) .WillOnce(Return(false)) .WillRepeatedly(Return(true)); - EXPECT_CALL(*mocked_file_ops, isReadable(A())) + EXPECT_CALL(*mock_file_ops, isReadable(A())) .WillOnce(Return(false)) .WillRepeatedly(Return(true)); @@ -4542,7 +4541,6 @@ TEST_F(ClientAlias, unaliasDashDashAllClashesWithOtherArguments) TEST_F(ClientAlias, failsIfUnableToCreateDirectory) { - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); MP_DELEGATE_MOCK_CALLS_ON_BASE(*mock_file_ops, write_transactionally, FileOps); EXPECT_CALL(*mock_file_ops, try_read_file(_)).WillOnce(Return(std::nullopt)); diff --git a/tests/unit/test_client_common.cpp b/tests/unit/test_client_common.cpp index fb8c4192f82..6018117bafe 100644 --- a/tests/unit/test_client_common.cpp +++ b/tests/unit/test_client_common.cpp @@ -22,6 +22,7 @@ #include "mock_cert_store.h" #include "mock_client_rpc.h" #include "mock_daemon.h" +#include "mock_file_ops.h" #include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_standard_paths.h" @@ -46,7 +47,7 @@ struct TestClientCommon : public mpt::DaemonTestFixture ON_CALL(mpt::MockStandardPaths::mock_instance(), writableLocation(mp::StandardPaths::GenericDataLocation)) .WillByDefault(Return(temp_dir.path())); - ON_CALL(*mock_utils, contents_of(_)).WillByDefault(Return(mpt::root_cert)); + ON_CALL(*mock_file_ops, read_file(_)).WillByDefault(Return(mpt::root_cert)); // delegate some functions to the orignal implementation ON_CALL(*mock_utils, make_dir(A(), A(), A())) @@ -77,6 +78,8 @@ struct TestClientCommon : public mpt::DaemonTestFixture std::unique_ptr mock_cert_store{std::make_unique()}; const mpt::MockUtils::GuardedMock utils_attr{mpt::MockUtils::inject()}; const mpt::MockUtils* mock_utils = utils_attr.first; + const mpt::MockFileOps::GuardedMock file_ops_attr{mpt::MockFileOps::inject()}; + const mpt::MockFileOps* mock_file_ops = file_ops_attr.first; const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = mpt::MockPermissionUtils::inject(); diff --git a/tests/unit/test_daemon.cpp b/tests/unit/test_daemon.cpp index dd76954a080..b833a5a97f5 100644 --- a/tests/unit/test_daemon.cpp +++ b/tests/unit/test_daemon.cpp @@ -106,7 +106,7 @@ struct Daemon : public mpt::DaemonTestFixture .WillByDefault([this](const QString& data_directory) { return mock_utils.Utils::filesystem_bytes_available(data_directory); }); - ON_CALL(mock_utils, contents_of(_)).WillByDefault(Return(mpt::root_cert)); + ON_CALL(mock_file_ops, read_file(_)).WillByDefault(Return(mpt::root_cert)); EXPECT_CALL(mock_platform, multipass_storage_location()) .Times(AnyNumber()) @@ -137,6 +137,9 @@ a few more tests for `false`, since there are different portions of code dependi mpt::MockUtils::GuardedMock mock_utils_injection{mpt::MockUtils::inject()}; mpt::MockUtils& mock_utils = *mock_utils_injection.first; + mpt::MockFileOps::GuardedMock mock_file_ops_injection{mpt::MockFileOps::inject()}; + mpt::MockFileOps& mock_file_ops = *mock_file_ops_injection.first; + mpt::MockPlatform::GuardedMock mock_platform_injection{mpt::MockPlatform::inject()}; mpt::MockPlatform& mock_platform = *mock_platform_injection.first; @@ -1092,8 +1095,7 @@ TEST_P(LaunchStorageCheckSuite, launchFailsWithInvalidDataDirectory) config_builder.data_directory = QString("invalid_data_directory"); mp::Daemon daemon{config_builder.build()}; - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, write_transactionally).Times(1); // avoid creating directory + EXPECT_CALL(mock_file_ops, write_transactionally).Times(1); // avoid creating directory std::stringstream stream; EXPECT_CALL(*mock_factory, create_virtual_machine).Times(0); @@ -1161,8 +1163,7 @@ TEST_F(Daemon, readsMacAddressesFromJson) EXPECT_THAT(list_reply.instance_list().instances(), instance_matcher); } - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, write_transactionally(Eq(filename), _)) + EXPECT_CALL(mock_file_ops, write_transactionally(Eq(filename), _)) .WillOnce(WithArg<1>([&mac_addr, &extra_interfaces](const QByteArrayView& data) { auto obj = boost::json::parse({data.begin(), data.end()}).as_object(); check_interfaces_in_json(obj, mac_addr, extra_interfaces); @@ -1242,8 +1243,7 @@ TEST_F(Daemon, writesAndReadsMountsInJson) EXPECT_THAT(list_reply.instance_list().instances(), instance_matcher); } - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, write_transactionally(Eq(filename), _)) + EXPECT_CALL(mock_file_ops, write_transactionally(Eq(filename), _)) .WillOnce(WithArg<1>([&mounts](const QByteArrayView& data) { auto obj = boost::json::parse({data.begin(), data.end()}); check_mounts_in_json(obj, mounts); @@ -1281,8 +1281,7 @@ TEST_F(Daemon, writesAndReadsOrderedMapsInJson) send_command({"list"}, stream); EXPECT_THAT(stream.str(), HasSubstr("real-zebraphant")); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, write_transactionally(Eq(filename), _)) + EXPECT_CALL(mock_file_ops, write_transactionally(Eq(filename), _)) .WillOnce(WithArg<1>([&uid_mappings, &gid_mappings](const QByteArrayView& data) { auto obj = boost::json::parse({data.begin(), data.end()}); check_maps_in_json(obj, uid_mappings, gid_mappings); @@ -1488,8 +1487,7 @@ TEST_F(Daemon, ctorDropsRemovedInstances) fmt::format("{{\n{},\n{}\n}}", std::move(stayed_json), std::move(gone_json))); config_builder.data_directory = temp_dir->path(); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, exists(A())) + EXPECT_CALL(mock_file_ops, exists(A())) .WillRepeatedly(Invoke([](const auto& p) { return p.filename() != "nowhere"; })); auto mock_image_vault = std::make_unique>(); @@ -1515,7 +1513,7 @@ TEST_F(Daemon, ctorDropsRemovedInstances) create_virtual_machine(Field(&mp::VirtualMachineDescription::vm_name, gone), _, _)) .Times(0); - EXPECT_CALL(*mock_file_ops, write_transactionally(Eq(filename), _)) + EXPECT_CALL(mock_file_ops, write_transactionally(Eq(filename), _)) .WillOnce(Return()) .WillOnce(WithArg<1>([&stayed, &gone](const QByteArrayView& data) { EXPECT_THAT(data.toByteArray().toStdString(), @@ -2272,10 +2270,9 @@ TEST_F(Daemon, purgePersistsInstances) const auto [temp_dir, filename] = plant_instance_json(json_contents); config_builder.data_directory = temp_dir->path(); - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, exists(A())) + EXPECT_CALL(mock_file_ops, exists(A())) .WillRepeatedly(Return(true)); - EXPECT_CALL(*mock_file_ops, write_transactionally(Eq(filename), _)) + EXPECT_CALL(mock_file_ops, write_transactionally(Eq(filename), _)) .WillOnce(Return()) .WillOnce(Return()) .WillOnce(WithArg<1>([&name1, &name2](const QByteArrayView& data) { diff --git a/tests/unit/test_daemon_find.cpp b/tests/unit/test_daemon_find.cpp index 8712bbd831b..3089845ce1e 100644 --- a/tests/unit/test_daemon_find.cpp +++ b/tests/unit/test_daemon_find.cpp @@ -18,11 +18,11 @@ #include "common.h" #include "daemon_test_fixture.h" #include "mock_cert_provider.h" +#include "mock_file_ops.h" #include "mock_image_host.h" #include "mock_permission_utils.h" #include "mock_platform.h" #include "mock_settings.h" -#include "mock_utils.h" #include "mock_vm_image_vault.h" #include @@ -42,7 +42,7 @@ struct DaemonFind : public mpt::DaemonTestFixture EXPECT_CALL(mock_settings, register_handler).WillRepeatedly(Return(nullptr)); EXPECT_CALL(mock_settings, unregister_handler).Times(AnyNumber()); EXPECT_CALL(mock_settings, get(Eq(mp::winterm_key))).WillRepeatedly(Return("none")); - ON_CALL(mock_utils, contents_of(_)).WillByDefault(Return(mpt::root_cert)); + ON_CALL(mock_file_ops, read_file(_)).WillByDefault(Return(mpt::root_cert)); } mpt::MockPlatform::GuardedMock attr{mpt::MockPlatform::inject()}; @@ -56,8 +56,8 @@ struct DaemonFind : public mpt::DaemonTestFixture mpt::MockPermissionUtils::inject(); mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; - mpt::MockUtils::GuardedMock mock_utils_injection{mpt::MockUtils::inject()}; - mpt::MockUtils& mock_utils = *mock_utils_injection.first; + mpt::MockFileOps::GuardedMock mock_file_ops_injection{mpt::MockFileOps::inject()}; + mpt::MockFileOps& mock_file_ops = *mock_file_ops_injection.first; }; TEST_F(DaemonFind, blankQueryReturnsAllData) diff --git a/tests/unit/test_daemon_wait_ready.cpp b/tests/unit/test_daemon_wait_ready.cpp index 4b54e18bc24..a2d36fe56bc 100644 --- a/tests/unit/test_daemon_wait_ready.cpp +++ b/tests/unit/test_daemon_wait_ready.cpp @@ -18,10 +18,10 @@ #include "common.h" #include "daemon_test_fixture.h" #include "mock_cert_provider.h" +#include "mock_file_ops.h" #include "mock_image_host.h" #include "mock_permission_utils.h" #include "mock_settings.h" -#include "mock_utils.h" #include @@ -40,7 +40,7 @@ struct DaemonWaitReady : public mpt::DaemonTestFixture EXPECT_CALL(mock_settings, register_handler).WillRepeatedly(Return(nullptr)); EXPECT_CALL(mock_settings, unregister_handler).Times(AnyNumber()); EXPECT_CALL(mock_settings, get(Eq(mp::winterm_key))).WillRepeatedly(Return("none")); - ON_CALL(mock_utils, contents_of(_)).WillByDefault(Return(mpt::root_cert)); + ON_CALL(mock_file_ops, read_file(_)).WillByDefault(Return(mpt::root_cert)); } mpt::MockSettings::GuardedMock mock_settings_injection = @@ -51,8 +51,8 @@ struct DaemonWaitReady : public mpt::DaemonTestFixture mpt::MockPermissionUtils::inject(); mpt::MockPermissionUtils& mock_permission_utils = *mock_permission_utils_injection.first; - mpt::MockUtils::GuardedMock mock_utils_injection{mpt::MockUtils::inject()}; - mpt::MockUtils& mock_utils = *mock_utils_injection.first; + mpt::MockFileOps::GuardedMock mock_file_ops_injection{mpt::MockFileOps::inject()}; + mpt::MockFileOps& mock_file_ops = *mock_file_ops_injection.first; const std::string wait_msg = fmt::format("Waiting for the Multipass daemon to be ready"); }; diff --git a/tests/unit/test_file_ops.cpp b/tests/unit/test_file_ops.cpp index 12bcc7bf127..d9cdbfd75c7 100644 --- a/tests/unit/test_file_ops.cpp +++ b/tests/unit/test_file_ops.cpp @@ -17,6 +17,7 @@ #include "common.h" #include "mock_file_ops.h" +#include "temp_dir.h" #include @@ -210,10 +211,22 @@ struct HighLevelFileOps : public Test .WillRepeatedly([this](Args&&... args) { return mock_file_ops.FileOps::write_transactionally(std::forward(args)...); }); + EXPECT_CALL(mock_file_ops, write_file(_, _)) + .WillRepeatedly([this](Args&&... args) { + return mock_file_ops.FileOps::write_file(std::forward(args)...); + }); + EXPECT_CALL(mock_file_ops, write_file(_, _, _)) + .WillRepeatedly([this](Args&&... args) { + return mock_file_ops.FileOps::write_file(std::forward(args)...); + }); EXPECT_CALL(mock_file_ops, try_read_file(A())) .WillRepeatedly([this](Args&&... args) { return mock_file_ops.FileOps::try_read_file(std::forward(args)...); }); + EXPECT_CALL(mock_file_ops, read_file(A())) + .WillRepeatedly([this](Args&&... args) { + return mock_file_ops.FileOps::read_file(std::forward(args)...); + }); } mpt::MockFileOps::GuardedMock guarded_mock_file_ops = mpt::MockFileOps::inject(); @@ -360,6 +373,91 @@ TEST_F(HighLevelFileOps, writeTransactionallyThrowsOnFailureToCommit) mpt::match_what(AllOf(HasSubstr("Could not commit"), HasSubstr(file_path.toStdString())))); } +TEST_F(HighLevelFileOps, writeFileWorks) +{ + EXPECT_CALL(mock_file_ops, exists(A())).WillOnce(Return(false)); + EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(mpt::FileNameMatches(Eq(file_path)), _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, + write(mpt::FileNameMatches(Eq(file_path)), + Eq(std::string{file_text}), + Eq(strlen(file_text)))) + .WillOnce(Return(14)); + EXPECT_CALL(mock_file_ops, flush).WillOnce(Return(true)); + + EXPECT_NO_THROW(mock_file_ops.write_file(file_path.toStdString(), file_text)); +} + +TEST_F(HighLevelFileOps, writeFileDoesNotOverwrite) +{ + EXPECT_CALL(mock_file_ops, exists(A())).WillOnce(Return(true)); + + MP_EXPECT_THROW_THAT(mock_file_ops.write_file(file_path.toStdString(), file_text), + std::runtime_error, + mpt::match_what(HasSubstr("already exists"))); +} + +TEST_F(HighLevelFileOps, writeFileOverwritesWhenAsked) +{ + EXPECT_CALL(mock_file_ops, exists(A())).Times(0); + EXPECT_CALL(mock_file_ops, mkpath(Eq(dir), Eq("."))).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(mpt::FileNameMatches(Eq(file_path)), _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, + write(mpt::FileNameMatches(Eq(file_path)), + Eq(std::string{file_text}), + Eq(strlen(file_text)))) + .WillOnce(Return(14)); + EXPECT_CALL(mock_file_ops, flush).WillOnce(Return(true)); + + EXPECT_NO_THROW(mock_file_ops.write_file(file_path.toStdString(), file_text, true)); +} + +TEST_F(HighLevelFileOps, writeFileFailsIfPathCannotBeCreated) +{ + EXPECT_CALL(mock_file_ops, exists(A())).WillOnce(Return(false)); + EXPECT_CALL(mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT(mock_file_ops.write_file(file_path.toStdString(), file_text), + std::runtime_error, + mpt::match_what(HasSubstr("failed to create dir"))); +} + +TEST_F(HighLevelFileOps, writeFileFailsIfFileCannotBeCreated) +{ + EXPECT_CALL(mock_file_ops, exists(A())).WillOnce(Return(false)); + EXPECT_CALL(mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(_, _)).WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT(mock_file_ops.write_file(file_path.toStdString(), file_text), + std::runtime_error, + mpt::match_what(HasSubstr("failed to open file"))); +} + +TEST_F(HighLevelFileOps, writeFileThrowsOnWriteError) +{ + EXPECT_CALL(mock_file_ops, exists(A())).WillOnce(Return(false)); + EXPECT_CALL(mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, write(A(), _, _)).WillOnce(Return(747)); + + MP_EXPECT_THROW_THAT(mock_file_ops.write_file(file_path.toStdString(), file_text), + std::runtime_error, + mpt::match_what(HasSubstr("failed to write to file"))); +} + +TEST_F(HighLevelFileOps, writeFileThrowsOnFailureToFlush) +{ + EXPECT_CALL(mock_file_ops, exists(A())).WillOnce(Return(false)); + EXPECT_CALL(mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, write(A(), _, _)).WillOnce(Return(strlen(file_text))); + EXPECT_CALL(mock_file_ops, flush(A())).WillOnce(Return(false)); + + MP_EXPECT_THROW_THAT(mock_file_ops.write_file(file_path.toStdString(), file_text), + std::runtime_error, + mpt::match_what(HasSubstr("failed to flush file"))); +} + TEST_F(HighLevelFileOps, tryReadFileReadsFromFile) { auto filestream = std::make_unique(); @@ -410,3 +508,23 @@ TEST_F(HighLevelFileOps, tryReadFileThrowsOnBadbit) EXPECT_THROW(mock_file_ops.try_read_file(":("), std::ios_base::failure); } + +TEST_F(HighLevelFileOps, readFileReadsFromFile) +{ + auto filestream = std::make_unique(); + *filestream << "Hello, world!"; + + EXPECT_CALL(mock_file_ops, exists(_, _)).WillOnce(Return(true)); + EXPECT_CALL(mock_file_ops, open_read(_, _)).WillOnce(Return(std::move(filestream))); + const auto filedata = mock_file_ops.try_read_file("exists"); + + EXPECT_EQ(filedata, "Hello, world!"); +} + +TEST_F(HighLevelFileOps, readFileThrowsForMissingFile) +{ + EXPECT_CALL(mock_file_ops, exists(_, _)).WillOnce(Return(false)); + const auto filedata = mock_file_ops.try_read_file("exists"); + + EXPECT_THROW(mock_file_ops.read_file(":("), std::filesystem::filesystem_error); +} diff --git a/tests/unit/test_image_vault.cpp b/tests/unit/test_image_vault.cpp index 1700ff509c1..538ef3659aa 100644 --- a/tests/unit/test_image_vault.cpp +++ b/tests/unit/test_image_vault.cpp @@ -568,7 +568,7 @@ TEST_F(ImageVault, usesImageFromPrepare) std::nullopt, instance_dir); - const auto image_data = mp::utils::contents_of(MP_PLATFORM.path_to_qstr(vm_image.image_path)); + const auto image_data = mpt::load(MP_PLATFORM.path_to_qstr(vm_image.image_path)); EXPECT_THAT(image_data, StrEq(expected_data)); EXPECT_THAT(vm_image.id, Eq(mpt::default_id)); } diff --git a/tests/unit/test_utils.cpp b/tests/unit/test_utils.cpp index 98858ebde80..2e59c49e373 100644 --- a/tests/unit/test_utils.cpp +++ b/tests/unit/test_utils.cpp @@ -40,25 +40,6 @@ namespace mpu = mp::utils; using namespace testing; -namespace -{ -std::string file_contents{"line 1 of file contents\nline 2\n"}; - -void check_file_contents(QFile& checked_file, const std::string& checked_contents) -{ - ASSERT_TRUE(checked_file.open(QIODevice::ReadOnly | QIODevice::Text)); - - QString actual_contents; - - while (!checked_file.atEnd()) - actual_contents += checked_file.readLine(); - - checked_file.close(); - - ASSERT_EQ(checked_contents, actual_contents.toStdString()); -} -} // namespace - TEST(Utils, hostnameBeginsWithLetterIsValid) { EXPECT_TRUE(mp::utils::valid_hostname("foo")); @@ -153,124 +134,6 @@ TEST(Utils, pathHomeUbuntuFooValid) EXPECT_FALSE(MP_UTILS.invalid_target_path(QString("/home/ubuntu/foo"))); } -TEST(Utils, makeFileWithContentWorks) -{ - mpt::TempDir temp_dir; - QString file_name = temp_dir.path() + "/test-file"; - - EXPECT_NO_THROW(MP_UTILS.make_file_with_content(file_name.toStdString(), file_contents)); - - QFile checked_file(file_name); - check_file_contents(checked_file, file_contents); -} - -TEST(Utils, makeFileWithContentDoesNotOverwrite) -{ - mpt::TempDir temp_dir; - QString file_name = temp_dir.path() + "/test-file"; - - EXPECT_NO_THROW(MP_UTILS.make_file_with_content(file_name.toStdString(), file_contents)); - - QFile checked_file(file_name); - check_file_contents(checked_file, file_contents); - - MP_EXPECT_THROW_THAT(MP_UTILS.make_file_with_content(file_name.toStdString(), "other stuff\n"), - std::runtime_error, - mpt::match_what(HasSubstr("already exists"))); - - check_file_contents(checked_file, file_contents); -} - -TEST(Utils, makeFileWithContentOverwritesWhenAsked) -{ - mpt::TempDir temp_dir; - QString file_name = temp_dir.path() + "/test-file"; - - EXPECT_NO_THROW(MP_UTILS.make_file_with_content(file_name.toStdString(), file_contents)); - - QFile checked_file(file_name); - check_file_contents(checked_file, file_contents); - - EXPECT_NO_THROW( - MP_UTILS.make_file_with_content(file_name.toStdString(), "other stuff\n", true)); - - check_file_contents(checked_file, "other stuff\n"); -} - -TEST(Utils, makeFileWithContentCreatesPath) -{ - mpt::TempDir temp_dir; - QString file_name = temp_dir.path() + "/new_dir/test-file"; - - EXPECT_NO_THROW(MP_UTILS.make_file_with_content(file_name.toStdString(), file_contents)); - - QFile checked_file(file_name); - check_file_contents(checked_file, file_contents); -} - -TEST(Utils, makeFileWithContentFailsIfPathCannotBeCreated) -{ - std::string file_name{"some_dir/test-file"}; - - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, exists(A())).WillOnce(Return(false)); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); - - MP_EXPECT_THROW_THAT(MP_UTILS.make_file_with_content(file_name, file_contents), - std::runtime_error, - mpt::match_what(HasSubstr("failed to create dir"))); -} - -TEST(Utils, makeFileWithContentFailsIfFileCannotBeCreated) -{ - std::string file_name{"some_dir/test-file"}; - - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, exists(A())).WillOnce(Return(false)); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(false)); - - MP_EXPECT_THROW_THAT(MP_UTILS.make_file_with_content(file_name, file_contents), - std::runtime_error, - mpt::match_what(HasSubstr("failed to open file"))); -} - -TEST(Utils, makeFileWithContentThrowsOnWriteError) -{ - std::string file_name{"some_dir/test-file"}; - - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, exists(A())).WillOnce(Return(false)); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)).WillOnce(Return(747)); - - MP_EXPECT_THROW_THAT(MP_UTILS.make_file_with_content(file_name, file_contents), - std::runtime_error, - mpt::match_what(HasSubstr("failed to write to file"))); -} - -TEST(Utils, makeFileWithContentThrowsOnFailureToFlush) -{ - std::string file_name{"some_dir/test-file"}; - - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, exists(A())).WillOnce(Return(false)); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)) - .WillOnce(Return(file_contents.size())); - EXPECT_CALL(*mock_file_ops, flush(A())).WillOnce(Return(false)); - - MP_EXPECT_THROW_THAT(MP_UTILS.make_file_with_content(file_name, file_contents), - std::runtime_error, - mpt::match_what(HasSubstr("failed to flush file"))); -} - TEST(Utils, expectedScryptHashReturned) { const auto passphrase = MP_UTILS.generate_scrypt_hash_for("passphrase"); @@ -433,32 +296,6 @@ TEST(Utils, uuidHasNoCurlyBrackets) EXPECT_FALSE(uuid.contains(QRegularExpression("[{}]"))); } -TEST(Utils, contentsOfActuallyReadsContents) -{ - mpt::TempDir temp_dir; - auto file_name = temp_dir.path() + "/test-file"; - std::string expected_content{"just a bit of test content here"}; - mpt::make_file_with_content(file_name, expected_content); - - auto content = mp::utils::contents_of(file_name); - EXPECT_THAT(content, StrEq(expected_content)); -} - -TEST(Utils, contentsOfThrowsOnMissingFile) -{ - EXPECT_THROW(mp::utils::contents_of("this-file-does-not-exist"), std::runtime_error); -} - -TEST(Utils, contentsOfEmptyContentsOnEmptyFile) -{ - mpt::TempDir temp_dir; - auto file_name = temp_dir.path() + "/empty_test_file"; - mpt::make_file_with_content(file_name, ""); - - auto content = mp::utils::contents_of(file_name); - EXPECT_TRUE(content.empty()); -} - TEST(Utils, splitReturnsTokenList) { std::vector expected_tokens; diff --git a/tests/unit/unix/test_daemon_rpc.cpp b/tests/unit/unix/test_daemon_rpc.cpp index bcebce17b54..95dfde25966 100644 --- a/tests/unit/unix/test_daemon_rpc.cpp +++ b/tests/unit/unix/test_daemon_rpc.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -43,7 +44,7 @@ struct TestDaemonRpc : public mpt::DaemonTestFixture EXPECT_CALL(*mock_platform, multipass_storage_location()) .Times(AnyNumber()) .WillRepeatedly(Return(QString())); - EXPECT_CALL(*mock_utils, contents_of(_)).WillRepeatedly(Return(mpt::root_cert)); + EXPECT_CALL(*mock_file_ops, read_file(_)).WillRepeatedly(Return(mpt::root_cert)); } mp::Rpc::Stub make_secure_stub() @@ -85,8 +86,11 @@ struct TestDaemonRpc : public mpt::DaemonTestFixture mpt::MockPlatform::GuardedMock platform_attr{mpt::MockPlatform::inject()}; mpt::MockPlatform* mock_platform = platform_attr.first; - mpt::MockUtils::GuardedMock attr{mpt::MockUtils::inject()}; - mpt::MockUtils* mock_utils = attr.first; + mpt::MockUtils::GuardedMock utils_attr{mpt::MockUtils::inject()}; + mpt::MockUtils* mock_utils = utils_attr.first; + + mpt::MockFileOps::GuardedMock file_ops_attr{mpt::MockFileOps::inject()}; + mpt::MockFileOps* mock_file_ops = file_ops_attr.first; const mpt::MockPermissionUtils::GuardedMock mock_permission_utils_injection = mpt::MockPermissionUtils::inject(); diff --git a/tests/unit/windows/test_platform_win.cpp b/tests/unit/windows/test_platform_win.cpp index 45662ebc359..a61aeed6a18 100644 --- a/tests/unit/windows/test_platform_win.cpp +++ b/tests/unit/windows/test_platform_win.cpp @@ -625,40 +625,26 @@ TEST(PlatformWin, createAliasScriptWorks) TEST(PlatformWin, createAliasScriptOverwrites) { - auto [mock_utils, guard1] = mpt::MockUtils::inject(); - auto [mock_file_ops, guard2] = mpt::MockFileOps::inject(); + auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_utils, make_file_with_content(_, _, true)).Times(1); + EXPECT_CALL(*mock_file_ops, write_file(_, _, true)).Times(1); EXPECT_NO_THROW( MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "other_command"})); } -TEST(PlatformWin, createAliasScriptThrowsIfCannotCreatePath) -{ - auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(false)); - - MP_EXPECT_THROW_THAT( - MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), - std::runtime_error, - mpt::match_what(HasSubstr("failed to create dir '"))); -} - TEST(PlatformWin, createAliasScriptThrowsIfCannotWriteScript) { auto [mock_file_ops, guard] = mpt::MockFileOps::inject(); - EXPECT_CALL(*mock_file_ops, mkpath(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, open(_, _)).WillOnce(Return(true)); - EXPECT_CALL(*mock_file_ops, write(A(), _, _)).WillOnce(Return(747)); + EXPECT_CALL(*mock_file_ops, write_file(_, _, true)) + .WillOnce(Throw(std::runtime_error{"intentional"})); MP_EXPECT_THROW_THAT( MP_PLATFORM.create_alias_script("alias_name", mp::AliasDefinition{"instance", "command"}), std::runtime_error, - mpt::match_what(HasSubstr("failed to write to file '"))); + mpt::match_what(StrEq("intentional"))); } TEST(PlatformWin, removeAliasScriptWorks) @@ -670,7 +656,7 @@ TEST(PlatformWin, removeAliasScriptWorks) writableLocation(mp::StandardPaths::HomeLocation)) .WillOnce(Return(tmp_dir.path())); - MP_UTILS.make_file_with_content(script_file.fileName().toStdString(), "script content\n"); + MP_FILEOPS.write_file(script_file.fileName().toStdString(), "script content\n"); EXPECT_NO_THROW(MP_PLATFORM.remove_alias_script("alias_name")); diff --git a/tests/unit/windows/test_smb_mount_handler.cpp b/tests/unit/windows/test_smb_mount_handler.cpp index 48e726181e0..fd547f7fa33 100644 --- a/tests/unit/windows/test_smb_mount_handler.cpp +++ b/tests/unit/windows/test_smb_mount_handler.cpp @@ -77,10 +77,10 @@ struct SmbMountHandlerTest : public ::Test EXPECT_CALL(file_ops, status) .WillOnce( Return(mp::fs::file_status{mp::fs::file_type::directory, mp::fs::perms::all})); + ON_CALL(file_ops, read_file).WillByDefault(Return("irrelevant")); + ON_CALL(file_ops, write_file(_, _)).WillByDefault(Return()); EXPECT_CALL(utils, make_dir(_, QString{"enc-keys"}, _)).WillOnce(Return("enc-keys")); EXPECT_CALL(platform, get_username).WillOnce(Return(username)); - ON_CALL(utils, contents_of).WillByDefault(Return("irrelevant")); - ON_CALL(utils, make_file_with_content(_, _)).WillByDefault(Return()); EXPECT_CALL(utils, make_uuid(std::make_optional(vm.get_name()))) .WillOnce(Return(vm_name_uuid)); EXPECT_CALL(utils, make_uuid(std::make_optional(target))).WillOnce(Return(target_uuid));