util: Generalize logger backend support

This commit introduces a public API change for changing log
destinations.

Refs: #3782
Change-Id: I329c60f510d9493af5959b34eea93b81e15fe698

ndn-cxx/util/logging.cpp

@@ -90,7 +90,9 @@ Logging::get()
 
 Logging::Logging()
 {
-  this->setDestinationImpl(shared_ptr<std::ostream>(&std::clog, [] (auto) {}));
+  // cannot call the static setDestination that uses the singleton Logging object that is not yet constructed
+  auto destination = makeDefaultStreamDestination(shared_ptr<std::ostream>(&std::clog, [] (auto) {}));
+  this->setDestinationImpl(std::move(destination));
 
   const char* environ = std::getenv("NDN_LOG");
   if (environ != nullptr) {
@@ -242,38 +244,64 @@ Logging::resetLevels()
 void
 Logging::setDestination(std::ostream& os)
 {
-  setDestination(shared_ptr<std::ostream>(&os, [] (auto) {}));
+  auto destination = makeDefaultStreamDestination(shared_ptr<std::ostream>(&os, [] (auto) {}));
+  setDestination(std::move(destination));
+}
+
+class TextOstreamBackend : public boost::log::sinks::text_ostream_backend
+{
+public:
+  TextOstreamBackend(std::shared_ptr<std::ostream> os)
+    : m_stdPtr(std::move(os))
+  {
+    auto_flush(true);
+    add_stream(boost::shared_ptr<std::ostream>(m_stdPtr.get(), [] (auto) {}));
+  }
+
+private:
+  // Quite a mess right now because Boost.Log uses boost::shared_ptr and we are using
+  // std::shared_ptr. When it is finally fixed, we can remove this mess.
+  std::shared_ptr<std::ostream> m_stdPtr;
+};
+
+boost::shared_ptr<boost::log::sinks::sink>
+Logging::makeDefaultStreamDestination(shared_ptr<std::ostream> os)
+{
+  auto backend = boost::make_shared<TextOstreamBackend>(std::move(os));
+  auto destination = boost::make_shared<boost::log::sinks::asynchronous_sink<TextOstreamBackend>>(backend);
+
+  namespace expr = boost::log::expressions;
+  destination->set_formatter(expr::stream
+                             << expr::attr<std::string>(log::timestamp.get_name())
+                             << " " << std::setw(5) << expr::attr<LogLevel>(log::severity.get_name()) << ": "
+                             << "[" << expr::attr<std::string>(log::module.get_name()) << "] "
+                             << expr::smessage);
+  return destination;
 }
 
 void
-Logging::setDestinationImpl(shared_ptr<std::ostream> os)
+Logging::setDestinationImpl(boost::shared_ptr<boost::log::sinks::sink> destination)
 {
   std::lock_guard<std::mutex> lock(m_mutex);
 
-  m_destination = std::move(os);
-
-  auto backend = boost::make_shared<boost::log::sinks::text_ostream_backend>();
-  backend->auto_flush(true);
-  backend->add_stream(boost::shared_ptr<std::ostream>(m_destination.get(), [] (auto) {}));
-
-  if (m_sink != nullptr) {
-    boost::log::core::get()->remove_sink(m_sink);
-    m_sink->flush();
-    m_sink.reset();
+  if (destination == m_destination) {
+    return;
   }
 
-  namespace expr = boost::log::expressions;
-  m_sink = boost::make_shared<Sink>(backend);
-  m_sink->set_formatter(expr::stream
-                        << expr::attr<std::string>(log::timestamp.get_name())
-                        << " " << std::setw(5) << expr::attr<LogLevel>(log::severity.get_name()) << ": "
-                        << "[" << expr::attr<std::string>(log::module.get_name()) << "] "
-                        << expr::smessage);
-  boost::log::core::get()->add_sink(m_sink);
+  if (m_destination != nullptr) {
+    boost::log::core::get()->remove_sink(m_destination);
+    m_destination->flush();
+  }
+
+  m_destination = std::move(destination);
+
+  if (m_destination != nullptr) {
+    boost::log::core::get()->add_sink(m_destination);
+  }
 }
 
 #ifdef NDN_CXX_HAVE_TESTS
-shared_ptr<std::ostream>
+boost::shared_ptr<boost::log::sinks::sink>
 Logging::getDestination() const
 {
   return m_destination;
@@ -298,7 +326,11 @@ Logging::getLevels() const
 void
 Logging::flushImpl()
 {
-  m_sink->flush();
+  std::lock_guard<std::mutex> lock(m_mutex);
+
+  if (m_destination != nullptr) {
+    m_destination->flush();
+  }
 }
 
 } // namespace util

ndn-cxx/util/logging.hpp

@@ -1,6 +1,6 @@
 /* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */
 /*
- * Copyright (c) 2013-2018 Regents of the University of California.
+ * Copyright (c) 2013-2019 Regents of the University of California.
  *
  * This file is part of ndn-cxx library (NDN C++ library with eXperimental eXtensions).
  *
@@ -80,19 +80,26 @@ public:
   static void
   setLevel(const std::string& config);
 
-  /** \brief Set log destination.
-   *  \param os a stream for log output
+  /** \brief Set or replace log destination.
+   *  \param destination log backend, e.g., returned by `makeDefaultStreamDestination`
    *
    *  The initial destination is `std::clog`.
+   *
+   *  Note that if \p destination is nullptr, the destination will be removed and the
+   *  application is expected to add its own.  If the application does not set a custom
+   *  destination (using this function or directly using Boost.Log routines), the default
+   *  Boost.Log destination will be used.  Refer to Boost.Log documentation and source code
+   *  for details.
    */
   static void
-  setDestination(shared_ptr<std::ostream> os);
+  setDestination(boost::shared_ptr<boost::log::sinks::sink> destination);
 
-  /** \brief Set log destination.
+  /** \brief Helper method to set stream log destination.
    *  \param os a stream for log output; caller must ensure it remains valid
    *            until setDestination() is invoked again or program exits
+   *`
+   *  This is equivalent to `setDestination(makeDefaultStreamDestination(shared_ptr<std::ostream>(&os, nullDeleter)))`.
    *
-   *  This is equivalent to `setDestination(shared_ptr<std::ostream>(&os, nullDeleter))`.
    */
   static void
   setDestination(std::ostream& os);
@@ -104,6 +111,11 @@ public:
   static void
   flush();
 
+  /** \brief Create stream log destination using default formatting
+   */
+  static boost::shared_ptr<boost::log::sinks::sink>
+  makeDefaultStreamDestination(shared_ptr<std::ostream> os);
+
 private:
   Logging();
 
@@ -138,7 +150,7 @@ private:
   setLevelImpl(const std::string& config);
 
   void
-  setDestinationImpl(shared_ptr<std::ostream> os);
+  setDestinationImpl(boost::shared_ptr<boost::log::sinks::sink> sink);
 
   void
   flushImpl();
@@ -154,7 +166,7 @@ NDN_CXX_PUBLIC_WITH_TESTS_ELSE_PRIVATE:
   void
   resetLevels();
 
-  shared_ptr<std::ostream>
+  boost::shared_ptr<boost::log::sinks::sink>
   getDestination() const;
 
   void
@@ -171,9 +183,7 @@ private:
   std::unordered_map<std::string, LogLevel> m_enabledLevel; ///< module prefix => minimum level
   std::unordered_multimap<std::string, Logger*> m_loggers; ///< module name => logger instance
 
-  using Sink = boost::log::sinks::asynchronous_sink<boost::log::sinks::text_ostream_backend>;
-  boost::shared_ptr<Sink> m_sink;
-  shared_ptr<std::ostream> m_destination;
+  boost::shared_ptr<boost::log::sinks::sink> m_destination;
 };
 
 inline std::set<std::string>
@@ -195,9 +205,9 @@ Logging::setLevel(const std::string& config)
 }
 
 inline void
-Logging::setDestination(shared_ptr<std::ostream> os)
+Logging::setDestination(boost::shared_ptr<boost::log::sinks::sink> destination)
 {
-  get().setDestinationImpl(std::move(os));
+  get().setDestinationImpl(std::move(destination));
 }
 
 inline void

tests/unit/util/logging.t.cpp

@@ -156,7 +156,7 @@ protected:
 
 private:
   std::unordered_map<std::string, LogLevel> m_oldEnabledLevel;
-  shared_ptr<std::ostream> m_oldDestination;
+  boost::shared_ptr<boost::log::sinks::sink> m_oldDestination;
 };
 
 BOOST_AUTO_TEST_SUITE(Util)
@@ -624,7 +624,7 @@ BOOST_AUTO_TEST_CASE(ChangeDestination)
   logFromModule1();
 
   auto os2 = make_shared<output_test_stream>();
-  Logging::setDestination(os2);
+  Logging::setDestination(Logging::makeDefaultStreamDestination(os2));
   weak_ptr<output_test_stream> os2weak(os2);
   os2.reset();
 
@@ -646,6 +646,16 @@ BOOST_AUTO_TEST_CASE(ChangeDestination)
   BOOST_CHECK(os2weak.expired());
 }
 
+BOOST_AUTO_TEST_CASE(SetNullptrDestination)
+{
+  Logging::setDestination(nullptr);
+  logFromModule1();
+
+  Logging::flush();
+  BOOST_CHECK(os.is_equal(""));
+  // The default Boost.Log output is still expected
+}
+
 BOOST_AUTO_TEST_SUITE_END() // TestLogging
 BOOST_AUTO_TEST_SUITE_END() // Util