Template engines are broken

- Add custom exception - Remove eager filtering so fails with better error message - fix #3562
jooby-project · Oct 19, 2024 · 372430a · 372430a
1 parent badd7ae
commit 372430a
Show file tree

Hide file tree

Showing 12 changed files with 106 additions and 21 deletions.
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/...jooby-handlebars/src/main/java/io/jooby/internal/handlebars/HandlebarsTemplateEngine.java b/...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/...es/jooby-thymeleaf/src/main/java/io/jooby/internal/thymeleaf/ThymeleafTemplateEngine.java b/...es/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}}!