Allow deleting left-over files after restore completed (#107)

- Adds new command line flag --delete-missing - Collects and deletes left-over files during RestoreController execution - Adds/updates tests - Updates documentation Resolves #106 {minor} Signed-off-by: Esta Nagy <[email protected]>
nagyesta · Jan 22, 2024 · dce4e11 · dce4e11
1 parent 8617457
commit dce4e11
Show file tree

Hide file tree

Showing 15 changed files with 280 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -39,6 +39,7 @@ File BaRJ comes with the following features
 - Backup archive integrity checks
 - Restore/unpack previous backup
 - Duplicate handling (storing duplicates of the same file only once)
+- Deletes left-over files from the restore directory (if they had been in scope for the backup)
 
 ### Planned features
 
@@ -54,3 +55,4 @@ File BaRJ comes with the following features
 # Limitations
 
 - The file permissions are only captured from the point of view of the current user on Windows or other non-POSIX compliant systems.
+- Restoring a backup on a different OS is not working well (See #94).
diff --git a/file-barj-core/README.md b/file-barj-core/README.md
@@ -40,10 +40,10 @@ at its core.
 implementation("com.github.nagyesta.file-barj:file-barj-core:+")
 ```
 
-### Creating a backup configuration
+### Writing an archive
 
 ```java
-
+//configuring the backup job
 final var configuration = BackupJobConfiguration.builder()
         .backupType(BackupType.FULL)
         .fileNamePrefix("test")
@@ -57,22 +57,28 @@ final var configuration = BackupJobConfiguration.builder()
         .chunkSizeMebibyte(1)
         .encryptionKey(null)
         .build();
-```
-
-### Writing an archive
-
-```java
 final var backupController = new BackupController(configuration, false);
+
+//executing the backup
 backupController.execute(1);
 ```
 
 ### Reading an archive
 
 ```java
-final var restoreController = new RestoreController(Path.of("/tmp/backup"), "test", null);
+//configuring the restore job
 final var restoreTargets = new RestoreTargets(
         Set.of(new RestoreTarget(Path.of("/source/dir"), Path.of("/tmp/restore/to"))));
-restoreController.execute(restoreTargets, 1, false);
+final var restoreTask = RestoreTask.builder()
+        .restoreTargets(restoreTargets)
+        .dryRun(true)
+        .threads(1)
+        .deleteFilesNotInBackup(false)
+        .build();
+final var restoreController = new RestoreController(Path.of("/tmp/backup"), "test", null);
+
+//executing the restore
+restoreController.execute(restoreTask);
 ```
 
 ## Further reading

diff --git a/...rc/main/java/com/github/nagyesta/filebarj/core/backup/worker/PosixFileMetadataParser.java b/...rc/main/java/com/github/nagyesta/filebarj/core/backup/worker/PosixFileMetadataParser.java
@@ -37,7 +37,7 @@ public class PosixFileMetadataParser implements FileMetadataParser {
     @Override
     public FileMetadata parse(
             @NonNull final File file, @NonNull final BackupJobConfiguration configuration) {
-        if (!file.exists()) {
+        if (!Files.exists(file.toPath(), LinkOption.NOFOLLOW_LINKS)) {
             return FileMetadata.builder()
                     .id(UUID.randomUUID())
                     .absolutePath(file.toPath().toAbsolutePath())

diff --git a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/config/RestoreTargets.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/config/RestoreTargets.java
@@ -11,7 +11,7 @@
  *
  * @param restoreTargets the restore targets
  */
-public record RestoreTargets(Set<RestoreTarget> restoreTargets) {
+public record RestoreTargets(@NonNull Set<RestoreTarget> restoreTargets) {
 
     /**
      * Converts the original path to the restore path.

diff --git a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/config/RestoreTask.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/config/RestoreTask.java
@@ -0,0 +1,32 @@
+package com.github.nagyesta.filebarj.core.config;
+
+import lombok.Builder;
+import lombok.Data;
+import lombok.NonNull;
+
+/**
+ * Defines the parameters of a restore task.
+ */
+@Data
+@Builder
+public class RestoreTask {
+
+    /**
+     * Defines the target directories to restore files to.
+     */
+    @NonNull
+    private final RestoreTargets restoreTargets;
+    /**
+     * The number of threads to use for parallel restore.
+     */
+    private final int threads;
+    /**
+     * Disallows file system changes when true.
+     */
+    private final boolean dryRun;
+    /**
+     * Allows deleting files from the restore targets which would have been in the backup scope but
+     * are not in the backup increment when true.
+     */
+    private final boolean deleteFilesNotInBackup;
+}
diff --git a/...e/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestoreController.java b/...e/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestoreController.java
@@ -3,6 +3,7 @@
 import com.github.nagyesta.filebarj.core.common.ManifestManager;
 import com.github.nagyesta.filebarj.core.common.ManifestManagerImpl;
 import com.github.nagyesta.filebarj.core.config.RestoreTargets;
+import com.github.nagyesta.filebarj.core.config.RestoreTask;
 import com.github.nagyesta.filebarj.core.model.FileMetadata;
 import com.github.nagyesta.filebarj.core.model.RestoreManifest;
 import com.github.nagyesta.filebarj.core.model.enums.FileType;
@@ -60,34 +61,31 @@ public RestoreController(
     /**
      * Execute the restore to the provided root directory.
      *
-     * @param restoreTargets the directory mappings where we want to restore our files
-     * @param threads        The number of threads to use
-     * @param dryRun         Whether to perform a dry-run (preventing file system changes)
+     * @param restoreTask the parameters of the task we need to perform when we execute the restore
      */
     public void execute(
-            @NonNull final RestoreTargets restoreTargets,
-            final int threads,
-            final boolean dryRun) {
-        if (threads < 1) {
-            throw new IllegalArgumentException("Invalid number of threads: " + threads);
+            @NonNull final RestoreTask restoreTask) {
+        if (restoreTask.getThreads() < 1) {
+            throw new IllegalArgumentException("Invalid number of threads: " + restoreTask.getThreads());
         }
         executionLock.lock();
         try {
-            this.threadPool = new ForkJoinPool(threads);
+            this.threadPool = new ForkJoinPool(restoreTask.getThreads());
             final var allEntries = manifest.getFilesOfLastManifest().values().stream().toList();
             final var contentSources = manifest.getExistingContentSourceFilesOfLastManifest();
             final long totalBackupSize = allEntries.stream()
                     .map(FileMetadata::getOriginalSizeBytes)
                     .reduce(0L, Long::sum);
-            restoreTargets.restoreTargets()
+            restoreTask.getRestoreTargets().restoreTargets()
                     .forEach(target -> log.info("Restoring {} to {}", target.backupPath(), target.restorePath()));
             log.info("Starting restore of {} MiB backup content (delta not known yet)", totalBackupSize / MEBIBYTE);
             final var startTimeMillis = System.currentTimeMillis();
-            final var pipeline = createRestorePipeline(restoreTargets, dryRun);
+            final var pipeline = createRestorePipeline(restoreTask.getRestoreTargets(), restoreTask.isDryRun());
             pipeline.restoreDirectories(allEntries.stream()
                     .filter(metadata -> metadata.getFileType() == FileType.DIRECTORY)
                     .toList());
             pipeline.restoreFiles(contentSources, threadPool);
+            pipeline.deleteLeftOverFiles(restoreTask.isDeleteFilesNotInBackup(), threadPool);
             pipeline.finalizePermissions(allEntries, threadPool);
             pipeline.evaluateRestoreSuccess(allEntries, threadPool);
             final var endTimeMillis = System.currentTimeMillis();

diff --git a/...ore/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestorePipeline.java b/...ore/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestorePipeline.java
@@ -4,6 +4,7 @@
 import com.github.nagyesta.filebarj.core.backup.worker.FileMetadataParserFactory;
 import com.github.nagyesta.filebarj.core.common.FileMetadataChangeDetector;
 import com.github.nagyesta.filebarj.core.common.FileMetadataChangeDetectorFactory;
+import com.github.nagyesta.filebarj.core.config.BackupSource;
 import com.github.nagyesta.filebarj.core.config.RestoreTargets;
 import com.github.nagyesta.filebarj.core.model.*;
 import com.github.nagyesta.filebarj.core.model.enums.Change;
@@ -103,12 +104,10 @@ public void restoreDirectories(@NonNull final List<FileMetadata> directories) {
      * @param contentSources the files with content to restore
      * @param threadPool     the thread pool we can use for parallel processing
      */
-    @SuppressWarnings("checkstyle:TodoComment")
     public void restoreFiles(
             @NonNull final Collection<FileMetadata> contentSources,
             @NonNull final ForkJoinPool threadPool) {
         log.info("Restoring {} items", contentSources.size());
-        //TODO: Deletions should not be ignored. We need to perform them in a separate step
         final var changeStatus = detectChanges(contentSources, threadPool, false);
         final var pathsToRestore = contentSources.stream()
                 .map(FileMetadata::getAbsolutePath)
@@ -161,6 +160,50 @@ public void finalizePermissions(
                 changedCount.get(type), totalCount.get(type), type));
     }
 
+    /**
+     * Deletes the left-over files in the restore target.
+     *
+     * @param deleteLeftOverFiles True if we should delete left-over files
+     * @param threadPool          The thread pool we can use for parallel processing
+     */
+    public void deleteLeftOverFiles(final boolean deleteLeftOverFiles, @NonNull final ForkJoinPool threadPool) {
+        if (!deleteLeftOverFiles) {
+            log.info("Skipping left-over files deletion...");
+            return;
+        }
+        log.info("Deleting left-over files (if any)...");
+        final var files = manifest.getFilesOfLastManifest().values();
+        final var modifiedSources = manifest.getConfiguration().getSources().stream()
+                .map(source -> BackupSource.builder()
+                        .path(restoreTargets.mapToRestorePath(source.getPath()))
+                        .includePatterns(source.getIncludePatterns())
+                        .excludePatterns(source.getExcludePatterns())
+                        .build())
+                .collect(Collectors.toSet());
+        final var pathsInRestoreTarget = modifiedSources.stream()
+                .map(BackupSource::listMatchingFilePaths)
+                .flatMap(Collection::stream)
+                .collect(Collectors.toSet());
+        final var pathsInBackup = threadPool.submit(() -> files.parallelStream()
+                .map(FileMetadata::getAbsolutePath)
+                .map(restoreTargets::mapToRestorePath)
+                .collect(Collectors.toSet())).join();
+        final var counter = new AtomicInteger(0);
+        threadPool.submit(() -> pathsInRestoreTarget.parallelStream()
+                .filter(path -> !pathsInBackup.contains(path))
+                .sorted(Comparator.reverseOrder())
+                .forEachOrdered(path -> {
+                    try {
+                        log.info("Deleting left-over file: {}", path);
+                        deleteIfExists(path);
+                        counter.incrementAndGet();
+                    } catch (final IOException e) {
+                        throw new ArchivalException("Failed to delete left-over file: " + path, e);
+                    }
+                })).join();
+        log.info("Deleted {} left-over files.", counter.get());
+    }
+
     /**
      * Checks the file statuses after the restore is completed and reports any files that could not
      * be fully restored (either having the wrong content or the wrong metadata).
@@ -585,7 +628,7 @@ private void restoreDirectory(@NotNull final FileMetadata fileMetadata) {
         final var path = restoreTargets.mapToRestorePath(fileMetadata.getAbsolutePath());
         log.debug("Restoring directory: {}", path);
         try {
-            if (Files.exists(path) && !Files.isDirectory(path)) {
+            if (Files.exists(path, LinkOption.NOFOLLOW_LINKS) && !Files.isDirectory(path, LinkOption.NOFOLLOW_LINKS)) {
                 deleteIfExists(path);
             }
             createDirectory(path);

diff --git a/...-barj-core/src/test/java/com/github/nagyesta/filebarj/core/config/RestoreTargetsTest.java b/...-barj-core/src/test/java/com/github/nagyesta/filebarj/core/config/RestoreTargetsTest.java
@@ -58,4 +58,15 @@ void testMapToRestorePathShouldThrowExceptionWhenCalledWithNull() {
 
         //then + exception
     }
+
+    @SuppressWarnings("DataFlowIssue")
+    @Test
+    void testConstructorShouldThrowExceptionWhenCalledWithNull() {
+        //given
+
+        //when
+        Assertions.assertThrows(IllegalArgumentException.class, () -> new RestoreTargets(null));
+
+        //then + exception
+    }
 }
diff --git a/file-barj-core/src/test/java/com/github/nagyesta/filebarj/core/config/RestoreTaskTest.java b/file-barj-core/src/test/java/com/github/nagyesta/filebarj/core/config/RestoreTaskTest.java
@@ -0,0 +1,46 @@
+package com.github.nagyesta.filebarj.core.config;
+
+import com.github.nagyesta.filebarj.core.TempFileAwareTest;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.nio.file.Path;
+import java.util.Set;
+
+class RestoreTaskTest extends TempFileAwareTest {
+
+    @SuppressWarnings("DataFlowIssue")
+    @Test
+    void testBuilderShouldThrowExceptionWhenCalledWithNullRestoreTargets() {
+        //given
+
+        //when
+        Assertions.assertThrows(IllegalArgumentException.class, () -> RestoreTask.builder()
+                .restoreTargets(null));
+
+        //then + exception
+    }
+
+    @Test
+    void testBuilderShouldCreateInstanceWhenCalledWithValidData() {
+        //given
+        final var dryRun = true;
+        final var threads = 2;
+        final var deleteFilesNotInBackup = true;
+        final var restoreTargets = new RestoreTargets(Set.of(new RestoreTarget(Path.of("source"), Path.of("target"))));
+
+        //when
+        final var actual = RestoreTask.builder()
+                .restoreTargets(restoreTargets)
+                .dryRun(dryRun)
+                .threads(threads)
+                .deleteFilesNotInBackup(deleteFilesNotInBackup)
+                .build();
+
+        //then
+        Assertions.assertEquals(dryRun, actual.isDryRun());
+        Assertions.assertEquals(threads, actual.getThreads());
+        Assertions.assertEquals(deleteFilesNotInBackup, actual.isDeleteFilesNotInBackup());
+        Assertions.assertEquals(restoreTargets, actual.getRestoreTargets());
+    }
+}