You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@freemarker.apache.org by dd...@apache.org on 2017/09/28 00:36:42 UTC
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.
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 <dd...@apache.org>
Authored: Thu Sep 28 02:06:39 2017 +0200
Committer: ddekany <dd...@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><@foo bar; i, jgt;${i},
+ * ${j}</@foo></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;