velocity-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nbu...@apache.org
Subject svn commit: r702218 - in /velocity/engine/trunk/src: java/org/apache/velocity/context/ java/org/apache/velocity/runtime/ java/org/apache/velocity/runtime/parser/node/ test/org/apache/velocity/test/
Date Mon, 06 Oct 2008 18:15:19 GMT
Author: nbubna
Date: Mon Oct  6 11:15:18 2008
New Revision: 702218

URL: http://svn.apache.org/viewvc?rev=702218&view=rev
Log:
VELOCITY-618 add support for a 'strict reference mode' for more rigorous template authoring
(patch and idea by Byron Foster)

Added:
    velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java 
 (with props)
Modified:
    velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java
    velocity/engine/trunk/src/java/org/apache/velocity/context/ProxyVMContext.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
    velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java

Modified: velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/context/AbstractContext.java Mon Oct
 6 11:15:18 2008
@@ -154,18 +154,13 @@
     public Object put(String key, Object value)
     {
         /*
-         * don't even continue if key or value is null
+         * don't even continue if key is null
          */
-
         if (key == null)
         {
             return null;
         }
-        else if (value == null)
-        {
-            return null;
-        }
-
+        
         return internalPut(key, value);
     }
 
@@ -219,7 +214,13 @@
             return false;
         }
 
-        return internalContainsKey(key);
+        boolean exists = internalContainsKey(key);
+        if (!exists && innerContext != null)
+        {
+            exists = innerContext.containsKey(key);
+        }
+        
+        return exists;
     }
 
     /**

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=702218&r1=702217&r2=702218&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 Oct
 6 11:15:18 2008
@@ -237,7 +237,17 @@
                 }
                 else
                 {
-                    return wrappedContext.get(ref.getRootString());
+                    Object obj = wrappedContext.get(ref.getRootString());
+                    if (obj == null && ref.strictRef)
+                    {
+                        if (!wrappedContext.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)
@@ -285,7 +295,9 @@
      */
     public boolean containsKey(Object key)
     {
-        return false;
+      return vmproxyhash.containsKey(key)
+          || localcontext.containsKey(key)
+          || super.containsKey(key);
     }
 
     /**

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java (original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/RuntimeConstants.java Mon Oct
 6 11:15:18 2008
@@ -54,6 +54,11 @@
     String RUNTIME_LOG_LOGSYSTEM_CLASS = "runtime.log.logsystem.class";
 
     /**
+     * Properties referenced in the template are required to exist the object
+     */
+    String RUNTIME_REFERENCES_STRICT = "runtime.references.strict";
+    
+    /**
      * @deprecated  This appears to have always been meaningless.
      */
     String RUNTIME_LOG_ERROR_STACKTRACE = "runtime.log.error.stacktrace";

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
(original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTIdentifier.java
Mon Oct  6 11:15:18 2008
@@ -26,6 +26,7 @@
 import org.apache.velocity.exception.MethodInvocationException;
 import org.apache.velocity.exception.TemplateInitException;
 import org.apache.velocity.exception.VelocityException;
+import org.apache.velocity.runtime.RuntimeConstants;
 import org.apache.velocity.runtime.parser.Parser;
 import org.apache.velocity.util.introspection.Info;
 import org.apache.velocity.util.introspection.IntrospectionCacheData;
@@ -55,6 +56,11 @@
      *  This is really immutable after the init, so keep one for this node
      */
     protected Info uberInfo;
+    
+    /**
+     * Indicates if we are running in strict reference mode.
+     */
+    protected boolean strictRef = false;
 
     /**
      * @param id
@@ -100,6 +106,8 @@
         uberInfo = new Info(context.getCurrentTemplateName(),
                 getLine(), getColumn());
 
+        strictRef = rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+        
         return data;
     }
 
@@ -170,7 +178,16 @@
 
         if (vg == null)
         {
-            return null;
+            if (strictRef)
+            {
+                throw new MethodInvocationException("Object '" + o.getClass().getName() +
             
+                    "' does not contain property '" + identifier + "'", null, identifier,
+                    uberInfo.getTemplateName(), uberInfo.getLine(), uberInfo.getColumn());
+            }
+            else
+            {
+                return null;
+            }
         }
 
         /*

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
(original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
Mon Oct  6 11:15:18 2008
@@ -28,6 +28,7 @@
 import org.apache.velocity.exception.MethodInvocationException;
 import org.apache.velocity.exception.TemplateInitException;
 import org.apache.velocity.exception.VelocityException;
+import org.apache.velocity.runtime.RuntimeConstants;
 import org.apache.velocity.runtime.parser.Parser;
 import org.apache.velocity.util.introspection.Info;
 import org.apache.velocity.util.introspection.IntrospectionCacheData;
@@ -57,6 +58,11 @@
     protected Info uberInfo;
 
     /**
+     * Indicates if we are running in strict reference mode.
+     */
+    protected boolean strictRef = false;
+
+    /**
      * @param id
      */
     public ASTMethod(int id)
@@ -107,6 +113,8 @@
         methodName = getFirstToken().image;
         paramCount = jjtGetNumChildren() - 1;
 
+        strictRef = rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+        
         return data;
     }
 
@@ -201,7 +209,24 @@
 
             if (method == null)
             {
-                return null;
+                if (strictRef)                  
+                {
+                    // Create a parameter list for the exception error message
+                    StringBuffer plist = new StringBuffer();
+                    for (int i=0; i<params.length; i++)
+                    {
+                      Class param = paramClasses[i];
+                      plist.append(param == null ? "null" : param.getName());
+                      if (i < params.length -1) plist.append(", ");
+                    }
+                    throw new MethodInvocationException("Object '" + o.getClass().getName()
+
+                      "' does not contain method " + methodName + "(" + plist + ")", 
+                      null, methodName, uberInfo.getTemplateName(), uberInfo.getLine(), uberInfo.getColumn());
+                }
+                else
+                {
+                    return null;
+                }
             }
         }
         catch( MethodInvocationException mie )

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
(original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
Mon Oct  6 11:15:18 2008
@@ -69,6 +69,11 @@
 
     private String literal = null;
 
+    /**
+     * Indicates if we are running in strict reference mode.
+     */
+    public boolean strictRef = false;
+    
     private int numChildren = 0;
 
     protected Info uberInfo;
@@ -142,10 +147,53 @@
         logOnNull =
             rsvc.getBoolean(RuntimeConstants.RUNTIME_LOG_REFERENCE_LOG_INVALID, true);
 
+        strictRef = rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+ 
+        /**
+         * In the case we are referencing a variable with #if($foo) or
+         * #if( ! $foo) then we allow variables to be undefined and we 
+         * set strictRef to false so that if the variable is undefined
+         * an exception is not thrown. 
+         */
+        if (strictRef && numChildren == 0)
+        {
+            logOnNull = false; // Strict mode allows nulls
+            
+            Node node = this.jjtGetParent();
+            if (node instanceof ASTNotNode     // #if( ! $foo)
+             || node instanceof ASTExpression  // #if( $foo )
+             || node instanceof ASTOrNode      // #if( $foo || ...
+             || node instanceof ASTAndNode)    // #if( $foo && ...
+            {
+                // Now scan up tree to see if we are in an If statement
+                while (node != null)
+                {
+                    if (node instanceof ASTIfStatement)
+                    {
+                       strictRef = false;
+                       break;
+                    }
+                    node = node.jjtGetParent();
+                }
+            }
+        }
+                
         return data;
     }
 
     /**
+     * Returns the template name, once init() has been called.
+     */
+    public String getTemplateName()
+    {
+        if (uberInfo == null)
+        {
+            return null;
+        }
+        return uberInfo.getTemplateName();
+    }
+
+    /**
      *  Returns the 'root string', the reference key
      * @return the root string.
      */
@@ -175,7 +223,7 @@
 
         Object result = getVariableValue(context, rootString);
 
-        if (result == null)
+        if (result == null && !strictRef)
         {
             return EventHandlerUtil.invalidGetMethod(rsvc, context, 
                     "$" + rootString, null, null, uberInfo);
@@ -200,9 +248,22 @@
             int failedChild = -1;
             for (int i = 0; i < numChildren; i++)
             {
+                if (strictRef && result == null)
+                {
+                    /**
+                     * At this point we know that an attempt is about to be made
+                     * to call a method or property on a null value.
+                     */
+                    String name = jjtGetChild(i).getFirstToken().image;
+                    throw new MethodInvocationException("Attempted to access '"  
+                        + name + "' on a null value", null, name, uberInfo.getTemplateName(),
+                        jjtGetChild(i).getLine(), jjtGetChild(i).getColumn());
+                  
+                }
                 previousResult = result;
                 result = jjtGetChild(i).execute(result,context);
-                if (result == null)
+                if (result == null && !strictRef)  // If strict and null then well
catch this
+                                                   // next time through the loop
                 {
                     failedChild = i;
                     break;
@@ -500,6 +561,14 @@
 
             if (result == null)
             {
+                if (strictRef)
+                {
+                    String name = jjtGetChild(i+1).getFirstToken().image;
+                    throw new MethodInvocationException("Attempted to access '"  
+                        + name + "' on a null value", null, name, uberInfo.getTemplateName(),
+                        jjtGetChild(i+1).getLine(), jjtGetChild(i+1).getColumn());
+                }            
+              
                 String msg = "reference set : template = "
                     + context.getCurrentTemplateName() +
                     " [line " + getLine() + ",column " +
@@ -525,7 +594,18 @@
                             value, uberInfo);
 
             if (vs == null)
-                return false;
+            {
+                if (strictRef)
+                {
+                    throw new MethodInvocationException("Object '" + result.getClass().getName()
+
+                       "' does not contain property '" + identifier + "'", null, identifier,
+                       uberInfo.getTemplateName(), uberInfo.getLine(), uberInfo.getColumn());
+                }
+                else
+                {
+                  return false;
+                }
+            }
 
             vs.invoke(result, value);
         }
@@ -770,7 +850,17 @@
      */
     public Object getVariableValue(Context context, String variable) throws MethodInvocationException
     {
-        return context.get(variable);
+        Object obj = context.get(variable);
+        if (strictRef && obj == null)
+        {
+          if (!context.containsKey(variable))
+          {
+            throw new MethodInvocationException("Variable '" + variable +
+                "' has not been set", null, identifier,
+                uberInfo.getTemplateName(), uberInfo.getLine(), uberInfo.getColumn());  
         
+          }
+        }
+        return obj;        
     }
 
 

Modified: velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java?rev=702218&r1=702217&r2=702218&view=diff
==============================================================================
--- velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
(original)
+++ velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTSetDirective.java
Mon Oct  6 11:15:18 2008
@@ -52,6 +52,11 @@
     protected Info uberInfo;
 
     /**
+     * Indicates if we are running in strict reference mode.
+     */
+    protected boolean strictRef = false;
+
+    /**
      * @param id
      */
     public ASTSetDirective(int id)
@@ -104,7 +109,9 @@
     
             logOnNull = rsvc.getBoolean(RuntimeConstants.RUNTIME_LOG_REFERENCE_LOG_INVALID,
true);
             allowNull = rsvc.getBoolean(RuntimeConstants.SET_NULL_ALLOWED, false);
-
+            strictRef = rsvc.getBoolean(RuntimeConstants.RUNTIME_REFERENCES_STRICT, false);
+            if (strictRef) allowNull = true;  // strictRef implies allowNull
+            
             /*
              *  grab this now.  No need to redo each time
              */
@@ -168,7 +175,7 @@
             }
         }
 
-        if ( value == null )
+        if ( value == null && !strictRef)
         {
             String rightReference = null;
             if (right instanceof ASTExpression)

Added: velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java
URL: http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java?rev=702218&view=auto
==============================================================================
--- velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java (added)
+++ velocity/engine/trunk/src/test/org/apache/velocity/test/StrictReferenceTestCase.java Mon
Oct  6 11:15:18 2008
@@ -0,0 +1,175 @@
+package org.apache.velocity.test;
+
+import org.apache.velocity.exception.MethodInvocationException;
+import org.apache.velocity.runtime.RuntimeConstants;
+
+/**
+ * Test strict reference mode turned on by the velocity property
+ * runtime.references.strict
+ */
+public class StrictReferenceTestCase extends BaseEvalTestCase
+{
+    public StrictReferenceTestCase(String name)
+    {
+        super(name);
+    }
+
+    public void setUp() throws Exception
+    {
+        super.setUp();
+        engine.setProperty(RuntimeConstants.RUNTIME_REFERENCES_STRICT, true);
+        context.put("NULL", null);
+        context.put("bar", null);
+        context.put("TRUE", Boolean.TRUE);
+    }
+
+
+    /**
+     * Test the modified behavior of #if in strict mode.  Mainly, that
+     * single variables references in #if statements use non strict rules
+     */
+    public void testIfStatement()
+    {
+        Fargo fargo = new Fargo();
+        fargo.next = new Fargo();
+        context.put("fargo", fargo);
+        assertEvalEquals("", "#if($bogus)xxx#end");
+        assertEvalEquals("xxx", "#if($fargo)xxx#end");
+        assertEvalEquals("", "#if( ! $fargo)xxx#end");
+        assertEvalEquals("xxx", "#if($bogus || $fargo)xxx#end");
+        assertEvalEquals("", "#if($bogus && $fargo)xxx#end");
+        assertEvalEquals("", "#if($fargo != $NULL && $bogus)xxx#end");
+        assertEvalEquals("xxx", "#if($fargo == $NULL || ! $bogus)xxx#end");
+        assertEvalEquals("xxx", "#if(! $bogus1 && ! $bogus2)xxx#end");
+        assertEvalEquals("xxx", "#if($fargo.prop == \"propiness\" && ! $bogus &&
$bar == $NULL)xxx#end");
+        assertEvalEquals("", "#if($bogus && $bogus.foo)xxx#end");
+
+        assertMethodEx("#if($bogus.foo)#end");
+        assertMethodEx("#if(!$bogus.foo)#end");
+    }
+    
+    
+    /**       
+     * We make sure that variables can actuall hold null
+     * values.
+     */
+    public void testAllowNullValues()
+        throws Exception
+    {
+        evaluate("$bar");
+        assertEvalEquals("true", "#if($bar == $NULL)true#end");
+        assertEvalEquals("false", "#set($foobar = $NULL)#if(!$foobar)false#end");
+        assertEvalEquals("13", "#set($list = [1, $NULL, 3])#foreach($item in $list)#if($item)$item#end#end");
+    }
+    
+    /**
+     * Test that variables references that have not been defined throw exceptions 
+     */
+    public void testStrictVariableRef()
+        throws Exception
+    {
+        // We expect a Method exception on the following
+        assertMethodEx("$bogus");
+        assertMethodEx("#macro(test)$bogus#end #test()");
+
+        assertMethodEx("#set($bar = $bogus)");
+
+        assertMethodEx("#if($bogus == \"bar\") #end");
+        assertMethodEx("#if($bogus != \"bar\") #end");
+        assertMethodEx("#if(\"bar\" == $bogus) #end");
+        assertMethodEx("#if($bogus > 1) #end");
+        assertMethodEx("#foreach($item in $bogus)#end");
+
+        // make sure no exceptions are thrown here    
+        evaluate("#set($foo = \"bar\") $foo");     
+        evaluate("#macro(test1 $foo1) $foo1 #end #test1(\"junk\")");
+        evaluate("#macro(test2) #set($foo2 = \"bar\") $foo2 #end #test2()");
+    }
+    
+    /**
+     * Test that exceptions are thrown when methods are called on
+     * references that contains objects that do not contains those
+     * methods.
+     */
+    public void testStrictMethodRef()
+    {
+        Fargo fargo = new Fargo();
+        fargo.next = new Fargo();
+        context.put("fargo", fargo);        
+
+        // Mainly want to make sure no exceptions are thrown here
+        assertEvalEquals("propiness", "$fargo.prop");
+        assertEvalEquals("$fargo.nullVal", "$fargo.nullVal");
+        assertEvalEquals("", "$!fargo.nullVal");
+        assertEvalEquals("propiness", "$fargo.next.prop");
+
+        assertMethodEx("$fargo.foobar");
+        assertMethodEx("$fargo.next.foobar");
+        assertMethodEx("$fargo.foobar()");
+        assertMethodEx("#set($fargo.next.prop = $TRUE)");
+        assertMethodEx("$fargo.next.setProp($TRUE)");
+    }
+  
+    /**
+     * Make sure exceptions are thrown when when we attempt to call
+     * methods on null values.
+     */
+    public void testStrictMethodOnNull()
+    {
+        Fargo fargo = new Fargo();
+        fargo.next = new Fargo();
+        context.put("fargo", fargo);
+
+        assertMethodEx("$NULL.bogus");
+        assertMethodEx("$fargo.nullVal.bogus");
+        assertMethodEx("$fargo.next.nullVal.bogus");
+        assertMethodEx("#if (\"junk\" == $fargo.nullVal.bogus)#end");
+        assertMethodEx("#if ($fargo.nullVal.bogus > 2)#end");
+        assertMethodEx("#set($fargo.next.nullVal.bogus = \"junk\")");
+        assertMethodEx("#set($foo = $NULL.bogus)");
+        assertMethodEx("#foreach($item in $fargo.next.nullVal.bogus)#end");
+
+        evaluate("$fargo.prop.toString()");
+        assertMethodEx("#set($fargo.prop = $NULL)$fargo.prop.next");
+
+        // make sure no exceptions are thrown here
+        evaluate("$fargo.next.next");
+        evaluate("$fargo.next.nullVal");
+        evaluate("#foreach($item in $fargo.nullVal)#end");
+    }
+        
+    /**
+     * Assert that we get a MethodInvocationException when calling evaluate
+     */
+    public void assertMethodEx(String template)
+    {
+        assertEvalException(template, MethodInvocationException.class);
+    }
+
+
+    public static class Fargo
+    {
+        String prop = "propiness";
+        Fargo next = null;
+      
+        public String getProp()
+        {
+            return prop;
+        }
+
+        public void setProp(String val)
+        {
+            this.prop = prop;
+        }
+
+        public String getNullVal()
+        {
+            return null;
+        }
+
+        public Fargo getNext()
+        {
+            return next;
+        }      
+    }  
+}

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

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

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



Mime
View raw message