From 44d28dc8488c284b703fe6c75e598a3168d2a0b3 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Mon, 17 Jan 2022 10:20:08 +0100 Subject: [PATCH] Template extensions - always propagate errors when evaluating paramers - resolves #22931 --- docs/src/main/asciidoc/qute-reference.adoc | 4 +- .../TimeTemplateExtensionsTest.java | 19 +++++++ .../extensions/TimeTemplateExtensions.java | 16 ++++-- .../java/io/quarkus/qute/CompletedStage.java | 4 ++ .../java/io/quarkus/qute/EvaluatedParams.java | 27 ++++++--- .../io/quarkus/qute/EvaluatedParamsTest.java | 55 +++++++++++++++++++ 6 files changed, 109 insertions(+), 16 deletions(-) diff --git a/docs/src/main/asciidoc/qute-reference.adoc b/docs/src/main/asciidoc/qute-reference.adoc index 9a944aca42b72..a71f33d0509ff 100644 --- a/docs/src/main/asciidoc/qute-reference.adoc +++ b/docs/src/main/asciidoc/qute-reference.adoc @@ -1572,7 +1572,7 @@ TIP: A list element can be accessed directly: `{list.10}` or `{list[10]}`. * `mod`: Modulo operation ** `{#if counter.mod(5) == 0}` -==== Strings +===== Strings * `fmt` or `format`: format the string instance via `java.lang.String.format()` ** `{myStr.fmt("arg1","arg2")}` @@ -1616,7 +1616,7 @@ TIP: A list element can be accessed directly: `{list.10}` or `{list[10]}`. ** `{time:format(myDate,'d MMM uuuu', myLocale)}` * `time:format(dateTime,pattern,locale,timeZone)`: Formats temporal objects from the `java.time` package, `java.util.Date`, `java.util.Calendar` and `java.lang.Number` -** `{time:format(myDate,'d MMM uuuu',myTimeZoneId)}` +** `{time:format(myDate,'d MMM uuuu',myLocale,myTimeZoneId)}` [[template_data]] === @TemplateData diff --git a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/extensions/TimeTemplateExtensionsTest.java b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/extensions/TimeTemplateExtensionsTest.java index 40e56e1ef05ea..073d509cdf07b 100644 --- a/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/extensions/TimeTemplateExtensionsTest.java +++ b/extensions/qute/deployment/src/test/java/io/quarkus/qute/deployment/extensions/TimeTemplateExtensionsTest.java @@ -1,12 +1,15 @@ package io.quarkus.qute.deployment.extensions; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; import java.time.LocalDate; import java.time.LocalDateTime; import java.util.Calendar; import java.util.Date; import java.util.Locale; +import java.util.Map; import javax.inject.Inject; @@ -14,7 +17,9 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import io.quarkus.qute.Engine; import io.quarkus.qute.Template; +import io.quarkus.qute.TemplateException; import io.quarkus.test.QuarkusUnitTest; public class TimeTemplateExtensionsTest { @@ -29,6 +34,9 @@ public class TimeTemplateExtensionsTest { @Inject Template foo; + @Inject + Engine engine; + @Test public void testFormat() { Calendar nowCal = Calendar.getInstance(); @@ -44,4 +52,15 @@ public void testFormat() { .data("myLocale", Locale.ENGLISH).render()); } + @Test + public void testInvalidParameter() { + try { + // input.birthday cannot be resolved + engine.parse("{time:format(input.birthday, 'uuuu')}").data("input", Map.of("name", "Quarkus Qute")).render(); + fail(); + } catch (TemplateException expected) { + assertTrue(expected.getMessage().startsWith("Property \"birthday\" not found on the base object")); + } + } + } diff --git a/extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/extensions/TimeTemplateExtensions.java b/extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/extensions/TimeTemplateExtensions.java index d24d7612f12a7..b8a10e73b5ca8 100644 --- a/extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/extensions/TimeTemplateExtensions.java +++ b/extensions/qute/runtime/src/main/java/io/quarkus/qute/runtime/extensions/TimeTemplateExtensions.java @@ -23,6 +23,10 @@ public class TimeTemplateExtensions { private static final Map FORMATTER_CACHE = new ConcurrentHashMap<>(); + public static void clearCache() { + FORMATTER_CACHE.clear(); + } + static String format(TemporalAccessor temporal, String pattern) { return FORMATTER_CACHE.computeIfAbsent(new Key(pattern, null, null), TimeTemplateExtensions::formatterForKey) .format(temporal); @@ -89,21 +93,23 @@ static final class Key { private final String pattern; private final Locale locale; private final ZoneId timeZone; + private final int hashCode; public Key(String pattern, Locale locale, ZoneId timeZone) { this.pattern = pattern; this.locale = locale; this.timeZone = timeZone; - } - - @Override - public int hashCode() { final int prime = 31; int result = 1; result = prime * result + ((locale == null) ? 0 : locale.hashCode()); result = prime * result + ((pattern == null) ? 0 : pattern.hashCode()); result = prime * result + ((timeZone == null) ? 0 : timeZone.hashCode()); - return result; + this.hashCode = result; + } + + @Override + public int hashCode() { + return hashCode; } @Override diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/CompletedStage.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/CompletedStage.java index 1c6bd2df9a2f0..4c961a79ad032 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/CompletedStage.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/CompletedStage.java @@ -38,6 +38,10 @@ private CompletedStage(T result, Throwable exception) { this.exception = exception; } + public boolean isFailure() { + return exception != null; + } + public T get() { if (exception != null) { // Throw an exception if completed exceptionally diff --git a/independent-projects/qute/core/src/main/java/io/quarkus/qute/EvaluatedParams.java b/independent-projects/qute/core/src/main/java/io/quarkus/qute/EvaluatedParams.java index 5475712d33d10..dfd87cc9286e7 100644 --- a/independent-projects/qute/core/src/main/java/io/quarkus/qute/EvaluatedParams.java +++ b/independent-projects/qute/core/src/main/java/io/quarkus/qute/EvaluatedParams.java @@ -9,7 +9,6 @@ import java.util.concurrent.ExecutionException; import java.util.function.Supplier; -@SuppressWarnings("rawtypes") public final class EvaluatedParams { static final EvaluatedParams EMPTY = new EvaluatedParams(CompletedStage.VOID, new Supplier[0]); @@ -29,12 +28,17 @@ public static EvaluatedParams evaluate(EvalContext context) { Supplier[] allResults = new Supplier[params.size()]; List> asyncResults = null; int i = 0; + CompletedStage failure = null; Iterator it = params.iterator(); while (it.hasNext()) { Expression expression = it.next(); CompletionStage result = context.evaluate(expression); if (result instanceof CompletedStage) { - allResults[i++] = (CompletedStage) result; + CompletedStage completed = (CompletedStage) result; + allResults[i++] = completed; + if (completed.isFailure()) { + failure = completed; + } // No async computation needed continue; } else { @@ -48,7 +52,7 @@ public static EvaluatedParams evaluate(EvalContext context) { } CompletionStage cs; if (asyncResults == null) { - cs = CompletedStage.VOID; + cs = failure != null ? failure : CompletedStage.VOID; } else if (asyncResults.size() == 1) { cs = asyncResults.get(0); } else { @@ -71,18 +75,23 @@ public static EvaluatedParams evaluateMessageParams(EvalContext context) { return EMPTY; } Supplier[] allResults = new Supplier[params.size()]; - List> asyncResults = null; + List> asyncResults = null; int i = 0; + CompletedStage failure = null; Iterator it = params.subList(1, params.size()).iterator(); while (it.hasNext()) { - CompletionStage result = context.evaluate(it.next()); + CompletionStage result = context.evaluate(it.next()); if (result instanceof CompletedStage) { - allResults[i++] = (CompletedStage) result; + CompletedStage completed = (CompletedStage) result; + allResults[i++] = completed; + if (completed.isFailure()) { + failure = completed; + } // No async computation needed continue; } else { - CompletableFuture fu = result.toCompletableFuture(); + CompletableFuture fu = result.toCompletableFuture(); if (asyncResults == null) { asyncResults = new ArrayList<>(); } @@ -92,7 +101,7 @@ public static EvaluatedParams evaluateMessageParams(EvalContext context) { } CompletionStage cs; if (asyncResults == null) { - cs = CompletedStage.VOID; + cs = failure != null ? failure : CompletedStage.VOID; } else if (asyncResults.size() == 1) { cs = asyncResults.get(0); } else { @@ -107,7 +116,7 @@ public static EvaluatedParams evaluateMessageParams(EvalContext context) { EvaluatedParams(CompletionStage stage) { this.stage = stage; if (stage instanceof CompletedStage) { - this.results = new Supplier[] { (CompletedStage) stage }; + this.results = new Supplier[] { (CompletedStage) stage }; } else { this.results = new Supplier[] { Futures.toSupplier(stage.toCompletableFuture()) }; } diff --git a/independent-projects/qute/core/src/test/java/io/quarkus/qute/EvaluatedParamsTest.java b/independent-projects/qute/core/src/test/java/io/quarkus/qute/EvaluatedParamsTest.java index a1e7a9e361941..4aee4e596d253 100644 --- a/independent-projects/qute/core/src/test/java/io/quarkus/qute/EvaluatedParamsTest.java +++ b/independent-projects/qute/core/src/test/java/io/quarkus/qute/EvaluatedParamsTest.java @@ -2,8 +2,11 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; +import java.util.List; import java.util.Locale; +import java.util.concurrent.CompletionStage; import java.util.concurrent.ExecutionException; import java.util.function.Supplier; import org.junit.jupiter.api.Test; @@ -32,7 +35,59 @@ public void testParameterTypesMatch() throws InterruptedException, ExecutionExce assertFalse(new EvaluatedParams(null, new Supplier[] { CompletedStage.of(10), CompletedStage.of("Foo") }) .parameterTypesMatch(false, new Class[] { Integer.class, Object[].class })); + } + + @Test + public void testInvalidParameter() { + EvaluatedParams params = EvaluatedParams.evaluate(new EvalContext() { + + @Override + public List getParams() { + Expression expr1 = ExpressionImpl.from("foo.bar"); + Expression expr2 = ExpressionImpl.from("bar.baz"); + return List.of(expr1, expr2); + } + + @Override + public String getName() { + return null; + } + + @Override + public Object getBase() { + return null; + } + + @Override + public Object getAttribute(String key) { + return null; + } + + @Override + public CompletionStage evaluate(Expression expression) { + if (expression.toOriginalString().equals("foo.bar")) { + return CompletedStage.of("foo"); + } else if (expression.toOriginalString().equals("bar.baz")) { + return CompletedStage.failure(new IllegalArgumentException()); + } + throw new IllegalStateException(); + } + @Override + public CompletionStage evaluate(String expression) { + return null; + } + }); + assertTrue(params.stage instanceof CompletedStage); + CompletedStage completed = (CompletedStage) params.stage; + assertTrue(completed.isFailure()); + try { + completed.get(); + fail(); + } catch (TemplateException expected) { + Throwable cause = expected.getCause(); + assertTrue(cause instanceof IllegalArgumentException); + } } }