[LOG4J2-1505] Create a Builder for FileAppender. Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/06022018 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/06022018 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/06022018
Branch: refs/heads/LOG4J2-1010&LOG4J2-1447-injectable-contextdata&better-datastructure Commit: 06022018216a54d950ae34afa03f8786f6878f23 Parents: 1b5bcb9 Author: Gary Gregory <ggreg...@apache.org> Authored: Mon Aug 8 12:00:09 2016 -0700 Committer: Gary Gregory <ggreg...@apache.org> Committed: Mon Aug 8 12:00:09 2016 -0700 ---------------------------------------------------------------------- .../log4j/core/appender/FileAppender.java | 390 ++++++++++++------- .../log4j/core/appender/FileAppenderTest.java | 59 ++- .../core/appender/OutputStreamAppenderTest.java | 14 +- .../core/config/CustomConfigurationTest.java | 13 +- src/changes/changes.xml | 3 + 5 files changed, 327 insertions(+), 152 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/06022018/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java index b4eb6f6..3f364cc 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/FileAppender.java @@ -24,10 +24,10 @@ import org.apache.logging.log4j.core.Filter; import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.config.Configuration; 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.PluginBuilderAttribute; +import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory; import org.apache.logging.log4j.core.config.plugins.PluginConfiguration; import org.apache.logging.log4j.core.config.plugins.PluginElement; -import org.apache.logging.log4j.core.config.plugins.PluginFactory; import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.logging.log4j.core.net.Advertiser; import org.apache.logging.log4j.core.util.Booleans; @@ -39,43 +39,213 @@ import org.apache.logging.log4j.core.util.Integers; @Plugin(name = "File", category = "Core", elementType = "appender", printObject = true) public final class FileAppender extends AbstractOutputStreamAppender<FileManager> { - static final int DEFAULT_BUFFER_SIZE = 8192; - private final String fileName; - private final Advertiser advertiser; - private Object advertisement; + /** + * Builds FileAppender instances. + */ + public static class Builder implements org.apache.logging.log4j.core.util.Builder<FileAppender> { - private FileAppender(final String name, final Layout<? extends Serializable> layout, final Filter filter, - final FileManager manager, final String filename, final boolean ignoreExceptions, - final boolean immediateFlush, final Advertiser advertiser) { + @PluginBuilderAttribute + private String fileName; - super(name, layout, filter, ignoreExceptions, immediateFlush, manager); - if (advertiser != null) { - final Map<String, String> configuration = new HashMap<>(layout.getContentFormat()); - configuration.putAll(manager.getContentFormat()); - configuration.put("contentType", layout.getContentType()); - configuration.put("name", name); - advertisement = advertiser.advertise(configuration); + @PluginBuilderAttribute + private boolean append = true; + + @PluginBuilderAttribute + private boolean locking; + + @PluginBuilderAttribute + private String name; + + @PluginBuilderAttribute + private boolean immediateFlush = true; + + @PluginBuilderAttribute + private boolean ignoreExceptions = true; + + @PluginBuilderAttribute + private boolean bufferedIo = true; + + @PluginBuilderAttribute + private int bufferSize = DEFAULT_BUFFER_SIZE; + + @PluginElement("Layout") + private Layout<? extends Serializable> layout; + + @PluginElement("Filter") + private Filter filter; + + @PluginBuilderAttribute + private boolean advertise; + + @PluginBuilderAttribute + private String advertiseUri; + + @PluginBuilderAttribute + private boolean lazyCreate; + + @PluginConfiguration + private Configuration config; + + @Override + public FileAppender build() { + if (locking && bufferedIo) { + LOGGER.warn("Locking and buffering are mutually exclusive. No buffering will occur for {}", fileName); + bufferedIo = false; + } + if (!bufferedIo && bufferSize > 0) { + LOGGER.warn("The bufferSize is set to {} but bufferedIo is not true: {}", bufferSize, bufferedIo); + } + if (name == null) { + LOGGER.error("No name provided for FileAppender"); + return null; + } + if (fileName == null) { + LOGGER.error("No filename provided for FileAppender with name {}", name); + return null; + } + if (layout == null) { + layout = PatternLayout.createDefaultLayout(); + } + + final FileManager manager = FileManager.getFileManager(fileName, append, locking, bufferedIo, lazyCreate, + advertiseUri, layout, bufferSize, immediateFlush); + if (manager == null) { + return null; + } + + return new FileAppender(name, layout, filter, manager, fileName, ignoreExceptions, + !bufferedIo || immediateFlush, advertise ? config.getAdvertiser() : null); } - this.fileName = filename; - this.advertiser = advertiser; - } - @Override - public void stop() { - super.stop(); - if (advertiser != null) { - advertiser.unadvertise(advertisement); + public String getAdvertiseUri() { + return advertiseUri; } - } - /** - * Returns the file name this appender is associated with. - * @return The File name. - */ - public String getFileName() { - return this.fileName; - } + public int getBufferSize() { + return bufferSize; + } + + public Configuration getConfig() { + return config; + } + + public String getFileName() { + return fileName; + } + + public Filter getFilter() { + return filter; + } + + public Layout<? extends Serializable> getLayout() { + return layout; + } + + public String getName() { + return name; + } + + public boolean isAdvertise() { + return advertise; + } + + public boolean isAppend() { + return append; + } + + public boolean isBufferedIo() { + return bufferedIo; + } + + public boolean isIgnoreExceptions() { + return ignoreExceptions; + } + + public boolean isImmediateFlush() { + return immediateFlush; + } + + public boolean isLazyCreate() { + return lazyCreate; + } + + public boolean isLocking() { + return locking; + } + + public Builder withAdvertise(final boolean advertise) { + this.advertise = advertise; + return this; + } + + public Builder withAdvertiseUri(final String advertiseUri) { + this.advertiseUri = advertiseUri; + return this; + } + public Builder withAppend(final boolean append) { + this.append = append; + return this; + } + + public Builder withBufferedIo(final boolean bufferedIo) { + this.bufferedIo = bufferedIo; + return this; + } + + public Builder withBufferSize(final int bufferSize) { + this.bufferSize = bufferSize; + return this; + } + + public Builder withConfig(final Configuration config) { + this.config = config; + return this; + } + + public Builder withFileName(final String fileName) { + this.fileName = fileName; + return this; + } + + public Builder withFilter(final Filter filter) { + this.filter = filter; + return this; + } + + public Builder withIgnoreExceptions(final boolean ignoreExceptions) { + this.ignoreExceptions = ignoreExceptions; + return this; + } + + public Builder withImmediateFlush(final boolean immediateFlush) { + this.immediateFlush = immediateFlush; + return this; + } + + public Builder withLayout(final Layout<? extends Serializable> layout) { + this.layout = layout; + return this; + } + + public Builder withLazyCreate(final boolean lazyCreate) { + this.lazyCreate = lazyCreate; + return this; + } + + public Builder withLocking(final boolean locking) { + this.locking = locking; + return this; + } + + public Builder withName(final String name) { + this.name = name; + return this; + } + } + + private static final int DEFAULT_BUFFER_SIZE = 8192; + /** * Create a File Appender. * @param fileName The name and path of the file. @@ -96,7 +266,7 @@ public final class FileAppender extends AbstractOutputStreamAppender<FileManager * @param advertiseUri The advertised URI which can be used to retrieve the file contents. * @param config The Configuration * @return The FileAppender. - * @deprecated Use {@link #createAppender(String, boolean, boolean, String, String, String, boolean, String, Layout<? extends Serializable>, Filter, String, String, boolean, Configuration)} + * @deprecated Use {@link #newBuilder()} */ @Deprecated public static FileAppender createAppender( @@ -109,119 +279,71 @@ public final class FileAppender extends AbstractOutputStreamAppender<FileManager final String ignore, final String bufferedIo, final String bufferSizeStr, - Layout<? extends Serializable> layout, + final Layout<? extends Serializable> layout, final Filter filter, final String advertise, final String advertiseUri, final Configuration config) { + return newBuilder() + .withAdvertise(Boolean.parseBoolean(advertise)) + .withAdvertiseUri(advertiseUri) + .withAppend(Booleans.parseBoolean(append, true)) + .withBufferedIo(Booleans.parseBoolean(bufferedIo, true)) + .withBufferSize(Integers.parseInt(bufferSizeStr, DEFAULT_BUFFER_SIZE)) + .withConfig(config) + .withFileName(fileName) + .withFilter(filter) + .withIgnoreExceptions(Booleans.parseBoolean(ignore, true)) + .withImmediateFlush(Booleans.parseBoolean(immediateFlush, true)) + .withLayout(layout) + .withLocking(Boolean.parseBoolean(locking)) + .withName(name) + .build(); // @formatter:on - final boolean isAppend = Booleans.parseBoolean(append, true); - final boolean isLocking = Boolean.parseBoolean(locking); - boolean isBuffered = Booleans.parseBoolean(bufferedIo, true); - final boolean isAdvertise = Boolean.parseBoolean(advertise); - if (isLocking && isBuffered) { - if (bufferedIo != null) { - LOGGER.warn("Locking and buffering are mutually exclusive. No buffering will occur for " + fileName); - } - isBuffered = false; - } - final int bufferSize = Integers.parseInt(bufferSizeStr, DEFAULT_BUFFER_SIZE); - if (!isBuffered && bufferSize > 0) { - LOGGER.warn("The bufferSize is set to {} but bufferedIO is not true: {}", bufferSize, bufferedIo); - } - final boolean isFlush = Booleans.parseBoolean(immediateFlush, true); - final boolean ignoreExceptions = Booleans.parseBoolean(ignore, true); + } + + @PluginBuilderFactory + public static Builder newBuilder() { + return new Builder(); + } + + private final String fileName; - if (name == null) { - LOGGER.error("No name provided for FileAppender"); - return null; - } + private final Advertiser advertiser; - if (fileName == null) { - LOGGER.error("No filename provided for FileAppender with name " + name); - return null; - } - if (layout == null) { - layout = PatternLayout.createDefaultLayout(); - } + private final Object advertisement; - final FileManager manager = FileManager.getFileManager(fileName, isAppend, isLocking, isBuffered, false, - advertiseUri, layout, bufferSize, isFlush); - if (manager == null) { - return null; - } + private FileAppender(final String name, final Layout<? extends Serializable> layout, final Filter filter, + final FileManager manager, final String filename, final boolean ignoreExceptions, + final boolean immediateFlush, final Advertiser advertiser) { - return new FileAppender(name, layout, filter, manager, fileName, ignoreExceptions, !isBuffered || isFlush, - isAdvertise ? config.getAdvertiser() : null); + super(name, layout, filter, ignoreExceptions, immediateFlush, manager); + if (advertiser != null) { + final Map<String, String> configuration = new HashMap<>(layout.getContentFormat()); + configuration.putAll(manager.getContentFormat()); + configuration.put("contentType", layout.getContentType()); + configuration.put("name", name); + advertisement = advertiser.advertise(configuration); + } else { + advertisement = null; + } + this.fileName = filename; + this.advertiser = advertiser; } /** - * Create a File Appender. - * @param fileName The name and path of the file. - * @param append "True" if the file should be appended to, "false" if it should be overwritten. - * The default is "true". - * @param locking "True" if the file should be locked. The default is "false". - * @param name The name of the Appender. - * @param immediateFlush "true" if the contents should be flushed on every write, "false" otherwise. The default - * is "true". - * @param ignoreExceptions If {@code "true"} (default) exceptions encountered when appending events are logged; otherwise - * they are propagated to the caller. - * @param bufferedIo "true" if I/O should be buffered, "false" otherwise. The default is "true". - * @param bufferSize buffer size for buffered IO (default is 8192). - * @param layout The layout to use to format the event. If no layout is provided the default PatternLayout - * will be used. - * @param filter The filter, if any, to use. - * @param advertise "true" if the appender configuration should be advertised, "false" otherwise. - * @param advertiseUri The advertised URI which can be used to retrieve the file contents. - * @param lazyCreate true if you want to lazy-create the file (a.k.a. on-demand.) - * @param config The Configuration - * @return The FileAppender. - * @since 2.7 + * Returns the file name this appender is associated with. + * @return The File name. */ - @PluginFactory - public static FileAppender createAppender( - // @formatter:off - @PluginAttribute("fileName") final String fileName, - @PluginAttribute(value = "append", defaultBoolean = true) final boolean append, - @PluginAttribute("locking") final boolean locking, - @PluginAttribute("name") final String name, - @PluginAttribute(value = "immediateFlush", defaultBoolean = true) final boolean immediateFlush, - @PluginAttribute(value = "ignoreExceptions", defaultBoolean = true) final boolean ignoreExceptions, - @PluginAttribute(value = "bufferedIo", defaultBoolean = true) boolean bufferedIo, - @PluginAttribute(value = "bufferSize", defaultInt = DEFAULT_BUFFER_SIZE) final int bufferSize, - @PluginElement("Layout") Layout<? extends Serializable> layout, - @PluginElement("Filter") final Filter filter, - @PluginAttribute("advertise") final boolean advertise, - @PluginAttribute("advertiseUri") final String advertiseUri, - @PluginAttribute("lazyCreate") final boolean lazyCreate, - @PluginConfiguration final Configuration config) { - // @formatter:on - if (locking && bufferedIo) { - LOGGER.warn("Locking and buffering are mutually exclusive. No buffering will occur for {}", fileName); - bufferedIo = false; - } - if (!bufferedIo && bufferSize > 0) { - LOGGER.warn("The bufferSize is set to {} but bufferedIo is not true: {}", bufferSize, bufferedIo); - } - if (name == null) { - LOGGER.error("No name provided for FileAppender"); - return null; - } - if (fileName == null) { - LOGGER.error("No filename provided for FileAppender with name {}", name); - return null; - } - if (layout == null) { - layout = PatternLayout.createDefaultLayout(); - } - - final FileManager manager = FileManager.getFileManager(fileName, append, locking, bufferedIo, lazyCreate, - advertiseUri, layout, bufferSize, immediateFlush); - if (manager == null) { - return null; - } - - return new FileAppender(name, layout, filter, manager, fileName, ignoreExceptions, !bufferedIo || immediateFlush, - advertise ? config.getAdvertiser() : null); + public String getFileName() { + return this.fileName; + } + + @Override + public void stop() { + super.stop(); + if (advertiser != null) { + advertiser.unadvertise(advertisement); + } } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/06022018/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java index 8a9603c..b706c31 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/FileAppenderTest.java @@ -33,6 +33,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.Appender; import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.impl.Log4jLogEvent; @@ -89,10 +90,19 @@ public class FileAppenderTest { @Test public void testLazyCreate() throws Exception { - final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) - .build(); - final FileAppender appender = FileAppender.createAppender(FILE_NAME, true, false, "test", false, false, false, - 1, layout, null, false, null, lazyCreate, null); + final Layout<String> layout = createPatternLayout(); + // @formatter:off + final FileAppender appender = FileAppender.newBuilder() + .withFileName(FILE_NAME) + .withName("test") + .withImmediateFlush(false) + .withIgnoreExceptions(false) + .withBufferedIo(false) + .withBufferSize(1) + .withLayout(layout) + .withLazyCreate(lazyCreate) + .build(); + // @formatter:on Assert.assertEquals(lazyCreate, appender.getManager().isLazyCreate()); try { Assert.assertNotEquals(lazyCreate, Files.exists(PATH)); @@ -104,12 +114,26 @@ public class FileAppenderTest { Assert.assertNotEquals(lazyCreate, Files.exists(PATH)); } + private static PatternLayout createPatternLayout() { + return PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) + .build(); + } + @Test public void testSmallestBufferSize() throws Exception { - final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) - .build(); - final FileAppender appender = FileAppender.createAppender(FILE_NAME, true, false, "test", false, false, false, - 1, layout, null, false, null, lazyCreate, null); + final Layout<String> layout = createPatternLayout(); + // @formatter:off + final FileAppender appender = FileAppender.newBuilder() + .withFileName(FILE_NAME) + .withName("test") + .withImmediateFlush(false) + .withIgnoreExceptions(false) + .withBufferedIo(false) + .withBufferSize(1) + .withLayout(layout) + .withLazyCreate(lazyCreate) + .build(); + // @formatter:on try { appender.start(); final File file = new File(FILE_NAME); @@ -204,12 +228,21 @@ public class FileAppenderTest { verifyFile(logEventCount * processCount); } - private static void writer(final boolean lock, final int logEventCount, final String name, boolean lazyCreate, + private static void writer(final boolean locking, final int logEventCount, final String name, boolean lazyCreate, boolean concurrent) throws Exception { - final Layout<String> layout = PatternLayout.newBuilder().withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) - .build(); - final FileAppender appender = FileAppender.createAppender(FILE_NAME, true, lock, "test", false, false, false, - FileAppender.DEFAULT_BUFFER_SIZE, layout, null, false, null, lazyCreate, null); + final Layout<String> layout = createPatternLayout(); + // @formatter:off + final FileAppender appender = FileAppender.newBuilder() + .withFileName(FILE_NAME) + .withName("test") + .withImmediateFlush(false) + .withIgnoreExceptions(false) + .withLocking(locking) + .withBufferedIo(false) + .withLayout(layout) + .withLazyCreate(lazyCreate) + .build(); + // @formatter:on Assert.assertEquals(lazyCreate, appender.getManager().isLazyCreate()); try { appender.start(); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/06022018/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/OutputStreamAppenderTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/OutputStreamAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/OutputStreamAppenderTest.java index dd17dfb..845f88f 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/OutputStreamAppenderTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/OutputStreamAppenderTest.java @@ -92,9 +92,17 @@ public class OutputStreamAppenderTest { public void testUpdatePatternWithFileAppender() { final LoggerContext ctx = (LoggerContext) LogManager.getContext(false); final Configuration config = ctx.getConfiguration(); - final Layout<?> layout = PatternLayout.createDefaultLayout(); - final Appender appender = FileAppender.createAppender("target/" + getClass().getName() + ".log", false, - false, "File", true, false, false, 4000, layout, null, false, null, false, config); + // @formatter:off + final Appender appender = FileAppender.newBuilder() + .withFileName("target/" + getClass().getName() + ".log") + .withAppend(false) + .withName("File") + .withIgnoreExceptions(false) + .withBufferedIo(false) + .withBufferSize(4000) + .withConfig(config) + .build(); + // @formatter:on appender.start(); config.addAppender(appender); ConfigurationTestUtils.updateLoggers(appender, config); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/06022018/log4j-core/src/test/java/org/apache/logging/log4j/core/config/CustomConfigurationTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/config/CustomConfigurationTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/config/CustomConfigurationTest.java index 6888204..6c7b5f0 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/config/CustomConfigurationTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/config/CustomConfigurationTest.java @@ -80,8 +80,17 @@ public class CustomConfigurationTest { .withPattern(PatternLayout.SIMPLE_CONVERSION_PATTERN) .withConfiguration(config) .build(); - final Appender appender = FileAppender.createAppender(LOG_FILE, false, false, "File", true, - false, false, 4000, layout, null, false, null, false, config); + // @formatter:off + final FileAppender appender = FileAppender.newBuilder() + .withFileName(LOG_FILE) + .withAppend(false) + .withName("File") + .withIgnoreExceptions(false) + .withBufferSize(4000) + .withBufferedIo(false) + .withLayout(layout) + .build(); + // @formatter:on appender.start(); config.addAppender(appender); final AppenderRef ref = AppenderRef.createAppenderRef("File", null, null); http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/06022018/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 922aef0..ad03663 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -72,6 +72,9 @@ <action issue="LOG4J2-1458" dev="ggregory" type="add" due-to="Gary Gregory"> [PatternLayout] Add an ANSI option to %message. </action> + <action issue="LOG4J2-1505" dev="ggregory" type="add" due-to="Gary Gregory"> + Create a Builder for FileAppender. + </action> <action issue="LOG4J2-1458" dev="ggregory" type="update" due-to="Gary Gregory"> Update Jackson from 2.7.5 to 2.8.0. </action>
