ReflectiveMethodResolver should provide a MethodFilter strategy [SPR-6764]

[spring-projects-issues]

Mark Fisher opened SPR-6764 and commented

It would be convenient if the resolver implementation provided a hook for more fine-grained control. What I have done in the meantime is added the following:

public MethodExecutor resolve(EvaluationContext context,
      Object targetObject, String name, Class<?>[] argumentTypes) throws AccessException {

   try {
      TypeConverter typeConverter = context.getTypeConverter();
      Class<?> type = (targetObject instanceof Class ? (Class<?>) targetObject : targetObject.getClass());

      // added this for Spring Integration message processor
      Method[] methods = ObjectUtils.containsElement(this.filteredTypes, type) ?
            this.methodFilter.filter(type.getMethods()) : type.getMethods();
   ...

---

Affects: 3.0 GA

Referenced from: commits 66f7083

[spring-projects-issues]

Andy Clement commented

Mark - we could get this into 3.0.1...

What are your 'filteredTypes' ? Is that an artifact from your subclass of ReflectiveMethodResolver? I forget a bit where we are on this - how were you plugging in your alternative strategy right now - was it via a new StandardEvaluationContext subtype or were you calling getMethodResolvers() on the default one and replacing the standard reflective method resolver with your own? (I do see you have another jira about changing StandardEvaluationContext to help with this)

[spring-projects-issues]

Mark Fisher commented

Andy,

The reason I have "filteredTypes" is that I am only applying filtering to my component's target object type. For example, one might have something like this in Spring Integration:

<transformer ref="foo" method="bar"/>

The SpEL expression created internally would be "#target.bar(x,y)" (where the args can vary depending on the candidate target methods), and the "target" variable is set (with the 'foo' bean as its value) when creating the StandardEvaluationContext (once). In that case #target would be included in the filteredTypes, because the candidate list should only contain certain methods (e.g. might be fine-tuned by annotations). In theory the expression could require method resolution on other types where type.getMethods() is fine.

I guess an alternate solution would be to provide a strategy for retrieving candidate methods on a target type rather than simply filtering a list/array of Method instances.

And yes, I'm currently doing the following (ugly stuff) to replace the resolver:

context.getMethodResolvers().clear();
...
context.getMethodResolvers().add(new FilteringReflectiveMethodResolver(...);

[spring-projects-issues]

Andy Clement commented

I'm thinking we can delay the opening up of StandardEvaluationContext if we make just the necessary changes to affect the filtering in the ReflectiveMethodResolver.

My current thought is to have, on the standard evaluation context:

void registerMethodFilter(Class<?> type, MethodFilter methodfilter)

I could imagine a ctor filter and possibly property filter down the line.

registerMethodFilter will call through to the ReflectiveMethodResolver, which will be managing a map of filters registered for types. Registering a null filter will clear any filtering for that type. The right filter be be looked up for any type upon which a method is about to be invoked and the filter called:

interface MethodFilter {

	/**
	 * Called by the internal SpEL method resolver to allow the user to organize the 
         * list of candidate methods that may be invoked.  The filter can remove methods 
         * that should not be considered candidates and it may sort the results.  
         * The method resolver will search through the methods as returned from the 
         * filter when looking for a suitable candidate to invoke.
	 * 
	 * @param methods the full list of methods the resolver will choose from
	 * @return a possible subset of input methods that may be sorted by order of relevance
	 */
	Method[] filter(Method[] methods);
	
}

Would that suit you?

One thing - I remember us talking about removing the void returning candidates if the expression is expected to return a value. I presume we are side stepping that now by using the method filter - as in your scenario you will remove the void-return methods.

In thinking about what to do for this bug I am thinking about further down the line when reflection is not used but instead generated invoker classes are used. By hiding the SpEL consumer from some of what is happening internally, we can switch from reflection to invoker classes without the user needing to make a change. (so I've avoided any mention of reflection in that api/doc up there)

[spring-projects-issues]

Mark Fisher commented

That definitely looks good. I think mapping filters to types will be sufficient for my case.

Yes, I'm currently handling the void-returning methods in my MethodFilter implementation (based on a boolean 'requiresReply' value).

You mentioned sorting, and so I'm wondering if we should perhaps use List as the parameter and return value type rather than arrays. What do you think?

[spring-projects-issues]

Andy Clement commented

I'll make it a List<Method>.

Just noticed - it seems a shame to create a type called MethodFilter which behaves so very differently to the MethodFilter in Spring ReflectionUtils. I wonder if Filter isn't the right name for what we are trying to do. The classic filter is 'boolean accept(Object something)' but here I'm passing a group altogether and saying 'remove what you like and order them how you want me to look at them'. Is there a better name for doing that action? Selector?

[spring-projects-issues]

Mark Fisher commented

MethodSelector might be good, but then I think I would expect a signature like:

List<Method> selectMethods(Class<?> type);

[spring-projects-issues]

Andy Clement commented

MethodFilter is added. it is as per the initial design

List<Method> filter(List<Method> methods)

and is registered on a StandardEvaluationContext through:

public void registerMethodFilter(Class<?> type, MethodFilter filter)

Registering a null filter clears the filter for that type (you cannot have multiple per type). Due to caching of methods, once a method is found, the filter will not be used again until the cached entry becomes stale. This simply means you should ensure filters are registered up front, before the first expression is evaluated.

Test committed too - see MethodInvocationTests.testMethodFiltering_SPR6764()