Merge pull request #126 from SAP/issue/124

Fix invalid JSON messages

Author: KarstenSchnitter

cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverter.java

@@ -51,10 +51,11 @@ public void format(LogEvent event, StringBuilder toAppendTo) {
 		addCustomFieldsFromArguments(contextData, event);
         int lengthBefore = toAppendTo.length();
 		converter.convert(toAppendTo, contextData);
-        // append comma only, if at least one field was added
-        // otherwise, there will be to commas ",," rendering the JSON invalid
-        if (toAppendTo.length() > lengthBefore) {
-            toAppendTo.append(",");
+        // remove comma from pattern, when no properties are added
+        // this is to avoid a double comma in the JSON
+        // Do not do this on empty messages
+        if (toAppendTo.length() == lengthBefore && lengthBefore > 0 && toAppendTo.charAt(lengthBefore - 1) == ',') {
+            toAppendTo.setLength(lengthBefore - 1);
         }
 	}
 

cf-java-logging-support-log4j2/src/main/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilder.java

@@ -56,6 +56,7 @@ public LayoutPatternBuilder addContextProperties(List<String> exclusions) {
 		sb.append("%").append(ContextPropsConverter.WORD);
         appendParameters(asList(Boolean.toString(sendDefaultValues)));
 		appendParameters(exclusions);
+        sb.append(",");
 		return this;
 	}
 

cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/converter/ContextPropsConverterTest.java

@@ -9,8 +9,6 @@
 import java.io.IOException;
 import java.util.Map;
 
-import org.apache.logging.log4j.core.LogEvent;
-import org.apache.logging.log4j.core.pattern.LogEventPatternConverter;
 import org.junit.Test;
 import org.slf4j.MDC;
 
@@ -107,10 +105,4 @@ public void testRegisteredCustomField() throws Exception {
 				not(hasEntry(SOME_KEY, SOME_VALUE)));
 	}
 
-    @Override
-    protected String format(LogEventPatternConverter cpc, LogEvent event) {
-        String converted = super.format(cpc, event);
-        return converted.length() > 0 ? converted.substring(0, converted.length() - 1) : converted;
-    }
-
 }

cf-java-logging-support-log4j2/src/test/java/com/sap/hcp/cf/log4j2/layout/LayoutPatternBuilderTest.java

@@ -107,7 +107,7 @@ public void applicationScenario() throws Exception {
 				.suppressExceptions().build();
 
 		assertThat(pattern, specificPart(is(
-                                            ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field}\"#cf\":{%cf{custom-field}}%ex{0} ")));
+                                            ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field},\"#cf\":{%cf{custom-field}}%ex{0} ")));
 	}
 
 	@Test
@@ -117,7 +117,7 @@ public void exceptionScenario() throws Exception {
 				.build();
 
 		assertThat(pattern, specificPart(is(
-                                            ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field}\"#cf\":{%cf{custom-field}},\"stacktrace\":%stacktrace")));
+                                            ",\"type\":\"log\",\"logger\":\"%replace{%logger}{\"}{\\\\\"}\",\"thread\":\"%replace{%thread}{\"}{\\\\\"}\",\"level\":\"%p\",\"categories\":%categories,\"msg\":%jsonmsg{escape},%ctxp{false}{excluded-field},\"#cf\":{%cf{custom-field}},\"stacktrace\":%stacktrace")));
 	}
 
 	private static Matcher<String> specificPart(Matcher<String> expected) {