Remove Context.getValue() method. (#528)

sjoerdtalsma · web-flow · commit 6eb1899aead0 · 2024-12-02T16:32:44.000+01:00
* Remove Context.getValue() method which should not be necessary.
* Add readme about open telemetry manager.

---------

Signed-off-by: Sjoerd Talsma &lt;sjoerdtalsma@users.noreply.github.com&gt;
diff --git a/context-propagation-api/src/main/java/nl/talsmasoftware/context/api/Context.java b/context-propagation-api/src/main/java/nl/talsmasoftware/context/api/Context.java
@@ -46,19 +46,6 @@
  */
 public interface Context<T> extends Closeable {
 
-    // TODO think about removing Context.getValue as ContextManager.getActiveContextValue() should suffice.
-
-    /**
-     * The value associated with this context.
-     *
-     * <p>
-     * Behaviour is not specified after the context is {@linkplain #close() closed} and depends
-     * on the {@linkplain ContextManager} implementation.<br>
-     *
-     * @return The value associated with this context.
-     */
-    T getValue();
-
     /**
      * Closes this context.
      *
diff --git a/context-propagation-api/src/main/java/nl/talsmasoftware/context/api/ContextManager.java b/context-propagation-api/src/main/java/nl/talsmasoftware/context/api/ContextManager.java
@@ -44,7 +44,6 @@ public interface ContextManager<T> {
      * The value of the currently active context, or {@code null} if no context is active.
      *
      * @return The value of the active context, or {@code null} if no context is active.
-     * @see Context#getValue()
      * @since 2.0.0
      */
     T getActiveContextValue();
diff --git a/context-propagation-api/src/main/java/nl/talsmasoftware/context/api/package-info.java b/context-propagation-api/src/main/java/nl/talsmasoftware/context/api/package-info.java
@@ -18,8 +18,7 @@
  *
  * <h2>{@linkplain nl.talsmasoftware.context.api.Context}</h2>
  * <p>
- * A {@linkplain nl.talsmasoftware.context.api.Context context} contains
- * a {@linkplain nl.talsmasoftware.context.api.Context#getValue() value}.<br>
+ * A {@linkplain nl.talsmasoftware.context.api.Context context} contains a value.<br>
  * There can be one active context per thread. A context remains active until it is closed or another context
  * is activated in the same thread. Normally, a context is backed by a {@linkplain java.lang.ThreadLocal ThreadLocal}
  * variable.
diff --git a/context-propagation-api/src/test/java/nl/talsmasoftware/context/dummy/DummyContext.java b/context-propagation-api/src/test/java/nl/talsmasoftware/context/dummy/DummyContext.java
@@ -41,10 +41,6 @@ public boolean isClosed() {
         return closed.get();
     }
 
-    public String getValue() {
-        return value;
-    }
-
     public void close() {
         if (closed.compareAndSet(false, true) && INSTANCE.get() == this) {
             DummyContext current = INSTANCE.get();
@@ -60,8 +56,8 @@ public void close() {
     }
 
     public static String currentValue() {
-        final Context<String> currentContext = INSTANCE.get();
-        return currentContext != null ? currentContext.getValue() : null;
+        final DummyContext currentContext = INSTANCE.get();
+        return currentContext != null ? currentContext.value : null;
     }
 
     public static void setCurrentValue(String value) {
diff --git a/context-propagation-api/src/test/java/nl/talsmasoftware/context/dummy/ThrowingContextManager.java b/context-propagation-api/src/test/java/nl/talsmasoftware/context/dummy/ThrowingContextManager.java
@@ -85,11 +85,6 @@ private Ctx(String newValue) {
             STORAGE.set(this);
         }
 
-        @Override
-        public String getValue() {
-            return value;
-        }
-
         @Override
         public void close() {
             if (onClose != null) try {
@@ -112,7 +107,7 @@ public void close() {
 
         private static String currentValue() {
             Ctx current = STORAGE.get();
-            return current != null ? current.getValue() : null;
+            return current != null ? current.value : null;
         }
 
         private static void remove() {
diff --git a/context-propagation-core/src/main/java/nl/talsmasoftware/context/core/threadlocal/AbstractThreadLocalContext.java b/context-propagation-core/src/main/java/nl/talsmasoftware/context/core/threadlocal/AbstractThreadLocalContext.java
@@ -56,11 +56,10 @@ public abstract class AbstractThreadLocalContext<T> implements Context<T> {
     protected final Context<T> parentContext;
 
     /**
-     * The actual value, so subclasses can still access it after the context has been closed,
-     * because the default {@link #getValue()} implementation will return <code>null</code> in that case.<br>
+     * The actual value, so subclasses can access it.<br>
      * Please be careful accessing the value after the context was closed:
-     * There is no pre-defined meaningful way to handle this situation, as this depends heavily
-     * on the desired features of the particular implementation.
+     * There is no pre-defined meaningful way to handle this situation,
+     * as this depends heavily on the desired features of the particular implementation.
      */
     protected final T value;
 
@@ -107,15 +106,6 @@ protected boolean isClosed() {
         return closed;
     }
 
-    /**
-     * Returns the value of this context instance, or <code>null</code> if it was already closed.
-     *
-     * @return The value of this context.
-     */
-    public T getValue() {
-        return closed ? null : value;
-    }
-
     /**
      * Closes this context and in case this context is the active context,
      * restores the active context to the (unclosed) parent context.<br>
@@ -131,7 +121,7 @@ public void close() {
 
     /**
      * Returns the classname of this context followed by <code>"{closed}"</code> if it has been closed already;
-     * otherwise the contained {@link #getValue() value} by this context will be added.
+     * otherwise the contained value by this context will be added.
      *
      * @return String representing this context class and either the current value or the fact that it was closed.
      */
diff --git a/context-propagation-core/src/test/java/nl/talsmasoftware/context/dummy/DummyContext.java b/context-propagation-core/src/test/java/nl/talsmasoftware/context/dummy/DummyContext.java
@@ -15,7 +15,6 @@
  */
 package nl.talsmasoftware.context.dummy;
 
-import nl.talsmasoftware.context.api.Context;
 import nl.talsmasoftware.context.core.threadlocal.AbstractThreadLocalContext;
 
 /**
@@ -35,8 +34,8 @@ public boolean isClosed() {
     }
 
     public static String currentValue() {
-        final Context<String> currentContext = INSTANCE.get();
-        return currentContext != null ? currentContext.getValue() : null;
+        final DummyContext currentContext = INSTANCE.get();
+        return currentContext != null ? currentContext.value : null;
     }
 
     public static void setCurrentValue(String value) {
diff --git a/context-propagation-core/src/test/java/nl/talsmasoftware/context/dummy/ThrowingContextManager.java b/context-propagation-core/src/test/java/nl/talsmasoftware/context/dummy/ThrowingContextManager.java
@@ -77,7 +77,7 @@ private Ctx(String newValue) {
 
         private static String currentValue() {
             Ctx current = AbstractThreadLocalContext.current(Ctx.class);
-            return current != null ? current.getValue() : null;
+            return current != null ? current.value : null;
         }
 
         private static void remove() {
diff --git a/managers/context-manager-locale/src/main/java/nl/talsmasoftware/context/managers/locale/CurrentLocaleHolder.java b/managers/context-manager-locale/src/main/java/nl/talsmasoftware/context/managers/locale/CurrentLocaleHolder.java
@@ -115,7 +115,7 @@ public static Context<Locale> set(Locale locale) {
      * @see #getOrDefault()
      */
     public static Optional<Locale> get() {
-        return Optional.ofNullable(LOCALE.get()).map(CurrentLocaleHolder::getValue);
+        return Optional.ofNullable(LOCALE.get()).map(holder -> holder.locale);
     }
 
     /**
@@ -130,16 +130,6 @@ public static Locale getOrDefault() {
         return get().orElseGet(Locale::getDefault);
     }
 
-    /**
-     * The locale in this holder.
-     *
-     * @return The locale in this holder.
-     */
-    @Override
-    public Locale getValue() {
-        return locale;
-    }
-
     /**
      * Find the nearest unclosed holder from this holder.
      *
diff --git a/managers/context-manager-locale/src/test/java/nl/talsmasoftware/context/managers/locale/CurrentLocaleHolderTest.java b/managers/context-manager-locale/src/test/java/nl/talsmasoftware/context/managers/locale/CurrentLocaleHolderTest.java
@@ -51,24 +51,18 @@ void verifyPropagationIntoNewThread() throws InterruptedException {
     void outOfSequenceClosingMustBeSupported() {
         // ctx1 = GERMAN
         Context<Locale> ctx1 = CurrentLocaleHolder.set(Locale.GERMAN);
-        assertThat(ctx1.getValue()).isEqualTo(Locale.GERMAN);
         assertThat(CurrentLocaleHolder.getOrDefault()).isEqualTo(Locale.GERMAN);
 
         // ctx2 = ENGLISH
         Context<Locale> ctx2 = CurrentLocaleHolder.set(Locale.ENGLISH);
-        assertThat(ctx2.getValue()).isEqualTo(Locale.ENGLISH);
         assertThat(CurrentLocaleHolder.getOrDefault()).isEqualTo(Locale.ENGLISH);
 
         // out-of-sequence, ctx1 is closed first.
         ctx1.close();
-        assertThat(ctx1.getValue()).isEqualTo(Locale.GERMAN);
-        assertThat(ctx2.getValue()).isEqualTo(Locale.ENGLISH);
         assertThat(CurrentLocaleHolder.getOrDefault()).isEqualTo(Locale.ENGLISH);
 
         // when ctx2 is closed, because ctx1 was already closed, the remaining context is empty
         ctx2.close();
-        assertThat(ctx1.getValue()).isEqualTo(Locale.GERMAN);
-        assertThat(ctx2.getValue()).isEqualTo(Locale.ENGLISH);
         assertThat(CurrentLocaleHolder.get()).isEmpty();
     }
 
@@ -83,7 +77,6 @@ void testToString_closed() {
         Context<Locale> closedContext = CurrentLocaleHolder.set(Locale.ENGLISH);
         closedContext.close();
 
-        assertThat(closedContext.getValue()).isEqualTo(Locale.ENGLISH);
         assertThat(closedContext).hasToString("CurrentLocaleHolder{closed}");
     }
 
diff --git a/managers/context-manager-locale/src/test/java/nl/talsmasoftware/context/managers/locale/CurrentLocaleManagerTest.java b/managers/context-manager-locale/src/test/java/nl/talsmasoftware/context/managers/locale/CurrentLocaleManagerTest.java
@@ -32,7 +32,6 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.containsString;
-import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasToString;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.nullValue;
@@ -65,14 +64,12 @@ public void cleanup() {
     @Test
     public void testLocalePropagation() throws ExecutionException, InterruptedException {
         try (Context<Locale> ctx1 = CurrentLocaleHolder.set(DUTCH)) {
-            assertThat(ctx1.getValue(), is(DUTCH));
             assertThat(MANAGER.getActiveContextValue(), is(DUTCH));
             assertThat(CurrentLocaleHolder.getOrDefault(), is(DUTCH));
 
             final CountDownLatch blocker = new CountDownLatch(1);
             Future<Locale> slowCall;
             try (Context<Locale> ctx2 = MANAGER.initializeNewContext(GERMAN)) {
-                assertThat(ctx2.getValue(), is(GERMAN));
                 assertThat(MANAGER.getActiveContextValue(), is(GERMAN));
                 assertThat(CurrentLocaleHolder.getOrDefault(), is(GERMAN));
 
@@ -112,28 +109,26 @@ public void testGetCurrentLocaleOrDefault() {
 
     @Test
     public void testClear() {
-        Context<Locale> dutchCtx = MANAGER.initializeNewContext(DUTCH);
-        Context<Locale> englishCtx = MANAGER.initializeNewContext(ENGLISH);
+        MANAGER.initializeNewContext(DUTCH);
+        assertThat(MANAGER.getActiveContextValue(), is(DUTCH));
+        MANAGER.initializeNewContext(ENGLISH);
+        assertThat(MANAGER.getActiveContextValue(), is(ENGLISH));
 
         MANAGER.clear();
         assertThat(MANAGER.getActiveContextValue(), is(nullValue()));
         assertThat(CurrentLocaleHolder.getOrDefault(), is(DEFAULT_LOCALE));
-
-        assertThat(englishCtx.getValue(), equalTo(ENGLISH));
-        assertThat(dutchCtx.getValue(), equalTo(DUTCH));
     }
 
     @Test
-    public void testClearActiveContexts() {
-        Context<Locale> dutchCtx = MANAGER.initializeNewContext(DUTCH);
-        Context<Locale> englishCtx = MANAGER.initializeNewContext(ENGLISH);
+    public void testClearAll() {
+        MANAGER.initializeNewContext(DUTCH);
+        assertThat(MANAGER.getActiveContextValue(), is(DUTCH));
+        MANAGER.initializeNewContext(ENGLISH);
+        assertThat(MANAGER.getActiveContextValue(), is(ENGLISH));
 
         ContextManager.clearAll();
         assertThat(MANAGER.getActiveContextValue(), is(nullValue()));
         assertThat(CurrentLocaleHolder.getOrDefault(), is(DEFAULT_LOCALE));
-
-        assertThat(englishCtx.getValue(), equalTo(ENGLISH));
-        assertThat(dutchCtx.getValue(), equalTo(DUTCH));
     }
 
     @Test
diff --git a/managers/context-manager-opentelemetry/src/main/java/nl/talsmasoftware/context/managers/opentelemetry/OpenTelemetryContextManager.java b/managers/context-manager-opentelemetry/src/main/java/nl/talsmasoftware/context/managers/opentelemetry/OpenTelemetryContextManager.java
@@ -83,12 +83,7 @@ public io.opentelemetry.context.Context getActiveContextValue() {
      */
     @Override
     public Context<io.opentelemetry.context.Context> initializeNewContext(final io.opentelemetry.context.Context value) {
-        return new ScopeWrappingContext<io.opentelemetry.context.Context>(value.makeCurrent()) {
-            @Override
-            public io.opentelemetry.context.Context getValue() {
-                return value;
-            }
-        };
+        return new ScopeWrappingContext<>(value.makeCurrent());
     }
 
     /**
diff --git a/managers/context-manager-opentelemetry/src/main/java/nl/talsmasoftware/context/managers/opentelemetry/ScopeWrappingContext.java b/managers/context-manager-opentelemetry/src/main/java/nl/talsmasoftware/context/managers/opentelemetry/ScopeWrappingContext.java
@@ -23,7 +23,7 @@
  *
  * @param <T> The type of the contained value, left for the subclass to provide.
  */
-abstract class ScopeWrappingContext<T> implements Context<T> {
+class ScopeWrappingContext<T> implements Context<T> {
     private final Scope scope;
 
     /**
diff --git a/managers/context-manager-opentelemetry/src/test/java/nl/talsmasoftware/context/managers/opentelemetry/OpenTelemetryContextManagerTest.java b/managers/context-manager-opentelemetry/src/test/java/nl/talsmasoftware/context/managers/opentelemetry/OpenTelemetryContextManagerTest.java
@@ -62,7 +62,7 @@ void initializeNewContext_makes_specified_Context_current() {
             // verify
             assertThat(Context.current().get(dummyKey)).isEqualTo(dummyValue);
             assertThat(context).isNotNull();
-            assertThat(context.getValue()).isSameAs(newContext);
+            assertThat(subject.getActiveContextValue()).isSameAs(newContext);
         }
 
         assertThat(Context.current().get(dummyKey)).isNull();
diff --git a/managers/context-manager-opentracing/src/main/java/nl/talsmasoftware/context/managers/opentracing/ContextScopeManager.java b/managers/context-manager-opentracing/src/main/java/nl/talsmasoftware/context/managers/opentracing/ContextScopeManager.java
@@ -153,7 +153,7 @@ private ThreadLocalSpanContext(Span newValue) {
 
         private static Span currentSpan() {
             ThreadLocalSpanContext current = SPAN_CONTEXT.get();
-            return current != null ? current.getValue() : null;
+            return current != null ? current.value : null;
         }
 
         private static void remove() {
diff --git a/managers/context-manager-opentracing/src/main/java/nl/talsmasoftware/context/managers/opentracing/SpanManager.java b/managers/context-manager-opentracing/src/main/java/nl/talsmasoftware/context/managers/opentracing/SpanManager.java
@@ -133,11 +133,6 @@ private SpanContext(Span span) {
             this.scope = span != null ? GlobalTracer.get().activateSpan(span) : null;
         }
 
-        @Override
-        public Span getValue() {
-            return closed.get() ? null : activeSpan();
-        }
-
         @Override
         public void close() {
             if (closed.compareAndSet(false, true) && scope != null) {
@@ -147,7 +142,7 @@ public void close() {
 
         @Override
         public String toString() {
-            return getClass().getSimpleName() + (closed.get() ? "{closed}" : "{" + getValue() + '}');
+            return getClass().getSimpleName() + (closed.get() ? "{closed}" : "{" + activeSpan() + '}');
         }
     }
 
diff --git a/managers/context-manager-servletrequest/src/main/java/nl/talsmasoftware/context/managers/servletrequest/ServletRequestContext.java b/managers/context-manager-servletrequest/src/main/java/nl/talsmasoftware/context/managers/servletrequest/ServletRequestContext.java
@@ -38,11 +38,6 @@ final class ServletRequestContext implements Context<ServletRequest> {
         CONTEXT.set(this);
     }
 
-    @Override
-    public ServletRequest getValue() {
-        return closed.get() ? null : value;
-    }
-
     @Override
     public void close() {
         if (closed.compareAndSet(false, true)) {
diff --git a/managers/context-manager-servletrequest/src/test/java/nl/talsmasoftware/context/managers/servletrequest/ServletRequestContextManagerTest.java b/managers/context-manager-servletrequest/src/test/java/nl/talsmasoftware/context/managers/servletrequest/ServletRequestContextManagerTest.java
@@ -88,11 +88,9 @@ public void testCurrentServletRequest() {
     public void testClearableImplementation() {
         final ServletRequest request = mock(ServletRequest.class);
         Context<ServletRequest> ctx = ServletRequestContextManager.provider().initializeNewContext(request);
-        assertThat(ctx.getValue(), is(sameInstance(request)));
         assertThat(ServletRequestContextManager.provider().getActiveContextValue(), is(sameInstance(request)));
 
         ContextManager.clearAll();
-        assertThat(ctx.getValue(), is(nullValue())); // must have been closed
         assertThat(ServletRequestContextManager.provider().getActiveContextValue(), is(nullValue()));
     }
 
diff --git a/readme.md b/readme.md
@@ -97,11 +97,12 @@ out of the box by this context-propagation library:
 
 - [SLF4J MDC (Mapped Diagnostic Context)][slf4j mdc propagation]
 - [Log4j 2 Thread Context][log4j2 thread context propagation]
+- [OpenTelemetry Context][opentelemetry context propagation]
 - [OpenTracing Span contexts][opentracing span propagation]
 - [Spring Security Context]
 - [Locale context][locale context]
 - [ServletRequest contexts][servletrequest propagation]
-- _Yours?_ Feel free to create an issue or pull-request
+- .. _Yours?_ Feel free to create an issue or pull-request
   if you believe there's a general context that was forgotten. 
 
 ## Custom contexts
@@ -165,12 +166,13 @@ configure various timers in the global default metric registry of your applicati
   [javadoc]: https://www.javadoc.io/doc/nl.talsmasoftware.context/context-propagation-root
 
 
+  [locale context]: managers/context-manager-locale
+  [log4j2 thread context propagation]: managers/context-manager-log4j2
+  [opentelemetry context propagation]: managers/context-manager-opentelemetry
+  [opentracing span propagation]: managers/context-manager-opentracing
   [servletrequest propagation]: managers/context-manager-servletrequest
   [slf4j mdc propagation]: managers/context-manager-slf4j
-  [log4j2 thread context propagation]: managers/context-manager-log4j2
-  [locale context]: managers/context-manager-locale
   [spring security context]: managers/context-manager-spring-security
-  [opentracing span propagation]: managers/context-manager-opentracing
   [context propagation metrics]: timers/context-timer-metrics
   [context propagation micrometer]: timers/context-timer-micrometer
   [micrometer]: https://micrometer.io

Original file line number	Diff line number	Diff line change
`@@ -44,7 +44,6 @@ public interface ContextManager<T> {`
`44`	`44`	`* The value of the currently active context, or {@code null} if no context is active.`
`45`	`45`	`*`
`46`	`46`	`* @return The value of the active context, or {@code null} if no context is active.`
`47`		`- * @see Context#getValue()`
`48`	`47`	`* @since 2.0.0`
`49`	`48`	`*/`
`50`	`49`	`T getActiveContextValue();`
Original file line number	Diff line number	Diff line change
`@@ -41,10 +41,6 @@ public boolean isClosed() {`
`41`	`41`	`return closed.get();`
`42`	`42`	`}`
`43`	`43`
`44`		`- public String getValue() {`
`45`		`- return value;`
`46`		`- }`
`47`		`-`
`48`	`44`	`public void close() {`
`49`	`45`	`if (closed.compareAndSet(false, true) && INSTANCE.get() == this) {`
`50`	`46`	`DummyContext current = INSTANCE.get();`
`@@ -60,8 +56,8 @@ public void close() {`
`60`	`56`	`}`
`61`	`57`
`62`	`58`	`public static String currentValue() {`
`63`		`- final Context<String> currentContext = INSTANCE.get();`
`64`		`- return currentContext != null ? currentContext.getValue() : null;`
	`59`	`+ final DummyContext currentContext = INSTANCE.get();`
	`60`	`+ return currentContext != null ? currentContext.value : null;`
`65`	`61`	`}`
`66`	`62`
`67`	`63`	`public static void setCurrentValue(String value) {`
Original file line number	Diff line number	Diff line change
`@@ -15,7 +15,6 @@`
`15`	`15`	`*/`
`16`	`16`	`package nl.talsmasoftware.context.dummy;`
`17`	`17`
`18`		`-import nl.talsmasoftware.context.api.Context;`
`19`	`18`	`import nl.talsmasoftware.context.core.threadlocal.AbstractThreadLocalContext;`
`20`	`19`
`21`	`20`	`/**`
`@@ -35,8 +34,8 @@ public boolean isClosed() {`
`35`	`34`	`}`
`36`	`35`
`37`	`36`	`public static String currentValue() {`
`38`		`- final Context<String> currentContext = INSTANCE.get();`
`39`		`- return currentContext != null ? currentContext.getValue() : null;`
	`37`	`+ final DummyContext currentContext = INSTANCE.get();`
	`38`	`+ return currentContext != null ? currentContext.value : null;`
`40`	`39`	`}`
`41`	`40`
`42`	`41`	`public static void setCurrentValue(String value) {`
Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ private Ctx(String newValue) {`
`77`	`77`
`78`	`78`	`private static String currentValue() {`
`79`	`79`	`Ctx current = AbstractThreadLocalContext.current(Ctx.class);`
`80`		`- return current != null ? current.getValue() : null;`
	`80`	`+ return current != null ? current.value : null;`
`81`	`81`	`}`
`82`	`82`
`83`	`83`	`private static void remove() {`