Change delayed injection to only change behavior of its own elements

- Introduce new BeforeInjecting annotation to not modify the (expected) order of annotations that do not belong to us
  - Avoids using hacky way of first initializing field to an Answer that will delegate on demand to the proper class...
- Remove PostConstruct support for Mockito's InjectMocks: we should not change the established behavior of external elements

src/test/java/fr/xephi/authme/DelayedInjectionRunner.java

@@ -1,154 +0,0 @@
-package fr.xephi.authme;
-
-import fr.xephi.authme.initialization.Injection;
-import fr.xephi.authme.initialization.InjectionHelper;
-import org.junit.runner.notification.RunNotifier;
-import org.junit.runners.BlockJUnit4ClassRunner;
-import org.junit.runners.model.FrameworkField;
-import org.junit.runners.model.FrameworkMethod;
-import org.junit.runners.model.InitializationError;
-import org.junit.runners.model.Statement;
-import org.mockito.InjectMocks;
-import org.mockito.Mock;
-import org.mockito.Mockito;
-import org.mockito.MockitoAnnotations;
-import org.mockito.internal.runners.util.FrameworkUsageValidator;
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
-
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
-/**
- * Custom JUnit runner which adds support for {@link DelayedInject}, along with Mockito's
- * {@link Mock}, {@link org.mockito.Spy} and {@link org.mockito.InjectMocks}.
- * <p>
- * Unlike Mockito's &#64;InjectMocks, fields annotated with {@link DelayedInject} will be
- * instantiated only right before a method is invoked on them. This allows a developer to
- * define the behavior of mocks the test class depends on. With {@link org.mockito.InjectMocks},
- * fields are instantiated even before {@link org.junit.Before} methods, making it impossible
- * to define behavior before the class is instantiated.
- * <p>
- * Note that it is required to declare all dependencies of classes annotated with
- * {@link DelayedInject} as {@link Mock} fields. If a dependency is missing, an exception
- * will be thrown.
- * <p>
- * Additionally, this runner adds support for {@link javax.annotation.PostConstruct} methods,
- * both for Mockito's &#64;InjectMocks and the custom &#64;DelayedInject.
- */
-public class DelayedInjectionRunner extends BlockJUnit4ClassRunner {
-
-    public DelayedInjectionRunner(Class<?> clazz) throws InitializationError {
-        super(clazz);
-    }
-
-    @Override
-    public Statement withBefores(FrameworkMethod method, Object target, Statement statement) {
-        // Initialize all mocks
-        MockitoAnnotations.initMocks(target);
-
-        // Add support for @DelayedInject and @PostConstruct
-        runPostConstructOnInjectMocksFields(target);
-        initializeDelayedMocks(target);
-
-        // Send to parent
-        return super.withBefores(method, target, statement);
-    }
-
-    @Override
-    public void run(final RunNotifier notifier) {
-        // add listener that validates framework usage at the end of each test
-        notifier.addListener(new FrameworkUsageValidator(notifier));
-        super.run(notifier);
-    }
-
-    private void runPostConstructOnInjectMocksFields(Object target) {
-        List<FrameworkField> delayedFields = getTestClass().getAnnotatedFields(InjectMocks.class);
-        for (FrameworkField field : delayedFields) {
-            Object o = ReflectionTestUtils.getFieldValue(field.getField(), target);
-            executePostConstructMethod(o);
-        }
-    }
-
-    private void initializeDelayedMocks(Object target) {
-        List<FrameworkField> delayedFields = getTestClass().getAnnotatedFields(DelayedInject.class);
-        for (FrameworkField field : delayedFields) {
-            setUpField(target, field.getField());
-        }
-    }
-
-    private void setUpField(Object target, Field field) {
-        final Injection<?> injection = InjectionHelper.getInjection(field.getType());
-        if (injection == null) {
-            throw new IllegalStateException("No injection method available for field '" + field.getName() + "'");
-        }
-        final Object[] dependencies = fulfillDependencies(target, injection.getDependencies());
-
-        Object delayedInjectionMock = Mockito.mock(field.getType(),
-            new DelayedInstantiatingAnswer(injection, dependencies));
-        ReflectionTestUtils.setField(field, target, delayedInjectionMock);
-    }
-
-    private Object[] fulfillDependencies(Object target, Class<?>[] dependencies) {
-        List<FrameworkField> availableMocks = getTestClass().getAnnotatedFields(Mock.class);
-        Map<Class<?>, Object> mocksByType = new HashMap<>();
-        for (FrameworkField frameworkField : availableMocks) {
-            Field field = frameworkField.getField();
-            Object fieldValue = ReflectionTestUtils.getFieldValue(field, target);
-            mocksByType.put(field.getType(), fieldValue);
-        }
-
-        Object[] resolvedValues = new Object[dependencies.length];
-        for (int i = 0; i < dependencies.length; ++i) {
-            Object o = mocksByType.get(dependencies[i]);
-            if (o == null) {
-                throw new IllegalStateException("No mock found for '" + dependencies[i] + "'. "
-                    + "All dependencies of @DelayedInject must be provided as @Mock fields");
-            }
-            resolvedValues[i] = o;
-        }
-        return resolvedValues;
-    }
-
-    /**
-     * Executes the class' PostConstruct method if available. Validates that all rules for
-     * {@link javax.annotation.PostConstruct} are met.
-     *
-     * @param object the object whose PostConstruct method should be run, if available
-     * @see InjectionHelper#getAndValidatePostConstructMethod
-     */
-    private static void executePostConstructMethod(Object object) {
-        Method postConstructMethod = InjectionHelper.getAndValidatePostConstructMethod(object.getClass());
-        if (postConstructMethod != null) {
-            ReflectionTestUtils.invokeMethod(postConstructMethod, object);
-        }
-    }
-
-    private static final class DelayedInstantiatingAnswer implements Answer<Object> {
-
-        private final Injection<?> injection;
-        private final Object[] dependencies;
-        private Object realObject;
-
-        public DelayedInstantiatingAnswer(Injection<?> injection, Object... dependencies) {
-            this.injection = injection;
-            this.dependencies = dependencies;
-        }
-
-        @Override
-        public Object answer(InvocationOnMock invocation) throws Throwable {
-            if (realObject == null) {
-                Object realObject = injection.instantiateWith(dependencies);
-                executePostConstructMethod(realObject);
-                this.realObject = realObject;
-            }
-
-            Method method = invocation.getMethod();
-            return ReflectionTestUtils.invokeMethod(method, realObject, invocation.getArguments());
-        }
-    }
-
-}

src/test/java/fr/xephi/authme/command/CommandMapperTest.java

@@ -1,15 +1,15 @@
 package fr.xephi.authme.command;
 
-import fr.xephi.authme.DelayedInject;
-import fr.xephi.authme.DelayedInjectionRunner;
 import fr.xephi.authme.command.TestCommandsUtil.TestLoginCommand;
 import fr.xephi.authme.command.TestCommandsUtil.TestRegisterCommand;
 import fr.xephi.authme.command.TestCommandsUtil.TestUnregisterCommand;
 import fr.xephi.authme.command.executable.HelpCommand;
 import fr.xephi.authme.permission.PermissionNode;
 import fr.xephi.authme.permission.PermissionsManager;
+import fr.xephi.authme.runner.BeforeInjecting;
+import fr.xephi.authme.runner.InjectDelayed;
+import fr.xephi.authme.runner.DelayedInjectionRunner;
 import org.bukkit.command.CommandSender;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -41,7 +41,7 @@ public class CommandMapperTest {
 
     private static Set<CommandDescription> commands;
 
-    @DelayedInject
+    @InjectDelayed
     private CommandMapper mapper;
 
     @Mock
@@ -55,7 +55,7 @@ public class CommandMapperTest {
         commands = TestCommandsUtil.generateCommands();
     }
 
-    @Before
+    @BeforeInjecting
     public void setUpMocks() {
         given(commandInitializer.getCommands()).willReturn(commands);
     }

src/test/java/fr/xephi/authme/listener/ListenerServiceTest.java

@@ -1,10 +1,11 @@
 package fr.xephi.authme.listener;
 
-import fr.xephi.authme.DelayedInject;
-import fr.xephi.authme.DelayedInjectionRunner;
 import fr.xephi.authme.cache.auth.PlayerCache;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.hooks.PluginHooks;
+import fr.xephi.authme.runner.BeforeInjecting;
+import fr.xephi.authme.runner.InjectDelayed;
+import fr.xephi.authme.runner.DelayedInjectionRunner;
 import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.properties.RegistrationSettings;
 import fr.xephi.authme.settings.properties.RestrictionSettings;
@@ -13,7 +14,6 @@ import org.bukkit.entity.Player;
 import org.bukkit.event.HandlerList;
 import org.bukkit.event.entity.EntityEvent;
 import org.bukkit.event.player.PlayerEvent;
-import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.Mock;
@@ -33,7 +33,7 @@ import static org.mockito.Mockito.verifyZeroInteractions;
 @RunWith(DelayedInjectionRunner.class)
 public class ListenerServiceTest {
 
-    @DelayedInject
+    @InjectDelayed
     private ListenerService listenerService;
 
     @Mock
@@ -48,8 +48,8 @@ public class ListenerServiceTest {
     @Mock
     private PlayerCache playerCache;
 
-    @Before
+    @BeforeInjecting
-    public void initializeTestSetup() {
+    public void initializeDefaultSettings() {
         given(settings.getProperty(RegistrationSettings.FORCE)).willReturn(true);
         given(settings.getProperty(RestrictionSettings.UNRESTRICTED_NAMES)).willReturn(
             Arrays.asList("npc1", "npc2", "npc3"));
@@ -127,6 +127,7 @@ public class ListenerServiceTest {
         given(settings.getProperty(RegistrationSettings.FORCE)).willReturn(false);
         EntityEvent event = mock(EntityEvent.class);
         given(event.getEntity()).willReturn(player);
+        listenerService.loadSettings(settings);
 
         // when
         boolean result = listenerService.shouldCancelEvent(event);

src/test/java/fr/xephi/authme/runner/BeforeInjecting.java

@@ -0,0 +1,14 @@
+package fr.xephi.authme.runner;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * Marks methods to run before {@link InjectDelayed} fields are instantiated.
+ */
+@Target(ElementType.METHOD)
+@Retention(RetentionPolicy.RUNTIME)
+public @interface BeforeInjecting {
+}

src/test/java/fr/xephi/authme/runner/DelayedInjectionRunner.java

@@ -0,0 +1,139 @@
+package fr.xephi.authme.runner;
+
+import fr.xephi.authme.ReflectionTestUtils;
+import fr.xephi.authme.initialization.Injection;
+import fr.xephi.authme.initialization.InjectionHelper;
+import org.junit.runner.notification.RunNotifier;
+import org.junit.runners.BlockJUnit4ClassRunner;
+import org.junit.runners.model.FrameworkField;
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.InitializationError;
+import org.junit.runners.model.Statement;
+import org.mockito.Mock;
+import org.mockito.MockitoAnnotations;
+import org.mockito.internal.runners.util.FrameworkUsageValidator;
+
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Custom JUnit runner which adds support for {@link InjectDelayed} and {@link BeforeInjecting}.
+ * This runner also initializes fields with Mockito's {@link Mock}, {@link org.mockito.Spy} and
+ * {@link org.mockito.InjectMocks}.
+ * <p>
+ * Mockito's {@link Mock} and {@link org.mockito.InjectMocks} are initialized <i>before</i>
+ * {@link org.junit.Before} methods are run. This leaves no possibility to initialize some mock
+ * behavior before {@link org.mockito.InjectMocks} fields get instantiated.
+ * <p>
+ * The runner fills this gap by introducing {@link BeforeInjecting}. At the time these methods
+ * are run Mockito's annotation will have taken effect but not {@link InjectDelayed}. Fields with
+ * this annotation are initialized after {@link BeforeInjecting} methods have been run.
+ * <p>
+ * Additionally, after a field annotated with {@link InjectDelayed} has been initialized, its
+ * {@link javax.annotation.PostConstruct} method will be invoked, if available.
+ * <p>
+ * Important: It is required to declare all dependencies of classes annotated with
+ * {@link InjectDelayed} as {@link Mock} fields. If a dependency is missing, an exception
+ * will be thrown.
+ */
+public class DelayedInjectionRunner extends BlockJUnit4ClassRunner {
+
+    public DelayedInjectionRunner(Class<?> clazz) throws InitializationError {
+        super(clazz);
+    }
+
+    @Override
+    public Statement withBefores(FrameworkMethod method, Object target, Statement statement) {
+        // Initialize all Mockito annotations
+        MockitoAnnotations.initMocks(target);
+
+        // Call chain normally: let parent handle @Before methods.
+        // Note that the chain of statements will be run from the end to the start,
+        // so @Before will be run AFTER our custom statements below
+        statement = super.withBefores(method, target, statement);
+
+        // Add support for @BeforeInjecting and @InjectDelayed (again, reverse order)
+        statement = withDelayedInjects(target, statement);
+        return withBeforeInjectings(target, statement);
+    }
+
+    @Override
+    public void run(final RunNotifier notifier) {
+        // add listener that validates framework usage at the end of each test
+        notifier.addListener(new FrameworkUsageValidator(notifier));
+        super.run(notifier);
+    }
+
+    /* Adds a Statement to the chain if @BeforeInjecting methods are present. */
+    private Statement withBeforeInjectings(Object target, Statement statement) {
+        List<FrameworkMethod> beforeInjectings = getTestClass().getAnnotatedMethods(BeforeInjecting.class);
+        return beforeInjectings.isEmpty()
+            ? statement
+            : new RunBeforeInjectings(statement, beforeInjectings, target);
+    }
+
+    /*
+     * Adds a Statement to the chain if @InjectDelayed methods are present.
+     * If fields have been found, the injection for the type is resolved and stored with the necessary dependencies.
+     */
+    private Statement withDelayedInjects(Object target, Statement statement) {
+        List<FrameworkField> delayedFields = getTestClass().getAnnotatedFields(InjectDelayed.class);
+        if (delayedFields.isEmpty()) {
+            return statement;
+        }
+
+        List<PendingInjection> pendingInjections = new ArrayList<>(delayedFields.size());
+        for (FrameworkField field : delayedFields) {
+            pendingInjections.add(createPendingInjection(target, field.getField()));
+        }
+        return new RunDelayedInjects(statement, pendingInjections, target);
+    }
+
+    /**
+     * Creates a {@link PendingInjection} for the given field's type, using the target's values.
+     *
+     * @param target the target to get dependencies from
+     * @param field the field to prepare an injection for
+     * @return the resulting object
+     */
+    private PendingInjection createPendingInjection(Object target, Field field) {
+        final Injection<?> injection = InjectionHelper.getInjection(field.getType());
+        if (injection == null) {
+            throw new IllegalStateException("No injection method available for field '" + field.getName() + "'");
+        }
+        final Object[] dependencies = fulfillDependencies(target, injection.getDependencies());
+        return new PendingInjection(field, injection, dependencies);
+    }
+
+    /**
+     * Returns a list of all objects for the given list of dependencies, retrieved from the given
+     * target's {@link @Mock} fields.
+     *
+     * @param target the target to get the required dependencies from
+     * @param dependencies the required dependency types
+     * @return the resolved dependencies
+     */
+    private Object[] fulfillDependencies(Object target, Class<?>[] dependencies) {
+        List<FrameworkField> availableMocks = getTestClass().getAnnotatedFields(Mock.class);
+        Map<Class<?>, Object> mocksByType = new HashMap<>();
+        for (FrameworkField frameworkField : availableMocks) {
+            Field field = frameworkField.getField();
+            Object fieldValue = ReflectionTestUtils.getFieldValue(field, target);
+            mocksByType.put(field.getType(), fieldValue);
+        }
+
+        Object[] resolvedValues = new Object[dependencies.length];
+        for (int i = 0; i < dependencies.length; ++i) {
+            Object o = mocksByType.get(dependencies[i]);
+            if (o == null) {
+                throw new IllegalStateException("No mock found for '" + dependencies[i] + "'. "
+                    + "All dependencies of @InjectDelayed must be provided as @Mock fields");
+            }
+            resolvedValues[i] = o;
+        }
+        return resolvedValues;
+    }
+}

src/test/java/fr/xephi/authme/runner/InjectDelayed.java

@@ -1,4 +1,4 @@
-package fr.xephi.authme;
+package fr.xephi.authme.runner;
 
 import java.lang.annotation.ElementType;
 import java.lang.annotation.Retention;
@@ -6,11 +6,11 @@ import java.lang.annotation.RetentionPolicy;
 import java.lang.annotation.Target;
 
 /**
- * Marks fields to be instantiated right before a method is invoked on them for the first time.
+ * Marks fields to instantiate with mocks after {@link BeforeInjecting} methods.
  *
  * @see DelayedInjectionRunner
  */
 @Target(ElementType.FIELD)
 @Retention(RetentionPolicy.RUNTIME)
-public @interface DelayedInject {
+public @interface InjectDelayed {
 }

src/test/java/fr/xephi/authme/runner/PendingInjection.java

@@ -0,0 +1,67 @@
+package fr.xephi.authme.runner;
+
+import fr.xephi.authme.ReflectionTestUtils;
+import fr.xephi.authme.initialization.Injection;
+import fr.xephi.authme.initialization.InjectionHelper;
+
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+
+/**
+ * Contains all necessary information to initialize a {@link InjectDelayed} field.
+ */
+public class PendingInjection {
+
+    private Field field;
+    private Object[] dependencies;
+    private Injection<?> injection;
+
+    public PendingInjection(Field field, Injection<?> injection, Object[] dependencies) {
+        this.field = field;
+        this.injection = injection;
+        this.dependencies = dependencies;
+    }
+
+    /**
+     * Constructs an object with the stored injection information.
+     *
+     * @return the constructed object
+     */
+    public Object instantiate() {
+        Object object = injection.instantiateWith(dependencies);
+        executePostConstructMethod(object);
+        return object;
+    }
+
+    /**
+     * Returns the field the constructed object should be assigned to.
+     *
+     * @return the field in the test class
+     */
+    public Field getField() {
+        return field;
+    }
+
+    /**
+     * Clears all fields (avoids keeping a reference to all dependencies).
+     */
+    public void clearFields() {
+        field = null;
+        dependencies = null;
+        injection = null;
+    }
+
+    /**
+     * Executes the class' PostConstruct method if available. Validates that all rules for
+     * {@link javax.annotation.PostConstruct} are met.
+     *
+     * @param object the object whose PostConstruct method should be run, if available
+     * @see InjectionHelper#getAndValidatePostConstructMethod
+     */
+    private static void executePostConstructMethod(Object object) {
+        Method postConstructMethod = InjectionHelper.getAndValidatePostConstructMethod(object.getClass());
+        if (postConstructMethod != null) {
+            ReflectionTestUtils.invokeMethod(postConstructMethod, object);
+        }
+    }
+}

src/test/java/fr/xephi/authme/runner/RunBeforeInjectings.java

@@ -0,0 +1,33 @@
+package fr.xephi.authme.runner;
+
+import fr.xephi.authme.ReflectionTestUtils;
+import org.junit.runners.model.FrameworkMethod;
+import org.junit.runners.model.Statement;
+
+import java.util.List;
+
+/**
+ * Statement for running {@link BeforeInjecting} methods. Such methods are run
+ * after Mockito's @Mock, @Spy and @InjectMocks have taken effect,
+ * but before {@link InjectDelayed} fields are handled.
+ */
+public class RunBeforeInjectings extends Statement {
+
+    private final Statement next;
+    private final List<FrameworkMethod> beforeInjectings;
+    private final Object target;
+
+    public RunBeforeInjectings(Statement next, List<FrameworkMethod> beforeInjectings, Object target) {
+        this.next = next;
+        this.beforeInjectings = beforeInjectings;
+        this.target = target;
+    }
+
+    @Override
+    public void evaluate() throws Throwable {
+        for (FrameworkMethod method : beforeInjectings) {
+            ReflectionTestUtils.invokeMethod(method.getMethod(), target);
+        }
+        next.evaluate();
+    }
+}

src/test/java/fr/xephi/authme/runner/RunDelayedInjects.java

@@ -0,0 +1,33 @@
+package fr.xephi.authme.runner;
+
+import fr.xephi.authme.ReflectionTestUtils;
+import org.junit.runners.model.Statement;
+
+import java.util.List;
+
+/**
+ * Statement for initializing {@link InjectDelayed} fields. These fields are
+ * constructed after {@link BeforeInjecting} and before JUnit's @Before.
+ */
+public class RunDelayedInjects extends Statement {
+
+    private final Statement next;
+    private final Object target;
+    private final List<PendingInjection> pendingInjections;
+
+    public RunDelayedInjects(Statement next, List<PendingInjection> pendingInjections, Object target) {
+        this.next = next;
+        this.pendingInjections = pendingInjections;
+        this.target = target;
+    }
+
+    @Override
+    public void evaluate() throws Throwable {
+        for (PendingInjection pendingInjection : pendingInjections) {
+            Object object = pendingInjection.instantiate();
+            ReflectionTestUtils.setField(pendingInjection.getField(), target, object);
+            pendingInjection.clearFields();
+        }
+        next.evaluate();
+    }
+}

src/test/java/fr/xephi/authme/task/PurgeServiceTest.java

@@ -1,13 +1,14 @@
 package fr.xephi.authme.task;
 
-import fr.xephi.authme.DelayedInject;
-import fr.xephi.authme.DelayedInjectionRunner;
 import fr.xephi.authme.ReflectionTestUtils;
 import fr.xephi.authme.TestHelper;
 import fr.xephi.authme.datasource.DataSource;
 import fr.xephi.authme.hooks.PluginHooks;
 import fr.xephi.authme.permission.PermissionsManager;
 import fr.xephi.authme.permission.PlayerStatePermission;
+import fr.xephi.authme.runner.BeforeInjecting;
+import fr.xephi.authme.runner.InjectDelayed;
+import fr.xephi.authme.runner.DelayedInjectionRunner;
 import fr.xephi.authme.settings.NewSetting;
 import fr.xephi.authme.settings.properties.PurgeSettings;
 import fr.xephi.authme.util.BukkitService;
@@ -16,7 +17,6 @@ import org.bukkit.Server;
 import org.bukkit.command.CommandSender;
 import org.bukkit.entity.Player;
 import org.hamcrest.Matchers;
-import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -52,7 +52,7 @@ import static org.mockito.Mockito.verifyZeroInteractions;
 @RunWith(DelayedInjectionRunner.class)
 public class PurgeServiceTest {
 
-    @DelayedInject
+    @InjectDelayed
     private PurgeService purgeService;
 
     @Mock
@@ -73,7 +73,7 @@ public class PurgeServiceTest {
         TestHelper.setupLogger();
     }
 
-    @Before
+    @BeforeInjecting
     public void initSettingDefaults() {
         given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(60);
     }
@@ -95,6 +95,7 @@ public class PurgeServiceTest {
         // given
         given(settings.getProperty(PurgeSettings.USE_AUTO_PURGE)).willReturn(true);
         given(settings.getProperty(PurgeSettings.DAYS_BEFORE_REMOVE_PLAYER)).willReturn(0);
+        purgeService.reload();
 
         // when
         purgeService.runAutoPurge();