diff --git a/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java b/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java index 9f01b5e9143..ec4646bff44 100644 --- a/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java +++ b/benchmarks/src/jmh/java/io/grpc/benchmarks/TransportBenchmark.java @@ -20,6 +20,7 @@ import com.google.protobuf.ByteString; import io.grpc.ManagedChannel; +import io.grpc.ManagedChannelBuilder; import io.grpc.Server; import io.grpc.Status; import io.grpc.StatusRuntimeException; @@ -30,7 +31,6 @@ import io.grpc.benchmarks.qps.AsyncServer; import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; -import io.grpc.internal.AbstractManagedChannelImplBuilder; import io.grpc.internal.AbstractServerImplBuilder; import io.grpc.netty.NegotiationType; import io.grpc.netty.NettyChannelBuilder; @@ -81,7 +81,7 @@ public enum Transport { @Setup public void setUp() throws Exception { AbstractServerImplBuilder serverBuilder; - AbstractManagedChannelImplBuilder channelBuilder; + ManagedChannelBuilder channelBuilder; switch (transport) { case INPROCESS: { diff --git a/core/src/main/java/io/grpc/inprocess/InProcessChannelBuilder.java b/core/src/main/java/io/grpc/inprocess/InProcessChannelBuilder.java index 3546e33357c..926c7835729 100644 --- a/core/src/main/java/io/grpc/inprocess/InProcessChannelBuilder.java +++ b/core/src/main/java/io/grpc/inprocess/InProcessChannelBuilder.java @@ -21,11 +21,14 @@ 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.ClientTransportFactory; import io.grpc.internal.ConnectionClientTransport; import io.grpc.internal.GrpcUtil; +import io.grpc.internal.ManagedChannelImplBuilder; +import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder; import io.grpc.internal.SharedResourceHolder; import java.net.SocketAddress; import java.util.concurrent.ScheduledExecutorService; @@ -42,7 +45,7 @@ */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1783") public final class InProcessChannelBuilder extends - AbstractManagedChannelImplBuilder { + ForwardingChannelBuilder { /** * Create a channel builder that will connect to the server with the given name. * @@ -67,18 +70,35 @@ public static InProcessChannelBuilder forAddress(String name, int port) { throw new UnsupportedOperationException("call forName() instead"); } + private final ManagedChannelImplBuilder managedChannelImplBuilder; private final String name; private ScheduledExecutorService scheduledExecutorService; private int maxInboundMetadataSize = Integer.MAX_VALUE; private boolean transportIncludeStatusCause = false; private InProcessChannelBuilder(String name) { - super(new InProcessSocketAddress(name), "localhost"); this.name = checkNotNull(name, "name"); + + final class InProcessChannelTransportFactoryBuilder implements ClientTransportFactoryBuilder { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return buildTransportFactory(); + } + } + + managedChannelImplBuilder = new ManagedChannelImplBuilder(new InProcessSocketAddress(name), + "localhost", new InProcessChannelTransportFactoryBuilder(), null); + // In-process transport should not record its traffic to the stats module. // https://github.com/grpc/grpc-java/issues/2284 - setStatsRecordStartedRpcs(false); - setStatsRecordFinishedRpcs(false); + managedChannelImplBuilder.setStatsRecordStartedRpcs(false); + managedChannelImplBuilder.setStatsRecordFinishedRpcs(false); + } + + @Internal + @Override + protected ManagedChannelBuilder delegate() { + return managedChannelImplBuilder; } @Override @@ -177,13 +197,15 @@ public InProcessChannelBuilder propagateCauseWithStatus(boolean enable) { return this; } - @Override - @Internal - protected ClientTransportFactory buildTransportFactory() { + ClientTransportFactory buildTransportFactory() { return new InProcessClientTransportFactory( name, scheduledExecutorService, maxInboundMetadataSize, transportIncludeStatusCause); } + void setStatsEnabled(boolean value) { + this.managedChannelImplBuilder.setStatsEnabled(value); + } + /** * Creates InProcess transports. Exposed for internal use, as it should be private. */ diff --git a/core/src/main/java/io/grpc/inprocess/InternalInProcessChannelBuilder.java b/core/src/main/java/io/grpc/inprocess/InternalInProcessChannelBuilder.java new file mode 100644 index 00000000000..1a017fe564c --- /dev/null +++ b/core/src/main/java/io/grpc/inprocess/InternalInProcessChannelBuilder.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.inprocess; + +import io.grpc.Internal; + +/** + * Internal {@link InProcessChannelBuilder} 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 InternalInProcessChannelBuilder { + + public static void setStatsEnabled(InProcessChannelBuilder builder, boolean value) { + builder.setStatsEnabled(value); + } + + private InternalInProcessChannelBuilder() {} +} diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index b92fdf5410a..aac6c25a8ee 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -51,7 +51,7 @@ import javax.annotation.Nullable; /** - * The base class for channel builders. + * Abstract base class for channel builders. * * @param The concrete type of this builder. */ diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java new file mode 100644 index 00000000000..02af7ef048c --- /dev/null +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -0,0 +1,198 @@ +/* + * 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.internal; + +import com.google.common.base.Preconditions; +import io.grpc.ManagedChannelBuilder; +import java.net.SocketAddress; +import java.util.concurrent.Executor; +import javax.annotation.Nullable; + +/** + * Default managed channel builder, for usage in Transport implementations. + */ +public final class ManagedChannelImplBuilder + extends AbstractManagedChannelImplBuilder { + + private boolean authorityCheckerDisabled; + @Deprecated + @Nullable + private OverrideAuthorityChecker authorityChecker; + + /** + * An interface for Transport implementors to provide the {@link ClientTransportFactory} + * appropriate for the channel. + */ + public interface ClientTransportFactoryBuilder { + ClientTransportFactory buildClientTransportFactory(); + } + + /** + * An interface for Transport implementors to provide a default port to {@link + * io.grpc.NameResolver} for use in cases where the target string doesn't include a port. The + * default implementation returns {@link GrpcUtil#DEFAULT_PORT_SSL}. + */ + public interface ChannelBuilderDefaultPortProvider { + int getDefaultPort(); + } + + /** + * Default implementation of {@link ChannelBuilderDefaultPortProvider} that returns a fixed port. + */ + public static final class FixedPortProvider implements ChannelBuilderDefaultPortProvider { + private final int port; + + public FixedPortProvider(int port) { + this.port = port; + } + + @Override + public int getDefaultPort() { + return port; + } + } + + private final class ManagedChannelDefaultPortProvider implements + ChannelBuilderDefaultPortProvider { + @Override + public int getDefaultPort() { + return ManagedChannelImplBuilder.super.getDefaultPort(); + } + } + + private final ClientTransportFactoryBuilder clientTransportFactoryBuilder; + private final ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider; + + /** + * Creates a new managed channel builder with a target string, which can be either a valid {@link + * io.grpc.NameResolver}-compliant URI, or an authority string. Transport implementors must + * provide client transport factory builder, and may set custom channel default port provider. + */ + public ManagedChannelImplBuilder(String target, + ClientTransportFactoryBuilder clientTransportFactoryBuilder, + @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) { + super(target); + this.clientTransportFactoryBuilder = Preconditions + .checkNotNull(clientTransportFactoryBuilder, "clientTransportFactoryBuilder"); + + if (channelBuilderDefaultPortProvider != null) { + this.channelBuilderDefaultPortProvider = channelBuilderDefaultPortProvider; + } else { + this.channelBuilderDefaultPortProvider = new ManagedChannelDefaultPortProvider(); + } + } + + /** + * Creates a new managed channel builder with the given server address, authority string of the + * channel. Transport implementors must provide client transport factory builder, and may set + * custom channel default port provider. + */ + public ManagedChannelImplBuilder(SocketAddress directServerAddress, String authority, + ClientTransportFactoryBuilder clientTransportFactoryBuilder, + @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) { + super(directServerAddress, authority); + this.clientTransportFactoryBuilder = Preconditions + .checkNotNull(clientTransportFactoryBuilder, "clientTransportFactoryBuilder"); + + if (channelBuilderDefaultPortProvider != null) { + this.channelBuilderDefaultPortProvider = channelBuilderDefaultPortProvider; + } else { + this.channelBuilderDefaultPortProvider = new ManagedChannelDefaultPortProvider(); + } + } + + @Override + protected ClientTransportFactory buildTransportFactory() { + return clientTransportFactoryBuilder.buildClientTransportFactory(); + } + + @Override + protected int getDefaultPort() { + return channelBuilderDefaultPortProvider.getDefaultPort(); + } + + /** Disable the check whether the authority is valid. */ + public ManagedChannelImplBuilder disableCheckAuthority() { + authorityCheckerDisabled = true; + return this; + } + + /** Enable previously disabled authority check. */ + public ManagedChannelImplBuilder enableCheckAuthority() { + authorityCheckerDisabled = false; + return this; + } + + @Deprecated + public interface OverrideAuthorityChecker { + String checkAuthority(String authority); + } + + @Deprecated + public void overrideAuthorityChecker(@Nullable OverrideAuthorityChecker authorityChecker) { + this.authorityChecker = authorityChecker; + } + + @Override + protected String checkAuthority(String authority) { + if (authorityCheckerDisabled) { + return authority; + } + if (authorityChecker != null) { + return authorityChecker.checkAuthority(authority); + } + return super.checkAuthority(authority); + } + + @Override + public void setStatsEnabled(boolean value) { + super.setStatsEnabled(value); + } + + @Override + public void setStatsRecordStartedRpcs(boolean value) { + super.setStatsRecordStartedRpcs(value); + } + + @Override + public void setStatsRecordFinishedRpcs(boolean value) { + super.setStatsRecordFinishedRpcs(value); + } + + @Override + public void setStatsRecordRealTimeMetrics(boolean value) { + super.setStatsRecordRealTimeMetrics(value); + } + + @Override + public void setTracingEnabled(boolean value) { + super.setTracingEnabled(value); + } + + @Override + public ObjectPool getOffloadExecutorPool() { + return super.getOffloadExecutorPool(); + } + + public static ManagedChannelBuilder forAddress(String name, int port) { + throw new UnsupportedOperationException("ClientTransportFactoryBuilder is required"); + } + + public static ManagedChannelBuilder forTarget(String target) { + throw new UnsupportedOperationException("ClientTransportFactoryBuilder is required"); + } +} diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java new file mode 100644 index 00000000000..19a2f800965 --- /dev/null +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplBuilderTest.java @@ -0,0 +1,162 @@ +/* + * 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.internal; + +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import io.grpc.internal.ManagedChannelImplBuilder.ChannelBuilderDefaultPortProvider; +import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder; +import io.grpc.internal.ManagedChannelImplBuilder.FixedPortProvider; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnit; +import org.mockito.junit.MockitoRule; + +/** Unit tests for {@link ManagedChannelImplBuilder}. */ +@RunWith(JUnit4.class) +public class ManagedChannelImplBuilderTest { + private static final int DUMMY_PORT = 42; + private static final String DUMMY_TARGET = "fake-target"; + private static final String DUMMY_AUTHORITY_VALID = "valid:1234"; + private static final String DUMMY_AUTHORITY_INVALID = "[ : : 1]"; + + @Rule public final MockitoRule mocks = MockitoJUnit.rule(); + @Rule public final ExpectedException thrown = ExpectedException.none(); + + @Mock private ClientTransportFactoryBuilder mockClientTransportFactoryBuilder; + @Mock private ChannelBuilderDefaultPortProvider mockChannelBuilderDefaultPortProvider; + private ManagedChannelImplBuilder builder; + + @Before + public void setUp() throws Exception { + builder = new ManagedChannelImplBuilder( + DUMMY_TARGET, + mockClientTransportFactoryBuilder, + mockChannelBuilderDefaultPortProvider); + } + + /** Ensure buildTransportFactory() delegates to the custom implementation. */ + @Test + public void buildTransportFactory() { + final ClientTransportFactory clientTransportFactory = mock(ClientTransportFactory.class); + when(mockClientTransportFactoryBuilder.buildClientTransportFactory()) + .thenReturn(clientTransportFactory); + assertEquals(clientTransportFactory, builder.buildTransportFactory()); + verify(mockClientTransportFactoryBuilder).buildClientTransportFactory(); + } + + /** Ensure getDefaultPort() returns default port when no custom implementation provided. */ + @Test + public void getDefaultPort_default() { + final ManagedChannelImplBuilder builderNoPortProvider = new ManagedChannelImplBuilder( + DUMMY_TARGET, mockClientTransportFactoryBuilder, null); + assertEquals(GrpcUtil.DEFAULT_PORT_SSL, builderNoPortProvider.getDefaultPort()); + } + + /** Ensure getDefaultPort() delegates to the custom implementation. */ + @Test + public void getDefaultPort_custom() { + when(mockChannelBuilderDefaultPortProvider.getDefaultPort()).thenReturn(DUMMY_PORT); + assertEquals(DUMMY_PORT, builder.getDefaultPort()); + verify(mockChannelBuilderDefaultPortProvider).getDefaultPort(); + } + + /** Test FixedPortProvider(int port). */ + @Test + public void getDefaultPort_fixedPortProvider() { + final ManagedChannelImplBuilder builderFixedPortProvider = new ManagedChannelImplBuilder( + DUMMY_TARGET, + mockClientTransportFactoryBuilder, + new FixedPortProvider(DUMMY_PORT)); + assertEquals(DUMMY_PORT, builderFixedPortProvider.getDefaultPort()); + } + + @Test + public void checkAuthority_validAuthorityAllowed() { + assertEquals(DUMMY_AUTHORITY_VALID, builder.checkAuthority(DUMMY_AUTHORITY_VALID)); + } + + @Test + public void checkAuthority_invalidAuthorityFailed() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Invalid authority"); + + builder.checkAuthority(DUMMY_AUTHORITY_INVALID); + } + + @Test + public void disableCheckAuthority_validAuthorityAllowed() { + builder.disableCheckAuthority(); + assertEquals(DUMMY_AUTHORITY_VALID, builder.checkAuthority(DUMMY_AUTHORITY_VALID)); + } + + @Test + public void disableCheckAuthority_invalidAuthorityAllowed() { + builder.disableCheckAuthority(); + assertEquals(DUMMY_AUTHORITY_INVALID, builder.checkAuthority(DUMMY_AUTHORITY_INVALID)); + } + + @Test + public void enableCheckAuthority_validAuthorityAllowed() { + builder.disableCheckAuthority().enableCheckAuthority(); + assertEquals(DUMMY_AUTHORITY_VALID, builder.checkAuthority(DUMMY_AUTHORITY_VALID)); + } + + @Test + public void disableCheckAuthority_invalidAuthorityFailed() { + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Invalid authority"); + + builder.disableCheckAuthority().enableCheckAuthority(); + builder.checkAuthority(DUMMY_AUTHORITY_INVALID); + } + + /** Ensure authority check can disabled with custom authority check implementation. */ + @Test + @SuppressWarnings("deprecation") + public void overrideAuthorityChecker_default() { + builder.overrideAuthorityChecker( + new io.grpc.internal.ManagedChannelImplBuilder.OverrideAuthorityChecker() { + @Override public String checkAuthority(String authority) { + return authority; + } + }); + assertEquals(DUMMY_AUTHORITY_INVALID, builder.checkAuthority(DUMMY_AUTHORITY_INVALID)); + } + + /** Ensure custom authority is ignored after disableCheckAuthority(). */ + @Test + @SuppressWarnings("deprecation") + public void overrideAuthorityChecker_ignored() { + builder.overrideAuthorityChecker( + new io.grpc.internal.ManagedChannelImplBuilder.OverrideAuthorityChecker() { + @Override public String checkAuthority(String authority) { + throw new IllegalArgumentException(); + } + }); + builder.disableCheckAuthority(); + assertEquals(DUMMY_AUTHORITY_INVALID, builder.checkAuthority(DUMMY_AUTHORITY_INVALID)); + } +} diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java index 2295dcb0ff2..c551d26449c 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java @@ -62,6 +62,7 @@ import io.grpc.Status; import io.grpc.StringMarshaller; import io.grpc.internal.FakeClock.ScheduledTask; +import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder; import io.grpc.internal.TestUtils.MockClientTransportInfo; import java.net.SocketAddress; import java.net.URI; @@ -159,21 +160,15 @@ public void setUp() { when(mockTransportFactory.getScheduledExecutorService()) .thenReturn(timer.getScheduledExecutorService()); - class Builder extends AbstractManagedChannelImplBuilder { - Builder(String target) { - super(target); - } - - @Override protected ClientTransportFactory buildTransportFactory() { - throw new UnsupportedOperationException(); - } - - @Override public Builder usePlaintext() { - throw new UnsupportedOperationException(); - } - } + ManagedChannelImplBuilder builder = new ManagedChannelImplBuilder("fake://target", + new ClientTransportFactoryBuilder() { + @Override public ClientTransportFactory buildClientTransportFactory() { + throw new UnsupportedOperationException(); + } + }, + null); - Builder builder = new Builder("fake://target") + builder .nameResolverFactory(mockNameResolverFactory) .defaultLoadBalancingPolicy(MOCK_POLICY_NAME) .idleTimeout(IDLE_TIMEOUT_SECONDS, TimeUnit.SECONDS) diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 3650911f142..ba17deba613 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -107,6 +107,8 @@ import io.grpc.internal.ClientTransportFactory.ClientTransportOptions; import io.grpc.internal.InternalSubchannel.TransportLogger; import io.grpc.internal.ManagedChannelImpl.ScParser; +import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder; +import io.grpc.internal.ManagedChannelImplBuilder.FixedPortProvider; import io.grpc.internal.ServiceConfigUtil.PolicySelection; import io.grpc.internal.TestUtils.MockClientTransportInfo; import io.grpc.stub.ClientCalls; @@ -269,7 +271,7 @@ public String getPolicyName() { private CallCredentials creds; @Mock private Executor offloadExecutor; - private ChannelBuilder channelBuilder; + private ManagedChannelImplBuilder channelBuilder; private boolean requestConnection = true; private BlockingQueue transports; private boolean panicExpected; @@ -325,14 +327,20 @@ public void setUp() throws Exception { when(balancerRpcExecutorPool.getObject()) .thenReturn(balancerRpcExecutor.getScheduledExecutorService()); - channelBuilder = - new ChannelBuilder() - .nameResolverFactory(new FakeNameResolverFactory.Builder(expectedUri).build()) - .defaultLoadBalancingPolicy(MOCK_POLICY_NAME) - .userAgent(USER_AGENT) - .idleTimeout( - AbstractManagedChannelImplBuilder.IDLE_MODE_MAX_TIMEOUT_DAYS, TimeUnit.DAYS) - .offloadExecutor(offloadExecutor); + channelBuilder = new ManagedChannelImplBuilder(TARGET, + new ClientTransportFactoryBuilder() { + @Override + public ClientTransportFactory buildClientTransportFactory() { + throw new UnsupportedOperationException(); + } + }, + new FixedPortProvider(DEFAULT_PORT)); + channelBuilder + .nameResolverFactory(new FakeNameResolverFactory.Builder(expectedUri).build()) + .defaultLoadBalancingPolicy(MOCK_POLICY_NAME) + .userAgent(USER_AGENT) + .idleTimeout(ManagedChannelImplBuilder.IDLE_MODE_MAX_TIMEOUT_DAYS, TimeUnit.DAYS) + .offloadExecutor(offloadExecutor); channelBuilder.executorPool = executorPool; channelBuilder.binlog = null; channelBuilder.channelz = channelz; @@ -3466,21 +3474,18 @@ public String getDefaultScheme() { } FakeNameResolverFactory2 factory = new FakeNameResolverFactory2(); - final class CustomBuilder extends AbstractManagedChannelImplBuilder { - - CustomBuilder() { - super(TARGET); - this.executorPool = ManagedChannelImplTest.this.executorPool; - this.channelz = ManagedChannelImplTest.this.channelz; - } - @Override - protected ClientTransportFactory buildTransportFactory() { - return mockTransportFactory; - } - } - - ManagedChannel mychannel = new CustomBuilder().nameResolverFactory(factory).build(); + ManagedChannelImplBuilder customBuilder = new ManagedChannelImplBuilder(TARGET, + new ClientTransportFactoryBuilder() { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return mockTransportFactory; + } + }, + null); + customBuilder.executorPool = executorPool; + customBuilder.channelz = channelz; + ManagedChannel mychannel = customBuilder.nameResolverFactory(factory).build(); ClientCall call1 = mychannel.newCall(TestMethodDescriptors.voidMethod(), CallOptions.DEFAULT); @@ -4025,22 +4030,6 @@ public void createResolvingOobChannel() throws Exception { } } - private static final class ChannelBuilder - extends AbstractManagedChannelImplBuilder { - - ChannelBuilder() { - super(TARGET); - } - - @Override protected ClientTransportFactory buildTransportFactory() { - throw new UnsupportedOperationException(); - } - - @Override protected int getDefaultPort() { - return DEFAULT_PORT; - } - } - private static final class FakeBackoffPolicyProvider implements BackoffPolicy.Provider { @Override public BackoffPolicy get() { diff --git a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java index de6098e5bd7..49f094a6adb 100644 --- a/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java +++ b/core/src/test/java/io/grpc/internal/ServiceConfigErrorHandlingTest.java @@ -46,6 +46,8 @@ import io.grpc.NameResolver; import io.grpc.NameResolver.ConfigOrError; import io.grpc.Status; +import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder; +import io.grpc.internal.ManagedChannelImplBuilder.FixedPortProvider; import java.net.SocketAddress; import java.net.URI; import java.util.ArrayList; @@ -151,7 +153,7 @@ public ConfigOrError parseLoadBalancingPolicyConfig( private ObjectPool balancerRpcExecutorPool; @Mock private Executor blockingExecutor; - private ChannelBuilder channelBuilder; + private ManagedChannelImplBuilder channelBuilder; private void createChannel(ClientInterceptor... interceptors) { checkState(channel == null); @@ -197,14 +199,21 @@ public void setUp() throws Exception { .thenReturn(timer.getScheduledExecutorService()); when(executorPool.getObject()).thenReturn(executor.getScheduledExecutorService()); - channelBuilder = - new ChannelBuilder() - .nameResolverFactory(new FakeNameResolverFactory.Builder(expectedUri).build()) - .defaultLoadBalancingPolicy(MOCK_POLICY_NAME) - .userAgent(USER_AGENT) - .idleTimeout( - AbstractManagedChannelImplBuilder.IDLE_MODE_MAX_TIMEOUT_DAYS, TimeUnit.DAYS) - .offloadExecutor(blockingExecutor); + channelBuilder = new ManagedChannelImplBuilder(TARGET, + new ClientTransportFactoryBuilder() { + @Override + public ClientTransportFactory buildClientTransportFactory() { + throw new UnsupportedOperationException(); + } + }, + new FixedPortProvider(DEFAULT_PORT)); + + channelBuilder + .nameResolverFactory(new FakeNameResolverFactory.Builder(expectedUri).build()) + .defaultLoadBalancingPolicy(MOCK_POLICY_NAME) + .userAgent(USER_AGENT) + .idleTimeout(ManagedChannelImplBuilder.IDLE_MODE_MAX_TIMEOUT_DAYS, TimeUnit.DAYS) + .offloadExecutor(blockingExecutor); channelBuilder.executorPool = executorPool; channelBuilder.binlog = null; channelBuilder.channelz = channelz; @@ -527,22 +536,6 @@ public void validConfig_thenNoConfig_withDefaultConfig() throws Exception { verify(mockLoadBalancer, never()).handleNameResolutionError(any(Status.class)); } - private static final class ChannelBuilder - extends AbstractManagedChannelImplBuilder { - - ChannelBuilder() { - super(TARGET); - } - - @Override protected ClientTransportFactory buildTransportFactory() { - throw new UnsupportedOperationException(); - } - - @Override protected int getDefaultPort() { - return DEFAULT_PORT; - } - } - private static final class FakeBackoffPolicyProvider implements BackoffPolicy.Provider { @Override public BackoffPolicy get() { diff --git a/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java b/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java index 1a87de58507..3ed71e234aa 100644 --- a/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java +++ b/cronet/src/main/java/io/grpc/cronet/CronetChannelBuilder.java @@ -26,10 +26,14 @@ import com.google.common.util.concurrent.MoreExecutors; import io.grpc.ChannelLogger; import io.grpc.ExperimentalApi; -import io.grpc.internal.AbstractManagedChannelImplBuilder; +import io.grpc.ForwardingChannelBuilder; +import io.grpc.Internal; +import io.grpc.ManagedChannelBuilder; import io.grpc.internal.ClientTransportFactory; import io.grpc.internal.ConnectionClientTransport; import io.grpc.internal.GrpcUtil; +import io.grpc.internal.ManagedChannelImplBuilder; +import io.grpc.internal.ManagedChannelImplBuilder.ClientTransportFactoryBuilder; import io.grpc.internal.SharedResourceHolder; import io.grpc.internal.TransportTracer; import java.lang.reflect.InvocationTargetException; @@ -46,8 +50,7 @@ /** Convenience class for building channels with the cronet transport. */ @ExperimentalApi("There is no plan to make this API stable, given transport API instability") -public final class CronetChannelBuilder extends - AbstractManagedChannelImplBuilder { +public final class CronetChannelBuilder extends ForwardingChannelBuilder { private static final String LOG_TAG = "CronetChannelBuilder"; @@ -81,6 +84,8 @@ public static CronetChannelBuilder forAddress(String name, int port) { private ScheduledExecutorService scheduledExecutorService; private final CronetEngine cronetEngine; + private final ManagedChannelImplBuilder managedChannelImplBuilder; + private TransportTracer.Factory transportTracerFactory = TransportTracer.getDefaultFactory(); private boolean alwaysUsePut = false; @@ -103,12 +108,27 @@ public static CronetChannelBuilder forAddress(String name, int port) { private int trafficStatsUid; private CronetChannelBuilder(String host, int port, CronetEngine cronetEngine) { - super( + final class CronetChannelTransportFactoryBuilder implements ClientTransportFactoryBuilder { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return buildTransportFactory(); + } + } + + managedChannelImplBuilder = new ManagedChannelImplBuilder( InetSocketAddress.createUnresolved(host, port), - GrpcUtil.authorityFromHostAndPort(host, port)); + GrpcUtil.authorityFromHostAndPort(host, port), + new CronetChannelTransportFactoryBuilder(), + null); this.cronetEngine = Preconditions.checkNotNull(cronetEngine, "cronetEngine"); } + @Internal + @Override + protected ManagedChannelBuilder delegate() { + return managedChannelImplBuilder; + } + /** * Sets the maximum message size allowed to be received on the channel. If not called, * defaults to {@link io.grpc.internal.GrpcUtil#DEFAULT_MAX_MESSAGE_SIZE}. @@ -188,8 +208,7 @@ public final CronetChannelBuilder scheduledExecutorService( return this; } - @Override - protected final ClientTransportFactory buildTransportFactory() { + ClientTransportFactory buildTransportFactory() { return new CronetTransportFactory( new TaggingStreamFactory( cronetEngine, trafficStatsTagSet, trafficStatsTag, trafficStatsUidSet, trafficStatsUid), diff --git a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java index fa7010d19a8..a0a484a64a7 100644 --- a/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java +++ b/interop-testing/src/main/java/io/grpc/testing/integration/TestServiceClient.java @@ -23,12 +23,13 @@ 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; @@ -402,7 +403,7 @@ protected ManagedChannelBuilder createChannelBuilder() { if (useAlts) { return AltsChannelBuilder.forAddress(serverHost, serverPort); } - AbstractManagedChannelImplBuilder builder; + if (!useOkHttp) { SslContext sslContext = null; if (useTestCa) { @@ -425,35 +426,36 @@ protected ManagedChannelBuilder createChannelBuilder() { if (fullStreamDecompression) { nettyBuilder.enableFullStreamDecompression(); } - builder = nettyBuilder; - } else { - OkHttpChannelBuilder okBuilder = OkHttpChannelBuilder.forAddress(serverHost, serverPort); - if (serverHostOverride != null) { - // Force the hostname to match the cert the server uses. - okBuilder.overrideAuthority( - GrpcUtil.authorityFromHostAndPort(serverHostOverride, serverPort)); - } - if (useTls) { - if (useTestCa) { - try { - SSLSocketFactory factory = TestUtils.newSslSocketFactoryForCa( - Platform.get().getProvider(), TestUtils.loadCert("ca.pem")); - okBuilder.sslSocketFactory(factory); - } catch (Exception e) { - throw new RuntimeException(e); - } + // Disable the default census stats interceptor, use testing interceptor instead. + InternalNettyChannelBuilder.setStatsEnabled(nettyBuilder, false); + return nettyBuilder.intercept(createCensusStatsClientInterceptor()); + } + + OkHttpChannelBuilder okBuilder = OkHttpChannelBuilder.forAddress(serverHost, serverPort); + if (serverHostOverride != null) { + // Force the hostname to match the cert the server uses. + okBuilder.overrideAuthority( + GrpcUtil.authorityFromHostAndPort(serverHostOverride, serverPort)); + } + if (useTls) { + if (useTestCa) { + try { + SSLSocketFactory factory = TestUtils.newSslSocketFactoryForCa( + Platform.get().getProvider(), TestUtils.loadCert("ca.pem")); + okBuilder.sslSocketFactory(factory); + } catch (Exception e) { + throw new RuntimeException(e); } - } else { - okBuilder.usePlaintext(); - } - if (fullStreamDecompression) { - okBuilder.enableFullStreamDecompression(); } - builder = okBuilder; + } else { + okBuilder.usePlaintext(); + } + if (fullStreamDecompression) { + okBuilder.enableFullStreamDecompression(); } // Disable the default census stats interceptor, use testing interceptor instead. - io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false); - return builder.intercept(createCensusStatsClientInterceptor()); + InternalOkHttpChannelBuilder.setStatsEnabled(okBuilder, false); + return okBuilder.intercept(createCensusStatsClientInterceptor()); } @Override diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java index a68181be831..a2036ecea91 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/AutoWindowSizingOnTest.java @@ -17,6 +17,7 @@ package io.grpc.testing.integration; import io.grpc.internal.AbstractServerImplBuilder; +import io.grpc.netty.InternalNettyChannelBuilder; import io.grpc.netty.NegotiationType; import io.grpc.netty.NettyChannelBuilder; import io.grpc.netty.NettyServerBuilder; @@ -39,7 +40,7 @@ protected NettyChannelBuilder createChannelBuilder() { .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) .initialFlowControlWindow(NettyChannelBuilder.DEFAULT_FLOW_CONTROL_WINDOW); // Disable the default census stats interceptor, use testing interceptor instead. - io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false); + InternalNettyChannelBuilder.setStatsEnabled(builder, false); return builder.intercept(createCensusStatsClientInterceptor()); } } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java index 5592801755b..8ed7dc76900 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyLocalChannelTest.java @@ -17,6 +17,7 @@ package io.grpc.testing.integration; import io.grpc.internal.AbstractServerImplBuilder; +import io.grpc.netty.InternalNettyChannelBuilder; import io.grpc.netty.NegotiationType; import io.grpc.netty.NettyChannelBuilder; import io.grpc.netty.NettyServerBuilder; @@ -57,7 +58,7 @@ protected NettyChannelBuilder createChannelBuilder() { .flowControlWindow(65 * 1024) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE); // Disable the default census stats interceptor, use testing interceptor instead. - io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false); + InternalNettyChannelBuilder.setStatsEnabled(builder, false); return builder.intercept(createCensusStatsClientInterceptor()); } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java index 1819b9f3008..353180cbafb 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2NettyTest.java @@ -22,6 +22,7 @@ import io.grpc.internal.AbstractServerImplBuilder; import io.grpc.internal.testing.TestUtils; import io.grpc.netty.GrpcSslContexts; +import io.grpc.netty.InternalNettyChannelBuilder; import io.grpc.netty.NettyChannelBuilder; import io.grpc.netty.NettyServerBuilder; import io.netty.handler.ssl.ClientAuth; @@ -71,7 +72,7 @@ protected NettyChannelBuilder createChannelBuilder() { .ciphers(TestUtils.preferredTestCiphers(), SupportedCipherSuiteFilter.INSTANCE) .build()); // Disable the default census stats interceptor, use testing interceptor instead. - io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false); + InternalNettyChannelBuilder.setStatsEnabled(builder, false); return builder.intercept(createCensusStatsClientInterceptor()); } catch (Exception ex) { throw new RuntimeException(ex); diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java index 041c7cafaa6..fc6e600789d 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java @@ -29,6 +29,7 @@ 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 @@ protected OkHttpChannelBuilder createChannelBuilder() { 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()); } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/InProcessTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/InProcessTest.java index fd403affcf5..66894834b95 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/InProcessTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/InProcessTest.java @@ -18,6 +18,7 @@ import io.grpc.inprocess.InProcessChannelBuilder; import io.grpc.inprocess.InProcessServerBuilder; +import io.grpc.inprocess.InternalInProcessChannelBuilder; import io.grpc.internal.AbstractServerImplBuilder; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -38,7 +39,7 @@ protected AbstractServerImplBuilder getServerBuilder() { protected InProcessChannelBuilder createChannelBuilder() { InProcessChannelBuilder builder = InProcessChannelBuilder.forName(SERVER_NAME); // Disable the default census stats interceptor, use testing interceptor instead. - io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false); + InternalInProcessChannelBuilder.setStatsEnabled(builder, false); return builder.intercept(createCensusStatsClientInterceptor()); } diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java index 8bc0494b79d..1144c75073c 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/TransportCompressionTest.java @@ -37,6 +37,7 @@ import io.grpc.ServerInterceptor; import io.grpc.internal.AbstractServerImplBuilder; import io.grpc.internal.GrpcUtil; +import io.grpc.netty.InternalNettyChannelBuilder; import io.grpc.netty.NettyChannelBuilder; import io.grpc.netty.NettyServerBuilder; import io.grpc.testing.integration.Messages.BoolValue; @@ -165,7 +166,7 @@ public void onHeaders(Metadata headers) { }) .usePlaintext(); // Disable the default census stats interceptor, use testing interceptor instead. - io.grpc.internal.TestingAccessor.setStatsEnabled(builder, false); + InternalNettyChannelBuilder.setStatsEnabled(builder, false); return builder.intercept(createCensusStatsClientInterceptor()); } diff --git a/netty/src/main/java/io/grpc/netty/InternalNettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/InternalNettyChannelBuilder.java index e66f28cac73..29423389193 100644 --- a/netty/src/main/java/io/grpc/netty/InternalNettyChannelBuilder.java +++ b/netty/src/main/java/io/grpc/netty/InternalNettyChannelBuilder.java @@ -31,14 +31,31 @@ public final class InternalNettyChannelBuilder { /** * Checks authority upon channel construction. The purpose of this interface is to raise the * visibility of {@link NettyChannelBuilder.OverrideAuthorityChecker}. + * @deprecated To be removed, use {@link #disableCheckAuthority(NettyChannelBuilder builder)} to + * disable authority check. */ + @Deprecated public interface OverrideAuthorityChecker extends NettyChannelBuilder.OverrideAuthorityChecker {} + /** + * Overrides authority checker. + * @deprecated To be removed, use {@link #disableCheckAuthority(NettyChannelBuilder builder)} to + * disable authority check. + */ + @Deprecated public static void overrideAuthorityChecker( NettyChannelBuilder channelBuilder, OverrideAuthorityChecker authorityChecker) { channelBuilder.overrideAuthorityChecker(authorityChecker); } + public static void disableCheckAuthority(NettyChannelBuilder builder) { + builder.disableCheckAuthority(); + } + + public static void enableCheckAuthority(NettyChannelBuilder builder) { + builder.enableCheckAuthority(); + } + /** A class that provides a Netty handler to control protocol negotiation. */ public interface ProtocolNegotiatorFactory extends NettyChannelBuilder.ProtocolNegotiatorFactory { @@ -68,6 +85,10 @@ public static void setStatsRecordStartedRpcs(NettyChannelBuilder builder, boolea builder.setStatsRecordStartedRpcs(value); } + public static void setStatsRecordFinishedRpcs(NettyChannelBuilder builder, boolean value) { + builder.setStatsRecordFinishedRpcs(value); + } + public static void setStatsRecordRealTimeMetrics(NettyChannelBuilder builder, boolean value) { builder.setStatsRecordRealTimeMetrics(value); } diff --git a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java index 59e86a117da..43d6b96d507 100644 --- a/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java +++ b/netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java @@ -28,15 +28,19 @@ import io.grpc.ChannelLogger; import io.grpc.EquivalentAddressGroup; import io.grpc.ExperimentalApi; +import io.grpc.ForwardingChannelBuilder; import io.grpc.HttpConnectProxiedSocketAddress; 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.FixedObjectPool; 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.ObjectPool; import io.grpc.internal.SharedResourcePool; import io.grpc.internal.TransportTracer; @@ -63,8 +67,7 @@ */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1784") @CanIgnoreReturnValue -public final class NettyChannelBuilder - extends AbstractManagedChannelImplBuilder { +public final class NettyChannelBuilder extends ForwardingChannelBuilder { // 1MiB. public static final int DEFAULT_FLOW_CONTROL_WINDOW = 1024 * 1024; @@ -85,16 +88,16 @@ public final class NettyChannelBuilder DEFAULT_AUTO_FLOW_CONTROL = Boolean.parseBoolean(autoFlowControl); } - private final Map, Object> channelOptions = - new HashMap<>(); - + private final ManagedChannelImplBuilder managedChannelImplBuilder; + private TransportTracer.Factory transportTracerFactory = TransportTracer.getDefaultFactory(); + private final Map, Object> channelOptions = new HashMap<>(); private NegotiationType negotiationType = NegotiationType.TLS; - private OverrideAuthorityChecker authorityChecker; private ChannelFactory channelFactory = DEFAULT_CHANNEL_FACTORY; private ObjectPool eventLoopGroupPool = DEFAULT_EVENT_LOOP_GROUP_POOL; private SslContext sslContext; private boolean autoFlowControl = DEFAULT_AUTO_FLOW_CONTROL; private int flowControlWindow = DEFAULT_FLOW_CONTROL_WINDOW; + private int maxInboundMessageSize = GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; private int maxHeaderListSize = GrpcUtil.DEFAULT_MAX_HEADER_LIST_SIZE; private long keepAliveTimeNanos = KEEPALIVE_TIME_NANOS_DISABLED; private long keepAliveTimeoutNanos = DEFAULT_KEEPALIVE_TIMEOUT_NANOS; @@ -142,14 +145,39 @@ public static NettyChannelBuilder forTarget(String target) { this(GrpcUtil.authorityFromHostAndPort(host, port)); } + private final class NettyChannelTransportFactoryBuilder implements ClientTransportFactoryBuilder { + @Override + public ClientTransportFactory buildClientTransportFactory() { + return buildTransportFactory(); + } + } + + private final class NettyChannelDefaultPortProvider implements ChannelBuilderDefaultPortProvider { + @Override + public int getDefaultPort() { + return NettyChannelBuilder.this.getDefaultPort(); + } + } + @CheckReturnValue NettyChannelBuilder(String target) { - super(target); + managedChannelImplBuilder = new ManagedChannelImplBuilder(target, + new NettyChannelTransportFactoryBuilder(), + new NettyChannelDefaultPortProvider()); } @CheckReturnValue NettyChannelBuilder(SocketAddress address) { - super(address, getAuthorityFromAddress(address)); + managedChannelImplBuilder = new ManagedChannelImplBuilder(address, + getAuthorityFromAddress(address), + new NettyChannelTransportFactoryBuilder(), + new NettyChannelDefaultPortProvider()); + } + + @Internal + @Override + protected ManagedChannelBuilder delegate() { + return managedChannelImplBuilder; } @CheckReturnValue @@ -408,10 +436,20 @@ public SocketAddress createSocketAddress( } } + /** + * 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 + public NettyChannelBuilder maxInboundMessageSize(int max) { + checkArgument(max >= 0, "negative max"); + maxInboundMessageSize = max; + return this; + } + @CheckReturnValue - @Internal - protected ClientTransportFactory buildTransportFactory() { + ClientTransportFactory buildTransportFactory() { assertEventLoopAndChannelType(); ProtocolNegotiator negotiator; @@ -427,12 +465,12 @@ protected ClientTransportFactory buildTransportFactory() { } } negotiator = createProtocolNegotiatorByType(negotiationType, localSslContext, - this.getOffloadExecutorPool()); + this.managedChannelImplBuilder.getOffloadExecutorPool()); } return new NettyTransportFactory( negotiator, channelFactory, channelOptions, - eventLoopGroupPool, autoFlowControl, flowControlWindow, maxInboundMessageSize(), + eventLoopGroupPool, autoFlowControl, flowControlWindow, maxInboundMessageSize, maxHeaderListSize, keepAliveTimeNanos, keepAliveTimeoutNanos, keepAliveWithoutCalls, transportTracerFactory, localSocketPicker, useGetForSafeMethods); } @@ -448,9 +486,8 @@ void assertEventLoopAndChannelType() { "Both EventLoopGroup and ChannelType should be provided or neither should be"); } - @Override @CheckReturnValue - protected int getDefaultPort() { + int getDefaultPort() { switch (negotiationType) { case PLAINTEXT: case PLAINTEXT_UPGRADE: @@ -462,10 +499,6 @@ protected int getDefaultPort() { } } - void overrideAuthorityChecker(@Nullable OverrideAuthorityChecker authorityChecker) { - this.authorityChecker = authorityChecker; - } - @VisibleForTesting @CheckReturnValue static ProtocolNegotiator createProtocolNegotiatorByType( @@ -484,19 +517,22 @@ static ProtocolNegotiator createProtocolNegotiatorByType( } } - @CheckReturnValue - interface OverrideAuthorityChecker { - String checkAuthority(String authority); + @Deprecated + interface OverrideAuthorityChecker extends ManagedChannelImplBuilder.OverrideAuthorityChecker {} + + @Deprecated + void overrideAuthorityChecker(@Nullable OverrideAuthorityChecker authorityChecker) { + this.managedChannelImplBuilder.overrideAuthorityChecker(authorityChecker); } - @Override - @CheckReturnValue - @Internal - protected String checkAuthority(String authority) { - if (authorityChecker != null) { - return authorityChecker.checkAuthority(authority); - } - return super.checkAuthority(authority); + NettyChannelBuilder disableCheckAuthority() { + this.managedChannelImplBuilder.disableCheckAuthority(); + return this; + } + + NettyChannelBuilder enableCheckAuthority() { + this.managedChannelImplBuilder.enableCheckAuthority(); + return this; } void protocolNegotiatorFactory(ProtocolNegotiatorFactory protocolNegotiatorFactory) { @@ -504,24 +540,24 @@ void protocolNegotiatorFactory(ProtocolNegotiatorFactory protocolNegotiatorFacto = checkNotNull(protocolNegotiatorFactory, "protocolNegotiatorFactory"); } - @Override - protected void setTracingEnabled(boolean value) { - super.setTracingEnabled(value); + void setTracingEnabled(boolean value) { + this.managedChannelImplBuilder.setTracingEnabled(value); } - @Override - protected void setStatsEnabled(boolean value) { - super.setStatsEnabled(value); + void setStatsEnabled(boolean value) { + this.managedChannelImplBuilder.setStatsEnabled(value); } - @Override - protected void setStatsRecordStartedRpcs(boolean value) { - super.setStatsRecordStartedRpcs(value); + void setStatsRecordStartedRpcs(boolean value) { + this.managedChannelImplBuilder.setStatsRecordStartedRpcs(value); } - @Override - protected void setStatsRecordRealTimeMetrics(boolean value) { - super.setStatsRecordRealTimeMetrics(value); + void setStatsRecordFinishedRpcs(boolean value) { + this.managedChannelImplBuilder.setStatsRecordFinishedRpcs(value); + } + + void setStatsRecordRealTimeMetrics(boolean value) { + this.managedChannelImplBuilder.setStatsRecordRealTimeMetrics(value); } @VisibleForTesting diff --git a/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java b/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java index 3621e6e2454..9a96d73e5c5 100644 --- a/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java +++ b/netty/src/test/java/io/grpc/netty/NettyChannelBuilderTest.java @@ -22,7 +22,7 @@ import static org.mockito.Mockito.mock; import io.grpc.ManagedChannel; -import io.grpc.netty.InternalNettyChannelBuilder.OverrideAuthorityChecker; +import io.grpc.internal.GrpcUtil; import io.grpc.netty.NettyTestUtil.TrackingObjectPoolForTest; import io.netty.channel.Channel; import io.netty.channel.ChannelFactory; @@ -44,7 +44,7 @@ public class NettyChannelBuilderTest { @Rule public final ExpectedException thrown = ExpectedException.none(); private final SslContext noSslContext = null; - + private void shutdown(ManagedChannel mc) throws Exception { mc.shutdownNow(); assertTrue(mc.awaitTermination(1, TimeUnit.SECONDS)); @@ -92,14 +92,35 @@ private void overrideAuthorityIsReadableHelper(NettyChannelBuilder builder, } @Test + @Deprecated public void overrideAllowsInvalidAuthority() { NettyChannelBuilder builder = new NettyChannelBuilder(new SocketAddress(){}); - InternalNettyChannelBuilder.overrideAuthorityChecker(builder, new OverrideAuthorityChecker() { - @Override - public String checkAuthority(String authority) { - return authority; - } - }); + InternalNettyChannelBuilder.overrideAuthorityChecker(builder, + new io.grpc.netty.InternalNettyChannelBuilder.OverrideAuthorityChecker() { + @Override + public String checkAuthority(String authority) { + return authority; + } + }); + Object unused = builder.overrideAuthority("[invalidauthority") + .negotiationType(NegotiationType.PLAINTEXT) + .buildTransportFactory(); + } + + @Test + @Deprecated + public void overrideFailsInvalidAuthority() { + NettyChannelBuilder builder = new NettyChannelBuilder(new SocketAddress(){}); + InternalNettyChannelBuilder.overrideAuthorityChecker(builder, + new io.grpc.netty.InternalNettyChannelBuilder.OverrideAuthorityChecker() { + @Override + public String checkAuthority(String authority) { + return GrpcUtil.checkAuthority(authority); + } + }); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Invalid authority:"); Object unused = builder.overrideAuthority("[invalidauthority") .negotiationType(NegotiationType.PLAINTEXT) .buildTransportFactory(); @@ -115,6 +136,27 @@ public void failOverrideInvalidAuthority() { builder.overrideAuthority("[invalidauthority"); } + @Test + public void disableCheckAuthorityAllowsInvalidAuthority() { + NettyChannelBuilder builder = new NettyChannelBuilder(new SocketAddress(){}) + .disableCheckAuthority(); + + Object unused = builder.overrideAuthority("[invalidauthority") + .negotiationType(NegotiationType.PLAINTEXT) + .buildTransportFactory(); + } + + @Test + public void enableCheckAuthorityFailOverrideInvalidAuthority() { + NettyChannelBuilder builder = new NettyChannelBuilder(new SocketAddress(){}) + .disableCheckAuthority() + .enableCheckAuthority(); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("Invalid authority:"); + builder.overrideAuthority("[invalidauthority"); + } + @Test public void failInvalidAuthority() { thrown.expect(IllegalArgumentException.class); diff --git a/okhttp/src/main/java/io/grpc/okhttp/InternalOkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/InternalOkHttpChannelBuilder.java new file mode 100644 index 00000000000..d328efd7145 --- /dev/null +++ b/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() {} +} diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index 26a76f2c8c4..a7759127e62 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java @@ -24,13 +24,17 @@ 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 @@ /** Convenience class for building channels with the OkHttp transport. */ @ExperimentalApi("https://github.com/grpc/grpc-java/issues/1785") -public class OkHttpChannelBuilder extends - AbstractManagedChannelImplBuilder { +public class OkHttpChannelBuilder extends ForwardingChannelBuilder { 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 static OkHttpChannelBuilder forTarget(String target) { 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 @@ protected OkHttpChannelBuilder(String host, int port) { } 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 OkHttpChannelBuilder maxInboundMetadataSize(int bytes) { 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 - protected final ClientTransportFactory buildTransportFactory() { + public final OkHttpChannelBuilder maxInboundMessageSize(int max) { + 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 @@ protected final ClientTransportFactory buildTransportFactory() { createSslSocketFactory(), hostnameVerifier, connectionSpec, - maxInboundMessageSize(), + maxInboundMessageSize, enableKeepAlive, keepAliveTimeNanos, keepAliveTimeoutNanos, @@ -385,8 +424,17 @@ protected final ClientTransportFactory buildTransportFactory() { useGetForSafeMethods); } - @Override - protected int getDefaultPort() { + final OkHttpChannelBuilder disableCheckAuthority() { + 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 @@ protected int getDefaultPort() { } } + final void setStatsEnabled(boolean value) { + this.managedChannelImplBuilder.setStatsEnabled(value); + } + @VisibleForTesting @Nullable SSLSocketFactory createSslSocketFactory() { diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java index 512107eabca..4aa1d1aa53c 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpChannelBuilderTest.java @@ -72,20 +72,26 @@ private void overrideAuthorityIsReadableHelper(OkHttpChannelBuilder builder, } @Test - public void overrideAllowsInvalidAuthority() { - OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234) { - @Override - protected String checkAuthority(String authority) { - return authority; - } - }; + public void failOverrideInvalidAuthority() { + OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234); + + 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() { - OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234); + public void enableCheckAuthorityFailOverrideInvalidAuthority() { + OkHttpChannelBuilder builder = new OkHttpChannelBuilder("good", 1234) + .disableCheckAuthority() + .enableCheckAuthority(); thrown.expect(IllegalArgumentException.class); thrown.expectMessage("Invalid authority:"); diff --git a/testing/src/main/java/io/grpc/internal/TestingAccessor.java b/testing/src/main/java/io/grpc/internal/TestingAccessor.java index d5b00e025fc..92dc114d9c6 100644 --- a/testing/src/main/java/io/grpc/internal/TestingAccessor.java +++ b/testing/src/main/java/io/grpc/internal/TestingAccessor.java @@ -20,13 +20,6 @@ * Test helper that allows accessing package-private stuff. */ public final class TestingAccessor { - /** - * Disable or enable client side census stats features. - */ - public static void setStatsEnabled( - AbstractManagedChannelImplBuilder builder, boolean statsEnabled) { - builder.setStatsEnabled(statsEnabled); - } /** * Disable or enable server side census stats features.