Skip to content

Commit cbbd510

Browse files
overheadhunteroverheadhunter
committed
Amendment to 086a4f4 to accommodate cache misses due to timeouts of logn-running copy operations. Thanks for the hint, @buddydvd
References #52
1 parent a038650 commit cbbd510

File tree

5 files changed

+65
-37
lines changed

5 files changed

+65
-37
lines changed

‎src/main/java/org/cryptomator/cryptofs/ConflictResolver.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ private Path renameConflictingFile(Path canonicalPath, Path conflictingPath, Str
122122
}
123123
LOG.info("Moving conflicting file {} to {}", conflictingPath, alternativePath);
124124
Path resolved = Files.move(conflictingPath, alternativePath, StandardCopyOption.ATOMIC_MOVE);
125-
longFileNameProvider.persistCachedIfDeflated(resolved);
125+
longFileNameProvider.getCached(resolved).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
126126
return resolved;
127127
} catch (AuthenticationFailedException e) {
128128
// not decryptable, no need to resolve any kind of conflict

‎src/main/java/org/cryptomator/cryptofs/CryptoFileSystemImpl.java

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import java.util.Collections;
5757
import java.util.EnumSet;
5858
import java.util.Map;
59+
import java.util.Optional;
5960
import java.util.Set;
6061
import java.util.stream.Collectors;
6162

@@ -304,7 +305,7 @@ void createDirectory(CryptoPath cleartextDir, FileAttribute<?>... attrs) throws
304305
// create dir if and only if the dirFile has been created right now (not if it has been created before):
305306
try {
306307
Files.createDirectories(ciphertextDir.path);
307-
longFileNameProvider.persistCachedIfDeflated(ciphertextDirFile);
308+
longFileNameProvider.getCached(ciphertextDirFile).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
308309
} catch (IOException e) {
309310
// make sure there is no orphan dir file:
310311
Files.delete(ciphertextDirFile);
@@ -355,7 +356,7 @@ private FileChannel newFileChannel(CryptoPath cleartextFilePath, EffectiveOpenOp
355356
} else {
356357
// might also throw FileAlreadyExists:
357358
FileChannel ch = openCryptoFiles.getOrCreate(ciphertextPath).newFileChannel(options);
358-
longFileNameProvider.persistCachedIfDeflated(ciphertextPath);
359+
longFileNameProvider.getCached(ciphertextPath).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
359360
return ch;
360361
}
361362
}
@@ -423,8 +424,10 @@ private void copySymlink(CryptoPath cleartextSource, CryptoPath cleartextTarget,
423424
Path ciphertextSourceFile = cryptoPathMapper.getCiphertextFilePath(cleartextSource, CiphertextFileType.SYMLINK);
424425
Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.SYMLINK);
425426
CopyOption[] resolvedOptions = ArrayUtils.without(options, LinkOption.NOFOLLOW_LINKS).toArray(CopyOption[]::new);
427+
Optional<LongFileNameProvider.DeflatedFileName> deflatedFileName = longFileNameProvider.getCached(ciphertextTargetFile);
426428
Files.copy(ciphertextSourceFile, ciphertextTargetFile, resolvedOptions);
427-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile);
429+
deflatedFileName.ifPresent(LongFileNameProvider.DeflatedFileName::persist);
430+
428431
} else {
429432
CryptoPath resolvedSource = symlinks.resolveRecursively(cleartextSource);
430433
CryptoPath resolvedTarget = symlinks.resolveRecursively(cleartextTarget);
@@ -436,17 +439,19 @@ private void copySymlink(CryptoPath cleartextSource, CryptoPath cleartextTarget,
436439
private void copyFile(CryptoPath cleartextSource, CryptoPath cleartextTarget, CopyOption[] options) throws IOException {
437440
Path ciphertextSourceFile = cryptoPathMapper.getCiphertextFilePath(cleartextSource, CiphertextFileType.FILE);
438441
Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.FILE);
442+
Optional<LongFileNameProvider.DeflatedFileName> deflatedFileName = longFileNameProvider.getCached(ciphertextTargetFile);
439443
Files.copy(ciphertextSourceFile, ciphertextTargetFile, options);
440-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile);
444+
deflatedFileName.ifPresent(LongFileNameProvider.DeflatedFileName::persist);
441445
}
442446

443447
private void copyDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarget, CopyOption[] options) throws IOException {
444448
// DIRECTORY (non-recursive as per contract):
445449
Path ciphertextTargetDirFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.DIRECTORY);
446450
if (Files.notExists(ciphertextTargetDirFile)) {
447451
// create new:
452+
Optional<LongFileNameProvider.DeflatedFileName> deflatedFileName = longFileNameProvider.getCached(ciphertextTargetDirFile);
448453
createDirectory(cleartextTarget);
449-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetDirFile);
454+
deflatedFileName.ifPresent(LongFileNameProvider.DeflatedFileName::persist);
450455
} else if (ArrayUtils.contains(options, StandardCopyOption.REPLACE_EXISTING)) {
451456
// keep existing (if empty):
452457
Path ciphertextTargetDir = cryptoPathMapper.getCiphertextDir(cleartextTarget).path;
@@ -525,7 +530,7 @@ private void moveSymlink(CryptoPath cleartextSource, CryptoPath cleartextTarget,
525530
Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.SYMLINK);
526531
try (OpenCryptoFiles.TwoPhaseMove twoPhaseMove = openCryptoFiles.prepareMove(ciphertextSourceFile, ciphertextTargetFile)) {
527532
Files.move(ciphertextSourceFile, ciphertextTargetFile, options);
528-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile);
533+
longFileNameProvider.getCached(ciphertextTargetFile).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
529534
twoPhaseMove.commit();
530535
}
531536
}
@@ -537,7 +542,7 @@ private void moveFile(CryptoPath cleartextSource, CryptoPath cleartextTarget, Co
537542
Path ciphertextTargetFile = cryptoPathMapper.getCiphertextFilePath(cleartextTarget, CiphertextFileType.FILE);
538543
try (OpenCryptoFiles.TwoPhaseMove twoPhaseMove = openCryptoFiles.prepareMove(ciphertextSourceFile, ciphertextTargetFile)) {
539544
Files.move(ciphertextSourceFile, ciphertextTargetFile, options);
540-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetFile);
545+
longFileNameProvider.getCached(ciphertextTargetFile).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
541546
twoPhaseMove.commit();
542547
}
543548
}
@@ -550,7 +555,7 @@ private void moveDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarge
550555
if (!ArrayUtils.contains(options, StandardCopyOption.REPLACE_EXISTING)) {
551556
// try to move, don't replace:
552557
Files.move(ciphertextSourceDirFile, ciphertextTargetDirFile, options);
553-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetDirFile);
558+
longFileNameProvider.getCached(ciphertextTargetDirFile).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
554559
} else if (ArrayUtils.contains(options, StandardCopyOption.ATOMIC_MOVE)) {
555560
// replace atomically (impossible):
556561
assert ArrayUtils.contains(options, StandardCopyOption.REPLACE_EXISTING);
@@ -569,7 +574,7 @@ private void moveDirectory(CryptoPath cleartextSource, CryptoPath cleartextTarge
569574
Files.delete(ciphertextTargetDir);
570575
}
571576
Files.move(ciphertextSourceDirFile, ciphertextTargetDirFile, options);
572-
longFileNameProvider.persistCachedIfDeflated(ciphertextTargetDirFile);
577+
longFileNameProvider.getCached(ciphertextTargetDirFile).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
573578
}
574579
dirIdProvider.move(ciphertextSourceDirFile, ciphertextTargetDirFile);
575580
cryptoPathMapper.invalidatePathMapping(cleartextSource);

‎src/main/java/org/cryptomator/cryptofs/LongFileNameProvider.java

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,19 @@
1313
import com.google.common.cache.CacheLoader;
1414
import com.google.common.cache.LoadingCache;
1515
import com.google.common.io.BaseEncoding;
16-
import com.google.common.util.concurrent.UncheckedExecutionException;
1716
import org.cryptomator.cryptolib.common.MessageDigestSupplier;
1817

1918
import javax.inject.Inject;
2019
import java.io.IOException;
2120
import java.io.UncheckedIOException;
22-
import java.nio.ByteBuffer;
2321
import java.nio.channels.WritableByteChannel;
2422
import java.nio.file.FileAlreadyExistsException;
2523
import java.nio.file.Files;
2624
import java.nio.file.Path;
2725
import java.nio.file.StandardOpenOption;
2826
import java.time.Duration;
2927
import java.util.Arrays;
28+
import java.util.Optional;
3029
import java.util.concurrent.ExecutionException;
3130

3231
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -86,34 +85,51 @@ public String deflate(String longFileName) {
8685
return shortName;
8786
}
8887

89-
public void persistCachedIfDeflated(Path ciphertextFile) throws IOException {
90-
String filename = ciphertextFile.getFileName().toString();
91-
if (isDeflated(filename)) {
92-
persistCached(filename);
93-
}
88+
private Path resolveMetadataFile(String shortName) {
89+
return metadataRoot.resolve(shortName.substring(0, 2)).resolve(shortName.substring(2, 4)).resolve(shortName);
9490
}
9591

96-
// visible for testing
97-
void persistCached(String shortName) throws IOException {
98-
readonlyFlag.assertWritable();
92+
public Optional<DeflatedFileName> getCached(Path ciphertextFile) {
93+
String shortName = ciphertextFile.getFileName().toString();
9994
String longName = longNames.getIfPresent(shortName);
100-
if (longName == null) {
101-
throw new IllegalStateException("Long name for " + shortName + " has not been shortened within the last " + MAX_CACHE_AGE);
102-
}
103-
Path file = resolveMetadataFile(shortName);
104-
Path fileDir = file.getParent();
105-
assert fileDir != null : "resolveMetadataFile returned path to a file";
106-
Files.createDirectories(fileDir);
107-
try (WritableByteChannel ch = Files.newByteChannel(file, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW)) {
108-
ch.write(UTF_8.encode(longName));
109-
} catch (FileAlreadyExistsException e) {
110-
// no-op: if the file already exists, we assume its content to be what we want (or we found a SHA1 collision ;-))
111-
assert Arrays.equals(Files.readAllBytes(file), longName.getBytes(UTF_8));
95+
if (longName != null) {
96+
return Optional.of(new DeflatedFileName(shortName, longName));
97+
} else {
98+
return Optional.empty();
11299
}
113100
}
114101

115-
private Path resolveMetadataFile(String shortName) {
116-
return metadataRoot.resolve(shortName.substring(0, 2)).resolve(shortName.substring(2, 4)).resolve(shortName);
102+
public class DeflatedFileName {
103+
104+
public final String shortName;
105+
public final String longName;
106+
107+
private DeflatedFileName(String shortName, String longName) {
108+
this.shortName = shortName;
109+
this.longName = longName;
110+
}
111+
112+
public void persist() {
113+
readonlyFlag.assertWritable();
114+
try {
115+
persistInternal();
116+
} catch (IOException e) {
117+
throw new UncheckedIOException(e);
118+
}
119+
}
120+
121+
private void persistInternal() throws IOException {
122+
Path file = resolveMetadataFile(shortName);
123+
Path fileDir = file.getParent();
124+
assert fileDir != null : "resolveMetadataFile returned path to a file";
125+
Files.createDirectories(fileDir);
126+
try (WritableByteChannel ch = Files.newByteChannel(file, StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW)) {
127+
ch.write(UTF_8.encode(longName));
128+
} catch (FileAlreadyExistsException e) {
129+
// no-op: if the file already exists, we assume its content to be what we want (or we found a SHA1 collision ;-))
130+
assert Arrays.equals(Files.readAllBytes(file), longName.getBytes(UTF_8));
131+
}
132+
}
117133
}
118134

119135
}

‎src/main/java/org/cryptomator/cryptofs/Symlinks.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public void createSymbolicLink(CryptoPath cleartextPath, Path target, FileAttrib
4343
EffectiveOpenOptions openOptions = EffectiveOpenOptions.from(EnumSet.of(StandardOpenOption.WRITE, StandardOpenOption.CREATE_NEW), readonlyFlag);
4444
ByteBuffer content = UTF_8.encode(target.toString());
4545
openCryptoFiles.writeCiphertextFile(ciphertextSymlinkFile, openOptions, content);
46-
longFileNameProvider.persistCachedIfDeflated(ciphertextSymlinkFile);
46+
longFileNameProvider.getCached(ciphertextSymlinkFile).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
4747
}
4848

4949
public CryptoPath readSymbolicLink(CryptoPath cleartextPath) throws IOException {

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

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@
1818
import java.nio.file.Files;
1919
import java.nio.file.NoSuchFileException;
2020
import java.nio.file.Path;
21+
import java.nio.file.Paths;
2122
import java.nio.file.ReadOnlyFileSystemException;
2223
import java.nio.file.SimpleFileVisitor;
2324
import java.nio.file.attribute.BasicFileAttributes;
2425
import java.util.Collections;
26+
import java.util.Optional;
2527
import java.util.concurrent.atomic.AtomicInteger;
2628

2729
public class LongFileNameProviderTest {
@@ -58,7 +60,7 @@ public void testDeflateAndInflate(@TempDir Path tmpPath) throws IOException {
5860
Assertions.assertEquals(orig, inflated1);
5961

6062
Assertions.assertEquals(0, countFiles(tmpPath));
61-
prov1.persistCached(deflated);
63+
prov1.getCached(Paths.get(deflated)).ifPresent(LongFileNameProvider.DeflatedFileName::persist);
6264
Assertions.assertEquals(1, countFiles(tmpPath));
6365

6466
LongFileNameProvider prov2 = new LongFileNameProvider(tmpPath, readonlyFlag);
@@ -89,9 +91,14 @@ public void testDeflateMultipleTimes(@TempDir Path tmpPath) {
8991
public void testPerstistCachedFailsOnReadOnlyFileSystems(@TempDir Path tmpPath) {
9092
LongFileNameProvider prov = new LongFileNameProvider(tmpPath, readonlyFlag);
9193

94+
String orig = "longName";
95+
String shortened = prov.deflate(orig);
96+
Optional<LongFileNameProvider.DeflatedFileName> cachedFileName = prov.getCached(Paths.get(shortened));
97+
98+
Assertions.assertTrue(cachedFileName.isPresent());
9299
Mockito.doThrow(new ReadOnlyFileSystemException()).when(readonlyFlag).assertWritable();
93100
Assertions.assertThrows(ReadOnlyFileSystemException.class, () -> {
94-
prov.persistCached("whatever");
101+
cachedFileName.get().persist();
95102
});
96103
}
97104

0 commit comments

Comments
 (0)