diff options
4 files changed, 26 insertions, 11 deletions
diff --git a/context/src/main/java/io/grpc/Context.java b/context/src/main/java/io/grpc/Context.java index 795eb4ec3..0e76c08e1 100644 --- a/context/src/main/java/io/grpc/Context.java +++ b/context/src/main/java/io/grpc/Context.java @@ -16,6 +16,7 @@ package io.grpc; +import io.grpc.Context.CheckReturnValue; import java.io.Closeable; import java.util.ArrayList; import java.util.concurrent.Callable; @@ -93,6 +94,7 @@ import java.util.logging.Logger; * </ul> */ /* @DoNotMock("Use ROOT for a non-null Context") // commented out to avoid dependencies */ +@CheckReturnValue public class Context { private static final Logger log = Logger.getLogger(Context.class.getName()); @@ -571,6 +573,7 @@ public class Context { * @param c {@link Callable} to call. * @return result of call. */ + @CanIgnoreReturnValue public <V> V call(Callable<V> c) throws Exception { Context previous = attach(); try { @@ -776,6 +779,7 @@ public class Context { * @return {@code true} if this context cancelled the context and notified listeners, * {@code false} if the context was already cancelled. */ + @CanIgnoreReturnValue public boolean cancel(Throwable cause) { boolean triggeredCancel = false; synchronized (this) { @@ -1008,6 +1012,7 @@ public class Context { } } + @CanIgnoreReturnValue private static <T> T checkNotNull(T reference, Object errorMessage) { if (reference == null) { throw new NullPointerException(String.valueOf(errorMessage)); @@ -1059,4 +1064,10 @@ public class Context { new Exception()); } } + + // Not using the standard com.google.errorprone.annotations.CheckReturnValue because that will + // introduce dependencies that some io.grpc.Context API consumers may not want. + @interface CheckReturnValue {} + + @interface CanIgnoreReturnValue {} } diff --git a/context/src/test/java/io/grpc/ContextTest.java b/context/src/test/java/io/grpc/ContextTest.java index ab0130bf7..c0b8f9697 100644 --- a/context/src/test/java/io/grpc/ContextTest.java +++ b/context/src/test/java/io/grpc/ContextTest.java @@ -60,6 +60,7 @@ import org.junit.runners.JUnit4; * Tests for {@link Context}. */ @RunWith(JUnit4.class) +@SuppressWarnings("CheckReturnValue") // false-positive in test for current ver errorprone plugin public class ContextTest { private static final Context.Key<String> PET = Context.key("pet"); diff --git a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java index aab28d626..c8f7254c1 100644 --- a/core/src/test/java/io/grpc/internal/ClientCallImplTest.java +++ b/core/src/test/java/io/grpc/internal/ClientCallImplTest.java @@ -137,7 +137,6 @@ public class ClientCallImplTest { @After public void tearDown() { - Context.ROOT.attach(); verifyZeroInteractions(streamTracerFactory); } @@ -500,7 +499,8 @@ public class ClientCallImplTest { public void callerContextPropagatedToListener() throws Exception { // Attach the context which is recorded when the call is created final Context.Key<String> testKey = Context.key("testing"); - Context.current().withValue(testKey, "testValue").attach(); + Context context = Context.current().withValue(testKey, "testValue"); + Context previous = context.attach(); ClientCallImpl<Void, Void> call = new ClientCallImpl<Void, Void>( method, @@ -512,10 +512,11 @@ public class ClientCallImplTest { false /* retryEnabled */) .setDecompressorRegistry(decompressorRegistry); - Context.ROOT.attach(); + context.detach(previous); // Override the value after creating the call, this should not be seen by callbacks - Context.current().withValue(testKey, "badValue").attach(); + context = Context.current().withValue(testKey, "badValue"); + previous = context.attach(); final AtomicBoolean onHeadersCalled = new AtomicBoolean(); final AtomicBoolean onMessageCalled = new AtomicBoolean(); @@ -555,6 +556,8 @@ public class ClientCallImplTest { } }, new Metadata()); + context.detach(previous); + verify(stream).start(listenerArgumentCaptor.capture()); ClientStreamListener listener = listenerArgumentCaptor.getValue(); listener.onReady(); @@ -587,7 +590,7 @@ public class ClientCallImplTest { false /* retryEnabled */) .setDecompressorRegistry(decompressorRegistry); - previous.attach(); + cancellableContext.detach(previous); call.start(callListener, new Metadata()); @@ -617,7 +620,7 @@ public class ClientCallImplTest { false /* retryEnabled */) .setDecompressorRegistry(decompressorRegistry); - previous.attach(); + cancellableContext.detach(previous); final SettableFuture<Status> statusFuture = SettableFuture.create(); call.start(new ClientCall.Listener<Void>() { @@ -803,9 +806,9 @@ public class ClientCallImplTest { public void expiredDeadlineCancelsStream_Context() { fakeClock.forwardTime(System.nanoTime(), TimeUnit.NANOSECONDS); - Context.current() - .withDeadlineAfter(1, TimeUnit.SECONDS, deadlineCancellationExecutor) - .attach(); + Context context = Context.current() + .withDeadlineAfter(1, TimeUnit.SECONDS, deadlineCancellationExecutor); + Context origContext = context.attach(); ClientCallImpl<Void, Void> call = new ClientCallImpl<Void, Void>( method, @@ -816,6 +819,8 @@ public class ClientCallImplTest { channelCallTracer, false /* retryEnabled */); + context.detach(origContext); + call.start(callListener, new Metadata()); fakeClock.forwardNanos(TimeUnit.SECONDS.toNanos(1) + 1); diff --git a/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java b/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java index a771cad9a..83687e33f 100644 --- a/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java @@ -39,7 +39,6 @@ import com.google.common.io.ByteStreams; import com.google.common.util.concurrent.SettableFuture; import io.grpc.Attributes; import io.grpc.CallOptions; -import io.grpc.Context; import io.grpc.Grpc; import io.grpc.Metadata; import io.grpc.MethodDescriptor; @@ -127,7 +126,6 @@ public class NettyClientTransportTest { @After public void teardown() throws Exception { - Context.ROOT.attach(); for (NettyClientTransport transport : transports) { transport.shutdown(Status.UNAVAILABLE); } |