argh! only meant to commit pom.xml. backing out other changes now... On Fri, Jul 25, 2008 at 11:21 AM, <[EMAIL PROTECTED]> wrote: > Author: nbubna > Date: Fri Jul 25 11:21:31 2008 > New Revision: 679872 > > URL: http://svn.apache.org/viewvc?rev=679872&view=rev > Log: > sync pom commons-lang version with build.properties > > Modified: > velocity/engine/trunk/pom.xml > > velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java > > velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java > > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java > > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java > > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java > > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java > > Modified: velocity/engine/trunk/pom.xml > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/pom.xml?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- velocity/engine/trunk/pom.xml (original) > +++ velocity/engine/trunk/pom.xml Fri Jul 25 11:21:31 2008 > @@ -138,7 +138,7 @@ > <dependency> > <groupId>commons-lang</groupId> > <artifactId>commons-lang</artifactId> > - <version>2.1</version> > + <version>2.4</version> > </dependency> > <dependency> > <groupId>oro</groupId> > > Modified: > velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- > velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java > (original) > +++ > velocity/engine/trunk/src/java/org/apache/velocity/runtime/VelocimacroFactory.java > Fri Jul 25 11:21:31 2008 > @@ -94,17 +94,6 @@ > */ > private Map libModMap; > > - /* > - * Map for holding macro execution information. An executing macro will > - * be identified by the pair <tamplateName, macroName>. > - */ > - private final Map templateMap; > - > - /* > - * Private variable for holding the allowed max calling depth. > - */ > - private int maxCallingDepth; > - > /** > * C'tor for the VelociMacro factory. > * > @@ -122,96 +111,8 @@ > */ > libModMap = new HashMap(); > vmManager = new VelocimacroManager(rsvc); > - templateMap = new HashMap(); > } > > - /** > - * This method is called before a macro is rendered. This method > - * checks whether a macro call is within the allowed calling depth. > - * If the macro call exceeds the allowed calling depth it will throw > - * an exception. > - * > - * @param macroName name of the macro > - * @param templateName name of the template file containing the macro > - * @throws MacroOverflowException if the number of macro calls exceeds > the specified value > - */ > - public void startMacroRendering(String macroName, String templateName) > - throws MacroOverflowException > - { > - maxCallingDepth = rsvc.getInt( > - RuntimeConstants.VM_MAX_DEPTH); > - > - /* > - * If this property is set to 0 or minus value we do not keep track > - * of the macro execution > - */ > - if (maxCallingDepth > 0) > - { > - synchronized(templateMap) > - { > - Stack macroStack = (Stack)templateMap.get(templateName); > - if (macroStack != null) > - { > - /* > - * If the macro stack size is larger than or equal to > - * maxCallingDepth allowed throw an exception > - */ > - if (macroStack.size() >= maxCallingDepth) > - { > - log.error("Max calling depth exceded in Template:" + > - templateName + "and Macro:" + macroName); > - > - String message = "Exceed maximum " + maxCallingDepth > + > - " macro calls. Call Stack:"; > - /* > - * Construct the message from the stack > - */ > - for (int i = 0; i < macroStack.size() - 1; i++) > - { > - message += macroStack.get(i) + "->"; > - } > - message += macroStack.peek(); > - > - /* > - Clean up the template map > - */ > - templateMap.remove(templateName); > - throw new MacroOverflowException(message); > - } > - macroStack.push(macroName); > - } > - else > - { > - macroStack = new Stack(); > - macroStack.push(macroName); > - templateMap.put(templateName, macroStack); > - } > - } > - } > - } > - > - /** > - * This method is called when a macro finishes rendering. Clears the > state > - * saved about the macro call. > - * > - * @param macroName name of the macro > - * @param templateName template name containg the macro > - */ > - public void endMacroRendering(String macroName, String templateName) > - { > - String name = null; > - Stack macroStack = (Stack)templateMap.get(templateName); > - if (macroStack != null && macroStack.size() > 0) > - { > - macroStack.pop(); > - } > - if (macroStack != null && macroStack.size() == 0) > - { > - templateMap.remove(templateName); > - } > - } > - > - > /** > * initialize the factory - setup all permissions > * load all global libraries. > > Modified: > velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- > velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java > (original) > +++ > velocity/engine/trunk/src/java/org/apache/velocity/runtime/directive/RuntimeMacro.java > Fri Jul 25 11:21:31 2008 > @@ -19,6 +19,7 @@ > * under the License. > */ > > +import org.apache.commons.lang.text.StrBuilder; > import org.apache.velocity.context.InternalContextAdapter; > import org.apache.velocity.runtime.parser.node.Node; > import org.apache.velocity.runtime.parser.Token; > @@ -124,26 +125,35 @@ > this.context = context; > this.node = node; > > - StringBuffer buffer = new StringBuffer(); > Token t = node.getFirstToken(); > > - /** > - * Retrieve the literal text > - */ > - while (t != null && t != node.getLastToken()) > + if (t == node.getLastToken()) > { > - buffer.append(t.image); > - t = t.next; > + literal = t.image; > } > - > - if (t != null) > + else > { > - buffer.append(t.image); > + // guessing that most macros are much longer than > + // the 32 char default capacity. let's guess 4x bigger :) > + StrBuilder text = new StrBuilder(128); > + /** > + * Retrieve the literal text > + */ > + while (t != null && t != node.getLastToken()) > + { > + text.append(t.image); > + t = t.next; > + } > + if (t != null) > + { > + text.append(t.image); > + } > + > + /** > + * Store the literal text > + */ > + literal = text.toString(); > } > - /** > - * Store the literal text > - */ > - literal = buffer.toString(); > } > > /** > > Modified: > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java > (original) > +++ > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/Resource.java > Fri Jul 25 11:21:31 2008 > @@ -85,6 +85,11 @@ > protected Object data = null; > > /** > + * Resource type (RESOURCE_TEMPLATE or RESOURCE_CONTENT) > + */ > + protected int type; > + > + /** > * Default constructor > */ > public Resource() > @@ -267,4 +272,20 @@ > { > return data; > } > + > + /** > + * Sets the type of this Resource (RESOURCE_TEMPLATE or RESOURCE_CONTENT) > + */ > + public void setType(int type) > + { > + this.type = type; > + } > + > + /** > + * @return type code of the Resource > + */ > + public int getType() > + { > + return type; > + } > } > > Modified: > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java > (original) > +++ > velocity/engine/trunk/src/java/org/apache/velocity/runtime/resource/ResourceManagerImpl.java > Fri Jul 25 11:21:31 2008 > @@ -99,11 +99,11 @@ > public synchronized void initialize(final RuntimeServices rsvc) > throws Exception > { > - if (isInit) > - { > - log.error("Re-initialization of ResourceLoader attempted!"); > - return; > - } > + if (isInit) > + { > + log.error("Re-initialization of ResourceLoader attempted!"); > + return; > + } > > ResourceLoader resourceLoader = null; > > @@ -259,6 +259,9 @@ > * Gets the named resource. Returned class type corresponds to specified > type (i.e. <code>Template</code> to <code> > * RESOURCE_TEMPLATE</code>). > * > + * This method is now unsynchronized which requires that ResourceCache > + * implementations be thread safe (as the default is). > + * > * @param resourceName The name of the resource to retrieve. > * @param resourceType The type of resource > (<code>RESOURCE_TEMPLATE</code>, <code>RESOURCE_CONTENT</code>, etc.). > * @param encoding The character encoding to use. > @@ -269,7 +272,7 @@ > * @throws ParseErrorException if template cannot be parsed due to > syntax (or other) error. > * @throws Exception if a problem in parse > */ > - public synchronized Resource getResource(final String resourceName, > final int resourceType, final String encoding) > + public Resource getResource(final String resourceName, final int > resourceType, final String encoding) > throws ResourceNotFoundException, > ParseErrorException, > Exception > @@ -289,13 +292,29 @@ > > if (resource != null) > { > - /* > - * refresh the resource > - */ > - > try > { > - refreshResource(resource, encoding); > + // avoids additional method call to refreshResource > + if (resource.requiresChecking()) > + { > + /* > + * both loadResource() and refreshResource() now return > + * a new Resource instance when they are called > + * (put in the cache when appropriate) in order to allow > + * several threads to parse the same template > simultaneously. > + * It is redundant work and will cause more garbage > collection but the > + * benefit is that it allows concurrent parsing and > processing > + * without race conditions when multiple requests try to > + * refresh/load the same template at the same time. > + * > + * Another alternative is to limit template > parsing/retrieval > + * so that only one thread can parse each template at a > time > + * but that creates a scalability bottleneck. > + * > + * See VELOCITY-606, VELOCITY-595 and VELOCITY-24 > + */ > + resource = refreshResource(resource, encoding); > + } > } > catch (ResourceNotFoundException rnfe) > { > @@ -316,7 +335,7 @@ > } > catch (RuntimeException re) > { > - throw re; > + throw re; > } > catch (Exception e) > { > @@ -330,8 +349,7 @@ > { > /* > * it's not in the cache, so load it. > - */ > - > + */ > resource = loadResource(resourceName, resourceType, encoding); > > if (resource.getResourceLoader().isCachingOn()) > @@ -352,7 +370,7 @@ > } > catch (RuntimeException re) > { > - throw re; > + throw re; > } > catch (Exception e) > { > @@ -383,9 +401,7 @@ > Exception > { > Resource resource = ResourceFactory.getResource(resourceName, > resourceType); > - > resource.setRuntimeServices(rsvc); > - > resource.setName(resourceName); > resource.setEncoding(encoding); > > @@ -475,12 +491,11 @@ > * @throws ParseErrorException if template cannot be parsed due to > syntax (or other) error. > * @throws Exception if a problem in parse > */ > - protected void refreshResource(final Resource resource, final String > encoding) > + protected Resource refreshResource(Resource resource, final String > encoding) > throws ResourceNotFoundException, > ParseErrorException, > Exception > { > - > /* > * The resource knows whether it needs to be checked > * or not, and the resource's loader can check to > @@ -489,52 +504,58 @@ > * the input stream and parse it to make a new > * AST for the resource. > */ > - if (resource.requiresChecking()) > + > + /* > + * touch() the resource to reset the counters > + */ > + resource.touch(); > + > + if (resource.isSourceModified()) > { > /* > - * touch() the resource to reset the counters > + * now check encoding info. It's possible that the newly > declared > + * encoding is different than the encoding already in the > resource > + * this strikes me as bad... > */ > > - resource.touch(); > - > - if (resource.isSourceModified()) > + if > (!org.apache.commons.lang.StringUtils.equals(resource.getEncoding(), > encoding)) > { > - /* > - * now check encoding info. It's possible that the newly > declared > - * encoding is different than the encoding already in the > resource > - * this strikes me as bad... > - */ > - > - if > (!org.apache.commons.lang.StringUtils.equals(resource.getEncoding(), > encoding)) > - { > - log.warn("Declared encoding for template '" + > + log.warn("Declared encoding for template '" + > resource.getName() + > "' is different on reload. Old = '" + > resource.getEncoding() + "' New = '" + encoding); > > - resource.setEncoding(encoding); > - } > - > - /* > - * read how old the resource is _before_ > - * processing (=>reading) it > - */ > - long howOldItWas = > resource.getResourceLoader().getLastModified(resource); > + resource.setEncoding(encoding); > + } > > - /* > - * read in the fresh stream and parse > - */ > + /* > + * read how old the resource is _before_ > + * processing (=>reading) it > + */ > + long howOldItWas = > resource.getResourceLoader().getLastModified(resource); > > - resource.process(); > + String resourceKey = resource.getType() + resource.getName(); > > - /* > - * now set the modification info and reset > - * the modification check counters > - */ > + /* > + * we create a copy to avoid partially overwriting a > + * template which may be in use in another thread > + */ > + > + Resource newResource = > + ResourceFactory.getResource(resource.getName(), > resource.getType()); > + > + newResource.setRuntimeServices(rsvc); > + newResource.setName(resource.getName()); > + newResource.setEncoding(resource.getEncoding()); > + newResource.setResourceLoader(resource.getResourceLoader()); > + > + newResource.process(); > + newResource.setLastModified(howOldItWas); > + resource = newResource; > > - resource.setLastModified(howOldItWas); > - } > + globalCache.put(resourceKey, newResource); > } > + return resource; > } > > /** > > Modified: > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java > (original) > +++ > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/ClassMap.java > Fri Jul 25 11:21:31 2008 > @@ -26,7 +26,7 @@ > import java.util.Iterator; > import java.util.List; > import java.util.Map; > - > +import org.apache.commons.lang.text.StrBuilder; > import org.apache.velocity.runtime.log.Log; > > /** > @@ -39,6 +39,7 @@ > * @author <a href="mailto:[EMAIL PROTECTED]">Attila Szegedi</a> > * @author <a href="mailto:[EMAIL PROTECTED]">Geir Magnusson Jr.</a> > * @author <a href="mailto:[EMAIL PROTECTED]">Henning P. Schmiedehausen</a> > + * @author Nathan Bubna > * @version $Id$ > */ > public class ClassMap > @@ -72,9 +73,7 @@ > log.debug("== Class: " + clazz); > } > > - methodCache = new MethodCache(log); > - > - populateMethodCache(); > + methodCache = createMethodCache(); > > if (debugReflection && log.isDebugEnabled()) > { > @@ -111,10 +110,11 @@ > * are taken from all the public methods > * that our class, its parents and their implemented interfaces provide. > */ > - private void populateMethodCache() > + private MethodCache createMethodCache() > { > + MethodCache methodCache = new MethodCache(log); > // > - // Build a list of all elements in the class hierarchy. This one is > bottom-first (i.e. we start > + // Looks through all elements in the class hierarchy. This one is > bottom-first (i.e. we start > // with the actual declaring class and its interfaces and then move up > (superclass etc.) until we > // hit java.lang.Object. That is important because it will give us the > methods of the declaring class > // which might in turn be abstract further up the tree. > @@ -126,70 +126,60 @@ > // until Velocity 1.4. As we always reflect all elements of the tree > (that's what we have a cache for), we will > // hit the public elements sooner or later because we reflect all the > public elements anyway. > // > - List classesToReflect = new ArrayList(); > - > // Ah, the miracles of Java for(;;) ... > for (Class classToReflect = getCachedClass(); classToReflect != null > ; classToReflect = classToReflect.getSuperclass()) > { > if (Modifier.isPublic(classToReflect.getModifiers())) > { > - classesToReflect.add(classToReflect); > - if (debugReflection && log.isDebugEnabled()) > - { > - log.debug("Adding " + classToReflect + " for > reflection"); > - } > + populateMethodCacheWith(methodCache, classToReflect); > } > Class [] interfaces = classToReflect.getInterfaces(); > for (int i = 0; i < interfaces.length; i++) > { > if (Modifier.isPublic(interfaces[i].getModifiers())) > { > - classesToReflect.add(interfaces[i]); > - if (debugReflection && log.isDebugEnabled()) > - { > - log.debug("Adding " + interfaces[i] + " for > reflection"); > - } > + populateMethodCacheWith(methodCache, interfaces[i]); > } > } > } > + // return the already initialized cache > + return methodCache; > + } > > - for (Iterator it = classesToReflect.iterator(); it.hasNext(); ) > + private void populateMethodCacheWith(MethodCache methodCache, Class > classToReflect) > + { > + if (debugReflection && log.isDebugEnabled()) > { > - Class classToReflect = (Class) it.next(); > - if (debugReflection && log.isDebugEnabled()) > - { > - log.debug("Reflecting " + classToReflect); > - } > - > + log.debug("Reflecting " + classToReflect); > + } > > - try > - { > - Method[] methods = classToReflect.getMethods(); > + try > + { > + Method[] methods = classToReflect.getDeclaredMethods(); > > - for (int i = 0; i < methods.length; i++) > + for (int i = 0; i < methods.length; i++) > + { > + // Strictly spoken that check shouldn't be necessary > + // because getMethods only returns public methods. > + int modifiers = methods[i].getModifiers(); > + if (Modifier.isPublic(modifiers)) // && !) > { > - // Strictly spoken that check shouldn't be necessary > - // because getMethods only returns public methods. > - int modifiers = methods[i].getModifiers(); > - if (Modifier.isPublic(modifiers)) // && !) > - { > - // Some of the interfaces contain abstract methods. > That is fine, because the actual object must > - // implement them anyway (else it wouldn't be > implementing the interface). If we find an abstract > - // method in a non-interface, we skip it, because we > do want to make sure that no abstract methods end up in > - // the cache. > - if (classToReflect.isInterface() || > !Modifier.isAbstract(modifiers)) > - { > - methodCache.put(methods[i]); > - } > + // Some of the interfaces contain abstract methods. That > is fine, because the actual object must > + // implement them anyway (else it wouldn't be > implementing the interface). If we find an abstract > + // method in a non-interface, we skip it, because we do > want to make sure that no abstract methods end up in > + // the cache. > + if (classToReflect.isInterface() || > !Modifier.isAbstract(modifiers)) > + { > + methodCache.put(methods[i]); > } > } > } > - catch (SecurityException se) // Everybody feels better with... > + } > + catch (SecurityException se) // Everybody feels better with... > + { > + if (log.isDebugEnabled()) > { > - if (log.isDebugEnabled()) > - { > - log.debug("While accessing methods of " + classToReflect > + ": ", se); > - } > + log.debug("While accessing methods of " + classToReflect + > ": ", se); > } > } > } > @@ -202,11 +192,9 @@ > */ > private static final class MethodCache > { > - private static final class CacheMiss { } > - > - private static final CacheMiss CACHE_MISS = new CacheMiss(); > + private static final Object CACHE_MISS = new Object(); > > - private static final Object OBJECT = new Object(); > + private static final String NULL_ARG = new > Object().getClass().getName(); > > private static final Map convertPrimitives = new HashMap(); > > @@ -230,6 +218,7 @@ > * name and actual arguments used to find it. > */ > private final Map cache = new HashMap(); > + private final Map locks = new HashMap(); > > /** Map of methods that are searchable according to method parameters > to find a match */ > private final MethodMap methodMap = new MethodMap(); > @@ -255,52 +244,55 @@ > * @return A Method object representing the method to invoke or null. > * @throws MethodMap.AmbiguousException When more than one method is > a match for the parameters. > */ > - public synchronized Method get(final String name, final Object [] > params) > + public Method get(final String name, final Object [] params) > throws MethodMap.AmbiguousException > { > - String methodKey = makeMethodKey(name, params); > + String methodKey = getLock(makeMethodKey(name, params)); > > - Object cacheEntry = cache.get(methodKey); > - > - // We looked this up before and failed. > - if (cacheEntry == CACHE_MISS) > + synchronized (methodKey) > { > - return null; > - } > + Object cacheEntry = cache.get(methodKey); > > - if (cacheEntry == null) > - { > - try > + // We looked this up before and failed. > + if (cacheEntry == CACHE_MISS) > { > - // That one is expensive... > - cacheEntry = methodMap.find(name, params); > + return null; > } > - catch(MethodMap.AmbiguousException ae) > + > + if (cacheEntry == null) > { > - /* > - * that's a miss :-) > - */ > - cache.put(methodKey, CACHE_MISS); > - throw ae; > - } > + try > + { > + // That one is expensive... > + cacheEntry = methodMap.find(name, params); > + } > + catch(MethodMap.AmbiguousException ae) > + { > + /* > + * that's a miss :-) > + */ > + cache.put(methodKey, CACHE_MISS); > + throw ae; > + } > > - cache.put(methodKey, > - (cacheEntry != null) ? cacheEntry : CACHE_MISS); > - } > + cache.put(methodKey, > + (cacheEntry != null) ? cacheEntry : CACHE_MISS); > + } > > - // Yes, this might just be null. > + // Yes, this might just be null. > > - return (Method) cacheEntry; > + return (Method) cacheEntry; > + } > } > > - public synchronized void put(Method method) > + private void put(Method method) > { > String methodKey = makeMethodKey(method); > - > - // We don't overwrite methods. Especially not if we fill the > - // cache from defined class towards java.lang.Object because > - // abstract methods in superclasses would else overwrite concrete > - // classes further down the hierarchy. > + > + // We don't overwrite methods because we fill the > + // cache from defined class towards java.lang.Object > + // and that would cause overridden methods to appear > + // as if they were not overridden. > if (cache.get(methodKey) == null) > { > cache.put(methodKey, method); > @@ -312,6 +304,18 @@ > } > } > > + private final String getLock(String key) { > + synchronized (locks) > + { > + String lock = (String)locks.get(key); > + if (lock == null) > + { > + return key; > + } > + return lock; > + } > + } > + > /** > * Make a methodKey for the given method using > * the concatenation of the name and the > @@ -323,10 +327,15 @@ > private String makeMethodKey(final Method method) > { > Class[] parameterTypes = method.getParameterTypes(); > + int args = parameterTypes.length; > + if (args == 0) > + { > + return method.getName(); > + } > > - StringBuffer methodKey = new StringBuffer(method.getName()); > + StrBuilder methodKey = new > StrBuilder((args+1)*16).append(method.getName()); > > - for (int j = 0; j < parameterTypes.length; j++) > + for (int j = 0; j < args; j++) > { > /* > * If the argument type is primitive then we want > @@ -353,18 +362,25 @@ > > private String makeMethodKey(String method, Object[] params) > { > - StringBuffer methodKey = new StringBuffer().append(method); > + int args = params.length; > + if (args == 0) > + { > + return method; > + } > > - for (int j = 0; j < params.length; j++) > + StrBuilder methodKey = new > StrBuilder((args+1)*16).append(method); > + > + for (int j = 0; j < args; j++) > { > Object arg = params[j]; > - > if (arg == null) > { > - arg = OBJECT; > + methodKey.append(NULL_ARG); > + } > + else > + { > + methodKey.append(arg.getClass().getName()); > } > - > - methodKey.append(arg.getClass().getName()); > } > > return methodKey.toString(); > > Modified: > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java > URL: > http://svn.apache.org/viewvc/velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java?rev=679872&r1=679871&r2=679872&view=diff > ============================================================================== > --- > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java > (original) > +++ > velocity/engine/trunk/src/java/org/apache/velocity/util/introspection/MethodMap.java > Fri Jul 25 11:21:31 2008 > @@ -21,7 +21,7 @@ > > import java.lang.reflect.Method; > import java.util.ArrayList; > -import java.util.Hashtable; > +import java.util.HashMap; > import java.util.Iterator; > import java.util.LinkedList; > import java.util.List; > @@ -45,7 +45,7 @@ > /** > * Keep track of all methods with the same name. > */ > - Map methodByNameMap = new Hashtable(); > + Map methodByNameMap = new HashMap(); > > /** > * Add a method to a list of methods by name. > @@ -132,99 +132,99 @@ > arg == null ? null : arg.getClass(); > } > > - return getMostSpecific(methodList, classes); > + return getBestMatch(methodList, classes); > } > > - /** > - * Simple distinguishable exception, used when > - * we run across ambiguous overloading. Caught > - * by the introspector. > - */ > - public static class AmbiguousException extends RuntimeException > - { > - /** > - * Version Id for serializable > - */ > - private static final long serialVersionUID = -2314636505414551663L; > - } > - > - > - private static Method getMostSpecific(List methods, Class[] classes) > - throws AmbiguousException > + private static Method getBestMatch(List methods, Class[] args) > { > - LinkedList applicables = getApplicables(methods, classes); > - > - if(applicables.isEmpty()) > + List equivalentMatches = null; > + Method bestMatch = null; > + Class[] bestMatchTypes = null; > + for (Iterator i = methods.iterator(); i.hasNext(); ) > { > - return null; > - } > - > - if(applicables.size() == 1) > - { > - return (Method)applicables.getFirst(); > - } > - > - /* > - * This list will contain the maximally specific methods. Hopefully > at > - * the end of the below loop, the list will contain exactly one > method, > - * (the most specific method) otherwise we have ambiguity. > - */ > - > - LinkedList maximals = new LinkedList(); > - > - for (Iterator applicable = applicables.iterator(); > - applicable.hasNext();) > - { > - Method app = (Method) applicable.next(); > - Class[] appArgs = app.getParameterTypes(); > - boolean lessSpecific = false; > - > - for (Iterator maximal = maximals.iterator(); > - !lessSpecific && maximal.hasNext();) > + Method method = (Method)i.next(); > + if (isApplicable(method, args)) > { > - Method max = (Method) maximal.next(); > - > - switch(moreSpecific(appArgs, max.getParameterTypes())) > + if (bestMatch == null) > { > - case MORE_SPECIFIC: > - { > - /* > - * This method is more specific than the previously > - * known maximally specific, so remove the old > maximum. > - */ > - > - maximal.remove(); > - break; > - } > - > - case LESS_SPECIFIC: > + bestMatch = method; > + bestMatchTypes = method.getParameterTypes(); > + } > + else > + { > + Class[] methodTypes = method.getParameterTypes(); > + switch (compare(methodTypes, bestMatchTypes)) > { > - /* > - * This method is less specific than some of the > - * currently known maximally specific methods, so we > - * won't add it into the set of maximally specific > - * methods > - */ > - > - lessSpecific = true; > - break; > + case MORE_SPECIFIC: > + if (equivalentMatches == null) > + { > + bestMatch = method; > + bestMatchTypes = methodTypes; > + } > + else > + { > + // have to beat all other ambiguous ones... > + int ambiguities = equivalentMatches.size(); > + for (int a=0; a < ambiguities; a++) > + { > + Method other = > (Method)equivalentMatches.get(a); > + switch (compare(methodTypes, > other.getParameterTypes())) > + { > + case MORE_SPECIFIC: > + // ...and thus replace them > all... > + bestMatch = method; > + bestMatchTypes = methodTypes; > + equivalentMatches = null; > + ambiguities = 0; > + break; > + > + case INCOMPARABLE: > + // ...join them... > + equivalentMatches.add(method); > + break; > + > + case LESS_SPECIFIC: > + // ...or just go away. > + break; > + } > + } > + } > + break; > + > + case INCOMPARABLE: > + if (equivalentMatches == null) > + { > + equivalentMatches = new > ArrayList(bestMatchTypes.length); > + } > + equivalentMatches.add(method); > + break; > + > + case LESS_SPECIFIC: > + // do nothing > + break; > } > } > } > - > - if(!lessSpecific) > - { > - maximals.addLast(app); > - } > } > - > - if(maximals.size() > 1) > + > + if (equivalentMatches != null) > { > - // We have more than one maximally specific method > throw new AmbiguousException(); > } > + return bestMatch; > + } > > - return (Method)maximals.getFirst(); > + /** > + * Simple distinguishable exception, used when > + * we run across ambiguous overloading. Caught > + * by the introspector. > + */ > + public static class AmbiguousException extends RuntimeException > + { > + /** > + * Version Id for serializable > + */ > + private static final long serialVersionUID = -2314636505414551663L; > } > > /** > @@ -235,7 +235,7 @@ > * @return MORE_SPECIFIC if c1 is more specific than c2, LESS_SPECIFIC if > * c1 is less specific than c2, INCOMPARABLE if they are incomparable. > */ > - private static int moreSpecific(Class[] c1, Class[] c2) > + private static int compare(Class[] c1, Class[] c2) > { > boolean c1MoreSpecific = false; > boolean c2MoreSpecific = false; > @@ -310,31 +310,6 @@ > } > > /** > - * Returns all methods that are applicable to actual argument types. > - * @param methods list of all candidate methods > - * @param classes the actual types of the arguments > - * @return a list that contains only applicable methods (number of > - * formal and actual arguments matches, and argument types are assignable > - * to formal types through a method invocation conversion). > - */ > - private static LinkedList getApplicables(List methods, Class[] classes) > - { > - LinkedList list = new LinkedList(); > - > - for (Iterator imethod = methods.iterator(); imethod.hasNext();) > - { > - Method method = (Method) imethod.next(); > - > - if(isApplicable(method, classes)) > - { > - list.add(method); > - } > - > - } > - return list; > - } > - > - /** > * Returns true if the supplied method is applicable to actual > * argument types. > * > > >
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
