Move the loading with reflection logic from the Tracer to Provider to be shared with stats. Add more tests.

3 files changed, 143 insertions, 81 deletions

diff --git a/core/src/main/java/com/google/instrumentation/common/Provider.java b/core/src/main/java/com/google/instrumentation/common/Provider.java
index d32fbad9..262135d6 100644
--- a/core/src/main/java/com/google/instrumentation/common/Provider.java
+++ b/core/src/main/java/com/google/instrumentation/common/Provider.java
@@ -13,6 +13,7 @@
 
 package com.google.instrumentation.common;
 
+import java.util.ServiceConfigurationError;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -52,4 +53,47 @@ public final class Provider {
       }
     }
   }
+
+  /**
+   * Tries to create an instance of the given rawClass as a subclass of the given superclass.
+   *
+   * @param rawClass The class that is initialized.
+   * @param superclass The initialized class must be a subclass of this.
+   * @return an instance of the class given rawClass which is a subclass of the given superclass.
+   * @throws ServiceConfigurationError if any error happens.
+   */
+  public static <T> T createInstance(Class<?> rawClass, Class<T> superclass) {
+    try {
+      return rawClass.asSubclass(superclass).getConstructor().newInstance();
+    } catch (Exception e) {
+      throw new ServiceConfigurationError(
+          "Provider " + rawClass.getName() + " could not be instantiated.", e);
+    }
+  }
+
+  /**
+   * Get the correct {@link ClassLoader} that must be used when loading using reflection.
+   *
+   * @return The correct {@code ClassLoader} that must be used when loading using reflection.
+   */
+  public static <T> ClassLoader getCorrectClassLoader(Class<T> superClass) {
+    if (isAndroid()) {
+      // When android:sharedUserId or android:process is used, Android will setup a dummy
+      // ClassLoader for the thread context (http://stackoverflow.com/questions/13407006),
+      // instead of letting users to manually set context class loader, we choose the
+      // correct class loader here.
+      return superClass.getClassLoader();
+    }
+    return Thread.currentThread().getContextClassLoader();
+  }
+
+  private static boolean isAndroid() {
+    try {
+      Class.forName("android.app.Application", /*initialize=*/ false, null);
+      return true;
+    } catch (Exception e) {
+      // If Application isn't loaded, it might as well not be Android.
+      return false;
+    }
+  }
 }
diff --git a/core/src/main/java/com/google/instrumentation/trace/Tracer.java b/core/src/main/java/com/google/instrumentation/trace/Tracer.java
index 8abeed97..6e789e97 100644
--- a/core/src/main/java/com/google/instrumentation/trace/Tracer.java
+++ b/core/src/main/java/com/google/instrumentation/trace/Tracer.java
@@ -17,9 +17,9 @@ import static com.google.common.base.Preconditions.checkNotNull;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.instrumentation.common.NonThrowingCloseable;
+import com.google.instrumentation.common.Provider;
 import com.google.instrumentation.common.Timestamp;
 
-import java.util.ServiceConfigurationError;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.annotation.Nullable;
@@ -54,8 +54,8 @@ public final class Tracer {
   private static final Logger logger = Logger.getLogger(Tracer.class.getName());
   private static final Tracer INSTANCE =
       new Tracer(
-          loadContextSpanHandler(getCorrectClassLoader(ContextSpanHandler.class)),
-          loadSpanFactory(getCorrectClassLoader(SpanFactory.class)));
+          loadContextSpanHandler(Provider.getCorrectClassLoader(ContextSpanHandler.class)),
+          loadSpanFactory(Provider.getCorrectClassLoader(SpanFactory.class)));
   private final ContextSpanHandler contextSpanHandler;
   private final SpanFactory spanFactory;
 
@@ -393,9 +393,7 @@ public final class Tracer {
     try {
       // Because of shading tools we must call Class.forName with the literal string name of the
       // class.
-      // TODO(bdrutu): Decide about the package name for implementation. Kun suggested
-      // com.google.instrumentation.impl.trace.*
-      return createInstance(
+      return Provider.createInstance(
           Class.forName("com.google.instrumentation.trace.SpanFactoryImpl", true, classLoader),
           SpanFactory.class);
     } catch (ClassNotFoundException e) {
@@ -410,7 +408,7 @@ public final class Tracer {
     try {
       // Because of shading tools we must call Class.forName with the literal string name of the
       // class.
-      return createInstance(
+      return Provider.createInstance(
           Class.forName(
               "com.google.instrumentation.trace.ContextSpanHandlerImpl", true, classLoader),
           ContextSpanHandler.class);
@@ -419,50 +417,4 @@ public final class Tracer {
     }
     return new NoopContextSpanHandler();
   }
-
-  // TODO(bdrutu): Move all the below methods to common.Provider to be shared by the stats library
-  // as well.
-
-  /**
-   * Tries to create an instance of the given rawClass as a subclass of the given superclass.
-   *
-   * @param rawClass The class that is initialized.
-   * @param superclass The initialized class must be a subclass of this.
-   * @return an instance of the class given rawClass which is a subclass of the given superclass.
-   * @throws ServiceConfigurationError if any error happens.
-   */
-  private static <T> T createInstance(Class<?> rawClass, Class<T> superclass) {
-    try {
-      return rawClass.asSubclass(superclass).getConstructor().newInstance();
-    } catch (Exception e) {
-      throw new ServiceConfigurationError(
-          "Provider " + rawClass.getName() + " could not be instantiated.", e);
-    }
-  }
-
-  /**
-   * Get the correct {@link ClassLoader} that must be used when loading using reflection.
-   *
-   * @return The correct {@code ClassLoader} that must be used when loading using reflection.
-   */
-  private static <T> ClassLoader getCorrectClassLoader(Class<T> superClass) {
-    if (isAndroid()) {
-      // When android:sharedUserId or android:process is used, Android will setup a dummy
-      // ClassLoader for the thread context (http://stackoverflow.com/questions/13407006),
-      // instead of letting users to manually set context class loader, we choose the
-      // correct class loader here.
-      return superClass.getClassLoader();
-    }
-    return Thread.currentThread().getContextClassLoader();
-  }
-
-  private static boolean isAndroid() {
-    try {
-      Class.forName("android.app.Application", /*initialize=*/ false, null);
-      return true;
-    } catch (Exception e) {
-      // If Application isn't loaded, it might as well not be Android.
-      return false;
-    }
-  }
 }
diff --git a/core/src/test/java/com/google/instrumentation/common/ProviderTest.java b/core/src/test/java/com/google/instrumentation/common/ProviderTest.java
index f125de25..087ffed6 100644
--- a/core/src/test/java/com/google/instrumentation/common/ProviderTest.java
+++ b/core/src/test/java/com/google/instrumentation/common/ProviderTest.java
@@ -15,47 +15,113 @@ package com.google.instrumentation.common;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import java.util.ServiceConfigurationError;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
-/**
- * Tests for {@link Provider}
- */
+/** Tests for {@link Provider} */
 @RunWith(JUnit4.class)
 public class ProviderTest {
+  static class GoodClass {
+    public GoodClass() {}
+  }
+
+  static class PrivateConstructorClass {
+    private PrivateConstructorClass() {}
+  }
+
+  static class NoDefaultConstructorClass {
+    public NoDefaultConstructorClass(int arg) {}
+  }
+
+  private static class PrivateClass {}
+
+  static interface MyInterface {}
+
+  static class MyInterfaceImpl implements MyInterface {
+    public MyInterfaceImpl() {}
+  }
+
   @Test
-  public void testGoodClass() throws Exception {
-    GetGen getGen0 =
-        Provider.newInstance("com.google.instrumentation.common.ProviderTest$GetGen", null);
-    assertThat(getGen0).isNotNull();
-    assertThat(getGen0.getGen()).isEqualTo(0);
-    for (int i = 1; i < 10; i++) {
-      GetGen getGenI =
-          Provider.newInstance("com.google.instrumentation.common.ProviderTest$GetGen", null);
-      assertThat(getGenI).isNotNull();
-      assertThat(getGenI.getGen()).isEqualTo(i);
-    }
-    assertThat(getGen0.getGen()).isEqualTo(0);
+  public void testGoodClass() {
+    assertThat(
+            Provider.<GoodClass>newInstance(
+                "com.google.instrumentation.common.ProviderTest$GoodClass", null))
+        .isNotNull();
   }
 
   @Test
-  public void testBadClass() throws Exception {
-    GetGen getGen =
-        Provider.newInstance("com.google.instrumentation.common.ProviderTest$BadClass", null);
-    assertThat(getGen).isNull();
+  public void testBadClass() {
+    assertThat(
+            Provider.<GoodClass>newInstance(
+                "com.google.instrumentation.common.ProviderTest$BadClass", null))
+        .isNull();
   }
 
-  static class GetGen {
-    static int genCount = 0;
-    int gen;
+  @Test(expected = ServiceConfigurationError.class)
+  public void createInstance_ThrowsErrorWhenClassIsPrivate() throws ClassNotFoundException {
+    Provider.createInstance(
+        Class.forName(
+            "com.google.instrumentation.common.ProviderTest$PrivateClass",
+            true,
+            Provider.getCorrectClassLoader(ProviderTest.class)),
+        PrivateClass.class);
+  }
+
+  @Test(expected = ServiceConfigurationError.class)
+  public void createInstance_ThrowsErrorWhenClassHasPrivateConstructor()
+      throws ClassNotFoundException {
+    Provider.createInstance(
+        Class.forName(
+            "com.google.instrumentation.common.ProviderTest$PrivateConstructorClass",
+            true,
+            Provider.getCorrectClassLoader(ProviderTest.class)),
+        PrivateConstructorClass.class);
+  }
+
+  @Test(expected = ServiceConfigurationError.class)
+  public void createInstance_ThrowsErrorWhenClassDoesNotHaveDefaultConstructor()
+      throws ClassNotFoundException {
+    Provider.createInstance(
+        Class.forName(
+            "com.google.instrumentation.common.ProviderTest$NoDefaultConstructorClass",
+            true,
+            Provider.getCorrectClassLoader(ProviderTest.class)),
+        NoDefaultConstructorClass.class);
+  }
 
-    public GetGen() {
-      gen = genCount++;
-    }
+  @Test(expected = ServiceConfigurationError.class)
+  public void createInstance_ThrowsErrorWhenClassIsNotASubclass() throws ClassNotFoundException {
+    Provider.createInstance(
+        Class.forName(
+            "com.google.instrumentation.common.ProviderTest$GoodClass",
+            true,
+            Provider.getCorrectClassLoader(ProviderTest.class)),
+        MyInterface.class);
+  }
 
-    public int getGen() {
-      return gen;
-    }
+  @Test
+  public void createInstance_GoodClass() throws ClassNotFoundException {
+    assertThat(
+            Provider.createInstance(
+                Class.forName(
+                    "com.google.instrumentation.common.ProviderTest$GoodClass",
+                    true,
+                    Provider.getCorrectClassLoader(ProviderTest.class)),
+                GoodClass.class))
+        .isNotNull();
+  }
+
+  @Test
+  public void createInstance_GoodSubclass() throws ClassNotFoundException {
+    assertThat(
+            Provider.createInstance(
+                Class.forName(
+                    "com.google.instrumentation.common.ProviderTest$MyInterfaceImpl",
+                    true,
+                    Provider.getCorrectClassLoader(ProviderTest.class)),
+                MyInterface.class))
+        .isNotNull();
   }
 }