Avoid NPE when registering a SpEL MethodFilter

Attempting to register a custom MethodFilter with a
StandardEvaluationContext after invoking setMethodResolvers() with a
custom list of MethodResolver instances results in a
NullPointerException. Based on the current documentation in
StandardEvaluationContext it is unclear what the expected behavior
should be, but either the implementation is broken, or the use case is
unsupported. In either case, allowing a NullPointerException to be
thrown is inappropriate.

This commit documents the fact that the SpEL MethodFilter is intended to
be used with the ReflectiveMethodResolver. Furthermore,
StandardEvaluationContext.registerMethodFilter() now throws an
IllegalStateException if the user attempts to set a filter after having
registered a custom set of resolvers.

Issue: SPR-9621

spring-expression/src/main/java/org/springframework/expression/spel/support/StandardEvaluationContext.java

 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -43,6 +43,7 @@ import org.springframework.util.Assert;
  *
  * @author Andy Clement
  * @author Juergen Hoeller
+ * @author Sam Brannen
  * @since 3.0
  */
 public class StandardEvaluationContext implements EvaluationContext {
@@ -218,16 +219,23 @@ public class StandardEvaluationContext implements EvaluationContext {
 	}
 
 	/**
-	 * Register a MethodFilter which will be called during method resolution for the
-	 * specified type.  The MethodFilter may remove methods and/or sort the methods
-	 * which will then be used by SpEL as the candidates to look through for a match.
-	 * 
+	 * Register a {@code MethodFilter} which will be called during method resolution
+	 * for the specified type.
+	 *
+	 * <p>The {@code MethodFilter} may remove methods and/or sort the methods which
+	 * will then be used by SpEL as the candidates to look through for a match.
+	 *
 	 * @param type the type for which the filter should be called
-	 * @param filter a MethodFilter, or NULL to deregister a filter for the type
+	 * @param filter a {@code MethodFilter}, or {@code null} to unregister a filter for the type
+	 * @throws IllegalStateException if the {@link ReflectiveMethodResolver} is not in use
 	 */
-	public void registerMethodFilter(Class<?> type, MethodFilter filter) {
+	public void registerMethodFilter(Class<?> type, MethodFilter filter) throws IllegalStateException {
 		ensureMethodResolversInitialized();
-		reflectiveMethodResolver.registerMethodFilter(type,filter);
+		if (reflectiveMethodResolver != null) {
+			reflectiveMethodResolver.registerMethodFilter(type, filter);
+		} else {
+			throw new IllegalStateException("Method filter cannot be set as the reflective method resolver is not in use");
+		}
 	}
 
 	private void ensurePropertyAccessorsInitialized() {

spring-expression/src/test/java/org/springframework/expression/spel/EvaluationTests.java

@@ -18,15 +18,22 @@ package org.springframework.expression.spel;
 
 import static org.junit.Assert.*;
 
+import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
 import org.junit.Test;
 
+import org.springframework.core.convert.TypeDescriptor;
+import org.springframework.expression.AccessException;
 import org.springframework.expression.EvaluationContext;
 import org.springframework.expression.EvaluationException;
 import org.springframework.expression.Expression;
 import org.springframework.expression.ExpressionParser;
+import org.springframework.expression.MethodExecutor;
+import org.springframework.expression.MethodFilter;
+import org.springframework.expression.MethodResolver;
 import org.springframework.expression.ParseException;
 import org.springframework.expression.spel.standard.SpelExpression;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
@@ -98,8 +105,8 @@ public class EvaluationTests extends ExpressionTestCase {
 	}
 
 
+	@SuppressWarnings("rawtypes")
 	static class TestClass {
-
 		public Foo wibble;
 		private Foo wibble2;
 		public Map map;
@@ -571,4 +578,48 @@ public class EvaluationTests extends ExpressionTestCase {
 		assertNull(exp.getValue());
 	}
 
+	/**
+	 * Verifies behavior requested in SPR-9621.
+	 */
+	@Test
+	public void customMethodFilter() throws Exception {
+		StandardEvaluationContext context = new StandardEvaluationContext();
+
+		// Register a custom MethodResolver...
+		List<MethodResolver> customResolvers = new ArrayList<MethodResolver>();
+		customResolvers.add(new CustomMethodResolver());
+		context.setMethodResolvers(customResolvers);
+
+		// or simply...
+		// context.setMethodResolvers(new ArrayList<MethodResolver>());
+
+		// Register a custom MethodFilter...
+		MethodFilter filter = new CustomMethodFilter();
+		try {
+			context.registerMethodFilter(String.class, filter);
+			fail("should have failed");
+		} catch (IllegalStateException ise) {
+			assertEquals(
+					"Method filter cannot be set as the reflective method resolver is not in use",
+					ise.getMessage());
+		}
+	}
+
+	static class CustomMethodResolver implements MethodResolver {
+
+		public MethodExecutor resolve(EvaluationContext context,
+				Object targetObject, String name,
+				List<TypeDescriptor> argumentTypes) throws AccessException {
+			return null;
+		}
+	}
+
+	static class CustomMethodFilter implements MethodFilter {
+
+		public List<Method> filter(List<Method> methods) {
+			return null;
+		}
+
+	}
+
 }

src/dist/changelog.txt

@@ -9,9 +9,10 @@ Changes in version 3.2 M2 (2012-08-xx)
 * spring-test module now depends on junit:junit-dep (SPR-6966)
 * now inferring return type of generic factory methods (SPR-9493)
 * SpEL now supports method invocations on integers (SPR-9612)
+* SpEL now supports case-insensitive null literals in expressions (SPR-9613)
 * SpEL now supports symbolic boolean operators for OR and AND (SPR-9614)
 * SpEL now supports nested double quotes in expressions (SPR-9620)
-* introduced support for case-insensitive null literals in SpEL expressions (SPR-9613)
+* SpEL now throws an ISE if a MethodFilter is registered against custom resolvers (SPR-9621)
 * now using BufferedInputStream in SimpleMetaDataReader to double performance (SPR-9528)
 * introduced "repeatCount" property in Quartz SimpleTriggerFactoryBean (SPR-9521)
 * introduced "jtaTransactionManager" property in Hibernate 4 LocalSessionFactoryBean/Builder (SPR-9480)