You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2023/04/19 11:37:44 UTC
[commons-io] branch master updated: IO-790 symbolic link file filter test incomplete, needs to handle windows issues (#450)
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-io.git
The following commit(s) were added to refs/heads/master by this push:
new c9ed8dac IO-790 symbolic link file filter test incomplete, needs to handle windows issues (#450)
c9ed8dac is described below
commit c9ed8dac48e4f9fd15b07fe551b6851baade8042
Author: Miguel Muñoz <Sw...@yahoo.com>
AuthorDate: Wed Apr 19 04:37:38 2023 -0700
IO-790 symbolic link file filter test incomplete, needs to handle windows issues (#450)
* IO-789_SymbolicLinkFileFilter_accepts_all_files,_not_just_symbolic_links fixed
* IO-789_SymbolicLinkFileFilter_accepts_all_files,_not_just_symbolic_links fixed.
Removed not the developers for now.
* IO-790 SymbolicLinkFileFilterTest Incomplete, needs to handle Windows issues.
* Roll back previous change.
* IO-790 SymbolicLinkFileFilterTest Incomplete, needs to handle Windows issues
* IO-790 SymbolicLinkFileFilterTest Incomplete, needs to handle Windows issues
Fixed Checkstyle problems
---
.../io/filefilter/SymbolicLinkFileFilter.java | 42 ++++-
.../io/filefilter/SymbolicLinkFileFilterTest.java | 181 ++++++++++++++++++++-
2 files changed, 213 insertions(+), 10 deletions(-)
diff --git a/src/main/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilter.java b/src/main/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilter.java
index 6dee2699..b5a384f8 100644
--- a/src/main/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilter.java
+++ b/src/main/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilter.java
@@ -61,6 +61,20 @@ import java.nio.file.attribute.BasicFileAttributes;
* @see FileFilterUtils#fileFileFilter()
*/
public class SymbolicLinkFileFilter extends AbstractFileFilter implements Serializable {
+ /*
+ * Note to developers: The unit test needs to create symbolic links to files. However, on
+ * Windows, this can't be done without admin privileges. This class is designed to allow a
+ * unit test to works around this by doing two things: 1) This separates the class logic from
+ * the call to identify a symbolic link, and 2) It allows the unit test to override that
+ * symbolic link call on Windows only.
+ * This means we can write unit tests will run on all machines. On Windows, the unit test
+ * can't create a symbolic link without admin privileges, so the unit tests won't
+ * completely test all the necessary behavior on Windows, but they will still test the class
+ * logic. Be careful not to break this, but be aware of it when writing unit tests. You can
+ * still maintain this class and its unit tests on Windows. The one method that won't get
+ * tested on Windows is not likely to change, and will be tested properly when it gets run
+ * on Apache servers.
+ */
/**
* Singleton instance of file filter.
@@ -87,25 +101,37 @@ public class SymbolicLinkFileFilter extends AbstractFileFilter implements Serial
}
/**
- * Checks to see if the file is a file.
+ * Checks to see if the file is a symbolic link.
*
* @param file the File to check
- * @return true if the file is a file
+ * @return true if the file exists and is a symbolic link to either another file or a directory,
+ * false otherwise.
*/
@Override
public boolean accept(final File file) {
- return file.isFile();
+ return isSymbolicLink(file.toPath());
}
/**
- * Checks to see if the file is a symbolic link.
- * @param file the File to check
+ * Checks to see if the file is a file.
*
- * @return true if the file is a symbolic link.
+ * @param path the File Path to check
+ * @return true if the file exists and is a symbolic link to either another file or a directory.
*/
@Override
- public FileVisitResult accept(final Path file, final BasicFileAttributes attributes) {
- return toFileVisitResult(Files.isSymbolicLink(file));
+ public FileVisitResult accept(final Path path, final BasicFileAttributes attributes) {
+ return toFileVisitResult(isSymbolicLink(path));
}
+ /**
+ * Package access, so the unit test may override to mock it. To
+ * facilitate unit testing, all calls to test if the file is a symbolic should go
+ * through this method. (See the unit test for why.)
+ *
+ * @param filePath The filePath to test
+ * @return true if the file exists and is a symbolic link to either a file or directory, false otherwise.
+ */
+ boolean isSymbolicLink(final Path filePath) {
+ return Files.isSymbolicLink(filePath);
+ }
}
diff --git a/src/test/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilterTest.java b/src/test/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilterTest.java
index 786a9527..cb66b8a1 100644
--- a/src/test/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilterTest.java
+++ b/src/test/java/org/apache/commons/io/filefilter/SymbolicLinkFileFilterTest.java
@@ -17,21 +17,198 @@
package org.apache.commons.io.filefilter;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-
+import java.io.File;
+import java.io.IOException;
import java.nio.file.FileVisitResult;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.function.BiFunction;
import org.apache.commons.io.file.PathUtils;
+import org.junit.jupiter.api.AfterAll;
+import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
/**
* Tests {@link SymbolicLinkFileFilter}.
*/
public class SymbolicLinkFileFilterTest {
+ public static final String TARGET_SHORT_NAME = "SLFF_Target";
+ public static final String TARGET_EXT = ".txt";
+ public static final String TARGET_NAME = TARGET_SHORT_NAME + TARGET_EXT;
+ public static final String DIRECTORY_NAME = "SLFF_TargetDirectory";
+ public static final String DIRECTORY_LINK_NAME = "SLFF_LinkDirectory";
+ public static final String MISSING = "Missing";
+ private static File testTargetFile; // hard file
+ private static Path testTargetPath; // hard file Path
+ private static File parentDirectoryFile; // System Temp directory
+ private static File testLinkFile; // symbolic link to hard file
+ private static String linkName; // Name of link file
+ private static Path testLinkPath; // symbolic link to hard file Path
+ private static File targetDirFile; //
+ private static Path targetDirPath; // hard directory Path
+ private static Path testLinkDirPath; // symbolic link to hardDirectory
+ private static File testLinkDirFile;
+ private static File missingFile; // non-existent file
+ private static SymbolicLinkFileFilter filter;
+
+ private static Path createRealSymbolicLink(Path link, Path target) {
+ try {
+ if (Files.exists(link)) {
+ Files.delete(link);
+ }
+ return Files.createSymbolicLink(link, target);
+ } catch (IOException e) {
+ throw new IllegalStateException("Failure to create Symbolic Link", e);
+ }
+ }
+
+ private static Path createMockSymbolicLink(Path lnk, Path tgt) {
+ try {
+ return Files.createFile(lnk);
+ } catch (IOException e) {
+ throw new IllegalStateException("Failure to create Symbolic Link", e);
+ }
+ }
+
+ // Mock filter for testing on Windows.
+ private static SymbolicLinkFileFilter createMockFilter() {
+ return new SymbolicLinkFileFilter() {
+ @Override
+ boolean isSymbolicLink(final Path filePath) {
+ return filePath.toFile().exists() && filePath.toString().contains("Link"); // Mock test
+ }
+ };
+ }
+
+ /**
+ * <p>Unit test setup creates a hard file, a symbolic link to the hard file, a hard directory,
+ * and a symbolic link to that directory. All are created in the temp directory</p>
+ * <p>Unit test teardown deletes all four of these files.</p>
+ *
+ * @throws IOException If it fails to create the temporary files
+ */
+ @BeforeAll
+ static void testSetup() throws IOException {
+ final BiFunction<Path, Path, Path> symbolicLinkCreator;
+
+ // We can't create symbolic links on Windows without admin privileges,
+ // so iff that's our OS, we mock them.
+ final String os = System.getProperty("os.name");
+ if (os.toLowerCase().contains("windows")) {
+ symbolicLinkCreator = SymbolicLinkFileFilterTest::createMockSymbolicLink;
+ filter = createMockFilter();
+ } else {
+ symbolicLinkCreator = SymbolicLinkFileFilterTest::createRealSymbolicLink;
+ filter = SymbolicLinkFileFilter.INSTANCE;
+ }
+
+ testTargetFile = File.createTempFile(TARGET_SHORT_NAME, TARGET_EXT);
+ testTargetPath = testTargetFile.toPath();
+ parentDirectoryFile = testTargetFile.getParentFile();
+ // parent directory
+ final Path parentDirectoryPath = parentDirectoryFile.toPath();
+ linkName = "SLFF_LinkTo" + testTargetFile.getName();
+ testLinkPath = symbolicLinkCreator.apply(parentDirectoryPath.resolve(linkName), testTargetPath);
+ testLinkFile = testLinkPath.toFile();
+ targetDirPath = Files.createDirectories(parentDirectoryPath.resolve(DIRECTORY_NAME));
+ targetDirFile = targetDirPath.toFile();
+ testLinkDirPath = symbolicLinkCreator.apply(parentDirectoryPath.resolve(DIRECTORY_LINK_NAME), targetDirPath);
+ testLinkDirFile = testLinkDirPath.toFile();
+ missingFile = new File(parentDirectoryPath.toFile(), MISSING);
+ }
+
+ @AfterAll
+ static void tearDown() {
+ // Fortunately, delete() doesn't throw an exception if the file doesn't exist.
+ testLinkDirFile.delete();
+ targetDirFile.delete();
+ testLinkFile.delete();
+ testTargetFile.delete();
+ }
+
@Test
public void testSymbolicLinkFileFilter() {
assertEquals(FileVisitResult.TERMINATE, SymbolicLinkFileFilter.INSTANCE.accept(PathUtils.current(), null));
+ }
+
+ @Test
+ public void testFileFilter_HardFile() {
+ assertFalse(filter.accept(testTargetFile));
+ }
+
+ @Test
+ public void testFileFilter_Link() {
+ assertTrue(filter.accept(testLinkFile));
+ }
+ @Test
+ public void testFileFilter_HardDirectory() {
+ assertFalse(filter.accept(targetDirFile));
+ }
+
+ @Test
+ public void testFileFilter_PathLink() {
+ assertTrue(filter.accept(testLinkDirFile));
+ }
+
+ @Test
+ public void testFileFilter_missingFile() {
+ assertFalse(filter.accept(missingFile));
+ }
+
+ @Test
+ public void testFileNameFilter_HardFile() {
+ assertFalse(filter.accept(parentDirectoryFile, TARGET_NAME));
+ }
+
+ @Test
+ public void testFileNameFilter_Link() {
+ assertTrue(filter.accept(parentDirectoryFile, linkName));
+ }
+
+ @Test
+ public void testFileNameFilter_HardDirectory() {
+ assertFalse(filter.accept(parentDirectoryFile, DIRECTORY_NAME));
+ }
+
+ @Test
+ public void testFileNameFilter_PathLink() {
+ assertTrue(filter.accept(parentDirectoryFile, DIRECTORY_LINK_NAME));
+ }
+
+ @Test
+ public void testFileNameFilter_missingFile() {
+ assertFalse(filter.accept(parentDirectoryFile, MISSING));
+ }
+
+ @Test
+ public void testPathFilter_HardFile() {
+ assertEquals(FileVisitResult.TERMINATE, filter.accept(testTargetPath, null));
+ }
+
+ @Test
+ public void testPathFilter_Link() {
+ assertEquals(FileVisitResult.CONTINUE, filter.accept(testLinkPath, null));
+ }
+
+ @Test
+ public void testPathFilter_HardDirectory() {
+ assertEquals(FileVisitResult.TERMINATE, filter.accept(targetDirPath, null));
+ }
+
+ @Test
+ public void testPathFilter_PathLink() {
+ assertEquals(FileVisitResult.CONTINUE, filter.accept(testLinkDirPath, null));
+ }
+
+ @Test
+ public void testPathFilter_missingFile() {
+ assertEquals(FileVisitResult.TERMINATE, filter.accept(missingFile.toPath(), null));
}
}