From 8f7400731e465adcdf46d4e3c4c343dc45ed4a89 Mon Sep 17 00:00:00 2001
From: gabime <gmelman1@gmail.com>
Date: Fri, 1 Sep 2023 16:23:09 +0300
Subject: [PATCH] ringbuffer_sink: replaced last_raw() and last_formatted()
 with drain_raw() and drain_formatted() and added tests

---
 include/spdlog/sinks/ringbuffer_sink.h | 31 ++++------
 tests/CMakeLists.txt                   |  4 +-
 tests/test_ringbuffer_sink.cpp         | 80 ++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 20 deletions(-)
 create mode 100644 tests/test_ringbuffer_sink.cpp

diff --git a/include/spdlog/sinks/ringbuffer_sink.h b/include/spdlog/sinks/ringbuffer_sink.h
index 3ca47c6f..844322b5 100644
--- a/include/spdlog/sinks/ringbuffer_sink.h
+++ b/include/spdlog/sinks/ringbuffer_sink.h
@@ -11,6 +11,7 @@
 #include <mutex>
 #include <string>
 #include <vector>
+#include <functional>
 
 namespace spdlog {
 namespace sinks {
@@ -25,34 +26,27 @@ public:
         : q_{n_items}
     {}
 
-    std::vector<details::log_msg_buffer> last_raw(size_t lim = 0)
+    void drain_raw(std::function<void(const details::log_msg_buffer&)> callback)
     {
         std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
-        auto items_available = q_.size();
-        auto n_items = lim > 0 ? (std::min)(lim, items_available) : items_available;
-        std::vector<details::log_msg_buffer> ret;
-        ret.reserve(n_items);
-        for (size_t i = (items_available - n_items); i < items_available; i++)
+        while (!q_.empty())
         {
-            ret.push_back(q_.at(i));
+            callback(q_.front());
+            q_.pop_front();
         }
-        return ret;
     }
 
-    std::vector<std::string> last_formatted(size_t lim = 0)
+    void drain(std::function<void(std::string_view)> callback)
     {
         std::lock_guard<Mutex> lock(base_sink<Mutex>::mutex_);
-        auto items_available = q_.size();
-        auto n_items = lim > 0 ? (std::min)(lim, items_available) : items_available;
-        std::vector<std::string> ret;
-        ret.reserve(n_items);
-        for (size_t i = (items_available - n_items); i < items_available; i++)
+        memory_buf_t formatted;
+        while (!q_.empty())
         {
-            memory_buf_t formatted;
-            base_sink<Mutex>::formatter_->format(q_.at(i), formatted);
-            ret.push_back(SPDLOG_BUF_TO_STRING(formatted));
+            formatted.clear();
+            base_sink<Mutex>::formatter_->format(q_.front(), formatted);
+            callback(std::string_view (formatted.data(), formatted.size()));
+            q_.pop_front();
         }
-        return ret;
     }
 
 protected:
@@ -70,5 +64,4 @@ using ringbuffer_sink_mt = ringbuffer_sink<std::mutex>;
 using ringbuffer_sink_st = ringbuffer_sink<details::null_mutex>;
 
 } // namespace sinks
-
 } // namespace spdlog
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
index 439b4fd9..7d93665b 100644
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@@ -44,7 +44,9 @@ set(SPDLOG_UTESTS_SOURCES
     test_cfg.cpp
     test_time_point.cpp
     test_stopwatch.cpp
-    test_circular_q.cpp)
+    test_circular_q.cpp
+    test_ringbuffer_sink.cpp
+)
 
 if(NOT SPDLOG_NO_EXCEPTIONS)
     list(APPEND SPDLOG_UTESTS_SOURCES test_errors.cpp)
diff --git a/tests/test_ringbuffer_sink.cpp b/tests/test_ringbuffer_sink.cpp
new file mode 100644
index 00000000..7e799b6c
--- /dev/null
+++ b/tests/test_ringbuffer_sink.cpp
@@ -0,0 +1,80 @@
+#include "includes.h"
+#include "spdlog/sinks/ringbuffer_sink.h"
+
+TEST_CASE("test_drain", "[ringbuffer_sink]")
+{
+    const size_t sink_size = 3;
+    auto sink = std::make_shared<spdlog::sinks::ringbuffer_sink_mt>(sink_size);
+    spdlog::logger l("logger", sink);
+    l.set_pattern("*** %v");
+
+    // log more than the sink size by one and test that the first message is dropped
+    // test 3 times to make sure the ringbuffer is working correctly multiple times
+    for (int i = 0; i < 3; i++)
+    {
+        for (size_t i = 0; i < sink_size + 1; ++i)
+        {
+            l.info("{}", i);
+        }
+
+        int counter = 0;
+        sink->drain([&](std::string_view msg) {
+            REQUIRE(msg == fmt::format("*** {}{}", counter + 1, spdlog::details::os::default_eol));
+            counter++;
+        });
+
+        REQUIRE(counter == sink_size);
+    }
+}
+
+TEST_CASE("test_drain_raw", "[ringbuffer_sink]")
+{
+    const size_t sink_size = 3;
+    auto sink = std::make_shared<spdlog::sinks::ringbuffer_sink_mt>(sink_size);
+    spdlog::logger l("logger", sink);
+
+    // log more than the sink size by one and test that the first message is dropped
+    // test 3 times to make sure the ringbuffer is working correctly multiple times
+    for (int i = 0; i < 3; i++)
+    {
+        for (size_t i = 0; i < sink_size + 1; ++i)
+        {
+            l.info("{}", i);
+        }
+
+        int counter = 0;
+        sink->drain_raw([&](const spdlog::details::log_msg_buffer &buffer) {
+            REQUIRE(buffer.payload.data() == std::to_string(counter + 1));
+            counter++;
+        });
+
+        REQUIRE(counter == sink_size);
+    }
+}
+
+TEST_CASE("test_empty", "[ringbuffer_sink]")
+{
+    const size_t sink_size = 3;
+    auto sink = std::make_shared<spdlog::sinks::ringbuffer_sink_mt>(sink_size);
+    spdlog::logger l("logger", sink);
+
+    sink->drain([&](std::string_view msg) {
+        REQUIRE_FALSE(true); // should not be called since the sink is empty
+    });
+}
+
+TEST_CASE("test_empty_size", "[ringbuffer_sink]")
+{
+    const size_t sink_size = 0;
+    auto sink = std::make_shared<spdlog::sinks::ringbuffer_sink_mt>(sink_size);
+    spdlog::logger l("logger", sink);
+
+    for (size_t i = 0; i < sink_size + 1; ++i)
+    {
+        l.info("{}", i);
+    }
+
+    sink->drain([&](std::string_view msg) {
+        REQUIRE_FALSE(true); // should not be called since the sink size is 0
+    });
+}
\ No newline at end of file