diff --git a/api/src/main/java/io/grpc/NameResolverProvider.java b/api/src/main/java/io/grpc/NameResolverProvider.java index 5decd7e9150..2c337cd5052 100644 --- a/api/src/main/java/io/grpc/NameResolverProvider.java +++ b/api/src/main/java/io/grpc/NameResolverProvider.java @@ -16,6 +16,8 @@ package io.grpc; +import io.grpc.NameResolver.Factory; + /** * Provider of name resolvers for name agnostic consumption. * @@ -48,4 +50,16 @@ public abstract class NameResolverProvider extends NameResolver.Factory { * @since 1.0.0 */ protected abstract int priority(); + + /** + * Returns the scheme associated with the provider. The provider normally should only create a + * {@link NameResolver} when target URI scheme matches the provider scheme. It temporarily + * delegates to {@link Factory#getDefaultScheme()} before {@link NameResolver.Factory} is + * deprecated in https://github.com/grpc/grpc-java/issues/7133. + * + * @since 1.40.0 + * */ + protected String getScheme() { + return getDefaultScheme(); + } } diff --git a/api/src/main/java/io/grpc/NameResolverRegistry.java b/api/src/main/java/io/grpc/NameResolverRegistry.java index 2e914e2fbc1..2e12bb77483 100644 --- a/api/src/main/java/io/grpc/NameResolverRegistry.java +++ b/api/src/main/java/io/grpc/NameResolverRegistry.java @@ -19,12 +19,14 @@ import static com.google.common.base.Preconditions.checkArgument; import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableMap; import java.net.URI; import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; +import java.util.HashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; @@ -44,12 +46,17 @@ public final class NameResolverRegistry { private static NameResolverRegistry instance; private final NameResolver.Factory factory = new NameResolverFactory(); + private static final String UNKNOWN_SCHEME = "unknown"; + @GuardedBy("this") + private String defaultScheme = UNKNOWN_SCHEME; @GuardedBy("this") private final LinkedHashSet allProviders = new LinkedHashSet<>(); - /** Immutable, sorted version of {@code allProviders}. Is replaced instead of mutating. */ + /** Generated from {@code allProviders}. Is mapping from scheme key to the highest priority + * {@link NameResolverProvider}. Is replaced instead of mutating. */ @GuardedBy("this") - private List effectiveProviders = Collections.emptyList(); + private ImmutableMap effectiveProviders = ImmutableMap.of(); + /** * Register a provider. @@ -81,16 +88,23 @@ public synchronized void deregister(NameResolverProvider provider) { } private synchronized void refreshProviders() { - List providers = new ArrayList<>(allProviders); - // Sort descending based on priority. - // sort() must be stable, as we prefer first-registered providers - Collections.sort(providers, Collections.reverseOrder(new Comparator() { - @Override - public int compare(NameResolverProvider o1, NameResolverProvider o2) { - return o1.priority() - o2.priority(); + Map refreshedProviders = new HashMap<>(); + int maxPriority = Integer.MIN_VALUE; + String refreshedDefaultScheme = UNKNOWN_SCHEME; + // We prefer first-registered providers + for (NameResolverProvider provider : allProviders) { + String scheme = provider.getScheme(); + NameResolverProvider existing = refreshedProviders.get(scheme); + if (existing == null || existing.priority() < provider.priority()) { + refreshedProviders.put(scheme, provider); + } + if (maxPriority < provider.priority()) { + maxPriority = provider.priority(); + refreshedDefaultScheme = provider.getScheme(); } - })); - effectiveProviders = Collections.unmodifiableList(providers); + } + effectiveProviders = ImmutableMap.copyOf(refreshedProviders); + defaultScheme = refreshedDefaultScheme; } /** @@ -120,10 +134,11 @@ public static synchronized NameResolverRegistry getDefaultRegistry() { } /** - * Returns effective providers, in priority order. + * Returns effective providers map from scheme to the highest priority NameResolverProvider of + * that scheme. */ @VisibleForTesting - synchronized List providers() { + synchronized Map providers() { return effectiveProviders; } @@ -149,23 +164,15 @@ private final class NameResolverFactory extends NameResolver.Factory { @Override @Nullable public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) { - List providers = providers(); - for (NameResolverProvider provider : providers) { - NameResolver resolver = provider.newNameResolver(targetUri, args); - if (resolver != null) { - return resolver; - } - } - return null; + NameResolverProvider provider = providers().get(targetUri.getScheme()); + return provider == null ? null : provider.newNameResolver(targetUri, args); } @Override public String getDefaultScheme() { - List providers = providers(); - if (providers.isEmpty()) { - return "unknown"; + synchronized (NameResolverRegistry.this) { + return defaultScheme; } - return providers.get(0).getDefaultScheme(); } } diff --git a/api/src/test/java/io/grpc/NameResolverRegistryTest.java b/api/src/test/java/io/grpc/NameResolverRegistryTest.java index 6002dcdaf57..55e43128109 100644 --- a/api/src/test/java/io/grpc/NameResolverRegistryTest.java +++ b/api/src/test/java/io/grpc/NameResolverRegistryTest.java @@ -25,6 +25,7 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.net.URI; import java.util.List; +import java.util.Map; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -59,12 +60,16 @@ public void deregister() { NameResolverProvider p1 = new BaseProvider(true, 5); NameResolverProvider p2 = new BaseProvider(true, 5); NameResolverProvider p3 = new BaseProvider(true, 5); + String sameScheme = p1.getDefaultScheme(); reg.register(p1); reg.register(p2); reg.register(p3); - assertThat(reg.providers()).containsExactly(p1, p2, p3).inOrder(); + assertThat(reg.providers().get(sameScheme)).isSameInstanceAs(p1); reg.deregister(p2); - assertThat(reg.providers()).containsExactly(p1, p3).inOrder(); + assertThat(reg.providers().get(sameScheme)).isSameInstanceAs(p1); + reg.deregister(p1); + assertThat(reg.providers().get(sameScheme)).isSameInstanceAs(p3); + } @Test @@ -75,12 +80,13 @@ public void provider_sorted() { NameResolverProvider p3 = new BaseProvider(true, 8); NameResolverProvider p4 = new BaseProvider(true, 3); NameResolverProvider p5 = new BaseProvider(true, 8); + String sameScheme = p1.getDefaultScheme(); reg.register(p1); reg.register(p2); reg.register(p3); reg.register(p4); reg.register(p5); - assertThat(reg.providers()).containsExactly(p3, p5, p1, p2, p4).inOrder(); + assertThat(reg.providers().get(sameScheme)).isSameInstanceAs(p3); } @Test @@ -93,7 +99,7 @@ public void getDefaultScheme_noProvider() { public void newNameResolver_providerReturnsNull() { NameResolverRegistry registry = new NameResolverRegistry(); registry.register( - new BaseProvider(true, 5) { + new BaseProvider(true, 5, "noScheme") { @Override public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) { assertThat(passedUri).isSameInstanceAs(uri); @@ -102,12 +108,13 @@ public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) } }); assertThat(registry.asFactory().newNameResolver(uri, args)).isNull(); + assertThat(registry.asFactory().getDefaultScheme()).isEqualTo("noScheme"); } @Test public void newNameResolver_providerReturnsNonNull() { NameResolverRegistry registry = new NameResolverRegistry(); - registry.register(new BaseProvider(true, 5) { + registry.register(new BaseProvider(true, 5, uri.getScheme()) { @Override public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) { return null; @@ -127,38 +134,75 @@ public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) } }; registry.register( - new BaseProvider(true, 4) { + new BaseProvider(true, 4, uri.getScheme()) { @Override public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) { return nr; } }); registry.register( - new BaseProvider(true, 3) { + new BaseProvider(true, 3, uri.getScheme()) { @Override public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) { fail("Should not be called"); throw new AssertionError(); } }); - assertThat(registry.asFactory().newNameResolver(uri, args)).isSameInstanceAs(nr); + assertThat(registry.asFactory().newNameResolver(uri, args)).isNull(); + assertThat(registry.asFactory().getDefaultScheme()).isEqualTo(uri.getScheme()); + } + + @Test + public void newNameResolver_multipleScheme() { + NameResolverRegistry registry = new NameResolverRegistry(); + registry.register(new BaseProvider(true, 5, uri.getScheme()) { + @Override + public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) { + return null; + } + }); + final NameResolver nr = new NameResolver() { + @Override public String getServiceAuthority() { + throw new UnsupportedOperationException(); + } + + @Override public void start(Listener2 listener) { + throw new UnsupportedOperationException(); + } + + @Override public void shutdown() { + throw new UnsupportedOperationException(); + } + }; + registry.register( + new BaseProvider(true, 4, "other") { + @Override + public NameResolver newNameResolver(URI passedUri, NameResolver.Args passedArgs) { + return nr; + } + }); + + assertThat(registry.asFactory().newNameResolver(uri, args)).isNull(); + assertThat(registry.asFactory().newNameResolver(URI.create("other:///0.0.0.0:80"), args)) + .isSameInstanceAs(nr); + assertThat(registry.asFactory().getDefaultScheme()).isEqualTo("dns"); } @Test public void newNameResolver_noProvider() { NameResolver.Factory factory = new NameResolverRegistry().asFactory(); assertThat(factory.newNameResolver(uri, args)).isNull(); + assertThat(factory.getDefaultScheme()).isEqualTo("unknown"); } @Test public void baseProviders() { - List providers = NameResolverRegistry.getDefaultRegistry().providers(); - assertThat(providers).hasSize(2); - // 2 name resolvers from grpclb and core, ordered with decreasing priorities. - assertThat(providers.get(0).getClass().getName()) + Map providers = + NameResolverRegistry.getDefaultRegistry().providers(); + assertThat(providers).hasSize(1); + // 2 name resolvers from grpclb and core, higher priority one is returned. + assertThat(providers.get("dns").getClass().getName()) .isEqualTo("io.grpc.grpclb.SecretGrpclbNameResolverProvider$Provider"); - assertThat(providers.get(1).getClass().getName()) - .isEqualTo("io.grpc.internal.DnsNameResolverProvider"); assertThat(NameResolverRegistry.getDefaultRegistry().asFactory().getDefaultScheme()) .isEqualTo("dns"); } @@ -184,10 +228,18 @@ NameResolverProvider.class, getClass().getClassLoader())) { private static class BaseProvider extends NameResolverProvider { private final boolean isAvailable; private final int priority; + private final String scheme; public BaseProvider(boolean isAvailable, int priority) { this.isAvailable = isAvailable; this.priority = priority; + this.scheme = null; + } + + public BaseProvider(boolean isAvailable, int priority, String scheme) { + this.isAvailable = isAvailable; + this.priority = priority; + this.scheme = scheme; } @Override @@ -207,7 +259,7 @@ public NameResolver newNameResolver(URI targetUri, NameResolver.Args args) { @Override public String getDefaultScheme() { - return "scheme" + getClass().getSimpleName(); + return scheme == null ? "scheme" + getClass().getSimpleName() : scheme; } } } diff --git a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java index 321d5503cc3..c71841b2678 100644 --- a/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java +++ b/xds/src/test/java/io/grpc/xds/XdsSdsClientServerTest.java @@ -88,8 +88,11 @@ public class XdsSdsClientServerTest { private TlsContextManagerImpl tlsContextManagerForServer; @Before - public void setUp() throws IOException { + public void setUp() throws Exception { port = XdsServerTestHelper.findFreePort(); + URI expectedUri = new URI("sdstest://localhost:" + port); + fakeNameResolverFactory = new FakeNameResolverFactory.Builder(expectedUri).build(); + NameResolverRegistry.getDefaultRegistry().register(fakeNameResolverFactory); } @After @@ -410,9 +413,6 @@ static EnvoyServerProtoData.Listener buildListener( private SimpleServiceGrpc.SimpleServiceBlockingStub getBlockingStub( final UpstreamTlsContext upstreamTlsContext, String overrideAuthority) throws URISyntaxException { - URI expectedUri = new URI("sdstest://localhost:" + port); - fakeNameResolverFactory = new FakeNameResolverFactory.Builder(expectedUri).build(); - NameResolverRegistry.getDefaultRegistry().register(fakeNameResolverFactory); ManagedChannelBuilder channelBuilder = Grpc.newChannelBuilder( "sdstest://localhost:" + port,