diff --git a/pom.xml b/pom.xml index 9edc1126..35a0e9df 100644 --- a/pom.xml +++ b/pom.xml @@ -2,7 +2,7 @@ 4.0.0 org.cryptomator cryptofs - 2.6.2 + 2.6.3 Cryptomator Crypto Filesystem This library provides the Java filesystem provider used by Cryptomator. https://github.com/cryptomator/cryptofs @@ -22,6 +22,7 @@ 4.3.0 2.44.2 31.1-jre + 3.1.4 2.0.3 @@ -81,6 +82,21 @@ guava ${guava.version} + + com.github.ben-manes.caffeine + caffeine + ${caffeine.version} + + + org.checkerframework + checker-qual + + + com.google.errorprone + error_prone_annotations + + + org.slf4j slf4j-api diff --git a/src/main/java/module-info.java b/src/main/java/module-info.java index 97e5da36..5cbfb094 100644 --- a/src/main/java/module-info.java +++ b/src/main/java/module-info.java @@ -1,5 +1,4 @@ import org.cryptomator.cryptofs.CryptoFileSystemProvider; -import org.cryptomator.cryptofs.common.CiphertextFileType; import org.cryptomator.cryptofs.health.api.HealthCheck; import org.cryptomator.cryptofs.health.dirid.DirIdCheck; import org.cryptomator.cryptofs.health.shortened.ShortenedNamesCheck; @@ -10,6 +9,7 @@ module org.cryptomator.cryptofs { requires transitive org.cryptomator.cryptolib; requires com.google.common; + requires com.github.benmanes.caffeine; requires org.slf4j; requires dagger; requires com.auth0.jwt; diff --git a/src/main/java/org/cryptomator/cryptofs/CryptoPathMapper.java b/src/main/java/org/cryptomator/cryptofs/CryptoPathMapper.java index 136d697f..8e3b8120 100644 --- a/src/main/java/org/cryptomator/cryptofs/CryptoPathMapper.java +++ b/src/main/java/org/cryptomator/cryptofs/CryptoPathMapper.java @@ -8,11 +8,9 @@ *******************************************************************************/ package org.cryptomator.cryptofs; -import com.google.common.base.Throwables; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; import com.google.common.io.BaseEncoding; import org.cryptomator.cryptofs.common.CiphertextFileType; import org.cryptomator.cryptofs.common.Constants; @@ -22,6 +20,7 @@ import javax.inject.Inject; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; @@ -32,7 +31,6 @@ import java.time.Duration; import java.util.Objects; import java.util.Optional; -import java.util.concurrent.ExecutionException; import static org.cryptomator.cryptofs.common.Constants.DATA_DIR_NAME; @@ -61,8 +59,8 @@ public class CryptoPathMapper { this.dirIdProvider = dirIdProvider; this.longFileNameProvider = longFileNameProvider; this.vaultConfig = vaultConfig; - this.ciphertextNames = CacheBuilder.newBuilder().maximumSize(MAX_CACHED_CIPHERTEXT_NAMES).build(CacheLoader.from(this::getCiphertextFileName)); - this.ciphertextDirectories = CacheBuilder.newBuilder().maximumSize(MAX_CACHED_DIR_PATHS).expireAfterWrite(MAX_CACHE_AGE).build(); + this.ciphertextNames = Caffeine.newBuilder().maximumSize(MAX_CACHED_CIPHERTEXT_NAMES).build(this::getCiphertextFileName); + this.ciphertextDirectories = Caffeine.newBuilder().maximumSize(MAX_CACHED_DIR_PATHS).expireAfterWrite(MAX_CACHE_AGE).build(); this.rootDirectory = resolveDirectory(Constants.ROOT_DIR_ID); } @@ -127,7 +125,7 @@ public CiphertextFilePath getCiphertextFilePath(CryptoPath cleartextPath) throws } public CiphertextFilePath getCiphertextFilePath(Path parentCiphertextDir, String parentDirId, String cleartextName) { - String ciphertextName = ciphertextNames.getUnchecked(new DirIdAndName(parentDirId, cleartextName)); + String ciphertextName = ciphertextNames.get(new DirIdAndName(parentDirId, cleartextName)); Path c9rPath = parentCiphertextDir.resolve(ciphertextName); if (ciphertextName.length() > vaultConfig.getShorteningThreshold()) { LongFileNameProvider.DeflatedFileName deflatedFileName = longFileNameProvider.deflate(c9rPath); @@ -160,13 +158,16 @@ public CiphertextDirectory getCiphertextDir(CryptoPath cleartextPath) throws IOE return rootDirectory; } else { try { - return ciphertextDirectories.get(cleartextPath, () -> { - Path dirFile = getCiphertextFilePath(cleartextPath).getDirFilePath(); - return resolveDirectory(dirFile); + return ciphertextDirectories.get(cleartextPath, p -> { + try { + Path dirFile = getCiphertextFilePath(p).getDirFilePath(); + return resolveDirectory(dirFile); + } catch (IOException e) { + throw new UncheckedIOException(e); + } }); - } catch (ExecutionException e) { - Throwables.throwIfInstanceOf(e.getCause(), IOException.class); - throw new IOException("Unexpected exception", e); + } catch (UncheckedIOException e) { + throw new IOException(e); } } } diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java index 627ee94b..c03e85a6 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdLoader.java @@ -1,10 +1,11 @@ package org.cryptomator.cryptofs; -import com.google.common.cache.CacheLoader; +import com.github.benmanes.caffeine.cache.CacheLoader; import javax.inject.Inject; import java.io.IOException; import java.io.InputStream; +import java.io.UncheckedIOException; import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.charset.StandardCharsets; @@ -14,7 +15,7 @@ import java.util.UUID; @CryptoFileSystemScoped -class DirectoryIdLoader extends CacheLoader { +class DirectoryIdLoader implements CacheLoader { private static final int MAX_DIR_ID_LENGTH = 1000; @@ -23,7 +24,7 @@ public DirectoryIdLoader() { } @Override - public String load(Path dirFilePath) throws IOException { + public String load(Path dirFilePath) throws UncheckedIOException { try (FileChannel ch = FileChannel.open(dirFilePath, StandardOpenOption.READ); InputStream in = Channels.newInputStream(ch)) { long size = ch.size(); @@ -39,6 +40,8 @@ public String load(Path dirFilePath) throws IOException { } } catch (NoSuchFileException e) { return UUID.randomUUID().toString(); + } catch (IOException e) { + throw new UncheckedIOException(e); } } diff --git a/src/main/java/org/cryptomator/cryptofs/DirectoryIdProvider.java b/src/main/java/org/cryptomator/cryptofs/DirectoryIdProvider.java index 94c79432..adfe0829 100644 --- a/src/main/java/org/cryptomator/cryptofs/DirectoryIdProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/DirectoryIdProvider.java @@ -8,14 +8,14 @@ *******************************************************************************/ package org.cryptomator.cryptofs; -import java.io.IOException; -import java.nio.file.Path; -import java.util.concurrent.ExecutionException; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.LoadingCache; import javax.inject.Inject; - -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.LoadingCache; +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Path; @CryptoFileSystemScoped class DirectoryIdProvider { @@ -26,13 +26,13 @@ class DirectoryIdProvider { @Inject public DirectoryIdProvider(DirectoryIdLoader directoryIdLoader) { - ids = CacheBuilder.newBuilder().maximumSize(MAX_CACHE_SIZE).build(directoryIdLoader); + this.ids = Caffeine.newBuilder().maximumSize(MAX_CACHE_SIZE).build(directoryIdLoader); } public String load(Path dirFilePath) throws IOException { try { return ids.get(dirFilePath); - } catch (ExecutionException e) { + } catch (UncheckedIOException e) { throw new IOException("Failed to load contents of directory file at path " + dirFilePath, e); } } @@ -62,4 +62,5 @@ public void move(Path srcDirFilePath, Path dstDirFilePath) { } } + } diff --git a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java index 94d32b68..8cfd6931 100644 --- a/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java +++ b/src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java @@ -8,10 +8,9 @@ *******************************************************************************/ package org.cryptomator.cryptofs; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.base.Throwables; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import com.google.common.io.BaseEncoding; import org.cryptomator.cryptolib.common.MessageDigestSupplier; @@ -24,7 +23,6 @@ import java.nio.file.Path; import java.nio.file.StandardOpenOption; import java.time.Duration; -import java.util.concurrent.ExecutionException; import static java.nio.charset.StandardCharsets.UTF_8; import static org.cryptomator.cryptofs.common.Constants.DEFLATED_FILE_SUFFIX; @@ -39,31 +37,28 @@ public class LongFileNameProvider { private static final Duration MAX_CACHE_AGE = Duration.ofMinutes(1); private final ReadonlyFlag readonlyFlag; - private final LoadingCache longNames; // Maps from c9s paths to inflated filenames + private final Cache longNames; // Maps from c9s paths to inflated filenames @Inject public LongFileNameProvider(ReadonlyFlag readonlyFlag) { this.readonlyFlag = readonlyFlag; - this.longNames = CacheBuilder.newBuilder().expireAfterAccess(MAX_CACHE_AGE).build(new Loader()); + this.longNames = Caffeine.newBuilder().expireAfterAccess(MAX_CACHE_AGE).build(); } - private class Loader extends CacheLoader { - - @Override - public String load(Path c9sPath) throws IOException { - Path longNameFile = c9sPath.resolve(INFLATED_FILE_NAME); - try (SeekableByteChannel ch = Files.newByteChannel(longNameFile, StandardOpenOption.READ)) { - if (ch.size() > MAX_FILENAME_BUFFER_SIZE) { - throw new IOException("Unexpectedly large file: " + longNameFile); - } - assert ch.size() <= MAX_FILENAME_BUFFER_SIZE; - ByteBuffer buf = ByteBuffer.allocate((int) ch.size()); - ch.read(buf); - buf.flip(); - return UTF_8.decode(buf).toString(); + private String load(Path c9sPath) throws UncheckedIOException { + Path longNameFile = c9sPath.resolve(INFLATED_FILE_NAME); + try (SeekableByteChannel ch = Files.newByteChannel(longNameFile, StandardOpenOption.READ)) { + if (ch.size() > MAX_FILENAME_BUFFER_SIZE) { + throw new IOException("Unexpectedly large file: " + longNameFile); } + assert ch.size() <= MAX_FILENAME_BUFFER_SIZE; + ByteBuffer buf = ByteBuffer.allocate((int) ch.size()); + ch.read(buf); + buf.flip(); + return UTF_8.decode(buf).toString(); + } catch (IOException e) { + throw new UncheckedIOException(e); } - } public boolean isDeflated(String possiblyDeflatedFileName) { @@ -72,10 +67,9 @@ public boolean isDeflated(String possiblyDeflatedFileName) { public String inflate(Path c9sPath) throws IOException { try { - return longNames.get(c9sPath); - } catch (ExecutionException e) { - Throwables.throwIfInstanceOf(e.getCause(), IOException.class); - throw new IllegalStateException("Unexpected exception", e); + return longNames.get(c9sPath, this::load); + } catch (UncheckedIOException e) { + throw e.getCause(); // rethrow original to keep exception types such as NoSuchFileException } } diff --git a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java index 436dde78..883247df 100644 --- a/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java +++ b/src/main/java/org/cryptomator/cryptofs/ch/CleartextFileChannel.java @@ -4,8 +4,8 @@ import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptofs.EffectiveOpenOptions; import org.cryptomator.cryptofs.fh.BufferPool; -import org.cryptomator.cryptofs.fh.ChunkCache; import org.cryptomator.cryptofs.fh.Chunk; +import org.cryptomator.cryptofs.fh.ChunkCache; import org.cryptomator.cryptofs.fh.ExceptionsDuringWrite; import org.cryptomator.cryptofs.fh.OpenFileModifiedDate; import org.cryptomator.cryptofs.fh.OpenFileSize; @@ -25,6 +25,7 @@ import java.nio.file.attribute.BasicFileAttributeView; import java.nio.file.attribute.FileTime; import java.time.Instant; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.concurrent.locks.ReadWriteLock; @@ -50,7 +51,7 @@ public class CleartextFileChannel extends AbstractFileChannel { private final ExceptionsDuringWrite exceptionsDuringWrite; private final ChannelCloseListener closeListener; private final CryptoFileSystemStats stats; - private boolean mustWriteHeader; + private final AtomicBoolean mustWriteHeader; @Inject public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeader fileHeader, @MustWriteHeader boolean mustWriteHeader, ReadWriteLock readWriteLock, Cryptor cryptor, ChunkCache chunkCache, BufferPool bufferPool, EffectiveOpenOptions options, @OpenFileSize AtomicLong fileSize, @OpenFileModifiedDate AtomicReference lastModified, Supplier attrViewProvider, ExceptionsDuringWrite exceptionsDuringWrite, ChannelCloseListener closeListener, CryptoFileSystemStats stats) { @@ -70,7 +71,7 @@ public CleartextFileChannel(FileChannel ciphertextFileChannel, FileHeader fileHe if (options.append()) { position = fileSize.get(); } - this.mustWriteHeader = mustWriteHeader; + this.mustWriteHeader = new AtomicBoolean(mustWriteHeader); if (options.createNew() || options.create()) { lastModified.compareAndSet(Instant.EPOCH, Instant.now()); } @@ -93,7 +94,7 @@ protected boolean isReadable() { } @Override - protected synchronized int readLocked(ByteBuffer dst, long position) throws IOException { + protected int readLocked(ByteBuffer dst, long position) throws IOException { int origLimit = dst.limit(); long limitConsideringEof = fileSize.get() - position; if (limitConsideringEof < 1) { @@ -106,10 +107,12 @@ protected synchronized int readLocked(ByteBuffer dst, long position) throws IOEx long pos = position + read; long chunkIndex = pos / payloadSize; // floor by int-truncation int offsetInChunk = (int) (pos % payloadSize); // known to fit in int, because payloadSize is int - ByteBuffer data = chunkCache.get(chunkIndex).data().duplicate().position(offsetInChunk); - int len = min(dst.remaining(), data.remaining()); // known to fit in int, because second argument is int - dst.put(data.limit(data.position() + len)); - read += len; + try (var chunk = chunkCache.getChunk(chunkIndex)) { + ByteBuffer data = chunk.data().duplicate().position(offsetInChunk); + int len = min(dst.remaining(), data.remaining()); // known to fit in int, because second argument is int + dst.put(data.limit(data.position() + len)); + read += len; + } } dst.limit(origLimit); stats.addBytesRead(read); @@ -153,18 +156,18 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti ByteBuffer cleartextChunkData = bufferPool.getCleartextBuffer(); src.copyTo(cleartextChunkData); cleartextChunkData.flip(); - Chunk chunk = new Chunk(cleartextChunkData, true); - chunkCache.set(chunkIndex, chunk); + chunkCache.putChunk(chunkIndex, cleartextChunkData).close(); } else { /* * TODO performance: * We don't actually need to read the current data into the cache. * It would suffice if store the written data and do reading when storing the chunk. */ - Chunk chunk = chunkCache.get(chunkIndex); - chunk.data().limit(Math.max(chunk.data().limit(), offsetInChunk + len)); // increase limit (if needed) - src.copyTo(chunk.data().duplicate().position(offsetInChunk)); // work on duplicate using correct offset - chunk.dirty().set(true); + try (Chunk chunk = chunkCache.getChunk(chunkIndex)) { + chunk.data().limit(Math.max(chunk.data().limit(), offsetInChunk + len)); // increase limit (if needed) + src.copyTo(chunk.data().duplicate().position(offsetInChunk)); // work on duplicate using correct offset + chunk.dirty().set(true); + } } written += len; } @@ -180,11 +183,10 @@ private long writeLockedInternal(ByteSource src, long position) throws IOExcepti } private void writeHeaderIfNeeded() throws IOException { - if (mustWriteHeader) { + if (mustWriteHeader.getAndSet(false)) { LOG.trace("{} - Writing file header.", this); ByteBuffer encryptedHeader = cryptor.fileHeaderCryptor().encryptHeader(fileHeader); ciphertextFileChannel.write(encryptedHeader, 0); - mustWriteHeader = false; // write the header only once! } } @@ -196,11 +198,13 @@ protected void truncateLocked(long newSize) throws IOException { long indexOfLastChunk = (newSize + cleartextChunkSize - 1) / cleartextChunkSize - 1; int sizeOfIncompleteChunk = (int) (newSize % cleartextChunkSize); // known to fit in int, because cleartextChunkSize is int if (sizeOfIncompleteChunk > 0) { - var chunk = chunkCache.get(indexOfLastChunk); - chunk.data().limit(sizeOfIncompleteChunk); - chunk.dirty().set(true); + try (var chunk = chunkCache.getChunk(indexOfLastChunk)) { + chunk.data().limit(sizeOfIncompleteChunk); + chunk.dirty().set(true); + } } long ciphertextFileSize = cryptor.fileHeaderCryptor().headerSize() + cryptor.fileContentCryptor().ciphertextSize(newSize); + chunkCache.flush(); chunkCache.invalidateAll(); // make sure no chunks _after_ newSize exist that would otherwise be written during the next cache eviction ciphertextFileChannel.truncate(ciphertextFileSize); position = min(newSize, position); @@ -230,7 +234,7 @@ private void forceInternal(boolean metaData) throws IOException { private void flush() throws IOException { if (isWritable()) { writeHeaderIfNeeded(); - chunkCache.invalidateAll(); // TODO performance: write chunks but keep them cached + chunkCache.flush(); exceptionsDuringWrite.throwIfPresent(); } } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java index b2af8c70..7cfbf443 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/Chunk.java @@ -10,8 +10,7 @@ import java.nio.ByteBuffer; import java.util.concurrent.atomic.AtomicBoolean; - -import static java.lang.String.format; +import java.util.concurrent.atomic.AtomicInteger; /** * A chunk of plaintext data. It has these rules: @@ -23,14 +22,18 @@ *
  • When no longer used, the cleartext ByteBuffer may be recycled
    • * */ -public record Chunk(ByteBuffer data, AtomicBoolean dirty) { +public record Chunk(ByteBuffer data, AtomicBoolean dirty, AtomicInteger currentAccesses, Runnable onClose) implements AutoCloseable { - public Chunk(ByteBuffer data, boolean dirty) { - this(data, new AtomicBoolean(dirty)); + public Chunk(ByteBuffer data, boolean dirty, Runnable onClose) { + this(data, new AtomicBoolean(dirty), new AtomicInteger(0), onClose); } public boolean isDirty() { return dirty.get(); } + @Override + public void close() { + onClose.run(); + } } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java index 215b58a6..199c4cb1 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkCache.java @@ -1,15 +1,21 @@ package org.cryptomator.cryptofs.fh; -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.RemovalNotification; +import com.github.benmanes.caffeine.cache.Caffeine; +import com.github.benmanes.caffeine.cache.RemovalCause; +import com.google.common.base.Preconditions; import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptolib.api.AuthenticationFailedException; import javax.inject.Inject; import java.io.IOException; import java.io.UncheckedIOException; -import java.util.concurrent.ExecutionException; +import java.nio.ByteBuffer; +import java.nio.channels.NonWritableChannelException; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.locks.ReadWriteLock; +import java.util.concurrent.locks.ReentrantReadWriteLock; +import java.util.function.BiFunction; @OpenFileScoped public class ChunkCache { @@ -20,54 +26,172 @@ public class ChunkCache { private final ChunkSaver chunkSaver; private final CryptoFileSystemStats stats; private final BufferPool bufferPool; - private final Cache chunks; + private final ExceptionsDuringWrite exceptionsDuringWrite; + private final ConcurrentMap staleChunks; + private final ConcurrentMap activeChunks; + + /** + * This lock ensures no chunks are passed between stale and active state while flushing, + * as flushing requires iteration over both sets. + */ + private final ReadWriteLock flushLock = new ReentrantReadWriteLock(); @Inject - public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats, BufferPool bufferPool) { + public ChunkCache(ChunkLoader chunkLoader, ChunkSaver chunkSaver, CryptoFileSystemStats stats, BufferPool bufferPool, ExceptionsDuringWrite exceptionsDuringWrite) { this.chunkLoader = chunkLoader; this.chunkSaver = chunkSaver; this.stats = stats; this.bufferPool = bufferPool; - this.chunks = CacheBuilder.newBuilder() // + this.exceptionsDuringWrite = exceptionsDuringWrite; + this.staleChunks = Caffeine.newBuilder() // .maximumSize(MAX_CACHED_CLEARTEXT_CHUNKS) // - .removalListener(this::removeChunk) // - .build(); + .evictionListener(this::evictStaleChunk) // + .build() // + .asMap(); + this.activeChunks = new ConcurrentHashMap<>(); } - private Chunk loadChunk(long chunkIndex) throws IOException { - stats.addChunkCacheMiss(); + /** + * Overwrites data at the given index and increments the access counter of the returned chunk + * + * @param chunkIndex Which chunk to overwrite + * @param chunkData The cleartext data + * @return The chunk + * @throws IllegalArgumentException If {@code chunkData}'s remaining bytes is not equal to the number of bytes fitting into a chunk + */ + public Chunk putChunk(long chunkIndex, ByteBuffer chunkData) throws IllegalArgumentException { + return activeChunks.compute(chunkIndex, (index, chunk) -> { + // stale chunk for this index is obsolete: + var staleChunk = staleChunks.remove(index); + if (staleChunk != null) { + bufferPool.recycle(staleChunk.data()); + } + // either create completely new chunk or replace all data of existing active chunk: + if (chunk == null) { + chunk = new Chunk(chunkData, true, () -> releaseChunk(chunkIndex)); + } else { + var dst = chunk.data().duplicate().clear(); + Preconditions.checkArgument(chunkData.remaining() == dst.remaining()); + dst.put(chunkData); + chunk.dirty().set(true); + } + chunk.currentAccesses().incrementAndGet(); + return chunk; + }); + } + + /** + * Returns the chunk for the given index, potentially loading the chunk into cache + * + * @param chunkIndex Which chunk to load + * @return The chunk + * @throws IOException If reading or decrypting the chunk failed + */ + public Chunk getChunk(long chunkIndex) throws IOException { + var lock = flushLock.readLock(); + lock.lock(); try { - return chunkLoader.load(chunkIndex); - } catch (AuthenticationFailedException e) { - // TODO provide means to pass an AuthenticationFailedException handler using an OpenOption - throw new IOException("Unauthentic ciphertext in chunk " + chunkIndex, e); + stats.addChunkCacheAccess(); + return activeChunks.compute(chunkIndex, this::acquireInternal); + } catch (UncheckedIOException | AuthenticationFailedException e) { + throw new IOException(e); + } finally { + lock.unlock(); } } - private void removeChunk(RemovalNotification removal) { + private Chunk acquireInternal(Long index, Chunk activeChunk) throws AuthenticationFailedException, UncheckedIOException { + Chunk result = activeChunk; + if (result == null) { + result = staleChunks.remove(index); + assert result == null || result.currentAccesses().get() == 0; + } + if (result == null) { + result = loadChunk(index); + } + + assert result != null; + result.currentAccesses().incrementAndGet(); + return result; + } + + private Chunk loadChunk(long chunkIndex) throws AuthenticationFailedException, UncheckedIOException { + stats.addChunkCacheMiss(); try { - chunkSaver.save(removal.getKey(), removal.getValue()); - bufferPool.recycle(removal.getValue().data()); + return new Chunk(chunkLoader.load(chunkIndex), false, () -> releaseChunk(chunkIndex)); } catch (IOException e) { throw new UncheckedIOException(e); } } - public Chunk get(long chunkIndex) throws IOException { + @SuppressWarnings("resource") + private void releaseChunk(long chunkIndex) { + var lock = flushLock.readLock(); + lock.lock(); try { - stats.addChunkCacheAccess(); - return chunks.get(chunkIndex, () -> loadChunk(chunkIndex)); - } catch (ExecutionException e) { - assert e.getCause() instanceof IOException; // the only checked exception thrown by #loadChunk(long) - throw (IOException) e.getCause(); + activeChunks.compute(chunkIndex, (index, chunk) -> { + assert chunk != null; + var accessCnt = chunk.currentAccesses().decrementAndGet(); + if (accessCnt == 0) { + staleChunks.put(index, chunk); + return null; //chunk is stale, remove from active + } else { + assert accessCnt > 0; + return chunk; //keep active + } + }); + } finally { + lock.unlock(); } } - public void set(long chunkIndex, Chunk data) { - chunks.put(chunkIndex, data); + /** + * Flushes cached data (but keeps them cached). + * @see #invalidateAll() + */ + public void flush() throws IOException { + var lock = flushLock.writeLock(); + lock.lock(); + BiFunction saveUnchecked = (index, chunk) -> { + try { + chunkSaver.save(index, chunk); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + return chunk; + }; + try { + activeChunks.replaceAll(saveUnchecked); + staleChunks.replaceAll(saveUnchecked); + } catch (UncheckedIOException e) { + throw new IOException(e); + } finally { + lock.unlock(); + } } + /** + * Removes stale chunks from cache. + */ public void invalidateAll() { - chunks.invalidateAll(); + var lock = flushLock.writeLock(); + lock.lock(); + try { + staleChunks.clear(); + } finally { + lock.unlock(); + } + } + + // visible for testing + void evictStaleChunk(Long index, Chunk chunk, RemovalCause removalCause) { + assert removalCause != RemovalCause.EXPLICIT; // as per spec of Caffeine#evictionListener(RemovalListener) + assert chunk.currentAccesses().get() == 0; + try { + chunkSaver.save(index, chunk); + } catch (IOException | NonWritableChannelException e) { + exceptionsDuringWrite.add(e); + } + bufferPool.recycle(chunk.data()); } } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java index 3524e95a..e2fafb04 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkLoader.java @@ -26,8 +26,7 @@ public ChunkLoader(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerH this.bufferPool = bufferPool; } - public Chunk load(Long chunkIndex) throws IOException, AuthenticationFailedException { - stats.addChunkCacheMiss(); + public ByteBuffer load(Long chunkIndex) throws IOException, AuthenticationFailedException { int chunkSize = cryptor.fileContentCryptor().ciphertextChunkSize(); long ciphertextPos = chunkIndex * chunkSize + cryptor.fileHeaderCryptor().headerSize(); ByteBuffer ciphertextBuf = bufferPool.getCiphertextBuffer(); @@ -42,7 +41,7 @@ public Chunk load(Long chunkIndex) throws IOException, AuthenticationFailedExcep cleartextBuf.flip(); stats.addBytesDecrypted(cleartextBuf.remaining()); } - return new Chunk(cleartextBuf, false); + return cleartextBuf; } finally { bufferPool.recycle(ciphertextBuf); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java index dbb798b3..02504f4b 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ChunkSaver.java @@ -13,16 +13,14 @@ class ChunkSaver { private final Cryptor cryptor; private final ChunkIO ciphertext; private final FileHeaderHolder headerHolder; - private final ExceptionsDuringWrite exceptionsDuringWrite; private final CryptoFileSystemStats stats; private final BufferPool bufferPool; @Inject - public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, ExceptionsDuringWrite exceptionsDuringWrite, CryptoFileSystemStats stats, BufferPool bufferPool) { + public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHolder, CryptoFileSystemStats stats, BufferPool bufferPool) { this.cryptor = cryptor; this.ciphertext = ciphertext; this.headerHolder = headerHolder; - this.exceptionsDuringWrite = exceptionsDuringWrite; this.stats = stats; this.bufferPool = bufferPool; } @@ -30,18 +28,17 @@ public ChunkSaver(Cryptor cryptor, ChunkIO ciphertext, FileHeaderHolder headerHo public void save(long chunkIndex, Chunk chunkData) throws IOException { if (chunkData.isDirty()) { long ciphertextPos = chunkIndex * cryptor.fileContentCryptor().ciphertextChunkSize() + cryptor.fileHeaderCryptor().headerSize(); - ByteBuffer cleartextBuf = chunkData.data().duplicate(); + ByteBuffer cleartextBuf = chunkData.data().asReadOnlyBuffer(); stats.addBytesEncrypted(cleartextBuf.remaining()); ByteBuffer ciphertextBuf = bufferPool.getCiphertextBuffer(); try { cryptor.fileContentCryptor().encryptChunk(cleartextBuf, ciphertextBuf, chunkIndex, headerHolder.get()); ciphertextBuf.flip(); ciphertext.write(ciphertextBuf, ciphertextPos); - } catch (IOException e) { - exceptionsDuringWrite.add(e); + chunkData.dirty().set(false); } finally { bufferPool.recycle(ciphertextBuf); - } // unchecked exceptions will be propagated to the thread causing removal + } } } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/ExceptionsDuringWrite.java b/src/main/java/org/cryptomator/cryptofs/fh/ExceptionsDuringWrite.java index 1586a6b7..b0cf0931 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/ExceptionsDuringWrite.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/ExceptionsDuringWrite.java @@ -18,7 +18,7 @@ public class ExceptionsDuringWrite { public ExceptionsDuringWrite() { } - public synchronized void add(IOException e) { + public synchronized void add(Exception e) { e.addSuppressed(e); } diff --git a/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java b/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java index 8849392d..34abc1e1 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/FileHeaderHolder.java @@ -39,11 +39,8 @@ public FileHeader get() { public FileHeader createNew() { LOG.trace("Generating file header for {}", path.get()); FileHeader newHeader = cryptor.fileHeaderCryptor().create(); - if (header.compareAndSet(null, newHeader)) { - return newHeader; - } else { - return header.get(); - } + header.set(newHeader); + return newHeader; } public FileHeader loadExisting(FileChannel ch) throws IOException { diff --git a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFiles.java b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFiles.java index 1fa916d7..4186e248 100644 --- a/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFiles.java +++ b/src/main/java/org/cryptomator/cryptofs/fh/OpenCryptoFiles.java @@ -51,7 +51,7 @@ public Optional get(Path ciphertextPath) { } /** - * Opens a file to {@link OpenCryptoFile#newFileChannel(EffectiveOpenOptions) retrieve a FileChannel}. If this file is already opened, a shared instance is returned. + * Opens a file to {@link OpenCryptoFile#newFileChannel(EffectiveOpenOptions, java.nio.file.attribute.FileAttribute[]) retrieve a FileChannel}. If this file is already opened, a shared instance is returned. * Getting the file channel should be the next invocation, since the {@link OpenFileScoped lifecycle} of the OpenFile strictly depends on the lifecycle of the channel. * * @param ciphertextPath Path of the file to open diff --git a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java index cfc773a1..93216d8b 100644 --- a/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java +++ b/src/test/java/org/cryptomator/cryptofs/CryptoFileChannelWriteReadIntegrationTest.java @@ -8,6 +8,7 @@ *******************************************************************************/ package org.cryptomator.cryptofs; +import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; import org.cryptomator.cryptofs.util.ByteBuffers; import org.cryptomator.cryptolib.api.Masterkey; @@ -17,7 +18,9 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.junit.jupiter.api.condition.EnabledOnOs; @@ -30,6 +33,7 @@ import org.mockito.Mockito; import java.io.IOException; +import java.lang.invoke.MethodHandles; import java.net.URI; import java.nio.ByteBuffer; import java.nio.channels.Channels; @@ -42,7 +46,12 @@ import java.nio.file.attribute.FileTime; import java.time.Instant; import java.time.temporal.ChronoUnit; +import java.util.Arrays; import java.util.List; +import java.util.Random; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; +import java.util.stream.IntStream; import java.util.stream.Stream; import static java.lang.Math.min; @@ -225,18 +234,39 @@ public void testReadWhileStillWriting() throws IOException { // tests https://github.com/cryptomator/cryptofs/issues/48 @Test - public void testTruncateExistingWhileStillOpen() throws IOException { - try (FileChannel ch1 = FileChannel.open(file, CREATE_NEW, WRITE)) { - ch1.write(StandardCharsets.UTF_8.encode("goodbye world"), 0); - ch1.force(true); // will generate a file header - try (FileChannel ch2 = FileChannel.open(file, CREATE, WRITE, TRUNCATE_EXISTING)) { // reuse existing file header, but will not re-write it - ch2.write(StandardCharsets.UTF_8.encode("hello world"), 0); + @DisplayName("writing from second channel while first is still open") + public void testWriteFromSecondChannelWhileStillOpen() throws IOException { + try (var ch1 = FileChannel.open(file, CREATE_NEW, WRITE)) { + ch1.write(StandardCharsets.UTF_8.encode("howdy world"), 0); + ch1.force(true); + try (var ch2 = FileChannel.open(file, CREATE, WRITE)) { // reuse existing file header, but will not re-write it + ch2.write(StandardCharsets.UTF_8.encode("hello"), 0); } } - try (FileChannel ch1 = FileChannel.open(file, READ)) { - ByteBuffer buf = ByteBuffer.allocate((int) ch1.size()); - ch1.read(buf); + try (var ch3 = FileChannel.open(file, READ)) { + ByteBuffer buf = ByteBuffer.allocate((int) ch3.size()); + ch3.read(buf); + Assertions.assertArrayEquals("hello world".getBytes(), buf.array()); + } + } + + // tests https://github.com/cryptomator/cryptofs/issues/160 + @Test + @DisplayName("TRUNCATE_EXISTING leads to new file header") + public void testNewFileHeaderWhenTruncateExisting() throws IOException { + try (var ch1 = FileChannel.open(file, CREATE_NEW, WRITE)) { + ch1.write(StandardCharsets.UTF_8.encode("this content will be truncated soon"), 0); + ch1.force(true); + try (var ch2 = FileChannel.open(file, CREATE, WRITE, TRUNCATE_EXISTING)) { // re-roll file header + ch2.write(StandardCharsets.UTF_8.encode("hello"), 0); + } + ch1.write(StandardCharsets.UTF_8.encode(" world"), 5); // should use new file key + } + + try (var ch3 = FileChannel.open(file, READ)) { + ByteBuffer buf = ByteBuffer.allocate((int) ch3.size()); + ch3.read(buf); Assertions.assertArrayEquals("hello world".getBytes(), buf.array()); } } @@ -463,6 +493,60 @@ public void testAppend(int dataSize) throws IOException { } } + @RepeatedTest(100) + public void testConcurrentRead() throws IOException, InterruptedException { + // prepare test data: + var content = new byte[10_000_000]; + for (int i = 0; i < 100_000; i++) { + byte b = (byte) (i % 255); + Arrays.fill(content, i * 100, (i + 1) * 100, b); + } + + try (var ch = FileChannel.open(file, CREATE, WRITE, TRUNCATE_EXISTING)) { + ch.write(ByteBuffer.wrap(content)); + } + + // read concurrently from same file channel: + var numThreads = 100; + var rnd = new Random(42L); + var results = new int[numThreads]; + var resultsHandle = MethodHandles.arrayElementVarHandle(int[].class); + Arrays.fill(results, 42); + var executor = Executors.newCachedThreadPool(); + try (var ch = FileChannel.open(file, READ)) { + for (int i = 0; i < numThreads; i++) { + int t = i; + int num = rnd.nextInt(50_000); + int pos = rnd.nextInt(400_000); + executor.submit(() -> { + ByteBuffer buf = ByteBuffer.allocate(num); + try { + int read = ch.read(buf, pos); + if (read != num) { + System.out.println("thread " + t + " read " + pos + " - " + (pos + num)); + resultsHandle.setOpaque(results, t, -1); // ERROR invalid number of bytes + } else if (Arrays.equals(content, pos, pos + num, buf.array(), 0, read)) { + resultsHandle.setOpaque(results, t, 0); // SUCCESS + } else { + System.out.println("thread " + t + " read " + pos + " - " + (pos + num)); + resultsHandle.setOpaque(results, t, -2); // ERROR invalid content + } + } catch (IOException e) { + e.printStackTrace(); + resultsHandle.setOpaque(results, t, -3); // ERROR I/O error + } + }); + } + executor.shutdown(); + boolean allTasksFinished = executor.awaitTermination(10, TimeUnit.SECONDS); + Assertions.assertTrue(allTasksFinished); + } + + Assertions.assertAll(IntStream.range(0, numThreads).mapToObj(t -> { + return () -> Assertions.assertEquals(0, resultsHandle.getOpaque(results, t), "thread " + t + " unsuccessful"); + })); + } + } } diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java index 3943207a..6141325f 100644 --- a/src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DirectoryIdLoaderTest.java @@ -7,6 +7,7 @@ import org.mockito.Mockito; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.file.FileSystem; @@ -83,10 +84,10 @@ public void testIOExceptionWhenExistingFileIsEmpty() throws IOException { when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel); when(channel.size()).thenReturn(0l); - IOException e = Assertions.assertThrows(IOException.class, () -> { + UncheckedIOException e = Assertions.assertThrows(UncheckedIOException.class, () -> { inTest.load(dirFilePath); }); - MatcherAssert.assertThat(e.getMessage(), containsString("Invalid, empty directory file")); + MatcherAssert.assertThat(e.getCause().getMessage(), containsString("Invalid, empty directory file")); } @Test @@ -95,10 +96,10 @@ public void testIOExceptionWhenExistingFileIsTooLarge() throws IOException { when(provider.newFileChannel(eq(dirFilePath), any())).thenReturn(channel); when(channel.size()).thenReturn((long) Integer.MAX_VALUE); - IOException e = Assertions.assertThrows(IOException.class, () -> { + UncheckedIOException e = Assertions.assertThrows(UncheckedIOException.class, () -> { inTest.load(dirFilePath); }); - MatcherAssert.assertThat(e.getMessage(), containsString("Unexpectedly large directory file")); + MatcherAssert.assertThat(e.getCause().getMessage(), containsString("Unexpectedly large directory file")); } } diff --git a/src/test/java/org/cryptomator/cryptofs/DirectoryIdProviderTest.java b/src/test/java/org/cryptomator/cryptofs/DirectoryIdProviderTest.java index 8525403c..33c6d04d 100644 --- a/src/test/java/org/cryptomator/cryptofs/DirectoryIdProviderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/DirectoryIdProviderTest.java @@ -9,6 +9,7 @@ import org.mockito.verification.VerificationMode; import java.io.IOException; +import java.io.UncheckedIOException; import java.nio.file.Path; import java.util.concurrent.ExecutionException; @@ -38,14 +39,14 @@ public void testLoadInvokesLoader() throws IOException { } @Test - public void testIOExceptionFromLoaderIsWrappedAndRethrown() throws IOException { + public void testIOExceptionFromLoaderIsWrappedAndRethrown() { IOException originalIoException = new IOException(); - when(loader.load(aPath)).thenThrow(originalIoException); + when(loader.load(aPath)).thenThrow(new UncheckedIOException(originalIoException)); IOException e = Assertions.assertThrows(IOException.class, () -> { inTest.load(aPath); }); - Assertions.assertTrue(e.getCause() instanceof ExecutionException); + Assertions.assertTrue(e.getCause() instanceof UncheckedIOException); Assertions.assertEquals(originalIoException, e.getCause().getCause()); } diff --git a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java index a2dfb966..fe9ac96f 100644 --- a/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java +++ b/src/test/java/org/cryptomator/cryptofs/ch/CleartextFileChannelTest.java @@ -76,7 +76,8 @@ public class CleartextFileChannelTest { public void setUp() throws IOException { when(cryptor.fileHeaderCryptor()).thenReturn(fileHeaderCryptor); when(cryptor.fileContentCryptor()).thenReturn(fileContentCryptor); - when(chunkCache.get(Mockito.anyLong())).then(invocation -> new Chunk(ByteBuffer.allocate(100), false)); + when(chunkCache.getChunk(Mockito.anyLong())).then(invocation -> new Chunk(ByteBuffer.allocate(100), false, () -> {})); + when(chunkCache.putChunk(Mockito.anyLong(), Mockito.any())).thenAnswer(invocation -> new Chunk(invocation.getArgument(1), true, () -> {})); when(bufferPool.getCleartextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(100)); when(fileHeaderCryptor.headerSize()).thenReturn(50); when(fileContentCryptor.cleartextChunkSize()).thenReturn(100); @@ -176,7 +177,7 @@ public void testForceInvalidatesChunkCacheWhenWritable() throws IOException { inTest.force(false); - verify(chunkCache).invalidateAll(); + verify(chunkCache).flush(); } @Test @@ -189,7 +190,7 @@ public void testForceRethrowsExceptionsDuringWrite() throws IOException { }); Assertions.assertEquals("exception during write", e.getMessage()); - verify(chunkCache).invalidateAll(); + verify(chunkCache).flush(); } @Test @@ -354,10 +355,10 @@ public void testReadFromMultipleChunks() throws IOException { inTest.read(buf, 5_000_000_000l); InOrder inOrder = Mockito.inOrder(chunkCache, chunkCache, chunkCache, chunkCache); - inOrder.verify(chunkCache).get(0l); // A - inOrder.verify(chunkCache).get(1l); // B - inOrder.verify(chunkCache).get(2l); // B - inOrder.verify(chunkCache).get(50_000_000l); // C + inOrder.verify(chunkCache).getChunk(0l); // A + inOrder.verify(chunkCache).getChunk(1l); // B + inOrder.verify(chunkCache).getChunk(2l); // B + inOrder.verify(chunkCache).getChunk(50_000_000l); // C inOrder.verifyNoMoreInteractions(); } @@ -407,9 +408,9 @@ public void testWriteToMultipleChunks() throws IOException { inTest.write(buf3, 5000); InOrder inOrder = Mockito.inOrder(chunkCache, chunkCache, chunkCache); - inOrder.verify(chunkCache).get(0l); // A - inOrder.verify(chunkCache).set(Mockito.eq(1l), Mockito.any()); // B - inOrder.verify(chunkCache).set(Mockito.eq(50l), Mockito.any()); // C + inOrder.verify(chunkCache).getChunk(0l); // A + inOrder.verify(chunkCache).putChunk(Mockito.eq(1l), Mockito.any()); // B + inOrder.verify(chunkCache).putChunk(Mockito.eq(50l), Mockito.any()); // C inOrder.verifyNoMoreInteractions(); } @@ -437,9 +438,9 @@ public void testWriteIncrementsPositionByAmountWritten() throws IOException { Assertions.assertEquals(110, written); Assertions.assertEquals(205, inTest.size()); - verify(chunkCache).get(0l); - verify(chunkCache).set(Mockito.eq(1l), Mockito.any()); - verify(chunkCache).get(2l); + verify(chunkCache).getChunk(0l); + verify(chunkCache).putChunk(Mockito.eq(1l), Mockito.any()); + verify(chunkCache).getChunk(2l); } @Test diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java index f62dd41b..d4cf0615 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkCacheTest.java @@ -1,13 +1,22 @@ package org.cryptomator.cryptofs.fh; +import com.github.benmanes.caffeine.cache.RemovalCause; import org.cryptomator.cryptofs.CryptoFileSystemStats; import org.cryptomator.cryptolib.api.AuthenticationFailedException; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Assumptions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.RepeatedTest; import org.junit.jupiter.api.Test; import org.mockito.Mockito; import java.io.IOException; import java.nio.ByteBuffer; +import java.nio.file.AccessDeniedException; +import java.time.Duration; +import java.util.concurrent.CountDownLatch; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -20,147 +29,269 @@ public class ChunkCacheTest { private final ChunkSaver chunkSaver = mock(ChunkSaver.class); private final CryptoFileSystemStats stats = mock(CryptoFileSystemStats.class); private final BufferPool bufferPool = mock(BufferPool.class); - private final ChunkCache inTest = new ChunkCache(chunkLoader, chunkSaver, stats, bufferPool); + private final ExceptionsDuringWrite exceptionsDuringWrite = mock(ExceptionsDuringWrite.class); + private ChunkCache inTest; + + @BeforeEach + public void setup() { + inTest = new ChunkCache(chunkLoader, chunkSaver, stats, bufferPool, exceptionsDuringWrite); + } @Test + @DisplayName("getChunk() invokes chunkLoader.load() on cache miss") public void testGetInvokesLoaderIfEntryNotInCache() throws IOException, AuthenticationFailedException { long index = 42L; - Chunk chunk = mock(Chunk.class); - when(chunkLoader.load(index)).thenReturn(chunk); + var data = ByteBuffer.allocate(0); + when(chunkLoader.load(index)).thenReturn(data); + + inTest.getChunk(index); - Assertions.assertSame(chunk, inTest.get(index)); + verify(chunkLoader).load(index); verify(stats).addChunkCacheAccess(); + verify(stats).addChunkCacheMiss(); } @Test - public void testGetDoesNotInvokeLoaderIfEntryInCacheFromPreviousGet() throws IOException, AuthenticationFailedException { - long index = 42L; - Chunk chunk = mock(Chunk.class); - when(chunkLoader.load(index)).thenReturn(chunk); - inTest.get(index); + @DisplayName("getChunk() propagates AuthenticationFailedException from loader") + public void testGetRethrowsAuthenticationFailedExceptionFromLoader() throws IOException, AuthenticationFailedException { + AuthenticationFailedException authenticationFailedException = new AuthenticationFailedException("Foo"); + when(chunkLoader.load(42L)).thenThrow(authenticationFailedException); - Assertions.assertSame(chunk, inTest.get(index)); - verify(stats, Mockito.times(2)).addChunkCacheAccess(); - verify(chunkLoader).load(index); + IOException e = Assertions.assertThrows(IOException.class, () -> { + inTest.getChunk(42L); + }); + Assertions.assertSame(authenticationFailedException, e.getCause()); } @Test - public void testGetDoesNotInvokeLoaderIfEntryInCacheFromPreviousSet() throws IOException { - long index = 42L; - Chunk chunk = mock(Chunk.class); - inTest.set(index, chunk); + @DisplayName("getChunk() rethrows RuntimeException from loader") + public void testGetThrowsUncheckedExceptionFromLoader() throws IOException, AuthenticationFailedException { + RuntimeException uncheckedException = new RuntimeException(); + when(chunkLoader.load(42L)).thenThrow(uncheckedException); - Assertions.assertSame(chunk, inTest.get(index)); - verify(stats).addChunkCacheAccess(); + Assertions.assertThrows(RuntimeException.class, () -> { + inTest.getChunk(42); + }); } @Test - public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCacheIsRequested() throws IOException, AuthenticationFailedException { - long firstIndex = 42L; - long indexNotInCache = 40L; - Chunk chunk = mock(Chunk.class); - inTest.set(firstIndex, chunk); - for (int i = 1; i < ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS; i++) { - inTest.set(firstIndex + i, mock(Chunk.class)); + @DisplayName("getChunk() propagates IOException from loader") + public void testGetRethrowsIOExceptionFromLoader() throws IOException, AuthenticationFailedException { + when(chunkLoader.load(42L)).thenThrow(new IOException()); + + Assertions.assertThrows(IOException.class, () -> { + inTest.getChunk(42L); + }); + } + + @Nested + @DisplayName("With active chunks [1] and stale chunks [42, 43, 44, 45, 46]") + public class Prepopulated { + + private Chunk activeChunk1; + private Chunk staleChunk42; + private Chunk staleChunk43; + private Chunk staleChunk44; + private Chunk staleChunk45; + private Chunk staleChunk46; + + @BeforeEach + public void setup() { + Assumptions.assumeTrue(ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS == 5); + + activeChunk1 = inTest.putChunk(1L, ByteBuffer.allocate(0)); + Assertions.assertEquals(1, activeChunk1.currentAccesses().get()); + + staleChunk42 = inTest.putChunk(42L, ByteBuffer.allocate(0)); + staleChunk43 = inTest.putChunk(43L, ByteBuffer.allocate(0)); + staleChunk44 = inTest.putChunk(44L, ByteBuffer.allocate(0)); + staleChunk45 = inTest.putChunk(45L, ByteBuffer.allocate(0)); + staleChunk46 = inTest.putChunk(46L, ByteBuffer.allocate(0)); + staleChunk42.close(); + staleChunk43.close(); + staleChunk44.close(); + staleChunk45.close(); + staleChunk46.close(); + Assertions.assertEquals(0, staleChunk42.currentAccesses().get()); + Assertions.assertEquals(0, staleChunk43.currentAccesses().get()); + Assertions.assertEquals(0, staleChunk44.currentAccesses().get()); + Assertions.assertEquals(0, staleChunk45.currentAccesses().get()); + Assertions.assertEquals(0, staleChunk46.currentAccesses().get()); } - when(chunkLoader.load(indexNotInCache)).thenReturn(mock(Chunk.class)); - inTest.get(indexNotInCache); + @Test + @DisplayName("getChunk() does not invoke chunkLoader.load() and returns active chunk") + public void testGetChunkActive() throws IOException { + var chunk = inTest.getChunk(1L); - verify(stats).addChunkCacheAccess(); - verify(chunkSaver).save(firstIndex, chunk); - verify(bufferPool).recycle(chunk.data()); - verifyNoMoreInteractions(chunkSaver); - } + Assertions.assertSame(activeChunk1, chunk); + Assertions.assertEquals(2, chunk.currentAccesses().get()); + verify(stats).addChunkCacheAccess(); + verifyNoMoreInteractions(stats); + verifyNoMoreInteractions(chunkLoader); + } - @Test - public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryNotInCacheIsSet() throws IOException { - long firstIndex = 42L; - long indexNotInCache = 40L; - Chunk chunk = mock(Chunk.class); - inTest.set(firstIndex, chunk); - for (int i = 1; i < ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS; i++) { - inTest.set(firstIndex + i, mock(Chunk.class)); + @Test + @DisplayName("getChunk() does not invoke chunkLoader.load() and returns stale chunk") + public void testGetChunkStale() throws IOException { + var chunk = inTest.getChunk(42L); + + Assertions.assertSame(staleChunk42, chunk); + Assertions.assertEquals(1, chunk.currentAccesses().get()); + verify(stats).addChunkCacheAccess(); + verifyNoMoreInteractions(stats); + verifyNoMoreInteractions(chunkLoader); } - inTest.set(indexNotInCache, mock(Chunk.class)); + @Test + @DisplayName("chunk.close() keeps chunk active if access count > 1") + public void testClosingActiveChunkThatIsReferencedTwice() throws IOException, AuthenticationFailedException { + try (var chunk = inTest.getChunk(1L)) { + Assertions.assertSame(activeChunk1, chunk); + Assertions.assertEquals(2, activeChunk1.currentAccesses().get()); + } // close - verify(chunkSaver).save(firstIndex, chunk); - verify(bufferPool).recycle(chunk.data()); - verifyNoMoreInteractions(chunkSaver); - } + Assertions.assertEquals(1, activeChunk1.currentAccesses().get()); + verifyNoMoreInteractions(chunkSaver); + verifyNoMoreInteractions(bufferPool); + } - @Test - public void testGetInvokesSaverIfMaxEntriesInCacheAreReachedAndAnEntryInCacheIsSet() throws IOException { - // TODO markuskreusch: this behaviour isn't actually needed, maybe we can somehow prevent saving in such situations? - long firstIndex = 42L; - Chunk chunk = mock(Chunk.class); - inTest.set(firstIndex, chunk); - for (int i = 1; i < ChunkCache.MAX_CACHED_CLEARTEXT_CHUNKS; i++) { - inTest.set(firstIndex + i, mock(Chunk.class)); + @RepeatedTest(30) + @DisplayName("chunk.close() triggers eviction of LRU stale chunk") + public void testClosingActiveChunkTriggersEvictionOfStaleChunk() throws IOException, AuthenticationFailedException { + var cdl = new CountDownLatch(1); + Mockito.doAnswer(invocation -> { + cdl.countDown(); + return null; + }).when(chunkSaver).save(Mockito.anyLong(), Mockito.any()); + + activeChunk1.close(); + + Assertions.assertTimeoutPreemptively(Duration.ofMillis(100), () -> { + cdl.await(); + }); + verify(chunkSaver).save(42L, staleChunk42); + verify(bufferPool).recycle(staleChunk42.data()); + verifyNoMoreInteractions(chunkSaver); } - inTest.set(firstIndex, mock(Chunk.class)); + @Test + @DisplayName("flush() saves all stale chunks") + public void testFlushInvokesSaverForAllStaleChunks() throws IOException, AuthenticationFailedException { + inTest.flush(); - verify(chunkSaver).save(firstIndex, chunk); - verify(bufferPool).recycle(chunk.data()); - verifyNoMoreInteractions(chunkSaver); - } + verify(chunkSaver).save(42L, staleChunk42); + verify(chunkSaver).save(43L, staleChunk43); + verify(chunkSaver).save(44L, staleChunk44); + verify(chunkSaver).save(45L, staleChunk45); + verify(chunkSaver).save(46L, staleChunk46); + } - @Test - public void testGetRethrowsAuthenticationFailedExceptionFromLoader() throws IOException, AuthenticationFailedException { - long index = 42L; - AuthenticationFailedException authenticationFailedException = new AuthenticationFailedException("Foo"); - when(chunkLoader.load(index)).thenThrow(authenticationFailedException); + @Test + @DisplayName("flush() saves all active chunks") + public void testFlushInvokesSaverForAllActiveChunks() throws IOException, AuthenticationFailedException { + try (var activeChunk2 = inTest.putChunk(2L, ByteBuffer.allocate(0))) { + inTest.flush(); - IOException e = Assertions.assertThrows(IOException.class, () -> { - inTest.get(index); - }); - Assertions.assertSame(authenticationFailedException, e.getCause()); - } + verify(chunkSaver).save(1L, activeChunk1); + verify(chunkSaver).save(2L, activeChunk2); + } + } - @Test - public void testGetThrowsUncheckedExceptionFromLoader() throws IOException, AuthenticationFailedException { - long index = 42L; - RuntimeException uncheckedException = new RuntimeException(); - when(chunkLoader.load(index)).thenThrow(uncheckedException); + @Test + @DisplayName("flush() does not evict cached chunks") + public void testFlushKeepsItemInCache() throws IOException, AuthenticationFailedException { + inTest.flush(); + + Assertions.assertSame(activeChunk1, inTest.getChunk(1L)); + Assertions.assertSame(staleChunk42, inTest.getChunk(42L)); + verifyNoMoreInteractions(chunkLoader); + } + + @Test + @DisplayName("flush() does not evict cached chunks despite I/O error") + public void testFlushKeepsItemInCacheDespiteIOException() throws IOException, AuthenticationFailedException { + Mockito.doThrow(new AccessDeniedException("")).when(chunkSaver).save(42L, staleChunk42); + + Assertions.assertThrows(IOException.class, () -> inTest.flush()); + + Assertions.assertSame(activeChunk1, inTest.getChunk(1L)); + Assertions.assertSame(staleChunk42, inTest.getChunk(42L)); + verifyNoMoreInteractions(chunkLoader); + } + + @Test + @DisplayName("invalidateAll() flushes stale chunks but keeps active chunks") + public void testInvalidateAll() throws IOException, AuthenticationFailedException { + when(chunkLoader.load(Mockito.anyLong())).thenReturn(ByteBuffer.allocate(0)); + + inTest.invalidateAll(); + + Assertions.assertSame(activeChunk1, inTest.getChunk(1L)); + Assertions.assertNotSame(staleChunk42, inTest.getChunk(42L)); + Assertions.assertNotSame(staleChunk43, inTest.getChunk(43L)); + Assertions.assertNotSame(staleChunk44, inTest.getChunk(44L)); + Assertions.assertNotSame(staleChunk45, inTest.getChunk(45L)); + Assertions.assertNotSame(staleChunk46, inTest.getChunk(46L)); + verify(chunkLoader).load(42L); + verify(chunkLoader).load(43L); + verify(chunkLoader).load(44L); + verify(chunkLoader).load(45L); + verify(chunkLoader).load(46L); + verifyNoMoreInteractions(chunkLoader); + } + + @Test + @DisplayName("putChunk() returns active chunk if already present") + public void testPutChunkReturnsActiveChunk() { + activeChunk1.dirty().set(false); + + var chunk = inTest.putChunk(1L, ByteBuffer.allocate(0)); + + Assertions.assertSame(activeChunk1, chunk); + Assertions.assertEquals(2, chunk.currentAccesses().get()); + Assertions.assertTrue(chunk.isDirty()); + } + + @Test + @DisplayName("putChunk() recycles stale chunk if present") + public void testPutChunkRecyclesStaleChunk() { + var chunk = inTest.putChunk(42L, ByteBuffer.allocate(0)); + + Assertions.assertNotSame(staleChunk42, chunk); + Assertions.assertEquals(1, chunk.currentAccesses().get()); + Assertions.assertTrue(chunk.isDirty()); + verify(bufferPool).recycle(staleChunk42.data()); + } + + @Test + @DisplayName("putChunk() returns new chunk if neither stale nor active") + public void testPutChunkReturnsNewChunk() { + var chunk = inTest.putChunk(100L, ByteBuffer.allocate(0)); + + Assertions.assertNotSame(activeChunk1, chunk); + Assertions.assertNotSame(staleChunk42, chunk); + Assertions.assertNotSame(staleChunk43, chunk); + Assertions.assertNotSame(staleChunk44, chunk); + Assertions.assertNotSame(staleChunk45, chunk); + Assertions.assertNotSame(staleChunk46, chunk); + Assertions.assertEquals(1, chunk.currentAccesses().get()); + Assertions.assertTrue(chunk.isDirty()); + } - RuntimeException e = Assertions.assertThrows(RuntimeException.class, () -> { - inTest.get(index); - }); - Assertions.assertSame(uncheckedException, e.getCause()); - } - @Test - public void testInvalidateAllInvokesSaverForAllEntriesInCache() throws IOException, AuthenticationFailedException { - long index = 42L; - long index2 = 43L; - Chunk chunk1 = mock(Chunk.class); - Chunk chunk2 = mock(Chunk.class); - when(chunk1.data()).thenReturn(ByteBuffer.allocate(42)); - when(chunk2.data()).thenReturn(ByteBuffer.allocate(23)); - when(chunkLoader.load(index)).thenReturn(chunk1); - inTest.get(index); - inTest.set(index2, chunk2); - - inTest.invalidateAll(); - - verify(chunkSaver).save(index, chunk1); - verify(bufferPool).recycle(chunk1.data()); - verify(chunkSaver).save(index2, chunk2); - verify(bufferPool).recycle(chunk2.data()); } @Test - public void testGetRethrowsIOExceptionFromLoader() throws IOException, AuthenticationFailedException { - long index = 42L; + @DisplayName("IOException during eviction of stale chunks is stored to exceptionsDuringWrite") + public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws IOException { IOException ioException = new IOException(); - when(chunkLoader.load(index)).thenThrow(ioException); + Chunk chunk = new Chunk(ByteBuffer.allocate(0), true, () -> {}); + Mockito.doThrow(ioException).when(chunkSaver).save(42L, chunk); - IOException e = Assertions.assertThrows(IOException.class, () -> { - inTest.get(index); - }); - Assertions.assertSame(ioException, e); + inTest.evictStaleChunk(42L, chunk, RemovalCause.EXPIRED); + + verify(exceptionsDuringWrite).add(ioException); } } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java index 8d05b987..dac1d303 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkLoaderTest.java @@ -65,12 +65,11 @@ public void testLoadReturnsEmptyChunkAfterEOF() throws IOException, Authenticati long chunkOffset = chunkIndex * CIPHERTEXT_CHUNK_SIZE + HEADER_SIZE; when(chunkIO.read(argThat(hasAtLeastRemaining(CIPHERTEXT_CHUNK_SIZE)), eq(chunkOffset))).thenReturn(-1); - Chunk chunk = inTest.load(chunkIndex); + var data = inTest.load(chunkIndex); - verify(stats).addChunkCacheMiss(); verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); - Assertions.assertEquals(0, chunk.data().remaining()); - Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); + Assertions.assertEquals(0, data.remaining()); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, data.capacity()); } @Test @@ -89,14 +88,13 @@ public void testLoadReturnsDecryptedDataInsideFile() throws IOException, Authent Mockito.any(), eq(chunkIndex), eq(header), eq(true) // ); - Chunk chunk = inTest.load(chunkIndex); + var data = inTest.load(chunkIndex); - verify(stats).addChunkCacheMiss(); - verify(stats).addBytesDecrypted(chunk.data().remaining()); + verify(stats).addBytesDecrypted(data.remaining()); verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); - assertThat(chunk.data(), contains(decryptedData.get())); - Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().remaining()); - Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); + assertThat(data, contains(decryptedData.get())); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, data.remaining()); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, data.capacity()); } @Test @@ -115,14 +113,13 @@ public void testLoadReturnsDecrytedDataNearEOF() throws IOException, Authenticat any(ByteBuffer.class), eq(chunkIndex), eq(header), eq(true) // ); - Chunk chunk = inTest.load(chunkIndex); + var data = inTest.load(chunkIndex); - verify(stats).addChunkCacheMiss(); - verify(stats).addBytesDecrypted(chunk.data().remaining()); + verify(stats).addBytesDecrypted(data.remaining()); verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); - assertThat(chunk.data(), contains(decryptedData.get())); - Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE - 3, chunk.data().remaining()); - Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, chunk.data().capacity()); + assertThat(data, contains(decryptedData.get())); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE - 3, data.remaining()); + Assertions.assertEquals(CLEARTEXT_CHUNK_SIZE, data.capacity()); } private Answer fillBufferWith(byte value, int amount) { diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java index e3f9e885..a5cf7717 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkSaverTest.java @@ -6,6 +6,7 @@ import org.cryptomator.cryptolib.api.FileContentCryptor; import org.cryptomator.cryptolib.api.FileHeader; import org.cryptomator.cryptolib.api.FileHeaderCryptor; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mockito; @@ -18,6 +19,7 @@ import static org.cryptomator.cryptofs.util.ByteBuffers.repeat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; @@ -37,9 +39,8 @@ public class ChunkSaverTest { private final CryptoFileSystemStats stats = mock(CryptoFileSystemStats.class); private final FileHeader header = mock(FileHeader.class); private final FileHeaderHolder headerHolder = mock(FileHeaderHolder.class); - private final ExceptionsDuringWrite exceptionsDuringWrite = mock(ExceptionsDuringWrite.class); private final BufferPool bufferPool = mock(BufferPool.class); - private final ChunkSaver inTest = new ChunkSaver(cryptor, chunkIO, headerHolder, exceptionsDuringWrite, stats, bufferPool); + private final ChunkSaver inTest = new ChunkSaver(cryptor, chunkIO, headerHolder, stats, bufferPool); @BeforeEach public void setup() throws IOException { @@ -53,13 +54,25 @@ public void setup() throws IOException { when(bufferPool.getCleartextBuffer()).thenAnswer(invocation -> ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE)); } + @Test + public void testSuccessfullWrittenChunkIsNonDirty() throws IOException { + long chunkIndex = 43L; + Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); + Chunk chunk = new Chunk(cleartext.get(), true, () -> {}); + doNothing().when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); + + inTest.save(chunkIndex, chunk); + + Assertions.assertFalse(chunk.isDirty()); + } + @Test public void testChunkInsideFileIsWritten() throws IOException { long chunkIndex = 43L; long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer(); - Chunk chunk = new Chunk(cleartext.get(), true); + Chunk chunk = new Chunk(cleartext.get(), true, () -> {}); doAnswer(invocation -> { ByteBuffer ciphertextBuf = invocation.getArgument(1); ciphertextBuf.put(ciphertext.get()); @@ -79,7 +92,7 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE - 10).asByteBuffer(); Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE - 10).asByteBuffer(); - Chunk chunk = new Chunk(cleartext.get(), true); + Chunk chunk = new Chunk(cleartext.get(), true, () -> {}); doAnswer(invocation -> { ByteBuffer ciphertextBuf = invocation.getArgument(1); ciphertextBuf.put(ciphertext.get()); @@ -96,7 +109,7 @@ public void testChunkContainingEndOfFileIsWrittenTillEndOfFile() throws IOExcept @Test public void testChunkThatWasNotWrittenIsNotWritten() throws IOException { Long chunkIndex = 43L; - Chunk chunk = new Chunk(ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE), false); + Chunk chunk = new Chunk(ByteBuffer.allocate(CLEARTEXT_CHUNK_SIZE), false, () -> {}); inTest.save(chunkIndex, chunk); @@ -105,25 +118,4 @@ public void testChunkThatWasNotWrittenIsNotWritten() throws IOException { verifyNoInteractions(bufferPool); } - @Test - public void testIOExceptionsDuringWriteAreAddedToExceptionsDuringWrite() throws IOException { - IOException ioException = new IOException(); - long chunkIndex = 43L; - long expectedPosition = HEADER_SIZE + chunkIndex * CIPHERTEXT_CHUNK_SIZE; - Supplier cleartext = () -> repeat(42).times(CLEARTEXT_CHUNK_SIZE).asByteBuffer(); - Supplier ciphertext = () -> repeat(50).times(CIPHERTEXT_CHUNK_SIZE).asByteBuffer(); - Chunk chunk = new Chunk(cleartext.get(), true); - doAnswer(invocation -> { - ByteBuffer ciphertextBuf = invocation.getArgument(1); - ciphertextBuf.put(ciphertext.get()); - return null; - }).when(fileContentCryptor).encryptChunk(argThat(contains(cleartext.get())), Mockito.any(), eq(chunkIndex), eq(header)); - when(chunkIO.write(argThat(contains(ciphertext.get())), eq(expectedPosition))).thenThrow(ioException); - - inTest.save(chunkIndex, chunk); - - verify(exceptionsDuringWrite).add(ioException); - verify(bufferPool).recycle(argThat(ByteBufferMatcher.hasCapacity(CIPHERTEXT_CHUNK_SIZE))); - } - } diff --git a/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java b/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java index 01716e12..4f05ee7e 100644 --- a/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java +++ b/src/test/java/org/cryptomator/cryptofs/fh/ChunkTest.java @@ -15,28 +15,10 @@ public class ChunkTest { - @Test - public void testChunkDataWrappingBufferIsNotDirty() { - ByteBuffer buffer = repeat(3).times(200).asByteBuffer(); - - Chunk inTest = new Chunk(buffer, false); - - Assertions.assertFalse(inTest.isDirty()); - } - - @Test - public void testEmptyChunkDataIsNotDirty() { - ByteBuffer buffer = ByteBuffer.allocate(0); - - Chunk inTest = new Chunk(buffer, false); - - Assertions.assertFalse(inTest.isDirty()); - } - @Test // https://github.com/cryptomator/cryptofs/issues/85 public void testRaceConditionsDuringRead() { ByteBuffer src = StandardCharsets.US_ASCII.encode("abcdefg"); - Chunk inTest = new Chunk(src, false); + Chunk inTest = new Chunk(src, false, () -> {}); int attempts = 4000; int threads = 6;