Injector - don't use instantiation fallback if PostConstruct method is present

- Do not instantiate classes with instantiation fallback if they have a PostConstruct method - thanks @sgdc3 for the hint
- Change Injector test to check exception messages also

src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java

@@ -260,7 +260,7 @@ public class AuthMeServiceInitializer {
                 postConstructMethod.setAccessible(true);
                 postConstructMethod.invoke(object);
             } catch (IllegalAccessException | InvocationTargetException e) {
-                throw new UnsupportedOperationException(e);
+                throw new UnsupportedOperationException("Error executing @PostConstruct method", e);
             }
         }
     }
@@ -287,7 +287,8 @@ public class AuthMeServiceInitializer {
                     throw new IllegalStateException("@PostConstruct method may not be static or have any parameters. "
                         + "Invalid method in " + clazz);
                 } else if (method.getReturnType() != void.class) {
-                    throw new IllegalStateException("@PostConstruct method must be void. Offending class: " + clazz);
+                    throw new IllegalStateException("@PostConstruct method must have return type void. "
+                        + "Offending class: " + clazz);
                 } else {
                     postConstructMethod = method;
                 }

src/main/java/fr/xephi/authme/initialization/InstantiationFallback.java

@@ -1,5 +1,6 @@
 package fr.xephi.authme.initialization;
 
+import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 import javax.inject.Provider;
 import java.lang.reflect.AccessibleObject;
@@ -8,7 +9,7 @@ import java.lang.reflect.InvocationTargetException;
 
 /**
  * Fallback instantiation method for classes with an accessible no-args constructor
- * and no no {@link Inject} annotations whatsoever.
+ * and no elements whatsoever annotated with {@link Inject} or {@link PostConstruct}.
  */
 public class InstantiationFallback<T> implements Injection<T> {
 
@@ -54,9 +55,9 @@ public class InstantiationFallback<T> implements Injection<T> {
                 Constructor<T> noArgsConstructor = getNoArgsConstructor(clazz);
                 // Return fallback only if we have no args constructor and no @Inject annotation anywhere
                 if (noArgsConstructor != null
-                    && !isInjectAnnotationPresent(clazz.getDeclaredConstructors())
-                    && !isInjectAnnotationPresent(clazz.getDeclaredFields())
-                    && !isInjectAnnotationPresent(clazz.getDeclaredMethods())) {
+                    && !isInjectionAnnotationPresent(clazz.getDeclaredConstructors())
+                    && !isInjectionAnnotationPresent(clazz.getDeclaredFields())
+                    && !isInjectionAnnotationPresent(clazz.getDeclaredMethods())) {
                     return new InstantiationFallback<>(noArgsConstructor);
                 }
                 return null;
@@ -73,9 +74,9 @@ public class InstantiationFallback<T> implements Injection<T> {
         }
     }
 
-    private static <A extends AccessibleObject> boolean isInjectAnnotationPresent(A[] accessibles) {
+    private static <A extends AccessibleObject> boolean isInjectionAnnotationPresent(A[] accessibles) {
         for (A accessible : accessibles) {
-            if (accessible.isAnnotationPresent(Inject.class)) {
+            if (accessible.isAnnotationPresent(Inject.class) || accessible.isAnnotationPresent(PostConstruct.class)) {
                 return true;
             }
         }

src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java

@@ -18,8 +18,11 @@ import fr.xephi.authme.initialization.samples.ProvidedClass;
 import fr.xephi.authme.initialization.samples.Size;
 import fr.xephi.authme.settings.NewSetting;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.not;
 import static org.hamcrest.Matchers.nullValue;
@@ -36,6 +39,11 @@ public class AuthMeServiceInitializerTest {
 
     private AuthMeServiceInitializer initializer;
 
+    // As we test many cases that throw exceptions, we use JUnit's ExpectedException Rule
+    // to make sure that we receive the exception we expect
+    @Rule
+    public ExpectedException expectedException = ExpectedException.none();
+
     @Before
     public void setInitializer() {
         initializer = new AuthMeServiceInitializer(ALLOWED_PACKAGE);
@@ -54,15 +62,17 @@ public class AuthMeServiceInitializerTest {
         }
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForInvalidPackage() {
         // given / when / then
+        expectRuntimeExceptionWith("outside of the allowed packages");
         initializer.get(InvalidClass.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForUnregisteredPrimitiveType() {
         // given / when / then
+        expectRuntimeExceptionWith("Primitive types must be provided");
         initializer.get(int.class);
     }
 
@@ -85,24 +95,27 @@ public class AuthMeServiceInitializerTest {
         assertThat(object.getGammaService(), equalTo(initializer.get(BetaManager.class).getDependencies()[1]));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldRecognizeCircularReferences() {
         // given / when / then
+        expectRuntimeExceptionWith("Found cyclic dependency");
         initializer.get(CircularClasses.Circular3.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForUnregisteredAnnotation() {
         // given
         initializer.provide(Size.class, 4523);
 
         // when / then
+        expectRuntimeExceptionWith("must be registered beforehand");
         initializer.get(ClassWithAnnotations.class);
     }
 
-    @Test(expected = RuntimeException.class)
-    public void shouldThrowForFieldInjectionWithNoDefaultConstructor() {
+    @Test
+    public void shouldThrowForFieldInjectionWithoutNoArgsConstructor() {
         // given / when / then
+        expectRuntimeExceptionWith("Did not find injection method");
         initializer.get(BadFieldInjection.class);
     }
 
@@ -124,36 +137,41 @@ public class AuthMeServiceInitializerTest {
             equalTo(result.getBetaManager().getDependencies()[1]));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForAnnotationAsKey() {
         // given / when / then
+        expectRuntimeExceptionWith("Cannot retrieve annotated elements in this way");
         initializer.get(Size.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForSecondRegistration() {
         // given / when / then
+        expectRuntimeExceptionWith("There is already an object present");
         initializer.register(ProvidedClass.class, new ProvidedClass(""));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForSecondAnnotationRegistration() {
         // given
         initializer.provide(Size.class, 12);
 
         // when / then
+        expectRuntimeExceptionWith("already registered");
         initializer.provide(Size.class, -8);
     }
 
-    @Test(expected = NullPointerException.class)
+    @Test
     public void shouldThrowForNullValueAssociatedToAnnotation() {
         // given / when / then
+        expectedException.expect(NullPointerException.class);
         initializer.provide(Duration.class, null);
     }
 
-    @Test(expected = NullPointerException.class)
+    @Test
     public void shouldThrowForRegisterWithNull() {
         // given / when / then
+        expectedException.expect(NullPointerException.class);
         initializer.register(String.class, null);
     }
 
@@ -170,39 +188,45 @@ public class AuthMeServiceInitializerTest {
         assertThat(testClass.getBetaManager(), not(nullValue()));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForInvalidPostConstructMethod() {
         // given / when / then
+        expectRuntimeExceptionWith("@PostConstruct method may not be static or have any parameters");
         initializer.get(InvalidPostConstruct.WithParams.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForStaticPostConstructMethod() {
         // given / when / then
+        expectRuntimeExceptionWith("@PostConstruct method may not be static or have any parameters");
         initializer.get(InvalidPostConstruct.Static.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldForwardExceptionFromPostConstruct() {
         // given / when / then
+        expectRuntimeExceptionWith("Error executing @PostConstruct method");
         initializer.get(InvalidPostConstruct.ThrowsException.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForMultiplePostConstructMethods() {
         // given / when / then
+        expectRuntimeExceptionWith("Multiple methods with @PostConstruct");
         initializer.get(InvalidPostConstruct.MultiplePostConstructs.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForPostConstructNotReturningVoid() {
         // given / when / then
+        expectRuntimeExceptionWith("@PostConstruct method must have return type void");
         initializer.get(InvalidPostConstruct.NotVoidReturnType.class);
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForAbstractNonRegisteredDependency() {
         // given / when / then
+        expectRuntimeExceptionWith("cannot be instantiated");
         initializer.get(ClassWithAbstractDependency.class);
     }
 
@@ -220,12 +244,13 @@ public class AuthMeServiceInitializerTest {
         assertThat(cwad.getAlphaService(), not(nullValue()));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForAlreadyRegisteredClass() {
         // given
         initializer.register(BetaManager.class, new BetaManager());
 
         // when / then
+        expectRuntimeExceptionWith("There is already an object present");
         initializer.register(BetaManager.class, new BetaManager());
     }
 
@@ -241,9 +266,10 @@ public class AuthMeServiceInitializerTest {
         assertThat(singletonScoped, not(sameInstance(requestScoped)));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForStaticFieldInjection() {
         // given / when / then
+        expectRuntimeExceptionWith("is static but annotated with @Inject");
         initializer.newInstance(InvalidStaticFieldInjection.class);
     }
 
@@ -283,10 +309,16 @@ public class AuthMeServiceInitializerTest {
         assertThat(providedClass.getWasReloaded(), equalTo(true));
     }
 
-    @Test(expected = RuntimeException.class)
+    @Test
     public void shouldThrowForNullSetting() {
         // given / when / then
+        expectRuntimeExceptionWith("Settings instance is null");
         initializer.performReloadOnServices();
     }
 
+    private void expectRuntimeExceptionWith(String message) {
+        expectedException.expect(RuntimeException.class);
+        expectedException.expectMessage(containsString(message));
+    }
+
 }

src/test/java/fr/xephi/authme/initialization/FieldInjectionTest.java

@@ -115,11 +115,9 @@ public class FieldInjectionTest {
     }
 
     private static class ThrowingConstructor {
-        @SuppressWarnings("unused")
         @Inject
         private ProvidedClass providedClass;
 
-        @SuppressWarnings("unused")
         public ThrowingConstructor() {
             throw new UnsupportedOperationException("Exception in constructor");
         }

src/test/java/fr/xephi/authme/initialization/InstantiationFallbackTest.java

@@ -65,4 +65,14 @@ public class InstantiationFallbackTest {
         assertThat(instantiation, nullValue());
     }
 
+    @Test
+    public void shouldReturnNullForClassWithPostConstruct() {
+        // given / when
+        Injection<InstantiationFallbackClasses.ClassWithPostConstruct> instantiation =
+            InstantiationFallback.provide(InstantiationFallbackClasses.ClassWithPostConstruct.class).get();
+
+        // then
+        assertThat(instantiation, nullValue());
+    }
+
 }

src/test/java/fr/xephi/authme/initialization/samples/InstantiationFallbackClasses.java

@@ -1,9 +1,10 @@
 package fr.xephi.authme.initialization.samples;
 
+import javax.annotation.PostConstruct;
 import javax.inject.Inject;
 
 /**
- * Sample class - triggers instantiation fallback.
+ * Sample class - tests various situations for the instantiation fallback.
  */
 public abstract class InstantiationFallbackClasses {
 
@@ -42,4 +43,12 @@ public abstract class InstantiationFallbackClasses {
         }
     }
 
+    // Class with @PostConstruct method should never be instantiated by instantiation fallback
+    public static final class ClassWithPostConstruct {
+        @PostConstruct
+        public void postConstructMethod() {
+            // --
+        }
+    }
+
 }

src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java

@@ -34,6 +34,9 @@ public abstract class InvalidPostConstruct {
     }
 
     public static final class ThrowsException {
+        @Inject
+        private ProvidedClass providedClass;
+
         @PostConstruct
         public void throwingPostConstruct() {
             throw new IllegalStateException("Exception in post construct");