Removed @PluginAttribute 'patternFlags' argument from @PluginFactory for RegexFilter. (#3086)

log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterTest.java

@@ -34,7 +34,6 @@ class AbstractFilterTest {
     @Test
     void testUnrolledBackwardsCompatible() {
         final ConcreteFilter filter = new ConcreteFilter();
-        final Filter.Result expected = Filter.Result.DENY;
         verifyMethodsWithUnrolledVarargs(filter, Filter.Result.DENY);
 
         filter.testResult = Filter.Result.ACCEPT;

log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java

@@ -54,7 +54,7 @@ void testAddMultipleSimpleFilters() {
         // into a CompositeFilter.class
         filterable.addFilter(filter);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
     @Test
@@ -67,7 +67,7 @@ void testAddMultipleEqualSimpleFilter() {
         // into a CompositeFilter.class
         filterable.addFilter(filter);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
     @Test
@@ -93,7 +93,7 @@ void testAddMultipleCompositeFilters() {
         // into a CompositeFilter.class
         filterable.addFilter(compositeFilter);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
     @Test
@@ -109,7 +109,7 @@ void testAddSimpleFilterAndCompositeFilter() {
         // into a CompositeFilter.class
         filterable.addFilter(compositeFilter);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
     @Test
@@ -125,7 +125,7 @@ void testAddCompositeFilterAndSimpleFilter() {
         // into a CompositeFilter.class
         filterable.addFilter(notInCompositeFilterFilter);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
     @Test
@@ -170,7 +170,7 @@ void testRemoveSimpleEqualFilterFromMultipleSimpleFilters() {
         filterable.addFilter(filterCopy);
         filterable.removeFilter(filterCopy);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
         filterable.removeFilter(filterCopy);
         assertEquals(filterOriginal, filterable.getFilter());
         filterable.removeFilter(filterOriginal);
@@ -224,7 +224,7 @@ void testRemoveSimpleFilterFromCompositeAndSimpleFilter() {
         // should not remove internal filter of compositeFilter
         filterable.removeFilter(anotherFilter);
         assertInstanceOf(CompositeFilter.class, filterable.getFilter());
-        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
     }
 
     @Test
@@ -247,9 +247,9 @@ void testRemoveFiltersFromComposite() {
 
         filterable.addFilter(compositeFilter);
         filterable.addFilter(anotherFilter);
-        assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
         filterable.removeFilter(filter1);
-        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+        assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFiltersArray().length);
         filterable.removeFilter(filter2);
         assertSame(anotherFilter, filterable.getFilter());
     }
@@ -274,11 +274,7 @@ public boolean equals(final Object o) {
 
             final EqualFilter that = (EqualFilter) o;
 
-            if (key != null ? !key.equals(that.key) : that.key != null) {
-                return false;
-            }
-
-            return true;
+            return key != null ? key.equals(that.key) : that.key == null;
         }
 
         @Override

log4j-core-test/src/test/java/org/apache/logging/log4j/core/filter/RegexFilterTest.java

@@ -19,6 +19,9 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -37,19 +40,23 @@
 class RegexFilterTest {
     @BeforeAll
     static void before() {
-        StatusLogger.getLogger().setLevel(Level.OFF);
+        StatusLogger.getLogger().getFallbackListener().setLevel(Level.OFF);
     }
 
     @Test
     void testRegexFilterDoesNotThrowWithAllTheParametersExceptRegexEqualNull() {
         assertDoesNotThrow(() -> {
-            RegexFilter.createFilter(".* test .*", null, null, null, null);
+            RegexFilter.newBuilder().setRegex(".* test .*").build();
         });
     }
 
     @Test
     void testThresholds() throws Exception {
-        RegexFilter filter = RegexFilter.createFilter(".* test .*", null, false, null, null);
+        RegexFilter filter = RegexFilter.newBuilder()
+                .setRegex(".* test .*")
+                .setUseRawMsg(false)
+                .build();
+        assertNotNull(filter);
         filter.start();
         assertTrue(filter.isStarted());
         assertSame(
@@ -65,7 +72,7 @@ void testThresholds() throws Exception {
                 .setMessage(new SimpleMessage("test")) //
                 .build();
         assertSame(Filter.Result.DENY, filter.filter(event));
-        filter = RegexFilter.createFilter(null, null, false, null, null);
+        filter = RegexFilter.newBuilder().build();
         assertNull(filter);
     }
 
@@ -82,38 +89,123 @@ void testDotAllPattern() throws Exception {
     }
 
     @Test
-    void testNoMsg() throws Exception {
-        final RegexFilter filter = RegexFilter.createFilter(".* test .*", null, false, null, null);
+    void testNoMsg() {
+
+        final RegexFilter filter = RegexFilter.newBuilder()
+                .setRegex(".* test .*")
+                .setUseRawMsg(false)
+                .build();
+
+        assertNotNull(filter);
+
         filter.start();
+
         assertTrue(filter.isStarted());
         assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, (Object) null, null));
         assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, (Message) null, null));
         assertSame(Filter.Result.DENY, filter.filter(null, Level.DEBUG, null, null, (Object[]) null));
     }
 
     @Test
-    void testParameterizedMsg() throws Exception {
+    void testParameterizedMsg() {
         final String msg = "params {} {}";
         final Object[] params = {"foo", "bar"};
 
         // match against raw message
-        final RegexFilter rawFilter = RegexFilter.createFilter(
-                "params \\{\\} \\{\\}",
-                null,
-                true, // useRawMsg
-                Result.ACCEPT,
-                Result.DENY);
+        final RegexFilter rawFilter = RegexFilter.newBuilder()
+                .setRegex("params \\{\\} \\{\\}")
+                .setUseRawMsg(true)
+                .setOnMatch(Result.ACCEPT)
+                .setOnMismatch(Result.DENY)
+                .build();
+
+        assertNotNull(rawFilter);
+
         final Result rawResult = rawFilter.filter(null, null, null, msg, params);
         assertThat(rawResult, equalTo(Result.ACCEPT));
 
         // match against formatted message
-        final RegexFilter fmtFilter = RegexFilter.createFilter(
-                "params foo bar",
-                null,
-                false, // useRawMsg
-                Result.ACCEPT,
-                Result.DENY);
+        final RegexFilter fmtFilter = RegexFilter.newBuilder()
+                .setRegex("params foo bar")
+                .setUseRawMsg(false)
+                .setOnMatch(Result.ACCEPT)
+                .setOnMismatch(Result.DENY)
+                .build();
+
+        assertNotNull(fmtFilter);
+
         final Result fmtResult = fmtFilter.filter(null, null, null, msg, params);
         assertThat(fmtResult, equalTo(Result.ACCEPT));
     }
+
+    /**
+     * A builder with no 'regex' expression should both be invalid and return null on 'build()'.
+     */
+    @Test
+    void testWithValidRegex() {
+
+        final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores
+
+        final RegexFilter.Builder builder = RegexFilter.newBuilder()
+                .setRegex(regex)
+                .setUseRawMsg(false)
+                .setOnMatch(Result.ACCEPT)
+                .setOnMismatch(Result.DENY);
+
+        final RegexFilter filter = builder.build();
+
+        assertNotNull(filter);
+
+        assertEquals(Result.ACCEPT, filter.filter("Hello_123"));
+
+        assertEquals(Result.DENY, filter.filter("Hello@123"));
+
+        assertEquals(regex, filter.getRegex());
+    }
+
+    @Test
+    void testRegexFilterGetters() {
+
+        final String regex = "^[a-zA-Z0-9_]+$"; // matches alphanumeric with underscores
+
+        final RegexFilter filter = RegexFilter.newBuilder()
+                .setRegex(regex)
+                .setUseRawMsg(false)
+                .setOnMatch(Result.ACCEPT)
+                .setOnMismatch(Result.DENY)
+                .build();
+
+        assertNotNull(filter);
+
+        assertEquals(regex, filter.getRegex());
+        assertFalse(filter.isUseRawMessage());
+        assertEquals(Result.ACCEPT, filter.getOnMatch());
+        assertEquals(Result.DENY, filter.getOnMismatch());
+        assertNotNull(filter.getPattern());
+        assertEquals(regex, filter.getPattern().pattern());
+    }
+
+    /**
+     * A builder with no 'regex' expression should both be invalid and return null on 'build()'.
+     */
+    @Test
+    void testBuilderWithoutRegexNotValid() {
+
+        final RegexFilter.Builder builder = RegexFilter.newBuilder();
+
+        assertNull(builder.build());
+    }
+
+    /**
+     * A builder with an invalid 'regex' expression should return null on 'build()'.
+     */
+    @Test
+    void testBuilderWithInvalidRegexNotValid() {
+
+        final RegexFilter.Builder builder = RegexFilter.newBuilder();
+
+        builder.setRegex("[a-z");
+
+        assertNull(builder.build());
+    }
 }

log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilter.java

@@ -16,6 +16,8 @@
  */
 package org.apache.logging.log4j.core.filter;
 
+import java.util.Objects;
+import java.util.Optional;
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.Marker;
 import org.apache.logging.log4j.core.AbstractLifeCycle;
@@ -43,16 +45,30 @@ public abstract static class AbstractFilterBuilder<B extends AbstractFilterBuild
         public static final String ATTR_ON_MISMATCH = "onMismatch";
         public static final String ATTR_ON_MATCH = "onMatch";
 
+        /**
+         * The action to perform when a match occurs.
+         */
         @PluginBuilderAttribute(ATTR_ON_MATCH)
-        private Result onMatch = Result.NEUTRAL;
+        protected Result onMatch = Result.NEUTRAL;
 
+        /**
+         * The action to perform when a mismatch occurs.
+         */
         @PluginBuilderAttribute(ATTR_ON_MISMATCH)
-        private Result onMismatch = Result.DENY;
+        protected Result onMismatch = Result.DENY;
 
+        /**
+         * Returns the action to apply when a match occurs
+         * @return the match result
+         */
         public Result getOnMatch() {
             return onMatch;
         }
 
+        /**
+         * Returns the action to apply when a mismatch occurs
+         * @return the mismatch result
+         */
         public Result getOnMismatch() {
             return onMismatch;
         }
@@ -110,6 +126,19 @@ protected AbstractFilter(final Result onMatch, final Result onMismatch) {
         this.onMismatch = onMismatch == null ? Result.DENY : onMismatch;
     }
 
+    /**
+     * Constructs a new instance configured by the given builder
+     * @param builder the builder
+     * @throws NullPointerException if the builder argument is {@code null}
+     */
+    protected AbstractFilter(final AbstractFilterBuilder<?> builder) {
+
+        Objects.requireNonNull(builder, "The 'builder' argument cannot be null.");
+
+        this.onMatch = Optional.ofNullable(builder.onMatch).orElse(Result.NEUTRAL);
+        this.onMismatch = Optional.ofNullable(builder.onMismatch).orElse(Result.DENY);
+    }
+
     @Override
     protected boolean equalsImpl(final Object obj) {
         if (this == obj) {