Author: byron
Date: Thu Feb 5 08:23:50 2009
New Revision: 741043
URL: http://svn.apache.org/viewvc?rev=741043&view=rev
Log:
VELOCITY-687 Change macro pass-by-name behavior to pass-by-value
Removed:
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity630TestCase.java
Modified:
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/context/ProxyVMContext.java
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/BlockMacroTestCase.java
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/VMContextLocalscopeTestCase.java
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity62TestCase.java
velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro.cmp
velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro2.cmp
Modified:
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/context/ProxyVMContext.java
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/context/ProxyVMContext.java?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
---
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/context/ProxyVMContext.java
(original)
+++
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/context/ProxyVMContext.java
Thu Feb 5 08:23:50 2009
@@ -19,24 +19,9 @@
* under the License.
*/
-import java.io.StringWriter;
import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
import java.util.Map;
-import org.apache.velocity.app.event.EventCartridge;
-import org.apache.velocity.exception.MethodInvocationException;
-import org.apache.velocity.exception.VelocityException;
-import org.apache.velocity.runtime.Renderable;
-import org.apache.velocity.runtime.RuntimeServices;
-import org.apache.velocity.runtime.parser.ParserTreeConstants;
-import org.apache.velocity.runtime.parser.node.ASTReference;
-import org.apache.velocity.runtime.parser.node.Node;
-import org.apache.velocity.runtime.parser.node.ASTBlock;
-import org.apache.velocity.runtime.resource.Resource;
-import org.apache.velocity.util.introspection.IntrospectionCacheData;
-
/**
* Context for Velocity macro arguments.
*
@@ -45,110 +30,35 @@
* reduces memory allocation upon macro invocations.
* Since the macro AST is now shared and RuntimeMacro directive is used,
* the earlier implementation of precalculating VMProxyArgs would not work.
- *
- * See <a href="http://issues.apache.org/jira/browse/VELOCITY-607";>Issue
607</a>
- * for more info on this class.
- * @author <a href="mailto:[email protected]";>Jarkko Viinamaki</a>
- * @version $Id$
- * @since 1.6
*/
public class ProxyVMContext extends ChainedInternalContextAdapter
{
- /** container for our macro AST node arguments. Size must be power of 2. */
- Map vmproxyhash = new HashMap(8, 0.8f);
-
/** container for any local or constant macro arguments. Size must be
power of 2. */
Map localcontext = new HashMap(8, 0.8f);
-
- /** support for local context scope feature, where all references are
local */
- private boolean localContextScope;
-
- /** needed for writing log entries. */
- private RuntimeServices rsvc;
-
+
+ /** If we are operating in global or localscope */
+ boolean localscope = true;
+
/**
* @param inner Velocity context for processing
* @param rsvc RuntimeServices provides logging reference
* @param localContextScope if true, all references are set to be local
*/
- public ProxyVMContext(InternalContextAdapter inner,
- RuntimeServices rsvc,
- boolean localContextScope)
- {
- super(inner);
-
- this.localContextScope = localContextScope;
- this.rsvc = rsvc;
- }
-
- /**
- * Used to put Velocity macro arguments into this context.
- *
- * @param context rendering context
- * @param macroArgumentName name of the macro argument that we received
- * @param literalMacroArgumentName ".literal.$"+macroArgumentName
- * @param argumentValue actual value of the macro argument
- *
- * @throws MethodInvocationException
- */
- public void addVMProxyArg(InternalContextAdapter context,
- String macroArgumentName,
- String literalMacroArgumentName,
- Node argumentValue) throws
MethodInvocationException
- {
- if (isConstant(argumentValue))
- {
- localcontext.put(macroArgumentName, argumentValue.value(context));
- }
- else
- {
- vmproxyhash.put(macroArgumentName, argumentValue);
- localcontext.put(literalMacroArgumentName, argumentValue);
- }
- }
-
- /**
- * Used to put Velocity macro bodyContext arguments into this context.
- *
- * @param context rendering context
- * @param macroArgumentName name of the macro argument that we received
- * @param literalMacroArgumentName ".literal.$"+macroArgumentName
- * @param argumentValue actual value of the macro body
- *
- * @throws MethodInvocationException
- */
- public void addVMProxyArg(InternalContextAdapter context,
- String macroArgumentName,
- String literalMacroArgumentName,
- Renderable argumentValue) throws
MethodInvocationException
+ public ProxyVMContext(InternalContextAdapter global, boolean
localScopeContext)
{
- localcontext.put(macroArgumentName, argumentValue);
+ super(global instanceof ProxyVMContext ?
((ProxyVMContext)global).getGlobal() : global);
+ localscope = localScopeContext;
}
/**
- * AST nodes that are considered constants can be directly
- * saved into the context. Dynamic values are stored in
- * another argument hashmap.
- *
- * @param node macro argument as AST node
- * @return true if the node is a constant value
+ * Get the global context from this ProxyVMContext
+ * @return
*/
- private boolean isConstant(Node node)
+ private InternalContextAdapter getGlobal()
{
- switch (node.getType())
- {
- case ParserTreeConstants.JJTINTEGERRANGE:
- case ParserTreeConstants.JJTREFERENCE:
- case ParserTreeConstants.JJTOBJECTARRAY:
- case ParserTreeConstants.JJTMAP:
- case ParserTreeConstants.JJTSTRINGLITERAL:
- case ParserTreeConstants.JJTTEXT:
- return (false);
- default:
- return (true);
- }
+ return innerContext;
}
-
+
/**
* Impl of the Context.put() method.
*
@@ -158,7 +68,10 @@
*/
public Object put(final String key, final Object value)
{
- return put(key, value, localContextScope);
+ if (localscope)
+ return localcontext.put(key, value);
+ else
+ return super.put(key, value);
}
/**
@@ -171,30 +84,12 @@
*/
public Object localPut(final String key, final Object value)
{
- return put(key, value, true);
- }
-
- /**
- * Internal put method to select between local and global scope.
- *
- * @param key name of item to set
- * @param value object to set to key
- * @param forceLocal True forces the object into the local scope.
- * @return old stored object
- */
- protected Object put(final String key, final Object value, final boolean
forceLocal)
- {
- Object old = localcontext.put(key, value);
- if (!forceLocal)
- {
- old = super.put(key, value);
- }
- return old;
+ return put(key, value);
}
/**
* Implementation of the Context.get() method. First checks
- * localcontext, then arguments, then global context.
+ * localcontext, then global context.
*
* @param key name of item to get
* @return stored object or null
@@ -202,70 +97,15 @@
public Object get(String key)
{
Object o = localcontext.get(key);
- if (o != null)
- {
- return o;
- }
-
- Node astNode = (Node) vmproxyhash.get(key);
-
- if (astNode != null)
+ if (o == null)
{
- int type = astNode.getType();
-
- // if the macro argument (astNode) is a reference, we need to
evaluate it
- // in case it is a multilevel node
- if (type == ParserTreeConstants.JJTREFERENCE)
+ // Make sure this isn't the case of the key existing, but the
value is null
+ if (!localcontext.containsKey(key))
{
- ASTReference ref = (ASTReference) astNode;
-
- if (ref.jjtGetNumChildren() > 0)
- {
- return ref.execute(null, innerContext);
- }
- else
- {
- Object obj = innerContext.get(ref.getRootString());
- if (obj == null && ref.strictRef)
- {
- if (!innerContext.containsKey(ref.getRootString()))
- {
- throw new MethodInvocationException("Parameter '"
+ ref.getRootString()
- + "' not defined", null, key,
ref.getTemplateName(),
- ref.getLine(), ref.getColumn());
- }
- }
- return obj;
- }
- }
- else if (type == ParserTreeConstants.JJTTEXT)
- {
- // this really shouldn't happen. text is just a throwaway arg
for #foreach()
- try
- {
- StringWriter writer = new StringWriter();
- astNode.render(innerContext, writer);
- return writer.toString();
- }
- catch (RuntimeException e)
- {
- throw e;
- }
- catch (Exception e)
- {
- String msg = "ProxyVMContext.get() : error rendering
reference";
- rsvc.getLog().error(msg, e);
- throw new VelocityException(msg, e);
- }
- }
- else
- {
- // use value method to render other dynamic nodes
- return astNode.value(innerContext);
+ o = super.get(key);
}
}
-
- return super.get(key);
+ return o;
}
/**
@@ -273,8 +113,7 @@
*/
public boolean containsKey(Object key)
{
- return vmproxyhash.containsKey(key)
- || localcontext.containsKey(key)
+ return localcontext.containsKey(key)
|| super.containsKey(key);
}
@@ -283,18 +122,7 @@
*/
public Object[] getKeys()
{
- if (localcontext.isEmpty())
- {
- return vmproxyhash.keySet().toArray();
- }
- else if (vmproxyhash.isEmpty())
- {
- return localcontext.keySet().toArray();
- }
-
- HashSet keys = new HashSet(localcontext.keySet());
- keys.addAll(vmproxyhash.keySet());
- return keys.toArray();
+ return localcontext.keySet().toArray();
}
/**
@@ -302,20 +130,10 @@
*/
public Object remove(Object key)
{
- Object loc = localcontext.remove(key);
- Object glo = null;
-
- vmproxyhash.remove(key);
-
- if (!localContextScope)
- {
- glo = super.remove(key);
- }
- if (loc != null)
- {
- return loc;
- }
- return glo;
+ if (localscope)
+ return localcontext.remove(key);
+ else
+ return super.remove(key);
}
}
Modified:
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
---
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java
(original)
+++
velocity/engine/branches/2.0_Exp/src/java/org/apache/velocity/runtime/directive/VelocimacroProxy.java
Thu Feb 5 08:23:50 2009
@@ -22,7 +22,6 @@
import java.io.IOException;
import java.io.Writer;
-import org.apache.commons.lang.StringUtils;
import org.apache.velocity.context.InternalContextAdapter;
import org.apache.velocity.context.ProxyVMContext;
import org.apache.velocity.exception.MacroOverflowException;
@@ -33,8 +32,6 @@
import org.apache.velocity.runtime.RuntimeConstants;
import org.apache.velocity.runtime.RuntimeServices;
import org.apache.velocity.runtime.log.Log;
-import org.apache.velocity.runtime.parser.ParserTreeConstants;
-import org.apache.velocity.runtime.parser.node.ASTDirective;
import org.apache.velocity.runtime.parser.node.Node;
import org.apache.velocity.runtime.parser.node.SimpleNode;
@@ -153,7 +150,7 @@
// wrap the current context and add the macro arguments
// the creation of this context is a major bottleneck (incl 2x HashMap)
- final ProxyVMContext vmc = new ProxyVMContext(context, rsvc,
localContextScope);
+ final ProxyVMContext vmc = new ProxyVMContext(context,
localContextScope);
int callArguments = node.jjtGetNumChildren();
@@ -162,23 +159,16 @@
// the 0th element is the macro name
for (int i = 1; i < argArray.length && i <= callArguments; i++)
{
- /*
- * literalArgArray[i] is needed for "render literal if null"
functionality.
- * The value is used in ASTReference render-method.
- *
- * The idea is to avoid generating the literal until
absolutely necessary.
- *
- * This makes VMReferenceMungeVisitor obsolete and it would
not work anyway
- * when the macro AST is shared
- */
- vmc.addVMProxyArg(context, argArray[i], literalArgArray[i],
node.jjtGetChild(i - 1));
+ // Add argument name and value to the new macro context
+ vmc.put(argArray[i], node.jjtGetChild(i - 1).value(context));
}
}
- // if this macro was invoked by a call directive, we might have a body
AST here. Put it into context.
+ // if this macro was invoked by a call directive, we might have a body
AST here.
+ // Put it into context.
if( body != null )
{
- vmc.addVMProxyArg(context, bodyReference, "", body);
+ vmc.put(bodyReference, body);
}
/*
Modified:
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/BlockMacroTestCase.java
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/BlockMacroTestCase.java?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
---
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/BlockMacroTestCase.java
(original)
+++
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/BlockMacroTestCase.java
Thu Feb 5 08:23:50 2009
@@ -29,7 +29,6 @@
public BlockMacroTestCase(String name)
{
super(name);
- // DEBUG = true;
}
public void testMultipleBodyContentIncludes() throws Exception
Modified:
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/VMContextLocalscopeTestCase.java
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/VMContextLocalscopeTestCase.java?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
---
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/VMContextLocalscopeTestCase.java
(original)
+++
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/VMContextLocalscopeTestCase.java
Thu Feb 5 08:23:50 2009
@@ -48,7 +48,7 @@
VelocityContext base = new VelocityContext();
base.put("outsideVar", "value1");
- ProxyVMContext vm = new ProxyVMContext(new
InternalContextAdapterImpl(base), this.instance, true);
+ ProxyVMContext vm = new ProxyVMContext(new
InternalContextAdapterImpl(base), true);
vm.put("newLocalVar", "value2");
// New variable put doesn't leak
Modified:
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
---
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
(original)
+++
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
Thu Feb 5 08:23:50 2009
@@ -63,7 +63,7 @@
"$a"+
"#end"+
"#test( \"$i\" )$i";
- assertEvalEquals("012", template);
+ assertEvalEquals("001", template);
}
public void testForIrrationallyFearedRelatedPossibleProblem2()
@@ -75,7 +75,7 @@
"$a"+
"#end"+
"#test( \"$i\" )$i";
- assertEvalEquals("aa0", template);
+ assertEvalEquals("aa1", template);
}
public void testForIrrationallyFearedRelatedPossibleProblem3()
@@ -97,7 +97,7 @@
"$a"+
"#end"+
"#test( $i.plus() )$i";
- assertEvalEquals("012", template);
+ assertEvalEquals("001", template);
}
public void testForIrrationallyFearedRelatedPossibleProblem5()
Modified:
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity62TestCase.java
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity62TestCase.java?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
---
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity62TestCase.java
(original)
+++
velocity/engine/branches/2.0_Exp/src/test/org/apache/velocity/test/issues/Velocity62TestCase.java
Thu Feb 5 08:23:50 2009
@@ -45,20 +45,20 @@
String template = "#macro( outer )#set( $foo = 'bar' )#inner()#end"+
"#macro( inner )$foo#end"+
"#inner()#outer()#inner()";
- assertEvalEquals("foobarfoo", template);
+ assertEvalEquals("foofoofoo", template);
}
public void testRecursive()
{
context.put("i", new Integer(1));
- String template = "#macro( recurse )"+
+ String template = "#macro(recurse $i)"+
"$i"+
"#if( $i < 5 )"+
"#set( $i = $i + 1 )"+
- "#recurse()"+
+ "#recurse($i)"+
"#end"+
"#end"+
- "#recurse()";
+ "#recurse(1)";
assertEvalEquals("12345", template);
}
Modified:
velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro.cmp
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro.cmp?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
--- velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro.cmp
(original)
+++ velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro.cmp Thu
Feb 5 08:23:50 2009
@@ -36,7 +36,7 @@
And there was a bug where I wasn't getting the params right for the
use-instance :
-Arg
:>$jdom.getRootElement().getChild("properties").getChild("author").getTextTrim()<
+Arg :>$i<
String literals should work as you expect :
Arg :>stringliteral<
Modified:
velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro2.cmp
URL:
http://svn.apache.org/viewvc/velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro2.cmp?rev=741043&r1=741042&r2=741043&view=diff
==============================================================================
--- velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro2.cmp
(original)
+++ velocity/engine/branches/2.0_Exp/test/templates/compare/velocimacro2.cmp
Thu Feb 5 08:23:50 2009
@@ -7,13 +7,13 @@
Hello from foo_two : hi
- Hello from foo : $notincontext
- Hello from foo : $notincontext.getThing()
+ Hello from foo : $a
+ Hello from foo : $a
- $notincontext : no
- $notincontext.woogie() : no
+ $a : no
+ $a : no
bar : yes
