Skip to content

Commit 212d44b

Browse files
infeoinfeo
authored
Merge pull request #270 from cryptomator/feature/269-truncate-while-write
Use truncate() of CleartextChannel when opening new Channel with `TRUNCATE_EXISTING`
2 parents 2dedb3b + 5363d4b commit 212d44b

File tree

3 files changed

+30
-42
lines changed

3 files changed

+30
-42
lines changed

‎src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFile.java‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ public class OpenCryptoFile implements Closeable {
2626

2727
private final FileCloseListener listener;
2828
private final AtomicReference<Instant> lastModified;
29-
private final ChunkCache chunkCache;
3029
private final Cryptor cryptor;
3130
private final FileHeaderHolder headerHolder;
3231
private final ChunkIO chunkIO;
@@ -37,11 +36,10 @@ public class OpenCryptoFile implements Closeable {
3736
private final AtomicInteger openChannelsCount = new AtomicInteger(0);
3837

3938
@Inject
40-
public OpenCryptoFile(FileCloseListener listener, ChunkCache chunkCache, Cryptor cryptor, FileHeaderHolder headerHolder, ChunkIO chunkIO, //
39+
public OpenCryptoFile(FileCloseListener listener, Cryptor cryptor, FileHeaderHolder headerHolder, ChunkIO chunkIO, //
4140
@CurrentOpenFilePath AtomicReference<Path> currentFilePath, @OpenFileSize AtomicLong fileSize, //
4241
@OpenFileModifiedDate AtomicReference<Instant> lastModified, OpenCryptoFileComponent component) {
4342
this.listener = listener;
44-
this.chunkCache = chunkCache;
4543
this.cryptor = cryptor;
4644
this.headerHolder = headerHolder;
4745
this.chunkIO = chunkIO;
@@ -70,15 +68,13 @@ public synchronized FileChannel newFileChannel(EffectiveOpenOptions options, Fil
7068
try {
7169
ciphertextFileChannel = path.getFileSystem().provider().newFileChannel(path, options.createOpenOptionsForEncryptedFile(), attrs);
7270
initFileHeader(options, ciphertextFileChannel);
73-
if (options.truncateExisting()) {
74-
chunkCache.invalidateStale();
75-
ciphertextFileChannel.truncate(cryptor.fileHeaderCryptor().headerSize());
76-
fileSize.set(0);
77-
}
7871
initFileSize(ciphertextFileChannel);
7972
cleartextFileChannel = component.newChannelComponent() //
8073
.create(ciphertextFileChannel, options, this::cleartextChannelClosed) //
8174
.channel();
75+
if (options.truncateExisting()) {
76+
cleartextFileChannel.truncate(0);
77+
}
8278
} finally {
8379
if (cleartextFileChannel == null) { // i.e. something didn't work
8480
cleartextChannelClosed(ciphertextFileChannel);

‎src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java‎

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,8 @@
5050
import java.util.List;
5151
import java.util.Random;
5252
import java.util.Set;
53+
import java.util.concurrent.CountDownLatch;
54+
import java.util.concurrent.ExecutorService;
5355
import java.util.concurrent.Executors;
5456
import java.util.concurrent.TimeUnit;
5557
import java.util.concurrent.atomic.AtomicBoolean;
@@ -617,11 +619,13 @@ public void testWriteThenDeleteThenRead() throws IOException {
617619
}
618620

619621
@RepeatedTest(50)
620-
public void testConcurrentWriteAndTruncate() throws IOException {
622+
public void testConcurrentWriteAndTruncate() throws IOException, InterruptedException {
621623
AtomicBoolean keepWriting = new AtomicBoolean(true);
622-
ByteBuffer buf = ByteBuffer.wrap("the quick brown fox jumps over the lazy dog".getBytes(StandardCharsets.UTF_8));
623-
var executor = Executors.newCachedThreadPool();
624-
try (FileChannel writingChannel = FileChannel.open(file, WRITE, CREATE)) {
624+
ByteBuffer buf = ByteBuffer.allocate(50_000); // 50 kiB
625+
626+
try (ExecutorService executor = Executors.newCachedThreadPool();
627+
FileChannel writingChannel = FileChannel.open(file, WRITE, CREATE)) {
628+
var cdl = new CountDownLatch(3);
625629
executor.submit(() -> {
626630
while (keepWriting.get()) {
627631
try {
@@ -630,20 +634,22 @@ public void testConcurrentWriteAndTruncate() throws IOException {
630634
throw new UncheckedIOException(e);
631635
}
632636
buf.flip();
637+
cdl.countDown();
633638
}
634639
});
640+
cdl.await();
635641
try (FileChannel truncatingChannel = FileChannel.open(file, WRITE, TRUNCATE_EXISTING)) {
636642
keepWriting.set(false);
637643
}
638-
executor.shutdown();
639644
}
640645

641-
Assertions.assertDoesNotThrow(() -> {
642-
try (FileChannel readingChannel = FileChannel.open(file, READ)) {
643-
var dst = ByteBuffer.allocate(buf.capacity());
646+
647+
try (FileChannel readingChannel = FileChannel.open(file, READ)) {
648+
var dst = ByteBuffer.allocate(buf.capacity());
649+
Assertions.assertDoesNotThrow(() -> {
644650
readingChannel.read(dst);
645-
}
646-
});
651+
});
652+
}
647653
}
648654

649655
}

‎src/test/java/org/cryptomator/cryptofs/fh/OpenCryptoFileTest.java‎

Lines changed: 10 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.io.UncheckedIOException;
2727
import java.nio.channels.FileChannel;
2828
import java.nio.file.FileSystem;
29-
import java.nio.file.Files;
3029
import java.nio.file.Path;
3130
import java.nio.file.StandardOpenOption;
3231
import java.nio.file.attribute.PosixFilePermissions;
@@ -48,7 +47,6 @@ public class OpenCryptoFileTest {
4847
private static AtomicReference<Path> CURRENT_FILE_PATH;
4948
private ReadonlyFlag readonlyFlag = mock(ReadonlyFlag.class);
5049
private FileCloseListener closeListener = mock(FileCloseListener.class);
51-
private ChunkCache chunkCache = mock(ChunkCache.class);
5250
private Cryptor cryptor = mock(Cryptor.class);
5351
private FileHeaderCryptor fileHeaderCryptor = mock(FileHeaderCryptor.class);
5452
private FileHeaderHolder headerHolder = mock(FileHeaderHolder.class);
@@ -72,7 +70,7 @@ public static void tearDown() throws IOException {
7270

7371
@Test
7472
public void testCloseTriggersCloseListener() {
75-
OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
73+
OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
7674
openCryptoFile.close();
7775
verify(closeListener).close(CURRENT_FILE_PATH.get(), openCryptoFile);
7876
}
@@ -83,7 +81,7 @@ public void testCloseImmediatelyIfOpeningFirstChannelFails() {
8381
UncheckedIOException expectedException = new UncheckedIOException(new IOException("fail!"));
8482
EffectiveOpenOptions options = Mockito.mock(EffectiveOpenOptions.class);
8583
Mockito.when(options.createOpenOptionsForEncryptedFile()).thenThrow(expectedException);
86-
OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
84+
OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
8785

8886
UncheckedIOException exception = Assertions.assertThrows(UncheckedIOException.class, () -> {
8987
openCryptoFile.newFileChannel(options);
@@ -93,19 +91,20 @@ public void testCloseImmediatelyIfOpeningFirstChannelFails() {
9391
}
9492

9593
@Test
96-
@DisplayName("Opening a file channel with TRUNCATE_EXISTING sets the file size to 0")
97-
public void testFileSizeZerodOnTruncateExisting() throws IOException {
94+
@DisplayName("Opening a file channel with TRUNCATE_EXISTING calls truncate(0) on the cleartextChannel")
95+
public void testCleartextChannelTruncateCalledOnTruncateExisting() throws IOException {
9896
EffectiveOpenOptions options = EffectiveOpenOptions.from(EnumSet.of(StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE, StandardOpenOption.TRUNCATE_EXISTING), readonlyFlag);
97+
var cleartextChannel = mock(CleartextFileChannel.class);
9998
Mockito.when(headerHolder.get()).thenReturn(Mockito.mock(FileHeader.class));
10099
Mockito.when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor);
101100
Mockito.when(fileHeaderCryptor.headerSize()).thenReturn(42);
102101
Mockito.when(openCryptoFileComponent.newChannelComponent()).thenReturn(channelComponentFactory);
103102
Mockito.when(channelComponentFactory.create(any(), any(), any())).thenReturn(channelComponent);
104-
Mockito.when(channelComponent.channel()).thenReturn(mock(CleartextFileChannel.class));
105-
OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
103+
Mockito.when(channelComponent.channel()).thenReturn(cleartextChannel);
104+
OpenCryptoFile openCryptoFile = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
106105

107106
openCryptoFile.newFileChannel(options);
108-
verify(fileSize).set(0L);
107+
verify(cleartextChannel).truncate(0L);
109108
}
110109

111110
@Nested
@@ -114,7 +113,7 @@ public class InitFilHeaderTests {
114113

115114
EffectiveOpenOptions options = Mockito.mock(EffectiveOpenOptions.class);
116115
FileChannel cipherFileChannel = Mockito.mock(FileChannel.class, "cipherFilechannel");
117-
OpenCryptoFile inTest = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
116+
OpenCryptoFile inTest = new OpenCryptoFile(closeListener, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, fileSize, lastModified, openCryptoFileComponent);
118117

119118
@Test
120119
@DisplayName("Skip file header init, if the file header already exists in memory")
@@ -198,7 +197,7 @@ public class FileChannelFactoryTest {
198197
public void setup() throws IOException {
199198
FS = Jimfs.newFileSystem("OpenCryptoFileTest.FileChannelFactoryTest", Configuration.unix().toBuilder().setAttributeViews("basic", "posix").build());
200199
CURRENT_FILE_PATH = new AtomicReference<>(FS.getPath("currentFile"));
201-
openCryptoFile = new OpenCryptoFile(closeListener, chunkCache, cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, realFileSize, lastModified, openCryptoFileComponent);
200+
openCryptoFile = new OpenCryptoFile(closeListener,cryptor, headerHolder, chunkIO, CURRENT_FILE_PATH, realFileSize, lastModified, openCryptoFileComponent);
202201
cleartextFileChannel = mock(CleartextFileChannel.class);
203202
listener = new AtomicReference<>();
204203
ciphertextChannel = new AtomicReference<>();
@@ -260,19 +259,6 @@ public void testGetSizeAfterCreatingSecondFileChannel() {
260259
Assertions.assertEquals(0l, openCryptoFile.size().get());
261260
}
262261

263-
264-
@Test
265-
@Order(20)
266-
@DisplayName("TRUNCATE_EXISTING leads to chunk cache invalidation")
267-
public void testTruncateExistingInvalidatesChunkCache() throws IOException {
268-
Mockito.when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor);
269-
Mockito.when(fileHeaderCryptor.headerSize()).thenReturn(43);
270-
Files.write(CURRENT_FILE_PATH.get(), new byte[0]);
271-
EffectiveOpenOptions options = EffectiveOpenOptions.from(EnumSet.of(StandardOpenOption.TRUNCATE_EXISTING, StandardOpenOption.WRITE), readonlyFlag);
272-
openCryptoFile.newFileChannel(options);
273-
verify(chunkCache).invalidateStale();
274-
}
275-
276262
@Test
277263
@Order(100)
278264
@DisplayName("closeListener triggers chunkIO.unregisterChannel()")

0 commit comments

Comments
 (0)