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 Commit: 3dfa8ce8a74a09de9e0830a364d5371802472f4f Parents: 19501e4 Author: ddekany <ddek...@apache.org> Authored: Sat Sep 16 16:52:37 2017 +0200 Committer: ddekany <ddek...@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><#list myObject.foos as + foo></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)) {