LOG4J2-435 use PathMatcher instead of custom logic for glob and regex matching
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/e11e58c6 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/e11e58c6 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/e11e58c6 Branch: refs/heads/master Commit: e11e58c6c34ca5bf3f8f19848853135a0e473fc0 Parents: 167abba Author: rpopma <[email protected]> Authored: Thu Nov 26 23:51:09 2015 +0900 Committer: rpopma <[email protected]> Committed: Thu Nov 26 23:51:09 2015 +0900 ---------------------------------------------------------------------- .../appender/rolling/action/IfFileName.java | 136 ++++++++----------- .../appender/rolling/action/IfFileNameTest.java | 71 +++------- 2 files changed, 75 insertions(+), 132 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/e11e58c6/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileName.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileName.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileName.java index dacf118..a746a20 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileName.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileName.java @@ -16,9 +16,11 @@ */ package org.apache.logging.log4j.core.appender.rolling.action; +import java.nio.file.FileSystem; +import java.nio.file.FileSystems; import java.nio.file.Path; +import java.nio.file.PathMatcher; import java.nio.file.attribute.BasicFileAttributes; -import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.logging.log4j.Logger; @@ -28,127 +30,95 @@ import org.apache.logging.log4j.core.config.plugins.PluginFactory; import org.apache.logging.log4j.status.StatusLogger; /** - * PathFilter that accepts files for deletion if their relative path matches either a path pattern or a regular - * expression. + * PathCondition that accepts files for deletion if their relative path matches either a glob pattern or a regular + * expression. If both a regular expression and a glob pattern are specified the glob pattern is used and the regular + * expression is ignored. * <p> - * If both a regular expression and a path pattern are specified the path pattern is used and the regular expression is - * ignored. - * <p> - * The path pattern may contain '?' and '*' wildcarts. + * The regular expression is a pattern as defined by the {@link Pattern} class. A glob is a simplified pattern expression + * described in {@link FileSystem#getPathMatcher(String)}. */ @Plugin(name = "IfFileName", category = "Core", printObject = true) public final class IfFileName implements PathCondition { private static final Logger LOGGER = StatusLogger.getLogger(); - private final Pattern regex; - private final String pathPattern; + private final PathMatcher pathMatcher; + private final String syntaxAndPattern; /** - * Constructs a FileNameFilter filter. If both a regular expression and a path pattern are specified the path + * Constructs a FileNameFilter filter. If both a regular expression and a glob pattern are specified the glob * pattern is used and the regular expression is ignored. * - * @param path the baseDir-relative path pattern of the files to delete (may contain '*' and '?' wildcarts) + * @param glob the baseDir-relative path pattern of the files to delete (may contain '*' and '?' wildcarts) * @param regex the regular expression that matches the baseDir-relative path of the file(s) to delete */ - private IfFileName(final String path, final String regex) { - if (regex == null && path == null) { - throw new IllegalArgumentException("Specify either a path or a regular expression. Both cannot be null."); + private IfFileName(final String glob, final String regex) { + if (regex == null && glob == null) { + throw new IllegalArgumentException("Specify either a path glob or a regular expression. " + + "Both cannot be null."); } - this.regex = regex != null ? Pattern.compile(regex) : null; - this.pathPattern = path; + syntaxAndPattern = createSyntaxAndPatternString(glob, regex); + pathMatcher = FileSystems.getDefault().getPathMatcher(syntaxAndPattern); } - /** - * Returns the compiled regular expression that matches the baseDir-relative path of the file(s) to delete, or - * {@code null} if no regular expression was specified. - * - * @return the compiled regular expression, or {@code null} - */ - public Pattern getRegex() { - return regex; + static String createSyntaxAndPatternString(final String glob, final String regex) { + if (glob != null) { + return glob.startsWith("glob:") ? glob : "glob:" + glob; + } + return regex.startsWith("regex:") ? regex : "regex:" + regex; } /** - * Returns the baseDir-relative path pattern of the files to delete, or {@code null} if not specified. This path - * pattern may contain '*' and '?' wildcarts. + * Returns the baseDir-relative path pattern of the files to delete. The returned string takes the form + * {@code syntax:pattern} where syntax is one of "glob" or "regex" and the pattern is either a {@linkplain Pattern + * regular expression} or a simplified pattern expression described under "glob" in + * {@link FileSystem#getPathMatcher(String)}. * - * @return relative path of the file(s) to delete (may contain '*' and '?' wildcarts) + * @return relative path of the file(s) to delete (may contain regular expression or wildcarts) */ - public String getPathPattern() { - return pathPattern; + public String getSyntaxAndPattern() { + return syntaxAndPattern; } /* * (non-Javadoc) - * - * @see org.apache.logging.log4j.core.appender.rolling.action.PathFilter#accept(java.nio.file.Path, - * java.nio.file.Path) + * @see org.apache.logging.log4j.core.appender.rolling.action.PathCondition#accept(java.nio.file.Path, java.nio.file.Path, java.nio.file.attribute.BasicFileAttributes) */ @Override public boolean accept(final Path basePath, final Path relativePath, final BasicFileAttributes attrs) { - if (pathPattern != null) { - final boolean result = isMatch(relativePath.toString(), pathPattern); - final String match = result ? "" : "not "; - LOGGER.trace("FileNameFilter: pathPattern '{}' does {}match relative path '{}'", pathPattern, match, - relativePath); - return result; - } else { - final Matcher matcher = regex.matcher(relativePath.toString()); - final boolean result = matcher.matches(); - final String match = result ? "" : "not "; - LOGGER.trace("FileNameFilter: regex '{}' does {}match relative path '{}'", regex.pattern(), match, - relativePath); - return result; - } - } + final boolean result = pathMatcher.matches(relativePath); - // package protected for unit tests - static boolean isMatch(final String text, final String pattern) { - int i = 0; - int j = 0; - int starIndex = -1; - int iIndex = -1; - - while (i < text.length()) { - if (j < pattern.length() && (pattern.charAt(j) == '?' || pattern.charAt(j) == text.charAt(i))) { - ++i; - ++j; - } else if (j < pattern.length() && pattern.charAt(j) == '*') { - starIndex = j; - iIndex = i; - j++; - } else if (starIndex != -1) { - j = starIndex + 1; - i = iIndex + 1; - iIndex++; - } else { - return false; - } - } + final String match = result ? " " : " not "; + LOGGER.trace("IfFileName: '{}' does{}match relative path '{}'", syntaxAndPattern, match, relativePath); + return result; + } - while (j < pattern.length() && pattern.charAt(j) == '*') { - ++j; - } - return j == pattern.length(); + /* + * (non-Javadoc) + * @see org.apache.logging.log4j.core.appender.rolling.action.PathCondition#beforeFileTreeWalk() + */ + @Override + public void beforeFileTreeWalk() { } /** - * Creates a FileNameFilter filter. If both a regular expression and a path pattern are specified the path pattern - * is used and the regular expression is ignored. + * Creates a IfFileName condition that returns true if either the specified + * {@linkplain FileSystem#getPathMatcher(String) glob pattern} or the regular expression matches the relative path. + * If both a regular expression and a glob pattern are specified the glob pattern is used and the regular expression + * is ignored. * - * @param path the baseDir-relative path pattern of the files to delete (may contain '*' and '?' wildcarts) + * @param glob the baseDir-relative path pattern of the files to delete (may contain '*' and '?' wildcarts) * @param regex the regular expression that matches the baseDir-relative path of the file(s) to delete - * @return A FileNameFilter filter. + * @return A IfFileName condition. + * @see FileSystem#getPathMatcher(String) */ @PluginFactory - public static IfFileName createNameFilter( // - @PluginAttribute("path") final String path, // + public static IfFileName createNameCondition( // + @PluginAttribute("glob") final String glob, // @PluginAttribute("regex") final String regex) { - return new IfFileName(path, regex); + return new IfFileName(glob, regex); } @Override public String toString() { - final String pattern = regex == null ? "null" : regex.pattern(); - return "FileNameFilter(regex=" + pattern + ", path=" + pathPattern + ")"; + return "IfFileName(" + syntaxAndPattern + ")"; } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/e11e58c6/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileNameTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileNameTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileNameTest.java index afb8fd2..5b81cfb 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileNameTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/IfFileNameTest.java @@ -17,10 +17,9 @@ package org.apache.logging.log4j.core.appender.rolling.action; -import java.nio.file.FileSystems; import java.nio.file.Path; +import java.nio.file.Paths; -import org.apache.logging.log4j.core.appender.rolling.action.IfFileName; import org.junit.Test; import static org.junit.Assert.*; @@ -28,79 +27,53 @@ import static org.junit.Assert.*; public class IfFileNameTest { @Test(expected = IllegalArgumentException.class) - public void testCreateNameFilterFailsIfBothRegexAndPathAreNull() { - IfFileName.createNameFilter(null, null); + public void testCreateNameConditionFailsIfBothRegexAndPathAreNull() { + IfFileName.createNameCondition(null, null); } @Test() - public void testCreateNameFilterAcceptsIfEitherRegexOrPathOrBothAreNonNull() { - IfFileName.createNameFilter("bar", null); - IfFileName.createNameFilter(null, "foo"); - IfFileName.createNameFilter("bar", "foo"); + public void testCreateNameConditionAcceptsIfEitherRegexOrPathOrBothAreNonNull() { + IfFileName.createNameCondition("bar", null); + IfFileName.createNameCondition(null, "foo"); + IfFileName.createNameCondition("bar", "foo"); } @Test - public void testGetRegexReturnsConstructorValue() { - assertEquals("bar", IfFileName.createNameFilter(null, "bar").getRegex().pattern()); - assertEquals(null, IfFileName.createNameFilter("path", null).getRegex()); - } - - @Test - public void testGetPathReturnsConstructorValue() { - assertEquals("path", IfFileName.createNameFilter("path", null).getPathPattern()); - assertEquals(null, IfFileName.createNameFilter(null, "bar").getPathPattern()); + public void testGetSyntaxAndPattern() { + assertEquals("glob:path", IfFileName.createNameCondition("path", null).getSyntaxAndPattern()); + assertEquals("glob:path", IfFileName.createNameCondition("glob:path", null).getSyntaxAndPattern()); + assertEquals("regex:bar", IfFileName.createNameCondition(null, "bar").getSyntaxAndPattern()); + assertEquals("regex:bar", IfFileName.createNameCondition(null, "regex:bar").getSyntaxAndPattern()); } @Test public void testAcceptUsesPathPatternIfExists() { - final IfFileName filter = IfFileName.createNameFilter("path", "regex"); - final Path relativePath = FileSystems.getDefault().getPath("path"); + final IfFileName filter = IfFileName.createNameCondition("path", "regex"); + final Path relativePath = Paths.get("path"); assertTrue(filter.accept(null, relativePath, null)); - final Path pathMatchingRegex = FileSystems.getDefault().getPath("regex"); + final Path pathMatchingRegex = Paths.get("regex"); assertFalse(filter.accept(null, pathMatchingRegex, null)); } @Test public void testAcceptUsesRegexIfNoPathPatternExists() { - final IfFileName regexFilter = IfFileName.createNameFilter(null, "regex"); - final Path pathMatchingRegex = FileSystems.getDefault().getPath("regex"); + final IfFileName regexFilter = IfFileName.createNameCondition(null, "regex"); + final Path pathMatchingRegex = Paths.get("regex"); assertTrue(regexFilter.accept(null, pathMatchingRegex, null)); - final Path noMatch = FileSystems.getDefault().getPath("nomatch"); + final Path noMatch = Paths.get("nomatch"); assertFalse(regexFilter.accept(null, noMatch, null)); } @Test public void testAcceptIgnoresBasePathAndAttributes() { - final IfFileName pathFilter = IfFileName.createNameFilter("path", null); - final Path relativePath = FileSystems.getDefault().getPath("path"); + final IfFileName pathFilter = IfFileName.createNameCondition("path", null); + final Path relativePath = Paths.get("path"); assertTrue(pathFilter.accept(null, relativePath, null)); - final IfFileName regexFilter = IfFileName.createNameFilter(null, "regex"); - final Path pathMatchingRegex = FileSystems.getDefault().getPath("regex"); + final IfFileName regexFilter = IfFileName.createNameCondition(null, "regex"); + final Path pathMatchingRegex = Paths.get("regex"); assertTrue(regexFilter.accept(null, pathMatchingRegex, null)); } - - @Test - public void testIsMatch() { - assertTrue(IfFileName.isMatch("abc", "???")); - assertTrue(IfFileName.isMatch("abc", "a??")); - assertTrue(IfFileName.isMatch("abc", "?b?")); - assertTrue(IfFileName.isMatch("abc", "??c")); - assertTrue(IfFileName.isMatch("abc", "ab?")); - assertTrue(IfFileName.isMatch("abc", "?bc")); - assertTrue(IfFileName.isMatch("abc", "*")); - assertTrue(IfFileName.isMatch("abc", "*bc")); - assertTrue(IfFileName.isMatch("abc", "*c")); - assertTrue(IfFileName.isMatch("abc", "*c*")); - assertTrue(IfFileName.isMatch("abc", "a*")); - assertTrue(IfFileName.isMatch("abc", "*a*")); - assertTrue(IfFileName.isMatch("abc", "*b*")); - - assertFalse(IfFileName.isMatch("abc", "????")); - assertFalse(IfFileName.isMatch("abc", "b*")); - assertFalse(IfFileName.isMatch("abc", "*b")); - } - }
