From 4f2aa80a0866ce827bcf114ce5fccb6dcf4379e5 Mon Sep 17 00:00:00 2001
From: Matej Novotny <manovotn@redhat.com>
Date: Sun, 10 Sep 2023 11:55:44 +0200
Subject: [PATCH] Revisit API for BeanContainer

---
 .../arc/test/unused/UnusedExclusionTest.java  |  2 +-
 .../io/quarkus/arc/runtime/BeanContainer.java | 73 ++++++++-----------
 .../arc/runtime/BeanContainerImpl.java        | 36 +++++++--
 .../java/io/quarkus/arc/ArcContainer.java     | 21 ------
 .../io/quarkus/arc/impl/ArcContainerImpl.java | 26 +------
 5 files changed, 62 insertions(+), 96 deletions(-)

diff --git a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unused/UnusedExclusionTest.java b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unused/UnusedExclusionTest.java
index 145dc34e435fe..b95f83bd9c860 100644
--- a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unused/UnusedExclusionTest.java
+++ b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unused/UnusedExclusionTest.java
@@ -74,7 +74,7 @@ public static class TestRecorder {
 
         public void test(BeanContainer beanContainer) {
             // This should trigger the warning - Gama was removed
-            Gama gama = beanContainer.beanInstance(Gama.class);
+            Gama gama = beanContainer.beanInstanceFactory(Gama.class).create().get();
             // Test that fallback was used - no injection was performed
             Assertions.assertNull(gama.beanManager);
         }
diff --git a/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainer.java b/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainer.java
index 7594bb3bb9c47..3be50196c7eba 100644
--- a/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainer.java
+++ b/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainer.java
@@ -1,73 +1,62 @@
 package io.quarkus.arc.runtime;
 
 import java.lang.annotation.Annotation;
+import java.util.function.Supplier;
 
 import io.quarkus.arc.ManagedContext;
 
 /**
  * Represents a CDI bean container.
+ * <p/>
+ * Extensions using this API can also leverage arbitrary methods from running {@link io.quarkus.arc.ArcContainer}
+ * which can be obtained by invoking a static method {@link io.quarkus.arc.Arc#container()}.
  */
 public interface BeanContainer {
 
     /**
-     * Returns a bean instance for given bean type and qualifiers.
+     * Resolves a bean instance for given bean type and qualifiers.
      * <p/>
-     * This method follows standard CDI rules meaning that if there are two or more eligible beans, an ambiguous
-     * dependency exception is thrown.
-     * Note that the method is allowed to return {@code null} if there is no matching bean which allows
-     * for fallback implementations.
+     * Performs standard CDI resolution meaning it either returns a bean instance or throws a corresponding exception
+     * if the dependency is either unsatisfied or ambiguous.
      *
-     * @param type
-     * @param qualifiers
-     * @return a bean instance or {@code null} if no matching bean is found
+     * @param beanType type of the bean
+     * @param beanQualifiers bean qualifiers
+     * @return a bean instance; never {@code null}
      */
-    default <T> T beanInstance(Class<T> type, Annotation... qualifiers) {
-        return beanInstanceFactory(type, qualifiers).create().get();
-    }
-
-    /**
-     * This method is deprecated and will be removed in future versions.
-     * Use {@link #beanInstance(Class, Annotation...)} instead.
-     * </p>
-     * As opposed to {@link #beanInstance(Class, Annotation...)}, this method does <b>NOT</b> follow CDI
-     * resolution rules and in case of ambiguous resolution performs a choice based on the class type parameter.
-     *
-     * @param type
-     * @param qualifiers
-     * @return a bean instance or {@code null} if no matching bean is found
-     */
-    @Deprecated
-    default <T> T instance(Class<T> type, Annotation... qualifiers) {
-        return instanceFactory(type, qualifiers).create().get();
-    }
+    <T> T beanInstance(Class<T> beanType, Annotation... beanQualifiers);
 
     /**
      * Returns an instance factory for given bean type and qualifiers.
      * <p/>
-     * This method follows standard CDI rules meaning that if there are two or more beans, an ambiguous dependency
-     * exception is thrown.
-     * Note that the factory itself is still allowed to return {@code null} if there is no matching bean which allows
-     * for fallback implementations.
+     * This method performs CDI ambiguous dependency resolution and throws and exception if there are two or more beans
+     * with given type and qualifiers.
+     * <p/>
+     * If no matching bean is found, uses a default fallback factory that will attempt to instantiate a non-CDI object
+     * of the given class via no-args constructor.
+     * <p/>
+     * If you need custom factory behavior, take a look at {@link #beanInstanceFactory(Supplier, Class, Annotation...)}
      *
-     * @param type
-     * @param qualifiers
+     * @param type bean type
+     * @param qualifiers bean qualifiers
      * @return a bean instance factory, never {@code null}
      */
     <T> Factory<T> beanInstanceFactory(Class<T> type, Annotation... qualifiers);
 
     /**
-     * This method is deprecated and will be removed in future versions.
-     * Use {@link #beanInstanceFactory(Class, Annotation...)} instead.
-     * </p>
-     * As opposed to {@link #beanInstanceFactory(Class, Annotation...)}, this method does <b>NOT</b> follow CDI
-     * resolution rules and in case of ambiguous resolution performs a choice based on the class type parameter.
+     * Returns an instance factory for given bean type and qualifiers.
+     * <p/>
+     * This method performs CDI ambiguous dependency resolution and throws and exception if there are two or more beans
+     * with given type and qualifiers.
+     * <p/>
+     * If no matching bean is found, delegates all calls to the supplied factory fallback.
      *
-     * @param type
-     * @param qualifiers
+     * @param fallbackSupplier supplier to delegate to if there is no bean
+     * @param type bean type
+     * @param qualifiers bean qualifiers
      * @return a bean instance factory, never {@code null}
      */
-    @Deprecated
-    <T> Factory<T> instanceFactory(Class<T> type, Annotation... qualifiers);
+    <T> Factory<T> beanInstanceFactory(Supplier<Factory<T>> fallbackSupplier, Class<T> type,
+            Annotation... qualifiers);
 
     /**
      * <pre>
diff --git a/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainerImpl.java b/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainerImpl.java
index 8f93f767bcadb..51b2cf3627224 100644
--- a/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainerImpl.java
+++ b/extensions/arc/runtime/src/main/java/io/quarkus/arc/runtime/BeanContainerImpl.java
@@ -21,24 +21,36 @@ class BeanContainerImpl implements BeanContainer {
         this.container = container;
     }
 
+    @Override
+    public <T> T beanInstance(Class<T> beanType, Annotation... beanQualifiers) {
+        return container.select(beanType, beanQualifiers).get();
+    }
+
     @Override
     public <T> Factory<T> beanInstanceFactory(Class<T> type, Annotation... qualifiers) {
         Supplier<InstanceHandle<T>> handleSupplier = container.beanInstanceSupplier(type, qualifiers);
-        return createFactory(handleSupplier, type, qualifiers);
+        return createFactory(handleSupplier, null, type, qualifiers);
     }
 
     @Override
-    public <T> Factory<T> instanceFactory(Class<T> type, Annotation... qualifiers) {
-        Supplier<InstanceHandle<T>> handleSupplier = container.instanceSupplier(type, qualifiers);
-        return createFactory(handleSupplier, type, qualifiers);
+    public <T> Factory<T> beanInstanceFactory(Supplier<Factory<T>> fallbackSupplier, Class<T> type,
+            Annotation... qualifiers) {
+        Supplier<InstanceHandle<T>> handleSupplier = container.beanInstanceSupplier(type, qualifiers);
+        return createFactory(handleSupplier, fallbackSupplier, type, qualifiers);
     }
 
-    private <T> Factory<T> createFactory(Supplier<InstanceHandle<T>> handleSupplier, Class<T> type, Annotation... qualifiers) {
+    private <T> Factory<T> createFactory(Supplier<InstanceHandle<T>> handleSupplier, Supplier<Factory<T>> fallbackSupplier,
+            Class<T> type, Annotation... qualifiers) {
         if (handleSupplier == null) {
             LOGGER.debugf(
                     "No matching bean found for type %s and qualifiers %s. The bean might have been marked as unused and removed during build.",
                     type, Arrays.toString(qualifiers));
-            return new DefaultInstanceFactory<>(type);
+            if (fallbackSupplier != null) {
+                return fallbackSupplier.get();
+            } else {
+                // by default, if there is no bean, return factory that tries to instantiate non-cdi object
+                return new DefaultInstanceFactory<>(type);
+            }
         }
         return new Factory<T>() {
             @Override
@@ -64,7 +76,16 @@ public ManagedContext requestContext() {
         return container.requestContext();
     }
 
-    private static final class DefaultInstanceFactory<T> implements BeanContainer.Factory<T> {
+    /**
+     * A default fallback {@link Factory} implementation used by
+     * {@link BeanContainer#beanInstanceFactory(Class, Annotation...)}.
+     * <p/>
+     * This factory attempts to create instances of given class by calling their no-arg constructor. Any exceptions
+     * related to lack of such constructor of failure to invoke it are simply re-thrown.
+     *
+     * @param <T> represents the type that this factory can create
+     */
+    private final class DefaultInstanceFactory<T> implements BeanContainer.Factory<T> {
 
         private final Class<T> type;
 
@@ -87,5 +108,4 @@ public T get() {
             }
         }
     }
-
 }
diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java
index 8baf18521209e..01af040bd7399 100644
--- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java
+++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java
@@ -88,12 +88,6 @@ public interface ArcContainer {
     /**
      * Returns a supplier that can be used to create new instances, or null if no matching bean can be found.
      *
-     * Note that if there are multiple sub classes of the given type this will return the exact match. This means
-     * that this can be used to directly instantiate superclasses of other beans without causing problems. This behavior differs
-     * to standard CDI rules where an ambiguous dependency would exist.
-     *
-     * see https://github.com/quarkusio/quarkus/issues/3369
-     *
      * @param type
      * @param qualifiers
      * @param <T>
@@ -101,21 +95,6 @@ public interface ArcContainer {
      */
     <T> Supplier<InstanceHandle<T>> beanInstanceSupplier(Class<T> type, Annotation... qualifiers);
 
-    /**
-     * This method is deprecated and will be removed in future versions.
-     * Use {@link #beanInstanceSupplier(Class, Annotation...)} instead.
-     * </p>
-     * As opposed to {@link #beanInstanceSupplier(Class, Annotation...)}, this method does <b>NOT</b> follow CDI
-     * resolution rules and in case of ambiguous resolution performs a choice based on the class type parameter.
-     *
-     * @param type
-     * @param qualifiers
-     * @return
-     * @param <T>
-     */
-    @Deprecated
-    <T> Supplier<InstanceHandle<T>> instanceSupplier(Class<T> type, Annotation... qualifiers);
-
     /**
      *
      * @param bean
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 12f6a9ad0a65b..a01607c33fc96 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
@@ -291,16 +291,6 @@ public <X> InstanceHandle<X> instance(Type type, Annotation... qualifiers) {
 
     @Override
     public <T> Supplier<InstanceHandle<T>> beanInstanceSupplier(Class<T> type, Annotation... qualifiers) {
-        return createInstanceSupplier(false, type, qualifiers);
-    }
-
-    @Override
-    public <T> Supplier<InstanceHandle<T>> instanceSupplier(Class<T> type, Annotation... qualifiers) {
-        return createInstanceSupplier(true, type, qualifiers);
-    }
-
-    private <T> Supplier<InstanceHandle<T>> createInstanceSupplier(boolean resolveAmbiguities, Class<T> type,
-            Annotation... qualifiers) {
         if (qualifiers == null || qualifiers.length == 0) {
             qualifiers = new Annotation[] { Default.Literal.INSTANCE };
         }
@@ -311,20 +301,8 @@ private <T> Supplier<InstanceHandle<T>> createInstanceSupplier(boolean resolveAm
         }
         Set<InjectableBean<?>> filteredBean = resolvedBeans;
         if (resolvedBeans.size() > 1) {
-            if (resolveAmbiguities) {
-                // this is non-standard CDI behavior that we momentarily keep to retain compatibility
-                // if there are multiple beans we look for an exact match
-                // this method is only called with the exact type required
-                // so ignoring subclasses is the correct behaviour
-                filteredBean = new HashSet<>();
-                for (InjectableBean<?> i : resolvedBeans) {
-                    if (i.getBeanClass().equals(type)) {
-                        filteredBean.add(i);
-                    }
-                }
-            } else {
-                throw new AmbiguousResolutionException("Beans: " + resolvedBeans);
-            }
+            throw new AmbiguousResolutionException("Beans: " + resolvedBeans);
+
         }
         @SuppressWarnings("unchecked")
         InjectableBean<T> bean = filteredBean.size() != 1 ? null