INT-2491: Scripting: Variables Improvements

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

* Add support variables for 'inline' scripts
* Add convenient attribute `variables` for script tags
* Introduce `BeanFactoryFallbackBinding` for groovy scripts.

INT-2491: Polishing

* Add check for duplication of provided variables
* Tests and Docs

Polishing

spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/AbstractScriptParser.java

@@ -16,14 +16,15 @@ import java.util.List;
 
 import org.w3c.dom.Element;
 
+import org.springframework.beans.BeanMetadataElement;
 import org.springframework.beans.factory.config.RuntimeBeanReference;
 import org.springframework.beans.factory.support.BeanDefinitionBuilder;
-import org.springframework.beans.factory.support.BeanDefinitionReaderUtils;
 import org.springframework.beans.factory.support.ManagedMap;
 import org.springframework.beans.factory.xml.AbstractSingleBeanDefinitionParser;
 import org.springframework.beans.factory.xml.ParserContext;
 import org.springframework.beans.factory.xml.XmlReaderContext;
 import org.springframework.integration.config.xml.IntegrationNamespaceUtils;
+import org.springframework.integration.scripting.DefaultScriptVariableGenerator;
 import org.springframework.integration.scripting.RefreshableResourceScriptSource;
 import org.springframework.scripting.support.StaticScriptSource;
 import org.springframework.util.CollectionUtils;
@@ -32,6 +33,7 @@ import org.springframework.util.xml.DomUtils;
 
 /**
  * @author David Turanski
+ * @author Artem Bilan
  *
  */
 public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionParser {
@@ -62,13 +64,6 @@ public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionP
 		List<Element> variableElements = DomUtils.getChildElementsByTagName(element, "variable");
 		String scriptVariableGeneratorName = element.getAttribute("script-variable-generator");
 
-		if (StringUtils.hasText(scriptText)
-				&& (variableElements.size() > 0 || StringUtils.hasText(scriptVariableGeneratorName))) {
-			parserContext.getReaderContext().error(
-					"Variable bindings or custom ScriptVariableGenerator are not allowed when using an inline groovy script. "
-							+ "Specify location of the script via 'location' attribute instead", element);
-			return;
-		}
 
 		if (StringUtils.hasText(scriptVariableGeneratorName) && variableElements.size() > 0) {
 			parserContext.getReaderContext().error(
@@ -88,33 +83,23 @@ public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionP
 				builder.addConstructorArgValue(new StaticScriptSource(scriptText));
 			}
 		}
+
+		BeanMetadataElement scriptVariableGeneratorDef = null;
+
 		if (!StringUtils.hasText(scriptVariableGeneratorName)) {
 			BeanDefinitionBuilder scriptVariableGeneratorBuilder = BeanDefinitionBuilder
-					.genericBeanDefinition("org.springframework.integration.scripting.DefaultScriptVariableGenerator");
-			ManagedMap<String, Object> variableMap = new ManagedMap<String, Object>();
-			for (Element childElement : variableElements) {
-				String variableName = childElement.getAttribute("name");
-				String variableValue = childElement.getAttribute("value");
-				String variableRef = childElement.getAttribute("ref");
-				if (!(StringUtils.hasText(variableValue) ^ StringUtils.hasText(variableRef))) {
-					parserContext.getReaderContext().error(
-							"Exactly one of the 'ref' attribute or 'value' attribute, " + " is required for element "
-									+ IntegrationNamespaceUtils.createElementDescription(element) + ".", element);
-				}
-				if (StringUtils.hasText(variableValue)) {
-					variableMap.put(variableName, variableValue);
-				}
-				else {
-					variableMap.put(variableName, new RuntimeBeanReference(variableRef));
-				}
-			}
+					.genericBeanDefinition(DefaultScriptVariableGenerator.class);
+			ManagedMap<String, Object> variableMap = buildVariablesMap(element, parserContext, variableElements);
 			if (!CollectionUtils.isEmpty(variableMap)) {
 				scriptVariableGeneratorBuilder.addConstructorArgValue(variableMap);
 			}
-			scriptVariableGeneratorName = BeanDefinitionReaderUtils.registerWithGeneratedName(
-					scriptVariableGeneratorBuilder.getBeanDefinition(), parserContext.getRegistry());
+			scriptVariableGeneratorDef = scriptVariableGeneratorBuilder.getBeanDefinition();
 		}
-		builder.addConstructorArgReference(scriptVariableGeneratorName);
+		else {
+			scriptVariableGeneratorDef = new RuntimeBeanReference(scriptVariableGeneratorName);
+		}
+
+		builder.addConstructorArgValue(scriptVariableGeneratorDef);
 		postProcess(builder, element, parserContext);
 	}
 
@@ -138,4 +123,61 @@ public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionP
 		return resourceScriptSourceBuilder.getBeanDefinition();
 	}
 
+	private ManagedMap<String, Object> buildVariablesMap(final Element element, final ParserContext parserContext,
+														 List<Element> variableElements) {
+		@SuppressWarnings("serial")
+		ManagedMap<String, Object> variableMap = new ManagedMap<String, Object>() {
+
+			@Override
+			public Object put(String key, Object value) {
+				if (this.containsKey(key)) {
+					parserContext.getReaderContext().error("Duplicated variable: " + key, element);
+				}
+				return super.put(key, value);
+			}
+
+		};
+
+		for (Element childElement : variableElements) {
+			String variableName = childElement.getAttribute("name");
+			String variableValue = childElement.getAttribute("value");
+			String variableRef = childElement.getAttribute("ref");
+			if (!(StringUtils.hasText(variableValue) ^ StringUtils.hasText(variableRef))) {
+				parserContext.getReaderContext().error(
+						"Exactly one of the 'ref' attribute or 'value' attribute, " + " is required for element "
+								+ IntegrationNamespaceUtils.createElementDescription(element) + ".", element);
+			}
+			if (StringUtils.hasText(variableValue)) {
+				variableMap.put(variableName, variableValue);
+			}
+			else {
+				variableMap.put(variableName, new RuntimeBeanReference(variableRef));
+			}
+		}
+
+		String variables = element.getAttribute("variables");
+		if (StringUtils.hasText(variables)) {
+			String[] variablePairs = StringUtils.commaDelimitedListToStringArray(variables);
+			for (String variablePair : variablePairs) {
+				String[] variableValue = variablePair.split("=");
+				if (variableValue.length != 2) {
+					parserContext.getReaderContext().error(
+							"Variable declarations in the 'variable' attribute must have the "
+									+ "form 'var=value'; found : '" + variablePair + "'", element);
+				}
+				String variable = variableValue[0].trim();
+				String value = variableValue[1];
+				if (variable.endsWith("-ref")) {
+					variable = variable.substring(0, variable.indexOf("-ref"));
+					variableMap.put(variable, new RuntimeBeanReference(value));
+				}
+				else {
+					variableMap.put(variable, value);
+				}
+			}
+		}
+
+		return variableMap;
+	}
+
 }

spring-integration-scripting/src/main/resources/org/springframework/integration/scripting/config/spring-integration-scripting-core-3.0.xsd

@@ -61,7 +61,7 @@
 			<xsd:annotation>
 				<xsd:documentation>
 					Reference to the ScriptVariableGenerator bean. This attribute is mutually
-					exclusive with any 'variable' sub-elements.
+					exclusive with any 'variable' sub-elements and 'variables' attribute.
 					</xsd:documentation>
 				<xsd:appinfo>
 					<tool:expected-type
@@ -78,6 +78,18 @@
 					</xsd:documentation>
 			</xsd:annotation>
 		</xsd:attribute>
+		<xsd:attribute name="variables">
+			<xsd:annotation>
+				<xsd:documentation>
+					Comma-delimited pairs of variables and their values.
+					the variable name can applies '-ref' suffix, which mean to determine
+					a variable value as a bean reference.
+					This attribute isn't mutually exclusive with 'variable' sub-elements
+					and all variables will be merged to one Map.
+					This attribute is mutually exclusive with 'script-variable-generator' attribute.
+				</xsd:documentation>
+			</xsd:annotation>
+		</xsd:attribute>
 	</xsd:complexType>
 
 </xsd:schema>

spring-integration-scripting/src/test/java/org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests-context.xml

@@ -10,30 +10,31 @@
 		http://www.springframework.org/schema/integration/scripting http://www.springframework.org/schema/integration/scripting/spring-integration-scripting.xsd">
 
 	<service-activator input-channel="referencedScriptInput">
-		<script:script 
+		<script:script
 				lang="python"
-				location="org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests.py">
+				location="org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests.py"
+				variables="foo2=#{'foo2'}, date2-ref=date">
 			<script:variable name="foo" value="foo"/>
 			<script:variable name="bar" value="bar"/>
 			<script:variable name="date" ref="date"/>
 		</script:script>
 	</service-activator>
-	
-	
+
+
 	<service-activator input-channel="withScriptVariableGenerator">
 		<script:script lang="ruby" location="org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests.rb"
 					   script-variable-generator="scriptVarSource"/>
 	</service-activator>
-	
-	
-	
-	<beans:bean id="scriptVarSource" 
-		class="org.springframework.integration.scripting.config.jsr223.Jsr223ServiceActivatorTests.SampleScriptVariSource"/>
+
+
+
+	<beans:bean id="scriptVarSource"
+		class="org.springframework.integration.scripting.config.jsr223.Jsr223ServiceActivatorTests$SampleScriptVariSource"/>
 
 	<service-activator input-channel="inlineScriptInput">
-		<script:script lang="ruby">
+		<script:script lang="ruby" variables="foo=#{'FOO'}, date-ref=date">
 			<![CDATA[
-				"inline-#{payload}"
+				"inline-#{payload} - #{foo} :#{date}"
 			]]>
 		</script:script>
 	</service-activator>
@@ -41,5 +42,5 @@
 	<beans:bean id="date" class="java.util.Date" scope="prototype">
 		<aop:scoped-proxy/>
 	</beans:bean>
-	
+
 </beans:beans>

spring-integration-scripting/src/test/java/org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests-fail-duplicated-variable-context.xml

@@ -7,14 +7,13 @@
 		http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd
 		http://www.springframework.org/schema/integration/scripting http://www.springframework.org/schema/integration/scripting/spring-integration-scripting.xsd">
 
-	<service-activator input-channel="inlineScriptInput">
-		<script:script lang="ruby">
-			<![CDATA[
-				return "inline-#{payload}-" + "#{foo}" + " - " + bar + " - " + date
-			]]>
+	<service-activator input-channel="duplicateVariableInput">
+		<script:script lang="ruby" variables="foo=FOO">
 			<script:variable name="foo" value="foo"/>
-			<script:variable name="bar" value="bar"/>
+			<![CDATA[
+				payload
+			]]>
 		</script:script>
 	</service-activator>
-	
+
 </beans:beans>

spring-integration-scripting/src/test/java/org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests.java

@@ -17,19 +17,21 @@
 package org.springframework.integration.scripting.config.jsr223;
 
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.hamcrest.Matchers;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 
+import org.springframework.beans.BeansException;
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.beans.factory.parsing.BeanDefinitionParsingException;
 import org.springframework.context.support.ClassPathXmlApplicationContext;
 import org.springframework.integration.Message;
 import org.springframework.integration.MessageChannel;
@@ -72,14 +74,24 @@ public class Jsr223ServiceActivatorTests {
 		String value1 = (String) replyChannel.receive(0).getPayload();
 		String value2 = (String) replyChannel.receive(0).getPayload();
 		String value3 = (String) replyChannel.receive(0).getPayload();
-		assertTrue(value1.startsWith("python-test-1-foo - bar"));
-		assertTrue(value2.startsWith("python-test-2-foo - bar"));
-		assertTrue(value3.startsWith("python-test-3-foo - bar"));
+		assertTrue(value1.startsWith("python-test-1-foo (foo2) - bar"));
+		assertTrue(value2.startsWith("python-test-2-foo (foo2) - bar"));
+		assertTrue(value3.startsWith("python-test-3-foo (foo2) - bar"));
 
 		// because we are using 'prototype bean the suffix date will be
 		// different
-		assertFalse(value1.substring(26).equals(value2.substring(26)));
-		assertFalse(value2.substring(26).equals(value3.substring(26)));
+		assertFalse(value1.substring(value1.indexOf(":") + 1, value1.lastIndexOf(":"))
+				.equals(value2.substring(value1.indexOf(":") + 1, value1.lastIndexOf(":"))));
+		assertFalse(value1.substring(value1.indexOf(":") +1, value1.lastIndexOf(":"))
+				.equals(value1.substring(value1.lastIndexOf(":"))));
+
+		assertFalse(value2.substring(value1.indexOf(":") + 1, value1.lastIndexOf(":"))
+				.equals(value3.substring(value1.indexOf(":") + 1, value1.lastIndexOf(":"))));
+
+		assertFalse(value1.substring(value1.lastIndexOf(":") + 1)
+				.equals(value2.substring(value1.lastIndexOf(":") + 1)));
+		assertFalse(value2.substring(value1.lastIndexOf(":") + 1)
+				.equals(value3.substring(value1.lastIndexOf(":") + 1)));
 
 		assertNull(replyChannel.receive(0));
 	}
@@ -118,22 +130,41 @@ public class Jsr223ServiceActivatorTests {
 			Message<?> message = MessageBuilder.withPayload("test-" + i).setReplyChannel(replyChannel).build();
 			this.inlineScriptInput.send(message);
 		}
-		assertEquals("inline-test-1", replyChannel.receive(0).getPayload());
-		assertEquals("inline-test-2", replyChannel.receive(0).getPayload());
-		assertEquals("inline-test-3", replyChannel.receive(0).getPayload());
+		String payload = (String) replyChannel.receive(0).getPayload();
+
+		assertThat(payload, Matchers.startsWith("inline-test-1 - FOO"));
+
+		payload = (String) replyChannel.receive(0).getPayload();
+		assertThat(payload, Matchers.startsWith("inline-test-2 - FOO"));
+
+		payload = (String) replyChannel.receive(0).getPayload();
+		assertThat(payload, Matchers.startsWith("inline-test-3 - FOO"));
+		assertTrue(payload.substring(payload.indexOf(":") + 1).matches(".+\\d{2}:\\d{2}:\\d{2}.+"));
+
 		assertNull(replyChannel.receive(0));
 
 	}
 
-	@Test(expected = BeanDefinitionParsingException.class)
-	public void inlineScriptAndVariables() throws Exception {
-		new ClassPathXmlApplicationContext("Jsr223ServiceActivatorTests-fail-context.xml", this.getClass());
+	@Test
+	public void variablesAndScriptVariableGenerator() throws Exception {
+		try {
+			new ClassPathXmlApplicationContext("Jsr223ServiceActivatorTests-fail-withgenerator-context.xml", this.getClass());
+			fail("BeansException expected.");
+		}
+		catch (BeansException e) {
+			assertThat(e.getMessage(), Matchers.containsString("'script-variable-generator' and 'variable' sub-elements are mutually exclusive."));
+		}
 	}
 
-	@Test(expected = BeanDefinitionParsingException.class)
-	public void variablesAndScriptVariableGenerator() throws Exception {
-		new ClassPathXmlApplicationContext("Jsr223ServiceActivatorTests-fail-withgenerator-context.xml",
-				this.getClass());
+	@Test
+	public void testDuplicateVariable() throws Exception {
+		try {
+			new ClassPathXmlApplicationContext("Jsr223ServiceActivatorTests-fail-duplicated-variable-context.xml", this.getClass());
+			fail("BeansException expected.");
+		}
+		catch (BeansException e) {
+			assertThat(e.getMessage(), Matchers.containsString("Duplicated variable: foo"));
+		}
 	}
 
 	public static class SampleScriptVariSource implements ScriptVariableGenerator {

spring-integration-scripting/src/test/java/org/springframework/integration/scripting/config/jsr223/Jsr223ServiceActivatorTests.py

@@ -1 +1 @@
-"python-%s-%s - %s - %s" %(payload,foo,bar,date)
+"python-%s-%s (%s) - %s - :%s:%s" %(payload,foo,foo2,bar,date,date2)

spring-integration-scripting/src/test/java/org/springframework/integration/scripting/jsr223/Jsr223ScriptExecutingMessageProcessorTests.java

@@ -1,11 +1,11 @@
 /*
  * Copyright 2002-2011 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. 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.
@@ -19,6 +19,9 @@ import java.util.Map;
 
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.Mockito;
+
+import org.springframework.beans.factory.BeanFactory;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.integration.Message;
 import org.springframework.integration.message.GenericMessage;
@@ -31,42 +34,45 @@ import org.springframework.scripting.support.ResourceScriptSource;
  *
  */
 public class Jsr223ScriptExecutingMessageProcessorTests {
+
 	ScriptExecutor executor;
+
 	@Before
 	public void setUp() {
 		executor = ScriptExecutorFactory.getScriptExecutor("jruby");
 	}
 	@Test
 	public void testExecuteWithVariables(){
-		 
-		
 		Map<String,Object> vars = new HashMap<String,Object>();
 		vars.put("one",1);
 		vars.put("two","two");
-		vars.put("three", new Integer(3));
-		
+		vars.put("three", 3);
+
 		ScriptSource scriptSource = new ResourceScriptSource(new ClassPathResource("/org/springframework/integration/scripting/jsr223/print_message.rb"));
-		
-		ScriptExecutingMessageProcessor messageProcessor = new ScriptExecutingMessageProcessor(scriptSource,executor,vars);
-		
+
+		ScriptExecutingMessageProcessor messageProcessor = new ScriptExecutingMessageProcessor(scriptSource, executor, vars);
+		messageProcessor.setBeanFactory(Mockito.mock(BeanFactory.class));
+
 		Message<?> message = new GenericMessage<String>("hello");
-		
+
 		Object obj = messageProcessor.processMessage(message);
-		
+
 		assertEquals("hello modified",obj.toString().substring(0,"hello modified".length()));
 	}
-	
+
 	@Test
 	public void testWithNoVars(){
 		ScriptSource scriptSource = new ResourceScriptSource(new ClassPathResource("/org/springframework/integration/scripting/jsr223/print_message.rb"));
-		
-		ScriptExecutingMessageProcessor messageProcessor = new ScriptExecutingMessageProcessor(scriptSource,executor);
-		
+
+		ScriptExecutingMessageProcessor messageProcessor = new ScriptExecutingMessageProcessor(scriptSource, executor);
+		messageProcessor.setBeanFactory(Mockito.mock(BeanFactory.class));
+
 		Message<?> message = new GenericMessage<String>("hello");
-		
+
 		Object obj = messageProcessor.processMessage(message);
-		
+
 		assertEquals("hello modified",obj.toString().substring(0,"hello modified".length()));
 	}
-	
+
 }
+