From 04a90bcad2044b78390da2d29c9166361bf84b59 Mon Sep 17 00:00:00 2001 From: zpencer Date: Tue, 15 May 2018 10:03:46 -0700 Subject: [PATCH] core: stabilize custom CallOptions API (#4457) Deprecate `of()` in favor of `create()` and `createWithDefault()`. Emphasize that the name string is only for debug purposes. Fixes #1869 --- .../java/io/grpc/CallOptionsBenchmark.java | 2 +- .../main/java/io/grpc/BinaryLogProvider.java | 2 +- core/src/main/java/io/grpc/CallOptions.java | 49 +++++++++++++++---- .../internal/ServiceConfigInterceptor.java | 2 +- .../test/java/io/grpc/CallOptionsTest.java | 4 +- .../java/io/grpc/ClientInterceptorsTest.java | 2 +- .../io/grpc/internal/CensusModulesTest.java | 2 +- .../internal/DelayedClientTransportTest.java | 3 +- 8 files changed, 48 insertions(+), 18 deletions(-) diff --git a/core/src/jmh/java/io/grpc/CallOptionsBenchmark.java b/core/src/jmh/java/io/grpc/CallOptionsBenchmark.java index 67026291f8..cc426e50f5 100644 --- a/core/src/jmh/java/io/grpc/CallOptionsBenchmark.java +++ b/core/src/jmh/java/io/grpc/CallOptionsBenchmark.java @@ -51,7 +51,7 @@ public class CallOptionsBenchmark { public void setUp() throws Exception { customOptions = new ArrayList>(customOptionsCount); for (int i = 0; i < customOptionsCount; i++) { - customOptions.add(CallOptions.Key.of("name " + i, "defaultvalue")); + customOptions.add(CallOptions.Key.createWithDefault("name " + i, "defaultvalue")); } allOpts = CallOptions.DEFAULT; diff --git a/core/src/main/java/io/grpc/BinaryLogProvider.java b/core/src/main/java/io/grpc/BinaryLogProvider.java index 61d9abc5dd..31fd14f9bd 100644 --- a/core/src/main/java/io/grpc/BinaryLogProvider.java +++ b/core/src/main/java/io/grpc/BinaryLogProvider.java @@ -34,7 +34,7 @@ import javax.annotation.Nullable; @Internal public abstract class BinaryLogProvider implements Closeable { public static final CallOptions.Key CLIENT_CALL_ID_CALLOPTION_KEY - = CallOptions.Key.of("binarylog-calloptions-key", null); + = CallOptions.Key.create("binarylog-calloptions-key"); @VisibleForTesting public static final Marshaller BYTEARRAY_MARSHALLER = new ByteArrayMarshaller(); diff --git a/core/src/main/java/io/grpc/CallOptions.java b/core/src/main/java/io/grpc/CallOptions.java index eca402e134..d94787f2cb 100644 --- a/core/src/main/java/io/grpc/CallOptions.java +++ b/core/src/main/java/io/grpc/CallOptions.java @@ -231,13 +231,12 @@ public final class CallOptions { /** * Key for a key-value pair. Uses reference equality. */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1869") public static final class Key { - private final String name; + private final String debugString; private final T defaultValue; - private Key(String name, T defaultValue) { - this.name = name; + private Key(String debugString, T defaultValue) { + this.debugString = debugString; this.defaultValue = defaultValue; } @@ -250,20 +249,50 @@ public final class CallOptions { @Override public String toString() { - return name; + return debugString; } /** * Factory method for creating instances of {@link Key}. * - * @param name the name of Key. + * @param debugString a string used to describe this key, used for debugging. * @param defaultValue default value to return when value for key not set * @param Key type * @return Key object + * @deprecated Use {@link #create} or {@link #createWithDefault} instead. */ - public static Key of(String name, T defaultValue) { - Preconditions.checkNotNull(name, "name"); - return new Key(name, defaultValue); + @Deprecated + public static Key of(String debugString, T defaultValue) { + Preconditions.checkNotNull(debugString, "debugString"); + return new Key(debugString, defaultValue); + } + + /** + * Factory method for creating instances of {@link Key}. The default value of the + * key is {@code null}. + * + * @param debugString a debug string that describes this key. + * @param Key type + * @return Key object + * @since 1.13.0 + */ + public static Key create(String debugString) { + Preconditions.checkNotNull(debugString, "debugString"); + return new Key(debugString, /*defaultValue=*/ null); + } + + /** + * Factory method for creating instances of {@link Key}. + * + * @param debugString a debug string that describes this key. + * @param defaultValue default value to return when value for key not set + * @param Key type + * @return Key object + * @since 1.13.0 + */ + public static Key createWithDefault(String debugString, T defaultValue) { + Preconditions.checkNotNull(debugString, "debugString"); + return new Key(debugString, defaultValue); } } @@ -272,8 +301,8 @@ public final class CallOptions { * * @param key The option key * @param value The option value. + * @since 1.13.0 */ - @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1869") public CallOptions withOption(Key key, T value) { Preconditions.checkNotNull(key, "key"); Preconditions.checkNotNull(value, "value"); diff --git a/core/src/main/java/io/grpc/internal/ServiceConfigInterceptor.java b/core/src/main/java/io/grpc/internal/ServiceConfigInterceptor.java index 3053610404..49a6e1c9f0 100644 --- a/core/src/main/java/io/grpc/internal/ServiceConfigInterceptor.java +++ b/core/src/main/java/io/grpc/internal/ServiceConfigInterceptor.java @@ -237,7 +237,7 @@ final class ServiceConfigInterceptor implements ClientInterceptor { } static final CallOptions.Key RETRY_POLICY_KEY = - CallOptions.Key.of("internal-retry-policy", null); + CallOptions.Key.create("internal-retry-policy"); @Override public ClientCall interceptCall( diff --git a/core/src/test/java/io/grpc/CallOptionsTest.java b/core/src/test/java/io/grpc/CallOptionsTest.java index 46e53a1f92..352bb3168e 100644 --- a/core/src/test/java/io/grpc/CallOptionsTest.java +++ b/core/src/test/java/io/grpc/CallOptionsTest.java @@ -43,8 +43,8 @@ public class CallOptionsTest { private Deadline.Ticker ticker = new FakeTicker(); private Deadline sampleDeadline = Deadline.after(1, NANOSECONDS, ticker); private CallCredentials sampleCreds = mock(CallCredentials.class); - private CallOptions.Key option1 = CallOptions.Key.of("option1", "default"); - private CallOptions.Key option2 = CallOptions.Key.of("option2", "default"); + private CallOptions.Key option1 = CallOptions.Key.createWithDefault("option1", "default"); + private CallOptions.Key option2 = CallOptions.Key.createWithDefault("option2", "default"); private ClientStreamTracer.Factory tracerFactory1 = new FakeTracerFactory("tracerFactory1"); private ClientStreamTracer.Factory tracerFactory2 = new FakeTracerFactory("tracerFactory2"); private CallOptions allSet = CallOptions.DEFAULT diff --git a/core/src/test/java/io/grpc/ClientInterceptorsTest.java b/core/src/test/java/io/grpc/ClientInterceptorsTest.java index 22b48baeef..755b3a0b5d 100644 --- a/core/src/test/java/io/grpc/ClientInterceptorsTest.java +++ b/core/src/test/java/io/grpc/ClientInterceptorsTest.java @@ -395,7 +395,7 @@ public class ClientInterceptorsTest { @Test public void customOptionAccessible() { - CallOptions.Key customOption = CallOptions.Key.of("custom", null); + CallOptions.Key customOption = CallOptions.Key.create("custom"); CallOptions callOptions = CallOptions.DEFAULT.withOption(customOption, "value"); ArgumentCaptor passedOptions = ArgumentCaptor.forClass(CallOptions.class); ClientInterceptor interceptor = diff --git a/core/src/test/java/io/grpc/internal/CensusModulesTest.java b/core/src/test/java/io/grpc/internal/CensusModulesTest.java index 2e49922dfc..3fc017530a 100644 --- a/core/src/test/java/io/grpc/internal/CensusModulesTest.java +++ b/core/src/test/java/io/grpc/internal/CensusModulesTest.java @@ -104,7 +104,7 @@ import org.mockito.MockitoAnnotations; @RunWith(JUnit4.class) public class CensusModulesTest { private static final CallOptions.Key CUSTOM_OPTION = - CallOptions.Key.of("option1", "default"); + CallOptions.Key.createWithDefault("option1", "default"); private static final CallOptions CALL_OPTIONS = CallOptions.DEFAULT.withOption(CUSTOM_OPTION, "customvalue"); diff --git a/core/src/test/java/io/grpc/internal/DelayedClientTransportTest.java b/core/src/test/java/io/grpc/internal/DelayedClientTransportTest.java index ebf04a59a5..fe2e14463e 100644 --- a/core/src/test/java/io/grpc/internal/DelayedClientTransportTest.java +++ b/core/src/test/java/io/grpc/internal/DelayedClientTransportTest.java @@ -78,7 +78,8 @@ public class DelayedClientTransportTest { @Captor private ArgumentCaptor statusCaptor; @Captor private ArgumentCaptor listenerCaptor; - private static final CallOptions.Key SHARD_ID = CallOptions.Key.of("shard-id", -1); + private static final CallOptions.Key SHARD_ID + = CallOptions.Key.createWithDefault("shard-id", -1); private static final Status SHUTDOWN_STATUS = Status.UNAVAILABLE.withDescription("shutdown called");