Merge pull request #3846 from canonical/refactor-image-vault-utils

Refactor image vault utils

Author: sharder996

include/multipass/file_ops.h

@@ -87,6 +87,8 @@ class FileOps : public Singleton<FileOps>
     virtual qint64 write(QFileDevice& file, const QByteArray& data) const;
     virtual bool flush(QFile& file) const;
 
+    virtual bool copy(const QString& from, const QString& to) const;
+
     // QSaveFile operations
     virtual bool commit(QSaveFile& file) const;
 
@@ -119,6 +121,8 @@ class FileOps : public Singleton<FileOps>
                                                                          std::error_code& err) const;
     virtual std::unique_ptr<DirIterator> dir_iterator(const fs::path& path, std::error_code& err) const;
     virtual fs::path weakly_canonical(const fs::path& path) const;
+
+    virtual fs::path remove_extension(const fs::path& path) const;
 };
 } // namespace multipass
 

include/multipass/vm_image_vault.h

@@ -42,15 +42,6 @@ namespace multipass
 class VMImageHost;
 namespace vault
 {
-// Helper functions and classes for all image vault types
-QString filename_for(const Path& path);
-QString copy(const QString& file_name, const QDir& output_dir);
-void delete_file(const Path& path);
-QString compute_image_hash(const Path& image_path);
-void verify_image_download(const Path& image_path, const QString& image_hash);
-QString extract_image(const Path& image_path, const ProgressMonitor& monitor, const bool delete_file = false);
-std::unordered_map<std::string, VMImageHost*> configure_image_host_map(const std::vector<VMImageHost*>& image_hosts);
-
 class DeleteOnException
 {
 public:

include/multipass/vm_image_vault_utils.h

@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) Canonical, Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 3.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#ifndef MULTIPASS_VM_IMAGE_VAULT_UTILS_H
+#define MULTIPASS_VM_IMAGE_VAULT_UTILS_H
+
+#include "file_ops.h"
+#include "xz_image_decoder.h"
+
+#include <functional>
+#include <string>
+#include <unordered_map>
+#include <vector>
+
+#define MP_IMAGE_VAULT_UTILS multipass::ImageVaultUtils::instance()
+
+namespace multipass
+{
+class VMImageHost;
+
+class ImageVaultUtils : public Singleton<ImageVaultUtils>
+{
+public:
+    ImageVaultUtils(const PrivatePass&) noexcept;
+
+    using Decoder = std::function<void(const QString&, const QString&)>;
+    using DefaultDecoderT = XzImageDecoder;
+
+    virtual QString copy_to_dir(const QString& file, const QDir& output_dir) const;
+    [[nodiscard]] virtual QString compute_hash(QIODevice& device) const;
+    [[nodiscard]] virtual QString compute_file_hash(const QString& file) const;
+
+    virtual void verify_file_hash(const QString& file, const QString& hash) const;
+
+    virtual QString extract_file(const QString& file, const Decoder& decoder, bool delete_original = false) const;
+
+    template <class DecoderT = DefaultDecoderT>
+    QString extract_file(const QString& file,
+                         const ProgressMonitor& monitor,
+                         bool delete_original = false,
+                         const DecoderT& = DecoderT{}) const;
+
+    using HostMap = std::unordered_map<std::string, VMImageHost*>;
+    using Hosts = std::vector<VMImageHost*>;
+
+    [[nodiscard]] virtual HostMap configure_image_host_map(const Hosts& image_hosts) const;
+};
+
+template <class DecoderT>
+QString ImageVaultUtils::extract_file(const QString& file,
+                                      const ProgressMonitor& monitor,
+                                      bool delete_original,
+                                      const DecoderT& decoder) const
+{
+    auto decoder_fn = [&monitor, &decoder](const QString& encoded_file, const QString& destination) {
+        return decoder.decode_to(encoded_file, destination, monitor);
+    };
+
+    return extract_file(file, decoder_fn, delete_original);
+}
+
+} // namespace multipass
+
+#endif // MULTIPASS_VM_IMAGE_VAULT_UTILS_H

include/multipass/xz_image_decoder.h

@@ -32,14 +32,13 @@ namespace multipass
 class XzImageDecoder
 {
 public:
-    XzImageDecoder(const Path& xz_file_path);
+    XzImageDecoder();
 
-    void decode_to(const Path& decoded_file_path, const ProgressMonitor& monitor);
+    void decode_to(const Path& xz_file_path, const Path& decoded_file_path, const ProgressMonitor& monitor) const;
 
     using XzDecoderUPtr = std::unique_ptr<xz_dec, decltype(xz_dec_end)*>;
 
 private:
-    QFile xz_file;
     XzDecoderUPtr xz_decoder;
 };
 } // namespace multipass

src/daemon/default_vm_image_vault.cpp

@@ -21,6 +21,7 @@
 #include <multipass/exceptions/create_image_exception.h>
 #include <multipass/exceptions/image_vault_exceptions.h>
 #include <multipass/exceptions/unsupported_image_exception.h>
+#include <multipass/file_ops.h>
 #include <multipass/json_utils.h>
 #include <multipass/logging/log.h>
 #include <multipass/platform.h>
@@ -30,7 +31,6 @@
 #include <multipass/url_downloader.h>
 #include <multipass/utils.h>
 #include <multipass/vm_image.h>
-#include <multipass/xz_image_decoder.h>
 
 #include <multipass/format.h>
 
@@ -170,7 +170,10 @@ void remove_source_images(const mp::VMImage& source_image, const mp::VMImage& pr
 {
     // The prepare phase may have been a no-op, check and only remove source images
     if (source_image.image_path != prepared_image.image_path)
-        mp::vault::delete_file(source_image.image_path);
+    {
+        QFile source_file{source_image.image_path};
+        MP_FILEOPS.remove(source_file);
+    }
 }
 
 void delete_image_dir(const mp::Path& image_path)
@@ -295,7 +298,7 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type,
         }
 
         vm_image = prepare(source_image);
-        vm_image.id = mp::vault::compute_image_hash(vm_image.image_path).toStdString();
+        vm_image.id = MP_IMAGE_VAULT_UTILS.compute_file_hash(vm_image.image_path).toStdString();
 
         remove_source_images(source_image, vm_image);
 
@@ -356,7 +359,8 @@ mp::VMImage mp::DefaultVMImageVault::fetch_image(const FetchType& fetch_type,
                                        last_modified.toString(),
                                        0,
                                        checksum.has_value()};
-                const auto image_filename = mp::vault::filename_for(image_url.path());
+
+                const auto image_filename = QFileInfo{image_url.path()}.fileName();
                 // Attempt to make a sane directory name based on the filename of the image
 
                 const auto image_dir_name =
@@ -620,8 +624,10 @@ mp::VMImage mp::DefaultVMImageVault::download_and_prepare_source_image(
     }
     else
     {
+        QFileInfo file_info{info.image_location};
+
         source_image.id = id.toStdString();
-        source_image.image_path = image_dir.filePath(mp::vault::filename_for(info.image_location));
+        source_image.image_path = image_dir.filePath(file_info.fileName());
         source_image.original_release = info.release_title.toStdString();
         source_image.release_date = info.version.toStdString();
 
@@ -642,12 +648,12 @@ mp::VMImage mp::DefaultVMImageVault::download_and_prepare_source_image(
         {
             mpl::log(mpl::Level::debug, category, fmt::format("Verifying hash \"{}\"", id));
             monitor(LaunchProgress::VERIFY, -1);
-            mp::vault::verify_image_download(source_image.image_path, id);
+            MP_IMAGE_VAULT_UTILS.verify_file_hash(source_image.image_path, id);
         }
 
         if (source_image.image_path.endsWith(".xz"))
         {
-            source_image.image_path = mp::vault::extract_image(source_image.image_path, monitor, true);
+            source_image.image_path = MP_IMAGE_VAULT_UTILS.extract_file(source_image.image_path, monitor, true);
         }
 
         auto prepared_image = prepare(source_image);
@@ -674,14 +680,14 @@ QString mp::DefaultVMImageVault::extract_image_from(const VMImage& source_image,
     const auto image_name = file_info.fileName().remove(".xz");
     const auto image_path = QDir(dest_dir).filePath(image_name);
 
-    return mp::vault::extract_image(image_path, monitor);
+    return MP_IMAGE_VAULT_UTILS.extract_file(image_path, monitor);
 }
 
 mp::VMImage mp::DefaultVMImageVault::image_instance_from(const VMImage& prepared_image, const mp::Path& dest_dir)
 {
     MP_UTILS.make_dir(dest_dir);
 
-    return {mp::vault::copy(prepared_image.image_path, dest_dir),
+    return {MP_IMAGE_VAULT_UTILS.copy_to_dir(prepared_image.image_path, dest_dir),
             prepared_image.id,
             prepared_image.original_release,
             prepared_image.current_release,

src/platform/backends/lxd/lxd_vm_image_vault.cpp

@@ -47,6 +47,7 @@
 #include <QTemporaryDir>
 
 #include <chrono>
+#include <multipass/file_ops.h>
 #include <thread>
 
 namespace mp = multipass;
@@ -82,15 +83,16 @@ QString post_process_downloaded_image(const QString& image_path, const mp::Progr
 
     if (image_path.endsWith(".xz"))
     {
-        new_image_path = mp::vault::extract_image(image_path, monitor, true);
+        new_image_path = MP_IMAGE_VAULT_UTILS.extract_file(image_path, monitor, true);
     }
 
     QString original_image_path{new_image_path};
     new_image_path = mp::backend::convert_to_qcow_if_necessary(new_image_path);
 
     if (original_image_path != new_image_path)
     {
-        mp::vault::delete_file(original_image_path);
+        QFile original_file{original_image_path};
+        MP_FILEOPS.remove(original_file);
     }
 
     return new_image_path;
@@ -261,7 +263,7 @@ mp::VMImage mp::LXDVMImageVault::fetch_image(const FetchType& fetch_type,
                 throw std::runtime_error(fmt::format("Custom image `{}` does not exist.", image_url.path()));
 
             source_image.image_path = image_url.path();
-            id = mp::vault::compute_image_hash(source_image.image_path);
+            id = MP_IMAGE_VAULT_UTILS.compute_file_hash(source_image.image_path);
             last_modified = QDateTime::currentDateTime();
         }
 
@@ -305,13 +307,13 @@ mp::VMImage mp::LXDVMImageVault::fetch_image(const FetchType& fetch_type,
             if (query.query_type != Query::Type::LocalFile)
             {
                 // TODO: Need to make this async like in DefaultVMImageVault
-                image_path = lxd_import_dir.filePath(mp::vault::filename_for(info.image_location));
+                image_path = lxd_import_dir.filePath(QFileInfo{info.image_location}.fileName());
 
                 url_download_image(info, image_path, monitor);
             }
             else
             {
-                image_path = mp::vault::copy(source_image.image_path, lxd_import_dir.path());
+                image_path = MP_IMAGE_VAULT_UTILS.copy_to_dir(source_image.image_path, lxd_import_dir.path());
             }
 
             image_path = post_process_downloaded_image(image_path, monitor);
@@ -522,7 +524,7 @@ void mp::LXDVMImageVault::url_download_image(const VMImageInfo& info, const QStr
     if (info.verify)
     {
         monitor(LaunchProgress::VERIFY, -1);
-        mp::vault::verify_image_download(image_path, info.id);
+        MP_IMAGE_VAULT_UTILS.verify_file_hash(image_path, info.id);
     }
 }
 

src/platform/backends/shared/base_vm_image_vault.h

@@ -23,6 +23,7 @@
 #include <multipass/vm_image.h>
 #include <multipass/vm_image_host.h>
 #include <multipass/vm_image_vault.h>
+#include <multipass/vm_image_vault_utils.h>
 
 #include <string>
 #include <utility>
@@ -34,7 +35,8 @@ class BaseVMImageVault : public VMImageVault
 {
 public:
     explicit BaseVMImageVault(const std::vector<VMImageHost*>& image_hosts)
-        : image_hosts{image_hosts}, remote_image_host_map{vault::configure_image_host_map(image_hosts)} {};
+        : image_hosts{image_hosts},
+          remote_image_host_map{MP_IMAGE_VAULT_UTILS.configure_image_host_map(image_hosts)} {};
 
     VMImageHost* image_host_for(const std::string& remote_name) const override
     {

src/utils/file_ops.cpp

@@ -170,6 +170,11 @@ bool mp::FileOps::flush(QFile& file) const
     return file.flush();
 }
 
+bool mp::FileOps::copy(const QString& from, const QString& to) const
+{
+    return QFile::copy(from, to);
+}
+
 bool mp::FileOps::commit(QSaveFile& file) const
 {
     return file.commit();
@@ -292,3 +297,8 @@ fs::path mp::FileOps::weakly_canonical(const fs::path& path) const
 {
     return fs::weakly_canonical(path);
 }
+
+fs::path mp::FileOps::remove_extension(const fs::path& path) const
+{
+    return path.parent_path() / path.stem();
+}

src/utils/utils.cpp

@@ -401,16 +401,6 @@ void mp::utils::validate_server_address(const std::string& address)
         throw std::runtime_error(fmt::format("invalid port number in address '{}'", address));
 }
 
-std::string mp::utils::filename_for(const std::string& path)
-{
-    return QFileInfo(QString::fromStdString(path)).fileName().toStdString();
-}
-
-bool mp::utils::is_dir(const std::string& path)
-{
-    return QFileInfo(QString::fromStdString(path)).isDir();
-}
-
 std::string mp::utils::match_line_for(const std::string& output, const std::string& matcher)
 {
     std::istringstream ss{output};