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/08/25 23:39:44 UTC
[18/19] 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 in
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
Commit: a4a406a1d2e7a994ea589a9e6efe5d7575f69da4
Parents: ce4b9e4
Author: ddekany <dd...@apache.org>
Authored: Sat Aug 26 01:37:32 2017 +0200
Committer: ddekany <dd...@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++) {