Repository: incubator-freemarker Updated Branches: refs/heads/3 e90480e99 -> 6f5aabd64
Removed a backward compatibility feature: Even for setting values that are class names without following `()` or other argument list, the INSTANCE field and the builder class will be searched now, and used instead of the constructor of the class. Earlier they weren't for backward compatibility. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/6f5aabd6 Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/6f5aabd6 Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/6f5aabd6 Branch: refs/heads/3 Commit: 6f5aabd644ff30b4e687b3f222c5c69dc4a59d66 Parents: e90480e Author: ddekany <[email protected]> Authored: Mon Feb 20 13:31:37 2017 +0100 Committer: ddekany <[email protected]> Committed: Mon Feb 20 13:31:37 2017 +0100 ---------------------------------------------------------------------- .../freemarker/core/ast/Configurable.java | 15 ++--- .../ast/_ObjectBuilderSettingEvaluator.java | 63 +------------------- src/manual/en_US/FM3-CHANGE-LOG.txt | 8 ++- .../core/ast/ObjectBuilderSettingsTest.java | 24 ++------ 4 files changed, 17 insertions(+), 93 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6f5aabd6/src/main/java/org/apache/freemarker/core/ast/Configurable.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/ast/Configurable.java b/src/main/java/org/apache/freemarker/core/ast/Configurable.java index 8023552..374feb3 100644 --- a/src/main/java/org/apache/freemarker/core/ast/Configurable.java +++ b/src/main/java/org/apache/freemarker/core/ast/Configurable.java @@ -2090,18 +2090,16 @@ public class Configurable { * <li> * <p>If you have no constructor arguments and property setters, and the <tt><i>className</i></tt> class has * a public static {@code INSTANCE} field, the value of that filed will be the value of the expression, and - * the constructor won't be called. Note that if you use the backward compatible - * syntax, where these's no parenthesis after the class name, then it will not look for {@code INSTANCE}. + * the constructor won't be called. * </li> * <li> * <p>If there exists a class named <tt><i>className</i>Builder</tt>, then that class will be instantiated * instead with the given constructor arguments, and the JavaBean properties of that builder instance will be * set. After that, the public <tt>build()</tt> method of the instance will be called, whose return value * will be the value of the whole expression. (The builder class and the <tt>build()</tt> method is simply - * found by name, there's no special interface to implement.) Note that if you use the backward compatible - * syntax, where these's no parenthesis after the class name, then it will not look for builder class. Note - * that if you have a builder class, you don't actually need a <tt><i>className</i></tt> class (since 2.3.24); - * after all, <tt><i>className</i>Builder.build()</tt> can return any kind of object. + * found by name, there's no special interface to implement.)Note that if you have a builder class, you don't + * actually need a <tt><i>className</i></tt> class (since 2.3.24); after all, + * <tt><i>className</i>Builder.build()</tt> can return any kind of object. * </li> * <li> * <p>Currently, the values of arguments and properties can only be one of these: @@ -2134,10 +2132,7 @@ public class Configurable { * rarely useful, apart from using {@code null}). * </li> * <li> - * <p>The top-level object builder expressions may omit {@code ()}. In that case, for backward compatibility, - * the {@code INSTANCE} field and the builder class is not searched, so the instance will be always - * created with its parameterless constructor. (This behavior will possibly change in 2.4.) The {@code ()} - * can't be omitted for nested expressions. + * <p>The top-level object builder expressions may omit {@code ()}. * </li> * <li> * <p>The following classes can be referred to with simple (unqualified) name instead of fully qualified name: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6f5aabd6/src/main/java/org/apache/freemarker/core/ast/_ObjectBuilderSettingEvaluator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/ast/_ObjectBuilderSettingEvaluator.java b/src/main/java/org/apache/freemarker/core/ast/_ObjectBuilderSettingEvaluator.java index 214d511..554e047 100644 --- a/src/main/java/org/apache/freemarker/core/ast/_ObjectBuilderSettingEvaluator.java +++ b/src/main/java/org/apache/freemarker/core/ast/_ObjectBuilderSettingEvaluator.java @@ -89,9 +89,6 @@ public class _ObjectBuilderSettingEvaluator { // Parser state: private int pos; - // Parsing results: - private boolean modernMode = false; - private _ObjectBuilderSettingEvaluator( String src, int pos, Class expectedClass, boolean allowNull, _SettingEvaluationEnvironment env) { this.src = src; @@ -126,12 +123,7 @@ public class _ObjectBuilderSettingEvaluator { Object value; skipWS(); - try { - value = ensureEvaled(fetchValue(false, true, false, true)); - } catch (LegacyExceptionWrapperSettingEvaluationExpression e) { - e.rethrowLegacy(); - value = null; // newer reached - } + value = ensureEvaled(fetchValue(false, true, false, true)); skipWS(); if (pos != src.length()) { @@ -181,8 +173,6 @@ public class _ObjectBuilderSettingEvaluator { } exp.className = shorthandToFullQualified(fetchedClassName); if (!fetchedClassName.equals(exp.className)) { - // Before 2.3.21 only full-qualified class names were allowed - modernMode = true; exp.canBeStaticField = false; } } @@ -209,9 +199,6 @@ public class _ObjectBuilderSettingEvaluator { } private void fetchParameterListInto(ExpressionWithParameters exp) throws _ObjectBuilderSettingEvaluationException { - // Before 2.3.21 there was no parameter list - modernMode = true; - skipWS(); if (fetchOptionalChar(")") != ')') { do { @@ -867,30 +854,6 @@ public class _ObjectBuilderSettingEvaluator { Class cl; - if (!modernMode) { - try { - try { - return _ClassUtil.forName(className).newInstance(); - } catch (InstantiationException e) { - throw new LegacyExceptionWrapperSettingEvaluationExpression(e); - } catch (IllegalAccessException e) { - throw new LegacyExceptionWrapperSettingEvaluationExpression(e); - } catch (ClassNotFoundException e) { - throw new LegacyExceptionWrapperSettingEvaluationExpression(e); - } - } catch (LegacyExceptionWrapperSettingEvaluationExpression e) { - if (!canBeStaticField) { - throw e; - } - // Silently try to interpret className as static filed, throw the original exception if that fails. - try { - return getStaticFieldValue(className); - } catch (_ObjectBuilderSettingEvaluationException e2) { - throw e; - } - } - } - boolean clIsBuilderClass; try { cl = _ClassUtil.forName(className + BUILDER_CLASS_POSTFIX); @@ -1091,28 +1054,4 @@ public class _ObjectBuilderSettingEvaluator { } - private static class LegacyExceptionWrapperSettingEvaluationExpression - extends _ObjectBuilderSettingEvaluationException { - - public LegacyExceptionWrapperSettingEvaluationExpression(Throwable cause) { - super("Legacy operation failed", cause); - if (!( - (cause instanceof ClassNotFoundException) - || (cause instanceof InstantiationException) - || (cause instanceof IllegalAccessException) - )) { - throw new IllegalArgumentException(); - } - } - - public void rethrowLegacy() throws ClassNotFoundException, InstantiationException, IllegalAccessException { - Throwable cause = getCause(); - if (cause instanceof ClassNotFoundException) throw (ClassNotFoundException) cause; - if (cause instanceof InstantiationException) throw (InstantiationException) cause; - if (cause instanceof IllegalAccessException) throw (IllegalAccessException) cause; - throw new BugException(); - } - - } - } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6f5aabd6/src/manual/en_US/FM3-CHANGE-LOG.txt ---------------------------------------------------------------------- diff --git a/src/manual/en_US/FM3-CHANGE-LOG.txt b/src/manual/en_US/FM3-CHANGE-LOG.txt index 049c3ed..a198052 100644 --- a/src/manual/en_US/FM3-CHANGE-LOG.txt +++ b/src/manual/en_US/FM3-CHANGE-LOG.txt @@ -77,7 +77,8 @@ the FreeMarer 3 changelog here: write protected (non-configurable). Also now they come from the pool that ObjectWrapper builders use. - WrappingTemplateModel.objectWrapper is now final, and its statically stored default value can't be set anymore. - Removed SimpleObjectWrapper deprecated paramerless constructor -- Removed ResourceBundleLocalizedString and LocalizedString: Hardly anybody has discovered these, and they had no JUnit coverage. +- Removed ResourceBundleLocalizedString and LocalizedString: Hardly anybody has discovered these, and they had no + JUnit coverage. - Added early draft of TemplateResolver, renamed TemplateCache to DefaultTemplateResolver. TemplateResolver is not yet directly used in Configuration. This was only added in a hurry, so that it's visible why the o.a.f.core.templateresolver subpackage name makes sense. @@ -90,4 +91,7 @@ the FreeMarer 3 changelog here: - Removed support for incompatibleImprovements before 3.0.0. So currently 3.0.0 is the only support value. - Changed the default of logTemplateExceptions to false. - Removed `String Configurable.getSetting(String)` and `Properties getSettings()`. It has never worked well, - and is impossible to implement properly. \ No newline at end of file + and is impossible to implement properly. +- Even for setting values that are class names without following `()` or other argument list, the INSTANCE field and + the builder class will be searched now, and used instead of the constructor of the class. Earlier they weren't for + backward compatibility. http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6f5aabd6/src/test/java/org/apache/freemarker/core/ast/ObjectBuilderSettingsTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/freemarker/core/ast/ObjectBuilderSettingsTest.java b/src/test/java/org/apache/freemarker/core/ast/ObjectBuilderSettingsTest.java index c041623..c7838fd 100644 --- a/src/test/java/org/apache/freemarker/core/ast/ObjectBuilderSettingsTest.java +++ b/src/test/java/org/apache/freemarker/core/ast/ObjectBuilderSettingsTest.java @@ -176,11 +176,11 @@ public class ObjectBuilderSettingsTest { @Test public void builderTest() throws Exception { { - // Backward-compatible mode, no builder: + // `()`-les syntax: TestBean2 res = (TestBean2) _ObjectBuilderSettingEvaluator.eval( "org.apache.freemarker.core.ast.ObjectBuilderSettingsTest$TestBean2", Object.class, false, _SettingEvaluationEnvironment.getCurrent()); - assertFalse(res.built); + assertTrue(res.built); assertEquals(0, res.x); } @@ -211,14 +211,14 @@ public class ObjectBuilderSettingsTest { @Test public void staticInstanceTest() throws Exception { - // Backward compatible mode: + // ()-les syntax: { TestBean5 res = (TestBean5) _ObjectBuilderSettingEvaluator.eval( "org.apache.freemarker.core.ast.ObjectBuilderSettingsTest$TestBean5", Object.class, false, _SettingEvaluationEnvironment.getCurrent()); assertEquals(0, res.i); assertEquals(0, res.x); - assertNotSame(TestBean5.INSTANCE, res); + assertSame(TestBean5.INSTANCE, res); //! } { @@ -266,13 +266,11 @@ public class ObjectBuilderSettingsTest { } { - // Backward-compatible mode TestBean3 res = (TestBean3) _ObjectBuilderSettingEvaluator.eval( "org.apache.freemarker.core.ast.ObjectBuilderSettingsTest$TestBean3", Object.class, false, _SettingEvaluationEnvironment.getCurrent()); assertEquals(0, res.x); - assertFalse(res.isWriteProtected()); - res.setX(2); + assertTrue(res.isWriteProtected()); // Still uses a builder } } @@ -460,18 +458,6 @@ public class ObjectBuilderSettingsTest { assertEquals(Configuration.VERSION_3_0_0, ((BeansWrapper) cfg.getObjectWrapper()).getIncompatibleImprovements()); } - - { - Properties props = new Properties(); - props.setProperty( - Configurable.OBJECT_WRAPPER_KEY, - "org.apache.freemarker.core.model.impl.beans.BeansWrapper"); - cfg.setSettings(props); - assertEquals(BeansWrapper.class, cfg.getObjectWrapper().getClass()); - assertFalse(((WriteProtectable) cfg.getObjectWrapper()).isWriteProtected()); - assertEquals(Configuration.VERSION_3_0_0, - ((BeansWrapper) cfg.getObjectWrapper()).getIncompatibleImprovements()); - } { Properties props = new Properties();
