From 1b4eed3b2b9c7e14442672382685b65a2d9df568 Mon Sep 17 00:00:00 2001 From: Sergey Kuznetsov Date: Wed, 7 Aug 2024 17:38:24 +0100 Subject: [PATCH] refactor: Move interval timer into a separate class (#1588) For #442. --- src/app/ClioApplication.cpp | 4 +- src/etl/CMakeLists.txt | 1 + src/etl/ETLService.hpp | 7 +- src/etl/impl/AmendmentBlock.hpp | 95 ------------------- src/etl/impl/AmendmentBlockHandler.cpp | 56 +++++++++++ src/etl/impl/AmendmentBlockHandler.hpp | 59 ++++++++++++ src/etl/impl/Transformer.hpp | 2 +- src/util/CMakeLists.txt | 3 +- .../util/Repeat.cpp | 29 +++--- src/util/Repeat.hpp | 90 ++++++++++++++++++ src/util/Retry.cpp | 5 - src/util/Retry.hpp | 1 - src/util/async/context/impl/Timer.hpp | 5 - src/util/log/Logger.cpp | 13 +-- src/util/log/Logger.hpp | 11 +++ src/web/DOSGuard.hpp | 13 +-- src/web/IntervalSweepHandler.cpp | 51 +++------- src/web/IntervalSweepHandler.hpp | 34 +------ tests/common/CMakeLists.txt | 2 +- tests/common/rpc/FakesAndMocks.hpp | 10 -- tests/common/util/AsioContextTestFixture.hpp | 8 ++ tests/common/util/LoggerFixtures.hpp | 16 ++++ tests/common/util/WithTimeout.cpp | 47 +++++++++ tests/common/util/WithTimeout.hpp | 36 +++++++ tests/unit/CMakeLists.txt | 3 +- tests/unit/DOSGuardTests.cpp | 27 +----- tests/unit/etl/AmendmentBlockHandlerTests.cpp | 33 ++++--- tests/unit/util/RepeatTests.cpp | 82 ++++++++++++++++ ...ests.cpp => IntervalSweepHandlerTests.cpp} | 31 +++--- tests/unit/web/ServerTests.cpp | 12 +-- 30 files changed, 498 insertions(+), 288 deletions(-) delete mode 100644 src/etl/impl/AmendmentBlock.hpp create mode 100644 src/etl/impl/AmendmentBlockHandler.cpp create mode 100644 src/etl/impl/AmendmentBlockHandler.hpp rename tests/common/util/FakeAmendmentBlockAction.hpp => src/util/Repeat.cpp (76%) create mode 100644 src/util/Repeat.hpp create mode 100644 tests/common/util/WithTimeout.cpp create mode 100644 tests/common/util/WithTimeout.hpp create mode 100644 tests/unit/util/RepeatTests.cpp rename tests/unit/web/{SweepHandlerTests.cpp => IntervalSweepHandlerTests.cpp} (66%) diff --git a/src/app/ClioApplication.cpp b/src/app/ClioApplication.cpp index 458482fc8..13db52aaf 100644 --- a/src/app/ClioApplication.cpp +++ b/src/app/ClioApplication.cpp @@ -93,9 +93,9 @@ ClioApplication::run() boost::asio::io_context ioc{threads}; // Rate limiter, to prevent abuse - auto sweepHandler = web::IntervalSweepHandler{config_, ioc}; auto whitelistHandler = web::WhitelistHandler{config_}; - auto dosGuard = web::DOSGuard{config_, whitelistHandler, sweepHandler}; + auto dosGuard = web::DOSGuard{config_, whitelistHandler}; + auto sweepHandler = web::IntervalSweepHandler{config_, ioc, dosGuard}; // Interface to the database auto backend = data::make_Backend(config_); diff --git a/src/etl/CMakeLists.txt b/src/etl/CMakeLists.txt index c3f04c8d2..adb552c20 100644 --- a/src/etl/CMakeLists.txt +++ b/src/etl/CMakeLists.txt @@ -10,6 +10,7 @@ target_sources( NetworkValidatedLedgers.cpp NFTHelpers.cpp Source.cpp + impl/AmendmentBlockHandler.cpp impl/ForwardingCache.cpp impl/ForwardingSource.cpp impl/GrpcSource.cpp diff --git a/src/etl/ETLService.hpp b/src/etl/ETLService.hpp index 5fded5785..b3ecefc1c 100644 --- a/src/etl/ETLService.hpp +++ b/src/etl/ETLService.hpp @@ -22,11 +22,11 @@ #include "data/BackendInterface.hpp" #include "data/LedgerCache.hpp" #include "etl/CacheLoader.hpp" -#include "etl/ETLHelpers.hpp" #include "etl/ETLState.hpp" #include "etl/LoadBalancer.hpp" +#include "etl/NetworkValidatedLedgersInterface.hpp" #include "etl/SystemState.hpp" -#include "etl/impl/AmendmentBlock.hpp" +#include "etl/impl/AmendmentBlockHandler.hpp" #include "etl/impl/ExtractionDataPipe.hpp" #include "etl/impl/Extractor.hpp" #include "etl/impl/LedgerFetcher.hpp" @@ -37,7 +37,6 @@ #include "util/log/Logger.hpp" #include -#include #include #include #include @@ -85,7 +84,7 @@ class ETLService { using ExtractorType = etl::impl::Extractor; using LedgerLoaderType = etl::impl::LedgerLoader; using LedgerPublisherType = etl::impl::LedgerPublisher; - using AmendmentBlockHandlerType = etl::impl::AmendmentBlockHandler<>; + using AmendmentBlockHandlerType = etl::impl::AmendmentBlockHandler; using TransformerType = etl::impl::Transformer; diff --git a/src/etl/impl/AmendmentBlock.hpp b/src/etl/impl/AmendmentBlock.hpp deleted file mode 100644 index 15b822f71..000000000 --- a/src/etl/impl/AmendmentBlock.hpp +++ /dev/null @@ -1,95 +0,0 @@ -//------------------------------------------------------------------------------ -/* - This file is part of clio: https://github.com/XRPLF/clio - Copyright (c) 2023, 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 "etl/SystemState.hpp" -#include "util/log/Logger.hpp" - -#include -#include -#include - -#include -#include - -namespace etl::impl { - -struct AmendmentBlockAction { - void - operator()() - { - static util::Logger const log{"ETL"}; - LOG(log.fatal()) << "Can't process new ledgers: The current ETL source is not compatible with the version of " - << "the libxrpl Clio is currently using. Please upgrade Clio to a newer version."; - } -}; - -template -class AmendmentBlockHandler { - std::reference_wrapper ctx_; - std::reference_wrapper state_; - boost::asio::steady_timer timer_; - std::chrono::milliseconds interval_; - - ActionCallableType action_; - -public: - template - AmendmentBlockHandler( - boost::asio::io_context& ioc, - SystemState& state, - DurationType interval = DurationType{1}, - ActionCallableType&& action = ActionCallableType() - ) - : ctx_{std::ref(ioc)} - , state_{std::ref(state)} - , timer_{ioc} - , interval_{std::chrono::duration_cast(interval)} - , action_{std::move(action)} - { - } - - ~AmendmentBlockHandler() - { - boost::asio::post(ctx_.get(), [this]() { timer_.cancel(); }); - } - - void - onAmendmentBlock() - { - state_.get().isAmendmentBlocked = true; - startReportingTimer(); - } - -private: - void - startReportingTimer() - { - action_(); - - timer_.expires_after(interval_); - timer_.async_wait([this](auto ec) { - if (!ec) - boost::asio::post(ctx_.get(), [this] { startReportingTimer(); }); - }); - } -}; - -} // namespace etl::impl diff --git a/src/etl/impl/AmendmentBlockHandler.cpp b/src/etl/impl/AmendmentBlockHandler.cpp new file mode 100644 index 000000000..f8ea09b32 --- /dev/null +++ b/src/etl/impl/AmendmentBlockHandler.cpp @@ -0,0 +1,56 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, 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 "etl/impl/AmendmentBlockHandler.hpp" + +#include "etl/SystemState.hpp" +#include "util/log/Logger.hpp" + +#include + +#include +#include +#include + +namespace etl::impl { + +AmendmentBlockHandler::ActionType const AmendmentBlockHandler::defaultAmendmentBlockAction = []() { + static util::Logger const log{"ETL"}; + LOG(log.fatal()) << "Can't process new ledgers: The current ETL source is not compatible with the version of " + << "the libxrpl Clio is currently using. Please upgrade Clio to a newer version."; +}; + +AmendmentBlockHandler::AmendmentBlockHandler( + boost::asio::io_context& ioc, + SystemState& state, + std::chrono::steady_clock::duration interval, + ActionType action +) + : state_{std::ref(state)}, repeat_{ioc}, interval_{interval}, action_{std::move(action)} +{ +} + +void +AmendmentBlockHandler::onAmendmentBlock() +{ + state_.get().isAmendmentBlocked = true; + repeat_.start(interval_, action_); +} + +} // namespace etl::impl diff --git a/src/etl/impl/AmendmentBlockHandler.hpp b/src/etl/impl/AmendmentBlockHandler.hpp new file mode 100644 index 000000000..423e76477 --- /dev/null +++ b/src/etl/impl/AmendmentBlockHandler.hpp @@ -0,0 +1,59 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2023, 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 "etl/SystemState.hpp" +#include "util/Repeat.hpp" + +#include +#include +#include + +#include +#include + +namespace etl::impl { + +class AmendmentBlockHandler { +public: + using ActionType = std::function; + +private: + std::reference_wrapper state_; + util::Repeat repeat_; + std::chrono::steady_clock::duration interval_; + + ActionType action_; + +public: + static ActionType const defaultAmendmentBlockAction; + + AmendmentBlockHandler( + boost::asio::io_context& ioc, + SystemState& state, + std::chrono::steady_clock::duration interval = std::chrono::seconds{1}, + ActionType action = defaultAmendmentBlockAction + ); + + void + onAmendmentBlock(); +}; + +} // namespace etl::impl diff --git a/src/etl/impl/Transformer.hpp b/src/etl/impl/Transformer.hpp index a9de96827..ab4647e10 100644 --- a/src/etl/impl/Transformer.hpp +++ b/src/etl/impl/Transformer.hpp @@ -23,7 +23,7 @@ #include "data/DBHelpers.hpp" #include "data/Types.hpp" #include "etl/SystemState.hpp" -#include "etl/impl/AmendmentBlock.hpp" +#include "etl/impl/AmendmentBlockHandler.hpp" #include "etl/impl/LedgerLoader.hpp" #include "util/Assert.hpp" #include "util/LedgerUtils.hpp" diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index aa219486a..dfb14783c 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -14,11 +14,12 @@ target_sources( prometheus/Prometheus.cpp Random.cpp Retry.cpp - SignalsHandler.cpp + Repeat.cpp requests/RequestBuilder.cpp requests/Types.cpp requests/WsConnection.cpp requests/impl/SslContext.cpp + SignalsHandler.cpp Taggable.cpp TerminationHandler.cpp TimeUtils.cpp diff --git a/tests/common/util/FakeAmendmentBlockAction.hpp b/src/util/Repeat.cpp similarity index 76% rename from tests/common/util/FakeAmendmentBlockAction.hpp rename to src/util/Repeat.cpp index cc89eb3b1..a5bfef7d9 100644 --- a/tests/common/util/FakeAmendmentBlockAction.hpp +++ b/src/util/Repeat.cpp @@ -1,7 +1,7 @@ //------------------------------------------------------------------------------ /* This file is part of clio: https://github.com/XRPLF/clio - Copyright (c) 2023, the clio developers. + Copyright (c) 2024, 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 @@ -17,17 +17,22 @@ */ //============================================================================== -#pragma once +#include "util/Repeat.hpp" -#include -#include +#include -struct FakeAmendmentBlockAction { - std::reference_wrapper callCount; +namespace util { - void - operator()() const - { - ++(callCount.get()); - } -}; +Repeat::Repeat(boost::asio::io_context& ioc) : timer_(ioc) +{ +} + +void +Repeat::stop() +{ + stopping_ = true; + timer_.cancel(); + semaphore_.acquire(); +} + +} // namespace util diff --git a/src/util/Repeat.hpp b/src/util/Repeat.hpp new file mode 100644 index 000000000..d3506f42b --- /dev/null +++ b/src/util/Repeat.hpp @@ -0,0 +1,90 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, 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 +#include +#include + +#include +#include +#include +#include + +namespace util { + +/** + * @brief A class to repeat some action at a regular interval + * @note io_context must be stopped before the Repeat object is destroyed. Otherwise it is undefined behavior + */ +class Repeat { + boost::asio::steady_timer timer_; + std::atomic_bool stopping_{false}; + std::binary_semaphore semaphore_{0}; + +public: + /** + * @brief Construct a new Repeat object + * + * @param ioc The io_context to use + */ + Repeat(boost::asio::io_context& ioc); + + /** + * @brief Stop repeating + * @note This method will block to ensure the repeating is actually stopped. But blocking time should be very short. + */ + void + stop(); + + /** + * @brief Start asynchronously repeating + * + * @tparam Action The action type + * @param interval The interval to repeat + * @param action The action to call regularly + */ + template + void + start(std::chrono::steady_clock::duration interval, Action&& action) + { + stopping_ = false; + startImpl(interval, std::forward(action)); + } + +private: + template + void + startImpl(std::chrono::steady_clock::duration interval, Action&& action) + { + timer_.expires_after(interval); + timer_.async_wait([this, interval, action = std::forward(action)](auto const&) mutable { + if (stopping_) { + semaphore_.release(); + return; + } + action(); + + startImpl(interval, std::forward(action)); + }); + } +}; + +} // namespace util diff --git a/src/util/Retry.cpp b/src/util/Retry.cpp index 41326cdc5..0f7202742 100644 --- a/src/util/Retry.cpp +++ b/src/util/Retry.cpp @@ -57,11 +57,6 @@ Retry::Retry(RetryStrategyPtr strategy, boost::asio::strand strand); - ~Retry(); /** * @brief Schedule a retry diff --git a/src/util/async/context/impl/Timer.hpp b/src/util/async/context/impl/Timer.hpp index 24658eae0..510ba3868 100644 --- a/src/util/async/context/impl/Timer.hpp +++ b/src/util/async/context/impl/Timer.hpp @@ -34,11 +34,6 @@ class SteadyTimer { timer_.async_wait(std::forward(fn)); } - ~SteadyTimer() - { - cancel(); - } - SteadyTimer(SteadyTimer&&) = default; SteadyTimer(SteadyTimer const&) = delete; diff --git a/src/util/log/Logger.cpp b/src/util/log/Logger.cpp index f9151ed8f..06a7782cd 100644 --- a/src/util/log/Logger.cpp +++ b/src/util/log/Logger.cpp @@ -150,24 +150,15 @@ LogService::init(util::Config const& config) // get default severity, can be overridden per channel using the `log_channels` array auto defaultSeverity = config.valueOr("log_level", Severity::NFO); - static constexpr std::array channels = { - "General", - "WebServer", - "Backend", - "RPC", - "ETL", - "Subscriptions", - "Performance", - }; std::unordered_map min_severity; - for (auto const& channel : channels) + for (auto const& channel : Logger::CHANNELS) min_severity[channel] = defaultSeverity; min_severity["Alert"] = Severity::WRN; // Channel for alerts, always warning severity for (auto const overrides = config.arrayOr("log_channels", {}); auto const& cfg : overrides) { auto name = cfg.valueOrThrow("channel", "Channel name is required"); - if (std::count(std::begin(channels), std::end(channels), name) == 0) + if (std::count(std::begin(Logger::CHANNELS), std::end(Logger::CHANNELS), name) == 0) throw std::runtime_error("Can't override settings for log channel " + name + ": invalid channel"); min_severity[name] = cfg.valueOr("log_level", defaultSeverity); diff --git a/src/util/log/Logger.hpp b/src/util/log/Logger.hpp index 0d434050c..46ecd4763 100644 --- a/src/util/log/Logger.hpp +++ b/src/util/log/Logger.hpp @@ -44,6 +44,7 @@ #include #include +#include #include #include #include @@ -176,6 +177,16 @@ class Logger final { }; public: + static constexpr std::array CHANNELS = { + "General", + "WebServer", + "Backend", + "RPC", + "ETL", + "Subscriptions", + "Performance", + }; + /** * @brief Construct a new Logger object that produces loglines for the * specified channel. diff --git a/src/web/DOSGuard.hpp b/src/web/DOSGuard.hpp index cae4675e0..2118bb821 100644 --- a/src/web/DOSGuard.hpp +++ b/src/web/DOSGuard.hpp @@ -58,9 +58,8 @@ class BaseDOSGuard { * @brief A simple denial of service guard used for rate limiting. * * @tparam WhitelistHandlerType The type of the whitelist handler - * @tparam SweepHandlerType The type of the sweep handler */ -template +template class BasicDOSGuard : public BaseDOSGuard { /** * @brief Accumulated state per IP, state will be reset accordingly @@ -90,19 +89,13 @@ class BasicDOSGuard : public BaseDOSGuard { * * @param config Clio config * @param whitelistHandler Whitelist handler that checks whitelist for IP addresses - * @param sweepHandler Sweep handler that implements the sweeping behaviour */ - BasicDOSGuard( - util::Config const& config, - WhitelistHandlerType const& whitelistHandler, - SweepHandlerType& sweepHandler - ) + BasicDOSGuard(util::Config const& config, WhitelistHandlerType const& whitelistHandler) : whitelistHandler_{std::cref(whitelistHandler)} , maxFetches_{config.valueOr("dos_guard.max_fetches", DEFAULT_MAX_FETCHES)} , maxConnCount_{config.valueOr("dos_guard.max_connections", DEFAULT_MAX_CONNECTIONS)} , maxRequestCount_{config.valueOr("dos_guard.max_requests", DEFAULT_MAX_REQUESTS)} { - sweepHandler.setup(this); } /** @@ -262,6 +255,6 @@ class BasicDOSGuard : public BaseDOSGuard { /** * @brief A simple denial of service guard used for rate limiting. */ -using DOSGuard = BasicDOSGuard; +using DOSGuard = BasicDOSGuard; } // namespace web diff --git a/src/web/IntervalSweepHandler.cpp b/src/web/IntervalSweepHandler.cpp index 305f3b3a4..11cadc40b 100644 --- a/src/web/IntervalSweepHandler.cpp +++ b/src/web/IntervalSweepHandler.cpp @@ -19,8 +19,6 @@ #include "web/IntervalSweepHandler.hpp" -#include "util/Assert.hpp" -#include "util/Constants.hpp" #include "util/config/Config.hpp" #include "web/DOSGuard.hpp" @@ -30,49 +28,22 @@ #include #include -#include +#include #include namespace web { -IntervalSweepHandler::IntervalSweepHandler(util::Config const& config, boost::asio::io_context& ctx) - : sweepInterval_{std::max( - 1u, - static_cast( - config.valueOr("dos_guard.sweep_interval", 1.0) * static_cast(util::MILLISECONDS_PER_SECOND) - ) - )} - , ctx_{std::ref(ctx)} - , timer_{ctx.get_executor()} +IntervalSweepHandler::IntervalSweepHandler( + util::Config const& config, + boost::asio::io_context& ctx, + web::BaseDOSGuard& dosGuard +) + : repeat_{std::ref(ctx)} { -} - -IntervalSweepHandler::~IntervalSweepHandler() -{ - boost::asio::post(ctx_.get(), [this]() { timer_.cancel(); }); -} - -void -IntervalSweepHandler::setup(web::BaseDOSGuard* guard) -{ - ASSERT(dosGuard_ == nullptr, "Cannot setup DOS guard more than once"); - dosGuard_ = guard; - ASSERT(dosGuard_ != nullptr, "DOS guard must be not null"); - - createTimer(); -} - -void -IntervalSweepHandler::createTimer() -{ - timer_.expires_after(sweepInterval_); - timer_.async_wait([this](boost::system::error_code const& error) { - if (error == boost::asio::error::operation_aborted) - return; - - dosGuard_->clear(); - boost::asio::post(ctx_.get(), [this] { createTimer(); }); - }); + auto const sweepInterval{std::max( + std::chrono::milliseconds{1u}, util::Config::toMilliseconds(config.valueOr("dos_guard.sweep_interval", 1.0)) + )}; + repeat_.start(sweepInterval, [&dosGuard] { dosGuard.clear(); }); } } // namespace web diff --git a/src/web/IntervalSweepHandler.hpp b/src/web/IntervalSweepHandler.hpp index 2ea4fe71e..af7307241 100644 --- a/src/web/IntervalSweepHandler.hpp +++ b/src/web/IntervalSweepHandler.hpp @@ -19,28 +19,20 @@ #pragma once +#include "util/Repeat.hpp" #include "util/config/Config.hpp" -#include #include -#include - -#include -#include namespace web { class BaseDOSGuard; /** - * @brief Sweep handler using a steady_timer and boost::asio::io_context. + * @brief Sweep handler clearing context every sweep interval from config. */ class IntervalSweepHandler { - std::chrono::milliseconds sweepInterval_; - std::reference_wrapper ctx_; - boost::asio::steady_timer timer_; - - web::BaseDOSGuard* dosGuard_ = nullptr; + util::Repeat repeat_; public: /** @@ -48,25 +40,9 @@ class IntervalSweepHandler { * * @param config Clio config to use * @param ctx The boost::asio::io_context to use + * @param dosGuard The DOS guard to use */ - IntervalSweepHandler(util::Config const& config, boost::asio::io_context& ctx); - - /** - * @brief Cancels the sweep timer. - */ - ~IntervalSweepHandler(); - - /** - * @brief This setup member function is called by @ref BasicDOSGuard during its initialization. - * - * @param guard Pointer to the dos guard - */ - void - setup(web::BaseDOSGuard* guard); - -private: - void - createTimer(); + IntervalSweepHandler(util::Config const& config, boost::asio::io_context& ctx, web::BaseDOSGuard& dosGuard); }; } // namespace web diff --git a/tests/common/CMakeLists.txt b/tests/common/CMakeLists.txt index afaefd702..10d5f5256 100644 --- a/tests/common/CMakeLists.txt +++ b/tests/common/CMakeLists.txt @@ -2,7 +2,7 @@ add_library(clio_testing_common) target_sources( clio_testing_common PRIVATE util/StringUtils.cpp util/TestHttpServer.cpp util/TestWsServer.cpp util/TestObject.cpp - util/AssignRandomPort.cpp + util/AssignRandomPort.cpp util/WithTimeout.cpp ) include(deps/gtest) diff --git a/tests/common/rpc/FakesAndMocks.hpp b/tests/common/rpc/FakesAndMocks.hpp index b25042793..8ddaff329 100644 --- a/tests/common/rpc/FakesAndMocks.hpp +++ b/tests/common/rpc/FakesAndMocks.hpp @@ -171,14 +171,4 @@ struct HandlerWithoutInputMock { MOCK_METHOD(Result, process, (rpc::Context const&), (const)); }; -// testing sweep handler by mocking dos guard -template -struct BasicDOSGuardMock : public web::BaseDOSGuard { - BasicDOSGuardMock(SweepHandler& handler) - { - handler.setup(this); - } - - MOCK_METHOD(void, clear, (), (noexcept, override)); -}; } // namespace tests::common diff --git a/tests/common/util/AsioContextTestFixture.hpp b/tests/common/util/AsioContextTestFixture.hpp index ad63b80b7..da00ad300 100644 --- a/tests/common/util/AsioContextTestFixture.hpp +++ b/tests/common/util/AsioContextTestFixture.hpp @@ -26,6 +26,7 @@ #include #include +#include #include #include @@ -97,6 +98,13 @@ struct SyncAsioContextTest : virtual public NoLoggerFixture { ctx.reset(); } + void + runContextFor(std::chrono::milliseconds duration) + { + ctx.run_for(duration); + ctx.reset(); + } + protected: boost::asio::io_context ctx; }; diff --git a/tests/common/util/LoggerFixtures.hpp b/tests/common/util/LoggerFixtures.hpp index 7774fc34b..dcaa6d810 100644 --- a/tests/common/util/LoggerFixtures.hpp +++ b/tests/common/util/LoggerFixtures.hpp @@ -22,10 +22,14 @@ #include "util/log/Logger.hpp" #include +#include +#include #include +#include #include #include +#include #include #include #include @@ -70,8 +74,14 @@ class LoggerFixture : virtual public ::testing::Test { core->remove_all_sinks(); boost::log::add_console_log(stream_, keywords::format = "%Channel%:%Severity% %Message%"); auto min_severity = expr::channel_severity_filter(util::log_channel, util::log_severity); + + std::ranges::for_each(util::Logger::CHANNELS, [&min_severity](char const* channel) { + min_severity[channel] = util::Severity::TRC; + }); + min_severity["General"] = util::Severity::DBG; min_severity["Trace"] = util::Severity::TRC; + core->set_filter(min_severity); core->set_logging_enabled(true); } @@ -89,6 +99,12 @@ class LoggerFixture : virtual public ::testing::Test { { ASSERT_TRUE(buffer_.getStrAndReset().empty()); } + + std::string + getLoggerString() + { + return buffer_.getStrAndReset(); + } }; /** diff --git a/tests/common/util/WithTimeout.cpp b/tests/common/util/WithTimeout.cpp new file mode 100644 index 000000000..9604fbde2 --- /dev/null +++ b/tests/common/util/WithTimeout.cpp @@ -0,0 +1,47 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, 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 "util/WithTimeout.hpp" + +#include + +#include +#include +#include +#include +#include + +namespace tests::common::util { + +void +withTimeout(std::chrono::steady_clock::duration timeout, std::function function) +{ + std::promise promise; + auto future = promise.get_future(); + std::thread t([&promise, &function] { + function(); + promise.set_value(); + }); + if (future.wait_for(timeout) == std::future_status::timeout) { + FAIL() << "Timeout " << std::chrono::duration_cast(timeout).count() << "ms exceeded"; + } + t.join(); +} + +} // namespace tests::common::util diff --git a/tests/common/util/WithTimeout.hpp b/tests/common/util/WithTimeout.hpp new file mode 100644 index 000000000..3d7821396 --- /dev/null +++ b/tests/common/util/WithTimeout.hpp @@ -0,0 +1,36 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, 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 +#include + +namespace tests::common::util { + +/** + * @brief Run a function with a timeout. If the function does not complete within the timeout, the test will fail. + * + * @param timeout The timeout duration + * @param function The function to run + */ +void +withTimeout(std::chrono::steady_clock::duration timeout, std::function function); + +} // namespace tests::common::util diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 40be1b784..74cd91b8e 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -121,6 +121,7 @@ target_sources( util/requests/WsConnectionTests.cpp util/RandomTests.cpp util/RetryTests.cpp + util/RepeatTests.cpp util/SignalsHandlerTests.cpp util/TimeUtilsTests.cpp util/TxUtilTests.cpp @@ -129,7 +130,7 @@ target_sources( web/impl/ServerSslContextTests.cpp web/RPCServerHandlerTests.cpp web/ServerTests.cpp - web/SweepHandlerTests.cpp + web/IntervalSweepHandlerTests.cpp web/WhitelistHandlerTests.cpp # New Config util/newconfig/ArrayViewTests.cpp diff --git a/tests/unit/DOSGuardTests.cpp b/tests/unit/DOSGuardTests.cpp index 50cc41bca..ac33520aa 100644 --- a/tests/unit/DOSGuardTests.cpp +++ b/tests/unit/DOSGuardTests.cpp @@ -38,7 +38,6 @@ constexpr auto JSONData = R"JSON( { "dos_guard": { "max_fetches": 100, - "sweep_interval": 1, "max_connections": 2, "max_requests": 3, "whitelist": [ @@ -55,33 +54,13 @@ struct MockWhitelistHandler { }; using MockWhitelistHandlerType = NiceMock; - -class FakeSweepHandler { -private: - using guardType = BasicDOSGuard; - guardType* dosGuard_; - -public: - void - setup(guardType* guard) - { - dosGuard_ = guard; - } - - void - sweep() - { - dosGuard_->clear(); - } -}; }; // namespace class DOSGuardTest : public NoLoggerFixture { protected: Config cfg{json::parse(JSONData)}; - FakeSweepHandler sweepHandler{}; MockWhitelistHandlerType whitelistHandler; - BasicDOSGuard guard{cfg, whitelistHandler, sweepHandler}; + BasicDOSGuard guard{cfg, whitelistHandler}; }; TEST_F(DOSGuardTest, Whitelisting) @@ -124,7 +103,7 @@ TEST_F(DOSGuardTest, ClearFetchCountOnTimer) EXPECT_FALSE(guard.add(IP, 1)); // can't add even 1 anymore EXPECT_FALSE(guard.isOk(IP)); - sweepHandler.sweep(); // pretend sweep called from timer + guard.clear(); // pretend sweep called from timer EXPECT_TRUE(guard.isOk(IP)); // can fetch again } @@ -148,6 +127,6 @@ TEST_F(DOSGuardTest, RequestLimitOnTimer) EXPECT_TRUE(guard.isOk(IP)); EXPECT_FALSE(guard.request(IP)); EXPECT_FALSE(guard.isOk(IP)); - sweepHandler.sweep(); + guard.clear(); EXPECT_TRUE(guard.isOk(IP)); // can request again } diff --git a/tests/unit/etl/AmendmentBlockHandlerTests.cpp b/tests/unit/etl/AmendmentBlockHandlerTests.cpp index df3789dba..a2a305c1c 100644 --- a/tests/unit/etl/AmendmentBlockHandlerTests.cpp +++ b/tests/unit/etl/AmendmentBlockHandlerTests.cpp @@ -18,37 +18,42 @@ //============================================================================== #include "etl/SystemState.hpp" -#include "etl/impl/AmendmentBlock.hpp" -#include "util/FakeAmendmentBlockAction.hpp" +#include "etl/impl/AmendmentBlockHandler.hpp" +#include "util/AsioContextTestFixture.hpp" #include "util/LoggerFixtures.hpp" #include "util/MockPrometheus.hpp" #include +#include #include #include #include -#include -using namespace testing; -using namespace etl; +using namespace etl::impl; -struct AmendmentBlockHandlerTest : util::prometheus::WithPrometheus, NoLoggerFixture { - using AmendmentBlockHandlerType = impl::AmendmentBlockHandler; - - boost::asio::io_context ioc_; +struct AmendmentBlockHandlerTest : util::prometheus::WithPrometheus, SyncAsioContextTest { + testing::StrictMock> actionMock; + etl::SystemState state; }; TEST_F(AmendmentBlockHandlerTest, CallToOnAmendmentBlockSetsStateAndRepeatedlyCallsAction) { - std::size_t callCount = 0; - SystemState state; - AmendmentBlockHandlerType handler{ioc_, state, std::chrono::nanoseconds{1}, {std::ref(callCount)}}; + AmendmentBlockHandler handler{ctx, state, std::chrono::nanoseconds{1}, actionMock.AsStdFunction()}; EXPECT_FALSE(state.isAmendmentBlocked); + EXPECT_CALL(actionMock, Call()).Times(testing::AtLeast(10)); handler.onAmendmentBlock(); EXPECT_TRUE(state.isAmendmentBlocked); - ioc_.run_for(std::chrono::milliseconds{1}); - EXPECT_TRUE(callCount >= 10); + runContextFor(std::chrono::milliseconds{1}); +} + +struct DefaultAmendmentBlockActionTest : LoggerFixture {}; + +TEST_F(DefaultAmendmentBlockActionTest, Call) +{ + AmendmentBlockHandler::defaultAmendmentBlockAction(); + auto const loggerString = getLoggerString(); + EXPECT_TRUE(loggerString.starts_with("ETL:FTL Can't process new ledgers")) << "LoggerString " << loggerString; } diff --git a/tests/unit/util/RepeatTests.cpp b/tests/unit/util/RepeatTests.cpp new file mode 100644 index 000000000..98a2a7c91 --- /dev/null +++ b/tests/unit/util/RepeatTests.cpp @@ -0,0 +1,82 @@ +//------------------------------------------------------------------------------ +/* + This file is part of clio: https://github.com/XRPLF/clio + Copyright (c) 2024, 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 "util/AsioContextTestFixture.hpp" +#include "util/Repeat.hpp" +#include "util/WithTimeout.hpp" + +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +using namespace util; +using testing::AtLeast; + +struct RepeatTests : SyncAsioContextTest { + Repeat repeat{ctx}; + testing::StrictMock> handlerMock; + + void + withRunningContext(std::function func) + { + tests::common::util::withTimeout(std::chrono::seconds{1000}, [this, func = std::move(func)]() { + auto workGuard = boost::asio::make_work_guard(ctx); + std::thread thread{[this]() { ctx.run(); }}; + func(); + workGuard.reset(); + thread.join(); + }); + } +}; + +TEST_F(RepeatTests, CallsHandler) +{ + repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); + EXPECT_CALL(handlerMock, Call).Times(AtLeast(10)); + runContextFor(std::chrono::milliseconds{20}); +} + +TEST_F(RepeatTests, StopsOnStop) +{ + withRunningContext([this]() { + repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); + EXPECT_CALL(handlerMock, Call).Times(AtLeast(1)); + std::this_thread::sleep_for(std::chrono::milliseconds{10}); + repeat.stop(); + }); +} + +TEST_F(RepeatTests, RunsAfterStop) +{ + withRunningContext([this]() { + for ([[maybe_unused]] auto _ : std::ranges::iota_view(0, 2)) { + repeat.start(std::chrono::milliseconds{1}, handlerMock.AsStdFunction()); + EXPECT_CALL(handlerMock, Call).Times(AtLeast(1)); + std::this_thread::sleep_for(std::chrono::milliseconds{10}); + repeat.stop(); + } + }); +} diff --git a/tests/unit/web/SweepHandlerTests.cpp b/tests/unit/web/IntervalSweepHandlerTests.cpp similarity index 66% rename from tests/unit/web/SweepHandlerTests.cpp rename to tests/unit/web/IntervalSweepHandlerTests.cpp index 809dd37cb..0e51e053d 100644 --- a/tests/unit/web/SweepHandlerTests.cpp +++ b/tests/unit/web/IntervalSweepHandlerTests.cpp @@ -17,9 +17,9 @@ */ //============================================================================== -#include "rpc/FakesAndMocks.hpp" #include "util/AsioContextTestFixture.hpp" #include "util/config/Config.hpp" +#include "web/DOSGuard.hpp" #include "web/IntervalSweepHandler.hpp" #include @@ -28,30 +28,29 @@ #include -using namespace util; using namespace web; -using namespace testing; -constexpr static auto JSONData = R"JSON( +struct IntervalSweepHandlerTest : SyncAsioContextTest { +protected: + constexpr static auto JSONData = R"JSON( { "dos_guard": { - "max_fetches": 100, - "sweep_interval": 0.1, - "max_connections": 2, - "whitelist": ["127.0.0.1"] + "sweep_interval": 0 } } )JSON"; -class DOSGuardIntervalSweepHandlerTest : public SyncAsioContextTest { -protected: - Config cfg{boost::json::parse(JSONData)}; - IntervalSweepHandler sweepHandler{cfg, ctx}; - tests::common::BasicDOSGuardMock guard{sweepHandler}; + struct DosGuardMock : BaseDOSGuard { + MOCK_METHOD(void, clear, (), (noexcept, override)); + }; + testing::StrictMock guardMock; + + util::Config cfg{boost::json::parse(JSONData)}; + IntervalSweepHandler sweepHandler{cfg, ctx, guardMock}; }; -TEST_F(DOSGuardIntervalSweepHandlerTest, SweepAfterInterval) +TEST_F(IntervalSweepHandlerTest, SweepAfterInterval) { - EXPECT_CALL(guard, clear()).Times(AtLeast(2)); - ctx.run_for(std::chrono::milliseconds(400)); + EXPECT_CALL(guardMock, clear()).Times(testing::AtLeast(10)); + runContextFor(std::chrono::milliseconds{20}); } diff --git a/tests/unit/web/ServerTests.cpp b/tests/unit/web/ServerTests.cpp index 62c26f911..07704d95e 100644 --- a/tests/unit/web/ServerTests.cpp +++ b/tests/unit/web/ServerTests.cpp @@ -127,14 +127,14 @@ struct WebServerTest : NoLoggerFixture { boost::asio::io_context ctxSync; std::string const port = std::to_string(tests::util::generateFreePort()); Config cfg{generateJSONWithDynamicPort(port)}; - IntervalSweepHandler sweepHandler = web::IntervalSweepHandler{cfg, ctxSync}; - WhitelistHandler whitelistHandler = web::WhitelistHandler{cfg}; - DOSGuard dosGuard = web::DOSGuard{cfg, whitelistHandler, sweepHandler}; + WhitelistHandler whitelistHandler{cfg}; + DOSGuard dosGuard{cfg, whitelistHandler}; + IntervalSweepHandler sweepHandler{cfg, ctxSync, dosGuard}; Config cfgOverload{generateJSONDataOverload(port)}; - IntervalSweepHandler sweepHandlerOverload = web::IntervalSweepHandler{cfgOverload, ctxSync}; - WhitelistHandler whitelistHandlerOverload = web::WhitelistHandler{cfgOverload}; - DOSGuard dosGuardOverload = web::DOSGuard{cfgOverload, whitelistHandlerOverload, sweepHandlerOverload}; + WhitelistHandler whitelistHandlerOverload{cfgOverload}; + DOSGuard dosGuardOverload{cfgOverload, whitelistHandlerOverload}; + IntervalSweepHandler sweepHandlerOverload{cfgOverload, ctxSync, dosGuardOverload}; // this ctx is for http server boost::asio::io_context ctx;