Qute - introduce strict rendering #18227

mkouba · 2021-06-29T08:33:32Z

i.e. if any expression is evaluated to "not found" value a
TemplateException is thrown and rendering is aborted
it's enabled by default
it can be disabled via io.quarkus.qute.strict-rendering=false

mkouba · 2021-06-29T08:36:21Z

Note that this is a breaking change - the default behavior has changed. Type-safe templates are not affected though.

This PR is related to #18177.

mkouba · 2021-06-29T09:52:06Z

...starts/src/main/java/io/quarkus/devtools/codestarts/core/reader/QuteCodestartFileReader.java

@@ -50,6 +50,8 @@ public static String readQuteFile(CodestartResource projectResource, Source sour
        final Engine engine = Engine.builder().addDefaults()
                .addResultMapper(new MissingValueMapper())
                .removeStandaloneLines(true)
+                // TODO This needs to be revisited
+                .strictRendering(false)


FYI @ia3andy

so what cases is it that you see that will break here ?

I did not examine the failures but a lot of codestarts and devtools tests failed so it's definitely related. A quick scan example: "Property "parent" not found on base object "java.util.HashMap" in expression {parent} in template 1 on line 6" in theQuarkusExtensionCodestartGenerationTest.

interesting - now curious which of these are actual bugs.

should map lookups be considered valid or is expectation you do {obj.parent ? false} ?

sucks we didnt notice this before 2.0 :/

I'll need to take a look at the tests. But it can be a bug in the test too ;-).

So I checked the tests and the problematic pattern looks like {#if uberjar}<quarkus.package.type>uber-jar</quarkus.package.type>{/if}. Previously this didn't fail because "not found" was considered a falsy value. In this PR, we assume that "not found" cannot be evaluated correctly by default and thus the rendering fails. Note that null would be OK because it means that we found something...

With strict rendering enabled the template would have to use something like {#if uberjar.or(false)} (elvis operator would not work here because it's a param of a section) or even {#if containsKey('uberjar')} (because the data object is a manually created Map).

That said, I think it's ok to disable the strict rendering for situations like these. I'll let @ia3andy to decide later. For now, I will add a more explaining comment.

should map lookups be considered valid or is expectation you do {obj.parent ? false} ?

I don't think we should add a special handling for any type. That might be even more confusing.

FroMage · 2021-06-30T13:46:58Z

I'm not sure. What's the rationale for changing the default in the case of non-type-safe templates? Seems to me that they're not type-safe so why throw? 🤷

mkouba · 2021-06-30T15:03:03Z

I'm not sure. What's the rationale for changing the default in the case of non-type-safe templates? Seems to me that they're not type-safe so why throw? shrug

So the main motivation is to help users to spot the problems with typos and similar kind of errors. Of course, this change does not affect users of @CheckedTemplate but it could help those who can't/don't want to use them. Note that qute is also used "standalone" and in this mode no type-safe checks are supported.

For example, a template without type-safe checks: My hero names: {hero.nams} and there's public class Hero { public String names; }. Currently, this expression results in My hero names: NOT_FOUND by default. After this change, the rendering would fail with a message like Property "nams" not found in expression {hero.nams} in template hero.html on line 2 by default.

Note that before this PR it was possible to change the quarkus.qute.property-not-found-strategy to throw an exception in similar cases. However, this only worked for standalone expressions, not for expressions used in sections, e.g. #{if hero.isFamou}.

gastaldi

LGTM

maxandersen · 2021-06-30T17:21:37Z

To be clear my main ask is that qute stand-alone at least don't let obvious errors be ignored. Ie. Typos In if statements.

Should at least be possible to enable.

quarkus-bot · 2021-07-02T10:37:05Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `cabafef`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	⚠️ Check →	Logs	Raw logs

Full information is available in the Build summary check run.

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/vertx-http/deployment

✖ io.quarkus.vertx.http.testrunner.tags.ExcludeTagsTestCase.checkTestsAreRun line 73 - More details - Source on GitHub

mkouba · 2021-07-02T11:57:50Z

Hm, I can see the following failure in JVM Tests - JDK 16:

Caused by: org.apache.maven.plugin.MojoExecutionException: Deployment artifact io.quarkus:quarkus-resteasy-reactive-kotlin-deployment::jar:999-SNAPSHOT is missing the following dependencies from its configuration: io.quarkus:quarkus-smallrye-context-propagation-deployment::jar, io.quarkus:quarkus-vertx-core-deployment::jar, io.quarkus:quarkus-netty-deployment::jar, io.quarkus:quarkus-jsonp-deployment::jar

Does it ring a bell? @gsmet @geoand

geoand · 2021-07-02T12:03:01Z

Huh.... I don't see that in #18222 for example that was rebased onto the new Kotlin layout

quarkus-bot · 2021-07-02T15:46:21Z

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building `ecb7b62`

Status	Name	Step	Test failures	Logs	Raw logs
✖	JVM Tests - JDK 11	`Build`	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 11 Windows	`Build`	⚠️ Check →	Logs	Raw logs
✖	JVM Tests - JDK 16	`Build`	⚠️ Check →	Logs	Raw logs

aloubyansky · 2021-07-02T15:59:14Z

#18353 should fix the CI

- i.e. if any expression is evaluated to "not found" value a TemplateException is thrown and rendering is aborted - it's enabled by default - it can be disabled via io.quarkus.qute.strict-rendering=false

quarkus-bot bot added the area/qute The template engine label Jun 29, 2021

mkouba requested review from FroMage, gastaldi and maxandersen June 29, 2021 08:39

mkouba force-pushed the qute-strict-rendering branch from b9b07fd to 0a87ec6 Compare June 29, 2021 09:51

quarkus-bot bot added area/codestarts area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/platform Issues related to definition and interaction with Quarkus Platform labels Jun 29, 2021

mkouba commented Jun 29, 2021

View reviewed changes

mkouba added the release/breaking-change label Jun 29, 2021

mkouba force-pushed the qute-strict-rendering branch from 0a87ec6 to cdffad4 Compare June 29, 2021 12:53

gastaldi approved these changes Jun 30, 2021

View reviewed changes

mkouba force-pushed the qute-strict-rendering branch from cdffad4 to d6a771a Compare July 1, 2021 12:48

mkouba added this to the 2.1 - main milestone Jul 1, 2021

mkouba force-pushed the qute-strict-rendering branch 2 times, most recently from b679712 to cabafef Compare July 2, 2021 08:12

quarkusio deleted a comment from quarkus-bot bot Jul 2, 2021

mkouba force-pushed the qute-strict-rendering branch from cabafef to ecb7b62 Compare July 2, 2021 10:51

quarkusio deleted a comment from quarkus-bot bot Jul 2, 2021

Qute - introduce strict rendering

f76ba44

- i.e. if any expression is evaluated to "not found" value a TemplateException is thrown and rendering is aborted - it's enabled by default - it can be disabled via io.quarkus.qute.strict-rendering=false

mkouba force-pushed the qute-strict-rendering branch from ecb7b62 to f76ba44 Compare July 2, 2021 20:13

mkouba merged commit 8b3d6c2 into quarkusio:main Jul 3, 2021

maxandersen mentioned this pull request Jul 5, 2021

Take advantage of codestarts to allow creating a quarkus application as a maven module (WIP) #17170

Closed

gastaldi mentioned this pull request Jul 5, 2021

Accessing DevConsole fails with InternalServerError in the current snapshot #18413

Closed

phillip-kruger mentioned this pull request Jul 5, 2021

Dev UI: Add strictRendering=false to Qute engine #18416

Merged

mkouba mentioned this pull request Jul 9, 2021

standalone Qute has no meaning full resultmapper to force errors #18177

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Qute - introduce strict rendering #18227

Qute - introduce strict rendering #18227

mkouba commented Jun 29, 2021

mkouba commented Jun 29, 2021

mkouba Jun 29, 2021

ia3andy Jun 29, 2021

maxandersen Jun 29, 2021

mkouba Jun 30, 2021

maxandersen Jun 30, 2021

mkouba Jun 30, 2021

mkouba Jul 1, 2021

FroMage commented Jun 30, 2021

mkouba commented Jun 30, 2021

gastaldi left a comment

maxandersen commented Jun 30, 2021

quarkus-bot bot commented Jul 2, 2021 •

edited

Loading

Failing Jobs - Building `cabafef`

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/vertx-http/deployment

mkouba commented Jul 2, 2021

geoand commented Jul 2, 2021

quarkus-bot bot commented Jul 2, 2021 •

edited

Loading

Failing Jobs - Building `ecb7b62`

aloubyansky commented Jul 2, 2021

Qute - introduce strict rendering #18227

Qute - introduce strict rendering #18227

Conversation

mkouba commented Jun 29, 2021

mkouba commented Jun 29, 2021

mkouba Jun 29, 2021

Choose a reason for hiding this comment

ia3andy Jun 29, 2021

Choose a reason for hiding this comment

maxandersen Jun 29, 2021

Choose a reason for hiding this comment

mkouba Jun 30, 2021

Choose a reason for hiding this comment

maxandersen Jun 30, 2021

Choose a reason for hiding this comment

mkouba Jun 30, 2021

Choose a reason for hiding this comment

mkouba Jul 1, 2021

Choose a reason for hiding this comment

FroMage commented Jun 30, 2021

mkouba commented Jun 30, 2021

gastaldi left a comment

Choose a reason for hiding this comment

maxandersen commented Jun 30, 2021

quarkus-bot bot commented Jul 2, 2021 • edited Loading

Failing Jobs - Building cabafef

Test Failures

⚙️ JVM Tests - JDK 11 #

📦 extensions/vertx-http/deployment

mkouba commented Jul 2, 2021

geoand commented Jul 2, 2021

quarkus-bot bot commented Jul 2, 2021 • edited Loading

Failing Jobs - Building ecb7b62

aloubyansky commented Jul 2, 2021

quarkus-bot bot commented Jul 2, 2021 •

edited

Loading

Failing Jobs - Building `cabafef`

quarkus-bot bot commented Jul 2, 2021 •

edited

Loading

Failing Jobs - Building `ecb7b62`