Merge branch 'cleanup'

* cleanup:
  Convert calling pattern on appender/layout pairing, and make it so appender contains the layout information. Old: 	message := format(pkt) 	write(message) -> os << message New: 	write(pkt) -> format(os, pkt) -> os << message
  Fix header search paths by importing submodules and using directives in the project config

.gitmodules

@@ -0,0 +1,9 @@
+[submodule "extern/expect"]
+	path = extern/expect
+	url = freenas:utility/expect.git
+[submodule "extern/resource_factory"]
+	path = extern/resource_factory
+	url = freenas:utility/resource_factory.git
+[submodule "extern/variant"]
+	path = extern/variant
+	url = freenas:utility/variant.git

extern/expect

@@ -0,0 +1 @@
+Subproject commit 5d08b8fcbfc385310666c55b5d070a99fab2c2c0

extern/resource_factory

@@ -0,0 +1 @@
+Subproject commit 1a2b2209461a0bb046e69b183e3ac491495fb1e4

extern/variant

@@ -0,0 +1 @@
+Subproject commit caba555bb2dac91535e786d84ee71a29c93118df

include/logger/detail/appender.h

@@ -3,15 +3,17 @@
 #include "logger/logger_fwd.h"
 
 namespace logging {
+  class layout;
   struct appender {
     virtual ~appender() = default;
-    virtual void write(std::string const & msg) = 0;
+    virtual void write(logpacket const & pkt) = 0;
     virtual void flush() = 0;
     
     bool should_log(level ll) const {
       return ll >= min_log_level;
     }
     
+    std::shared_ptr<layout> layout;
     level min_log_level;
   };
 }

include/logger/detail/layout.h

@@ -7,6 +7,6 @@
 namespace logging {
   struct layout {
     virtual ~layout() = default;
-    virtual std::string format(logpacket const & pkt) const = 0;
+    virtual void format(std::ostream & os, logpacket const & pkt) const = 0;
   };
 }

include/logger/detail/logger_impl.h

@@ -8,17 +8,13 @@
 
 namespace logging {
   class appender;
-  class layout;
 
   struct logger_impl {
-    using p_appender = std::shared_ptr<appender>;
-    using p_layout = std::shared_ptr<layout>;
-
     bool should_log(level ll) const;
     void write(logpacket const & pkt);
     void flush();
 
-    std::vector<std::pair<p_appender, p_layout>> impls;
+    std::vector<std::shared_ptr<appender>> impls;
     level min_log_level;
   };
 }

include/logger/properties.h

@@ -12,7 +12,7 @@
 #include <string>
 #include <vector>
 
-#include "../../../types/variant/variant.hpp"
+#include "variant/variant.hpp"
 
 namespace logging {
   struct properties;

logger.xcodeproj/project.pbxproj

@@ -416,7 +416,7 @@
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				ONLY_ACTIVE_ARCH = YES;
 				SDKROOT = macosx;
-				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/../../paradigm/declarative/expect/include/";
+				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/extern/expect/include/ $(PROJECT_DIR)/extern/resource_factory/include/ $(PROJECT_DIR)/extern/";
 			};
 			name = Debug;
 		};
@@ -450,7 +450,7 @@
 				GCC_WARN_UNUSED_FUNCTION = YES;
 				GCC_WARN_UNUSED_VARIABLE = YES;
 				SDKROOT = macosx;
-				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/../../paradigm/declarative/expect/include/";
+				USER_HEADER_SEARCH_PATHS = "$(PROJECT_DIR)/include/ $(PROJECT_DIR)/extern/expect/include/ $(PROJECT_DIR)/extern/resource_factory/include/ $(PROJECT_DIR)/extern/";
 			};
 			name = Release;
 		};

src/log_manager.cxx

@@ -10,8 +10,8 @@
 #include <memory>
 #include <unordered_map>
 
-#include "../../../paradigm/declarative/expect/include/expect/expect.hpp"
-#include "../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "expect/expect.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "common.h"
 #include "logger/c_logger.h"
@@ -39,7 +39,7 @@ struct logging::manager_impl {
   void configure_loggers(properties const & props);
 
   p_logger default_logger;
-  cache<std::pair<p_appender, p_layout>> appenders;
+  cache<p_appender> appenders;
   cache<p_logger> loggers;
 };
 
@@ -93,10 +93,10 @@ void inject_log_level(properties const & props, T & val) {
 void manager_impl::configure_appenders(properties const & props) {
   // TODO: Support multiple File loggers, etc.
   for (auto & app : props["appenders"].object()) {
-    auto pair = std::make_pair(load_appender(app.first, app.second),
-                               load_layout(app.first, app.second));
-    inject_log_level(app.second, *pair.first);
-    appenders[app.first] = pair;
+    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;
   }
 }
 

src/logger_impl.cxx

@@ -19,15 +19,15 @@ bool logger_impl::should_log(level ll) const {
 
 void logger_impl::write(logpacket const & pkt) {
   if (!should_log(pkt.level)) return;
-  for (auto & pair : impls) {
-    if (pair.first->should_log(pkt.level)) {
-      pair.first->write(pair.second->format(pkt));
+  for (auto & appender : impls) {
+    if (appender->should_log(pkt.level)) {
+      appender->write(pkt);
     }
   }
 }
 
 void logger_impl::flush() {
-  for (auto & pair : impls) {
-    pair.first->flush();
+  for (auto & appender : impls) {
+    appender->flush();
   }
 }

src/loggers/console_appender.cxx

@@ -7,10 +7,11 @@
 
 #include <iostream>
 
-#include "../../../../paradigm/declarative/expect/include/expect/expect.hpp"
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "expect/expect.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/appender.h"
+#include "logger/detail/layout.h"
 #include "logger/log_manager.h"
 #include "logger/properties.h"
 
@@ -22,7 +23,7 @@ public:
   
   explicit console_appender(std::ostream& os);
   
-  void write(std::string const & msg) override;
+  void write(logpacket const & pkt) override;
   
   void flush() override;
 private:
@@ -42,8 +43,8 @@ std::shared_ptr<appender> console_appender::create(properties const& props) {
 
 console_appender::console_appender(std::ostream& os) : out_(os) {}
 
-void console_appender::write(std::string const & msg) {
-  out_ << msg;
+void console_appender::write(logpacket const & pkt) {
+  layout->format(out_, pkt);
 }
 
 void console_appender::flush() {

src/loggers/default_layout.cxx

@@ -5,7 +5,7 @@
 //  Created by Sam Jaffe on 4/3/19.
 //
 
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/layout.h"
 #include "logger/log_manager.h"
@@ -15,15 +15,15 @@ using namespace logging;
 
 struct default_layout : public layout {
   static std::shared_ptr<layout> create(properties const &);
-  std::string format(logpacket const & pkt) const override;
+  void format(std::ostream & os, logpacket const & pkt) const override;
 };
 
 std::shared_ptr<layout> default_layout::create(properties const &) {
   return std::make_shared<default_layout>();
 }
 
-std::string default_layout::format(logpacket const & pkt) const {
-  return pkt.message.str() + "\n";
+void default_layout::format(std::ostream & os, logpacket const & pkt) const {
+  os << pkt.message.str() << "\n";
 }
 
 namespace {

src/loggers/file_appender.cxx

@@ -7,10 +7,11 @@
 
 #include <fstream>
 
-#include "../../../../paradigm/declarative/expect/include/expect/expect.hpp"
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "expect/expect.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/appender.h"
+#include "logger/detail/layout.h"
 #include "logger/log_manager.h"
 #include "logger/properties.h"
 
@@ -26,7 +27,7 @@ public:
   
   explicit file_appender(const std::string& fname, bool append);
   
-  void write(std::string const & msg) override;
+  void write(logpacket const & pkt) override;
   void flush() override;
 private:
   bool flush_immediately_{false};
@@ -58,8 +59,8 @@ static std::ios_base::openmode mode(bool append) {
 file_appender::file_appender(const std::string& fname, bool append)
 : out_(fname, mode(append)) {}
 
-void file_appender::write(std::string const & msg) {
-  out_ << msg;
+void file_appender::write(logpacket const & pkt) {
+  layout->format(out_, pkt);
   if (flush_immediately_) { flush(); }
 }
 

src/loggers/pattern_layout.cxx

@@ -5,7 +5,7 @@
 //  Created by Sam Jaffe on 4/4/19.
 //
 
-#include "../../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/detail/layout.h"
 #include "logger/format.h"
@@ -18,7 +18,7 @@ struct pattern_layout : public layout {
   static std::shared_ptr<layout> create(properties const &);
   
   pattern_layout(std::string const & fmt);
-  std::string format(logpacket const & pkt) const override;
+  void format(std::ostream & os, logpacket const & pkt) const override;
   
   class format formatter;
 };
@@ -30,8 +30,8 @@ std::shared_ptr<layout> pattern_layout::create(properties const & props) {
 pattern_layout::pattern_layout(std::string const & fmt) :
   formatter(format::parse_format_string(fmt)) {}
 
-std::string pattern_layout::format(logpacket const & pkt) const {
-  return formatter.process(pkt);
+void pattern_layout::format(std::ostream & os, logpacket const & pkt) const {
+  os << formatter.process(pkt);
 }
 
 namespace {

src/loggers/properties.cxx

@@ -7,7 +7,7 @@
 
 #include "logger/properties.h"
 
-#include "../../../../paradigm/declarative/expect/include/expect/expect.hpp"
+#include "expect/expect.hpp"
 
 #include "logger/exception.h"
 

test/c_logger_test.cxx

@@ -31,30 +31,27 @@ TEST_F(CLoggerTest, FlushesOnFlushCall) {
 }
 
 TEST_F(CLoggerTest, LogsWithFmtCode) {
-  using testing::_;
-  EXPECT_CALL(*layout, format(MessageEq("5"))).Times(1);
+  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(*layout, format(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;
-  EXPECT_CALL(*layout, format(Field(&logpacket::level, level::error))).Times(1);
+  auto IsError = Field(&logpacket::level, level::error);
+  EXPECT_CALL(*appender, write(IsError)).Times(1);
   t_logger("TEST", pimpl).errorf("%d", 5);
 }
 
 TEST_F(CLoggerTest, LogsRawData) {
-  using testing::_;
-  EXPECT_CALL(*layout, format(MessageEq("5"))).Times(1);
+  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
   t_logger("", pimpl).error("5");
 }
 

test/log_manager_test.cxx

@@ -5,7 +5,7 @@
 //  Created by Sam Jaffe on 4/2/19.
 //
 
-#include "../../../types/resource_factory/include/resource_factory/prototype_factory.hpp"
+#include "resource_factory/prototype_factory.hpp"
 
 #include "logger/c_logger.h"
 #include "logger/log_manager.h"
@@ -64,8 +64,7 @@ TEST_F(LogManagerTest, CanFetchInjectedMock) {
   mgr.configure(MIN_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE"));
-  EXPECT_CALL(*layout, format(_)).WillRepeatedly(ReturnMessage());
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE")));
 
   c_logger l = mgr.c_get();
   l.error("TEST MESSAGE");
@@ -76,8 +75,7 @@ TEST_F(LogManagerTest, MultiplexMockLogsToMultipleImpls) {
   mgr.configure(MULTIPLEX_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE")).Times(2);
-  EXPECT_CALL(*layout, format(_)).WillRepeatedly(ReturnMessage());
+  EXPECT_CALL(*appender, write(MessageEq("TEST MESSAGE"))).Times(2);
 
   c_logger l = mgr.c_get();
   l.error("TEST MESSAGE");
@@ -88,9 +86,8 @@ TEST_F(LogManagerTest, LevelCanBeBakedIntoAppenderProperties) {
   mgr.configure(APPENDER_LEVEL_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE")).Times(1);
-  EXPECT_CALL(*appender, write("LOWER MESSAGE")).Times(0);
-  EXPECT_CALL(*layout, format(_)).WillRepeatedly(ReturnMessage());
+  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");
@@ -102,8 +99,7 @@ TEST_F(LogManagerTest, LevelCanBeBakedIntoLoggerProperties) {
   mgr.configure(LOGGER_LEVEL_PROPERTY_SCHEMA);
   
   EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-  EXPECT_CALL(*appender, write("TEST MESSAGE")).Times(0);
-  EXPECT_CALL(*layout, format(_)).Times(0);
+  EXPECT_CALL(*appender, write(_)).Times(0);
   
   c_logger l = mgr.c_get();
   l.warn("TEST MESSAGE");

test/logger_test.cxx

@@ -29,7 +29,8 @@ TEST_F(LoggerTest, FlushesOnFlushCall) {
 }
 
 TEST_F(LoggerTest, LogsWithBraceFmtCode) {
-  EXPECT_CALL(*layout, format(MessageEq("5"))).Times(1);
+  using testing::_;
+  EXPECT_CALL(*appender, write(MessageEq("5"))).Times(1);
   t_logger("", pimpl).log(level::error, "{}", 5);
 }
 
@@ -41,6 +42,7 @@ TEST_F(LoggerTest, DoesNotLogAboveLevel) {
 }
 
 TEST_F(LoggerTest, LogCurlyBraceLiteralByDoubling) {
-  EXPECT_CALL(*layout, format(MessageEq("{}"))).Times(1);
+  using testing::_;
+  EXPECT_CALL(*appender, write(MessageEq("{}"))).Times(1);
   t_logger("", pimpl).log(level::error, "{{}}", 5);
 }

test/mock_logger.h

@@ -10,6 +10,8 @@
 
 #include <gmock/gmock.h>
 
+#include <sstream>
+
 #include "logger/detail/appender.h"
 #include "logger/detail/layout.h"
 #include "logger/detail/logger_impl.h"
@@ -36,15 +38,25 @@ 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(std::string const &));
+  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; }
+  void flush() override {}
+  void write(logging::logpacket const & pkt) override {
+    layout->format(stream, pkt);
+  }
+  std::stringstream stream;
 };
 
 struct MockLayout : public logging::layout {
-  MOCK_CONST_METHOD1(format, std::string(logging::logpacket const &));
+  MOCK_CONST_METHOD2(format, void(std::ostream&, logging::logpacket const&));
 };
 
-ACTION(ReturnMessage) {
-  return arg0.message.str();
+ACTION(LogMessage) {
+  arg0 << arg1.message.str();
 }
 
 MATCHER_P(MessageEq, value, "") {
@@ -54,25 +66,21 @@ MATCHER_P(MessageEq, value, "") {
 struct LoggerTest : public testing::Test {
   void SetUp() override {
     appender.reset(new MockAppender);
-    layout.reset(new MockLayout);
     pimpl = std::make_shared<logging::logger_impl>();
-    pimpl->impls.push_back({appender, layout});
+    pimpl->impls.push_back(appender);
     
     using testing::_;
     using testing::AnyNumber;
 
     EXPECT_CALL(*appender, write(_)).Times(AnyNumber());
     EXPECT_CALL(*appender, flush()).Times(AnyNumber());
-    ON_CALL(*layout, format(_)).WillByDefault(ReturnMessage());
   }
   void TearDown() override {
     pimpl.reset();
-    layout.reset();
     appender.reset();
   }
   
   std::shared_ptr<MockAppender> appender;
-  std::shared_ptr<MockLayout> layout;
   std::shared_ptr<logging::logger_impl> pimpl;
 };