Reverse PreDestroy Order (#883)

* Reverse PreDestroy Order

Now will execute same priority predestroy hooks in reverse order

Author: SentryMan

blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder.java

@@ -8,7 +8,7 @@ public class MyDestroyOrder {
 
   private static final List<String> ordering = Collections.synchronizedList(new ArrayList<>());
 
-  public static void add(String val){
+  public static void add(String val) {
     ordering.add(val);
   }
 

blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder2.java

@@ -0,0 +1,45 @@
+package org.example.myapp;
+
+import io.avaje.inject.PreDestroy;
+import jakarta.inject.Singleton;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+public class MyDestroyOrder2 {
+
+  private static final List<String> ordering = Collections.synchronizedList(new ArrayList<>());
+
+  public static void add(String val) {
+    ordering.add(val);
+  }
+
+  public static List<String> ordering() {
+    return ordering;
+  }
+
+  @Singleton
+  public static class One {
+    @PreDestroy
+    public void close() {
+      add("One");
+    }
+  }
+
+  @Singleton
+  public static class Two {
+
+    final One one;
+
+    public Two(One one) {
+      this.one = one;
+    }
+
+    @PreDestroy
+    public void close() {
+      add("Two");
+    }
+  }
+
+}

blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java

@@ -3,19 +3,35 @@
 import io.avaje.inject.BeanScope;
 import org.junit.jupiter.api.Test;
 
-import java.util.List;
-
 import static org.assertj.core.api.Assertions.assertThat;
 
 class MyDestroyOrderTest {
 
   @Test
-  void ordering() {
+  void ordering_byPriority() {
     MyDestroyOrder.ordering().clear();
     try (BeanScope beanScope = BeanScope.builder().build()) {
       beanScope.get(HelloService.class);
     }
-    List<String> ordering = MyDestroyOrder.ordering();
-    assertThat(ordering).containsExactly("HelloService", "AppConfig", "MyNamed", "MyMetaDataRepo", "ExampleService", "AppHelloData");
+    assertThat(MyDestroyOrder.ordering())
+      .containsExactly(
+        "HelloService",
+        "MyNamed",
+        "AppConfig",
+        "MyMetaDataRepo",
+        "ExampleService",
+        "AppHelloData");
+  }
+
+  @Test
+  void ordering_expect_reverseOfDependencies() {
+    MyDestroyOrder2.ordering().clear();
+    try (BeanScope beanScope = BeanScope.builder().build()) {
+      beanScope.get(HelloService.class);
+    }
+    assertThat(MyDestroyOrder2.ordering())
+      .containsExactly(
+        "Two",
+        "One");
   }
 }

inject/src/main/java/io/avaje/inject/spi/DBuilder.java

@@ -1,16 +1,26 @@
 package io.avaje.inject.spi;
 
-import io.avaje.inject.BeanEntry;
-import io.avaje.inject.BeanScope;
-import jakarta.inject.Provider;
-import org.jspecify.annotations.Nullable;
+import static io.avaje.inject.spi.DBeanScope.combine;
 
 import java.lang.reflect.Type;
-import java.util.*;
+import java.util.ArrayDeque;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Deque;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
-import static io.avaje.inject.spi.DBeanScope.combine;
+import org.jspecify.annotations.Nullable;
+
+import io.avaje.inject.BeanEntry;
+import io.avaje.inject.BeanScope;
+import jakarta.inject.Provider;
 
 class DBuilder implements Builder {
 
@@ -20,7 +30,8 @@ class DBuilder implements Builder {
   private final List<Runnable> postConstruct = new ArrayList<>();
 
   private final List<Consumer<BeanScope>> postConstructConsumers = new ArrayList<>();
-  private final List<ClosePair> preDestroy = new ArrayList<>();
+  private final Deque<ClosePair> preDestroy = new ArrayDeque<>();
+
   /** List of field injection closures. */
   private final List<Consumer<Builder>> injectors = new ArrayList<>();
   /** The beans created and added to the scope during building. */
@@ -226,13 +237,13 @@ public final void addPreDestroy(AutoCloseable invoke) {
 
   @Override
   public final void addPreDestroy(AutoCloseable invoke, int priority) {
-    preDestroy.add(new ClosePair(priority, invoke));
+    preDestroy.addFirst(new ClosePair(priority, invoke));
   }
 
   @Override
   public final void addAutoClosable(Object maybeAutoCloseable) {
     if (maybeAutoCloseable instanceof AutoCloseable) {
-      preDestroy.add(new ClosePair(1000, (AutoCloseable) maybeAutoCloseable));
+      preDestroy.addFirst(new ClosePair(1000, (AutoCloseable) maybeAutoCloseable));
     }
   }
 
@@ -333,12 +344,7 @@ public final <T> Provider<T> getProviderFor(Class<?> cls, Type type) {
       if (bean != null) {
         return bean;
       }
-      final String msg =
-          "Unable to inject an instance for generic type "
-              + type
-              + " usually provided by "
-              + cls
-              + "?";
+      final String msg = "Unable to inject an instance for generic type " + type + " usually provided by " + cls + "?";
       throw new IllegalStateException(msg);
     };
   }
@@ -378,12 +384,12 @@ public final boolean containsAllProfiles(List<String> type) {
 
   @Override
   public final boolean contains(String type) {
-    return beanMap.contains(type)  || (parent != null && parent.contains(type));
+    return beanMap.contains(type) || parent != null && parent.contains(type);
   }
 
   @Override
   public final boolean contains(Type type) {
-    return beanMap.contains(type) || (parent != null && parent.contains(type));
+    return beanMap.contains(type) || parent != null && parent.contains(type);
   }
 
   @Override
@@ -450,12 +456,10 @@ public final BeanScope build(boolean withShutdownHook, long start) {
     return scope.start(start);
   }
 
-  /**
-   * Return the PreDestroy methods in priority order.
-   */
+  /** Return the PreDestroy methods in priority order. */
   private List<AutoCloseable> preDestroy() {
-    Collections.sort(preDestroy);
     return preDestroy.stream()
+      .sorted()
       .map(ClosePair::closeable)
       .collect(Collectors.toList());
   }