You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@velocity.apache.org by nb...@apache.org on 2008/12/08 22:45:50 UTC

svn commit: r724498 - in /velocity/engine/trunk/src: java/org/apache/velocity/context/ProxyVMContext.java test/org/apache/velocity/test/issues/Velocity615TestCase.java

Author: nbubna
Date: Mon Dec  8 13:45:50 2008
New Revision: 724498

URL: http://svn.apache.org/viewvc?rev=724498&view=rev
Log:
VELOCITY-615 clean up ProxyVMContext implementation and fix a few obscure bugs in the process

Added:
    velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java   (with props)
Modified:
    velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java

Modified: velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java?rev=724498&r1=724497&r2=724498&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java Mon Dec  8 13:45:50 2008
@@ -21,6 +21,7 @@
 
 import java.io.StringWriter;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 
@@ -57,9 +58,6 @@
     /** container for any local or constant macro arguments. Size must be power of 2. */
     Map localcontext = new HashMap(8, 0.8f);;
 
-    /** context that we are wrapping */
-    InternalContextAdapter wrappedContext;
-
     /** support for local context scope feature, where all references are local */
     private boolean localContextScope;
 
@@ -79,8 +77,6 @@
 
         this.localContextScope = localContextScope;
         this.rsvc = rsvc;
-
-        wrappedContext = inner;
     }
 
     /**
@@ -168,56 +164,28 @@
      */
     protected Object put(final String key, final Object value, final boolean forceLocal)
     {
-        Node astNode = (Node) vmproxyhash.get(key);
-
-        if (astNode != null)
-        {
-            if (astNode.getType() == ParserTreeConstants.JJTREFERENCE)
-            {
-                ASTReference ref = (ASTReference) astNode;
-
-                if (ref.jjtGetNumChildren() > 0)
-                    ref.setValue(wrappedContext, value);
-                else
-                    wrappedContext.put(ref.getRootString(), value);
-
-            }
-            else
-            {
-                rsvc.getLog().error("ProxyVMContext.put() : New value cannot be assigned to a constant: "
-                                    + key + " / " + get("$" + key + ".literal"));
-            }
-            return null;
-        }
-        else
+        Object old = localcontext.put(key, value);
+        if (!forceLocal)
         {
-            if (forceLocal)
-            {
-                return localcontext.put(key, value);
-            }
-            else
-            {
-                if (localcontext.containsKey(key))
-                {
-                    return localcontext.put(key, value);
-                }
-                else
-                {
-                    return super.put(key, value);
-                }
-            }
+            old = super.put(key, value);
         }
+        return old;
     }
 
     /**
-     * Implementation of the Context.get() method.
+     * Implementation of the Context.get() method.  First checks
+     * localcontext, then arguments, then global context.
      * 
      * @param key name of item to get
      * @return stored object or null
      */
     public Object get(String key)
     {
-        Object o = null;
+        Object o = localcontext.get(key);
+        if (o != null)
+        {
+            return o;
+        }
 
         Node astNode = (Node) vmproxyhash.get(key);
 
@@ -233,14 +201,14 @@
 
                 if (ref.jjtGetNumChildren() > 0)
                 {
-                    return ref.execute(null, wrappedContext);
+                    return ref.execute(null, innerContext);
                 }
                 else
                 {
-                    Object obj = wrappedContext.get(ref.getRootString());
+                    Object obj = innerContext.get(ref.getRootString());
                     if (obj == null && ref.strictRef)
                     {
-                        if (!wrappedContext.containsKey(ref.getRootString()))
+                        if (!innerContext.containsKey(ref.getRootString()))
                         {
                             throw new MethodInvocationException("Parameter '" + ref.getRootString() 
                                 + "' not defined", null, key, ref.getTemplateName(), 
@@ -256,8 +224,7 @@
                 try
                 {
                     StringWriter writer = new StringWriter();
-                    astNode.render(wrappedContext, writer);
-
+                    astNode.render(innerContext, writer);
                     return writer.toString();
                 }
                 catch (RuntimeException e)
@@ -274,20 +241,11 @@
             else
             {
                 // use value method to render other dynamic nodes
-                return astNode.value(wrappedContext);
-            }
-        }
-        else
-        {
-            o = localcontext.get(key);
-
-            if (o == null)
-            {
-                o = super.get(key);
+                return astNode.value(innerContext);
             }
         }
 
-        return o;
+        return super.get(key);
     }
 
     /**
@@ -305,7 +263,18 @@
      */
     public Object[] getKeys()
     {
-        return vmproxyhash.keySet().toArray();
+        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();
     }
 
     /**
@@ -313,24 +282,18 @@
      */
     public Object remove(Object key)
     {
-        if (vmproxyhash.containsKey(key))
+        Object loc = localcontext.remove(key);
+        Object arg = vmproxyhash.remove(key);
+        Object glo = null;
+        if (!localContextScope)
         {
-            return vmproxyhash.remove(key);
+            glo = super.remove(key);
         }
-        else
+        if (loc != null)
         {
-            if (localContextScope)
-            {
-                return localcontext.remove(key);
-            }
-
-            Object oldValue = localcontext.remove(key);
-            if (oldValue == null)
-            {
-                oldValue = super.remove(key);
-            }
-            return oldValue;
+            return loc;
         }
+        return glo;
     }
 
 }

Added: velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java?rev=724498&view=auto
==============================================================================
--- velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java (added)
+++ velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java Mon Dec  8 13:45:50 2008
@@ -0,0 +1,130 @@
+package org.apache.velocity.test.issues;
+
+/*
+ * 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.test.BaseEvalTestCase;
+import org.apache.velocity.VelocityContext;
+import org.apache.velocity.runtime.RuntimeConstants;
+
+/**
+ * This class tests VELOCITY-615.
+ */
+public class Velocity615TestCase extends BaseEvalTestCase
+{
+    public Velocity615TestCase(String name)
+    {
+       super(name);
+    }
+
+    public void setUp() throws Exception
+    {
+        super.setUp();
+        engine.setProperty("velocimacro.permissions.allow.inline", "true");
+        engine.setProperty("velocimacro.permissions.allow.inline.to.replace.global", "false");
+        engine.setProperty("velocimacro.permissions.allow.inline.local.scope", "true");
+        engine.setProperty("velocimacro.context.localscope", "false");
+        engine.setProperty("velocimacro.arguments.strict", "true");
+    }
+
+    public void testIt()
+    {
+        String template = "#set( $foo = 'old' )"+
+                          "#macro( test $foo )"+
+                            "#set( $foo = \"new $foo \" )"+
+                            "$foo"+
+                          "#end"+
+                          "#test( 'foo' )"+
+                          "$foo";
+        assertEvalEquals("new foo new foo ", template);
+    }
+
+    public void testForIrrationallyFearedRelatedPossibleProblem()
+    {
+        context.put("i", new Inc());
+        String template = "#macro( test $a )"+
+                            "$a"+
+                            "$a"+
+                          "#end"+
+                          "#test( \"$i\" )$i";
+        assertEvalEquals("012", template);
+    }
+
+    public void testForIrrationallyFearedRelatedPossibleProblem2()
+    {
+        context.put("i", new Inc());
+        String template = "#macro( test $a )"+
+                            "#set( $a = 'a' )"+
+                            "$a"+
+                            "$a"+
+                          "#end"+
+                          "#test( \"$i\" )$i";
+        assertEvalEquals("aa0", template);
+    }
+
+    public void testForIrrationallyFearedRelatedPossibleProblem3()
+    {
+        context.put("i", new Inc());
+        String template = "#macro( test $a )"+
+                            "$a"+
+                            "$a"+
+                          "#end"+
+                          "#test( $i )$i";
+        assertEvalEquals("012", template);
+    }
+
+    public void testForIrrationallyFearedRelatedPossibleProblem4()
+    {
+        context.put("i", new Inc());
+        String template = "#macro( test $a )"+
+                            "$a"+
+                            "$a"+
+                          "#end"+
+                          "#test( $i.plus() )$i";
+        assertEvalEquals("012", template);
+    }
+
+    public void testForIrrationallyFearedRelatedPossibleProblem5()
+    {
+        context.put("i", new Inc());
+        String template = "#macro( test $a )"+
+                            "#set( $a = $i )"+
+                            "$a"+
+                            "$a"+
+                          "#end"+
+                          "#test( 'a' )$i";
+        assertEvalEquals("012", template);
+    }
+
+    public static class Inc
+    {
+        private int i=0;
+
+        public int plus()
+        {
+            return i++;
+        }
+
+        public String toString()
+        {
+            return String.valueOf(i++);
+        }
+    }
+
+}

Propchange: velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
------------------------------------------------------------------------------
    svn:executable = *

Propchange: velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
------------------------------------------------------------------------------
    svn:keywords = Revision

Propchange: velocity/engine/trunk/src/test/org/apache/velocity/test/issues/Velocity615TestCase.java
------------------------------------------------------------------------------
    svn:mime-type = text/plain