Template engines are broken

jknack · jknack · commit 372430ad316d · 2024-10-19T11:05:29.000-03:00
- Add custom exception - Remove eager filtering so fails with better error message - fix #3562
diff --git a/jooby/src/main/java/io/jooby/ModelAndView.java b/jooby/src/main/java/io/jooby/ModelAndView.java
@@ -7,6 +7,8 @@
 
 import java.util.Locale;
 import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import edu.umd.cs.findbugs.annotations.NonNull;
 import edu.umd.cs.findbugs.annotations.Nullable;
@@ -19,6 +21,21 @@
  */
 public class ModelAndView<T> {
 
+  /** Thrown by template engine when they are not capable of rendering a {@link ModelAndView}. */
+  public static class UnsupportedModelAndView extends IllegalArgumentException {
+    /**
+     * Constructor.
+     *
+     * @param supported List of supported model implementation.
+     */
+    public UnsupportedModelAndView(Class<?>... supported) {
+      super(
+          "Only "
+              + Set.of(supported).stream().map(Class::getName).collect(Collectors.joining(", "))
+              + " are supported");
+    }
+  }
+
   /** View name. */
   private final String view;
 
diff --git a/jooby/src/main/java/io/jooby/TemplateEngine.java b/jooby/src/main/java/io/jooby/TemplateEngine.java
@@ -34,7 +34,7 @@ public interface TemplateEngine extends MessageEncoder {
    * @return Rendered template.
    * @throws Exception If something goes wrong.
    */
-  DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception;
+  DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception;
 
   @Override
   default DataBuffer encode(@NonNull Context ctx, @NonNull Object value) throws Exception {
diff --git a/modules/jooby-freemarker/src/main/java/io/jooby/freemarker/FreemarkerTemplateEngine.java b/modules/jooby-freemarker/src/main/java/io/jooby/freemarker/FreemarkerTemplateEngine.java
@@ -33,7 +33,7 @@ public List<String> extensions() {
   }
 
   @Override
-  public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception {
+  public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception {
     var buffer = ctx.getBufferFactory().allocateBuffer();
     var template = freemarker.getTemplate(modelAndView.getView());
     var writer = buffer.asWriter();
diff --git a/modules/jooby-handlebars/src/main/java/io/jooby/internal/handlebars/HandlebarsTemplateEngine.java b/modules/jooby-handlebars/src/main/java/io/jooby/internal/handlebars/HandlebarsTemplateEngine.java
@@ -35,7 +35,7 @@ public List<String> extensions() {
   }
 
   @Override
-  public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception {
+  public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception {
     var template = handlebars.compile(modelAndView.getView());
     var engineModel =
         com.github.jknack.handlebars.Context.newBuilder(modelAndView.getModel())
diff --git a/modules/jooby-jte/src/main/java/io/jooby/jte/JteTemplateEngine.java b/modules/jooby-jte/src/main/java/io/jooby/jte/JteTemplateEngine.java
@@ -32,7 +32,7 @@ public List<String> extensions() {
   }
 
   @Override
-  public DataBuffer render(Context ctx, ModelAndView modelAndView) {
+  public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) {
     var buffer = ctx.getBufferFactory().allocateBuffer();
     var output = new DataBufferOutput(buffer, StandardCharsets.UTF_8);
     var attributes = ctx.getAttributes();
diff --git a/modules/jooby-pebble/src/main/java/io/jooby/pebble/PebbleTemplateEngine.java b/modules/jooby-pebble/src/main/java/io/jooby/pebble/PebbleTemplateEngine.java
@@ -34,12 +34,7 @@ public List<String> extensions() {
   }
 
   @Override
-  public boolean supports(@NonNull ModelAndView modelAndView) {
-    return TemplateEngine.super.supports(modelAndView) && modelAndView instanceof MapModelAndView;
-  }
-
-  @Override
-  public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception {
+  public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception {
     if (modelAndView instanceof MapModelAndView mapModelAndView) {
       var buffer = ctx.getBufferFactory().allocateBuffer();
       var template = engine.getTemplate(modelAndView.getView());
@@ -52,8 +47,7 @@ public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exceptio
       template.evaluate(buffer.asWriter(), model, locale);
       return buffer;
     } else {
-      throw new IllegalArgumentException(
-          "Only " + MapModelAndView.class.getName() + " are supported");
+      throw new ModelAndView.UnsupportedModelAndView(MapModelAndView.class);
     }
   }
 }
diff --git a/modules/jooby-pebble/src/test/java/io/jooby/pebble/PebbleModuleTest.java b/modules/jooby-pebble/src/test/java/io/jooby/pebble/PebbleModuleTest.java
@@ -7,11 +7,13 @@
 
 import static java.util.Collections.singletonList;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Paths;
 import java.util.Collections;
 import java.util.Locale;
+import java.util.Map;
 
 import org.junit.jupiter.api.Test;
 
@@ -59,6 +61,19 @@ public void render() throws Exception {
     assertEquals("Hello foo bar var!", output.toString(StandardCharsets.UTF_8));
   }
 
+  @Test
+  public void shouldUnsupportedModelAndView() {
+    PebbleEngine.Builder builder =
+        PebbleModule.create()
+            .build(new Environment(getClass().getClassLoader(), ConfigFactory.empty()));
+    PebbleTemplateEngine engine =
+        new PebbleTemplateEngine(builder, Collections.singletonList(".peb"));
+    MockContext ctx = new MockContext();
+    assertThrows(
+        ModelAndView.UnsupportedModelAndView.class,
+        () -> engine.render(ctx, new ModelAndView<>("index.peb", Map.of("foo", "bar"))));
+  }
+
   @Test
   public void renderFileSystem() throws Exception {
     PebbleEngine.Builder builder =
diff --git a/modules/jooby-thymeleaf/src/main/java/io/jooby/internal/thymeleaf/ThymeleafTemplateEngine.java b/modules/jooby-thymeleaf/src/main/java/io/jooby/internal/thymeleaf/ThymeleafTemplateEngine.java
@@ -34,13 +34,7 @@ public List<String> extensions() {
   }
 
   @Override
-  public boolean supports(@NonNull ModelAndView modelAndView) {
-    return io.jooby.TemplateEngine.super.supports(modelAndView)
-        && modelAndView instanceof MapModelAndView;
-  }
-
-  @Override
-  public DataBuffer render(io.jooby.Context ctx, ModelAndView modelAndView) {
+  public @NonNull DataBuffer render(io.jooby.Context ctx, ModelAndView<?> modelAndView) {
     if (modelAndView instanceof MapModelAndView mapModelAndView) {
       Map<String, Object> model = new HashMap<>(ctx.getAttributes());
       model.putAll(mapModelAndView.getModel());
@@ -59,8 +53,7 @@ public DataBuffer render(io.jooby.Context ctx, ModelAndView modelAndView) {
       templateEngine.process(templateName, context, buffer.asWriter());
       return buffer;
     } else {
-      throw new IllegalArgumentException(
-          "Only " + MapModelAndView.class.getName() + " are supported");
+      throw new ModelAndView.UnsupportedModelAndView(MapModelAndView.class);
     }
   }
 }
diff --git a/tests/pom.xml b/tests/pom.xml
@@ -63,6 +63,11 @@
       <artifactId>jooby-handlebars</artifactId>
       <version>${jooby.version}</version>
     </dependency>
+    <dependency>
+      <groupId>io.jooby</groupId>
+      <artifactId>jooby-pebble</artifactId>
+      <version>${jooby.version}</version>
+    </dependency>
     <dependency>
       <groupId>io.jooby</groupId>
       <artifactId>jooby-freemarker</artifactId>
diff --git a/tests/src/test/java/io/jooby/i3562/Issue3562.java b/tests/src/test/java/io/jooby/i3562/Issue3562.java
@@ -0,0 +1,52 @@
+/*
+ * Jooby https://jooby.io
+ * Apache License Version 2.0 https://jooby.io/LICENSE.txt
+ * Copyright 2014 Edgar Espina
+ */
+package io.jooby.i3562;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import java.util.Map;
+
+import io.jooby.ModelAndView;
+import io.jooby.junit.ServerTest;
+import io.jooby.junit.ServerTestRunner;
+import io.jooby.pebble.PebbleModule;
+import io.jooby.thymeleaf.ThymeleafModule;
+
+public class Issue3562 {
+  @ServerTest
+  public void unsupportedModelAndView(ServerTestRunner runner) {
+    runner
+        .define(
+            app -> {
+              app.install(new ThymeleafModule());
+              app.install(new PebbleModule());
+
+              app.get(
+                  "/3562/thymeleaf",
+                  ctx -> new ModelAndView<>("index.html", Map.of("name", "thymeleaf")));
+              app.get(
+                  "/3562/pebble",
+                  ctx -> new ModelAndView<>("index.pebble", Map.of("name", "Pebble")));
+
+              app.error((ctx, cause, status) -> ctx.send(cause.getMessage()));
+            })
+        .ready(
+            client -> {
+              client.get(
+                  "/3562/thymeleaf",
+                  rsp -> {
+                    assertEquals(
+                        "Only io.jooby.MapModelAndView are supported", rsp.body().string().trim());
+                  });
+              client.get(
+                  "/3562/pebble",
+                  rsp -> {
+                    assertEquals(
+                        "Only io.jooby.MapModelAndView are supported", rsp.body().string().trim());
+                  });
+            });
+  }
+}
diff --git a/tests/src/test/java/io/jooby/test/TemplateEngineTest.java b/tests/src/test/java/io/jooby/test/TemplateEngineTest.java
@@ -12,6 +12,7 @@
 import io.jooby.handlebars.HandlebarsModule;
 import io.jooby.junit.ServerTest;
 import io.jooby.junit.ServerTestRunner;
+import io.jooby.pebble.PebbleModule;
 import io.jooby.thymeleaf.ThymeleafModule;
 
 public class TemplateEngineTest {
@@ -23,10 +24,12 @@ public void templateEngines(ServerTestRunner runner) {
               app.install(new ThymeleafModule());
               app.install(new HandlebarsModule());
               app.install(new FreemarkerModule());
+              app.install(new PebbleModule());
 
               app.get("/1", ctx -> ModelAndView.map("index.hbs").put("name", "Handlebars"));
               app.get("/2", ctx -> ModelAndView.map("index.ftl").put("name", "Freemarker"));
               app.get("/3", ctx -> ModelAndView.map("index.html").put("name", "Thymeleaf"));
+              app.get("/4", ctx -> ModelAndView.map("index.pebble").put("name", "Pebble"));
             })
         .ready(
             client -> {
@@ -54,6 +57,11 @@ public void templateEngines(ServerTestRunner runner) {
                             + "</html>",
                         rsp.body().string().replace("\r", "").trim());
                   });
+              client.get(
+                  "/4",
+                  rsp -> {
+                    assertEquals("Hello Pebble!", rsp.body().string().trim());
+                  });
             });
   }
 
diff --git a/tests/src/test/resources/views/index.pebble b/tests/src/test/resources/views/index.pebble
@@ -0,0 +1 @@
+Hello {{name}}!

Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ public List<String> extensions() {`
`33`	`33`	`}`
`34`	`34`
`35`	`35`	`@Override`
`36`		`- public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception {`
	`36`	`+ public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception {`
`37`	`37`	`var buffer = ctx.getBufferFactory().allocateBuffer();`
`38`	`38`	`var template = freemarker.getTemplate(modelAndView.getView());`
`39`	`39`	`var writer = buffer.asWriter();`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ public List<String> extensions() {`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`@Override`
`38`		`- public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception {`
	`38`	`+ public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception {`
`39`	`39`	`var template = handlebars.compile(modelAndView.getView());`
`40`	`40`	`var engineModel =`
`41`	`41`	`com.github.jknack.handlebars.Context.newBuilder(modelAndView.getModel())`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ public List<String> extensions() {`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`@Override`
`35`		`- public DataBuffer render(Context ctx, ModelAndView modelAndView) {`
	`35`	`+ public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) {`
`36`	`36`	`var buffer = ctx.getBufferFactory().allocateBuffer();`
`37`	`37`	`var output = new DataBufferOutput(buffer, StandardCharsets.UTF_8);`
`38`	`38`	`var attributes = ctx.getAttributes();`
Original file line number	Diff line number	Diff line change
`@@ -34,12 +34,7 @@ public List<String> extensions() {`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`@Override`
`37`		`- public boolean supports(@NonNull ModelAndView modelAndView) {`
`38`		`- return TemplateEngine.super.supports(modelAndView) && modelAndView instanceof MapModelAndView;`
`39`		`- }`
`40`		`-`
`41`		`- @Override`
`42`		`- public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exception {`
	`37`	`+ public DataBuffer render(Context ctx, ModelAndView<?> modelAndView) throws Exception {`
`43`	`38`	`if (modelAndView instanceof MapModelAndView mapModelAndView) {`
`44`	`39`	`var buffer = ctx.getBufferFactory().allocateBuffer();`
`45`	`40`	`var template = engine.getTemplate(modelAndView.getView());`
`@@ -52,8 +47,7 @@ public DataBuffer render(Context ctx, ModelAndView modelAndView) throws Exceptio`
`52`	`47`	`template.evaluate(buffer.asWriter(), model, locale);`
`53`	`48`	`return buffer;`
`54`	`49`	`} else {`
`55`		`- throw new IllegalArgumentException(`
`56`		`- "Only " + MapModelAndView.class.getName() + " are supported");`
	`50`	`+ throw new ModelAndView.UnsupportedModelAndView(MapModelAndView.class);`
`57`	`51`	`}`
`58`	`52`	`}`
`59`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -34,13 +34,7 @@ public List<String> extensions() {`
`34`	`34`	`}`
`35`	`35`
`36`	`36`	`@Override`
`37`		`- public boolean supports(@NonNull ModelAndView modelAndView) {`
`38`		`- return io.jooby.TemplateEngine.super.supports(modelAndView)`
`39`		`- && modelAndView instanceof MapModelAndView;`
`40`		`- }`
`41`		`-`
`42`		`- @Override`
`43`		`- public DataBuffer render(io.jooby.Context ctx, ModelAndView modelAndView) {`
	`37`	`+ public @NonNull DataBuffer render(io.jooby.Context ctx, ModelAndView<?> modelAndView) {`
`44`	`38`	`if (modelAndView instanceof MapModelAndView mapModelAndView) {`
`45`	`39`	`Map<String, Object> model = new HashMap<>(ctx.getAttributes());`
`46`	`40`	`model.putAll(mapModelAndView.getModel());`
`@@ -59,8 +53,7 @@ public DataBuffer render(io.jooby.Context ctx, ModelAndView modelAndView) {`
`59`	`53`	`templateEngine.process(templateName, context, buffer.asWriter());`
`60`	`54`	`return buffer;`
`61`	`55`	`} else {`
`62`		`- throw new IllegalArgumentException(`
`63`		`- "Only " + MapModelAndView.class.getName() + " are supported");`
	`56`	`+ throw new ModelAndView.UnsupportedModelAndView(MapModelAndView.class);`
`64`	`57`	`}`
`65`	`58`	`}`
`66`	`59`	`}`