Convert to using a slightly smarter wrapper object that manages the relation between the layout manager and the appender.

include/logger/detail/appender.h

@@ -3,18 +3,22 @@
 #include "logger/logger_fwd.h"
 
 namespace logging {
-  class layout;
   struct appender {
-    appender(level min = level::debug) : min_log_level(min) {}
     virtual ~appender() = default;
-    virtual void write(logpacket const & pkt) = 0;
+    virtual std::ostream & stream() = 0;
+    virtual void write(logpacket const & packet, layout & withLayout) = 0;
+    virtual bool should_log(level ll) const = 0;
     virtual void flush() = 0;
+  };
+  
+  struct simple_appender : appender {
+    simple_appender() = default;
+    simple_appender(properties const & props);
     
-    bool should_log(level ll) const {
-      return ll >= min_log_level;
-    }
+    void write(logpacket const & packet, layout & withLayout) override;
+    bool should_log(level ll) const override;
+    void flush() override;
     
-    std::shared_ptr<layout> layout;
-    level min_log_level;
+    level threshold;
   };
 }

include/logger/detail/layout.h

@@ -8,5 +8,8 @@ namespace logging {
   struct layout {
     virtual ~layout() = default;
     virtual void format(std::ostream & os, logpacket const & pkt) const = 0;
+    virtual std::string header() const { return ""; }
+    virtual std::string footer() const { return ""; }
+    virtual std::string separator() const { return ""; }
   };
 }

include/logger/detail/logger_impl.h

@@ -7,14 +7,27 @@
 #include <utility>
 
 namespace logging {
-  class appender;
+  class log_appender {
+  public:
+    log_appender(p_appender append, p_layout layout);
+    ~log_appender();
+    
+    void write(logpacket const & packet);
+    bool should_log(level ll) const;
+    void flush();
+    
+  private:
+    p_appender appender_;
+    p_layout layout_;
+    bool has_logged_;
+  };
 
   struct logger_impl {
     bool should_log(level ll) const;
     void write(logpacket const & pkt);
     void flush();
 
-    std::vector<std::shared_ptr<appender>> impls;
+    std::vector<std::shared_ptr<log_appender>> impls;
     level min_log_level;
   };
 }

include/logger/logger_fwd.h

@@ -14,7 +14,13 @@
 namespace logging {
   enum class level : int;
   struct logpacket;
-  
+  struct properties;
+  // Implementation classes
+  class appender;
+  class layout;
+  using p_appender = std::shared_ptr<appender>;
+  using p_layout = std::shared_ptr<layout>;
+
   std::ostream & operator<<(std::ostream & os, level l);
   
   struct location_info {

src/log_manager.cxx

@@ -26,8 +26,7 @@ INSTANTIATE_PROTOTYPE_FACTORY_2(logging::layouts);
 
 using namespace logging;
 
-using p_appender = std::shared_ptr<appender>;
-using p_layout = std::shared_ptr<layout>;
+using p_logapp = std::shared_ptr<log_appender>;
 using p_logger = std::shared_ptr<logger_impl>;
 
 struct logging::manager_impl {
@@ -39,7 +38,7 @@ struct logging::manager_impl {
   void configure_loggers(properties const & props);
 
   p_logger default_logger;
-  cache<p_appender> appenders;
+  cache<p_logapp> appenders;
   cache<p_logger> loggers;
 };
 
@@ -85,8 +84,8 @@ static p_layout load_layout(std::string const & source,
 
 template <typename T>
 void inject_log_level(properties const & props, T & val) {
-  if (props.contains("level")) {
-    val.min_log_level = level_from_string(props["level"]);
+  if (props.contains("threshold")) {
+    val.min_log_level = level_from_string(props["threshold"]);
   }
 }
 
@@ -94,9 +93,8 @@ void manager_impl::configure_appenders(properties const & props) {
   // TODO: Support multiple File loggers, etc.
   for (auto & app : props["appenders"].object()) {
     auto appender = load_appender(app.first, app.second);
-    appender->layout = load_layout(app.first, app.second);
-    inject_log_level(app.second, *appender);
-    appenders[app.first] = appender;
+    auto layout = load_layout(app.first, app.second);
+    appenders[app.first].reset(new log_appender(appender, layout));
   }
 }
 

src/logger_impl.cxx

@@ -7,12 +7,57 @@
 
 #include "logger_impl.h"
 
+#include "common.h"
 #include "logger/detail/appender.h"
 #include "logger/detail/layout.h"
 #include "logger/logpacket.h"
+#include "logger/properties.h"
 
 using namespace logging;
 
+simple_appender::simple_appender(properties const & props)
+: threshold(level_from_string(props["threshold"])) {}
+
+void simple_appender::write(logpacket const & packet, layout & withLayout) {
+  withLayout.format(stream(), packet);
+}
+
+bool simple_appender::should_log(level ll) const {
+  return ll >= threshold;
+}
+
+void simple_appender::flush() {
+  stream().flush();
+}
+
+log_appender::log_appender(p_appender append, p_layout layout)
+: appender_(append),
+  layout_(layout),
+  has_logged_(false)
+{
+  appender_->stream() << layout_->header();
+}
+
+log_appender::~log_appender() {
+  appender_->stream() << layout_->footer();
+}
+
+void log_appender::write(logpacket const & packet) {
+  if (has_logged_) {
+    appender_->stream() << layout_->separator();
+  }
+  appender_->write(packet, *layout_);
+  has_logged_ = true;
+}
+
+bool log_appender::should_log(level ll) const {
+  return appender_->should_log(ll);
+}
+
+void log_appender::flush() {
+  appender_->flush();
+}
+
 bool logger_impl::should_log(level ll) const {
   return ll >= min_log_level;
 }

src/loggers/console_appender.cxx

@@ -17,39 +17,40 @@
 
 using namespace logging;
 
-class console_appender : public appender {
+class console_appender : public simple_appender {
 public:
   static std::shared_ptr<appender> create(properties const& props);
   
-  explicit console_appender(std::ostream& os);
+  explicit console_appender(properties const& props);
   
-  void write(logpacket const & pkt) override;
-  
-  void flush() override;
+  std::ostream & stream() override { return out_; }
 private:
   std::ostream& out_;
 };
 
+using namespace logging::property;
+properties const DEFAULT_CONSOLE_APPENDER{_obj({
+  {"target", {}},
+  {"threshold", _v("ERROR")}
+})};
+
 std::shared_ptr<appender> console_appender::create(properties const& props) {
-  const std::string target = props["target"];
+  properties const actual = DEFAULT_CONSOLE_APPENDER.mergedWith(props);
+  return std::make_shared<console_appender>(actual);
+}
+
+static std::ostream & get_ostream(std::string const & target) {
   if (target == "SYSTEM_OUT") {
-    return std::make_shared<console_appender>(std::cout);
+    return std::cout;
   } else if (target == "SYSTEM_ERR") {
-    return std::make_shared<console_appender>(std::cerr);
+    return std::cerr;
   } else {
     throw std::logic_error{target + " is not a valid console"};
   }
 }
 
-console_appender::console_appender(std::ostream& os) : out_(os) {}
-
-void console_appender::write(logpacket const & pkt) {
-  layout->format(out_, pkt);
-}
-
-void console_appender::flush() {
-  out_.flush();
-}
+console_appender::console_appender(properties const& props)
+: simple_appender(props), out_(get_ostream(props["target"])) {}
 
 namespace {
   bool _ = appenders::instance().bind("Console", console_appender::create);

src/loggers/file_appender.cxx

@@ -17,25 +17,21 @@
 
 using namespace logging;
 
-namespace logging {
-  level level_from_string(std::string const & value);
-}
-
 struct buffer : std::filebuf {
   buffer(properties const & props);
   buffer * setbuf(char* data, std::streamsize n);
 };
 
-class file_appender : public appender {
+class file_appender : public simple_appender {
 public:
   static std::shared_ptr<appender> create(properties const & props);
   
   explicit file_appender(properties const & props);
   
-  void write(logpacket const & pkt) override;
-  void flush() override;
+  std::ostream & stream() override { return out_; }
+  void write(logpacket const & packet, layout & withLayout) override;
 private:
-  bool flush_immediately_{false};
+  bool flush_immediately_;
   buffer rdbuf_;
   std::ostream out_;
   std::unique_ptr<char[]> buffer_;
@@ -72,7 +68,7 @@ buffer * buffer::setbuf(char* data, std::streamsize n) {
 }
 
 file_appender::file_appender(properties const & props)
-: appender(level_from_string(props["threshold"])),
+: simple_appender(props),
   flush_immediately_(props["immediateFlush"]),
   rdbuf_(props), out_(&rdbuf_) {
   if (props["bufferedIO"]) {
@@ -82,13 +78,11 @@ file_appender::file_appender(properties const & props)
   }
 }
 
-void file_appender::write(logpacket const & pkt) {
-  layout->format(out_, pkt);
+void file_appender::write(logpacket const & packet, layout & withLayout) {
+  withLayout.format(out_, packet);
   if (flush_immediately_) { flush(); }
 }
 
-void file_appender::flush() { out_.flush(); }
-
 namespace {
   bool _ = appenders::instance().bind("File", file_appender::create);
 }

test/c_logger_test.cxx

@@ -31,40 +31,44 @@ TEST_F(CLoggerTest, FlushesOnFlushCall) {
 }
 
 TEST_F(CLoggerTest, LogsWithFmtCode) {
-  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
+  using testing::_;
+  EXPECT_CALL(*appender, write(MessageEq("5"), _)).Times(1);
   t_logger("", pimpl).errorf("%d", 5);
 }
 
 // TODO: This is wrong
 TEST_F(CLoggerTest, FmtLogHasNameInHeader) {
+  using testing::_;
   using testing::Field;
-  EXPECT_CALL(*appender, write(Field(&logpacket::logger, "TEST"))).Times(1);
+  EXPECT_CALL(*appender, write(Field(&logpacket::logger, "TEST"), _)).Times(1);
   t_logger("TEST", pimpl).errorf("%d", 5);
 }
 
 // TODO: This is wrong
 TEST_F(CLoggerTest, FmtLogHasLevelInHeader) {
+  using testing::_;
   using testing::Field;
   auto IsError = Field(&logpacket::level, level::error);
-  EXPECT_CALL(*appender, write(IsError)).Times(1);
+  EXPECT_CALL(*appender, write(IsError, _)).Times(1);
   t_logger("TEST", pimpl).errorf("%d", 5);
 }
 
 TEST_F(CLoggerTest, LogsRawData) {
-  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
+  using testing::_;
+  EXPECT_CALL(*appender, write(MessageEq("5"), _)).Times(1);
   t_logger("", pimpl).error("5");
 }
 
 TEST_F(CLoggerTest, DoesNotLogAboveLevel) {
   using testing::_;
   pimpl->min_log_level = level::fatal;
-  EXPECT_CALL(*appender, write(_)).Times(0);
+  EXPECT_CALL(*appender, write(_, _)).Times(0);
   t_logger("", pimpl).errorf("%d", 5);
 }
 
 TEST_F(CLoggerTest, DoesNotRawLogAboveLevel) {
   using testing::_;
   pimpl->min_log_level = level::fatal;
-  EXPECT_CALL(*appender, write(_)).Times(0);
+  EXPECT_CALL(*appender, write(_, _)).Times(0);
   t_logger("", pimpl).error("5");
 }

test/console_appender_test.cxx

@@ -21,7 +21,8 @@ protected:
   
   std::string cout() const { return cout_->str(); }
   std::string cerr() const { return cerr_->str(); }
-  std::shared_ptr<logging::appender> get(logging::properties const &) const;
+  std::shared_ptr<logging::log_appender>
+  get(logging::properties const &) const;
 private:
   std::shared_ptr<StubLayout> playout;
   std::unique_ptr<scoped_buffer_capture_t> cout_, cerr_;
@@ -39,11 +40,10 @@ void ConsoleAppenderTest::TearDown() {
   playout.reset();
 }
 
-std::shared_ptr<logging::appender>
+std::shared_ptr<logging::log_appender>
 ConsoleAppenderTest::get(logging::properties const & props) const {
   auto pappender = logging::appenders::instance().get("Console", props);
-  pappender->layout = playout;
-  return pappender;
+  return std::make_shared<logging::log_appender>(pappender, playout);
 }
 
 TEST_F(ConsoleAppenderTest, ErrorOnUnknownTarget) {

test/file_appender_test.cxx

@@ -22,7 +22,8 @@ protected:
   void TearDown() override;
   
   std::string filename() const { return data; }
-  std::shared_ptr<logging::appender> get(logging::properties const &) const;
+  std::shared_ptr<logging::log_appender>
+  get(logging::properties const &) const;
 private:
   std::shared_ptr<StubLayout> playout;
   char data[24];
@@ -45,11 +46,10 @@ void FileAppenderTest::TearDown() {
   memset(data, 0, sizeof(data));
 }
 
-std::shared_ptr<logging::appender>
+std::shared_ptr<logging::log_appender>
 FileAppenderTest::get(logging::properties const & props) const {
   auto pappender = logging::appenders::instance().get("File", props);
-  pappender->layout = playout;
-  return pappender;
+  return std::make_shared<logging::log_appender>(pappender, playout);
 }
 
 TEST_F(FileAppenderTest, ThrowsIfNoFilename) {

test/log_manager_test.cxx

@@ -27,9 +27,17 @@ private:
   layouts::scoped_binding lbinding_;
 };
 
+namespace logging {
+  level level_from_string(std::string const & value);
+}
+
 void LogManagerTest::SetUp() {
-  auto GetMock = [this](properties const &) {
-    return appender = std::make_shared<MockAppender>();
+  auto GetMock = [this](properties const & props) {
+    appender = std::make_shared<MockAppender>();
+    if (props.contains("threshold")) {
+      appender->threshold = level_from_string(props["threshold"]);
+    }
+    return appender;
   };
   abinding_ = appenders::instance().bind_scoped("Mock", GetMock);
   auto GetMockLayout = [this](properties const &) {
@@ -64,7 +72,7 @@ TEST_F(LogManagerTest, CanFetchInjectedMock) {
   mgr.configure(MIN_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE")));
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"), _));
 
   c_logger l = mgr.c_get();
   l.error("TEST MESSAGE");
@@ -75,7 +83,7 @@ TEST_F(LogManagerTest, MultiplexMockLogsToMultipleImpls) {
   mgr.configure(MULTIPLEX_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"))).Times(2);
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"), _)).Times(2);
 
   c_logger l = mgr.c_get();
   l.error("TEST MESSAGE");
@@ -86,8 +94,8 @@ TEST_F(LogManagerTest, LevelCanBeBakedIntoAppenderProperties) {
   mgr.configure(APPENDER_LEVEL_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"))).Times(1);
-  EXPECT_CALL(*appender, write(MessageEq("LOWER MESSAGE"))).Times(0);
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"), _)).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("LOWER MESSAGE"), _)).Times(0);
   
   c_logger l = mgr.c_get();
   l.warn("TEST MESSAGE");
@@ -99,7 +107,7 @@ TEST_F(LogManagerTest, LevelCanBeBakedIntoLoggerProperties) {
   mgr.configure(LOGGER_LEVEL_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write(_)).Times(0);
+  EXPECT_CALL(*appender, write(_, _)).Times(0);
   
   c_logger l = mgr.c_get();
   l.warn("TEST MESSAGE");

test/logger_test.cxx

@@ -30,19 +30,19 @@ TEST_F(LoggerTest, FlushesOnFlushCall) {
 
 TEST_F(LoggerTest, LogsWithBraceFmtCode) {
   using testing::_;
-  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("5"), _)).Times(1);
   t_logger("", pimpl).log(level::error, "{}", 5);
 }
 
 TEST_F(LoggerTest, DoesNotLogAboveLevel) {
   using testing::_;
   pimpl->min_log_level = level::fatal;
-  EXPECT_CALL(*appender, write(_)).Times(0);
+  EXPECT_CALL(*appender, write(_, _)).Times(0);
   t_logger("", pimpl).log(level::error, "{}", 5);
 }
 
 TEST_F(LoggerTest, LogCurlyBraceLiteralByDoubling) {
   using testing::_;
-  EXPECT_CALL(*appender, write(MessageEq("{}"))).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("{}"), _)).Times(1);
   t_logger("", pimpl).log(level::error, "{{}}", 5);
 }

test/mock_logger.h

@@ -34,21 +34,24 @@ namespace logging {
   }
 }
 
-struct MockAppender : public logging::appender {
-  MockAppender() { SetLogLevel(logging::level::trace); }
-  void SetLogLevel(logging::level ll) { min_log_level = ll; }
-  MOCK_METHOD0(flush, void());
-  MOCK_METHOD1(write, void(logging::logpacket const &));
-};
-
 struct StubAppender : public logging::appender {
-  StubAppender() { SetLogLevel(logging::level::trace); }
-  void SetLogLevel(logging::level ll) { min_log_level = ll; }
+  StubAppender() : threshold(logging::level::trace) { }
+  std::ostream & stream() override { return sstream; }
   void flush() override {}
-  void write(logging::logpacket const & pkt) override {
-    layout->format(stream, pkt);
+  void write(logging::logpacket const & pkt, logging::layout & lay) override {
+    lay.format(sstream, pkt);
+  }
+  bool should_log(logging::level ll) const override {
+    return ll >= threshold;
   }
-  std::stringstream stream;
+  logging::level threshold;
+  std::stringstream sstream;
+};
+
+struct MockAppender : public StubAppender {
+  MockAppender() { }
+  MOCK_METHOD0(flush, void());
+  MOCK_METHOD2(write, void(logging::logpacket const &, logging::layout &));
 };
 
 struct MockLayout : public logging::layout {
@@ -73,12 +76,14 @@ struct LoggerTest : public testing::Test {
   void SetUp() override {
     appender.reset(new MockAppender);
     pimpl = std::make_shared<logging::logger_impl>();
-    pimpl->impls.push_back(appender);
+    auto layout = std::make_shared<StubLayout>();
+    auto log = std::make_shared<logging::log_appender>(appender, layout);
+    pimpl->impls.push_back(log);
     
     using testing::_;
     using testing::AnyNumber;
 
-    EXPECT_CALL(*appender, write(_)).Times(AnyNumber());
+    EXPECT_CALL(*appender, write(_, _)).Times(AnyNumber());
     EXPECT_CALL(*appender, flush()).Times(AnyNumber());
   }
   void TearDown() override {

test/test_properties.cxx

@@ -36,7 +36,7 @@ properties const APPENDER_LEVEL_PROPERTY_SCHEMA{_obj({
   {"configuration", _obj({
     {"appenders", _obj({
       {"Mock", _obj({
-        {"level", _v("Warning")},
+        {"threshold", _v("Warning")},
         {"MockLayout", _v("")}
       })}
     })},
@@ -54,13 +54,13 @@ properties const LOGGER_LEVEL_PROPERTY_SCHEMA{_obj({
   {"configuration", _obj({
     {"appenders", _obj({
       {"Mock", _obj({
-        {"level", _v("Warning")},
+        {"threshold", _v("Warning")},
         {"MockLayout", _v("")}
       })}
     })},
     {"loggers", _obj({
       {"root", _obj({
-        {"level", _v("Error")},
+        {"threshold", _v("Error")},
         {"appenders", _obj({
           {"ref", _v("Mock")}
         })}