From f0144857894d9a97ae456c1c21f29afff15e0402 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Thu, 19 May 2016 21:50:48 +0200 Subject: [PATCH] Service injector - implement stricter requirements for PostConstruct methods - Implement similar restrictions as prescribed by the PostConstruct documentation: - Class may have at most one method annotated with PostConstruct - PostConstruct method must return void - Javadoc: replace mentions of injector construction where any injection method was meant --- .../AuthMeServiceInitializer.java | 64 +++++++++++++------ .../authme/permission/PermissionsManager.java | 7 +- .../AuthMeServiceInitializerTest.java | 14 +++- .../samples/InvalidPostConstruct.java | 26 +++++++- .../samples/PostConstructTestClass.java | 13 +--- 5 files changed, 86 insertions(+), 38 deletions(-) diff --git a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java index c6d7fd85..23c2f01d 100644 --- a/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java +++ b/src/main/java/fr/xephi/authme/initialization/AuthMeServiceInitializer.java @@ -144,8 +144,8 @@ public class AuthMeServiceInitializer { } /** - * Instantiates the given class by locating an @Inject constructor and retrieving - * or instantiating its parameters. + * Instantiates the given class by locating its @Inject elements and retrieving + * or instantiating the required instances. * * @param clazz the class to instantiate * @param traversedClasses collection of classes already traversed @@ -164,13 +164,13 @@ public class AuthMeServiceInitializer { validateInjectionHasNoCircularDependencies(injection.getDependencies(), traversedClasses); Object[] dependencies = resolveDependencies(injection, traversedClasses); T object = injection.instantiateWith(dependencies); - executePostConstructMethods(object); + executePostConstructMethod(object); return object; } /** - * Resolves the dependencies for the given constructor, i.e. returns a collection that satisfy - * the constructor's parameter types by retrieving elements or instantiating them where necessary. + * Resolves the dependencies for the given class instantiation, i.e. returns a collection that satisfy + * the class' dependencies by retrieving elements or instantiating them where necessary. * * @param injection the injection parameters * @param traversedClasses collection of traversed classes @@ -247,21 +247,20 @@ public class AuthMeServiceInitializer { + "allowed packages. It must be provided explicitly or the package must be passed to the constructor."); } - private static void executePostConstructMethods(Object object) { - for (Method method : object.getClass().getDeclaredMethods()) { - if (method.isAnnotationPresent(PostConstruct.class)) { - if (method.getParameterTypes().length == 0 && !Modifier.isStatic(method.getModifiers())) { - try { - method.setAccessible(true); - method.invoke(object); - } catch (IllegalAccessException | InvocationTargetException e) { - throw new UnsupportedOperationException(e); - } - } else { - throw new IllegalStateException(String.format("@PostConstruct methods may not be static or have " - + " any parameters. Method '%s' of class '%s' is either static or has parameters", - method.getName(), object.getClass().getSimpleName())); - } + /** + * Executes an object's method annotated with {@link PostConstruct} if present. + * Throws an exception if there are multiple such methods, or if the method is static. + * + * @param object the object to execute the post construct method for + */ + private static void executePostConstructMethod(Object object) { + Method postConstructMethod = getAndValidatePostConstructMethod(object.getClass()); + if (postConstructMethod != null) { + try { + postConstructMethod.setAccessible(true); + postConstructMethod.invoke(object); + } catch (IllegalAccessException | InvocationTargetException e) { + throw new UnsupportedOperationException(e); } } } @@ -272,6 +271,31 @@ public class AuthMeServiceInitializer { } } + /** + * Validate and locate the given class' post construct method. Returns {@code null} if none present. + * + * @param clazz the class to search + * @return post construct method, or null + */ + private static Method getAndValidatePostConstructMethod(Class clazz) { + Method postConstructMethod = null; + for (Method method : clazz.getDeclaredMethods()) { + if (method.isAnnotationPresent(PostConstruct.class)) { + if (postConstructMethod != null) { + throw new IllegalStateException("Multiple methods with @PostConstruct on " + clazz); + } else if (method.getParameterTypes().length > 0 || Modifier.isStatic(method.getModifiers())) { + 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); + } else { + postConstructMethod = method; + } + } + } + return postConstructMethod; + } + @SafeVarargs private static Injection firstNotNull(Provider>... providers) { for (Provider> provider : providers) { diff --git a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java index 9245a6a9..2c94d6f6 100644 --- a/src/main/java/fr/xephi/authme/permission/PermissionsManager.java +++ b/src/main/java/fr/xephi/authme/permission/PermissionsManager.java @@ -96,11 +96,9 @@ public class PermissionsManager implements PermissionsService { /** * Setup and hook into the permissions systems. - * - * @return The detected permissions system. */ @PostConstruct - public PermissionsSystemType setup() { + public void setup() { // Force-unhook from current hooked permissions systems unhook(); @@ -177,7 +175,7 @@ public class PermissionsManager implements PermissionsService { ConsoleLogger.info("Hooked into " + type.getName() + "!"); // Return the used permissions system type - return type; + return; } catch (Exception ex) { // An error occurred, show a warning message @@ -187,7 +185,6 @@ public class PermissionsManager implements PermissionsService { // No recognized permissions system found, show a message and return ConsoleLogger.info("No supported permissions system found! Permissions are disabled!"); - return null; } /** diff --git a/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java b/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java index a81571d1..81b2eb66 100644 --- a/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java +++ b/src/test/java/fr/xephi/authme/initialization/AuthMeServiceInitializerTest.java @@ -166,7 +166,7 @@ public class AuthMeServiceInitializerTest { PostConstructTestClass testClass = initializer.get(PostConstructTestClass.class); // then - assertThat(testClass.werePostConstructsCalled(), equalTo(true)); + assertThat(testClass.wasPostConstructCalled(), equalTo(true)); assertThat(testClass.getBetaManager(), not(nullValue())); } @@ -188,6 +188,18 @@ public class AuthMeServiceInitializerTest { initializer.get(InvalidPostConstruct.ThrowsException.class); } + @Test(expected = RuntimeException.class) + public void shouldThrowForMultiplePostConstructMethods() { + // given / when / then + initializer.get(InvalidPostConstruct.MultiplePostConstructs.class); + } + + @Test(expected = RuntimeException.class) + public void shouldThrowForPostConstructNotReturningVoid() { + // given / when / then + initializer.get(InvalidPostConstruct.NotVoidReturnType.class); + } + @Test(expected = RuntimeException.class) public void shouldThrowForAbstractNonRegisteredDependency() { // given / when / then diff --git a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java index 23130e5e..80b6c83e 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/InvalidPostConstruct.java @@ -9,10 +9,8 @@ import javax.inject.Inject; public abstract class InvalidPostConstruct { public static final class WithParams { - @SuppressWarnings("unused") @Inject private AlphaService alphaService; - @SuppressWarnings("unused") @Inject private ProvidedClass providedClass; @@ -41,4 +39,28 @@ public abstract class InvalidPostConstruct { throw new IllegalStateException("Exception in post construct"); } } + + public static final class NotVoidReturnType { + @Inject + private ProvidedClass providedClass; + + @PostConstruct + public int returnsInt() { + return 42; + } + } + + public static final class MultiplePostConstructs { + @Inject + private ProvidedClass providedClass; + + @PostConstruct + public void postConstruct1() { + // -- + } + @PostConstruct + public void postConstruct2() { + // -- + } + } } diff --git a/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java b/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java index 375e58fa..a8b9e64e 100644 --- a/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java +++ b/src/test/java/fr/xephi/authme/initialization/samples/PostConstructTestClass.java @@ -17,22 +17,15 @@ public class PostConstructTestClass implements SettingsDependent { @Inject private BetaManager betaManager; private boolean wasPostConstructCalled = false; - private boolean wasSecondPostConstructCalled = false; private boolean wasReloaded = false; @PostConstruct - protected void setFieldToTrue() { + public void postConstructMethod() { wasPostConstructCalled = true; } - @PostConstruct - public int otherPostConstructMethod() { - wasSecondPostConstructCalled = true; - return 42; - } - - public boolean werePostConstructsCalled() { - return wasPostConstructCalled && wasSecondPostConstructCalled; + public boolean wasPostConstructCalled() { + return wasPostConstructCalled; } public BetaManager getBetaManager() {