From 7b56b7ede287de89672facd74773da588c95affb Mon Sep 17 00:00:00 2001 From: Christian Edward Gruber Date: Wed, 12 Jun 2013 11:19:36 -0700 Subject: [PATCH] Various fixes from review. --- .../java/dagger/internal/FailoverLoader.java | 30 ++++++++----------- .../src/main/java/dagger/internal/Loader.java | 3 +- .../main/java/dagger/internal/Modules.java | 2 ++ .../internal/loaders/GeneratedAdapters.java | 26 ++++++++-------- .../loaders/ReflectiveModuleAdapter.java | 4 +-- 5 files changed, 31 insertions(+), 34 deletions(-) diff --git a/core/src/main/java/dagger/internal/FailoverLoader.java b/core/src/main/java/dagger/internal/FailoverLoader.java index 7881e13f85d..be9b476acf6 100644 --- a/core/src/main/java/dagger/internal/FailoverLoader.java +++ b/core/src/main/java/dagger/internal/FailoverLoader.java @@ -1,6 +1,6 @@ /* - * Copyright (C) 2012 Square, Inc. - * Copyright (C) 2012 Google, Inc. + * Copyright (C) 2013 Square, Inc. + * Copyright (C) 2013 Google, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -53,23 +53,17 @@ public final class FailoverLoader implements Loader { ClassLoader classLoader, boolean mustHaveInjections) { try { Binding result = GeneratedAdapters.initInjectAdapter(className, classLoader); - if (result != null) { - return result; - } - - // Fall back on reflection. - Class c; - try { + if (result == null) { // A null classloader is the system classloader. classLoader = (classLoader != null) ? classLoader : ClassLoader.getSystemClassLoader(); - c = classLoader.loadClass(className); - } catch (ClassNotFoundException e) { - throw new RuntimeException(e); + Class c = classLoader.loadClass(className); + if (!c.isInterface()) { + result = ReflectiveAtInjectBinding.create(c, mustHaveInjections); + } } - if (c.isInterface()) { - return null; - } - return ReflectiveAtInjectBinding.create(c, mustHaveInjections); + return result; + } catch (ClassNotFoundException e) { + throw new RuntimeException(e); } catch (RuntimeException e) { logNotFound("Binding", className, e); throw e; @@ -80,9 +74,9 @@ public final class FailoverLoader implements Loader { try { StaticInjection result = GeneratedAdapters.initStaticInjection(injectedClass); if (result != null) { - return result; + result = ReflectiveStaticInjection.create(injectedClass); } - return ReflectiveStaticInjection.create(injectedClass); + return result; } catch (RuntimeException e) { logNotFound("Static injection", injectedClass.getName(), e); throw e; diff --git a/core/src/main/java/dagger/internal/Loader.java b/core/src/main/java/dagger/internal/Loader.java index 0250aa6688a..c2016a5e106 100644 --- a/core/src/main/java/dagger/internal/Loader.java +++ b/core/src/main/java/dagger/internal/Loader.java @@ -23,7 +23,8 @@ */ public interface Loader { /** - * Returns a binding that uses {@code @Inject} annotations, or null if no such binding can beNo. + * Returns a binding that uses {@code @Inject} annotations, or null if no valid binding can + * be found or created. */ Binding getAtInjectBinding( String key, String className, ClassLoader classLoader, boolean mustHaveInjections); diff --git a/core/src/main/java/dagger/internal/Modules.java b/core/src/main/java/dagger/internal/Modules.java index 9d25abe19bd..ef994cd315c 100644 --- a/core/src/main/java/dagger/internal/Modules.java +++ b/core/src/main/java/dagger/internal/Modules.java @@ -25,6 +25,8 @@ */ public final class Modules { + private Modules() { } + /** * Returns a full set of module adapters, including module adapters for included * modules. diff --git a/core/src/main/java/dagger/internal/loaders/GeneratedAdapters.java b/core/src/main/java/dagger/internal/loaders/GeneratedAdapters.java index 3ae2229685d..2682da0f6fa 100644 --- a/core/src/main/java/dagger/internal/loaders/GeneratedAdapters.java +++ b/core/src/main/java/dagger/internal/loaders/GeneratedAdapters.java @@ -1,5 +1,6 @@ /* - * Copyright (C) 2012 Square, Inc. + * Copyright (C) 2013 Square, Inc. + * Copyright (C) 2013 Google, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -27,7 +28,7 @@ * A utility for loading and initializing generated adapters. */ public final class GeneratedAdapters { - public static final String SEPARATOR = "$$"; + private static final String SEPARATOR = "$$"; public static final String INJECT_ADAPTER_SUFFIX = SEPARATOR + "InjectAdapter"; public static final String MODULE_ADAPTER_SUFFIX = SEPARATOR + "ModuleAdapter"; public static final String STATIC_INJECTION_SUFFIX = SEPARATOR + "StaticInjection"; @@ -36,40 +37,39 @@ public final class GeneratedAdapters { private GeneratedAdapters() { } public static ModuleAdapter initModuleAdapter(Class moduleClass) { - return instantiate(moduleClass.getName(), MODULE_ADAPTER_SUFFIX, moduleClass.getClassLoader()); + return instantiate(moduleClass.getName() + MODULE_ADAPTER_SUFFIX, moduleClass.getClassLoader()); } public static Binding initInjectAdapter(String className, ClassLoader classLoader) { - return instantiate(className, INJECT_ADAPTER_SUFFIX, classLoader); + return instantiate(className + INJECT_ADAPTER_SUFFIX, classLoader); } public static StaticInjection initStaticInjection(Class injectedClass) { - return instantiate(injectedClass.getName(), STATIC_INJECTION_SUFFIX, + return instantiate(injectedClass.getName() + STATIC_INJECTION_SUFFIX, injectedClass.getClassLoader()); } - private static T instantiate(String className, String suffix, ClassLoader classLoader) { - String name = className + suffix; + private static T instantiate(String name, ClassLoader classLoader) { try { // A null classloader is the system classloader. classLoader = (classLoader != null) ? classLoader : ClassLoader.getSystemClassLoader(); Class generatedClass = classLoader.loadClass(name); - Constructor constructor = generatedClass.getConstructor(); + Constructor constructor = generatedClass.getDeclaredConstructor(); constructor.setAccessible(true); return (T) constructor.newInstance(); } catch (ClassNotFoundException e) { if (logger.isLoggable(Level.FINE)) { - logger.log(Level.FINE, className + " could not be found.", e); + logger.log(Level.FINE, name + " could not be found.", e); } return null; // Not finding a class is not inherently an error, unlike finding a bad class. } catch (NoSuchMethodException e) { - throw new RuntimeException("No valid constructor found on " + className, e); + throw new RuntimeException("No default constructor found on " + name, e); } catch (InstantiationException e) { - throw new RuntimeException("Failed to initialize " + className, e); + throw new RuntimeException("Failed to initialize " + name, e); } catch (IllegalAccessException e) { - throw new RuntimeException("Failed to initialize " + className, e); + throw new RuntimeException("Failed to initialize " + name, e); } catch (InvocationTargetException e) { - throw new RuntimeException("Error while initializing " + className, e.getCause()); + throw new RuntimeException("Error while initializing " + name, e.getCause()); } } } diff --git a/core/src/main/java/dagger/internal/loaders/ReflectiveModuleAdapter.java b/core/src/main/java/dagger/internal/loaders/ReflectiveModuleAdapter.java index 2de8165457f..1ed3ee9f284 100644 --- a/core/src/main/java/dagger/internal/loaders/ReflectiveModuleAdapter.java +++ b/core/src/main/java/dagger/internal/loaders/ReflectiveModuleAdapter.java @@ -60,7 +60,7 @@ private static String[] injectableTypesToKeys(Class[] injectableTypes) { } @Override public void getBindings(Map> bindings) { - for (Class c = moduleClass; c != Object.class; c = c.getSuperclass()) { + for (Class c = moduleClass; !c.equals(Object.class); c = c.getSuperclass()) { for (Method method : c.getDeclaredMethods()) { Provides provides = method.getAnnotation(Provides.class); if (provides != null) { @@ -138,7 +138,7 @@ public static ModuleAdapter createAdaptor(Class moduleClass) if (annotation == null) { throw new IllegalArgumentException("No @Module on " + moduleClass.getName()); } - if (moduleClass.getSuperclass() != Object.class) { + if (!moduleClass.getSuperclass().equals(Object.class)) { throw new IllegalArgumentException( "Modules must not extend from other classes: " + moduleClass.getName()); }