velocity-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From by...@apache.org
Subject svn commit: r741043 - in /velocity/engine/branches/2.0_Exp: src/java/org/apache/velocity/context/ src/java/org/apache/velocity/runtime/directive/ src/test/org/apache/velocity/test/ src/test/org/apache/velocity/test/issues/ test/templates/compare/
Date Thu, 05 Feb 2009 08:23:51 GMT
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:wyla@removeme.sci.fi">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



Mime
View raw message