From 61101e02289a6d1706373e0a0d571aeb4ecb6061 Mon Sep 17 00:00:00 2001 From: Matej Novotny Date: Tue, 20 Jul 2021 15:25:05 +0200 Subject: [PATCH] Arc - Add qualifier checks on non-binding and repeating qualifiers. Make sure events cannot be selected with type variables. Added tests are mostly copies of existing TCK tests. --- .../java/io/quarkus/arc/impl/EventImpl.java | 7 ++ .../java/io/quarkus/arc/impl/Qualifiers.java | 19 ++++ .../arc/test/event/select/BreakInEvent.java | 4 + .../test/event/select/EventSelectTest.java | 87 +++++++++++++++++++ .../test/event/select/NotABindingType.java | 13 +++ .../arc/test/event/select/SecurityEvent.java | 5 ++ .../event/select/SecurityEvent_Illegal.java | 4 + .../arc/test/event/select/SecuritySensor.java | 14 +++ .../arc/test/event/select/SystemTest.java | 30 +++++++ .../duplicate/bindings/BindingTypeA.java | 31 +++++++ .../DuplicateBindingsResolutionTest.java | 45 ++++++++++ 11 files changed, 259 insertions(+) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/BreakInEvent.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/EventSelectTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/NotABindingType.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent_Illegal.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecuritySensor.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SystemTest.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/BindingTypeA.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/DuplicateBindingsResolutionTest.java diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/EventImpl.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/EventImpl.java index 3e39d397e9e28d..f1b5672e3220bd 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/EventImpl.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/EventImpl.java @@ -115,6 +115,7 @@ private Notifier getNotifier(Class runtimeType) { @Override public Event select(Annotation... qualifiers) { + Qualifiers.verify(qualifiers); Set mergedQualifiers = new HashSet<>(this.qualifiers); Collections.addAll(mergedQualifiers, qualifiers); return new EventImpl(eventType, mergedQualifiers); @@ -122,6 +123,7 @@ public Event select(Annotation... qualifiers) { @Override public Event select(Class subtype, Annotation... qualifiers) { + Qualifiers.verify(qualifiers); Set mergerdQualifiers = new HashSet<>(this.qualifiers); Collections.addAll(mergerdQualifiers, qualifiers); return new EventImpl(subtype, mergerdQualifiers); @@ -129,6 +131,11 @@ public Event select(Class subtype, Annotation... qualifiers) @Override public Event select(TypeLiteral 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 mergerdQualifiers = new HashSet<>(this.qualifiers); Collections.addAll(mergerdQualifiers, qualifiers); return new EventImpl(subtype.getType(), mergerdQualifiers); diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Qualifiers.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Qualifiers.java index c3e799d8c1d3b8..496e96ebd614bc 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/Qualifiers.java +++ b/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,15 +25,32 @@ private Qualifiers() { } static void verify(Iterable qualifiers) { + Map, 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, 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, Integer> timesQualifierSeen) { + timesQualifierSeen.forEach((aClass, timesSeen) -> { + // if the qualifier was declared more than once and wasn't repeatable + if (timesSeen > 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"); + } + }); } static boolean hasQualifiers(Set beanQualifiers, Map> qualifierNonbindingMembers, diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/BreakInEvent.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/BreakInEvent.java new file mode 100644 index 00000000000000..2d88e75bea3c64 --- /dev/null +++ b/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 { +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/EventSelectTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/EventSelectTest.java new file mode 100644 index 00000000000000..c78c78e6bd0039 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/EventSelectTest.java @@ -0,0 +1,87 @@ +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 void testEventSelectThrowsExceptionIfEventTypeHasTypeVariable() { + Assertions.assertThrows(IllegalArgumentException.class, + this::dotestEventSelectThrowsExceptionIfEventTypeHasTypeVariable, + "Event#select should throw IllegalArgumentException if the event uses type variable"); + } + + private void dotestEventSelectThrowsExceptionIfEventTypeHasTypeVariable() { + SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get(); + sensor.securityEvent.select(new TypeLiteral>() { + }); + } + + @Test + public void testEventSelectThrowsExceptionForDuplicateBindingType() { + Assertions.assertThrows(IllegalArgumentException.class, + this::dotestEventSelectThrowsExceptionForDuplicateBindingType, + "Event#select should throw IllegalArgumentException when there are duplicate bindings specified."); + } + + private void dotestEventSelectThrowsExceptionForDuplicateBindingType() { + SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get(); + sensor.securityEvent.select(new SystemTest.SystemTestLiteral("a") { + }, new SystemTest.SystemTestLiteral("b") { + }); + } + + @Test + public void testEventSelectWithSubtypeThrowsExceptionForDuplicateBindingType() { + Assertions.assertThrows(IllegalArgumentException.class, + this::dotestEventSelectWithSubtypeThrowsExceptionForDuplicateBindingType, + "Event#select should throw IllegalArgumentException when selecting a subtype with duplicate bindings."); + } + + private void dotestEventSelectWithSubtypeThrowsExceptionForDuplicateBindingType() { + SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get(); + sensor.securityEvent.select(BreakInEvent.class, new SystemTest.SystemTestLiteral("a") { + }, new SystemTest.SystemTestLiteral("b") { + }); + } + + @Test + public void testEventSelectThrowsExceptionIfAnnotationIsNotBindingType() { + Assertions.assertThrows(IllegalArgumentException.class, + this::dotestEventSelectThrowsExceptionIfAnnotationIsNotBindingType, + "Event#select should throw IllegalArgumentException if the annotation is not a binding type."); + } + + private void dotestEventSelectThrowsExceptionIfAnnotationIsNotBindingType() { + SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get(); + sensor.securityEvent.select(new AnnotationLiteral() { + }); + } + + @Test + public void testEventSelectWithSubtypeThrowsExceptionIfAnnotationIsNotBindingType() { + Assertions.assertThrows(IllegalArgumentException.class, + this::dotestEventSelectWithSubtypeThrowsExceptionIfAnnotationIsNotBindingType, + "Event#select should throw IllegalArgumentException when selecting a subtype and using annotation that is not a binding type."); + } + + private void dotestEventSelectWithSubtypeThrowsExceptionIfAnnotationIsNotBindingType() { + SecuritySensor sensor = Arc.container().select(SecuritySensor.class).get(); + sensor.securityEvent.select(BreakInEvent.class, new AnnotationLiteral() { + }); + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/NotABindingType.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/NotABindingType.java new file mode 100644 index 00000000000000..0129e574c256c8 --- /dev/null +++ b/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 { +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent.java new file mode 100644 index 00000000000000..2c82900fc21f1a --- /dev/null +++ b/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 { +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent_Illegal.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecurityEvent_Illegal.java new file mode 100644 index 00000000000000..1ea370c2778e9f --- /dev/null +++ b/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 extends SecurityEvent { +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecuritySensor.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SecuritySensor.java new file mode 100644 index 00000000000000..91584749e7798a --- /dev/null +++ b/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; +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SystemTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/event/select/SystemTest.java new file mode 100644 index 00000000000000..07a57bee535058 --- /dev/null +++ b/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 implements SystemTest { + + private final String value; + + public SystemTestLiteral(String value) { + this.value = value; + } + + public String value() { + return value; + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/BindingTypeA.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/BindingTypeA.java new file mode 100644 index 00000000000000..7c0fe87504208b --- /dev/null +++ b/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 implements BindingTypeA { + + private final String value; + + public BindingTypeABinding(String value) { + this.value = value; + } + + public String value() { + return value; + } + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/DuplicateBindingsResolutionTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/DuplicateBindingsResolutionTest.java new file mode 100644 index 00000000000000..776fa8dcea24ae --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/observers/duplicate/bindings/DuplicateBindingsResolutionTest.java @@ -0,0 +1,45 @@ +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() { + Assertions.assertThrows(IllegalArgumentException.class, this::tryToResolveObserverMethods, + "Did not see IllegalArgumentException"); + } + + private void tryToResolveObserverMethods() { + CDI.current().getBeanManager().resolveObserverMethods(new AnEventType(), + new BindingTypeA.BindingTypeABinding("a1"), new BindingTypeA.BindingTypeABinding("a2")); + } + + public static class AnEventType { + } + + @Dependent + public static class AnObserver { + public boolean wasNotified = false; + + public void observer(@Observes AnEventType event) { + wasNotified = true; + } + } +}