diff --git a/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java b/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java index 75084238c9..ef1015c4fa 100644 --- a/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java +++ b/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java @@ -132,8 +132,7 @@ public class TransportBenchmark { int port = pickUnusedPort(); InetSocketAddress address = new InetSocketAddress("localhost", port); serverBuilder = NettyServerBuilder.forAddress(address); - channelBuilder = OkHttpChannelBuilder.forAddress("localhost", port) - .negotiationType(io.grpc.okhttp.NegotiationType.PLAINTEXT); + channelBuilder = OkHttpChannelBuilder.forAddress("localhost", port).usePlaintext(); break; } default: diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/ReconnectTestClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/ReconnectTestClient.java index 8cee911f86..a89e8788ab 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/ReconnectTestClient.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/ReconnectTestClient.java @@ -70,8 +70,10 @@ public class ReconnectTestClient { .negotiationType(NegotiationType.PLAINTEXT).build(); controlStub = ReconnectServiceGrpc.newBlockingStub(controlChannel); if (useOkhttp) { - retryChannel = OkHttpChannelBuilder.forAddress("127.0.0.1", serverRetryPort) - .negotiationType(io.grpc.okhttp.NegotiationType.TLS).build(); + retryChannel = + OkHttpChannelBuilder.forAddress("127.0.0.1", serverRetryPort) + .useTransportSecurity() + .build(); } else { retryChannel = NettyChannelBuilder.forAddress("127.0.0.1", serverRetryPort) .negotiationType(NegotiationType.TLS).build(); diff --git a/okhttp/src/main/java/io/grpc/okhttp/NegotiationType.java b/okhttp/src/main/java/io/grpc/okhttp/NegotiationType.java index 57f1b59a3b..ebd8fd93d9 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/NegotiationType.java +++ b/okhttp/src/main/java/io/grpc/okhttp/NegotiationType.java @@ -18,7 +18,12 @@ package io.grpc.okhttp; /** * Identifies the negotiation used for starting up HTTP/2. + * + * @deprecated use {@link OkHttpChannelBuilder#usePlaintext()} or {@link + * OkHttpChannelBuilder#useTransportSecurity()} directly rather than {@link + * OkHttpChannelBuilder#negotiationType(NegotiationType)}. */ +@Deprecated public enum NegotiationType { /** * Uses TLS ALPN/NPN negotiation, assumes an SSL connection. diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index 005504b1c8..542e5bd65b 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java @@ -62,6 +62,18 @@ import javax.net.ssl.TrustManagerFactory; public class OkHttpChannelBuilder extends AbstractManagedChannelImplBuilder { + /** Identifies the negotiation used for starting up HTTP/2. */ + private enum NegotiationType { + /** Uses TLS ALPN/NPN negotiation, assumes an SSL connection. */ + TLS, + + /** + * Just assume the connection is plaintext (non-SSL) and the remote endpoint supports HTTP/2 + * directly without an upgrade. + */ + PLAINTEXT + } + /** * ConnectionSpec closely matching the default configuration that could be used as a basis for * modification. @@ -183,9 +195,22 @@ public class OkHttpChannelBuilder extends * {@link #sslSocketFactory} to override the socket factory used. * *

Default: TLS + * + * @deprecated use {@link #usePlaintext()} or {@link #useTransportSecurity()} instead. */ - public final OkHttpChannelBuilder negotiationType(NegotiationType type) { - negotiationType = Preconditions.checkNotNull(type, "type"); + @Deprecated + public final OkHttpChannelBuilder negotiationType(io.grpc.okhttp.NegotiationType type) { + Preconditions.checkNotNull(type, "type"); + switch (type) { + case TLS: + negotiationType = NegotiationType.TLS; + break; + case PLAINTEXT: + negotiationType = NegotiationType.PLAINTEXT; + break; + default: + throw new AssertionError("Unknown negotiation type: " + type); + } return this; } @@ -262,16 +287,11 @@ public class OkHttpChannelBuilder extends } /** - * Override the default {@link SSLSocketFactory} and enable {@link NegotiationType#TLS} - * negotiation. - * - *

By default, when TLS is enabled, SSLSocketFactory.getDefault() will be used. - * - *

{@link NegotiationType#TLS} will be applied by calling this method. + * Override the default {@link SSLSocketFactory} and enable TLS negotiation. */ public final OkHttpChannelBuilder sslSocketFactory(SSLSocketFactory factory) { this.sslSocketFactory = factory; - negotiationType(NegotiationType.TLS); + negotiationType = NegotiationType.TLS; return this; } @@ -320,7 +340,7 @@ public class OkHttpChannelBuilder extends } /** - * Equivalent to using {@link #negotiationType(NegotiationType)} with {@code PLAINTEXT}. + * Equivalent to using {@link #negotiationType} with {@code PLAINTEXT}. * * @deprecated use {@link #usePlaintext()} instead. */ @@ -328,28 +348,31 @@ public class OkHttpChannelBuilder extends @Deprecated public final OkHttpChannelBuilder usePlaintext(boolean skipNegotiation) { if (skipNegotiation) { - negotiationType(NegotiationType.PLAINTEXT); + negotiationType(io.grpc.okhttp.NegotiationType.PLAINTEXT); } else { throw new IllegalArgumentException("Plaintext negotiation not currently supported"); } return this; } - /** - * Equivalent to using {@link #negotiationType(NegotiationType)} with {@code PLAINTEXT}. - */ + /** Sets the negotiation type for the HTTP/2 connection to plaintext. */ @Override public final OkHttpChannelBuilder usePlaintext() { - negotiationType(NegotiationType.PLAINTEXT); + negotiationType = NegotiationType.PLAINTEXT; return this; } /** - * Equivalent to using {@link #negotiationType(NegotiationType)} with {@code TLS}. + * Sets the negotiation type for the HTTP/2 connection to TLS (this is the default). + * + *

With TLS enabled, a default {@link SSLSocketFactory} is created using the best {@link + * java.security.Provider} available and is NOT based on {@link SSLSocketFactory#getDefault}. To + * more precisely control the TLS configuration call {@link #sslSocketFactory} to override the + * socket factory used. */ @Override public final OkHttpChannelBuilder useTransportSecurity() { - negotiationType(NegotiationType.TLS); + negotiationType = NegotiationType.TLS; return this; } diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java index 7b02b9f0c1..98d171162c 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java @@ -77,9 +77,7 @@ public class OkHttpChannelBuilderTest { } }; - builder.overrideAuthority("[invalidauthority") - .negotiationType(NegotiationType.PLAINTEXT) - .buildTransportFactory(); + builder.overrideAuthority("[invalidauthority").usePlaintext().buildTransportFactory(); } @Test diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java index 5b5fe91e99..2c3360718f 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpTransportTest.java @@ -37,12 +37,13 @@ import org.junit.runners.JUnit4; @RunWith(JUnit4.class) public class OkHttpTransportTest extends AbstractTransportTest { private final FakeClock fakeClock = new FakeClock(); - private ClientTransportFactory clientFactory = OkHttpChannelBuilder - // Although specified here, address is ignored because we never call build. - .forAddress("localhost", 0) - .negotiationType(NegotiationType.PLAINTEXT) - .setTransportTracerFactory(fakeClockTransportTracer) - .buildTransportFactory(); + private ClientTransportFactory clientFactory = + OkHttpChannelBuilder + // Although specified here, address is ignored because we never call build. + .forAddress("localhost", 0) + .usePlaintext() + .setTransportTracerFactory(fakeClockTransportTracer) + .buildTransportFactory(); @After public void releaseClientFactory() {