Clarify specs for target strings, especially for IPv6.

See the javadocs of ManagedChannelBuilder.forTarget().

The most interesting case is passing an IPv6 address as target. It can
be either be passed as an authority, where brackets should not be
escaped ([::1]), or as a path of a full URI, where brackets must be
escaped (dns:///%5B::1%5D).

Previously, dns:///[::1], being an invalid URI (brackets not allowed in
path), would be converted to dns:////dns:///%5B::1%5D and passed to
DnsNameResolver. Though it would fail eventually, the error would be
very confusing to users. I changed the logic so that it would try with
dns:/// only if the target string doesn't look like an intended URI
target.

I have restricted the "URI target" to be absolute and hierarchical,
i.e., must start with scheme://. I couldn't find a way to better tell if
a string is intended to be a URI, but I am open to other options.

Refactored tests:

- Move the tests for getNameResolver() into a separate file
  ManagedChannelImplGetNameResolverTest, because those tests are not
  quite compatible with the facility provided by ManagedChannelImplTest.
- Create DnsNameResolverTest. Move DnsNameResolver out of the factory
  class to accommodate for the test.

core/src/main/java/io/grpc/DnsNameResolver.java

@@ -0,0 +1,122 @@
+/*
+ * Copyright 2015, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc;
+
+import com.google.common.base.Preconditions;
+
+import io.grpc.internal.GrpcUtil;
+import io.grpc.internal.SharedResourceHolder;
+
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.URI;
+import java.util.ArrayList;
+import java.util.concurrent.ExecutorService;
+
+import javax.annotation.Nullable;
+
+/**
+ * A DNS-based {@link NameResolver}.
+ *
+ * @see DnsNameResolverFactory
+ */
+final class DnsNameResolver extends NameResolver {
+  private final String authority;
+  private final String host;
+  private final int port;
+  private ExecutorService executor;
+
+  DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params) {
+    // TODO: if a DNS server is provided as nsAuthority, use it.
+    // https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
+
+    // Must prepend a "//" to the name when constructing a URI, otherwise it will be treated as an
+    // opaque URI, thus the authority and host of the resulted URI would be null.
+    URI nameUri = URI.create("//" + name);
+    authority = Preconditions.checkNotNull(nameUri.getAuthority(),
+        "nameUri (%s) doesn't have an authority", nameUri);
+    host = Preconditions.checkNotNull(nameUri.getHost(), "host");
+    if (nameUri.getPort() == -1) {
+      Integer defaultPort = params.get(NameResolver.Factory.PARAMS_DEFAULT_PORT);
+      if (defaultPort != null) {
+        port = defaultPort;
+      } else {
+        throw new IllegalArgumentException(
+            "name '" + name + "' doesn't contain a port, and default port is not set in params");
+      }
+    } else {
+      port = nameUri.getPort();
+    }
+  }
+
+  @Override
+  public String getServiceAuthority() {
+    return authority;
+  }
+
+  @Override
+  public synchronized void start(final Listener listener) {
+    Preconditions.checkState(executor == null, "already started");
+    executor = SharedResourceHolder.get(GrpcUtil.SHARED_CHANNEL_EXECUTOR);
+    executor.execute(new Runnable() {
+      @Override
+      public void run() {
+        InetAddress[] inetAddrs;
+        try {
+          inetAddrs = InetAddress.getAllByName(host);
+        } catch (Exception e) {
+          listener.onError(Status.UNAVAILABLE.withCause(e));
+          return;
+        }
+        ArrayList<ResolvedServerInfo> servers
+            = new ArrayList<ResolvedServerInfo>(inetAddrs.length);
+        for (int i = 0; i < inetAddrs.length; i++) {
+          InetAddress inetAddr = inetAddrs[i];
+          servers.add(
+              new ResolvedServerInfo(new InetSocketAddress(inetAddr, port), Attributes.EMPTY));
+        }
+        listener.onUpdate(servers, Attributes.EMPTY);
+      }
+    });
+  }
+
+  @Override
+  public synchronized void shutdown() {
+    if (executor != null) {
+      executor = SharedResourceHolder.release(GrpcUtil.SHARED_CHANNEL_EXECUTOR, executor);
+    }
+  }
+
+  int getPort() {
+    return port;
+  }
+}

core/src/main/java/io/grpc/DnsNameResolverFactory.java

@@ -33,19 +33,10 @@ package io.grpc;
 
 import com.google.common.base.Preconditions;
 
-import io.grpc.internal.GrpcUtil;
-import io.grpc.internal.SharedResourceHolder;
-
-import java.net.InetAddress;
-import java.net.InetSocketAddress;
 import java.net.URI;
-import java.util.ArrayList;
-import java.util.concurrent.ExecutorService;
-
-import javax.annotation.Nullable;
 
 /**
- * A factory for DNS-based {@link NameResolver}s.
+ * A factory for {@link DnsNameResolver}.
  *
  * <p>It resolves a target URI whose scheme is {@code "dns"}. The (optional) authority of the target
  * URI is reserved for the address of alternative DNS server (not implemented yet). The path of the
@@ -83,72 +74,4 @@ public final class DnsNameResolverFactory extends NameResolver.Factory {
   public static DnsNameResolverFactory getInstance() {
     return instance;
   }
-
-  private static class DnsNameResolver extends NameResolver {
-    private final String authority;
-    private final String host;
-    private final int port;
-    private ExecutorService executor;
-
-    DnsNameResolver(@Nullable String nsAuthority, String name, Attributes params) {
-      // TODO: if a DNS server is provided as nsAuthority, use it.
-      // https://www.captechconsulting.com/blogs/accessing-the-dusty-corners-of-dns-with-java
-
-      // Must prepend a "//" to the name when constructing a URI, otherwise
-      // the authority and host of the resulted URI would be null.
-      URI nameUri = URI.create("//" + name);
-      authority = Preconditions.checkNotNull(nameUri.getAuthority(),
-          "nameUri (%s) doesn't have an authority", nameUri);
-      host = Preconditions.checkNotNull(nameUri.getHost(), "host");
-      if (nameUri.getPort() == -1) {
-        Integer defaultPort = params.get(NameResolver.Factory.PARAMS_DEFAULT_PORT);
-        if (defaultPort != null) {
-          port = defaultPort;
-        } else {
-          throw new IllegalArgumentException(
-              "name '" + name + "' doesn't contain a port, and default port is not set in params");
-        }
-      } else {
-        port = nameUri.getPort();
-      }
-    }
-
-    @Override
-    public String getServiceAuthority() {
-      return authority;
-    }
-
-    @Override
-    public synchronized void start(final Listener listener) {
-      Preconditions.checkState(executor == null, "already started");
-      executor = SharedResourceHolder.get(GrpcUtil.SHARED_CHANNEL_EXECUTOR);
-      executor.execute(new Runnable() {
-        @Override
-        public void run() {
-          InetAddress[] inetAddrs;
-          try {
-            inetAddrs = InetAddress.getAllByName(host);
-          } catch (Exception e) {
-            listener.onError(Status.UNAVAILABLE.withCause(e));
-            return;
-          }
-          ArrayList<ResolvedServerInfo> servers
-              = new ArrayList<ResolvedServerInfo>(inetAddrs.length);
-          for (int i = 0; i < inetAddrs.length; i++) {
-            InetAddress inetAddr = inetAddrs[i];
-            servers.add(
-                new ResolvedServerInfo(new InetSocketAddress(inetAddr, port), Attributes.EMPTY));
-          }
-          listener.onUpdate(servers, Attributes.EMPTY);
-        }
-      });
-    }
-
-    @Override
-    public synchronized void shutdown() {
-      if (executor != null) {
-        executor = SharedResourceHolder.release(GrpcUtil.SHARED_CHANNEL_EXECUTOR, executor);
-      }
-    }
-  }
 }

core/src/main/java/io/grpc/ManagedChannelBuilder.java

@@ -46,19 +46,22 @@ public abstract class ManagedChannelBuilder<T extends ManagedChannelBuilder<T>>
 
   /**
    * Creates a channel with a target string, which can be either a valid {@link
-   * NameResolver}-compliant URI, or a HOST:PORT string.
+   * NameResolver}-compliant URI, or an authority string.
    *
-   * <p>Example {@code NameResolver}-compliant URIs:
+   * <p>A {@code NameResolver}-compliant URI is an aboslute hierarchical URI as defined by {@link
+   * java.net.URI}. Example URIs:
    * <ul>
    *   <li>{@code "dns:///foo.googleapis.com:8080"}</li>
    *   <li>{@code "dns:///foo.googleapis.com"}</li>
+   *   <li>{@code "dns:///%5B2001:db8:85a3:8d3:1319:8a2e:370:7348%5D:443"}</li>
    *   <li>{@code "dns://8.8.8.8/foo.googleapis.com:8080"}</li>
    *   <li>{@code "dns://8.8.8.8/foo.googleapis.com"}</li>
    *   <li>{@code "zookeeper://zk.example.com:9900/example_service"}</li>
    * </ul>
    *
-   * <p>Example HOST:PORT strings, which will be converted to {@code NameResolver}-compliant URIs by
+   * <p>An authority string will be converted to a {@code NameResolver}-compliant URI, which has
-   * prepending {@code "dns:///"}.
+   * {@code "dns"} as the scheme, no authority, and the original authority string as its path after
+   * properly escaped. Example authority strings:
    * <ul>
    *   <li>{@code "localhost"}</li>
    *   <li>{@code "127.0.0.1"}</li>

core/src/main/java/io/grpc/internal/ManagedChannelImpl.java

@@ -69,6 +69,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
+import java.util.regex.Pattern;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
@@ -82,6 +83,11 @@ public final class ManagedChannelImpl extends ManagedChannel {
   private static final ListenableFuture<ClientTransport> NULL_VALUE_TRANSPORT_FUTURE =
         Futures.immediateFuture(null);
 
+  // Matching this pattern means the target string is a URI target or at least intended to be one.
+  // A URI target must be an absolute hierarchical URI.
+  // From RFC 2396: scheme = alpha *( alpha | digit | "+" | "-" | "." )
+  private static final Pattern URI_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9+-.]*:/.*");
+
   private final ClientTransportFactory transportFactory;
   private final Executor executor;
   private final boolean usingSharedExecutor;
@@ -180,8 +186,7 @@ public final class ManagedChannelImpl extends ManagedChannel {
       // "localhost" is parsed as the scheme. Will fall into the next branch and try
       // "dns:///localhost:8080".
     } catch (URISyntaxException e) {
-      // Can happen with ip addresses like "[::1]:1234" or 127.0.0.1:1234.  Also can happen with
+      // Can happen with ip addresses like "[::1]:1234" or 127.0.0.1:1234.
-      // bogus urls like "dns:///[::1]:1234", which are not properly uriencoded.
       uriSyntaxErrors.append(e.getMessage());
     }
     if (targetUri != null) {
@@ -193,7 +198,9 @@ public final class ManagedChannelImpl extends ManagedChannel {
       // unmapped scheme. Just fall through and will try "dns:///foo.googleapis.com:8080"
     }
 
-    // If we reached here, the targetUri couldn't be used, so try again.
+    // 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.
       try {
         targetUri = new URI("dns", null, "/" + target, null);
       } catch (URISyntaxException e) {
@@ -206,6 +213,7 @@ public final class ManagedChannelImpl extends ManagedChannel {
           return resolver;
         }
       }
+    }
     throw new IllegalArgumentException(String.format(
         "cannot find a NameResolver for %s%s",
         target, uriSyntaxErrors.length() > 0 ? " (" + uriSyntaxErrors.toString() + ")" : ""));

core/src/test/java/io/grpc/DnsNameResolverTest.java

@@ -0,0 +1,91 @@
+/*
+ * Copyright 2015, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.net.URI;
+
+/** Unit tests for {@link DnsNameResolver}. */
+@RunWith(JUnit4.class)
+public class DnsNameResolverTest {
+
+  private DnsNameResolverFactory factory = DnsNameResolverFactory.getInstance();
+
+  private static final int DEFAULT_PORT = 887;
+  private static final Attributes NAME_RESOLVER_PARAMS =
+      Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, DEFAULT_PORT).build();
+
+  @Test
+  public void invalidDnsName() throws Exception {
+    testInvalidUri(new URI("dns", null, "/[invalid]", null));
+  }
+
+  @Test
+  public void validIpv6() throws Exception {
+    testValidUri(new URI("dns", null, "/[::1]", null), "[::1]", DEFAULT_PORT);
+  }
+
+  @Test
+  public void validDnsNameWithoutPort() throws Exception {
+    testValidUri(new URI("dns", null, "/foo.googleapis.com", null),
+        "foo.googleapis.com", DEFAULT_PORT);
+  }
+
+  @Test
+  public void validDnsNameWithPort() throws Exception {
+    testValidUri(new URI("dns", null, "/foo.googleapis.com:456", null),
+        "foo.googleapis.com:456", 456);
+  }
+
+  private void testInvalidUri(URI uri) {
+    try {
+      factory.newNameResolver(uri, NAME_RESOLVER_PARAMS);
+      fail("Should have failed");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
+  private void testValidUri(URI uri, String exportedAuthority, int expectedPort) {
+    DnsNameResolver resolver = (DnsNameResolver) factory.newNameResolver(uri, NAME_RESOLVER_PARAMS);
+    assertNotNull(resolver);
+    assertEquals(expectedPort, resolver.getPort());
+    assertEquals(exportedAuthority, resolver.getServiceAuthority());
+  }
+}

core/src/test/java/io/grpc/internal/ManagedChannelImplGetNameResolverTest.java

@@ -0,0 +1,172 @@
+/*
+ * Copyright 2015, Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *
+ *    * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package io.grpc.internal;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.fail;
+
+import io.grpc.Attributes;
+import io.grpc.NameResolver;
+import io.grpc.NameResolver.Factory;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+import java.net.URI;
+
+/** Unit tests for {@link ManagedChannelImpl#getNameResolver}. */
+@RunWith(JUnit4.class)
+public class ManagedChannelImplGetNameResolverTest {
+  private static final Attributes NAME_RESOLVER_PARAMS =
+      Attributes.newBuilder().set(NameResolver.Factory.PARAMS_DEFAULT_PORT, 447).build();
+
+  @Test
+  public void invalidUriTarget() {
+    testInvalidTarget("dns:///[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));
+  }
+
+  @Test
+  public void validAuthorityTarget() throws Exception {
+    testValidTarget("foo.googleapis.com:8080",
+        new URI("dns", null, "/foo.googleapis.com:8080", null));
+  }
+
+  @Test
+  public void validUriTarget() throws Exception {
+    testValidTarget("scheme:///foo.googleapis.com:8080",
+        new URI("scheme", null, "/foo.googleapis.com:8080", null));
+  }
+
+  @Test
+  public void validIpv4AuthorityTarget() throws Exception {
+    testValidTarget("127.0.0.1:1234", new URI("dns", null, "/127.0.0.1:1234", null));
+  }
+
+  @Test
+  public void validIpv4UriTarget() throws Exception {
+    testValidTarget("dns:///127.0.0.1:1234", new URI("dns", null, "/127.0.0.1:1234", null));
+  }
+
+  @Test
+  public void validIpv6AuthorityTarget() throws Exception {
+    testValidTarget("[::1]:1234", new URI("dns", null, "/[::1]:1234", null));
+  }
+
+  @Test
+  public void invalidIpv6UriTarget() throws Exception {
+    testInvalidTarget("dns:///[::1]:1234");
+  }
+
+  @Test
+  public void validIpv6UriTarget() throws Exception {
+    testValidTarget("dns:///%5B::1%5D:1234", new URI("dns", null, "/[::1]:1234", null));
+  }
+
+  @Test
+  public void validTargetNoResovler() {
+    Factory nameResolverFactory = new NameResolver.Factory() {
+      @Override
+      public NameResolver newNameResolver(URI targetUri, Attributes params) {
+        return null;
+      }
+    };
+    try {
+      ManagedChannelImpl.getNameResolver(
+          "foo.googleapis.com:8080", nameResolverFactory, NAME_RESOLVER_PARAMS);
+      fail("Should fail");
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
+  private void testValidTarget(String target, URI expectedUri) {
+    Factory nameResolverFactory = new FakeNameResolverFactory(expectedUri.getScheme());
+    FakeNameResolver nameResolver = (FakeNameResolver) ManagedChannelImpl.getNameResolver(
+        target, nameResolverFactory, NAME_RESOLVER_PARAMS);
+    assertNotNull(nameResolver);
+    assertEquals(expectedUri, nameResolver.uri);
+  }
+
+  private void testInvalidTarget(String target) {
+    Factory nameResolverFactory = new FakeNameResolverFactory("dns");
+
+    try {
+      FakeNameResolver nameResolver = (FakeNameResolver) ManagedChannelImpl.getNameResolver(
+          target, nameResolverFactory, NAME_RESOLVER_PARAMS);
+      fail("Should have failed, but got resolver with " + nameResolver.uri);
+    } catch (IllegalArgumentException e) {
+      // expected
+    }
+  }
+
+  private static class FakeNameResolverFactory extends NameResolver.Factory {
+    final String expectedScheme;
+
+    FakeNameResolverFactory(String expectedScheme) {
+      this.expectedScheme = expectedScheme;
+    }
+
+    @Override
+    public NameResolver newNameResolver(URI targetUri, Attributes params) {
+      if (expectedScheme.equals(targetUri.getScheme())) {
+        return new FakeNameResolver(targetUri);
+      }
+      return null;
+    }
+  }
+
+  private static class FakeNameResolver extends NameResolver {
+    final URI uri;
+
+    FakeNameResolver(URI uri) {
+      this.uri = uri;
+    }
+
+    @Override public String getServiceAuthority() {
+      return uri.getAuthority();
+    }
+
+    @Override public void start(final Listener listener) {}
+
+    @Override public void shutdown() {}
+  }
+}

core/src/test/java/io/grpc/internal/ManagedChannelImplTest.java

@@ -59,7 +59,6 @@ import io.grpc.ManagedChannel;
 import io.grpc.Metadata;
 import io.grpc.MethodDescriptor;
 import io.grpc.NameResolver;
-import io.grpc.NameResolver.Factory;
 import io.grpc.ResolvedServerInfo;
 import io.grpc.SimpleLoadBalancerFactory;
 import io.grpc.Status;
@@ -80,7 +79,6 @@ import org.mockito.stubbing.Answer;
 
 import java.net.SocketAddress;
 import java.net.URI;
-import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -149,7 +147,7 @@ public class ManagedChannelImplTest {
   @Test
   public void immediateDeadlineExceeded() {
     ManagedChannel channel = createChannel(
-        new FakeNameResolverFactory(server, expectedUri, true), NO_INTERCEPTOR);
+        new FakeNameResolverFactory(true), NO_INTERCEPTOR);
     ClientCall<String, Integer> call =
         channel.newCall(method, CallOptions.DEFAULT.withDeadlineNanoTime(System.nanoTime()));
     call.start(mockCallListener, new Metadata());
@@ -160,7 +158,7 @@ public class ManagedChannelImplTest {
   @Test
   public void shutdownWithNoTransportsEverCreated() {
     ManagedChannel channel = createChannel(
-        new FakeNameResolverFactory(server, expectedUri, true), NO_INTERCEPTOR);
+        new FakeNameResolverFactory(true), NO_INTERCEPTOR);
     verifyNoMoreInteractions(mockTransportFactory);
     channel.shutdown();
     assertTrue(channel.isShutdown());
@@ -170,7 +168,7 @@ public class ManagedChannelImplTest {
   @Test
   public void twoCallsAndGracefulShutdown() {
     ManagedChannel channel = createChannel(
-        new FakeNameResolverFactory(server, expectedUri, true), NO_INTERCEPTOR);
+        new FakeNameResolverFactory(true), NO_INTERCEPTOR);
     verifyNoMoreInteractions(mockTransportFactory);
     ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
     verifyNoMoreInteractions(mockTransportFactory);
@@ -237,98 +235,6 @@ public class ManagedChannelImplTest {
     verifyNoMoreInteractions(mockStream);
   }
 
-  @Test
-  public void getNameResolver_invalidUriWithoutScheme() {
-    Factory nameResolverFactory =  new FakeNameResolverFactory(server, expectedUri, true);
-    thrown.expect(IllegalArgumentException.class);
-
-    ManagedChannelImpl.getNameResolver("[invalid", nameResolverFactory, Attributes.EMPTY);
-  }
-
-  @Test
-  public void getNameResolver_invalidUriWithScheme() {
-    Factory nameResolverFactory =  new FakeNameResolverFactory(server, expectedUri, true);
-    thrown.expect(IllegalArgumentException.class);
-
-    ManagedChannelImpl.getNameResolver("scheme://[invalid", nameResolverFactory, Attributes.EMPTY);
-  }
-
-  @Test
-  public void getNameResolver_validHost() {
-    Factory nameResolverFactory = new FakeNameResolverFactory(server, expectedUri, true);
-
-    NameResolver res = ManagedChannelImpl.getNameResolver(
-        target, nameResolverFactory, NAME_RESOLVER_PARAMS);
-
-    assertEquals(serviceName, res.getServiceAuthority());
-  }
-
-  @Test
-  public void getNameResolver_validHostWithoutSchema() throws URISyntaxException {
-    expectedUri = new URI("dns:///foo.googleapis.com:8080");
-    Factory nameResolverFactory = new NameResolver.Factory() {
-      @Override
-      public NameResolver newNameResolver(URI targetUri, Attributes params) {
-        if (targetUri.equals(expectedUri)) {
-          NameResolver resolver = mock(NameResolver.class);
-          when(resolver.getServiceAuthority()).thenReturn("foo.googleapis.com:8080");
-          return resolver;
-        }
-        return null;
-      }
-    };
-
-    NameResolver res = ManagedChannelImpl.getNameResolver(
-        "foo.googleapis.com:8080", nameResolverFactory, NAME_RESOLVER_PARAMS);
-
-    assertEquals("foo.googleapis.com:8080", res.getServiceAuthority());
-  }
-
-  @Test
-  public void getNameResolver_validIpHostWithoutSchema() {
-    Factory nameResolverFactory = new NameResolver.Factory() {
-      @Override
-      public NameResolver newNameResolver(URI targetUri, Attributes params) {
-        NameResolver resolver = mock(NameResolver.class);
-        when(resolver.getServiceAuthority()).thenReturn("127.0.0.1:8080");
-        return resolver;
-      }
-    };
-
-    NameResolver res = ManagedChannelImpl.getNameResolver(
-        "127.0.0.1:8080", nameResolverFactory, NAME_RESOLVER_PARAMS);
-
-    assertEquals("127.0.0.1:8080", res.getServiceAuthority());
-  }
-
-  @Test
-  public void getNameResolver_validTargetNoResovler() {
-    Factory nameResolverFactory = new NameResolver.Factory() {
-      @Override
-      public NameResolver newNameResolver(URI targetUri, Attributes params) {
-        return null;
-      }
-    };
-    thrown.expect(IllegalArgumentException.class);
-
-    ManagedChannelImpl.getNameResolver(target, nameResolverFactory, NAME_RESOLVER_PARAMS);
-  }
-
-  @Test
-  public void getNameResolver_validTargetDnsResovler() {
-    Factory nameResolverFactory = new NameResolver.Factory() {
-      @Override
-      public NameResolver newNameResolver(URI targetUri, Attributes params) {
-        if (targetUri.getScheme().equals("dns")) {
-          return mock(NameResolver.class);
-        }
-        return null;
-      }
-    };
-
-    ManagedChannelImpl.getNameResolver("[::1]:1234", nameResolverFactory, NAME_RESOLVER_PARAMS);
-  }
-
   @Test
   public void interceptor() throws Exception {
     final AtomicLong atomic = new AtomicLong();
@@ -342,7 +248,7 @@ public class ManagedChannelImplTest {
       }
     };
     ManagedChannel channel = createChannel(
-        new FakeNameResolverFactory(server, expectedUri, true), Arrays.asList(interceptor));
+        new FakeNameResolverFactory(true), Arrays.asList(interceptor));
     assertNotNull(channel.newCall(method, CallOptions.DEFAULT));
     assertEquals(1, atomic.get());
   }
@@ -350,7 +256,7 @@ public class ManagedChannelImplTest {
   @Test
   public void testNoDeadlockOnShutdown() {
     ManagedChannel channel = createChannel(
-        new FakeNameResolverFactory(server, expectedUri, true), NO_INTERCEPTOR);
+        new FakeNameResolverFactory(true), NO_INTERCEPTOR);
     // Force creation of transport
     ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
     Metadata headers = new Metadata();
@@ -415,7 +321,7 @@ public class ManagedChannelImplTest {
   @Test
   public void nameResolvedAfterChannelShutdown() {
     FakeNameResolverFactory nameResolverFactory =
-        new FakeNameResolverFactory(server, expectedUri, false);
+        new FakeNameResolverFactory(false);
     ManagedChannel channel = createChannel(nameResolverFactory, NO_INTERCEPTOR);
     ClientCall<String, Integer> call = channel.newCall(method, CallOptions.DEFAULT);
     Metadata headers = new Metadata();
@@ -442,16 +348,12 @@ public class ManagedChannelImplTest {
     }
   }
 
-  private static class FakeNameResolverFactory extends NameResolver.Factory {
+  private class FakeNameResolverFactory extends NameResolver.Factory {
-    final ResolvedServerInfo server;
     final boolean resolvedAtStart;
     final ArrayList<FakeNameResolver> resolvers = new ArrayList<FakeNameResolver>();
-    final URI expectedUri;
 
-    FakeNameResolverFactory(ResolvedServerInfo server, URI expectedUri, boolean resolvedAtStart) {
+    FakeNameResolverFactory(boolean resolvedAtStart) {
-      this.server = server;
       this.resolvedAtStart = resolvedAtStart;
-      this.expectedUri = expectedUri;
     }
 
     @Override