From ffc7bb907387c16dfb0e3833a215dee0b6dc30dc Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Mon, 20 Sep 2021 15:29:51 +1000 Subject: [PATCH 1/2] Fix issue with default beans resolution If there is more than one Foo for Instance then default beans are included in the result list. This result in HTTP basic auth always being enabled if there is more than one authenticator. --- .../io/quarkus/arc/impl/ArcContainerImpl.java | 13 +++-- .../defaultbean/DefaultClassBeanTest.java | 49 ++++++++++++++++++- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java index 11b52ed2909a6..c7b2773ff6660 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java @@ -569,13 +569,14 @@ private static Set> resolve(List> matching) } // Try to resolve the ambiguity - List> resolved = new ArrayList<>(matching); + List> nonDefault = new ArrayList<>(matching); - resolved.removeIf(InjectableBean::isDefaultBean); - if (resolved.size() == 1) { - return Collections.singleton(resolved.get(0)); + nonDefault.removeIf(InjectableBean::isDefaultBean); + if (nonDefault.size() == 1) { + return Collections.singleton(nonDefault.get(0)); } + List> resolved = new ArrayList<>(nonDefault); resolved.removeIf(not(ArcContainerImpl::isAlternativeOrDeclaredOnAlternative)); if (resolved.size() == 1) { return Collections.singleton(resolved.get(0)); @@ -587,8 +588,10 @@ private static Set> resolve(List> matching) if (resolved.size() == 1) { return Collections.singleton(resolved.get(0)); } + return new HashSet<>(resolved); } - return new HashSet<>(matching); + //return all non-default beans + return new HashSet<>(nonDefault); } private static boolean isAlternativeOrDeclaredOnAlternative(InjectableBean bean) { diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java index eb5b24fa5de1f..0f9b8f4dbfb26 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java @@ -6,10 +6,12 @@ import io.quarkus.arc.DefaultBean; import io.quarkus.arc.test.ArcTestContainer; import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.inject.Instance; import javax.enterprise.inject.Produces; import javax.enterprise.inject.spi.CDI; import javax.inject.Inject; import javax.inject.Singleton; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -17,13 +19,21 @@ public class DefaultClassBeanTest { @RegisterExtension public ArcTestContainer container = new ArcTestContainer(Producer.class, - GreetingBean.class, Hello.class, PingBean.class); + GreetingBean.class, Hello.class, PingBean.class, Author.class, SciFi.class, Fantasy.class, Detective.class); @Test public void testInjection() { Hello hello = Arc.container().instance(Hello.class).get(); assertEquals("hello", hello.hello()); assertEquals("pong", hello.ping()); + var result = hello.instance(); + StringBuilder sb = new StringBuilder(); + for (var i : result) { + i.write(sb); + } + Assertions.assertTrue(sb.toString().contains("SciFi")); + Assertions.assertTrue(sb.toString().contains("Fantasy")); + Assertions.assertFalse(sb.toString().contains("Detective")); } @Test @@ -40,6 +50,9 @@ static class Hello { @Inject PingBean ping; + @Inject + Instance instance; + String hello() { return bean.greet(); } @@ -48,6 +61,9 @@ String ping() { return ping.ping(); } + public Instance instance() { + return instance; + } } @DefaultBean // This one is overriden by Producer.greetingBean() @@ -85,4 +101,35 @@ String greet() { } + interface Author { + void write(StringBuilder sb); + } + + @Singleton + static class SciFi implements Author { + + @Override + public void write(StringBuilder sb) { + sb.append("SciFi"); + } + } + + @Singleton + static class Fantasy implements Author { + + @Override + public void write(StringBuilder sb) { + sb.append("Fantasy"); + } + } + + @Singleton + @DefaultBean + static class Detective implements Author { + + @Override + public void write(StringBuilder sb) { + sb.append("Detective"); + } + } } From 7d7c9fb251976f47d4aa6cb8b605ef941e2fce2c Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Mon, 20 Sep 2021 11:20:48 +0200 Subject: [PATCH 2/2] Improve the algorithm, add javadoc to InjectableInstance - also fix HealthCheckDefaultScopeTest --- .../test/HealthCheckDefaultScopeTest.java | 5 +- .../io/quarkus/arc/InjectableInstance.java | 16 ++++++ .../io/quarkus/arc/impl/ArcContainerImpl.java | 51 ++++++++++--------- .../defaultbean/DefaultClassBeanTest.java | 7 ++- .../defaultbean/DefaultProducerFieldTest.java | 47 ++++++++++++++++- 5 files changed, 99 insertions(+), 27 deletions(-) diff --git a/extensions/smallrye-health/deployment/src/test/java/io/quarkus/smallrye/health/test/HealthCheckDefaultScopeTest.java b/extensions/smallrye-health/deployment/src/test/java/io/quarkus/smallrye/health/test/HealthCheckDefaultScopeTest.java index c7dd3f7983b3b..c7befa8178085 100644 --- a/extensions/smallrye-health/deployment/src/test/java/io/quarkus/smallrye/health/test/HealthCheckDefaultScopeTest.java +++ b/extensions/smallrye-health/deployment/src/test/java/io/quarkus/smallrye/health/test/HealthCheckDefaultScopeTest.java @@ -6,6 +6,7 @@ import static java.lang.annotation.ElementType.TYPE; import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.is; import java.lang.annotation.Retention; @@ -41,11 +42,11 @@ public void testHealth() { when().get("/q/health/live").then() .body("status", is("UP"), "checks.status", contains("UP", "UP"), - "checks.name", contains("noScope", "noScopeStereotype")); + "checks.name", containsInAnyOrder("noScope", "noScopeStereotype")); when().get("/q/health/live").then() .body("status", is("DOWN"), "checks.status", contains("DOWN", "DOWN"), - "checks.name", contains("noScope", "noScopeStereotype")); + "checks.name", containsInAnyOrder("noScope", "noScopeStereotype")); } finally { RestAssured.reset(); } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableInstance.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableInstance.java index 41ffcb3fceda1..ccf3b100789d9 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableInstance.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableInstance.java @@ -1,6 +1,7 @@ package io.quarkus.arc; import java.lang.annotation.Annotation; +import java.util.Iterator; import javax.enterprise.context.Dependent; import javax.enterprise.inject.Instance; import javax.enterprise.util.TypeLiteral; @@ -33,4 +34,19 @@ public interface InjectableInstance extends Instance { */ void clearCache(); + /** + * This method attempts to resolve ambiguities. + *

+ * In general, if multiple beans are eligible then the container eliminates all beans that are: + *

    + *
  • not alternatives, except for producer methods and fields of beans that are alternatives,
    • + *
  • default beans.
    • + *
+ * + * @return an iterator over the contextual references of the disambiguated beans + * @see DefaultBean + */ + @Override + Iterator iterator(); + } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java index c7b2773ff6660..5681fa2b86bb6 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java @@ -565,33 +565,38 @@ private static Set> resolve(List> matching) if (matching.isEmpty()) { return Collections.emptySet(); } else if (matching.size() == 1) { - return Collections.singleton(matching.get(0)); + return Set.of(matching.get(0)); } + // Try to resolve the ambiguity and return the set of disambiguated beans - // Try to resolve the ambiguity + // First remove the default beans List> nonDefault = new ArrayList<>(matching); - nonDefault.removeIf(InjectableBean::isDefaultBean); - if (nonDefault.size() == 1) { - return Collections.singleton(nonDefault.get(0)); - } - - List> resolved = new ArrayList<>(nonDefault); - resolved.removeIf(not(ArcContainerImpl::isAlternativeOrDeclaredOnAlternative)); - if (resolved.size() == 1) { - return Collections.singleton(resolved.get(0)); - } else if (resolved.size() > 1) { - resolved.sort(ArcContainerImpl::compareAlternativeBeans); - // Keep only the highest priorities - Integer highest = getAlternativePriority(resolved.get(0)); - resolved.removeIf(injectableBean -> !highest.equals(getAlternativePriority(injectableBean))); - if (resolved.size() == 1) { - return Collections.singleton(resolved.get(0)); - } - return new HashSet<>(resolved); - } - //return all non-default beans - return new HashSet<>(nonDefault); + if (nonDefault.isEmpty()) { + // All the matching beans were default + return Set.copyOf(matching); + } else if (nonDefault.size() == 1) { + return Set.of(nonDefault.get(0)); + } + + // More than one non-default bean remains - eliminate beans that don't have a priority + List> priorityBeans = new ArrayList<>(nonDefault); + priorityBeans.removeIf(not(ArcContainerImpl::isAlternativeOrDeclaredOnAlternative)); + if (priorityBeans.isEmpty()) { + // No alternative/priority beans are present + return Set.copyOf(nonDefault); + } else if (priorityBeans.size() == 1) { + return Set.of(priorityBeans.get(0)); + } else { + // Keep only the highest priorities + priorityBeans.sort(ArcContainerImpl::compareAlternativeBeans); + Integer highest = getAlternativePriority(priorityBeans.get(0)); + priorityBeans.removeIf(bean -> !highest.equals(getAlternativePriority(bean))); + if (priorityBeans.size() == 1) { + return Set.of(priorityBeans.get(0)); + } + return Set.copyOf(priorityBeans); + } } private static boolean isAlternativeOrDeclaredOnAlternative(InjectableBean bean) { diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java index 0f9b8f4dbfb26..8f34f2ec435b2 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultClassBeanTest.java @@ -5,6 +5,8 @@ import io.quarkus.arc.Arc; import io.quarkus.arc.DefaultBean; import io.quarkus.arc.test.ArcTestContainer; +import java.util.Set; +import java.util.stream.Collectors; import javax.enterprise.context.ApplicationScoped; import javax.enterprise.inject.Instance; import javax.enterprise.inject.Produces; @@ -34,6 +36,9 @@ public void testInjection() { Assertions.assertTrue(sb.toString().contains("SciFi")); Assertions.assertTrue(sb.toString().contains("Fantasy")); Assertions.assertFalse(sb.toString().contains("Detective")); + Set detectives = Arc.container().beanManager().createInstance().select(Detective.class).stream() + .collect(Collectors.toSet()); + Assertions.assertEquals(1, detectives.size()); } @Test @@ -61,7 +66,7 @@ String ping() { return ping.ping(); } - public Instance instance() { + Instance instance() { return instance; } } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultProducerFieldTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultProducerFieldTest.java index 6853c7922d557..e86d047bb4e82 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultProducerFieldTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/defaultbean/DefaultProducerFieldTest.java @@ -1,11 +1,15 @@ package io.quarkus.arc.test.defaultbean; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; import io.quarkus.arc.Arc; import io.quarkus.arc.DefaultBean; import io.quarkus.arc.test.ArcTestContainer; +import java.util.List; +import java.util.stream.Collectors; import javax.enterprise.context.ApplicationScoped; +import javax.enterprise.inject.Instance; import javax.enterprise.inject.Produces; import javax.enterprise.inject.spi.CDI; import javax.inject.Inject; @@ -17,7 +21,7 @@ public class DefaultProducerFieldTest { @RegisterExtension public ArcTestContainer container = new ArcTestContainer(Producer.class, - GreetingBean.class, Hello.class); + GreetingBean.class, Hello.class, Fantasy.class); @Test public void testInjection() { @@ -29,16 +33,32 @@ public void testSelect() { assertEquals("hola", CDI.current().select(GreetingBean.class).get().greet()); } + @Test + public void testInstanceIterator() { + List authors = Arc.container().instance(Hello.class).get().instance().stream().collect(Collectors.toList()); + assertEquals(2, authors.size()); + String result = authors.stream().map(Author::get).collect(Collectors.joining()); + assertTrue(result.contains("SciFi")); + assertTrue(result.contains("Fantasy")); + } + @ApplicationScoped static class Hello { @Inject GreetingBean bean; + @Inject + Instance instance; + String hello() { return bean.greet(); } + Instance instance() { + return instance; + } + } @Singleton @@ -63,6 +83,31 @@ String greet() { }; + @Produces + @Singleton + @DefaultBean + Author sciFi = new Author() { + + @Override + public String get() { + return "SciFi"; + } + }; + + } + + interface Author { + String get(); + } + + @Singleton + @DefaultBean + static class Fantasy implements Author { + + @Override + public String get() { + return "Fantasy"; + } } }