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

Support complex expressions in Qute #6369

Open
gunnarmorling opened this issue Jan 1, 2020 · 20 comments
Open

Support complex expressions in Qute #6369

gunnarmorling opened this issue Jan 1, 2020 · 20 comments

Comments

@gunnarmorling
Copy link
Collaborator

@gunnarmorling gunnarmorling commented Jan 1, 2020

I'd like to use boolean operators like && and ||. Currently, when using an expression such as {update && todo.completed}, I'm getting a stacktrace like this (both update and todo are variables in the template context):

java.lang.IllegalArgumentException: Not a virtual method: &&(todo
	at io.quarkus.qute.Expressions.parseVirtualMethodParams(Expressions.java:32)
	at io.quarkus.qute.EvaluatorImpl$EvalContextImpl.<init>(EvaluatorImpl.java:118)
	at io.quarkus.qute.EvaluatorImpl.resolveReference(EvaluatorImpl.java:69)
	at io.quarkus.qute.EvaluatorImpl.lambda$resolveReference$1(EvaluatorImpl.java:72)
	at java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:981)
	at java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2124)
	at java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:110)
	at io.quarkus.qute.EvaluatorImpl.resolveReference(EvaluatorImpl.java:70)
	at io.quarkus.qute.EvaluatorImpl.evaluate(EvaluatorImpl.java:48)
	at io.quarkus.qute.ResolutionContextImpl.evaluate(ResolutionContextImpl.java:31)
	at io.quarkus.qute.ExpressionNode.resolve(ExpressionNode.java:25)
	at io.quarkus.qute.SectionNode$SectionResolutionContextImpl.execute(SectionNode.java:112)
	at io.quarkus.qute.SectionHelper$SectionResolutionContext.execute(SectionHelper.java:31)
	at io.quarkus.qute.Parser$1$1.resolve(Parser.java:67)
	at io.quarkus.qute.SectionNode.resolve(SectionNode.java:33)
	at io.quarkus.qute.TemplateImpl.renderData(TemplateImpl.java:92)
	at io.quarkus.qute.TemplateImpl.access$200(TemplateImpl.java:14)
	at io.quarkus.qute.TemplateImpl$TemplateInstanceImpl.renderAsync(TemplateImpl.java:73)
	at io.quarkus.resteasy.qute.runtime.TemplateResponseFilter.filter(TemplateResponseFilter.java:58)
	at org.jboss.resteasy.core.interception.jaxrs.ContainerResponseContextImpl.filter(ContainerResponseContextImpl.java:361)
	at org.jboss.resteasy.core.ServerResponseWriter.executeFilters(ServerResponseWriter.java:232)
	at org.jboss.resteasy.core.ServerResponseWriter.writeNomapResponse(ServerResponseWriter.java:97)
	at org.jboss.resteasy.core.ServerResponseWriter.writeNomapResponse(ServerResponseWriter.java:70)
	at org.jboss.resteasy.core.SynchronousDispatcher.writeResponse(SynchronousDispatcher.java:578)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:508)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$invoke$4(SynchronousDispatcher.java:252)
	at org.jboss.resteasy.core.SynchronousDispatcher.lambda$preprocess$0(SynchronousDispatcher.java:153)
	at org.jboss.resteasy.core.interception.jaxrs.PreMatchContainerRequestContext.filter(PreMatchContainerRequestContext.java:363)
	at org.jboss.resteasy.core.SynchronousDispatcher.preprocess(SynchronousDispatcher.java:156)
	at org.jboss.resteasy.core.SynchronousDispatcher.invoke(SynchronousDispatcher.java:238)
	at io.quarkus.resteasy.runtime.standalone.RequestDispatcher.service(RequestDispatcher.java:73)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.dispatch(VertxRequestHandler.java:120)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler.access$000(VertxRequestHandler.java:36)
	at io.quarkus.resteasy.runtime.standalone.VertxRequestHandler$1.run(VertxRequestHandler.java:85)
	at io.quarkus.runtime.CleanableExecutor$CleaningRunnable.run(CleanableExecutor.java:224)
	at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at org.jboss.threads.ContextClassLoaderSavingRunnable.run(ContextClassLoaderSavingRunnable.java:35)
	at org.jboss.threads.EnhancedQueueExecutor.safeRun(EnhancedQueueExecutor.java:2011)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.doRunTask(EnhancedQueueExecutor.java:1535)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1426)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:29)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:29)
	at java.lang.Thread.run(Thread.java:748)
	at org.jboss.threads.JBossThread.run(JBossThread.java:479)

Interestingly, behavior is different when using an if:

{#if update && todo.completed}
...
{/if}

In this case no exception is raised but it seems the second operand is silently ignored.

@mkouba

This comment has been minimized.

Copy link
Contributor

@mkouba mkouba commented Jan 3, 2020

Neither of these is supported ATM (and it's documented ;-). I personally tend to avoid complex expressions in templates (as it usually leads to more error prone code) and follow the idea of templates with minimal logic. However, the {#if update && todo.completed} use case makes sense and we'll need to support it sooner or later.

@gsmet

This comment has been minimized.

Copy link
Member

@gsmet gsmet commented Jan 3, 2020

FWIW, we also need some expression language support in the Spring Security extension. /cc @geoand

It might be worth it to use the same implementation.

For the record, Hibernate Validator uses Jakarta EL.

@geoand

This comment has been minimized.

Copy link
Contributor

@geoand geoand commented Jan 3, 2020

Yeah I had to do a pretty lame implementation of a small subset of SpEl for the security stuff.
It would be nice if we had some kind of actual EL support we could use everywhere.

@mkouba

This comment has been minimized.

Copy link
Contributor

@mkouba mkouba commented Jan 3, 2020

So I don't want to repeat myself but I really don't think it's a good idea to build template expressions on top of a complex EL by default, of course it could be optional... ;-)

@gsmet

This comment has been minimized.

Copy link
Member

@gsmet gsmet commented Jan 3, 2020

@mkouba I don't think we want to push you to do what you don't feel right, just wanted to say that there is a common need for this sort of things.

@gunnarmorling

This comment has been minimized.

Copy link
Collaborator Author

@gunnarmorling gunnarmorling commented Jan 3, 2020

Complexity surely is in the eye of the beholder, but boolean expressions with && or || are fairly common in template logic to conditionally display contents. I also was missing method calls, in particular equals(), as otherwise string comparisons are impossible.

@gsmet

This comment has been minimized.

Copy link
Member

@gsmet gsmet commented Jan 3, 2020

Yeah, my intuition is that using an existing EL implementation will be easier than implementing our own EL implementation/parser rules but I'll let @mkouba and @geoand decide.

I agree that keeping the templates simple is a good thing but I also foresee we will at least need some advanced features.

@dmlloyd

This comment has been minimized.

Copy link
Member

@dmlloyd dmlloyd commented Jan 3, 2020

I'm not sure an EL makes sense when you've already got a template language: then you have two languages, possibly with overlap, certainly with combinatorial effects... Wouldn't it be better to define one language with a complete grammar and predictable behaviors?

@gsmet

This comment has been minimized.

Copy link
Member

@gsmet gsmet commented Jan 3, 2020

Well, if included in the template engine, at least let's make the syntax consistent with what we will use for Spring Security where we definitely need an EL.

@geoand

This comment has been minimized.

Copy link
Contributor

@geoand geoand commented Jan 3, 2020

From my PoV, if we define the minimum common set of expressions we could reuse everywhere, it would be great.
I don't think anyone here wants to have multiple mini ELs all over the place

@nimo23

This comment has been minimized.

Copy link

@nimo23 nimo23 commented Jan 3, 2020

It would be a good idea to provide Jakarta EL as the primary choice EL language in Quarkus. I understand that Qute is optimized for build time but why not provide a quarkus extension on top of Jakarta EL which is also optimized for build time? With this, there is no need for Qute EL and can be removed from Quarkus codebase.

@dmlloyd

This comment has been minimized.

Copy link
Member

@dmlloyd dmlloyd commented Jan 4, 2020

Qute is not an EL, it's a template language which also supports expressions. The choice of expression language should be a technical decision, not a political one: it seems possible that Jakarta EL is not going to be the right fit.

@nimo23

This comment has been minimized.

Copy link

@nimo23 nimo23 commented Jan 4, 2020

Ok, so Qute is something like Velocity or Freemarker but optimized for quarkus runtime. The Jakarta EL is primarly used in JSF but can also be used standalone.

I dont know why Jakarta EL is not the right fit for Qute but there will be definitly a need for more advanced expressions in the future because we see it in the use cases of velocity or freemarker or jsf. There is a possibility that Jakarta EL will be used in future for other things in quarkus (for example, if someone provides a jsf-extension).

Would something like quarkus-el-extension (backed by jakarta el) used by quarkus-qute-extension (where the qute template engine uses the decoupled EL as expression language) make it possible to be a better fit? I mean it is the same what JSF/Facelets, Hibernate, etc does: uses a decoupled expression language (Jakarta EL) for its templating

@mkouba

This comment has been minimized.

Copy link
Contributor

@mkouba mkouba commented Jan 6, 2020

I also was missing method calls, in particular equals(),

"Method calls" are supported but not available out of the box. You can use @TemplateData or Template Extension Methods.

as otherwise string comparisons are impossible

{#if foo.name is 'bar'}, {#if foo.name eq bar.name} and {#if foo.name == 'bar'} - all these should work.

@gunnarmorling FYI ;-)

@geoand

This comment has been minimized.

Copy link
Contributor

@geoand geoand commented Jan 6, 2020

{#if foo.name is 'bar'}, {#if foo.name eq bar.name} and {#if foo.name == 'bar'} - all these should work.

That's very similar to what I have in @PreAuthorize for Spring Security :)

@FroMage

This comment has been minimized.

Copy link
Member

@FroMage FroMage commented Jan 6, 2020

"Method calls" are supported but not available out of the box

Didn't we want to auto-register methods for classes we know are used in the template thanks to typing info?

I definitely vote for supporting boolean, math and comparison expressions.

@gunnarmorling

This comment has been minimized.

Copy link
Collaborator Author

@gunnarmorling gunnarmorling commented Jan 6, 2020

IMO a strict subset of Unified EL would be good, if we can or don't want to support the full thing. This would reduce friction when coming from or combining Qute and Unified EL in a project, e.g. when using HV.

@mkouba

This comment has been minimized.

Copy link
Contributor

@mkouba mkouba commented Jan 6, 2020

Didn't we want to auto-register methods for classes we know are used in the template thanks to typing info?

That's the plan. But so far we only support "properties" because it's much simpler to implement.

@gunnarmorling

This comment has been minimized.

Copy link
Collaborator Author

@gunnarmorling gunnarmorling commented Jan 10, 2020

A related issue was documented in a comment here:

The bean I use in my template contains a java.util.Map and I need to test, if a certain value is contained in this map.

@mkouba

This comment has been minimized.

Copy link
Contributor

@mkouba mkouba commented Jan 28, 2020

FYI we already support a bit more complex expressions in 1.2.0, for example:

  • {#if !item.active || (item.sold && item.price == 100)}
  • {#if item.age > 10 && item.price > 500}
  • {#if (item.age > 10 || item.price > 500) && user.loggedIn}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.