Skip to content

Commit

Permalink
Qute type-safe validation - validate namespace extension methods
Browse files Browse the repository at this point in the history 
- also improve the error message if no resolver is found for a namespace
at runtime
- resolves quarkusio#20701

(cherry picked from commit 2cef352)
  • Loading branch information
@mkouba @aloubyansky
mkouba authored and aloubyansky committed Oct 18, 2021
1 parent 79b41d4 commit 0a23066
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 37 deletions.
15 changes: 12 additions & 3 deletions ...ions/qute/deployment/src/main/java/io/quarkus/qute/deployment/MessageBundleProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,14 @@ public String apply(String id) {
.filter(TemplateDataBuildItem::hasNamespace)
.collect(Collectors.toMap(TemplateDataBuildItem::getNamespace, Function.identity()));

Map<String, List<TemplateExtensionMethodBuildItem>> namespaceExtensionMethods = templateExtensionMethods.stream()
.filter(TemplateExtensionMethodBuildItem::hasNamespace)
.sorted(Comparator.comparingInt(TemplateExtensionMethodBuildItem::getPriority).reversed())
.collect(Collectors.groupingBy(TemplateExtensionMethodBuildItem::getNamespace));

List<TemplateExtensionMethodBuildItem> regularExtensionMethods = templateExtensionMethods.stream()
.filter(Predicate.not(TemplateExtensionMethodBuildItem::hasNamespace)).collect(Collectors.toUnmodifiableList());

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

// bundle name -> (key -> method)
Expand Down Expand Up @@ -474,9 +482,10 @@ 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, lookupConfig, namedBeans, namespaceTemplateData);
results, excludes, incorrectExpressions, expression, index,
implicitClassToMembersUsed, templateIdToPathFun, generatedIdsToMatches,
checkedTemplate, lookupConfig, namedBeans, namespaceTemplateData,
regularExtensionMethods, namespaceExtensionMethods);
Match match = results.get(param.toOriginalString());
if (match != null && !match.isEmpty() && !Types.isAssignableFrom(match.type(),
methodParams.get(idx), index)) {
Expand Down
93 changes: 64 additions & 29 deletions extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/QuteProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
Expand Down Expand Up @@ -499,6 +500,14 @@ public String apply(String id) {
.filter(TemplateDataBuildItem::hasNamespace)
.collect(Collectors.toMap(TemplateDataBuildItem::getNamespace, Function.identity()));

Map<String, List<TemplateExtensionMethodBuildItem>> namespaceExtensionMethods = templateExtensionMethods.stream()
.filter(TemplateExtensionMethodBuildItem::hasNamespace)
.sorted(Comparator.comparingInt(TemplateExtensionMethodBuildItem::getPriority).reversed())
.collect(Collectors.groupingBy(TemplateExtensionMethodBuildItem::getNamespace));

List<TemplateExtensionMethodBuildItem> regularExtensionMethods = templateExtensionMethods.stream()
.filter(Predicate.not(TemplateExtensionMethodBuildItem::hasNamespace)).collect(Collectors.toUnmodifiableList());

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

for (TemplateAnalysis templateAnalysis : templatesAnalysis.getAnalysis()) {
Expand All @@ -512,10 +521,10 @@ public String apply(String id) {
if (expression.isLiteral()) {
continue;
}
Match match = validateNestedExpressions(templateAnalysis, null, new HashMap<>(), templateExtensionMethods,
excludes, incorrectExpressions, expression, index, implicitClassToMembersUsed,
templateIdToPathFun, generatedIdsToMatches, checkedTemplate,
lookupConfig, namedBeans, namespaceTemplateData);
Match match = validateNestedExpressions(templateAnalysis, null, new HashMap<>(), excludes, incorrectExpressions,
expression, index, implicitClassToMembersUsed, templateIdToPathFun, generatedIdsToMatches,
checkedTemplate, lookupConfig, namedBeans, namespaceTemplateData, regularExtensionMethods,
namespaceExtensionMethods);
generatedIdsToMatches.put(expression.getGeneratedId(), match);
}
expressionMatches
Expand Down Expand Up @@ -573,21 +582,24 @@ static String buildIgnorePattern(Iterable<String> names) {
}

static Match validateNestedExpressions(TemplateAnalysis templateAnalysis, ClassInfo rootClazz, Map<String, Match> results,
List<TemplateExtensionMethodBuildItem> templateExtensionMethods, List<TypeCheckExcludeBuildItem> excludes,
BuildProducer<IncorrectExpressionBuildItem> incorrectExpressions, Expression expression, IndexView index,
List<TypeCheckExcludeBuildItem> excludes, BuildProducer<IncorrectExpressionBuildItem> incorrectExpressions,
Expression expression, IndexView index,
Map<DotName, Set<String>> implicitClassToMembersUsed, Function<String, String> templateIdToPathFun,
Map<Integer, Match> generatedIdsToMatches, CheckedTemplateBuildItem checkedTemplate,
LookupConfig lookupConfig, Map<String, BeanInfo> namedBeans,
Map<String, TemplateDataBuildItem> namespaceTemplateData) {
Map<String, TemplateDataBuildItem> namespaceTemplateData,
List<TemplateExtensionMethodBuildItem> regularExtensionMethods,
Map<String, List<TemplateExtensionMethodBuildItem>> namespaceExtensionMethods) {

// Validate the parameters of nested virtual methods
for (Expression.Part part : expression.getParts()) {
if (part.isVirtualMethod()) {
for (Expression param : part.asVirtualMethod().getParameters()) {
if (!results.containsKey(param.toOriginalString())) {
validateNestedExpressions(templateAnalysis, null, results, templateExtensionMethods, excludes,
validateNestedExpressions(templateAnalysis, null, results, excludes,
incorrectExpressions, param, index, implicitClassToMembersUsed, templateIdToPathFun,
generatedIdsToMatches, checkedTemplate, lookupConfig, namedBeans, namespaceTemplateData);
generatedIdsToMatches, checkedTemplate, lookupConfig, namedBeans, namespaceTemplateData,
regularExtensionMethods, namespaceExtensionMethods);
}
}
}
Expand All @@ -597,6 +609,7 @@ static Match validateNestedExpressions(TemplateAnalysis templateAnalysis, ClassI

String namespace = expression.getNamespace();
TemplateDataBuildItem templateData = null;
List<TemplateExtensionMethodBuildItem> extensionMethods = null;

if (namespace != null) {
if (namespace.equals(INJECT_NAMESPACE)) {
Expand All @@ -618,8 +631,11 @@ static Match validateNestedExpressions(TemplateAnalysis templateAnalysis, ClassI
filter = filter.and(templateData::filter);
lookupConfig = new FirstPassLookupConfig(lookupConfig, filter, true);
} else {
// All other namespaces are ignored
return putResult(match, results, expression);
extensionMethods = namespaceExtensionMethods.get(namespace);
if (extensionMethods == null) {
// All other namespaces are ignored
return putResult(match, results, expression);
}
}
}
}
Expand All @@ -645,7 +661,29 @@ static Match validateNestedExpressions(TemplateAnalysis templateAnalysis, ClassI
Iterator<Info> iterator = parts.iterator();
Info root = iterator.next();

if (rootClazz == null) {
if (extensionMethods != null) {
// Namespace is used and at least one namespace extension method exists for the given namespace
TemplateExtensionMethodBuildItem extensionMethod = findTemplateExtensionMethod(root, null, extensionMethods,
expression, index, templateIdToPathFun, results);
if (extensionMethod != null) {
MethodInfo method = extensionMethod.getMethod();
ClassInfo returnType = index.getClassByName(method.returnType().name());
if (returnType != null) {
match.setValues(returnType, method.returnType());
} else {
// Return type not available
return putResult(match, results, expression);
}
} else {
// No namespace extension method found - incorrect expression
incorrectExpressions.produce(new IncorrectExpressionBuildItem(expression.toOriginalString(),
String.format("no matching namespace [%s] extension method found", namespace), expression.getOrigin()));
match.clearValues();
return putResult(match, results, expression);
}

} else if (rootClazz == null) {
// No namespace is used or no declarative resolver (extension methods, @TemplateData, etc.)
if (root.isTypeInfo()) {
// E.g. |org.acme.Item|
match.setValues(root.asTypeInfo().rawClass, root.asTypeInfo().resolvedType);
Expand Down Expand Up @@ -750,7 +788,7 @@ static Match validateNestedExpressions(TemplateAnalysis templateAnalysis, ClassI

if (member == null) {
// Then try to find an etension method
extensionMethod = findTemplateExtensionMethod(info, match.type(), templateExtensionMethods, expression,
extensionMethod = findTemplateExtensionMethod(info, match.type(), regularExtensionMethods, expression,
index,
templateIdToPathFun, results);
if (extensionMethod != null) {
Expand Down Expand Up @@ -1086,15 +1124,10 @@ public Function<FieldInfo, String> apply(ClassInfo clazz) {
.entrySet()) {
Map<String, List<TemplateExtensionMethodBuildItem>> namespaceToMethods = classEntry.getValue();
for (Entry<String, List<TemplateExtensionMethodBuildItem>> nsEntry : namespaceToMethods.entrySet()) {
Map<Integer, List<TemplateExtensionMethodBuildItem>> priorityToMethods = new HashMap<>();
for (TemplateExtensionMethodBuildItem method : nsEntry.getValue()) {
List<TemplateExtensionMethodBuildItem> methods = priorityToMethods.get(method.getPriority());
if (methods == null) {
methods = new ArrayList<>();
priorityToMethods.put(method.getPriority(), methods);
}
methods.add(method);
}

Map<Integer, List<TemplateExtensionMethodBuildItem>> priorityToMethods = nsEntry.getValue().stream()
.collect(Collectors.groupingBy(TemplateExtensionMethodBuildItem::getPriority));

for (Entry<Integer, List<TemplateExtensionMethodBuildItem>> priorityEntry : priorityToMethods.entrySet()) {
try (NamespaceResolverCreator namespaceResolverCreator = extensionMethodGenerator
.createNamespaceResolver(priorityEntry.getValue().get(0).getMethod().declaringClass(),
Expand Down Expand Up @@ -1597,11 +1630,7 @@ private static TemplateExtensionMethodBuildItem findTemplateExtensionMethod(Info
}
String name = info.isProperty() ? info.asProperty().name : info.asVirtualMethod().name;
for (TemplateExtensionMethodBuildItem extensionMethod : templateExtensionMethods) {
if (extensionMethod.hasNamespace()) {
// Skip namespace extensions
continue;
}
if (!Types.isAssignableFrom(extensionMethod.getMatchType(), matchType, index)) {
if (matchType != null && !Types.isAssignableFrom(extensionMethod.getMatchType(), matchType, index)) {
// If "Bar extends Foo" then Bar should be matched for the extension method "int get(Foo)"
continue;
}
Expand All @@ -1610,7 +1639,13 @@ private static TemplateExtensionMethodBuildItem findTemplateExtensionMethod(Info
continue;
}
List<Type> parameters = extensionMethod.getMethod().parameters();
int realParamSize = parameters.size() - (TemplateExtension.ANY.equals(extensionMethod.getMatchName()) ? 2 : 1);
int realParamSize = parameters.size();
if (!extensionMethod.hasNamespace()) {
realParamSize -= 1;
}
if (TemplateExtension.ANY.equals(extensionMethod.getMatchName())) {
realParamSize -= 1;
}
if (realParamSize > 0 && !info.isVirtualMethod()) {
// If method accepts additional params the info must be a virtual method
continue;
Expand All @@ -1636,7 +1671,7 @@ private static TemplateExtensionMethodBuildItem findTemplateExtensionMethod(Info
// Check parameter types if available
boolean matches = true;
// Skip base and name param if needed
int idx = TemplateExtension.ANY.equals(extensionMethod.getMatchName()) ? 2 : 1;
int idx = parameters.size() - realParamSize;

for (Expression param : virtualMethod.getParameters()) {

Expand Down
2 changes: 1 addition & 1 deletion extensions/qute/deployment/src/main/java/io/quarkus/qute/deployment/TypeInfos.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ static List<Info> create(Expression expression, IndexView index, Function<String
List<Info> infos = new ArrayList<>();
boolean splitParts = true;
for (Expression.Part part : expression.getParts()) {
if (splitParts) {
if (splitParts && part.getTypeInfo().contains(TYPE_INFO_SEPARATOR)) {
List<String> infoParts = Expressions.splitTypeInfoParts(part.getTypeInfo());
for (String infoPart : infoParts) {
infos.add(create(infoPart, part, index, templateIdToPathFun, expression.getOrigin()));
Expand Down
48 changes: 48 additions & 0 deletions extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/TypeInfosTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
package io.quarkus.qute.deployment;

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

import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;

import org.jboss.jandex.Index;
import org.jboss.jandex.IndexView;
import org.jboss.jandex.Indexer;
import org.junit.jupiter.api.Test;

import io.quarkus.qute.Engine;
import io.quarkus.qute.Expression;
import io.quarkus.qute.deployment.TypeInfos.Info;

public class TypeInfosTest {

@Test
Expand All @@ -19,6 +29,27 @@ public void testHintPattern() {
assertHints("<set#10>loop-element>", "<set#10>");
}

@Test
public void testCreate() throws IOException {
List<Expression> expressions = Engine.builder().build()
.parse("{@io.quarkus.qute.deployment.TypeInfosTest$Foo foo}{config:['foo.bar.baz']}{foo.name}")
.getExpressions();
;
IndexView index = index(Foo.class);

List<Info> infos = TypeInfos.create(expressions.get(0), index, id -> "dummy");
assertEquals(1, infos.size());
assertTrue(infos.get(0).isProperty());
assertEquals("foo.bar.baz", infos.get(0).value);

infos = TypeInfos.create(expressions.get(1), index, id -> "dummy");
assertEquals(2, infos.size());
assertTrue(infos.get(0).isTypeInfo());
assertEquals("io.quarkus.qute.deployment.TypeInfosTest$Foo", infos.get(0).asTypeInfo().rawClass.name().toString());
assertTrue(infos.get(1).isProperty());
assertEquals("name", infos.get(1).value);
}

private void assertHints(String hintStr, String... expectedHints) {
Matcher m = TypeInfos.HintInfo.HINT_PATTERN.matcher(hintStr);
List<String> hints = new ArrayList<>();
Expand All @@ -28,4 +59,21 @@ private void assertHints(String hintStr, String... expectedHints) {
assertEquals(Arrays.asList(expectedHints), hints);
}

private static Index index(Class<?>... classes) throws IOException {
Indexer indexer = new Indexer();
for (Class<?> clazz : classes) {
try (InputStream stream = TypeInfosTest.class.getClassLoader()
.getResourceAsStream(clazz.getName().replace('.', '/') + ".class")) {
indexer.index(stream);
}
}
return indexer.complete();
}

public static class Foo {

public String name;

}

}
58 changes: 58 additions & 0 deletions ...o/quarkus/qute/deployment/extensions/NamespaceTemplateExtensionValidationFailureTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
package io.quarkus.qute.deployment.extensions;

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.TemplateException;
import io.quarkus.qute.TemplateExtension;
import io.quarkus.test.QuarkusUnitTest;

public class NamespaceTemplateExtensionValidationFailureTest {

@RegisterExtension
static final QuarkusUnitTest config = new QuarkusUnitTest()
.setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
.addAsResource(new StringAsset(
"{bro:surname}\n"
+ "{bro:name.bubu}"),
"templates/foo.html")
.addClasses(SomeExtensions.class))
.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 (2)"), te.getMessage());
assertTrue(te.getMessage().contains("no matching namespace [bro] extension method found"), te.getMessage());
assertTrue(te.getMessage().contains("property/method [bubu] not found on class [java.lang.String]"),
te.getMessage());
});

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

@TemplateExtension(namespace = "bro")
public static class SomeExtensions {

static String name() {
return "bubu";
}

}

}
Loading

0 comments on commit 0a23066

Please sign in to comment.