[logging-log4j2] 01/04: Refactor `MarkerManager` and other resource locks

[pk...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

pkarwasz pushed a commit to branch release-2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git

commit 97b4a19064bbfd72f650ef74e6291e1978f41350
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Jun 20 21:31:02 2022 +0200

    Refactor `MarkerManager` and other resource locks
    
    Only one test needs write permission to `MarkerManager`
---
 .../{UsingThreadContextMap.java => Resources.java} | 26 ++++-------------
 .../log4j/test/junit/UsingThreadContextMap.java    |  2 +-
 .../apache/logging/log4j/AbstractLoggerTest.java   | 22 +++++++-------
 .../java/org/apache/logging/log4j/LoggerTest.java  | 34 ++++++++++++++--------
 .../java/org/apache/logging/log4j/MarkerTest.java  | 11 +++++--
 .../logging/log4j/simple/SimpleLoggerTest.java     |  2 --
 6 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/Resources.java
similarity index 51%
copy from log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java
copy to log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/Resources.java
index b42902cd8b..4f85703d85 100644
--- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java
+++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/Resources.java
@@ -17,29 +17,15 @@
 
 package org.apache.logging.log4j.test.junit;
 
-import org.junit.jupiter.api.extension.ExtendWith;
-import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
-import org.junit.jupiter.api.parallel.Resources;
-
-import java.lang.annotation.Documented;
-import java.lang.annotation.ElementType;
-import java.lang.annotation.Inherited;
-import java.lang.annotation.Retention;
-import java.lang.annotation.RetentionPolicy;
-import java.lang.annotation.Target;
 
 /**
- * Marks a test class as using {@link org.apache.logging.log4j.spi.ThreadContextMap} APIs. This will automatically clear and
- * restore the thread context map (MDC) for each test invocation.
+ * Constants to use the the {@link ResourceLock} annotation.
  *
- * @since 2.14.0
  */
-@Retention(RetentionPolicy.RUNTIME)
-@Target(ElementType.TYPE)
-@Documented
-@Inherited
-@ExtendWith(ThreadContextExtension.class)
-@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ)
-public @interface UsingThreadContextMap {
+public class Resources {
+
+    public static final String THREAD_CONTEXT = "log4j2.ThreadContext";
+
+    public static final String MARKER_MANAGER = "log4j2.MarkerManager";
 }
diff --git a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java
index b42902cd8b..074ba0b58c 100644
--- a/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java
+++ b/log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/UsingThreadContextMap.java
@@ -36,7 +36,7 @@ import java.lang.annotation.Target;
  * @since 2.14.0
  */
 @Retention(RetentionPolicy.RUNTIME)
-@Target(ElementType.TYPE)
+@Target({ElementType.TYPE, ElementType.METHOD})
 @Documented
 @Inherited
 @ExtendWith(ThreadContextExtension.class)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
index f563c5f769..a51612dbf0 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/AbstractLoggerTest.java
@@ -16,14 +16,6 @@
  */
 package org.apache.logging.log4j;
 
-import static org.hamcrest.CoreMatchers.containsString;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
-
 import java.util.List;
 
 import org.apache.logging.log4j.message.Message;
@@ -35,15 +27,25 @@ import org.apache.logging.log4j.spi.AbstractLogger;
 import org.apache.logging.log4j.spi.MessageFactory2Adapter;
 import org.apache.logging.log4j.status.StatusData;
 import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.test.junit.Resources;
 import org.apache.logging.log4j.test.junit.StatusLoggerLevel;
 import org.apache.logging.log4j.util.Constants;
 import org.apache.logging.log4j.util.MessageSupplier;
 import org.apache.logging.log4j.util.Supplier;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
 
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+
 @StatusLoggerLevel("WARN")
-@ResourceLock("log4j2.MarkerManager")
+@ResourceLock(value = Resources.MARKER_MANAGER, mode = ResourceAccessMode.READ)
 public class AbstractLoggerTest {
 
     private static final StringBuilder CHAR_SEQ = new StringBuilder("CharSeq");
@@ -64,7 +66,7 @@ public class AbstractLoggerTest {
 
     private static final Message param = new ParameterizedMessage(pattern, p1, p2);
 
-    private static final Marker MARKER = MarkerManager.getMarker("TEST");
+    private final Marker MARKER = MarkerManager.getMarker("TEST");
     private static final String MARKER_NAME = "TEST";
 
     private static final LogEvent[] EVENTS = new LogEvent[] {
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/LoggerTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/LoggerTest.java
index 39637040b2..0eb5f8d6d5 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/LoggerTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/LoggerTest.java
@@ -16,6 +16,11 @@
  */
 package org.apache.logging.log4j;
 
+import java.util.Date;
+import java.util.List;
+import java.util.Locale;
+import java.util.Properties;
+
 import org.apache.logging.log4j.message.EntryMessage;
 import org.apache.logging.log4j.message.JsonMessage;
 import org.apache.logging.log4j.message.Message;
@@ -28,22 +33,27 @@ import org.apache.logging.log4j.message.StringFormatterMessageFactory;
 import org.apache.logging.log4j.message.StructuredDataMessage;
 import org.apache.logging.log4j.spi.MessageFactory2Adapter;
 import org.apache.logging.log4j.test.TestLogger;
+import org.apache.logging.log4j.test.junit.Resources;
+import org.apache.logging.log4j.test.junit.UsingThreadContextMap;
 import org.apache.logging.log4j.util.Strings;
 import org.apache.logging.log4j.util.Supplier;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
-import java.util.Date;
-import java.util.List;
-import java.util.Locale;
-import java.util.Properties;
+import org.junitpioneer.jupiter.ReadsSystemProperty;
 
-import static org.hamcrest.CoreMatchers.*;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.endsWith;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.MatcherAssert.assertThat;
-import static org.junit.jupiter.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
-@ResourceLock("log4j2.MarkerManager")
-@ResourceLock("log4j2.TestLogger")
+@ResourceLock(value = Resources.MARKER_MANAGER, mode = ResourceAccessMode.READ)
+@ReadsSystemProperty
 public class LoggerTest {
 
     private static class TestParameterizedMessageFactory {
@@ -54,7 +64,7 @@ public class LoggerTest {
         // empty
     }
 
-    private final TestLogger logger = (TestLogger) LogManager.getLogger("LoggerTest");
+    private final TestLogger logger = (TestLogger) LogManager.getLogger(LoggerTest.class);
     private final Marker marker = MarkerManager.getMarker("test");
     private final List<String> results = logger.getEntries();
 
@@ -65,12 +75,12 @@ public class LoggerTest {
         logger.atWarn().withThrowable(new Throwable("This is a test")).log((Message) new SimpleMessage("Log4j rocks!"));
         assertEquals(3, results.size());
         assertThat("Incorrect message 1", results.get(0),
-                equalTo(" DEBUG org.apache.logging.log4j.LoggerTest.builder(LoggerTest.java:63) Hello"));
+                equalTo(" DEBUG org.apache.logging.log4j.LoggerTest.builder(LoggerTest.java:73) Hello"));
         assertThat("Incorrect message 2", results.get(1), equalTo("test ERROR Hello John"));
         assertThat("Incorrect message 3", results.get(2),
                 startsWith(" WARN Log4j rocks! java.lang.Throwable: This is a test"));
         assertThat("Throwable incorrect in message 3", results.get(2),
-                containsString("at org.apache.logging.log4j.LoggerTest.builder(LoggerTest.java:65)"));
+                containsString("org.apache.logging.log4j.LoggerTest.builder(LoggerTest.java:75)"));
     }
 
     @Test
@@ -533,8 +543,8 @@ public class LoggerTest {
     }
 
     @Test
+    @UsingThreadContextMap
     public void mdc() {
-
         ThreadContext.put("TestYear", Integer.toString(2010));
         logger.debug("Debug message");
         final String testYear = ThreadContext.get("TestYear");
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/MarkerTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/MarkerTest.java
index a286ca2ba5..5c652341fc 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/MarkerTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/MarkerTest.java
@@ -16,13 +16,18 @@
  */
 package org.apache.logging.log4j;
 
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import org.apache.logging.log4j.test.junit.Resources;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.ResourceAccessMode;
 import org.junit.jupiter.api.parallel.ResourceLock;
 
-import static org.junit.jupiter.api.Assertions.*;
-
-@ResourceLock("log4j2.MarkerManager")
+@ResourceLock(value = Resources.MARKER_MANAGER, mode = ResourceAccessMode.READ_WRITE)
 public class MarkerTest {
 
     @BeforeEach
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/simple/SimpleLoggerTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/simple/SimpleLoggerTest.java
index ab1c328e3e..8b0586e7f7 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/simple/SimpleLoggerTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/simple/SimpleLoggerTest.java
@@ -23,10 +23,8 @@ import org.apache.logging.log4j.util.Constants;
 import org.junit.jupiter.api.Tag;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
-import org.junit.jupiter.api.parallel.ResourceLock;
 
 @Tag("smoke")
-@ResourceLock("log4j2.LoggerContextFactory")
 public class SimpleLoggerTest {
 
     @RegisterExtension