okhttp: OkHttpChannelBuilder extends a public API class

interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java

@@ -23,13 +23,13 @@ import io.grpc.ManagedChannelBuilder;
 import io.grpc.alts.AltsChannelBuilder;
 import io.grpc.alts.ComputeEngineChannelBuilder;
 import io.grpc.alts.GoogleDefaultChannelBuilder;
-import io.grpc.internal.AbstractManagedChannelImplBuilder;
 import io.grpc.internal.GrpcUtil;
 import io.grpc.internal.testing.TestUtils;
 import io.grpc.netty.GrpcSslContexts;
 import io.grpc.netty.InternalNettyChannelBuilder;
 import io.grpc.netty.NegotiationType;
 import io.grpc.netty.NettyChannelBuilder;
+import io.grpc.okhttp.InternalOkHttpChannelBuilder;
 import io.grpc.okhttp.OkHttpChannelBuilder;
 import io.grpc.okhttp.internal.Platform;
 import io.netty.handler.ssl.SslContext;
@@ -403,7 +403,7 @@ public class TestServiceClient {
       if (useAlts) {
         return AltsChannelBuilder.forAddress(serverHost, serverPort);
       }
-      AbstractManagedChannelImplBuilder<?> builder;
+
       if (!useOkHttp) {
         SslContext sslContext = null;
         if (useTestCa) {
@@ -429,34 +429,33 @@ public class TestServiceClient {
         // Disable the default census stats interceptor, use testing interceptor instead.
         InternalNettyChannelBuilder.setStatsEnabled(nettyBuilder, false);
         return nettyBuilder.intercept(createCensusStatsClientInterceptor());
-      } else {
+      }
-        OkHttpChannelBuilder okBuilder = OkHttpChannelBuilder.forAddress(serverHost, serverPort);
+
-        if (serverHostOverride != null) {
+      OkHttpChannelBuilder okBuilder = OkHttpChannelBuilder.forAddress(serverHost, serverPort);
-          // Force the hostname to match the cert the server uses.
+      if (serverHostOverride != null) {
-          okBuilder.overrideAuthority(
+        // Force the hostname to match the cert the server uses.
-              GrpcUtil.authorityFromHostAndPort(serverHostOverride, serverPort));
+        okBuilder.overrideAuthority(
-        }
+            GrpcUtil.authorityFromHostAndPort(serverHostOverride, serverPort));
-        if (useTls) {
+      }
-          if (useTestCa) {
+      if (useTls) {
-            try {
+        if (useTestCa) {
-              SSLSocketFactory factory = TestUtils.newSslSocketFactoryForCa(
+          try {
-                  Platform.get().getProvider(), TestUtils.loadCert("ca.pem"));
+            SSLSocketFactory factory = TestUtils.newSslSocketFactoryForCa(
-              okBuilder.sslSocketFactory(factory);
+                Platform.get().getProvider(), TestUtils.loadCert("ca.pem"));
-            } catch (Exception e) {
+            okBuilder.sslSocketFactory(factory);
-              throw new RuntimeException(e);
+          } catch (Exception e) {
-            }
+            throw new RuntimeException(e);
           }
-        } else {
-          okBuilder.usePlaintext();
         }
-        if (fullStreamDecompression) {
+      } else {
-          okBuilder.enableFullStreamDecompression();
+        okBuilder.usePlaintext();
-        }
+      }
-        builder = okBuilder;
+      if (fullStreamDecompression) {
+        okBuilder.enableFullStreamDecompression();
       }
       // Disable the default census stats interceptor, use testing interceptor instead.
-      io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false);
+      InternalOkHttpChannelBuilder.setStatsEnabled(okBuilder, false);
-      return builder.intercept(createCensusStatsClientInterceptor());
+      return okBuilder.intercept(createCensusStatsClientInterceptor());
     }
 
     @Override

interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java

@@ -29,6 +29,7 @@ import io.grpc.internal.testing.StreamRecorder;
 import io.grpc.internal.testing.TestUtils;
 import io.grpc.netty.GrpcSslContexts;
 import io.grpc.netty.NettyServerBuilder;
+import io.grpc.okhttp.InternalOkHttpChannelBuilder;
 import io.grpc.okhttp.OkHttpChannelBuilder;
 import io.grpc.okhttp.internal.Platform;
 import io.grpc.stub.StreamObserver;
@@ -102,7 +103,7 @@ public class Http2OkHttpTest extends AbstractInteropTest {
       throw new RuntimeException(e);
     }
     // Disable the default census stats interceptor, use testing interceptor instead.
-    io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false);
+    InternalOkHttpChannelBuilder.setStatsEnabled(builder, false);
     return builder.intercept(createCensusStatsClientInterceptor());
   }
 

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

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2020 The gRPC Authors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.grpc.okhttp;
+
+import io.grpc.Internal;
+
+/**
+ * Internal {@link OkHttpChannelBuilder} accessor.  This is intended for usage internal to the gRPC
+ * team.  If you *really* think you need to use this, contact the gRPC team first.
+ */
+@Internal
+public final class InternalOkHttpChannelBuilder {
+
+  public static void setStatsEnabled(OkHttpChannelBuilder builder, boolean value) {
+    builder.setStatsEnabled(value);
+  }
+
+  private InternalOkHttpChannelBuilder() {}
+}

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

@@ -24,13 +24,17 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import io.grpc.ChannelLogger;
 import io.grpc.ExperimentalApi;
+import io.grpc.ForwardingChannelBuilder;
 import io.grpc.Internal;
-import io.grpc.internal.AbstractManagedChannelImplBuilder;
+import io.grpc.ManagedChannelBuilder;
 import io.grpc.internal.AtomicBackoff;
 import io.grpc.internal.ClientTransportFactory;
 import io.grpc.internal.ConnectionClientTransport;
 import io.grpc.internal.GrpcUtil;
 import io.grpc.internal.KeepAliveManager;
+import io.grpc.internal.ManagedChannelImplBuilder;
+import io.grpc.internal.ManagedChannelImplBuilder.ChannelBuilderDefaultPortProvider;
+import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder;
 import io.grpc.internal.SharedResourceHolder;
 import io.grpc.internal.SharedResourceHolder.Resource;
 import io.grpc.internal.TransportTracer;
@@ -54,10 +58,12 @@ import javax.net.ssl.SSLSocketFactory;
 
 /** Convenience class for building channels with the OkHttp transport. */
 @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1785")
-public class OkHttpChannelBuilder extends
+public class OkHttpChannelBuilder extends ForwardingChannelBuilder<OkHttpChannelBuilder> {
-        AbstractManagedChannelImplBuilder<OkHttpChannelBuilder> {
 
   public static final int DEFAULT_FLOW_CONTROL_WINDOW = 65535;
+  private final ManagedChannelImplBuilder managedChannelImplBuilder;
+  private TransportTracer.Factory transportTracerFactory = TransportTracer.getDefaultFactory();
+
 
   /** Identifies the negotiation used for starting up HTTP/2. */
   private enum NegotiationType {
@@ -127,6 +133,7 @@ public class OkHttpChannelBuilder extends
   private long keepAliveTimeoutNanos = DEFAULT_KEEPALIVE_TIMEOUT_NANOS;
   private int flowControlWindow = DEFAULT_FLOW_CONTROL_WINDOW;
   private boolean keepAliveWithoutCalls;
+  private int maxInboundMessageSize = GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE;
   private int maxInboundMetadataSize = Integer.MAX_VALUE;
 
   /**
@@ -140,7 +147,29 @@ public class OkHttpChannelBuilder extends
   }
 
   private OkHttpChannelBuilder(String target) {
-    super(target);
+    final class OkHttpChannelTransportFactoryBuilder implements ClientTransportFactoryBuilder {
+      @Override
+      public ClientTransportFactory buildClientTransportFactory() {
+        return buildTransportFactory();
+      }
+    }
+
+    final class OkHttpChannelDefaultPortProvider implements ChannelBuilderDefaultPortProvider {
+      @Override
+      public int getDefaultPort() {
+        return OkHttpChannelBuilder.this.getDefaultPort();
+      }
+    }
+
+    managedChannelImplBuilder = new ManagedChannelImplBuilder(target,
+        new OkHttpChannelTransportFactoryBuilder(),
+        new OkHttpChannelDefaultPortProvider());
+  }
+
+  @Internal
+  @Override
+  protected final ManagedChannelBuilder<?> delegate() {
+    return managedChannelImplBuilder;
   }
 
   @VisibleForTesting
@@ -363,9 +392,19 @@ public class OkHttpChannelBuilder extends
     return this;
   }
 
+  /**
+   * Sets the maximum message size allowed for a single gRPC frame. If an inbound messages
+   * larger than this limit is received it will not be processed and the RPC will fail with
+   * RESOURCE_EXHAUSTED.
+   */
   @Override
-  @Internal
+  public final OkHttpChannelBuilder maxInboundMessageSize(int max) {
-  protected final ClientTransportFactory buildTransportFactory() {
+    Preconditions.checkArgument(max >= 0, "negative max");
+    maxInboundMessageSize = max;
+    return this;
+  }
+
+  final ClientTransportFactory buildTransportFactory() {
     boolean enableKeepAlive = keepAliveTimeNanos != KEEPALIVE_TIME_NANOS_DISABLED;
     return new OkHttpTransportFactory(
         transportExecutor,
@@ -374,7 +413,7 @@ public class OkHttpChannelBuilder extends
         createSslSocketFactory(),
         hostnameVerifier,
         connectionSpec,
-        maxInboundMessageSize(),
+        maxInboundMessageSize,
         enableKeepAlive,
         keepAliveTimeNanos,
         keepAliveTimeoutNanos,
@@ -385,8 +424,17 @@ public class OkHttpChannelBuilder extends
         useGetForSafeMethods);
   }
 
-  @Override
+  final OkHttpChannelBuilder disableCheckAuthority() {
-  protected int getDefaultPort() {
+    this.managedChannelImplBuilder.disableCheckAuthority();
+    return this;
+  }
+
+  final OkHttpChannelBuilder enableCheckAuthority() {
+    this.managedChannelImplBuilder.enableCheckAuthority();
+    return this;
+  }
+
+  final int getDefaultPort() {
     switch (negotiationType) {
       case PLAINTEXT:
         return GrpcUtil.DEFAULT_PORT_PLAINTEXT;
@@ -397,6 +445,10 @@ public class OkHttpChannelBuilder extends
     }
   }
 
+  final void setStatsEnabled(boolean value) {
+    this.managedChannelImplBuilder.setStatsEnabled(value);
+  }
+
   @VisibleForTesting
   @Nullable
   SSLSocketFactory createSslSocketFactory() {

okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java

@@ -72,20 +72,26 @@ public class OkHttpChannelBuilderTest {
   }
 
   @Test
-  public void overrideAllowsInvalidAuthority() {
+  public void failOverrideInvalidAuthority() {
-    OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234) {
+    OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234);
-      @Override
-      protected String checkAuthority(String authority) {
-        return authority;
-      }
-    };
 
+    thrown.expect(IllegalArgumentException.class);
+    thrown.expectMessage("Invalid authority:");
+    builder.overrideAuthority("[invalidauthority");
+  }
+
+  @Test
+  public void disableCheckAuthorityAllowsInvalidAuthority() {
+    OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234)
+        .disableCheckAuthority();
     builder.overrideAuthority("[invalidauthority").usePlaintext().buildTransportFactory();
   }
 
   @Test
-  public void failOverrideInvalidAuthority() {
+  public void enableCheckAuthorityFailOverrideInvalidAuthority() {
-    OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234);
+    OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234)
+        .disableCheckAuthority()
+        .enableCheckAuthority();
 
     thrown.expect(IllegalArgumentException.class);
     thrown.expectMessage("Invalid authority:");