Repository: PebbleTemplates/pebble
Pull request #475 (2019-09-11 01:15:47)

Fix issue of embed tag changing cached templates
Fix issue of embed tag changing cached templates and breaking includes using the same-named template from the cache.

For example, let's say `templateA.peb` embeds `templateX.peb`, while `templateB.peb` just includes it. If template caching is enabled, both the embed and include tag will use the same cached `PebbleTemplateImpl` object. But for the embed tag to work, it needed to change some of the data in that template, so that later on, when it's just included later, the modified template is being included.

By making a shallow copy of the cached template, the embed tag can make the changes it needs to without affecting what's already in the cache.

ebussieres: @cjbrooks12 Build is failling 
cjbrooks12: Build's fixed

Patch:
diff --git a/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java b/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
index a93458c0..ae0cc5dd 100644
--- a/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
+++ b/pebble/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
@@ -195,6 +195,19 @@ public class PebbleTemplateImpl implements PebbleTemplate {
         new ArrayList<>(), new HashMap<>(), scopeChain, null, this.engine.getEvaluationOptions());
   }
 
+  /**
+   * Return a shallow copy of this template.
+   *
+   * @return A new template instance with the same data
+   */
+  private PebbleTemplateImpl shallowCopy() {
+    PebbleTemplateImpl copy = new PebbleTemplateImpl(engine, rootNode, name);
+    copy.blocks.putAll(this.blocks);
+    copy.macros.putAll(this.macros);
+
+    return copy;
+  }
+
   /**
    * Imports a template.
    *
@@ -291,7 +304,12 @@ public class PebbleTemplateImpl implements PebbleTemplate {
   ) throws IOException {
     // get the template to embed
     String embeddedTemplateName = this.resolveRelativePath(name);
-    final PebbleTemplateImpl embeddedTemplate = (PebbleTemplateImpl) this.engine.getTemplate(embeddedTemplateName);
+
+    // make a shallow copy of the template so we can safely modify its blocks without affecting other templates in the
+    // template cache. Include and extend will use the same object from the cache, so we need to make sure embeds do not
+    // impact those other tags or change anything in the cache.
+    final PebbleTemplateImpl embeddedTemplate =
+            ((PebbleTemplateImpl) this.engine.getTemplate(embeddedTemplateName)).shallowCopy();
 
     // push a child scope based on the current scope
     context.scopedShallowWithoutInheritanceChain(embeddedTemplate, additionalVariables, (newContext) -> {


Test patch:
diff --git a/pebble/src/test/java/com/mitchellbosecke/pebble/EmbedCachingTagTest.java b/pebble/src/test/java/com/mitchellbosecke/pebble/EmbedCachingTagTest.java
new file mode 100644
index 00000000..28fa41b4
--- /dev/null
+++ b/pebble/src/test/java/com/mitchellbosecke/pebble/EmbedCachingTagTest.java
@@ -0,0 +1,71 @@
+package com.mitchellbosecke.pebble;
+
+import com.mitchellbosecke.pebble.error.PebbleException;
+import com.mitchellbosecke.pebble.loader.ClasspathLoader;
+import com.mitchellbosecke.pebble.loader.DelegatingLoader;
+import com.mitchellbosecke.pebble.loader.StringLoader;
+import com.mitchellbosecke.pebble.template.PebbleTemplate;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.io.Writer;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+
+
+public class EmbedCachingTagTest {
+
+    private final PebbleEngine pebble;
+    private final Map<String, Object> context;
+
+    public EmbedCachingTagTest() {
+        StringLoader stringLoader = new StringLoader();
+        ClasspathLoader classpathLoader = new ClasspathLoader();
+        classpathLoader.setPrefix("templates/embed/cache");
+
+        pebble = new PebbleEngine.Builder()
+                .loader(new DelegatingLoader(Arrays.asList(
+                        classpathLoader,
+                        stringLoader
+                )))
+                .strictVariables(false)
+                .cacheActive(true)
+                .build();
+
+        Writer writer = new StringWriter();
+        context = new HashMap<>();
+        context.put("foo", "FOO");
+        context.put("bar", "BAR");
+    }
+
+    @Test
+    public void testEmbedNotChangingCachedTemplate() throws PebbleException, IOException {
+        Writer writer1 = new StringWriter();
+        PebbleTemplate template1 = pebble.getTemplate("template1.peb");
+        template1.evaluate(writer1, context);
+        String result1 = writer1.toString();
+
+        Writer writer2 = new StringWriter();
+        PebbleTemplate template2 = pebble.getTemplate("template2.peb");
+        template2.evaluate(writer2, context);
+        String result2 = writer2.toString();
+
+        assertEquals("" +
+                "BEFORE BASE\n" +
+                "EMBED OVERRIDE\n" +
+                "AFTER BASE",
+                result1
+        );
+        assertEquals("" +
+                "BEFORE BASE\n" +
+                "EMBED BASE\n" +
+                "AFTER BASE",
+                result2
+        );
+    }
+
+}
diff --git a/pebble/src/test/resources/templates/embed/cache/template.base.peb b/pebble/src/test/resources/templates/embed/cache/template.base.peb
new file mode 100644
index 00000000..7ea3efbb
--- /dev/null
+++ b/pebble/src/test/resources/templates/embed/cache/template.base.peb
@@ -0,0 +1,5 @@
+BEFORE BASE
+{% block embedBlock1 %}
+EMBED BASE
+{% endblock %}
+AFTER BASE
\ No newline at end of file
diff --git a/pebble/src/test/resources/templates/embed/cache/template1.peb b/pebble/src/test/resources/templates/embed/cache/template1.peb
new file mode 100644
index 00000000..68adc237
--- /dev/null
+++ b/pebble/src/test/resources/templates/embed/cache/template1.peb
@@ -0,0 +1,5 @@
+{% embed './template.base.peb' %}
+{% block 'embedBlock1' %}
+EMBED OVERRIDE
+{% endblock %}
+{% endembed %}
diff --git a/pebble/src/test/resources/templates/embed/cache/template2.peb b/pebble/src/test/resources/templates/embed/cache/template2.peb
new file mode 100644
index 00000000..c8e02307
--- /dev/null
+++ b/pebble/src/test/resources/templates/embed/cache/template2.peb
@@ -0,0 +1,1 @@
+{% include './template.base.peb' %}