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 <[email protected]> Authored: Sat Aug 26 01:37:32 2017 +0200 Committer: ddekany <[email protected]> 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++) {
