You should avoid using Class.forName(). We have LoaderUtil.loadClass() for this.
---------- Forwarded message ---------- From: <[email protected]> Date: 23 September 2016 at 11:56 Subject: logging-log4j2 git commit: LOG4J2-1611 LOG4J2-1010 LOG4J2-1447 context injector should use ContextDataFactory; ContextDataFactory now supports pre-sizing To: [email protected] Repository: logging-log4j2 Updated Branches: refs/heads/master a22b7ae64 -> cceb1808d LOG4J2-1611 LOG4J2-1010 LOG4J2-1447 context injector should use ContextDataFactory; ContextDataFactory now supports pre-sizing Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/ commit/cceb1808 Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/cceb1808 Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/cceb1808 Branch: refs/heads/master Commit: cceb1808df85bdd0e0093a47cf8d010268532622 Parents: a22b7ae Author: rpopma <[email protected]> Authored: Sat Sep 24 01:56:27 2016 +0900 Committer: rpopma <[email protected]> Committed: Sat Sep 24 01:56:27 2016 +0900 ---------------------------------------------------------------------- .../log4j/core/impl/ContextDataFactory.java | 53 ++++++++-- .../core/impl/ThreadContextDataInjector.java | 7 +- ...actoryPropertySetMissingConstructorTest.java | 41 ++++++++ .../impl/ContextDataFactoryPropertySetTest.java | 52 ++++++++++ .../log4j/core/impl/ContextDataFactoryTest.java | 47 +++++++++ .../log4j/core/impl/FactoryTestStringMap.java | 103 +++++++++++++++++++ ...ctoryTestStringMapWithoutIntConstructor.java | 99 ++++++++++++++++++ 7 files changed, 392 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/main/java/org/apache/logging/log4j/ core/impl/ContextDataFactory.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ContextDataFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/ impl/ContextDataFactory.java index 49d2b97..9b48dfd 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/ impl/ContextDataFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/ impl/ContextDataFactory.java @@ -16,11 +16,13 @@ */ package org.apache.logging.log4j.core.impl; -import org.apache.logging.log4j.core.LogEvent; +import java.lang.reflect.Constructor; + import org.apache.logging.log4j.core.ContextDataInjector; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.util.PropertiesUtil; import org.apache.logging.log4j.util.SortedArrayStringMap; import org.apache.logging.log4j.util.StringMap; -import org.apache.logging.log4j.util.PropertiesUtil; /** * Factory for creating the StringMap instances used to initialize LogEvents' @@ -30,7 +32,8 @@ import org.apache.logging.log4j.util.PropertiesUtil; * <p> * By default returns {@code SortedArrayStringMap} objects. Can be configured by setting system property * {@code "log4j2.ContextData"} to the fully qualified class name of a class implementing the - * {@code StringMap} interface. The class must have a public default constructor. + * {@code StringMap} interface. The class must have a public default constructor, and if possible should also have a + * public constructor that takes a single {@code int} argument for the initial capacity. * </p> * * @see LogEvent#getContextData() @@ -39,15 +42,53 @@ import org.apache.logging.log4j.util.PropertiesUtil; * @since 2.7 */ public class ContextDataFactory { + private static final String CLASS_NAME = PropertiesUtil.getProperties() .getStringProperty("log4j2.ContextData"); + private static final Class<?> CACHED_CLASS = createCachedClass(CLASS_NAME); + private static final Constructor<?> CACHED_CONSTRUCTOR = createCachedConstructor(CACHED_CLASS); + + private static Class<?> createCachedClass(final String className) { + if (className == null) { + return null; + } + try { + return Class.forName(className); + } catch (final Exception any) { + return null; + } + } + + private static Constructor<?> createCachedConstructor(final Class<?> cachedClass) { + if (cachedClass == null) { + return null; + } + try { + return cachedClass.getDeclaredConstructor(int.class); + } catch (final Exception any) { + return null; + } + } @SuppressWarnings("unchecked") public static StringMap createContextData() { - final String CLASS = PropertiesUtil.getProperties() .getStringProperty("log4j2.ContextData", - SortedArrayStringMap.class.getName()); + if (CACHED_CLASS == null) { + return new SortedArrayStringMap(); + } try { - return (StringMap) Class.forName(CLASS).newInstance(); + return (StringMap) CACHED_CLASS.newInstance(); } catch (final Exception any) { return new SortedArrayStringMap(); } } + + @SuppressWarnings("unchecked") + public static StringMap createContextData(final int initialCapacity) { + if (CACHED_CONSTRUCTOR == null) { + return new SortedArrayStringMap(initialCapacity); + } + try { + return (StringMap) CACHED_CONSTRUCTOR. newInstance(initialCapacity); + } catch (final Exception any) { + return new SortedArrayStringMap(initialCapacity); + } + } } http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ ThreadContextDataInjector.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjector.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ ThreadContextDataInjector.java index 698d53a..832a8cb 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ ThreadContextDataInjector.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/impl/ ThreadContextDataInjector.java @@ -23,10 +23,9 @@ import org.apache.logging.log4j.ThreadContext; import org.apache.logging.log4j.ThreadContextAccess; import org.apache.logging.log4j.core.ContextDataInjector; import org.apache.logging.log4j.core.config.Property; +import org.apache.logging.log4j.spi.ThreadContextMap; import org.apache.logging.log4j.util.ReadOnlyStringMap; -import org.apache.logging.log4j.util.SortedArrayStringMap; import org.apache.logging.log4j.util.StringMap; -import org.apache.logging.log4j.spi.ThreadContextMap; /** * {@code ThreadContextDataInjector} contains a number of strategies for copying key-value pairs from the various @@ -82,7 +81,7 @@ public class ThreadContextDataInjector { // data. Note that we cannot reuse the specified StringMap: some Loggers may have properties defined // and others not, so the LogEvent's context data may have been replaced with an immutable copy from // the ThreadContext - this will throw an UnsupportedOperationException if we try to modify it. - final StringMap result = new SortedArrayStringMap(props.size() + copy.size()); + final StringMap result = ContextDataFactory.createContextData(props.size() + copy.size()); copyProperties(props, result); copyThreadContextMap(copy, result); return result; @@ -175,7 +174,7 @@ public class ThreadContextDataInjector { // data. Note that we cannot reuse the specified StringMap: some Loggers may have properties defined // and others not, so the LogEvent's context data may have been replaced with an immutable copy from // the ThreadContext - this will throw an UnsupportedOperationException if we try to modify it. - final StringMap result = new SortedArrayStringMap(props.size() + immutableCopy.size()); + final StringMap result = ContextDataFactory.createContextData(props.size() + immutableCopy.size()); copyProperties(props, result); result.putAll(immutableCopy); return result; http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetMissingConstructorTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetMissingConstructorTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetMissingConstructorTest.java new file mode 100644 index 0000000..6e8d621 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetMissingConstructorTest.java @@ -0,0 +1,41 @@ +/* + * 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.impl; + +import java.lang.reflect.Field; + +import org.apache.logging.log4j.util.SortedArrayStringMap; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Tests the ContextDataFactory class. + */ +public class ContextDataFactoryPropertySetMissingConstructorTest { + + @Test + public void intArgReturnsSortedArrayString MapIfPropertySpecifiedButMissingIntConstructor() throws Exception { + System.setProperty("log4j2.ContextData", FactoryTestStringMapWithoutIntConstructor.class.getName()); + assertTrue(ContextDataFactory.createContextData(2) instanceof SortedArrayStringMap); + SortedArrayStringMap actual = (SortedArrayStringMap) ContextDataFactory.createContextData(2); + Field thresholdField = SortedArrayStringMap.class. getDeclaredField("threshold"); + thresholdField.setAccessible(true); + assertEquals(2, thresholdField.getInt(actual)); + System.clearProperty("log4j2.ContextData"); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetTest.java b/log4j-core/src/test/java/ org/apache/logging/log4j/core/impl/ContextDataFactoryPropertySetTest.java new file mode 100644 index 0000000..b5bb3b2 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryPropertySetTest.java @@ -0,0 +1,52 @@ +/* + * 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.impl; + +import java.lang.reflect.Field; + +import org.apache.logging.log4j.util.SortedArrayStringMap; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Tests the ContextDataFactory class. + */ +public class ContextDataFactoryPropertySetTest { + + @Test + public void noArgReturnsSpecifiedImplIfPropertySpecified() throws Exception { + System.setProperty("log4j2.ContextData", FactoryTestStringMap.class.getName()); + assertTrue(ContextDataFactory.createContextData() instanceof FactoryTestStringMap); + System.clearProperty("log4j2.ContextData"); + } + + @Test + public void intArgReturnsSpecifiedImplIfPropertySpecified() throws Exception { + System.setProperty("log4j2.ContextData", FactoryTestStringMap.class.getName()); + assertTrue(ContextDataFactory.createContextData(2) instanceof FactoryTestStringMap); + System.clearProperty("log4j2.ContextData"); + } + + @Test + public void intArgSetsCapacityIfPropertySpecified() throws Exception { + System.setProperty("log4j2.ContextData", FactoryTestStringMap.class.getName()); + FactoryTestStringMap actual = (FactoryTestStringMap) ContextDataFactory.createContextData(2); + assertEquals(2, actual.initialCapacity); + System.clearProperty("log4j2.ContextData"); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ ContextDataFactoryTest.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/ impl/ContextDataFactoryTest.java b/log4j-core/src/test/java/ org/apache/logging/log4j/core/impl/ContextDataFactoryTest.java new file mode 100644 index 0000000..aca1c27 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/ impl/ContextDataFactoryTest.java @@ -0,0 +1,47 @@ +/* + * 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.impl; + +import java.lang.reflect.Field; + +import org.apache.logging.log4j.util.SortedArrayStringMap; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Tests the ContextDataFactory class. + */ +public class ContextDataFactoryTest { + @Test + public void noArgReturnsSortedArrayStringMapIfNoPropertySpecified() throws Exception { + assertTrue(ContextDataFactory.createContextData() instanceof SortedArrayStringMap); + } + + @Test + public void intArgReturnsSortedArrayStringMapIfNoPropertySpecified() throws Exception { + assertTrue(ContextDataFactory.createContextData(2) instanceof SortedArrayStringMap); + } + + @Test + public void intArgSetsCapacityIfNoPropertySpecified() throws Exception { + SortedArrayStringMap actual = (SortedArrayStringMap) ContextDataFactory.createContextData(2); + Field thresholdField = SortedArrayStringMap.class. getDeclaredField("threshold"); + thresholdField.setAccessible(true); + assertEquals(2, thresholdField.getInt(actual)); + } +} \ No newline at end of file http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ FactoryTestStringMap.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/FactoryTestStringMap.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/ impl/FactoryTestStringMap.java new file mode 100644 index 0000000..9f7fcc7 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/ impl/FactoryTestStringMap.java @@ -0,0 +1,103 @@ +/* + * 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.impl; + +import java.util.Map; + +import org.apache.logging.log4j.util.BiConsumer; +import org.apache.logging.log4j.util.ReadOnlyStringMap; +import org.apache.logging.log4j.util.StringMap; +import org.apache.logging.log4j.util.TriConsumer; + +/** + * Dummy implementation of the StringMap interface for testing. + */ +public class FactoryTestStringMap implements StringMap { + int initialCapacity; + + public FactoryTestStringMap() { + } + + public FactoryTestStringMap(final int initialCapacity) { + this.initialCapacity = initialCapacity; + } + + @Override + public Map<String, String> toMap() { + return null; + } + + @Override + public boolean containsKey(final String key) { + return false; + } + + @Override + public <V> void forEach(final BiConsumer<String, ? super V> action) { + + } + + @Override + public <V, S> void forEach(final TriConsumer<String, ? super V, S> action, final S state) { + + } + + @Override + public <V> V getValue(final String key) { + return null; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public int size() { + return 0; + } + + @Override + public void clear() { + + } + + @Override + public void freeze() { + + } + + @Override + public boolean isFrozen() { + return false; + } + + @Override + public void putAll(final ReadOnlyStringMap source) { + + } + + @Override + public void putValue(final String key, final Object value) { + + } + + @Override + public void remove(final String key) { + + } +} http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/ cceb1808/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ FactoryTestStringMapWithoutIntConstructor.java ---------------------------------------------------------------------- diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ FactoryTestStringMapWithoutIntConstructor.java b/log4j-core/src/test/java/ org/apache/logging/log4j/core/impl/FactoryTestStringMapWithoutInt Constructor.java new file mode 100644 index 0000000..c87e729 --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/impl/ FactoryTestStringMapWithoutIntConstructor.java @@ -0,0 +1,99 @@ +/* + * 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.impl; + +import java.util.Map; + +import org.apache.logging.log4j.util.BiConsumer; +import org.apache.logging.log4j.util.ReadOnlyStringMap; +import org.apache.logging.log4j.util.StringMap; +import org.apache.logging.log4j.util.TriConsumer; + +/** + * Dummy implementation of the StringMap interface for testing. + */ +public class FactoryTestStringMapWithoutIntConstructor implements StringMap { + int initialCapacity; + + public FactoryTestStringMapWithoutIntConstructor() { + } + + @Override + public Map<String, String> toMap() { + return null; + } + + @Override + public boolean containsKey(final String key) { + return false; + } + + @Override + public <V> void forEach(final BiConsumer<String, ? super V> action) { + + } + + @Override + public <V, S> void forEach(final TriConsumer<String, ? super V, S> action, final S state) { + + } + + @Override + public <V> V getValue(final String key) { + return null; + } + + @Override + public boolean isEmpty() { + return false; + } + + @Override + public int size() { + return 0; + } + + @Override + public void clear() { + + } + + @Override + public void freeze() { + + } + + @Override + public boolean isFrozen() { + return false; + } + + @Override + public void putAll(final ReadOnlyStringMap source) { + + } + + @Override + public void putValue(final String key, final Object value) { + + } + + @Override + public void remove(final String key) { + + } +} -- Matt Sicker <[email protected]>
