From 16247153f68b1f8679b1e7d8506e90af53a64e58 Mon Sep 17 00:00:00 2001 From: Kun Zhang Date: Tue, 8 Dec 2015 17:52:05 -0800 Subject: [PATCH] Move default scheme decision from ManagedChannelImpl to NameResolver.Factory. This makes it possible to use a default scheme other than "dns" by installing a custom NameResolver.Factory to the channel builder. --- .../java/io/grpc/DnsNameResolverFactory.java | 8 +++++- core/src/main/java/io/grpc/NameResolver.java | 7 +++++ .../java/io/grpc/NameResolverRegistry.java | 26 ++++++++++++------- .../AbstractManagedChannelImplBuilder.java | 8 +++++- .../io/grpc/internal/ManagedChannelImpl.java | 5 ++-- ...ManagedChannelImplGetNameResolverTest.java | 23 ++++++++++------ .../grpc/internal/ManagedChannelImplTest.java | 10 +++++++ ...anagedChannelImplTransportManagerTest.java | 5 ++++ 8 files changed, 71 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/io/grpc/DnsNameResolverFactory.java b/core/src/main/java/io/grpc/DnsNameResolverFactory.java index 5af73f828f..39c2238fc7 100644 --- a/core/src/main/java/io/grpc/DnsNameResolverFactory.java +++ b/core/src/main/java/io/grpc/DnsNameResolverFactory.java @@ -53,11 +53,12 @@ import java.net.URI; @ExperimentalApi public final class DnsNameResolverFactory extends NameResolver.Factory { + private static final String SCHEME = "dns"; private static final DnsNameResolverFactory instance = new DnsNameResolverFactory(); @Override public NameResolver newNameResolver(URI targetUri, Attributes params) { - if ("dns".equals(targetUri.getScheme())) { + if (SCHEME.equals(targetUri.getScheme())) { String targetPath = Preconditions.checkNotNull(targetUri.getPath(), "targetPath"); Preconditions.checkArgument(targetPath.startsWith("/"), "the path component (%s) of the target (%s) must start with '/'", targetPath, targetUri); @@ -68,6 +69,11 @@ public final class DnsNameResolverFactory extends NameResolver.Factory { } } + @Override + public String getDefaultScheme() { + return SCHEME; + } + private DnsNameResolverFactory() { } diff --git a/core/src/main/java/io/grpc/NameResolver.java b/core/src/main/java/io/grpc/NameResolver.java index 878b492874..29db2df9fe 100644 --- a/core/src/main/java/io/grpc/NameResolver.java +++ b/core/src/main/java/io/grpc/NameResolver.java @@ -87,6 +87,13 @@ public abstract class NameResolver { */ @Nullable public abstract NameResolver newNameResolver(URI targetUri, Attributes params); + + /** + * Returns the default scheme, which will be used to construct a URI when {@link + * ManagedChannelBuilder#forTarget(String)} is given an authority string instead of a compliant + * URI. + */ + public abstract String getDefaultScheme(); } /** diff --git a/core/src/main/java/io/grpc/NameResolverRegistry.java b/core/src/main/java/io/grpc/NameResolverRegistry.java index 345dac91a2..c22055938f 100644 --- a/core/src/main/java/io/grpc/NameResolverRegistry.java +++ b/core/src/main/java/io/grpc/NameResolverRegistry.java @@ -31,6 +31,8 @@ package io.grpc; +import com.google.common.base.Preconditions; + import java.net.URI; import java.util.concurrent.CopyOnWriteArrayList; @@ -43,23 +45,24 @@ import javax.annotation.concurrent.ThreadSafe; @ExperimentalApi @ThreadSafe public final class NameResolverRegistry extends NameResolver.Factory { - private static final NameResolverRegistry defaultRegistry = new NameResolverRegistry(); + private static final NameResolverRegistry defaultRegistry = + new NameResolverRegistry(DnsNameResolverFactory.getInstance()); private final CopyOnWriteArrayList registry = new CopyOnWriteArrayList(); - private NameResolverRegistry() { - // To prevent instantiation - } - - static { - defaultRegistry.register(DnsNameResolverFactory.getInstance()); - } - public static NameResolverRegistry getDefaultRegistry() { return defaultRegistry; } + private final String defaultScheme; + + private NameResolverRegistry(NameResolver.Factory defaultResolverFactory) { + register(defaultResolverFactory); + defaultScheme = Preconditions.checkNotNull( + defaultResolverFactory.getDefaultScheme(), "defaultScheme"); + } + /** * Registers a {@link NameResolver.Factory}. */ @@ -83,4 +86,9 @@ public final class NameResolverRegistry extends NameResolver.Factory { } return null; } + + @Override + public String getDefaultScheme() { + return defaultScheme; + } } diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index a610d6cbe1..990e368b39 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -65,6 +65,7 @@ import javax.annotation.Nullable; */ public abstract class AbstractManagedChannelImplBuilder > extends ManagedChannelBuilder { + private static final String DIRECT_ADDRESS_SCHEME = "directaddress"; @Nullable private Executor executor; @@ -99,7 +100,7 @@ public abstract class AbstractManagedChannelImplBuilder } protected AbstractManagedChannelImplBuilder(SocketAddress directServerAddress, String authority) { - this.target = "directaddress:///" + directServerAddress; + this.target = DIRECT_ADDRESS_SCHEME + ":///" + directServerAddress; this.directServerAddress = directServerAddress; this.nameResolverFactory = new DirectAddressNameResolverFactory(directServerAddress, authority); } @@ -280,5 +281,10 @@ public abstract class AbstractManagedChannelImplBuilder public void shutdown() {} }; } + + @Override + public String getDefaultScheme() { + return DIRECT_ADDRESS_SCHEME; + } } } diff --git a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java index b473e56e05..ec6dbb94ef 100644 --- a/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java +++ b/core/src/main/java/io/grpc/internal/ManagedChannelImpl.java @@ -215,9 +215,10 @@ public final class ManagedChannelImpl extends ManagedChannel { // If we reached here, the targetUri couldn't be used. if (!URI_PATTERN.matcher(target).matches()) { - // It doesn't look like a URI target. Maybe it's a DNS name. + // It doesn't look like a URI target. Maybe it's an authority string. Try with the default + // scheme from the factory. try { - targetUri = new URI("dns", null, "/" + target, null); + targetUri = new URI(nameResolverFactory.getDefaultScheme(), null, "/" + target, null); } catch (URISyntaxException e) { // Should not be possible. throw new IllegalArgumentException(e); diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java index 7e741f409d..dbbf078f65 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java @@ -53,21 +53,18 @@ public class ManagedChannelImplGetNameResolverTest { @Test public void invalidUriTarget() { - testInvalidTarget("dns:///[invalid]"); + testInvalidTarget("defaultscheme:///[invalid]"); } @Test public void validTargetWithInvalidDnsName() throws Exception { - // "dns:///[invalid" is a valid URI target, it's just "[invalid" is not a valid DNS name. Such - // error will be handled by DnsNameResolver (tested in DnsNameResolverTest), but not in - // getNameResolver(). - testValidTarget("[invalid]", new URI("dns", null, "/[invalid]", null)); + testValidTarget("[valid]", new URI("defaultscheme", null, "/[valid]", null)); } @Test public void validAuthorityTarget() throws Exception { testValidTarget("foo.googleapis.com:8080", - new URI("dns", null, "/foo.googleapis.com:8080", null)); + new URI("defaultscheme", null, "/foo.googleapis.com:8080", null)); } @Test @@ -78,7 +75,7 @@ public class ManagedChannelImplGetNameResolverTest { @Test public void validIpv4AuthorityTarget() throws Exception { - testValidTarget("127.0.0.1:1234", new URI("dns", null, "/127.0.0.1:1234", null)); + testValidTarget("127.0.0.1:1234", new URI("defaultscheme", null, "/127.0.0.1:1234", null)); } @Test @@ -88,7 +85,7 @@ public class ManagedChannelImplGetNameResolverTest { @Test public void validIpv6AuthorityTarget() throws Exception { - testValidTarget("[::1]:1234", new URI("dns", null, "/[::1]:1234", null)); + testValidTarget("[::1]:1234", new URI("defaultscheme", null, "/[::1]:1234", null)); } @Test @@ -108,6 +105,11 @@ public class ManagedChannelImplGetNameResolverTest { public NameResolver newNameResolver(URI targetUri, Attributes params) { return null; } + + @Override + public String getDefaultScheme() { + return "defaultscheme"; + } }; try { ManagedChannelImpl.getNameResolver( @@ -152,6 +154,11 @@ public class ManagedChannelImplGetNameResolverTest { } return null; } + + @Override + public String getDefaultScheme() { + return expectedScheme; + } } private static class FakeNameResolver extends NameResolver { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java index ef3fc936a5..0a5e539265 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java @@ -434,6 +434,11 @@ public class ManagedChannelImplTest { return resolver; } + @Override + public String getDefaultScheme() { + return "fake"; + } + void allResolved() { for (FakeNameResolver resolver : resolvers) { resolver.resolved(); @@ -486,6 +491,11 @@ public class ManagedChannelImplTest { @Override public void shutdown() {} }; } + + @Override + public String getDefaultScheme() { + return "fake"; + } } private class SpyingLoadBalancerFactory extends LoadBalancer.Factory { diff --git a/core/src/test/java/io/grpc/internal/ManagedChannelImplTransportManagerTest.java b/core/src/test/java/io/grpc/internal/ManagedChannelImplTransportManagerTest.java index bc606e93b4..a597ce1b4d 100644 --- a/core/src/test/java/io/grpc/internal/ManagedChannelImplTransportManagerTest.java +++ b/core/src/test/java/io/grpc/internal/ManagedChannelImplTransportManagerTest.java @@ -98,6 +98,11 @@ public class ManagedChannelImplTransportManagerTest { } }; } + + @Override + public String getDefaultScheme() { + return "fake"; + } }; private final ExecutorService executor = Executors.newSingleThreadExecutor();