vy commented on code in PR #3921:
URL: https://github.com/apache/logging-log4j2/pull/3921#discussion_r2401717784
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java:
##########
Review Comment:
Would you mind putting all `final LoggerContext context = new
LoggerContext("test")` lines in try-with-resources blocks, please? That is,
```
try (final LoggerContext context = new LoggerContext("test")) {
// ...
}
```
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java:
##########
@@ -130,4 +136,54 @@ 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();
+ final LoggerContext context = new LoggerContext("test");
+
+ assertThrows(NullPointerException.class, () ->
factory.getConfiguration(context, "test", (List<URI>) null));
+ }
+
+ @Test
+ void testGetConfigurationWithEmptyUris() {
+ final ConfigurationFactory factory =
ConfigurationFactory.getInstance();
+ final LoggerContext context = new LoggerContext("test");
+
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> factory.getConfiguration(context, "test",
Collections.emptyList()));
Review Comment:
Could you also add a test for a list containing `null`, please? That is,
`Collections.emptyList(null)`.
##########
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.21.0")
Review Comment:
```suggestion
@Version("2.26.0")
```
See [Fixing API compatibility check
failures](https://github.com/apache/logging-log4j2/blob/2.x/BUILDING.adoc#fixing-api-compatibility-check-failures).
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/config/ConfigurationFactoryTest.java:
##########
@@ -130,4 +136,54 @@ 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();
+ final LoggerContext context = new LoggerContext("test");
+
+ assertThrows(NullPointerException.class, () ->
factory.getConfiguration(context, "test", (List<URI>) null));
+ }
+
+ @Test
+ void testGetConfigurationWithEmptyUris() {
+ final ConfigurationFactory factory =
ConfigurationFactory.getInstance();
+ final LoggerContext context = new LoggerContext("test");
+
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> factory.getConfiguration(context, "test",
Collections.emptyList()));
+ }
+
+ @Test
+ void testGetConfigurationWithSingleUri() throws Exception {
+ final ConfigurationFactory factory =
ConfigurationFactory.getInstance();
+ final LoggerContext context = new LoggerContext("test");
+
+ final URL resource = getClass().getResource("/log4j-test1.xml");
+ assertNotNull(resource);
+
+ final List<URI> 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();
+ final LoggerContext context = new LoggerContext("test");
+
+ final URL resource1 = getClass().getResource("/log4j-test1.xml");
+ final URL resource2 =
getClass().getResource("/log4j-test1.properties");
Review Comment:
If I'm not mistaken, both files are semantically identical. Could you use a
totally different configuration for the `resource2`, please?
##########
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.21.0")
Review Comment:
```suggestion
@Version("2.26.0")
```
##########
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.21.0")
Review Comment:
```suggestion
@Version("2.26.0")
```
##########
src/changelog/.2.x.x/add_getConfiguration_method_for_mulitiple_URIs.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd";
+ type="added">
+ <issue id="3775"
link="https://github.com/apache/logging-log4j2/issues/3775"/>
Review Comment:
```suggestion
<issue id="3775"
link="https://github.com/apache/logging-log4j2/issues/3775"/>
<issue id="3921"
link="https://github.com/apache/logging-log4j2/pull/3921"/>
```
##########
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.21.0")
Review Comment:
```suggestion
@Version("2.26.0")
```
##########
src/changelog/.2.x.x/add_getConfiguration_method_for_mulitiple_URIs.xml:
##########
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="
+ https://logging.apache.org/xml/ns
+ https://logging.apache.org/xml/ns/log4j-changelog-0.xsd";
+ type="added">
+ <issue id="3775"
link="https://github.com/apache/logging-log4j2/issues/3775"/>
+ <description format="asciidoc">
+ Adds a new public API to ConfigurationFactory for creating configurations
from multiple URIs, automatically combining them into a CompositeConfiguration
when needed.
Review Comment:
Add a new `ConfigurationFactory::getConfiguration` method accepting multiple
``URI``s
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
