From f751398cfe97e29d50dd2435d8d47c6c6ab4cd7e Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Thu, 6 Apr 2017 11:04:33 -0400 Subject: [PATCH] ARFOGateway: Directory LiteralExpression->Value... Using a `LiteralExpression` in the `AbstractRemoteFileOutboundGateway` (when the user provides a `File` for the directory) causes us to create a new `File` object each time, instead of using the user-supplied `File`. Use a `ValueExpression` instead. Move the `FileWritingMessageHandler` logic to `ExpressionUtils` and use it from both places. Also add a String setter variant for convenient Java Configuration. --- .../expression/ExpressionUtils.java | 50 +++++++++++++++++-- .../file/FileWritingMessageHandler.java | 31 +----------- .../AbstractRemoteFileOutboundGateway.java | 27 ++++++---- ...utboundChannelAdapterIntegrationTests.java | 13 ++--- 4 files changed, 74 insertions(+), 47 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/expression/ExpressionUtils.java b/spring-integration-core/src/main/java/org/springframework/integration/expression/ExpressionUtils.java index 71786cfdd8..91bd877e58 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/expression/ExpressionUtils.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/expression/ExpressionUtils.java @@ -16,6 +16,8 @@ package org.springframework.integration.expression; +import java.io.File; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -23,24 +25,31 @@ import org.springframework.beans.factory.BeanFactory; import org.springframework.context.expression.BeanFactoryResolver; import org.springframework.context.expression.MapAccessor; import org.springframework.core.convert.ConversionService; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; import org.springframework.expression.spel.support.StandardEvaluationContext; import org.springframework.expression.spel.support.StandardTypeConverter; import org.springframework.integration.context.IntegrationContextUtils; import org.springframework.integration.support.utils.IntegrationUtils; +import org.springframework.messaging.Message; +import org.springframework.util.Assert; /** - * Utility class with static methods for helping with establishing environments for - * SpEL expressions. + * Utility class with static methods for helping with evaluation of SpEL expressions. * * @author Gary Russell * @author Oleg Zhurakousky * @author Artem Bilan * @since 2.2 */ -public abstract class ExpressionUtils { +public final class ExpressionUtils { private static final Log logger = LogFactory.getLog(ExpressionUtils.class); + private ExpressionUtils() { + super(); + } + /** * Create a {@link StandardEvaluationContext} with a {@link MapAccessor} in its * property accessor property and the supplied {@link ConversionService} in its @@ -97,4 +106,39 @@ public abstract class ExpressionUtils { return evaluationContext; } + /** + * Evaluate an expression and return a {@link File} object; the expression can evaluate + * to a {@link String} or {@link File}. + * @param expression the expression. + * @param evaluationContext the evaluation context. + * @param message the message (if available). + * @param name the name of the result of the evaluation. + * @return the File. + * @since 5.0 + */ + public static File expressionToFile(Expression expression, EvaluationContext evaluationContext, Message message, + String name) { + File file; + Object value = expression.getValue(evaluationContext, message); + if (value == null) { + throw new IllegalStateException(String.format("The provided %s expression (%s) must not evaluate to null.", + name, expression.getExpressionString())); + } + else if (value instanceof File) { + file = (File) value; + } + else if (value instanceof String) { + String path = (String) value; + Assert.hasText(path, String.format("Unable to resolve %s for the provided Expression '%s'.", name, + expression.getExpressionString())); + file = new File(path); + } + else { + throw new IllegalStateException(String.format( + "The provided %s expression (%s) must evaluate to type java.io.File or String, not %s.", name, + expression.getExpressionString(), value.getClass().getName())); + } + return file; + } + } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java b/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java index 9b334ffda2..6df2f08008 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java @@ -767,35 +767,8 @@ public class FileWritingMessageHandler extends AbstractReplyProducingMessageHand } private File evaluateDestinationDirectoryExpression(Message message) { - - final File destinationDirectory; - - final Object destinationDirectoryToUse = this.destinationDirectoryExpression.getValue( - this.evaluationContext, message); - - if (destinationDirectoryToUse == null) { - throw new IllegalStateException(String.format("The provided " + - "destinationDirectoryExpression (%s) must not resolve to null.", - this.destinationDirectoryExpression.getExpressionString())); - } - else if (destinationDirectoryToUse instanceof String) { - - final String destinationDirectoryPath = (String) destinationDirectoryToUse; - - Assert.hasText(destinationDirectoryPath, String.format( - "Unable to resolve destination directory name for the provided Expression '%s'.", - this.destinationDirectoryExpression.getExpressionString())); - destinationDirectory = new File(destinationDirectoryPath); - } - else if (destinationDirectoryToUse instanceof File) { - destinationDirectory = (File) destinationDirectoryToUse; - } - else { - throw new IllegalStateException(String.format("The provided " + - "destinationDirectoryExpression (%s) must be of type " + - "java.io.File or be a String.", this.destinationDirectoryExpression.getExpressionString())); - } - + final File destinationDirectory = ExpressionUtils.expressionToFile(this.destinationDirectoryExpression, + this.evaluationContext, message, "Destination Directory"); validateDestinationDirectory(destinationDirectory, this.autoCreateDirectory); return destinationDirectory; } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java index 006d1cff7d..ede918b499 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java @@ -33,11 +33,11 @@ import java.util.Set; import org.springframework.expression.EvaluationContext; import org.springframework.expression.Expression; -import org.springframework.expression.common.LiteralExpression; import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.integration.IntegrationMessageHeaderAccessor; import org.springframework.integration.expression.ExpressionUtils; import org.springframework.integration.expression.FunctionExpression; +import org.springframework.integration.expression.ValueExpression; import org.springframework.integration.file.FileHeaders; import org.springframework.integration.file.filters.FileListFilter; import org.springframework.integration.file.remote.AbstractFileInfo; @@ -358,18 +358,29 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ public void setLocalDirectory(File localDirectory) { if (localDirectory != null) { - this.localDirectoryExpression = new LiteralExpression(localDirectory.getAbsolutePath()); + this.localDirectoryExpression = new ValueExpression<>(localDirectory); } } /** - * Specify a SpEL expression to evaluate directory path where remote files will be transferred to. + * Specify a SpEL expression to evaluate the directory path to which remote files will + * be transferred. * @param localDirectoryExpression the SpEL to determine the local directory. */ public void setLocalDirectoryExpression(Expression localDirectoryExpression) { this.localDirectoryExpression = localDirectoryExpression; } + /** + * Specify a SpEL expression to evaluate the directory path to which remote files will + * be transferred. + * @param localDirectoryExpression the SpEL to determine the local directory. + * @since 5.0 + */ + public void setLocalDirectoryExpressionString(String localDirectoryExpression) { + this.localDirectoryExpression = EXPRESSION_PARSER.parseExpression(localDirectoryExpression); + } + /** * A {@code boolean} flag to identify if local directory should be created automatically. * Defaults to {@code true}. @@ -496,8 +507,8 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply if ((Command.GET.equals(this.command) && !this.options.contains(Option.STREAM)) || Command.MGET.equals(this.command)) { Assert.notNull(this.localDirectoryExpression, "localDirectory must not be null"); - if (this.localDirectoryExpression instanceof LiteralExpression) { - File localDirectory = new File(this.localDirectoryExpression.getExpressionString()); + if (this.localDirectoryExpression instanceof ValueExpression) { + File localDirectory = this.localDirectoryExpression.getValue(File.class); try { if (!localDirectory.exists()) { if (this.autoCreateLocalDirectory) { @@ -1059,10 +1070,8 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply if (remoteDirectory != null) { evaluationContext.setVariable("remoteDirectory", remoteDirectory); } - //TODO see org.springframework.integration.context.CustomConversionServiceFactoryBean -// File localDir = this.localDirectoryExpression.getValue(evaluationContext, message, File.class); - String localDirPath = this.localDirectoryExpression.getValue(evaluationContext, message, String.class); - File localDir = new File(localDirPath); + File localDir = ExpressionUtils.expressionToFile(this.localDirectoryExpression, evaluationContext, message, + "Local Directory"); if (!localDir.exists()) { Assert.isTrue(localDir.mkdirs(), "Failed to make local directory: " + localDir); } diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/FileOutboundChannelAdapterIntegrationTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/FileOutboundChannelAdapterIntegrationTests.java index 85ce1e7175..31c1f0f36e 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/FileOutboundChannelAdapterIntegrationTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/FileOutboundChannelAdapterIntegrationTests.java @@ -135,7 +135,8 @@ public class FileOutboundChannelAdapterIntegrationTests { this.inputChannelSaveToSubDirEmptyStringExpression.send(message); } catch (MessageHandlingException e) { - Assert.assertEquals("Unable to resolve destination directory name for the provided Expression '' ''.", e.getCause().getMessage()); + Assert.assertEquals("Unable to resolve Destination Directory for the provided Expression '' ''.", + e.getCause().getMessage()); return; } @@ -192,8 +193,8 @@ public class FileOutboundChannelAdapterIntegrationTests { this.inputChannelSaveToSubDirWithFile.send(messageWithFileHeader); } catch (MessageHandlingException e) { - Assert.assertEquals("The provided destinationDirectoryExpression " + - "(headers['subDirectory']) must not resolve to null.", + Assert.assertEquals("The provided Destination Directory expression " + + "(headers['subDirectory']) must not evaluate to null.", e.getCause().getMessage()); return; @@ -214,9 +215,9 @@ public class FileOutboundChannelAdapterIntegrationTests { this.inputChannelSaveToSubDirWithFile.send(messageWithFileHeader); } catch (MessageHandlingException e) { - Assert.assertEquals("The provided destinationDirectoryExpression" + - " (headers['subDirectory']) must be of type " + - "java.io.File or be a String.", + Assert.assertEquals("The provided Destination Directory expression" + + " (headers['subDirectory']) must evaluate to type " + + "java.io.File or String, not java.lang.Integer.", e.getCause().getMessage()); return;