okhttp: deprecate OkHttpChannelBuilder#negotiationType (#4533)

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:

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();

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.

okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java

@@ -62,6 +62,18 @@ import javax.net.ssl.TrustManagerFactory;
 public class OkHttpChannelBuilder extends
         AbstractManagedChannelImplBuilder<OkHttpChannelBuilder> {
 
+  /** 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.
    *
    * <p>Default: <code>TLS</code>
+   *
+   * @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.
-   *
-   * <p>By default, when TLS is enabled, <code>SSLSocketFactory.getDefault()</code> will be used.
-   *
-   * <p>{@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).
+   *
+   * <p>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;
   }
 

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

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() {