ppkarwasz commented on code in PR #2438:
URL: https://github.com/apache/logging-log4j2/pull/2438#discussion_r1554553444


##########
log4j-api/src/main/java/org/apache/logging/log4j/spi/internal/DefaultScopedContextProvider.java:
##########
@@ -0,0 +1,399 @@
+/*
+ * 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.spi.internal;
+
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.function.Supplier;
+import org.apache.logging.log4j.Logger;
+import org.apache.logging.log4j.ScopedContext;
+import org.apache.logging.log4j.ThreadContext;
+import org.apache.logging.log4j.spi.ScopedContextProvider;
+import org.apache.logging.log4j.status.StatusLogger;
+import org.apache.logging.log4j.util.StringMap;
+
+/**
+ * An implementation of {@link ScopedContextProvider} that uses the simplest 
implementation.
+ * @since 2.24.0
+ */
+public class DefaultScopedContextProvider implements ScopedContextProvider {
+
+    public static final Logger LOGGER = StatusLogger.getLogger();
+    public static final ScopedContextProvider INSTANCE = new 
DefaultScopedContextProvider();
+
+    private final ThreadLocal<Instance> scopedContext = new ThreadLocal<>();
+
+    /**
+     * Returns an immutable Map containing all the key/value pairs as Object 
objects.
+     * @return The current context Instance.
+     */
+    private Optional<Instance> getContext() {
+        return Optional.ofNullable(scopedContext.get());
+    }
+
+    /**
+     * Add the ScopeContext.
+     * @param context The ScopeContext.
+     */
+    private void addScopedContext(final Instance context) {
+        Instance current = scopedContext.get();
+        scopedContext.set(context);
+    }
+
+    /**
+     * Remove the top ScopeContext.
+     */
+    private void removeScopedContext() {
+        final Instance current = scopedContext.get();
+        if (current != null) {
+            if (current.parent != null) {
+                scopedContext.set(current.parent);
+            } else {
+                scopedContext.remove();
+            }
+        }
+    }
+
+    @Override
+    public Map<String, ?> getContextMap() {
+        final Optional<Instance> context = getContext();
+        return context.isPresent()
+                        && context.get().contextMap != null
+                        && !context.get().contextMap.isEmpty()
+                ? Collections.unmodifiableMap(context.get().contextMap)
+                : Collections.emptyMap();
+    }
+
+    /**
+     * Return the value of the key from the current ScopedContext, if there is 
one and the key exists.
+     * @param key The key.
+     * @return The value of the key in the current ScopedContext.
+     */
+    @Override
+    public Object getValue(final String key) {
+        final Optional<Instance> context = getContext();
+        return context.map(instance -> instance.contextMap)
+                .map(map -> map.get(key))
+                .orElse(null);
+    }
+
+    /**
+     * Return String value of the key from the current ScopedContext, if there 
is one and the key exists.
+     * @param key The key.
+     * @return The value of the key in the current ScopedContext.
+     */
+    @Override
+    public String getString(final String key) {
+        final Optional<Instance> context = getContext();
+        if (context.isPresent()) {
+            final Object obj = context.get().contextMap.get(key);
+            if (obj != null) {
+                return obj.toString();
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Adds all the String rendered objects in the context map to the provided 
Map.
+     * @param map The Map to add entries to.
+     */
+    @Override
+    public void addContextMapTo(final StringMap map) {
+        final Optional<Instance> context = getContext();
+        if (context.isPresent()) {
+            final Map<String, ?> contextMap = context.get().contextMap;
+            if (contextMap != null && !contextMap.isEmpty()) {
+                contextMap.forEach((key, value) -> map.putValue(key, 
value.toString()));
+            }
+        }
+    }
+
+    @Override
+    public ScopedContext.Instance newScopedContext() {
+        return getContext().isPresent() ? getContext().get() : null;
+    }
+
+    /**
+     * Creates a ScopedContext Instance with a key/value pair.
+     *
+     * @param key   the key to add.
+     * @param value the value associated with the key.
+     * @return the Instance constructed if a valid key and value were 
provided. Otherwise, either the
+     * current Instance is returned or a new Instance is created if there is 
no current Instance.
+     */
+    @Override
+    public ScopedContext.Instance newScopedContext(final String key, final 
Object value) {
+        if (value != null) {
+            final Instance parent = getContext().isPresent() ? 
getContext().get() : null;
+            if (parent != null) {
+                return new Instance(parent, key, value);
+            } else {
+                return new Instance(this, key, value);
+            }
+        } else {
+            if (getContext().isPresent()) {
+                final Map<String, ?> map = getContextMap();
+                map.remove(key);
+                return new Instance(this, map);
+            }
+        }
+        return newScopedContext();
+    }
+
+    /**
+     * Creates a ScopedContext Instance with a Map of keys and values.
+     * @param map the Map.
+     * @return the ScopedContext Instance constructed.
+     */
+    @Override
+    public ScopedContext.Instance newScopedContext(final Map<String, ?> map) {
+        if (map != null && !map.isEmpty()) {
+            final Map<String, Object> objectMap = new HashMap<>();
+            if (getContext().isPresent()) {
+                objectMap.putAll(getContext().get().contextMap);
+            }
+            map.forEach((key, value) -> {
+                if (value == null || (value instanceof String && ((String) 
value).isEmpty())) {
+                    objectMap.remove(key);
+                } else {
+                    objectMap.put(key, value);
+                }
+            });
+            return new Instance(this, objectMap);
+        } else {
+            return getContext().isPresent() ? getContext().get() : null;
+        }
+    }
+
+    private static void setupContext(
+            final Map<String, Object> contextMap,
+            final Map<String, String> threadContextMap,
+            final Collection<String> contextStack,
+            final Instance context) {
+        Instance scopedContext = context;
+        // If the current context has a Map then we can just use it.
+        if (context.contextMap == null) {
+            do {
+                if (scopedContext.contextMap != null) {
+                    // Once we hit a scope with an already populated Map we 
won't need to go any further.
+                    contextMap.putAll(scopedContext.contextMap);
+                    break;
+                } else if (scopedContext.key != null) {
+                    contextMap.putIfAbsent(scopedContext.key, 
scopedContext.value);
+                }
+                scopedContext = scopedContext.parent;
+            } while (scopedContext != null);
+            scopedContext = new Instance(context.getProvider(), contextMap);
+        }
+        if (threadContextMap != null && !threadContextMap.isEmpty()) {
+            ThreadContext.putAll(threadContextMap);
+        }
+        if (contextStack != null) {
+            ThreadContext.setStack(contextStack);
+        }
+        context.getProvider().addScopedContext(scopedContext);
+    }
+
+    private static final class Instance implements ScopedContext.Instance {
+
+        private final DefaultScopedContextProvider provider;
+        private final Instance parent;
+        private final String key;
+        private final Object value;
+        private final Map<String, ?> contextMap;

Review Comment:
   Point taken.
   
   Would you consider having a `KeyValueInstance` class with `parent`, `key` 
and `value` and a `MapInstance` with `contextMap`? I think it would improve the 
readability of the code. From my perspective it took me some time to understand 
why you don't have null checks when you call 
`getContext().get().contextMap.get(key)`. By splitting the classes it would be 
obvious to anyone that `MapInstance#contextMap` is not null.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to