From 561957d5574b323b2bba5d1235826e671c55cdfc Mon Sep 17 00:00:00 2001 From: Sergii Tkachenko Date: Mon, 14 Sep 2020 13:28:10 -0400 Subject: [PATCH] Inline AbstractManagedChannelImplBuilder: convert methods calling super() --- .../AbstractManagedChannelImplBuilder.java | 102 --------------- .../internal/ManagedChannelImplBuilder.java | 119 ++++++++++++++---- 2 files changed, 96 insertions(+), 125 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index f838d9448484..67f5618268b3 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -17,13 +17,10 @@ package io.grpc.internal; import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.Preconditions; -import io.grpc.Attributes; import io.grpc.BinaryLog; import io.grpc.ClientInterceptor; import io.grpc.CompressorRegistry; import io.grpc.DecompressorRegistry; -import io.grpc.EquivalentAddressGroup; import io.grpc.InternalChannelz; import io.grpc.ManagedChannelBuilder; import io.grpc.NameResolver; @@ -31,11 +28,7 @@ import io.grpc.ProxyDetector; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.net.SocketAddress; -import java.net.URI; -import java.net.URISyntaxException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.concurrent.Executor; @@ -51,8 +44,6 @@ */ public abstract class AbstractManagedChannelImplBuilder > extends ManagedChannelBuilder { - private static final String DIRECT_ADDRESS_SCHEME = "directaddress"; - private static final Logger log = Logger.getLogger(AbstractManagedChannelImplBuilder.class.getName()); @@ -103,11 +94,6 @@ public static ManagedChannelBuilder forTarget(String target) { // Access via getter, which may perform authority override as needed protected NameResolver.Factory nameResolverFactory = nameResolverRegistry.asFactory(); - final String target; - - @Nullable - protected final SocketAddress directServerAddress; - @Nullable String userAgent; @@ -142,8 +128,6 @@ public static ManagedChannelBuilder forTarget(String target) { Map defaultServiceConfig; boolean lookUpServiceConfig = true; - protected TransportTracer.Factory transportTracerFactory = TransportTracer.getDefaultFactory(); - protected int maxInboundMessageSize = GrpcUtil.DEFAULT_MAX_MESSAGE_SIZE; @Nullable @@ -158,32 +142,6 @@ public static ManagedChannelBuilder forTarget(String target) { protected boolean recordRealTimeMetrics = false; protected boolean tracingEnabled = true; - protected AbstractManagedChannelImplBuilder(String target) { - this.target = Preconditions.checkNotNull(target, "target"); - this.directServerAddress = null; - } - - /** - * Returns a target string for the SocketAddress. It is only used as a placeholder, because - * DirectAddressNameResolverFactory will not actually try to use it. However, it must be a valid - * URI. - */ - @VisibleForTesting - static String makeTargetStringForDirectAddress(SocketAddress address) { - try { - return new URI(DIRECT_ADDRESS_SCHEME, "", "/" + address, null).toString(); - } catch (URISyntaxException e) { - // It should not happen. - throw new RuntimeException(e); - } - } - - protected AbstractManagedChannelImplBuilder(SocketAddress directServerAddress, String authority) { - this.target = makeTargetStringForDirectAddress(directServerAddress); - this.directServerAddress = directServerAddress; - this.nameResolverFactory = new DirectAddressNameResolverFactory(directServerAddress, authority); - } - // Temporarily disable retry when stats or tracing is enabled to avoid breakage, until we know // what should be the desired behavior for retry + stats/tracing. // TODO(zdapeng): FIX IT @@ -253,22 +211,6 @@ final List getEffectiveInterceptors() { return effectiveInterceptors; } - /** - * Subclasses should override this method to provide the {@link ClientTransportFactory} - * appropriate for this channel. This method is meant for Transport implementors and should not - * be used by normal users. - */ - protected abstract ClientTransportFactory buildTransportFactory(); - - /** - * Subclasses can override this method to provide a default port to {@link NameResolver} for use - * in cases where the target string doesn't include a port. The default implementation returns - * {@link GrpcUtil#DEFAULT_PORT_SSL}. - */ - protected int getDefaultPort() { - return GrpcUtil.DEFAULT_PORT_SSL; - } - /** * Returns a {@link NameResolver.Factory} for the channel. */ @@ -279,48 +221,4 @@ NameResolver.Factory getNameResolverFactory() { return new OverrideAuthorityNameResolverFactory(nameResolverFactory, authorityOverride); } } - - private static class DirectAddressNameResolverFactory extends NameResolver.Factory { - final SocketAddress address; - final String authority; - - DirectAddressNameResolverFactory(SocketAddress address, String authority) { - this.address = address; - this.authority = authority; - } - - @Override - public NameResolver newNameResolver(URI notUsedUri, NameResolver.Args args) { - return new NameResolver() { - @Override - public String getServiceAuthority() { - return authority; - } - - @Override - public void start(Listener2 listener) { - listener.onResult( - ResolutionResult.newBuilder() - .setAddresses(Collections.singletonList(new EquivalentAddressGroup(address))) - .setAttributes(Attributes.EMPTY) - .build()); - } - - @Override - public void shutdown() {} - }; - } - - @Override - public String getDefaultScheme() { - return DIRECT_ADDRESS_SCHEME; - } - } - - /** - * Returns the internal offload executor pool for offloading tasks. - */ - protected ObjectPool getOffloadExecutorPool() { - return this.offloadExecutorPool; - } } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java index de600d61890f..3d80a599a857 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImplBuilder.java @@ -19,15 +19,19 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.util.concurrent.MoreExecutors; +import io.grpc.Attributes; import io.grpc.BinaryLog; import io.grpc.ClientInterceptor; import io.grpc.CompressorRegistry; import io.grpc.DecompressorRegistry; +import io.grpc.EquivalentAddressGroup; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import io.grpc.NameResolver; import io.grpc.ProxyDetector; import java.net.SocketAddress; +import java.net.URI; +import java.net.URISyntaxException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -44,8 +48,6 @@ public final class ManagedChannelImplBuilder extends AbstractManagedChannelImplBuilder { - private boolean authorityCheckerDisabled; - /** * An interface for Transport implementors to provide the {@link ClientTransportFactory} * appropriate for the channel. @@ -89,16 +91,31 @@ public int getDefaultPort() { } } - private final class ManagedChannelDefaultPortProvider implements + private static final class ManagedChannelDefaultPortProvider implements ChannelBuilderDefaultPortProvider { @Override public int getDefaultPort() { - return ManagedChannelImplBuilder.super.getDefaultPort(); + return GrpcUtil.DEFAULT_PORT_SSL; } } + private static final String DIRECT_ADDRESS_SCHEME = "directaddress"; + + final String target; + @Nullable + private final SocketAddress directServerAddress; private final ClientTransportFactoryBuilder clientTransportFactoryBuilder; private final ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider; + private boolean authorityCheckerDisabled; + + + 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"); + } /** * Creates a new managed channel builder with a target string, which can be either a valid {@link @@ -108,9 +125,10 @@ public int getDefaultPort() { public ManagedChannelImplBuilder(String target, ClientTransportFactoryBuilder clientTransportFactoryBuilder, @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) { - super(target); + this.target = Preconditions.checkNotNull(target, "target"); this.clientTransportFactoryBuilder = Preconditions .checkNotNull(clientTransportFactoryBuilder, "clientTransportFactoryBuilder"); + this.directServerAddress = null; if (channelBuilderDefaultPortProvider != null) { this.channelBuilderDefaultPortProvider = channelBuilderDefaultPortProvider; @@ -127,9 +145,11 @@ public ManagedChannelImplBuilder(String target, public ManagedChannelImplBuilder(SocketAddress directServerAddress, String authority, ClientTransportFactoryBuilder clientTransportFactoryBuilder, @Nullable ChannelBuilderDefaultPortProvider channelBuilderDefaultPortProvider) { - super(directServerAddress, authority); + this.target = makeTargetStringForDirectAddress(directServerAddress); this.clientTransportFactoryBuilder = Preconditions .checkNotNull(clientTransportFactoryBuilder, "clientTransportFactoryBuilder"); + this.directServerAddress = directServerAddress; + this.nameResolverFactory = new DirectAddressNameResolverFactory(directServerAddress, authority); if (channelBuilderDefaultPortProvider != null) { this.channelBuilderDefaultPortProvider = channelBuilderDefaultPortProvider; @@ -138,13 +158,35 @@ public ManagedChannelImplBuilder(SocketAddress directServerAddress, String autho } } - @Override - protected ClientTransportFactory buildTransportFactory() { + /** + * Returns a target string for the SocketAddress. It is only used as a placeholder, because + * DirectAddressNameResolverFactory will not actually try to use it. However, it must be a valid + * URI. + */ + @VisibleForTesting + static String makeTargetStringForDirectAddress(SocketAddress address) { + try { + return new URI(DIRECT_ADDRESS_SCHEME, "", "/" + address, null).toString(); + } catch (URISyntaxException e) { + // It should not happen. + throw new RuntimeException(e); + } + } + + /** + * Transport implementors must provide {@link ClientTransportFactoryBuilder} that returns {@link + * ClientTransportFactory} appropriate the channel. This method is meant for Transport + * implementors and should not be used by normal users. + */ + ClientTransportFactory buildTransportFactory() { return clientTransportFactoryBuilder.buildClientTransportFactory(); } - @Override - protected int getDefaultPort() { + /** + * Returns a default port to {@link NameResolver} for use in cases where the target string doesn't + * include a port. The default implementation returns {@link GrpcUtil#DEFAULT_PORT_SSL}. + */ + int getDefaultPort() { return channelBuilderDefaultPortProvider.getDefaultPort(); } @@ -272,8 +314,8 @@ public ManagedChannelImplBuilder idleTimeout(long value, TimeUnit unit) { } /** - * 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 + * 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 @@ -425,8 +467,8 @@ public void setStatsEnabled(boolean value) { } /** - * Disable or enable stats recording for RPC upstarts. Effective only if {@link - * #setStatsEnabled} is set to true. Enabled by default. + * Disable or enable stats recording for RPC upstarts. Effective only if {@link #setStatsEnabled} + * is set to true. Enabled by default. */ public void setStatsRecordStartedRpcs(boolean value) { recordStartedRpcs = value; @@ -451,8 +493,8 @@ public void setStatsRecordRealTimeMetrics(boolean value) { /** * Disable or enable tracing features. Enabled by default. * - *

For the current release, calling {@code setTracingEnabled(true)} may have a side effect that - * disables retry. + *

For the current release, calling {@code setTracingEnabled(true)} may have a side effect + * that disables retry. */ public void setTracingEnabled(boolean value) { tracingEnabled = value; @@ -486,16 +528,47 @@ String checkAuthority(String authority) { return GrpcUtil.checkAuthority(authority); } - @Override + /** + * Returns the internal offload executor pool for offloading tasks. + */ public ObjectPool getOffloadExecutorPool() { - return super.getOffloadExecutorPool(); + return this.offloadExecutorPool; } - public static ManagedChannelBuilder forAddress(String name, int port) { - throw new UnsupportedOperationException("ClientTransportFactoryBuilder is required"); - } + private static class DirectAddressNameResolverFactory extends NameResolver.Factory { + final SocketAddress address; + final String authority; - public static ManagedChannelBuilder forTarget(String target) { - throw new UnsupportedOperationException("ClientTransportFactoryBuilder is required"); + DirectAddressNameResolverFactory(SocketAddress address, String authority) { + this.address = address; + this.authority = authority; + } + + @Override + public NameResolver newNameResolver(URI notUsedUri, NameResolver.Args args) { + return new NameResolver() { + @Override + public String getServiceAuthority() { + return authority; + } + + @Override + public void start(Listener2 listener) { + listener.onResult( + ResolutionResult.newBuilder() + .setAddresses(Collections.singletonList(new EquivalentAddressGroup(address))) + .setAttributes(Attributes.EMPTY) + .build()); + } + + @Override + public void shutdown() {} + }; + } + + @Override + public String getDefaultScheme() { + return DIRECT_ADDRESS_SCHEME; + } } }