From 97d48fa7714646899a33858d1b6bfccfcf8b6629 Mon Sep 17 00:00:00 2001
From: Matej Novotny <manovotn@redhat.com>
Date: Wed, 6 Sep 2023 12:26:59 +0200
Subject: [PATCH] Arc - Revisit BeanContainer API

---
 .../arc/test/unused/UnusedExclusionTest.java  |  5 +-
 .../io/quarkus/arc/runtime/BeanContainer.java | 59 +++++--------------
 .../arc/runtime/BeanContainerImpl.java        | 37 ++----------
 .../common/runtime/ArcBeanFactory.java        | 37 +++++-------
 .../java/io/quarkus/arc/ArcContainer.java     | 21 -------
 .../io/quarkus/arc/impl/ArcContainerImpl.java | 26 +-------
 6 files changed, 40 insertions(+), 145 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 145dc34e435fe8..02d9d987184f67 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,9 +74,8 @@ public static class TestRecorder {
 
         public void test(BeanContainer beanContainer) {
             // This should trigger the warning - Gama was removed
-            Gama gama = beanContainer.beanInstance(Gama.class);
-            // Test that fallback was used - no injection was performed
-            Assertions.assertNull(gama.beanManager);
+            // Test that null was returned as there is no Gama bean present
+            Assertions.assertNull(beanContainer.beanInstanceFactory(Gama.class).create());
         }
 
     }
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 7594bb3bb9c47c..a3ff9058bf00cc 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
@@ -6,69 +6,38 @@
 
 /**
  * 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.
+     * However, note that the factory itself is still allowed to return {@code null} if there is no matching bean which
+     * allows for fallback implementations.
      *
-     * @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.
-     *
-     * @param type
-     * @param qualifiers
-     * @return a bean instance factory, never {@code null}
-     */
-    @Deprecated
-    <T> Factory<T> instanceFactory(Class<T> type, Annotation... qualifiers);
-
     /**
      * <pre>
      * ManagedContext requestContext = beanContainer.requestContext();
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 8f93f767bcadbe..74eaf6f1bd7f2a 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
@@ -1,7 +1,6 @@
 package io.quarkus.arc.runtime;
 
 import java.lang.annotation.Annotation;
-import java.lang.reflect.InvocationTargetException;
 import java.util.Arrays;
 import java.util.function.Supplier;
 
@@ -22,14 +21,13 @@ class BeanContainerImpl implements BeanContainer {
     }
 
     @Override
-    public <T> Factory<T> beanInstanceFactory(Class<T> type, Annotation... qualifiers) {
-        Supplier<InstanceHandle<T>> handleSupplier = container.beanInstanceSupplier(type, qualifiers);
-        return createFactory(handleSupplier, type, qualifiers);
+    public <T> T beanInstance(Class<T> beanType, Annotation... beanQualifiers) {
+        return container.select(beanType, beanQualifiers).get();
     }
 
     @Override
-    public <T> Factory<T> instanceFactory(Class<T> type, Annotation... qualifiers) {
-        Supplier<InstanceHandle<T>> handleSupplier = container.instanceSupplier(type, qualifiers);
+    public <T> Factory<T> beanInstanceFactory(Class<T> type, Annotation... qualifiers) {
+        Supplier<InstanceHandle<T>> handleSupplier = container.beanInstanceSupplier(type, qualifiers);
         return createFactory(handleSupplier, type, qualifiers);
     }
 
@@ -38,7 +36,8 @@ private <T> Factory<T> createFactory(Supplier<InstanceHandle<T>> handleSupplier,
             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);
+            // factories can return null if there is no bean, so we return empty factory
+            return (Factory<T>) Factory.EMPTY;
         }
         return new Factory<T>() {
             @Override
@@ -64,28 +63,4 @@ public ManagedContext requestContext() {
         return container.requestContext();
     }
 
-    private static final class DefaultInstanceFactory<T> implements BeanContainer.Factory<T> {
-
-        private final Class<T> type;
-
-        DefaultInstanceFactory(Class<T> type) {
-            this.type = type;
-        }
-
-        @Override
-        public BeanContainer.Instance<T> create() {
-            try {
-                T instance = type.getDeclaredConstructor().newInstance();
-                return new BeanContainer.Instance<T>() {
-                    @Override
-                    public T get() {
-                        return instance;
-                    }
-                };
-            } catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException e) {
-                throw new RuntimeException(e);
-            }
-        }
-    }
-
 }
diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/runtime/src/main/java/io/quarkus/resteasy/reactive/common/runtime/ArcBeanFactory.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/runtime/src/main/java/io/quarkus/resteasy/reactive/common/runtime/ArcBeanFactory.java
index 25656077526e07..ca75a1117c4744 100644
--- a/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/runtime/src/main/java/io/quarkus/resteasy/reactive/common/runtime/ArcBeanFactory.java
+++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-common/runtime/src/main/java/io/quarkus/resteasy/reactive/common/runtime/ArcBeanFactory.java
@@ -23,27 +23,22 @@ public String toString() {
     @Override
     public BeanInstance<T> createInstance() {
         BeanContainer.Instance<T> instance;
-        try {
-            instance = factory.create();
-            return new BeanInstance<T>() {
-                @Override
-                public T getInstance() {
-                    return instance.get();
-                }
-
-                @Override
-                public void close() {
-                    instance.close();
-                }
-            };
-        } catch (Exception e) {
-            if (factory.getClass().getName().contains("DefaultInstanceFactory")) {
-                throw new IllegalArgumentException(
-                        "Unable to create class '" + targetClassName
-                                + "'. To fix the problem, make sure this class is a CDI bean.",
-                        e);
-            }
-            throw e;
+        instance = factory.create();
+        if (instance == null) {
+            throw new IllegalArgumentException(
+                    "Unable to create class '" + targetClassName
+                            + "'. To fix the problem, make sure this class is a CDI bean.");
         }
+        return new BeanInstance<T>() {
+            @Override
+            public T getInstance() {
+                return instance.get();
+            }
+
+            @Override
+            public void close() {
+                instance.close();
+            }
+        };
     }
 }
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 8baf18521209eb..01af040bd73997 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 12f6a9ad0a65b9..a01607c33fc965 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