Arc - Add qualifier checks on non-binding and repeating qualifiers, make sure events cannot be selected with type variables

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/EventImpl.java

@@ -115,20 +115,27 @@ private Notifier<? super T> getNotifier(Class<?> runtimeType) {
 
     @Override
     public Event<T> select(Annotation... qualifiers) {
+        Qualifiers.verify(qualifiers);
         Set<Annotation> mergedQualifiers = new HashSet<>(this.qualifiers);
         Collections.addAll(mergedQualifiers, qualifiers);
         return new EventImpl<T>(eventType, mergedQualifiers);
     }
 
     @Override
     public <U extends T> Event<U> select(Class<U> subtype, Annotation... qualifiers) {
+        Qualifiers.verify(qualifiers);
         Set<Annotation> mergerdQualifiers = new HashSet<>(this.qualifiers);
         Collections.addAll(mergerdQualifiers, qualifiers);
         return new EventImpl<U>(subtype, mergerdQualifiers);
     }
 
     @Override
     public <U extends T> Event<U> select(TypeLiteral<U> subtype, Annotation... qualifiers) {
+        Qualifiers.verify(qualifiers);
+        if (Types.containsTypeVariable(subtype.getType())) {
+            throw new IllegalArgumentException(
+                    "Event#select(TypeLiteral, Annotation...) cannot be used with type variable parameter");
+        }
         Set<Annotation> mergerdQualifiers = new HashSet<>(this.qualifiers);
         Collections.addAll(mergerdQualifiers, qualifiers);
         return new EventImpl<U>(subtype.getType(), mergerdQualifiers);

independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Qualifiers.java

@@ -1,10 +1,12 @@
 package io.quarkus.arc.impl;
 
 import java.lang.annotation.Annotation;
+import java.lang.annotation.Repeatable;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
@@ -23,14 +25,32 @@ private Qualifiers() {
     }
 
     static void verify(Iterable<Annotation> qualifiers) {
+        Map<Class<? extends Annotation>, Integer> timesQualifierWasSeen = new HashMap<>();
         for (Annotation qualifier : qualifiers) {
             verifyQualifier(qualifier.annotationType());
+            timesQualifierWasSeen.compute(qualifier.annotationType(), (k, v) -> (v == null) ? 1 : (v + 1));
         }
+        checkQualifiersForDuplicates(timesQualifierWasSeen);
     }
 
     static void verify(Annotation... qualifiers) {
+        Map<Class<? extends Annotation>, Integer> timesQualifierWasSeen = new HashMap<>();
         for (Annotation qualifier : qualifiers) {
             verifyQualifier(qualifier.annotationType());
+            timesQualifierWasSeen.compute(qualifier.annotationType(), (k, v) -> (v == null) ? 1 : (v + 1));
+        }
+        checkQualifiersForDuplicates(timesQualifierWasSeen);
+    }
+
+    // in various cases, specification requires to check qualifiers for duplicates and throw IAE
+    private static void checkQualifiersForDuplicates(Map<Class<? extends Annotation>, Integer> timesQualifierSeen) {
+        for (Map.Entry<Class<? extends Annotation>, Integer> entry : timesQualifierSeen.entrySet()) {
+            Class<? extends Annotation> aClass = entry.getKey();
+            // if the qualifier was declared more than once and wasn't repeatable
+            if (entry.getValue() > 1 && aClass.getAnnotation(Repeatable.class) == null) {
+                throw new IllegalArgumentException("The qualifier " + aClass + " was used repeatedly " +
+                        "but it is not annotated with @java.lang.annotation.Repeatable");
+            }
         }
     }
 

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/BreakInEvent.java

@@ -0,0 +1,4 @@
+package io.quarkus.arc.test.event.select;
+
+public class BreakInEvent extends SecurityEvent {
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/EventSelectTest.java

@@ -0,0 +1,86 @@
+package io.quarkus.arc.test.event.select;
+
+import io.quarkus.arc.Arc;
+import io.quarkus.arc.test.ArcTestContainer;
+import javax.enterprise.util.AnnotationLiteral;
+import javax.enterprise.util.TypeLiteral;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+/**
+ * Tests that event selection throws exceptions under certain circumstances.
+ */
+public class EventSelectTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = new ArcTestContainer(BreakInEvent.class, NotABindingType.class,
+            SecurityEvent.class, SecurityEvent_Illegal.class,
+            SecuritySensor.class, SystemTest.class);
+
+    @Test
+    public <T> void testEventSelectThrowsExceptionIfEventTypeHasTypeVariable() {
+        try {
+            SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get();
+            sensor.securityEvent.select(new TypeLiteral<SecurityEvent_Illegal<T>>() {
+            });
+            Assertions.fail("Event#select should throw IllegalArgumentException if the event uses type variable");
+        } catch (IllegalArgumentException iae) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testEventSelectThrowsExceptionForDuplicateBindingType() {
+        try {
+            SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get();
+            sensor.securityEvent.select(new SystemTest.SystemTestLiteral("a") {
+            },
+                    new SystemTest.SystemTestLiteral("b") {
+                    });
+            Assertions.fail("Event#select should throw IllegalArgumentException when there are duplicate bindings specified.");
+        } catch (IllegalArgumentException iae) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testEventSelectWithSubtypeThrowsExceptionForDuplicateBindingType() {
+        try {
+            SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get();
+            sensor.securityEvent.select(BreakInEvent.class, new SystemTest.SystemTestLiteral("a") {
+            },
+                    new SystemTest.SystemTestLiteral("b") {
+                    });
+            Assertions.fail(
+                    "Event#select should throw IllegalArgumentException when selecting a subtype with duplicate bindings.");
+        } catch (IllegalArgumentException iae) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testEventSelectThrowsExceptionIfAnnotationIsNotBindingType() {
+        try {
+            SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get();
+            sensor.securityEvent.select(new AnnotationLiteral<NotABindingType>() {
+            });
+            Assertions.fail("Event#select should throw IllegalArgumentException if the annotation is not a binding type.");
+        } catch (IllegalArgumentException iae) {
+            // expected
+        }
+    }
+
+    @Test
+    public void testEventSelectWithSubtypeThrowsExceptionIfAnnotationIsNotBindingType() {
+        try {
+            SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get();
+            sensor.securityEvent.select(BreakInEvent.class, new AnnotationLiteral<NotABindingType>() {
+            });
+            Assertions.fail(
+                    "Event#select should throw IllegalArgumentException when selecting a subtype and using annotation that is not a binding type.");
+        } catch (IllegalArgumentException iae) {
+            // expected
+        }
+    }
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/NotABindingType.java

@@ -0,0 +1,13 @@
+package io.quarkus.arc.test.event.select;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+@Target({ ElementType.TYPE, ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD })
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+public @interface NotABindingType {
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent.java

@@ -0,0 +1,5 @@
+package io.quarkus.arc.test.event.select;
+
+// dummy payload
+public class SecurityEvent {
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent_Illegal.java

@@ -0,0 +1,4 @@
+package io.quarkus.arc.test.event.select;
+
+public class SecurityEvent_Illegal<T> extends SecurityEvent {
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecuritySensor.java

@@ -0,0 +1,14 @@
+package io.quarkus.arc.test.event.select;
+
+import javax.enterprise.context.Dependent;
+import javax.enterprise.event.Event;
+import javax.enterprise.inject.Any;
+import javax.inject.Inject;
+
+@Dependent
+public class SecuritySensor {
+
+    @Inject
+    @Any
+    Event<SecurityEvent> securityEvent;
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SystemTest.java

@@ -0,0 +1,30 @@
+package io.quarkus.arc.test.event.select;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import javax.enterprise.util.AnnotationLiteral;
+import javax.inject.Qualifier;
+
+@Target({ ElementType.TYPE, ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD })
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+@Qualifier
+public @interface SystemTest {
+    String value() default "";
+
+    class SystemTestLiteral extends AnnotationLiteral<SystemTest> implements SystemTest {
+
+        private final String value;
+
+        public SystemTestLiteral(String value) {
+            this.value = value;
+        }
+
+        public String value() {
+            return value;
+        }
+    }
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/BindingTypeA.java

@@ -0,0 +1,31 @@
+package io.quarkus.arc.test.observers.duplicate.bindings;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import javax.enterprise.util.AnnotationLiteral;
+import javax.inject.Qualifier;
+
+@Target({ ElementType.TYPE, ElementType.METHOD, ElementType.PARAMETER, ElementType.FIELD })
+@Retention(RetentionPolicy.RUNTIME)
+@Documented
+@Qualifier
+public @interface BindingTypeA {
+
+    String value() default "";
+
+    class BindingTypeABinding extends AnnotationLiteral<BindingTypeA> implements BindingTypeA {
+
+        private final String value;
+
+        public BindingTypeABinding(String value) {
+            this.value = value;
+        }
+
+        public String value() {
+            return value;
+        }
+    }
+}

independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/DuplicateBindingsResolutionTest.java

@@ -0,0 +1,46 @@
+package io.quarkus.arc.test.observers.duplicate.bindings;
+
+import io.quarkus.arc.test.ArcTestContainer;
+import java.lang.annotation.Annotation;
+import javax.enterprise.context.Dependent;
+import javax.enterprise.event.Observes;
+import javax.enterprise.inject.spi.CDI;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+/**
+ * Tests that when you try to resolve observer methods via
+ * {@link javax.enterprise.inject.spi.BeanManager#resolveObserverMethods(Object, Annotation...)},
+ * you will get an exception if you pass in twice the same annotation that is not repeatable.
+ */
+public class DuplicateBindingsResolutionTest {
+
+    @RegisterExtension
+    public ArcTestContainer container = new ArcTestContainer(BindingTypeA.class, BindingTypeA.BindingTypeABinding.class,
+            AnEventType.class, AnObserver.class);
+
+    @Test
+    public void testDuplicateBindingTypesWhenResolvingFails() {
+        try {
+            CDI.current().getBeanManager().resolveObserverMethods(new AnEventType(),
+                    new BindingTypeA.BindingTypeABinding("a1"), new BindingTypeA.BindingTypeABinding("a2"));
+            Assertions.fail(
+                    "BM#resolveObserverMethods should throw IllegalArgumentException if supplied with duplicate bindings");
+        } catch (IllegalArgumentException iae) {
+            // expected
+        }
+    }
+
+    public static class AnEventType {
+    }
+
+    @Dependent
+    public static class AnObserver {
+        public boolean wasNotified = false;
+
+        public void observer(@Observes AnEventType event) {
+            wasNotified = true;
+        }
+    }
+}