This is an automated email from the ASF dual-hosted git repository.
ddekany pushed a commit to branch 2.3-gae
in repository https://gitbox.apache.org/repos/asf/freemarker.git
The following commit(s) were added to refs/heads/2.3-gae by this push:
new ec3fd7c9 Corrected/improved Javadoc related to classicCompatible mode.
Fixed issue where setClassicCompatibleAsInt did not mirror the value change in
the Properties object. Added more tests for classicCompatible=true behavior.
Some minor code cleanup (simplifications allowed by newer Java versions).
ec3fd7c9 is described below
commit ec3fd7c96c038f5b814df11b2ebd222dc5258a15
Author: ddekany <[email protected]>
AuthorDate: Tue Jul 9 18:16:38 2024 +0200
Corrected/improved Javadoc related to classicCompatible mode. Fixed issue
where setClassicCompatibleAsInt did not mirror the value change in the
Properties object. Added more tests for classicCompatible=true behavior. Some
minor code cleanup (simplifications allowed by newer Java versions).
---
.../main/java/freemarker/core/Configurable.java | 159 ++++++++++++---------
.../src/main/java/freemarker/core/EvalUtil.java | 2 +-
.../freemarker/core/ClassicCompatibleBugsTest.java | 59 --------
.../freemarker/core/ClassicCompatibleTest.java | 117 +++++++++++++++
4 files changed, 208 insertions(+), 129 deletions(-)
diff --git a/freemarker-core/src/main/java/freemarker/core/Configurable.java
b/freemarker-core/src/main/java/freemarker/core/Configurable.java
index 63b7e85c..f4204c20 100644
--- a/freemarker-core/src/main/java/freemarker/core/Configurable.java
+++ b/freemarker-core/src/main/java/freemarker/core/Configurable.java
@@ -455,7 +455,7 @@ public class Configurable {
cFormat = _TemplateAPI.getDefaultCFormat(incompatibleImprovements);
- classicCompatible = Integer.valueOf(0);
+ classicCompatible = 0;
properties.setProperty(CLASSIC_COMPATIBLE_KEY,
classicCompatible.toString());
templateExceptionHandler =
_TemplateAPI.getDefaultTemplateExceptionHandler(incompatibleImprovements);
@@ -485,8 +485,7 @@ public class Configurable {
apiBuiltinEnabled = Boolean.FALSE;
properties.setProperty(API_BUILTIN_ENABLED_KEY,
apiBuiltinEnabled.toString());
- logTemplateExceptions = Boolean.valueOf(
-
_TemplateAPI.getDefaultLogTemplateExceptions(incompatibleImprovements));
+ logTemplateExceptions =
_TemplateAPI.getDefaultLogTemplateExceptions(incompatibleImprovements);
properties.setProperty(LOG_TEMPLATE_EXCEPTIONS_KEY,
logTemplateExceptions.toString());
// outputEncoding and urlEscapingCharset defaults to null,
@@ -494,7 +493,7 @@ public class Configurable {
setBooleanFormat(BOOLEAN_FORMAT_LEGACY_DEFAULT);
- customAttributes = new HashMap();
+ customAttributes = new HashMap<>();
customDateFormats = Collections.emptyMap();
customNumberFormats = Collections.emptyMap();
@@ -564,96 +563,117 @@ public class Configurable {
void setParent(Configurable parent) {
this.parent = parent;
}
-
- /**
- * Toggles the "Classic Compatible" mode. For a comprehensive description
- * of this mode, see {@link #isClassicCompatible()}.
- */
- public void setClassicCompatible(boolean classicCompatibility) {
- this.classicCompatible = Integer.valueOf(classicCompatibility ? 1 : 0);
- properties.setProperty(CLASSIC_COMPATIBLE_KEY,
classicCompatibilityIntToString(classicCompatible));
- }
/**
- * Same as {@link #setClassicCompatible(boolean)}, but allows some extra
values.
- *
- * @param classicCompatibility {@code 0} means {@code false}, {@code 1}
means {@code true},
- * {@code 2} means {@code true} but with emulating bugs in early 2.x
classic-compatibility mode. Currently
- * {@code 2} affects how booleans are converted to string; with {@code
1} it's always {@code "true"}/{@code ""},
- * but with {@code 2} it's {@code "true"}/{@code "false"} for values
wrapped by {@link BeansWrapper} as then
- * {@link Boolean#toString()} prevails. Note that {@code
someBoolean?string} will always consistently format the
- * boolean according the {@code boolean_format} setting, just like in
FreeMarker 2.3 and later.
- */
- public void setClassicCompatibleAsInt(int classicCompatibility) {
- if (classicCompatibility < 0 || classicCompatibility > 2) {
- throw new IllegalArgumentException("Unsupported
\"classicCompatibility\": " + classicCompatibility);
- }
- this.classicCompatible = Integer.valueOf(classicCompatibility);
- }
-
- private String classicCompatibilityIntToString(Integer i) {
- if (i == null) return null;
- else if (i.intValue() == 0) return MiscUtil.C_FALSE;
- else if (i.intValue() == 1) return MiscUtil.C_TRUE;
- else return i.toString();
- }
-
- /**
- * Returns whether the engine runs in the "Classic Compatibile" mode.
- * When this mode is active, the engine behavior is altered in following
- * way: (these resemble the behavior of the 1.7.x line of FreeMarker
engine,
- * now named "FreeMarker Classic", hence the name).
+ * Sets if the template engine runs in the "classic compatible" mode; Do
NOT use this mode, unless you need it for
+ * backward compatibility with FreeMarker 1 era (now called FreeMarker
Classic) templates!
+ * To be more compatible with 1.7.x templates, we emulate behavior that's
otherwise undesirable/confusing. Hiding
+ * wrong variable names is the most obvious disadvantage, but some
actually want that. But there are other
+ * undesirable effects too. Because for 1.7.x every simple value was a
string, the comparison rules are quite
+ * problematic (see below). Also note that relative template paths doesn't
work as expected, as 1.7.x treated all
+ * template paths absolute.
+ *
+ * In case you just want to limit the effect of errors in templates,
consider using
+ * {@link #setTemplateExceptionHandler(TemplateExceptionHandler)} instead.
+ *
+ * <p>The list of behavioral changes in "classic compatible" mode are
these:
* <ul>
- * <li>handle undefined expressions gracefully. Namely when an expression
- * "expr" evaluates to null:
+ * <li>Handle undefined expressions gracefully. When expression "expr"
evaluates to {@code null}:
* <ul>
* <li>
- * in <code><assign varname=expr></code> directive,
+ * in <code><assign varName = expr></code> directive,
* or in <code>${expr}</code> directive,
- * or in {@code otherexpr == expr},
- * or in {@code otherexpr != expr},
+ * or in {@code otherExpr == expr},
+ * or in {@code otherExpr != expr},
* or in {@code hash[expr]},
* or in {@code expr[keyOrIndex]} (since 2.3.20),
* or in {@code expr.key} (since 2.3.20),
* then it's treated as empty string.
* </li>
- * <li>as argument of <code><list expr as item></code> or
+ * <li>as argument of <code><list expr as item></code> or
* <code><foreach item in expr></code>, the loop body is not
executed
* (as if it were a 0-length list)
* </li>
* <li>as argument of <code><if></code> directive, or on other
places where a
- * boolean expression is expected, it's treated as false
+ * boolean expression is expected, it's treated as {@code false}
* </li>
* </ul>
* </li>
* <li>Non-boolean models are accepted in <code><if></code>
directive,
* or as operands of logical operators. "Empty" models (zero-length
string,
- * empty sequence or hash) are evaluated as false, all others are
evaluated as
- * true.</li>
- * <li>When boolean value is treated as a string (i.e. output in
- * <code>${...}</code> directive, or concatenated with other string),
true
- * values are converted to string "true", false values are converted to
- * empty string. Except, if the value of the setting is {@code 2}, it will
be
- * formatted according the {@code boolean_format} setting, just like in
- * 2.3.20 and later.
- * </li>
- * <li>Scalar models supplied to <code><list></code> and
+ * empty sequence or hash) are treated as {@code false}, all others are
treated as
+ * {@code true}.</li>
+ * <li>When boolean value is treated as a string (i.e. output in
+ * <code>${...}</code> directive, or concatenated with other string),
{@code true}
+ * values are converted to string {@code "true"}, {@code false} values
are converted to
+ * empty string. Except, {@link #setClassicCompatibleAsInt(int)} can set
slightly different behavior; see
+ * there.</li>
+ * <li>Scalar models supplied to <code><list></code> and
* <code><foreach></code> are treated as a one-element list
consisting
* of the passed model.
* </li>
+ * <li>When comparing values of incompatible types, we convert all of them
to strings, and then compare them.
+ * This is a quite fragile mode of operation, as how numbers, dates,
etc. are converted to string depends on
+ * various settings, like {@code number_format}, {@code date_format},
{@code locale} etc. So for example, usually,
+ * {@code 1000} (the number) is equals to {@code "1000"} (the string),
but if the {@code number_format} or
+ * {@code locale} requires thousands grouping, then suddenly they aren't
equal, as then {@code 1000} is converted
+ * to something like {@code "1,000"}. Also note that {@code false} (the
boolean) is not equal to {@code "false"},
+ * if {@code false} is converted to the empty string, which is usually
the case (see above).
+ * </li>
* <li>Paths parameter of <code><include></code> will be interpreted
as
- * absolute path.
+ * absolute path.
* </li>
* </ul>
- * In all other aspects, the engine is a 2.1 engine even in compatibility
+ *
+ * <p>In all other aspects, the engine is a 2.1 engine even in
compatibility
* mode - you don't lose any of the new functionality by enabling it.
+ *
+ * @see #setClassicCompatibleAsInt(int)
+ */
+ public void setClassicCompatible(boolean classicCompatible) {
+ setClassicCompatibleAsInt(this.classicCompatible = classicCompatible ?
1 : 0);
+ }
+
+ /**
+ * Same as {@link #setClassicCompatible(boolean)}, but allows some extra
values.
+ *
+ * @param classicCompatible {@code 0} means {@code false}, {@code 1} means
{@code true},
+ * {@code 2} means {@code true} but with emulating bugs in early 2.x
classic-compatibility mode. Currently
+ * {@code 2} affects how booleans are converted to string
<em>implicitly</em> (as in <code>${aBoolean}</code>).
+ * (Explicit conversions like <code>${aBoolean?string}</code>, and
<code>${aBoolean?c}</code> always format as
+ * in non-classic mode regardless of this number.)
+ * With {@code 1} it's always {@code "true"}/{@code ""}.
+ * With {@code 2} it's {@code "true"}/{@code "false"} for values
wrapped by {@link BeansWrapper}, as then
+ * {@link Boolean#toString()} prevails.
+ * With {@code 3} implicit boolean conversion works like in
non-classic-compatible mode.
+ */
+ public void setClassicCompatibleAsInt(int classicCompatible) {
+ if (classicCompatible < 0 || classicCompatible > 2) {
+ throw new IllegalArgumentException("\"classicCompatible\" int
value out of range: " + classicCompatible);
+ }
+ this.classicCompatible = classicCompatible;
+ properties.setProperty(CLASSIC_COMPATIBLE_KEY,
classicCompatibilityIntToString(this.classicCompatible));
+ }
+
+ private String classicCompatibilityIntToString(Integer i) {
+ if (i == null) return null;
+ else if (i == 0) return MiscUtil.C_FALSE;
+ else if (i == 1) return MiscUtil.C_TRUE;
+ else return i.toString();
+ }
+
+ /**
+ * Getter pair of {@link #setClassicCompatible(boolean)}.
*/
public boolean isClassicCompatible() {
- return classicCompatible != null ? classicCompatible.intValue() != 0 :
parent.isClassicCompatible();
+ return classicCompatible != null ? classicCompatible != 0 :
parent.isClassicCompatible();
}
+ /**
+ * Getter pair of {@link #setClassicCompatibleAsInt(int)}
+ */
public int getClassicCompatibleAsInt() {
- return classicCompatible != null ? classicCompatible.intValue() :
parent.getClassicCompatibleAsInt();
+ return classicCompatible != null ? classicCompatible :
parent.getClassicCompatibleAsInt();
}
/**
@@ -1020,17 +1040,18 @@ public class Configurable {
* The string value for the boolean {@code true} and {@code false} values,
usually intended for human consumption
* (not for a computer language), separated with comma. For example,
{@code "yes,no"}. Note that white-space is
* significant, so {@code "yes, no"} is WRONG (unless you want that
leading space before "no"). Because the proper
- * way of formatting booleans depends on the context too much, it's
probably the best to leave this setting on its
+ * way of formatting booleans depends on the context too much, it's
probably best to leave this setting on its
* default, which will enforce explicit formatting, like
<code>${aBoolean?string('on', 'off')}</code>.
*
- * <p>For backward compatibility the default is {@code "true,false"}, but
using that value is denied for automatic
+ * <p>For backward compatibility, the default is {@code "true,false"}, but
using that value is denied for automatic
* boolean-to-string conversion, like <code>${myBoolean}</code> will fail
with it. If you generate the piece of
* output for "computer audience" as opposed to "human audience", then you
should write
* <code>${myBoolean?c}</code>, which will print {@code true} or {@code
false}. If you really want to always
- * format for computer audience, then it's might be reasonable to set this
setting to {@code c}.
+ * format for computer audience, then it's might be reasonable to set this
setting to {@code "c"} (which is like
+ * {@code "true,false"}, but works for automatic conversion).
*
- * <p>Note that automatic boolean-to-string conversion only exists since
FreeMarker 2.3.20. Earlier this setting
- * only influenced the result of {@code myBool?string}.
+ * <p>Note that implicit boolean-to-string conversion only exists since
FreeMarker 2.3.20. Earlier this setting
+ * only influenced the result of {@code myBool?string} (explicit
boolean-to-string conversion).
*/
public void setBooleanFormat(String booleanFormat) {
validateBooleanFormat(booleanFormat);
@@ -2681,7 +2702,7 @@ public class Configurable {
} else if (CLASSIC_COMPATIBLE_KEY_SNAKE_CASE.equals(name)
|| CLASSIC_COMPATIBLE_KEY_CAMEL_CASE.equals(name)) {
char firstChar;
- if (value != null && value.length() > 0) {
+ if (value != null && !value.isEmpty()) {
firstChar = value.charAt(0);
} else {
firstChar = 0;
@@ -2689,7 +2710,7 @@ public class Configurable {
if (Character.isDigit(firstChar) || firstChar == '+' ||
firstChar == '-') {
setClassicCompatibleAsInt(Integer.parseInt(value));
} else {
- setClassicCompatible(value != null ?
StringUtil.getYesNo(value) : false);
+ setClassicCompatible(value != null &&
StringUtil.getYesNo(value));
}
} else if (TEMPLATE_EXCEPTION_HANDLER_KEY_SNAKE_CASE.equals(name)
|| TEMPLATE_EXCEPTION_HANDLER_KEY_CAMEL_CASE.equals(name))
{
diff --git a/freemarker-core/src/main/java/freemarker/core/EvalUtil.java
b/freemarker-core/src/main/java/freemarker/core/EvalUtil.java
index 03755368..329fa691 100644
--- a/freemarker-core/src/main/java/freemarker/core/EvalUtil.java
+++ b/freemarker-core/src/main/java/freemarker/core/EvalUtil.java
@@ -507,7 +507,7 @@ class EvalUtil {
return booleanValue ? MiscUtil.C_TRUE : "";
} else if (compatMode == 2) {
if (tm instanceof BeanModel) {
- // In 2.1, bean-wrapped booleans where strings, so
that has overridden the boolean behavior:
+ // In 2.1, bean-wrapped booleans were strings, so that
has overridden the boolean behavior:
return
_BeansAPI.getAsClassicCompatibleString((BeanModel) tm);
} else {
return booleanValue ? MiscUtil.C_TRUE : "";
diff --git
a/freemarker-core/src/test/java/freemarker/core/ClassicCompatibleBugsTest.java
b/freemarker-core/src/test/java/freemarker/core/ClassicCompatibleBugsTest.java
deleted file mode 100644
index 610c9959..00000000
---
a/freemarker-core/src/test/java/freemarker/core/ClassicCompatibleBugsTest.java
+++ /dev/null
@@ -1,59 +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 freemarker.core;
-
-import java.io.IOException;
-import java.util.List;
-
-import org.junit.Test;
-
-import freemarker.template.Configuration;
-import freemarker.template.TemplateException;
-import freemarker.test.TemplateTest;
-
-public class ClassicCompatibleBugsTest extends TemplateTest {
- @Override
- protected Configuration createConfiguration() {
- Configuration conf = new Configuration(Configuration.VERSION_2_3_33);
- conf.setClassicCompatible(true);
- conf.setBooleanFormat("c");
- return conf;
- }
-
- /**
- * FREEMARKER-227
- */
- @Test
- public void testLenientValueComparisonInSeqBuiltIns() throws
TemplateException, IOException {
- addToDataModel("seq", List.of(1, "2", 3.0));
-
- assertOutput("${seq?seq_contains(1)?string}", "true");
- assertOutput("${seq?seq_contains('1')?string}", "true");
-
- assertOutput("${seq?seq_contains(2)?string}", "true");
- assertOutput("${seq?seq_contains('2')?string}", "true");
-
- assertOutput("${seq?seq_contains(3)?string}", "true");
- assertOutput("${seq?seq_contains('3')?string}", "true");
-
- assertOutput("${seq?seq_contains(4)?string}", "false");
- assertOutput("${seq?seq_contains('4')?string}", "false");
- }
-}
diff --git
a/freemarker-core/src/test/java/freemarker/core/ClassicCompatibleTest.java
b/freemarker-core/src/test/java/freemarker/core/ClassicCompatibleTest.java
new file mode 100644
index 00000000..c9cdd7be
--- /dev/null
+++ b/freemarker-core/src/test/java/freemarker/core/ClassicCompatibleTest.java
@@ -0,0 +1,117 @@
+/*
+ * 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 freemarker.core;
+
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
+
+import java.io.IOException;
+import java.util.List;
+
+import org.junit.Test;
+
+import freemarker.ext.beans.BeansWrapper;
+import freemarker.template.Configuration;
+import freemarker.template.TemplateException;
+import freemarker.template.TemplateModel;
+import freemarker.test.TemplateTest;
+
+/**
+ * Note that we also have {@code classic-compatible.ftl} in the "template test
suite", but now I prefer adding new test
+ * cases in Java instead.
+ */
+public class ClassicCompatibleTest extends TemplateTest {
+ @Override
+ protected Configuration createConfiguration() {
+ Configuration conf = new Configuration(Configuration.VERSION_2_3_33);
+ conf.setClassicCompatible(true);
+ return conf;
+ }
+
+ /**
+ * FREEMARKER-227
+ */
+ @Test
+ public void testLenientValueComparisonInSeqBuiltIns() throws
TemplateException, IOException {
+ addToDataModel("seq", List.of(1, "2", 3.0, true, "false"));
+
+ assertOutput("${seq?seq_contains(1)?c}", "true");
+ assertOutput("${seq?seq_contains('1')?c}", "true");
+
+ assertOutput("${seq?seq_contains(2)?c}", "true");
+ assertOutput("${seq?seq_contains('2')?c}", "true");
+
+ assertOutput("${seq?seq_contains(3)?c}", "true");
+ assertOutput("${seq?seq_contains('3')?c}", "true");
+
+ assertOutput("${seq?seq_contains(4)?c}", "false");
+ assertOutput("${seq?seq_contains('4')?c}", "false");
+
+ assertOutput("${seq?seq_contains('true')?c}", "true");
+ assertOutput("${seq?seq_contains(true)?c}", "true");
+
+ assertOutput("${seq?seq_contains('false')?c}", "true");
+ assertOutput("${seq?seq_contains(false)?c}", "false"); // Because
false is converted to ""
+
+ // These are actually undesirable/confusing, but I guess it was the
closes to 1.7.x behavior, and anyway the
+ // classicCompatible mode works like this for a very long time now:
+ getConfiguration().setNumberFormat("0.0");
+ assertOutput("${seq?seq_contains(2)?c}", "false");
+ assertOutput("${seq?seq_contains('2')?c}", "true");
+ assertOutput("${seq?seq_contains(3)?c}", "true");
+ assertOutput("${seq?seq_contains('3')?c}", "false");
+ assertOutput("${seq?seq_contains('3.0')?c}", "true");
+ }
+
+ @Test
+ public void testMissingValueBuiltIns() throws TemplateException,
IOException {
+ addToDataModel("nothing", TemplateModel.NOTHING);
+ assertOutput("[${missing}] [${missing!'-'}]", "[] [-]");
+ assertOutput("[${nothing}] [${nothing!'-'}]", "[] []");
+ }
+
+ @Test
+ public void testBooleanFormat() throws TemplateException, IOException {
+ Configuration conf = getConfiguration();
+ assertThat(conf.getClassicCompatibleAsInt(), equalTo(1));
+ assertThat(conf.getBooleanFormat(), equalTo("true,false"));
+ addToDataModel("beanTrue", new
BeansWrapper(Configuration.VERSION_2_3_33).wrap(true));
+ addToDataModel("beanFalse", new
BeansWrapper(Configuration.VERSION_2_3_33).wrap(false));
+
+ assertOutput("[${true}] [${false}]", "[true] []");
+ assertOutput("[${beanTrue}] [${beanFalse}]", "[true] []");
+ assertOutput("[${true?c}] [${false?c}]", "[true] [false]");
+ assertOutput("[${true?string}] [${false?string}]", "[true] [false]");
+
+ conf.setBooleanFormat("y,n");
+
+ assertOutput("[${true}] [${false}]", "[true] []");
+ assertOutput("[${beanTrue}] [${beanFalse}]", "[true] []");
+ assertOutput("[${true?c}] [${false?c}]", "[true] [false]");
+ assertOutput("[${true?string}] [${false?string}]", "[y] [n]");
+
+ conf.setClassicCompatibleAsInt(2);
+ assertOutput("[${true}] [${false}]", "[true] []");
+ assertOutput("[${beanTrue}] [${beanFalse}]", "[true] [false]");
+ assertOutput("[${true?c}] [${false?c}]", "[true] [false]");
+ assertOutput("[${true?string}] [${false?string}]", "[y] [n]");
+ }
+
+}
