Skip to content

Commit

Permalink
#1113 Ensure hierarchy is resolved correctly on binding action, clear…
Browse files Browse the repository at this point in the history
… scope on top-level fallback lookup
  • Loading branch information
GuusLieben committed Oct 25, 2024
1 parent 66e05e9 commit d694c50
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ public interface Binder {
* @return The binding function
*/
default <C> BindingFunction<C> bind(Class<C> type) {
return this.bind(ComponentKey.of(type));
// Strict, so new hierarchies are created if needed, rather than using loose lookup
ComponentKey<C> componentKey = ComponentKey.builder(type)
.strict(true)
.build();
return this.bind(componentKey);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ public HierarchyCache(
this.binder = binder;
}

public void put(BindingHierarchy<?> hierarchy) {
this.hierarchies.put(hierarchy.key().view(), hierarchy);
public <T> void put(BindingHierarchy<T> hierarchy) {
put(hierarchy.key().view(), hierarchy);
}

public <T> void put(ComponentKeyView<T> view, BindingHierarchy<T> updated) {
Expand Down Expand Up @@ -95,7 +95,13 @@ private <T> BindingHierarchy<?> computeHierarchy(ComponentKey<T> key, boolean us
// the application context. This is useful for components that are not explicitly scoped,
// but are still accessed through a scope.
if(useGlobalIfAbsent && this.globalBinder != this.binder) {
return this.globalBinder.hierarchy(key);
ComponentKey<T> unscopedKey = key.mutable()
// Need to drop the scope, otherwise we risk the global binder being an orchestrator
// which delegates based on the scope of the key, which would defeat the point of
// attempting a top-level lookup.
.scope(null)
.build();
return this.globalBinder.hierarchy(unscopedKey);
}
return new NativePrunableBindingHierarchy<>(key);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.dockbox.hartshorn.util.TypeUtils;
import org.dockbox.hartshorn.util.collections.HashSetMultiMap;
import org.dockbox.hartshorn.util.collections.MultiMap;
import org.dockbox.hartshorn.util.collections.UnmodifiableMultiMap;
import org.dockbox.hartshorn.util.option.Option;

public class ScopeAwareHierarchicalBinder implements HierarchicalBinder, ContainedHierarchyLookup {
Expand Down Expand Up @@ -59,6 +60,16 @@ protected Scope scope() {
return this.scope;
}

@Override
public <C> BindingFunction<C> bind(Class<C> type) {
// Strict, so new hierarchies are created if needed, rather than using loose lookup
ComponentKey<C> componentKey = ComponentKey.builder(type)
.strict(true)
.scope(this.scope) // No explicit scope provided, so expected to use the current scope instead
.build();
return this.bind(componentKey);
}

protected Scope applicationScope() {
return this.application.defaultProvider().scope();
}
Expand Down Expand Up @@ -103,7 +114,7 @@ public <T> BindingHierarchy<T> hierarchy(ComponentKey<T> key) {
public MultiMap<Scope, BindingHierarchy<?>> hierarchies() {
MultiMap<Scope, BindingHierarchy<?>> map = new HashSetMultiMap<>();
map.putAll(this.scope(), this.hierarchyCache().hierarchies());
return map;
return new UnmodifiableMultiMap<>(map);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.util.concurrent.ConcurrentHashMap;

import org.dockbox.hartshorn.inject.ComponentKey;
import org.dockbox.hartshorn.inject.ComponentKeyView;
import org.dockbox.hartshorn.util.IllegalModificationException;
import org.dockbox.hartshorn.util.option.Option;

Expand All @@ -37,32 +38,34 @@
*/
public class ConcurrentHashSingletonCache implements SingletonCache {

private final Map<ComponentKey<?>, Object> cache = new ConcurrentHashMap<>();
private final Set<ComponentKey<?>> locked = ConcurrentHashMap.newKeySet();
private final Map<ComponentKeyView<?>, Object> cache = new ConcurrentHashMap<>();
private final Set<ComponentKeyView<?>> locked = ConcurrentHashMap.newKeySet();

@Override
public void lock(ComponentKey<?> key) {
if (!this.cache.containsKey(key)) {
ComponentKeyView<?> keyView = new ComponentKeyView<>(key);
if (!this.cache.containsKey(keyView)) {
throw new IllegalModificationException("Cannot lock a key that is not present in the cache");
}
this.locked.add(key);
this.locked.add(keyView);
}

@Override
public <T> void put(ComponentKey<T> key, T instance) {
if (this.locked.contains(key) && this.cache.get(key) != instance) {
ComponentKeyView<T> keyView = new ComponentKeyView<>(key);
if (this.locked.contains(keyView) && this.cache.get(keyView) != instance) {
throw new IllegalModificationException("Another instance is already stored for key '" + key + "'");
}
this.cache.put(key, instance);
this.cache.put(keyView, instance);
}

@Override
public <T> Option<T> get(ComponentKey<T> key) {
return Option.of(key.type().cast(this.cache.get(key)));
return Option.of(key.type().cast(this.cache.get(new ComponentKeyView<>(key))));
}

@Override
public <T> boolean contains(ComponentKey<T> key) {
return this.cache.containsKey(key);
return this.cache.containsKey(new ComponentKeyView<>(key));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,26 +17,31 @@
package test.org.dockbox.hartshorn.inject.binding.defaults;

import org.dockbox.hartshorn.inject.annotations.Inject;
import org.dockbox.hartshorn.inject.component.ComponentContainer;
import org.dockbox.hartshorn.inject.component.ComponentRegistry;
import org.dockbox.hartshorn.test.junit.HartshornIntegrationTest;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
import org.slf4j.Logger;

import test.org.dockbox.hartshorn.launchpad.context.ApplicationContextTests;

@HartshornIntegrationTest(includeBasePackages = false)
public class DefaultBindingsTests {

@Inject
private Logger loggerField;

@Inject
private ComponentRegistry componentRegistry;

@Test
void loggerCanBeInjected(@Inject Logger loggerParameter) {
Assertions.assertNotNull(loggerParameter);
// Name should match the consuming class' name, and not the name of the configuration that uses it
Assertions.assertEquals(loggerParameter.getName(), ApplicationContextTests.class.getName());
ComponentContainer<?> container = componentRegistry.container(this.getClass()).orElseGet(Assertions::fail);
String expectedName = container.name();
Assertions.assertEquals(expectedName, loggerParameter.getName());

Assertions.assertNotNull(this.loggerField);
Assertions.assertEquals(this.loggerField.getName(), ApplicationContextTests.class.getName());
Assertions.assertEquals(expectedName, this.loggerField.getName());
}
}

0 comments on commit d694c50

Please sign in to comment.