You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by rp...@apache.org on 2016/05/17 15:32:16 UTC

logging-log4j2 git commit: LOG4J2-1348 applied latest patch where CloseableThreadContext returns an instance that allows chained method calls like CloseableThreadContext.put(key, value).put(anotherKey, anotherValue);

Repository: logging-log4j2
Updated Branches:
  refs/heads/master 94b5a8062 -> 7fca94f59


LOG4J2-1348 applied latest patch where CloseableThreadContext returns an instance that allows chained method calls like CloseableThreadContext.put(key, value).put(anotherKey, anotherValue);


Project: http://git-wip-us.apache.org/repos/asf/logging-log4j2/repo
Commit: http://git-wip-us.apache.org/repos/asf/logging-log4j2/commit/7fca94f5
Tree: http://git-wip-us.apache.org/repos/asf/logging-log4j2/tree/7fca94f5
Diff: http://git-wip-us.apache.org/repos/asf/logging-log4j2/diff/7fca94f5

Branch: refs/heads/master
Commit: 7fca94f59224aceb99ee833d2758e5a6ae4650ca
Parents: 94b5a80
Author: rpopma <rp...@apache.org>
Authored: Wed May 18 00:32:49 2016 +0900
Committer: rpopma <rp...@apache.org>
Committed: Wed May 18 00:32:49 2016 +0900

----------------------------------------------------------------------
 .../logging/log4j/CloseableThreadContext.java   | 195 +++++++++----------
 .../log4j/CloseableThreadContextTest.java       |  94 +++++++--
 src/site/xdoc/manual/thread-context.xml         |  11 +-
 3 files changed, 178 insertions(+), 122 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fca94f5/log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java b/log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java
index d2f7957..ff256f2 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/CloseableThreadContext.java
@@ -17,42 +17,41 @@
 package org.apache.logging.log4j;
 
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 
-import org.apache.logging.log4j.util.Strings;
-
 /**
  * Adds entries to the {@link ThreadContext stack or map} and them removes them when the object is closed, e.g. as part
  * of a try-with-resources.
  *
  * @since 2.6
  */
-public class CloseableThreadContext implements AutoCloseable {
+public class CloseableThreadContext {
+
+    private CloseableThreadContext() {
+    }
 
     /**
      * Pushes new diagnostic context information on to the Thread Context Stack. The information will be popped off when
      * the instance is closed.
      *
-     * @param message
-     *            The new diagnostic context information.
+     * @param message The new diagnostic context information.
      * @return a new instance that will back out the changes when closed.
      */
-    public static CloseableThreadContext push(final String message) {
-        return new CloseableThreadContext(message);
+    public static CloseableThreadContext.Instance push(final String message) {
+        return new CloseableThreadContext.Instance().push(message);
     }
 
     /**
      * Pushes new diagnostic context information on to the Thread Context Stack. The information will be popped off when
      * the instance is closed.
      *
-     * @param message
-     *            The new diagnostic context information.
-     * @param args
-     *            Parameters for the message.
+     * @param message The new diagnostic context information.
+     * @param args    Parameters for the message.
      * @return a new instance that will back out the changes when closed.
      */
-    public static CloseableThreadContext push(final String message, final Object... args) {
-        return new CloseableThreadContext(message, args);
+    public static CloseableThreadContext.Instance push(final String message, final Object... args) {
+        return new CloseableThreadContext.Instance().push(message, args);
     }
 
     /**
@@ -60,113 +59,101 @@ public class CloseableThreadContext implements AutoCloseable {
      * {@link ThreadContext} will be replaced with the supplied values, and restored back to their original values when
      * the instance is closed.
      *
-     * @param firstKey
-     *            The first key to be added
-     * @param firstValue
-     *            The first value to be added
-     * @param subsequentKeyValuePairs
-     *            Any subsequent key/value pairs to be added. Note: If the last key does not have a corresponding value
-     *            then an empty String will be used as a value.
+     * @param key   The  key to be added
+     * @param value The value to be added
      * @return a new instance that will back out the changes when closed.
      */
-    public static CloseableThreadContext put(final String firstKey, final String firstValue,
-            final String... subsequentKeyValuePairs) {
-        return new CloseableThreadContext(firstKey, firstValue, subsequentKeyValuePairs);
+    public static CloseableThreadContext.Instance put(final String key, final String value) {
+        return new CloseableThreadContext.Instance().put(key, value);
     }
 
-    private final boolean isStack;
+    public static class Instance implements AutoCloseable {
 
-    private final Map<String, String> oldValues = new HashMap<>();
+        private int pushCount = 0;
+        private final Map<String, String> originalValues = new HashMap<>();
 
-    /**
-     * Creates an instance of a {@code CloseableThreadContext} that pushes new diagnostic context information on to the
-     * Thread Context Stack. The information will be popped off when the instance is closed.
-     *
-     * @param message
-     *            The new diagnostic context information.
-     */
-    public CloseableThreadContext(final String message) {
-        this.isStack = true;
-        ThreadContext.push(message);
-    }
+        private Instance() {
+        }
 
-    /**
-     * Creates an instance of a {@code CloseableThreadContext} that pushes new diagnostic context information on to the
-     * Thread Context Stack. The information will be popped off when the instance is closed.
-     *
-     * @param message
-     *            The new diagnostic context information.
-     * @param args
-     *            Parameters for the message.
-     */
-    public CloseableThreadContext(final String message, final Object... args) {
-        this.isStack = true;
-        ThreadContext.push(message, args);
-    }
+        /**
+         * Pushes new diagnostic context information on to the Thread Context Stack. The information will be popped off when
+         * the instance is closed.
+         *
+         * @param message The new diagnostic context information.
+         * @return the instance that will back out the changes when closed.
+         */
+        public Instance push(final String message) {
+            ThreadContext.push(message);
+            pushCount++;
+            return this;
+        }
 
-    /**
-     * Creates an instance of a {@code CloseableThreadContext} that populates the Thread Context Map with the supplied
-     * key/value pairs. Any existing keys in the ThreadContext will be replaced with the supplied values, and restored
-     * back to their original values when the instance is closed.
-     *
-     * @param firstKey
-     *            The first key to be added
-     * @param firstValue
-     *            The first value to be added
-     * @param subsequentKeyValuePairs
-     *            Any subsequent key/value pairs to be added. Note: If the last key does not have a corresponding value
-     *            then an empty String will be used as a value.
-     */
-    public CloseableThreadContext(final String firstKey, final String firstValue,
-            final String... subsequentKeyValuePairs) {
-        this.isStack = false;
-        storeAndSet(firstKey, firstValue);
-        for (int i = 0; i < subsequentKeyValuePairs.length; i += 2) {
-            final String key = subsequentKeyValuePairs[i];
-            final String value = (i + 1) < subsequentKeyValuePairs.length ? subsequentKeyValuePairs[i + 1] : Strings.EMPTY;
-            storeAndSet(key, value);
+        /**
+         * Pushes new diagnostic context information on to the Thread Context Stack. The information will be popped off when
+         * the instance is closed.
+         *
+         * @param message The new diagnostic context information.
+         * @param args    Parameters for the message.
+         * @return the instance that will back out the changes when closed.
+         */
+        public Instance push(final String message, final Object[] args) {
+            ThreadContext.push(message, args);
+            pushCount++;
+            return this;
         }
-    }
 
-    /**
-     * Removes the values from the {@link ThreadContext}.
-     * <p>
-     * If this {@code CloseableThreadContext} was added to the {@link ThreadContext} <em>stack</em>, then this will pop
-     * the diagnostic information off the stack.
-     * </p>
-     * <p>
-     * If the {@code CloseableThreadContext} was added to the {@link ThreadContext} <em>map</em>, then this will either
-     * remove the values that were added, or restore them to their original values it they already existed.
-     * </p>
-     */
-    @Override
-    public void close() {
-        if (this.isStack) {
+        /**
+         * Populates the Thread Context Map with the supplied key/value pairs. Any existing keys in the
+         * {@link ThreadContext} will be replaced with the supplied values, and restored back to their original values when
+         * the instance is closed.
+         *
+         * @param key   The  key to be added
+         * @param value The value to be added
+         * @return the instance that will back out the changes when closed.
+         */
+        public Instance put(final String key, final String value) {
+            // If there are no existing values, a null will be stored as an old value
+            if (!originalValues.containsKey(key)) {
+                originalValues.put(key, ThreadContext.get(key));
+            }
+            ThreadContext.put(key, value);
+            return this;
+        }
+
+        /**
+         * Removes the values from the {@link ThreadContext}.
+         * <p>
+         * Values pushed to the {@link ThreadContext} <em>stack</em> will be popped off.
+         * </p>
+         * <p>
+         * Values put on the {@link ThreadContext} <em>map</em> will be removed, or restored to their original values it they already existed.
+         * </p>
+         */
+        @Override
+        public void close() {
             closeStack();
-        } else {
             closeMap();
         }
-    }
 
-    private void closeMap() {
-        for (final Map.Entry<String, String> entry : oldValues.entrySet()) {
-            // If the old value was null, remove it from the ThreadContext
-            if (null == entry.getValue()) {
-                ThreadContext.remove(entry.getKey());
-            } else {
-                ThreadContext.put(entry.getKey(), entry.getValue());
+        private void closeMap() {
+            for (Iterator<Map.Entry<String, String>> it = originalValues.entrySet().iterator(); it.hasNext(); ) {
+                final Map.Entry<String, String> entry = it.next();
+                final String key = entry.getKey();
+                final String originalValue = entry.getValue();
+                if (null == originalValue) {
+                    ThreadContext.remove(key);
+                } else {
+                    ThreadContext.put(key, originalValue);
+                }
+                it.remove();
             }
         }
-    }
-
-    private String closeStack() {
-        return ThreadContext.pop();
-    }
 
-    private void storeAndSet(final String key, final String value) {
-        // If there are no existing values, a null will be stored as an old value
-        oldValues.put(key, ThreadContext.get(key));
-        ThreadContext.put(key, value);
+        private void closeStack() {
+            for (int i = 0; i < pushCount; i++) {
+                ThreadContext.pop();
+            }
+            pushCount = 0;
+        }
     }
-
 }

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fca94f5/log4j-api/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
----------------------------------------------------------------------
diff --git a/log4j-api/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java b/log4j-api/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
index 63f3ec8..ed312fe 100644
--- a/log4j-api/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
+++ b/log4j-api/src/test/java/org/apache/logging/log4j/CloseableThreadContextTest.java
@@ -39,7 +39,7 @@ public class CloseableThreadContextTest {
 
     @Test
     public void shouldAddAnEntryToTheMap() throws Exception {
-        try (final CloseableThreadContext ignored = CloseableThreadContext.put(key, value)) {
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value)) {
             assertThat(ThreadContext.get(key), is(value));
         }
     }
@@ -48,7 +48,7 @@ public class CloseableThreadContextTest {
     public void shouldAddTwoEntriesToTheMap() throws Exception {
         final String key2 = "key2";
         final String value2 = "value2";
-        try (final CloseableThreadContext ignored = CloseableThreadContext.put(key, value, key2, value2)) {
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value).put(key2, value2)) {
             assertThat(ThreadContext.get(key), is(value));
             assertThat(ThreadContext.get(key2), is(value2));
         }
@@ -59,9 +59,9 @@ public class CloseableThreadContextTest {
         final String oldValue = "oldValue";
         final String innerValue = "innerValue";
         ThreadContext.put(key, oldValue);
-        try (final CloseableThreadContext ignored = CloseableThreadContext.put(key, value)) {
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value)) {
             assertThat(ThreadContext.get(key), is(value));
-            try (final CloseableThreadContext ignored2 = CloseableThreadContext.put(key, innerValue)) {
+            try (final CloseableThreadContext.Instance ignored2 = CloseableThreadContext.put(key, innerValue)) {
                 assertThat(ThreadContext.get(key), is(innerValue));
             }
             assertThat(ThreadContext.get(key), is(value));
@@ -73,27 +73,48 @@ public class CloseableThreadContextTest {
     public void shouldPreserveOldEntriesFromTheMapWhenAutoClosed() throws Exception {
         final String oldValue = "oldValue";
         ThreadContext.put(key, oldValue);
-        try (final CloseableThreadContext ignored = CloseableThreadContext.put(key, value)) {
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value)) {
             assertThat(ThreadContext.get(key), is(value));
         }
         assertThat(ThreadContext.get(key), is(oldValue));
     }
 
     @Test
+    public void ifTheSameKeyIsAddedTwiceTheOriginalShouldBeUsed() throws Exception {
+        final String oldValue = "oldValue";
+        final String secondValue = "innerValue";
+        ThreadContext.put(key, oldValue);
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value).put(key, secondValue)) {
+            assertThat(ThreadContext.get(key), is(secondValue));
+        }
+        assertThat(ThreadContext.get(key), is(oldValue));
+    }
+
+    @Test
     public void shouldPushAndPopAnEntryToTheStack() throws Exception {
         final String message = "message";
-        try (final CloseableThreadContext ignored = CloseableThreadContext.push(message)) {
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.push(message)) {
             assertThat(ThreadContext.peek(), is(message));
         }
         assertThat(ThreadContext.peek(), is(""));
     }
 
     @Test
+    public void shouldPushAndPopTwoEntriesToTheStack() throws Exception {
+        final String message1 = "message1";
+        final String message2 = "message2";
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.push(message1).push(message2)) {
+            assertThat(ThreadContext.peek(), is(message2));
+        }
+        assertThat(ThreadContext.peek(), is(""));
+    }
+
+    @Test
     public void shouldPushAndPopAParameterizedEntryToTheStack() throws Exception {
         final String parameterizedMessage = "message {}";
         final String parameterizedMessageParameter = "param";
         final String formattedMessage = parameterizedMessage.replace("{}", parameterizedMessageParameter);
-        try (final CloseableThreadContext ignored = CloseableThreadContext.push(parameterizedMessage,
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.push(parameterizedMessage,
                 parameterizedMessageParameter)) {
             assertThat(ThreadContext.peek(), is(formattedMessage));
         }
@@ -102,19 +123,66 @@ public class CloseableThreadContextTest {
 
     @Test
     public void shouldRemoveAnEntryFromTheMapWhenAutoClosed() throws Exception {
-        try (final CloseableThreadContext ignored = CloseableThreadContext.put(key, value)) {
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value)) {
             assertThat(ThreadContext.get(key), is(value));
         }
         assertThat(ThreadContext.containsKey(key), is(false));
     }
 
     @Test
-    public void shouldUseAnEmptyStringIfNoValueIsSupplied() throws Exception {
-        final String key2 = "key2";
-        try (final CloseableThreadContext ignored = CloseableThreadContext.put(key, value, key2)) {
+    public void shouldAddEntriesToBothStackAndMap() throws Exception {
+        final String stackValue = "something";
+        try (final CloseableThreadContext.Instance ignored = CloseableThreadContext.put(key, value).push(stackValue)) {
             assertThat(ThreadContext.get(key), is(value));
-            assertThat(ThreadContext.get(key2), is(""));
+            assertThat(ThreadContext.peek(), is(stackValue));
         }
+        assertThat(ThreadContext.containsKey(key), is(false));
+        assertThat(ThreadContext.peek(), is(""));
     }
 
-}
\ No newline at end of file
+    @Test
+    public void canReuseCloseableThreadContext() throws Exception {
+        final String stackValue = "something";
+        // Create a ctc and close it
+        final CloseableThreadContext.Instance ctc = CloseableThreadContext.push(stackValue).put(key, value);
+        assertThat(ThreadContext.get(key), is(value));
+        assertThat(ThreadContext.peek(), is(stackValue));
+        ctc.close();
+
+        assertThat(ThreadContext.containsKey(key), is(false));
+        assertThat(ThreadContext.peek(), is(""));
+
+        final String anotherKey = "key2";
+        final String anotherValue = "value2";
+        final String anotherStackValue = "something else";
+        // Use it again
+        ctc.push(anotherStackValue).put(anotherKey, anotherValue);
+        assertThat(ThreadContext.get(anotherKey), is(anotherValue));
+        assertThat(ThreadContext.peek(), is(anotherStackValue));
+        ctc.close();
+
+        assertThat(ThreadContext.containsKey(anotherKey), is(false));
+        assertThat(ThreadContext.peek(), is(""));
+    }
+
+    @Test
+    public void closeIsIdempotent() throws Exception {
+
+        final String originalMapValue = "map to keep";
+        final String originalStackValue = "stack to keep";
+        ThreadContext.put(key, originalMapValue);
+        ThreadContext.push(originalStackValue);
+
+        final String newMapValue = "temp map value";
+        final String newStackValue = "temp stack to keep";
+        final CloseableThreadContext.Instance ctc = CloseableThreadContext.push(newStackValue).put(key, newMapValue);
+
+        ctc.close();
+        assertThat(ThreadContext.get(key), is(originalMapValue));
+        assertThat(ThreadContext.peek(), is(originalStackValue));
+
+        ctc.close();
+        assertThat(ThreadContext.get(key), is(originalMapValue));
+        assertThat(ThreadContext.peek(), is(originalStackValue));
+    }
+}

http://git-wip-us.apache.org/repos/asf/logging-log4j2/blob/7fca94f5/src/site/xdoc/manual/thread-context.xml
----------------------------------------------------------------------
diff --git a/src/site/xdoc/manual/thread-context.xml b/src/site/xdoc/manual/thread-context.xml
index a073dae..316de96 100644
--- a/src/site/xdoc/manual/thread-context.xml
+++ b/src/site/xdoc/manual/thread-context.xml
@@ -100,13 +100,13 @@ ThreadContext.clear();</pre>
 
           <h4>CloseableThreadContext</h4>
 		  <p>When placing items on the stack or map, it's necessary to remove then again when appropriate. To assist with
-		  this, the <tt>CloseableThreadContext</tt> implements the <a href="http://docs.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html">AutoCloseable 
-		  interface</a>. This allows items to be pushed to the stack or put in the map, and removed when the <tt>close()</tt> method is called - 
-		  or automatically as part of a try-with-resources. For example, to temporarily push something on to the stack and then remove it: 
+		  this, the <tt>CloseableThreadContext</tt> implements the <a href="http://docs.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html">AutoCloseable
+		  interface</a>. This allows items to be pushed to the stack or put in the map, and removed when the <tt>close()</tt> method is called -
+		  or automatically as part of a try-with-resources. For example, to temporarily push something on to the stack and then remove it:
           </p>
 		  <pre class="prettyprint linenums">
 // Add to the ThreadContext stack for this try block only;
-try (final CloseableThreadContext ctc = CloseableThreadContext.push(UUID.randomUUID().toString())) { 
+try (final CloseableThreadContext.Instance ctc = CloseableThreadContext.push(UUID.randomUUID().toString())) {
 
     logger.debug("Message 1");
 .
@@ -120,7 +120,8 @@ try (final CloseableThreadContext ctc = CloseableThreadContext.push(UUID.randomU
           </p>
 		  <pre class="prettyprint linenums">
 // Add to the ThreadContext map for this try block only;
-try (final CloseableThreadContext ctc = CloseableThreadContext.put("id", UUID.randomUUID().toString(), "loginId", session.getAttribute("loginId"))) { 
+try (final CloseableThreadContext.Instance ctc = CloseableThreadContext.put("id", UUID.randomUUID().toString())
+                                                                .put("loginId", session.getAttribute("loginId"))) {
 
     logger.debug("Message 1");
 .