From a2d1edfbb95113fe354fe90c40f1a098eb6d2cd0 Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Fri, 5 Feb 2021 15:07:34 -0500 Subject: [PATCH] GH-3488: Fix Persistent Filters with Recursion Resolves https://github.com/spring-projects/spring-integration/issues/3488 Resolves two problems: - When changes are made deep in the directory tree, they were not detected because the directory is in the metadata store and only passes the filter if a file immediately under it is changed, changing the directory's timestamp. This is solved by subclassing `AbstractDirectoryAwareFileListFilter`, allowing its `alwaysAcceptDirectories` property to be set. - Only the filename was used as a metadata key; causing problems if a file with the same name appears multiple times in the tree. This is solved with a new property on `AbstractDirectoryAwareFileListFilter` used by the gateways to determine whether to filter the raw file names returned by the session (previous behavior) or the full path relative to the root directory. **cherry-pick to 5.4.x, 5.3.x** * Some code style clean up # Conflicts: # spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java # src/reference/asciidoc/whats-new.adoc --- .../AbstractDirectoryAwareFileListFilter.java | 23 +++++- ...actPersistentAcceptOnceFileListFilter.java | 9 ++- .../file/filters/CompositeFileListFilter.java | 17 +++-- .../file/filters/FileListFilter.java | 11 ++- ...temPersistentAcceptOnceFileListFilter.java | 7 +- .../AbstractRemoteFileOutboundGateway.java | 72 ++++++++++++++----- .../filters/CompositeFileListFilterTests.java | 64 +++++++++-------- ...rsistentAcceptOnceFileListFilterTests.java | 40 ++++++----- .../file/remote/RemoteFileTestSupport.java | 13 +++- .../file/remote/StreamingInboundTests.java | 9 ++- ...FtpPersistentAcceptOnceFileListFilter.java | 8 ++- ...ftpPersistentAcceptOnceFileListFilter.java | 8 ++- .../SftpServerOutboundTests-context.xml | 15 +++- .../outbound/SftpServerOutboundTests.java | 17 +++++ src/reference/asciidoc/file.adoc | 14 ++++ src/reference/asciidoc/ftp.adoc | 7 ++ src/reference/asciidoc/sftp.adoc | 7 ++ 17 files changed, 261 insertions(+), 80 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractDirectoryAwareFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractDirectoryAwareFileListFilter.java index fc0aed1641..c1b1b1eb2a 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractDirectoryAwareFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractDirectoryAwareFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2019 the original author or authors. + * Copyright 2016-2021 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. @@ -21,7 +21,10 @@ package org.springframework.integration.file.filters; * This permits, for example, pattern matching on just files when using recursion * to examine a directory tree. * + * @param the file type. + * * @author Gary Russell + * * @since 5.0 * */ @@ -29,6 +32,8 @@ public abstract class AbstractDirectoryAwareFileListFilter extends AbstractFi private boolean alwaysAcceptDirectories; + private boolean forRecursion; + /** * Set to true so that filters that support this feature can unconditionally pass * directories; default false. @@ -38,6 +43,22 @@ public abstract class AbstractDirectoryAwareFileListFilter extends AbstractFi this.alwaysAcceptDirectories = alwaysAcceptDirectories; } + @Override + public boolean isForRecursion() { + return this.forRecursion; + } + + /** + * Set to true to inform a recursive gateway operation to use the full file path as + * the metadata key. Also sets {@link #alwaysAcceptDirectories}. + * @param forRecursion true to use the full path. + * @since 5.3.6 + */ + public void setForRecursion(boolean forRecursion) { + this.forRecursion = forRecursion; + this.alwaysAcceptDirectories = forRecursion; + } + protected boolean alwaysAccept(F file) { return file != null && this.alwaysAcceptDirectories && isDirectory(file); } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractPersistentAcceptOnceFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractPersistentAcceptOnceFileListFilter.java index e159ee6d2f..2e22ac35b8 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractPersistentAcceptOnceFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AbstractPersistentAcceptOnceFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -31,13 +31,15 @@ import org.springframework.util.Assert; * Files are deemed as already 'seen' if they exist in the store and have the * same modified time as the current file. * + * @param the file type. + * * @author Gary Russell * @author Artem Bilan * * @since 3.0 * */ -public abstract class AbstractPersistentAcceptOnceFileListFilter extends AbstractFileListFilter +public abstract class AbstractPersistentAcceptOnceFileListFilter extends AbstractDirectoryAwareFileListFilter implements ReversibleFileListFilter, ResettableFileListFilter, Closeable { protected final ConcurrentMetadataStore store; // NOSONAR @@ -73,6 +75,9 @@ public abstract class AbstractPersistentAcceptOnceFileListFilter extends Abst @Override public boolean accept(F file) { + if (alwaysAccept(file)) { + return true; + } String key = buildKey(file); String newValue = value(file); String oldValue = this.store.putIfAbsent(key, newValue); diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java index cf573c1a08..7019884521 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/CompositeFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -56,6 +56,8 @@ public class CompositeFileListFilter private boolean allSupportAccept = true; + private boolean oneIsForRecursion; + public CompositeFileListFilter() { this.fileFilters = new LinkedHashSet<>(); @@ -63,9 +65,14 @@ public class CompositeFileListFilter public CompositeFileListFilter(Collection> fileFilters) { this.fileFilters = new LinkedHashSet<>(fileFilters); - this.allSupportAccept = fileFilters.stream().allMatch(FileListFilter::supportsSingleFileFiltering); + this.allSupportAccept = fileFilters.stream().allMatch(FileListFilter::supportsSingleFileFiltering); + this.oneIsForRecursion = fileFilters.stream().anyMatch(FileListFilter::isForRecursion); } + @Override + public boolean isForRecursion() { + return this.oneIsForRecursion; + } @Override public void close() throws IOException { @@ -77,7 +84,6 @@ public class CompositeFileListFilter } public CompositeFileListFilter addFilter(FileListFilter filter) { - this.allSupportAccept &= filter.supportsSingleFileFiltering(); return addFilters(Collections.singletonList(filter)); } @@ -114,11 +120,10 @@ public class CompositeFileListFilter throw new IllegalStateException(e); } } + this.allSupportAccept &= elf.supportsSingleFileFiltering(); + this.oneIsForRecursion |= elf.isForRecursion(); } this.fileFilters.addAll(filtersToAdd); - if (this.allSupportAccept) { - this.allSupportAccept = filtersToAdd.stream().allMatch(FileListFilter::supportsSingleFileFiltering); - } return this; } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileListFilter.java index 48335ce23d..16da0ba6a5 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -64,4 +64,13 @@ public interface FileListFilter { return false; } + /** + * Return true if this filter is being used for recursion. + * @return whether or not to filter based on the full path. + * @since 5.3.6 + */ + default boolean isForRecursion() { + return false; + } + } diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileSystemPersistentAcceptOnceFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileSystemPersistentAcceptOnceFileListFilter.java index 0efc826ea3..35f6534261 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileSystemPersistentAcceptOnceFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/FileSystemPersistentAcceptOnceFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -23,6 +23,7 @@ import org.springframework.integration.metadata.ConcurrentMetadataStore; /** * @author Gary Russell + * * @since 3.0 * */ @@ -52,5 +53,9 @@ public class FileSystemPersistentAcceptOnceFileListFilter extends AbstractPersis return file.exists(); } + @Override + protected boolean isDirectory(File file) { + return file.isDirectory(); + } } 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 3bf418a78b..54d86e81ca 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 @@ -97,6 +97,8 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ private FileListFilter filter; + private boolean filterAfterEnhancement; + /** * A {@link FileListFilter} that runs against the local file system view when * using MPUT. @@ -402,6 +404,15 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply */ public void setFilter(FileListFilter filter) { this.filter = filter; + this.filterAfterEnhancement = filter != null + && filter.isForRecursion() + && filter.supportsSingleFileFiltering() + && this.options.contains(Option.RECURSIVE); + if (filter != null && !filter.isForRecursion()) { + this.logger.warn("When using recursion, you will normally want to set the filter's " + + "'forRecursion' property; otherwise files added deep into the " + + "directory tree may not be detected"); + } } /** @@ -947,12 +958,19 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply List lsFiles = new ArrayList<>(); String remoteDirectory = buildRemotePath(directory, subDirectory); - F[] files = session.list(remoteDirectory); - boolean recursion = this.options.contains(Option.RECURSIVE); + F[] list = session.list(remoteDirectory); + List files; + if (!this.filterAfterEnhancement) { + files = filterFiles(list); + } + else { + files = Arrays.asList(list); + } if (!ObjectUtils.isEmpty(files)) { - for (F file : filterFiles(files)) { + for (F file : files) { if (file != null) { - processFile(session, directory, subDirectory, lsFiles, recursion, file); + processFile(session, directory, subDirectory, lsFiles, this.options.contains(Option.RECURSIVE), + file); } } } @@ -974,29 +992,51 @@ public abstract class AbstractRemoteFileOutboundGateway extends AbstractReply return (this.filter != null) ? this.filter.filterFiles(files) : Arrays.asList(files); } + protected final F filterFile(F file) { + if (this.filter.accept(file)) { + return file; + } + else { + return null; + } + } + private void processFile(Session session, String directory, String subDirectory, List lsFiles, boolean recursion, F file) throws IOException { - String fileName = getFilename(file); - String fileSep = this.remoteFileTemplate.getRemoteFileSeparator(); - boolean isDots = ".".equals(fileName) - || "..".equals(fileName) - || fileName.endsWith(fileSep + ".") - || fileName.endsWith(fileSep + ".."); - if (this.options.contains(Option.SUBDIRS) || !isDirectory(file)) { - if (recursion && StringUtils.hasText(subDirectory) && (!isDots || this.options.contains(Option.ALL))) { - lsFiles.add(enhanceNameWithSubDirectory(file, subDirectory)); + F fileToAdd = file; + if (recursion && StringUtils.hasText(subDirectory)) { + fileToAdd = enhanceNameWithSubDirectory(file, subDirectory); + } + if (this.filterAfterEnhancement) { + if (!this.filter.accept(fileToAdd)) { + return; } + } + String fileName = getFilename(fileToAdd); + final boolean isDirectory = isDirectory(file); + boolean isDots = hasDots(fileName); + if ((this.options.contains(Option.SUBDIRS) || !isDirectory) + && (!isDots || this.options.contains(Option.ALL))) { + else if (this.options.contains(Option.ALL) || !isDots) { lsFiles.add(file); } } - if (recursion && isDirectory(file) && !isDots) { - lsFiles.addAll(listFilesInRemoteDir(session, directory, - subDirectory + fileName + fileSep)); + + if (recursion && isDirectory && !isDots) { + lsFiles.addAll(listFilesInRemoteDir(session, directory, fileName + + this.remoteFileTemplate.getRemoteFileSeparator())); } } + private boolean hasDots(String fileName) { + String fileSeparator = this.remoteFileTemplate.getRemoteFileSeparator(); + return ".".equals(fileName) + || "..".equals(fileName) + || fileName.endsWith(fileSeparator + ".") + || fileName.endsWith(fileSeparator + ".."); + } protected final List filterMputFiles(File[] files) { if (files == null) { return Collections.emptyList(); diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/filters/CompositeFileListFilterTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/filters/CompositeFileListFilterTests.java index ccbef80db2..60cf5bb104 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/filters/CompositeFileListFilterTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/filters/CompositeFileListFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 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. @@ -30,7 +30,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; -import org.junit.Test; +import org.junit.jupiter.api.Test; /** * @author Iwein Fuld @@ -56,7 +56,7 @@ public class CompositeFileListFilterTests { List returnedFiles = Collections.singletonList(fileMock); when(fileFilterMock1.filterFiles(isA(File[].class))).thenReturn(returnedFiles); when(fileFilterMock2.filterFiles(isA(File[].class))).thenReturn(returnedFiles); - assertThat(compositeFileFilter.filterFiles(new File[] { fileMock })).isEqualTo(returnedFiles); + assertThat(compositeFileFilter.filterFiles(new File[]{ fileMock })).isEqualTo(returnedFiles); verify(fileFilterMock1).filterFiles(isA(File[].class)); verify(fileFilterMock2).filterFiles(isA(File[].class)); compositeFileFilter.close(); @@ -70,7 +70,7 @@ public class CompositeFileListFilterTests { List returnedFiles = Collections.singletonList(fileMock); when(fileFilterMock1.filterFiles(isA(File[].class))).thenReturn(returnedFiles); when(fileFilterMock2.filterFiles(isA(File[].class))).thenReturn(returnedFiles); - assertThat(compositeFileFilter.filterFiles(new File[] { fileMock })).isEqualTo(returnedFiles); + assertThat(compositeFileFilter.filterFiles(new File[]{ fileMock })).isEqualTo(returnedFiles); verify(fileFilterMock1).filterFiles(isA(File[].class)); verify(fileFilterMock2).filterFiles(isA(File[].class)); compositeFileFilter.close(); @@ -84,7 +84,7 @@ public class CompositeFileListFilterTests { when(fileFilterMock2.filterFiles(isA(File[].class))).thenReturn(new ArrayList<>()); when(fileFilterMock1.filterFiles(isA(File[].class))).thenReturn(new ArrayList<>()); - assertThat(compositeFileFilter.filterFiles(new File[] { fileMock }).isEmpty()).isTrue(); + assertThat(compositeFileFilter.filterFiles(new File[]{ fileMock }).isEmpty()).isTrue(); compositeFileFilter.close(); } @@ -95,7 +95,7 @@ public class CompositeFileListFilterTests { compositeFileFilter.addFilter(this.fileFilterMock2); List noFiles = new ArrayList<>(); when(this.fileFilterMock1.filterFiles(isA(File[].class))).thenReturn(noFiles); - assertThat(compositeFileFilter.filterFiles(new File[] { this.fileMock })).isEqualTo(noFiles); + assertThat(compositeFileFilter.filterFiles(new File[]{ this.fileMock })).isEqualTo(noFiles); verify(fileFilterMock1).filterFiles(isA(File[].class)); verify(fileFilterMock2, never()).filterFiles(isA(File[].class)); @@ -108,17 +108,17 @@ public class CompositeFileListFilterTests { CompositeFileListFilter compo = new CompositeFileListFilter<>(Collections.singletonList(new FileListFilter() { - @Override - public List filterFiles(String[] files) { - return Collections.emptyList(); - } + @Override + public List filterFiles(String[] files) { + return Collections.emptyList(); + } - @Override - public boolean supportsSingleFileFiltering() { - return true; - } + @Override + public boolean supportsSingleFileFiltering() { + return true; + } - })); + })); assertThatExceptionOfType(UnsupportedOperationException.class).isThrownBy(() -> compo.accept("foo")); compo.close(); } @@ -128,25 +128,32 @@ public class CompositeFileListFilterTests { CompositeFileListFilter compo = new CompositeFileListFilter<>(Collections.singletonList(new FileListFilter() { - @Override - public List filterFiles(String[] files) { - return Collections.emptyList(); - } + @Override + public List filterFiles(String[] files) { + return Collections.emptyList(); + } - @Override - public boolean supportsSingleFileFiltering() { - return true; - } + @Override + public boolean supportsSingleFileFiltering() { + return true; + } - @Override - public boolean accept(String file) { - return true; - } - })); + @Override + public boolean isForRecursion() { + return true; + } + + @Override + public boolean accept(String file) { + return true; + } + + })); assertThat(compo.accept("foo")).isTrue(); compo.addFilter(s -> null); assertThat(compo.supportsSingleFileFiltering()).isFalse(); + assertThat(compo.isForRecursion()).isTrue(); compo.close(); } @@ -155,6 +162,7 @@ public class CompositeFileListFilterTests { CompositeFileListFilter compo = new CompositeFileListFilter<>(Collections.singletonList(s -> null)); assertThat(compo.supportsSingleFileFiltering()).isFalse(); + assertThat(compo.isForRecursion()).isFalse(); compo.close(); } diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/filters/PersistentAcceptOnceFileListFilterTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/filters/PersistentAcceptOnceFileListFilterTests.java index 7a93412aeb..1030c32471 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/filters/PersistentAcceptOnceFileListFilterTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/filters/PersistentAcceptOnceFileListFilterTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -24,7 +24,6 @@ import java.io.Flushable; import java.io.IOException; import java.util.Arrays; import java.util.List; -import java.util.concurrent.Callable; import java.util.concurrent.CountDownLatch; import java.util.concurrent.Executors; import java.util.concurrent.Future; @@ -32,13 +31,15 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.integration.metadata.ConcurrentMetadataStore; import org.springframework.integration.metadata.SimpleMetadataStore; /** * @author Gary Russell + * @author Artem Bilan + * * @since 3.0 * */ @@ -69,26 +70,21 @@ public class PersistentAcceptOnceFileListFilterTests extends AcceptOnceFileListF final FileSystemPersistentAcceptOnceFileListFilter filter = new FileSystemPersistentAcceptOnceFileListFilter(store, "foo:"); final File file = File.createTempFile("foo", ".txt"); - assertThat(filter.filterFiles(new File[] { file }).size()).isEqualTo(1); + assertThat(filter.filterFiles(new File[]{ file }).size()).isEqualTo(1); String ts = store.get("foo:" + file.getAbsolutePath()); assertThat(ts).isEqualTo(String.valueOf(file.lastModified())); - assertThat(filter.filterFiles(new File[] { file }).size()).isEqualTo(0); + assertThat(filter.filterFiles(new File[]{ file }).size()).isEqualTo(0); file.setLastModified(file.lastModified() + 5000L); - assertThat(filter.filterFiles(new File[] { file }).size()).isEqualTo(1); + assertThat(filter.filterFiles(new File[]{ file }).size()).isEqualTo(1); ts = store.get("foo:" + file.getAbsolutePath()); assertThat(ts).isEqualTo(String.valueOf(file.lastModified())); - assertThat(filter.filterFiles(new File[] { file }).size()).isEqualTo(0); + assertThat(filter.filterFiles(new File[]{ file }).size()).isEqualTo(0); suspend.set(true); file.setLastModified(file.lastModified() + 5000L); - Future result = Executors.newSingleThreadExecutor().submit(new Callable() { - - @Override - public Integer call() throws Exception { - return filter.filterFiles(new File[] {file}).size(); - } - }); + Future result = Executors.newSingleThreadExecutor() + .submit(() -> filter.filterFiles(new File[]{ file }).size()); assertThat(latch2.await(10, TimeUnit.SECONDS)).isTrue(); store.put("foo:" + file.getAbsolutePath(), "43"); latch1.countDown(); @@ -102,8 +98,8 @@ public class PersistentAcceptOnceFileListFilterTests extends AcceptOnceFileListF @Override @Test public void testRollback() { - AbstractPersistentAcceptOnceFileListFilter filter = new AbstractPersistentAcceptOnceFileListFilter( - new SimpleMetadataStore(), "rollback:") { + AbstractPersistentAcceptOnceFileListFilter filter = + new AbstractPersistentAcceptOnceFileListFilter<>(new SimpleMetadataStore(), "rollback:") { @Override protected long modified(String file) { @@ -114,6 +110,12 @@ public class PersistentAcceptOnceFileListFilterTests extends AcceptOnceFileListF protected String fileName(String file) { return file; } + + @Override + protected boolean isDirectory(String file) { + return false; + } + }; doTestRollback(filter); } @@ -122,7 +124,7 @@ public class PersistentAcceptOnceFileListFilterTests extends AcceptOnceFileListF public void testRollbackFileSystem() throws Exception { FileSystemPersistentAcceptOnceFileListFilter filter = new FileSystemPersistentAcceptOnceFileListFilter( new SimpleMetadataStore(), "rollback:"); - File[] files = new File[] {new File("foo"), new File("bar"), new File("baz")}; + File[] files = new File[]{ new File("foo"), new File("bar"), new File("baz") }; List passed = filter.filterFiles(files); assertThat(passed.size()).isEqualTo(0); for (File file : files) { @@ -155,7 +157,7 @@ public class PersistentAcceptOnceFileListFilterTests extends AcceptOnceFileListF class MS extends SimpleMetadataStore implements Flushable, Closeable { @Override - public void flush() throws IOException { + public void flush() { flushes.incrementAndGet(); } @@ -176,7 +178,7 @@ public class PersistentAcceptOnceFileListFilterTests extends AcceptOnceFileListF FileSystemPersistentAcceptOnceFileListFilter filter = new FileSystemPersistentAcceptOnceFileListFilter( store, prefix); final File file = File.createTempFile("foo", ".txt"); - File[] files = new File[] { file }; + File[] files = new File[]{ file }; List passed = filter.filterFiles(files); assertThat(Arrays.equals(files, passed.toArray())).isTrue(); filter.rollback(passed.get(0), passed); diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java index 758967ddd1..ec9e6e1cbd 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/RemoteFileTestSupport.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2020 the original author or authors. + * Copyright 2016-2021 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. @@ -32,6 +32,7 @@ import org.junit.jupiter.api.io.TempDir; * Abstract base class for tests requiring remote file servers, e.g. (S)FTP. * * @author Gary Russell + * * @since 4.3 * */ @@ -48,6 +49,8 @@ public abstract class RemoteFileTestSupport { protected static File localTemporaryFolder; + protected static File scratchTemporaryFolder; + protected volatile File sourceRemoteDirectory; protected volatile File targetRemoteDirectory; @@ -169,6 +172,14 @@ public abstract class RemoteFileTestSupport { fos.close(); } + public static File getScratchTempFolder() { + if (scratchTemporaryFolder == null) { + scratchTemporaryFolder = new File(temporaryFolder.toFile().getAbsolutePath() + File.separator + "scratch"); + scratchTemporaryFolder.mkdirs(); + } + return scratchTemporaryFolder; + } + protected static File getRemoteTempFolder() { if (remoteTemporaryFolder == null) { remoteTemporaryFolder = new File(temporaryFolder.toFile().getAbsolutePath() + File.separator + "source"); diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/StreamingInboundTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/StreamingInboundTests.java index 5e6c244e42..ea03e417cc 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/StreamingInboundTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/StreamingInboundTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2016-2019 the original author or authors. + * Copyright 2016-2021 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. @@ -38,7 +38,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import java.util.stream.Stream; -import org.junit.Test; +import org.junit.jupiter.api.Test; import org.springframework.beans.factory.BeanFactory; import org.springframework.integration.IntegrationMessageHeaderAccessor; @@ -390,6 +390,11 @@ public class StreamingInboundTests { return file; } + @Override + protected boolean isDirectory(String file) { + return false; + } + } } diff --git a/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/filters/FtpPersistentAcceptOnceFileListFilter.java b/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/filters/FtpPersistentAcceptOnceFileListFilter.java index ebdb5af6dc..d62ce63e41 100644 --- a/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/filters/FtpPersistentAcceptOnceFileListFilter.java +++ b/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/filters/FtpPersistentAcceptOnceFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -27,6 +27,7 @@ import org.springframework.integration.metadata.ConcurrentMetadataStore; * 'seen' this file. * * @author Gary Russell + * * @since 3.0 * */ @@ -46,4 +47,9 @@ public class FtpPersistentAcceptOnceFileListFilter extends AbstractPersistentAcc return file.getName(); } + @Override + protected boolean isDirectory(FTPFile file) { + return file.isDirectory(); + } + } diff --git a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/filters/SftpPersistentAcceptOnceFileListFilter.java b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/filters/SftpPersistentAcceptOnceFileListFilter.java index 364175a749..1b250bc2a8 100644 --- a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/filters/SftpPersistentAcceptOnceFileListFilter.java +++ b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/filters/SftpPersistentAcceptOnceFileListFilter.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2019 the original author or authors. + * Copyright 2013-2021 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. @@ -27,6 +27,7 @@ import com.jcraft.jsch.ChannelSftp.LsEntry; * * @author Gary Russell * @author David Liu + * * @since 3.0 * */ @@ -46,4 +47,9 @@ public class SftpPersistentAcceptOnceFileListFilter extends AbstractPersistentAc return file.getFilename(); } + @Override + protected boolean isDirectory(LsEntry file) { + return file.getAttrs().isDir(); + } + } 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 4fed9784e6..4702261fce 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 @@ -87,7 +87,7 @@ expression="payload" command-options="-R" mode="REPLACE_IF_MODIFIED" - filter="dotStarDotTxtFilter" + filter="persistentFilter" local-directory-expression="@extraConfig.targetLocalDirectoryName + #remoteDirectory" local-filename-generator-expression="#remoteFileName.replaceFirst('sftpSource', 'localTarget')" reply-channel="output"/> @@ -98,6 +98,19 @@ + + + + + + + + + + + + (dir)); + result = this.output.receive(1000); + assertThat(result).isNotNull(); + files = (List) result.getPayload(); + assertThat(files).hasSize(1); + assertThat(files.get(0).getFilename()).isEqualTo("subSftpSource/subSftpSource2.txt"); + assertThat(this.store.get("testsubSftpSource/subSftpSource2.txt")).isNotNull(); } private long setModifiedOnSource1() { diff --git a/src/reference/asciidoc/file.adoc b/src/reference/asciidoc/file.adoc index 75b34a4642..e2355922f3 100644 --- a/src/reference/asciidoc/file.adoc +++ b/src/reference/asciidoc/file.adoc @@ -78,6 +78,13 @@ When used with a shared data store (such as `Redis` with the `RedisMetadataStore Since version 4.1.5, this filter has a new property (`flushOnUpdate`), which causes it to flush the metadata store on every update (if the store implements `Flushable`). ==== +The persistent file list filters now have a boolean property `forRecursion`. +Setting this property to `true`, also sets `alwaysAcceptDirectories`, which means that the recursive operation on the outbound gateways (`ls` and `mget`) will now always traverse the full directory tree each time. +This is to solve a problem where changes deep in the directory tree were not detected. +In addition, `forRecursion=true` causes the full path to files to be used as the metadata store keys; this solves a problem where the filter did not work properly if a file with the same name appears multiple times in different directories. +IMPORTANT: This means that existing keys in a persistent metadata store will not be found for files beneath the top level directory. +For this reason, the property is `false` by default; this may change in a future release. + The following example configures a `FileReadingMessageSource` with a filter: ==== @@ -1189,3 +1196,10 @@ If a filter returns `true` in `supportsSingleFileFiltering`, it **must** impleme If a remote filter does not support single file filtering (such as the `AbstractMarkerFilePresentFileListFilter`), the adapters revert to the previous behavior. If multiple filters are in used (using a `CompositeFileListFilter` or `ChainFileListFilter`), then **all** of the delegate filters must support single file filtering in order for the composite filter to support it. + +The persistent file list filters now have a boolean property `forRecursion`. +Setting this property to `true`, also sets `alwaysAcceptDirectories`, which means that the recursive operation on the outbound gateways (`ls` and `mget`) will now always traverse the full directory tree each time. +This is to solve a problem where changes deep in the directory tree were not detected. +In addition, `forRecursion=true` causes the full path to files to be used as the metadata store keys; this solves a problem where the filter did not work properly if a file with the same name appears multiple times in different directories. +IMPORTANT: This means that existing keys in a persistent metadata store will not be found for files beneath the top level directory. +For this reason, the property is `false` by default; this may change in a future release. diff --git a/src/reference/asciidoc/ftp.adoc b/src/reference/asciidoc/ftp.adoc index 89b8230eae..9623e715cd 100644 --- a/src/reference/asciidoc/ftp.adoc +++ b/src/reference/asciidoc/ftp.adoc @@ -1165,6 +1165,13 @@ The `-dirs` option is not allowed (the recursive `mget` uses the recursive `ls` Typically, you would use the `#remoteDirectory` variable in the `local-directory-expression` so that the remote directory structure is retained locally. ===== +The persistent file list filters now have a boolean property `forRecursion`. +Setting this property to `true`, also sets `alwaysAcceptDirectories`, which means that the recursive operation on the outbound gateways (`ls` and `mget`) will now always traverse the full directory tree each time. +This is to solve a problem where changes deep in the directory tree were not detected. +In addition, `forRecursion=true` causes the full path to files to be used as the metadata store keys; this solves a problem where the filter did not work properly if a file with the same name appears multiple times in different directories. +IMPORTANT: This means that existing keys in a persistent metadata store will not be found for files beneath the top level directory. +For this reason, the property is `false` by default; this may change in a future release. + Starting with version 5.0, the `FtpSimplePatternFileListFilter` and `FtpRegexPatternFileListFilter` can be configured to always pass directories by setting the `alwaysAcceptDirectories` property to `true`. Doing so allows recursion for a simple pattern, as the following examples show: diff --git a/src/reference/asciidoc/sftp.adoc b/src/reference/asciidoc/sftp.adoc index 737d149efc..c29a44737f 100644 --- a/src/reference/asciidoc/sftp.adoc +++ b/src/reference/asciidoc/sftp.adoc @@ -1125,6 +1125,13 @@ The `-dirs` option is not allowed (the recursive `mget` uses the recursive `ls` Typically, you would use the `#remoteDirectory` variable in the `local-directory-expression` so that the remote directory structure is retained locally. ===== +The persistent file list filters now have a boolean property `forRecursion`. +Setting this property to `true`, also sets `alwaysAcceptDirectories`, which means that the recursive operation on the outbound gateways (`ls` and `mget`) will now always traverse the full directory tree each time. +This is to solve a problem where changes deep in the directory tree were not detected. +In addition, `forRecursion=true` causes the full path to files to be used as the metadata store keys; this solves a problem where the filter did not work properly if a file with the same name appears multiple times in different directories. +IMPORTANT: This means that existing keys in a persistent metadata store will not be found for files beneath the top level directory. +For this reason, the property is `false` by default; this may change in a future release. + Starting with version 5.0, you can configure the `SftpSimplePatternFileListFilter` and `SftpRegexPatternFileListFilter` to always pass directories by setting the `alwaysAcceptDirectorties` to `true`. Doing so allows recursion for a simple pattern, as the following examples show: