Repository: PebbleTemplates/pebble
Pull request #253 (2017-02-07 21:20:19)

Macros are called too often, if in an include hierarchy
I just discovered, that macros are called too often if you frequently import the same macro in different templates in a hierarchy.

Example:
"index" (import and call macro A, include template "child") -> (first call of macro A)
"child" (import and call macro A) -> (second call of macro A)

In fact, the macro A is currently called 3 times! Once in the "index" template and 2x in the "child" template, because it searches in the hierarchy and executes each macro in the upper hierarchy.

This fix solves this issue (hopefully). I added a test to show the issue.

------------
disclaimer: I really like pebble and we use it in a large site. Thank you for your work!


Patch:
diff --git a/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java b/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
index cf828f56..ea343e79 100644
--- a/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
+++ b/src/main/java/com/mitchellbosecke/pebble/template/PebbleTemplateImpl.java
@@ -318,6 +318,8 @@ public class PebbleTemplateImpl implements PebbleTemplate {
                 if (template.hasMacro(macroName)) {
                     found = true;
                     result = template.macro(context, macroName, args, false);
+                    // If a macro was found and executed, dont search for more
+                    break;
                 }
             }
         }


Test patch:
diff --git a/src/test/java/com/mitchellbosecke/pebble/macro/PebbleExtension.java b/src/test/java/com/mitchellbosecke/pebble/macro/PebbleExtension.java
new file mode 100644
index 00000000..e22bb666
--- /dev/null
+++ b/src/test/java/com/mitchellbosecke/pebble/macro/PebbleExtension.java
@@ -0,0 +1,17 @@
+package com.mitchellbosecke.pebble.macro;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import com.mitchellbosecke.pebble.extension.AbstractExtension;
+import com.mitchellbosecke.pebble.extension.Filter;
+
+public class PebbleExtension extends AbstractExtension{
+  @Override
+  public Map<String, Filter> getFilters() {
+    Map<String, Filter> f = new HashMap<>();
+    f.put(TestFilter.FILTER_NAME, new TestFilter());
+      return f;
+  }
+
+}
diff --git a/src/test/java/com/mitchellbosecke/pebble/macro/TestFilter.java b/src/test/java/com/mitchellbosecke/pebble/macro/TestFilter.java
new file mode 100644
index 00000000..6f276106
--- /dev/null
+++ b/src/test/java/com/mitchellbosecke/pebble/macro/TestFilter.java
@@ -0,0 +1,32 @@
+package com.mitchellbosecke.pebble.macro;
+
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+
+import com.mitchellbosecke.pebble.extension.Filter;
+
+public class TestFilter implements Filter {
+
+	static int counter = 0;
+
+	public static final String FILTER_NAME = "testfilter";
+
+	@Override
+	public List<String> getArgumentNames() {
+		return Arrays.asList("content");
+	}
+
+	@Override
+	public Object apply(Object input, Map<String, Object> args) {
+		String content = (String) input;
+		counter++;
+		content = content + "?" + "Hello";
+		return content;
+	}
+
+	public static int getCounter() {
+		return counter;
+	}
+
+}
diff --git a/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java b/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
new file mode 100644
index 00000000..4536f598
--- /dev/null
+++ b/src/test/java/com/mitchellbosecke/pebble/macro/TestMacroCalls.java
@@ -0,0 +1,53 @@
+package com.mitchellbosecke.pebble.macro;
+
+import static org.junit.Assert.assertEquals;
+
+import java.io.IOException;
+import java.io.StringWriter;
+import java.io.Writer;
+
+import org.junit.Test;
+
+import com.mitchellbosecke.pebble.PebbleEngine;
+import com.mitchellbosecke.pebble.error.PebbleException;
+import com.mitchellbosecke.pebble.template.PebbleTemplate;
+
+/**
+ * Test calls of macros
+ *
+ */
+public class TestMacroCalls {
+
+	
+	/**
+	 * Checks, if macros are called to often
+	 * @throws PebbleException
+	 * @throws IOException
+	 */
+    @Test
+    public void testmultipleMacroCalls() throws PebbleException, IOException {
+    	// Build a pebble engine with one configured filter ("testfilter" - TestFilter.java)
+        PebbleEngine pebble = new PebbleEngine.Builder()
+        		.extension(new PebbleExtension())
+        		.strictVariables(false).build();
+        // Resets the counter of the filter
+        TestFilter.counter = 0;
+        /*
+         * Runs the test scenario:
+         * 	index-template with an import of the macro and a call to this macro (Call #1)
+         * 	index-templates includes "include.peb"
+         *  
+         *  include.peb with an import of the macro and a call to this macro (Call #2)
+         *  
+         *  We track the number of macro-calls by using a small "Filter" (TestFilter) that just
+         *  counts, how often it is called.
+         */
+        PebbleTemplate template = pebble.getTemplate("templates/macros/index.peb");
+        Writer writer = new StringWriter();
+        template.evaluate(writer);
+        
+        
+        // We expect, that the TestFilter was called 2x
+        assertEquals(2, TestFilter.getCounter());
+    }
+}
diff --git a/src/test/resources/templates/macros/include.peb b/src/test/resources/templates/macros/include.peb
new file mode 100644
index 00000000..3eb18387
--- /dev/null
+++ b/src/test/resources/templates/macros/include.peb
@@ -0,0 +1,3 @@
+Include
+{% import "templates/macros/macro.peb" %}
+Call 2: {{ test() }}
diff --git a/src/test/resources/templates/macros/index.peb b/src/test/resources/templates/macros/index.peb
new file mode 100644
index 00000000..0cb62bad
--- /dev/null
+++ b/src/test/resources/templates/macros/index.peb
@@ -0,0 +1,6 @@
+Hello
+
+{% import "templates/macros/macro.peb" %}
+Call 1: {{ test() }}
+
+{% include "templates/macros/include.peb" %}
diff --git a/src/test/resources/templates/macros/macro.peb b/src/test/resources/templates/macros/macro.peb
new file mode 100644
index 00000000..5ce0a2aa
--- /dev/null
+++ b/src/test/resources/templates/macros/macro.peb
@@ -0,0 +1,3 @@
+{% macro test() %}
+Macro-Output: {{ "test" | testfilter }}
+{% endmacro %}