Repository: incubator-freemarker Updated Branches: refs/heads/3 02aea4474 -> 2548f5cdc
Forward ported #attempt error reporting improvements from 2.3.27. Furthermore, #attempt now logs errors into the org.apache.freemarker.core.Runtime.Attempt log chategory (by default). It doesn't create an additional debug level log entry anymore. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/2548f5cd Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/2548f5cd Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/2548f5cd Branch: refs/heads/3 Commit: 2548f5cdc78fe05372ba314fc1e0415d154fd7ba Parents: 02aea44 Author: ddekany <ddek...@apache.org> Authored: Wed Jun 28 17:04:52 2017 +0200 Committer: ddekany <ddek...@apache.org> Committed: Wed Jun 28 17:04:52 2017 +0200 ---------------------------------------------------------------------- FM3-CHANGE-LOG.txt | 6 +- .../freemarker/core/AttemptLoggingTest.java | 100 +++++++++++++++++++ .../freemarker/core/ConfigurationTest.java | 32 +++++- .../core/TemplateConfigurationTest.java | 1 + .../core/AttemptExceptionReporter.java | 45 +++++++++ .../apache/freemarker/core/Configuration.java | 21 ++++ .../org/apache/freemarker/core/Environment.java | 35 ++++--- .../core/LoggingAttemptExceptionReporter.java | 43 ++++++++ .../core/MutableProcessingConfiguration.java | 87 +++++++++++++++- .../core/ProcessingConfiguration.java | 26 ++++- .../org/apache/freemarker/core/Template.java | 14 ++- .../freemarker/core/TemplateConfiguration.java | 27 ++++- 12 files changed, 410 insertions(+), 27 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/FM3-CHANGE-LOG.txt ---------------------------------------------------------------------- diff --git a/FM3-CHANGE-LOG.txt b/FM3-CHANGE-LOG.txt index a930bc6..ba175ee 100644 --- a/FM3-CHANGE-LOG.txt +++ b/FM3-CHANGE-LOG.txt @@ -264,6 +264,8 @@ the FreeMarer 3 changelog here: "allowsNothing" were renamed to "allow_nothing" and "allowNothing". - TemplateExceptionHandler.IGNORE_HANDLER, RETHROW_HANDLER, DEBUG_HANDLER and HTLM_DEBUG_HANDLER was renamed to IGNORE, RETHROW, DEBUG and HTML_DEBUG +- AttemptExceptionReporter.LOG_ERROR_REPORTER and LOG_WARN_REPORTER was renamed to + LOG_ERROR and LOG_WARN (to be consistent with the new TemplateExceptionHandler names) - Renamed the `cacheStorage` Configuration setting to `templateCacheStorage`. - Renamed the `localizedLookup` Configuration setting to `localizedLookup`. - Changed the defaults of some Configuration settings: @@ -285,4 +287,6 @@ the FreeMarer 3 changelog here: get(), to be more similar to the Java 8 API. - Removed long deprecated `#{}` interpolations. They are treated as plain static text now. (The template converter tool translates these to `${}` interpolations. For example `#{x}` is simply - translated to `${b}`, while `#{x; m1M3}` is translated to `${x?string('0.0##')}`). \ No newline at end of file + translated to `${b}`, while `#{x; m1M3}` is translated to `${x?string('0.0##')}`). +- #attempt now logs errors into the org.apache.freemarker.core.Runtime.Attempt log chategory (by + default). It doesn't create an additional debug level log entry anymore. \ No newline at end of file http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core-test/src/test/java/org/apache/freemarker/core/AttemptLoggingTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/AttemptLoggingTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/AttemptLoggingTest.java new file mode 100644 index 0000000..2b89776 --- /dev/null +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/AttemptLoggingTest.java @@ -0,0 +1,100 @@ +/* + * 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.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; + +import java.io.IOException; +import java.io.Writer; +import java.util.ArrayList; +import java.util.List; + +import org.apache.freemarker.test.TemplateTest; +import org.apache.freemarker.test.TestConfigurationBuilder; +import org.junit.Test; + +public class AttemptLoggingTest extends TemplateTest { + + @Test + public void standardConfigTest() throws IOException, TemplateException { + assertOutput("<#attempt>${missingVar1}<#recover>r</#attempt>", "r"); + // Here, we should have an ERROR entry in the log that refers to an exception in an #attempt block. But we can't + // easily assert that automatically, so it has to be checked manually... + + setConfiguration(new TestConfigurationBuilder() + .attemptExceptionReporter(AttemptExceptionReporter.LOG_WARN) + .build()); + assertOutput("<#attempt>${missingVar2}<#recover>r</#attempt>", "r"); + // Again, it must be checked manually if there's a WARN entry + } + + @Test + public void customConfigTest() throws IOException, TemplateException { + List<String> reports = new ArrayList<String>(); + setConfiguration(new TestConfigurationBuilder() + .attemptExceptionReporter(new TestAttemptExceptionReporter(reports)) + .build()); + + assertOutput( + "<#attempt>${missingVar1}<#recover>r</#attempt>" + + "<#attempt>${missingVar2}<#recover>r</#attempt>", + "rr"); + assertEquals(2, reports.size()); + assertThat(reports.get(0), containsString("missingVar1")); + assertThat(reports.get(1), containsString("missingVar2")); + } + + @Test + public void dontReportSuppressedExceptionsTest() throws IOException, TemplateException { + List<String> reports = new ArrayList<String>(); + setConfiguration(new TestConfigurationBuilder() + .attemptExceptionReporter(new TestAttemptExceptionReporter(reports)) + .templateExceptionHandler(new TemplateExceptionHandler() { + public void handleTemplateException(TemplateException te, Environment env, Writer out) + throws TemplateException { + try { + out.write("[E]"); + } catch (IOException e) { + throw new TemplateException("Failed to write to the output", e, env); + } + } + }) + .build()); + + assertOutput("<#attempt>${missingVar1}t<#recover>r</#attempt>", "[E]t"); + + assertEquals(0, reports.size()); + } + + private static final class TestAttemptExceptionReporter implements AttemptExceptionReporter { + private final List<String> reports; + + private TestAttemptExceptionReporter(List<String> reports) { + this.reports = reports; + } + + public void report(TemplateException te, Environment env) { + reports.add(te.getMessage()); + } + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java index 913b388..8882a3c 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/ConfigurationTest.java @@ -611,11 +611,35 @@ public class ConfigurationTest { @Test public void testSetICIViaSetSettingAPI() throws ConfigurationException { - Builder cfg = new Builder(VERSION_3_0_0); - assertEquals(DEFAULT_INCOMPATIBLE_IMPROVEMENTS, cfg.getIncompatibleImprovements()); + Builder cfgB = new Builder(VERSION_3_0_0); + assertEquals(DEFAULT_INCOMPATIBLE_IMPROVEMENTS, cfgB.getIncompatibleImprovements()); // This is the only valid value ATM: - cfg.setSetting(INCOMPATIBLE_IMPROVEMENTS_KEY, "3.0.0"); - assertEquals(VERSION_3_0_0, cfg.getIncompatibleImprovements()); + cfgB.setSetting(INCOMPATIBLE_IMPROVEMENTS_KEY, "3.0.0"); + assertEquals(VERSION_3_0_0, cfgB.getIncompatibleImprovements()); + } + + @Test + public void testSetAttemptExceptionReporter() throws TemplateException { + Configuration.Builder cfgB = new Configuration.Builder(Configuration.VERSION_3_0_0); + assertEquals(AttemptExceptionReporter.LOG_ERROR, cfgB.getAttemptExceptionReporter()); + assertFalse(cfgB.isAttemptExceptionReporterSet()); + cfgB.setSetting(MutableProcessingConfiguration.ATTEMPT_EXCEPTION_REPORTER_KEY, "log_warn"); + assertEquals(AttemptExceptionReporter.LOG_WARN, cfgB.getAttemptExceptionReporter()); + assertTrue(cfgB.isAttemptExceptionReporterSet()); + cfgB.setSetting(MutableProcessingConfiguration.ATTEMPT_EXCEPTION_REPORTER_KEY, "default"); + assertEquals(AttemptExceptionReporter.LOG_ERROR, cfgB.getAttemptExceptionReporter()); + assertFalse(cfgB.isAttemptExceptionReporterSet()); + + assertEquals(AttemptExceptionReporter.LOG_ERROR, + new Configuration.Builder(Configuration.VERSION_3_0_0) + .build() + .getAttemptExceptionReporter()); + + assertEquals(AttemptExceptionReporter.LOG_WARN, + new Configuration.Builder(Configuration.VERSION_3_0_0) + .attemptExceptionReporter(AttemptExceptionReporter.LOG_WARN) + .build() + .getAttemptExceptionReporter()); } @Test http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java ---------------------------------------------------------------------- diff --git a/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java b/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java index 4b6d4b3..355c6e7 100644 --- a/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java +++ b/freemarker-core-test/src/test/java/org/apache/freemarker/core/TemplateConfigurationTest.java @@ -167,6 +167,7 @@ public class TemplateConfigurationTest { SETTING_ASSIGNMENTS.put("outputEncoding", StandardCharsets.UTF_16); SETTING_ASSIGNMENTS.put("showErrorTips", false); SETTING_ASSIGNMENTS.put("templateExceptionHandler", TemplateExceptionHandler.IGNORE); + SETTING_ASSIGNMENTS.put("attemptExceptionReporter", AttemptExceptionReporter.LOG_WARN); SETTING_ASSIGNMENTS.put("timeFormat", "@HH:mm"); SETTING_ASSIGNMENTS.put("timeZone", NON_DEFAULT_TZ); SETTING_ASSIGNMENTS.put("arithmeticEngine", ConservativeArithmeticEngine.INSTANCE); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/AttemptExceptionReporter.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/AttemptExceptionReporter.java b/freemarker-core/src/main/java/org/apache/freemarker/core/AttemptExceptionReporter.java new file mode 100644 index 0000000..b10df40 --- /dev/null +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/AttemptExceptionReporter.java @@ -0,0 +1,45 @@ +/* + * 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; + +/** + * Used for the + * {@link ProcessingConfiguration#getAttemptExceptionReporter() attemptExceptionReported} configuration setting. + */ +public interface AttemptExceptionReporter { + + /** + * Logs the exception into the "freemarker.runtime" log category with "error" log level. This is the default + * {@link AttemptExceptionReporter}. The error message will explain that the error was handled by an + * {@code #attempt} block. + */ + AttemptExceptionReporter LOG_ERROR = new LoggingAttemptExceptionReporter(false); + + /** + * Like {@link #LOG_ERROR}, but it logs with "warn" log level. + */ + AttemptExceptionReporter LOG_WARN = new LoggingAttemptExceptionReporter(true); + + /** + * Called to log or otherwise report the error that has occurred inside an {@code #attempt} block. + */ + void report(TemplateException te, Environment env); + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java b/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java index 8122e92..9f31556 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/Configuration.java @@ -254,6 +254,7 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc private final TimeZone sqlDateAndTimeTimeZone; private final String booleanFormat; private final TemplateExceptionHandler templateExceptionHandler; + private final AttemptExceptionReporter attemptExceptionReporter; private final ArithmeticEngine arithmeticEngine; private final ObjectWrapper objectWrapper; private final Charset outputEncoding; @@ -419,6 +420,7 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc sqlDateAndTimeTimeZone = builder.getSQLDateAndTimeTimeZone(); booleanFormat = builder.getBooleanFormat(); templateExceptionHandler = builder.getTemplateExceptionHandler(); + attemptExceptionReporter = builder.getAttemptExceptionReporter(); arithmeticEngine = builder.getArithmeticEngine(); outputEncoding = builder.getOutputEncoding(); urlEscapingCharset = builder.getURLEscapingCharset(); @@ -524,6 +526,20 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc return true; } + @Override + public AttemptExceptionReporter getAttemptExceptionReporter() { + return attemptExceptionReporter; + } + + /** + * Always {@code true} in {@link Configuration}-s; even if this setting wasn't set in the builder, it gets a default + * value in the {@link Configuration}. + */ + @Override + public boolean isAttemptExceptionReporterSet() { + return true; + } + private static class DefaultSoftCacheStorage extends SoftCacheStorage { // Nothing to override } @@ -2673,6 +2689,11 @@ public final class Configuration implements TopLevelConfiguration, CustomStateSc } @Override + protected AttemptExceptionReporter getDefaultAttemptExceptionReporter() { + return AttemptExceptionReporter.LOG_ERROR; + } + + @Override protected ArithmeticEngine getDefaultArithmeticEngine() { return BigDecimalArithmeticEngine.INSTANCE; } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java b/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java index e058da8..c825dc3 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/Environment.java @@ -104,7 +104,6 @@ public final class Environment extends MutableProcessingConfiguration<Environmen private static final ThreadLocal<Environment> TLS_ENVIRONMENT = new ThreadLocal(); private static final Logger LOG = _CoreLogs.RUNTIME; - private static final Logger LOG_ATTEMPT = _CoreLogs.ATTEMPT; // Do not use this object directly; deepClone it first! DecimalFormat isn't // thread-safe. @@ -575,10 +574,6 @@ public final class Environment extends MutableProcessingConfiguration<Environmen out = prevOut; } if (thrownException != null) { - if (LOG_ATTEMPT.isDebugEnabled()) { - LOG_ATTEMPT.debug("Error in attempt block " + - attemptBlock.getStartLocationQuoted(), thrownException); - } try { recoveredErrorStack.add(thrownException); visit(recoverySection); @@ -887,19 +882,22 @@ public final class Environment extends MutableProcessingConfiguration<Environmen } lastThrowable = templateException; - // Log the exception if we are inside an #attempt block; it has to be logged, as it certainly won't bubble up - // to the caller of FreeMarker. - if (LOG.isErrorEnabled() && isInAttemptBlock()) { - LOG.error("Error executing FreeMarker template", templateException); - } + try { + // Stop exception is not passed to the handler, but + // explicitly rethrown. + if (templateException instanceof StopException) { + throw templateException; + } - // Stop exception is not passed to the handler, but explicitly rethrown. - if (templateException instanceof StopException) { - throw templateException; + // Finally, pass the exception to the handler + getTemplateExceptionHandler().handleTemplateException(templateException, this, out); + } catch (TemplateException e) { + // Note that if the TemplateExceptionHandler doesn't rethrow the exception, we don't get in there. + if (isInAttemptBlock()) { + this.getAttemptExceptionReporter().report(templateException, this); + } + throw e; } - - // Finally, pass the exception to the handler - getTemplateExceptionHandler().handleTemplateException(templateException, this, out); } @Override @@ -914,6 +912,11 @@ public final class Environment extends MutableProcessingConfiguration<Environmen } @Override + protected AttemptExceptionReporter getDefaultAttemptExceptionReporter() { + return getMainTemplate().getAttemptExceptionReporter(); + } + + @Override protected ArithmeticEngine getDefaultArithmeticEngine() { return getMainTemplate().getArithmeticEngine(); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/LoggingAttemptExceptionReporter.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/LoggingAttemptExceptionReporter.java b/freemarker-core/src/main/java/org/apache/freemarker/core/LoggingAttemptExceptionReporter.java new file mode 100644 index 0000000..6f120f0 --- /dev/null +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/LoggingAttemptExceptionReporter.java @@ -0,0 +1,43 @@ +/* + * 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; + +/** + * Default {@link AttemptExceptionReporter} implementation, factored out from {@link AttemptExceptionReporter} so that + * we can have static field. + */ +class LoggingAttemptExceptionReporter implements AttemptExceptionReporter { + + private final boolean logAsWarn; + + public LoggingAttemptExceptionReporter(boolean logAsWarn) { + this.logAsWarn = logAsWarn; + } + + public void report(TemplateException te, Environment env) { + String message = "Error executing FreeMarker template part in the #attempt block"; + if (!logAsWarn) { + _CoreLogs.ATTEMPT.error(message, te); + } else { + _CoreLogs.ATTEMPT.warn(message, te); + } + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java b/freemarker-core/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java index efdb953..07ee06e 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java @@ -150,7 +150,14 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces public static final String TEMPLATE_EXCEPTION_HANDLER_KEY_CAMEL_CASE = "templateExceptionHandler"; /** Alias to the {@code ..._SNAKE_CASE} variation due to backward compatibility constraints. */ public static final String TEMPLATE_EXCEPTION_HANDLER_KEY = TEMPLATE_EXCEPTION_HANDLER_KEY_SNAKE_CASE; - + + /** Legacy, snake case ({@code like_this}) variation of the setting name. */ + public static final String ATTEMPT_EXCEPTION_REPORTER_KEY_SNAKE_CASE = "attempt_exception_reporter"; + /** Modern, camel case ({@code likeThis}) variation of the setting name. */ + public static final String ATTEMPT_EXCEPTION_REPORTER_KEY_CAMEL_CASE = "attemptExceptionReporter"; + /** Alias to the {@code ..._SNAKE_CASE} variation due to backward compatibility constraints. */ + public static final String ATTEMPT_EXCEPTION_REPORTER_KEY = ATTEMPT_EXCEPTION_REPORTER_KEY_SNAKE_CASE; + /** Legacy, snake case ({@code like_this}) variation of the setting name. */ public static final String ARITHMETIC_ENGINE_KEY_SNAKE_CASE = "arithmetic_engine"; /** Modern, camel case ({@code likeThis}) variation of the setting name. */ @@ -239,6 +246,7 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces // Must be sorted alphabetically! API_BUILTIN_ENABLED_KEY_SNAKE_CASE, ARITHMETIC_ENGINE_KEY_SNAKE_CASE, + ATTEMPT_EXCEPTION_REPORTER_KEY_SNAKE_CASE, AUTO_FLUSH_KEY_SNAKE_CASE, AUTO_IMPORT_KEY_SNAKE_CASE, AUTO_INCLUDE_KEY_SNAKE_CASE, @@ -265,6 +273,7 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces // Must be sorted alphabetically! API_BUILTIN_ENABLED_KEY_CAMEL_CASE, ARITHMETIC_ENGINE_KEY_CAMEL_CASE, + ATTEMPT_EXCEPTION_REPORTER_KEY_CAMEL_CASE, AUTO_FLUSH_KEY_CAMEL_CASE, AUTO_IMPORT_KEY_CAMEL_CASE, AUTO_INCLUDE_KEY_CAMEL_CASE, @@ -297,6 +306,7 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces private boolean sqlDateAndTimeTimeZoneSet; private String booleanFormat; private TemplateExceptionHandler templateExceptionHandler; + private AttemptExceptionReporter attemptExceptionReporter; private ArithmeticEngine arithmeticEngine; private Charset outputEncoding; private boolean outputEncodingSet; @@ -879,6 +889,47 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces } /** + * Setter pair of {@link #getAttemptExceptionReporter()} + */ + public void setAttemptExceptionReporter(AttemptExceptionReporter attemptExceptionReporter) { + _NullArgumentException.check("attemptExceptionReporter", attemptExceptionReporter); + this.attemptExceptionReporter = attemptExceptionReporter; + } + + /** + * Fluent API equivalent of {@link #setAttemptExceptionReporter(AttemptExceptionReporter)} + */ + public SelfT attemptExceptionReporter(AttemptExceptionReporter value) { + setAttemptExceptionReporter(value); + return self(); + } + + /** + * Resets the setting value as if it was never set (but it doesn't affect the value inherited from another + * {@link ProcessingConfiguration}). + */ + public void unsetAttemptExceptionReporter() { + attemptExceptionReporter = null; + } + + @Override + public AttemptExceptionReporter getAttemptExceptionReporter() { + return isAttemptExceptionReporterSet() + ? attemptExceptionReporter : getDefaultAttemptExceptionReporter(); + } + + /** + * Returns the value the getter method returns when the setting is not set (possibly by inheriting the setting value + * from another {@link ProcessingConfiguration}), or throws {@link CoreSettingValueNotSetException}. + */ + protected abstract AttemptExceptionReporter getDefaultAttemptExceptionReporter(); + + @Override + public boolean isAttemptExceptionReporterSet() { + return attemptExceptionReporter != null; + } + + /** * Setter pair of {@link #getArithmeticEngine()} */ public void setArithmeticEngine(ArithmeticEngine arithmeticEngine) { @@ -1407,9 +1458,18 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces * If the value does not contain dot, then it must be one of these predefined values (case insensitive): * {@code "rethrow"} (means {@link TemplateExceptionHandler#RETHROW}), * {@code "debug"} (means {@link TemplateExceptionHandler#DEBUG}), - * {@code "html_debug"} (means {@link TemplateExceptionHandler#HTML_DEBUG}), - * {@code "ignore"} (means {@link TemplateExceptionHandler#IGNORE}), + * {@code "htmlDebug"} (means {@link TemplateExceptionHandler#HTML_DEBUG}), + * {@code "ignore"} (means {@link TemplateExceptionHandler#IGNORE}), or * {@code "default"} (only allowed for {@link Configuration} instances) for the default. + * + * <li><p>{@code "attempt_exception_reporter"}: + * See {@link #setAttemptExceptionReporter(AttemptExceptionReporter)}. + * <br>String value: If the value contains dot, then it's interpreted as an <a href="#fm_obe">object builder + * expression</a>. + * If the value does not contain dot, then it must be one of these predefined values (case insensitive): + * {@code "logError"} (means {@link AttemptExceptionReporter#LOG_ERROR}), + * {@code "logWarn"} (means {@link AttemptExceptionReporter#LOG_WARN}), or + * {@code "default"} (only allowed for {@link Configuration} instances) for the default value. * * <li><p>{@code "arithmetic_engine"}: * See {@link #setArithmeticEngine(ArithmeticEngine)}. @@ -1819,6 +1879,27 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces setTemplateExceptionHandler((TemplateExceptionHandler) _ObjectBuilderSettingEvaluator.eval( value, TemplateExceptionHandler.class, false, _SettingEvaluationEnvironment.getCurrent())); } + } else if (ATTEMPT_EXCEPTION_REPORTER_KEY_SNAKE_CASE.equals(name) + || ATTEMPT_EXCEPTION_REPORTER_KEY_CAMEL_CASE.equals(name)) { + if (value.indexOf('.') == -1) { + if ("log_error".equalsIgnoreCase(value) || "logError".equals(value)) { + setAttemptExceptionReporter( + AttemptExceptionReporter.LOG_ERROR); + } else if ("log_warn".equalsIgnoreCase(value) || "logWarn".equals(value)) { + setAttemptExceptionReporter( + AttemptExceptionReporter.LOG_WARN); + } else if (DEFAULT_VALUE.equalsIgnoreCase(value) + && this instanceof Configuration.ExtendableBuilder) { + unsetAttemptExceptionReporter(); + } else { + throw new InvalidSettingValueException( + name, value, + "No such predefined template exception handler name"); + } + } else { + setTemplateExceptionHandler((TemplateExceptionHandler) _ObjectBuilderSettingEvaluator.eval( + value, TemplateExceptionHandler.class, false, _SettingEvaluationEnvironment.getCurrent())); + } } else if (ARITHMETIC_ENGINE_KEY_SNAKE_CASE.equals(name) || ARITHMETIC_ENGINE_KEY_CAMEL_CASE.equals(name)) { if (value.indexOf('.') == -1) { if ("bigdecimal".equalsIgnoreCase(value)) { http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/ProcessingConfiguration.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/ProcessingConfiguration.java b/freemarker-core/src/main/java/org/apache/freemarker/core/ProcessingConfiguration.java index 324bb3d..1002c02 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/ProcessingConfiguration.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/ProcessingConfiguration.java @@ -418,7 +418,10 @@ public interface ProcessingConfiguration { * Neither is it meant to be used to roll back the printed output. These should be solved outside template * processing when the exception raises from {@link Template#process(Object, Writer) Template.process}. * {@link TemplateExceptionHandler} meant to be used if you want to include special content <em>in</em> the template - * output, or if you want to suppress certain exceptions. + * output, or if you want to suppress certain exceptions. If you suppress an exception then it's the responsibility + * of the {@link TemplateExceptionHandler} to log the exception (if you want it to be logged). + * + * @see #getAttemptExceptionReporter() */ TemplateExceptionHandler getTemplateExceptionHandler(); @@ -430,6 +433,27 @@ public interface ProcessingConfiguration { boolean isTemplateExceptionHandlerSet(); /** + * Specifies how exceptions handled (and hence suppressed) by an {@code #attempt} blocks will be logged or otherwise + * reported. The default value is {@link AttemptExceptionReporter#LOG_ERROR}. + * + * <p>Note that {@code #attempt} is not supposed to be a general purpose error handler mechanism, like {@code try} + * is in Java. It's for decreasing the impact of unexpected errors, by making it possible that only part of the + * page is going down, instead of the whole page. But it's still an error, something that someone should fix. So the + * error should be reported, not just ignored in a custom {@link AttemptExceptionReporter}-s. + * + * <p>The {@link AttemptExceptionReporter} is not invoked if the {@link TemplateExceptionHandler} has + * suppressed the exception. + */ + AttemptExceptionReporter getAttemptExceptionReporter(); + + /** + * Tells if this setting is set directly in this object. If not, then depending on the implementing class, reading + * the setting mights returns a default value, or returns the value of the setting from a parent object, or throws + * a {@link CoreSettingValueNotSetException}. + */ + boolean isAttemptExceptionReporterSet(); + + /** * The arithmetic engine used to perform arithmetic operations. * Its {@link Configuration}-level default is {@link BigDecimalArithmeticEngine#INSTANCE}. * Note that this setting overlaps with {@link ParsingConfiguration#getArithmeticEngine()}. http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/Template.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/Template.java b/freemarker-core/src/main/java/org/apache/freemarker/core/Template.java index 0126d60..43e2902 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/Template.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/Template.java @@ -929,7 +929,8 @@ public class Template implements ProcessingConfiguration, CustomStateScope { @Override public TemplateExceptionHandler getTemplateExceptionHandler() { - return tCfg != null && tCfg.isTemplateExceptionHandlerSet() ? tCfg.getTemplateExceptionHandler() : cfg.getTemplateExceptionHandler(); + return tCfg != null && tCfg.isTemplateExceptionHandlerSet() ? tCfg.getTemplateExceptionHandler() + : cfg.getTemplateExceptionHandler(); } @Override @@ -938,6 +939,17 @@ public class Template implements ProcessingConfiguration, CustomStateScope { } @Override + public AttemptExceptionReporter getAttemptExceptionReporter() { + return tCfg != null && tCfg.isAttemptExceptionReporterSet() ? tCfg.getAttemptExceptionReporter() + : cfg.getAttemptExceptionReporter(); + } + + @Override + public boolean isAttemptExceptionReporterSet() { + return tCfg != null && tCfg.isAttemptExceptionReporterSet(); + } + + @Override public ArithmeticEngine getArithmeticEngine() { return tCfg != null && tCfg.isArithmeticEngineSet() ? tCfg.getArithmeticEngine() : cfg.getArithmeticEngine(); } http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/2548f5cd/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java ---------------------------------------------------------------------- diff --git a/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java b/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java index 60fa9ed..17583e5 100644 --- a/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java +++ b/freemarker-core/src/main/java/org/apache/freemarker/core/TemplateConfiguration.java @@ -62,6 +62,7 @@ public final class TemplateConfiguration implements ParsingAndProcessingConfigur private final boolean sqlDateAndTimeTimeZoneSet; private final String booleanFormat; private final TemplateExceptionHandler templateExceptionHandler; + private final AttemptExceptionReporter attemptExceptionReporter; private final ArithmeticEngine arithmeticEngine; private final Charset outputEncoding; private final boolean outputEncodingSet; @@ -100,7 +101,10 @@ public final class TemplateConfiguration implements ParsingAndProcessingConfigur sqlDateAndTimeTimeZoneSet = builder.isSQLDateAndTimeTimeZoneSet(); sqlDateAndTimeTimeZone = sqlDateAndTimeTimeZoneSet ? builder.getSQLDateAndTimeTimeZone() : null; booleanFormat = builder.isBooleanFormatSet() ? builder.getBooleanFormat() : null; - templateExceptionHandler = builder.isTemplateExceptionHandlerSet() ? builder.getTemplateExceptionHandler() : null; + templateExceptionHandler = builder.isTemplateExceptionHandlerSet() ? builder.getTemplateExceptionHandler() + : null; + attemptExceptionReporter = builder.isAttemptExceptionReporterSet() ? builder.getAttemptExceptionReporter() + : null; arithmeticEngine = builder.isArithmeticEngineSet() ? builder.getArithmeticEngine() : null; outputEncodingSet = builder.isOutputEncodingSet(); outputEncoding = outputEncodingSet ? builder.getOutputEncoding() : null; @@ -429,6 +433,19 @@ public final class TemplateConfiguration implements ParsingAndProcessingConfigur } @Override + public AttemptExceptionReporter getAttemptExceptionReporter() { + if (!isAttemptExceptionReporterSet()) { + throw new CoreSettingValueNotSetException("attemptExceptionReporter"); + } + return attemptExceptionReporter; + } + + @Override + public boolean isAttemptExceptionReporterSet() { + return attemptExceptionReporter != null; + } + + @Override public Charset getOutputEncoding() { if (!isOutputEncodingSet()) { throw new CoreSettingValueNotSetException(""); @@ -676,6 +693,11 @@ public final class TemplateConfiguration implements ParsingAndProcessingConfigur } @Override + protected AttemptExceptionReporter getDefaultAttemptExceptionReporter() { + throw new CoreSettingValueNotSetException("attemptExceptionReporter"); + } + + @Override protected ArithmeticEngine getDefaultArithmeticEngine() { throw new CoreSettingValueNotSetException("arithmeticEngine"); } @@ -822,6 +844,9 @@ public final class TemplateConfiguration implements ParsingAndProcessingConfigur if (tc.isTemplateExceptionHandlerSet()) { setTemplateExceptionHandler(tc.getTemplateExceptionHandler()); } + if (tc.isAttemptExceptionReporterSet()) { + setAttemptExceptionReporter(tc.getAttemptExceptionReporter()); + } if (tc.isTimeFormatSet()) { setTimeFormat(tc.getTimeFormat()); }