Skip to content

Commit

Permalink
Qute type-safe validation improvements
Browse files Browse the repository at this point in the history
- validate expressions that start with a namespace coming from a TemplateData annotation
- validate nested "inject:" namespace expressions, ie. used as method
params
- resolves quarkusio#20621
- also introduce io.quarkus.qute.TemplateData.SIMPLENAME and fix
TemplateData javadoc

(cherry picked from commit 5709bd6)
  • Loading branch information
mkouba authored and aloubyansky committed Oct 18, 2021
1 parent 8d55d12 commit b05709c
Show file tree
Hide file tree
Showing 11 changed files with 513 additions and 149 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.quarkus.qute.deployment;

import static io.quarkus.deployment.annotations.ExecutionTime.STATIC_INIT;
import static java.util.stream.Collectors.toMap;

import java.io.File;
import java.io.IOException;
Expand Down Expand Up @@ -43,10 +44,12 @@

import io.quarkus.arc.deployment.AdditionalBeanBuildItem;
import io.quarkus.arc.deployment.BeanArchiveIndexBuildItem;
import io.quarkus.arc.deployment.BeanDiscoveryFinishedBuildItem;
import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem;
import io.quarkus.arc.deployment.BeanRegistrationPhaseBuildItem.BeanConfiguratorBuildItem;
import io.quarkus.arc.deployment.SyntheticBeanBuildItem;
import io.quarkus.arc.processor.Annotations;
import io.quarkus.arc.processor.BeanInfo;
import io.quarkus.arc.processor.DotNames;
import io.quarkus.deployment.ApplicationArchive;
import io.quarkus.deployment.GeneratedClassGizmoAdaptor;
Expand Down Expand Up @@ -78,6 +81,7 @@
import io.quarkus.qute.Expression.Part;
import io.quarkus.qute.Namespaces;
import io.quarkus.qute.Resolver;
import io.quarkus.qute.deployment.QuteProcessor.LookupConfig;
import io.quarkus.qute.deployment.QuteProcessor.Match;
import io.quarkus.qute.deployment.TemplatesAnalysisBuildItem.TemplateAnalysis;
import io.quarkus.qute.generator.Descriptors;
Expand Down Expand Up @@ -346,6 +350,8 @@ void validateMessageBundleMethodsInTemplates(TemplatesAnalysisBuildItem analysis
BuildProducer<IncorrectExpressionBuildItem> incorrectExpressions,
BuildProducer<ImplicitValueResolverBuildItem> implicitClasses,
List<CheckedTemplateBuildItem> checkedTemplates,
BeanDiscoveryFinishedBuildItem beanDiscovery,
List<TemplateDataBuildItem> templateData,
QuteConfig config) {

IndexView index = beanArchiveIndex.getIndex();
Expand All @@ -356,6 +362,18 @@ public String apply(String id) {
}
};

// IMPLEMENTATION NOTE:
// We do not support injection of synthetic beans with names
// Dependency on the ValidationPhaseBuildItem would result in a cycle in the build chain
Map<String, BeanInfo> namedBeans = beanDiscovery.beanStream().withName()
.collect(toMap(BeanInfo::getName, Function.identity()));

Map<String, TemplateDataBuildItem> namespaceTemplateData = templateData.stream()
.filter(TemplateDataBuildItem::hasNamespace)
.collect(Collectors.toMap(TemplateDataBuildItem::getNamespace, Function.identity()));

LookupConfig lookupConfig = new QuteProcessor.FixedLookupConfig(index, QuteProcessor.initDefaultMembersFilter(), false);

// bundle name -> (key -> method)
Map<String, Map<String, MethodInfo>> bundleMethodsMap = new HashMap<>();
for (MessageBundleMethodBuildItem messageBundleMethod : messageBundleMethods) {
Expand Down Expand Up @@ -456,9 +474,9 @@ public String apply(String id) {
if (param.hasTypeInfo()) {
Map<String, Match> results = new HashMap<>();
QuteProcessor.validateNestedExpressions(exprEntry.getKey(), defaultBundleInterface,
results, templateExtensionMethods, excludes,
incorrectExpressions, expression, index, implicitClassToMembersUsed,
templateIdToPathFun, generatedIdsToMatches, checkedTemplate);
results, templateExtensionMethods, excludes, incorrectExpressions, expression,
index, implicitClassToMembersUsed, templateIdToPathFun, generatedIdsToMatches,
checkedTemplate, lookupConfig, namedBeans, namespaceTemplateData);
Match match = results.get(param.toOriginalString());
if (match != null && !match.isEmpty() && !Types.isAssignableFrom(match.type(),
methodParams.get(idx), index)) {
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
package io.quarkus.qute.deployment;

import java.util.Arrays;
import java.util.regex.Pattern;

import org.jboss.jandex.AnnotationTarget;
import org.jboss.jandex.AnnotationTarget.Kind;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.FieldInfo;
import org.jboss.jandex.MethodInfo;

import io.quarkus.builder.item.MultiBuildItem;

final class TemplateDataBuildItem extends MultiBuildItem {

private final ClassInfo targetClass;
private final String namespace;
private final String[] ignore;
private final Pattern[] ignorePatterns;
private final boolean ignoreSuperclasses;
private final boolean properties;

public TemplateDataBuildItem(ClassInfo targetClass, String namespace, String[] ignore, boolean ignoreSuperclasses,
boolean properties) {
this.targetClass = targetClass;
this.namespace = namespace;
this.ignore = ignore;
this.ignoreSuperclasses = ignoreSuperclasses;
this.properties = properties;
if (ignore.length > 0) {
ignorePatterns = new Pattern[ignore.length];
for (int i = 0; i < ignore.length; i++) {
ignorePatterns[i] = Pattern.compile(ignore[i]);
}
} else {
ignorePatterns = null;
}
}

public ClassInfo getTargetClass() {
return targetClass;
}

public boolean hasNamespace() {
return namespace != null;
}

public String getNamespace() {
return namespace;
}

public String[] getIgnore() {
return ignore;
}

public boolean isIgnoreSuperclasses() {
return ignoreSuperclasses;
}

public boolean isProperties() {
return properties;
}

boolean filter(AnnotationTarget target) {
String name = null;
if (target.kind() == Kind.METHOD) {
MethodInfo method = target.asMethod();
if (properties && !method.parameters().isEmpty()) {
return false;
}
name = method.name();
} else if (target.kind() == Kind.FIELD) {
FieldInfo field = target.asField();
name = field.name();
}
if (ignorePatterns != null) {
for (int i = 0; i < ignorePatterns.length; i++) {
if (ignorePatterns[i].matcher(name).matches()) {
return false;
}
}
}
return true;
}

@Override
public String toString() {
return "TemplateDataBuildItem [targetClass=" + targetClass + ", namespace=" + namespace + ", ignore="
+ Arrays.toString(ignore) + ", ignorePatterns=" + Arrays.toString(ignorePatterns) + ", ignoreSuperclasses="
+ ignoreSuperclasses + ", properties=" + properties + "]";
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ static Info create(String typeInfo, Expression.Part part, IndexView index, Funct
if (part.isVirtualMethod() || Expressions.isVirtualMethod(typeInfo)) {
return new VirtualMethodInfo(typeInfo, part.asVirtualMethod(), hint);
}
return new PropertyInfo(typeInfo, part, hint);
return new PropertyInfo(part.getName(), part, hint);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ public class AsyncTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClass(TemplateDataTest.Foo.class)
.addClass(Foo.class)
.addAsResource(new StringAsset("{foo.val} is not {foo.val.setScale(2,roundingMode)}"),
"templates/foo.txt"));

Expand All @@ -34,14 +34,14 @@ public class AsyncTest {
@Test
public void testAsyncRendering() {
CompletionStage<String> async = foo.data("roundingMode", RoundingMode.HALF_UP)
.data("foo", new TemplateDataTest.Foo(new BigDecimal("123.4563"))).renderAsync();
.data("foo", new Foo(new BigDecimal("123.4563"))).renderAsync();
assertEquals("123.4563 is not 123.46", async.toCompletableFuture().join());
}

@Test
public void testAsyncRenderingAsUni() {
Uni<String> uni = Uni.createFrom().completionStage(() -> foo.data("roundingMode", RoundingMode.HALF_UP)
.data("foo", new TemplateDataTest.Foo(new BigDecimal("123.4563"))).renderAsync());
.data("foo", new Foo(new BigDecimal("123.4563"))).renderAsync());
assertEquals("123.4563 is not 123.46", uni.await().indefinitely());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class MultiTest {
@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClass(TemplateDataTest.Foo.class)
.addClass(Foo.class)
.addAsResource(new StringAsset("{foo.val} is not {foo.val.setScale(2,roundingMode)}"),
"templates/foo.txt"));

Expand All @@ -33,7 +33,7 @@ public class MultiTest {
@Test
public void testCreateMulti() {
Multi<String> multi = foo.data("roundingMode", RoundingMode.HALF_UP)
.data("foo", new TemplateDataTest.Foo(new BigDecimal("123.4563"))).createMulti();
.data("foo", new Foo(new BigDecimal("123.4563"))).createMulti();
assertEquals("123.4563 is not 123.46", multi
.collect().in(StringBuffer::new, StringBuffer::append)
.onItem().transform(StringBuffer::toString)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ public boolean isBaz() {

}

@TemplateData(namespace = "TransactionType")
// namespace is TransactionType
@TemplateData(namespace = TemplateData.SIMPLENAME)
public static enum TransactionType {

FOO,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package io.quarkus.qute.deployment;

import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;

import org.jboss.shrinkwrap.api.ShrinkWrap;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.jboss.shrinkwrap.api.spec.JavaArchive;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.qute.TemplateData;
import io.quarkus.qute.TemplateException;
import io.quarkus.test.QuarkusUnitTest;

public class TemplateDataValidationTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addClasses(MyClass.class)
.addAsResource(new StringAsset(
"{foo_My:BAZ}"),
"templates/foo.txt"))
.assertException(t -> {
Throwable e = t;
TemplateException te = null;
while (e != null) {
if (e instanceof TemplateException) {
te = (TemplateException) e;
break;
}
e = e.getCause();
}
assertNotNull(te);
assertTrue(te.getMessage().contains("Found template problems (1)"), te.getMessage());
assertTrue(te.getMessage().contains("foo_My:BAZ"), te.getMessage());
});

@Test
public void test() {
fail();
}

@TemplateData(namespace = "foo_My")
public static class MyClass {

public static final String FOO = "foo";

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@ public class ValidationFailuresTest {
+ "{#each movie.mainCharacters}{it.boom(1)}{/}"
// Template extension method must accept one param
+ "{movie.toNumber}"
+ "{#each movie}{it}{/each}"),
+ "{#each movie}{it}{/each}"
// Bean not found
+ "{movie.findService(inject:ageBean)}"),
"templates/movie.html"))
.assertException(t -> {
Throwable e = t;
Expand All @@ -48,7 +50,7 @@ public class ValidationFailuresTest {
e = e.getCause();
}
assertNotNull(te);
assertTrue(te.getMessage().contains("Found template problems (9)"), te.getMessage());
assertTrue(te.getMessage().contains("Found template problems (10)"), te.getMessage());
assertTrue(te.getMessage().contains("movie.foo"), te.getMessage());
assertTrue(te.getMessage().contains("movie.getName('foo')"), te.getMessage());
assertTrue(te.getMessage().contains("movie.findService(age)"), te.getMessage());
Expand All @@ -57,6 +59,7 @@ public class ValidationFailuresTest {
assertTrue(te.getMessage().contains("movie.toNumber(age)"), te.getMessage());
assertTrue(te.getMessage().contains("it.boom(1)"), te.getMessage());
assertTrue(te.getMessage().contains("movie.toNumber"), te.getMessage());
assertTrue(te.getMessage().contains("inject:ageBean"), te.getMessage());
assertTrue(
te.getMessage().contains("Unsupported iterable type found: io.quarkus.qute.deployment.typesafe.Movie"),
te.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@
public @interface TemplateData {

/**
* Constant value for {@link #key()} indicating that the annotated element's name should be used as-is.
* Constant value for {@link #namespace()} indicating that the fully qualified class name of the target class should be
* used. Dots and dollar signs are replaced by underscores.
*/
String UNDERSCORED_FQCN = "<<undescored fqcn>>";

/**
* Constant value for {@link #namespace()} indicating that the simple name of the target class should be used.
*/
String SIMPLENAME = "<<simplename>>";

/**
* The class a value resolver should be generated for. By default, the annotated type.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,12 @@ private void generate(DotName className, int priority) {
if (namespace.isBlank()) {
namespace = null;
}
if (namespace != null && namespace.equals(TemplateData.UNDERSCORED_FQCN)) {
namespace = clazzName.replace(".", "_").replace("$", "_");
if (namespace != null) {
if (namespace.equals(TemplateData.UNDERSCORED_FQCN)) {
namespace = underscoredFullyQualifiedName(clazzName);
} else if (namespace.equals(TemplateData.SIMPLENAME)) {
namespace = simpleName(clazz);
}
}
}

Expand Down Expand Up @@ -1101,7 +1105,7 @@ public static String capitalize(String name) {
* @param clazz
* @return the simple name for the given top-level or nested class
*/
static String simpleName(ClassInfo clazz) {
public static String simpleName(ClassInfo clazz) {
switch (clazz.nestingType()) {
case TOP_LEVEL:
return simpleName(clazz.name());
Expand Down Expand Up @@ -1198,6 +1202,10 @@ public static boolean isVarArgs(MethodInfo method) {
return (method.flags() & 0x00000080) != 0;
}

public static String underscoredFullyQualifiedName(String name) {
return name.replace(".", "_").replace("$", "_");
}

private static class Match {

final String name;
Expand Down

0 comments on commit b05709c

Please sign in to comment.