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 2017/09/16 14:52:50 UTC

incubator-freemarker git commit: Added a new BeansWrapper setting, preferIndexedReadMethod. With this one can address the Java 8 compatibility problem with indexed property read methods without changing the incompatibleImprovements of the object wrapper.

Repository: incubator-freemarker
Updated Branches:
  refs/heads/2.3-gae 19501e419 -> 3dfa8ce8a


Added a new BeansWrapper setting, preferIndexedReadMethod. With this one can address the Java 8 compatibility problem with indexed property read methods without changing the incompatibleImprovements of the object wrapper.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/3dfa8ce8
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/3dfa8ce8
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/3dfa8ce8

Branch: refs/heads/2.3-gae
Commit: 3dfa8ce8a74a09de9e0830a364d5371802472f4f
Parents: 19501e4
Author: ddekany <dd...@apache.org>
Authored: Sat Sep 16 16:52:37 2017 +0200
Committer: ddekany <dd...@apache.org>
Committed: Sat Sep 16 16:52:37 2017 +0200

----------------------------------------------------------------------
 .../java/freemarker/ext/beans/BeanModel.java    |  4 +-
 .../java/freemarker/ext/beans/BeansWrapper.java | 65 +++++++++++---------
 .../ext/beans/BeansWrapperConfiguration.java    | 15 +++++
 src/manual/en_US/book.xml                       | 28 ++++++---
 .../freemarker/template/ConfigurationTest.java  |  4 ++
 .../template/DefaultObjectWrapperTest.java      | 11 +++-
 6 files changed, 86 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3dfa8ce8/src/main/java/freemarker/ext/beans/BeanModel.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/ext/beans/BeanModel.java b/src/main/java/freemarker/ext/beans/BeanModel.java
index 312ec98..4139b09 100644
--- a/src/main/java/freemarker/ext/beans/BeanModel.java
+++ b/src/main/java/freemarker/ext/beans/BeanModel.java
@@ -50,7 +50,6 @@ import freemarker.template.TemplateModelException;
 import freemarker.template.TemplateModelIterator;
 import freemarker.template.TemplateModelWithAPISupport;
 import freemarker.template.TemplateScalarModel;
-import freemarker.template._TemplateAPI;
 import freemarker.template.utility.StringUtil;
 
 /**
@@ -221,8 +220,7 @@ implements
         TemplateModel resultModel = UNKNOWN;
         if (desc instanceof IndexedPropertyDescriptor) {
             IndexedPropertyDescriptor pd = (IndexedPropertyDescriptor) desc;
-            if (wrapper.getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_27
-                    && pd.getReadMethod() != null) {
+            if (!wrapper.getPreferIndexedReadMethod() && pd.getReadMethod() != null) {
                 resultModel = wrapper.invokeMethod(object, pd.getReadMethod(), null);
                 // cachedModel remains null, as we don't cache these
             } else {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3dfa8ce8/src/main/java/freemarker/ext/beans/BeansWrapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/ext/beans/BeansWrapper.java b/src/main/java/freemarker/ext/beans/BeansWrapper.java
index 0195234..3dbb3e1 100644
--- a/src/main/java/freemarker/ext/beans/BeansWrapper.java
+++ b/src/main/java/freemarker/ext/beans/BeansWrapper.java
@@ -179,11 +179,12 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable {
     private volatile boolean writeProtected;
     
     private TemplateModel nullModel = null;
-    private int defaultDateType; // initialized by PropertyAssignments.apply
+    private int defaultDateType; // initialized from the BeansWrapperConfiguration
     private ObjectWrapper outerIdentity = this;
     private boolean methodsShadowItems = true;
-    private boolean simpleMapWrapper;  // initialized by PropertyAssignments.apply
-    private boolean strict;  // initialized by PropertyAssignments.apply
+    private boolean simpleMapWrapper;  // initialized from the BeansWrapperConfiguration
+    private boolean strict;  // initialized from the BeansWrapperConfiguration
+    private boolean preferIndexedReadMethod; // initialized from the BeansWrapperConfiguration
     
     private final Version incompatibleImprovements;
     
@@ -250,13 +251,8 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable {
      *     </li>  
      *     <li>
      *       <p>2.3.27 (or higher):
-     *       If the same JavaBean property has both an indexed property reader (like {@code String getFoo(int)}) and
-     *       a non-indexed property reader (like {@code String[] getFoo()}), and {@link Introspector} exposes both
-     *       (which apparently only happens since Java 8), we will use the non-indexed property reader method, while
-     *       before this improvement we have used the indexed property method. When using the indexed property reader,
-     *       FreeMarker doesn't know the size of the array, so the value becomes unlistable. Before Java 8 this problem
-     *       haven't surfaced, as {@link Introspector} has only exposed the non-indexed property reader method when both
-     *       kind of read method was present. So this can be seen as a Java 8 compatibility fix.  
+     *       The default of the {@link #setPreferIndexedReadMethod(boolean) preferIndexedReadMethod} setting changes
+     *       from {@code true} to {@code false}.
      *     </li>  
      *   </ul>
      *   
@@ -344,6 +340,7 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable {
         this.incompatibleImprovements = bwConf.getIncompatibleImprovements();  // normalized
         
         simpleMapWrapper = bwConf.isSimpleMapWrapper();
+        preferIndexedReadMethod =  bwConf.getPreferIndexedReadMethod();
         defaultDateType = bwConf.getDefaultDateType();
         outerIdentity = bwConf.getOuterIdentity() != null ? bwConf.getOuterIdentity() : this;
         strict = bwConf.isStrict();
@@ -523,31 +520,40 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable {
         return simpleMapWrapper;
     }
 
-    // I have commented this out, as it won't be in 2.3.20 yet.
-    /*
     /**
-     * Tells which non-backward-compatible overloaded method selection fixes to apply;
-     * see {@link #setOverloadedMethodSelection(Version)}.
-     * /
-    public Version getOverloadedMethodSelection() {
-        return overloadedMethodSelection;
+     * Getter pair of {@link #setPreferIndexedReadMethod(boolean)} 
+     * 
+     * @since 2.3.27
+     */
+    public boolean getPreferIndexedReadMethod() {
+        return preferIndexedReadMethod;
     }
 
     /**
-     * Sets which non-backward-compatible overloaded method selection fixes to apply.
-     * This has similar logic as {@link Configuration#setIncompatibleImprovements(Version)},
-     * but only applies to this aspect.
+     * Sets if when a JavaBean property has both a normal read method (like {@code String[] getFoos()}) and an indexed
+     * read method (like {@code String getFoos(int index)}), and the Java {@link Introspector} exposes both (which only
+     * happens since Java 8, apparently), which read method will be used when the property is accessed with the
+     * shorthand syntax (like {@code myObj.foos}). Before {@link #getIncompatibleImprovements() incompatibleImprovements}
+     * 2.3.27 it defaults to {@code true} for backward compatibility (although it's actually less backward compatible if
+     * you are just switching to Java 8; see later), but the recommended value and the default starting with
+     * {@link #getIncompatibleImprovements() incompatibleImprovements} 2.3.27 is {@code false}. This setting has no
+     * effect on properties that only has normal read method, or only has indexed read method. In case a property has
+     * both, using the indexed reader method is disadvantageous, as then FreeMarker can't tell what the highest allowed
+     * index is, and so the property will be unlistable ({@code <#list foo as myObj.foos>} will fail).
      * 
-     * Currently significant values:
-     * <ul>
-     *   <li>2.3.21: Completetlly rewritten overloaded method selection, fixes several issues with the old one.</li>
-     * </ul>
-     * /
-    public void setOverloadedMethodSelection(Version version) {
-        overloadedMethodSelection = version;
+     * <p>
+     * Apparently, this setting only matters since Java 8, as before that {@link Introspector} did not expose the
+     * indexed reader method if there was also a normal reader method. As with Java 8 the behavior of
+     * {@link Introspector} has changed, some old templates started to break, as the property has suddenly become
+     * unlistable (see earlier why). So setting this to {@code false} can be seen as a Java 8 compatibility fix.
+     * 
+     * @since 2.3.27
+     */
+    public void setPreferIndexedReadMethod(boolean preferIndexedReadMethod) {
+        checkModifiable();
+        this.preferIndexedReadMethod = preferIndexedReadMethod;
     }
-    */
-    
+
     /**
      * Sets the method exposure level. By default, set to <code>EXPOSE_SAFE</code>.
      * @param exposureLevel can be any of the <code>EXPOSE_xxx</code>
@@ -1746,6 +1752,7 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable {
         return "simpleMapWrapper=" + simpleMapWrapper + ", "
                + "exposureLevel=" + classIntrospector.getExposureLevel() + ", "
                + "exposeFields=" + classIntrospector.getExposeFields() + ", "
+               + "preferIndexedReadMethod=" + preferIndexedReadMethod + ", "
                + "treatDefaultMethodsAsBeanMembers="
                + classIntrospector.getTreatDefaultMethodsAsBeanMembers() + ", "
                + "sharedClassIntrospCache="

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3dfa8ce8/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java b/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java
index ba19633..905bde9 100644
--- a/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java
+++ b/src/main/java/freemarker/ext/beans/BeansWrapperConfiguration.java
@@ -46,6 +46,7 @@ public abstract class BeansWrapperConfiguration implements Cloneable {
     
     // Properties and their *defaults*:
     private boolean simpleMapWrapper = false;
+    private boolean preferIndexedReadMethod;
     private int defaultDateType = TemplateDateModel.UNKNOWN;
     private ObjectWrapper outerIdentity = null;
     private boolean strict = false;
@@ -81,6 +82,8 @@ public abstract class BeansWrapperConfiguration implements Cloneable {
                 : BeansWrapper.normalizeIncompatibleImprovementsVersion(incompatibleImprovements);
         this.incompatibleImprovements = incompatibleImprovements;
         
+        preferIndexedReadMethod = incompatibleImprovements.intValue() < _TemplateAPI.VERSION_INT_2_3_27;
+        
         classIntrospectorBuilder = new ClassIntrospectorBuilder(incompatibleImprovements);
     }
     
@@ -97,6 +100,7 @@ public abstract class BeansWrapperConfiguration implements Cloneable {
         int result = 1;
         result = prime * result + incompatibleImprovements.hashCode();
         result = prime * result + (simpleMapWrapper ? 1231 : 1237);
+        result = prime * result + (preferIndexedReadMethod ? 1231 : 1237);
         result = prime * result + defaultDateType;
         result = prime * result + (outerIdentity != null ? outerIdentity.hashCode() : 0);
         result = prime * result + (strict ? 1231 : 1237);
@@ -118,6 +122,7 @@ public abstract class BeansWrapperConfiguration implements Cloneable {
         
         if (!incompatibleImprovements.equals(other.incompatibleImprovements)) return false;
         if (simpleMapWrapper != other.simpleMapWrapper) return false;
+        if (preferIndexedReadMethod != other.preferIndexedReadMethod) return false;
         if (defaultDateType != other.defaultDateType) return false;
         if (outerIdentity != other.outerIdentity) return false;
         if (strict != other.strict) return false;
@@ -148,6 +153,16 @@ public abstract class BeansWrapperConfiguration implements Cloneable {
     public void setSimpleMapWrapper(boolean simpleMapWrapper) {
         this.simpleMapWrapper = simpleMapWrapper;
     }
+    
+    /** @since 2.3.27 */
+    public boolean getPreferIndexedReadMethod() {
+        return preferIndexedReadMethod;
+    }
+
+    /** See {@link BeansWrapper#setPreferIndexedReadMethod(boolean)}. @since 2.3.27 */
+    public void setPreferIndexedReadMethod(boolean preferIndexedReadMethod) {
+        this.preferIndexedReadMethod = preferIndexedReadMethod;
+    }
 
     public int getDefaultDateType() {
         return defaultDateType;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3dfa8ce8/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index e127a79..f1dc57a 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -26974,18 +26974,26 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
             </listitem>
 
             <listitem>
+              <para>Added new <literal>BeansWrapper</literal> setting,
+              <literal>preferIndexedReadMethod</literal>. This was added to
+              address a Java 8 compatibility problem; see the bug fix entry
+              below for more information.</para>
+            </listitem>
+
+            <listitem>
               <para>Bug fixed: <literal>BeansWrapper</literal> and
               <literal>DefaultObjectWrapper</literal>, starting from Java 8,
               when the same JavaBeans property has both non-indexed read
-              method (like <literal>String[] getFoo()</literal>) and indexed
-              read method (like <literal>String getFoo(int index)</literal>),
+              method (like <literal>String[] getFoos()</literal>) and indexed
+              read method (like <literal>String getFoos(int index)</literal>),
               has mistakenly used the indexed read method to access the
               property. This is a problem because then the array size was
               unknown, and thus the property has suddenly become unlistable on
-              Java 8. To enable the fix (where it will use the non-indexed
-              read method), you have to increase the value of the
-              <literal>incompatibleImprovements</literal> constructor argument
-              of the used <literal>DefaultObjectWrapper</literal> or
+              Java 8 (that is, <literal>&lt;#list myObject.foos as
+              foo&gt;</literal> fails). To enable the fix (where it will use
+              the non-indexed read method), you should to increase the value
+              of the <literal>incompatibleImprovements</literal> constructor
+              argument of the used <literal>DefaultObjectWrapper</literal> or
               <literal>BeansWrapper</literal> to 2.3.27. Note that if you
               leave the <literal>object_wrapper</literal> setting of the
               <literal>Configuration</literal> on its default, it's enough to
@@ -26993,8 +27001,12 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
               linkend="pgui_config_incompatible_improvements_how_to_set"><literal>incompatibleImprovements</literal>
               setting</link> of the <literal>Configuration</literal> to
               2.3.27, as that's inherited by the default
-              <literal>object_wrapper</literal>. Note that this bug haven't
-              surfaced before Java 8, as then
+              <literal>object_wrapper</literal>. In case increasing the
+              <literal>incompatibleImprovements</literal> is not an option
+              (because of the other changes it brings), you can instead set
+              the <literal>preferIndexedReadMethod</literal> property of the
+              object wrapper to <literal>false</literal>. Note that this bug
+              haven't surfaced before Java 8, as then
               <literal>java.beans.Inrospector</literal> has only exposed the
               non-indexed method when both kind of read method was
               present.</para>

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3dfa8ce8/src/test/java/freemarker/template/ConfigurationTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/template/ConfigurationTest.java b/src/test/java/freemarker/template/ConfigurationTest.java
index 2beeff0..226306f 100644
--- a/src/test/java/freemarker/template/ConfigurationTest.java
+++ b/src/test/java/freemarker/template/ConfigurationTest.java
@@ -167,6 +167,10 @@ public class ConfigurationTest extends TestCase {
         assertFalse(((DefaultObjectWrapper) cfg.getObjectWrapper()).getTreatDefaultMethodsAsBeanMembers());
         cfg.setIncompatibleImprovements(Configuration.VERSION_2_3_26);
         assertTrue(((DefaultObjectWrapper) cfg.getObjectWrapper()).getTreatDefaultMethodsAsBeanMembers());
+        assertTrue(((DefaultObjectWrapper) cfg.getObjectWrapper()).getPreferIndexedReadMethod());
+        cfg.setIncompatibleImprovements(Configuration.VERSION_2_3_27);
+        assertTrue(((DefaultObjectWrapper) cfg.getObjectWrapper()).getTreatDefaultMethodsAsBeanMembers());
+        assertFalse(((DefaultObjectWrapper) cfg.getObjectWrapper()).getPreferIndexedReadMethod());
     }
 
     private void assertUses2322ObjectWrapper(Configuration cfg) {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/3dfa8ce8/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
index 26bcf9a..833f509 100644
--- a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
+++ b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
@@ -1001,7 +1001,7 @@ public class DefaultObjectWrapperTest {
     }
     
     @Test
-    public void assertCanWrapDOM() throws SAXException, IOException, ParserConfigurationException,
+    public void testCanWrapDOM() throws SAXException, IOException, ParserConfigurationException,
             TemplateModelException {
         DocumentBuilder db = DocumentBuilderFactory.newInstance().newDocumentBuilder();
         InputSource is = new InputSource();
@@ -1009,6 +1009,15 @@ public class DefaultObjectWrapperTest {
         Document doc = db.parse(is);        
         assertTrue(OW22.wrap(doc) instanceof TemplateNodeModel);
     }
+
+    @Test
+    public void testPreferIndexedReadMethodAndIcI() {
+        assertTrue(new DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_26).build().getPreferIndexedReadMethod());
+        assertTrue(new DefaultObjectWrapper(Configuration.VERSION_2_3_26).getPreferIndexedReadMethod());
+        
+        assertFalse(new DefaultObjectWrapperBuilder(Configuration.VERSION_2_3_27).build().getPreferIndexedReadMethod());
+        assertFalse(new DefaultObjectWrapper(Configuration.VERSION_2_3_27).getPreferIndexedReadMethod());
+    }
     
     private void assertSizeThroughAPIModel(int expectedSize, TemplateModel normalModel) throws TemplateModelException {
         if (!(normalModel instanceof TemplateModelWithAPISupport)) {