diff --git a/include/multipass/image_host/base_image_host.h b/include/multipass/image_host/base_image_host.h index b547018b6bc..7e6654371c9 100644 --- a/include/multipass/image_host/base_image_host.h +++ b/include/multipass/image_host/base_image_host.h @@ -23,6 +23,8 @@ #include #include +#include + namespace multipass { @@ -31,19 +33,29 @@ class BaseVMImageHost : public VMImageHost public: BaseVMImageHost(URLDownloader* downloader); - void for_each_entry_do(const Action& action) final; - VMImageInfo info_for_full_hash(const std::string& full_hash) final; + std::optional info_for(const Query& query) const final; + std::vector> all_info_for(const Query& query) const final; + VMImageInfo info_for_full_hash(const std::string& full_hash) const final; + std::vector all_images_for(const std::string& remote_name, + const bool allow_unsupported) const final; + void for_each_entry_do(const Action& action) const final; void update_manifests(const bool force_update); protected: void on_manifest_update_failure(const std::string& details); void on_manifest_empty(const std::string& details); - virtual void for_each_entry_do_impl(const Action& action) = 0; - virtual VMImageInfo info_for_full_hash_impl(const std::string& full_hash) = 0; + virtual std::optional info_for_impl(const Query& query) const = 0; + virtual std::vector> all_info_for_impl( + const Query& query) const = 0; + virtual VMImageInfo info_for_full_hash_impl(const std::string& full_hash) const = 0; + virtual std::vector all_images_for_impl(const std::string& remote_name, + const bool allow_unsupported) const = 0; + virtual void for_each_entry_do_impl(const Action& action) const = 0; virtual void clear() = 0; virtual void fetch_manifests(const bool force_update) = 0; + mutable std::shared_mutex manifest_mutex; URLDownloader* const url_downloader; }; diff --git a/include/multipass/image_host/custom_image_host.h b/include/multipass/image_host/custom_image_host.h index c32fcbbe37a..ac2086b5a20 100644 --- a/include/multipass/image_host/custom_image_host.h +++ b/include/multipass/image_host/custom_image_host.h @@ -42,18 +42,19 @@ class CustomVMImageHost final : public BaseVMImageHost public: CustomVMImageHost(URLDownloader* downloader); - std::optional info_for(const Query& query) override; - std::vector> all_info_for(const Query& query) override; - std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) override; - std::vector supported_remotes() override; + std::vector supported_remotes() const override; private: - void for_each_entry_do_impl(const Action& action) override; - VMImageInfo info_for_full_hash_impl(const std::string& full_hash) override; + std::optional info_for_impl(const Query& query) const override; + std::vector> all_info_for_impl( + const Query& query) const override; + std::vector all_images_for_impl(const std::string& remote_name, + const bool allow_unsupported) const override; + void for_each_entry_do_impl(const Action& action) const override; + VMImageInfo info_for_full_hash_impl(const std::string& full_hash) const override; void fetch_manifests(const bool force_update) override; void clear() override; - CustomManifest* manifest_from(const std::string& remote_name); + const CustomManifest& manifest_from(const std::string& remote_name) const; const QString arch; std::pair> manifest; diff --git a/include/multipass/image_host/ubuntu_image_host.h b/include/multipass/image_host/ubuntu_image_host.h index e176c9f57d9..39cca3c7cb3 100644 --- a/include/multipass/image_host/ubuntu_image_host.h +++ b/include/multipass/image_host/ubuntu_image_host.h @@ -38,18 +38,19 @@ class UbuntuVMImageHost final : public BaseVMImageHost UbuntuVMImageHost(std::vector> remotes, URLDownloader* downloader); - std::optional info_for(const Query& query) override; - std::vector> all_info_for(const Query& query) override; - std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) override; - std::vector supported_remotes() override; + std::vector supported_remotes() const override; private: - void for_each_entry_do_impl(const Action& action) override; - VMImageInfo info_for_full_hash_impl(const std::string& full_hash) override; + std::optional info_for_impl(const Query& query) const override; + std::vector> all_info_for_impl( + const Query& query) const override; + std::vector all_images_for_impl(const std::string& remote_name, + const bool allow_unsupported) const override; + void for_each_entry_do_impl(const Action& action) const override; + VMImageInfo info_for_full_hash_impl(const std::string& full_hash) const override; void fetch_manifests(const bool force_update) override; void clear() override; - SimpleStreamsManifest* manifest_from(const std::string& remote); + const SimpleStreamsManifest& manifest_from(const std::string& remote) const; const VMImageInfo* match_alias(const QString& key, const SimpleStreamsManifest& manifest) const; std::vector>> manifests; diff --git a/include/multipass/image_host/vm_image_host.h b/include/multipass/image_host/vm_image_host.h index 496eda63e4f..64564ea6af4 100644 --- a/include/multipass/image_host/vm_image_host.h +++ b/include/multipass/image_host/vm_image_host.h @@ -36,13 +36,14 @@ class VMImageHost : private DisabledCopyMove using Action = std::function; virtual ~VMImageHost() = default; - virtual std::optional info_for(const Query& query) = 0; - virtual std::vector> all_info_for(const Query& query) = 0; - virtual VMImageInfo info_for_full_hash(const std::string& full_hash) = 0; + virtual std::optional info_for(const Query& query) const = 0; + virtual std::vector> all_info_for( + const Query& query) const = 0; + virtual VMImageInfo info_for_full_hash(const std::string& full_hash) const = 0; virtual std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) = 0; - virtual void for_each_entry_do(const Action& action) = 0; - virtual std::vector supported_remotes() = 0; + const bool allow_unsupported) const = 0; + virtual void for_each_entry_do(const Action& action) const = 0; + virtual std::vector supported_remotes() const = 0; virtual void update_manifests(const bool force_update) = 0; protected: diff --git a/src/image_host/base_image_host.cpp b/src/image_host/base_image_host.cpp index ef08bfca606..f00b4e15181 100644 --- a/src/image_host/base_image_host.cpp +++ b/src/image_host/base_image_host.cpp @@ -32,18 +32,42 @@ mp::BaseVMImageHost::BaseVMImageHost(URLDownloader* downloader) : url_downloader { } -void mp::BaseVMImageHost::for_each_entry_do(const Action& action) +auto mp::BaseVMImageHost::info_for(const Query& query) const -> std::optional { - for_each_entry_do_impl(action); + std::shared_lock lock{manifest_mutex}; + return info_for_impl(query); +} + +auto mp::BaseVMImageHost::all_info_for(const Query& query) const + -> std::vector> +{ + std::shared_lock lock{manifest_mutex}; + return all_info_for_impl(query); } -auto mp::BaseVMImageHost::info_for_full_hash(const std::string& full_hash) -> VMImageInfo +auto mp::BaseVMImageHost::info_for_full_hash(const std::string& full_hash) const -> VMImageInfo { + std::shared_lock lock{manifest_mutex}; return info_for_full_hash_impl(full_hash); } +auto mp::BaseVMImageHost::all_images_for(const std::string& remote_name, + const bool allow_unsupported) const + -> std::vector +{ + std::shared_lock lock{manifest_mutex}; + return all_images_for_impl(remote_name, allow_unsupported); +} + +void mp::BaseVMImageHost::for_each_entry_do(const Action& action) const +{ + std::shared_lock lock{manifest_mutex}; + for_each_entry_do_impl(action); +} + void mp::BaseVMImageHost::update_manifests(const bool force_update) { + std::lock_guard lock{manifest_mutex}; clear(); fetch_manifests(force_update); } diff --git a/src/image_host/custom_image_host.cpp b/src/image_host/custom_image_host.cpp index 6e7379015a4..c0897534208 100644 --- a/src/image_host/custom_image_host.cpp +++ b/src/image_host/custom_image_host.cpp @@ -121,44 +121,42 @@ mp::CustomVMImageHost::CustomVMImageHost(URLDownloader* downloader) { } -std::optional mp::CustomVMImageHost::info_for(const Query& query) +std::optional mp::CustomVMImageHost::info_for_impl(const Query& query) const { - auto custom_manifest = manifest_from(query.remote_name); + const auto& custom_manifest = manifest_from(query.remote_name); - auto it = custom_manifest->image_records.find(query.release); + auto it = custom_manifest.image_records.find(query.release); - if (it == custom_manifest->image_records.end()) + if (it == custom_manifest.image_records.end()) return std::nullopt; return *it->second; } -std::vector> mp::CustomVMImageHost::all_info_for( - const Query& query) +std::vector> mp::CustomVMImageHost::all_info_for_impl( + const Query& query) const { std::vector> images; - if (auto image = info_for(query)) + if (auto image = info_for_impl(query)) images.emplace_back(query.remote_name, std::move(*image)); return images; } -std::vector mp::CustomVMImageHost::all_images_for(const std::string& remote_name, - const bool allow_unsupported) +std::vector mp::CustomVMImageHost::all_images_for_impl( + const std::string& remote_name, + const bool allow_unsupported) const { - if (auto custom_manifest = manifest_from(remote_name)) - return custom_manifest->products; - - return {}; + return manifest_from(remote_name).products; } -std::vector mp::CustomVMImageHost::supported_remotes() +std::vector mp::CustomVMImageHost::supported_remotes() const { return {remote}; } -void mp::CustomVMImageHost::for_each_entry_do_impl(const Action& action) +void mp::CustomVMImageHost::for_each_entry_do_impl(const Action& action) const { for (const auto& info : manifest.second->products) { @@ -166,7 +164,7 @@ void mp::CustomVMImageHost::for_each_entry_do_impl(const Action& action) } } -mp::VMImageInfo mp::CustomVMImageHost::info_for_full_hash_impl(const std::string& full_hash) +mp::VMImageInfo mp::CustomVMImageHost::info_for_full_hash_impl(const std::string& full_hash) const { for (const auto& product : manifest.second->products) { @@ -198,11 +196,11 @@ void mp::CustomVMImageHost::clear() manifest = std::pair>{}; } -mp::CustomManifest* mp::CustomVMImageHost::manifest_from(const std::string& remote_name) +const mp::CustomManifest& mp::CustomVMImageHost::manifest_from(const std::string& remote_name) const { - if (remote_name != manifest.first) + if (remote_name != manifest.first || !manifest.second) throw std::runtime_error( fmt::format("Remote \"{}\" is unknown or unreachable.", remote_name)); - return manifest.second.get(); + return *manifest.second; } diff --git a/src/image_host/ubuntu_image_host.cpp b/src/image_host/ubuntu_image_host.cpp index a10e6df26a1..769063d27df 100644 --- a/src/image_host/ubuntu_image_host.cpp +++ b/src/image_host/ubuntu_image_host.cpp @@ -68,9 +68,9 @@ mp::UbuntuVMImageHost::UbuntuVMImageHost( { } -std::optional mp::UbuntuVMImageHost::info_for(const Query& query) +std::optional mp::UbuntuVMImageHost::info_for_impl(const Query& query) const { - auto images = all_info_for(query); + auto images = all_info_for_impl(query); if (images.size() == 0) return std::nullopt; @@ -86,8 +86,8 @@ std::optional mp::UbuntuVMImageHost::info_for(const Query& quer return images.front().second; } -std::vector> mp::UbuntuVMImageHost::all_info_for( - const Query& query) +std::vector> mp::UbuntuVMImageHost::all_info_for_impl( + const Query& query) const { auto key = key_from(query.release); @@ -106,9 +106,9 @@ std::vector> mp::UbuntuVMImageHost::all_ for (const auto& remote_name : remotes_to_search) { - auto* manifest = manifest_from(remote_name); + const auto& manifest = manifest_from(remote_name); - if (const auto* info = match_alias(key, *manifest); info) + if (const auto* info = match_alias(key, manifest); info) { if (!info->supported && !query.allow_unsupported) throw mp::UnsupportedImageException(query.release); @@ -119,7 +119,7 @@ std::vector> mp::UbuntuVMImageHost::all_ { std::unordered_set found_hashes; - for (const auto& entry : manifest->products) + for (const auto& entry : manifest.products) { if (entry.id.startsWith(key) && (entry.supported || query.allow_unsupported) && found_hashes.find(entry.id.toStdString()) == found_hashes.end()) @@ -134,7 +134,7 @@ std::vector> mp::UbuntuVMImageHost::all_ return images; } -mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string& full_hash) +mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string& full_hash) const { for (const auto& manifest : manifests) { @@ -150,13 +150,14 @@ mp::VMImageInfo mp::UbuntuVMImageHost::info_for_full_hash_impl(const std::string throw mp::ImageNotFoundException(full_hash); } -std::vector mp::UbuntuVMImageHost::all_images_for(const std::string& remote_name, - const bool allow_unsupported) +std::vector mp::UbuntuVMImageHost::all_images_for_impl( + const std::string& remote_name, + const bool allow_unsupported) const { std::vector images; - auto manifest = manifest_from(remote_name); + const auto& manifest = manifest_from(remote_name); - for (const auto& entry : manifest->products) + for (const auto& entry : manifest.products) { if (entry.supported || allow_unsupported) { @@ -171,7 +172,7 @@ std::vector mp::UbuntuVMImageHost::all_images_for(const std::st return images; } -void mp::UbuntuVMImageHost::for_each_entry_do_impl(const Action& action) +void mp::UbuntuVMImageHost::for_each_entry_do_impl(const Action& action) const { for (const auto& [remote_name, manifest] : manifests) { @@ -182,7 +183,7 @@ void mp::UbuntuVMImageHost::for_each_entry_do_impl(const Action& action) } } -std::vector mp::UbuntuVMImageHost::supported_remotes() +std::vector mp::UbuntuVMImageHost::supported_remotes() const { std::vector supported_remotes; @@ -253,7 +254,8 @@ void mp::UbuntuVMImageHost::clear() manifests.clear(); } -mp::SimpleStreamsManifest* mp::UbuntuVMImageHost::manifest_from(const std::string& remote) +const mp::SimpleStreamsManifest& mp::UbuntuVMImageHost::manifest_from( + const std::string& remote) const { const auto it = std::find_if( manifests.cbegin(), @@ -267,7 +269,7 @@ mp::SimpleStreamsManifest* mp::UbuntuVMImageHost::manifest_from(const std::strin "mirror is enabled, please confirm it is valid.", remote)); - return it->second.get(); + return *it->second; } const mp::VMImageInfo* mp::UbuntuVMImageHost::match_alias( diff --git a/tests/unit/mock_image_host.h b/tests/unit/mock_image_host.h index 23608b83ea5..89b94ae2611 100644 --- a/tests/unit/mock_image_host.h +++ b/tests/unit/mock_image_host.h @@ -87,18 +87,18 @@ class MockImageHost : public VMImageHost ON_CALL(*this, supported_remotes()).WillByDefault(Return(remote)); }; - MOCK_METHOD(std::optional, info_for, (const Query&), (override)); + MOCK_METHOD(std::optional, info_for, (const Query&), (const, override)); MOCK_METHOD((std::vector>), all_info_for, (const Query&), - (override)); - MOCK_METHOD(VMImageInfo, info_for_full_hash, (const std::string&), (override)); + (const, override)); + MOCK_METHOD(VMImageInfo, info_for_full_hash, (const std::string&), (const, override)); MOCK_METHOD(std::vector, all_images_for, (const std::string&, const bool), - (override)); - MOCK_METHOD(void, for_each_entry_do, (const Action&), (override)); - MOCK_METHOD(std::vector, supported_remotes, (), (override)); + (const, override)); + MOCK_METHOD(void, for_each_entry_do, (const Action&), (const, override)); + MOCK_METHOD(std::vector, supported_remotes, (), (const, override)); MOCK_METHOD(void, update_manifests, (bool), (override)); TempFile image; diff --git a/tests/unit/stub_image_host.h b/tests/unit/stub_image_host.h index 5901d9c3f4a..b32189c7ba6 100644 --- a/tests/unit/stub_image_host.h +++ b/tests/unit/stub_image_host.h @@ -25,34 +25,34 @@ namespace test { struct StubVMImageHost final : public multipass::VMImageHost { - std::optional info_for(const multipass::Query& query) override + std::optional info_for(const multipass::Query& query) const override { return std::optional{ VMImageInfo{{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, -1, {}}}; }; std::vector> all_info_for( - const multipass::Query& query) override + const multipass::Query& query) const override { return {}; }; - multipass::VMImageInfo info_for_full_hash(const std::string& full_hash) override + multipass::VMImageInfo info_for_full_hash(const std::string& full_hash) const override { return {{}, {}, {}, {}, {}, {}, {}, {}, {}, {}, -1, {}}; }; std::vector all_images_for(const std::string& remote_name, - const bool allow_unsupported) override + const bool allow_unsupported) const override { return {}; }; - void for_each_entry_do(const Action&) override + void for_each_entry_do(const Action&) const override { } - std::vector supported_remotes() override + std::vector supported_remotes() const override { return {}; }