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: Forward ported from 2.3-gae: FREEMARKER-48: Better handling of unchecked exceptions thrown by custom TemplateModel-s. Note that in 3 we don't need the wrapUncheckedExceptions configuration setting.
Date Thu, 28 Sep 2017 00:36:42 GMT
Repository: incubator-freemarker
Updated Branches:
  refs/heads/3 59d2f30b7 -> 6c8b795f4


Forward ported from 2.3-gae: FREEMARKER-48: Better handling of unchecked exceptions thrown
by custom TemplateModel-s. Note that in 3 we don't need the wrapUncheckedExceptions configuration
setting.


Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/6c8b795f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/6c8b795f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/6c8b795f

Branch: refs/heads/3
Commit: 6c8b795f4a7f6e0b80183e429a3c6d3e8b8cdf4d
Parents: 59d2f30
Author: ddekany <ddekany@apache.org>
Authored: Thu Sep 28 02:06:39 2017 +0200
Committer: ddekany <ddekany@apache.org>
Committed: Thu Sep 28 02:36:29 2017 +0200

----------------------------------------------------------------------
 FM3-CHANGE-LOG.txt                              |   3 +-
 .../core/UncheckedExceptionHandlingTest.java    | 141 +++++++++++++++++++
 .../apache/freemarker/core/ASTDirReturn.java    |   2 +-
 .../freemarker/core/ASTDynamicTopLevelCall.java |  27 ++--
 .../apache/freemarker/core/ASTExpression.java   |   9 +-
 .../core/BreakOrContinueException.java          |  21 ++-
 .../org/apache/freemarker/core/CallPlace.java   |   5 +-
 .../freemarker/core/FlowControlException.java   |  37 +++++
 ...nterruptionSupportTemplatePostProcessor.java |   2 +-
 .../core/model/TemplateDirectiveModel.java      |  14 +-
 10 files changed, 241 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/FM3-CHANGE-LOG.txt
----------------------------------------------------------------------
diff --git a/FM3-CHANGE-LOG.txt b/FM3-CHANGE-LOG.txt
index 0febe03..0480d51 100644
--- a/FM3-CHANGE-LOG.txt
+++ b/FM3-CHANGE-LOG.txt
@@ -268,7 +268,8 @@ Core / Configuration
     DefaultTemplateNameFormat24 was renamed to DefaultTemplateNameFormat.
 - Removed the logTemplateExceptions (log_template_exceptions) setting. FreeMarker now behaves
as if it was false.
   When a FreeMarker method throws an exception, the caller is responsible for either logging
it or letting it bubble up.
-- Removed the strict_syntax setting, and so also the support for FTL tags without #. This
was a FreeMarker 1.x
+- Removed the wrapUncheckedExceptions setting. FreeMarker now behaves as if it was true.
+- Removed the strictSyntax setting, and so also the support for FTL tags without #. This
was a FreeMarker 1.x
   compatibility option.
 - Renamed the `cacheStorage` Configuration setting to `templateCacheStorage`.
 - Renamed the `localizedLookup` Configuration setting to `localizedTemplateLookup`.

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java
----------------------------------------------------------------------
diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java
b/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java
new file mode 100644
index 0000000..fbe6267
--- /dev/null
+++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/UncheckedExceptionHandlingTest.java
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+import java.io.Writer;
+
+import org.apache.freemarker.core.model.ArgumentArrayLayout;
+import org.apache.freemarker.core.model.TemplateDirectiveModel;
+import org.apache.freemarker.core.model.TemplateFunctionModel;
+import org.apache.freemarker.core.model.TemplateModel;
+import org.apache.freemarker.core.util.CallableUtils;
+import org.apache.freemarker.test.TemplateTest;
+import org.junit.Test;
+
+import com.google.common.collect.ImmutableMap;
+
+public class UncheckedExceptionHandlingTest extends TemplateTest {
+    
+    @Override
+    protected Object createDataModel() {
+        return ImmutableMap.of(
+                "f", MyErronousFunction.INSTANCE,
+                "d", MyErronousDirective.INSTANCE,
+                "fd", MyFilterDirective.INSTANCE);
+    }
+
+    @Test
+    public void test() {
+        assertThat(
+                assertErrorContains("${f()}", TemplateException.class, "unchecked exception").getCause(),
+                instanceOf(MyUncheckedException.class));
+        assertThat(
+                assertErrorContains("<@d />", TemplateException.class, "unchecked exception").getCause(),
+                instanceOf(MyUncheckedException.class));
+        assertThat(
+                assertErrorContains("${f('NPE')}", TemplateException.class, "unchecked exception").getCause(),
+                instanceOf(NullPointerException.class));
+        assertThat(
+                assertErrorContains("<@d 'NPE' />", TemplateException.class, "unchecked
exception").getCause(),
+                instanceOf(NullPointerException.class));
+    }
+
+    @Test
+    public void testFlowControlWorks() throws IOException, TemplateException {
+        assertOutput("<#list 1..2 as i>a<@fd>b<#break>c</@>d</#list>.",
"ab.");
+        assertOutput("<#list 1..2 as i>a<@fd>b<#continue>c</@>d</#list>.",
"abab.");
+        assertOutput("<#function f()><@fd><#return 1></@></#function>${f()}.",
"1.");
+    }
+    
+    public static class MyErronousFunction implements TemplateFunctionModel {
+        private static final MyErronousFunction INSTANCE = new MyErronousFunction();
+
+        @Override
+        public TemplateModel execute(TemplateModel[] args, CallPlace callPlace, Environment
env)
+                throws TemplateException {
+            if ("NPE".equals(CallableUtils.getOptionalStringArgument(args, 0, this))) {
+                throw new NullPointerException();
+            } else {
+                throw new MyUncheckedException();
+            }
+        }
+
+        @Override
+        public ArgumentArrayLayout getFunctionArgumentArrayLayout() {
+            return ArgumentArrayLayout.SINGLE_POSITIONAL_PARAMETER;
+        }
+
+    }
+
+    public static class MyErronousDirective implements TemplateDirectiveModel {
+        private static final MyErronousDirective INSTANCE = new MyErronousDirective();
+
+        @Override
+        public void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment
env)
+                throws TemplateException {
+            if ("NPE".equals(CallableUtils.getOptionalStringArgument(args, 0, this))) {
+                throw new NullPointerException();
+            } else {
+                throw new MyUncheckedException();
+            }
+        }
+
+        @Override
+        public ArgumentArrayLayout getDirectiveArgumentArrayLayout() {
+            return ArgumentArrayLayout.SINGLE_POSITIONAL_PARAMETER;
+        }
+
+        @Override
+        public boolean isNestedContentSupported() {
+            return false;
+        }
+        
+    }
+
+    public static class MyFilterDirective implements TemplateDirectiveModel {
+        private static final MyFilterDirective INSTANCE = new MyFilterDirective();
+        
+        @Override
+        public void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment
env)
+                throws TemplateException, IOException {
+            callPlace.executeNestedContent(null, out, env);
+        }
+
+        @Override
+        public ArgumentArrayLayout getDirectiveArgumentArrayLayout() {
+            return ArgumentArrayLayout.PARAMETERLESS;
+        }
+
+        @Override
+        public boolean isNestedContentSupported() {
+            return true;
+        }
+        
+    }
+    
+    public static class MyUncheckedException extends RuntimeException {
+        //
+    }
+    
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java
index 8493310..c5d23b3 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDirReturn.java
@@ -60,7 +60,7 @@ final class ASTDirReturn extends ASTDirective {
         return "#return";
     }
     
-    public static class Return extends RuntimeException {
+    public static class Return extends FlowControlException {
         static final Return INSTANCE = new Return();
         private Return() {
         }

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java
index 7e713bd..dd8394f 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTDynamicTopLevelCall.java
@@ -30,6 +30,7 @@ import org.apache.freemarker.core.model.TemplateDirectiveModel;
 import org.apache.freemarker.core.model.TemplateFunctionModel;
 import org.apache.freemarker.core.model.TemplateModel;
 import org.apache.freemarker.core.util.BugException;
+import org.apache.freemarker.core.util.CallableUtils;
 import org.apache.freemarker.core.util.CommonSupplier;
 import org.apache.freemarker.core.util.StringToIndexMap;
 import org.apache.freemarker.core.util._StringUtils;
@@ -122,15 +123,24 @@ class ASTDynamicTopLevelCall extends ASTDirective implements CallPlace
 {
                 positionalArgs, namedArgs, argsLayout, callableValue,
                 false, env);
 
-        if (directive != null) {
-            directive.execute(execArgs, this, env.getOut(), env);
-        } else {
-            TemplateModel result = function.execute(execArgs, this, env);
-            if (result == null) {
-                throw new TemplateException(env, "Function has returned no value (or null)");
+        try {
+            if (directive != null) {
+                directive.execute(execArgs, this, env.getOut(), env);
+            } else {
+                TemplateModel result = function.execute(execArgs, this, env);
+                if (result == null) {
+                    throw new TemplateException(env, "Function has returned no value (or
null)");
+                }
+                // TODO [FM3] Implement it when we have a such language... it should work
like `${f()}`.
+                throw new BugException("Top-level function call not yet implemented");
             }
-            // TODO [FM3] Implement it when we have a such language... it should work like
`${f()}`.
-            throw new BugException("Top-level function call not yet implemented");
+        } catch (TemplateException | FlowControlException | IOException e) {
+            throw e;
+        } catch (Exception e) {
+            throw CallableUtils.newGenericExecuteException(
+                    "An unchecked exception was thrown; see the cause exception",
+                    directive != null ? directive : function, directive == null,
+                    e);
         }
 
         return null;
@@ -315,6 +325,7 @@ class ASTDynamicTopLevelCall extends ASTDirective implements CallPlace
 {
         return null;
     }
 
+    @Override
     @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
     @SuppressFBWarnings(value={ "IS2_INCONSISTENT_SYNC", "DC_DOUBLECHECK" }, justification="Performance
tricks")
     public Object getOrCreateCustomData(Object providerIdentity, CommonSupplier<?>
supplier)

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java
index c10beac..d92b477 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ASTExpression.java
@@ -67,7 +67,14 @@ abstract class ASTExpression extends ASTNode {
     }
     
     final TemplateModel eval(Environment env) throws TemplateException {
-        return constantValue != null ? constantValue : _eval(env);
+        try {
+            return constantValue != null ? constantValue : _eval(env);
+        } catch (FlowControlException | TemplateException e) {
+            throw e;
+        } catch (Exception e) {
+                throw new TemplateException(
+                        this, e, env, "Expression has thrown an unchecked exception; see
the cause exception.");
+        }
     }
     
     String evalAndCoerceToPlainText(Environment env) throws TemplateException {

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java
index c83cb43..5b22161 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/BreakOrContinueException.java
@@ -1,3 +1,22 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
 package org.apache.freemarker.core;
 
 /**
@@ -5,7 +24,7 @@ package org.apache.freemarker.core;
  */
 // TODO [FM3] This is not a good mechanism (like what if we have <#list ...><@m><#break><@></#list>,
and inside `m`
 // there's <#list ...><#nested></#list>)
-class BreakOrContinueException extends RuntimeException {
+class BreakOrContinueException extends FlowControlException {
     static final BreakOrContinueException BREAK_INSTANCE = new BreakOrContinueException();
     static final BreakOrContinueException CONTINUE_INSTANCE = new BreakOrContinueException();
     

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java b/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java
index 628c9ec..d204084 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/CallPlace.java
@@ -57,8 +57,9 @@ public interface CallPlace {
      * Executes the nested content; it there's none, it just does nothing.
      *
      * @param nestedContentArgs
-     *         The nested content parameter values to pass to the nested content (as in {@code
<@foo bar; i, j>${i},
-     *         ${j}</@foo>} there are 2 such parameters, whose value you set here),
or {@code null} if there's none.
+     *         The nested content parameter values to pass to the nested content (as in <code>&lt;@foo
bar; i, jgt;${i},
+     *         ${j}&lt;/@foo&gt;</code> there are 2 such parameters, whose
value you set here), or {@code null} if
+     *         there's none.
      *         This array must be {@link #getNestedContentParameterCount()} long, or else
FreeMarker will throw an
      *         {@link TemplateException} with a descriptive error message that tells to user
that they need to declare
      *         that many nested content parameters as the length of this array. If you want
to allow the  caller to not

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java
new file mode 100644
index 0000000..f7c10bc
--- /dev/null
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/FlowControlException.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.freemarker.core;
+
+/**
+ * Exception that's not really an exception, just used for flow control.
+ */
+// TODO [FM3] Can we solve controlling program flow without exceptions?
+@SuppressWarnings("serial")
+class FlowControlException extends RuntimeException {
+
+    FlowControlException() {
+        super();
+    }
+
+    FlowControlException(String message) {
+        super(message);
+    }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java
index a1b60bf..be7877c 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ThreadInterruptionSupportTemplatePostProcessor.java
@@ -126,7 +126,7 @@ class ThreadInterruptionSupportTemplatePostProcessor extends TemplatePostProcess
      * <p>ATTENTION: This is used by https://github.com/kenshoo/freemarker-online.
Don't break backward
      * compatibility without updating that project too! 
      */
-    static class TemplateProcessingThreadInterruptedException extends RuntimeException {
+    static class TemplateProcessingThreadInterruptedException extends FlowControlException
{
         
         TemplateProcessingThreadInterruptedException() {
             super("Template processing thread \"interrupted\" flag was set.");

http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/6c8b795f/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
----------------------------------------------------------------------
diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
index 3eb140d..c834f97 100644
--- a/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
+++ b/freemarker-core/src/main/java/org/apache/freemarker/core/model/TemplateDirectiveModel.java
@@ -28,7 +28,11 @@ import org.apache.freemarker.core.util.CallableUtils;
 public interface TemplateDirectiveModel extends TemplateCallableModel {
 
     /**
-     * Invokes the directive.
+     * Invokes the callable object.
+     * <p>
+     * This method shouldn't deliberately throw {@link RuntimeException}, nor {@link IOException}
that wasn't caused by
+     * writing to the output. Such exceptions should be catched inside the method and wrapped
inside a
+     * {@link TemplateException}. 
      *
      * @param args
      *         The array of argument values. Not {@code null}. If a parameter was omitted
on the caller side, the
@@ -55,10 +59,10 @@ public interface TemplateDirectiveModel extends TemplateCallableModel
{
      *         specifically allow that, typically, implementations that are just adapters
towards FreeMarker-unaware
      *         callables (for example, {@link JavaMethodModel} is like that).
      *
-     * @throws TemplateException
-     *         If any problem occurs that's not an {@link IOException} during writing the
template output.
-     * @throws IOException
-     *         When writing the template output fails.
+     * @throws TemplateException If any problem occurs that's not an {@link IOException}
during writing the template
+     *          output.
+     * @throws IOException When writing the template output fails. Other {@link IOException}-s
should be catched in this
+     *          method and wrapped into {@link TemplateException}.
      */
     void execute(TemplateModel[] args, CallPlace callPlace, Writer out, Environment env)
             throws TemplateException, IOException;


Mime
View raw message