core: NameResolverProvider should use ServiceProviders util (#4005)

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<Class<?>> HARDCODED_CLASSES =
+      NameResolverProvider.HARDCODED_CLASSES;
+}

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.Iterator;
-import java.util.Comparator;
 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<Integer> PARAMS_DEFAULT_PORT =
       NameResolver.Factory.PARAMS_DEFAULT_PORT;
+  @VisibleForTesting
+  static final Iterable<Class<?>> HARDCODED_CLASSES = new HardcodedClasses();
+
+  private static final List<NameResolverProvider> providers = ServiceProviders.loadAll(
+      NameResolverProvider.class,
+      HARDCODED_CLASSES,
+      NameResolverProvider.class.getClassLoader(),
+      new ServiceProviders.PriorityAccessor<NameResolverProvider>() {
+        @Override
+        public boolean isAvailable(NameResolverProvider provider) {
+          return provider.isAvailable();
+        }
+
+        @Override
+        public int getPriority(NameResolverProvider provider) {
+          return provider.priority();
+        }
+      });
 
-  private static final List<NameResolverProvider> providers
-      = load(NameResolverProvider.class.getClassLoader());
   private static final NameResolver.Factory factory = new NameResolverFactory(providers);
 
-  @VisibleForTesting
-  static List<NameResolverProvider> load(ClassLoader classLoader) {
-    Iterable<NameResolverProvider> candidates;
-    if (isAndroid()) {
-      candidates = getCandidatesViaHardCoded();
-    } else {
-      candidates = getCandidatesViaServiceLoader(classLoader);
-    }
-    List<NameResolverProvider> list = new ArrayList<NameResolverProvider>();
-    for (NameResolverProvider current : candidates) {
-      if (!current.isAvailable()) {
-        continue;
-      }
-      list.add(current);
-    }
-    // Sort descending based on priority.
-    Collections.sort(list, Collections.reverseOrder(new Comparator<NameResolverProvider>() {
-      @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<NameResolverProvider> getCandidatesViaServiceLoader(
-      ClassLoader classLoader) {
-    Iterable<NameResolverProvider> 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<NameResolverProvider> 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<NameResolverProvider> list = new ArrayList<NameResolverProvider>();
-    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<Class<?>> {
+    @Override
+    public Iterator<Class<?>> iterator() {
+      List<Class<?>> list = new ArrayList<Class<?>>();
+      // 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();
+    }
+  }
 }

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.concurrent.Callable;
-import java.util.ServiceConfigurationError;
-import java.util.regex.Pattern;
 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<NameResolverProvider> providers = NameResolverProvider.load(cl);
-      assertEquals(Collections.<NameResolverProvider>emptyList(), providers);
-    } finally {
-      Thread.currentThread().setContextClassLoader(ccl);
-    }
-  }
-
-  @Test
-  public void multipleProvider() {
-    ClassLoader cl = new ReplacingClassLoader(
-        getClass().getClassLoader(), serviceFile,
-        "io/grpc/NameResolverProviderTest-multipleProvider.txt");
-    List<NameResolverProvider> 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.<NameResolverProvider>emptyList(), NameResolverProvider.load(cl));
-  }
-
   @Test
   public void getDefaultScheme_noProvider() {
     List<NameResolverProvider> providers = Collections.<NameResolverProvider>emptyList();
@@ -133,117 +87,17 @@ public class NameResolverProviderTest {
   }
 
   @Test
-  public void getCandidatesViaHardCoded_triesToLoadClasses() throws Exception {
+  public void getClassesViaHardcoded_triesToLoadClasses() throws Exception {
-    ClassLoader cl = getClass().getClassLoader();
+    ServiceProvidersTestUtil.testHardcodedClasses(
-    final RuntimeException toThrow = new RuntimeException();
+        HardcodedClassesCallable.class.getName(),
-    // Prevent DnsNameResolverProvider from being known
+        getClass().getClassLoader(),
-    cl = new FilteringClassLoader(cl, serviceFile);
+        ImmutableSet.of("io.grpc.internal.DnsNameResolverProvider"));
-    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);
-    }
   }
 
-  @Test
+  public static final class HardcodedClassesCallable implements Callable<Iterator<Class<?>>> {
-  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;
-    }
-
     @Override
-    public URL getResource(String name) {
+    public Iterator<Class<?>> call() throws Exception {
-      if (resource.equals(name)) {
+      return new HardcodedClasses().iterator();
-        return null;
-      }
-      return super.getResource(name);
-    }
-
-    @Override
-    public Enumeration<URL> getResources(String name) throws IOException {
-      if (resource.equals(name)) {
-        return new Enumeration<URL>() {
-          @Override public boolean hasMoreElements() {
-            return false;
-          }
-
-          @Override public URL nextElement() {
-            throw new NoSuchElementException();
-          }
-        };
-      }
-      return super.getResources(name);
     }
   }
 
@@ -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");
-    }
-  }
 }

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.
+   *
+   * <p>{@code callableClassName} is a {@code Callable<Iterable<Class<?>>} rather than the iterable
+   * class name itself so that the iterable class can be non-public.
+   *
+   * <p>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<String> hardcodedClassNames) throws Exception {
+    final Set<String> notLoaded = new HashSet<String>(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);
+  }
+}

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;
       }