Bug fixed, with incompatible_improvements set to 2.3.24: The #import directive meant to copy the library variable into a global variable if it's executed in the main namespace, but that haven't happened when the imported template was already imported earlier in another namespace. (Also fixed some minor typos elsewhere, and added more import/include tests.)
Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/55cb12e2 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/55cb12e2 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/55cb12e2 Branch: refs/heads/2.3 Commit: 55cb12e21fd92101b23a2fed112f1e77a2c4eb91 Parents: 576625f Author: ddekany <[email protected]> Authored: Mon Dec 28 00:46:49 2015 +0100 Committer: ddekany <[email protected]> Committed: Mon Dec 28 00:46:49 2015 +0100 ---------------------------------------------------------------------- src/main/java/freemarker/core/Environment.java | 7 +- .../java/freemarker/template/Configuration.java | 26 ++++++- src/manual/en_US/book.xml | 28 ++++++- .../freemarker/core/IncludeAndImportTest.java | 81 ++++++++++++++++++++ 4 files changed, 136 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55cb12e2/src/main/java/freemarker/core/Environment.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/core/Environment.java b/src/main/java/freemarker/core/Environment.java index 5ebf855..5ae1bf7 100644 --- a/src/main/java/freemarker/core/Environment.java +++ b/src/main/java/freemarker/core/Environment.java @@ -2421,7 +2421,7 @@ public final class Environment extends Configurable { /** * Same as {@link #getTemplateForInclusion(String, String, boolean, boolean)} with {@code false} - * {@code ignoreMissign} argument. + * {@code ignoreMissing} argument. */ public Template getTemplateForInclusion(String name, String encoding, boolean parse) throws IOException { @@ -2560,11 +2560,14 @@ public final class Environment extends Configurable { if (existingNamespace != null) { if (namespace != null) { setVariable(namespace, existingNamespace); + if (isIcI2324OrLater() && currentNamespace == mainNamespace) { + globalNamespace.put(namespace, existingNamespace); + } } } else { Namespace newNamespace = new Namespace(loadedTemplate); if (namespace != null) { - currentNamespace.put(namespace, newNamespace); + setVariable(namespace, newNamespace); if (currentNamespace == mainNamespace) { globalNamespace.put(namespace, newNamespace); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55cb12e2/src/main/java/freemarker/template/Configuration.java ---------------------------------------------------------------------- diff --git a/src/main/java/freemarker/template/Configuration.java b/src/main/java/freemarker/template/Configuration.java index a913cb7..bd7d46c 100644 --- a/src/main/java/freemarker/template/Configuration.java +++ b/src/main/java/freemarker/template/Configuration.java @@ -775,6 +775,11 @@ public class Configuration extends Configurable implements Cloneable, ParserConf * {@code incompatibleImprovements} version number than that of the {@link Configuration}, if that's * really what you want.) * </li> + * <li><p> + * Fixed bug: The {@code #import} directive meant to copy the library variable into a global variable if + * it's executed in the main namespace, but that haven't happened when the imported template was already + * imported earlier in another namespace. + * </li> * </ul> * </li> * </ul> @@ -2977,7 +2982,17 @@ public class Configuration extends Configurable implements Cloneable, ParserConf /** * Adds an invisible <code>#import <i>templateName</i> as <i>namespaceVarName</i></code> at the beginning of all - * templates. The order of the imports will be the same as the order in which they were added with this method. + * top-level templates (that is, to all templates that weren't included/imported from another template). While it + * only affects the top-level (main) template directly, as the imports there will create a global variable, the + * imports will be visible from the further imported templates too (though note that + * {@link #getIncompatibleImprovements()} set to 2.3.24 fixes a rarely surfacing bug here). + * + * <p> + * The order of the imports will be the same as the order in which they were added with this method. Note that if + * there are also auto-includes ({@link #addAutoInclude(String)}), those inclusions will be executed after the + * auto-includes. + * + * @see #setAutoImports(Map) */ public void addAutoImport(String namespaceVarName, String templateName) { // "synchronized" is removed from the API as it's not safe to set anything after publishing the Configuration @@ -3040,7 +3055,14 @@ public class Configuration extends Configurable implements Cloneable, ParserConf /** * Adds an invisible <code>#include <i>templateName</i> as <i>namespaceVarName</i></code> at the beginning of all - * templates. The order of the inclusions will be the same as the order in which they were added with this method. + * top-level templates (that is, to all templates that weren't included/imported from another template). + * + * <p> + * The order of the inclusions will be the same as the order in which they were added with this method. Note that if + * there are also auto-imports ({@link #addAutoImport(String, String)}), those imports will be executed before the + * auto-includes, hence the library variables are accessible for the auto-includes. + * + * @see #setAutoIncludes(List) */ public void addAutoInclude(String templateName) { // "synchronized" is removed from the API as it's not safe to set anything after publishing the Configuration http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55cb12e2/src/manual/en_US/book.xml ---------------------------------------------------------------------- diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml index 2745adb..a08d339 100644 --- a/src/manual/en_US/book.xml +++ b/src/manual/en_US/book.xml @@ -19527,7 +19527,9 @@ or by the caller of <literal>import</literal> (as if you would create it with <literal>assign</literal> directive), with the name given with the <literal><replaceable>hash</replaceable></literal> - parameter.</para> + parameter. If the import happens in the namespace of the main + template, the hash variable is also created in the global + namespace.</para> <para>If you call <literal>import</literal> with the same <literal><replaceable>path</replaceable></literal> for multiple @@ -26915,6 +26917,17 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> </listitem> <listitem> + <para>Bug fixed, with + <literal>incompatible_improvements</literal> set to 2.3.24 + (<link linkend="topic.defaultObjectWrapperIcI">see how + here...</link>): The <literal>#import</literal> directive meant + to copy the library variable into a global variable if it's + executed in the main namespace, but that haven't happened when + the imported template was already imported earlier in another + namespace.</para> + </listitem> + + <listitem> <para>Internal code cleanup: Mostly source code formatting, also many parser construction/setup cleanup</para> </listitem> @@ -26951,7 +26964,7 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> but some might used these regardless to introspect templates. Earlier it had an <quote>embedded template</quote> parameter inside, now it has 0 (for purely static string literals), one or - multiple <quote>value part</quote>-ts, which are + more <quote>value part</quote>-s, which are <literal>String</literal>-s and <literal>Interpolation</literal>-s.</para> </listitem> @@ -27240,6 +27253,17 @@ TemplateModel x = env.getVariable("x"); // get variable x</programlisting> context, but also the auto-escaping policy of it (basically, if auto-escapig is on or off).</para> </listitem> + + <listitem> + <para>Bug fixed, with + <literal>incompatible_improvements</literal> set to 2.3.24 + (<link linkend="topic.defaultObjectWrapperIcI">see how + here...</link>): The <literal>#import</literal> directive meant + to copy the library variable into a global variable if it's + executed in the main namespace, but that haven't happened when + the imported template was already imported earlier in another + namespace.</para> + </listitem> </itemizedlist> </section> </section> http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/55cb12e2/src/test/java/freemarker/core/IncludeAndImportTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/freemarker/core/IncludeAndImportTest.java b/src/test/java/freemarker/core/IncludeAndImportTest.java new file mode 100644 index 0000000..6cd9d32 --- /dev/null +++ b/src/test/java/freemarker/core/IncludeAndImportTest.java @@ -0,0 +1,81 @@ +package freemarker.core; + +import java.io.IOException; + +import org.junit.Before; +import org.junit.Test; + +import freemarker.cache.StringTemplateLoader; +import freemarker.template.Configuration; +import freemarker.template.TemplateException; +import freemarker.test.TemplateTest; + +public class IncludeAndImportTest extends TemplateTest { + + @Override + protected Configuration createConfiguration() throws Exception { + Configuration cfg = super.createConfiguration(); + cfg.setTemplateLoader(new StringTemplateLoader()); + return cfg; + } + + @Before + public void setup() { + addTemplate("inc1.ftl", "[inc1]<#global inc1Cnt = (inc1Cnt!0) + 1><#global history = (history!) + 'I'>"); + addTemplate("inc2.ftl", "[inc2]"); + addTemplate("inc3.ftl", "[inc3]"); + addTemplate("lib1.ftl", "<#global lib1Cnt = (lib1Cnt!0) + 1><#global history = (history!) + 'L1'>" + + "<#macro m>In lib1</#macro>"); + addTemplate("lib2.ftl", "<#global history = (history!) + 'L2'>" + + "<#macro m>In lib2 (<@lib1.m/>)</#macro>"); + addTemplate("lib3.ftl", "<#import 'lib1.ftl' as lib1>"); + } + + @Test + public void includeSameTwice() throws IOException, TemplateException { + assertOutput("<#include 'inc1.ftl'>${inc1Cnt}<#include 'inc1.ftl'>${inc1Cnt}", "[inc1]1[inc1]2"); + } + + @Test + public void importSameTwice() throws IOException, TemplateException { + assertOutput("<#import 'lib1.ftl' as i1>${lib1Cnt} <#import 'lib1.ftl' as i2>${lib1Cnt}", "1 1"); + } + + @Test + public void importInMainCreatesGlobal() throws IOException, TemplateException { + String ftl = "${.main.lib1???c} ${.globals.lib1???c}" + + "<#import 'lib1.ftl' as lib1> ${.main.lib1???c} ${.globals.lib1???c}"; + String expectedOut = "false false true true"; + assertOutput(ftl, expectedOut); + // No difference: + getConfiguration().setIncompatibleImprovements(Configuration.VERSION_2_3_24); + assertOutput(ftl, expectedOut); + } + + @Test + public void importInMainCreatesGlobalBugfix() throws IOException, TemplateException { + // An import in the main namespace should create a global variable, but there's a bug where that doesn't happen + // if the imported library was already initialized elsewhere. + String ftl = "<#import 'lib3.ftl' as lib3>${lib1Cnt} ${.main.lib1???c} ${.globals.lib1???c}, " + + "<#import 'lib1.ftl' as lib1>${lib1Cnt} ${.main.lib1???c} ${.globals.lib1???c}"; + assertOutput(ftl, "1 false false, 1 true false"); + // Activate bugfix: + getConfiguration().setIncompatibleImprovements(Configuration.VERSION_2_3_24); + assertOutput(ftl, "1 false false, 1 true true"); + } + + /** + * Tests the order of auto-includes and auto-imports, also that they only effect the main template directly. + */ + @Test + public void autoIncludeAndAutoImport() throws IOException, TemplateException { + getConfiguration().addAutoInclude("inc1.ftl"); + getConfiguration().addAutoInclude("inc2.ftl"); + getConfiguration().addAutoImport("lib1", "lib1.ftl"); + getConfiguration().addAutoImport("lib2", "lib2.ftl"); + assertOutput( + "<#include 'inc3.ftl'>[main] ${inc1Cnt}, ${history}, <@lib1.m/>, <@lib2.m/>", + "[inc1][inc2][inc3][main] 1, L1L2I, In lib1, In lib2 (In lib1)"); + } + +}
