grpclb: Add RFC 3986 URI support

grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java

@@ -19,14 +19,17 @@
 import com.google.common.base.Preconditions;
 import com.google.common.base.Stopwatch;
 import io.grpc.InternalServiceProviders;
+import io.grpc.NameResolver;
 import io.grpc.NameResolver.Args;
 import io.grpc.NameResolverProvider;
+import io.grpc.Uri;
 import io.grpc.internal.GrpcUtil;
 import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.net.URI;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.List;
 
 /**
  * A provider for {@code io.grpc.grpclb.GrpclbNameResolver}.
@@ -56,27 +59,47 @@ public static final class Provider extends NameResolverProvider {
     private static final boolean IS_ANDROID = InternalServiceProviders
         .isAndroid(SecretGrpclbNameResolverProvider.class.getClassLoader());
 
+    @Override
+    public NameResolver newNameResolver(Uri targetUri, final NameResolver.Args args) {
+      if (SCHEME.equals(targetUri.getScheme())) {
+        List<String> pathSegments = targetUri.getPathSegments();
+        Preconditions.checkArgument(
+            !pathSegments.isEmpty(),
+            "expected 1 path segment in target %s but found %s",
+            targetUri,
+            pathSegments);
+        return newNameResolver(targetUri.getAuthority(), pathSegments.get(0), args);
+      } else {
+        return null;
+      }
+    }
+
     @Override
     public GrpclbNameResolver newNameResolver(URI targetUri, Args args) {
+      // TODO(jdcormie): Remove once RFC 3986 migration is complete.
       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);
-        String name = targetPath.substring(1);
-        return new GrpclbNameResolver(
-            targetUri.getAuthority(),
-            name,
-            args,
-            GrpcUtil.SHARED_CHANNEL_EXECUTOR,
-            Stopwatch.createUnstarted(),
-            IS_ANDROID);
+        return newNameResolver(targetUri.getAuthority(), targetPath.substring(1), args);
       } else {
         return null;
       }
     }
 
+    private GrpclbNameResolver newNameResolver(
+        String authority, String domainNameToResolve, final NameResolver.Args args) {
+      return new GrpclbNameResolver(
+          authority,
+          domainNameToResolve,
+          args,
+          GrpcUtil.SHARED_CHANNEL_EXECUTOR,
+          Stopwatch.createUnstarted(),
+          IS_ANDROID);
+    }
+
     @Override
     public String getDefaultScheme() {
       return SCHEME;

grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java

@@ -24,15 +24,19 @@
 import io.grpc.NameResolver;
 import io.grpc.NameResolver.ServiceConfigParser;
 import io.grpc.SynchronizationContext;
+import io.grpc.Uri;
 import io.grpc.internal.DnsNameResolverProvider;
 import io.grpc.internal.GrpcUtil;
 import java.net.URI;
+import java.util.Arrays;
 import org.junit.Test;
 import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+import org.junit.runners.Parameterized.Parameters;
 
 /** Unit tests for {@link SecretGrpclbNameResolverProvider}. */
-@RunWith(JUnit4.class)
+@RunWith(Parameterized.class)
 public class SecretGrpclbNameResolverProviderTest {
 
   private final SynchronizationContext syncContext = new SynchronizationContext(
@@ -53,6 +57,13 @@ public void uncaughtException(Thread t, Throwable e) {
   private SecretGrpclbNameResolverProvider.Provider provider =
       new SecretGrpclbNameResolverProvider.Provider();
 
+  @Parameters(name = "enableRfc3986UrisParam={0}")
+  public static Iterable<Object[]> data() {
+    return Arrays.asList(new Object[][] {{true}, {false}});
+  }
+
+  @Parameter public boolean enableRfc3986UrisParam;
+
   @Test
   public void isAvailable() {
     assertThat(provider.isAvailable()).isTrue();
@@ -66,43 +77,49 @@ public void priority_shouldBeHigherThanDefaultDnsNameResolver() {
   }
 
   @Test
-  public void newNameResolver() {
-    assertThat(provider.newNameResolver(URI.create("dns:///localhost:443"), args))
+  public void newNameResolverReturnsCorrectType() {
+    assertThat(newNameResolver("dns:///localhost:443", args))
         .isInstanceOf(GrpclbNameResolver.class);
-    assertThat(provider.newNameResolver(URI.create("notdns:///localhost:443"), args)).isNull();
+    assertThat(newNameResolver("notdns:///localhost:443", args)).isNull();
   }
 
   @Test
   public void invalidDnsName() throws Exception {
-    testInvalidUri(new URI("dns", null, "/[invalid]", null));
+    testInvalidUri("dns:/%5Binvalid%5D");
   }
 
   @Test
   public void validIpv6() throws Exception {
-    testValidUri(new URI("dns", null, "/[::1]", null));
+    testValidUri("dns:/%5B::1%5D");
   }
 
   @Test
   public void validDnsNameWithoutPort() throws Exception {
-    testValidUri(new URI("dns", null, "/foo.googleapis.com", null));
+    testValidUri("dns:/foo.googleapis.com");
   }
 
   @Test
   public void validDnsNameWithPort() throws Exception {
-    testValidUri(new URI("dns", null, "/foo.googleapis.com:456", null));
+    testValidUri("dns:/foo.googleapis.com:456");
   }
 
-  private void testInvalidUri(URI uri) {
+  private void testInvalidUri(String uri) {
     try {
-      provider.newNameResolver(uri, args);
+      newNameResolver(uri, args);
       fail("Should have failed");
     } catch (IllegalArgumentException e) {
       // expected
     }
   }
 
-  private void testValidUri(URI uri) {
-    GrpclbNameResolver resolver = provider.newNameResolver(uri, args);
+  private void testValidUri(String uri) {
+    NameResolver resolver = newNameResolver(uri, args);
     assertThat(resolver).isNotNull();
   }
+
+  private NameResolver newNameResolver(String uriString, NameResolver.Args args) {
+    return enableRfc3986UrisParam
+        ? provider.newNameResolver(Uri.create(uriString), args)
+        : provider.newNameResolver(URI.create(uriString), args);
+  }
 }