Repository: logging-log4j2 Updated Branches: refs/heads/LOG4J-1012 db31dad0c -> 3f87e7acd
LOG4J2-1135 fix broken compression on rollover Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/a5447aee Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/a5447aee Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/a5447aee Branch: refs/heads/LOG4J-1012 Commit: a5447aee9b6b1f9a0a5bbf384a6640e182a3397f Parents: 6ad2d29 Author: rpopma <[email protected]> Authored: Mon Sep 28 18:50:22 2015 +0200 Committer: rpopma <[email protected]> Committed: Mon Sep 28 18:50:22 2015 +0200 ---------------------------------------------------------------------- .../rolling/DefaultRolloverStrategy.java | 56 ++++++++++++-------- .../rolling/RollingAppenderSizeTest.java | 56 ++++++++++++++------ src/changes/changes.xml | 3 ++ 3 files changed, 76 insertions(+), 39 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/a5447aee/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java index 8c26650..af5ee25 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/DefaultRolloverStrategy.java @@ -82,21 +82,22 @@ public class DefaultRolloverStrategy implements RolloverStrategy { /** * Enumerates over supported file extensions. + * <p> + * Package-protected for unit tests. */ - private enum FileExtensions { + enum FileExtensions { ZIP(".zip") { @Override Action createCompressAction(final String renameTo, final String compressedName, final boolean deleteSource, final int compressionLevel) { - return new ZipCompressAction(new File(baseName(renameTo)), new File(compressedName), deleteSource, - compressionLevel); + return new ZipCompressAction(source(renameTo), target(compressedName), deleteSource, compressionLevel); } }, - GZIP(".gz") { + GZ(".gz") { @Override Action createCompressAction(final String renameTo, final String compressedName, final boolean deleteSource, final int compressionLevel) { - return new GzCompressAction(new File(baseName(renameTo)), new File(compressedName), deleteSource); + return new GzCompressAction(source(renameTo), target(compressedName), deleteSource); } }, BZIP2(".bz2") { @@ -104,8 +105,7 @@ public class DefaultRolloverStrategy implements RolloverStrategy { Action createCompressAction(final String renameTo, final String compressedName, final boolean deleteSource, final int compressionLevel) { // One of "gz", "bzip2", "xz", "pack200", or "deflate". - return new CommonsCompressAction("bzip2", new File(baseName(renameTo)), new File(compressedName), - deleteSource); + return new CommonsCompressAction("bzip2", source(renameTo), target(compressedName), deleteSource); } }, DEFLATE(".deflate") { @@ -113,8 +113,7 @@ public class DefaultRolloverStrategy implements RolloverStrategy { Action createCompressAction(final String renameTo, final String compressedName, final boolean deleteSource, final int compressionLevel) { // One of "gz", "bzip2", "xz", "pack200", or "deflate". - return new CommonsCompressAction("deflate", new File(baseName(renameTo)), new File(compressedName), - deleteSource); + return new CommonsCompressAction("deflate", source(renameTo), target(compressedName), deleteSource); } }, PACK200(".pack200") { @@ -122,8 +121,7 @@ public class DefaultRolloverStrategy implements RolloverStrategy { Action createCompressAction(final String renameTo, final String compressedName, final boolean deleteSource, final int compressionLevel) { // One of "gz", "bzip2", "xz", "pack200", or "deflate". - return new CommonsCompressAction("pack200", new File(baseName(renameTo)), new File(compressedName), - deleteSource); + return new CommonsCompressAction("pack200", source(renameTo), target(compressedName), deleteSource); } }, XY(".xy") { @@ -131,8 +129,7 @@ public class DefaultRolloverStrategy implements RolloverStrategy { Action createCompressAction(final String renameTo, final String compressedName, final boolean deleteSource, final int compressionLevel) { // One of "gz", "bzip2", "xz", "pack200", or "deflate". - return new CommonsCompressAction("xy", new File(baseName(renameTo)), new File(compressedName), - deleteSource); + return new CommonsCompressAction("xy", source(renameTo), target(compressedName), deleteSource); } }; @@ -154,13 +151,26 @@ public class DefaultRolloverStrategy implements RolloverStrategy { int length() { return extension.length(); } - - String baseName(final String name) { - return name.substring(0, name.length() - length()); + + File source(String fileName) { + return new File(fileName); + } + + File target(String fileName) { + return new File(fileName); } abstract Action createCompressAction(String renameTo, String compressedName, boolean deleteSource, int compressionLevel); + + static FileExtensions lookup(String fileExtension) { + for (FileExtensions ext : values()) { + if (ext.isExtensionFor(fileExtension)) { + return ext; + } + } + return null; + } }; /** @@ -489,16 +499,16 @@ public class DefaultRolloverStrategy implements RolloverStrategy { manager.getPatternProcessor().formatFileName(subst, buf, fileIndex); final String currentFileName = manager.getFileName(); - final String renameTo = buf.toString(); + String renameTo = buf.toString(); final String compressedName = renameTo; Action compressAction = null; - if (FileExtensions.GZIP.isExtensionFor(renameTo)) { - compressAction = FileExtensions.GZIP.createCompressAction(renameTo, compressedName, true, compressionLevel); - } else if (FileExtensions.ZIP.isExtensionFor(renameTo)) { - compressAction = FileExtensions.ZIP.createCompressAction(renameTo, compressedName, true, compressionLevel); - } else if (FileExtensions.BZIP2.isExtensionFor(renameTo)) { - compressAction = FileExtensions.BZIP2.createCompressAction(renameTo, compressedName, true, compressionLevel); + for (FileExtensions ext : FileExtensions.values()) { // LOG4J2-1077 support other compression formats + if (ext.isExtensionFor(renameTo)) { + renameTo = renameTo.substring(0, renameTo.length() - ext.length()); // LOG4J2-1135 omit extension! + compressAction = ext.createCompressAction(renameTo, compressedName, true, compressionLevel); + break; + } } final FileRenameAction renameAction = http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/a5447aee/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeTest.java index 18e7901..d8db5bc 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/RollingAppenderSizeTest.java @@ -16,11 +16,18 @@ */ package org.apache.logging.log4j.core.appender.rolling; +import java.io.ByteArrayOutputStream; import java.io.File; +import java.io.FileInputStream; +import java.nio.charset.Charset; import java.util.Arrays; import java.util.Collection; +import org.apache.commons.compress.compressors.CompressorInputStream; +import org.apache.commons.compress.compressors.CompressorStreamFactory; +import org.apache.commons.compress.utils.IOUtils; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.util.Closer; import org.apache.logging.log4j.junit.LoggerContextRule; import org.junit.After; import org.junit.Before; @@ -29,10 +36,10 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; -import static org.apache.logging.log4j.hamcrest.FileMatchers.hasName; -import static org.apache.logging.log4j.hamcrest.Descriptors.that; -import static org.hamcrest.Matchers.endsWith; -import static org.hamcrest.Matchers.hasItemInArray; +import static org.apache.logging.log4j.hamcrest.Descriptors.*; +import static org.apache.logging.log4j.hamcrest.FileMatchers.*; +import static org.hamcrest.Matchers.*; + import static org.junit.Assert.*; /** @@ -49,17 +56,10 @@ public class RollingAppenderSizeTest { @Parameterized.Parameters(name = "{0} \u2192 {1}") public static Collection<Object[]> data() { - return Arrays.asList( - new Object[][]{ - { "log4j-rolling-gz.xml", ".gz" }, - { "log4j-rolling-zip.xml", ".zip" }, - // Apache Commons Compress - { "log4j-rolling-bzip2.xml", ".bz2" }, - { "log4j-rolling-deflate.xml", ".deflate" }, - { "log4j-rolling-pack200.xml", ".pack200" }, - { "log4j-rolling-xy.xml", ".xy" }, - } - ); + return Arrays.asList(new Object[][] { {"log4j-rolling-gz.xml", ".gz"}, {"log4j-rolling-zip.xml", ".zip"}, + // Apache Commons Compress + {"log4j-rolling-bzip2.xml", ".bz2"}, {"log4j-rolling-deflate.xml", ".deflate"}, + {"log4j-rolling-pack200.xml", ".pack200"}, {"log4j-rolling-xy.xml", ".xy"},}); } @Rule @@ -82,7 +82,7 @@ public class RollingAppenderSizeTest { @Test public void testAppender() throws Exception { - for (int i=0; i < 100; ++i) { + for (int i = 0; i < 100; ++i) { logger.debug("This is test message number " + i); } final File dir = new File(DIR); @@ -90,6 +90,30 @@ public class RollingAppenderSizeTest { final File[] files = dir.listFiles(); assertNotNull(files); assertThat(files, hasItemInArray(that(hasName(that(endsWith(fileExtension)))))); + + DefaultRolloverStrategy.FileExtensions ext = DefaultRolloverStrategy.FileExtensions.lookup(fileExtension); + if (ext == null || DefaultRolloverStrategy.FileExtensions.ZIP == ext + || DefaultRolloverStrategy.FileExtensions.XY == ext + || DefaultRolloverStrategy.FileExtensions.PACK200 == ext) { + return; // commons compress cannot deflate zip and xy? TODO test decompressing these formats + } + for (File file : files) { + if (file.getName().endsWith(fileExtension)) { + CompressorInputStream in = null; + try (FileInputStream fis = new FileInputStream(file)) { + in = new CompressorStreamFactory().createCompressorInputStream(ext.name().toLowerCase(), fis); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + IOUtils.copy(in, baos); + String text = new String(baos.toByteArray(), Charset.defaultCharset()); + String[] lines = text.split("[\\r\\n]+"); + for (String line : lines) { + assertTrue(line.contains("DEBUG o.a.l.l.c.a.r.RollingAppenderSizeTest [main] This is test message number")); + } + } finally { + Closer.close(in); + } + } + } } private static void deleteDir() { http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/a5447aee/src/changes/changes.xml ---------------------------------------------------------------------- diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 146b4e2..42928ae 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -24,6 +24,9 @@ </properties> <body> <release version="2.4.1" date="2015-MM-DD" description="GA Release 2.4.1"> + <action issue="LOG4J2-1135" dev="rpopma" type="fix"> + Compression on rollover was broken: log file was renamed to .zip but not compressed. + </action> <action issue="LOG4J2-1129" dev="rgoers" type="add"> Allow PatternLayout to select a pattern to use based on some selection criteria. </action>
