From fc7e105b9cdf15b72047cb6596375dfb4e4e3265 Mon Sep 17 00:00:00 2001 From: Spencer Fang Date: Mon, 6 Nov 2017 20:52:28 -0800 Subject: [PATCH 1/2] core: noop ProxyDetector for GAE+JDK7 --- .../AbstractManagedChannelImplBuilder.java | 4 +--- .../internal/DnsNameResolverProvider.java | 2 +- .../main/java/io/grpc/internal/GrpcUtil.java | 24 +++++++++++++++++++ .../java/io/grpc/internal/ProxyDetector.java | 11 --------- .../io/grpc/internal/DnsNameResolverTest.java | 2 +- .../grpc/internal/InternalSubchannelTest.java | 2 +- .../ManagedChannelImplIdlenessTest.java | 2 +- .../grpc/internal/ManagedChannelImplTest.java | 2 +- 8 files changed, 30 insertions(+), 19 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index 471f8292b9a..60387b83cd9 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -116,8 +116,6 @@ public static ManagedChannelBuilder forTarget(String target) { String authorityOverride; - private ProxyDetector proxyDetector = ProxyDetector.DEFAULT_INSTANCE; - LoadBalancer.Factory loadBalancerFactory = DEFAULT_LOAD_BALANCER_FACTORY; boolean fullStreamDecompression; @@ -338,7 +336,7 @@ public ManagedChannel build() { SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR), GrpcUtil.STOPWATCH_SUPPLIER, getEffectiveInterceptors(), - proxyDetector); + GrpcUtil.getProxyDetector()); } @VisibleForTesting diff --git a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java index 2841b4d07c7..6dcc9c6ee31 100644 --- a/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java +++ b/core/src/main/java/io/grpc/internal/DnsNameResolverProvider.java @@ -48,7 +48,7 @@ public DnsNameResolver newNameResolver(URI targetUri, Attributes params) { "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri); String name = targetPath.substring(1); return new DnsNameResolver(targetUri.getAuthority(), name, params, GrpcUtil.TIMER_SERVICE, - GrpcUtil.SHARED_CHANNEL_EXECUTOR, ProxyDetector.DEFAULT_INSTANCE); + GrpcUtil.SHARED_CHANNEL_EXECUTOR, GrpcUtil.getProxyDetector()); } else { return null; } diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index 1102fbf6a85..34cf08e294b 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/core/src/main/java/io/grpc/internal/GrpcUtil.java @@ -43,6 +43,7 @@ import java.lang.reflect.Method; import java.net.HttpURLConnection; import java.net.InetSocketAddress; +import java.net.SocketAddress; import java.net.URI; import java.net.URISyntaxException; import java.nio.charset.Charset; @@ -223,6 +224,29 @@ public byte[] parseAsciiString(byte[] serialized) { */ public static final long SERVER_KEEPALIVE_TIME_NANOS_DISABLED = Long.MAX_VALUE; + /** The default proxy detector. */ + public static final ProxyDetector DEFAULT_PROXY_DETECTOR = new ProxyDetectorImpl(); + + /** A proxy detector that always claims no proxy is needed. */ + public static final ProxyDetector NOOP_PROXY_DETECTOR = new ProxyDetector() { + @Nullable + @Override + public ProxyParameters proxyFor(SocketAddress targetServerAddress) { + return null; + } + }; + + /** + * Returns a proxy detector appropriate for the current environment. + */ + public static ProxyDetector getProxyDetector() { + if (IS_RESTRICTED_APPENGINE) { + return NOOP_PROXY_DETECTOR; + } else { + return DEFAULT_PROXY_DETECTOR; + } + } + /** * Maps HTTP error response status codes to transport codes, as defined in diff --git a/core/src/main/java/io/grpc/internal/ProxyDetector.java b/core/src/main/java/io/grpc/internal/ProxyDetector.java index 40710a5ec46..6de66836974 100644 --- a/core/src/main/java/io/grpc/internal/ProxyDetector.java +++ b/core/src/main/java/io/grpc/internal/ProxyDetector.java @@ -24,17 +24,6 @@ * {@link java.net.SocketAddress}. */ public interface ProxyDetector { - ProxyDetector DEFAULT_INSTANCE = new ProxyDetectorImpl(); - - /** A proxy detector that always claims no proxy is needed, for unit test convenience. */ - ProxyDetector NOOP_INSTANCE = new ProxyDetector() { - @Nullable - @Override - public ProxyParameters proxyFor(SocketAddress targetServerAddress) { - return null; - } - }; - /** * Given a target address, returns which proxy address should be used. If no proxy should be * used, then return value will be null. diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java index 9ee537e54ba..ee0324f1de9 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverTest.java @@ -112,7 +112,7 @@ public void close(ExecutorService instance) { private ArgumentCaptor statusCaptor; private DnsNameResolver newResolver(String name, int port) { - return newResolver(name, port, mockResolver, ProxyDetector.NOOP_INSTANCE); + return newResolver(name, port, mockResolver, GrpcUtil.NOOP_PROXY_DETECTOR); } private DnsNameResolver newResolver( diff --git a/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java b/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java index 30c757e3624..8fce655091b 100644 --- a/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java +++ b/core/src/test/java/io/grpc/internal/InternalSubchannelTest.java @@ -885,7 +885,7 @@ public ProxyParameters proxyFor(SocketAddress targetServerAddress) { } private void createInternalSubchannel(SocketAddress ... addrs) { - createInternalSubChannelWithProxy(ProxyDetector.NOOP_INSTANCE, addrs); + createInternalSubChannelWithProxy(GrpcUtil.NOOP_PROXY_DETECTOR, addrs); } private void createInternalSubChannelWithProxy( diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java index 94e652b036b..b8dae419e77 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplIdlenessTest.java @@ -143,7 +143,7 @@ class Builder extends AbstractManagedChannelImplBuilder { builder, mockTransportFactory, new FakeBackoffPolicyProvider(), oobExecutorPool, timer.getStopwatchSupplier(), Collections.emptyList(), - ProxyDetector.NOOP_INSTANCE); + GrpcUtil.NOOP_PROXY_DETECTOR); newTransports = TestUtils.captureTransports(mockTransportFactory); for (int i = 0; i < 2; i++) { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index 672e24ef263..bb9236d6ef7 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -215,7 +215,7 @@ class Builder extends AbstractManagedChannelImplBuilder { checkState(channel == null); channel = new ManagedChannelImpl( builder, mockTransportFactory, new FakeBackoffPolicyProvider(), - oobExecutorPool, timer.getStopwatchSupplier(), interceptors, ProxyDetector.NOOP_INSTANCE); + oobExecutorPool, timer.getStopwatchSupplier(), interceptors, GrpcUtil.NOOP_PROXY_DETECTOR); if (requestConnection) { // Force-exit the initial idle-mode From 545b154ac8dcacc3356c63303327b66e6b9bbcd6 Mon Sep 17 00:00:00 2001 From: Spencer Fang Date: Tue, 7 Nov 2017 07:38:08 -0800 Subject: [PATCH 2/2] be consistent about multiline comment whitespace --- core/src/main/java/io/grpc/internal/GrpcUtil.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/io/grpc/internal/GrpcUtil.java b/core/src/main/java/io/grpc/internal/GrpcUtil.java index 34cf08e294b..b031a632987 100644 --- a/core/src/main/java/io/grpc/internal/GrpcUtil.java +++ b/core/src/main/java/io/grpc/internal/GrpcUtil.java @@ -224,10 +224,14 @@ public byte[] parseAsciiString(byte[] serialized) { */ public static final long SERVER_KEEPALIVE_TIME_NANOS_DISABLED = Long.MAX_VALUE; - /** The default proxy detector. */ + /** + * The default proxy detector. + */ public static final ProxyDetector DEFAULT_PROXY_DETECTOR = new ProxyDetectorImpl(); - /** A proxy detector that always claims no proxy is needed. */ + /** + * A proxy detector that always claims no proxy is needed. + */ public static final ProxyDetector NOOP_PROXY_DETECTOR = new ProxyDetector() { @Nullable @Override