From 12dba9fb977f4ca3e62da1f0e29739ca5a534ea8 Mon Sep 17 00:00:00 2001 From: Jelte Jansen Date: Mon, 10 Dec 2012 10:31:47 +0100 Subject: [PATCH] [2445] Remove LogBuffer class and pull its functionality into BufferAppender (one unit test temporarily disabled, might be removed in next commit or updated to get around the protected method it uses) --- src/lib/log/log_buffer_impl.cc | 34 ++++----- src/lib/log/log_buffer_impl.h | 93 ++++++++---------------- src/lib/log/tests/log_buffer_unittest.cc | 38 +++++----- 3 files changed, 65 insertions(+), 100 deletions(-) diff --git a/src/lib/log/log_buffer_impl.cc b/src/lib/log/log_buffer_impl.cc index 05cfe3c7ff..e7bbff8ce7 100644 --- a/src/lib/log/log_buffer_impl.cc +++ b/src/lib/log/log_buffer_impl.cc @@ -22,7 +22,7 @@ namespace isc { namespace log { namespace internal { -LogBuffer::~LogBuffer() { +BufferAppender::~BufferAppender() { // If there is anything left in the buffer, // it means no reconfig has been done, and // we can assume the logging system was either @@ -36,21 +36,7 @@ LogBuffer::~LogBuffer() { } void -LogBuffer::add(const log4cplus::spi::InternalLoggingEvent& event) { - if (flushed_) { - isc_throw(LogBufferAddAfterFlush, - "Internal log buffer has been flushed already"); - } - // get a clone, and put the pointer in a shared_pt - std::auto_ptr event_aptr = - event.clone(); - boost::shared_ptr event_sptr( - event_aptr.release()); - stored_.push_back(event_sptr); -} - -void -LogBuffer::flushStdout() { +BufferAppender::flushStdout() { // This does not show a bit of information normal log messages // do, so perhaps we should try and setup a new logger here // However, as this is called from a destructor, it may not @@ -69,7 +55,7 @@ LogBuffer::flushStdout() { } void -LogBuffer::flush() { +BufferAppender::flush() { LoggerEventPtrList stored_copy; stored_.swap(stored_copy); @@ -80,18 +66,26 @@ LogBuffer::flush() { logger.log((*it)->getLogLevel(), (*it)->getMessage()); } - stored_.clear(); flushed_ = true; } size_t -LogBuffer::getBufferSize() const { +BufferAppender::getBufferSize() const { return (stored_.size()); } void BufferAppender::append(const log4cplus::spi::InternalLoggingEvent& event) { - buffer_.add(event); + if (flushed_) { + isc_throw(LogBufferAddAfterFlush, + "Internal log buffer has been flushed already"); + } + // get a clone, and put the pointer in a shared_pt + std::auto_ptr event_aptr = + event.clone(); + boost::shared_ptr event_sptr( + event_aptr.release()); + stored_.push_back(event_sptr); } } // end namespace internal diff --git a/src/lib/log/log_buffer_impl.h b/src/lib/log/log_buffer_impl.h index e18550a4cd..498bd5a87c 100644 --- a/src/lib/log/log_buffer_impl.h +++ b/src/lib/log/log_buffer_impl.h @@ -42,10 +42,11 @@ public: typedef std::vector > LoggerEventPtrList; -/// \brief Buffering class for logging event +/// \brief Buffering Logger Appender /// -/// This class is used to store logging events; it simply keeps any -/// event that is passed to \c add(), and will replay them to the +/// This class can be set as an Appender for log4cplus loggers, +/// and is used to store logging events; it simply keeps any +/// event that is passed to \c append(), and will replay them to the /// logger that they were originally created for when \c flush() is /// called. /// @@ -58,80 +59,48 @@ typedef std::vector > /// consistent place, and are not lost. /// /// Given this goal, this class has an extra check; it will raise -/// an exception if \c add() is called after flush(). +/// an exception if \c append() is called after flush(). /// /// If the LogBuffer instance is destroyed before being flushed, it will /// dump any event it has left to stdout. -class LogBuffer { - -public: - LogBuffer() : flushed_(false) {}; - - ~LogBuffer(); - - /// \brief add the given event to the list of stored events - /// - /// This is called by the BufferAppender. - /// - /// \param event The event to store - /// \exception LogBufferAddAfterFlush if this method is called - /// when \c flush() has been called previously - void add(const log4cplus::spi::InternalLoggingEvent& event); - - /// \brief Flush all stored events to their loggers - /// - /// All events are replayed to their loggers (which should have - /// other appenders when this is called. - /// Once this method has been called, no more events can be - /// added through calls to \c add(); if \c add() is called after flush(), - /// an exception will be raised. - /// If flush for any reason fails, the remaining events are dropped. - void flush(); - - /// \brief Returns number of stored events - /// - /// Mostly for testing purposes - size_t getBufferSize() const; -private: - /// \brief Simplified flush() to stdout - /// - /// Used in the desctructor; all remainging stored events are - /// printed to stdout, in case flush() was never called. - void flushStdout(); - LoggerEventPtrList stored_; - bool flushed_; -}; - -/// \brief Log4cplus appender for our buffer -/// -/// This class can be set as an Appender for log4cplus loggers -/// -/// When logging an event, it will not actually log anything, but -/// merely add it to its internal LogBuffer class BufferAppender : public log4cplus::Appender { public: /// \brief Constructor /// - /// Constructs a BufferAppender with its own LogBuffer instance - BufferAppender() {} + /// Constructs a BufferAppender that buffers log evens + BufferAppender() : flushed_(false) {} + + /// \brief Destructor + /// + /// Any remaining events are flushed to stdout (there should + /// only be any events remaining if flush() was never called) + ~BufferAppender(); + + /// \brief Close the appender + /// + /// This class has no specialized handling for this method virtual void close() {} /// \brief Flush the internal buffer - void flush() { - buffer_.flush(); - } - - /// \brief Access to the internal log buffer /// - /// This is mostly for testing - LogBuffer& getLogBuffer() { - return (buffer_); - } + /// Events that have been stored (after calls to \c append() + /// are replayed to the logger. Should only be called after + /// new appenders have been set to the logger. + void flush(); + + /// \brief Returns the number of stored logging events + /// + /// Mainly useful for testing + size_t getBufferSize() const; protected: virtual void append(const log4cplus::spi::InternalLoggingEvent& event); private: - LogBuffer buffer_; + /// \brief Helper for the destructor, flush events to stdout + void flushStdout(); + + LoggerEventPtrList stored_; + bool flushed_; }; } // end namespace internal diff --git a/src/lib/log/tests/log_buffer_unittest.cc b/src/lib/log/tests/log_buffer_unittest.cc index cf33b60bca..33a4b010fa 100644 --- a/src/lib/log/tests/log_buffer_unittest.cc +++ b/src/lib/log/tests/log_buffer_unittest.cc @@ -57,8 +57,8 @@ protected: buffer_appender2->flush(); } - //LogBuffer buffer_appender1->getLogBuffer(). - //LogBuffer buffer_appender2->getLogBuffer(). + //LogBuffer buffer_appender1-> + //LogBuffer buffer_appender2-> BufferAppender* buffer_appender1; BufferAppender* buffer_appender2; log4cplus::SharedAppenderPtr appender1; @@ -69,28 +69,28 @@ protected: // Test that log events are indeed stored, and that they are // flushed to the new appenders of their logger TEST_F(LogBufferTest, flush) { - ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize()); - ASSERT_EQ(0, buffer_appender2->getLogBuffer().getBufferSize()); + ASSERT_EQ(0, buffer_appender1->getBufferSize()); + ASSERT_EQ(0, buffer_appender2->getBufferSize()); // Create a Logger, log a few messages with the first appender logger.addAppender(appender1); LOG4CPLUS_INFO(logger, "Foo"); - ASSERT_EQ(1, buffer_appender1->getLogBuffer().getBufferSize()); + ASSERT_EQ(1, buffer_appender1->getBufferSize()); LOG4CPLUS_INFO(logger, "Foo"); - ASSERT_EQ(2, buffer_appender1->getLogBuffer().getBufferSize()); + ASSERT_EQ(2, buffer_appender1->getBufferSize()); LOG4CPLUS_INFO(logger, "Foo"); - ASSERT_EQ(3, buffer_appender1->getLogBuffer().getBufferSize()); + ASSERT_EQ(3, buffer_appender1->getBufferSize()); // Second buffer should still be empty - ASSERT_EQ(0, buffer_appender2->getLogBuffer().getBufferSize()); + ASSERT_EQ(0, buffer_appender2->getBufferSize()); // Replace the appender by the second one, and call flush; // this should cause all events to be moved to the second buffer logger.removeAllAppenders(); logger.addAppender(appender2); buffer_appender1->flush(); - ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize()); - ASSERT_EQ(3, buffer_appender2->getLogBuffer().getBufferSize()); + ASSERT_EQ(0, buffer_appender1->getBufferSize()); + ASSERT_EQ(3, buffer_appender2->getBufferSize()); } // Once flushed, logging new messages with the same buffer should fail @@ -99,39 +99,41 @@ TEST_F(LogBufferTest, addAfterFlush) { buffer_appender1->flush(); EXPECT_THROW(LOG4CPLUS_INFO(logger, "Foo"), LogBufferAddAfterFlush); // It should not have been added - ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize()); + ASSERT_EQ(0, buffer_appender1->getBufferSize()); // But logging should work again as long as a different buffer is used logger.removeAllAppenders(); logger.addAppender(appender2); LOG4CPLUS_INFO(logger, "Foo"); - ASSERT_EQ(1, buffer_appender2->getLogBuffer().getBufferSize()); + ASSERT_EQ(1, buffer_appender2->getBufferSize()); } +/* TEST_F(LogBufferTest, addDirectly) { // A few direct calls log4cplus::spi::InternalLoggingEvent event("buffer", log4cplus::INFO_LOG_LEVEL, "Bar", "file", 123); - buffer_appender1->getLogBuffer().add(event); - ASSERT_EQ(1, buffer_appender1->getLogBuffer().getBufferSize()); + buffer_appender1->append(event); + ASSERT_EQ(1, buffer_appender1->getBufferSize()); // Do one from a smaller scope to make sure destruction doesn't harm { log4cplus::spi::InternalLoggingEvent event2("buffer", log4cplus::INFO_LOG_LEVEL, "Bar", "file", 123); - buffer_appender1->getLogBuffer().add(event2); + buffer_appender1->append(event2); } - ASSERT_EQ(2, buffer_appender1->getLogBuffer().getBufferSize()); + ASSERT_EQ(2, buffer_appender1->getBufferSize()); // And flush them to the next logger.removeAllAppenders(); logger.addAppender(appender2); buffer_appender1->flush(); - ASSERT_EQ(0, buffer_appender1->getLogBuffer().getBufferSize()); - ASSERT_EQ(2, buffer_appender2->getLogBuffer().getBufferSize()); + ASSERT_EQ(0, buffer_appender1->getBufferSize()); + ASSERT_EQ(2, buffer_appender2->getBufferSize()); } +*/ } }