From 3609e59831830d221d6ed0f41dca4763007c4a4f Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Fri, 8 Aug 2025 14:26:40 -0400 Subject: [PATCH 1/7] Reverse PreDestroy Order Now all things be equal will execute predestroy hooks in reverse order --- .../java/io/avaje/inject/spi/DBuilder.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java index 544b5cdf..2460dfc2 100644 --- a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java +++ b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java @@ -1,16 +1,24 @@ 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.ArrayList; +import java.util.Arrays; +import java.util.Collections; +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 { @@ -378,12 +386,12 @@ public final boolean containsAllProfiles(List 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,13 +458,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 preDestroy() { + Collections.reverse(preDestroy); Collections.sort(preDestroy); - return preDestroy.stream() - .map(ClosePair::closeable) - .collect(Collectors.toList()); + return preDestroy.stream().map(ClosePair::closeable).collect(Collectors.toList()); } } From dc7c01e802a239ca98243ec4d22718fe12f8eeb1 Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Fri, 8 Aug 2025 14:41:35 -0400 Subject: [PATCH 2/7] Update MyDestroyOrderTest.java --- .../org/example/myapp/MyDestroyOrderTest.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java index 56e405f6..6d5924df 100644 --- a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java +++ b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java @@ -1,11 +1,12 @@ package org.example.myapp; -import io.avaje.inject.BeanScope; -import org.junit.jupiter.api.Test; +import static org.assertj.core.api.Assertions.assertThat; import java.util.List; -import static org.assertj.core.api.Assertions.assertThat; +import org.junit.jupiter.api.Test; + +import io.avaje.inject.BeanScope; class MyDestroyOrderTest { @@ -16,6 +17,13 @@ void ordering() { beanScope.get(HelloService.class); } List ordering = MyDestroyOrder.ordering(); - assertThat(ordering).containsExactly("HelloService", "AppConfig", "MyNamed", "MyMetaDataRepo", "ExampleService", "AppHelloData"); + assertThat(ordering) + .containsExactly( + "HelloService", + "MyNamed", + "AppConfig", + "MyMetaDataRepo", + "ExampleService", + "AppHelloData"); } } From f90e588a77e7a04e1fdafd62172c78720019d271 Mon Sep 17 00:00:00 2001 From: Josiah Noel <32279667+SentryMan@users.noreply.github.com> Date: Fri, 8 Aug 2025 18:26:11 -0400 Subject: [PATCH 3/7] use a deque --- .../src/main/java/io/avaje/inject/spi/DBuilder.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java index 2460dfc2..b9d3d569 100644 --- a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java +++ b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java @@ -3,9 +3,11 @@ import static io.avaje.inject.spi.DBeanScope.combine; import java.lang.reflect.Type; +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; @@ -28,7 +30,8 @@ class DBuilder implements Builder { private final List postConstruct = new ArrayList<>(); private final List> postConstructConsumers = new ArrayList<>(); - private final List preDestroy = new ArrayList<>(); + private final Deque preDestroy = new ArrayDeque<>(); + /** List of field injection closures. */ private final List> injectors = new ArrayList<>(); /** The beans created and added to the scope during building. */ @@ -234,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.push(new ClosePair(priority, invoke)); } @Override public final void addAutoClosable(Object maybeAutoCloseable) { if (maybeAutoCloseable instanceof AutoCloseable) { - preDestroy.add(new ClosePair(1000, (AutoCloseable) maybeAutoCloseable)); + preDestroy.push(new ClosePair(1000, (AutoCloseable) maybeAutoCloseable)); } } @@ -460,8 +463,6 @@ public final BeanScope build(boolean withShutdownHook, long start) { /** Return the PreDestroy methods in priority order. */ private List preDestroy() { - Collections.reverse(preDestroy); - Collections.sort(preDestroy); - return preDestroy.stream().map(ClosePair::closeable).collect(Collectors.toList()); + return preDestroy.stream().sorted().map(ClosePair::closeable).collect(Collectors.toList()); } } From 54b7490ace20348bf982f9046b594249364313d1 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Sat, 9 Aug 2025 13:58:08 +1200 Subject: [PATCH 4/7] Add test for PreDestroy ordering based on constructor dependency --- .../org/example/myapp/MyDestroyOrder.java | 2 +- .../org/example/myapp/MyDestroyOrder2.java | 45 +++++++++++++++++++ .../org/example/myapp/MyDestroyOrderTest.java | 17 +++++-- .../java/io/avaje/inject/spi/DBuilder.java | 4 +- 4 files changed, 62 insertions(+), 6 deletions(-) create mode 100644 blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder2.java diff --git a/blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder.java b/blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder.java index 71e793b7..753468b9 100644 --- a/blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder.java +++ b/blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder.java @@ -8,7 +8,7 @@ public class MyDestroyOrder { private static final List ordering = Collections.synchronizedList(new ArrayList<>()); - public static void add(String val){ + public static void add(String val) { ordering.add(val); } diff --git a/blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder2.java b/blackbox-test-inject/src/main/java/org/example/myapp/MyDestroyOrder2.java new file mode 100644 index 00000000..344c649e --- /dev/null +++ b/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 ordering = Collections.synchronizedList(new ArrayList<>()); + + public static void add(String val) { + ordering.add(val); + } + + public static List 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"); + } + } + +} diff --git a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java index 6d5924df..a724806d 100644 --- a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java +++ b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java @@ -11,13 +11,12 @@ class MyDestroyOrderTest { @Test - void ordering() { + void ordering_byPriority() { MyDestroyOrder.ordering().clear(); try (BeanScope beanScope = BeanScope.builder().build()) { beanScope.get(HelloService.class); } - List ordering = MyDestroyOrder.ordering(); - assertThat(ordering) + assertThat(MyDestroyOrder.ordering()) .containsExactly( "HelloService", "MyNamed", @@ -26,4 +25,16 @@ void ordering() { "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"); + } } diff --git a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java index b9d3d569..43c2ad60 100644 --- a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java +++ b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java @@ -237,13 +237,13 @@ public final void addPreDestroy(AutoCloseable invoke) { @Override public final void addPreDestroy(AutoCloseable invoke, int priority) { - preDestroy.push(new ClosePair(priority, invoke)); + preDestroy.addFirst(new ClosePair(priority, invoke)); } @Override public final void addAutoClosable(Object maybeAutoCloseable) { if (maybeAutoCloseable instanceof AutoCloseable) { - preDestroy.push(new ClosePair(1000, (AutoCloseable) maybeAutoCloseable)); + preDestroy.addFirst(new ClosePair(1000, (AutoCloseable) maybeAutoCloseable)); } } From 645f63a58327afffa77755e1146bd86ac28167bd Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Sat, 9 Aug 2025 14:00:32 +1200 Subject: [PATCH 5/7] Format only --- inject/src/main/java/io/avaje/inject/spi/DBuilder.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java index 43c2ad60..43a3ce0b 100644 --- a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java +++ b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java @@ -344,12 +344,7 @@ public final Provider 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); }; } @@ -389,7 +384,7 @@ public final boolean containsAllProfiles(List 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 From 7510158cee6ef561967853f246ac1413716511df Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Sat, 9 Aug 2025 14:02:08 +1200 Subject: [PATCH 6/7] Format only --- .../java/org/example/myapp/MyDestroyOrderTest.java | 14 +++++++------- .../main/java/io/avaje/inject/spi/DBuilder.java | 5 ++++- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java index a724806d..596199b5 100644 --- a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java +++ b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java @@ -17,13 +17,13 @@ void ordering_byPriority() { beanScope.get(HelloService.class); } assertThat(MyDestroyOrder.ordering()) - .containsExactly( - "HelloService", - "MyNamed", - "AppConfig", - "MyMetaDataRepo", - "ExampleService", - "AppHelloData"); + .containsExactly( + "HelloService", + "MyNamed", + "AppConfig", + "MyMetaDataRepo", + "ExampleService", + "AppHelloData"); } @Test diff --git a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java index 43a3ce0b..feb76f6d 100644 --- a/inject/src/main/java/io/avaje/inject/spi/DBuilder.java +++ b/inject/src/main/java/io/avaje/inject/spi/DBuilder.java @@ -458,6 +458,9 @@ public final BeanScope build(boolean withShutdownHook, long start) { /** Return the PreDestroy methods in priority order. */ private List preDestroy() { - return preDestroy.stream().sorted().map(ClosePair::closeable).collect(Collectors.toList()); + return preDestroy.stream() + .sorted() + .map(ClosePair::closeable) + .collect(Collectors.toList()); } } From 56490b6d668e39cbe028acc20714957755c40ad0 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Sat, 9 Aug 2025 14:03:17 +1200 Subject: [PATCH 7/7] Format only --- .../test/java/org/example/myapp/MyDestroyOrderTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java index 596199b5..39b6b585 100644 --- a/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java +++ b/blackbox-test-inject/src/test/java/org/example/myapp/MyDestroyOrderTest.java @@ -1,12 +1,9 @@ package org.example.myapp; -import static org.assertj.core.api.Assertions.assertThat; - -import java.util.List; - +import io.avaje.inject.BeanScope; import org.junit.jupiter.api.Test; -import io.avaje.inject.BeanScope; +import static org.assertj.core.api.Assertions.assertThat; class MyDestroyOrderTest { @@ -32,7 +29,7 @@ void ordering_expect_reverseOfDependencies() { try (BeanScope beanScope = BeanScope.builder().build()) { beanScope.get(HelloService.class); } - assertThat( MyDestroyOrder2.ordering()) + assertThat(MyDestroyOrder2.ordering()) .containsExactly( "Two", "One");