INT-3133-2: PAs Inheritance Refactoring

JIRA: https://jira.springsource.org/browse/INT-3133

INT-3133-2: Addressing PR's comments

spring-integration-core/src/main/java/org/springframework/integration/config/IntegrationEvaluationContextFactoryBean.java

@@ -17,10 +17,7 @@
 package org.springframework.integration.config;
 
 import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 
@@ -76,43 +73,44 @@ import org.springframework.util.Assert;
 public class IntegrationEvaluationContextFactoryBean implements FactoryBean<StandardEvaluationContext>,
 		ApplicationContextAware, InitializingBean {
 
-	private volatile List<PropertyAccessor> propertyAccessors = new ArrayList<PropertyAccessor>();
+	private volatile Map<String, PropertyAccessor> propertyAccessors = new LinkedHashMap<String, PropertyAccessor>();
 
 	private volatile Map<String, Method> functions = new LinkedHashMap<String, Method>();
 
 	private TypeConverter typeConverter = new StandardTypeConverter();
 
-	private ApplicationContext applicationContext;
 	private BeanResolver beanResolver;
 
+	private ApplicationContext applicationContext;
+
+	private volatile boolean initialized;
+
 	@Override
 	public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
 		this.applicationContext = applicationContext;
 	}
 
-	public void setPropertyAccessors(PropertyAccessor... accessors) {
-		Assert.noNullElements(accessors, "Cannot have null elements in accessors");
-		List<PropertyAccessor> propertyAccessors = new ArrayList<PropertyAccessor>();
-		loadDefaultPropertyAccessors(propertyAccessors);
-		Collections.addAll(propertyAccessors, accessors);
-		this.propertyAccessors = propertyAccessors;
+	public void setPropertyAccessors(Map<String, PropertyAccessor> accessors) {
+		Assert.isTrue(!this.initialized, "'propertyAccessors' can't be changed after initialization.");
+		Assert.notNull(accessors, "'accessors' must not be null.");
+		Assert.noNullElements(accessors.values().toArray(), "'accessors' cannot have null values.");
+		this.propertyAccessors = new LinkedHashMap<String, PropertyAccessor>(accessors);
+	}
+
+	public Map<String, PropertyAccessor> getPropertyAccessors() {
+		return propertyAccessors;
 	}
 
 	public void setFunctions(Map<String, Method> functionsArg) {
-		Map<String, Method> functions = new LinkedHashMap<String, Method>();
-		for (Entry<String, Method> function : functionsArg.entrySet()) {
-			Assert.notNull(function.getValue(), "Method cannot be null");
-			functions.put(function.getKey(), function.getValue());
-		}
-		this.functions = functions;
+		Assert.isTrue(!this.initialized, "'functions' can't be changed after initialization.");
+		Assert.notNull(functionsArg, "'functions' must not be null.");
+		Assert.noNullElements(functionsArg.values().toArray(), "'functions' cannot have null values.");
+		this.functions = new LinkedHashMap<String, Method>(functionsArg);
 	}
 
 
 	@Override
 	public void afterPropertiesSet() throws Exception {
-		if (this.propertyAccessors.isEmpty()) {
-			this.loadDefaultPropertyAccessors(this.propertyAccessors);
-		}
 		if (this.applicationContext != null) {
 			this.beanResolver = new BeanFactoryResolver(this.applicationContext);
 			ConversionService conversionService = IntegrationContextUtils.getConversionService(this.applicationContext);
@@ -130,13 +128,32 @@ public class IntegrationEvaluationContextFactoryBean implements FactoryBean<Stan
 
 			try {
 				SpelPropertyAccessorRegistrar propertyAccessorRegistrar = this.applicationContext.getBean(SpelPropertyAccessorRegistrar.class);
-				this.propertyAccessors.addAll(propertyAccessorRegistrar.getPropertyAccessors());
+				for (Entry<String, PropertyAccessor> entry : propertyAccessorRegistrar.getPropertyAccessors().entrySet()) {
+					if (!this.propertyAccessors.containsKey(entry.getKey())) {
+						this.propertyAccessors.put(entry.getKey(), entry.getValue());
+					}
+				}
 			}
 			catch (NoSuchBeanDefinitionException e) {
-				// There is no 'SpelPropertyAccessorRegistrar' bean with the parent application context
-				// Ignore it
+				// There is no 'SpelPropertyAccessorRegistrar' bean in the application context.
+			}
+
+			ApplicationContext parent = this.applicationContext.getParent();
+
+			if (parent != null && parent.containsBean(IntegrationContextUtils.INTEGRATION_EVALUATION_CONTEXT_BEAN_NAME)) {
+				IntegrationEvaluationContextFactoryBean parentFactoryBean =
+						parent.getBean("&" + IntegrationContextUtils.INTEGRATION_EVALUATION_CONTEXT_BEAN_NAME,
+								IntegrationEvaluationContextFactoryBean.class);
+
+				for (Entry<String, PropertyAccessor> entry : parentFactoryBean.getPropertyAccessors().entrySet()) {
+					if (!this.propertyAccessors.containsKey(entry.getKey())) {
+						this.propertyAccessors.put(entry.getKey(), entry.getValue());
+					}
+				}
 			}
 		}
+
+		this.initialized = true;
 	}
 
 	@Override
@@ -146,10 +163,12 @@ public class IntegrationEvaluationContextFactoryBean implements FactoryBean<Stan
 		evaluationContext.setBeanResolver(this.beanResolver);
 		evaluationContext.setTypeConverter(this.typeConverter);
 
-		for (PropertyAccessor propertyAccessor : this.propertyAccessors) {
+		for (PropertyAccessor propertyAccessor : this.propertyAccessors.values()) {
 			evaluationContext.addPropertyAccessor(propertyAccessor);
 		}
 
+		evaluationContext.addPropertyAccessor(new MapAccessor());
+
 		for (Entry<String, Method> functionEntry : this.functions.entrySet()) {
 			evaluationContext.registerFunction(functionEntry.getKey(), functionEntry.getValue());
 		}
@@ -157,10 +176,6 @@ public class IntegrationEvaluationContextFactoryBean implements FactoryBean<Stan
 		return evaluationContext;
 	}
 
-	private void loadDefaultPropertyAccessors(List<PropertyAccessor> propertyAccessors) {
-		propertyAccessors.add(new MapAccessor());
-	}
-
 	@Override
 	public Class<?> getObjectType() {
 		return StandardEvaluationContext.class;

spring-integration-core/src/main/java/org/springframework/integration/config/SpelPropertyAccessorRegistrar.java

@@ -16,15 +16,8 @@
 
 package org.springframework.integration.config;
 
-import java.util.Collection;
 import java.util.Map;
 
-import org.springframework.beans.BeansException;
-import org.springframework.beans.factory.BeanFactory;
-import org.springframework.beans.factory.InitializingBean;
-import org.springframework.beans.factory.NoSuchBeanDefinitionException;
-import org.springframework.context.ApplicationContext;
-import org.springframework.context.ApplicationContextAware;
 import org.springframework.expression.PropertyAccessor;
 
 /**
@@ -34,46 +27,20 @@ import org.springframework.expression.PropertyAccessor;
  * @author Artem Bilan
  * @since 3.0
  */
-class SpelPropertyAccessorRegistrar implements ApplicationContextAware, InitializingBean {
+class SpelPropertyAccessorRegistrar {
 
 	private final Map<String, PropertyAccessor> propertyAccessors;
 
-	private ApplicationContext applicationContext;
-
 	SpelPropertyAccessorRegistrar(Map<String, PropertyAccessor> propertyAccessors) {
 		this.propertyAccessors = propertyAccessors;
 	}
 
-	Collection<PropertyAccessor> getPropertyAccessors() {
-		return propertyAccessors.values();
+	Map<String, PropertyAccessor> getPropertyAccessors() {
+		return propertyAccessors;
 	}
 
-	@Override
-	public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
-		this.applicationContext = applicationContext;
-	}
-
-	@Override
-	public void afterPropertiesSet() throws Exception {
-		SpelPropertyAccessorRegistrar parentPropertyAccessorRegistrar = null;
-		try {
-			BeanFactory parentBeanFactory = this.applicationContext.getParentBeanFactory();
-			if (parentBeanFactory != null) {
-				parentPropertyAccessorRegistrar = parentBeanFactory.getBean(SpelPropertyAccessorRegistrar.class);
-			}
-		}
-		catch (NoSuchBeanDefinitionException e) {
-			// There is no 'SpelPropertyAccessorRegistrar' bean with the parent application context
-			// Ignore it
-		}
-
-		if (parentPropertyAccessorRegistrar != null) {
-			for (Map.Entry<String, PropertyAccessor> entry : parentPropertyAccessorRegistrar.propertyAccessors.entrySet()) {
-				if (!this.propertyAccessors.containsKey(entry.getKey())) {
-					this.propertyAccessors.put(entry.getKey(), entry.getValue());
-				}
-			}
-		}
+	void addPropertyAccessor(String name, PropertyAccessor propertyAccessor) {
+		this.propertyAccessors.put(name, propertyAccessor);
 	}
 
 }

spring-integration-core/src/test/java/org/springframework/integration/config/xml/parentApplicationContext.xml

@@ -6,4 +6,4 @@
            http://www.springframework.org/schema/integration
            http://www.springframework.org/schema/integration/spring-integration.xsd">
 
-</beans:beans>
+</beans:beans>

spring-integration-core/src/test/java/org/springframework/integration/config/xml/superParentApplicationContext.xml

@@ -9,4 +9,4 @@
 	<beans:bean id="taskScheduler"
 		class="org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler" />
 
-</beans:beans>
+</beans:beans>

spring-integration-core/src/test/java/org/springframework/integration/expression/ParentContextTests.java

@@ -22,9 +22,11 @@ import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 
@@ -39,6 +41,7 @@ import org.springframework.integration.Message;
 import org.springframework.integration.MessageChannel;
 import org.springframework.integration.channel.QueueChannel;
 import org.springframework.integration.config.IntegrationEvaluationContextFactoryBean;
+import org.springframework.integration.context.IntegrationContextUtils;
 import org.springframework.integration.json.JsonPathUtils;
 import org.springframework.integration.message.GenericMessage;
 import org.springframework.integration.test.util.TestUtils;
@@ -46,9 +49,7 @@ import org.springframework.integration.test.util.TestUtils;
 /**
  * @author Gary Russell
  * @author Artem Bilan
- *
  * @since 3.0
- *
  */
 public class ParentContextTests {
 
@@ -62,7 +63,6 @@ public class ParentContextTests {
 	 * and parent contexts work. Verifies that PropertyAccessors are inherited in the child context
 	 * and the parent's ones are last in the propertyAccessors list of EvaluationContext.
 	 * Verifies that SpEL functions are inherited from parent context and overridden with the same 'id'.
-	 *
 	 */
 	@Test
 	@SuppressWarnings("unchecked")
@@ -70,7 +70,7 @@ public class ParentContextTests {
 		AbstractApplicationContext parent = new ClassPathXmlApplicationContext("ParentContext-context.xml", this.getClass());
 
 		Object parentEvaluationContextFactoryBean = parent.getBean(IntegrationEvaluationContextFactoryBean.class);
-		Map<?,?> parentFunctions = TestUtils.getPropertyValue(parentEvaluationContextFactoryBean, "functions", Map.class);
+		Map<?, ?> parentFunctions = TestUtils.getPropertyValue(parentEvaluationContextFactoryBean, "functions", Map.class);
 		assertEquals(3, parentFunctions.size());
 		Object jsonPath = parentFunctions.get("jsonPath");
 		assertNotNull(jsonPath);
@@ -81,7 +81,7 @@ public class ParentContextTests {
 		child.refresh();
 
 		Object childEvaluationContextFactoryBean = child.getBean(IntegrationEvaluationContextFactoryBean.class);
-		Map<?,?> childFunctions = TestUtils.getPropertyValue(childEvaluationContextFactoryBean, "functions", Map.class);
+		Map<?, ?> childFunctions = TestUtils.getPropertyValue(childEvaluationContextFactoryBean, "functions", Map.class);
 		assertEquals(4, childFunctions.size());
 		assertTrue(childFunctions.containsKey("barParent"));
 		jsonPath = childFunctions.get("jsonPath");
@@ -139,6 +139,17 @@ public class ParentContextTests {
 		out = child.getBean("parentOut", QueueChannel.class).receive(0);
 		assertNotNull(out);
 		assertEquals("foo", out.getPayload());
+
+		IntegrationEvaluationContextFactoryBean evaluationContextFactoryBean =
+				child.getBean("&" + IntegrationContextUtils.INTEGRATION_EVALUATION_CONTEXT_BEAN_NAME,
+						IntegrationEvaluationContextFactoryBean.class);
+		try {
+			evaluationContextFactoryBean.setPropertyAccessors(Collections.<String, PropertyAccessor>emptyMap());
+			fail("IllegalArgumentException expected.");
+		}
+		catch (Exception e) {
+			assertThat(e, Matchers.instanceOf(IllegalArgumentException.class));
+		}
 	}
 
 	public static class Foo implements IntegrationEvaluationContextAware {

spring-integration-core/src/test/java/org/springframework/integration/transformer/SpelTransformerIntegrationTests-context.xml

@@ -25,16 +25,19 @@
 
 	<spel-function id="trim" class="org.springframework.util.StringUtils" method="trimWhitespace"/>
 
+
 	<beans:bean id="integrationEvaluationContext" class="org.springframework.integration.config.IntegrationEvaluationContextFactoryBean">
 		<beans:property name="propertyAccessors">
-			<util:list>
-				<beans:bean class="org.springframework.integration.transformer.SpelTransformerIntegrationTests$FooAccessor"/>
-			</util:list>
+			<util:map>
+				<beans:entry key="foo">
+					<beans:bean class="org.springframework.integration.transformer.SpelTransformerIntegrationTests$FooAccessor"/>
+				</beans:entry>
+			</util:map>
 		</beans:property>
 		<beans:property name="functions">
 			<util:map>
 				<beans:entry key="bar"
-					value="#{T(org.springframework.integration.transformer.SpelTransformerIntegrationTests$BarFunction).getMethod('bar', T(org.springframework.integration.Message))}"/>
+				value="#{T(org.springframework.integration.transformer.SpelTransformerIntegrationTests$BarFunction).getMethod('bar', T(org.springframework.integration.Message))}"/>
 			</util:map>
 		</beans:property>
 	</beans:bean>

spring-integration-core/src/test/java/org/springframework/integration/transformer/SpelTransformerIntegrationTests.java

@@ -21,7 +21,7 @@ import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-import java.util.List;
+import java.util.Map;
 
 import org.hamcrest.Matchers;
 import org.junit.Test;
@@ -121,7 +121,7 @@ public class SpelTransformerIntegrationTests {
 		assertNotNull(reply);
 		assertTrue(reply.getPayload() instanceof String);
 		assertEquals("baz", reply.getPayload());
-		assertEquals(4, TestUtils.getPropertyValue(this.evaluationContextFactoryBean, "propertyAccessors", List.class).size());
+		assertEquals(3, TestUtils.getPropertyValue(this.evaluationContextFactoryBean, "propertyAccessors", Map.class).size());
 	}
 
 	@Test

src/reference/docbook/spel.xml

@@ -56,9 +56,11 @@
 		<programlisting language="xml"><![CDATA[<beans:bean id="integrationEvaluationContext"
 			class="org.springframework.integration.config.IntegrationEvaluationContextFactoryBean">
 	<property name="propertyAccessors">
-		<list>
-			<bean class="foo.MyCustomPropertyAccessor"/>
-		</list>
+		<util:map>
+			<beans:entry key="foo">
+				<beans:bean class="<bean class="foo.MyCustomPropertyAccessor"/>"/>
+			</beans:entry>
+		</util:map>
 	</property>
 	<property name="functions">
 		<map>