Merge pull request #83 from hkupty/mdc-write-safety

MDC: Replace internal implementation

CHANGELOG.md

@@ -6,6 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) with an addendum:
 the PATCH component is omitted when its value is `0`.
 
+## Unreleased
+
+### `penna-core`
+
+#### Changed
+
+- Replace internal MDC storage implementation([#83](https://github.com/hkupty/penna/pull/83))
+
 ## 0.7.2
 
 ### `penna-core`

penna-core/src/main/java/penna/core/internals/store/StringMap.java

@@ -0,0 +1,11 @@
+package penna.core.internals.store;
+
+import java.util.Map;
+
+/**
+ * The internal MDC implementation assumes a {@link Map} of Strings to String, so this interface
+ * exists to ensure we can experiment and replace implementations without breaking SLF4J's binding implementation
+ */
+public sealed interface StringMap extends Map<String, String> permits StringTreeMap {
+    StringMap copy();
+}

penna-core/src/main/java/penna/core/internals/store/StringTreeMap.java

@@ -0,0 +1,29 @@
+package penna.core.internals.store;
+
+import java.util.Map;
+import java.util.TreeMap;
+
+/**
+ * Simple wrapper over {@link TreeMap} just to provide two basic guarantees:
+ * - We control the copying of the map;
+ * - We control the initialization;
+ * <br />
+ * This is not strictly necessary, but it is a very performant implementation of {@link StringMap},
+ * which is our goal.
+ */
+public final class StringTreeMap extends TreeMap<String, String> implements StringMap {
+
+    public StringTreeMap() {
+        super();
+    }
+
+    public StringTreeMap(Map<? extends String, ? extends String> m) {
+        super();
+        super.putAll(m);
+    }
+
+    @Override
+    public StringMap copy() {
+        return new StringTreeMap(this);
+    }
+}

penna-core/src/main/java/penna/core/slf4j/PennaMDCAdapter.java

@@ -7,57 +7,61 @@
 import java.util.Map;
 import java.util.function.BiConsumer;
 
+/**
+ * This is just a proxy over the internal implementation of MDC.
+ * The MDC storage is a thread-local variable managed by {@link Mdc.Control}.
+ */
 public final class PennaMDCAdapter implements MDCAdapter {
 
     public void forEach(BiConsumer<String, String> action) {
-        var node = Mdc.Inner.mdcStorage.get();
+        var node = Mdc.Control.mdcStorage.get();
         node.forEach(action);
     }
 
     public boolean isNotEmpty() {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         return inner.isNotEmpty();
     }
 
     @Override
     public void put(String key, String val) {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         inner.put(key, val);
     }
 
     @Override
     public String get(String key) {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         return inner.get(key);
     }
 
     @Override
     public void remove(String key) {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         inner.remove(key);
     }
 
     @Override
     public void clear() {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         inner.clear();
     }
 
     @Override
     public Map<String, String> getCopyOfContextMap() {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         return inner.getCopyOfContextMap();
     }
 
     @Override
     public void setContextMap(Map<String, String> contextMap) {
-        Mdc inner = Mdc.Inner.mdcStorage.get();
+        Mdc inner = Mdc.Control.mdcStorage.get();
         inner.setContextMap(contextMap);
     }
 
     @Override
     public void pushByKey(String key, String value) {
-        // Intentionally left black, not supported at the moment
+        // Intentionally left blank, not supported at the moment
     }
 
     @Override
@@ -72,6 +76,6 @@ public Deque<String> getCopyOfDequeByKey(String key) {
 
     @Override
     public void clearDequeByKey(String key) {
-        // Intentionally left black, not supported at the moment
+        // Intentionally left blank, not supported at the moment
     }
 }

penna-core/src/main/java/penna/core/slf4j/mdc/EmptyMdc.java

@@ -0,0 +1,93 @@
+package penna.core.slf4j.mdc;
+
+import org.jetbrains.annotations.NotNull;
+import penna.core.internals.store.StringTreeMap;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.BiConsumer;
+
+public final class EmptyMdc implements Mdc {
+    @Override
+    public String put(String key, String val) {
+        var inner = new StringTreeMap();
+        inner.put(key, val);
+        Control.mdcStorage.set(new MdcStorage(inner));
+        return key;
+    }
+
+    @Override
+    public String remove(Object key) {
+        return null;
+    }
+
+    @Override
+    public void putAll(@NotNull Map<? extends String, ? extends String> m) {
+        Control.mdcStorage.set(new MdcStorage(new StringTreeMap(m)));
+    }
+
+    @Override
+    public void clear() {}
+
+    @NotNull
+    @Override
+    public Set<String> keySet() {
+        return Set.of();
+    }
+
+    @NotNull
+    @Override
+    public Collection<String> values() {
+        return List.of();
+    }
+
+    @NotNull
+    @Override
+    public Set<Entry<String, String>> entrySet() {
+        return Set.of();
+    }
+
+    @Override
+    public boolean isNotEmpty() {
+        return false;
+    }
+
+    @Override
+    public int size() {
+        return 0;
+    }
+
+    @Override
+    public void remove(String key) {}
+
+    @Override
+    public boolean isEmpty() {return true;}
+
+    @Override
+    public boolean containsKey(Object key) {
+        return false;
+    }
+
+    @Override
+    public boolean containsValue(Object value) {
+        return false;
+    }
+
+    @Override
+    public String get(Object key) {return null;}
+
+    @Override
+    public void forEach(BiConsumer<? super String, ? super String> action) {}
+
+    @Override
+    public String get(String key) {
+        return null;
+    }
+
+    @Override
+    public Map<String, String> getCopyOfContextMap() {
+        return this;
+    }
+}