Skip to content

Commit

Permalink
ArC: filter out class-retained or missing annotations
Browse files Browse the repository at this point in the history
Sometimes, ArC used to call the `AnnotationLiteralProcessor` with
annotations that are either class-retained, or their classes are missing
(which is perfectly legal). It is a minority of callers though -- most
of them only call `AnnotationLiteralProcessor` with qualifiers or
interceptor bindings. Those must always be runtime-retained and their
classes must be present, and it is an error if they are not.

At the same time, we don't want to generate annotation literals for
class-retained annotations or for annotations whose classes are missing.

The `AnnotationLiteralProcessor` already checks that the annotation class
exists, so with this commit, it also checks that the annotation for which
the literal is generated is runtime-retained. If not, an exception is
thrown. This lets us know that there's a problem somewhere.

There are 2 places where ArC deals with arbitrary annotations and may need
to generate annotation literals for them; in both cases, the code has to
process all annotations present on an injection point. In these cases, it
is straightforward to filter out class-retained annotations and missing
annotation classes.
  • Loading branch information
Ladicek committed Nov 23, 2022
1 parent de814ab commit 46268e3
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ public ResultHandle process(BytecodeCreator bytecode, ClassOutput classOutput, C
* the annotation members have the same values as the given annotation instance.
* An implementation of the annotation type will be generated automatically.
* <p>
* It is expected that given annotation instance is runtime-retained; an exception is thrown
* if not. Further, it is expected that the annotation type is available (that is,
* {@code annotationClass != null}); an exception is thrown if not. Callers that expect
* they always deal with runtime-retained annotations whose classes are available do not
* have to check (and will get decent errors for free), but callers that can possibly deal
* with class-retained annotations or missing annotation classes must check explicitly.
* <p>
* We call the generated implementation of the annotation type an <em>annotation literal class</em>
* and the instance produced by the generated bytecode an <em>annotation literal instance</em>,
* even though the generated code doesn't use CDI's {@code AnnotationLiteral} anymore.
Expand All @@ -84,6 +91,10 @@ public ResultHandle process(BytecodeCreator bytecode, ClassOutput classOutput, C
* @return an annotation literal instance result handle
*/
public ResultHandle create(BytecodeCreator bytecode, ClassInfo annotationClass, AnnotationInstance annotationInstance) {
if (!annotationInstance.runtimeVisible()) {
throw new IllegalArgumentException("Annotation does not have @Retention(RUNTIME): " + annotationInstance);
}

Objects.requireNonNull(annotationClass, "Annotation class not available: " + annotationInstance);
AnnotationLiteralClassInfo literal = cache.getValue(new CacheKey(annotationClass));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1932,9 +1932,14 @@ public static ResultHandle collectInjectionPointAnnotations(ClassOutput classOut
annotationHandle = constructor
.readStaticField(FieldDescriptor.of(InjectLiteral.class, "INSTANCE", InjectLiteral.class));
} else {
// Create annotation literal if needed
ClassInfo literalClass = getClassByName(beanDeployment.getBeanArchiveIndex(), annotation.name());
annotationHandle = annotationLiterals.create(constructor, literalClass, annotation);
if (!annotation.runtimeVisible()) {
continue;
}
ClassInfo annotationClass = getClassByName(beanDeployment.getBeanArchiveIndex(), annotation.name());
if (annotationClass == null) {
continue;
}
annotationHandle = annotationLiterals.create(constructor, annotationClass, annotation);
}
constructor.invokeInterfaceMethod(MethodDescriptors.SET_ADD, annotationsHandle,
annotationHandle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -311,10 +311,16 @@ private static void generateBeanManagerBytecode(GeneratorContext ctx) {
private static void generateResourceBytecode(GeneratorContext ctx) {
ResultHandle annotations = ctx.constructor.newInstance(MethodDescriptor.ofConstructor(HashSet.class));
// For a resource field the required qualifiers contain all annotations declared on the field
// (hence we need to check if they are runtime-retained and their classes are available)
if (!ctx.injectionPoint.getRequiredQualifiers().isEmpty()) {
for (AnnotationInstance annotation : ctx.injectionPoint.getRequiredQualifiers()) {
// Create annotation literal first
if (!annotation.runtimeVisible()) {
continue;
}
ClassInfo annotationClass = getClassByName(ctx.beanDeployment.getBeanArchiveIndex(), annotation.name());
if (annotationClass == null) {
continue;
}
ctx.constructor.invokeInterfaceMethod(MethodDescriptors.SET_ADD, annotations,
ctx.annotationLiterals.create(ctx.constructor, annotationClass, annotation));
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.arc.processor;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.IOException;
Expand Down Expand Up @@ -31,6 +32,10 @@ public enum SimpleEnum {
BAZ,
}

@Retention(RetentionPolicy.CLASS)
public @interface ClassRetainedAnnotation {
}

@Retention(RetentionPolicy.RUNTIME)
public @interface SimpleAnnotation {
String value();
Expand Down Expand Up @@ -327,6 +332,35 @@ public void test() throws ReflectiveOperationException {
annotation.toString());
}

@Test
public void missingAnnotationClass() {
AnnotationLiteralProcessor literals = new AnnotationLiteralProcessor(index, ignored -> true);

TestClassLoader cl = new TestClassLoader(getClass().getClassLoader());
try (ClassCreator creator = ClassCreator.builder().classOutput(cl).className(generatedClass).build()) {
MethodCreator method = creator.getMethodCreator("hello", void.class);

assertThrows(NullPointerException.class, () -> {
literals.create(method, null, simpleAnnotationJandex("foobar"));
});
}
}

@Test
public void classRetainedAnnotation() {
AnnotationLiteralProcessor literals = new AnnotationLiteralProcessor(index, ignored -> true);

TestClassLoader cl = new TestClassLoader(getClass().getClassLoader());
try (ClassCreator creator = ClassCreator.builder().classOutput(cl).className(generatedClass).build()) {
MethodCreator method = creator.getMethodCreator("hello", void.class);

assertThrows(IllegalArgumentException.class, () -> {
literals.create(method, Index.singleClass(ClassRetainedAnnotation.class),
AnnotationInstance.builder(ClassRetainedAnnotation.class).build());
});
}
}

private static void verify(ComplexAnnotation ann) {
assertEquals(true, ann.bool());
assertEquals((byte) 1, ann.b());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,15 @@
import javax.persistence.criteria.CriteriaUpdate;
import javax.persistence.metamodel.Metamodel;

import org.jboss.jandex.AnnotationInstance;
import org.jboss.jandex.DotName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.arc.Arc;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.arc.ResourceReferenceProvider;
import io.quarkus.arc.processor.InjectionPointsTransformer;
import io.quarkus.arc.test.ArcTestContainer;

public class ResourceInjectionTest {
Expand All @@ -50,7 +53,26 @@ public class ResourceInjectionTest {
public ArcTestContainer container = ArcTestContainer.builder()
.beanClasses(EEResourceField.class, JpaClient.class)
.resourceReferenceProviders(EntityManagerProvider.class, DummyProvider.class)
.resourceAnnotations(PersistenceContext.class, Dummy.class).build();
.resourceAnnotations(PersistenceContext.class, Dummy.class)
.injectionPointsTransformers(new InjectionPointsTransformer() {
@Override
public boolean appliesTo(org.jboss.jandex.Type requiredType) {
return requiredType.name().toString().equals(String.class.getName());
}

@Override
public void transform(TransformationContext transformationContext) {
if (transformationContext.getAllAnnotations()
.stream()
.anyMatch(it -> it.name().toString().equals(Dummy.class.getName()))) {
// pretend that the injection point has an annotation whose class is missing
transformationContext.transform()
.add(AnnotationInstance.builder(DotName.createSimple("missing.NonNull")).build())
.done();
}
}
})
.build();

@Test
public void testInjection() {
Expand Down

0 comments on commit 46268e3

Please sign in to comment.