#749 Add support for a @PreDestroy method on a factory, alternative to …

…@bean(destroyMethod) Example: ``` @factory final class MyFactory { @bean MyComponent myComponent() { ... } @PreDestroy(priority = 3000) void destroyMyComponent(MyComponent bean) { ... } ``` - ONLY valid on factory components - An alternative to @bean(destroyMethod) - The @PreDestroy method matches to a @bean by type only (ignores qualifiers, only 1 argument to the destroy method)
avaje · Jan 6, 2025 · d428679 · d428679
1 parent ba4c6ea
commit d428679
Show file tree

Hide file tree

Showing 11 changed files with 196 additions and 20 deletions.
diff --git a/blackbox-test-inject/src/main/java/org/example/myapp/MyNestedDestroy.java b/blackbox-test-inject/src/main/java/org/example/myapp/MyNestedDestroy.java
@@ -4,16 +4,16 @@
 
 public class MyNestedDestroy {
 
-  public static AtomicInteger started = new AtomicInteger();
-  public static AtomicInteger stopped = new AtomicInteger();
+  public static AtomicInteger STARTED = new AtomicInteger();
+  public static AtomicInteger STOPPED = new AtomicInteger();
 
   public static void reset() {
-    started.set(0);
-    stopped.set(0);
+    STARTED.set(0);
+    STOPPED.set(0);
   }
 
   public void start() {
-    started.incrementAndGet();
+    STARTED.incrementAndGet();
   }
 
   public Reaper reaper() {
@@ -23,7 +23,7 @@ public Reaper reaper() {
   public static class Reaper {
 
     public void stop() {
-      stopped.incrementAndGet();
+      STOPPED.incrementAndGet();
     }
   }
 }
diff --git a/blackbox-test-inject/src/main/java/org/example/myapp/config/AFactory.java b/blackbox-test-inject/src/main/java/org/example/myapp/config/AFactory.java
@@ -2,18 +2,49 @@
 
 import io.avaje.inject.Bean;
 import io.avaje.inject.Factory;
+import io.avaje.inject.PreDestroy;
 import org.example.myapp.MyNestedDestroy;
 
 import java.io.IOException;
+import java.util.concurrent.atomic.AtomicInteger;
 
 @Factory
-class AFactory {
+public class AFactory {
+
+  public static AtomicInteger DESTROY_COUNT_BEAN = new AtomicInteger();
+  public static AtomicInteger DESTROY_COUNT_COMPONENT = new AtomicInteger();
+  public static AtomicInteger DESTROY_COUNT_AFOO = new AtomicInteger();
+
+  public static void reset() {
+    DESTROY_COUNT_BEAN.set(0);
+    DESTROY_COUNT_AFOO.set(0);
+    DESTROY_COUNT_COMPONENT.set(0);
+  }
 
   @Bean(initMethod = "start", destroyMethod = "reaper().stop()")
   MyNestedDestroy lifecycle2() {
     return new MyNestedDestroy();
   }
 
+  @PreDestroy(priority = 3000)
+  void dest2(MyNestedDestroy bean) {
+    DESTROY_COUNT_BEAN.incrementAndGet();
+  }
+
+//  // compiler error when type does not match any @Bean method
+//  @PreDestroy(priority = 3001)
+//  void destErr(Object bean) {
+//
+//  }
+
+  /**
+   * NO args so this is the normal factory component destroy method.
+   */
+  @PreDestroy
+  void factoryDestroy() {
+    DESTROY_COUNT_COMPONENT.incrementAndGet();
+  }
+
   @Bean
   A0.Builder build0() {
     return new I0();
@@ -40,6 +71,11 @@ AFoo buildAfoo() throws IOException {
     return new AFoo();
   }
 
+  @PreDestroy
+  void destAfoo(AFoo bean) {
+    DESTROY_COUNT_AFOO.incrementAndGet();
+  }
+
   static class I0 implements A0.Builder {
 
   }

diff --git a/blackbox-test-inject/src/test/java/org/example/myapp/HelloServiceTest.java b/blackbox-test-inject/src/test/java/org/example/myapp/HelloServiceTest.java
@@ -4,6 +4,7 @@
 import io.avaje.inject.aop.InvocationException;
 import org.example.myapp.aspect.MyAroundAspect;
 import org.example.myapp.aspect.MyMultiInvokeAspect;
+import org.example.myapp.config.AFactory;
 import org.junit.jupiter.api.Test;
 
 import java.io.IOException;
@@ -17,12 +18,19 @@ class HelloServiceTest {
   @Test
   void lifecycles() {
     MyNestedDestroy.reset();
+    AFactory.reset();
     try (BeanScope beanScope = BeanScope.builder().build()) {
       assertThat(beanScope.get(MyNestedDestroy.class)).isNotNull();
-      assertThat(MyNestedDestroy.started.get()).isEqualTo(1);
-      assertThat(MyNestedDestroy.stopped.get()).isEqualTo(0);
+      assertThat(MyNestedDestroy.STARTED.get()).isEqualTo(1);
+      assertThat(MyNestedDestroy.STOPPED.get()).isEqualTo(0);
+      assertThat(AFactory.DESTROY_COUNT_BEAN.get()).isEqualTo(0);
+      assertThat(AFactory.DESTROY_COUNT_AFOO.get()).isEqualTo(0);
+      assertThat(AFactory.DESTROY_COUNT_COMPONENT.get()).isEqualTo(0);
     }
-    assertThat(MyNestedDestroy.stopped.get()).isEqualTo(1);
+    assertThat(MyNestedDestroy.STOPPED.get()).isEqualTo(1);
+    assertThat(AFactory.DESTROY_COUNT_BEAN.get()).isEqualTo(1);
+    assertThat(AFactory.DESTROY_COUNT_AFOO.get()).isEqualTo(1);
+    assertThat(AFactory.DESTROY_COUNT_COMPONENT.get()).isEqualTo(1);
   }
 
   /**

diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/BeanReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/BeanReader.java
@@ -541,4 +541,8 @@ boolean needsTryForMethodInjection() {
   boolean isDelayed() {
     return delayed;
   }
+
+  void validate() {
+    typeReader.validate();
+  }
 }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/DestroyMethods.java b/inject-generator/src/main/java/io/avaje/inject/generator/DestroyMethods.java
@@ -0,0 +1,77 @@
+package io.avaje.inject.generator;
+
+import javax.lang.model.element.Element;
+import javax.lang.model.element.ExecutableElement;
+import java.util.*;
+import java.util.stream.Collectors;
+
+/**
+ * Holds extra PreDestroy methods for a factory.
+ * <p>
+ * These methods are expected to relate back to a {@code @Bean} method
+ * on the same factory.
+ */
+final class DestroyMethods {
+
+  private final Map<String, DestroyMethod> methods = new HashMap<>();
+  private final Set<String> matchedTypes = new HashSet<>();
+
+  void add(ExecutableElement element) {
+    Integer priority = PreDestroyPrism.getOptionalOn(element)
+      .map(PreDestroyPrism::priority)
+      .orElse(null);
+
+    var destroyMethod = new DestroyMethod(element, priority);
+    methods.put(destroyMethod.matchType, destroyMethod);
+  }
+
+  DestroyMethod match(String returnTypeRaw) {
+    var match = methods.get(returnTypeRaw);
+    if (match != null) {
+      matchedTypes.add(returnTypeRaw);
+    }
+    return match;
+  }
+
+  /**
+   * Return PreDestroy methods that were not matched to a {@code @Bean} method
+   * on the same factory.
+   */
+  List<DestroyMethod> unmatched() {
+    return methods.values()
+      .stream()
+      .filter(entry -> !matchedTypes.contains(entry.matchType()))
+      .collect(Collectors.toList());
+  }
+
+  static final class DestroyMethod {
+
+    private final String method;
+    private final Integer priority;
+    private final String matchType;
+    private final ExecutableElement element;
+
+    DestroyMethod(ExecutableElement element, Integer priority) {
+      this.element = element;
+      this.method = element.getSimpleName().toString();
+      this.matchType = element.getParameters().get(0).asType().toString();
+      this.priority = priority;
+    }
+
+    String method() {
+      return method;
+    }
+
+    Integer priority() {
+      return priority;
+    }
+
+    String matchType() {
+      return matchType;
+    }
+
+    Element element() {
+      return element;
+    }
+  }
+}
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/MethodReader.java
@@ -15,6 +15,7 @@ final class MethodReader {
 
   private static final String CODE_COMMENT_BUILD_FACTORYBEAN = "  /**\n   * Create and register %s via factory bean method %s#%s().\n   */";
 
+  private final TypeExtendsInjection factory;
   private final ExecutableElement element;
   private final String factoryType;
   private final String methodName;
@@ -40,10 +41,11 @@ final class MethodReader {
   private MethodParam observeParameter;
 
   MethodReader(ExecutableElement element, TypeElement beanType, ImportTypeMap importTypes) {
-    this(element, beanType, null, null, importTypes);
+    this(null, element, beanType, null, null, importTypes);
   }
 
-  MethodReader(ExecutableElement element, TypeElement beanType, BeanPrism bean, String qualifierName, ImportTypeMap importTypes) {
+  MethodReader(TypeExtendsInjection factory, ExecutableElement element, TypeElement beanType, BeanPrism bean, String qualifierName, ImportTypeMap importTypes) {
+    this.factory = factory;
     this.element = element;
     if (bean != null) {
       prototype = PrototypePrism.isPresent(element);
@@ -255,8 +257,8 @@ void builderBuildAddBean(Append writer) {
       }
       String indent = optionalType ? "        " : "      ";
       writer.indent(indent);
-      var hasLifecycleMethods = hasLifecycleMethods();
-
+      var matchedPreDestroyMethod = factory.matchPreDestroy(returnTypeRaw);
+      var hasLifecycleMethods = matchedPreDestroyMethod != null || hasLifecycleMethods();
       if (hasLifecycleMethods && multiRegister) {
         writer.append("bean.stream()").eol().indent(indent).append("    .map(");
       } else if (hasLifecycleMethods) {
@@ -292,7 +294,7 @@ void builderBuildAddBean(Append writer) {
         writer.indent(indent).append(addPostConstruct, initMethod).eol();
       }
 
-      var priority = destroyPriority == null || destroyPriority == 1000 ? "" : ", " + destroyPriority;
+      var priority = priority(destroyPriority);
       if (notEmpty(destroyMethod)) {
         var addPreDestroy =
           multiRegister
@@ -317,13 +319,27 @@ void builderBuildAddBean(Append writer) {
       } else if (multiRegister && hasInitMethod) {
         writer.indent(indent).append("    .forEach(x -> {});").eol();
       }
+      if (matchedPreDestroyMethod != null) {
+        // PreDestroy method on the factory
+        var addPreDestroy =
+          multiRegister
+            ? "    .forEach($bean -> builder.addPreDestroy(%s%s));"
+            : "builder.addPreDestroy(%s%s);";
+        var methodPriority = priority(matchedPreDestroyMethod.priority());
+        var method = String.format("() -> factory.%s($bean)", matchedPreDestroyMethod.method());
+        writer.indent(indent).append(addPreDestroy, method, methodPriority).eol();
+      }
 
       if (optionalType) {
         writer.append("      }").eol();
       }
     }
   }
 
+  static String priority(Integer priority) {
+    return priority == null || priority == 1000 ? "" : ", " + priority;
+  }
+
   static String addPreDestroy(String destroyMethod) {
     if (!destroyMethod.contains(".")) {
       return "$bean::" + destroyMethod;

diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java b/inject-generator/src/main/java/io/avaje/inject/generator/SimpleBeanWriter.java
@@ -135,6 +135,7 @@ private void writeStaticFactoryBeanMethods() {
     for (MethodReader factoryMethod : beanReader.factoryMethods()) {
       writeFactoryBeanMethod(factoryMethod);
     }
+    beanReader.validate();
   }
 
   private void writeFactoryBeanMethod(MethodReader method) {

diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsInjection.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsInjection.java
@@ -1,5 +1,6 @@
 package io.avaje.inject.generator;
 
+import static io.avaje.inject.generator.APContext.logError;
 import static io.avaje.inject.generator.ProcessingContext.getImportedAspect;
 
 import javax.lang.model.element.AnnotationMirror;
@@ -30,6 +31,7 @@ final class TypeExtendsInjection {
   private final TypeElement baseType;
   private final boolean factory;
   private final List<AspectPair> typeAspects;
+  private final DestroyMethods factoryPreDestroyMethods = new DestroyMethods();
   private Element postConstructMethod;
   private Element preDestroyMethod;
   private Integer preDestroyPriority;
@@ -129,9 +131,17 @@ private void readMethod(Element element, TypeElement type) {
       checkAspect = false;
     }
     if (AnnotationUtil.hasAnnotationWithName(element, "PreDestroy")) {
-      preDestroyMethod = element;
-      checkAspect = false;
-      PreDestroyPrism.getOptionalOn(element).ifPresent(preDestroy -> preDestroyPriority = preDestroy.priority());
+      int paramCount = methodElement.getParameters().size();
+      if (paramCount == 0) {
+        // normal component PreDestroy method
+        preDestroyMethod = element;
+        checkAspect = false;
+        PreDestroyPrism.getOptionalOn(element).ifPresent(preDestroy -> preDestroyPriority = preDestroy.priority());
+      } else if (paramCount == 1 && factory) {
+        // additional factory PreDestroy to match a @Bean method
+        checkAspect = false;
+        factoryPreDestroyMethods.add(methodElement);
+      }
     }
     if (checkAspect) {
       checkForAspect(methodElement);
@@ -163,10 +173,9 @@ private void checkForAspect(ExecutableElement methodElement) {
     }
   }
 
-
   private void addFactoryMethod(ExecutableElement methodElement, BeanPrism bean) {
     String qualifierName = Util.named(methodElement);
-    factoryMethods.add(new MethodReader(methodElement, baseType, bean, qualifierName, importTypes).read());
+    factoryMethods.add(new MethodReader(this, methodElement, baseType, bean, qualifierName, importTypes).read());
   }
 
   BeanAspects hasAspects() {
@@ -238,4 +247,14 @@ MethodReader constructor() {
     }
     return null;
   }
+
+  DestroyMethods.DestroyMethod matchPreDestroy(String returnTypeRaw) {
+    return factoryPreDestroyMethods.match(returnTypeRaw);
+  }
+
+  void validate() {
+    for (DestroyMethods.DestroyMethod destroyMethod : factoryPreDestroyMethods.unmatched()) {
+      logError(destroyMethod.element(), "Unused @PreDestroy method, no matching @Bean method for type " + destroyMethod.matchType());
+    }
+  }
 }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeExtendsReader.java
@@ -262,4 +262,8 @@ private void readInterfacesOf(TypeMirror anInterface) {
   private boolean isPublic(Element element) {
     return element != null && element.getModifiers().contains(Modifier.PUBLIC);
   }
+
+  void validate() {
+    extendsInjection.validate();
+  }
 }
diff --git a/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java b/inject-generator/src/main/java/io/avaje/inject/generator/TypeReader.java
@@ -165,4 +165,8 @@ void extraImports(ImportTypeMap importTypes) {
       genericTypes.forEach(t -> importTypes.addAll(t.importTypes()));
     }
   }
+
+  void validate() {
+    extendsReader.validate();
+  }
 }
diff --git a/inject-generator/src/test/java/io/avaje/inject/generator/MethodReaderTest.java b/inject-generator/src/test/java/io/avaje/inject/generator/MethodReaderTest.java
@@ -16,4 +16,11 @@ void addPreDestroy() {
   void addPreDestroyNested() {
     assertThat(MethodReader.addPreDestroy("foo().bar()")).isEqualTo("() -> $bean.foo().bar()");
   }
+
+  @Test
+  void priority() {
+    assertThat(MethodReader.priority(null)).isEqualTo("");
+    assertThat(MethodReader.priority(1000)).isEqualTo("");
+    assertThat(MethodReader.priority(52)).isEqualTo(", 52");
+  }
 }