From b266343926751ab6e20616213c7431e0fc3da37b Mon Sep 17 00:00:00 2001 From: Sebastian Stenzel Date: Fri, 11 Oct 2019 09:14:23 +0200 Subject: [PATCH] Version7Migrator will now perform migration _after_ visiting a directory to avoid changes to elements while still iterating --- .../migration/v7/FilePathMigration.java | 4 ++ .../migration/v7/MigratingVisitor.java | 66 +++++++++++++++++++ .../migration/v7/Version7Migrator.java | 30 +-------- .../migration/v7/Version7MigratorTest.java | 45 +++++++++++++ 4 files changed, 116 insertions(+), 29 deletions(-) create mode 100644 src/main/java/org/cryptomator/cryptofs/migration/v7/MigratingVisitor.java diff --git a/src/main/java/org/cryptomator/cryptofs/migration/v7/FilePathMigration.java b/src/main/java/org/cryptomator/cryptofs/migration/v7/FilePathMigration.java index 3456b6c7..58f8246e 100644 --- a/src/main/java/org/cryptomator/cryptofs/migration/v7/FilePathMigration.java +++ b/src/main/java/org/cryptomator/cryptofs/migration/v7/FilePathMigration.java @@ -173,6 +173,10 @@ Path getTargetPath(String attemptSuffix) { } } } + + public Path getOldPath() { + return oldPath; + } // visible for testing String getOldCanonicalName() { diff --git a/src/main/java/org/cryptomator/cryptofs/migration/v7/MigratingVisitor.java b/src/main/java/org/cryptomator/cryptofs/migration/v7/MigratingVisitor.java new file mode 100644 index 00000000..551632c6 --- /dev/null +++ b/src/main/java/org/cryptomator/cryptofs/migration/v7/MigratingVisitor.java @@ -0,0 +1,66 @@ +package org.cryptomator.cryptofs.migration.v7; + +import org.cryptomator.cryptofs.migration.api.MigrationProgressListener; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.io.IOException; +import java.nio.file.FileAlreadyExistsException; +import java.nio.file.FileVisitResult; +import java.nio.file.Path; +import java.nio.file.SimpleFileVisitor; +import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Optional; + +class MigratingVisitor extends SimpleFileVisitor { + + private static final Logger LOG = LoggerFactory.getLogger(MigratingVisitor.class); + + private final Path vaultRoot; + private final MigrationProgressListener progressListener; + private final long estimatedTotalFiles; + + public MigratingVisitor(Path vaultRoot, MigrationProgressListener progressListener, long estimatedTotalFiles) { + this.vaultRoot = vaultRoot; + this.progressListener = progressListener; + this.estimatedTotalFiles = estimatedTotalFiles; + } + + private Collection migrationsInCurrentDir = new ArrayList<>(); + private long migratedFiles = 0; + + // Step 1: Collect files to be migrated + @Override + public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { + final Optional migration; + try { + migration = FilePathMigration.parse(vaultRoot, file); + } catch (UninflatableFileException e) { + LOG.warn("SKIP {} because inflation failed.", file); + return FileVisitResult.CONTINUE; + } + migration.ifPresent(migrationsInCurrentDir::add); + return FileVisitResult.CONTINUE; + } + + // Step 2: Only after visiting this dir, we will perform any changes to avoid "ConcurrentModificationExceptions" + @Override + public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOException { + for (FilePathMigration migration : migrationsInCurrentDir) { + migratedFiles++; + progressListener.update(MigrationProgressListener.ProgressState.MIGRATING, (double) migratedFiles / estimatedTotalFiles); + try { + Path migratedFile = migration.migrate(); + LOG.info("MOVED {} to {}", migration.getOldPath(), migratedFile); + } catch (FileAlreadyExistsException e) { + LOG.error("Failed to migrate " + migration.getOldPath() + " due to FileAlreadyExistsException. Already migrated on a different machine?.", e); + return FileVisitResult.TERMINATE; + } + } + migrationsInCurrentDir.clear(); + return FileVisitResult.CONTINUE; + } + +} diff --git a/src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java b/src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java index 6b10aea9..cf5fcde7 100644 --- a/src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java +++ b/src/main/java/org/cryptomator/cryptofs/migration/v7/Version7Migrator.java @@ -20,7 +20,6 @@ import javax.inject.Inject; import java.io.IOException; -import java.nio.file.FileAlreadyExistsException; import java.nio.file.FileVisitOption; import java.nio.file.FileVisitResult; import java.nio.file.Files; @@ -30,7 +29,6 @@ import java.nio.file.StandardOpenOption; import java.nio.file.attribute.BasicFileAttributes; import java.util.EnumSet; -import java.util.Optional; import java.util.concurrent.atomic.LongAdder; public class Version7Migrator implements Migrator { @@ -92,33 +90,7 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { private void migrateFileNames(Path vaultRoot, MigrationProgressListener progressListener, long totalFiles) throws IOException { assert totalFiles > 0; Path dataDir = vaultRoot.resolve("d"); - Files.walkFileTree(dataDir, EnumSet.noneOf(FileVisitOption.class), 3, new SimpleFileVisitor() { - - long migratedFiles = 0; - - @Override - public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) throws IOException { - migratedFiles++; - progressListener.update(MigrationProgressListener.ProgressState.MIGRATING, (double) migratedFiles / totalFiles); - final Optional migration; - try { - migration = FilePathMigration.parse(vaultRoot, file); - } catch (UninflatableFileException e) { - LOG.warn("SKIP {} because inflation failed.", file); - return FileVisitResult.CONTINUE; - } - if (migration.isPresent()) { - try { - Path migratedFile = migration.get().migrate(); - LOG.info("MOVED {} to {}", file, migratedFile); - } catch (FileAlreadyExistsException e) { - LOG.error("Failed to migrate " + file + " due to FileAlreadyExistsException. Already migrated?.", e); - return FileVisitResult.TERMINATE; - } - } - return FileVisitResult.CONTINUE; - } - }); + Files.walkFileTree(dataDir, EnumSet.noneOf(FileVisitOption.class), 3, new MigratingVisitor(vaultRoot, progressListener, totalFiles)); } } diff --git a/src/test/java/org/cryptomator/cryptofs/migration/v7/Version7MigratorTest.java b/src/test/java/org/cryptomator/cryptofs/migration/v7/Version7MigratorTest.java index 3fe6a345..aa3e758d 100644 --- a/src/test/java/org/cryptomator/cryptofs/migration/v7/Version7MigratorTest.java +++ b/src/test/java/org/cryptomator/cryptofs/migration/v7/Version7MigratorTest.java @@ -69,4 +69,49 @@ public void testMDirectoryGetsDeleted() throws IOException { Assertions.assertFalse(Files.exists(metaDir)); } + @Test + public void testMigrationOfNormalFile() throws IOException { + Path dir = dataDir.resolve("AA/BBBBBCCCCCDDDDDEEEEEFFFFFGGGGG"); + Files.createDirectories(dir); + Path fileBeforeMigration = dir.resolve("MZUWYZLOMFWWK==="); + Path fileAfterMigration = dir.resolve("ZmlsZW5hbWU=.c9r"); + Files.createFile(fileBeforeMigration); + + Migrator migrator = new Version7Migrator(cryptorProvider); + migrator.migrate(vaultRoot, "masterkey.cryptomator", "test"); + + Assertions.assertFalse(Files.exists(fileBeforeMigration)); + Assertions.assertTrue(Files.exists(fileAfterMigration)); + } + + @Test + public void testMigrationOfNormalDirectory() throws IOException { + Path dir = dataDir.resolve("AA/BBBBBCCCCCDDDDDEEEEEFFFFFGGGGG"); + Files.createDirectories(dir); + Path fileBeforeMigration = dir.resolve("0MZUWYZLOMFWWK==="); + Path fileAfterMigration = dir.resolve("ZmlsZW5hbWU=.c9r/dir.c9r"); + Files.createFile(fileBeforeMigration); + + Migrator migrator = new Version7Migrator(cryptorProvider); + migrator.migrate(vaultRoot, "masterkey.cryptomator", "test"); + + Assertions.assertFalse(Files.exists(fileBeforeMigration)); + Assertions.assertTrue(Files.exists(fileAfterMigration)); + } + + @Test + public void testMigrationOfNormalSymlink() throws IOException { + Path dir = dataDir.resolve("AA/BBBBBCCCCCDDDDDEEEEEFFFFFGGGGG"); + Files.createDirectories(dir); + Path fileBeforeMigration = dir.resolve("1SMZUWYZLOMFWWK==="); + Path fileAfterMigration = dir.resolve("ZmlsZW5hbWU=.c9r/symlink.c9r"); + Files.createFile(fileBeforeMigration); + + Migrator migrator = new Version7Migrator(cryptorProvider); + migrator.migrate(vaultRoot, "masterkey.cryptomator", "test"); + + Assertions.assertFalse(Files.exists(fileBeforeMigration)); + Assertions.assertTrue(Files.exists(fileAfterMigration)); + } + }