This is an automated email from the ASF dual-hosted git repository. ggregory pushed a commit to branch release-2.x in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
commit 05db5f9527254632b59aed2a1d78a32c5ab74f16 Author: Gary Gregory <[email protected]> AuthorDate: Mon Dec 27 12:42:26 2021 -0500 Refactor to reuse existing code. --- .../db/jdbc/DataSourceConnectionSource.java | 20 ++++++----- .../apache/logging/log4j/core/net/JndiManager.java | 9 +++-- .../jdbc/AbstractJdbcAppenderDataSourceTest.java | 13 +++++--- .../db/jdbc/AbstractJdbcDataSourceTest.java | 39 ++++++++++++++++++++++ .../db/jdbc/DataSourceConnectionSourceTest.java | 2 +- .../jdbc/JdbcAppenderMapMessageDataSourceTest.java | 2 +- .../log4j/core/appender/mom/JmsAppenderTest.java | 3 +- .../logging/log4j/core/net/JndiManagerTest.java | 9 +++-- src/site/xdoc/manual/appenders.xml | 10 ++++-- src/site/xdoc/manual/configuration.xml.vm | 12 +++++-- 10 files changed, 93 insertions(+), 26 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java index a791df9..6a55a84 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSource.java @@ -18,8 +18,8 @@ package org.apache.logging.log4j.core.appender.db.jdbc; import java.sql.Connection; import java.sql.SQLException; +import java.util.Objects; -import javax.naming.InitialContext; import javax.naming.NamingException; import javax.sql.DataSource; @@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.Core; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttribute; import org.apache.logging.log4j.core.config.plugins.PluginFactory; +import org.apache.logging.log4j.core.net.JndiManager; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.Strings; @@ -42,7 +43,7 @@ public final class DataSourceConnectionSource extends AbstractConnectionSource { private final String description; private DataSourceConnectionSource(final String dataSourceName, final DataSource dataSource) { - this.dataSource = dataSource; + this.dataSource = Objects.requireNonNull(dataSource, "dataSource"); this.description = "dataSource{ name=" + dataSourceName + ", value=" + dataSource + " }"; } @@ -59,25 +60,26 @@ public final class DataSourceConnectionSource extends AbstractConnectionSource { /** * Factory method for creating a connection source within the plugin manager. * - * @param jndiName The full JNDI path where the data source is bound. Should start with java:/comp/env or - * environment-equivalent. + * @param jndiName The full JNDI path where the data source is bound. Must start with java:/comp/env or environment-equivalent. * @return the created connection source. */ @PluginFactory public static DataSourceConnectionSource createConnectionSource(@PluginAttribute("jndiName") final String jndiName) { + if (!JndiManager.isJndiJdbcEnabled()) { + LOGGER.error("JNDI must be enabled by setting log4j2.enableJndiJdbc=true"); + return null; + } if (Strings.isEmpty(jndiName)) { LOGGER.error("No JNDI name provided."); return null; } - try { - final InitialContext context = new InitialContext(); - final DataSource dataSource = (DataSource) context.lookup(jndiName); + @SuppressWarnings("resource") + final DataSource dataSource = JndiManager.getDefaultManager(DataSourceConnectionSource.class.getCanonicalName()).lookup(jndiName); if (dataSource == null) { - LOGGER.error("No data source found with JNDI name [" + jndiName + "]."); + LOGGER.error("No DataSource found with JNDI name [" + jndiName + "]."); return null; } - return new DataSourceConnectionSource(jndiName, dataSource); } catch (final NamingException e) { LOGGER.error(e.getMessage(), e); diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java index d73d413..5e807b2 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/net/JndiManager.java @@ -43,6 +43,7 @@ public class JndiManager extends AbstractManager { private static final String JAVA_SCHEME = "java"; private static final boolean JNDI_CONTEXT_SELECTOR_ENABLED = isJndiEnabled("ContextSelector"); + private static final boolean JNDI_JDBC_ENABLED = isJndiEnabled("Jdbc"); private static final boolean JNDI_JMS_ENABLED = isJndiEnabled("Jms"); private static final boolean JNDI_LOOKUP_ENABLED = isJndiEnabled("Lookup"); @@ -53,13 +54,17 @@ public class JndiManager extends AbstractManager { } public static boolean isJndiEnabled() { - return isJndiContextSelectorEnabled() || isJndiJmsEnabled() || isJndiLookupEnabled(); + return isJndiContextSelectorEnabled() || isJndiJdbcEnabled() || isJndiJmsEnabled() || isJndiLookupEnabled(); } public static boolean isJndiContextSelectorEnabled() { return JNDI_CONTEXT_SELECTOR_ENABLED; } + public static boolean isJndiJdbcEnabled() { + return JNDI_JDBC_ENABLED; + } + public static boolean isJndiJmsEnabled() { return JNDI_JMS_ENABLED; } @@ -208,7 +213,7 @@ public class JndiManager extends AbstractManager { } LOGGER.warn("Unsupported JNDI URI - {}", name); } catch (URISyntaxException ex) { - LOGGER.warn("Invalid JNDI URI - {}", name); + LOGGER.warn("Invalid JNDI URI - {}", name); } return null; } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java index c4a7312..adf9e90 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcAppenderDataSourceTest.java @@ -16,12 +16,19 @@ */ package org.apache.logging.log4j.core.appender.db.jdbc; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.BDDMockito.given; +import static org.mockito.Mockito.mock; + import java.io.ByteArrayOutputStream; import java.io.PrintWriter; import java.sql.Connection; import java.sql.ResultSet; import java.sql.SQLException; import java.sql.Statement; + import javax.sql.DataSource; import org.apache.logging.log4j.LogManager; @@ -35,14 +42,10 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.RuleChain; -import static org.junit.Assert.*; -import static org.mockito.BDDMockito.given; -import static org.mockito.Mockito.mock; - /** * Abstract unit test for JdbcAppender using a {@link DataSource} configuration. */ -public abstract class AbstractJdbcAppenderDataSourceTest { +public abstract class AbstractJdbcAppenderDataSourceTest extends AbstractJdbcDataSourceTest { @Rule public final RuleChain rules; diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java new file mode 100644 index 0000000..78bebee --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/AbstractJdbcDataSourceTest.java @@ -0,0 +1,39 @@ +/* + * 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.appender.db.jdbc; + +import javax.sql.DataSource; + +import org.junit.AfterClass; +import org.junit.BeforeClass; + +/** + * Abstract unit test for JDBC using a {@link DataSource} configuration. + */ +public abstract class AbstractJdbcDataSourceTest { + + @AfterClass + public static void afterClass() throws Exception { + System.clearProperty("log4j2.enableJndiJdbc"); + } + + @BeforeClass + public static void beforeClass() throws Exception { + System.setProperty("log4j2.enableJndiJdbc", "true"); + } + +} diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java index a49404a..ea9183a 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/DataSourceConnectionSourceTest.java @@ -37,7 +37,7 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @RunWith(Parameterized.class) -public class DataSourceConnectionSourceTest { +public class DataSourceConnectionSourceTest extends AbstractJdbcDataSourceTest { @Parameterized.Parameters(name = "{0}") public static Object[][] data() { diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java index bee9786..77dcf31 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/db/jdbc/JdbcAppenderMapMessageDataSourceTest.java @@ -46,7 +46,7 @@ import org.junit.rules.RuleChain; /** * Unit tests {@link MapMessage}s for JdbcAppender using a {@link DataSource} configuration. */ -public class JdbcAppenderMapMessageDataSourceTest { +public class JdbcAppenderMapMessageDataSourceTest extends AbstractJdbcDataSourceTest { @Rule public final RuleChain rules; diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java index 4c5b1a9..253cb06 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/mom/JmsAppenderTest.java @@ -48,6 +48,7 @@ import org.apache.logging.log4j.junit.LoggerContextRule; import org.apache.logging.log4j.message.Message; import org.apache.logging.log4j.message.SimpleMessage; import org.apache.logging.log4j.message.StringMapMessage; +import org.junit.AfterClass; import org.junit.Before; import org.junit.BeforeClass; import org.junit.Rule; @@ -84,7 +85,7 @@ public class JmsAppenderTest { @Rule public RuleChain rules = RuleChain.outerRule(jndiRule).around(ctx); - @BeforeClass + @AfterClass public static void afterClass() throws Exception { System.clearProperty("log4j2.enableJndiJms"); } diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java index e421734..0b3501c 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/net/JndiManagerTest.java @@ -30,13 +30,18 @@ import org.junit.jupiter.api.Test; public class JndiManagerTest { @Test + public void testIsJndiContextSelectorEnabled() { + assertFalse(JndiManager.isJndiContextSelectorEnabled()); + } + + @Test public void testIsJndiEnabled() { assertFalse(JndiManager.isJndiEnabled()); } @Test - public void testIsJndiContextSelectorEnabled() { - assertFalse(JndiManager.isJndiContextSelectorEnabled()); + public void testIsJndiJdbcEnabled() { + assertFalse(JndiManager.isJndiJdbcEnabled()); } @Test diff --git a/src/site/xdoc/manual/appenders.xml b/src/site/xdoc/manual/appenders.xml index c0f8c6d..42fb1b8 100644 --- a/src/site/xdoc/manual/appenders.xml +++ b/src/site/xdoc/manual/appenders.xml @@ -995,9 +995,13 @@ CREATE TABLE logs ( </subsection> <a name="JDBCAppender"/> <subsection name="JDBCAppender"> - <p>The JDBCAppender writes log events to a relational database table using standard JDBC. It can be configured - to obtain JDBC connections using a JNDI <code>DataSource</code> or a custom factory method. Whichever - approach you take, it <strong><em>must</em></strong> be backed by a connection pool. Otherwise, logging + <p>The JDBC Appender writes log events to a relational database table using standard JDBC. It can be configured + to obtain JDBC connections using a JNDI <code>DataSource</code> or a custom factory method.</p> + <p>The JDBC Appender configured with a <code>DataSource</code> requires JNDI support so as of release 2.17.1 + this appender will not function unless <code>log4j2.enableJndiJdbc=true</code> is configured as a system property + or environment variable. See the <a href="./configuration.html#enableJndiJdbc">enableJndiJdbc</a> system property.</p> + <p> + Whichever approach you take, it <strong><em>must</em></strong> be backed by a connection pool. Otherwise, logging performance will suffer greatly. If batch statements are supported by the configured JDBC driver and a <code>bufferSize</code> is configured to be a positive number, then log events will be batched. Note that as of Log4j 2.8, there are two ways to configure log event to column mappings: the original <code>ColumnConfig</code> diff --git a/src/site/xdoc/manual/configuration.xml.vm b/src/site/xdoc/manual/configuration.xml.vm index 7ddd97c..247eade 100644 --- a/src/site/xdoc/manual/configuration.xml.vm +++ b/src/site/xdoc/manual/configuration.xml.vm @@ -2157,11 +2157,19 @@ public class AwesomeTest { </td> </tr> <tr> + <td><a name="enableJndiJdbc"/>log4j2.enableJndiJdbc</td> + <td>LOG4J_ENABLE_JNDI_JDBC</td> + <td>false</td> + <td> + When true, a Log4j JDBC Appender configured with a <code>DataSource</code> which uses JNDI's java protocol is enabled. When false, the default, they are disabled. + </td> + </tr> + <tr> <td><a name="enableJndiJms"/>log4j2.enableJndiJms</td> <td>LOG4J_ENABLE_JNDI_JMS</td> <td>false</td> <td> - When true, the Log4j JMS Appender that uses JNDI's java protocol is enabled. When false, the default, they are disabled. + When true, a Log4j JMS Appender that uses JNDI's java protocol is enabled. When false, the default, they are disabled. </td> </tr> <tr> @@ -2169,7 +2177,7 @@ public class AwesomeTest { <td>LOG4J_ENABLE_JNDI_LOOKUP</td> <td>false</td> <td> - When true, the Log4j lookup that uses JNDI's java protocol is enabled. When false, the default, they are disabled. + When true, a Log4j lookup that uses JNDI's java protocol is enabled. When false, the default, they are disabled. </td> </tr> <tr>
