Implement fixes indicated by pull request comments
* Fix misplaced comment
* Add guard clauses to remove unnecessary if blocks
* Remove unnecessary return statement
* Move LocationInfo serialization to separate function
* Add "pretty-print" option.
diff --git a/src/main/cpp/jsonlayout.cpp b/src/main/cpp/jsonlayout.cpp
index 60bf5ef..66ff09f 100644
--- a/src/main/cpp/jsonlayout.cpp
+++ b/src/main/cpp/jsonlayout.cpp
@@ -34,7 +34,12 @@
IMPLEMENT_LOG4CXX_OBJECT(JSONLayout)
-JSONLayout::JSONLayout() : locationInfo(false), dateFormat()
+JSONLayout::JSONLayout() :
+ locationInfo(false),
+ prettyPrint(false),
+ dateFormat(),
+ ppIndentL1(" "),
+ ppIndentL2(" ")
{
}
@@ -46,32 +51,62 @@
{
setLocationInfo(OptionConverter::toBoolean(value, false));
}
+
+ if (StringHelper::equalsIgnoreCase(option,
+ LOG4CXX_STR("PRETTYPRINT"), LOG4CXX_STR("prettyprint")))
+ {
+ setPrettyPrint(OptionConverter::toBoolean(value, false));
+ }
}
void JSONLayout::format(LogString& output,
const spi::LoggingEventPtr& event,
Pool& p) const
{
- output.append("{ ");
+ output.append("{");
+ output.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ output.append(ppIndentL1);
+ }
appendQuotedEscapedString(output, "timestamp");
output.append(": ");
LogString timestamp;
dateFormat.format(timestamp, event->getTimeStamp(), p);
appendQuotedEscapedString(output, timestamp);
- output.append(", ");
+ output.append(",");
+ output.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ output.append(ppIndentL1);
+ }
appendQuotedEscapedString(output, "level");
output.append(": ");
LogString level;
event->getLevel()->toString(level);
appendQuotedEscapedString(output, level);
- output.append(", ");
+ output.append(",");
+ output.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ output.append(ppIndentL1);
+ }
appendQuotedEscapedString(output, "logger");
output.append(": ");
appendQuotedEscapedString(output, event->getLoggerName());
- output.append(", ");
+ output.append(",");
+ output.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ output.append(ppIndentL1);
+ }
appendQuotedEscapedString(output, "message");
output.append(": ");
@@ -84,37 +119,13 @@
if (locationInfo)
{
- output.append(", ");
- appendQuotedEscapedString(output, "location_info");
- output.append(": { ");
- const LocationInfo& locInfo = event->getLocationInformation();
-
- appendQuotedEscapedString(output, "file");
- output.append(": ");
- LOG4CXX_DECODE_CHAR(fileName, locInfo.getFileName());
- appendQuotedEscapedString(output, fileName);
- output.append(", ");
-
- appendQuotedEscapedString(output, "line");
- output.append(": ");
- LogString lineNumber;
- StringHelper::toString(locInfo.getLineNumber(), p, lineNumber);
- appendQuotedEscapedString(output, lineNumber);
- output.append(", ");
-
- appendQuotedEscapedString(output, "class");
- output.append(": ");
- appendQuotedEscapedString(output, locInfo.getClassName());
- output.append(", ");
-
- appendQuotedEscapedString(output, "method");
- output.append(": ");
- appendQuotedEscapedString(output, locInfo.getMethodName());
-
- output.append(" } ");
+ output.append(",");
+ output.append(prettyPrint ? "\n" : " ");
+ appendSerializedLocationInfo(output, event, p);
}
- output.append(" }");
+ output.append(prettyPrint ? "\n" : " ");
+ output.append("}");
output.append(LOG4CXX_EOL);
}
@@ -133,8 +144,8 @@
0x0c, /* \f form feed */
0x0d, /* \r carriage return */
0x22, /* \" double quote */
- 0x5c
- }; /* \\ backslash */
+ 0x5c /* \\ backslash */
+ };
size_t start = 0;
@@ -216,8 +227,6 @@
/* add trailing quote */
buf.push_back(0x22);
- return;
-
}
void JSONLayout::appendSerializedMDC(LogString& buf,
@@ -226,32 +235,55 @@
LoggingEvent::KeySet keys = event->getMDCKeySet();
- if (!keys.empty())
+ if (keys.empty())
{
- buf.append(", ");
- appendQuotedEscapedString(buf, "context_map");
- buf.append(": { ");
-
- for (LoggingEvent::KeySet::iterator it = keys.begin();
- it != keys.end(); ++it)
- {
- appendQuotedEscapedString(buf, *it);
- buf.append(": ");
- LogString value;
- event->getMDC(*it, value);
- appendQuotedEscapedString(buf, value);
-
- /* if this isn't the last k:v pair, we need a comma */
- if (it + 1 != keys.end())
- {
- buf.append(", ");
- }
- }
-
- buf.append(" }");
+ return;
}
- return;
+ buf.append(",");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL1);
+ }
+
+ appendQuotedEscapedString(buf, "context_map");
+ buf.append(": {");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ for (LoggingEvent::KeySet::iterator it = keys.begin();
+ it != keys.end(); ++it)
+ {
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL2);
+ }
+
+ appendQuotedEscapedString(buf, *it);
+ buf.append(": ");
+ LogString value;
+ event->getMDC(*it, value);
+ appendQuotedEscapedString(buf, value);
+
+ /* if this isn't the last k:v pair, we need a comma */
+ if (it + 1 != keys.end())
+ {
+ buf.append(",");
+ buf.append(prettyPrint ? "\n" : " ");
+ }
+ else
+ {
+ buf.append(prettyPrint ? "\n" : " ");
+ }
+ }
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL1);
+ }
+
+ buf.append("}");
}
@@ -261,13 +293,106 @@
LogString ndcVal;
- if (event->getNDC(ndcVal))
+ if (!event->getNDC(ndcVal))
{
- buf.append(", ");
- appendQuotedEscapedString(buf, "context_stack");
- buf.append(": [ ");
- appendQuotedEscapedString(buf, ndcVal);
- buf.append(" ]");
+ return;
}
+
+ buf.append(",");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL1);
+ }
+
+ appendQuotedEscapedString(buf, "context_stack");
+ buf.append(": [");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL2);
+ }
+
+ appendQuotedEscapedString(buf, ndcVal);
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL1);
+ }
+
+ buf.append("]");
+
+}
+
+void JSONLayout::appendSerializedLocationInfo(LogString& buf,
+ const LoggingEventPtr& event, Pool& p) const
+{
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL1);
+ }
+
+ appendQuotedEscapedString(buf, "location_info");
+ buf.append(": {");
+ buf.append(prettyPrint ? "\n" : " ");
+ const LocationInfo& locInfo = event->getLocationInformation();
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL2);
+ }
+
+ appendQuotedEscapedString(buf, "file");
+ buf.append(": ");
+ LOG4CXX_DECODE_CHAR(fileName, locInfo.getFileName());
+ appendQuotedEscapedString(buf, fileName);
+ buf.append(",");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL2);
+ }
+
+ appendQuotedEscapedString(buf, "line");
+ buf.append(": ");
+ LogString lineNumber;
+ StringHelper::toString(locInfo.getLineNumber(), p, lineNumber);
+ appendQuotedEscapedString(buf, lineNumber);
+ buf.append(",");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL2);
+ }
+
+ appendQuotedEscapedString(buf, "class");
+ buf.append(": ");
+ appendQuotedEscapedString(buf, locInfo.getClassName());
+ buf.append(",");
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL2);
+ }
+
+ appendQuotedEscapedString(buf, "method");
+ buf.append(": ");
+ appendQuotedEscapedString(buf, locInfo.getMethodName());
+ buf.append(prettyPrint ? "\n" : " ");
+
+ if (prettyPrint)
+ {
+ buf.append(ppIndentL1);
+ }
+
+ buf.append("}");
+
}
diff --git a/src/main/include/log4cxx/jsonlayout.h b/src/main/include/log4cxx/jsonlayout.h
index c34725c..dd997d1 100644
--- a/src/main/include/log4cxx/jsonlayout.h
+++ b/src/main/include/log4cxx/jsonlayout.h
@@ -34,16 +34,22 @@
private:
// Print no location info by default
bool locationInfo; //= false
+ bool prettyPrint; //= false
helpers::ISO8601DateFormat dateFormat;
protected:
+ LogString ppIndentL1;
+ LogString ppIndentL2;
+
void appendQuotedEscapedString(LogString& buf, const LogString& input) const;
void appendSerializedMDC(LogString& buf,
const spi::LoggingEventPtr& event) const;
void appendSerializedNDC(LogString& buf,
const spi::LoggingEventPtr& event) const;
+ void appendSerializedLocationInfo(LogString& buf,
+ const spi::LoggingEventPtr& event, log4cxx::helpers::Pool& p) const;
public:
DECLARE_LOG4CXX_OBJECT(JSONLayout)
@@ -66,6 +72,7 @@
this->locationInfo = locationInfoFlag;
}
+
/**
Returns the current value of the <b>LocationInfo</b> option.
*/
@@ -75,6 +82,27 @@
}
/**
+ The <b>PrettyPrint</b> option takes a boolean value. By
+ default, it is set to false which means output by this layout will
+ be one line per log event. If the option is set to true, then
+ then each log event will produce multiple lines, each indented
+ for readability.
+ */
+ inline void setPrettyPrint(bool prettyPrintFlag)
+ {
+ this->prettyPrint = prettyPrintFlag;
+ }
+
+ /**
+ Returns the current value of the <b>PrettyPrint</b> option.
+ */
+ inline bool getPrettyPrint() const
+ {
+ return prettyPrint;
+ }
+
+
+ /**
Returns the content type output by this layout, i.e "application/json".
*/
virtual LogString getContentType() const
diff --git a/src/test/cpp/jsonlayouttest.cpp b/src/test/cpp/jsonlayouttest.cpp
index 239878e..88862da 100644
--- a/src/test/cpp/jsonlayouttest.cpp
+++ b/src/test/cpp/jsonlayouttest.cpp
@@ -47,8 +47,13 @@
LOGUNIT_TEST(testAppendQuotedEscapedStringWithPrintableChars);
LOGUNIT_TEST(testAppendQuotedEscapedStringWithControlChars);
LOGUNIT_TEST(testAppendSerializedMDC);
+ LOGUNIT_TEST(testAppendSerializedMDCWithPrettyPrint);
LOGUNIT_TEST(testAppendSerializedNDC);
+ LOGUNIT_TEST(testAppendSerializedNDCWithPrettyPrint);
+ LOGUNIT_TEST(testAppendSerializedLocationInfo);
+ LOGUNIT_TEST(testAppendSerializedLocationInfoWithPrettyPrint);
LOGUNIT_TEST(testFormat);
+ LOGUNIT_TEST(testFormatWithPrettyPrint);
LOGUNIT_TEST(testGetSetLocationInfo);
LOGUNIT_TEST_SUITE_END();
@@ -178,6 +183,41 @@
}
+ /**
+ * Tests appendSerializedMDC with prettyPrint set to true.
+ */
+ void testAppendSerializedMDCWithPrettyPrint()
+ {
+
+ LoggingEventPtr event1 = new LoggingEvent(LOG4CXX_STR("Logger"),
+ Level::getInfo(),
+ LOG4CXX_STR("A message goes here."),
+ LOG4CXX_LOCATION);
+
+ MDC::put("key1", "value1");
+ MDC::put("key2", "value2");
+
+ LogString output1;
+
+ LogString expected1;
+ expected1.append(",\n")
+ .append(ppIndentL1)
+ .append("\"context_map\": {\n")
+ .append(ppIndentL2)
+ .append("\"key1\": \"value1\",\n")
+ .append(ppIndentL2)
+ .append("\"key2\": \"value2\"\n")
+ .append(ppIndentL1)
+ .append("}");
+
+ setPrettyPrint(true);
+ appendSerializedMDC(output1, event1);
+
+ LOGUNIT_ASSERT_EQUAL(expected1, output1);
+
+ }
+
+
/**
* Tests appendSerializedNDC.
@@ -201,9 +241,104 @@
LOGUNIT_ASSERT_EQUAL(expected1, output1);
+ }
+
+
+ /**
+ * Tests appendSerializedNDC with prettyPrint set to true.
+ */
+ void testAppendSerializedNDCWithPrettyPrint()
+ {
+
+ LoggingEventPtr event1 = new LoggingEvent(LOG4CXX_STR("Logger"),
+ Level::getInfo(),
+ LOG4CXX_STR("A message goes here."),
+ LOG4CXX_LOCATION);
+
+ NDC::push("one");
+ NDC::push("two");
+ NDC::push("three");
+
+ LogString output1;
+ LogString expected1;
+ expected1.append(",\n")
+ .append(ppIndentL1)
+ .append("\"context_stack\": [\n")
+ .append(ppIndentL2)
+ .append("\"one two three\"\n")
+ .append(ppIndentL1)
+ .append("]");
+
+ setPrettyPrint(true);
+
+ appendSerializedNDC(output1, event1);
+
+ LOGUNIT_ASSERT_EQUAL(expected1, output1);
}
+ /**
+ * Tests appendSerializedLocationInfo.
+ */
+ void testAppendSerializedLocationInfo()
+ {
+ Pool p;
+
+ LoggingEventPtr event1 = new LoggingEvent(LOG4CXX_STR("Logger"),
+ Level::getInfo(),
+ LOG4CXX_STR("A message goes here."),
+ spi::LocationInfo("FooFile", "BarFunc", 42));
+
+ LogString output1;
+ LogString expected1;
+ expected1.append("\"location_info\": { ")
+ .append("\"file\": \"FooFile\", ")
+ .append("\"line\": \"42\", ")
+ .append("\"class\": \"\", ")
+ .append("\"method\": \"BarFunc\" }");
+
+ appendSerializedLocationInfo(output1, event1, p);
+
+ LOGUNIT_ASSERT_EQUAL(expected1, output1);
+
+ }
+
+ /**
+ * Tests appendSerializedLocationInfo with prettyPrint set to true.
+ */
+ void testAppendSerializedLocationInfoWithPrettyPrint()
+ {
+
+ Pool p;
+
+ LoggingEventPtr event1 = new LoggingEvent(LOG4CXX_STR("Logger"),
+ Level::getInfo(),
+ LOG4CXX_STR("A message goes here."),
+ spi::LocationInfo("FooFile", "BarFunc", 42));
+
+ LogString output1;
+ LogString expected1;
+ expected1.append(ppIndentL1)
+ .append("\"location_info\": {\n")
+ .append(ppIndentL2)
+ .append("\"file\": \"FooFile\",\n")
+ .append(ppIndentL2)
+ .append("\"line\": \"42\",\n")
+ .append(ppIndentL2)
+ .append("\"class\": \"\",\n")
+ .append(ppIndentL2)
+ .append("\"method\": \"BarFunc\"\n")
+ .append(ppIndentL1)
+ .append("}");
+
+ setPrettyPrint(true);
+ appendSerializedLocationInfo(output1, event1, p);
+
+ LOGUNIT_ASSERT_EQUAL(expected1, output1);
+
+ }
+
+
/**
* Tests format.
@@ -216,7 +351,7 @@
LoggingEventPtr event1 = new LoggingEvent(LOG4CXX_STR("Logger"),
Level::getInfo(),
LOG4CXX_STR("A message goes here."),
- LOG4CXX_LOCATION);
+ spi::LocationInfo("FooFile", "BarFunc", 42));
LogString timestamp;
helpers::ISO8601DateFormat dateFormat;
@@ -239,8 +374,12 @@
.append("\"logger\": \"Logger\", ")
.append("\"message\": \"A message goes here.\"");
+ setLocationInfo(true);
+
appendSerializedMDC(expected1, event1);
appendSerializedNDC(expected1, event1);
+ expected1.append(", ");
+ appendSerializedLocationInfo(expected1, event1, p);
expected1.append(" }\n");
@@ -249,6 +388,62 @@
LOGUNIT_ASSERT_EQUAL(expected1, output1);
}
+ /**
+ * Tests format with PrettyPrint set to true.
+ */
+ void testFormatWithPrettyPrint()
+ {
+
+ Pool p;
+
+ LoggingEventPtr event1 = new LoggingEvent(LOG4CXX_STR("Logger"),
+ Level::getInfo(),
+ LOG4CXX_STR("A message goes here."),
+ spi::LocationInfo("FooFile", "BarFunc", 42));
+
+ LogString timestamp;
+ helpers::ISO8601DateFormat dateFormat;
+ dateFormat.format(timestamp, event1->getTimeStamp(), p);
+
+ NDC::push("one");
+ NDC::push("two");
+ NDC::push("three");
+
+ MDC::put("key1", "value1");
+ MDC::put("key2", "value2");
+
+ LogString output1;
+ LogString expected1;
+
+ expected1.append("{\n")
+ .append(ppIndentL1)
+ .append("\"timestamp\": \"")
+ .append(timestamp)
+ .append("\",\n")
+ .append(ppIndentL1)
+ .append("\"level\": \"INFO\",\n")
+ .append(ppIndentL1)
+ .append("\"logger\": \"Logger\",\n")
+ .append(ppIndentL1)
+ .append("\"message\": \"A message goes here.\"");
+
+
+ setPrettyPrint(true);
+ setLocationInfo(true);
+
+ appendSerializedMDC(expected1, event1);
+ appendSerializedNDC(expected1, event1);
+ expected1.append(",\n");
+ appendSerializedLocationInfo(expected1, event1, p);
+
+ expected1.append("\n}\n");
+
+ format(output1, event1, p);
+
+ LOGUNIT_ASSERT_EQUAL(expected1, output1);
+
+ }
+
/**
* Tests getLocationInfo and setLocationInfo.
@@ -263,6 +458,19 @@
LOGUNIT_ASSERT_EQUAL(false, layout.getLocationInfo());
}
+ /**
+ * Tests getPrettyPrint and setPrettyPrint.
+ */
+ void testGetSetPrettyPrint()
+ {
+ JSONLayout layout;
+ LOGUNIT_ASSERT_EQUAL(false, layout.getPrettyPrint());
+ layout.setPrettyPrint(true);
+ LOGUNIT_ASSERT_EQUAL(true, layout.getPrettyPrint());
+ layout.setPrettyPrint(false);
+ LOGUNIT_ASSERT_EQUAL(false, layout.getPrettyPrint());
+ }
+
};
