Skip to content

Commit

Permalink
Qute CDI integration - fix named dependent beans
Browse files Browse the repository at this point in the history 
- dependent beans are now: 1) shared accross all expressions for a single rendering operation, 2) destroyed correctly after
the rendering finished
- resolves quarkusio#24137
  • Loading branch information
@mkouba
mkouba committed Apr 6, 2022
1 parent 0dbe434 commit 5c11996
Show file tree
Hide file tree
Showing 16 changed files with 320 additions and 67 deletions.
4 changes: 2 additions & 2 deletions ...qute/deployment/src/test/java/io/quarkus/qute/deployment/PropertyNotFoundDevModeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ public class PropertyNotFoundDevModeTest {

@Test
public void testExceptionIsThrown() {
assertEquals("Entry \"foo\" not found in the data map in expression {foo.surname} in template foo on line 1",
assertEquals("Entry \"foo\" not found in the data map in expression {foo.surname} in template foo.html on line 1",
RestAssured.get("test-foo").then().statusCode(200).extract().body().asString());
assertEquals(
"Property \"name\" not found on the base object \"java.lang.String\" in expression {bar.name} in template bar on line 1",
"Property \"name\" not found on the base object \"java.lang.String\" in expression {bar.name} in template bar.html on line 1",
RestAssured.get("test-bar").then().statusCode(200).extract().body().asString());
}

Expand Down
13 changes: 9 additions & 4 deletions ...ployment/src/test/java/io/quarkus/qute/deployment/inject/InjectNamespaceResolverTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

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

import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.LongAdder;

import javax.annotation.PreDestroy;
Expand Down Expand Up @@ -34,12 +35,12 @@ public class InjectNamespaceResolverTest {

@Test
public void testInjection() {
assertEquals("pong != simple and pong != simple", foo.render());
assertEquals(2, SimpleBean.DESTROYS.longValue());
assertEquals("pong != simple1 and pong != simple1", foo.render());
assertEquals(1, SimpleBean.DESTROYS.longValue());

// Test the convenient Qute class
// By default, the content type is plain text
assertEquals("pong::<br>", Qute.fmt("{cdi:hello.ping}::{}", "<br>"));
assertEquals("pong::simple2::<br>", Qute.fmt("{cdi:hello.ping}::{cdi:simple.ping}::{}", "<br>"));
assertEquals("pong::&lt;br&gt;",
Qute.fmt("{cdi:hello.ping}::{newLine}").contentType("text/html").data("newLine", "<br>").render());
}
Expand All @@ -48,10 +49,14 @@ public void testInjection() {
@Dependent
public static class SimpleBean {

static final AtomicInteger COUNTER = new AtomicInteger();

static final LongAdder DESTROYS = new LongAdder();

private final int id = COUNTER.incrementAndGet();

public String ping() {
return "simple";
return "simple" + id;
}

@PreDestroy
Expand Down
151 changes: 137 additions & 14 deletions extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/EngineProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,19 @@
import java.net.URL;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.function.Function;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.regex.Pattern;

import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.context.Dependent;
import javax.enterprise.event.Event;
import javax.enterprise.event.Observes;
import javax.enterprise.inject.Produces;
Expand All @@ -23,17 +29,22 @@
import org.jboss.logging.Logger;

import io.quarkus.arc.Arc;
import io.quarkus.arc.ArcContainer;
import io.quarkus.arc.InjectableBean;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.qute.Engine;
import io.quarkus.qute.EngineBuilder;
import io.quarkus.qute.EvalContext;
import io.quarkus.qute.Expression;
import io.quarkus.qute.HtmlEscaper;
import io.quarkus.qute.NamespaceResolver;
import io.quarkus.qute.Qute;
import io.quarkus.qute.ReflectionValueResolver;
import io.quarkus.qute.Resolver;
import io.quarkus.qute.Results;
import io.quarkus.qute.Template;
import io.quarkus.qute.TemplateInstance;
import io.quarkus.qute.TemplateInstance.Initializer;
import io.quarkus.qute.TemplateLocator.TemplateLocation;
import io.quarkus.qute.UserTagSectionHelper;
import io.quarkus.qute.ValueResolver;
Expand All @@ -51,6 +62,7 @@ public class EngineProducer {

public static final String INJECT_NAMESPACE = "inject";
public static final String CDI_NAMESPACE = "cdi";
public static final String DEPENDENT_INSTANCES = "q_dep_inst";

private static final String TAGS = "tags/";

Expand All @@ -65,6 +77,7 @@ public class EngineProducer {
private final Pattern templatePathExclude;
private final Locale defaultLocale;
private final Charset defaultCharset;
private final ArcContainer container;

public EngineProducer(QuteContext context, QuteConfig config, QuteRuntimeConfig runtimeConfig,
Event<EngineBuilder> builderReady, Event<Engine> engineReady, ContentTypes contentTypes, LaunchMode launchMode,
Expand All @@ -77,6 +90,7 @@ public EngineProducer(QuteContext context, QuteConfig config, QuteRuntimeConfig
this.templatePathExclude = config.templatePathExclude;
this.defaultLocale = locales.defaultLocale;
this.defaultCharset = config.defaultCharset;
this.container = Arc.container();

LOGGER.debugf("Initializing Qute [templates: %s, tags: %s, resolvers: %s", context.getTemplatePaths(), tags,
context.getResolverClasses());
Expand Down Expand Up @@ -146,17 +160,8 @@ public EngineProducer(QuteContext context, QuteConfig config, QuteRuntimeConfig
builderReady.fire(builder);

// Resolve @Named beans
Function<EvalContext, Object> cdiFun = new Function<EvalContext, Object>() {

@Override
public Object apply(EvalContext ctx) {
try (InstanceHandle<Object> bean = Arc.container().instance(ctx.getName())) {
return bean.isAvailable() ? bean.get() : Results.NotFound.from(ctx);
}
}
};
builder.addNamespaceResolver(NamespaceResolver.builder(INJECT_NAMESPACE).resolve(cdiFun).build());
builder.addNamespaceResolver(NamespaceResolver.builder(CDI_NAMESPACE).resolve(cdiFun).build());
builder.addNamespaceResolver(NamespaceResolver.builder(INJECT_NAMESPACE).resolve(this::resolveInject).build());
builder.addNamespaceResolver(NamespaceResolver.builder(CDI_NAMESPACE).resolve(this::resolveInject).build());

// Add generated resolvers
for (String resolverClass : context.getResolverClasses()) {
Expand Down Expand Up @@ -187,15 +192,68 @@ public Object apply(EvalContext ctx) {
builder.addTemplateInstanceInitializer(createInitializer(initializerClass));
}

// Add a special initializer for templates that contain a inject/cdi namespace expressions
Map<String, Boolean> discoveredInjectTemplates = new HashMap<>();
builder.addTemplateInstanceInitializer(new Initializer() {

@Override
public void accept(TemplateInstance instance) {
Boolean hasInject = discoveredInjectTemplates.get(instance.getTemplate().getGeneratedId());
if (hasInject == null) {
hasInject = hasInjectExpression(instance.getTemplate());
}
if (hasInject) {
// Add dependent beans map if the template contains a cdi namespace expression
instance.setAttribute(DEPENDENT_INSTANCES, new ConcurrentHashMap<>());
// Add a close action to destroy all dependent beans
instance.onRendered(new Runnable() {
@Override
public void run() {
Object dependentInstances = instance.getAttribute(EngineProducer.DEPENDENT_INSTANCES);
if (dependentInstances != null) {
@SuppressWarnings("unchecked")
ConcurrentMap<String, InstanceHandle<?>> existing = (ConcurrentMap<String, InstanceHandle<?>>) dependentInstances;
for (InstanceHandle<?> handle : existing.values()) {
handle.close();
}
}
}
});
}
}
});

builder.timeout(runtimeConfig.timeout);
builder.useAsyncTimeout(runtimeConfig.useAsyncTimeout);

engine = builder.build();

// Load discovered templates
// Load discovered template files
Map<String, List<Template>> discovered = new HashMap<>();
for (String path : context.getTemplatePaths()) {
engine.getTemplate(path);
Template template = engine.getTemplate(path);
if (template != null) {
for (String suffix : config.suffixes) {
if (path.endsWith(suffix)) {
String pathNoSuffix = path.substring(0, path.length() - (suffix.length() + 1));
List<Template> templates = discovered.get(pathNoSuffix);
if (templates == null) {
templates = new ArrayList<>();
discovered.put(pathNoSuffix, templates);
}
templates.add(template);
break;
}
}
discoveredInjectTemplates.put(template.getGeneratedId(), hasInjectExpression(template));
}
}
// If it's a default suffix then register a path without suffix as well
// hello.html -> hello, hello.html
for (Entry<String, List<Template>> e : discovered.entrySet()) {
processDefaultTemplate(e.getKey(), e.getValue(), config, engine);
}

engineReady.fire(engine);

// Set the engine instance
Expand Down Expand Up @@ -291,6 +349,71 @@ Variant createVariant(String path) {
return new Variant(defaultLocale, defaultCharset, contentType);
}

private Object resolveInject(EvalContext ctx) {
InjectableBean<?> bean = container.namedBean(ctx.getName());
if (bean != null) {
if (bean.getScope().equals(Dependent.class)) {
// Dependent beans are shared for a template instance
Object dependentInstances = ctx.getAttribute(EngineProducer.DEPENDENT_INSTANCES);
if (dependentInstances != null) {
@SuppressWarnings("unchecked")
ConcurrentMap<String, InstanceHandle<?>> existing = (ConcurrentMap<String, InstanceHandle<?>>) dependentInstances;
InstanceHandle<?> handle = existing.get(ctx.getName());
if (handle == null) {
handle = container.instance(bean);
existing.put(ctx.getName(), handle);
}
return handle.get();
}
}
return container.instance(bean).get();
}
return Results.NotFound.from(ctx);
}

private boolean hasInjectExpression(Template template) {
for (Expression expression : template.getExpressions()) {
if (isInjectExpression(expression)) {
return true;
}
}
return false;
}

private boolean isInjectExpression(Expression expression) {
String namespace = expression.getNamespace();
if (namespace != null && (CDI_NAMESPACE.equals(namespace) || INJECT_NAMESPACE.equals(namespace))) {
return true;
}
for (Expression.Part part : expression.getParts()) {
if (part.isVirtualMethod()) {
for (Expression param : part.asVirtualMethod().getParameters()) {
if (param.isLiteral()) {
continue;
}
if (isInjectExpression(param)) {
return true;
}
}
}
}
return false;
}

private void processDefaultTemplate(String path, List<Template> templates, QuteConfig config, Engine engine) {
if (engine.isTemplateLoaded(path)) {
return;
}
for (String suffix : config.suffixes) {
for (Template template : templates) {
if (template.getId().endsWith(suffix)) {
engine.putTemplate(path, template);
return;
}
}
}
}

static class ResourceTemplateLocation implements TemplateLocation {

private final URL resource;
Expand Down
13 changes: 13 additions & 0 deletions extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/TemplateProducer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,11 @@ public Optional<Variant> getVariant() {
throw new UnsupportedOperationException("Injected templates do not support getVariant()");
}

@Override
public String getId() {
throw new UnsupportedOperationException("Injected templates do not support getId()");
}

@Override
public String toString() {
return "Injectable template [path=" + path + "]";
Expand Down Expand Up @@ -170,6 +175,11 @@ protected Engine engine() {
return engine;
}

@Override
public Template getTemplate() {
return template();
}

private TemplateInstance templateInstance() {
TemplateInstance instance = template().instance();
if (dataMap != null) {
Expand All @@ -180,6 +190,9 @@ private TemplateInstance templateInstance() {
if (!attributes.isEmpty()) {
attributes.forEach(instance::setAttribute);
}
if (!renderedActions.isEmpty()) {
renderedActions.forEach(instance::onRendered);
}
return instance;
}

Expand Down
8 changes: 4 additions & 4 deletions independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ Collection<Resource> generateSyntheticBean(BeanInfo bean) {

implementGetIdentifier(bean, beanCreator);
implementSupplierGet(beanCreator);
if (!bean.hasDefaultDestroy()) {
if (bean.hasDestroyLogic()) {
implementDestroy(bean, beanCreator, providerType, Collections.emptyMap(), isApplicationClass, baseName);
}
implementCreate(classOutput, beanCreator, bean, providerType, baseName,
Expand Down Expand Up @@ -308,7 +308,7 @@ Collection<Resource> generateClassBean(BeanInfo bean, ClassInfo beanClass) {

implementGetIdentifier(bean, beanCreator);
implementSupplierGet(beanCreator);
if (!bean.hasDefaultDestroy()) {
if (bean.hasDestroyLogic()) {
implementDestroy(bean, beanCreator, providerType, injectionPointToProviderSupplierField, isApplicationClass,
baseName);
}
Expand Down Expand Up @@ -412,7 +412,7 @@ Collection<Resource> generateProducerMethodBean(BeanInfo bean, MethodInfo produc

implementGetIdentifier(bean, beanCreator);
implementSupplierGet(beanCreator);
if (!bean.hasDefaultDestroy()) {
if (bean.hasDestroyLogic()) {
implementDestroy(bean, beanCreator, providerType, injectionPointToProviderField, isApplicationClass, baseName);
}
implementCreate(classOutput, beanCreator, bean, providerType, baseName,
Expand Down Expand Up @@ -501,7 +501,7 @@ Collection<Resource> generateProducerFieldBean(BeanInfo bean, FieldInfo producer

implementGetIdentifier(bean, beanCreator);
implementSupplierGet(beanCreator);
if (!bean.hasDefaultDestroy()) {
if (bean.hasDestroyLogic()) {
implementDestroy(bean, beanCreator, providerType, null, isApplicationClass, baseName);
}
implementCreate(classOutput, beanCreator, bean, providerType, baseName,
Expand Down
23 changes: 14 additions & 9 deletions independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -337,17 +337,22 @@ boolean isSubclassRequired() {
|| lifecycleInterceptors.containsKey(InterceptionType.PRE_DESTROY);
}

boolean hasDefaultDestroy() {
if (isInterceptor()) {
return true;
/**
*
* @return {@code true} if the bean requires some customized destroy logic
*/
public boolean hasDestroyLogic() {
if (isInterceptor() || isDecorator()) {
return false;
}
if (isClassBean()) {
return getLifecycleInterceptors(InterceptionType.PRE_DESTROY).isEmpty()
&& Beans.getCallbacks(target.get().asClass(), DotNames.PRE_DESTROY, beanDeployment.getBeanArchiveIndex())
.isEmpty();
} else {
return disposer == null && destroyerConsumer == null;
if (disposer != null || destroyerConsumer != null) {
// producer with disposer or custom bean with destruction logic
return true;
}
// test class bean with @PreDestroy interceptor or callback
return isClassBean() && (!getLifecycleInterceptors(InterceptionType.PRE_DESTROY).isEmpty()
|| !Beans.getCallbacks(target.get().asClass(), DotNames.PRE_DESTROY, beanDeployment.getBeanArchiveIndex())
.isEmpty());
}

public boolean isForceApplicationClass() {
Expand Down
Loading

0 comments on commit 5c11996

Please sign in to comment.