Synthetic observers w/ identical attributes should use unique identifier

extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/StartupBuildSteps.java

@@ -5,6 +5,7 @@
 import javax.enterprise.context.spi.CreationalContext;
 
 import org.jboss.jandex.AnnotationInstance;
+import org.jboss.jandex.AnnotationTarget.Kind;
 import org.jboss.jandex.AnnotationValue;
 import org.jboss.jandex.DotName;
 import org.jboss.logging.Logger;
@@ -97,6 +98,11 @@ private void registerStartupObserver(ObserverRegistrationPhaseBuildItem observer
         ObserverConfigurator configurator = observerRegistrationPhase.getContext().configure()
                 .beanClass(bean.getBeanClass())
                 .observedType(StartupEvent.class);
+        if (startup.target().kind() == Kind.METHOD) {
+            configurator.id(startup.target().asMethod().toString());
+        } else if (startup.target().kind() == Kind.FIELD) {
+            configurator.id(startup.target().asField().name());
+        }
         AnnotationValue priority = startup.value();
         if (priority != null) {
             configurator.priority(priority.asInt());

extensions/arc/deployment/src/test/java/io/quarkus/arc/test/startup/StartupAnnotationTest.java

@@ -72,18 +72,21 @@ public void transform(TransformationContext context) {
     @Test
     public void testStartup() {
         // StartMe, SingletonStartMe, ProducerStartMe, DependentStartMe
-        assertEquals(11, LOG.size(), "Unexpected number of log messages: " + LOG);
+        assertEquals(14, LOG.size(), "Unexpected number of log messages: " + LOG);
         assertEquals("startMe_c", LOG.get(0));
         assertEquals("startMe_c", LOG.get(1));
         assertEquals("startMe_pc", LOG.get(2));
         assertEquals("singleton_c", LOG.get(3));
         assertEquals("singleton_pc", LOG.get(4));
         assertEquals("producer_pc", LOG.get(5));
-        assertEquals("producer", LOG.get(6));
+        assertEquals("produce_long", LOG.get(6));
         assertEquals("producer_pd", LOG.get(7));
-        assertEquals("dependent_c", LOG.get(8));
-        assertEquals("dependent_pc", LOG.get(9));
-        assertEquals("dependent_pd", LOG.get(10));
+        assertEquals("producer_pc", LOG.get(8));
+        assertEquals("produce_string", LOG.get(9));
+        assertEquals("producer_pd", LOG.get(10));
+        assertEquals("dependent_c", LOG.get(11));
+        assertEquals("dependent_pc", LOG.get(12));
+        assertEquals("dependent_pd", LOG.get(13));
     }
 
     // This component should be started first
@@ -154,10 +157,17 @@ static class ProducerStartMe {
         @Startup(Integer.MAX_VALUE - 1)
         @Produces
         String produceString() {
-            LOG.add("producer");
+            LOG.add("produce_string");
             return "ok";
         }
 
+        @Startup(Integer.MAX_VALUE - 2)
+        @Produces
+        Long produceLong() {
+            LOG.add("produce_long");
+            return 1l;
+        }
+
         @PostConstruct
         void init() {
             LOG.add("producer_pc");

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java

@@ -970,7 +970,8 @@ private void addSyntheticBean(BeanInfo bean) {
     }
 
     private void addSyntheticObserver(ObserverConfigurator configurator) {
-        observers.add(ObserverInfo.create(this, configurator.beanClass, null, null, null, null, configurator.observedType,
+        observers.add(ObserverInfo.create(configurator.id, this, configurator.beanClass, null, null, null, null,
+                configurator.observedType,
                 configurator.observedQualifiers,
                 Reception.ALWAYS, configurator.transactionPhase, configurator.isAsync, configurator.priority,
                 observerTransformers, buildContext,

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ObserverConfigurator.java

@@ -14,24 +14,22 @@
 import org.jboss.jandex.Type.Kind;
 
 /**
+ * Configures a synthetic observer.
+ * <p>
  * This construct is not thread-safe.
+ * 
+ * @see ObserverRegistrar
  */
 public final class ObserverConfigurator implements Consumer<AnnotationInstance> {
 
     final Consumer<ObserverConfigurator> consumer;
-
+    String id;
     DotName beanClass;
-
     Type observedType;
-
     final Set<AnnotationInstance> observedQualifiers;
-
     int priority;
-
     boolean isAsync;
-
     TransactionPhase transactionPhase;
-
     Consumer<MethodCreator> notifyConsumer;
 
     public ObserverConfigurator(Consumer<ObserverConfigurator> consumer) {
@@ -42,6 +40,18 @@ public ObserverConfigurator(Consumer<ObserverConfigurator> consumer) {
         this.transactionPhase = TransactionPhase.IN_PROGRESS;
     }
 
+    /**
+     * A unique identifier should be used for multiple synthetic observer methods with the same
+     * attributes (including the bean class).
+     * 
+     * @param id
+     * @return self
+     */
+    public ObserverConfigurator id(String id) {
+        this.id = id;
+        return this;
+    }
+
     public ObserverConfigurator beanClass(DotName beanClass) {
         this.beanClass = beanClass;
         return this;

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ObserverGenerator.java

@@ -33,6 +33,8 @@
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
+import java.util.Optional;
 import java.util.Set;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
@@ -103,7 +105,10 @@ Collection<Resource> generate(ObserverInfo observer) {
 
         StringBuilder sigBuilder = new StringBuilder();
         if (observer.isSynthetic()) {
-            // This is not unique but best effort
+            // If a unique id is not specified then the signature is not unique but the best effort
+            if (observer.getId() != null) {
+                sigBuilder.append(observer.getId());
+            }
             sigBuilder.append(observer.getObservedType().toString()).append(observer.getQualifiers().toString())
                     .append(observer.isAsync()).append(observer.getPriority()).append(observer.getTransactionPhase());
         } else {
@@ -134,7 +139,22 @@ Collection<Resource> generate(ObserverInfo observer) {
             targetPackage = DotNames.packageName(observer.getObserverMethod().declaringClass().name());
         }
         String generatedName = generatedNameFromTarget(targetPackage, baseName.toString(), "");
+
+        Optional<Entry<ObserverInfo, String>> generatedClass = observerToGeneratedName.entrySet().stream()
+                .filter(e -> e.getValue().equals(generatedName)).findAny();
+
         observerToGeneratedName.put(observer, generatedName);
+        if (generatedClass.isPresent()) {
+            if (observer.isSynthetic()) {
+                throw new IllegalStateException(
+                        "A synthetic observer with the generated class name " + generatedName + " already exists for "
+                                + generatedClass.get().getKey());
+            } else {
+                // Inherited observer methods share the same generated class
+                return Collections.emptyList();
+            }
+        }
+
         if (existingClasses.contains(generatedName)) {
             return Collections.emptyList();
         }

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ObserverInfo.java

@@ -43,7 +43,7 @@ static ObserverInfo create(BeanInfo declaringBean, MethodInfo observerMethod, In
         } else {
             priority = ObserverMethod.DEFAULT_PRIORITY;
         }
-        return create(declaringBean.getDeployment(), declaringBean.getTarget().get().asClass().name(), declaringBean,
+        return create(null, declaringBean.getDeployment(), declaringBean.getTarget().get().asClass().name(), declaringBean,
                 observerMethod, injection,
                 eventParameter,
                 observerMethod.parameters().get(eventParameter.position()),
@@ -53,7 +53,7 @@ static ObserverInfo create(BeanInfo declaringBean, MethodInfo observerMethod, In
                 buildContext, jtaCapabilities, null);
     }
 
-    static ObserverInfo create(BeanDeployment beanDeployment, DotName beanClass, BeanInfo declaringBean,
+    static ObserverInfo create(String id, BeanDeployment beanDeployment, DotName beanClass, BeanInfo declaringBean,
             MethodInfo observerMethod, Injection injection,
             MethodParameterInfo eventParameter, Type observedType, Set<AnnotationInstance> qualifiers, Reception reception,
             TransactionPhase transactionPhase, boolean isAsync, int priority,
@@ -102,11 +102,12 @@ static ObserverInfo create(BeanDeployment beanDeployment, DotName beanClass, Bea
             LOGGER.warnf("The observer %s#%s makes use of '%s' transactional observers but no " +
                     "JTA capabilities were detected.", info, transactionPhase);
         }
-        return new ObserverInfo(beanDeployment, beanClass, declaringBean, observerMethod, injection, eventParameter, isAsync,
-                priority,
-                reception, transactionPhase, observedType, qualifiers, notify);
+        return new ObserverInfo(id, beanDeployment, beanClass, declaringBean, observerMethod, injection, eventParameter,
+                isAsync, priority, reception, transactionPhase, observedType, qualifiers, notify);
     }
 
+    private final String id;
+
     private final BeanDeployment beanDeployment;
 
     private final DotName beanClass;
@@ -137,11 +138,12 @@ static ObserverInfo create(BeanDeployment beanDeployment, DotName beanClass, Bea
 
     private final Consumer<MethodCreator> notify;
 
-    ObserverInfo(BeanDeployment beanDeployment, DotName beanClass, BeanInfo declaringBean, MethodInfo observerMethod,
+    ObserverInfo(String id, BeanDeployment beanDeployment, DotName beanClass, BeanInfo declaringBean, MethodInfo observerMethod,
             Injection injection,
             MethodParameterInfo eventParameter,
             boolean isAsync, int priority, Reception reception, TransactionPhase transactionPhase,
             Type observedType, Set<AnnotationInstance> qualifiers, Consumer<MethodCreator> notify) {
+        this.id = id;
         this.beanDeployment = beanDeployment;
         this.beanClass = beanClass;
         this.declaringBean = declaringBean;
@@ -168,6 +170,16 @@ public ObserverInfo asObserver() {
         return this;
     }
 
+    /**
+     * A unique identifier should be used for multiple synthetic observer methods with the same
+     * attributes (including the bean class).
+     * 
+     * @return the optional identifier
+     */
+    public String getId() {
+        return id;
+    }
+
     BeanDeployment getBeanDeployment() {
         return beanDeployment;
     }
@@ -312,6 +324,15 @@ int initEventMetadataParam(MethodInfo observerMethod) {
         return -1;
     }
 
+    @Override
+    public String toString() {
+        StringBuilder builder = new StringBuilder();
+        builder.append("ObserverInfo [beanClass=").append(beanClass).append(", priority=").append(priority).append(", isAsync=")
+                .append(isAsync).append(", reception=").append(reception).append(", transactionPhase=").append(transactionPhase)
+                .append(", observedType=").append(observedType).append(", qualifiers=").append(qualifiers).append("]");
+        return builder.toString();
+    }
+
     private static class ObserverTransformationContext extends AnnotationsTransformationContext<Set<AnnotationInstance>>
             implements TransformationContext {
 

independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ObserverRegistrar.java

@@ -16,7 +16,7 @@ public interface ObserverRegistrar extends BuildExtension {
     interface RegistrationContext extends BuildContext {
 
         /**
-         * Configura a new synthetic observer. The observer is not added to the deployment unless the
+         * Configure a new synthetic observer. The observer is not added to the deployment unless the
          * {@link ObserverConfigurator#done()} method is called.
          * 
          * @return a new synthetic observer configurator

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/ArcTestContainer.java

@@ -171,11 +171,7 @@ public Builder alternativePriorities(AlternativePriorities priorities) {
         }
 
         public ArcTestContainer build() {
-            return new ArcTestContainer(resourceReferenceProviders, beanClasses, resourceAnnotations, beanRegistrars,
-                    observerRegistrars, contextRegistrars, interceptorBindingRegistrars, annotationsTransformers,
-                    injectionsPointsTransformers,
-                    observerTransformers, beanDeploymentValidators, shouldFail, removeUnusedBeans, exclusions,
-                    alternativePriorities);
+            return new ArcTestContainer(this);
         }
 
     }
@@ -211,39 +207,41 @@ public ArcTestContainer build() {
     private final AlternativePriorities alternativePriorities;
 
     public ArcTestContainer(Class<?>... beanClasses) {
-        this(Collections.emptyList(), Arrays.asList(beanClasses), Collections.emptyList(), Collections.emptyList(),
-                Collections.emptyList(),
-                Collections.emptyList(), Collections.emptyList(), Collections.emptyList(),
-                Collections.emptyList(), Collections.emptyList(), Collections.emptyList(), false, false,
-                Collections.emptyList(), null);
+        this.resourceReferenceProviders = Collections.emptyList();
+        this.beanClasses = Arrays.asList(beanClasses);
+        this.resourceAnnotations = Collections.emptyList();
+        this.beanRegistrars = Collections.emptyList();
+        this.observerRegistrars = Collections.emptyList();
+        this.contextRegistrars = Collections.emptyList();
+        this.bindingRegistrars = Collections.emptyList();
+        this.annotationsTransformers = Collections.emptyList();
+        this.injectionPointsTransformers = Collections.emptyList();
+        this.observerTransformers = Collections.emptyList();
+        this.beanDeploymentValidators = Collections.emptyList();
+        this.buildFailure = new AtomicReference<Throwable>(null);
+        this.shouldFail = false;
+        this.removeUnusedBeans = false;
+        this.exclusions = Collections.emptyList();
+        this.alternativePriorities = null;
     }
 
-    public ArcTestContainer(List<Class<?>> resourceReferenceProviders, List<Class<?>> beanClasses,
-            List<Class<? extends Annotation>> resourceAnnotations,
-            List<BeanRegistrar> beanRegistrars, List<ObserverRegistrar> observerRegistrars,
-            List<ContextRegistrar> contextRegistrars,
-            List<InterceptorBindingRegistrar> bindingRegistrars,
-            List<AnnotationsTransformer> annotationsTransformers, List<InjectionPointsTransformer> ipTransformers,
-            List<ObserverTransformer> observerTransformers,
-            List<BeanDeploymentValidator> beanDeploymentValidators, boolean shouldFail, boolean removeUnusedBeans,
-            List<Predicate<BeanInfo>> exclusions,
-            AlternativePriorities alternativePriorities) {
-        this.resourceReferenceProviders = resourceReferenceProviders;
-        this.beanClasses = beanClasses;
-        this.resourceAnnotations = resourceAnnotations;
-        this.beanRegistrars = beanRegistrars;
-        this.observerRegistrars = observerRegistrars;
-        this.contextRegistrars = contextRegistrars;
-        this.bindingRegistrars = bindingRegistrars;
-        this.annotationsTransformers = annotationsTransformers;
-        this.injectionPointsTransformers = ipTransformers;
-        this.observerTransformers = observerTransformers;
-        this.beanDeploymentValidators = beanDeploymentValidators;
+    public ArcTestContainer(Builder builder) {
+        this.resourceReferenceProviders = builder.resourceReferenceProviders;
+        this.beanClasses = builder.beanClasses;
+        this.resourceAnnotations = builder.resourceAnnotations;
+        this.beanRegistrars = builder.beanRegistrars;
+        this.observerRegistrars = builder.observerRegistrars;
+        this.contextRegistrars = builder.contextRegistrars;
+        this.bindingRegistrars = builder.interceptorBindingRegistrars;
+        this.annotationsTransformers = builder.annotationsTransformers;
+        this.injectionPointsTransformers = builder.injectionsPointsTransformers;
+        this.observerTransformers = builder.observerTransformers;
+        this.beanDeploymentValidators = builder.beanDeploymentValidators;
         this.buildFailure = new AtomicReference<Throwable>(null);
-        this.shouldFail = shouldFail;
-        this.removeUnusedBeans = removeUnusedBeans;
-        this.exclusions = exclusions;
-        this.alternativePriorities = alternativePriorities;
+        this.shouldFail = builder.shouldFail;
+        this.removeUnusedBeans = builder.removeUnusedBeans;
+        this.exclusions = builder.exclusions;
+        this.alternativePriorities = builder.alternativePriorities;
     }
 
     // this is where we start Arc, we operate on a per-method basis

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/buildextension/observers/SyntheticObserverErrorTest.java

@@ -0,0 +1,35 @@
+package io.quarkus.arc.test.buildextension.observers;
+
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import io.quarkus.arc.processor.ObserverRegistrar;
+import io.quarkus.arc.test.ArcTestContainer;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+public class SyntheticObserverErrorTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = ArcTestContainer.builder()
+            .observerRegistrars(new ObserverRegistrar() {
+                @Override
+                public void register(RegistrationContext context) {
+                    context.configure().observedType(String.class).notify(mc -> {
+                        mc.returnValue(null);
+                    }).done();
+
+                    context.configure().observedType(String.class).notify(mc -> {
+                        mc.returnValue(null);
+                    }).done();
+                }
+            }).shouldFail().build();
+
+    @Test
+    public void testSyntheticObserver() {
+        Throwable error = container.getFailure();
+        assertNotNull(error);
+        assertTrue(error instanceof IllegalStateException);
+    }
+
+}