diff --git a/core/src/main/java/io/grpc/InternalNameResolverProvider.java b/core/src/main/java/io/grpc/InternalNameResolverProvider.java new file mode 100644 index 0000000000..dc3a76b325 --- /dev/null +++ b/core/src/main/java/io/grpc/InternalNameResolverProvider.java @@ -0,0 +1,25 @@ +/* + * Copyright 2018, gRPC Authors All rights reserved. + * + * 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 com.google.common.annotations.VisibleForTesting; + +public class InternalNameResolverProvider { + @VisibleForTesting + public static final Iterable> HARDCODED_CLASSES = + NameResolverProvider.HARDCODED_CLASSES; +} diff --git a/core/src/main/java/io/grpc/NameResolverProvider.java b/core/src/main/java/io/grpc/NameResolverProvider.java index 45f16124af..601537438f 100644 --- a/core/src/main/java/io/grpc/NameResolverProvider.java +++ b/core/src/main/java/io/grpc/NameResolverProvider.java @@ -20,11 +20,8 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import java.net.URI; import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; +import java.util.Iterator; import java.util.List; -import java.util.ServiceConfigurationError; -import java.util.ServiceLoader; /** * Provider of name resolvers for name agnostic consumption. @@ -41,82 +38,27 @@ 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 = new HardcodedClasses(); + + private static final List providers = ServiceProviders.loadAll( + NameResolverProvider.class, + HARDCODED_CLASSES, + NameResolverProvider.class.getClassLoader(), + new ServiceProviders.PriorityAccessor() { + @Override + public boolean isAvailable(NameResolverProvider provider) { + return provider.isAvailable(); + } + + @Override + public int getPriority(NameResolverProvider provider) { + return provider.priority(); + } + }); - private static final List providers - = load(NameResolverProvider.class.getClassLoader()); private static final NameResolver.Factory factory = new NameResolverFactory(providers); - @VisibleForTesting - static List load(ClassLoader classLoader) { - Iterable candidates; - if (isAndroid()) { - candidates = getCandidatesViaHardCoded(); - } else { - candidates = getCandidatesViaServiceLoader(classLoader); - } - List list = new ArrayList(); - for (NameResolverProvider current : candidates) { - if (!current.isAvailable()) { - continue; - } - list.add(current); - } - // Sort descending based on priority. - Collections.sort(list, Collections.reverseOrder(new Comparator() { - @Override - public int compare(NameResolverProvider f1, NameResolverProvider f2) { - return f1.priority() - f2.priority(); - } - })); - return Collections.unmodifiableList(list); - } - - /** - * Loads service providers for the {@link NameResolverProvider} service using - * {@link ServiceLoader}. - */ - @VisibleForTesting - public static Iterable getCandidatesViaServiceLoader( - ClassLoader classLoader) { - Iterable i - = ServiceLoader.load(NameResolverProvider.class, classLoader); - // Attempt to load using the context class loader and ServiceLoader. - // This allows frameworks like http://aries.apache.org/modules/spi-fly.html to plug in. - if (!i.iterator().hasNext()) { - i = ServiceLoader.load(NameResolverProvider.class); - } - return i; - } - - /** - * Load providers from a hard-coded list. This avoids using getResource(), which has performance - * problems on Android (see https://github.com/grpc/grpc-java/issues/2037). Any provider that may - * be used on Android is free to be added here. - */ - @VisibleForTesting - public static Iterable getCandidatesViaHardCoded() { - // 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/ - List list = new ArrayList(); - try { - list.add(create(Class.forName("io.grpc.internal.DnsNameResolverProvider"))); - } catch (ClassNotFoundException ex) { - // ignore - } - return list; - } - - @VisibleForTesting - static NameResolverProvider create(Class rawClass) { - try { - return rawClass.asSubclass(NameResolverProvider.class).getConstructor().newInstance(); - } catch (Throwable t) { - throw new ServiceConfigurationError( - "Provider " + rawClass.getName() + " could not be instantiated: " + t, t); - } - } - /** * Returns non-{@code null} ClassLoader-wide providers, in preference order. */ @@ -133,18 +75,6 @@ public abstract class NameResolverProvider extends NameResolver.Factory { return new NameResolverFactory(providers); } - private static boolean isAndroid() { - try { - // Specify a class loader instead of null because we may be running under Robolectric - Class.forName("android.app.Application", /*initialize=*/ false, - NameResolverProvider.class.getClassLoader()); - return true; - } catch (Exception e) { - // If Application isn't loaded, it might as well not be Android. - return false; - } - } - /** * Whether this provider is available for use, taking the current environment into consideration. * If {@code false}, no other methods are safe to be called. @@ -190,4 +120,21 @@ public abstract class NameResolverProvider extends NameResolver.Factory { + "This is probably due to a broken build. If using ProGuard, check your configuration"); } } + + @VisibleForTesting + static final class HardcodedClasses implements Iterable> { + @Override + public Iterator> iterator() { + List> list = new ArrayList>(); + // 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 { + list.add(Class.forName("io.grpc.internal.DnsNameResolverProvider")); + } catch (ClassNotFoundException e) { + // ignore + } + return list.iterator(); + } + } } diff --git a/core/src/test/java/io/grpc/NameResolverProviderTest.java b/core/src/test/java/io/grpc/NameResolverProviderTest.java index 9cd820f5ca..8ecfe70fbf 100644 --- a/core/src/test/java/io/grpc/NameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/NameResolverProviderTest.java @@ -17,25 +17,19 @@ package io.grpc; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; 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 com.google.common.collect.ImmutableSet; +import io.grpc.NameResolverProvider.HardcodedClasses; import io.grpc.internal.DnsNameResolverProvider; -import java.io.IOException; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.net.URI; -import java.net.URL; import java.util.Collections; -import java.util.Enumeration; +import java.util.Iterator; import java.util.List; -import java.util.NoSuchElementException; -import java.util.ServiceConfigurationError; -import java.util.regex.Pattern; +import java.util.concurrent.Callable; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -43,49 +37,9 @@ import org.junit.runners.JUnit4; /** Unit tests for {@link NameResolverProvider}. */ @RunWith(JUnit4.class) public class NameResolverProviderTest { - private final String serviceFile = "META-INF/services/io.grpc.NameResolverProvider"; private final URI uri = URI.create("dns:///localhost"); private final Attributes attributes = Attributes.EMPTY; - @Test - public void noProvider() { - ClassLoader ccl = Thread.currentThread().getContextClassLoader(); - try { - ClassLoader cl = new ReplacingClassLoader( - getClass().getClassLoader(), serviceFile, - "io/grpc/NameResolverProviderTest-doesNotExist.txt"); - Thread.currentThread().setContextClassLoader(cl); - List providers = NameResolverProvider.load(cl); - assertEquals(Collections.emptyList(), providers); - } finally { - Thread.currentThread().setContextClassLoader(ccl); - } - } - - @Test - public void multipleProvider() { - ClassLoader cl = new ReplacingClassLoader( - getClass().getClassLoader(), serviceFile, - "io/grpc/NameResolverProviderTest-multipleProvider.txt"); - List providers = NameResolverProvider.load(cl); - assertEquals(3, providers.size()); - assertSame(Available7Provider.class, providers.get(0).getClass()); - assertSame(Available5Provider.class, providers.get(1).getClass()); - assertSame(Available0Provider.class, providers.get(2).getClass()); - assertEquals("schemeAvailable7Provider", - NameResolverProvider.asFactory(providers).getDefaultScheme()); - assertSame(Available7Provider.nameResolver, - NameResolverProvider.asFactory(providers).newNameResolver(uri, attributes)); - } - - @Test - public void unavailableProvider() { - ClassLoader cl = new ReplacingClassLoader( - getClass().getClassLoader(), serviceFile, - "io/grpc/NameResolverProviderTest-unavailableProvider.txt"); - assertEquals(Collections.emptyList(), NameResolverProvider.load(cl)); - } - @Test public void getDefaultScheme_noProvider() { List providers = Collections.emptyList(); @@ -133,117 +87,17 @@ public class NameResolverProviderTest { } @Test - public void getCandidatesViaHardCoded_triesToLoadClasses() throws Exception { - ClassLoader cl = getClass().getClassLoader(); - final RuntimeException toThrow = new RuntimeException(); - // Prevent DnsNameResolverProvider from being known - cl = new FilteringClassLoader(cl, serviceFile); - cl = new ClassLoader(cl) { - @Override - public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { - if (name.startsWith("io.grpc.internal.")) { - throw toThrow; - } else { - return super.loadClass(name, resolve); - } - } - }; - cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); - ClassLoader ccl = Thread.currentThread().getContextClassLoader(); - try { - Thread.currentThread().setContextClassLoader(cl); - invokeGetCandidatesViaHardCoded(cl); - fail("Expected exception"); - } catch (RuntimeException ex) { - assertSame(toThrow, ex); - } finally { - Thread.currentThread().setContextClassLoader(ccl); - } + public void getClassesViaHardcoded_triesToLoadClasses() throws Exception { + ServiceProvidersTestUtil.testHardcodedClasses( + HardcodedClassesCallable.class.getName(), + getClass().getClassLoader(), + ImmutableSet.of("io.grpc.internal.DnsNameResolverProvider")); } - @Test - public void getCandidatesViaHardCoded_ignoresMissingClasses() throws Exception { - ClassLoader cl = getClass().getClassLoader(); - // Prevent DnsNameResolverProvider from being known - cl = new FilteringClassLoader(cl, serviceFile); - cl = new ClassLoader(cl) { - @Override - public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { - if (name.startsWith("io.grpc.internal.")) { - throw new ClassNotFoundException(); - } else { - return super.loadClass(name, resolve); - } - } - }; - cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); - ClassLoader ccl = Thread.currentThread().getContextClassLoader(); - try { - Thread.currentThread().setContextClassLoader(cl); - Iterable i = invokeGetCandidatesViaHardCoded(cl); - assertFalse("Iterator should be empty", i.iterator().hasNext()); - } finally { - Thread.currentThread().setContextClassLoader(ccl); - } - } - - @Test - public void create_throwsErrorOnMisconfiguration() throws Exception { - class PrivateClass {} - - try { - NameResolverProvider.create(PrivateClass.class); - fail("Expected exception"); - } catch (ServiceConfigurationError e) { - assertTrue("Expected ClassCastException cause: " + e.getCause(), - e.getCause() instanceof ClassCastException); - } - } - - private static Iterable invokeGetCandidatesViaHardCoded(ClassLoader cl) throws Exception { - // An error before the invoke likely means there is a bug in the test - Class klass = Class.forName(NameResolverProvider.class.getName(), true, cl); - Method getCandidatesViaHardCoded = klass.getMethod("getCandidatesViaHardCoded"); - try { - return (Iterable) getCandidatesViaHardCoded.invoke(null); - } catch (InvocationTargetException ex) { - if (ex.getCause() instanceof Exception) { - throw (Exception) ex.getCause(); - } - throw ex; - } - } - - private static class FilteringClassLoader extends ClassLoader { - private final String resource; - - public FilteringClassLoader(ClassLoader parent, String resource) { - super(parent); - this.resource = resource; - } - + public static final class HardcodedClassesCallable implements Callable>> { @Override - public URL getResource(String name) { - if (resource.equals(name)) { - return null; - } - return super.getResource(name); - } - - @Override - public Enumeration getResources(String name) throws IOException { - if (resource.equals(name)) { - return new Enumeration() { - @Override public boolean hasMoreElements() { - return false; - } - - @Override public URL nextElement() { - throw new NoSuchElementException(); - } - }; - } - return super.getResources(name); + public Iterator> call() throws Exception { + return new HardcodedClasses().iterator(); } } @@ -276,40 +130,4 @@ public class NameResolverProviderTest { return "scheme" + getClass().getSimpleName(); } } - - public static class Available0Provider extends BaseProvider { - public Available0Provider() { - super(true, 0); - } - } - - public static class Available5Provider extends BaseProvider { - public Available5Provider() { - super(true, 5); - } - } - - public static class Available7Provider extends BaseProvider { - public static final NameResolver nameResolver = mock(NameResolver.class); - - public Available7Provider() { - super(true, 7); - } - - @Override - public NameResolver newNameResolver(URI targetUri, Attributes params) { - return nameResolver; - } - } - - public static class UnavailableProvider extends BaseProvider { - public UnavailableProvider() { - super(false, 10); - } - - @Override - protected int priority() { - throw new RuntimeException("purposefully broken"); - } - } } diff --git a/core/src/test/java/io/grpc/ServiceProvidersTestUtil.java b/core/src/test/java/io/grpc/ServiceProvidersTestUtil.java new file mode 100644 index 0000000000..d7bc7f3d6e --- /dev/null +++ b/core/src/test/java/io/grpc/ServiceProvidersTestUtil.java @@ -0,0 +1,81 @@ +/* + * Copyright 2018, gRPC Authors All rights reserved. + * + * 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.assertWithMessage; + +import com.google.common.collect.Iterators; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; +import java.util.regex.Pattern; + +final class ServiceProvidersTestUtil { + /** + * Creates an iterator from the callable class via reflection, and checks that all expected + * classes were loaded. + * + *

{@code callableClassName} is a {@code Callable>} rather than the iterable + * class name itself so that the iterable class can be non-public. + * + *

We accept class names as input so that we can test against classes not in the + * testing class path. + */ + static void testHardcodedClasses( + String callableClassName, + ClassLoader cl, + Set hardcodedClassNames) throws Exception { + final Set notLoaded = new HashSet(hardcodedClassNames); + cl = new ClassLoader(cl) { + @Override + public Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if (notLoaded.remove(name)) { + throw new ClassNotFoundException(); + } else { + return super.loadClass(name, resolve); + } + } + }; + cl = new StaticTestingClassLoader(cl, Pattern.compile("io\\.grpc\\.[^.]*")); + // Some classes fall back to the context class loader. + // Ensure that the context class loader is not an accidental backdoor. + ClassLoader ccl = Thread.currentThread().getContextClassLoader(); + try { + Thread.currentThread().setContextClassLoader(cl); + Object[] results = Iterators.toArray( + invokeIteratorCallable(callableClassName, cl), Object.class); + assertWithMessage("The Iterable loaded a class that was not in hardcodedClassNames") + .that(results).isEmpty(); + assertWithMessage( + "The Iterable did not attempt to load some classes from hardcodedClassNames") + .that(notLoaded).isEmpty(); + } finally { + Thread.currentThread().setContextClassLoader(ccl); + } + } + + private static Iterator invokeIteratorCallable( + String callableClassName, ClassLoader cl) throws Exception { + Class klass = Class.forName(callableClassName, true, cl); + Constructor ctor = klass.getDeclaredConstructor(); + Object instance = ctor.newInstance(); + Method callMethod = klass.getMethod("call"); + return (Iterator) callMethod.invoke(instance); + } +} diff --git a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java index d69888fcd5..d752f1b93b 100644 --- a/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java +++ b/core/src/test/java/io/grpc/internal/DnsNameResolverProviderTest.java @@ -22,6 +22,8 @@ import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import io.grpc.Attributes; +import io.grpc.InternalNameResolverProvider; +import io.grpc.InternalServiceProviders; import io.grpc.NameResolverProvider; import java.net.URI; import org.junit.Test; @@ -36,7 +38,8 @@ public class DnsNameResolverProviderTest { @Test public void provided() { for (NameResolverProvider current - : NameResolverProvider.getCandidatesViaServiceLoader(getClass().getClassLoader())) { + : InternalServiceProviders.getCandidatesViaServiceLoader( + NameResolverProvider.class, getClass().getClassLoader())) { if (current instanceof DnsNameResolverProvider) { return; } @@ -46,7 +49,8 @@ public class DnsNameResolverProviderTest { @Test public void providedHardCoded() { - for (NameResolverProvider current : NameResolverProvider.getCandidatesViaHardCoded()) { + for (NameResolverProvider current : InternalServiceProviders.getCandidatesViaHardCoded( + NameResolverProvider.class, InternalNameResolverProvider.HARDCODED_CLASSES)) { if (current instanceof DnsNameResolverProvider) { return; }