From 525932668fbae041cadded7cd1e4f7e927686ae0 Mon Sep 17 00:00:00 2001 From: Jesse Brault Date: Sat, 25 Jan 2025 00:39:37 -0600 Subject: [PATCH] Fix bug where @Singleton classes bound toSelf() caused stack overflow. --- TODO.md | 6 +- .../java/groowt/util/di/DefaultRegistry.java | 61 +++++++++++++------ .../groowt/util/di/SingletonScopeHandler.java | 25 +++++--- .../di/DefaultRegistryObjectFactoryTests.java | 22 +++++++ 4 files changed, 84 insertions(+), 30 deletions(-) diff --git a/TODO.md b/TODO.md index 2c6ed7d..00296ad 100644 --- a/TODO.md +++ b/TODO.md @@ -24,9 +24,9 @@ For example: - [ ] Get rid of wvc compiler dependency on di ## 0.1.3 -- [ ] refactor tools/gradle start scripts to use dist instead of custom bin script - - [ ] have custom bin/* scripts which point to dist(s) for convenience -- [ ] di bug: @Singleton toSelf() causes stack overflow +- [ ] ~~refactor tools/gradle start scripts to use dist instead of custom bin script~~ + - [ ] ~~have custom bin/* scripts which point to dist(s) for convenience~~ +- [x] di bug: @Singleton toSelf() causes stack overflow - [ ] `OutletContainer` trait or interface for components which can contain an `` child. - [ ] `Context` should have methods for simply finding an ancestor of a certain type without the need for a predicate. diff --git a/util/di/src/main/java/groowt/util/di/DefaultRegistry.java b/util/di/src/main/java/groowt/util/di/DefaultRegistry.java index 419f5e0..f3af321 100644 --- a/util/di/src/main/java/groowt/util/di/DefaultRegistry.java +++ b/util/di/src/main/java/groowt/util/di/DefaultRegistry.java @@ -3,31 +3,58 @@ package groowt.util.di; import org.jetbrains.annotations.Nullable; import java.lang.annotation.Annotation; -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; +import java.util.*; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; public class DefaultRegistry implements Registry { - protected record ClassKeyBinding(Class key, Binding binding) {} + protected static class BindingContainer { - protected final Collection> classBindings = new ArrayList<>(); + private final Map, Binding> bindings = new HashMap<>(); + + @SuppressWarnings("unchecked") + public @Nullable Binding get(Class key) { + for (final var entry : bindings.entrySet()) { + if (entry.getKey().isAssignableFrom(key)) { + return (Binding) entry.getValue(); + } + } + return null; + } + + public void put(Class key, Binding binding) { + this.bindings.put(key, binding); + } + + public void remove(Class key) { + this.bindings.remove(key); + } + + public void removeIf(Class key, Predicate> filter) { + if (filter.test(this.get(key))) { + this.bindings.remove(key); + } + } + + public void clear() { + this.bindings.clear(); + } + + } + + protected final BindingContainer bindingContainer = new BindingContainer(); protected final Collection extensions = new ArrayList<>(); @Override public void removeBinding(Class key) { - this.classBindings.removeIf(classKeyBinding -> classKeyBinding.key().equals(key)); + this.bindingContainer.remove(key); } - @SuppressWarnings("unchecked") @Override public void removeBindingIf(Class key, Predicate> filter) { - this.classBindings.removeIf(classKeyBinding -> - classKeyBinding.key().equals(key) && filter.test((Binding) classKeyBinding.binding()) - ); + this.bindingContainer.removeIf(key, filter); } private List getAllRegistryExtensions(Class extensionType) { @@ -117,18 +144,12 @@ public class DefaultRegistry implements Registry { public void bind(Class key, Consumer> configure) { final var configurator = new SimpleBindingConfigurator<>(key); configure.accept(configurator); - this.classBindings.add(new ClassKeyBinding<>(key, configurator.getBinding())); + this.bindingContainer.put(key, configurator.getBinding()); } - @SuppressWarnings("unchecked") @Override - public @Nullable Binding getBinding(Class key) { - for (final var classKeyBinding : this.classBindings) { - if (key.isAssignableFrom(classKeyBinding.key())) { - return (Binding) classKeyBinding.binding(); - } - } - return null; + public @Nullable Binding getBinding(Class key) { + return this.bindingContainer.get(key); } private KeyBinder findKeyBinder(Class keyClass) { @@ -189,7 +210,7 @@ public class DefaultRegistry implements Registry { @Override public void clearAllBindings() { - this.classBindings.clear(); + this.bindingContainer.clear(); for (final var extension : this.extensions) { if (extension instanceof KeyBinder keyBinder) { keyBinder.clearAllBindings(); diff --git a/util/di/src/main/java/groowt/util/di/SingletonScopeHandler.java b/util/di/src/main/java/groowt/util/di/SingletonScopeHandler.java index 0be9ab6..56f884e 100644 --- a/util/di/src/main/java/groowt/util/di/SingletonScopeHandler.java +++ b/util/di/src/main/java/groowt/util/di/SingletonScopeHandler.java @@ -1,8 +1,9 @@ package groowt.util.di; +import jakarta.inject.Provider; import jakarta.inject.Singleton; -import static groowt.util.di.BindingUtil.toSingleton; +import static groowt.util.di.BindingUtil.toLazySingleton; public final class SingletonScopeHandler implements ScopeHandler { @@ -19,12 +20,22 @@ public final class SingletonScopeHandler implements ScopeHandler { RegistryObjectFactory objectFactory ) { final Binding potentialBinding = this.owner.getBinding(dependencyClass); - if (potentialBinding != null) { - return potentialBinding; - } else { - this.owner.bind(dependencyClass, toSingleton(objectFactory.createInstance(dependencyClass))); - return this.owner.getBinding(dependencyClass); - } + return switch (potentialBinding) { + case ClassBinding(Class from, Class to) -> { + this.owner.bind(from, toLazySingleton(() -> objectFactory.createInstance(to))); + yield this.owner.getBinding(from); + } + case ProviderBinding(Class from, Provider provider) -> { + this.owner.bind(from, toLazySingleton(provider::get)); + yield this.owner.getBinding(from); + } + case SingletonBinding singletonBinding -> singletonBinding; + case LazySingletonBinding lazySingletonBinding -> lazySingletonBinding; + case null -> { + this.owner.bind(dependencyClass, toLazySingleton(() -> objectFactory.createInstance(dependencyClass))); + yield this.owner.getBinding(dependencyClass); + } + }; } @Override diff --git a/util/di/src/test/java/groowt/util/di/DefaultRegistryObjectFactoryTests.java b/util/di/src/test/java/groowt/util/di/DefaultRegistryObjectFactoryTests.java index d733ada..5381de8 100644 --- a/util/di/src/test/java/groowt/util/di/DefaultRegistryObjectFactoryTests.java +++ b/util/di/src/test/java/groowt/util/di/DefaultRegistryObjectFactoryTests.java @@ -2,6 +2,7 @@ package groowt.util.di; import jakarta.inject.Inject; import jakarta.inject.Named; +import jakarta.inject.Singleton; import org.junit.jupiter.api.Test; import static groowt.util.di.BindingUtil.*; @@ -250,4 +251,25 @@ public class DefaultRegistryObjectFactoryTests { assertEquals("Given Greeting", g.greet()); } + @Singleton + public static final class SingletonGreeter implements Greeter { + + @Override + public String greet() { + return "Hello, World!"; + } + + } + + @Test + public void singletonDoesNotOverflow() { + final var b = DefaultRegistryObjectFactory.Builder.withDefaults(); + b.configureRegistry(r -> { + r.bind(SingletonGreeter.class, toSelf()); + }); + final var f = b.build(); + final var g = f.get(SingletonGreeter.class); + assertEquals("Hello, World!", g.greet()); + } + }