From cafe5a7bcc63e1cdaeadcd1dd6f5e0171f272965 Mon Sep 17 00:00:00 2001 From: Jacek Gajek Date: Mon, 20 Jan 2025 17:11:37 +0100 Subject: [PATCH] Fix #161 and #160. Handle open file restrictions during move and delete operations Introduce a helper method to throw consistent "resource busy" exceptions for open files. Add logic to prevent moving open files and refine handling of delete attempts on open files. Include corresponding test cases to verify the behavior. --- .../memoryfilesystem/MemoryFileSystem.java | 16 ++++++++++-- .../MemoryFileSystemCopyTest.java | 26 +++++++++++++++++++ .../MemoryFileSystemTest.java | 7 +++-- 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/github/marschall/memoryfilesystem/MemoryFileSystem.java b/src/main/java/com/github/marschall/memoryfilesystem/MemoryFileSystem.java index dd0bf85..b466557 100644 --- a/src/main/java/com/github/marschall/memoryfilesystem/MemoryFileSystem.java +++ b/src/main/java/com/github/marschall/memoryfilesystem/MemoryFileSystem.java @@ -125,6 +125,8 @@ class MemoryFileSystem extends FileSystem implements FileSystemContext { private final boolean supportFileChannelOnDirectory; + private static final String DEVICE_OR_RESOURCE_BUSY_MESSAGE = "Device or resource busy."; + static { Set unsupported = new HashSet<>(3); unsupported.add("lastAccessTime"); @@ -1174,7 +1176,7 @@ void delete(AbstractPath abstractPath) throws IOException { if (child instanceof MemoryFile) { MemoryFile file = (MemoryFile) child; if (file.openCount() > 0) { - throw new FileSystemException(abstractPath.toString(), null, "file still open"); + throwResourceBusyException(abstractPath); } file.markForDeletion(); } @@ -1208,7 +1210,7 @@ boolean deleteIfExists(AbstractPath abstractPath) throws IOException { if (child instanceof MemoryFile) { MemoryFile file = (MemoryFile) child; if (file.openCount() > 0) { - throw new FileSystemException(abstractPath.toString(), null, "file still open"); + throwResourceBusyException(abstractPath); } file.markForDeletion(); } @@ -1472,6 +1474,12 @@ static void handleTwoPathOperation(CopyContext copyContext, MemoryDirectory firs } if (copyContext.operation.isMove()) { + if (sourceEntry instanceof MemoryFile) { + MemoryFile sourceFile = (MemoryFile) sourceEntry; + if (sourceFile.openCount() > 0) { + throwResourceBusyException(copyContext.source.path); + } + } sourceParent.removeEntry(sourceElementName); targetParent.addEntry(targetElementName, sourceEntry, copyContext.target.path); String newOriginalName = targetContext.path.getMemoryFileSystem().storeTransformer.transform(targetContext.elementName); @@ -1505,6 +1513,10 @@ private static MemoryEntry getCopySource(CopyContext copyContext, MemoryEntry so return toCopy; } + private static void throwResourceBusyException(AbstractPath abstractPath) throws FileSystemException { + throw new FileSystemException(abstractPath.toString(), null, DEVICE_OR_RESOURCE_BUSY_MESSAGE); + } + @Override public String toString() { return MemoryFileSystem.class.getSimpleName() + '[' + this.key + ']'; diff --git a/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemCopyTest.java b/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemCopyTest.java index d75c159..9872cdd 100644 --- a/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemCopyTest.java +++ b/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemCopyTest.java @@ -11,12 +11,14 @@ import static java.nio.file.StandardCopyOption.REPLACE_EXISTING; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.IOException; +import java.nio.channels.FileChannel; import java.nio.file.DirectoryNotEmptyException; import java.nio.file.DirectoryStream; import java.nio.file.FileAlreadyExistsException; @@ -25,6 +27,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.BasicFileAttributes; import java.nio.file.attribute.FileTime; @@ -363,6 +366,7 @@ void moveRootIntoSubfolder() throws IOException { assertThrows(FileSystemException.class, () -> Files.move(dir, sub)); } + @Test void moveRoot() { Path root = this.extension.getFileSystem().getPath("/"); Path path = this.extension.getFileSystem().getPath("/a"); @@ -461,6 +465,28 @@ void renameFile() throws IOException { assertEquals(Collections.singletonList(ac), aKids); } + @Test + void moveFileInUse() throws IOException { + FileSystem fileSystem = this.extension.getFileSystem(); + + // move /a/c to /b/c + + Path a = fileSystem.getPath("/a"); + Files.createDirectory(a); + Path ab = fileSystem.getPath("/a/b"); + Files.createFile(ab); + Path ac = fileSystem.getPath("/a/c"); + + assertThat(a, exists()); + assertThat(ab, exists()); + assertThat(ac, not(exists())); + + try (FileChannel ignored = FileChannel.open(ab, StandardOpenOption.WRITE)) { + FileSystemException e = assertThrows(FileSystemException.class, () -> Files.move(ab, ac)); + assertThat(e.getMessage(), is("/a/b: Device or resource busy.")); + } + } + @Test void moveSameFile() throws IOException { FileSystem fileSystem = this.extension.getFileSystem(); diff --git a/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemTest.java b/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemTest.java index e9f3ffa..9ce98fc 100644 --- a/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemTest.java +++ b/src/test/java/com/github/marschall/memoryfilesystem/MemoryFileSystemTest.java @@ -20,6 +20,7 @@ import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.isA; import static org.hamcrest.Matchers.lessThan; import static org.hamcrest.Matchers.not; @@ -888,8 +889,10 @@ void dontDeleteOpenFile() throws IOException { FileSystem fileSystem = this.extension.getFileSystem(); Path path = fileSystem.getPath("test"); try (SeekableByteChannel channel = Files.newByteChannel(path, CREATE_NEW, WRITE)) { - assertThrows(FileSystemException.class , () -> Files.delete(path), "you shound't be able to delete a file wile it's open"); - assertThrows(FileSystemException.class , () -> Files.deleteIfExists(path), "you shound't be able to delete a file wile it's open"); + FileSystemException deleteException = assertThrows(FileSystemException.class , () -> Files.delete(path), "you shouldn't be able to delete a file while it's open"); + assertThat(deleteException.getMessage(), is(path + ": Device or resource busy.")); + FileSystemException deleteIfExistsException = assertThrows(FileSystemException.class , () -> Files.deleteIfExists(path), "you shouldn't be able to delete a file while it's open"); + assertThat(deleteIfExistsException.getMessage(), is(path + ": Device or resource busy.")); } }