Updated. On 23 September 2016 at 13:08, Remko Popma <[email protected]> wrote:
> Good catch! Can you help? 3am again here... > > Sent from my iPhone > > On 2016/09/24, at 2:55, Matt Sicker <[email protected]> wrote: > > 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/c > ceb1808/log4j-core/src/main/java/org/apache/logging/log4j/co > re/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/imp > l/ContextDataFactory.java > index 49d2b97..9b48dfd 100644 > --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/imp > l/ContextDataFactory.java > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/imp > l/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/c > ceb1808/log4j-core/src/main/java/org/apache/logging/log4j/co > re/impl/ThreadContextDataInjector.java > ---------------------------------------------------------------------- > diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/imp > l/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/imp > l/ThreadContextDataInjector.java > +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/imp > l/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/c > ceb1808/log4j-core/src/test/java/org/apache/logging/log4j/co > re/impl/ContextDataFactoryPropertySetMissingConstructorTest.java > ---------------------------------------------------------------------- > diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/ContextDataFactoryPropertySetMissingConstructorTest.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/ContextDataFactoryPropertySetMissingConstructorTest.java > new file mode 100644 > index 0000000..6e8d621 > --- /dev/null > +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/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.get > DeclaredField("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/c > ceb1808/log4j-core/src/test/java/org/apache/logging/log4j/co > re/impl/ContextDataFactoryPropertySetTest.java > ---------------------------------------------------------------------- > diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/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/imp > l/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/c > ceb1808/log4j-core/src/test/java/org/apache/logging/log4j/co > re/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/imp > l/ContextDataFactoryTest.java > new file mode 100644 > index 0000000..aca1c27 > --- /dev/null > +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/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.get > DeclaredField("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/c > ceb1808/log4j-core/src/test/java/org/apache/logging/log4j/co > re/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/imp > l/FactoryTestStringMap.java > new file mode 100644 > index 0000000..9f7fcc7 > --- /dev/null > +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/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/c > ceb1808/log4j-core/src/test/java/org/apache/logging/log4j/co > re/impl/FactoryTestStringMapWithoutIntConstructor.java > ---------------------------------------------------------------------- > diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/FactoryTestStringMapWithoutIntConstructor.java > b/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/FactoryTestStringMapWithoutIntConstructor.java > new file mode 100644 > index 0000000..c87e729 > --- /dev/null > +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/imp > l/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]> > > -- Matt Sicker <[email protected]>
