You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by mi...@apache.org on 2016/06/03 09:41:43 UTC
[49/50] logging-log4j2 git commit: LOG4J2-997 - Add filter and remove
filter were not working properly in AbstractFilterable.
LOG4J2-997 - Add filter and remove filter were not working properly in AbstractFilterable.
Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/9acd705c
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/9acd705c
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/9acd705c
Branch: refs/heads/LOG4J-1181
Commit: 9acd705caaced6c50de0907a5592ea351ba69b27
Parents: d3b5841
Author: Ralph Goers <rg...@nextiva.com>
Authored: Thu Jun 2 23:41:20 2016 -0700
Committer: Ralph Goers <rg...@nextiva.com>
Committed: Thu Jun 2 23:41:20 2016 -0700
----------------------------------------------------------------------
.../log4j/core/filter/AbstractFilterable.java | 14 +-
.../log4j/core/filter/CompositeFilter.java | 27 +-
.../core/filter/AbstractFilterableTest.java | 281 +++++++++++++++++++
src/changes/changes.xml | 3 +
4 files changed, 316 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/9acd705c/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilterable.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilterable.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilterable.java
index 859a5aa..4e77c14 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilterable.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/AbstractFilterable.java
@@ -54,9 +54,12 @@ public abstract class AbstractFilterable extends AbstractLifeCycle implements Fi
*/
@Override
public synchronized void addFilter(final Filter filter) {
+ if (filter == null) {
+ return;
+ }
if (this.filter == null) {
this.filter = filter;
- } else if (filter instanceof CompositeFilter) {
+ } else if (this.filter instanceof CompositeFilter) {
this.filter = ((CompositeFilter) this.filter).addFilter(filter);
} else {
final Filter[] filters = new Filter[] {this.filter, filter};
@@ -70,10 +73,13 @@ public abstract class AbstractFilterable extends AbstractLifeCycle implements Fi
*/
@Override
public synchronized void removeFilter(final Filter filter) {
- if (this.filter == filter) {
+ if (this.filter == null || filter == null) {
+ return;
+ }
+ if (this.filter == filter || this.filter.equals(filter)) {
this.filter = null;
- } else if (filter instanceof CompositeFilter) {
- CompositeFilter composite = (CompositeFilter) filter;
+ } else if (this.filter instanceof CompositeFilter) {
+ CompositeFilter composite = (CompositeFilter) this.filter;
composite = composite.removeFilter(filter);
if (composite.size() > 1) {
this.filter = composite;
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/9acd705c/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
index 9e5754a..3933fa0 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/filter/CompositeFilter.java
@@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.filter;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
@@ -55,9 +56,19 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
// null does nothing
return this;
}
- final Filter[] copy = Arrays.copyOf(this.filters, this.filters.length + 1);
- copy[this.filters.length] = filter;
- return new CompositeFilter(copy);
+ if (filter instanceof CompositeFilter) {
+ int size = this.filters.length + ((CompositeFilter) filter).size();
+ final Filter[] copy = Arrays.copyOf(this.filters, size);
+ int index = this.filters.length;
+ for (Filter currentFilter : ((CompositeFilter) filter).filters) {
+ copy[index] = currentFilter;
+ }
+ return new CompositeFilter(copy);
+ } else {
+ final Filter[] copy = Arrays.copyOf(this.filters, this.filters.length + 1);
+ copy[this.filters.length] = filter;
+ return new CompositeFilter(copy);
+ }
}
public CompositeFilter removeFilter(final Filter filter) {
@@ -68,8 +79,14 @@ public final class CompositeFilter extends AbstractLifeCycle implements Iterable
// This is not a great implementation but simpler than copying Apache Commons
// Lang ArrayUtils.removeElement() and associated bits (MutableInt),
// which is OK since removing a filter should not be on the critical path.
- final List<Filter> filterList = Arrays.asList(this.filters);
- filterList.remove(filter);
+ final List<Filter> filterList = new ArrayList<>(Arrays.asList(this.filters));
+ if (filter instanceof CompositeFilter) {
+ for (Filter currentFilter : ((CompositeFilter) filter).filters) {
+ filterList.remove(currentFilter);
+ }
+ } else {
+ filterList.remove(filter);
+ }
return new CompositeFilter(filterList.toArray(new Filter[this.filters.length - 1]));
}
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/9acd705c/log4j-core/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java
----------------------------------------------------------------------
diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java
new file mode 100644
index 0000000..332b03c
--- /dev/null
+++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/filter/AbstractFilterableTest.java
@@ -0,0 +1,281 @@
+/* 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.filter;
+
+import static org.junit.Assert.*;
+
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Filter;
+import org.junit.Before;
+import org.junit.Test;
+
+public class AbstractFilterableTest {
+
+ MockedAbstractFilterable filterable;
+
+ @Before
+ public void setup() {
+ filterable = new MockedAbstractFilterable();
+ }
+
+ @Test
+ public void testAddSimpleFilter() throws Exception {
+ Filter filter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+
+ filterable.addFilter(filter);
+ assertSame(filter, filterable.getFilter());
+ }
+
+ @Test
+ public void testAddMultipleSimpleFilters() throws Exception {
+ Filter filter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+
+ filterable.addFilter(filter);
+ assertSame(filter, filterable.getFilter());
+ // adding a second filter converts the filter
+ // into a CompositeFilter.class
+ filterable.addFilter(filter);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ }
+
+ @Test
+ public void testAddMultipleEqualSimpleFilter() throws Exception {
+ Filter filter = new EqualFilter("test");
+
+ filterable.addFilter(filter);
+ assertSame(filter, filterable.getFilter());
+ // adding a second filter converts the filter
+ // into a CompositeFilter.class
+ filterable.addFilter(filter);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ }
+
+ @Test
+ public void testAddCompositeFilter() throws Exception {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[] {filter1, filter2});
+
+ filterable.addFilter(compositeFilter);
+ assertSame(compositeFilter, filterable.getFilter());
+ }
+
+ @Test
+ public void testAddMultipleCompositeFilters() throws Exception {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter3 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2, filter3});
+
+ filterable.addFilter(compositeFilter);
+ assertSame(compositeFilter, filterable.getFilter());
+ // adding a second filter converts the filter
+ // into a CompositeFilter.class
+ filterable.addFilter(compositeFilter);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(6, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ }
+
+ @Test
+ public void testAddSimpleFilterAndCompositeFilter() throws Exception {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter notInCompositeFilterFilter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2});
+
+ filterable.addFilter(notInCompositeFilterFilter);
+ assertSame(notInCompositeFilterFilter, filterable.getFilter());
+ // adding a second filter converts the filter
+ // into a CompositeFilter.class
+ filterable.addFilter(compositeFilter);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ }
+
+ @Test
+ public void testAddCompositeFilterAndSimpleFilter() throws Exception {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter notInCompositeFilterFilter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2});
+
+ filterable.addFilter(compositeFilter);
+ assertSame(compositeFilter, filterable.getFilter());
+ // adding a second filter converts the filter
+ // into a CompositeFilter.class
+ filterable.addFilter(notInCompositeFilterFilter);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ }
+
+ @Test
+ public void testRemoveSimpleFilterFromSimpleFilter() throws Exception {
+ Filter filter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+
+ filterable.addFilter(filter);
+ filterable.removeFilter(filter);
+ assertNull(filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveSimpleEqualFilterFromSimpleFilter() throws Exception {
+ Filter filterOriginal = new EqualFilter("test");
+ Filter filterCopy = new EqualFilter("test");
+
+ filterable.addFilter(filterOriginal);
+ filterable.removeFilter(filterCopy);
+ assertNull(filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveSimpleEqualFilterFromTwoSimpleFilters() throws Exception {
+ Filter filterOriginal = new EqualFilter("test");
+ Filter filterCopy = new EqualFilter("test");
+
+ filterable.addFilter(filterOriginal);
+ filterable.addFilter(filterOriginal);
+ filterable.removeFilter(filterCopy);
+ assertSame(filterOriginal, filterable.getFilter());
+ filterable.removeFilter(filterCopy);
+ assertNull(filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveSimpleEqualFilterFromMultipleSimpleFilters() throws Exception {
+ Filter filterOriginal = new EqualFilter("test");
+ Filter filterCopy = new EqualFilter("test");
+
+ filterable.addFilter(filterOriginal);
+ filterable.addFilter(filterOriginal);
+ filterable.addFilter(filterCopy);
+ filterable.removeFilter(filterCopy);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ filterable.removeFilter(filterCopy);
+ assertEquals(filterOriginal, filterable.getFilter());
+ filterable.removeFilter(filterOriginal);
+ assertNull(filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveNullFromSingleSimpleFilter() throws Exception {
+ Filter filter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+
+ filterable.addFilter(filter);
+ filterable.removeFilter(null);
+ assertSame(filter, filterable.getFilter());
+ }
+
+
+ @Test
+ public void testRemoveNonExistingFilterFromSingleSimpleFilter() throws Exception {
+ Filter filter = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter newFilter = ThresholdFilter.createFilter(Level.WARN, null, null);
+
+ filterable.addFilter(filter);
+ filterable.removeFilter(newFilter);
+ assertSame(filter, filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveSimpleFilterFromCompositeFilter() {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2});
+
+ filterable.addFilter(compositeFilter);
+
+ // should remove internal filter of compositeFilter
+ filterable.removeFilter(filter1);
+ assertFalse(filterable.getFilter() instanceof CompositeFilter);
+
+ assertEquals(filter2, filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveSimpleFilterFromCompositeAndSimpleFilter() {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2});
+ Filter anotherFilter = ThresholdFilter.createFilter(Level.WARN, null, null);
+
+
+ filterable.addFilter(compositeFilter);
+ filterable.addFilter(anotherFilter);
+
+ // should not remove internal filter of compositeFilter
+ filterable.removeFilter(anotherFilter);
+ assertTrue(filterable.getFilter() instanceof CompositeFilter);
+ assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ }
+
+ @Test
+ public void testRemoveCompositeFilterFromCompositeFilter() {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2});
+
+ filterable.addFilter(compositeFilter);
+ filterable.removeFilter(compositeFilter);
+ assertNull(filterable.getFilter());
+ }
+
+ @Test
+ public void testRemoveFiltersFromComposite() {
+ Filter filter1 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter filter2 = ThresholdFilter.createFilter(Level.ERROR, null, null);
+ Filter compositeFilter = CompositeFilter.createFilters(new Filter[]{filter1, filter2});
+ Filter anotherFilter = ThresholdFilter.createFilter(Level.WARN, null, null);
+
+ filterable.addFilter(compositeFilter);
+ filterable.addFilter(anotherFilter);
+ assertEquals(3, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ filterable.removeFilter(filter1);
+ assertEquals(2, ((CompositeFilter) filterable.getFilter()).getFilters().size());
+ filterable.removeFilter(filter2);
+ assertSame(anotherFilter, filterable.getFilter());
+ }
+
+ private class MockedAbstractFilterable extends AbstractFilterable {}
+
+ private class EqualFilter extends AbstractFilter {
+ private final String key;
+ public EqualFilter(String key) {
+ this.key = key;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (!(o instanceof EqualFilter)) return false;
+
+ EqualFilter that = (EqualFilter) o;
+
+ if (key != null ? !key.equals(that.key) : that.key != null) return false;
+
+ return true;
+ }
+
+ @Override
+ public int hashCode() {
+ return key != null ? key.hashCode() : 0;
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/9acd705c/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 046e4a1..a0d1493 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -24,6 +24,9 @@
</properties>
<body>
<release version="2.6.1" date="2016-MM-DD" description="GA Release 2.6.1">
+ <action issue="LOG4J2-997" dev="rgoers" type="fix" due-to="Maytee Chinavanichkit">
+ Add filter and remove filter were not working properly in AbstractFilterable.
+ </action>
<action issue="LOG4J2-1032" dev="rgoers" type="fix">
Change RenameAction to use java.nio to better report rename failures.
</action>