From 920f4b2badba4f2c99efbf929d7ebd5b6a5edb22 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 23 Feb 2026 13:22:34 -0800 Subject: [PATCH 1/2] grpclb: Rewrite URI() calls as URI#create in test cases. This change is a no-op. The create() form is clearer than positional arguments to a heavily overloaded constructor. --- .../grpc/grpclb/SecretGrpclbNameResolverProviderTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java index 24b1c781f58..8951d3154b9 100644 --- a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java +++ b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java @@ -74,22 +74,22 @@ public void newNameResolver() { @Test public void invalidDnsName() throws Exception { - testInvalidUri(new URI("dns", null, "/[invalid]", null)); + testInvalidUri(URI.create("dns:/%5Binvalid%5D")); } @Test public void validIpv6() throws Exception { - testValidUri(new URI("dns", null, "/[::1]", null)); + testValidUri(URI.create("dns:/%5B::1%5D")); } @Test public void validDnsNameWithoutPort() throws Exception { - testValidUri(new URI("dns", null, "/foo.googleapis.com", null)); + testValidUri(URI.create("dns:/foo.googleapis.com")); } @Test public void validDnsNameWithPort() throws Exception { - testValidUri(new URI("dns", null, "/foo.googleapis.com:456", null)); + testValidUri(URI.create("dns:/foo.googleapis.com:456")); } private void testInvalidUri(URI uri) { From 1b697376d6da6a52f0decd5e4f95c38f376a4304 Mon Sep 17 00:00:00 2001 From: John Cormie Date: Mon, 23 Feb 2026 14:18:54 -0800 Subject: [PATCH 2/2] grpclb: Implement newNameResolver(io.grpc.Uri). --- .../SecretGrpclbNameResolverProvider.java | 39 +++++++++++++---- .../SecretGrpclbNameResolverProviderTest.java | 43 +++++++++++++------ 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java b/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java index 8952ea1d8fb..0b46270af4d 100644 --- a/grpclb/src/main/java/io/grpc/grpclb/SecretGrpclbNameResolverProvider.java +++ b/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 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; diff --git a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java b/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java index 8951d3154b9..9720354c7f6 100644 --- a/grpclb/src/test/java/io/grpc/grpclb/SecretGrpclbNameResolverProviderTest.java +++ b/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 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(URI.create("dns:/%5Binvalid%5D")); + testInvalidUri("dns:/%5Binvalid%5D"); } @Test public void validIpv6() throws Exception { - testValidUri(URI.create("dns:/%5B::1%5D")); + testValidUri("dns:/%5B::1%5D"); } @Test public void validDnsNameWithoutPort() throws Exception { - testValidUri(URI.create("dns:/foo.googleapis.com")); + testValidUri("dns:/foo.googleapis.com"); } @Test public void validDnsNameWithPort() throws Exception { - testValidUri(URI.create("dns:/foo.googleapis.com:456")); + 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); + } }