From 93819f22b6454a36ac1bf0fe8a795bfc5fd2d46c Mon Sep 17 00:00:00 2001 From: yybmion Date: Mon, 8 Sep 2025 19:42:42 +0900 Subject: [PATCH 1/2] Add getConfiguration method for multiple URIs --- .../core/config/ConfigurationFactoryTest.java | 65 +++++++++++++++++++ .../builder/CustomConfigurationFactory.java | 2 +- .../core/config/ConfigurationFactory.java | 47 ++++++++++++++ .../log4j/core/config/json/package-info.java | 2 +- .../core/config/properties/package-info.java | 2 +- .../log4j/core/config/xml/package-info.java | 2 +- .../log4j/core/config/yaml/package-info.java | 2 +- ...onfiguration_method_for_mulitiple_URIs.xml | 13 ++++ 8 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 src/changelog/.2.x.x/add_getConfiguration_method_for_mulitiple_URIs.xml diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java index f2d81f13472..afb97649c5f 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java @@ -19,14 +19,19 @@ import static org.apache.logging.log4j.util.Unbox.box; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.net.URI; +import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collections; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -36,6 +41,7 @@ import org.apache.logging.log4j.core.Logger; import org.apache.logging.log4j.core.LoggerContext; import org.apache.logging.log4j.core.appender.ConsoleAppender; +import org.apache.logging.log4j.core.config.composite.CompositeConfiguration; import org.apache.logging.log4j.core.filter.ThreadContextMapFilter; import org.apache.logging.log4j.core.test.junit.LoggerContextSource; import org.apache.logging.log4j.test.junit.TempLoggingDir; @@ -130,4 +136,63 @@ void properties(final LoggerContext context) throws IOException { final Path logFile = loggingPath.resolve("test-properties.log"); checkFileLogger(context, logFile); } + + @Test + void testGetConfigurationWithNullUris() { + final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + try (final LoggerContext context = new LoggerContext("test")) { + assertThrows(NullPointerException.class, () -> factory.getConfiguration(context, "test", (List) null)); + } + } + + @Test + void testGetConfigurationWithEmptyUris() { + final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + try (final LoggerContext context = new LoggerContext("test")) { + assertThrows( + IllegalArgumentException.class, + () -> factory.getConfiguration(context, "test", Collections.emptyList())); + } + } + + @Test + void testGetConfigurationWithNullInList() { + final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + try (final LoggerContext context = new LoggerContext("test")) { + final List listWithNull = Collections.singletonList(null); + assertThrows(ConfigurationException.class, () -> factory.getConfiguration(context, "test", listWithNull)); + } + } + + @Test + void testGetConfigurationWithSingleUri() throws Exception { + final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + try (final LoggerContext context = new LoggerContext("test")) { + final URL resource = getClass().getResource("/log4j-test1.xml"); + assertNotNull(resource); + + final List singleUri = Collections.singletonList(resource.toURI()); + final Configuration config = factory.getConfiguration(context, "test", singleUri); + + assertNotNull(config); + assertFalse(config instanceof CompositeConfiguration); + } + } + + @Test + void testGetConfigurationWithMultipleUris() throws Exception { + final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + try (final LoggerContext context = new LoggerContext("test")) { + final URL resource1 = getClass().getResource("/log4j-test1.xml"); + final URL resource2 = getClass().getResource("/log4j-xinclude.xml"); + assertNotNull(resource1); + assertNotNull(resource2); + + final List multipleUris = Arrays.asList(resource1.toURI(), resource2.toURI()); + final Configuration config = factory.getConfiguration(context, "test", multipleUris); + + assertNotNull(config); + assertTrue(config instanceof CompositeConfiguration); + } + } } diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/builder/CustomConfigurationFactory.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/builder/CustomConfigurationFactory.java index 737c2181e77..63822ccd900 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/builder/CustomConfigurationFactory.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/builder/CustomConfigurationFactory.java @@ -72,7 +72,7 @@ static Configuration addTestFixtures(final String name, final ConfigurationBuild @Override public Configuration getConfiguration(final LoggerContext loggerContext, final ConfigurationSource source) { - return getConfiguration(loggerContext, source.toString(), null); + return getConfiguration(loggerContext, source.toString(), (URI) null); } @Override diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java index 505ca9c9cd9..8d6b69ad794 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import org.apache.logging.log4j.Level; @@ -333,6 +334,52 @@ public Configuration getConfiguration( return getConfiguration(loggerContext, name, configLocation); } + /** + * Creates a Configuration from multiple configuration URIs. + * If multiple URIs are successfully loaded, they will be combined into a CompositeConfiguration. + * + * @param loggerContext the logger context (may be null) + * @param name the configuration name (may be null) + * @param uris the list of configuration URIs (must not be null or empty) + * @return a Configuration created from the provided URIs + * @throws NullPointerException if uris is null + * @throws IllegalArgumentException if uris is empty + * @throws ConfigurationException if no valid configuration could be created + * from any of the provided URIs + * @since 2.26.0 + */ + public Configuration getConfiguration(final LoggerContext loggerContext, final String name, final List uris) { + + Objects.requireNonNull(uris, "uris parameter cannot be null"); + + if (uris.isEmpty()) { + throw new IllegalArgumentException("URI list cannot be empty"); + } + + final List configurations = new ArrayList<>(); + + for (final URI uri : uris) { + + if (uri == null) { + throw new ConfigurationException("URI list contains null element"); + } + + final Configuration config = getConfiguration(loggerContext, name, uri); + + if (config == null) { + throw new ConfigurationException("Failed to create configuration from: " + uri); + } + + if (!(config instanceof AbstractConfiguration)) { + throw new ConfigurationException("Configuration at " + uri + " is not an AbstractConfiguration"); + } + + configurations.add((AbstractConfiguration) config); + } + + return configurations.size() == 1 ? configurations.get(0) : new CompositeConfiguration(configurations); + } + static boolean isClassLoaderUri(final URI uri) { if (uri == null) { return false; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/package-info.java index 9d9e5d60eb3..0eab26e30a4 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/json/package-info.java @@ -18,7 +18,7 @@ * Classes and interfaces supporting configuration of Log4j 2 with JSON. */ @Export -@Version("2.20.1") +@Version("2.26.0") package org.apache.logging.log4j.core.config.json; import org.osgi.annotation.bundle.Export; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/package-info.java index 1563a62e0ec..a7689f23217 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/properties/package-info.java @@ -18,7 +18,7 @@ * Configuration using Properties files. */ @Export -@Version("2.20.1") +@Version("2.26.0") package org.apache.logging.log4j.core.config.properties; import org.osgi.annotation.bundle.Export; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/package-info.java index f0a6c9d7e4f..248ae965cec 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/xml/package-info.java @@ -18,7 +18,7 @@ * Classes and interfaces supporting configuration of Log4j 2 with XML. */ @Export -@Version("2.20.2") +@Version("2.26.0") package org.apache.logging.log4j.core.config.xml; import org.osgi.annotation.bundle.Export; diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/package-info.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/package-info.java index c476f1d82c1..b574f5ff409 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/package-info.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/yaml/package-info.java @@ -18,7 +18,7 @@ * Classes and interfaces supporting configuration of Log4j 2 with YAML. */ @Export -@Version("2.20.1") +@Version("2.26.0") package org.apache.logging.log4j.core.config.yaml; import org.osgi.annotation.bundle.Export; diff --git a/src/changelog/.2.x.x/add_getConfiguration_method_for_mulitiple_URIs.xml b/src/changelog/.2.x.x/add_getConfiguration_method_for_mulitiple_URIs.xml new file mode 100644 index 00000000000..6a30033ce68 --- /dev/null +++ b/src/changelog/.2.x.x/add_getConfiguration_method_for_mulitiple_URIs.xml @@ -0,0 +1,13 @@ + + + + + + Add a new `ConfigurationFactory::getConfiguration` method accepting multiple `URI`s + + From a738f0da511c4ab71b1b3f1fe1091b165964cac8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Sat, 11 Oct 2025 21:03:46 +0200 Subject: [PATCH 2/2] Apply review feedback --- .../core/config/ConfigurationFactoryTest.java | 52 +++++------- .../core/config/ConfigurationFactory.java | 84 ++++++++++++------- 2 files changed, 75 insertions(+), 61 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java index afb97649c5f..5c10212aa17 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java @@ -19,7 +19,6 @@ import static org.apache.logging.log4j.util.Unbox.box; import static org.junit.jupiter.api.Assertions.assertAll; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -27,7 +26,6 @@ import java.io.IOException; import java.net.URI; -import java.net.URL; import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; @@ -48,6 +46,7 @@ import org.apache.logging.log4j.util.Strings; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; +import org.mockito.Mockito; class ConfigurationFactoryTest { @@ -139,43 +138,41 @@ void properties(final LoggerContext context) throws IOException { @Test void testGetConfigurationWithNullUris() { - final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + final ConfigurationFactory factory = Mockito.spy(ConfigurationFactory.getInstance()); try (final LoggerContext context = new LoggerContext("test")) { - assertThrows(NullPointerException.class, () -> factory.getConfiguration(context, "test", (List) null)); + factory.getConfiguration(context, "test", (List) null); + Mockito.verify(factory).getConfiguration(Mockito.same(context), Mockito.eq("test"), (URI) Mockito.isNull()); } } @Test void testGetConfigurationWithEmptyUris() { - final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + final ConfigurationFactory factory = Mockito.spy(ConfigurationFactory.getInstance()); try (final LoggerContext context = new LoggerContext("test")) { - assertThrows( - IllegalArgumentException.class, - () -> factory.getConfiguration(context, "test", Collections.emptyList())); + factory.getConfiguration(context, "test", Collections.emptyList()); + Mockito.verify(factory).getConfiguration(Mockito.same(context), Mockito.eq("test"), (URI) Mockito.isNull()); } } @Test void testGetConfigurationWithNullInList() { - final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + final ConfigurationFactory factory = Mockito.spy(ConfigurationFactory.getInstance()); try (final LoggerContext context = new LoggerContext("test")) { final List listWithNull = Collections.singletonList(null); - assertThrows(ConfigurationException.class, () -> factory.getConfiguration(context, "test", listWithNull)); + factory.getConfiguration(context, "test", listWithNull); + Mockito.verify(factory).getConfiguration(Mockito.same(context), Mockito.eq("test"), (URI) Mockito.isNull()); } } @Test void testGetConfigurationWithSingleUri() throws Exception { - final ConfigurationFactory factory = ConfigurationFactory.getInstance(); + final ConfigurationFactory factory = Mockito.spy(ConfigurationFactory.getInstance()); try (final LoggerContext context = new LoggerContext("test")) { - final URL resource = getClass().getResource("/log4j-test1.xml"); - assertNotNull(resource); - - final List singleUri = Collections.singletonList(resource.toURI()); - final Configuration config = factory.getConfiguration(context, "test", singleUri); - - assertNotNull(config); - assertFalse(config instanceof CompositeConfiguration); + final URI configLocation = + getClass().getResource("/log4j-test1.xml").toURI(); + factory.getConfiguration(context, "test", Collections.singletonList(configLocation)); + Mockito.verify(factory) + .getConfiguration(Mockito.same(context), Mockito.eq("test"), Mockito.eq(configLocation)); } } @@ -183,16 +180,13 @@ void testGetConfigurationWithSingleUri() throws Exception { void testGetConfigurationWithMultipleUris() throws Exception { final ConfigurationFactory factory = ConfigurationFactory.getInstance(); try (final LoggerContext context = new LoggerContext("test")) { - final URL resource1 = getClass().getResource("/log4j-test1.xml"); - final URL resource2 = getClass().getResource("/log4j-xinclude.xml"); - assertNotNull(resource1); - assertNotNull(resource2); - - final List multipleUris = Arrays.asList(resource1.toURI(), resource2.toURI()); - final Configuration config = factory.getConfiguration(context, "test", multipleUris); - - assertNotNull(config); - assertTrue(config instanceof CompositeConfiguration); + final URI configLocation1 = + getClass().getResource("/log4j-test1.xml").toURI(); + final URI configLocation2 = + getClass().getResource("/log4j-xinclude.xml").toURI(); + final List configLocations = Arrays.asList(configLocation1, configLocation2); + final Configuration config = factory.getConfiguration(context, "test", configLocations); + assertInstanceOf(CompositeConfiguration.class, config); } } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java index 8d6b69ad794..ebc1474fdf3 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/ConfigurationFactory.java @@ -28,6 +28,7 @@ import java.util.Objects; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.core.LoggerContext; @@ -335,49 +336,68 @@ public Configuration getConfiguration( } /** - * Creates a Configuration from multiple configuration URIs. - * If multiple URIs are successfully loaded, they will be combined into a CompositeConfiguration. + * {@return a {@link Configuration} created using provided configuration location {@link URI}s} + * If the provided list of {@code URI}s is null or empty, {@code getConfiguration(loggerContext, name, (URI) null)} will be returned. + * + * @param loggerContext a logger context, may be null + * @param name a configuration name, may be null + * @param configLocations configuration location {@code URI}s, may be null or empty + * @throws ConfigurationException if configuration could not be created * - * @param loggerContext the logger context (may be null) - * @param name the configuration name (may be null) - * @param uris the list of configuration URIs (must not be null or empty) - * @return a Configuration created from the provided URIs - * @throws NullPointerException if uris is null - * @throws IllegalArgumentException if uris is empty - * @throws ConfigurationException if no valid configuration could be created - * from any of the provided URIs * @since 2.26.0 */ - public Configuration getConfiguration(final LoggerContext loggerContext, final String name, final List uris) { - - Objects.requireNonNull(uris, "uris parameter cannot be null"); - - if (uris.isEmpty()) { - throw new IllegalArgumentException("URI list cannot be empty"); - } - - final List configurations = new ArrayList<>(); - - for (final URI uri : uris) { - - if (uri == null) { - throw new ConfigurationException("URI list contains null element"); - } + public Configuration getConfiguration( + final LoggerContext loggerContext, final String name, final List configLocations) { - final Configuration config = getConfiguration(loggerContext, name, uri); + // Sanitize URIs + final List distinctConfigLocations = configLocations == null + ? Collections.emptyList() + : configLocations.stream().filter(Objects::nonNull).distinct().collect(Collectors.toList()); + // Short-circuit if provided URIs are null or empty + if (distinctConfigLocations.isEmpty()) { + final Configuration config = getConfiguration(loggerContext, name, (URI) null); if (config == null) { - throw new ConfigurationException("Failed to create configuration from: " + uri); + throw new ConfigurationException("Configuration could not be created"); } + return config; + } - if (!(config instanceof AbstractConfiguration)) { - throw new ConfigurationException("Configuration at " + uri + " is not an AbstractConfiguration"); + // Short-circuit if there is only a single URI + if (distinctConfigLocations.size() == 1) { + final URI configLocation = distinctConfigLocations.get(0); + final Configuration config = getConfiguration(loggerContext, name, configLocation); + if (config == null) { + final String message = + String.format("Configuration could not be created from location: `%s`", configLocation); + throw new ConfigurationException(message); } - - configurations.add((AbstractConfiguration) config); + return config; } - return configurations.size() == 1 ? configurations.get(0) : new CompositeConfiguration(configurations); + // Create individual configurations + final List configs = distinctConfigLocations.stream() + .map(configLocation -> { + final Configuration config = getConfiguration(loggerContext, name, configLocation); + if (config == null) { + final String message = + String.format("Configuration could not be created from location: `%s`", configLocation); + throw new ConfigurationException(message); + } + if (!(config instanceof AbstractConfiguration)) { + final String message = String.format( + "Configuration created from location `%s` was expected to be of type `%s`, found: `%s`", + configLocation, + AbstractConfiguration.class.getCanonicalName(), + config.getClass().getCanonicalName()); + throw new ConfigurationException(message); + } + return (AbstractConfiguration) config; + }) + .collect(Collectors.toList()); + + // Combine created configurations + return new CompositeConfiguration(configs); } static boolean isClassLoaderUri(final URI uri) {