Repository: incubator-freemarker Updated Branches: refs/heads/3 ee44af74e -> c203787e1
Cleaned up setting value error handling; now we always throw ConfigurationSettingValueException. Thus MutableProcessingConfiguration.getEnvironment could be removed too. Project: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/commit/c203787e Tree: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/tree/c203787e Diff: http://git-wip-us.apache.org/repos/asf/incubator-freemarker/diff/c203787e Branch: refs/heads/3 Commit: c203787e1ca4f7a43a5d65a61c89393de7ffc9ab Parents: ee44af7 Author: ddekany <[email protected]> Authored: Sat Apr 15 14:44:32 2017 +0200 Committer: ddekany <[email protected]> Committed: Sat Apr 15 14:44:32 2017 +0200 ---------------------------------------------------------------------- .../apache/freemarker/core/Configuration.java | 50 ++++++++++++-------- .../ConfigurationSettingValueException.java | 47 ++++++++++++++++++ ...onfigurationSettingValueStringException.java | 38 --------------- .../core/MutableProcessingConfiguration.java | 41 ++++++---------- .../freemarker/core/ConfigurationTest.java | 14 +++--- 5 files changed, 97 insertions(+), 93 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/c203787e/src/main/java/org/apache/freemarker/core/Configuration.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/Configuration.java b/src/main/java/org/apache/freemarker/core/Configuration.java index f4f0242..ce53af7 100644 --- a/src/main/java/org/apache/freemarker/core/Configuration.java +++ b/src/main/java/org/apache/freemarker/core/Configuration.java @@ -2457,7 +2457,8 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else if ("disable".equals(value)) { setAutoEscapingPolicy(DISABLE_AUTO_ESCAPING_POLICY); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException( name, value, + "No such predefined auto escaping policy name"); } } else if (OUTPUT_FORMAT_KEY_SNAKE_CASE.equals(name) || OUTPUT_FORMAT_KEY_CAMEL_CASE.equals(name)) { if (value.equalsIgnoreCase(DEFAULT_VALUE)) { @@ -2472,9 +2473,8 @@ public final class Configuration extends MutableProcessingConfiguration<Configur value, List.class, true, _SettingEvaluationEnvironment.getCurrent()); for (Object item : list) { if (!(item instanceof OutputFormat)) { - throw new _MiscTemplateException(getEnvironment(), - "Invalid value for setting ", new _DelayedJQuote(name), ": List items must be " - + OutputFormat.class.getName() + " intances, in: ", value); + throw new ConfigurationSettingValueException(name, value, + "List items must be " + OutputFormat.class.getName() + " instances."); } } setRegisteredCustomOutputFormats(list); @@ -2496,23 +2496,27 @@ public final class Configuration extends MutableProcessingConfiguration<Configur Iterator it = map.entrySet().iterator(); while (it.hasNext()) { Map.Entry ent = (Map.Entry) it.next(); - String pname = (String) ent.getKey(); - int pvalue; + String pName = (String) ent.getKey(); + int pValue; try { - pvalue = Integer.parseInt((String) ent.getValue()); + pValue = Integer.parseInt((String) ent.getValue()); } catch (NumberFormatException e) { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, + "Malformed integer number (shown quoted): " + _StringUtil.jQuote(ent.getValue())); } - if ("soft".equalsIgnoreCase(pname)) { - softSize = pvalue; - } else if ("strong".equalsIgnoreCase(pname)) { - strongSize = pvalue; + if ("soft".equalsIgnoreCase(pName)) { + softSize = pValue; + } else if ("strong".equalsIgnoreCase(pName)) { + strongSize = pValue; } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, + "Unsupported cache parameter name (shown quoted): " + + _StringUtil.jQuote(ent.getValue())); } } if (softSize == 0 && strongSize == 0) { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, + "Either cache soft- or strong size must be set and non-0."); } setCacheStorage(new MruCacheStorage(strongSize, softSize)); } else { @@ -2540,7 +2544,7 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else if (unit.equals("h")) { multipier = 1000 * 60 * 60; } else if (!unit.isEmpty()) { - throw invalidSettingValueException(name, value, + throw new ConfigurationSettingValueException(name, value, "Unrecognized time unit " + _StringUtil.jQuote(unit) + ". Valid units are: ms, s, m, h"); } else { multipier = 0; @@ -2548,7 +2552,7 @@ public final class Configuration extends MutableProcessingConfiguration<Configur int parsedValue = Integer.parseInt(valueWithoutUnit); if (multipier == 0 && parsedValue != 0) { - throw invalidSettingValueException(name, value, + throw new ConfigurationSettingValueException(name, value, "Time unit must be specified for a non-0 value (examples: 500 ms, 3 s, 2 m, 1 h)."); } @@ -2559,7 +2563,7 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else if ("static_text".equals(value) || "staticText".equals(value)) { setTemplateLanguage(TemplateLanguage.STATIC_TEXT); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, "Unsupported template language name"); } } else if (TAG_SYNTAX_KEY_SNAKE_CASE.equals(name) || TAG_SYNTAX_KEY_CAMEL_CASE.equals(name)) { if ("auto_detect".equals(value) || "autoDetect".equals(value)) { @@ -2569,7 +2573,7 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else if ("square_bracket".equals(value) || "squareBracket".equals(value)) { setTagSyntax(SQUARE_BRACKET_TAG_SYNTAX); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, "No such predefined tag syntax name"); } } else if (NAMING_CONVENTION_KEY_SNAKE_CASE.equals(name) || NAMING_CONVENTION_KEY_CAMEL_CASE.equals(name)) { if ("auto_detect".equals(value) || "autoDetect".equals(value)) { @@ -2579,7 +2583,8 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else if ("camel_case".equals(value) || "camelCase".equals(value)) { setNamingConvention(CAMEL_CASE_NAMING_CONVENTION); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, + "No such predefined naming convention name."); } } else if (TAB_SIZE_KEY_SNAKE_CASE.equals(name) || TAB_SIZE_KEY_CAMEL_CASE.equals(name)) { setTabSize(Integer.parseInt(value)); @@ -2610,7 +2615,8 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else if (value.equalsIgnoreCase("default_2_4_0")) { setTemplateNameFormat(DefaultTemplateNameFormat.INSTANCE); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException(name, value, + "No such predefined template name format"); } } else if (TEMPLATE_CONFIGURATIONS_KEY_SNAKE_CASE.equals(name) || TEMPLATE_CONFIGURATIONS_KEY_CAMEL_CASE.equals(name)) { @@ -2623,8 +2629,10 @@ public final class Configuration extends MutableProcessingConfiguration<Configur } else { unknown = true; } + } catch (ConfigurationSettingValueException e) { + throw e; } catch (Exception e) { - throw settingValueAssignmentException(name, value, e); + throw new ConfigurationSettingValueException(name, value, e); } if (unknown) { super.setSetting(name, value); http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/c203787e/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueException.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueException.java b/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueException.java new file mode 100644 index 0000000..c480418 --- /dev/null +++ b/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueException.java @@ -0,0 +1,47 @@ +/* + * 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 org.apache.freemarker.core.util._NullArgumentException; +import org.apache.freemarker.core.util._StringUtil; + +/** + * Thrown by {@link Configuration#setSetting(String, String)}; The setting name was recognized, but its value + * couldn't be parsed or the setting couldn't be set for some other reason. This exception should always have a + * cause exception. + */ +@SuppressWarnings("serial") +public class ConfigurationSettingValueException extends ConfigurationException { + + ConfigurationSettingValueException(String name, String value, Throwable cause) { + super(createMessage(name, value, "; see cause exception.", ""), cause); + _NullArgumentException.check("cause", cause); + } + + ConfigurationSettingValueException(String name, String value, String reason) { + super(createMessage(name, value, ", because: ", reason)); + _NullArgumentException.check("reason", reason); + } + + private static String createMessage(String name, String value, String detail1, String detail2) { + return "Failed to set FreeMarker configuration setting " + _StringUtil.jQuote(name) + + " to value " + _StringUtil.jQuote(value) + detail1 + detail2; + } + +} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/c203787e/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueStringException.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueStringException.java b/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueStringException.java deleted file mode 100644 index 9743c3b..0000000 --- a/src/main/java/org/apache/freemarker/core/ConfigurationSettingValueStringException.java +++ /dev/null @@ -1,38 +0,0 @@ -/* - * 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 org.apache.freemarker.core.util._NullArgumentException; -import org.apache.freemarker.core.util._StringUtil; - -/** - * Thrown by {@link Configuration#setSetting(String, String)}; The setting name was recognized, but its value - * couldn't be parsed or the setting couldn't be set for some other reason. This exception should always have a - * cause exception. - */ -@SuppressWarnings("serial") -public class ConfigurationSettingValueStringException extends ConfigurationException { - - ConfigurationSettingValueStringException(String name, String value, Throwable cause) { - super("Failed to set FreeMarker configuration setting " + _StringUtil.jQuote(name) - + " to value " + _StringUtil.jQuote(value) + "; see cause exception.", cause); - _NullArgumentException.check("cause", cause); - } - -} http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/c203787e/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java b/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java index bc037e5..a7daf31 100644 --- a/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java +++ b/src/main/java/org/apache/freemarker/core/MutableProcessingConfiguration.java @@ -1690,7 +1690,7 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces * @param value the string that describes the new value of the setting. * * @throws UnknownConfigurationSettingException if the name is wrong. - * @throws ConfigurationSettingValueStringException if the new value of the setting can't be set for any other reasons. + * @throws ConfigurationSettingValueException if the new value of the setting can't be set for any other reasons. */ public void setSetting(String name, String value) throws ConfigurationException { boolean unknown = false; @@ -1746,7 +1746,9 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces } else if (DEFAULT_VALUE.equalsIgnoreCase(value) && this instanceof Configuration) { ((Configuration) this).unsetTemplateExceptionHandler(); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException( + name, value, + "No such predefined template exception handler name"); } } else { setTemplateExceptionHandler((TemplateExceptionHandler) _ObjectBuilderSettingEvaluator.eval( @@ -1759,7 +1761,8 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces } else if ("conservative".equalsIgnoreCase(value)) { setArithmeticEngine(ConservativeArithmeticEngine.INSTANCE); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException( + name, value, "No such predefined arithmetical engine name"); } } else { setArithmeticEngine((ArithmeticEngine) _ObjectBuilderSettingEvaluator.eval( @@ -1810,7 +1813,7 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces } else if (segmentKey.equals(TRUSTED_TEMPLATES)) { trustedTemplates = segmentValue; } else { - throw new GenericParseException( + throw new ConfigurationSettingValueException(name, value, "Unrecognized list segment key: " + _StringUtil.jQuote(segmentKey) + ". Supported keys are: \"" + ALLOWED_CLASSES + "\", \"" + TRUSTED_TEMPLATES + "\""); @@ -1823,7 +1826,10 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces value, TemplateClassResolver.class, false, _SettingEvaluationEnvironment.getCurrent())); } else { - throw invalidSettingValueException(name, value); + throw new ConfigurationSettingValueException( + name, value, + "Not predefined class resolved name, nor follows class resolver definition syntax, nor " + + "looks like class name"); } } else if (LOG_TEMPLATE_EXCEPTIONS_KEY_SNAKE_CASE.equals(name) || LOG_TEMPLATE_EXCEPTIONS_KEY_CAMEL_CASE.equals(name)) { @@ -1840,8 +1846,10 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces } else { unknown = true; } + } catch (ConfigurationSettingValueException e) { + throw e; } catch (Exception e) { - throw settingValueAssignmentException(name, value, e); + throw new ConfigurationSettingValueException(name, value, e); } if (unknown) { throw unknownSettingException(name); @@ -1889,12 +1897,6 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces return tz; } - protected Environment getEnvironment() { - return this instanceof Environment - ? (Environment) this - : Environment.getCurrentEnvironment(); - } - /** * Creates the exception that should be thrown when a setting name isn't recognized. */ @@ -1925,21 +1927,6 @@ public abstract class MutableProcessingConfiguration<SelfT extends MutableProces return null; } - protected final ConfigurationSettingValueStringException settingValueAssignmentException( - String name, String value, Throwable cause) { - return new ConfigurationSettingValueStringException(name, value, cause); - } - - protected final TemplateException invalidSettingValueException(String name, String value) { - return invalidSettingValueException(name, value, null); - } - - protected final TemplateException invalidSettingValueException(String name, String value, String reason) { - return new _MiscTemplateException(getEnvironment(), - "Invalid value for setting ", new _DelayedJQuote(name), ": ", new _DelayedJQuote(value), - (reason != null ? ": " : null), (reason != null ? reason : null)); - } - /** * Set the settings stored in a <code>Properties</code> object. * http://git-wip-us.apache.org/repos/asf/incubator-freemarker/blob/c203787e/src/test/java/org/apache/freemarker/core/ConfigurationTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/freemarker/core/ConfigurationTest.java b/src/test/java/org/apache/freemarker/core/ConfigurationTest.java index b04e792..6d83128 100644 --- a/src/test/java/org/apache/freemarker/core/ConfigurationTest.java +++ b/src/test/java/org/apache/freemarker/core/ConfigurationTest.java @@ -666,7 +666,7 @@ public class ConfigurationTest extends TestCase { try { cfg.setSetting(Configuration.OUTPUT_FORMAT_KEY, "null"); - } catch (ConfigurationSettingValueStringException e) { + } catch (ConfigurationSettingValueException e) { assertThat(e.getCause().getMessage(), containsString(UndefinedOutputFormat.class.getSimpleName())); } } @@ -740,8 +740,8 @@ public class ConfigurationTest extends TestCase { try { cfg.setSetting(Configuration.REGISTERED_CUSTOM_OUTPUT_FORMATS_KEY_SNAKE_CASE, "[TemplateConfiguration()]"); fail(); - } catch (Exception e) { - assertThat(e.getCause().getMessage(), containsString(OutputFormat.class.getSimpleName())); + } catch (ConfigurationSettingValueException e) { + assertThat(e.getMessage(), containsString(OutputFormat.class.getSimpleName())); } } @@ -975,16 +975,16 @@ public class ConfigurationTest extends TestCase { try { cfg.setSetting(Configuration.TEMPLATE_UPDATE_DELAY_KEY, "5"); assertEquals(5000L, cfg.getTemplateUpdateDelayMilliseconds()); - } catch (ConfigurationSettingValueStringException e) { - assertThat(e.getCause().getMessage(), containsStringIgnoringCase("unit must be specified")); + } catch (ConfigurationSettingValueException e) { + assertThat(e.getMessage(), containsStringIgnoringCase("unit must be specified")); } cfg.setSetting(Configuration.TEMPLATE_UPDATE_DELAY_KEY, "0"); assertEquals(0L, cfg.getTemplateUpdateDelayMilliseconds()); try { cfg.setSetting(Configuration.TEMPLATE_UPDATE_DELAY_KEY, "5 foo"); assertEquals(5000L, cfg.getTemplateUpdateDelayMilliseconds()); - } catch (ConfigurationSettingValueStringException e) { - assertThat(e.getCause().getMessage(), containsStringIgnoringCase("\"foo\"")); + } catch (ConfigurationSettingValueException e) { + assertThat(e.getMessage(), containsStringIgnoringCase("\"foo\"")); } cfg.setSetting(Configuration.TEMPLATE_UPDATE_DELAY_KEY, "3 ms");
