From bc206a186f5897cfcd259ac667136740422a6b4b Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Mon, 30 Sep 2024 17:36:52 +0200 Subject: [PATCH] Qute: fix generated ValueResolvers - fixes #43489 - consider default methods in class hierarchy; previously, interface default methods were only considered if a ValueResolver was generated for an interface - also consider inherited fields in a class hierarchy --- .../DefaultMethodValidationSuccessTest.java | 2 +- .../generator/ValueResolverGenerator.java | 78 ++++++++++++------- .../qute/generator/SimpleGeneratorTest.java | 4 +- .../qute/generator/hierarchy/FirstLevel.java | 9 +++ .../generator/hierarchy/HierarchyTest.java | 60 ++++++++++++++ .../qute/generator/hierarchy/Level1.java | 10 +++ .../qute/generator/hierarchy/Level2.java | 8 ++ .../qute/generator/hierarchy/Level3.java | 15 ++++ .../qute/generator/hierarchy/Level4.java | 10 +++ .../qute/generator/hierarchy/SecondLevel.java | 9 +++ .../it/qute/DefaultMethodResource.java | 38 +++++++++ .../templates/DefaultMethodResource/hello.txt | 1 + .../java/io/quarkus/it/qute/QuteTestCase.java | 1 + 13 files changed, 212 insertions(+), 33 deletions(-) create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/FirstLevel.java create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/HierarchyTest.java create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level1.java create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level2.java create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level3.java create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level4.java create mode 100644 independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/SecondLevel.java create mode 100644 integration-tests/qute/src/main/java/io/quarkus/it/qute/DefaultMethodResource.java create mode 100644 integration-tests/qute/src/main/resources/templates/DefaultMethodResource/hello.txt diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/DefaultMethodValidationSuccessTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/DefaultMethodValidationSuccessTest.java index dc564a6e1eefb..6357a70d80bc5 100644 --- a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/DefaultMethodValidationSuccessTest.java +++ b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/typesafe/DefaultMethodValidationSuccessTest.java @@ -16,7 +16,7 @@ public class DefaultMethodValidationSuccessTest { @RegisterExtension static final QuarkusUnitTest config = new QuarkusUnitTest() .withApplicationRoot((jar) -> jar - .addClasses(Movie.class, MovieExtensions.class) + .addClasses(Name.class, Something.class) .addAsResource(new StringAsset( "{@io.quarkus.qute.deployment.typesafe.DefaultMethodValidationSuccessTest$Name name}Hello {name.fullName()}::{name.fullName}!"), "templates/name.html")); diff --git a/independent-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java b/independent-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java index f145f7631b761..b390b7b5a60a9 100644 --- a/independent-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java +++ b/independent-projects/qute/generator/src/main/java/io/quarkus/qute/generator/ValueResolverGenerator.java @@ -266,41 +266,59 @@ private boolean implementResolve(ClassCreator valueResolver, String clazzName, C ResultHandle paramsCount = resolve.invokeInterfaceMethod(Descriptors.COLLECTION_SIZE, params); Function fieldToGetterFun = forceGettersFunction != null ? forceGettersFunction.apply(clazz) : null; - // First collect and sort methods (getters must come before is/has properties, etc.) - List methods = new ArrayList<>(); - for (MethodInfo method : clazz.methods()) { - if (filter.test(method)) { - methods.add(new MethodKey(method)); + // First collect methods and fields from the class hierarchy + Set methods = new HashSet<>(); + List fields = new ArrayList<>(); + ClassInfo target = clazz; + while (target != null) { + for (MethodInfo method : target.methods()) { + if (filter.test(method)) { + methods.add(new MethodKey(method)); + } } - } - methods.sort(null); - - if (!ignoreSuperclasses && !clazz.isEnum()) { - DotName superName = clazz.superName(); - while (superName != null && !superName.equals(DotNames.OBJECT)) { - ClassInfo superClass = index.getClassByName(superName); - if (superClass != null) { - for (MethodInfo method : superClass.methods()) { - if (filter.test(method)) { - methods.add(new MethodKey(method)); - } - } - superName = superClass.superName(); - } else { - superName = null; - LOGGER.warnf("Skipping super class %s - not found in the index", clazz.superClassType()); + for (FieldInfo field : target.fields()) { + if (filter.test(field)) { + fields.add(field); + } + } + DotName superName = target.superName(); + if (ignoreSuperclasses || target.isEnum() || superName == null || superName.equals(DotNames.OBJECT)) { + target = null; + } else { + target = index.getClassByName(superName); + if (target == null) { + LOGGER.warnf("Skipping super class %s - not found in the index", superName); } } } - List fields = new ArrayList<>(); - for (FieldInfo field : clazz.fields()) { - if (filter.test(field)) { - fields.add(field); + // Find non-implemented default interface methods + target = clazz; + while (target != null) { + for (DotName interfaceName : target.interfaceNames()) { + ClassInfo interfaceClass = index.getClassByName(interfaceName); + if (interfaceClass == null) { + LOGGER.warnf("Skipping implemented interface %s - not found in the index", interfaceName); + continue; + } + for (MethodInfo method : interfaceClass.methods()) { + if (method.isDefault() && filter.test(method)) { + methods.add(new MethodKey(method)); + } + } + } + DotName superName = target.superName(); + if (ignoreSuperclasses || target.isEnum() || superName == null || superName.equals(DotNames.OBJECT)) { + target = null; + } else { + target = index.getClassByName(superName); } } - if (methods.isEmpty() && fields.isEmpty()) { + // Sort methods, getters must come before is/has properties, etc. + List sortedMethods = methods.stream().sorted().toList(); + + if (sortedMethods.isEmpty() && fields.isEmpty()) { // No members return false; } @@ -311,7 +329,7 @@ private boolean implementResolve(ClassCreator valueResolver, String clazzName, C Map> matches = new HashMap<>(); Map> varargsMatches = new HashMap<>(); - for (MethodKey methodKey : methods) { + for (MethodKey methodKey : sortedMethods) { MethodInfo method = methodKey.method; if (method.parametersCount() == 0) { noParamMethods.add(methodKey); @@ -365,7 +383,7 @@ private boolean implementResolve(ClassCreator valueResolver, String clazzName, C public void accept(BytecodeCreator bc) { Type returnType = method.returnType(); ResultHandle invokeRet; - if (Modifier.isInterface(clazz.flags())) { + if (Modifier.isInterface(method.declaringClass().flags())) { invokeRet = bc.invokeInterfaceMethod(MethodDescriptor.of(method), base); } else { invokeRet = bc.invokeVirtualMethod(MethodDescriptor.of(method), base); @@ -378,7 +396,7 @@ public void accept(BytecodeCreator bc) { for (FieldInfo field : fields) { String getterName = fieldToGetterFun != null ? fieldToGetterFun.apply(field) : null; - if (getterName != null && noneMethodMatches(methods, getterName) && matchedNames.add(getterName)) { + if (getterName != null && noneMethodMatches(sortedMethods, getterName) && matchedNames.add(getterName)) { LOGGER.debugf("Forced getter added: %s", field); List matching; if (matchedNames.add(field.name())) { diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/SimpleGeneratorTest.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/SimpleGeneratorTest.java index 6246789efd319..a84dd142a52c4 100644 --- a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/SimpleGeneratorTest.java +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/SimpleGeneratorTest.java @@ -170,7 +170,7 @@ public void testArrays() { assertEquals("true,false,false,false,false,", engine.parse("{#for i in 5}{i_isFirst},{/for}").render()); } - private Resolver newResolver(String className) + public static Resolver newResolver(String className) throws ClassNotFoundException, InstantiationException, IllegalAccessException, IllegalArgumentException, InvocationTargetException, NoSuchMethodException, SecurityException { ClassLoader cl = Thread.currentThread().getContextClassLoader(); @@ -181,7 +181,7 @@ private Resolver newResolver(String className) return (Resolver) clazz.getDeclaredConstructor().newInstance(); } - static Index index(Class... classes) throws IOException { + public static Index index(Class... classes) throws IOException { Indexer indexer = new Indexer(); for (Class clazz : classes) { try (InputStream stream = SimpleGeneratorTest.class.getClassLoader() diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/FirstLevel.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/FirstLevel.java new file mode 100644 index 0000000000000..763546213ca2d --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/FirstLevel.java @@ -0,0 +1,9 @@ +package io.quarkus.qute.generator.hierarchy; + +public interface FirstLevel { + + default int firstLevel() { + return 1; + } + +} diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/HierarchyTest.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/HierarchyTest.java new file mode 100644 index 0000000000000..88789c388a40f --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/HierarchyTest.java @@ -0,0 +1,60 @@ +package io.quarkus.qute.generator.hierarchy; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.IOException; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import org.jboss.jandex.DotName; +import org.jboss.jandex.Index; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import io.quarkus.qute.Engine; +import io.quarkus.qute.EngineBuilder; +import io.quarkus.qute.ValueResolver; +import io.quarkus.qute.generator.SimpleGeneratorTest; +import io.quarkus.qute.generator.TestClassOutput; +import io.quarkus.qute.generator.ValueResolverGenerator; + +public class HierarchyTest { + + static Set generatedTypes = new HashSet<>(); + + @BeforeAll + public static void init() throws IOException { + TestClassOutput classOutput = new TestClassOutput(); + Index index = SimpleGeneratorTest.index(Level1.class, Level2.class, Level3.class, Level4.class, FirstLevel.class, + SecondLevel.class); + ValueResolverGenerator generator = ValueResolverGenerator.builder().setIndex(index).setClassOutput(classOutput) + .addClass(index.getClassByName(DotName.createSimple(Level4.class.getName()))) + .build(); + + generator.generate(); + generatedTypes.addAll(generator.getGeneratedTypes()); + } + + @Test + public void testHierarchy() throws Exception { + EngineBuilder builder = Engine.builder().addDefaults(); + for (String generatedType : generatedTypes) { + builder.addValueResolver((ValueResolver) SimpleGeneratorTest.newResolver(generatedType)); + } + Engine engine = builder.build(); + + Level4 level4 = new Level4(); + assertEquals(1, level4.getLevel1()); + assertEquals(1, level4.firstLevel()); + assertEquals(2, level4.secondLevel()); + assertEquals(4, level4.getLevel4()); + assertEquals(4, level4.overridenLevel); + + assertEquals("1::1::2::4::4", + engine.parse( + "{level.level1}::{level.firstLevel}::{level.secondLevel}::{level.getLevel4}::{level.overridenLevel}") + .render(Map.of("level", level4))); + } + +} diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level1.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level1.java new file mode 100644 index 0000000000000..8895e3679509e --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level1.java @@ -0,0 +1,10 @@ +package io.quarkus.qute.generator.hierarchy; + +public class Level1 implements FirstLevel { + + public int firstLevel = 1; + + public int getLevel1() { + return 1; + } +} diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level2.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level2.java new file mode 100644 index 0000000000000..5b6c90bb72945 --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level2.java @@ -0,0 +1,8 @@ +package io.quarkus.qute.generator.hierarchy; + +public class Level2 extends Level1 implements SecondLevel { + + public int getLevel2() { + return 2; + } +} diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level3.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level3.java new file mode 100644 index 0000000000000..2e1f7f1849e73 --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level3.java @@ -0,0 +1,15 @@ +package io.quarkus.qute.generator.hierarchy; + +public class Level3 extends Level2 { + + public int overridenLevel = 3; + + public int getLevel3() { + return 3; + } + + // This method should be overriden + public int getLevel4() { + return 34; + } +} diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level4.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level4.java new file mode 100644 index 0000000000000..5a071b07835cf --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/Level4.java @@ -0,0 +1,10 @@ +package io.quarkus.qute.generator.hierarchy; + +public class Level4 extends Level3 { + + public int overridenLevel = 4; + + public int getLevel4() { + return 4; + } +} diff --git a/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/SecondLevel.java b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/SecondLevel.java new file mode 100644 index 0000000000000..a9713575eadc2 --- /dev/null +++ b/independent-projects/qute/generator/src/test/java/io/quarkus/qute/generator/hierarchy/SecondLevel.java @@ -0,0 +1,9 @@ +package io.quarkus.qute.generator.hierarchy; + +public interface SecondLevel { + + default int secondLevel() { + return 2; + } + +} diff --git a/integration-tests/qute/src/main/java/io/quarkus/it/qute/DefaultMethodResource.java b/integration-tests/qute/src/main/java/io/quarkus/it/qute/DefaultMethodResource.java new file mode 100644 index 0000000000000..68da431a5e14a --- /dev/null +++ b/integration-tests/qute/src/main/java/io/quarkus/it/qute/DefaultMethodResource.java @@ -0,0 +1,38 @@ +package io.quarkus.it.qute; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; + +import io.quarkus.qute.TemplateInstance; + +@Path("/defaultmethod") +public class DefaultMethodResource { + + @GET + @Produces(MediaType.TEXT_PLAIN) + public TemplateInstance get() { + return new hello(new Name()); + } + + record hello(Name name) implements TemplateInstance { + }; + + public static class Name implements Something { + + public String name() { + return "M"; + } + } + + public interface Something { + + String name(); + + default String fullName() { + return name() + "K"; + } + } + +} diff --git a/integration-tests/qute/src/main/resources/templates/DefaultMethodResource/hello.txt b/integration-tests/qute/src/main/resources/templates/DefaultMethodResource/hello.txt new file mode 100644 index 0000000000000..0a181d9a27831 --- /dev/null +++ b/integration-tests/qute/src/main/resources/templates/DefaultMethodResource/hello.txt @@ -0,0 +1 @@ +Hello {name.fullName}! \ No newline at end of file diff --git a/integration-tests/qute/src/test/java/io/quarkus/it/qute/QuteTestCase.java b/integration-tests/qute/src/test/java/io/quarkus/it/qute/QuteTestCase.java index 4f43b07e835c1..7e875a44db5b0 100644 --- a/integration-tests/qute/src/test/java/io/quarkus/it/qute/QuteTestCase.java +++ b/integration-tests/qute/src/test/java/io/quarkus/it/qute/QuteTestCase.java @@ -27,6 +27,7 @@ public void testTemplates() throws InterruptedException { .contentType(is(ContentType.HTML.toString())) .body(containsString("Hello Ciri!")); RestAssured.when().get("/beer").then().body(containsString("Beer Pilsner, completed: true, done: true")); + RestAssured.when().get("/defaultmethod").then().body(containsString("Hello MK!")); } @Test