Skip to content

Commit b266343

Browse files
overheadhunteroverheadhunter
committed
Version7Migrator will now perform migration _after_ visiting a directory to avoid changes to elements while still iterating
1 parent ca063e9 commit b266343

File tree

4 files changed

+116
-29
lines changed

4 files changed

+116
-29
lines changed

‎src/main/java/org/cryptomator/cryptofs/migration/v7/FilePathMigration.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,10 @@ Path getTargetPath(String attemptSuffix) {
173173
}
174174
}
175175
}
176+
177+
public Path getOldPath() {
178+
return oldPath;
179+
}
176180

177181
// visible for testing
178182
String getOldCanonicalName() {

‎src/main/java/org/cryptomator/cryptofs/migration/v7/MigratingVisitor.java

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package org.cryptomator.cryptofs.migration.v7;
2+
3+
import org.cryptomator.cryptofs.migration.api.MigrationProgressListener;
4+
import org.slf4j.Logger;
5+
import org.slf4j.LoggerFactory;
6+
7+
import java.io.IOException;
8+
import java.nio.file.FileAlreadyExistsException;
9+
import java.nio.file.FileVisitResult;
10+
import java.nio.file.Path;
11+
import java.nio.file.SimpleFileVisitor;
12+
import java.nio.file.attribute.BasicFileAttributes;
13+
import java.util.ArrayList;
14+
import java.util.Collection;
15+
import java.util.Optional;
16+
17+
class MigratingVisitor extends SimpleFileVisitor<Path> {
18+
19+
private static final Logger LOG = LoggerFactory.getLogger(MigratingVisitor.class);
20+
21+
private final Path vaultRoot;
22+
private final MigrationProgressListener progressListener;
23+
private final long estimatedTotalFiles;
24+
25+
public MigratingVisitor(Path vaultRoot, MigrationProgressListener progressListener, long estimatedTotalFiles) {
26+
this.vaultRoot = vaultRoot;
27+
this.progressListener = progressListener;
28+
this.estimatedTotalFiles = estimatedTotalFiles;
29+
}
30+
31+
private Collection<FilePathMigration> migrationsInCurrentDir = new ArrayList<>();
32+
private long migratedFiles = 0;
33+
34+
// Step 1: Collect files to be migrated
35+
@Override
36+
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
37+
final Optional<FilePathMigration> migration;
38+
try {
39+
migration = FilePathMigration.parse(vaultRoot, file);
40+
} catch (UninflatableFileException e) {
41+
LOG.warn("SKIP {} because inflation failed.", file);
42+
return FileVisitResult.CONTINUE;
43+
}
44+
migration.ifPresent(migrationsInCurrentDir::add);
45+
return FileVisitResult.CONTINUE;
46+
}
47+
48+
// Step 2: Only after visiting this dir, we will perform any changes to avoid "ConcurrentModificationExceptions"
49+
@Override
50+
public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException {
51+
for (FilePathMigration migration : migrationsInCurrentDir) {
52+
migratedFiles++;
53+
progressListener.update(MigrationProgressListener.ProgressState.MIGRATING, (double) migratedFiles / estimatedTotalFiles);
54+
try {
55+
Path migratedFile = migration.migrate();
56+
LOG.info("MOVED {} to {}", migration.getOldPath(), migratedFile);
57+
} catch (FileAlreadyExistsException e) {
58+
LOG.error("Failed to migrate " + migration.getOldPath() + " due to FileAlreadyExistsException. Already migrated on a different machine?.", e);
59+
return FileVisitResult.TERMINATE;
60+
}
61+
}
62+
migrationsInCurrentDir.clear();
63+
return FileVisitResult.CONTINUE;
64+
}
65+
66+
}

‎src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020

2121
import javax.inject.Inject;
2222
import java.io.IOException;
23-
import java.nio.file.FileAlreadyExistsException;
2423
import java.nio.file.FileVisitOption;
2524
import java.nio.file.FileVisitResult;
2625
import java.nio.file.Files;
@@ -30,7 +29,6 @@
3029
import java.nio.file.StandardOpenOption;
3130
import java.nio.file.attribute.BasicFileAttributes;
3231
import java.util.EnumSet;
33-
import java.util.Optional;
3432
import java.util.concurrent.atomic.LongAdder;
3533

3634
public class Version7Migrator implements Migrator {
@@ -92,33 +90,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) {
9290
private void migrateFileNames(Path vaultRoot, MigrationProgressListener progressListener, long totalFiles) throws IOException {
9391
assert totalFiles > 0;
9492
Path dataDir = vaultRoot.resolve("d");
95-
Files.walkFileTree(dataDir, EnumSet.noneOf(FileVisitOption.class), 3, new SimpleFileVisitor<Path>() {
96-
97-
long migratedFiles = 0;
98-
99-
@Override
100-
public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException {
101-
migratedFiles++;
102-
progressListener.update(MigrationProgressListener.ProgressState.MIGRATING, (double) migratedFiles / totalFiles);
103-
final Optional<FilePathMigration> migration;
104-
try {
105-
migration = FilePathMigration.parse(vaultRoot, file);
106-
} catch (UninflatableFileException e) {
107-
LOG.warn("SKIP {} because inflation failed.", file);
108-
return FileVisitResult.CONTINUE;
109-
}
110-
if (migration.isPresent()) {
111-
try {
112-
Path migratedFile = migration.get().migrate();
113-
LOG.info("MOVED {} to {}", file, migratedFile);
114-
} catch (FileAlreadyExistsException e) {
115-
LOG.error("Failed to migrate " + file + " due to FileAlreadyExistsException. Already migrated?.", e);
116-
return FileVisitResult.TERMINATE;
117-
}
118-
}
119-
return FileVisitResult.CONTINUE;
120-
}
121-
});
93+
Files.walkFileTree(dataDir, EnumSet.noneOf(FileVisitOption.class), 3, new MigratingVisitor(vaultRoot, progressListener, totalFiles));
12294
}
12395

12496
}

‎src/test/java/org/cryptomator/cryptofs/migration/v7/Version7MigratorTest.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,4 +69,49 @@ public void testMDirectoryGetsDeleted() throws IOException {
6969
Assertions.assertFalse(Files.exists(metaDir));
7070
}
7171

72+
@Test
73+
public void testMigrationOfNormalFile() throws IOException {
74+
Path dir = dataDir.resolve("AA/BBBBBCCCCCDDDDDEEEEEFFFFFGGGGG");
75+
Files.createDirectories(dir);
76+
Path fileBeforeMigration = dir.resolve("MZUWYZLOMFWWK===");
77+
Path fileAfterMigration = dir.resolve("ZmlsZW5hbWU=.c9r");
78+
Files.createFile(fileBeforeMigration);
79+
80+
Migrator migrator = new Version7Migrator(cryptorProvider);
81+
migrator.migrate(vaultRoot, "masterkey.cryptomator", "test");
82+
83+
Assertions.assertFalse(Files.exists(fileBeforeMigration));
84+
Assertions.assertTrue(Files.exists(fileAfterMigration));
85+
}
86+
87+
@Test
88+
public void testMigrationOfNormalDirectory() throws IOException {
89+
Path dir = dataDir.resolve("AA/BBBBBCCCCCDDDDDEEEEEFFFFFGGGGG");
90+
Files.createDirectories(dir);
91+
Path fileBeforeMigration = dir.resolve("0MZUWYZLOMFWWK===");
92+
Path fileAfterMigration = dir.resolve("ZmlsZW5hbWU=.c9r/dir.c9r");
93+
Files.createFile(fileBeforeMigration);
94+
95+
Migrator migrator = new Version7Migrator(cryptorProvider);
96+
migrator.migrate(vaultRoot, "masterkey.cryptomator", "test");
97+
98+
Assertions.assertFalse(Files.exists(fileBeforeMigration));
99+
Assertions.assertTrue(Files.exists(fileAfterMigration));
100+
}
101+
102+
@Test
103+
public void testMigrationOfNormalSymlink() throws IOException {
104+
Path dir = dataDir.resolve("AA/BBBBBCCCCCDDDDDEEEEEFFFFFGGGGG");
105+
Files.createDirectories(dir);
106+
Path fileBeforeMigration = dir.resolve("1SMZUWYZLOMFWWK===");
107+
Path fileAfterMigration = dir.resolve("ZmlsZW5hbWU=.c9r/symlink.c9r");
108+
Files.createFile(fileBeforeMigration);
109+
110+
Migrator migrator = new Version7Migrator(cryptorProvider);
111+
migrator.migrate(vaultRoot, "masterkey.cryptomator", "test");
112+
113+
Assertions.assertFalse(Files.exists(fileBeforeMigration));
114+
Assertions.assertTrue(Files.exists(fileAfterMigration));
115+
}
116+
72117
}

0 commit comments

Comments
 (0)