From 0903df967680a7be8ed80d4d2edb78ceae25c459 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 23 Jun 2017 11:16:04 -0400 Subject: [PATCH] INT-4303: (S)FTP MGET with FileExistsMode.IGNORE JIRA: https://jira.spring.io/browse/INT-4303 Don't emit ignored files in the output message payload. Also, always preserve the timestamp when using `FileExistsMode.REPLACE_IF_MODIFIED`. Some code style polishing --- .../AbstractRemoteFileOutboundGateway.java | 27 +++++++++++++------ .../FtpServerOutboundTests-context.xml | 1 + .../ftp/outbound/FtpServerOutboundTests.java | 5 ++++ src/reference/asciidoc/ftp.adoc | 6 ++++- src/reference/asciidoc/sftp.adoc | 7 ++++- 5 files changed, 36 insertions(+), 10 deletions(-) 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 bf436f8faf..99f28547ab 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 @@ -1039,12 +1039,18 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply if (!appending && !tempFile.renameTo(localFile)) { throw new MessagingException("Failed to rename local file"); } - if (this.options.contains(Option.PRESERVE_TIMESTAMP)) { + if (this.options.contains(Option.PRESERVE_TIMESTAMP) + || FileExistsMode.REPLACE_IF_MODIFIED.equals(fileExistsMode)) { localFile.setLastModified(getModified(fileInfo)); } } else if (FileExistsMode.REPLACE_IF_MODIFIED.equals(fileExistsMode)) { - logger.debug("Local file '" + localFile + "' has the same modified timestamp, ignored"); + if (logger.isDebugEnabled()) { + logger.debug("Local file '" + localFile + "' has the same modified timestamp, ignored"); + } + if (this.command.equals(Command.MGET)) { + localFile = null; + } } else if (!FileExistsMode.IGNORE.equals(fileExistsMode)) { throw new MessageHandlingException(message, "Local file " + localFile + " already exists"); @@ -1053,6 +1059,9 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply if (logger.isDebugEnabled()) { logger.debug("Existing file skipped: " + localFile); } + if (this.command.equals(Command.MGET)) { + localFile = null; + } } return localFile; } @@ -1098,9 +1107,10 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ String fileName = this.getRemoteFilename(fullFileName); String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName); - File file = get(message, session, actualRemoteDirectory, - fullFileName, fileName, lsEntry.getFileInfo()); - files.add(file); + File file = get(message, session, actualRemoteDirectory, fullFileName, fileName, lsEntry.getFileInfo()); + if (file != null) { + files.add(file); + } } } catch (Exception e) { @@ -1141,9 +1151,10 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ String fileName = this.getRemoteFilename(fullFileName); String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName); - File file = get(message, session, actualRemoteDirectory, - fullFileName, fileName, lsEntry.getFileInfo()); - files.add(file); + File file = get(message, session, actualRemoteDirectory, fullFileName, fileName, lsEntry.getFileInfo()); + if (file != null) { + files.add(file); + } } } catch (Exception e) { diff --git a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests-context.xml b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests-context.xml index 67a5000b2f..945141eda9 100644 --- a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests-context.xml +++ b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests-context.xml @@ -42,6 +42,7 @@ command="mget" command-options="-f -P" expression="payload" + mode="IGNORE" local-directory-expression="@extraConfig.targetLocalDirectoryName + (#remoteDirectory ?: '')" local-filename-generator-expression="#remoteFileName.replaceFirst('ftpSource', 'localTarget')" reply-channel="output"/> diff --git a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests.java b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests.java index d9970eebef..ed65c503df 100644 --- a/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests.java +++ b/spring-integration-ftp/src/test/java/org/springframework/integration/ftp/outbound/FtpServerOutboundTests.java @@ -228,6 +228,11 @@ public class FtpServerOutboundTests extends FtpTestSupport { for (File file : localFiles) { assertThat(file.getPath().replaceAll(Matcher.quoteReplacement(File.separator), "/"), containsString(dir)); } + this.inboundMGet.send(new GenericMessage(dir + "*.txt")); + result = this.output.receive(1000); + assertNotNull(result); + localFiles = (List) result.getPayload(); + assertThat(localFiles.size(), equalTo(0)); } @Test diff --git a/src/reference/asciidoc/ftp.adoc b/src/reference/asciidoc/ftp.adoc index 74b94b6b4f..818d55fd06 100644 --- a/src/reference/asciidoc/ftp.adoc +++ b/src/reference/asciidoc/ftp.adoc @@ -886,7 +886,7 @@ You can either do that in your custom code, or route a copy of the message to a *mget* -_mget_ retrieves multiple remote files based on a pattern and supports the following option: +_mget_ retrieves multiple remote files based on a pattern and supports the following options: * -P - preserve the timestamps of the remote files @@ -896,9 +896,13 @@ _mget_ retrieves multiple remote files based on a pattern and supports the follo The message payload resulting from an _mget_ operation is a `List` object - a List of File objects, each representing a retrieved file. +IMPORTANT: Startng with _version 5.0_, if the `FileExistsMode` is `IGNORE`, the payload of the output message will no longer contain files that were not fetched due to the file already existing. +Previously, the array contained all files, including those that already existed. + The expression used to determine the remote path should produce a result that ends with `*` - e.g. `foo/*` will fetch the complete tree under `foo`. Starting with _version 5.0_, a recursive `MGET`, combined with the new `FileExistsMode.REPLACE_IF_MODIFIED` mode, can be used to periodically synchronize an entire remote directory tree locally. +This mode will set the local file last modified timestamp with the remote timestamp, regardless of the `-P` (preserve timestamp) option. .Notes for when using recursion (`-R`) [IMPORTANT] diff --git a/src/reference/asciidoc/sftp.adoc b/src/reference/asciidoc/sftp.adoc index a4841ab342..f246037e4d 100644 --- a/src/reference/asciidoc/sftp.adoc +++ b/src/reference/asciidoc/sftp.adoc @@ -908,7 +908,7 @@ You can either do that in your custom code, or route a copy of the message to a *mget* -_mget_ retrieves multiple remote files based on a pattern and supports the following option: +_mget_ retrieves multiple remote files based on a pattern and supports the following options: * -P - preserve the timestamps of the remote files @@ -918,9 +918,14 @@ _mget_ retrieves multiple remote files based on a pattern and supports the follo The message payload resulting from an _mget_ operation is a `List` object - a List of File objects, each representing a retrieved file. +IMPORTANT: Startng with _version 5.0_, if the `FileExistsMode` is `IGNORE`, the payload of the output message will no longer contain files that were not fetched due to the file already existing. +Previously, the array contained all files, including those that already existed. + + The expression used to determine the remote path should produce a result that ends with `*` - e.g. `foo/*` will fetch the complete tree under `foo`. Starting with _version 5.0_, a recursive `MGET`, combined with the new `FileExistsMode.REPLACE_IF_MODIFIED` mode, can be used to periodically synchronize an entire remote directory tree locally. +This mode will set the local file last modified timestamp to the remote file timestamp, regardless of the `-P` (preserve timestamp) option. .Notes for when using recursion (`-R`) [IMPORTANT]