From 5a391239a3911d967f5953247ba62702c7db65e6 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Wed, 21 Jun 2023 10:14:11 +0200 Subject: [PATCH] Qute: `@EngineConfiguration` validation - consider superclasses - fixes #34163 --- .../qute/deployment/QuteProcessor.java | 64 +++++++------------ .../io/quarkus/qute/deployment/Types.java | 23 +++++++ .../io/quarkus/qute/deployment/TypesTest.java | 54 ++++++++++++++++ .../resolver/CustomResolversTest.java | 11 +++- 4 files changed, 108 insertions(+), 44 deletions(-) diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java index ce524f9772349..e4effe48efbc7 100644 --- a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java +++ b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java @@ -464,6 +464,7 @@ TemplatesAnalysisBuildItem analyzeTemplates(List template TemplateFilePathsBuildItem filePaths, List checkedTemplates, List messageBundleMethods, List globals, QuteConfig config, Optional engineConfigurations, + BeanArchiveIndexBuildItem beanArchiveIndex, BuildProducer checkedFragmentValidations) { long start = System.nanoTime(); @@ -490,7 +491,8 @@ TemplatesAnalysisBuildItem analyzeTemplates(List template // Register additional section factories if (engineConfigurations.isPresent()) { Collection sectionFactories = engineConfigurations.get().getConfigurations().stream() - .filter(c -> isImplementorOf(c, Names.SECTION_HELPER_FACTORY)).collect(Collectors.toList()); + .filter(c -> Types.isImplementorOf(c, Names.SECTION_HELPER_FACTORY, beanArchiveIndex.getIndex())) + .collect(Collectors.toList()); // Use the deployment class loader - it can load application classes; it's non-persistent and isolated ClassLoader tccl = Thread.currentThread().getContextClassLoader(); for (ClassInfo factoryClass : sectionFactories) { @@ -2078,11 +2080,11 @@ CustomTemplateLocatorPatternsBuildItem validateAndCollectCustomTemplateLocatorLo // Collect TemplateLocators annotated with io.quarkus.qute.Locate for (AnnotationInstance locate : beanArchiveIndex.getIndex().getAnnotations(Names.LOCATE)) { AnnotationTarget locateTarget = locate.target(); - if (isTargetClass(locateTarget)) { - if (isNotTemplateLocatorImpl(locateTarget)) { - reportFoundInvalidTarget(validationErrors, locateTarget); - } else { + if (locateTarget.kind() == Kind.CLASS) { + if (Types.isImplementorOf(locateTarget.asClass(), Names.TEMPLATE_LOCATOR, beanArchiveIndex.getIndex())) { addLocationRegExToLocators(locationPatterns, locate.value(), locateTarget, validationErrors); + } else { + reportFoundInvalidTarget(validationErrors, locateTarget); } } } @@ -2090,14 +2092,14 @@ CustomTemplateLocatorPatternsBuildItem validateAndCollectCustomTemplateLocatorLo // Collect TemplateLocators annotated with multiple 'io.quarkus.qute.Locate' for (AnnotationInstance locates : beanArchiveIndex.getIndex().getAnnotations(Names.LOCATES)) { AnnotationTarget locatesTarget = locates.target(); - if (isTargetClass(locatesTarget)) { - if (isNotTemplateLocatorImpl(locatesTarget)) { - reportFoundInvalidTarget(validationErrors, locatesTarget); - } else { + if (locatesTarget.kind() == Kind.CLASS) { + if (Types.isImplementorOf(locatesTarget.asClass(), Names.TEMPLATE_LOCATOR, beanArchiveIndex.getIndex())) { // locates.value() is array of 'io.quarkus.qute.Locate' for (AnnotationInstance locate : locates.value().asNestedArray()) { addLocationRegExToLocators(locationPatterns, locate.value(), locatesTarget, validationErrors); } + } else { + reportFoundInvalidTarget(validationErrors, locatesTarget); } } } @@ -2111,9 +2113,16 @@ void collectEngineConfigurations( BuildProducer engineConfig, BuildProducer validationErrors) { + Collection engineConfigAnnotations = beanArchiveIndex.getIndex() + .getAnnotations(Names.ENGINE_CONFIGURATION); + if (engineConfigAnnotations.isEmpty()) { + return; + } + List engineConfigClasses = new ArrayList<>(); + IndexView index = beanArchiveIndex.getIndex(); - for (AnnotationInstance annotation : beanArchiveIndex.getIndex().getAnnotations(Names.ENGINE_CONFIGURATION)) { + for (AnnotationInstance annotation : engineConfigAnnotations) { AnnotationTarget target = annotation.target(); if (target.kind() == Kind.CLASS) { ClassInfo targetClass = target.asClass(); @@ -2125,7 +2134,7 @@ void collectEngineConfigurations( new TemplateException(String.format( "Only top-level and static nested classes may be annotated with @%s: %s", EngineConfiguration.class.getSimpleName(), targetClass.name())))); - } else if (isImplementorOf(targetClass, Names.SECTION_HELPER_FACTORY)) { + } else if (Types.isImplementorOf(targetClass, Names.SECTION_HELPER_FACTORY, index)) { if (targetClass.hasNoArgsConstructor()) { engineConfigClasses.add(targetClass); } else { @@ -2135,8 +2144,8 @@ void collectEngineConfigurations( "A class annotated with @%s that also implements io.quarkus.qute.SectionHelperFactory must declare a no-args constructor: %s", EngineConfiguration.class.getSimpleName(), targetClass.name())))); } - } else if (isImplementorOf(targetClass, Names.VALUE_RESOLVER) - || isImplementorOf(targetClass, Names.NAMESPACE_RESOLVER)) { + } else if (Types.isImplementorOf(targetClass, Names.VALUE_RESOLVER, index) + || Types.isImplementorOf(targetClass, Names.NAMESPACE_RESOLVER, index)) { engineConfigClasses.add(targetClass); } else { validationErrors.produce( @@ -2151,10 +2160,7 @@ void collectEngineConfigurations( } } } - - if (!engineConfigClasses.isEmpty()) { - engineConfig.produce(new EngineConfigurationsBuildItem(engineConfigClasses)); - } + engineConfig.produce(new EngineConfigurationsBuildItem(engineConfigClasses)); } private void addLocationRegExToLocators(Collection locationToLocators, @@ -2181,30 +2187,6 @@ private void reportFoundInvalidTarget(BuildProducer va locateTarget.asClass().name().toString())))); } - private boolean isTargetClass(AnnotationTarget locateTarget) { - return locateTarget.kind() == Kind.CLASS; - } - - private boolean isNotTemplateLocatorImpl(AnnotationTarget locateTarget) { - boolean targetImplTemplateLocator = false; - for (DotName interfaceName : locateTarget.asClass().interfaceNames()) { - if (interfaceName.equals(Names.TEMPLATE_LOCATOR)) { - targetImplTemplateLocator = true; - break; - } - } - return !targetImplTemplateLocator; - } - - private static boolean isImplementorOf(ClassInfo target, DotName interfaceName) { - for (DotName name : target.asClass().interfaceNames()) { - if (name.equals(interfaceName)) { - return true; - } - } - return false; - } - @BuildStep TemplateVariantsBuildItem collectTemplateVariants(List templatePaths) throws IOException { Set allPaths = templatePaths.stream().map(TemplatePathBuildItem::getPath).collect(Collectors.toSet()); diff --git a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/Types.java b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/Types.java index 4cb99a9c95e8b..362d2aed769d1 100644 --- a/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/Types.java +++ b/extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/Types.java @@ -215,4 +215,27 @@ static Type box(Primitive primitive) { } } + static boolean isImplementorOf(ClassInfo target, DotName interfaceName, IndexView index) { + if (target.interfaceNames().contains(interfaceName)) { + // Direct implementor + return true; + } + DotName superName = target.superName(); + if (superName != null && !superName.equals(DotName.OBJECT_NAME)) { + ClassInfo superClass = index.getClassByName(superName); + if (superClass != null && isImplementorOf(superClass, interfaceName, index)) { + // Superclass is implementor + return true; + } + } + for (DotName name : target.interfaceNames()) { + ClassInfo interfaceClass = index.getClassByName(name); + if (interfaceClass != null && isImplementorOf(interfaceClass, interfaceName, index)) { + // Superinterface is implementor + return true; + } + } + return false; + } + } diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/TypesTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/TypesTest.java index 764a7635b1b06..1be95694e93f2 100644 --- a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/TypesTest.java +++ b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/TypesTest.java @@ -7,6 +7,7 @@ import java.io.InputStream; import java.io.Serializable; import java.math.BigDecimal; +import java.util.concurrent.CompletionStage; import org.jboss.jandex.ArrayType; import org.jboss.jandex.ClassType; @@ -18,6 +19,8 @@ import org.jboss.jandex.Type; import org.junit.jupiter.api.Test; +import io.quarkus.qute.EvalContext; +import io.quarkus.qute.ValueResolver; import io.quarkus.qute.deployment.Types.AssignabilityCheck; public class TypesTest { @@ -59,6 +62,19 @@ public void testIsAssignableFrom() throws IOException { assertFalse(assignabilityCheck.isAssignableFrom(byteArrayType, intArrayType)); } + @Test + public void testIsImplementorOf() throws IOException { + Index index = index(MyResolver1.class, MyResolver2.class, MyResolver3.class, MyResolver4.class, CoolResolver.class, + BaseResolver.class, String.class); + + assertTrue(Types.isImplementorOf(index.getClassByName(MyResolver1.class), Names.VALUE_RESOLVER, index)); + assertTrue(Types.isImplementorOf(index.getClassByName(MyResolver2.class), Names.VALUE_RESOLVER, index)); + assertTrue(Types.isImplementorOf(index.getClassByName(MyResolver3.class), Names.VALUE_RESOLVER, index)); + assertTrue(Types.isImplementorOf(index.getClassByName(MyResolver4.class), Names.VALUE_RESOLVER, index)); + assertTrue(Types.isImplementorOf(index.getClassByName(CoolResolver.class), Names.VALUE_RESOLVER, index)); + assertFalse(Types.isImplementorOf(index.getClassByName(String.class), Names.VALUE_RESOLVER, index)); + } + private static Index index(Class... classes) throws IOException { Indexer indexer = new Indexer(); for (Class clazz : classes) { @@ -70,4 +86,42 @@ private static Index index(Class... classes) throws IOException { return indexer.complete(); } + public static class MyResolver1 implements ValueResolver { + + @Override + public CompletionStage resolve(EvalContext context) { + return null; + } + + } + + public static class MyResolver2 extends BaseResolver { + + } + + public static class BaseResolver implements ValueResolver { + + @Override + public CompletionStage resolve(EvalContext context) { + return null; + } + } + + public interface CoolResolver extends ValueResolver { + + } + + public static class MyResolver3 implements CoolResolver { + + @Override + public CompletionStage resolve(EvalContext context) { + return null; + } + + } + + public static class MyResolver4 extends MyResolver3 { + + } + } diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/engineconfigurations/resolver/CustomResolversTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/engineconfigurations/resolver/CustomResolversTest.java index d6bf336e3d3d1..25090d0aec413 100644 --- a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/engineconfigurations/resolver/CustomResolversTest.java +++ b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/engineconfigurations/resolver/CustomResolversTest.java @@ -22,8 +22,9 @@ public class CustomResolversTest { @RegisterExtension static final QuarkusUnitTest config = new QuarkusUnitTest() - .withApplicationRoot(root -> root.addClasses(CustomValueResolver.class, CustomNamespaceResolver.class) - .addAsResource(new StringAsset("{bool.foo}::{custom:bar}"), "templates/foo.html")); + .withApplicationRoot( + root -> root.addClasses(CustomValueResolver.class, CustomNamespaceResolver.class, ResolverBase.class) + .addAsResource(new StringAsset("{bool.foo}::{custom:bar}"), "templates/foo.html")); @Inject Template foo; @@ -34,7 +35,7 @@ public void testResolvers() { } @EngineConfiguration - static class CustomValueResolver implements ValueResolver { + static class CustomValueResolver extends ResolverBase { @Override public boolean appliesTo(EvalContext context) { @@ -48,6 +49,10 @@ public CompletionStage resolve(EvalContext context) { } + static abstract class ResolverBase implements ValueResolver { + + } + @EngineConfiguration static class CustomNamespaceResolver implements NamespaceResolver {