From b30f4041734b5c42fd2cca272f0967e768002bbc Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Sun, 19 Feb 2017 12:32:49 -0500 Subject: [PATCH] INT-4230: Fix (S)FTP Preserve Timestamp on MGET JIRA: https://jira.spring.io/browse/INT-4230 The `get()` method only preserves the timestamp when an LS operation is done within it. For MGET ops, we don't perform another LS, so the timestamp was not updated. Add code to the recursive and non-recursive MGET methods to preserve the timestamp if so configured. --- .../AbstractRemoteFileOutboundGateway.java | 12 +++- .../FtpServerOutboundTests-context.xml | 5 +- .../ftp/outbound/FtpServerOutboundTests.java | 62 +++++++++++++----- .../SftpServerOutboundTests-context.xml | 4 +- .../outbound/SftpServerOutboundTests.java | 63 ++++++++++++++----- 5 files changed, 106 insertions(+), 40 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 e3dc1a3831..9911a10228 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2017 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. @@ -954,8 +954,11 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ String fileName = this.getRemoteFilename(fullFileName); String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName); - File file = this.get(message, session, actualRemoteDirectory, + File file = get(message, session, actualRemoteDirectory, fullFileName, fileName, false); + if (this.options.contains(Option.PRESERVE_TIMESTAMP)) { + file.setLastModified(getModified(lsEntry.getFileInfo())); + } files.add(file); } } @@ -997,8 +1000,11 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ String fileName = this.getRemoteFilename(fullFileName); String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName); - File file = this.get(message, session, actualRemoteDirectory, + File file = get(message, session, actualRemoteDirectory, fullFileName, fileName, false); + if (this.options.contains(Option.PRESERVE_TIMESTAMP)) { + file.setLastModified(getModified(lsEntry.getFileInfo())); + } files.add(file); } } 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 ff2b41f008..78f0e0d3dd 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 @@ -20,6 +20,7 @@ (dir + " ftpSource1.txt")); Message result = this.output.receive(1000); assertNotNull(result); File localFile = (File) result.getPayload(); - assertThat(localFile.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir.toUpperCase())); + assertThat(localFile.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir.toUpperCase())); + assertPreserved(modified, localFile); dir = "ftpSource/subFtpSource/"; this.inboundGet.send(new GenericMessage(dir + "subFtpSource1.txt")); result = this.output.receive(1000); assertNotNull(result); localFile = (File) result.getPayload(); - assertThat(localFile.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir.toUpperCase())); + assertThat(localFile.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir.toUpperCase())); } @Test @@ -192,6 +195,7 @@ public class FtpServerOutboundTests extends FtpTestSupport { @SuppressWarnings("unchecked") public void testInt2866LocalDirectoryExpressionMGET() { String dir = "ftpSource/"; + long modified = setModifiedOnSource1(); this.inboundMGet.send(new GenericMessage(dir + "*.txt")); Message result = this.output.receive(1000); assertNotNull(result); @@ -199,10 +203,14 @@ public class FtpServerOutboundTests extends FtpTestSupport { assertThat(localFiles.size(), Matchers.greaterThan(0)); + boolean assertedModified = false; for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), containsString(dir)); + if (file.getPath().contains("localTarget1")) { + assertedModified = assertPreserved(modified, file); + } } + assertTrue(assertedModified); dir = "ftpSource/subFtpSource/"; this.inboundMGet.send(new GenericMessage(dir + "*.txt")); @@ -213,8 +221,7 @@ public class FtpServerOutboundTests extends FtpTestSupport { assertThat(localFiles.size(), Matchers.greaterThan(0)); for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), containsString(dir)); } } @@ -242,21 +249,42 @@ public class FtpServerOutboundTests extends FtpTestSupport { @SuppressWarnings("unchecked") public void testInt3172LocalDirectoryExpressionMGETRecursive() { String dir = "ftpSource/"; + long modified = setModifiedOnSource1(); this.inboundMGetRecursive.send(new GenericMessage("*")); Message result = this.output.receive(1000); assertNotNull(result); List localFiles = (List) result.getPayload(); assertEquals(3, localFiles.size()); + boolean assertedModified = false; for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir)); + if (file.getPath().contains("localTarget1")) { + assertedModified = assertPreserved(modified, file); + } } - assertThat(localFiles.get(2).getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir + "subFtpSource")); + assertTrue(assertedModified); + assertThat(localFiles.get(2).getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir + "subFtpSource")); } + private long setModifiedOnSource1() { + File firstRemote = new File(getSourceRemoteDirectory(), " ftpSource1.txt"); + firstRemote.setLastModified(System.currentTimeMillis() - 1_000_000); + long modified = firstRemote.lastModified(); + assertTrue(modified > 0); + return modified; + } + + private boolean assertPreserved(long modified, File file) { + // ftp only has 1 minute resolution + assertTrue("lastModified wrong by " + (modified - file.lastModified()), + Math.abs(file.lastModified() - modified) < 61_000); + return true; + } + @Test @SuppressWarnings("unchecked") public void testInt3172LocalDirectoryExpressionMGETRecursiveFiltered() { @@ -269,11 +297,11 @@ public class FtpServerOutboundTests extends FtpTestSupport { assertEquals(2, localFiles.size()); for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir)); } - assertThat(localFiles.get(1).getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir + "subFtpSource")); + assertThat(localFiles.get(1).getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir + "subFtpSource")); } diff --git a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml index 8423aa63f8..0738f2f876 100644 --- a/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml +++ b/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpServerOutboundTests-context.xml @@ -20,6 +20,7 @@ session = this.sessionFactory.getSession(); String dir = "sftpSource/"; + long modified = setModifiedOnSource1(); this.inboundGet.send(new GenericMessage(dir + " sftpSource1.txt")); Message result = this.output.receive(1000); assertNotNull(result); File localFile = (File) result.getPayload(); - assertThat(localFile.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir.toUpperCase())); + assertThat(localFile.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir.toUpperCase())); + assertPreserved(modified, localFile); dir = "sftpSource/subSftpSource/"; this.inboundGet.send(new GenericMessage(dir + "subSftpSource1.txt")); result = this.output.receive(1000); assertNotNull(result); localFile = (File) result.getPayload(); - assertThat(localFile.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir.toUpperCase())); + assertThat(localFile.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir.toUpperCase())); Session session2 = this.sessionFactory.getSession(); assertSame(TestUtils.getPropertyValue(session, "targetSession.jschSession"), TestUtils.getPropertyValue(session2, "targetSession.jschSession")); @@ -171,6 +174,7 @@ public class SftpServerOutboundTests extends SftpTestSupport { @SuppressWarnings("unchecked") public void testInt2866LocalDirectoryExpressionMGET() { String dir = "sftpSource/"; + long modified = setModifiedOnSource1(); this.inboundMGet.send(new GenericMessage(dir + "*.txt")); Message result = this.output.receive(1000); assertNotNull(result); @@ -178,10 +182,15 @@ public class SftpServerOutboundTests extends SftpTestSupport { assertThat(localFiles.size(), Matchers.greaterThan(0)); + boolean assertedModified = false; for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir)); + if (file.getPath().contains("localTarget1")) { + assertedModified = assertPreserved(modified, file); + } } + assertTrue(assertedModified); dir = "sftpSource/subSftpSource/"; this.inboundMGet.send(new GenericMessage(dir + "*.txt")); @@ -192,8 +201,8 @@ public class SftpServerOutboundTests extends SftpTestSupport { assertThat(localFiles.size(), Matchers.greaterThan(0)); for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir)); } } @@ -201,21 +210,41 @@ public class SftpServerOutboundTests extends SftpTestSupport { @SuppressWarnings("unchecked") public void testInt3172LocalDirectoryExpressionMGETRecursive() { String dir = "sftpSource/"; + long modified = setModifiedOnSource1(); this.inboundMGetRecursive.send(new GenericMessage(dir + "*")); Message result = this.output.receive(1000); assertNotNull(result); List localFiles = (List) result.getPayload(); assertEquals(3, localFiles.size()); + boolean assertedModified = false; for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir)); + if (file.getPath().contains("localTarget1")) { + assertedModified = assertPreserved(modified, file); + } } - assertThat(localFiles.get(2).getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir + "subSftpSource")); + assertTrue(assertedModified); + assertThat(localFiles.get(2).getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir + "subSftpSource")); } + private long setModifiedOnSource1() { + File firstRemote = new File(getSourceRemoteDirectory(), " sftpSource1.txt"); + firstRemote.setLastModified(System.currentTimeMillis() - 1_000_000); + long modified = firstRemote.lastModified(); + assertTrue(modified > 0); + return modified; + } + + private boolean assertPreserved(long modified, File file) { + assertTrue("lastModified wrong by " + (modified - file.lastModified()), + Math.abs(file.lastModified() - modified) < 1_000); + return true; + } + @Test @SuppressWarnings("unchecked") public void testInt3172LocalDirectoryExpressionMGETRecursiveFiltered() { @@ -228,11 +257,11 @@ public class SftpServerOutboundTests extends SftpTestSupport { assertEquals(2, localFiles.size()); for (File file : localFiles) { - assertThat(file.getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir)); + assertThat(file.getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir)); } - assertThat(localFiles.get(1).getPath().replaceAll(java.util.regex.Matcher.quoteReplacement(File.separator), "/"), - Matchers.containsString(dir + "subSftpSource")); + assertThat(localFiles.get(1).getPath().replaceAll(quoteReplacement(File.separator), "/"), + containsString(dir + "subSftpSource")); }