From f884954c560ccb2b46cbfbd6a7a401f1e22c373b Mon Sep 17 00:00:00 2001 From: Gwenneg Lepage Date: Tue, 5 Mar 2024 00:55:22 +0100 Subject: [PATCH] Detect empty annotation name at build time --- .../unleash/EmptyAnnotationNameException.java | 20 +++++ .../quarkiverse/unleash/UnleashProcessor.java | 54 +++++++++++-- .../EmptyAnnotationNameExceptionTest.java | 77 +++++++++++++++++++ .../runtime/AbstractVariantProducer.java | 3 - .../runtime/FeatureToggleProducer.java | 3 - 5 files changed, 146 insertions(+), 11 deletions(-) create mode 100644 deployment/src/main/java/io/quarkiverse/unleash/EmptyAnnotationNameException.java create mode 100644 deployment/src/test/java/io/quarkiverse/unleash/EmptyAnnotationNameExceptionTest.java diff --git a/deployment/src/main/java/io/quarkiverse/unleash/EmptyAnnotationNameException.java b/deployment/src/main/java/io/quarkiverse/unleash/EmptyAnnotationNameException.java new file mode 100644 index 0000000..d42adae --- /dev/null +++ b/deployment/src/main/java/io/quarkiverse/unleash/EmptyAnnotationNameException.java @@ -0,0 +1,20 @@ +package io.quarkiverse.unleash; + +import org.jboss.jandex.ClassInfo; + +public class EmptyAnnotationNameException extends RuntimeException { + + private static final String MESSAGE = "@" + FeatureToggle.class.getName() + " and @" + FeatureVariant.class.getName() + + " annotations must have a non empty name attribute [class=%s]"; + + private final ClassInfo classInfo; + + public EmptyAnnotationNameException(ClassInfo classInfo) { + super(String.format(MESSAGE, classInfo.name())); + this.classInfo = classInfo; + } + + public ClassInfo getClassInfo() { + return classInfo; + } +} diff --git a/deployment/src/main/java/io/quarkiverse/unleash/UnleashProcessor.java b/deployment/src/main/java/io/quarkiverse/unleash/UnleashProcessor.java index f35dd62..359cc25 100644 --- a/deployment/src/main/java/io/quarkiverse/unleash/UnleashProcessor.java +++ b/deployment/src/main/java/io/quarkiverse/unleash/UnleashProcessor.java @@ -15,12 +15,25 @@ import jakarta.enterprise.inject.spi.InjectionPoint; import jakarta.inject.Inject; -import org.jboss.jandex.*; +import org.jboss.jandex.AnnotationInstance; +import org.jboss.jandex.AnnotationTarget; +import org.jboss.jandex.ClassInfo; +import org.jboss.jandex.DotName; +import org.jboss.jandex.FieldInfo; +import org.jboss.jandex.IndexView; +import org.jboss.jandex.MethodParameterInfo; +import org.jboss.jandex.ParameterizedType; import org.jboss.jandex.Type; import io.getunleash.Unleash; import io.getunleash.Variant; -import io.quarkiverse.unleash.runtime.*; +import io.quarkiverse.unleash.runtime.AbstractVariantProducer; +import io.quarkiverse.unleash.runtime.FeatureToggleProducer; +import io.quarkiverse.unleash.runtime.ToggleVariantProducer; +import io.quarkiverse.unleash.runtime.ToggleVariantStringProducer; +import io.quarkiverse.unleash.runtime.UnleashLifecycleManager; +import io.quarkiverse.unleash.runtime.UnleashRecorder; +import io.quarkiverse.unleash.runtime.UnleashResourceProducer; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; import io.quarkus.arc.deployment.GeneratedBeanBuildItem; import io.quarkus.arc.deployment.GeneratedBeanGizmoAdaptor; @@ -115,29 +128,60 @@ AdditionalBeanBuildItem additionalBeans() { } @BuildStep - void validateFeatureToggleAnnotations(CombinedIndexBuildItem combinedIndex, + void validateAnnotations(CombinedIndexBuildItem combinedIndex, BuildProducer validationErrors) { List throwables = new ArrayList<>(); + for (AnnotationInstance annotation : combinedIndex.getIndex().getAnnotations(DN_FEATURE_TOGGLE)) { AnnotationTarget target = annotation.target(); + String name = annotation.value("name").asString(); switch (target.kind()) { case FIELD -> { + ClassInfo declaringClass = target.asField().declaringClass(); if (DN_PRIMITIVE_BOOLEAN.equals(target.asField().type().name())) { - ClassInfo declaringClass = target.asField().declaringClass(); throwables.add(new BooleanFeatureToggleException(declaringClass)); } + if (name == null || name.isEmpty()) { + throwables.add(new EmptyAnnotationNameException(declaringClass)); + } } case METHOD_PARAMETER -> { + ClassInfo declaringClass = target.asMethodParameter().method().declaringClass(); if (DN_PRIMITIVE_BOOLEAN.equals(target.asMethodParameter().type().name())) { - ClassInfo declaringClass = target.asMethodParameter().method().declaringClass(); throwables.add(new BooleanFeatureToggleException(declaringClass)); } + if (name == null || name.isEmpty()) { + throwables.add(new EmptyAnnotationNameException(declaringClass)); + } + } + default -> { + // No validation required for all other target kinds. + } + } + } + + for (AnnotationInstance annotation : combinedIndex.getIndex().getAnnotations(DN_FEATURE_VARIANT)) { + AnnotationTarget target = annotation.target(); + String name = annotation.value("name").asString(); + switch (target.kind()) { + case FIELD -> { + if (name == null || name.isEmpty()) { + ClassInfo declaringClass = target.asField().declaringClass(); + throwables.add(new EmptyAnnotationNameException(declaringClass)); + } + } + case METHOD_PARAMETER -> { + if (name == null || name.isEmpty()) { + ClassInfo declaringClass = target.asMethodParameter().method().declaringClass(); + throwables.add(new EmptyAnnotationNameException(declaringClass)); + } } default -> { // No validation required for all other target kinds. } } } + validationErrors.produce(new ValidationErrorBuildItem(throwables.toArray(new Throwable[0]))); } diff --git a/deployment/src/test/java/io/quarkiverse/unleash/EmptyAnnotationNameExceptionTest.java b/deployment/src/test/java/io/quarkiverse/unleash/EmptyAnnotationNameExceptionTest.java new file mode 100644 index 0000000..ff4709c --- /dev/null +++ b/deployment/src/test/java/io/quarkiverse/unleash/EmptyAnnotationNameExceptionTest.java @@ -0,0 +1,77 @@ +package io.quarkiverse.unleash; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +import java.util.Arrays; + +import jakarta.enterprise.context.ApplicationScoped; +import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.inject.Instance; +import jakarta.enterprise.inject.spi.DeploymentException; +import jakarta.inject.Singleton; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.getunleash.Variant; +import io.quarkus.test.QuarkusUnitTest; + +public class EmptyAnnotationNameExceptionTest { + + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest() + .withApplicationRoot( + (jar) -> jar.addClasses(EmptyFeatureToggleNameField.class, EmptyFeatureToggleNameConstructorArgument.class, + EmptyFeatureVariantNameField.class, EmptyFeatureVariantNameConstructorArgument.class)) + .assertException(t -> { + assertEquals(DeploymentException.class, t.getClass()); + assertEquals(4, t.getSuppressed().length); + assertEmptyAnnotationNameException(t, EmptyFeatureToggleNameField.class); + assertEmptyAnnotationNameException(t, EmptyFeatureToggleNameConstructorArgument.class); + assertEmptyAnnotationNameException(t, EmptyFeatureVariantNameField.class); + assertEmptyAnnotationNameException(t, EmptyFeatureVariantNameConstructorArgument.class); + }); + + private static void assertEmptyAnnotationNameException(Throwable t, Class expectedClassName) { + assertEquals(1, Arrays.stream(t.getSuppressed()).filter(throwable -> { + if (throwable instanceof EmptyAnnotationNameException e) { + return expectedClassName.getName().equals(e.getClassInfo().name().toString()); + } + return false; + }).count()); + } + + @Test + void shouldNotBeInvoked() { + fail("A deployment exception should be thrown before this method is ever invoked"); + } + + @ApplicationScoped + static class EmptyFeatureToggleNameField { + + @FeatureToggle(name = "") + Instance toggle; + } + + @Singleton + static class EmptyFeatureToggleNameConstructorArgument { + + public EmptyFeatureToggleNameConstructorArgument(@FeatureToggle(name = "") Instance toggle) { + } + } + + @RequestScoped + static class EmptyFeatureVariantNameField { + + @FeatureVariant(name = "") + Instance variant; + } + + @Singleton + static class EmptyFeatureVariantNameConstructorArgument { + + public EmptyFeatureVariantNameConstructorArgument(@FeatureVariant(name = "") Instance variant) { + } + } +} diff --git a/runtime/src/main/java/io/quarkiverse/unleash/runtime/AbstractVariantProducer.java b/runtime/src/main/java/io/quarkiverse/unleash/runtime/AbstractVariantProducer.java index ef4896c..cbd63e4 100644 --- a/runtime/src/main/java/io/quarkiverse/unleash/runtime/AbstractVariantProducer.java +++ b/runtime/src/main/java/io/quarkiverse/unleash/runtime/AbstractVariantProducer.java @@ -22,9 +22,6 @@ protected FeatureVariant getFeatureVariant(InjectionPoint injectionPoint) { break; } } - if (ft == null || ft.name().isEmpty()) { - throw new IllegalStateException("No feature toggle name of the variant specified"); - } return ft; } diff --git a/runtime/src/main/java/io/quarkiverse/unleash/runtime/FeatureToggleProducer.java b/runtime/src/main/java/io/quarkiverse/unleash/runtime/FeatureToggleProducer.java index aa77d6a..bb04a1a 100644 --- a/runtime/src/main/java/io/quarkiverse/unleash/runtime/FeatureToggleProducer.java +++ b/runtime/src/main/java/io/quarkiverse/unleash/runtime/FeatureToggleProducer.java @@ -24,9 +24,6 @@ boolean getFeatureToggle(InjectionPoint injectionPoint) { break; } } - if (ft == null || ft.name().isEmpty()) { - throw new IllegalStateException("No feature toggle name specified"); - } return unleash.isEnabled(ft.name(), ft.defaultValue()); }