Refactor Log Event Handler (#14)

* Add implementation

Signed-off-by: jparisu <javierparis@eprosima.com>

* Add tests

Signed-off-by: jparisu <javierparis@eprosima.com>

* Fix ASAN issue

Signed-off-by: jparisu <javierparis@eprosima.com>

* Add documentation

Signed-off-by: jparisu <javierparis@eprosima.com>

* Fix windows compilation

Signed-off-by: jparisu <javierparis@eprosima.com>

* Add LogChecker documentation

Signed-off-by: jparisu <javierparis@eprosima.com>

* fix bug in new implementation

Signed-off-by: jparisu <javierparis@eprosima.com>

* uncrustify

Signed-off-by: jparisu <javierparis@eprosima.com>

Signed-off-by: jparisu <javierparis@eprosima.com>

Author: jparisu

cpp_utils/include/cpp_utils/event/LogEventHandler.hpp

@@ -22,31 +22,45 @@
 #include <functional>
 
 #include <cpp_utils/Log.hpp>
+#include <cpp_utils/types/Atomicable.hpp>
 #include <cpp_utils/event/EventHandler.hpp>
+#include <cpp_utils/memory/owner_ptr.hpp>
 #include <cpp_utils/library/library_dll.h>
 
 namespace eprosima {
 namespace utils {
 namespace event {
 
+//! Data Type to be shared between a LogEventHandler and a LogConsumerConnection.
+using LogConsumerConnectionCallbackType = std::function<void (const utils::Log::Entry&)>;
+
 /**
  * It implements the functionality to raise callback every time a Log msg is consumed.
  *
- * @warning Fast DDS Log takes the ownership of the pointer of every new consumer (because of reasons...)
- * Thus, in order to create this kind of handler, it must be created from a pointer (new) and the ownership
- * of the pointer will be lost as soon as it is created.
+ * As Fast DDS Log requires to own a unique_ptr with a consumer, this class is separated from \c LogConsumer.
+ * The actual \c LogConsumer used is of class \c LogConsumerConnection and every time it consumes an \c Entry ,
+ * it calls this class.
+ * As \c LogConsumerConnection variable will survive this object, an owner/lessee object is shared between
+ * both objects, so connection keeps calling this callback as long as this object lives, and after this death
+ * it will do nothing.
  */
-class LogEventHandler : public EventHandler<utils::Log::Entry>, utils::LogConsumer
+class LogEventHandler : public EventHandler<utils::Log::Entry>
 {
 public:
 
-    // This class does not have constructor without callback.
-    // This is because of the lost of the pointer once it is registered in Fast. This makes it simpler.
+    /**
+     * Construct without callback.
+     *
+     * It registers in Log the LogConsumer that will call this object.
+     */
+    CPP_UTILS_DllAPI LogEventHandler();
 
     /**
      * Construct a Log Event Handler with callback and enable it.
      *
-     * Calls \c set_callback
+     * It registers in Log the LogConsumer that will call this object.
+     *
+     * Calls \c set_callback .
      *
      * @param callback callback to call every time a log entry is consumed.
      */
@@ -60,34 +74,32 @@ class LogEventHandler : public EventHandler<utils::Log::Entry>, utils::LogConsum
      */
     CPP_UTILS_DllAPI ~LogEventHandler();
 
-    CPP_UTILS_DllAPI void Consume(
-            const utils::Log::Entry& entry) override;
-
 protected:
 
     /**
-     * @brief Override \c callback_set_nts_ from \c EventHandler .
+     * @brief Consumes an \c Entry given from the \c LogConsumerConnection .
      *
-     * The first time is called, it registers the consumer into Log.
+     * @param entry entry consumed.
      */
-    virtual void callback_set_nts_() noexcept override;
-
-    //! Whether the object has been registered in the Log (as soon as it is created in this class)
-    std::atomic<bool> first_registered_;
+    CPP_UTILS_DllAPI virtual void consume_(
+            const utils::Log::Entry& entry);
 
     /**
-     * @brief  Vector of Log entries consumed so far.
+     * @brief Shared object between this and \c LogConsumerConnection registered.
      *
-     * Guarded by \c entries_mutex_ .
+     * When this is destroyed, the ptr is released and the lessee in \c LogConsumerConnection will no longer
+     * be valid, so that object will do nothing with any new Entry.
      */
-    std::vector<utils::Log::Entry> entries_consumed_;
+    OwnerPtr<LogConsumerConnectionCallbackType> connection_callback_;
 
-    //! Guard access to \c entries_consumed_
-    mutable std::mutex entries_mutex_;
+    /**
+     * @brief Vector of Log entries consumed so far.
+     *
+     * Guarded by itself.
+     */
+    SharedAtomicable<std::vector<utils::Log::Entry>> entries_consumed_;
 };
 
 } /* namespace event */
 } /* namespace utils */
 } /* namespace eprosima */
-
-

cpp_utils/include/cpp_utils/event/LogSevereEventHandler.hpp

@@ -48,18 +48,16 @@ class LogSevereEventHandler : public LogEventHandler
             std::function<void(utils::Log::Entry)> callback,
             const utils::Log::Kind threshold = utils::Log::Kind::Warning);
 
-    //! Override parent \c Consume method but only consuming logs above the \c threshold_ kind.
-    CPP_UTILS_DllAPI void Consume(
-            const utils::Log::Entry& entry) override;
-
 protected:
 
+    //! Override parent \c consume_ method but only consuming logs above the \c threshold_ kind.
+    CPP_UTILS_DllAPI void consume_(
+            const utils::Log::Entry& entry) override;
+
     //! Minimum Log kind accepted to consumed.
     utils::Log::Kind threshold_;
 };
 
 } /* namespace event */
 } /* namespace utils */
 } /* namespace eprosima */
-
-

cpp_utils/include/cpp_utils/testing/LogChecker.hpp

@@ -25,32 +25,62 @@
 
 namespace eprosima {
 namespace utils {
-namespace test {
+namespace testing {
 
+/**
+ * @brief This is an auxiliary class to check the logs produced in a test.
+ *
+ * The main idea is to create one of this objects at the beginning of a test execution, and it will
+ * have a counter of the logs consumed (only those higher than threshold given).
+ * At the end, \c check_valid() should be called in order to know if the logs consumed are between the minimum
+ * and maximum logs expected.
+ *
+ * In order to automatically check that no warnings nor errors are produced by a test,
+ * call \c DEFAULT_LOG_TESTER macro at the beginning of the test.
+ * To use specific arguments, use \c INSTANTIATE_LOG_TESTER instead.
+ */
 class LogChecker
 {
 public:
 
+    /**
+     * @brief Construct a LogChecker object.
+     *
+     * @param threshold minimum log level that will be taken into account when counting logs consumed.
+     * @param expected_severe_logs the number of logs this object expects to consumed.
+     * @param max_severe_logs the maximum number of logs this object will allow.
+     */
     CPP_UTILS_DllAPI LogChecker(
             utils::Log::Kind threshold = utils::Log::Kind::Warning,
-            uint32_t expected_severe_logs = 0,
-            uint32_t max_severe_logs = 0);
+            unsigned int expected_severe_logs = 0,
+            unsigned int max_severe_logs = 0);
 
-    CPP_UTILS_DllAPI ~LogChecker();
+    //! Default destructor.
+    CPP_UTILS_DllAPI ~LogChecker() = default;
 
+    /**
+     * @brief Whether the logs consumed so far are between the limits expected.
+     *
+     * @return true if logs consumed are equal ot higher than \c expected_severe_logs
+     * and equal or lower than \c max_severe_logs .
+     * false otherwise.
+     */
     CPP_UTILS_DllAPI bool check_valid();
 
 protected:
 
     /**
-     * @brief Pointer to the event handler log consumer
+     * @brief Log Handler object.
      *
-     * @attention: this must be a raw pointer as Fast takes ownership of the consumer.
+     * It is a Severe one to only take into account those logs higher than threashold
      */
-    utils::event::LogSevereEventHandler* log_consumer_;
+    utils::event::LogSevereEventHandler log_consumer_;
+
+    //! Expected minimum number of logs.
+    unsigned int expected_severe_logs_;
 
-    uint32_t expected_severe_logs_;
-    uint32_t max_severe_logs_;
+    //! Expected maximum number of logs.
+    unsigned int max_severe_logs_;
 };
 
 /**
@@ -70,11 +100,11 @@ class LogChecker
  */
 #define INSTANTIATE_LOG_TESTER(threshold, expected, max) \
     std::unique_ptr< \
-        eprosima::utils::test::LogChecker, \
-        std::function<void(eprosima::utils::test::LogChecker*)>> \
+        eprosima::utils::testing::LogChecker, \
+        std::function<void(eprosima::utils::testing::LogChecker*)>> \
     log_tester( \
-        new eprosima::utils::test::LogChecker(threshold, expected, max), \
-        [](eprosima::utils::test::LogChecker* t){ ASSERT_TRUE( t->check_valid()); delete t; \
+        new eprosima::utils::testing::LogChecker(threshold, expected, max), \
+        [](eprosima::utils::testing::LogChecker* t){ ASSERT_TRUE( t->check_valid()); delete t; \
         })
 
 /**
@@ -83,6 +113,6 @@ class LogChecker
  */
 #define DEFAULT_LOG_TESTER INSTANTIATE_LOG_TESTER(eprosima::utils::Log::Kind::Warning, 0, 0)
 
-} /* namespace test */
+} /* namespace testing */
 } /* namespace utils */
 } /* namespace eprosima */

cpp_utils/src/cpp/event/logging/LogConsumerConnection.cpp

@@ -0,0 +1,52 @@
+// Copyright 2022 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @file LogConsumerConnection.cpp
+ *
+ */
+
+#include <cpp_utils/Log.hpp>
+#include <cpp_utils/exception/InitializationException.hpp>
+
+#include "event/logging/LogConsumerConnection.hpp"
+
+namespace eprosima {
+namespace utils {
+namespace event {
+
+LogConsumerConnection::LogConsumerConnection(
+        LesseePtr<LogConsumerConnectionCallbackType>&& callback)
+    : callback_(std::move(callback))
+{
+    // Do nothing
+}
+
+void LogConsumerConnection::Consume(
+        const utils::Log::Entry& entry)
+{
+    // Check whether the callback still exists
+    auto callback_persistent = callback_.lock();
+
+    if (callback_persistent)
+    {
+        // In case it still exists, call it
+        callback_persistent->operator ()(
+                entry);
+    }
+}
+
+} /* namespace event */
+} /* namespace utils */
+} /* namespace eprosima */

cpp_utils/src/cpp/event/logging/LogConsumerConnection.hpp

@@ -0,0 +1,69 @@
+// Copyright 2022 Proyectos y Sistemas de Mantenimiento SL (eProsima).
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+/**
+ * @file LogConsumerConnection.hpp
+ */
+
+#pragma once
+
+#include <atomic>
+#include <functional>
+
+#include <cpp_utils/Log.hpp>
+#include <cpp_utils/event/LogEventHandler.hpp>
+#include <cpp_utils/memory/owner_ptr.hpp>
+#include <cpp_utils/library/library_dll.h>
+
+namespace eprosima {
+namespace utils {
+namespace event {
+
+/**
+ * This class represents a \c LogConsumer that will be registered in Fast DDS Log as a unique_ptr and with each
+ * \c Entry consumed it will call a shared function from a \c LogEventHandler .
+ * As long as the EventHandler exists, it will manage these callbacks. When it dies, this object ptr to the function
+ * will no longer be valid and thus it will do nothing.
+ */
+class LogConsumerConnection : public utils::LogConsumer
+{
+public:
+
+    //! Construct this class with a Lessee from the actual shared ptr.
+    CPP_UTILS_DllAPI LogConsumerConnection(
+            LesseePtr<LogConsumerConnectionCallbackType>&& callback);
+
+    //! Default destructor
+    CPP_UTILS_DllAPI ~LogConsumerConnection() noexcept = default;
+
+    /**
+     * @brief Implements \c LogConsumer \c Consume method.
+     *
+     * This will call \c callback_ as longs as it is valid.
+     * Notice that while calling it the function could not be removed, as it will be guarded from a \c GuardedPtr .
+     *
+     * @param entry entry to consume
+     */
+    CPP_UTILS_DllAPI void Consume(
+            const utils::Log::Entry& entry) override;
+
+protected:
+
+    //! Lessee to the shared callback object.
+    LesseePtr<LogConsumerConnectionCallbackType> callback_;
+};
+
+} /* namespace event */
+} /* namespace utils */
+} /* namespace eprosima */