freemarker-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ddek...@apache.org
Subject incubator-freemarker git commit: 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
Date Mon, 20 Feb 2017 12:31:44 GMT
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 <ddekany@apache.org>
Authored: Mon Feb 20 13:31:37 2017 +0100
Committer: ddekany <ddekany@apache.org>
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();


Mime
View raw message