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">[EMAIL PROTECTED]</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: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]