From 999001e4f75cf8ddddb47c7b97369db066fe65ad Mon Sep 17 00:00:00 2001 From: Nikolas Grottendieck Date: Thu, 23 Dec 2021 11:00:13 +0100 Subject: [PATCH] implement putAll and putAllCloseable - SLF4J-387 Implements putAll and putAllCloseable described in SLF4J-387 Signed-off-by: Nikolas Grottendieck --- slf4j-api/src/main/java/org/slf4j/MDC.java | 76 ++++++++++- .../java/org/slf4j/jul/InvocationTest.java | 123 ++++++++++++++++++ .../java/org/slf4j/nop/InvocationTest.java | 32 +++++ .../org/slf4j/reload4j/InvocationTest.java | 106 +++++++++++++++ .../java/org/slf4j/simple/InvocationTest.java | 40 ++++++ 5 files changed, 376 insertions(+), 1 deletion(-) diff --git a/slf4j-api/src/main/java/org/slf4j/MDC.java b/slf4j-api/src/main/java/org/slf4j/MDC.java index ec6cb76c4..48c3517d4 100644 --- a/slf4j-api/src/main/java/org/slf4j/MDC.java +++ b/slf4j-api/src/main/java/org/slf4j/MDC.java @@ -26,7 +26,9 @@ import java.io.Closeable; import java.util.Deque; +import java.util.HashSet; import java.util.Map; +import java.util.Set; import org.slf4j.helpers.BasicMDCAdapter; import org.slf4j.helpers.NOPMDCAdapter; @@ -74,13 +76,25 @@ public class MDC { */ public static class MDCCloseable implements Closeable { private final String key; + private final Set keys; private MDCCloseable(String key) { this.key = key; + this.keys = null; + } + + private MDCCloseable(Set keys) { + this.key = null; + this.keys = new HashSet<>(keys); } public void close() { - MDC.remove(this.key); + if (this.key != null) { + MDC.remove(this.key); + } + if (this.keys != null) { + this.keys.forEach(MDC::remove); + } } } @@ -123,6 +137,32 @@ public static void put(String key, String val) throws IllegalArgumentException { mdcAdapter.put(key, val); } + /** + * Put a diagnostic context map (the entries parameter) as identified with each + * key into the current thread's diagnostic context map. The + * entries parameter and its keys cannot be null. The + * value of each entry can be null only if the underlying implementation + * supports it. + * + *

+ * This method delegates all work to the MDC of the underlying logging system. + * + * @param entries entries to put in the map + * + * @throws IllegalArgumentException + * in case the "entries" parameter is null + * @since 2.0.0 + */ + public static void putAll(Map entries) throws IllegalArgumentException { + if (entries == null) { + throw new IllegalArgumentException("entries parameter cannot be null"); + } + if (mdcAdapter == null) { + throw new IllegalStateException("MDCAdapter cannot be null. See also " + NULL_MDCA_URL); + } + entries.forEach((k,v) -> mdcAdapter.put(k, v)); + } + /** * Put a diagnostic context value (the val parameter) as identified with the * key parameter into the current thread's diagnostic context map. The @@ -156,6 +196,40 @@ public static MDCCloseable putCloseable(String key, String val) throws IllegalAr return new MDCCloseable(key); } + /** + * Put a diagnostic context map (the entries parameter) as identified with each + * key into the current thread's diagnostic context map. The + * entries parameter and its keys cannot be null. The + * value of each entry can be null only if the underlying implementation + * supports it. + * + *

+ * This method delegates all work to the MDC of the underlying logging system. + *

+ * This method returns a Closeable object which can remove all keys + * that are part of the entries when close is called. + * + *

+ * Useful with Java 9 for example : + * + * try(MDC.MDCCloseable closeable = MDC.putAllCloseable(Map.of(key1, value, key2, value2)) { + * .... + * } + * + * + * @param entries entries to put in the map + * @return a Closeable who can remove all originally supplied keys + * when close is called. + * + * @throws IllegalArgumentException + * in case the "entries" parameter is null + * @since 2.0.0 + */ + public static MDCCloseable putAllCloseable(Map entries) throws IllegalArgumentException { + MDC.putAll(entries); + return new MDCCloseable(entries.keySet()); + } + /** * Get the diagnostic context identified by the key parameter. The * key parameter cannot be null. diff --git a/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java b/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java index 22669c2bf..539f49c24 100755 --- a/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java +++ b/slf4j-jdk14/src/test/java/org/slf4j/jul/InvocationTest.java @@ -30,6 +30,8 @@ import org.junit.Test; import org.slf4j.*; +import java.util.HashMap; +import java.util.Map; import java.util.logging.Handler; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -170,6 +172,127 @@ public void testMDC() { } } + @Test + public void testMDCContextMapValues() { + Map map = new HashMap<>(); + map.put("ka", "va"); + map.put("kb", "vb"); + + MDC.put("k", "v"); + assertEquals("v", MDC.get("k")); + MDC.setContextMap(map); + assertNull(MDC.get("k")); + assertEquals("va", MDC.get("ka")); + assertEquals("vb", MDC.get("kb")); + } + + + @Test + public void testMDCPutAll() { + Map values = new HashMap<>(); + values.put("k1", "v1"); + values.put("k2", "v2"); + + MDC.put("k", "v"); + MDC.putAll(values); + + assertNotNull(MDC.get("k")); + assertNotNull(MDC.get("k1")); + assertNotNull(MDC.get("k2")); + MDC.remove("k1"); + MDC.remove("k2"); + assertNotNull(MDC.get("k")); + assertNull(MDC.get("k1")); + assertNull(MDC.get("k2")); + MDC.clear(); + } + + @Test + public void testMDCCloseable() { + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putCloseable("try-with", "v")) { + assertNotNull(MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNotNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseableOverwrites() { + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putCloseable("pre", "v2")) { + assertNotNull(MDC.get("pre")); + assertEquals("v2", MDC.get("pre")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNull(MDC.get("pre")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAll() { + Map values = new HashMap<>(); + values.put("try-with", "v"); + + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putAllCloseable(values)) { + assertNotNull(MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNotNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAllOverwrites() { + Map values = new HashMap<>(); + values.put("pre", "v2"); + values.put("try-with", "v"); + + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putAllCloseable(values)) { + assertNotNull(MDC.get("pre")); + assertEquals("v2", MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAllImmutable() { + Map values = new HashMap<>(); + values.put("try-with", "v"); + + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putAllCloseable(values)) { + values.remove("try-with"); + assertNotNull(MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNotNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + private void assertLogMessage(String expected, int index) { LogRecord logRecord = listHandler.recordList.get(index); Assert.assertNotNull(logRecord); diff --git a/slf4j-nop/src/test/java/org/slf4j/nop/InvocationTest.java b/slf4j-nop/src/test/java/org/slf4j/nop/InvocationTest.java index 8cb7c0364..54d31e30c 100644 --- a/slf4j-nop/src/test/java/org/slf4j/nop/InvocationTest.java +++ b/slf4j-nop/src/test/java/org/slf4j/nop/InvocationTest.java @@ -33,6 +33,9 @@ import org.slf4j.Marker; import org.slf4j.MarkerFactory; +import java.util.HashMap; +import java.util.Map; + /** * Test whether invoking the SLF4J API causes problems or not. * @@ -115,6 +118,21 @@ public void testMDC() { MDC.clear(); } + @Test + public void testMDCPutAll() { + Map values = new HashMap<>(); + values.put("key1", "value1"); + values.put("key2", "value2"); + MDC.putAll(values); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + MDC.remove("key1"); + MDC.remove("key2"); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + MDC.clear(); + } + @Test public void testMDCCloseable() { MDC.MDCCloseable closeable = MDC.putCloseable("k", "v"); @@ -123,4 +141,18 @@ public void testMDCCloseable() { assertNull(MDC.get("k")); MDC.clear(); } + + @Test + public void testMDCCloseablePutAll() { + Map values = new HashMap<>(); + values.put("key1", "value1"); + values.put("key2", "value2"); + MDC.MDCCloseable closeable = MDC.putAllCloseable(values); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + closeable.close(); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + MDC.clear(); + } } diff --git a/slf4j-reload4j/src/test/java/org/slf4j/reload4j/InvocationTest.java b/slf4j-reload4j/src/test/java/org/slf4j/reload4j/InvocationTest.java index 12d6db190..357f59729 100644 --- a/slf4j-reload4j/src/test/java/org/slf4j/reload4j/InvocationTest.java +++ b/slf4j-reload4j/src/test/java/org/slf4j/reload4j/InvocationTest.java @@ -188,6 +188,112 @@ public void testMDCContextMapValues() { assertEquals("vb", MDC.get("kb")); } + @Test + public void testMDCPutAll() { + Map values = new HashMap<>(); + values.put("k1", "v1"); + values.put("k2", "v2"); + + MDC.put("k", "v"); + MDC.putAll(values); + + assertNotNull(MDC.get("k")); + assertNotNull(MDC.get("k1")); + assertNotNull(MDC.get("k2")); + MDC.remove("k1"); + MDC.remove("k2"); + assertNotNull(MDC.get("k")); + assertNull(MDC.get("k1")); + assertNull(MDC.get("k2")); + MDC.clear(); + } + + @Test + public void testMDCCloseable() { + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putCloseable("try-with", "v")) { + assertNotNull(MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNotNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseableOverwrites() { + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putCloseable("pre", "v2")) { + assertNotNull(MDC.get("pre")); + assertEquals("v2", MDC.get("pre")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNull(MDC.get("pre")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAllImmutable() { + Map values = new HashMap<>(); + values.put("try-with", "v"); + + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putAllCloseable(values)) { + values.remove("try-with"); + assertNotNull(MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNotNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAll() { + Map values = new HashMap<>(); + values.put("try-with", "v"); + + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putAllCloseable(values)) { + assertNotNull(MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNotNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAllOverwrites() { + Map values = new HashMap<>(); + values.put("pre", "v2"); + values.put("try-with", "v"); + + MDC.put("pre", "v"); + try(MDC.MDCCloseable mdcCloseable = MDC.putAllCloseable(values)) { + assertNotNull(MDC.get("pre")); + assertEquals("v2", MDC.get("pre")); + assertNotNull(MDC.get("try-with")); + assertNull(MDC.get("post")); + } + MDC.put("post", "v"); + assertNull(MDC.get("pre")); + assertNull(MDC.get("try-with")); + assertNotNull(MDC.get("post")); + MDC.clear(); + } + @Test public void testCallerInfo() { Logger logger = LoggerFactory.getLogger("testMarker"); diff --git a/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java b/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java index 39d3da0ea..2f1f17b4f 100644 --- a/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java +++ b/slf4j-simple/src/test/java/org/slf4j/simple/InvocationTest.java @@ -27,6 +27,8 @@ import static org.junit.Assert.assertNull; import java.io.PrintStream; +import java.util.HashMap; +import java.util.Map; import org.junit.After; import org.junit.Before; @@ -140,4 +142,42 @@ public void testMDC() { assertNull(MDC.get("k")); MDC.clear(); } + + @Test + public void testMDCPutAll() { + Map values = new HashMap<>(); + values.put("key1", "value1"); + values.put("key2", "value2"); + MDC.putAll(values); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + MDC.remove("key1"); + MDC.remove("key2"); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + MDC.clear(); + } + + @Test + public void testMDCCloseable() { + MDC.MDCCloseable closeable = MDC.putCloseable("k", "v"); + assertNull(MDC.get("k")); + closeable.close(); + assertNull(MDC.get("k")); + MDC.clear(); + } + + @Test + public void testMDCCloseablePutAll() { + Map values = new HashMap<>(); + values.put("key1", "value1"); + values.put("key2", "value2"); + MDC.MDCCloseable closeable = MDC.putAllCloseable(values); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + closeable.close(); + assertNull(MDC.get("key1")); + assertNull(MDC.get("key2")); + MDC.clear(); + } }