This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/2.x by this push:
new e87064f22d Reset the fallback listener on `StatusLogger#reset()`
(#2280)
e87064f22d is described below
commit e87064f22d50fe46c085e2e20d5b1c80a6261e7e
Author: Volkan Yazıcı <[email protected]>
AuthorDate: Mon Feb 12 20:02:48 2024 +0100
Reset the fallback listener on `StatusLogger#reset()` (#2280)
---
.../log4j/status/StatusConsoleListenerTest.java | 55 ++++++++++++---
.../log4j/status/StatusLoggerResetTest.java | 79 ++++++++++++++++++++++
.../log4j/status/StatusConsoleListener.java | 74 +++++++++++++++++---
.../apache/logging/log4j/status/StatusLogger.java | 21 +++---
4 files changed, 201 insertions(+), 28 deletions(-)
diff --git
a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java
b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java
index 0b93ae1fbc..5fb84eb317 100644
---
a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java
+++
b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java
@@ -16,15 +16,18 @@
*/
package org.apache.logging.log4j.status;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
+
import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
-import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;
-import org.mockito.Mockito;
public class StatusConsoleListenerTest {
@@ -34,16 +37,16 @@ public class StatusConsoleListenerTest {
void StatusData_getFormattedStatus_should_be_used() {
// Create the listener.
- final PrintStream stream = Mockito.mock(PrintStream.class);
+ final PrintStream stream = mock(PrintStream.class);
final StatusConsoleListener listener = new
StatusConsoleListener(Level.ALL, stream);
// Log a message.
- final Message message = Mockito.mock(Message.class);
- final StatusData statusData = Mockito.spy(new StatusData(null,
Level.TRACE, message, null, null));
+ final Message message = mock(Message.class);
+ final StatusData statusData = spy(new StatusData(null, Level.TRACE,
message, null, null));
listener.log(statusData);
// Verify the call.
- Mockito.verify(statusData).getFormattedStatus();
+ verify(statusData).getFormattedStatus();
}
@Test
@@ -86,7 +89,7 @@ public class StatusConsoleListenerTest {
final String output = outputStream.toString(encoding);
// Verify the output.
- Assertions.assertThat(output)
+ assertThat(output)
.contains(expectedThrowable.getMessage())
.contains(expectedMessage.getFormattedMessage())
.doesNotContain(discardedThrowable.getMessage())
@@ -94,10 +97,42 @@ public class StatusConsoleListenerTest {
}
@Test
- void non_system_streams_should_be_closed() throws Exception {
- final PrintStream stream = Mockito.mock(PrintStream.class);
+ void non_system_streams_should_be_closed() {
+ final PrintStream stream = mock(PrintStream.class);
final StatusConsoleListener listener = new
StatusConsoleListener(Level.WARN, stream);
listener.close();
- Mockito.verify(stream).close();
+ verify(stream).close();
+ }
+
+ @Test
+ void close_should_reset_to_initials() {
+
+ // Create the listener
+ final PrintStream initialStream = mock(PrintStream.class);
+ final Level initialLevel = Level.TRACE;
+ final StatusConsoleListener listener = new
StatusConsoleListener(initialLevel, initialStream);
+
+ // Verify the initial state
+ assertThat(listener.getStatusLevel()).isEqualTo(initialLevel);
+ assertThat(listener).hasFieldOrPropertyWithValue("stream",
initialStream);
+
+ // Update the state
+ final PrintStream newStream = mock(PrintStream.class);
+ listener.setStream(newStream);
+ final Level newLevel = Level.DEBUG;
+ listener.setLevel(newLevel);
+
+ // Verify the update
+ verify(initialStream).close();
+ assertThat(listener.getStatusLevel()).isEqualTo(newLevel);
+ assertThat(listener).hasFieldOrPropertyWithValue("stream", newStream);
+
+ // Close the listener
+ listener.close();
+
+ // Verify the reset
+ verify(newStream).close();
+ assertThat(listener.getStatusLevel()).isEqualTo(initialLevel);
+ assertThat(listener).hasFieldOrPropertyWithValue("stream",
initialStream);
}
}
diff --git
a/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerResetTest.java
b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerResetTest.java
new file mode 100644
index 0000000000..07fe4ca295
--- /dev/null
+++
b/log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusLoggerResetTest.java
@@ -0,0 +1,79 @@
+/*
+ * 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.status;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import java.io.IOException;
+import java.io.PrintStream;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
+import org.junit.jupiter.api.Test;
+
+class StatusLoggerResetTest {
+
+ @Test
+ void test_reset() throws IOException {
+
+ // Create the fallback listener
+ final PrintStream fallbackListenerInitialStream =
mock(PrintStream.class);
+ final Level fallbackListenerInitialLevel = Level.INFO;
+ final StatusConsoleListener fallbackListener =
+ new StatusConsoleListener(fallbackListenerInitialLevel,
fallbackListenerInitialStream);
+
+ // Create the `StatusLogger`
+ final StatusLogger.Config loggerConfig = new
StatusLogger.Config(false, 0, null);
+ final StatusLogger logger = new StatusLogger(
+ StatusLoggerResetTest.class.getSimpleName(),
+ ParameterizedNoReferenceMessageFactory.INSTANCE,
+ loggerConfig,
+ fallbackListener);
+
+ // Create and register a listener
+ final Level listenerLevel = Level.DEBUG;
+
assertThat(listenerLevel.isLessSpecificThan(fallbackListenerInitialLevel))
+ .isTrue();
+ final StatusListener listener = mock(StatusListener.class);
+ when(listener.getStatusLevel()).thenReturn(listenerLevel);
+ logger.registerListener(listener);
+
+ // Verify the `StatusLogger` state
+ assertThat(logger.getLevel()).isEqualTo(listenerLevel);
+ assertThat(logger.getListeners()).containsExactly(listener);
+
+ // Update the fallback listener
+ final PrintStream fallbackListenerNewStream = mock(PrintStream.class);
+ fallbackListener.setStream(fallbackListenerNewStream);
+ verify(fallbackListenerInitialStream).close();
+ final Level fallbackListenerNewLevel = Level.TRACE;
+
assertThat(fallbackListenerNewLevel.isLessSpecificThan(listenerLevel)).isTrue();
+ fallbackListener.setLevel(fallbackListenerNewLevel);
+
+ // Verify the `StatusLogger` state
+ assertThat(logger.getLevel()).isEqualTo(fallbackListenerNewLevel);
+
+ // Reset the `StatusLogger` and verify the state
+ logger.reset();
+ verify(listener).close();
+ verify(fallbackListenerNewStream).close();
+ assertThat(logger.getLevel()).isEqualTo(fallbackListenerInitialLevel);
+ assertThat(logger.getListeners()).isEmpty();
+ }
+}
diff --git
a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
index 3e30bf1994..99bcd8d1a7 100644
---
a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
+++
b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusConsoleListener.java
@@ -18,9 +18,12 @@ package org.apache.logging.log4j.status;
import static java.util.Objects.requireNonNull;
+import edu.umd.cs.findbugs.annotations.Nullable;
import java.io.IOException;
import java.io.OutputStream;
import java.io.PrintStream;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import org.apache.logging.log4j.Level;
/**
@@ -29,8 +32,16 @@ import org.apache.logging.log4j.Level;
@SuppressWarnings("UseOfSystemOutOrSystemErr")
public class StatusConsoleListener implements StatusListener {
+ private final Lock lock = new ReentrantLock();
+
+ private final Level initialLevel;
+
+ private final PrintStream initialStream;
+
+ // `volatile` is necessary to correctly read the `level` without holding
the lock
private volatile Level level;
+ // `volatile` is necessary to correctly read the `stream` without holding
the lock
private volatile PrintStream stream;
/**
@@ -54,8 +65,8 @@ public class StatusConsoleListener implements StatusListener {
* @throws NullPointerException on null {@code level} or {@code stream}
*/
public StatusConsoleListener(final Level level, final PrintStream stream) {
- this.level = requireNonNull(level, "level");
- this.stream = requireNonNull(stream, "stream");
+ this.initialLevel = this.level = requireNonNull(level, "level");
+ this.initialStream = this.stream = requireNonNull(stream, "stream");
}
/**
@@ -65,7 +76,16 @@ public class StatusConsoleListener implements StatusListener
{
* @throws NullPointerException on null {@code level}
*/
public void setLevel(final Level level) {
- this.level = requireNonNull(level, "level");
+ requireNonNull(level, "level");
+ // Check if a mutation (and locking!) is necessary at all
+ if (!this.level.equals(level)) {
+ lock.lock();
+ try {
+ this.level = level;
+ } finally {
+ lock.unlock();
+ }
+ }
}
/**
@@ -76,7 +96,23 @@ public class StatusConsoleListener implements StatusListener
{
* @since 2.23.0
*/
public void setStream(final PrintStream stream) {
- this.stream = requireNonNull(stream, "stream");
+ requireNonNull(stream, "stream");
+ // Check if a mutation (and locking!) is necessary at all
+ if (this.stream != stream) {
+ @Nullable OutputStream oldStream = null;
+ lock.lock();
+ try {
+ if (this.stream != stream) {
+ oldStream = this.stream;
+ this.stream = stream;
+ }
+ } finally {
+ lock.unlock();
+ }
+ if (oldStream != null) {
+ closeNonSystemStream(oldStream);
+ }
+ }
}
/**
@@ -113,13 +149,33 @@ public class StatusConsoleListener implements
StatusListener {
@Deprecated
public void setFilters(final String... filters) {}
+ /**
+ * Resets the level and output stream to its initial values, and closes
the output stream, if it is a non-system one.
+ */
@Override
- public void close() throws IOException {
- // Get local copy of the `volatile` member
- final OutputStream localStream = stream;
+ public void close() {
+ final OutputStream oldStream;
+ lock.lock();
+ try {
+ oldStream = stream;
+ stream = initialStream;
+ level = initialLevel;
+ } finally {
+ lock.unlock();
+ }
+ closeNonSystemStream(oldStream);
+ }
+
+ private static void closeNonSystemStream(final OutputStream stream) {
// Close only non-system streams
- if (localStream != System.out && localStream != System.err) {
- localStream.close();
+ if (stream != System.out && stream != System.err) {
+ try {
+ stream.close();
+ } catch (IOException error) {
+ // We are at the lowest level of the system.
+ // Hence, there is nothing better we can do but dumping the
failure.
+ error.printStackTrace(System.err);
+ }
}
}
}
diff --git
a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
index af6d668790..d605693219 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/status/StatusLogger.java
@@ -182,6 +182,7 @@ public class StatusLogger extends AbstractLogger {
private final int bufferCapacity;
+ @Nullable
private final Level fallbackListenerLevel;
@Nullable
@@ -193,22 +194,23 @@ public class StatusLogger extends AbstractLogger {
*
* @param debugEnabled the value of the {@value DEBUG_PROPERTY_NAME}
property
* @param bufferCapacity the value of the {@value MAX_STATUS_ENTRIES}
property
- * @param fallbackListenerLevel the value of the {@value
DEFAULT_STATUS_LISTENER_LEVEL} property
* @param instantFormatter the value of the {@value
STATUS_DATE_FORMAT} property
*/
- public Config(
- boolean debugEnabled,
- int bufferCapacity,
- Level fallbackListenerLevel,
- @Nullable DateTimeFormatter instantFormatter) {
+ public Config(boolean debugEnabled, int bufferCapacity, @Nullable
DateTimeFormatter instantFormatter) {
this.debugEnabled = debugEnabled;
if (bufferCapacity < 0) {
throw new IllegalArgumentException(
"was expecting a positive `bufferCapacity`, found: " +
bufferCapacity);
}
this.bufferCapacity = bufferCapacity;
- this.fallbackListenerLevel = requireNonNull(fallbackListenerLevel,
"fallbackListenerLevel");
- this.instantFormatter = requireNonNull(instantFormatter,
"instantFormatter");
+ // Public ctor intentionally doesn't set `fallbackListenerLevel`.
+ // Because, if public ctor is used, it means user is
programmatically creating a `Config` instance.
+ // Hence, they will use the public `StatusLogger` ctor too.
+ // There they need to provide the fallback listener explicitly
anyway.
+ // Therefore, there is no need to ask for a
`fallbackListenerLevel` here.
+ // Since this `fallbackListenerLevel` is only used by the private
`StatusLogger` ctor.
+ this.fallbackListenerLevel = null;
+ this.instantFormatter = instantFormatter;
}
/**
@@ -441,7 +443,7 @@ public class StatusLogger extends AbstractLogger {
}
/**
- * Clears the event buffer and removes the <em>registered</em> (not the
fallback one!) listeners.
+ * Clears the event buffer, removes the <em>registered</em> (not the
fallback one!) listeners, and resets the fallback listener.
*/
public void reset() {
listenerWriteLock.lock();
@@ -455,6 +457,7 @@ public class StatusLogger extends AbstractLogger {
} finally {
listenerWriteLock.unlock();
}
+ fallbackListener.close();
buffer.clear();
}
