-
Notifications
You must be signed in to change notification settings - Fork 36
Feature: Invalidate dir cache correctly #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
84e7167
invalidate/move also all cleartext path cache entries starting with t…
infeo 3515794
refactor dir cache to own inner static class
infeo eb06489
extend refactoring by moving inner CryptoPathMapper classes to own cl…
infeo 325c13f
add unit tests for dir cache
infeo bffacaf
removed TODOs and renamed a function
infeo 17b0e0a
add integrate-like-tests in cryptoPathMapper
infeo 0cd494c
doc doc doc
infeo eda73ff
reduce diff by reverting renames
infeo ec01bb6
apply suggestions
infeo File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package org.cryptomator.cryptofs; | ||
|
|
||
| import java.nio.file.Path; | ||
| import java.util.Objects; | ||
|
|
||
| //own file due to dagger | ||
| public record CipherDir(String dirId, Path contentDirPath) { | ||
|
|
||
| public CipherDir(String dirId, Path contentDirPath) { | ||
| this.dirId = Objects.requireNonNull(dirId); | ||
| this.contentDirPath = Objects.requireNonNull(contentDirPath); | ||
| } | ||
|
|
||
| } | ||
13 changes: 13 additions & 0 deletions
13
src/main/java/org/cryptomator/cryptofs/CipherNodeNameParameters.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package org.cryptomator.cryptofs; | ||
|
|
||
| import java.util.Objects; | ||
|
|
||
| //own file due to dagger | ||
| public record CipherNodeNameParameters(String dirId, String clearNodeName) { | ||
|
infeo marked this conversation as resolved.
Outdated
|
||
|
|
||
| public CipherNodeNameParameters(String dirId, String clearNodeName) { | ||
| this.dirId = Objects.requireNonNull(dirId); | ||
| this.clearNodeName = Objects.requireNonNull(clearNodeName); | ||
| } | ||
|
|
||
| } | ||
79 changes: 79 additions & 0 deletions
79
src/main/java/org/cryptomator/cryptofs/ClearToCipherDirCache.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| package org.cryptomator.cryptofs; | ||
|
|
||
| import com.github.benmanes.caffeine.cache.AsyncCache; | ||
| import com.github.benmanes.caffeine.cache.Caffeine; | ||
|
|
||
| import java.io.IOException; | ||
| import java.time.Duration; | ||
| import java.util.ArrayList; | ||
| import java.util.concurrent.CompletableFuture; | ||
|
|
||
| public class ClearToCipherDirCache { | ||
|
infeo marked this conversation as resolved.
Outdated
|
||
|
|
||
| private static final int MAX_CACHED_PATHS = 5000; | ||
| private static final Duration MAX_CACHE_AGE = Duration.ofSeconds(20); | ||
|
|
||
| private final AsyncCache<CryptoPath, CipherDir> ciphertextDirectories = Caffeine.newBuilder() // | ||
| .maximumSize(MAX_CACHED_PATHS) // | ||
| .expireAfterWrite(MAX_CACHE_AGE) // | ||
| .buildAsync(); | ||
|
|
||
| /** | ||
| * Removes all (key,value) entries, where {@code key.startsWith(oldPrefix) == true}. | ||
| * | ||
| * @param basePrefix The prefix key which the keys are checked against | ||
| */ | ||
| void removeAllKeysWithPrefix(CryptoPath basePrefix) { | ||
| ciphertextDirectories.asMap().keySet().removeIf(p -> p.startsWith(basePrefix)); | ||
| } | ||
|
infeo marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Remaps all (key,value) entries, where {@code key.startsWith(oldPrefix) == true}. | ||
| * The new key is computed by replacing the oldPrefix with the newPrefix. | ||
| * | ||
| * @param oldPrefix the prefix key which the keys are checked against | ||
| * @param newPrefix the prefix key which replaces {@code oldPrefix} | ||
| */ | ||
| void recomputeAllKeysWithPrefix(CryptoPath oldPrefix, CryptoPath newPrefix) { | ||
| var remappedEntries = new ArrayList<CacheEntry>(); | ||
| ciphertextDirectories.asMap().entrySet().removeIf(e -> { | ||
| if (e.getKey().startsWith(oldPrefix)) { | ||
| var remappedPath = newPrefix.resolve(oldPrefix.relativize(e.getKey())); | ||
| return remappedEntries.add(new CacheEntry(remappedPath, e.getValue())); | ||
| } else { | ||
| return false; | ||
| } | ||
| }); | ||
| remappedEntries.forEach(e -> ciphertextDirectories.put(e.clearPath(), e.cipherDir())); | ||
| } | ||
|
|
||
|
infeo marked this conversation as resolved.
|
||
|
|
||
| /** | ||
| * Gets the cipher directory for the given cleartext path. If a cache miss occurs, the mapping is loaded with the {@code ifAbsent} function. | ||
| * @param cleartextPath Cleartext path key | ||
| * @param ifAbsent Function to compute the (cleartextPath, cipherDir) mapping on a cache miss. | ||
| * @return a {@link CipherDir}, containing the dirId and the ciphertext content directory path | ||
| * @throws IOException if the loading function throws an IOExcecption | ||
| */ | ||
|
infeo marked this conversation as resolved.
Outdated
|
||
| CipherDir get(CryptoPath cleartextPath, CipherDirLoader ifAbsent) throws IOException { | ||
| var futureMapping = new CompletableFuture<CipherDir>(); | ||
| var currentMapping = ciphertextDirectories.asMap().putIfAbsent(cleartextPath, futureMapping); | ||
| if (currentMapping != null) { | ||
| return currentMapping.join(); | ||
| } else { | ||
| futureMapping.complete(ifAbsent.load()); | ||
| return futureMapping.join(); | ||
| } | ||
| } | ||
|
|
||
|
infeo marked this conversation as resolved.
Outdated
|
||
| @FunctionalInterface | ||
| interface CipherDirLoader { | ||
|
|
||
| CipherDir load() throws IOException; | ||
| } | ||
|
|
||
| private record CacheEntry(CryptoPath clearPath, CompletableFuture<CipherDir> cipherDir) { | ||
|
|
||
| } | ||
|
|
||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
contentDirPathtoc9rDirPathfor consistency?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'll revert it to
pathfor a small diff and consistency.c9rDirPathis, from my point of view, the most misleading, because the directory with thec9rsuffix contains the dir.c9r file with the dir-id. For a cleartext directory/foo/barthere are always two directories on the cipher side:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, this class cannot be package private. It is used in different packages, e.g.
DirectoryStreamFactoryorMissingContentDir.