refactor: Move interval timer into a separate class (#1588)

For #442.

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)

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 <typename SweepHandler>
-struct BasicDOSGuardMock : public web::BaseDOSGuard {
-    BasicDOSGuardMock(SweepHandler& handler)
-    {
-        handler.setup(this);
-    }
-
-    MOCK_METHOD(void, clear, (), (noexcept, override));
-};
 }  // namespace tests::common

tests/common/util/AsioContextTestFixture.hpp

@@ -26,6 +26,7 @@
 #include <boost/asio/spawn.hpp>
 #include <gmock/gmock.h>
 
+#include <chrono>
 #include <optional>
 #include <thread>
 
@@ -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;
 };

tests/common/util/LoggerFixtures.hpp

@@ -22,10 +22,14 @@
 #include "util/log/Logger.hpp"
 
 #include <boost/log/core/core.hpp>
+#include <boost/log/expressions/predicates/channel_severity_filter.hpp>
+#include <boost/log/keywords/format.hpp>
 #include <boost/log/utility/setup/common_attributes.hpp>
+#include <boost/log/utility/setup/console.hpp>
 #include <boost/log/utility/setup/formatter_parser.hpp>
 #include <gtest/gtest.h>
 
+#include <algorithm>
 #include <mutex>
 #include <ostream>
 #include <sstream>
@@ -70,8 +74,14 @@ public:
         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 @@ protected:
     {
         ASSERT_TRUE(buffer_.getStrAndReset().empty());
     }
+
+    std::string
+    getLoggerString()
+    {
+        return buffer_.getStrAndReset();
+    }
 };
 
 /**

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 <gtest/gtest.h>
+
+#include <chrono>
+#include <cstdlib>
+#include <functional>
+#include <future>
+#include <thread>
+
+namespace tests::common::util {
+
+void
+withTimeout(std::chrono::steady_clock::duration timeout, std::function<void()> function)
+{
+    std::promise<void> 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<std::chrono::milliseconds>(timeout).count() << "ms exceeded";
+    }
+    t.join();
+}
+
+}  // namespace tests::common::util

tests/common/util/WithTimeout.hpp

@@ -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
@@ -19,15 +19,18 @@
 
 #pragma once
 
-#include <cstddef>
+#include <chrono>
 #include <functional>
 
-struct FakeAmendmentBlockAction {
-    std::reference_wrapper<std::size_t> callCount;
+namespace tests::common::util {
 
-    void
-    operator()() const
-    {
-        ++(callCount.get());
-    }
-};
+/**
+ * @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<void()> function);
+
+}  // namespace tests::common::util

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

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<MockWhitelistHandler>;
-
-class FakeSweepHandler {
-private:
-    using guardType = BasicDOSGuard<MockWhitelistHandlerType, FakeSweepHandler>;
-    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<MockWhitelistHandlerType, FakeSweepHandler> guard{cfg, whitelistHandler, sweepHandler};
+    BasicDOSGuard<MockWhitelistHandlerType> 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
 }

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 <boost/asio/io_context.hpp>
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
 #include <chrono>
 #include <cstddef>
-#include <functional>
 
-using namespace testing;
-using namespace etl;
+using namespace etl::impl;
 
-struct AmendmentBlockHandlerTest : util::prometheus::WithPrometheus, NoLoggerFixture {
-    using AmendmentBlockHandlerType = impl::AmendmentBlockHandler<FakeAmendmentBlockAction>;
-
-    boost::asio::io_context ioc_;
+struct AmendmentBlockHandlerTest : util::prometheus::WithPrometheus, SyncAsioContextTest {
+    testing::StrictMock<testing::MockFunction<void()>> 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;
 }

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 <boost/asio/error.hpp>
+#include <boost/asio/executor_work_guard.hpp>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include <chrono>
+#include <functional>
+#include <ranges>
+#include <thread>
+#include <utility>
+
+using namespace util;
+using testing::AtLeast;
+
+struct RepeatTests : SyncAsioContextTest {
+    Repeat repeat{ctx};
+    testing::StrictMock<testing::MockFunction<void()>> handlerMock;
+
+    void
+    withRunningContext(std::function<void()> 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();
+        }
+    });
+}

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 <boost/json/parse.hpp>
@@ -28,30 +28,29 @@
 
 #include <chrono>
 
-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<IntervalSweepHandler> guard{sweepHandler};
+    struct DosGuardMock : BaseDOSGuard {
+        MOCK_METHOD(void, clear, (), (noexcept, override));
+    };
+    testing::StrictMock<DosGuardMock> 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});
 }

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;