You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2015/12/28 00:47:02 UTC

[2/2] incubator-freemarker git commit: 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 imp

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-gae
Commit: 55cb12e21fd92101b23a2fed112f1e77a2c4eb91
Parents: 576625f
Author: ddekany <dd...@apache.org>
Authored: Mon Dec 28 00:46:49 2015 +0100
Committer: ddekany <dd...@apache.org>
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)");
+    }
+    
+}