From c19f0c2c5c47557186bc0314c6bf27e736df98c1 Mon Sep 17 00:00:00 2001 From: dpb Date: Thu, 19 Oct 2017 13:30:18 -0700 Subject: [PATCH] Avoid calling Class.getCanonicalName(), which is slow on Android, during normal execution. Make Preconditions.checkNotNull call Class.getCanonicalName() for Class arguments. Otherwise, ensure it's called only when debugging or in error cases. Fixes https://github.com/google/dagger/issues/819. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=172789896 --- java/dagger/android/AndroidInjection.java | 33 ++++++++----------- .../android/DispatchingAndroidInjector.java | 4 +-- .../support/AndroidSupportInjection.java | 17 ++++++---- java/dagger/internal/Preconditions.java | 12 ++++--- 4 files changed, 33 insertions(+), 33 deletions(-) diff --git a/java/dagger/android/AndroidInjection.java b/java/dagger/android/AndroidInjection.java index a3d92d8586a..102a429104a 100644 --- a/java/dagger/android/AndroidInjection.java +++ b/java/dagger/android/AndroidInjection.java @@ -16,6 +16,7 @@ package dagger.android; +import static android.util.Log.DEBUG; import static dagger.internal.Preconditions.checkNotNull; import android.app.Activity; @@ -53,10 +54,7 @@ public static void inject(Activity activity) { AndroidInjector activityInjector = ((HasActivityInjector) application).activityInjector(); - checkNotNull( - activityInjector, - "%s.activityInjector() returned null", - application.getClass().getCanonicalName()); + checkNotNull(activityInjector, "%s.activityInjector() returned null", application.getClass()); activityInjector.inject(activity); } @@ -85,18 +83,18 @@ public static void inject(Activity activity) { public static void inject(Fragment fragment) { checkNotNull(fragment, "fragment"); HasFragmentInjector hasFragmentInjector = findHasFragmentInjector(fragment); - Log.d( - TAG, - String.format( - "An injector for %s was found in %s", - fragment.getClass().getCanonicalName(), - hasFragmentInjector.getClass().getCanonicalName())); + if (Log.isLoggable(TAG, DEBUG)) { + Log.d( + TAG, + String.format( + "An injector for %s was found in %s", + fragment.getClass().getCanonicalName(), + hasFragmentInjector.getClass().getCanonicalName())); + } AndroidInjector fragmentInjector = hasFragmentInjector.fragmentInjector(); checkNotNull( - fragmentInjector, - "%s.fragmentInjector() returned null", - hasFragmentInjector.getClass().getCanonicalName()); + fragmentInjector, "%s.fragmentInjector() returned null", hasFragmentInjector.getClass()); fragmentInjector.inject(fragment); } @@ -138,10 +136,7 @@ public static void inject(Service service) { } AndroidInjector serviceInjector = ((HasServiceInjector) application).serviceInjector(); - checkNotNull( - serviceInjector, - "%s.serviceInjector() returned null", - application.getClass().getCanonicalName()); + checkNotNull(serviceInjector, "%s.serviceInjector() returned null", application.getClass()); serviceInjector.inject(service); } @@ -170,7 +165,7 @@ public static void inject(BroadcastReceiver broadcastReceiver, Context context) checkNotNull( broadcastReceiverInjector, "%s.broadcastReceiverInjector() returned null", - application.getClass().getCanonicalName()); + application.getClass()); broadcastReceiverInjector.inject(broadcastReceiver); } @@ -198,7 +193,7 @@ public static void inject(ContentProvider contentProvider) { checkNotNull( contentProviderInjector, "%s.contentProviderInjector() returned null", - application.getClass().getCanonicalName()); + application.getClass()); contentProviderInjector.inject(contentProvider); } diff --git a/java/dagger/android/DispatchingAndroidInjector.java b/java/dagger/android/DispatchingAndroidInjector.java index 1a9584f4b1d..a35fb9e699a 100644 --- a/java/dagger/android/DispatchingAndroidInjector.java +++ b/java/dagger/android/DispatchingAndroidInjector.java @@ -76,9 +76,7 @@ public boolean maybeInject(T instance) { try { AndroidInjector injector = checkNotNull( - factory.create(instance), - "%s.create(I) should not return null.", - factory.getClass().getCanonicalName()); + factory.create(instance), "%s.create(I) should not return null.", factory.getClass()); injector.inject(instance); return true; diff --git a/java/dagger/android/support/AndroidSupportInjection.java b/java/dagger/android/support/AndroidSupportInjection.java index 6a22fbd522c..f5e206e6062 100644 --- a/java/dagger/android/support/AndroidSupportInjection.java +++ b/java/dagger/android/support/AndroidSupportInjection.java @@ -16,6 +16,7 @@ package dagger.android.support; +import static android.util.Log.DEBUG; import static dagger.internal.Preconditions.checkNotNull; import android.app.Activity; @@ -54,19 +55,21 @@ public final class AndroidSupportInjection { public static void inject(Fragment fragment) { checkNotNull(fragment, "fragment"); HasSupportFragmentInjector hasSupportFragmentInjector = findHasFragmentInjector(fragment); - Log.d( - TAG, - String.format( - "An injector for %s was found in %s", - fragment.getClass().getCanonicalName(), - hasSupportFragmentInjector.getClass().getCanonicalName())); + if (Log.isLoggable(TAG, DEBUG)) { + Log.d( + TAG, + String.format( + "An injector for %s was found in %s", + fragment.getClass().getCanonicalName(), + hasSupportFragmentInjector.getClass().getCanonicalName())); + } AndroidInjector fragmentInjector = hasSupportFragmentInjector.supportFragmentInjector(); checkNotNull( fragmentInjector, "%s.supportFragmentInjector() returned null", - hasSupportFragmentInjector.getClass().getCanonicalName()); + hasSupportFragmentInjector.getClass()); fragmentInjector.inject(fragment); } diff --git a/java/dagger/internal/Preconditions.java b/java/dagger/internal/Preconditions.java index 99e180530c1..7b64da1a50a 100644 --- a/java/dagger/internal/Preconditions.java +++ b/java/dagger/internal/Preconditions.java @@ -58,7 +58,8 @@ public static T checkNotNull(T reference, String errorMessage) { * message is formed by replacing the single {@code %s} placeholder in the template with * {@code errorMessageArg}. * @param errorMessageArg the argument to be substituted into the message template. Converted to a - * string using {@link String#valueOf(Object)}. + * string using {@link String#valueOf(Object)}, except for {@link Class} objects, which use + * {@link Class#getCanonicalName()}. * @return the non-null reference that was validated * @throws NullPointerException if {@code reference} is null * @throws IllegalArgumentException if {@code errorMessageTemplate} doesn't contain exactly one @@ -67,7 +68,7 @@ public static T checkNotNull(T reference, String errorMessage) { public static T checkNotNull( T reference, String errorMessageTemplate, Object errorMessageArg) { if (reference == null) { - // Poor-persons version of String.format, which is not GWT-compatible + // Simple implementation of String.format, which is not GWT-compatible if (!errorMessageTemplate.contains("%s")) { throw new IllegalArgumentException("errorMessageTemplate has no format specifiers"); } @@ -75,8 +76,11 @@ public static T checkNotNull( throw new IllegalArgumentException( "errorMessageTemplate has more than one format specifier"); } - throw new NullPointerException( - errorMessageTemplate.replaceFirst("%s", String.valueOf(errorMessageArg))); + String argString = + errorMessageArg instanceof Class + ? ((Class) errorMessageArg).getCanonicalName() + : String.valueOf(errorMessageArg); + throw new NullPointerException(errorMessageTemplate.replace("%s", argString)); } return reference; }