From f4d48fec62cf4e59e91cb83db34fbb32edf48f3b Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 22 Apr 2019 10:22:46 -0700 Subject: [PATCH] core: Add NameResolverRegistry NameResolverRegistry takes on all the logic previously in NameResolverProvider. But it also allows manual registration of NameResolvers, which is useful when the providers have complex construction or need objects injected into them. This also avoids a circular dependency during class loading since previously loading any Provider searched for all Providers via ClassLoader since ClassLoader handling was static within the parent class. Fixes #5562 --- .../java/io/grpc/NameResolverProvider.java | 98 +------- .../java/io/grpc/NameResolverRegistry.java | 184 +++++++++++++++ .../io/grpc/NameResolverProviderTest.java | 163 -------------- .../io/grpc/NameResolverRegistryTest.java | 209 ++++++++++++++++++ .../AbstractManagedChannelImplBuilder.java | 4 +- 5 files changed, 401 insertions(+), 257 deletions(-) create mode 100644 api/src/main/java/io/grpc/NameResolverRegistry.java delete mode 100644 api/src/test/java/io/grpc/NameResolverProviderTest.java create mode 100644 api/src/test/java/io/grpc/NameResolverRegistryTest.java diff --git a/api/src/main/java/io/grpc/NameResolverProvider.java b/api/src/main/java/io/grpc/NameResolverProvider.java index 0347a174aa..8875092b04 100644 --- a/api/src/main/java/io/grpc/NameResolverProvider.java +++ b/api/src/main/java/io/grpc/NameResolverProvider.java @@ -16,14 +16,7 @@ package io.grpc; -import com.google.common.annotations.VisibleForTesting; -import java.net.URI; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; -import javax.annotation.Nullable; /** * Provider of name resolvers for name agnostic consumption. @@ -35,8 +28,6 @@ import javax.annotation.Nullable; @ExperimentalApi("https://github.com/grpc/grpc-java/issues/4159") public abstract class NameResolverProvider extends NameResolver.Factory { - private static final Logger logger = Logger.getLogger(NameResolverProvider.class.getName()); - /** * The port number used in case the target or the underlying naming system doesn't provide a * port number. @@ -48,36 +39,24 @@ public abstract class NameResolverProvider extends NameResolver.Factory { public static final Attributes.Key PARAMS_DEFAULT_PORT = NameResolver.Factory.PARAMS_DEFAULT_PORT; - @VisibleForTesting - static final Iterable> HARDCODED_CLASSES = getHardCodedClasses(); - - private static final List providers = ServiceProviders.loadAll( - NameResolverProvider.class, - HARDCODED_CLASSES, - NameResolverProvider.class.getClassLoader(), - new NameResolverPriorityAccessor()); - - private static final NameResolver.Factory factory = new NameResolverFactory(providers); - /** * Returns non-{@code null} ClassLoader-wide providers, in preference order. * * @since 1.0.0 + * @deprecated Has no replacement */ + @Deprecated public static List providers() { - return providers; + return NameResolverRegistry.getDefaultRegistry().providers(); } /** * @since 1.0.0 + * @deprecated Use NameResolverRegistry.getDefaultRegistry().asFactory() */ + @Deprecated public static NameResolver.Factory asFactory() { - return factory; - } - - @VisibleForTesting - static NameResolver.Factory asFactory(List providers) { - return new NameResolverFactory(providers); + return NameResolverRegistry.getDefaultRegistry().asFactory(); } /** @@ -97,69 +76,4 @@ public abstract class NameResolverProvider extends NameResolver.Factory { * @since 1.0.0 */ protected abstract int priority(); - - private static final class NameResolverFactory extends NameResolver.Factory { - private final List providers; - - NameResolverFactory(List providers) { - this.providers = Collections.unmodifiableList(new ArrayList<>(providers)); - } - - @Override - @Nullable - public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) { - checkForProviders(); - for (NameResolverProvider provider : providers) { - NameResolver resolver = provider.newNameResolver(targetUri, helper); - if (resolver != null) { - return resolver; - } - } - return null; - } - - @Override - public String getDefaultScheme() { - checkForProviders(); - return providers.get(0).getDefaultScheme(); - } - - private void checkForProviders() { - if (providers.isEmpty()) { - String msg = "No NameResolverProviders found via ServiceLoader, including for DNS. " - + "This is probably due to a broken build. If using ProGuard, check your configuration"; - throw new RuntimeException(msg); - } - } - } - - @VisibleForTesting - static final List> getHardCodedClasses() { - // Class.forName(String) is used to remove the need for ProGuard configuration. Note that - // ProGuard does not detect usages of Class.forName(String, boolean, ClassLoader): - // https://sourceforge.net/p/proguard/bugs/418/ - try { - return Collections.>singletonList( - Class.forName("io.grpc.internal.DnsNameResolverProvider")); - } catch (ClassNotFoundException e) { - logger.log(Level.FINE, "Unable to find DNS NameResolver", e); - } - return Collections.emptyList(); - } - - private static final class NameResolverPriorityAccessor - implements ServiceProviders.PriorityAccessor { - - NameResolverPriorityAccessor() {} - - @Override - public boolean isAvailable(NameResolverProvider provider) { - return provider.isAvailable(); - } - - @Override - public int getPriority(NameResolverProvider provider) { - return provider.priority(); - } - } } diff --git a/api/src/main/java/io/grpc/NameResolverRegistry.java b/api/src/main/java/io/grpc/NameResolverRegistry.java new file mode 100644 index 0000000000..cb44db96fb --- /dev/null +++ b/api/src/main/java/io/grpc/NameResolverRegistry.java @@ -0,0 +1,184 @@ +/* + * Copyright 2019 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import static com.google.common.base.Preconditions.checkArgument; + +import com.google.common.annotations.VisibleForTesting; +import java.net.URI; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.logging.Level; +import java.util.logging.Logger; +import javax.annotation.Nullable; +import javax.annotation.concurrent.GuardedBy; +import javax.annotation.concurrent.ThreadSafe; + +/** + * Registry of {@link NameResolverProvider}s. The {@link #getDefaultRegistry default instance} + * loads providers at runtime through the Java service provider mechanism. + * + * @since 1.21.0 + */ +@ExperimentalApi("https://github.com/grpc/grpc-java/issues/4159") +@ThreadSafe +public final class NameResolverRegistry { + private static final Logger logger = Logger.getLogger(NameResolverRegistry.class.getName()); + private static NameResolverRegistry instance; + + private final NameResolver.Factory factory = new NameResolverFactory(); + + @GuardedBy("this") + private final LinkedHashSet allProviders = new LinkedHashSet<>(); + /** Immutable, sorted version of {@code allProviders}. Is replaced instead of mutating. */ + @GuardedBy("this") + private List effectiveProviders = Collections.emptyList(); + + /** + * Register a provider. + * + *

If the provider's {@link NameResolverProvider#isAvailable isAvailable()} returns + * {@code false}, this method will throw {@link IllegalArgumentException}. + * + *

Providers will be used in priority order. In case of ties, providers are used in + * registration order. + */ + public synchronized void register(NameResolverProvider provider) { + addProvider(provider); + refreshProviders(); + } + + private synchronized void addProvider(NameResolverProvider provider) { + checkArgument(provider.isAvailable(), "isAvailable() returned false"); + allProviders.add(provider); + } + + /** + * Deregisters a provider. No-op if the provider is not in the registry. + * + * @param provider the provider that was added to the register via {@link #register}. + */ + public synchronized void deregister(NameResolverProvider provider) { + allProviders.remove(provider); + refreshProviders(); + } + + 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 p1, NameResolverProvider p2) { + return p1.priority() - p2.priority(); + } + })); + effectiveProviders = Collections.unmodifiableList(providers); + } + + /** + * Returns the default registry that loads providers via the Java service loader mechanism. + */ + public static synchronized NameResolverRegistry getDefaultRegistry() { + if (instance == null) { + List providerList = ServiceProviders.loadAll( + NameResolverProvider.class, + getHardCodedClasses(), + NameResolverProvider.class.getClassLoader(), + new NameResolverPriorityAccessor()); + if (providerList.isEmpty()) { + logger.warning("No NameResolverProviders found via ServiceLoader, including for DNS. This " + + "is probably due to a broken build. If using ProGuard, check your configuration"); + } + instance = new NameResolverRegistry(); + for (NameResolverProvider provider : providerList) { + logger.fine("Service loader found " + provider); + if (provider.isAvailable()) { + instance.addProvider(provider); + } + } + instance.refreshProviders(); + } + return instance; + } + + /** + * Returns effective providers, in priority order. + */ + @VisibleForTesting + synchronized List providers() { + return effectiveProviders; + } + + public NameResolver.Factory asFactory() { + return factory; + } + + @VisibleForTesting + static List> getHardCodedClasses() { + // Class.forName(String) is used to remove the need for ProGuard configuration. Note that + // ProGuard does not detect usages of Class.forName(String, boolean, ClassLoader): + // https://sourceforge.net/p/proguard/bugs/418/ + ArrayList> list = new ArrayList<>(); + try { + list.add(Class.forName("io.grpc.internal.DnsNameResolverProvider")); + } catch (ClassNotFoundException e) { + logger.log(Level.FINE, "Unable to find DNS NameResolver", e); + } + return Collections.unmodifiableList(list); + } + + private final class NameResolverFactory extends NameResolver.Factory { + @Override + @Nullable + public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) { + List providers = providers(); + for (NameResolverProvider provider : providers) { + NameResolver resolver = provider.newNameResolver(targetUri, helper); + if (resolver != null) { + return resolver; + } + } + return null; + } + + @Override + public String getDefaultScheme() { + List providers = providers(); + if (providers.isEmpty()) { + return "unknown"; + } + return providers.get(0).getDefaultScheme(); + } + } + + private static final class NameResolverPriorityAccessor + implements ServiceProviders.PriorityAccessor { + @Override + public boolean isAvailable(NameResolverProvider provider) { + return provider.isAvailable(); + } + + @Override + public int getPriority(NameResolverProvider provider) { + return provider.priority(); + } + } +} diff --git a/api/src/test/java/io/grpc/NameResolverProviderTest.java b/api/src/test/java/io/grpc/NameResolverProviderTest.java deleted file mode 100644 index 5b5f6a8a95..0000000000 --- a/api/src/test/java/io/grpc/NameResolverProviderTest.java +++ /dev/null @@ -1,163 +0,0 @@ -/* - * Copyright 2016 The gRPC Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.grpc; - -import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verifyZeroInteractions; - -import io.grpc.internal.DnsNameResolverProvider; -import java.net.URI; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.concurrent.Callable; -import org.junit.After; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.JUnit4; - -/** Unit tests for {@link NameResolverProvider}. */ -@RunWith(JUnit4.class) -public class NameResolverProviderTest { - private final URI uri = URI.create("dns:///localhost"); - private final NameResolver.Helper helper = mock(NameResolver.Helper.class); - - @After - public void wrapUp() { - // The helper is not implemented. Make sure it's not used in the test. - verifyZeroInteractions(helper); - } - - @Test - public void getDefaultScheme_noProvider() { - List providers = Collections.emptyList(); - NameResolver.Factory factory = NameResolverProvider.asFactory(providers); - try { - factory.getDefaultScheme(); - fail("Expected exception"); - } catch (RuntimeException ex) { - assertTrue(ex.toString(), ex.getMessage().contains("No NameResolverProviders found")); - } - } - - @Test - public void newNameResolver_providerReturnsNull() { - List providers = Collections.singletonList( - new BaseProvider(true, 5) { - @Override - public NameResolver newNameResolver(URI passedUri, NameResolver.Helper passedHelper) { - assertSame(uri, passedUri); - assertSame(helper, passedHelper); - return null; - } - }); - assertNull(NameResolverProvider.asFactory(providers).newNameResolver(uri, helper)); - } - - @Test - public void newNameResolver_noProvider() { - List providers = Collections.emptyList(); - NameResolver.Factory factory = NameResolverProvider.asFactory(providers); - try { - factory.newNameResolver(uri, helper); - fail("Expected exception"); - } catch (RuntimeException ex) { - assertTrue(ex.toString(), ex.getMessage().contains("No NameResolverProviders found")); - } - } - - @Test - public void baseProviders() { - List providers = NameResolverProvider.providers(); - assertEquals(1, providers.size()); - assertSame(DnsNameResolverProvider.class, providers.get(0).getClass()); - assertEquals("dns", NameResolverProvider.asFactory().getDefaultScheme()); - } - - @Test - public void getClassesViaHardcoded_classesPresent() throws Exception { - List> classes = NameResolverProvider.getHardCodedClasses(); - assertThat(classes).hasSize(1); - assertThat(classes.get(0).getName()).isEqualTo("io.grpc.internal.DnsNameResolverProvider"); - } - - @Test - public void provided() { - for (NameResolverProvider current - : InternalServiceProviders.getCandidatesViaServiceLoader( - NameResolverProvider.class, getClass().getClassLoader())) { - if (current instanceof DnsNameResolverProvider) { - return; - } - } - fail("DnsNameResolverProvider not registered"); - } - - @Test - public void providedHardCoded() { - for (NameResolverProvider current : InternalServiceProviders.getCandidatesViaHardCoded( - NameResolverProvider.class, NameResolverProvider.HARDCODED_CLASSES)) { - if (current instanceof DnsNameResolverProvider) { - return; - } - } - fail("DnsNameResolverProvider not registered"); - } - - public static final class HardcodedClassesCallable implements Callable>> { - @Override - public Iterator> call() { - return NameResolverProvider.getHardCodedClasses().iterator(); - } - } - - private static class BaseProvider extends NameResolverProvider { - private final boolean isAvailable; - private final int priority; - - public BaseProvider(boolean isAvailable, int priority) { - this.isAvailable = isAvailable; - this.priority = priority; - } - - @Override - protected boolean isAvailable() { - return isAvailable; - } - - @Override - protected int priority() { - return priority; - } - - @Override - public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) { - throw new UnsupportedOperationException(); - } - - @Override - public String getDefaultScheme() { - return "scheme" + getClass().getSimpleName(); - } - } -} diff --git a/api/src/test/java/io/grpc/NameResolverRegistryTest.java b/api/src/test/java/io/grpc/NameResolverRegistryTest.java new file mode 100644 index 0000000000..815462a38d --- /dev/null +++ b/api/src/test/java/io/grpc/NameResolverRegistryTest.java @@ -0,0 +1,209 @@ +/* + * Copyright 2016 The gRPC Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.grpc; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verifyZeroInteractions; + +import io.grpc.internal.DnsNameResolverProvider; +import java.net.URI; +import java.util.List; +import org.junit.After; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link NameResolverRegistry}. */ +@RunWith(JUnit4.class) +public class NameResolverRegistryTest { + private final URI uri = URI.create("dns:///localhost"); + private final NameResolver.Helper helper = mock(NameResolver.Helper.class); + + @After + public void wrapUp() { + // The helper is not implemented. Make sure it's not used in the test. + verifyZeroInteractions(helper); + } + + @Test + public void register_unavilableProviderThrows() { + NameResolverRegistry reg = new NameResolverRegistry(); + try { + reg.register(new BaseProvider(false, 5)); + fail("Should throw"); + } catch (IllegalArgumentException e) { + assertThat(e.getMessage()).contains("isAvailable() returned false"); + } + assertThat(reg.providers()).isEmpty(); + } + + @Test + public void deregister() { + NameResolverRegistry reg = new NameResolverRegistry(); + NameResolverProvider p1 = new BaseProvider(true, 5); + NameResolverProvider p2 = new BaseProvider(true, 5); + NameResolverProvider p3 = new BaseProvider(true, 5); + reg.register(p1); + reg.register(p2); + reg.register(p3); + assertThat(reg.providers()).containsExactly(p1, p2, p3).inOrder(); + reg.deregister(p2); + assertThat(reg.providers()).containsExactly(p1, p3).inOrder(); + } + + @Test + public void provider_sorted() { + NameResolverRegistry reg = new NameResolverRegistry(); + NameResolverProvider p1 = new BaseProvider(true, 5); + NameResolverProvider p2 = new BaseProvider(true, 3); + NameResolverProvider p3 = new BaseProvider(true, 8); + NameResolverProvider p4 = new BaseProvider(true, 3); + NameResolverProvider p5 = new BaseProvider(true, 8); + 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(); + } + + @Test + public void getDefaultScheme_noProvider() { + NameResolver.Factory factory = new NameResolverRegistry().asFactory(); + assertThat(factory.getDefaultScheme()).isEqualTo("unknown"); + } + + @Test + public void newNameResolver_providerReturnsNull() { + NameResolverRegistry registry = new NameResolverRegistry(); + registry.register( + new BaseProvider(true, 5) { + @Override + public NameResolver newNameResolver(URI passedUri, NameResolver.Helper passedHelper) { + assertThat(passedUri).isSameAs(uri); + assertThat(passedHelper).isSameAs(helper); + return null; + } + }); + assertThat(registry.asFactory().newNameResolver(uri, helper)).isNull(); + } + + @Test + public void newNameResolver_providerReturnsNonNull() { + NameResolverRegistry registry = new NameResolverRegistry(); + registry.register(new BaseProvider(true, 5) { + @Override + public NameResolver newNameResolver(URI passedUri, NameResolver.Helper passedHelper) { + return null; + } + }); + final NameResolver nr = new NameResolver() { + @Override public String getServiceAuthority() { + throw new UnsupportedOperationException(); + } + + @Override public void start(Observer observer) { + throw new UnsupportedOperationException(); + } + + @Override public void shutdown() { + throw new UnsupportedOperationException(); + } + }; + registry.register( + new BaseProvider(true, 4) { + @Override + public NameResolver newNameResolver(URI passedUri, NameResolver.Helper passedHelper) { + return nr; + } + }); + registry.register( + new BaseProvider(true, 3) { + @Override + public NameResolver newNameResolver(URI passedUri, NameResolver.Helper passedHelper) { + fail("Should not be called"); + throw new AssertionError(); + } + }); + assertThat(registry.asFactory().newNameResolver(uri, helper)).isSameAs(nr); + } + + @Test + public void newNameResolver_noProvider() { + NameResolver.Factory factory = new NameResolverRegistry().asFactory(); + assertThat(factory.newNameResolver(uri, helper)).isNull(); + } + + @Test + public void baseProviders() { + List providers = NameResolverRegistry.getDefaultRegistry().providers(); + assertThat(providers).hasSize(1); + assertThat(providers.get(0)).isInstanceOf(DnsNameResolverProvider.class); + assertThat(NameResolverRegistry.getDefaultRegistry().asFactory().getDefaultScheme()) + .isEqualTo("dns"); + } + + @Test + public void getClassesViaHardcoded_classesPresent() throws Exception { + List> classes = NameResolverRegistry.getHardCodedClasses(); + assertThat(classes).containsExactly(io.grpc.internal.DnsNameResolverProvider.class); + } + + @Test + public void provided() { + for (NameResolverProvider current + : InternalServiceProviders.getCandidatesViaServiceLoader( + NameResolverProvider.class, getClass().getClassLoader())) { + if (current instanceof DnsNameResolverProvider) { + return; + } + } + fail("DnsNameResolverProvider not registered"); + } + + private static class BaseProvider extends NameResolverProvider { + private final boolean isAvailable; + private final int priority; + + public BaseProvider(boolean isAvailable, int priority) { + this.isAvailable = isAvailable; + this.priority = priority; + } + + @Override + protected boolean isAvailable() { + return isAvailable; + } + + @Override + protected int priority() { + return priority; + } + + @Override + public NameResolver newNameResolver(URI targetUri, NameResolver.Helper helper) { + throw new UnsupportedOperationException(); + } + + @Override + public String getDefaultScheme() { + return "scheme" + getClass().getSimpleName(); + } + } +} diff --git a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java index 85ba518717..dc8a9231c8 100644 --- a/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java +++ b/core/src/main/java/io/grpc/internal/AbstractManagedChannelImplBuilder.java @@ -31,7 +31,7 @@ import io.grpc.InternalChannelz; import io.grpc.ManagedChannel; import io.grpc.ManagedChannelBuilder; import io.grpc.NameResolver; -import io.grpc.NameResolverProvider; +import io.grpc.NameResolverRegistry; import io.grpc.ProxyDetector; import io.opencensus.trace.Tracing; import java.net.SocketAddress; @@ -85,7 +85,7 @@ public abstract class AbstractManagedChannelImplBuilder SharedResourcePool.forResource(GrpcUtil.SHARED_CHANNEL_EXECUTOR); private static final NameResolver.Factory DEFAULT_NAME_RESOLVER_FACTORY = - NameResolverProvider.asFactory(); + NameResolverRegistry.getDefaultRegistry().asFactory(); private static final DecompressorRegistry DEFAULT_DECOMPRESSOR_REGISTRY = DecompressorRegistry.getDefaultInstance();