Author: nbubna
Date: Fri Aug 15 17:54:35 2008
New Revision: 686428
URL: http://svn.apache.org/viewvc?rev=686428&view=rev
Log:
VELOCITY-531 make #if handle null values intuitively for ! == and != operations
Added:
velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
(with props)
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTEQNode.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTNENode.java
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTReference.java
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTEQNode.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTEQNode.java?rev=686428&r1=686427&r2=686428&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTEQNode.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTEQNode.java
Fri Aug 15 17:54:35 2008
@@ -85,24 +85,6 @@
Object right = jjtGetChild(1).value(context);
/*
- * they could be null if they are references and not in the context
- */
-
- if (left == null || right == null)
- {
- log.error((left == null ? "Left" : "Right")
- + " side ("
- + jjtGetChild( (left == null? 0 : 1) ).literal()
- + ") of '==' operation "
- + "has null value. "
- + "If a reference, it may not be in the context."
- + " Operation not possible. "
- + context.getCurrentTemplateName() + " [line " +
getLine()
- + ", column " + getColumn() + "]");
- return false;
- }
-
- /*
* convert to Number if applicable
*/
if (left instanceof TemplateNumber)
@@ -117,50 +99,67 @@
/*
* If comparing Numbers we do not care about the Class.
*/
-
if (left instanceof Number && right instanceof Number)
{
return MathUtils.compare( (Number)left, (Number)right) == 0;
}
-
-
- /**
- * assume that if one class is a subclass of the other
- * that we should use the equals operator
- */
-
- if (left.getClass().isAssignableFrom(right.getClass()) ||
- right.getClass().isAssignableFrom(left.getClass()) )
+ /**
+ * if both are not null, then assume that if one class
+ * is a subclass of the other that we should use the equals operator
+ */
+ if (left != null && right != null &&
+ (left.getClass().isAssignableFrom(right.getClass()) ||
+ right.getClass().isAssignableFrom(left.getClass())))
{
return left.equals( right );
}
- else
+
+ /*
+ * Ok, time to compare string values
+ */
+ left = (left == null) ? null : left.toString();
+ right = (right == null) ? null: right.toString();
+
+ if (left == null && right == null)
{
- /**
- * Compare the String representations
- */
- if ((left.toString() == null) || (right.toString() == null))
+ if (log.isDebugEnabled())
{
- boolean culprit = (left.toString() == null);
- log.error((culprit ? "Left" : "Right")
- + " string side "
- + "String representation ("
- + jjtGetChild((culprit ? 0 : 1) ).literal()
- + ") of '!=' operation has null value."
- + " Operation not possible. "
- + context.getCurrentTemplateName() + " [line " +
getLine()
- + ", column " + getColumn() + "]");
-
- return false;
+ log.debug("Both right (" + getLiteral(false) + " and left "
+ + getLiteral(true) + " sides of '==' operation
returned null."
+ + "If references, they may not be in the context."
+ + getLocation(context));
}
-
- else
+ return true;
+ }
+ else if (left == null || right == null)
+ {
+ if (log.isDebugEnabled())
{
- return left.toString().equals(right.toString());
+ log.debug((left == null ? "Left" : "Right")
+ + " side (" + getLiteral(left == null)
+ + ") of '==' operation has null value. If it is a "
+ + "reference, it may not be in the context or its "
+ + "toString() returned null. " + getLocation(context));
+
}
+ return false;
}
+ else
+ {
+ return left.equals(right);
+ }
+ }
+ private String getLiteral(boolean left)
+ {
+ return jjtGetChild(left ? 0 : 1).literal();
+ }
+
+ private String getLocation(InternalContextAdapter context)
+ {
+ return context.getCurrentTemplateName() + " [line " + getLine()
+ + ", column " + getColumn() + "]";
}
/**
Modified:
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTNENode.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTNENode.java?rev=686428&r1=686427&r2=686428&view=diff
==============================================================================
---
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTNENode.java
(original)
+++
velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTNENode.java
Fri Aug 15 17:54:35 2008
@@ -70,23 +70,6 @@
Object right = jjtGetChild(1).value( context );
/*
- * null check
- */
-
- if ( left == null || right == null)
- {
- log.error((left == null ? "Left" : "Right")
- + " side ("
- + jjtGetChild( (left == null? 0 : 1) ).literal()
- + ") of '!=' operation has null value."
- + " Operation not possible. "
- + context.getCurrentTemplateName() + " [line " +
getLine()
- + ", column " + getColumn() + "]");
- return false;
-
- }
-
- /*
* convert to Number if applicable
*/
if (left instanceof TemplateNumber)
@@ -106,43 +89,62 @@
return MathUtils.compare ( (Number)left,(Number)right) != 0;
}
-
- /**
- * assume that if one class is a subclass of the other
- * that we should use the equals operator
- */
-
- if (left.getClass().isAssignableFrom(right.getClass()) ||
- right.getClass().isAssignableFrom(left.getClass()) )
+ /**
+ * if both are not null, then assume that if one class
+ * is a subclass of the other that we should use the equals operator
+ */
+ if (left != null && right != null &&
+ (left.getClass().isAssignableFrom(right.getClass()) ||
+ right.getClass().isAssignableFrom(left.getClass())))
{
return !left.equals( right );
}
- else
+
+ /*
+ * Ok, time to compare string values
+ */
+ left = (left == null) ? null : left.toString();
+ right = (right == null) ? null: right.toString();
+
+ if (left == null && right == null)
{
- /**
- * Compare the String representations
- */
- if ((left.toString() == null) || (right.toString() == null))
+ if (log.isDebugEnabled())
{
- boolean culprit = (left.toString() == null);
- log.error((culprit ? "Left" : "Right")
- + " string side "
- + "String representation ("
- + jjtGetChild((culprit ? 0 : 1) ).literal()
- + ") of '!=' operation has null value."
- + " Operation not possible. "
- + context.getCurrentTemplateName() + " [line " +
getLine()
- + ", column " + getColumn() + "]");
- return false;
+ log.debug("Both right (" + getLiteral(false) + " and left "
+ + getLiteral(true) + " sides of '!=' operation
returned null."
+ + "If references, they may not be in the context."
+ + getLocation(context));
}
-
- else
+ return false;
+ }
+ else if (left == null || right == null)
+ {
+ if (log.isDebugEnabled())
{
- return !left.toString().equals(right.toString());
- }
+ log.debug((left == null ? "Left" : "Right")
+ + " side (" + getLiteral(left == null)
+ + ") of '!=' operation has null value. If it is a "
+ + "reference, it may not be in the context or its "
+ + "toString() returned null. " + getLocation(context));
+ }
+ return true;
}
+ else
+ {
+ return !left.equals(right);
+ }
+ }
+ private String getLiteral(boolean left)
+ {
+ return jjtGetChild(left ? 0 : 1).literal();
+ }
+
+ private String getLocation(InternalContextAdapter context)
+ {
+ return context.getCurrentTemplateName() + " [line " + getLine()
+ + ", column " + getColumn() + "]";
}
/**
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=686428&r1=686427&r2=686428&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
Fri Aug 15 17:54:35 2008
@@ -427,6 +427,10 @@
else
return false;
}
+ else if (value.toString() == null)
+ {
+ return false;
+ }
else
return true;
}
Added:
velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
URL:
http://svn.apache.org/viewvc/velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java?rev=686428&view=auto
==============================================================================
--- velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
(added)
+++ velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
Fri Aug 15 17:54:35 2008
@@ -0,0 +1,141 @@
+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 java.io.StringWriter;
+import java.lang.reflect.Array;
+import java.util.Arrays;
+import java.util.ArrayList;
+import java.util.List;
+import junit.framework.Test;
+import junit.framework.TestCase;
+import junit.framework.TestSuite;
+import org.apache.velocity.VelocityContext;
+import org.apache.velocity.app.VelocityEngine;
+import org.apache.velocity.runtime.RuntimeConstants;
+import org.apache.velocity.runtime.log.SystemLogChute;
+
+/**
+ * Used to check that nulls are properly handled in #if statements
+ */
+public class IfNullTestCase extends TestCase
+{
+ private VelocityEngine engine;
+ private VelocityContext context;
+
+ public IfNullTestCase(final String name)
+ {
+ super(name);
+ }
+
+ public static Test suite ()
+ {
+ return new TestSuite(IfNullTestCase.class);
+ }
+
+ public void setUp() throws Exception
+ {
+ engine = new VelocityEngine();
+
+ // make the engine's log output go to the test-report
+ SystemLogChute log = new SystemLogChute();
+ log.setEnabledLevel(SystemLogChute.INFO_ID);
+ log.setSystemErrLevel(SystemLogChute.WARN_ID);
+ engine.setProperty(RuntimeConstants.RUNTIME_LOG_LOGSYSTEM, log);
+
+ context = new VelocityContext();
+ context.put("nullToString", new NullToString());
+ context.put("notnull", new Object());
+ }
+
+ public void tearDown()
+ {
+ engine = null;
+ context = null;
+ }
+
+ public void testIfEquals()
+ {
+ // both null
+ assertEvalEquals("foo", "#if( $null == $otherNull )foo#{else}bar#end");
+ assertEvalEquals("foo", "#if( $null == $nullToString
)foo#{else}bar#end");
+ assertEvalEquals("foo", "#if( $nullToString == $null
)foo#{else}bar#end");
+ // left null, right not
+ assertEvalEquals("bar", "#if( $nullToString == $notnull
)foo#{else}bar#end");
+ assertEvalEquals("bar", "#if( $null == $notnull )foo#{else}bar#end");
+ // right null, left not
+ assertEvalEquals("bar", "#if( $notnull == $nullToString
)foo#{else}bar#end");
+ assertEvalEquals("bar", "#if( $notnull == $null )foo#{else}bar#end");
+ }
+
+ public void testIfNotEquals()
+ {
+ // both null
+ assertEvalEquals("bar", "#if( $null != $otherNull )foo#{else}bar#end");
+ assertEvalEquals("bar", "#if( $null != $nullToString
)foo#{else}bar#end");
+ assertEvalEquals("bar", "#if( $nullToString != $null
)foo#{else}bar#end");
+ // left null, right not
+ assertEvalEquals("foo", "#if( $nullToString != $notnull
)foo#{else}bar#end");
+ assertEvalEquals("foo", "#if( $null != $notnull )foo#{else}bar#end");
+ // right null, left not
+ assertEvalEquals("foo", "#if( $notnull != $nullToString
)foo#{else}bar#end");
+ assertEvalEquals("foo", "#if( $notnull != $null )foo#{else}bar#end");
+ }
+
+ public void testIfValue()
+ {
+ assertEvalEquals("bar", "#if( $null )foo#{else}bar#end");
+ assertEvalEquals("bar", "#if( $nullToString )foo#{else}bar#end");
+ assertEvalEquals("foo", "#if( !$null )foo#{else}bar#end");
+ assertEvalEquals("foo", "#if( !$nullToString )foo#{else}bar#end");
+ }
+
+ protected void assertEvalEquals(String expected, String template)
+ {
+ try
+ {
+ String result = evaluate(template);
+ assertEquals(expected, result);
+ }
+ catch (Exception e)
+ {
+ throw new RuntimeException(e);
+ }
+ }
+
+ private String evaluate(String template) throws Exception
+ {
+ StringWriter writer = new StringWriter();
+ // use template as its own name, since our templates are short
+ engine.evaluate(context, writer, template, template);
+ return writer.toString();
+ }
+
+ public static class NullToString
+ {
+ public String toString()
+ {
+ return null;
+ }
+ }
+
+}
+
+
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
------------------------------------------------------------------------------
svn:executable = *
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
------------------------------------------------------------------------------
svn:keywords = Revision
Propchange:
velocity/engine/trunk/src/test/org/apache/velocity/test/IfNullTestCase.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
