diff options
11 files changed, 275 insertions, 70 deletions
diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/DeprecatedRuntimeConstants.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/DeprecatedRuntimeConstants.java index 2d876919..3789a76e 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/DeprecatedRuntimeConstants.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/DeprecatedRuntimeConstants.java @@ -274,4 +274,12 @@ public interface DeprecatedRuntimeConstants */ String OLD_SPACE_GOBBLING = "space.gobbling"; + /** + * When displaying null or invalid non-quiet references, use the argument literal reference + * instead of the one in the macro block. Defaults to false. + * @since 2.1 + * @Deprecated since 2.2, see {@link RuntimeConstants#VM_ENABLE_BC_MODE} + **/ + String OLD_VM_ENABLE_BC_MODE = "velocimacro.arguments.preserve_literals"; + } diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/RuntimeConstants.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/RuntimeConstants.java index 9b640e8d..89d3c41f 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/RuntimeConstants.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/RuntimeConstants.java @@ -331,11 +331,21 @@ public interface RuntimeConstants extends DeprecatedRuntimeConstants String VM_ARGUMENTS_STRICT = "velocimacro.arguments.strict"; /** - * When displaying null or invalid non-quiet references, use the argument literal reference - * instead of the one in the macro block. Defaults to false. - * @since 2.1 - **/ - String VM_PRESERVE_ARGUMENTS_LITERALS = "velocimacro.arguments.preserve_literals"; + * This flag enable the 1.7 backward compatible mode for velocimacros (defaults to false): + * <ul> + * <li> + * preserve argument literals: when displaying null or invalid non-quiet references, + * use the argument literal reference instead of the one in the macro block. Defaults to false. + * </li> + * <li> + * use global values for missing arguments: when calling a macro with fewer arguments than declared, + * if those arguments don't have an explicit default value in the macro definition, default values will + * be looked for in the global context + * </li> + * </ul> + * @since 2.2 + */ + String VM_ENABLE_BC_MODE = "velocimacro.enable_bc_mode"; /** * Specify the maximum depth for macro calls diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java index e8467fcf..ee4bddd3 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java @@ -56,7 +56,7 @@ public class VelocimacroProxy extends Directive private boolean strictArguments; private int maxCallDepth; private String bodyReference; - private boolean preserveArgumentsLiterals; + private boolean enableBCmode; private static final Object NULL_VALUE_MARKER = new Object(); @@ -99,7 +99,7 @@ public class VelocimacroProxy extends Directive // for performance reasons we precache these strings - they are needed in // "render literal if null" functionality - if (preserveArgumentsLiterals) + if (enableBCmode) { literalArgArray = new String[macroArgs.size()]; for (int i = 0; i < macroArgs.size(); i++) @@ -160,7 +160,7 @@ public class VelocimacroProxy extends Directive // get name of the reference that refers to AST block passed to block macro call bodyReference = rsvc.getString(RuntimeConstants.VM_BODY_REFERENCE, "bodyContent"); - preserveArgumentsLiterals = rsvc.getBoolean(RuntimeConstants.VM_PRESERVE_ARGUMENTS_LITERALS, false); + enableBCmode = rsvc.getBoolean(RuntimeConstants.VM_ENABLE_BC_MODE, false); } /** @@ -247,9 +247,10 @@ public class VelocimacroProxy extends Directive { MacroArg macroArg = macroArgs.get(i); current = context.get(macroArg.name); - if (current == values[(i-1) * 2 + 1]) + Object given = values[(i-1) * 2 + 1]; + Object old = values[(i-1) * 2]; + if (current == given || current == null && given == NULL_VALUE_MARKER) { - Object old = values[(i-1) * 2]; if (old == null) { context.remove(macroArg.name); @@ -264,11 +265,11 @@ public class VelocimacroProxy extends Directive } } - if (preserveArgumentsLiterals) + if (enableBCmode) { /* allow for nested calls */ Deque<String> literalsStack = (Deque<String>)context.get(literalArgArray[i]); - if (literalsStack != null) /* may be null if argument was missing in macro call */ + if (literalsStack != null) /* shouldn't be null */ { literalsStack.removeFirst(); if (literalsStack.size() == 0) @@ -354,6 +355,8 @@ public class VelocimacroProxy extends Directive // Changed two dimensional array to single dimensional to optimize memory lookups Object[] values = new Object[macroArgs.size() * 2]; + boolean warnedMissingArguments = false; + // Move arguments into the macro's context. Start at one to skip macro name for (int i = 1; i < macroArgs.size(); i++) { @@ -394,18 +397,26 @@ public class VelocimacroProxy extends Directive } else { - // Backward compatibility logging, Mainly for MacroForwardDefinedTestCase - log.debug("VM #{}: too few arguments to macro. Wanted {} got {}", - macroArgs.get(0).name, macroArgs.size() - 1, callArgNum); - break; + if (!warnedMissingArguments) + { + // Backward compatibility logging, Mainly for MacroForwardDefinedTestCase + log.debug("VM #{}: too few arguments to macro. Wanted {} got {}", + macroArgs.get(0).name, macroArgs.size() - 1, callArgNum); + warnedMissingArguments = true; + } + if (enableBCmode) + { + // use the global context value as default + newVal = oldVal; + } } values[(i-1) * 2 + 1] = newVal; - /* when preserveArgumentsLiterals is true, we still store the actual reference passed to the macro + /* when enableBCmode is true, we still store the actual reference passed to the macro even if the value is not null, because *if* the argument is set to null *during* the macro rendering we still expect the passed argument literal to be displayed to be fully backward compatible. */ - if (preserveArgumentsLiterals && /* newVal == null && */ argNode != null) + if (enableBCmode && /* newVal == null && */ argNode != null) { /* allow nested macro calls for B.C. */ Deque<String> literalsStack = (Deque<String>)context.get(literalArgArray[i]); @@ -415,7 +426,7 @@ public class VelocimacroProxy extends Directive context.put(literalArgArray[i], literalsStack); } /* Reflects the strange 1.7 behavor... */ - if (argNode instanceof ASTReference || argNode instanceof ASTStringLiteral) + if (argNode != null && (argNode instanceof ASTReference || argNode instanceof ASTStringLiteral)) { literalsStack.addFirst(argNode.literal()); } diff --git a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java index 527de725..747780dd 100644 --- a/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java +++ b/velocity-engine-core/src/main/java/org/apache/velocity/runtime/parser/node/ASTReference.java @@ -157,7 +157,7 @@ public class ASTReference extends SimpleNode strictEscape = rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT_ESCAPE, false); strictRef = rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false); - lookupAlternateLiteral = rsvc.getBoolean(RuntimeConstants.VM_PRESERVE_ARGUMENTS_LITERALS, false); + lookupAlternateLiteral = rsvc.getBoolean(RuntimeConstants.VM_ENABLE_BC_MODE, false); /* * the only thing we can do in init() is getRoot() diff --git a/velocity-engine-core/src/main/resources/org/apache/velocity/runtime/defaults/velocity.properties b/velocity-engine-core/src/main/resources/org/apache/velocity/runtime/defaults/velocity.properties index 30119a24..3d96b636 100644 --- a/velocity-engine-core/src/main/resources/org/apache/velocity/runtime/defaults/velocity.properties +++ b/velocity-engine-core/src/main/resources/org/apache/velocity/runtime/defaults/velocity.properties @@ -118,14 +118,16 @@ velocimacro.arguments.strict = false velocimacro.body_reference = bodyContent # ---------------------------------------------------------------------------- -# VELOCIMACRO PRESERVE ARGUMENTS LITERALS +# VELOCIMACRO ENABLE BC MODE # ---------------------------------------------------------------------------- -# if true, when a macro has to render a null or invalid argument reference +# Backward compatibility for 1.7 macros behavior. +# If true, when a macro has to render a null or invalid argument reference # which is not quiet, it will print the provided literal reference instead -# of the one found in the body of the macro +# of the one found in the body of the macro ; and if a macro argument is +# without an explicit default value is missing from the macro call, its value +# will be looked up in the global context # ---------------------------------------------------------------------------- -velocimacro.arguments.preserve_literals = false - +velocimacro.enable_bc_mode = false # ---------------------------------------------------------------------------- # STRICT REFERENCE MODE diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/PreserveArgumentsLiteralsTestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/PreserveArgumentsLiteralsTestCase.java deleted file mode 100644 index ff627e18..00000000 --- a/velocity-engine-core/src/test/java/org/apache/velocity/test/PreserveArgumentsLiteralsTestCase.java +++ /dev/null @@ -1,45 +0,0 @@ -package org.apache.velocity.test; - -/* - * 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. - */ - -import org.apache.velocity.app.VelocityEngine; -import org.apache.velocity.runtime.RuntimeConstants; - -/** - * This class tests the mode where velocimacros do preserve arguments literals - */ - -public class PreserveArgumentsLiteralsTestCase extends BaseTestCase -{ - public PreserveArgumentsLiteralsTestCase(final String name) - { - super(name); - } - - protected void setUpEngine(VelocityEngine engine) - { - engine.setProperty(RuntimeConstants.VM_PRESERVE_ARGUMENTS_LITERALS, true); - } - - public void testPreserveLiterals() - { - assertEvalEquals("$bar","#macro(m $foo)$foo#end#m($bar)"); - } -} diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/VelocimacroBCModeTestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/VelocimacroBCModeTestCase.java new file mode 100644 index 00000000..72a27c15 --- /dev/null +++ b/velocity-engine-core/src/test/java/org/apache/velocity/test/VelocimacroBCModeTestCase.java @@ -0,0 +1,99 @@ +package org.apache.velocity.test; + +/* + * 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. + */ + +import org.apache.velocity.Template; +import org.apache.velocity.VelocityContext; +import org.apache.velocity.app.VelocityEngine; +import org.apache.velocity.runtime.RuntimeConstants; + +import java.io.BufferedWriter; +import java.io.FileOutputStream; +import java.io.OutputStreamWriter; +import java.io.Writer; + +/** + * This class tests the mode where velocimacros do preserve arguments literals + */ + +public class VelocimacroBCModeTestCase extends BaseTestCase +{ + private static final String BASE_DIR = TEST_COMPARE_DIR + "/bc_mode"; + private static final String CMP_DIR = BASE_DIR + "/compare"; + private static final String RESULTS_DIR = TEST_RESULT_DIR + "/bc_mode"; + + public VelocimacroBCModeTestCase(final String name) + { + super(name); + } + + protected void setUpEngine(VelocityEngine engine) + { + boolean bcMode = !getName().contains("NoPreserve"); + engine.setProperty(RuntimeConstants.VM_ENABLE_BC_MODE, bcMode); + engine.setProperty("file.resource.loader.path", TEST_COMPARE_DIR + "/bc_mode"); + } + + public void testPreserveLiterals() + { + assertEvalEquals("$bar","#macro(m $foo)$foo#end#m($bar)"); + } + + public void testGlobalDefaults() + { + assertEvalEquals("foo","#macro(m $foo)$foo#end#set($foo='foo')#m()"); + } + + public void testVariousCasesPreserve() throws Exception + { + doTestVariousCases("bc_mode_enabled"); + } + + public void testVariousCasesNoPreserve() throws Exception + { + doTestVariousCases("bc_mode_disabled"); + } + + private void doTestVariousCases(String compare_ext) throws Exception + { + assureResultsDirectoryExists(RESULTS_DIR); + String basefilename = "test_bc_mode"; + Template template = engine.getTemplate( getFileName(null, basefilename, "vtl") ); + context = new VelocityContext(); + FileOutputStream fos; + Writer fwriter; + + fos = new FileOutputStream (getFileName(RESULTS_DIR, basefilename, RESULT_FILE_EXT)); + + fwriter = new BufferedWriter( new OutputStreamWriter(fos) ); + + template.merge(context, fwriter); + fwriter.flush(); + fwriter.close(); + + if (!isMatch(RESULTS_DIR, CMP_DIR, basefilename, RESULT_FILE_EXT, compare_ext)) + { + String result = getFileContents(RESULTS_DIR, basefilename, RESULT_FILE_EXT); + String compare = getFileContents(CMP_DIR, basefilename, compare_ext); + + assertEquals(compare, result); + } + } +} diff --git a/velocity-engine-core/src/test/java/org/apache/velocity/test/issues/Velocity904TestCase.java b/velocity-engine-core/src/test/java/org/apache/velocity/test/issues/Velocity904TestCase.java index b22e198b..86caa7cf 100755 --- a/velocity-engine-core/src/test/java/org/apache/velocity/test/issues/Velocity904TestCase.java +++ b/velocity-engine-core/src/test/java/org/apache/velocity/test/issues/Velocity904TestCase.java @@ -35,6 +35,7 @@ public class Velocity904TestCase extends BaseTestCase @Override protected void setUpEngine(VelocityEngine engine) { + // that will also test the deprecation of velocimacro.arguments.preserve_literals towards velocimacro.enable_bc_mode engine.setProperty("velocimacro.arguments.preserve_literals", getName().contains("NoPreserve") ? "false" : "true"); } diff --git a/velocity-engine-core/src/test/resources/bc_mode/compare/test_bc_mode.bc_mode_disabled b/velocity-engine-core/src/test/resources/bc_mode/compare/test_bc_mode.bc_mode_disabled new file mode 100644 index 00000000..59d87824 --- /dev/null +++ b/velocity-engine-core/src/test/resources/bc_mode/compare/test_bc_mode.bc_mode_disabled @@ -0,0 +1,40 @@ + + + + +A) Null Values +1. missing argument +foo=$foo => disp miss [foo=$foo] => foo=$foo +foo=$foo => setn miss [foo=$foo] => foo=$foo +foo=$foo => setv miss [foo=inn] => foo=inn +2. null argument +foo=$foo => disp null [foo=$foo] => foo=$foo +foo=$foo => setn null [foo=$foo] => foo=$foo +foo=$foo => setv null [foo=inn] => foo=inn +3. non-colliding argument +foo=$foo => disp ncol [foo=$foo] => foo=$foo +foo=$foo => setn ncol [foo=$foo] => foo=$foo +foo=$foo => setv ncol [foo=inn] => foo=inn +4. colliding argument +foo=$foo => disp coll [foo=$foo] => foo=$foo +foo=$foo => setn coll [foo=$foo] => foo=$foo +foo=$foo => setv coll [foo=inn] => foo=inn + +B) Non-null Values +1. missing argument +foo=foo => disp miss [foo=$foo] => foo=foo +foo=foo => setn miss [foo=$foo] => foo=foo +foo=foo => setv miss [foo=inn] => foo=inn +2. null argument +foo=foo => disp null [foo=$foo] => foo=foo +foo=foo => setn null [foo=$foo] => foo=foo +foo=foo => setv null [foo=inn] => foo=inn +3. non-colliding argument +foo=foo => disp ncol [foo=bar] => foo=foo +foo=foo => setn ncol [foo=$foo] => foo=$foo +foo=foo => setv ncol [foo=bar] => foo=foo +4. colliding argument +foo=foo => disp coll [foo=foo] => foo=foo +foo=foo => setn coll [foo=$foo] => foo=$foo +foo=foo => setv coll [foo=foo] => foo=foo + diff --git a/velocity-engine-core/src/test/resources/bc_mode/compare/test_bc_mode.bc_mode_enabled b/velocity-engine-core/src/test/resources/bc_mode/compare/test_bc_mode.bc_mode_enabled new file mode 100644 index 00000000..934d659e --- /dev/null +++ b/velocity-engine-core/src/test/resources/bc_mode/compare/test_bc_mode.bc_mode_enabled @@ -0,0 +1,40 @@ + + + + +A) Null Values +1. missing argument +foo=$foo => disp miss [foo=$foo] => foo=$foo +foo=$foo => setn miss [foo=$foo] => foo=$foo +foo=$foo => setv miss [foo=inn] => foo=inn +2. null argument +foo=$foo => disp null [foo=$null] => foo=$foo +foo=$foo => setn null [foo=$foo] => foo=$foo +foo=$foo => setv null [foo=inn] => foo=inn +3. non-colliding argument +foo=$foo => disp ncol [foo=$bar] => foo=$foo +foo=$foo => setn ncol [foo=$foo] => foo=$foo +foo=$foo => setv ncol [foo=inn] => foo=inn +4. colliding argument +foo=$foo => disp coll [foo=$foo] => foo=$foo +foo=$foo => setn coll [foo=$foo] => foo=$foo +foo=$foo => setv coll [foo=inn] => foo=inn + +B) Non-null Values +1. missing argument +foo=foo => disp miss [foo=foo] => foo=foo +foo=foo => setn miss [foo=$foo] => foo=$foo +foo=foo => setv miss [foo=foo] => foo=foo +2. null argument +foo=foo => disp null [foo=$null] => foo=foo +foo=foo => setn null [foo=$foo] => foo=foo +foo=foo => setv null [foo=inn] => foo=inn +3. non-colliding argument +foo=foo => disp ncol [foo=bar] => foo=foo +foo=foo => setn ncol [foo=$foo] => foo=$foo +foo=foo => setv ncol [foo=bar] => foo=foo +4. colliding argument +foo=foo => disp coll [foo=foo] => foo=foo +foo=foo => setn coll [foo=$foo] => foo=$foo +foo=foo => setv coll [foo=foo] => foo=foo + diff --git a/velocity-engine-core/src/test/resources/bc_mode/test_bc_mode.vtl b/velocity-engine-core/src/test/resources/bc_mode/test_bc_mode.vtl new file mode 100644 index 00000000..8ab1050b --- /dev/null +++ b/velocity-engine-core/src/test/resources/bc_mode/test_bc_mode.vtl @@ -0,0 +1,39 @@ +#macro(store)#set($foo_ = $foo)#set($bar_ = $bar)#end## +#macro(reset)#set($foo = $foo_)#set($bar = $bar_)#end## +#macro(state)foo=$foo#end## + +#macro(disp $foo)[foo=$foo]#end +#macro(setn $foo)#set($foo=$null)#disp($foo)#end +#macro(setv $foo)#if(!$foo)#set($foo='inn')#end#disp($foo)#end +#macro(sub)#set($foo='sub')#end + +#macro(test) +1. missing argument +#store#state => disp miss #disp() => #state#reset +#store#state => setn miss #setn() => #state#reset +#store#state => setv miss #setv() => #state#reset +2. null argument +#store#state => disp null #disp($null) => #state#reset +#store#state => setn null #setn($null) => #state#reset +#store#state => setv null #setv($null) => #state#reset +3. non-colliding argument +#store#state => disp ncol #disp($bar) => #state#reset +#store#state => setn ncol #setn($bar) => #state#reset +#store#state => setv ncol #setv($bar) => #state#reset +4. colliding argument +#store#state => disp coll #disp($foo) => #state#reset +#store#state => setn coll #setn($foo) => #state#reset +#store#state => setv coll #setv($foo) => #state#reset +#end + + +A) Null Values +#set($foo = $null) +#set($bar = $null) +#test() + +B) Non-null Values +#set($foo = 'foo') +#set($bar = 'bar') +#test() + |