Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Qute: fix template global class generation in the dev mode #45771

Merged
merged 1 commit into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,13 @@ public void transform(TransformationContext ctx) {
knownMissingClasses, Thread.currentThread().getContextClassLoader());
}
Set<DotName> generatedClassNames = new HashSet<>();
for (GeneratedBeanBuildItem generatedBeanClass : generatedBeans) {
IndexingUtil.indexClass(generatedBeanClass.getName(), additionalBeanIndexer, applicationIndex, additionalIndex,
knownMissingClasses, Thread.currentThread().getContextClassLoader(), generatedBeanClass.getData());
generatedClassNames.add(DotName.createSimple(generatedBeanClass.getName().replace('/', '.')));
generatedClass.produce(new GeneratedClassBuildItem(true, generatedBeanClass.getName(), generatedBeanClass.getData(),
generatedBeanClass.getSource()));
for (GeneratedBeanBuildItem generatedBean : generatedBeans) {
IndexingUtil.indexClass(generatedBean.getName(), additionalBeanIndexer, applicationIndex, additionalIndex,
knownMissingClasses, Thread.currentThread().getContextClassLoader(), generatedBean.getData());
generatedClassNames.add(DotName.createSimple(generatedBean.getName().replace('/', '.')));
generatedClass.produce(new GeneratedClassBuildItem(generatedBean.isApplicationClass(), generatedBean.getName(),
generatedBean.getData(),
generatedBean.getSource()));
}

PersistentClassIndex index = liveReloadBuildItem.getContextObject(PersistentClassIndex.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/
public final class GeneratedBeanBuildItem extends MultiBuildItem {

private final boolean applicationClass;
private final String name;
private final byte[] data;
private final String source;
Expand All @@ -17,9 +18,14 @@ public GeneratedBeanBuildItem(String name, byte[] data) {
}

public GeneratedBeanBuildItem(String name, byte[] data, String source) {
this(name, data, source, true);
}

public GeneratedBeanBuildItem(String name, byte[] data, String source, boolean applicationClass) {
this.name = name;
this.data = data;
this.source = source;
this.applicationClass = applicationClass;
}

public String getName() {
Expand All @@ -38,4 +44,8 @@ public String getSource() {
return source;
}

public boolean isApplicationClass() {
return applicationClass;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import java.io.Writer;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Predicate;

import io.quarkus.bootstrap.BootstrapDebug;
import io.quarkus.deployment.annotations.BuildProducer;
Expand All @@ -13,10 +14,23 @@ public class GeneratedBeanGizmoAdaptor implements ClassOutput {

private final BuildProducer<GeneratedBeanBuildItem> classOutput;
private final Map<String, StringWriter> sources;
private final Predicate<String> applicationClassPredicate;

public GeneratedBeanGizmoAdaptor(BuildProducer<GeneratedBeanBuildItem> classOutput) {
this(classOutput, new Predicate<String>() {

@Override
public boolean test(String t) {
return true;
}
});
}

public GeneratedBeanGizmoAdaptor(BuildProducer<GeneratedBeanBuildItem> classOutput,
Predicate<String> applicationClassPredicate) {
this.classOutput = classOutput;
this.sources = BootstrapDebug.debugSourcesDir() != null ? new ConcurrentHashMap<>() : null;
this.applicationClassPredicate = applicationClassPredicate;
}

@Override
Expand All @@ -28,7 +42,7 @@ public void write(String className, byte[] bytes) {
source = sw.toString();
}
}
classOutput.produce(new GeneratedBeanBuildItem(className, bytes, source));
classOutput.produce(new GeneratedBeanBuildItem(className, bytes, source, applicationClassPredicate.test(className)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
package io.quarkus.qute.deployment;

import java.lang.reflect.Modifier;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.Predicate;

import org.jboss.jandex.DotName;

import io.quarkus.arc.deployment.GeneratedBeanBuildItem;
import io.quarkus.arc.deployment.GeneratedBeanGizmoAdaptor;
import io.quarkus.arc.deployment.ValidationPhaseBuildItem.ValidationErrorBuildItem;
import io.quarkus.deployment.IsDevelopment;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;
import io.quarkus.deployment.annotations.BuildSteps;
import io.quarkus.deployment.builditem.ApplicationIndexBuildItem;
import io.quarkus.dev.console.DevConsoleManager;
import io.quarkus.gizmo.ClassCreator;
import io.quarkus.gizmo.FieldCreator;
import io.quarkus.gizmo.MethodCreator;
import io.quarkus.qute.TemplateGlobal;
import io.quarkus.qute.runtime.devmode.QuteErrorPageSetup;

@BuildSteps(onlyIf = IsDevelopment.class)
Expand All @@ -28,4 +39,33 @@ void collectGeneratedContents(List<TemplatePathBuildItem> templatePaths,
DevConsoleManager.setGlobal(QuteErrorPageSetup.GENERATED_CONTENTS, contents);
}

// This build step is only used to for a QuarkusDevModeTest that contains the QuteDummyTemplateGlobalMarker interface
@BuildStep
void generateTestTemplateGlobal(ApplicationIndexBuildItem applicationIndex,
BuildProducer<GeneratedBeanBuildItem> generatedBeanClasses) {
if (applicationIndex.getIndex().getClassByName(
DotName.createSimple("io.quarkus.qute.deployment.devmode.QuteDummyTemplateGlobalMarker")) != null) {
// If the marker interface is present then we generate a dummy class annotated with @TemplateGlobal
GeneratedBeanGizmoAdaptor gizmoAdaptor = new GeneratedBeanGizmoAdaptor(generatedBeanClasses,
new Predicate<String>() {
@Override
public boolean test(String t) {
return false;
}
});
try (ClassCreator classCreator = ClassCreator.builder().className("io.quarkus.qute.test.QuteDummyGlobals")
.classOutput(gizmoAdaptor).build()) {
classCreator.addAnnotation(TemplateGlobal.class);

FieldCreator quteDummyFoo = classCreator.getFieldCreator("quteDummyFoo", String.class);
quteDummyFoo.setModifiers(Modifier.STATIC);

MethodCreator staticInitializer = classCreator.getMethodCreator("<clinit>", void.class);
staticInitializer.setModifiers(Modifier.STATIC);
staticInitializer.writeStaticField(quteDummyFoo.getFieldDescriptor(), staticInitializer.load("bar"));
staticInitializer.returnVoid();
}
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -2094,7 +2094,13 @@ public Function<FieldInfo, String> apply(ClassInfo clazz) {
}

if (!templateGlobals.isEmpty()) {
TemplateGlobalGenerator globalGenerator = new TemplateGlobalGenerator(classOutput, GLOBAL_NAMESPACE, -1000, index);
Set<String> generatedGlobals = new HashSet<>();
// The initial priority is increased during live reload so that priorities of non-application globals
// do not conflict with priorities of application globals
int initialPriority = -1000 + existingValueResolvers.globals.size();

TemplateGlobalGenerator globalGenerator = new TemplateGlobalGenerator(classOutput, GLOBAL_NAMESPACE,
initialPriority, index);

Map<DotName, Map<String, AnnotationTarget>> classToTargets = new HashMap<>();
Map<DotName, List<TemplateGlobalBuildItem>> classToGlobals = templateGlobals.stream()
Expand All @@ -2105,12 +2111,19 @@ public Function<FieldInfo, String> apply(ClassInfo clazz) {
}

for (Entry<DotName, Map<String, AnnotationTarget>> e : classToTargets.entrySet()) {
globalGenerator.generate(index.getClassByName(e.getKey()), e.getValue());
String generatedClass = existingValueResolvers.getGeneratedGlobalClass(e.getKey());
if (generatedClass != null) {
generatedGlobals.add(generatedClass);
} else {
generatedClass = globalGenerator.generate(index.getClassByName(e.getKey()), e.getValue());
existingValueResolvers.addGlobal(e.getKey(), generatedClass, applicationClassPredicate);
}
}
generatedGlobals.addAll(globalGenerator.getGeneratedTypes());

for (String generatedType : globalGenerator.getGeneratedTypes()) {
globalProviders.produce(new TemplateGlobalProviderBuildItem(generatedType));
reflectiveClass.produce(ReflectiveClassBuildItem.builder(generatedType).build());
for (String globalType : generatedGlobals) {
globalProviders.produce(new TemplateGlobalProviderBuildItem(globalType));
reflectiveClass.produce(ReflectiveClassBuildItem.builder(globalType).build());
}
}
}
Expand All @@ -2122,12 +2135,18 @@ public Function<FieldInfo, String> apply(ClassInfo clazz) {
static class ExistingValueResolvers {

final Map<String, String> identifiersToGeneratedClass = new HashMap<>();
// class declaring globals -> generated type
final Map<String, String> globals = new HashMap<>();

boolean contains(MethodInfo extensionMethod) {
return identifiersToGeneratedClass
.containsKey(toKey(extensionMethod));
}

String getGeneratedGlobalClass(DotName declaringClassName) {
return globals.get(declaringClassName.toString());
}

String getGeneratedClass(MethodInfo extensionMethod) {
return identifiersToGeneratedClass.get(toKey(extensionMethod));
}
Expand All @@ -2138,6 +2157,12 @@ void add(MethodInfo extensionMethod, String className, Predicate<DotName> applic
}
}

void addGlobal(DotName declaringClassName, String generatedClassName, Predicate<DotName> applicationClassPredicate) {
if (!applicationClassPredicate.test(declaringClassName)) {
globals.put(declaringClassName.toString(), generatedClassName);
}
}

private String toKey(MethodInfo extensionMethod) {
return extensionMethod.declaringClass().toString() + "#" + extensionMethod.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class ExistingValueResolversDevModeTest {
.addClass(TestRoute.class)
.addAsResource(new StringAsset(
"{#let a = 3}{#let b = a.minus(2)}b={b}{/}{/}"),
"templates/let.html"));
"templates/test.html"));

@Test
public void testExistingValueResolvers() {
Expand All @@ -29,7 +29,7 @@ public void testExistingValueResolvers() {
.statusCode(200)
.body(Matchers.equalTo("b=1"));

config.modifyResourceFile("templates/let.html", t -> t.concat("::MODIFIED"));
config.modifyResourceFile("templates/test.html", t -> t.concat("::MODIFIED"));

given().get("test")
.then()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package io.quarkus.qute.deployment.devmode;

/**
* Marker interface for {@link TemplateGlobalDevModeTest}.
*/
public interface QuteDummyTemplateGlobalMarker {

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

import static io.restassured.RestAssured.given;

import org.hamcrest.Matchers;
import org.jboss.shrinkwrap.api.asset.StringAsset;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusDevModeTest;

/**
* Test that template globals added in the dev mode are generated correctly after live reload.
* <p>
* The {@link QuteDummyTemplateGlobalMarker} is used to identify an application archive where a dummy built-in class with
* template globals is added.
*/
public class TemplateGlobalDevModeTest {
Copy link
Contributor Author

@mkouba mkouba Jan 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@FroMage ok, so I've added this test. It's a bit hacky but the build step that generates a dummy global is only executed in the dev mode and if QuteDummyTemplateGlobalMarker is present.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can add custom build steps in your test, no?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no, it's only for ProdMode tests. Well… I suppose we could add that to dev mode tests too…

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah no, it's only for ProdMode tests. Well… I suppose we could add that to dev mode tests too…

And in QuarkusUnitTest...

I have no idea how complicated this could be 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely some work, up to you, I don't mind your build step :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case, it's out of scope of this PR. We could file a new issue and revisit the test later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI #45811


@RegisterExtension
static final QuarkusDevModeTest config = new QuarkusDevModeTest()
.withApplicationRoot(root -> root
.addClasses(TestRoute.class, QuteDummyTemplateGlobalMarker.class)
.addAsResource(new StringAsset(
"{quteDummyFoo}:{testFoo ?: 'NA'}"),
"templates/test.html"));

@Test
public void testTemplateGlobals() {
given().get("test")
.then()
.statusCode(200)
.body(Matchers.equalTo("bar:NA"));

// Add application globals - the priority sequence should be automatically
// increased before it's used for TestGlobals
config.addSourceFile(TestGlobals.class);

given().get("test")
.then()
.statusCode(200)
.body(Matchers.equalTo("bar:baz"));
}

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

import io.quarkus.qute.TemplateGlobal;

@TemplateGlobal
public class TestGlobals {

public static String testFoo() {
return "baz";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
public class TestRoute {

@Inject
Template let;
Template test;

@Route(path = "test")
public void test(RoutingContext ctx) {
ctx.end(let.render());
ctx.end(test.render());
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ public class TemplateGlobalGenerator extends AbstractGenerator {
private final String namespace;
private int priority;

public TemplateGlobalGenerator(ClassOutput classOutput, String namespace, int priority, IndexView index) {
public TemplateGlobalGenerator(ClassOutput classOutput, String namespace, int initialPriority, IndexView index) {
super(index, classOutput);
this.namespace = namespace;
this.priority = priority;
this.priority = initialPriority;
}

public void generate(ClassInfo declaringClass, Map<String, AnnotationTarget> targets) {
public String generate(ClassInfo declaringClass, Map<String, AnnotationTarget> targets) {

String baseName;
if (declaringClass.enclosingClass() != null) {
Expand All @@ -65,7 +65,8 @@ public void generate(ClassInfo declaringClass, Map<String, AnnotationTarget> tar
}
String targetPackage = packageName(declaringClass.name());
String generatedName = generatedNameFromTarget(targetPackage, baseName, SUFFIX);
generatedTypes.add(generatedName.replace('/', '.'));
String generatedClassName = generatedName.replace('/', '.');
generatedTypes.add(generatedClassName);

ClassCreator provider = ClassCreator.builder().classOutput(classOutput).className(generatedName)
.interfaces(TemplateGlobalProvider.class).build();
Expand Down Expand Up @@ -141,6 +142,7 @@ public void accept(BytecodeCreator bc) {
resolve.returnValue(resolve.invokeStaticMethod(Descriptors.RESULTS_NOT_FOUND_EC, evalContext));

provider.close();
return generatedClassName;
}

public Set<String> getGeneratedTypes() {
Expand Down
Loading