Why is this fixed in AbstactStringLayout, with ripple effect to JSON and YAML layouts?
The problem seems to be in Serializer. It is still unclear to me why Serializer needs a LogEvent, (away from PC now) but I assume it is because of StrSubstitutor. I would like it much better if StrSubstitutor could be fixed so that a LogEvent becomes optional, or, if that is not feasible, Sent from my iPhone > On 2016/08/04, at 7:03, ggreg...@apache.org wrote: > > Repository: logging-log4j2 > Updated Branches: > refs/heads/master 0f1b0dc00 -> ffc6c8f68 > > > [LOG4J2-1482] Improper header in CsvParameterLayout. > > Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo > Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/ffc6c8f6 > Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/ffc6c8f6 > Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/ffc6c8f6 > > Branch: refs/heads/master > Commit: ffc6c8f68d260e8e14b140f0f05cbc77081efc2c > Parents: 0f1b0dc > Author: Gary Gregory <ggreg...@apache.org> > Authored: Wed Aug 3 15:03:26 2016 -0700 > Committer: Gary Gregory <ggreg...@apache.org> > Committed: Wed Aug 3 15:03:26 2016 -0700 > > ---------------------------------------------------------------------- > .../log4j/core/impl/DefaultLogEventFactory.java | 5 ++ > .../log4j/core/layout/AbstractStringLayout.java | 14 +-- > .../logging/log4j/core/layout/JsonLayout.java | 5 +- > .../logging/log4j/core/layout/YamlLayout.java | 5 +- > .../log4j/core/layout/Log4j2_1482_CoreTest.java | 20 +++++ > .../log4j/core/layout/Log4j2_1482_Test.java | 89 ++++++++++++++++++++ > log4j-core/src/test/resources/log4j2-1482.xml | 27 ++++++ > log4j-slf4j-impl/pom.xml | 5 ++ > .../logging/slf4j/Log4j2_1482_Slf4jTest.java | 41 +++++++++ > .../src/test/resources/log4j2-1482.xml | 27 ++++++ > src/changes/changes.xml | 3 + > 11 files changed, 231 insertions(+), 10 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java > > b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java > index ef74c50..127b02a 100644 > --- > a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java > +++ > b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/DefaultLogEventFactory.java > @@ -29,6 +29,11 @@ import org.apache.logging.log4j.message.Message; > */ > public class DefaultLogEventFactory implements LogEventFactory { > > + private static final DefaultLogEventFactory instance = new > DefaultLogEventFactory(); > + > + public static DefaultLogEventFactory getInstance() { > + return instance; > + } > > /** > * Creates a log event. > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java > > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java > index 9e6270e..5ac98e7 100644 > --- > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java > +++ > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java > @@ -20,6 +20,8 @@ import org.apache.logging.log4j.core.LogEvent; > import org.apache.logging.log4j.core.StringLayout; > import org.apache.logging.log4j.core.config.Configuration; > import org.apache.logging.log4j.core.config.LoggerConfig; > +import org.apache.logging.log4j.core.impl.DefaultLogEventFactory; > +import org.apache.logging.log4j.core.impl.LogEventFactory; > import org.apache.logging.log4j.core.util.Constants; > import org.apache.logging.log4j.core.util.StringEncoder; > import org.apache.logging.log4j.util.PropertiesUtil; > @@ -202,7 +204,7 @@ public abstract class AbstractStringLayout extends > AbstractLayout<String> implem > */ > @Override > public byte[] getFooter() { > - return serializeToBytes(footerSerializer, super.getFooter()); > + return serializeToBytes(footerSerializer, super.getFooter(), > DefaultLogEventFactory.getInstance()); > } > > public Serializer getFooterSerializer() { > @@ -216,28 +218,28 @@ public abstract class AbstractStringLayout extends > AbstractLayout<String> implem > */ > @Override > public byte[] getHeader() { > - return serializeToBytes(headerSerializer, super.getHeader()); > + return serializeToBytes(headerSerializer, super.getHeader(), > DefaultLogEventFactory.getInstance()); > } > > public Serializer getHeaderSerializer() { > return headerSerializer; > } > > - protected byte[] serializeToBytes(final Serializer serializer, final > byte[] defaultValue) { > - final String serializable = serializeToString(serializer); > + protected byte[] serializeToBytes(final Serializer serializer, final > byte[] defaultValue, final LogEventFactory logEventFactory) { > + final String serializable = serializeToString(serializer, > logEventFactory); > if (serializer == null) { > return defaultValue; > } > return StringEncoder.toBytes(serializable, getCharset()); > } > > - protected String serializeToString(final Serializer serializer) { > + protected String serializeToString(final Serializer serializer, final > LogEventFactory logEventFactory) { > if (serializer == null) { > return null; > } > final LoggerConfig rootLogger = getConfiguration().getRootLogger(); > // Using "" for the FQCN, does it matter? > - final LogEvent logEvent = > rootLogger.getLogEventFactory().createEvent(rootLogger.getName(), null, > Strings.EMPTY, > + final LogEvent logEvent = > logEventFactory.createEvent(rootLogger.getName(), null, Strings.EMPTY, > rootLogger.getLevel(), null, null, null); > return serializer.toSerializable(logEvent); > } > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java > > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java > index e9d87ae..239a59f 100644 > --- > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java > +++ > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/JsonLayout.java > @@ -32,6 +32,7 @@ import org.apache.logging.log4j.core.config.plugins.Plugin; > import org.apache.logging.log4j.core.config.plugins.PluginAttribute; > import org.apache.logging.log4j.core.config.plugins.PluginConfiguration; > import org.apache.logging.log4j.core.config.plugins.PluginFactory; > +import org.apache.logging.log4j.core.impl.DefaultLogEventFactory; > > /** > * Appends a series of JSON events as strings serialized as bytes. > @@ -828,7 +829,7 @@ public final class JsonLayout extends > AbstractJacksonLayout { > return null; > } > final StringBuilder buf = new StringBuilder(); > - final String str = serializeToString(getHeaderSerializer()); > + final String str = serializeToString(getHeaderSerializer(), > DefaultLogEventFactory.getInstance()); > if (str != null) { > buf.append(str); > } > @@ -848,7 +849,7 @@ public final class JsonLayout extends > AbstractJacksonLayout { > } > final StringBuilder buf = new StringBuilder(); > buf.append(this.eol); > - final String str = serializeToString(getFooterSerializer()); > + final String str = serializeToString(getFooterSerializer(), > DefaultLogEventFactory.getInstance()); > if (str != null) { > buf.append(str); > } > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java > > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java > index 6f3e103..4b7a0c6 100644 > --- > a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java > +++ > b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/YamlLayout.java > @@ -32,6 +32,7 @@ import org.apache.logging.log4j.core.config.plugins.Plugin; > import org.apache.logging.log4j.core.config.plugins.PluginAttribute; > import org.apache.logging.log4j.core.config.plugins.PluginConfiguration; > import org.apache.logging.log4j.core.config.plugins.PluginFactory; > +import org.apache.logging.log4j.core.impl.DefaultLogEventFactory; > import org.apache.logging.log4j.util.Strings; > > /** > @@ -728,7 +729,7 @@ public final class YamlLayout extends > AbstractJacksonLayout { > return null; > } > final StringBuilder buf = new StringBuilder(); > - final String str = serializeToString(getHeaderSerializer()); > + final String str = serializeToString(getHeaderSerializer(), > DefaultLogEventFactory.getInstance()); > if (str != null) { > buf.append(str); > } > @@ -748,7 +749,7 @@ public final class YamlLayout extends > AbstractJacksonLayout { > } > final StringBuilder buf = new StringBuilder(); > buf.append(this.eol); > - final String str = serializeToString(getFooterSerializer()); > + final String str = serializeToString(getFooterSerializer(), > DefaultLogEventFactory.getInstance()); > if (str != null) { > buf.append(str); > } > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java > new file mode 100644 > index 0000000..24a38b6 > --- /dev/null > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_CoreTest.java > @@ -0,0 +1,20 @@ > +package org.apache.logging.log4j.core.layout; > + > +import org.apache.logging.log4j.LogManager; > +import org.apache.logging.log4j.Logger; > + > +public class Log4j2_1482_CoreTest extends Log4j2_1482_Test { > + > + @Override > + protected void log(int runNumber) { > + if (runNumber == 2) { > + // System.out.println("Set a breakpoint here."); > + } > + final Logger logger = LogManager.getLogger("auditcsvfile"); > + final int val1 = 9, val2 = 11, val3 = 12; > + logger.info("Info Message!", val1, val2, val3); > + logger.info("Info Message!", val1, val2, val3); > + logger.info("Info Message!", val1, val2, val3); > + } > + > +} > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java > ---------------------------------------------------------------------- > diff --git > a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java > > b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java > new file mode 100644 > index 0000000..d25a6ac > --- /dev/null > +++ > b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/Log4j2_1482_Test.java > @@ -0,0 +1,89 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache license, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the license for the specific language governing permissions and > + * limitations under the license. > + */ > + > +package org.apache.logging.log4j.core.layout; > + > +import java.io.File; > +import java.io.IOException; > +import java.nio.charset.Charset; > +import java.nio.file.Files; > +import java.nio.file.Path; > +import java.nio.file.Paths; > +import java.util.Arrays; > +import java.util.List; > + > +import org.apache.logging.log4j.core.LoggerContext; > +import org.apache.logging.log4j.core.config.Configurator; > +import org.apache.logging.log4j.junit.CleanFolders; > +import org.junit.Assert; > +import org.junit.Rule; > +import org.junit.Test; > + > +/** > + * Tests https://issues.apache.org/jira/browse/LOG4J2-1482 > + */ > +public abstract class Log4j2_1482_Test { > + > + static final String CONFIG_LOCATION = "log4j2-1482.xml"; > + > + static final String FOLDER = "target/log4j2-1482"; > + > + private static final int LOOP_COUNT = 10; > + > + static void assertFileContents(int runNumber) throws IOException { > + Path path = Paths.get(FOLDER + "/audit.tmp"); > + List<String> lines = Files.readAllLines(path, > Charset.defaultCharset()); > + int i = 1; > + final int size = lines.size(); > + for (String string : lines) { > + if (string.startsWith(",,")) { > + Path folder = Paths.get(FOLDER); > + File[] files = folder.toFile().listFiles(); > + Arrays.sort(files); > + System.out.println("Run " + runNumber + ": " + > Arrays.toString(files)); > + Assert.fail( > + String.format("Run %,d, line %,d of %,d: \"%s\" in > %s", runNumber, i++, size, string, lines)); > + } > + } > + } > + > + @Rule > + public CleanFolders cleanFolders = new CleanFolders(FOLDER); > + > + protected abstract void log(int runNumber) ; > + > + private void loopingRun(int loopCount) throws IOException { > + for (int i = 1; i <= loopCount; i++) { > + try (LoggerContext loggerContext = > Configurator.initialize(getClass().getName(), > + CONFIG_LOCATION)) { > + log(i); > + } > + assertFileContents(i); > + } > + } > + > + @Test > + public void testLoopingRun() throws IOException { > + loopingRun(LOOP_COUNT); > + } > + > + @Test > + public void testSingleRun() throws IOException { > + loopingRun(1); > + } > + > +} > \ No newline at end of file > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-core/src/test/resources/log4j2-1482.xml > ---------------------------------------------------------------------- > diff --git a/log4j-core/src/test/resources/log4j2-1482.xml > b/log4j-core/src/test/resources/log4j2-1482.xml > new file mode 100644 > index 0000000..e17953c > --- /dev/null > +++ b/log4j-core/src/test/resources/log4j2-1482.xml > @@ -0,0 +1,27 @@ > +<?xml version="1.0" encoding="UTF-8"?> > +<Configuration status="warn" name="MyApp" packages=""> > + <Properties> > + <Property name="audit-path">target/log4j2-1482</Property> > + <Property name="file-name">audit</Property> > + <Property name="file-header">param1,param2,param3${sys:line.separator} > + </Property> > + </Properties> > + > + <Appenders> > + <RollingFile name="auditfile" fileName="${audit-path}/${file-name}.tmp" > + filePattern="${audit-path}/${file-name}-%d{yyyy-MM-dd}-%i.csv"> > + <CsvParameterLayout delimiter="," header="${file-header}"> > + </CsvParameterLayout> > + <Policies> > + <SizeBasedTriggeringPolicy size="80 B" /> > + </Policies> > + <DefaultRolloverStrategy max="2" /> > + </RollingFile> > + </Appenders> > + > + <Loggers> > + <Root level="info"> > + <AppenderRef ref="auditfile" /> > + </Root> > + </Loggers> > +</Configuration> > \ No newline at end of file > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-slf4j-impl/pom.xml > ---------------------------------------------------------------------- > diff --git a/log4j-slf4j-impl/pom.xml b/log4j-slf4j-impl/pom.xml > index aca5799..21b02b1 100644 > --- a/log4j-slf4j-impl/pom.xml > +++ b/log4j-slf4j-impl/pom.xml > @@ -58,6 +58,11 @@ > <scope>test</scope> > </dependency> > <dependency> > + <groupId>org.apache.commons</groupId> > + <artifactId>commons-csv</artifactId> > + <scope>test</scope> > + </dependency> > + <dependency> > <groupId>org.apache.logging.log4j</groupId> > <artifactId>log4j-core</artifactId> > <scope>test</scope> > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java > ---------------------------------------------------------------------- > diff --git > a/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java > > b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java > new file mode 100644 > index 0000000..d621e76 > --- /dev/null > +++ > b/log4j-slf4j-impl/src/test/java/org/apache/logging/slf4j/Log4j2_1482_Slf4jTest.java > @@ -0,0 +1,41 @@ > +/* > + * Licensed to the Apache Software Foundation (ASF) under one or more > + * contributor license agreements. See the NOTICE file distributed with > + * this work for additional information regarding copyright ownership. > + * The ASF licenses this file to You under the Apache license, Version 2.0 > + * (the "License"); you may not use this file except in compliance with > + * the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, software > + * distributed under the License is distributed on an "AS IS" BASIS, > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. > + * See the license for the specific language governing permissions and > + * limitations under the license. > + */ > + > +package org.apache.logging.slf4j; > + > +import org.apache.logging.log4j.core.layout.Log4j2_1482_Test; > +import org.slf4j.Logger; > +import org.slf4j.LoggerFactory; > + > +/** > + * Tests https://issues.apache.org/jira/browse/LOG4J2-1482 > + */ > +public class Log4j2_1482_Slf4jTest extends Log4j2_1482_Test { > + > + @Override > + protected void log(int runNumber) { > + if (runNumber == 2) { > + // System.out.println("Set a breakpoint here."); > + } > + final Logger logger = LoggerFactory.getLogger("auditcsvfile"); > + final int val1 = 9, val2 = 11, val3 = 12; > + logger.info("Info Message!", val1, val2, val3); > + logger.info("Info Message!", val1, val2, val3); > + logger.info("Info Message!", val1, val2, val3); > + } > + > +} > \ No newline at end of file > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml > ---------------------------------------------------------------------- > diff --git a/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml > b/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml > new file mode 100644 > index 0000000..e17953c > --- /dev/null > +++ b/log4j-slf4j-impl/src/test/resources/log4j2-1482.xml > @@ -0,0 +1,27 @@ > +<?xml version="1.0" encoding="UTF-8"?> > +<Configuration status="warn" name="MyApp" packages=""> > + <Properties> > + <Property name="audit-path">target/log4j2-1482</Property> > + <Property name="file-name">audit</Property> > + <Property name="file-header">param1,param2,param3${sys:line.separator} > + </Property> > + </Properties> > + > + <Appenders> > + <RollingFile name="auditfile" fileName="${audit-path}/${file-name}.tmp" > + filePattern="${audit-path}/${file-name}-%d{yyyy-MM-dd}-%i.csv"> > + <CsvParameterLayout delimiter="," header="${file-header}"> > + </CsvParameterLayout> > + <Policies> > + <SizeBasedTriggeringPolicy size="80 B" /> > + </Policies> > + <DefaultRolloverStrategy max="2" /> > + </RollingFile> > + </Appenders> > + > + <Loggers> > + <Root level="info"> > + <AppenderRef ref="auditfile" /> > + </Root> > + </Loggers> > +</Configuration> > \ No newline at end of file > > http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ffc6c8f6/src/changes/changes.xml > ---------------------------------------------------------------------- > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index b451094..31f280d 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -24,6 +24,9 @@ > </properties> > <body> > <release version="2.7" date="2016-MM-DD" description="GA Release 2.7"> > + <action issue="LOG4J2-1482" dev="ggregory" type="fix" due-to="Gary > Gregory, Sumit Singhal"> > + Improper header in CsvParameterLayout. > + </action> > <action issue="LOG4J2-1199" dev="rpopma" type="fix"> > Document that JVM Input Arguments Lookup (JMX) is not available on > Google App Engine. > </action> > --------------------------------------------------------------------- To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org For additional commands, e-mail: log4j-dev-h...@logging.apache.org
