Optimize JSR250 lifecycle annotation detection

Author: adrienlauer

core/src/main/java/org/seedstack/seed/core/internal/lifecycle/ConstructionProvisionListener.java

@@ -12,13 +12,25 @@
 import org.seedstack.shed.reflect.Classes;
 
 import javax.annotation.PostConstruct;
+import javax.annotation.PreDestroy;
 
 import static org.seedstack.shed.reflect.AnnotationPredicates.elementAnnotatedWith;
 
-class ConstructionMatcher extends AbstractMatcher<Binding<?>> {
+class LifecycleMatcher extends AbstractMatcher<Binding<?>> {
     @Override
     public boolean matches(Binding<?> binding) {
-        return hasPostConstruct(binding.getKey().getTypeLiteral().getRawType());
+        // Singletons being auto-closeable or having at least one @PreDestroy method match
+        // Any binding (any scope) having at least one @PostConstruct method match
+        Class<?> rawType = binding.getKey().getTypeLiteral().getRawType();
+        return (binding.acceptScopingVisitor(SingletonScopingVisitor.INSTANCE) && (isAutoCloseable(rawType) || hasPreDestroy(rawType))) || hasPostConstruct(rawType);
+    }
+
+    private boolean hasPreDestroy(Class<?> rawType) {
+        return Classes.from(rawType)
+                .traversingInterfaces()
+                .traversingSuperclasses()
+                .methods()
+                .anyMatch(elementAnnotatedWith(PreDestroy.class, true));
     }
 
     private boolean hasPostConstruct(Class<?> rawType) {
@@ -28,4 +40,8 @@ private boolean hasPostConstruct(Class<?> rawType) {
                 .methods()
                 .anyMatch(elementAnnotatedWith(PostConstruct.class, true));
     }
+
+    private boolean isAutoCloseable(Class<?> rawType) {
+        return AutoCloseable.class.isAssignableFrom(rawType);
+    }
 }

core/src/main/java/org/seedstack/seed/core/internal/lifecycle/DestructionMatcher.java

@@ -32,8 +32,7 @@ protected void configure() {
             lifecycleListenerMultibinder.addBinding().to(lifecycleListenerClass);
         }
 
-        // Bind lifecycle provision listeners
-        bindListener(new ConstructionMatcher(), new ConstructionProvisionListener());
-        bindListener(new DestructionMatcher(), new DestructionProvisionListener(lifecycleManager));
+        // Bind lifecycle provision listener
+        bindListener(new LifecycleMatcher(), new LifecycleProvisionListener(lifecycleManager));
     }
 }

core/src/main/java/org/seedstack/seed/core/internal/lifecycle/ConstructionMatcher.java renamed to core/src/main/java/org/seedstack/seed/core/internal/lifecycle/LifecycleMatcher.java

@@ -7,21 +7,20 @@
  */
 package org.seedstack.seed.core.internal.lifecycle;
 
-import static org.seedstack.shed.reflect.AnnotationPredicates.elementAnnotatedWith;
-import static org.seedstack.shed.reflect.ReflectUtils.invoke;
-import static org.seedstack.shed.reflect.ReflectUtils.makeAccessible;
-
 import com.google.inject.spi.ProvisionListener;
+import org.seedstack.shed.reflect.Classes;
 
 import javax.annotation.PostConstruct;
 import javax.annotation.PreDestroy;
 
-import org.seedstack.shed.reflect.Classes;
+import static org.seedstack.shed.reflect.AnnotationPredicates.elementAnnotatedWith;
+import static org.seedstack.shed.reflect.ReflectUtils.invoke;
+import static org.seedstack.shed.reflect.ReflectUtils.makeAccessible;
 
-class DestructionProvisionListener implements ProvisionListener {
+class LifecycleProvisionListener implements ProvisionListener {
     private final LifecycleManager lifecycleManager;
 
-    DestructionProvisionListener(LifecycleManager lifecycleManager) {
+    LifecycleProvisionListener(LifecycleManager lifecycleManager) {
         this.lifecycleManager = lifecycleManager;
     }
 
@@ -33,11 +32,18 @@ public <T> void onProvision(ProvisionInvocation<T> provisionInvocation) {
                 .traversingSuperclasses()
                 .methods()
                 .forEach(m -> {
-                    if (elementAnnotatedWith(PreDestroy.class, true).test(m)) {
-                        lifecycleManager.registerPreDestroy(provision, m);
+                    if (elementAnnotatedWith(PostConstruct.class, true).test(m)) {
+                        invoke(makeAccessible(m), provision);
                     }
-                    if (provision instanceof AutoCloseable) {
-                        lifecycleManager.registerAutoCloseable((AutoCloseable) provision);
+
+                    // End-of-life management are limited to singletons (in-line with the matching condition in LifecycleMatcher)
+                    if (provisionInvocation.getBinding().acceptScopingVisitor(SingletonScopingVisitor.INSTANCE)) {
+                        if (elementAnnotatedWith(PreDestroy.class, true).test(m)) {
+                            lifecycleManager.registerPreDestroy(provision, m);
+                        }
+                        if (provision instanceof AutoCloseable) {
+                            lifecycleManager.registerAutoCloseable((AutoCloseable) provision);
+                        }
                     }
                 });
     }

core/src/main/java/org/seedstack/seed/core/internal/lifecycle/LifecycleModule.java

@@ -0,0 +1,43 @@
+/*
+ * Copyright © 2013-2021, The SeedStack authors <http://seedstack.org>
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+package org.seedstack.seed.core.internal.lifecycle;
+
+import com.google.inject.Scope;
+import com.google.inject.Scopes;
+import com.google.inject.spi.BindingScopingVisitor;
+
+import javax.inject.Singleton;
+import java.lang.annotation.Annotation;
+
+final class SingletonScopingVisitor implements BindingScopingVisitor<Boolean> {
+    static final SingletonScopingVisitor INSTANCE = new SingletonScopingVisitor();
+
+    private SingletonScopingVisitor() {
+    }
+
+    @Override
+    public Boolean visitEagerSingleton() {
+        return true;
+    }
+
+    @Override
+    public Boolean visitScope(Scope scope) {
+        return scope == Scopes.SINGLETON;
+    }
+
+    @Override
+    public Boolean visitScopeAnnotation(Class<? extends Annotation> scopeAnnotation) {
+        return scopeAnnotation.isAssignableFrom(Singleton.class) || scopeAnnotation.isAssignableFrom(
+                com.google.inject.Singleton.class);
+    }
+
+    @Override
+    public Boolean visitNoScoping() {
+        return false;
+    }
+}

core/src/main/java/org/seedstack/seed/core/internal/lifecycle/DestructionProvisionListener.java renamed to core/src/main/java/org/seedstack/seed/core/internal/lifecycle/LifecycleProvisionListener.java

@@ -35,6 +35,7 @@ public class LifecycleIT implements LifecycleListener {
     private static boolean ignoredClosedWasCalled;
     private static boolean proxyClosedWasCalled;
     private static boolean classProxyClosedWasCalled;
+    private static boolean preDestroySingletonCalled;
     private static boolean preDestroyCalled;
     private static boolean postConstructSingletonCalled;
     private static boolean postConstructCalled;
@@ -54,16 +55,16 @@ public static void tearDownClass() {
         assertThat(stoppingWasCalled).isTrue();
         assertThat(closedWasCalled).isTrue();
         assertThat(ignoredClosedWasCalled).isFalse();
-        assertThat(preDestroyCalled).isTrue();
+        assertThat(preDestroySingletonCalled).isTrue();
     }
 
     @Before
     public void setUp() {
         kernel = Seed.createKernel();
         childInjector = kernel.objectGraph().as(Injector.class).createChildInjector(binder -> {
             binder.bind(PreDestroyFixture.class);
-            binder.bind(PostConstructFixture.class);
-            binder.bind(PostConstructSingletonFixture.class);
+            binder.bind(StatelessFixture.class);
+            binder.bind(SingletonFixture.class);
             binder.bind(AutoCloseableFixture.class);
             binder.bind(IgnoredAutoCloseableFixture.class);
             binder.bind(AutoCloseable.class)
@@ -79,7 +80,7 @@ public void setUp() {
         assertThat(closedWasCalled).isFalse();
         assertThat(ignoredClosedWasCalled).isFalse();
         assertThat(postConstructSingletonCalled).isTrue();
-        assertThat(preDestroyCalled).isFalse();
+        assertThat(preDestroySingletonCalled).isFalse();
     }
 
     @After
@@ -88,7 +89,7 @@ public void tearDown() {
         assertThat(stoppingWasCalled).isFalse();
         assertThat(closedWasCalled).isFalse();
         assertThat(ignoredClosedWasCalled).isFalse();
-        assertThat(preDestroyCalled).isFalse();
+        assertThat(preDestroySingletonCalled).isFalse();
         Seed.disposeKernel(kernel);
     }
 
@@ -103,10 +104,12 @@ public void lifecycle_callbacks_are_invoked() {
         assertThat(postConstructSingletonCalled).isTrue();
 
         assertThat(postConstructCalled).isFalse();
-        childInjector.getInstance(PostConstructFixture.class);
+        assertThat(preDestroyCalled).isFalse();
+        childInjector.getInstance(StatelessFixture.class);
         assertThat(postConstructCalled).isTrue();
-
         assertThat(preDestroyCalled).isFalse();
+
+        assertThat(preDestroySingletonCalled).isFalse();
     }
 
     @Override
@@ -157,24 +160,30 @@ public void close() {
 
     @Singleton
     private static class PreDestroyFixture {
-        @PreDestroy
-        public void preDestroy() {
-            preDestroyCalled = true;
-        }
     }
 
     @Singleton
-    private static class PostConstructSingletonFixture {
+    private static class SingletonFixture {
         @PostConstruct
         public void postConstruct() {
             postConstructSingletonCalled = true;
         }
+
+        @PreDestroy
+        public void preDestroy() {
+            preDestroySingletonCalled = true;
+        }
     }
 
-    private static class PostConstructFixture {
+    private static class StatelessFixture {
         @PostConstruct
         public void postConstruct() {
             postConstructCalled = true;
         }
+
+        @PreDestroy
+        public void preDestroy() {
+            preDestroyCalled = true;
+        }
     }
 }