ppkarwasz commented on code in PR #3394:
URL: https://github.com/apache/logging-log4j2/pull/3394#discussion_r1928983602
##########
log4j-core-test/${test:logging.path}/AsyncLoggerTest.log:
##########
@@ -0,0 +1,75 @@
+INFO c.f.Bar mapvalue [stackvalue] {KEY=mapvalue, configProp=configValue,
configProp2=configValue2} Async logger msg
Review Comment:
Please check what you are committing. This whole directory should not be in
Git.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java:
##########
@@ -0,0 +1,128 @@
+package org.apache.logging.log4j.core.appender;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.post;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static
com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
+import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.DefaultConfiguration;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.layout.JsonLayout;
+import org.apache.logging.log4j.core.lookup.JavaLookup;
+import org.apache.logging.log4j.core.net.ssl.KeyStoreConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslKeyStoreConstants;
+import org.apache.logging.log4j.core.net.ssl.TrustStoreConfiguration;
+import org.apache.logging.log4j.message.SimpleMessage;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import java.net.URL;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@StatusLoggerLevel(Level.ERROR) // Ensure the logger captures ERROR level
messages
+class HttpAppenderBuilderTest {
Review Comment:
You added another copy of the test to the `log4j-core/src/main`. Please
remove it.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/filter/ScriptFilter.java:
##########
@@ -145,16 +154,6 @@ public static ScriptFilter createFilter(
LOGGER.error("Script support is not enabled");
return null;
}
- if (script instanceof ScriptRef) {
- if (configuration.getScriptManager().getScript(script.getName())
== null) {
- logger.error("No script with name {} has been declared.",
script.getName());
- return null;
- }
- } else {
- if (!configuration.getScriptManager().addScript(script)) {
- return null;
- }
- }
Review Comment:
Now you can restore this code and remove the one in `start()`.
##########
log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java:
##########
@@ -687,28 +687,37 @@ protected void doConfigure() {
configurationStrSubstitutor.setVariableResolver(interpolator);
}
+ Node scriptsNode = null;
+ for (final Node node : rootNode.getChildren()) {
+ if ("Scripts".equalsIgnoreCase(node.getName())) {
+ scriptsNode = node;
+ createConfiguration(scriptsNode, null);
+ if (scriptsNode.getObject() != null) {
+ for (final AbstractScript script :
scriptsNode.getObject(AbstractScript[].class)) {
Review Comment:
The `scriptsNode` variable is not really useful here. Otherwise it looks
good to me.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java:
##########
@@ -0,0 +1,128 @@
+package org.apache.logging.log4j.core.appender;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.post;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static
com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
+import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.DefaultConfiguration;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.layout.JsonLayout;
+import org.apache.logging.log4j.core.lookup.JavaLookup;
+import org.apache.logging.log4j.core.net.ssl.KeyStoreConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslKeyStoreConstants;
+import org.apache.logging.log4j.core.net.ssl.TrustStoreConfiguration;
+import org.apache.logging.log4j.message.SimpleMessage;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import java.net.URL;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@StatusLoggerLevel(Level.ERROR) // Ensure the logger captures ERROR level
messages
+class HttpAppenderBuilderTest {
+
+ @Test
+ @UsingStatusListener
+ void testMissingLayout(final ListStatusListener listener) throws Exception
{
+ // Build the appender without a layout
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setName("Http")
+ .setUrl(new URL("https://localhost";))
+ .build();
+
+ // Assert that the appender is null
+ assertNull(appender, "Appender should be null when layout is missing");
+
+ // Verify that an ERROR log message was recorded
+ assertTrue(
+ listener.findStatusData(Level.ERROR)
+ .stream()
+ .anyMatch(status ->
status.getMessage().getFormattedMessage()
+ .contains("No layout configured for
HttpAppender 'Http'")),
+ "Expected error message was not logged"
+ );
+ }
+
+ @Test
+ @UsingStatusListener
+ void testMissingUrl(final ListStatusListener listener) {
+ // Build the appender without a URL
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setName("Http")
+ .setLayout(new JsonLayout.Builder().build())
+ .build();
+
+ // Assert that the appender is null
+ assertNull(appender, "Appender should be null when URL is missing");
+
+ // Verify that an ERROR log message was recorded
+ assertTrue(
+ listener.findStatusData(Level.ERROR)
+ .stream()
Review Comment:
This does not compile: `findStatusData` already returns a `Stream`!
##########
log4j-core-test/src/test/resources/log4j-script-filters.xml:
##########
@@ -18,7 +18,13 @@
<Configuration status="ERROR">
<Appenders>
<List name="List">
- <PatternLayout pattern="[%-5level] %c{1.} %msg%n"/>
+ <PatternLayout pattern="[%-5level] %c{1.} %msg%n"/>[suvrat@suvrat
logging-log4j2]$ git push
+Username for 'https://github.com': suvrat1629
+Password for 'https://[email protected]':
+remote: Support for password authentication was removed on August 13, 2021.
+remote: Please see
https://docs.github.com/get-started/getting-started-with-git/about-remote-repositories#cloning-with-https-urls
for information on currently recommended modes of authentication.
+fatal: Authentication failed for
'https://github.com/Suvrat1629/logging-log4j2.git/'
+
Review Comment:
I am reopening this conversation: the unintentional copy paste is still in
the file.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java:
##########
@@ -0,0 +1,128 @@
+package org.apache.logging.log4j.core.appender;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.post;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static
com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
+import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.DefaultConfiguration;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.layout.JsonLayout;
+import org.apache.logging.log4j.core.lookup.JavaLookup;
+import org.apache.logging.log4j.core.net.ssl.KeyStoreConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslKeyStoreConstants;
+import org.apache.logging.log4j.core.net.ssl.TrustStoreConfiguration;
+import org.apache.logging.log4j.message.SimpleMessage;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import java.net.URL;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@StatusLoggerLevel(Level.ERROR) // Ensure the logger captures ERROR level
messages
+class HttpAppenderBuilderTest {
+
+ @Test
+ @UsingStatusListener
+ void testMissingLayout(final ListStatusListener listener) throws Exception
{
+ // Build the appender without a layout
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setName("Http")
+ .setUrl(new URL("https://localhost";))
+ .build();
+
+ // Assert that the appender is null
+ assertNull(appender, "Appender should be null when layout is missing");
+
+ // Verify that an ERROR log message was recorded
+ assertTrue(
+ listener.findStatusData(Level.ERROR)
+ .stream()
Review Comment:
Same here.
##########
log4j-core-test/src/test/java/org/apache/logging/log4j/core/appender/HttpAppenderBuilderTest.java:
##########
@@ -0,0 +1,128 @@
+package org.apache.logging.log4j.core.appender;
+
+import static com.github.tomakehurst.wiremock.client.WireMock.aResponse;
+import static com.github.tomakehurst.wiremock.client.WireMock.containing;
+import static com.github.tomakehurst.wiremock.client.WireMock.equalTo;
+import static com.github.tomakehurst.wiremock.client.WireMock.post;
+import static com.github.tomakehurst.wiremock.client.WireMock.postRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.put;
+import static com.github.tomakehurst.wiremock.client.WireMock.putRequestedFor;
+import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
+import static
com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
+import com.github.tomakehurst.wiremock.junit5.WireMockExtension;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.core.Appender;
+import org.apache.logging.log4j.core.LogEvent;
+import org.apache.logging.log4j.core.config.Configuration;
+import org.apache.logging.log4j.core.config.DefaultConfiguration;
+import org.apache.logging.log4j.core.config.Property;
+import org.apache.logging.log4j.core.impl.Log4jLogEvent;
+import org.apache.logging.log4j.core.layout.JsonLayout;
+import org.apache.logging.log4j.core.lookup.JavaLookup;
+import org.apache.logging.log4j.core.net.ssl.KeyStoreConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslConfiguration;
+import org.apache.logging.log4j.core.net.ssl.SslKeyStoreConstants;
+import org.apache.logging.log4j.core.net.ssl.TrustStoreConfiguration;
+import org.apache.logging.log4j.message.SimpleMessage;
+import org.apache.logging.log4j.status.StatusData;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledOnOs;
+import org.junit.jupiter.api.condition.OS;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import java.net.URL;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+@StatusLoggerLevel(Level.ERROR) // Ensure the logger captures ERROR level
messages
+class HttpAppenderBuilderTest {
+
+ @Test
+ @UsingStatusListener
+ void testMissingLayout(final ListStatusListener listener) throws Exception
{
+ // Build the appender without a layout
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setName("Http")
+ .setUrl(new URL("https://localhost";))
+ .build();
+
+ // Assert that the appender is null
+ assertNull(appender, "Appender should be null when layout is missing");
+
+ // Verify that an ERROR log message was recorded
+ assertTrue(
+ listener.findStatusData(Level.ERROR)
+ .stream()
+ .anyMatch(status ->
status.getMessage().getFormattedMessage()
+ .contains("No layout configured for
HttpAppender 'Http'")),
+ "Expected error message was not logged"
+ );
+ }
+
+ @Test
+ @UsingStatusListener
+ void testMissingUrl(final ListStatusListener listener) {
+ // Build the appender without a URL
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setName("Http")
+ .setLayout(new JsonLayout.Builder().build())
+ .build();
+
+ // Assert that the appender is null
+ assertNull(appender, "Appender should be null when URL is missing");
+
+ // Verify that an ERROR log message was recorded
+ assertTrue(
+ listener.findStatusData(Level.ERROR)
+ .stream()
+ .anyMatch(status ->
status.getMessage().getFormattedMessage()
+ .contains("No URL configured for HttpAppender
'Http'")),
+ "Expected error message was not logged"
+ );
+ }
+
+ @Test
+ @UsingStatusListener
+ void testMissingName(final ListStatusListener listener) throws Exception {
+ // Build the appender without a name
+ HttpAppender appender = HttpAppender.newBuilder()
+ .setUrl(new URL("https://localhost";))
+ .setLayout(new JsonLayout.Builder().build())
+ .build();
+
+ // Assert that the appender is null
+ assertNull(appender, "Appender should be null when name is missing");
+
+ // Verify that an ERROR log message was recorded
+ assertTrue(
+ listener.findStatusData(Level.ERROR)
+ .stream()
Review Comment:
Same here
--
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]
