From c97ac195a54ab2194c25c9c93c617f8ca9f90b3e Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 30 Mar 2016 17:10:10 -0400 Subject: [PATCH] INT-3976: Remove an explicit `scriptClassName` JIRA: https://jira.spring.io/browse/INT-3976 The class name in case of `StaticScriptSource` (for inline scripts) is required only for Groovy native engine. Even if `GroovyScriptParser` provides an explicit `scriptClassName` option, we still can configure `GroovyScriptExecutingMessageProcessor` with the `StaticScriptSource` from Java without the script class name. Therefore the logic in the `GroovyScriptParser` isn't robust. * Remove the `GroovyScriptParser` `scriptClassName` logic altogether * Change the `GroovyScriptExecutingMessageProcessor` to rely on the Groovy native `scriptClassName` generation in case of the `scriptSource` is provided without the class name option * Fix the test for the actual generated script class name --- ...GroovyScriptExecutingMessageProcessor.java | 11 ++++++++-- .../groovy/config/GroovyScriptParser.java | 17 ++-------------- .../config/GroovyServiceActivatorTests.java | 12 +++++------ .../config/AbstractScriptParser.java | 16 +++------------ .../scripting/config/jsr223/ScriptParser.java | 20 ++++--------------- 5 files changed, 24 insertions(+), 52 deletions(-) diff --git a/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/GroovyScriptExecutingMessageProcessor.java b/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/GroovyScriptExecutingMessageProcessor.java index 4c516b3740..364559121c 100644 --- a/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/GroovyScriptExecutingMessageProcessor.java +++ b/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/GroovyScriptExecutingMessageProcessor.java @@ -34,6 +34,7 @@ import org.springframework.scripting.ScriptSource; import org.springframework.scripting.groovy.GroovyObjectCustomizer; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; +import org.springframework.util.StringUtils; import groovy.lang.Binding; import groovy.lang.GString; @@ -164,8 +165,14 @@ public class GroovyScriptExecutingMessageProcessor extends AbstractScriptExecuti try { // synchronized double check if (this.scriptClass == null || scriptSource.isModified()) { - this.scriptClass = this.groovyClassLoader.parseClass( - scriptSource.getScriptAsString(), scriptSource.suggestedClassName()); + String className = scriptSource.suggestedClassName(); + if (StringUtils.hasText(className)) { + this.scriptClass = + this.groovyClassLoader.parseClass(scriptSource.getScriptAsString(), className); + } + else { + this.scriptClass = this.groovyClassLoader.parseClass(scriptSource.getScriptAsString()); + } } } finally { diff --git a/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/config/GroovyScriptParser.java b/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/config/GroovyScriptParser.java index 099ae252df..505b974d5b 100644 --- a/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/config/GroovyScriptParser.java +++ b/spring-integration-groovy/src/main/java/org/springframework/integration/groovy/config/GroovyScriptParser.java @@ -24,8 +24,6 @@ import org.springframework.integration.config.xml.IntegrationNamespaceUtils; import org.springframework.integration.groovy.GroovyScriptExecutingMessageProcessor; import org.springframework.integration.scripting.config.AbstractScriptParser; -import groovy.lang.Script; - /** * Parser for the <groovy:script/> element. * @@ -39,16 +37,8 @@ public class GroovyScriptParser extends AbstractScriptParser { @Override - protected String getBeanClassName(Element element) { - return GroovyScriptExecutingMessageProcessor.class.getName(); - } - - /* (non-Javadoc) - * @see org.springframework.integration.config.xml.AbstractScriptParser#getScriptSourceClassName() - */ - @Override - protected String getScriptSourceClassName() { - return Script.class.getName(); + protected Class getBeanClass(Element element) { + return GroovyScriptExecutingMessageProcessor.class; } protected void postProcess(BeanDefinitionBuilder builder, Element element, ParserContext parserContext) { @@ -57,7 +47,4 @@ public class GroovyScriptParser extends AbstractScriptParser { IntegrationNamespaceUtils.setValueIfAttributeDefined(builder, element, "compile-static"); } - - - } diff --git a/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyServiceActivatorTests.java b/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyServiceActivatorTests.java index 6dc494a1fb..9bd626bb9e 100644 --- a/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyServiceActivatorTests.java +++ b/spring-integration-groovy/src/test/java/org/springframework/integration/groovy/config/GroovyServiceActivatorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -192,7 +192,7 @@ public class GroovyServiceActivatorTests { Throwable cause = e.getCause(); assertEquals(MissingPropertyException.class, cause.getClass()); assertThat(cause.getMessage(), - Matchers.containsString("No such property: ReplyRequiredException for class: groovy_lang")); + Matchers.containsString("No such property: ReplyRequiredException for class: script")); throw e; } @@ -200,7 +200,7 @@ public class GroovyServiceActivatorTests { @Test(expected = BeanDefinitionParsingException.class) public void variablesAndScriptVariableGenerator() throws Exception { - new ClassPathXmlApplicationContext("GroovyServiceActivatorTests-fail-withgenerator-context.xml", + new ClassPathXmlApplicationContext("GroovyServiceActivatorTests-fail-withgenerator-context.xml", this.getClass()).close(); } @@ -212,7 +212,7 @@ public class GroovyServiceActivatorTests { public static class SampleScriptVariSource implements ScriptVariableGenerator { - + public Map generateScriptVariables(Message message) { Map variables = new HashMap(); variables.put("foo", "foo"); @@ -222,7 +222,7 @@ public class GroovyServiceActivatorTests { variables.put("headers", message.getHeaders()); return variables; } - + } @@ -233,7 +233,7 @@ public class GroovyServiceActivatorTests { public void customize(GroovyObject goo) { this.executed = true; } - + } } diff --git a/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/AbstractScriptParser.java b/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/AbstractScriptParser.java index 9e0b8aa5e7..dd8555e033 100644 --- a/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/AbstractScriptParser.java +++ b/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/AbstractScriptParser.java @@ -50,16 +50,11 @@ public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionP return true; } - @Override - protected abstract String getBeanClassName(Element element); - - protected abstract String getScriptSourceClassName(); - @Override protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) { String scriptLocation = element.getAttribute(LOCATION_ATTRIBUTE); String scriptText = DomUtils.getTextValue(element); - if (!(StringUtils.hasText(scriptLocation) ^ StringUtils.hasText(scriptText))) { + if (StringUtils.hasText(scriptLocation) == StringUtils.hasText(scriptText)) { parserContext.getReaderContext().error( "Either the 'location' attribute or inline script text must be provided, but not both.", element); return; @@ -80,12 +75,7 @@ public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionP scriptLocation)); } else { - if (getScriptSourceClassName() != null) { - builder.addConstructorArgValue(new StaticScriptSource(scriptText, getScriptSourceClassName())); - } - else { - builder.addConstructorArgValue(new StaticScriptSource(scriptText)); - } + builder.addConstructorArgValue(new StaticScriptSource(scriptText)); } BeanMetadataElement scriptVariableGeneratorDef = null; @@ -150,7 +140,7 @@ public abstract class AbstractScriptParser extends AbstractSingleBeanDefinitionP String variableName = childElement.getAttribute("name"); String variableValue = childElement.getAttribute("value"); String variableRef = childElement.getAttribute("ref"); - if (!(StringUtils.hasText(variableValue) ^ StringUtils.hasText(variableRef))) { + 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); diff --git a/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/jsr223/ScriptParser.java b/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/jsr223/ScriptParser.java index cea39cde61..ee18852c79 100644 --- a/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/jsr223/ScriptParser.java +++ b/spring-integration-scripting/src/main/java/org/springframework/integration/scripting/config/jsr223/ScriptParser.java @@ -24,11 +24,13 @@ import org.w3c.dom.Element; import org.springframework.beans.factory.support.BeanDefinitionBuilder; import org.springframework.beans.factory.xml.ParserContext; import org.springframework.integration.scripting.config.AbstractScriptParser; +import org.springframework.integration.scripting.jsr223.ScriptExecutingMessageProcessor; import org.springframework.integration.scripting.jsr223.ScriptExecutorFactory; import org.springframework.util.StringUtils; /** * @author David Turanski + * @author Artem Bilan * @since 2.1 */ public class ScriptParser extends AbstractScriptParser { @@ -36,16 +38,8 @@ public class ScriptParser extends AbstractScriptParser { private static final String LANGUAGE_ATTRIBUTE = "lang"; @Override - protected String getBeanClassName(Element element) { - return "org.springframework.integration.scripting.jsr223.ScriptExecutingMessageProcessor"; - } - - /* (non-Javadoc) - * @see org.springframework.integration.config.xml.AbstractScriptParser#getScriptSourceClassName() - */ - @Override - protected String getScriptSourceClassName() { - return null; + protected Class getBeanClass(Element element) { + return ScriptExecutingMessageProcessor.class; } @Override @@ -71,12 +65,6 @@ public class ScriptParser extends AbstractScriptParser { builder.addConstructorArgValue(ScriptExecutorFactory.getScriptExecutor(language)); } - /** - * @param scriptLocation - * @param parserContext - * @param element - * @return the language - */ private String getLanguageFromFileExtension(String scriptLocation, ParserContext parserContext, Element element) { ScriptEngineManager engineManager = new ScriptEngineManager(); ScriptEngine engine = null;