From 278f7b1b58691248ee4ef2a41146994cf0648375 Mon Sep 17 00:00:00 2001 From: cyan317 <120398799+cindyyan317@users.noreply.github.com> Date: Tue, 21 Jan 2025 14:10:01 +0000 Subject: [PATCH] feat: Block clio if migration is blocking (#1834) Add: - Block server if migration is blocking - Initialise the migration related table when server starts against empty DB Add MigrationInspectorInterface. server uses inspector to check the migrators status. --- src/app/ClioApplication.cpp | 11 ++ src/migration/CMakeLists.txt | 2 +- src/migration/MigrationInspectorFactory.hpp | 65 +++++++++ src/migration/MigrationInspectorInterface.hpp | 79 +++++++++++ src/migration/MigrationManagerInterface.hpp | 46 +------ src/migration/README.md | 10 +- .../cassandra/CassandraMigrationManager.hpp | 20 ++- src/migration/impl/MigrationInspectorBase.hpp | 121 ++++++++++++++++ src/migration/impl/MigrationManagerBase.hpp | 59 +------- src/migration/impl/MigratorsRegister.hpp | 46 +++++++ tests/common/migration/TestMigrators.hpp | 16 ++- tests/unit/CMakeLists.txt | 2 + .../migration/MigrationInspectorBaseTests.cpp | 130 ++++++++++++++++++ .../MigrationInspectorFactoryTests.cpp | 75 ++++++++++ .../migration/MigrationManagerBaseTests.cpp | 1 - .../unit/migration/MigratorRegisterTests.cpp | 49 +++++-- 16 files changed, 612 insertions(+), 120 deletions(-) create mode 100644 src/migration/MigrationInspectorFactory.hpp create mode 100644 src/migration/MigrationInspectorInterface.hpp create mode 100644 src/migration/impl/MigrationInspectorBase.hpp create mode 100644 tests/unit/migration/MigrationInspectorBaseTests.cpp create mode 100644 tests/unit/migration/MigrationInspectorFactoryTests.cpp diff --git a/src/app/ClioApplication.cpp b/src/app/ClioApplication.cpp index 6a64206f..393c7b38 100644 --- a/src/app/ClioApplication.cpp +++ b/src/app/ClioApplication.cpp @@ -26,6 +26,7 @@ #include "etl/LoadBalancer.hpp" #include "etl/NetworkValidatedLedgers.hpp" #include "feed/SubscriptionManager.hpp" +#include "migration/MigrationInspectorFactory.hpp" #include "rpc/Counters.hpp" #include "rpc/RPCEngine.hpp" #include "rpc/WorkQueue.hpp" @@ -103,6 +104,16 @@ ClioApplication::run(bool const useNgWebServer) // Interface to the database auto backend = data::makeBackend(config_); + { + auto const migrationInspector = migration::makeMigrationInspector(config_, backend); + // Check if any migration is blocking Clio server starting. + if (migrationInspector->isBlockingClio() and backend->hardFetchLedgerRangeNoThrow()) { + LOG(util::LogService::error()) + << "Existing Migration is blocking Clio, Please complete the database migration first."; + return EXIT_FAILURE; + } + } + // Manages clients subscribed to streams auto subscriptions = feed::SubscriptionManager::makeSubscriptionManager(config_, backend); diff --git a/src/migration/CMakeLists.txt b/src/migration/CMakeLists.txt index 95a18031..bc4499aa 100644 --- a/src/migration/CMakeLists.txt +++ b/src/migration/CMakeLists.txt @@ -5,4 +5,4 @@ target_sources( cassandra/impl/ObjectsAdapter.cpp cassandra/impl/TransactionsAdapter.cpp ) -target_link_libraries(clio_migration PRIVATE clio_util clio_etl) +target_link_libraries(clio_migration PRIVATE clio_util clio_data) diff --git a/src/migration/MigrationInspectorFactory.hpp b/src/migration/MigrationInspectorFactory.hpp new file mode 100644 index 00000000..039a5dc7 --- /dev/null +++ b/src/migration/MigrationInspectorFactory.hpp @@ -0,0 +1,65 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#pragma once + +#include "data/BackendInterface.hpp" +#include "migration/MigrationInspectorInterface.hpp" +#include "migration/MigratiorStatus.hpp" +#include "migration/cassandra/CassandraMigrationManager.hpp" +#include "util/Assert.hpp" +#include "util/log/Logger.hpp" +#include "util/newconfig/ConfigDefinition.hpp" + +#include +#include + +#include +#include + +namespace migration { + +/** + * @brief A factory function that creates migration inspector instance and initializes the migration table if needed. + * + * @param config The config. + * @param backend The backend instance. It should be initialized before calling this function. + * @return A shared_ptr instance + */ +inline std::shared_ptr +makeMigrationInspector( + util::config::ClioConfigDefinition const& config, + std::shared_ptr const& backend +) +{ + ASSERT(backend != nullptr, "Backend is not initialized"); + + auto inspector = std::make_shared(backend); + + // Database is empty, we need to initialize the migration table if it is a writeable backend + if (not config.get("read_only") and not backend->hardFetchLedgerRangeNoThrow()) { + migration::MigratorStatus migrated(migration::MigratorStatus::Migrated); + for (auto const& name : inspector->allMigratorsNames()) { + backend->writeMigratorStatus(name, migrated.toString()); + } + } + return inspector; +} + +} // namespace migration diff --git a/src/migration/MigrationInspectorInterface.hpp b/src/migration/MigrationInspectorInterface.hpp new file mode 100644 index 00000000..a3382bc0 --- /dev/null +++ b/src/migration/MigrationInspectorInterface.hpp @@ -0,0 +1,79 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#pragma once + +#include "migration/MigratiorStatus.hpp" + +#include +#include +#include + +namespace migration { + +/** + * @brief The interface for the migration inspector.The Clio server application will use this interface to inspect + * the migration status. + */ +struct MigrationInspectorInterface { + virtual ~MigrationInspectorInterface() = default; + + /** + * @brief Get the status of all the migrators + * @return A vector of tuple, the first element is the migrator's name, the second element is the status of the + */ + virtual std::vector> + allMigratorsStatusPairs() const = 0; + + /** + * @brief Get all registered migrators' names + * + * @return A vector of migrators' names + */ + virtual std::vector + allMigratorsNames() const = 0; + + /** + * @brief Get the status of a migrator by its name + * + * @param name The migrator's name + * @return The status of the migrator + */ + virtual MigratorStatus + getMigratorStatusByName(std::string const& name) const = 0; + + /** + * @brief Get the description of a migrator by its name + * + * @param name The migrator's name + * @return The description of the migrator + */ + virtual std::string + getMigratorDescriptionByName(std::string const& name) const = 0; + + /** + * @brief Return if Clio server is blocked + * + * @return True if Clio server is blocked by migration, false otherwise + */ + virtual bool + isBlockingClio() const = 0; +}; + +} // namespace migration diff --git a/src/migration/MigrationManagerInterface.hpp b/src/migration/MigrationManagerInterface.hpp index 863c336a..e76bfeb5 100644 --- a/src/migration/MigrationManagerInterface.hpp +++ b/src/migration/MigrationManagerInterface.hpp @@ -19,59 +19,23 @@ #pragma once -#include "migration/MigratiorStatus.hpp" +#include "migration/MigrationInspectorInterface.hpp" #include -#include -#include namespace migration { /** - * @brief The interface for the migration manager. This interface is tend to be implemented for specific database. The - * application layer will use this interface to run the migrations. + * @brief The interface for the migration manager. The migration application layer will use this interface to run the + * migrations. Unlike the MigrationInspectorInterface which only provides the status of migration, this interface + * contains the acutal migration running method. */ -struct MigrationManagerInterface { - virtual ~MigrationManagerInterface() = default; - +struct MigrationManagerInterface : virtual public MigrationInspectorInterface { /** * @brief Run the the migration according to the given migrator's name */ virtual void runMigration(std::string const&) = 0; - - /** - * @brief Get the status of all the migrators - * @return A vector of tuple, the first element is the migrator's name, the second element is the status of the - */ - virtual std::vector> - allMigratorsStatusPairs() const = 0; - - /** - * @brief Get all registered migrators' names - * - * @return A vector of migrators' names - */ - virtual std::vector - allMigratorsNames() const = 0; - - /** - * @brief Get the status of a migrator by its name - * - * @param name The migrator's name - * @return The status of the migrator - */ - virtual MigratorStatus - getMigratorStatusByName(std::string const& name) const = 0; - - /** - * @brief Get the description of a migrator by its name - * - * @param name The migrator's name - * @return The description of the migrator - */ - virtual std::string - getMigratorDescriptionByName(std::string const& name) const = 0; }; } // namespace migration diff --git a/src/migration/README.md b/src/migration/README.md index 087c0802..9c297a89 100644 --- a/src/migration/README.md +++ b/src/migration/README.md @@ -40,9 +40,11 @@ A migrator satisfies the `MigratorSpec`(impl/Spec.hpp) concept. It contains: -- A `name` which will be used to identify the migrator. User will refer this migrator in command-line tool by this name. The name needs to be different with other migrators, otherwise a compilation error will be raised. +- A `kNAME` which will be used to identify the migrator. User will refer this migrator in command-line tool by this name. The name needs to be different with other migrators, otherwise a compilation error will be raised. -- A `description` which is the detail information of the migrator. +- A `kDESCRIPTION` which is the detail information of the migrator. + +- An optional `kCAN_BLOCK_CLIO` which indicates whether the migrator can block the Clio server. If it's absent, the migrator can't block server. If there is a blocking migrator not completed, the Clio server will fail to start. - A static function `runMigration`, it will be called when user run `--migrate name`. It accepts two parameters: backend, which provides the DB operations interface, and cfg, which provides migration-related configuration. Each migrator can have its own configuration under `.migration` session. @@ -65,8 +67,8 @@ Most indexes are based on either ledger states or transactions. We provide the ` If you need to do full scan against other table, you can follow below steps: - Describe the table which needs full scan in a struct. It has to satisfy the `TableSpec`(cassandra/Spec.hpp) concept, containing static member: - Tuple type `Row`, it's the type of each field in a row. The order of types should match what database will return in a row. Key types should come first, followed by other field types sorted in alphabetical order. - - `PARTITION_KEY`, it's the name of the partition key of the table. - - `TABLE_NAME` + - `kPARTITION_KEY`, it's the name of the partition key of the table. + - `kTABLE_NAME` - Inherent from `FullTableScannerAdapterBase`. - Implement `onRowRead`, its parameter is the `Row` we defined. It's the callback function when a row is read. diff --git a/src/migration/cassandra/CassandraMigrationManager.hpp b/src/migration/cassandra/CassandraMigrationManager.hpp index 65b0f221..a658a820 100644 --- a/src/migration/cassandra/CassandraMigrationManager.hpp +++ b/src/migration/cassandra/CassandraMigrationManager.hpp @@ -19,20 +19,32 @@ #pragma once +#include "data/BackendInterface.hpp" #include "migration/cassandra/CassandraMigrationBackend.hpp" +#include "migration/impl/MigrationInspectorBase.hpp" #include "migration/impl/MigrationManagerBase.hpp" #include "migration/impl/MigratorsRegister.hpp" -namespace migration::cassandra { +namespace { // Register migrators here // MigratorsRegister template using CassandraSupportedMigrators = migration::impl::MigratorsRegister; -// Register with MigrationBackend which proceeds the migration -using MigrationProcesser = CassandraSupportedMigrators; +// Instantiates with the backend which supports actual migration running +using MigrationProcesser = CassandraSupportedMigrators; + +// Instantiates with backend interface, it doesn't support actual migration. But it can be used to inspect the migrators +// status +using MigrationQuerier = CassandraSupportedMigrators; + +} // namespace + +namespace migration::cassandra { + +using CassandraMigrationInspector = migration::impl::MigrationInspectorBase; -// The Cassandra migration manager using CassandraMigrationManager = migration::impl::MigrationManagerBase; + } // namespace migration::cassandra diff --git a/src/migration/impl/MigrationInspectorBase.hpp b/src/migration/impl/MigrationInspectorBase.hpp new file mode 100644 index 00000000..fe712ec6 --- /dev/null +++ b/src/migration/impl/MigrationInspectorBase.hpp @@ -0,0 +1,121 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#pragma once + +#include "migration/MigrationInspectorInterface.hpp" +#include "migration/MigratiorStatus.hpp" + +#include +#include +#include +#include +#include + +namespace migration::impl { + +/** + * @brief The migration inspector implementation for Cassandra. It will report the migration status for Cassandra + * database. + * + * @tparam SupportedMigrators The migrators resgister that contains all the migrators + */ +template +class MigrationInspectorBase : virtual public MigrationInspectorInterface { +protected: + SupportedMigrators migrators_; + +public: + /** + * @brief Construct a new Cassandra Migration Inspector object + * + * @param backend The backend of the Cassandra database + */ + explicit MigrationInspectorBase(std::shared_ptr backend) + : migrators_{std::move(backend)} + { + } + + /** + * @brief Get the status of all the migrators + * + * @return A vector of tuple, the first element is the migrator's name, the second element is the status of the + * migrator + */ + std::vector> + allMigratorsStatusPairs() const override + { + return migrators_.getMigratorsStatus(); + } + + /** + * @brief Get the status of a migrator by its name + * + * @param name The name of the migrator + * @return The status of the migrator + */ + MigratorStatus + getMigratorStatusByName(std::string const& name) const override + { + return migrators_.getMigratorStatus(name); + } + + /** + * @brief Get all registered migrators' names + * + * @return A vector of string, the names of all the migrators + */ + std::vector + allMigratorsNames() const override + { + auto const names = migrators_.getMigratorNames(); + return std::vector{names.begin(), names.end()}; + } + + /** + * @brief Get the description of a migrator by its name + * + * @param name The name of the migrator + * @return The description of the migrator + */ + std::string + getMigratorDescriptionByName(std::string const& name) const override + { + return migrators_.getMigratorDescription(name); + } + + /** + * @brief Return if there is uncomplete migrator blocking the server + * + * @return True if server is blocked, false otherwise + */ + bool + isBlockingClio() const override + { + return std::ranges::any_of(migrators_.getMigratorNames(), [&](auto const& migrator) { + if (auto canBlock = migrators_.canMigratorBlockClio(migrator); canBlock.has_value() and *canBlock and + migrators_.getMigratorStatus(std::string(migrator)) == MigratorStatus::Status::NotMigrated) { + return true; + } + return false; + }); + } +}; + +} // namespace migration::impl diff --git a/src/migration/impl/MigrationManagerBase.hpp b/src/migration/impl/MigrationManagerBase.hpp index c4f2d61f..76c21231 100644 --- a/src/migration/impl/MigrationManagerBase.hpp +++ b/src/migration/impl/MigrationManagerBase.hpp @@ -20,14 +20,12 @@ #pragma once #include "migration/MigrationManagerInterface.hpp" -#include "migration/MigratiorStatus.hpp" +#include "migration/impl/MigrationInspectorBase.hpp" #include "util/newconfig/ObjectView.hpp" #include #include -#include #include -#include namespace migration::impl { @@ -38,8 +36,7 @@ namespace migration::impl { * @tparam SupportedMigrators The migrators resgister that contains all the migrators */ template -class MigrationManagerBase : public MigrationManagerInterface { - SupportedMigrators migrators_; +class MigrationManagerBase : public MigrationManagerInterface, public MigrationInspectorBase { // contains only migration related settings util::config::ObjectView config_; @@ -54,7 +51,7 @@ public: std::shared_ptr backend, util::config::ObjectView config ) - : migrators_{backend}, config_{std::move(config)} + : MigrationInspectorBase{std::move(backend)}, config_{std::move(config)} { } @@ -66,55 +63,7 @@ public: void runMigration(std::string const& name) override { - migrators_.runMigrator(name, config_); - } - - /** - * @brief Get the status of all the migrators - * - * @return A vector of tuple, the first element is the migrator's name, the second element is the status of the - * migrator - */ - std::vector> - allMigratorsStatusPairs() const override - { - return migrators_.getMigratorsStatus(); - } - - /** - * @brief Get the status of a migrator by its name - * - * @param name The name of the migrator - * @return The status of the migrator - */ - MigratorStatus - getMigratorStatusByName(std::string const& name) const override - { - return migrators_.getMigratorStatus(name); - } - - /** - * @brief Get all registered migrators' names - * - * @return A vector of string, the names of all the migrators - */ - std::vector - allMigratorsNames() const override - { - auto const names = migrators_.getMigratorNames(); - return std::vector{names.begin(), names.end()}; - } - - /** - * @brief Get the description of a migrator by its name - * - * @param name The name of the migrator - * @return The description of the migrator - */ - std::string - getMigratorDescriptionByName(std::string const& name) const override - { - return migrators_.getMigratorDescription(name); + this->migrators_.runMigrator(name, config_); } }; diff --git a/src/migration/impl/MigratorsRegister.hpp b/src/migration/impl/MigratorsRegister.hpp index b27479b5..95bbc783 100644 --- a/src/migration/impl/MigratorsRegister.hpp +++ b/src/migration/impl/MigratorsRegister.hpp @@ -22,6 +22,7 @@ #include "data/BackendInterface.hpp" #include "migration/MigratiorStatus.hpp" #include "migration/impl/Spec.hpp" +#include "util/Assert.hpp" #include "util/Concepts.hpp" #include "util/log/Logger.hpp" #include "util/newconfig/ObjectView.hpp" @@ -30,10 +31,12 @@ #include #include #include +#include #include #include #include #include +#include #include namespace migration::impl { @@ -47,6 +50,11 @@ concept MigrationBackend = requires { requires std::same_as concept BackendMatchAllMigrators = (MigrationBackend && ...); +template +concept HasCanBlockClio = requires(T t) { + { t.kCAN_BLOCK_CLIO }; +}; + /** *@brief The register of migrators. It will dispatch the migration to the corresponding migrator. It also *hold the shared pointer of backend, which is used by the migrators. @@ -81,6 +89,23 @@ class MigratorsRegister { return (T::kNAME == targetName) ? T::kDESCRIPTION : ""; } + template + static constexpr bool + canBlockClioHelper(std::string_view targetName) + { + if (targetName == First::kNAME) { + if constexpr (HasCanBlockClio) { + return First::kCAN_BLOCK_CLIO; + } + return false; + } + if constexpr (sizeof...(Rest) > 0) { + return canBlockClioHelper(targetName); + } + ASSERT(false, "The migrator name is not found"); + std::unreachable(); + } + public: /** * @brief The backend type which is used by the migrators @@ -179,6 +204,27 @@ public: return result.empty() ? "No Description" : result; } } + + /** + * @brief Return if the given migrator can block Clio server + * + * @param name The migrator's name + * @return std::nullopt if the migrator name is not found, or a boolean value indicating whether the migrator is + * blocking Clio server. + */ + std::optional + canMigratorBlockClio(std::string_view name) const + { + if constexpr (sizeof...(MigratorType) == 0) { + return std::nullopt; + } else { + auto const migratiors = getMigratorNames(); + if (std::ranges::find(migratiors, name) == migratiors.end()) + return std::nullopt; + + return canBlockClioHelper(name); + } + } }; } // namespace migration::impl diff --git a/tests/common/migration/TestMigrators.hpp b/tests/common/migration/TestMigrators.hpp index 38131d50..03fd64cb 100644 --- a/tests/common/migration/TestMigrators.hpp +++ b/tests/common/migration/TestMigrators.hpp @@ -26,13 +26,10 @@ struct SimpleTestMigrator { using Backend = MockMigrationBackend; static constexpr auto kNAME = "SimpleTestMigrator"; static constexpr auto kDESCRIPTION = "The migrator for version 0 -> 1"; - static void - runMigration(std::shared_ptr, util::config::ObjectView const&) - { - } + static constexpr auto kCAN_BLOCK_CLIO = true; static void - reset() + runMigration(std::shared_ptr, util::config::ObjectView const&) { } }; @@ -45,9 +42,16 @@ struct SimpleTestMigrator2 { runMigration(std::shared_ptr, util::config::ObjectView const&) { } +}; + +struct SimpleTestMigrator3 { + using Backend = MockMigrationBackend; + static constexpr auto kNAME = "SimpleTestMigrator3"; + static constexpr auto kDESCRIPTION = "The migrator for version 3 -> 4"; + static constexpr auto kCAN_BLOCK_CLIO = false; static void - reset() + runMigration(std::shared_ptr, util::config::ObjectView const&) { } }; diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index e9df2ad7..62d19127 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -60,6 +60,8 @@ target_sources( migration/cassandra/SpecTests.cpp migration/MigratorRegisterTests.cpp migration/MigratorStatusTests.cpp + migration/MigrationInspectorFactoryTests.cpp + migration/MigrationInspectorBaseTests.cpp migration/MigrationManagerBaseTests.cpp migration/MigrationManagerFactoryTests.cpp migration/SpecTests.cpp diff --git a/tests/unit/migration/MigrationInspectorBaseTests.cpp b/tests/unit/migration/MigrationInspectorBaseTests.cpp new file mode 100644 index 00000000..d070ee6f --- /dev/null +++ b/tests/unit/migration/MigrationInspectorBaseTests.cpp @@ -0,0 +1,130 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== +#include "data/BackendInterface.hpp" +#include "migration/MigratiorStatus.hpp" +#include "migration/TestMigrators.hpp" +#include "migration/impl/MigrationManagerBase.hpp" +#include "migration/impl/MigratorsRegister.hpp" +#include "util/MockBackendTestFixture.hpp" +#include "util/MockPrometheus.hpp" + +#include +#include + +#include +#include +#include +#include + +using TestMigratorRegister = + migration::impl::MigratorsRegister; + +using TestCassandramigrationInspector = migration::impl::MigrationInspectorBase; + +struct MigrationInspectorBaseTest : public util::prometheus::WithMockPrometheus, public MockBackendTest { + MigrationInspectorBaseTest() + { + migrationInspector_ = std::make_shared(backend_); + } + +protected: + std::shared_ptr migrationInspector_; +}; + +TEST_F(MigrationInspectorBaseTest, AllStatus) +{ + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator", testing::_)).WillOnce(testing::Return("Migrated")); + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator2", testing::_)) + .WillOnce(testing::Return("NotMigrated")); + auto const status = migrationInspector_->allMigratorsStatusPairs(); + EXPECT_EQ(status.size(), 2); + EXPECT_TRUE( + std::ranges::find(status, std::make_tuple("SimpleTestMigrator", migration::MigratorStatus::Migrated)) != + status.end() + ); + EXPECT_TRUE( + std::ranges::find(status, std::make_tuple("SimpleTestMigrator2", migration::MigratorStatus::NotMigrated)) != + status.end() + ); +} + +TEST_F(MigrationInspectorBaseTest, AllNames) +{ + auto const names = migrationInspector_->allMigratorsNames(); + EXPECT_EQ(names.size(), 2); + EXPECT_EQ(names[0], "SimpleTestMigrator"); + EXPECT_EQ(names[1], "SimpleTestMigrator2"); +} + +TEST_F(MigrationInspectorBaseTest, Description) +{ + EXPECT_EQ(migrationInspector_->getMigratorDescriptionByName("unknown"), "No Description"); + EXPECT_EQ( + migrationInspector_->getMigratorDescriptionByName("SimpleTestMigrator"), "The migrator for version 0 -> 1" + ); + EXPECT_EQ( + migrationInspector_->getMigratorDescriptionByName("SimpleTestMigrator2"), "The migrator for version 1 -> 2" + ); +} + +TEST_F(MigrationInspectorBaseTest, getMigratorStatusByName) +{ + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator", testing::_)).WillOnce(testing::Return("Migrated")); + EXPECT_EQ(migrationInspector_->getMigratorStatusByName("SimpleTestMigrator"), migration::MigratorStatus::Migrated); + + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator2", testing::_)) + .WillOnce(testing::Return("NotMigrated")); + EXPECT_EQ( + migrationInspector_->getMigratorStatusByName("SimpleTestMigrator2"), migration::MigratorStatus::NotMigrated + ); +} + +TEST_F(MigrationInspectorBaseTest, oneMigratorBlockingClio) +{ + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator", testing::_)) + .WillOnce(testing::Return("NotMigrated")); + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator2", testing::_)).Times(0); + + EXPECT_TRUE(migrationInspector_->isBlockingClio()); +} + +TEST_F(MigrationInspectorBaseTest, oneMigratorBlockingClioGetMigrated) +{ + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator", testing::_)).WillOnce(testing::Return("Migrated")); + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator2", testing::_)).Times(0); + EXPECT_FALSE(migrationInspector_->isBlockingClio()); +} + +TEST_F(MigrationInspectorBaseTest, noMigratorBlockingClio) +{ + EXPECT_CALL(*backend_, fetchMigratorStatus).Times(0); + + auto const migrations = migration::impl::MigrationInspectorBase< + migration::impl::MigratorsRegister>(backend_); + EXPECT_FALSE(migrations.isBlockingClio()); +} + +TEST_F(MigrationInspectorBaseTest, isBlockingClioWhenNoMigrator) +{ + EXPECT_CALL(*backend_, fetchMigratorStatus).Times(0); + + auto const migrations = + migration::impl::MigrationInspectorBase>(backend_); + EXPECT_FALSE(migrations.isBlockingClio()); +} diff --git a/tests/unit/migration/MigrationInspectorFactoryTests.cpp b/tests/unit/migration/MigrationInspectorFactoryTests.cpp new file mode 100644 index 00000000..9c91a8ac --- /dev/null +++ b/tests/unit/migration/MigrationInspectorFactoryTests.cpp @@ -0,0 +1,75 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2025, the clio developers. + + Permission to use, copy, modify, and distribute this software for any + purpose with or without fee is hereby granted, provided that the above + copyright notice and this permission notice appear in all copies. + + THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. +*/ +//============================================================================== + +#include "data/Types.hpp" +#include "migration/MigrationInspectorFactory.hpp" +#include "util/MockBackendTestFixture.hpp" +#include "util/MockPrometheus.hpp" +#include "util/newconfig/ConfigDefinition.hpp" +#include "util/newconfig/ConfigValue.hpp" +#include "util/newconfig/Types.hpp" + +#include +#include + +#include + +using namespace testing; + +struct MigrationInspectorFactoryTests : public util::prometheus::WithPrometheus, public MockBackendTest { +protected: + util::config::ClioConfigDefinition const readerConfig_ = util::config::ClioConfigDefinition{ + {"read_only", util::config::ConfigValue{util::config::ConfigType::Boolean}.defaultValue(true)} + }; +}; + +struct MigrationInspectorFactoryTestsDeathTest : public MigrationInspectorFactoryTests {}; + +TEST_F(MigrationInspectorFactoryTestsDeathTest, NullBackend) +{ + EXPECT_DEATH(migration::makeMigrationInspector(readerConfig_, nullptr), ".*"); +} + +TEST_F(MigrationInspectorFactoryTests, NotInitMigrationTableIfReader) +{ + EXPECT_CALL(*backend_, hardFetchLedgerRange).Times(0); + + EXPECT_NE(migration::makeMigrationInspector(readerConfig_, backend_), nullptr); +} + +TEST_F(MigrationInspectorFactoryTests, BackendIsWriterAndDBEmpty) +{ + EXPECT_CALL(*backend_, hardFetchLedgerRange).WillOnce(Return(std::nullopt)); + + util::config::ClioConfigDefinition const writerConfig = util::config::ClioConfigDefinition{ + {"read_only", util::config::ConfigValue{util::config::ConfigType::Boolean}.defaultValue(false)} + }; + EXPECT_NE(migration::makeMigrationInspector(writerConfig, backend_), nullptr); +} + +TEST_F(MigrationInspectorFactoryTests, BackendIsWriterAndDBNotEmpty) +{ + LedgerRange const range{.minSequence = 1, .maxSequence = 5}; + EXPECT_CALL(*backend_, hardFetchLedgerRange).WillOnce(Return(range)); + + util::config::ClioConfigDefinition const writerConfig = util::config::ClioConfigDefinition{ + {"read_only", util::config::ConfigValue{util::config::ConfigType::Boolean}.defaultValue(false)} + }; + EXPECT_NE(migration::makeMigrationInspector(writerConfig, backend_), nullptr); +} diff --git a/tests/unit/migration/MigrationManagerBaseTests.cpp b/tests/unit/migration/MigrationManagerBaseTests.cpp index 9cf8c60b..3f78c188 100644 --- a/tests/unit/migration/MigrationManagerBaseTests.cpp +++ b/tests/unit/migration/MigrationManagerBaseTests.cpp @@ -54,7 +54,6 @@ struct MigrationManagerBaseTest : public util::prometheus::WithMockPrometheus, p MigrationManagerBaseTest() { auto mockBackendPtr = backend_.operator std::shared_ptr(); - TestMigratorRegister const migratorRegister(mockBackendPtr); migrationManager = std::make_shared(mockBackendPtr, cfg.getObject("migration")); } }; diff --git a/tests/unit/migration/MigratorRegisterTests.cpp b/tests/unit/migration/MigratorRegisterTests.cpp index 91e8460b..d2cf7263 100644 --- a/tests/unit/migration/MigratorRegisterTests.cpp +++ b/tests/unit/migration/MigratorRegisterTests.cpp @@ -67,8 +67,8 @@ TEST_F(MigratorRegisterTests, EmptyMigratorRegister) EXPECT_EQ(migratorRegister.getMigratorDescription("unknown"), "No Description"); } -using MultipleMigratorRegister = - migration::impl::MigratorsRegister; +using MultipleMigratorRegister = migration::impl:: + MigratorsRegister; struct MultipleMigratorRegisterTests : public util::prometheus::WithMockPrometheus, public MockMigrationBackendTest { std::optional migratorRegister; @@ -82,11 +82,11 @@ struct MultipleMigratorRegisterTests : public util::prometheus::WithMockPromethe TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenError) { EXPECT_CALL(*backend_, fetchMigratorStatus(testing::_, testing::_)) - .Times(2) + .Times(3) .WillRepeatedly(testing::Return(std::nullopt)); auto const status = migratorRegister->getMigratorsStatus(); - EXPECT_EQ(status.size(), 2); + EXPECT_EQ(status.size(), 3); EXPECT_TRUE( std::ranges::find(status, std::make_tuple("SimpleTestMigrator", migration::MigratorStatus::NotMigrated)) != status.end() @@ -95,16 +95,20 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenError) std::ranges::find(status, std::make_tuple("SimpleTestMigrator2", migration::MigratorStatus::NotMigrated)) != status.end() ); + EXPECT_TRUE( + std::ranges::find(status, std::make_tuple("SimpleTestMigrator3", migration::MigratorStatus::NotMigrated)) != + status.end() + ); } TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenReturnInvalidStatus) { EXPECT_CALL(*backend_, fetchMigratorStatus(testing::_, testing::_)) - .Times(2) + .Times(3) .WillRepeatedly(testing::Return("Invalid")); auto const status = migratorRegister->getMigratorsStatus(); - EXPECT_EQ(status.size(), 2); + EXPECT_EQ(status.size(), 3); EXPECT_TRUE( std::ranges::find(status, std::make_tuple("SimpleTestMigrator", migration::MigratorStatus::NotMigrated)) != status.end() @@ -113,6 +117,10 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenReturnInvalidStatus) std::ranges::find(status, std::make_tuple("SimpleTestMigrator2", migration::MigratorStatus::NotMigrated)) != status.end() ); + EXPECT_TRUE( + std::ranges::find(status, std::make_tuple("SimpleTestMigrator3", migration::MigratorStatus::NotMigrated)) != + status.end() + ); } TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenOneMigrated) @@ -120,9 +128,11 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenOneMigrated) EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator", testing::_)).WillOnce(testing::Return("Migrated")); EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator2", testing::_)) .WillOnce(testing::Return("NotMigrated")); + EXPECT_CALL(*backend_, fetchMigratorStatus("SimpleTestMigrator3", testing::_)) + .WillOnce(testing::Return("NotMigrated")); auto const status = migratorRegister->getMigratorsStatus(); - EXPECT_EQ(status.size(), 2); + EXPECT_EQ(status.size(), 3); EXPECT_TRUE( std::ranges::find(status, std::make_tuple("SimpleTestMigrator", migration::MigratorStatus::Migrated)) != status.end() @@ -131,6 +141,10 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorsStatusWhenOneMigrated) std::ranges::find(status, std::make_tuple("SimpleTestMigrator2", migration::MigratorStatus::NotMigrated)) != status.end() ); + EXPECT_TRUE( + std::ranges::find(status, std::make_tuple("SimpleTestMigrator3", migration::MigratorStatus::NotMigrated)) != + status.end() + ); } TEST_F(MultipleMigratorRegisterTests, GetMigratorStatus) @@ -158,9 +172,10 @@ TEST_F(MultipleMigratorRegisterTests, GetMigratorStatusWhenError) TEST_F(MultipleMigratorRegisterTests, Names) { auto names = migratorRegister->getMigratorNames(); - EXPECT_EQ(names.size(), 2); + EXPECT_EQ(names.size(), 3); EXPECT_TRUE(std::ranges::find(names, "SimpleTestMigrator") != names.end()); EXPECT_TRUE(std::ranges::find(names, "SimpleTestMigrator2") != names.end()); + EXPECT_TRUE(std::ranges::find(names, "SimpleTestMigrator3") != names.end()); } TEST_F(MultipleMigratorRegisterTests, Description) @@ -181,3 +196,21 @@ TEST_F(MultipleMigratorRegisterTests, MigrateNormalMigrator) EXPECT_CALL(*backend_, writeMigratorStatus("SimpleTestMigrator", "Migrated")).Times(1); EXPECT_NO_THROW(migratorRegister->runMigrator("SimpleTestMigrator", gCfg.getObject("migration"))); } + +TEST_F(MultipleMigratorRegisterTests, canBlock) +{ + auto canBlock = migratorRegister->canMigratorBlockClio("SimpleTestMigrator"); + EXPECT_TRUE(canBlock); + EXPECT_TRUE(*canBlock); + + canBlock = migratorRegister->canMigratorBlockClio("SimpleTestMigrator2"); + EXPECT_TRUE(canBlock); + EXPECT_FALSE(*canBlock); + + canBlock = migratorRegister->canMigratorBlockClio("SimpleTestMigrator3"); + EXPECT_TRUE(canBlock); + EXPECT_FALSE(*canBlock); + + canBlock = migratorRegister->canMigratorBlockClio("NotAMigrator"); + EXPECT_FALSE(canBlock); +}