You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by wg...@apache.org on 2006/09/28 22:19:33 UTC
svn commit: r451013 - in /jakarta/velocity/engine/trunk/src:
java/org/apache/velocity/runtime/parser/node/ASTMethod.java
test/org/apache/velocity/test/IntrospectionCacheDataTestCase.java
test/org/apache/velocity/test/MethodCacheKeyTestCase.java
Author: wglass
Date: Thu Sep 28 13:19:29 2006
New Revision: 451013
URL: http://svn.apache.org/viewvc?view=rev&rev=451013
Log:
Modified recent method cache patch to improve style and robustness. Revised equals & hashcode, added equals unit test. Related to VELOCITY-453.
Added:
jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java (with props)
Modified:
jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/IntrospectionCacheDataTestCase.java
Modified: jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java
URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java?view=diff&rev=451013&r1=451012&r2=451013
==============================================================================
--- jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java (original)
+++ jakarta/velocity/engine/trunk/src/java/org/apache/velocity/runtime/parser/node/ASTMethod.java Thu Sep 28 13:19:29 2006
@@ -1,7 +1,7 @@
package org.apache.velocity.runtime.parser.node;
/*
- * Copyright 2000-2004 The Apache Software Foundation.
+ * Copyright 2000-2006 The Apache Software Foundation.
*
* Licensed under the Apache License, Version 2.0 (the "License")
* you may not use this file except in compliance with the License.
@@ -46,7 +46,7 @@
*/
public class ASTMethod extends SimpleNode
{
- protected String methodName = "";
+ private String methodName = "";
private int paramCount = 0;
/**
@@ -143,7 +143,7 @@
* check the cache
*/
- MethodCacheKey mck = new MethodCacheKey(paramClasses);
+ MethodCacheKey mck = new MethodCacheKey(methodName, paramClasses);
IntrospectionCacheData icd = context.icacheGet( mck );
/*
@@ -315,35 +315,55 @@
/**
* Internal class used as key for method cache. Combines
- * ASTMethod fields with array of parameter classes.
+ * ASTMethod fields with array of parameter classes. Has
+ * public access (and complete constructor) for unit test
+ * purposes.
*/
- class MethodCacheKey
+ public class MethodCacheKey
{
- Class[] params;
+ private final String methodName;
+ private final Class[] params;
- MethodCacheKey(Class[] params)
+ public MethodCacheKey(String methodName, Class[] params)
{
+ this.methodName = methodName;
this.params = params;
}
- private final ASTMethod getASTMethod()
- {
- return ASTMethod.this;
- }
-
/**
* @see java.lang.Object#equals(java.lang.Object)
*/
public boolean equals(Object o)
{
+ /**
+ * null check is not strictly needed (due to sole
+ * initialization above) but it seems more safe
+ * to include it.
+ */
if (o instanceof MethodCacheKey)
{
- final MethodCacheKey other = (MethodCacheKey) o;
- if (params.length == other.params.length && methodName.equals(other.getASTMethod().methodName))
+ final MethodCacheKey other = (MethodCacheKey) o;
+ if ((params == null) || (other.params == null))
+ {
+ return params == other.params;
+ }
+
+ /**
+ * similarly, do null check on each array subscript.
+ */
+ else if (params.length == other.params.length &&
+ methodName.equals(other.methodName))
{
for (int i = 0; i < params.length; ++i)
{
- if (params[i] != other.params[i])
+ if (params[i] == null)
+ {
+ if (params[i] != other.params[i])
+ {
+ return false;
+ }
+ }
+ else if (!params[i].equals(other.params[i]))
{
return false;
}
@@ -358,19 +378,36 @@
/**
* @see java.lang.Object#hashCode()
*/
- public int hashCode()
+ public int hashCode()
{
- int result = 0;
- for (int i = 0; i < params.length; ++i)
+ int result = 17;
+
+ /**
+ * null check is not strictly needed (due to sole
+ * initialization above) but it seems more safe to
+ * include it.
+ */
+ if (params == null)
+ {
+ return result;
+ }
+
+ for (int i = 0; i < params.length; ++i)
{
final Class param = params[i];
if (param != null)
{
- result ^= param.hashCode();
+ result = result * 37 + param.hashCode();
}
}
- return result * 37 + methodName.hashCode();
- }
+
+ if (methodName != null)
+ {
+ result = result * 37 + methodName.hashCode();
+ }
+
+ return result;
+ }
}
}
Modified: jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/IntrospectionCacheDataTestCase.java
URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/IntrospectionCacheDataTestCase.java?view=diff&rev=451013&r1=451012&r2=451013
==============================================================================
--- jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/IntrospectionCacheDataTestCase.java (original)
+++ jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/IntrospectionCacheDataTestCase.java Thu Sep 28 13:19:29 2006
@@ -1,7 +1,7 @@
package org.apache.velocity.test;
/*
- * Copyright 2000-2004 The Apache Software Foundation.
+ * Copyright 2000-2006 The Apache Software Foundation.
*
* Licensed under the Apache License, Version 2.0 (the "License")
* you may not use this file except in compliance with the License.
@@ -19,6 +19,8 @@
import java.io.IOException;
import java.io.StringWriter;
+import junit.framework.TestCase;
+
import org.apache.velocity.VelocityContext;
import org.apache.velocity.app.Velocity;
import org.apache.velocity.exception.MethodInvocationException;
@@ -26,8 +28,6 @@
import org.apache.velocity.exception.ResourceNotFoundException;
import org.apache.velocity.util.introspection.IntrospectionCacheData;
-import junit.framework.TestCase;
-
/**
* Checks that arrays are cached correctly in the Introspector.
*
@@ -36,36 +36,43 @@
*/
public class IntrospectionCacheDataTestCase extends TestCase
{
-
- private static class CacheHitCountingVelocityContext extends VelocityContext
- {
- public int cacheHit = 0;
-
- public IntrospectionCacheData icacheGet(Object key)
+
+ private static class CacheHitCountingVelocityContext extends VelocityContext
{
- final IntrospectionCacheData result = super.icacheGet(key);
- if (result != null) {
- ++cacheHit;
- }
- return result;
+ public int cacheHit = 0;
+
+ public IntrospectionCacheData icacheGet(Object key)
+ {
+ final IntrospectionCacheData result = super.icacheGet(key);
+ if (result != null) {
+ ++cacheHit;
+ }
+ return result;
+ }
+
}
-
- }
-
- public void testCache() throws ParseErrorException, MethodInvocationException,
- ResourceNotFoundException, IOException
- {
- CacheHitCountingVelocityContext context = new CacheHitCountingVelocityContext();
- context.put("this", this);
- StringWriter w = new StringWriter();
- Velocity.evaluate(context, w, "test", "$this.exec('a')$this.exec('b')");
- assertEquals("[a][b]", w.toString());
- assertTrue(context.cacheHit > 0);
- }
-
- public String exec(String value)
- {
- return "[" + value + "]";
- }
-
+
+ public void testCache() throws ParseErrorException, MethodInvocationException,
+ ResourceNotFoundException, IOException
+ {
+ CacheHitCountingVelocityContext context = new CacheHitCountingVelocityContext();
+ context.put("this", this);
+ StringWriter w = new StringWriter();
+ Velocity.evaluate(context, w, "test", "$this.exec('a')$this.exec('b')");
+ assertEquals("[a][b]", w.toString());
+ assertTrue(context.cacheHit > 0);
+ }
+
+
+ /**
+ * For use when acting as a context reference.
+ *
+ * @param value
+ * @return
+ */
+ public String exec(String value)
+ {
+ return "[" + value + "]";
+ }
+
}
Added: jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java
URL: http://svn.apache.org/viewvc/jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java?view=auto&rev=451013
==============================================================================
--- jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java (added)
+++ jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java Thu Sep 28 13:19:29 2006
@@ -0,0 +1,95 @@
+package org.apache.velocity.test;
+
+/*
+ * Copyright 2000-2006 The Apache Software Foundation.
+ *
+ * Licensed 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 junit.framework.TestCase;
+
+import org.apache.commons.lang.ArrayUtils;
+import org.apache.velocity.runtime.parser.node.ASTMethod;
+
+/**
+ * Checks that the equals method works correctly when caching method keys.
+ *
+ * @author <a href="Will Glass-Husain">wglass@forio.com</a>
+ * @version $Id$
+ */
+public class MethodCacheKeyTestCase extends TestCase
+{
+
+ public void testMethodKeyCacheEquals()
+ {
+ Class [] elements1 = new Class [] { Object.class };
+ ASTMethod m1 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck1 = m1.new MethodCacheKey("test",elements1);
+
+ selfEqualsAssertions(mck1);
+
+ Class [] elements2 = new Class [] { Object.class };
+ ASTMethod m2 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck2 = m2.new MethodCacheKey("test",elements2);
+
+ assertTrue(mck1.equals(mck2));
+
+ Class [] elements3 = new Class [] { String.class };
+ ASTMethod m3 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck3 = m3.new MethodCacheKey("test",elements3);
+
+ assertFalse(mck1.equals(mck3));
+
+ Class [] elements4 = new Class [] { Object.class };
+ ASTMethod m4 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck4 = m4.new MethodCacheKey("boo",elements4);
+
+ assertFalse(mck1.equals(mck4));
+
+ /** check for potential NPE's **/
+ Class [] elements5 = ArrayUtils.EMPTY_CLASS_ARRAY;
+ ASTMethod m5 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck5 = m5.new MethodCacheKey("boo",elements5);
+ selfEqualsAssertions(mck5);
+
+ Class [] elements6 = null;
+ ASTMethod m6 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck6 = m6.new MethodCacheKey("boo",elements6);
+ selfEqualsAssertions(mck6);
+
+ Class [] elements7 = new Class [] {};
+ ASTMethod m7 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck7 = m7.new MethodCacheKey("boo",elements7);
+ selfEqualsAssertions(mck7);
+
+ Class [] elements8 = new Class [] {null};
+ ASTMethod m8 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck8 = m8.new MethodCacheKey("boo",elements8);
+ selfEqualsAssertions(mck8);
+
+ Class [] elements9 = new Class [] { Object.class };
+ ASTMethod m9 = new ASTMethod(23);
+ ASTMethod.MethodCacheKey mck9 = m9.new MethodCacheKey("boo",elements9);
+ selfEqualsAssertions(mck9);
+
+ }
+
+ private void selfEqualsAssertions(ASTMethod.MethodCacheKey mck)
+ {
+ assertTrue(mck.equals(mck));
+ assertTrue(!mck.equals(null));
+ assertTrue(!mck.equals((ASTMethod.MethodCacheKey) null));
+ }
+
+
+}
Propchange: jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java
------------------------------------------------------------------------------
svn:eol-style = native
Propchange: jakarta/velocity/engine/trunk/src/test/org/apache/velocity/test/MethodCacheKeyTestCase.java
------------------------------------------------------------------------------
svn:keywords = Id Author Date Revision
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org