diff --git a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java index 7ed4b431..ca507291 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java @@ -26,7 +26,6 @@ public class OpenCryptoFile implements Closeable { private final FileCloseListener listener; private final AtomicReference lastModified; - private final ChunkCache chunkCache; private final Cryptor cryptor; private final FileHeaderHolder headerHolder; private final ChunkIO chunkIO; @@ -37,11 +36,10 @@ public class OpenCryptoFile implements Closeable { private final AtomicInteger openChannelsCount = new AtomicInteger(0); @Inject - public OpenCryptoFile(FileCloseListener listener, ChunkCache chunkCache, Cryptor cryptor, FileHeaderHolder headerHolder, ChunkIO chunkIO, // + public OpenCryptoFile(FileCloseListener listener, Cryptor cryptor, FileHeaderHolder headerHolder, ChunkIO chunkIO, // @CurrentOpenFilePath AtomicReference currentFilePath, @OpenFileSize AtomicLong fileSize, // @OpenFileModifiedDate AtomicReference lastModified, OpenCryptoFileComponent component) { this.listener = listener; - this.chunkCache = chunkCache; this.cryptor = cryptor; this.headerHolder = headerHolder; this.chunkIO = chunkIO; @@ -70,15 +68,13 @@ public synchronized FileChannel newFileChannel(EffectiveOpenOptions options, Fil try { ciphertextFileChannel = path.getFileSystem().provider().newFileChannel(path, options.createOpenOptionsForEncryptedFile(), attrs); initFileHeader(options, ciphertextFileChannel); - if (options.truncateExisting()) { - chunkCache.invalidateStale(); - ciphertextFileChannel.truncate(cryptor.fileHeaderCryptor().headerSize()); - fileSize.set(0); - } initFileSize(ciphertextFileChannel); cleartextFileChannel = component.newChannelComponent() // .create(ciphertextFileChannel, options, this::cleartextChannelClosed) // .channel(); + if (options.truncateExisting()) { + cleartextFileChannel.truncate(0); + } } finally { if (cleartextFileChannel == null) { // i.e. something didn't work cleartextChannelClosed(ciphertextFileChannel); diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java index 2bcbaf42..949ae7fc 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java @@ -50,6 +50,8 @@ import java.util.List; import java.util.Random; import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; @@ -617,11 +619,13 @@ public void testWriteThenDeleteThenRead() throws IOException { } @RepeatedTest(50) - public void testConcurrentWriteAndTruncate() throws IOException { + public void testConcurrentWriteAndTruncate() throws IOException, InterruptedException { AtomicBoolean keepWriting = new AtomicBoolean(true); - ByteBuffer buf = ByteBuffer.wrap("the quick brown fox jumps over the lazy dog".getBytes(StandardCharsets.UTF_8)); - var executor = Executors.newCachedThreadPool(); - try (FileChannel writingChannel = FileChannel.open(file, WRITE, CREATE)) { + ByteBuffer buf = ByteBuffer.allocate(50_000); // 50 kiB + + try (ExecutorService executor = Executors.newCachedThreadPool(); + FileChannel writingChannel = FileChannel.open(file, WRITE, CREATE)) { + var cdl = new CountDownLatch(3); executor.submit(() -> { while (keepWriting.get()) { try { @@ -630,20 +634,22 @@ public void testConcurrentWriteAndTruncate() throws IOException { throw new UncheckedIOException(e); } buf.flip(); + cdl.countDown(); } }); + cdl.await(); try (FileChannel truncatingChannel = FileChannel.open(file, WRITE, TRUNCATE_EXISTING)) { keepWriting.set(false); } - executor.shutdown(); } - Assertions.assertDoesNotThrow(() -> { - try (FileChannel readingChannel = FileChannel.open(file, READ)) { - var dst = ByteBuffer.allocate(buf.capacity()); + + try (FileChannel readingChannel = FileChannel.open(file, READ)) { + var dst = ByteBuffer.allocate(buf.capacity()); + Assertions.assertDoesNotThrow(() -> { readingChannel.read(dst); - } - }); + }); + } } } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java b/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java index 7f90893f..e50b7d57 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java @@ -26,7 +26,6 @@ import java.io.UncheckedIOException; import java.nio.channels.FileChannel; import java.nio.file.FileSystem; -import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.nio.file.attribute.PosixFilePermissions; @@ -48,7 +47,6 @@ public class OpenCryptoFileTest { private static AtomicReference CURRENT_FILE_PATH; private ReadonlyFlag readonlyFlag = mock(ReadonlyFlag.class); private FileCloseListener closeListener = mock(FileCloseListener.class); - private ChunkCache chunkCache = mock(ChunkCache.class); private Cryptor cryptor = mock(Cryptor.class); private FileHeaderCryptor fileHeaderCryptor = mock(FileHeaderCryptor.class); private FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); @@ -72,7 +70,7 @@ public static void tearDown() throws IOException { @Test public void testCloseTriggersCloseListener() { - OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); openCryptoFile.close(); verify(closeListener).close(CURRENT_FILE_PATH.get(), openCryptoFile); } @@ -83,7 +81,7 @@ public void testCloseImmediatelyIfOpeningFirstChannelFails() { UncheckedIOException expectedException = new UncheckedIOException(new IOException("fail!")); EffectiveOpenOptions options = Mockito.mock(EffectiveOpenOptions.class); Mockito.when(options.createOpenOptionsForEncryptedFile()).thenThrow(expectedException); - OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); UncheckedIOException exception = Assertions.assertThrows(UncheckedIOException.class, () -> { openCryptoFile.newFileChannel(options); @@ -93,19 +91,20 @@ public void testCloseImmediatelyIfOpeningFirstChannelFails() { } @Test - @DisplayName("Opening a file channel with TRUNCATE_EXISTING sets the file size to 0") - public void testFileSizeZerodOnTruncateExisting() throws IOException { + @DisplayName("Opening a file channel with TRUNCATE_EXISTING calls truncate(0) on the cleartextChannel") + public void testCleartextChannelTruncateCalledOnTruncateExisting() throws IOException { EffectiveOpenOptions options = EffectiveOpenOptions.from(EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING), readonlyFlag); + var cleartextChannel = mock(CleartextFileChannel.class); Mockito.when(headerHolder.get()).thenReturn(Mockito.mock(FileHeader.class)); Mockito.when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); Mockito.when(fileHeaderCryptor.headerSize()).thenReturn(42); Mockito.when(openCryptoFileComponent.newChannelComponent()).thenReturn(channelComponentFactory); Mockito.when(channelComponentFactory.create(any(), any(), any())).thenReturn(channelComponent); - Mockito.when(channelComponent.channel()).thenReturn(mock(CleartextFileChannel.class)); - OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + Mockito.when(channelComponent.channel()).thenReturn(cleartextChannel); + OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); openCryptoFile.newFileChannel(options); - verify(fileSize).set(0L); + verify(cleartextChannel).truncate(0L); } @Nested @@ -114,7 +113,7 @@ public class InitFilHeaderTests { EffectiveOpenOptions options = Mockito.mock(EffectiveOpenOptions.class); FileChannel cipherFileChannel = Mockito.mock(FileChannel.class, "cipherFilechannel"); - OpenCryptoFile inTest = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); + OpenCryptoFile inTest = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent); @Test @DisplayName("Skip file header init, if the file header already exists in memory") @@ -198,7 +197,7 @@ public class FileChannelFactoryTest { public void setup() throws IOException { FS = Jimfs.newFileSystem("OpenCryptoFileTest.FileChannelFactoryTest", Configuration.unix().toBuilder().setAttributeViews("basic", "posix").build()); CURRENT_FILE_PATH = new AtomicReference<>(FS.getPath("currentFile")); - openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, realFileSize, lastModified, openCryptoFileComponent); + openCryptoFile = new OpenCryptoFile(closeListener,cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, realFileSize, lastModified, openCryptoFileComponent); cleartextFileChannel = mock(CleartextFileChannel.class); listener = new AtomicReference<>(); ciphertextChannel = new AtomicReference<>(); @@ -260,19 +259,6 @@ public void testGetSizeAfterCreatingSecondFileChannel() { Assertions.assertEquals(0l, openCryptoFile.size().get()); } - - @Test - @Order(20) - @DisplayName("TRUNCATE_EXISTING leads to chunk cache invalidation") - public void testTruncateExistingInvalidatesChunkCache() throws IOException { - Mockito.when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); - Mockito.when(fileHeaderCryptor.headerSize()).thenReturn(43); - Files.write(CURRENT_FILE_PATH.get(), new byte[0]); - EffectiveOpenOptions options = EffectiveOpenOptions.from(EnumSet.of(StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE), readonlyFlag); - openCryptoFile.newFileChannel(options); - verify(chunkCache).invalidateStale(); - } - @Test @Order(100) @DisplayName("closeListener triggers chunkIO.unregisterChannel()")