Make StatusLogger self-contained and testable

Author: vy

log4j-api-test/pom.xml

@@ -138,7 +138,6 @@
     <dependency>
       <groupId>org.mockito</groupId>
       <artifactId>mockito-core</artifactId>
-      <scope>test</scope>
     </dependency>
     <dependency>
       <groupId>org.mockito</groupId>

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/StatusLoggerExtension.java renamed to log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/StatusListenerExtension.java

@@ -43,12 +43,12 @@
 import org.junit.platform.commons.support.ModifierSupport;
 import org.junit.platform.commons.support.ReflectionSupport;
 
-class StatusLoggerExtension extends TypeBasedParameterResolver<ListStatusListener>
+class StatusListenerExtension extends TypeBasedParameterResolver<ListStatusListener>
         implements BeforeAllCallback, BeforeEachCallback, TestExecutionExceptionHandler {
 
     private static final Object KEY = ListStatusListener.class;
 
-    public StatusLoggerExtension() {
+    public StatusListenerExtension() {
         super(ListStatusListener.class);
     }
 

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/StatusLoggerMockExtension.java

@@ -0,0 +1,63 @@
+/*
+ * 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.test.junit;
+
+import static org.apache.logging.log4j.test.junit.ExtensionContextAnchor.getAttribute;
+import static org.apache.logging.log4j.test.junit.ExtensionContextAnchor.setAttribute;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+
+import org.apache.logging.log4j.status.StatusLogger;
+import org.junit.jupiter.api.extension.AfterAllCallback;
+import org.junit.jupiter.api.extension.BeforeAllCallback;
+import org.junit.jupiter.api.extension.BeforeEachCallback;
+import org.junit.jupiter.api.extension.ExtensionContext;
+
+/**
+ * Replaces {@link StatusLogger} static instance with a mocked one.
+ * <p>
+ * <b>Warning!</b>
+ * Many classes store the result of {@link StatusLogger#getLogger()} in {@code static} field.
+ * Hence, the mock replacement must be performed before anybody tries to access it.
+ * Similarly, we cannot replace the mock in between tests, since it is already stored in {@code static} fields.
+ * That is why we only reset the mocked instance before each test.
+ * </p>
+ */
+class StatusLoggerMockExtension implements BeforeAllCallback, BeforeEachCallback, AfterAllCallback {
+
+    private static final String KEY_PREFIX = StatusLoggerMockExtension.class.getSimpleName() + '.';
+
+    private static final String INITIAL_STATUS_LOGGER_KEY = KEY_PREFIX + "initialStatusLogger";
+
+    @Override
+    public void beforeAll(final ExtensionContext context) throws Exception {
+        setAttribute(INITIAL_STATUS_LOGGER_KEY, StatusLogger.getLogger(), context);
+        final StatusLogger statusLogger = mock(StatusLogger.class);
+        StatusLogger.setLogger(statusLogger);
+    }
+
+    @Override
+    public void beforeEach(final ExtensionContext context) throws Exception {
+        reset(StatusLogger.getLogger());
+    }
+
+    @Override
+    public void afterAll(final ExtensionContext context) {
+        final StatusLogger statusLogger = getAttribute(INITIAL_STATUS_LOGGER_KEY, StatusLogger.class, context);
+        StatusLogger.setLogger(statusLogger);
+    }
+}

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusListener.java

@@ -37,5 +37,5 @@
 @Documented
 @ExtendWith(ExtensionContextAnchor.class)
 @ExtendWith(TestPropertyResolver.class)
-@ExtendWith(StatusLoggerExtension.class)
+@ExtendWith(StatusListenerExtension.class)
 public @interface UsingStatusListener {}

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingStatusLoggerMock.java

@@ -0,0 +1,35 @@
+/*
+ * 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.test.junit;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+/**
+ * Shortcut to {@link StatusLoggerMockExtension}.
+ */
+@Retention(RUNTIME)
+@Target({TYPE, METHOD})
+@Documented
+@ExtendWith(StatusLoggerMockExtension.class)
+public @interface UsingStatusLoggerMock {}

log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java

@@ -15,7 +15,7 @@
  * limitations under the license.
  */
 @Export
-@Version("2.21.1")
+@Version("2.22.0")
 package org.apache.logging.log4j.test.junit;
 
 import org.osgi.annotation.bundle.Export;

log4j-api-test/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java

@@ -42,9 +42,11 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
+import org.junitpioneer.jupiter.SetSystemProperty;
 
 @StatusLoggerLevel("WARN")
 @ResourceLock(value = Resources.MARKER_MANAGER, mode = ResourceAccessMode.READ)
+@SetSystemProperty(key = StatusLogger.BUFFER_CAPACITY_PROPERTY_NAME, value = "200")
 public class AbstractLoggerTest {
 
     private static final StringBuilder CHAR_SEQ = new StringBuilder("CharSeq");

log4j-api-test/src/test/java/org/apache/logging/log4j/status/StatusConsoleListenerTest.java

@@ -19,11 +19,9 @@
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
 import org.apache.logging.log4j.Level;
-import org.apache.logging.log4j.LogBuilder;
 import org.apache.logging.log4j.message.Message;
 import org.apache.logging.log4j.message.MessageFactory;
 import org.apache.logging.log4j.message.ParameterizedNoReferenceMessageFactory;
-import org.apache.logging.log4j.simple.SimpleLogger;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
@@ -33,38 +31,19 @@ public class StatusConsoleListenerTest {
     public static final MessageFactory MESSAGE_FACTORY = ParameterizedNoReferenceMessageFactory.INSTANCE;
 
     @Test
-    void SimpleLogger_should_be_used() {
-
-        // Create a mock `SimpleLoggerFactory`.
-        final SimpleLogger logger = Mockito.mock(SimpleLogger.class);
-        final LogBuilder logBuilder = Mockito.mock(LogBuilder.class);
-        Mockito.when(logger.atLevel(Mockito.any())).thenReturn(logBuilder);
-        Mockito.when(logBuilder.withThrowable(Mockito.any())).thenReturn(logBuilder);
-        Mockito.when(logBuilder.withLocation(Mockito.any())).thenReturn(logBuilder);
-        final SimpleLoggerFactory loggerFactory = Mockito.mock(SimpleLoggerFactory.class);
-        Mockito.when(loggerFactory.createSimpleLogger(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any()))
-                .thenReturn(logger);
+    void StatusData_getFormattedStatus_should_be_used() {
 
         // Create the listener.
         final PrintStream stream = Mockito.mock(PrintStream.class);
-        final Level level = Mockito.mock(Level.class);
-        final StatusConsoleListener listener = new StatusConsoleListener(level, stream, loggerFactory);
+        final StatusConsoleListener listener = new StatusConsoleListener(Level.ALL, stream);
 
         // Log a message.
-        final StackTraceElement caller = Mockito.mock(StackTraceElement.class);
         final Message message = Mockito.mock(Message.class);
-        final Throwable throwable = Mockito.mock(Throwable.class);
-        final StatusData statusData = new StatusData(caller, level, message, throwable, null);
+        final StatusData statusData = Mockito.spy(new StatusData(null, Level.TRACE, message, null, null));
         listener.log(statusData);
 
         // Verify the call.
-        Mockito.verify(loggerFactory)
-                .createSimpleLogger(
-                        Mockito.eq("StatusConsoleListener"), Mockito.same(level), Mockito.any(), Mockito.same(stream));
-        Mockito.verify(logger).atLevel(Mockito.same(level));
-        Mockito.verify(logBuilder).withThrowable(Mockito.same(throwable));
-        Mockito.verify(logBuilder).withLocation(Mockito.same(caller));
-        Mockito.verify(logBuilder).log(Mockito.same(message));
+        Mockito.verify(statusData).getFormattedStatus();
     }
 
     @Test

log4j-api/src/main/java/org/apache/logging/log4j/spi/AbstractLogger.java

@@ -38,7 +38,6 @@
 import org.apache.logging.log4j.util.LoaderUtil;
 import org.apache.logging.log4j.util.MessageSupplier;
 import org.apache.logging.log4j.util.PerformanceSensitive;
-import org.apache.logging.log4j.util.PropertiesUtil;
 import org.apache.logging.log4j.util.StackLocatorUtil;
 import org.apache.logging.log4j.util.Supplier;
 
@@ -86,14 +85,14 @@ public abstract class AbstractLogger implements ExtendedLogger, LocationAwareLog
     /**
      * The default MessageFactory class.
      */
-    public static final Class<? extends MessageFactory> DEFAULT_MESSAGE_FACTORY_CLASS = createClassForProperty(
-            "log4j2.messageFactory", ReusableMessageFactory.class, ParameterizedMessageFactory.class);
+    public static final Class<? extends MessageFactory> DEFAULT_MESSAGE_FACTORY_CLASS =
+            ParameterizedMessageFactory.class;
 
     /**
      * The default FlowMessageFactory class.
      */
     public static final Class<? extends FlowMessageFactory> DEFAULT_FLOW_MESSAGE_FACTORY_CLASS =
-            createFlowClassForProperty("log4j2.flowMessageFactory", DefaultFlowMessageFactory.class);
+            DefaultFlowMessageFactory.class;
 
     private static final long serialVersionUID = 2L;
 
@@ -198,32 +197,6 @@ protected Message catchingMsg(final Throwable throwable) {
         return messageFactory.newMessage(CATCHING);
     }
 
-    private static Class<? extends MessageFactory> createClassForProperty(
-            final String property,
-            final Class<ReusableMessageFactory> reusableParameterizedMessageFactoryClass,
-            final Class<ParameterizedMessageFactory> parameterizedMessageFactoryClass) {
-        try {
-            final String fallback = Constants.ENABLE_THREADLOCALS
-                    ? reusableParameterizedMessageFactoryClass.getName()
-                    : parameterizedMessageFactoryClass.getName();
-            final String clsName = PropertiesUtil.getProperties().getStringProperty(property, fallback);
-            return LoaderUtil.loadClass(clsName).asSubclass(MessageFactory.class);
-        } catch (final Throwable throwable) {
-            return parameterizedMessageFactoryClass;
-        }
-    }
-
-    private static Class<? extends FlowMessageFactory> createFlowClassForProperty(
-            final String property, final Class<DefaultFlowMessageFactory> defaultFlowMessageFactoryClass) {
-        try {
-            final String clsName = PropertiesUtil.getProperties()
-                    .getStringProperty(property, defaultFlowMessageFactoryClass.getName());
-            return LoaderUtil.loadClass(clsName).asSubclass(FlowMessageFactory.class);
-        } catch (final Throwable throwable) {
-            return defaultFlowMessageFactoryClass;
-        }
-    }
-
     private static MessageFactory2 createDefaultMessageFactory() {
         try {
             final MessageFactory result = LoaderUtil.newInstanceOf(DEFAULT_MESSAGE_FACTORY_CLASS);