freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject [6/6] incubator-freemarker git commit: Bug fixed: BeansWrapper and DefaultObjectWrapper, starting from Java 8, when the same JavaBeans property has both non-indexed read method (like String[] getFoo()) and indexed read method (like String getFoo(int inde
Date Fri, 25 Aug 2017 23:38:19 GMT
Bug fixed: BeansWrapper and DefaultObjectWrapper, starting from Java 8, when the same JavaBeans
property has both non-indexed read method (like String[] getFoo()) and indexed read method
(like String getFoo(int index)), 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 incompatibleImprovements constructor argument
of the used DefaultObjectWrapper or BeansWrapper to 2.3.27. Note that if you leave the object_wrapper
setting of the Configuration on its default, it's enough to increase the incompatibleImprovements
setting of the Configuration to 2.3.27, as that's inherited by the default object_wrapper.
Note that this bug haven't surfaced before Java 8, as then java.beans.Inrospector has only
exposed the non-indexed method when both kind of read 
 method was present.


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

Branch: refs/heads/2.3-gae
Commit: a4a406a1d2e7a994ea589a9e6efe5d7575f69da4
Parents: ce4b9e4
Author: ddekany <ddekany@apache.org>
Authored: Sat Aug 26 01:37:32 2017 +0200
Committer: ddekany <ddekany@apache.org>
Committed: Sat Aug 26 01:37:32 2017 +0200

----------------------------------------------------------------------
 .../java/freemarker/ext/beans/BeanModel.java    | 16 ++++--
 .../java/freemarker/ext/beans/BeansWrapper.java | 14 ++++-
 .../freemarker/ext/beans/SimpleMethodModel.java | 21 ++++---
 .../java/freemarker/template/Configuration.java | 11 +++-
 .../java/freemarker/template/_TemplateAPI.java  |  1 +
 src/manual/en_US/book.xml                       | 27 +++++++++
 .../ext/beans/BeansWrapperMiscTest.java         | 58 +++++++++++++++++++-
 .../template/DefaultObjectWrapperTest.java      |  2 +-
 8 files changed, 134 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/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 8ec19ea..312ec98 100644
--- a/src/main/java/freemarker/ext/beans/BeanModel.java
+++ b/src/main/java/freemarker/ext/beans/BeanModel.java
@@ -50,6 +50,7 @@ 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;
 
 /**
@@ -219,10 +220,17 @@ implements
 
         TemplateModel resultModel = UNKNOWN;
         if (desc instanceof IndexedPropertyDescriptor) {
-            Method readMethod = ((IndexedPropertyDescriptor) desc).getIndexedReadMethod();

-            resultModel = cachedModel = 
-                new SimpleMethodModel(object, readMethod, 
-                        ClassIntrospector.getArgTypes(classInfo, readMethod), wrapper);
+            IndexedPropertyDescriptor pd = (IndexedPropertyDescriptor) desc;
+            if (wrapper.getIncompatibleImprovements().intValue() >= _TemplateAPI.VERSION_INT_2_3_27
+                    && pd.getReadMethod() != null) {
+                resultModel = wrapper.invokeMethod(object, pd.getReadMethod(), null);
+                // cachedModel remains null, as we don't cache these
+            } else {
+                Method readMethod = pd.getIndexedReadMethod(); 
+                resultModel = cachedModel = 
+                    new SimpleMethodModel(object, readMethod, 
+                            ClassIntrospector.getArgTypes(classInfo, readMethod), wrapper);
+            }
         } else if (desc instanceof PropertyDescriptor) {
             PropertyDescriptor pd = (PropertyDescriptor) desc;
             resultModel = wrapper.invokeMethod(object, pd.getReadMethod(), null);

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/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 f108ad6..ea9158d 100644
--- a/src/main/java/freemarker/ext/beans/BeansWrapper.java
+++ b/src/main/java/freemarker/ext/beans/BeansWrapper.java
@@ -19,6 +19,7 @@
 
 package freemarker.ext.beans;
 
+import java.beans.Introspector;
 import java.beans.PropertyDescriptor;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Array;
@@ -247,6 +248,16 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable
{
      *       {@code true}. Thus, Java 8 default methods (and the bean properties they define)
are exposed, despite that
      *       {@link java.beans.Introspector} (the official JavaBeans introspector) ignores
them, at least as of Java 8. 
      *     </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.  
+     *     </li>  
      *   </ul>
      *   
      *   <p>Note that the version will be normalized to the lowest version where the
same incompatible
@@ -844,7 +855,8 @@ public class BeansWrapper implements RichObjectWrapper, WriteProtectable
{
         if (incompatibleImprovements.intValue() < _TemplateAPI.VERSION_INT_2_3_0) {
             throw new IllegalArgumentException("Version must be at least 2.3.0.");
         }
-        return incompatibleImprovements.intValue() >= _TemplateAPI.VERSION_INT_2_3_26
? Configuration.VERSION_2_3_26
+        return incompatibleImprovements.intValue() >= _TemplateAPI.VERSION_INT_2_3_27
? Configuration.VERSION_2_3_27
+                : incompatibleImprovements.intValue() == _TemplateAPI.VERSION_INT_2_3_26
? Configuration.VERSION_2_3_26
                 : is2324Bugfixed(incompatibleImprovements) ? Configuration.VERSION_2_3_24
                 : is2321Bugfixed(incompatibleImprovements) ? Configuration.VERSION_2_3_21
                 : Configuration.VERSION_2_3_0;

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/src/main/java/freemarker/ext/beans/SimpleMethodModel.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/ext/beans/SimpleMethodModel.java b/src/main/java/freemarker/ext/beans/SimpleMethodModel.java
index e7e1ae1..9b8150b 100644
--- a/src/main/java/freemarker/ext/beans/SimpleMethodModel.java
+++ b/src/main/java/freemarker/ext/beans/SimpleMethodModel.java
@@ -24,13 +24,16 @@ import java.lang.reflect.Method;
 import java.util.Collections;
 import java.util.List;
 
+import freemarker.core._DelayedFTLTypeDescription;
+import freemarker.core._DelayedToString;
+import freemarker.core._ErrorDescriptionBuilder;
+import freemarker.core._TemplateModelException;
 import freemarker.core._UnexpectedTypeErrorExplainerTemplateModel;
 import freemarker.template.SimpleNumber;
 import freemarker.template.TemplateMethodModelEx;
 import freemarker.template.TemplateModel;
 import freemarker.template.TemplateModelException;
 import freemarker.template.TemplateSequenceModel;
-import freemarker.template.utility.ClassUtil;
 
 /**
  * A class that will wrap a reflected method call into a
@@ -81,13 +84,15 @@ public final class SimpleMethodModel extends SimpleMethod
     }
 
     public int size() throws TemplateModelException {
-        throw new TemplateModelException(
-                "Getting the number of items or enumerating the items is not supported on
this "
-                + ClassUtil.getFTLTypeDescription(this) + " value.\n"
-                + "("
-                + "Hint 1: Maybe you wanted to call this method first and then do something
with its return value. "
-                + "Hint 2: Getting items by intex possibly works, hence it's a \"+sequence\"."
-                + ")");
+        throw new _TemplateModelException(
+                new _ErrorDescriptionBuilder(
+                "Getting the number of items or listing the items is not supported on this
",
+                new _DelayedFTLTypeDescription(this), " value, because this value wraps the
following Java method, "
+                + "not a real listable value: ", new _DelayedToString(getMember()))
+                .tips(
+                        "Maybe you should to call this method first and then do something
with its return value.",
+                        "obj.someMethod(i) and obj.someMethod[i] does the same for this method,
hence it's a "
+                        + "\"+sequence\"."));
     }
     
     @Override

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/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 efa14ee..8b14da6 100644
--- a/src/main/java/freemarker/template/Configuration.java
+++ b/src/main/java/freemarker/template/Configuration.java
@@ -414,7 +414,7 @@ public class Configuration extends Configurable implements Cloneable,
ParserConf
     /** FreeMarker version 2.3.26 (an {@link #Configuration(Version) incompatible improvements
break-point}) */
     public static final Version VERSION_2_3_26 = new Version(2, 3, 26);
 
-    /** FreeMarker version 2.3.26 (an {@link #Configuration(Version) incompatible improvements
break-point}) */
+    /** FreeMarker version 2.3.27 (an {@link #Configuration(Version) incompatible improvements
break-point}) */
     public static final Version VERSION_2_3_27 = new Version(2, 3, 27);
     
     /** The default of {@link #getIncompatibleImprovements()}, currently {@link #VERSION_2_3_0}.
*/
@@ -833,6 +833,15 @@ public class Configuration extends Configurable implements Cloneable,
ParserConf
      *          properties they define); see {@link BeansWrapper#BeansWrapper(Version)}.

      *     </ul>
      *   </li>
+     *   <li><p>
+     *     2.3.27 (or higher):
+     *     <ul>
+     *       <li><p>
+     *          {@link BeansWrapper} and {@link DefaultObjectWrapper} now prefers the non-indexed
JavaBean property
+     *          read method over the indexed read method when Java 8 exposes both;
+     *          see {@link BeansWrapper#BeansWrapper(Version)}. 
+     *     </ul>
+     *   </li>
      * </ul>
      * 
      * @throws IllegalArgumentException

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/src/main/java/freemarker/template/_TemplateAPI.java
----------------------------------------------------------------------
diff --git a/src/main/java/freemarker/template/_TemplateAPI.java b/src/main/java/freemarker/template/_TemplateAPI.java
index 22cbc21..62960c8 100644
--- a/src/main/java/freemarker/template/_TemplateAPI.java
+++ b/src/main/java/freemarker/template/_TemplateAPI.java
@@ -48,6 +48,7 @@ public class _TemplateAPI {
     public static final int VERSION_INT_2_3_24 = Configuration.VERSION_2_3_24.intValue();
     public static final int VERSION_INT_2_3_25 = Configuration.VERSION_2_3_25.intValue();
     public static final int VERSION_INT_2_3_26 = Configuration.VERSION_2_3_26.intValue();
+    public static final int VERSION_INT_2_3_27 = Configuration.VERSION_2_3_27.intValue();
     public static final int VERSION_INT_2_4_0 = Version.intValueFor(2, 4, 0);
     
     public static void checkVersionNotNullAndSupported(Version incompatibleImprovements)
{

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/src/manual/en_US/book.xml
----------------------------------------------------------------------
diff --git a/src/manual/en_US/book.xml b/src/manual/en_US/book.xml
index 4961955..809822b 100644
--- a/src/manual/en_US/book.xml
+++ b/src/manual/en_US/book.xml
@@ -26965,6 +26965,33 @@ TemplateModel x = env.getVariable("x");  // get variable x</programlisting>
             </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>),
+              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
+              <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
+              increase the <link
+              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>java.beans.Inrospector</literal> has only exposed the
+              non-indexed method when both kind of read method was
+              present.</para>
+            </listitem>
+
+            <listitem>
               <para>Bug fixed: When the
               <literal>TemplateExceptionHandler</literal> suppresses (i.e.,
               doesn't re-throw) an exception, the <link

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/src/test/java/freemarker/ext/beans/BeansWrapperMiscTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/freemarker/ext/beans/BeansWrapperMiscTest.java b/src/test/java/freemarker/ext/beans/BeansWrapperMiscTest.java
index 697a98c..917b2b3 100644
--- a/src/test/java/freemarker/ext/beans/BeansWrapperMiscTest.java
+++ b/src/test/java/freemarker/ext/beans/BeansWrapperMiscTest.java
@@ -19,20 +19,30 @@
 
 package freemarker.ext.beans;
 
+import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 
+import java.util.Collections;
+
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
 
+import freemarker.template.Configuration;
 import freemarker.template.TemplateBooleanModel;
 import freemarker.template.TemplateHashModel;
+import freemarker.template.TemplateMethodModelEx;
+import freemarker.template.TemplateModel;
+import freemarker.template.TemplateModelException;
+import freemarker.template.TemplateScalarModel;
+import freemarker.template.TemplateSequenceModel;
+import freemarker.template.utility.Constants;
 
 @RunWith(JUnit4.class)
 public class BeansWrapperMiscTest {
 
     @Test
-    public void booleans() throws Exception {
+    public void booleansTest() throws Exception {
         final BeansWrapper bw = new BeansWrapper();
 
         assertTrue(((TemplateBooleanModel) bw.wrap(Boolean.TRUE)).getAsBoolean());
@@ -54,4 +64,50 @@ public class BeansWrapperMiscTest {
         assertSame(tm, bw.wrap(Boolean.TRUE));
     }
     
+    @Test
+    public void java8IndexedMethodIssueTest() throws TemplateModelException {
+        {
+            BeansWrapper bw = new BeansWrapper(Configuration.VERSION_2_3_26);
+            TemplateHashModel beanTM = (TemplateHashModel) bw.wrap(new BeanWithBothIndexedAndArrayProperty());
+            TemplateModel fooTM = beanTM.get("foo");
+            assertThat(fooTM, instanceOf(TemplateMethodModelEx.class));
+            assertThat(fooTM, instanceOf(TemplateSequenceModel.class));
+            assertEquals("b",
+                    ((TemplateScalarModel) ((TemplateMethodModelEx) fooTM).exec(
+                            Collections.singletonList(Constants.ONE)))
+                    .getAsString());
+            try {
+                ((TemplateSequenceModel) fooTM).size();
+                fail();
+            } catch (TemplateModelException e) {
+                // Expected
+            }
+        }
+        
+        {
+            BeansWrapper bw = new BeansWrapper(Configuration.VERSION_2_3_27);
+            TemplateHashModel beanTM = (TemplateHashModel) bw.wrap(new BeanWithBothIndexedAndArrayProperty());
+            TemplateModel fooTM = beanTM.get("foo");
+            assertThat(fooTM, not(instanceOf(TemplateMethodModelEx.class)));
+            assertThat(fooTM, instanceOf(TemplateSequenceModel.class));
+            assertEquals("b",
+                    ((TemplateScalarModel) ((TemplateSequenceModel) fooTM).get(1)).getAsString());
+            assertEquals(2, ((TemplateSequenceModel) fooTM).size());
+        }
+    }
+    
+    public static class BeanWithBothIndexedAndArrayProperty {
+        
+        private final static String[] FOO = new String[] { "a", "b" };
+        
+        public String[] getFoo() {
+            return FOO;
+        }
+        
+        public String getFoo(int index) {
+            return FOO[index];
+        }
+        
+    }
+    
 }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/a4a406a1/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 9a9e283..26bcf9a 100644
--- a/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
+++ b/src/test/java/freemarker/template/DefaultObjectWrapperTest.java
@@ -93,7 +93,7 @@ public class DefaultObjectWrapperTest {
         expected.add(Configuration.VERSION_2_3_24);
         expected.add(Configuration.VERSION_2_3_24); // no non-BC change in 2.3.25
         expected.add(Configuration.VERSION_2_3_26);
-        expected.add(Configuration.VERSION_2_3_26); // no non-BC change in 2.3.27
+        expected.add(Configuration.VERSION_2_3_27);
 
         List<Version> actual = new ArrayList<Version>();
         for (int i = _TemplateAPI.VERSION_INT_2_3_0; i <= Configuration.getVersion().intValue();
i++) {


Mime
View raw message