Improve restore process and link handling (#85)

- Updates backup process to save symbolic link values as-is (no longer force absolute paths) - Updates restore process to avoid a wasteful recalculation of some file sets - Adds additional logs to make it more clear what is happening during backup and restore - Fixes some typos - Changes how exists checks are done in case of symbolic links (links are no longer followed) to avoid collisions Resolves #84 {minor} Signed-off-by: Esta Nagy <[email protected]>
nagyesta · Jan 1, 2024 · 4e910b5 · 4e910b5
1 parent a374d6c
commit 4e910b5
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 15 deletions.
diff --git a/...ore/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BackupController.java b/...ore/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BackupController.java
@@ -89,16 +89,17 @@ public void execute(final int threads) {
 
     private void listAffectedFilesFromBackupSources() {
         log.info("Listing affected files from {} backup sources", manifest.getConfiguration().getSources().size());
-        this.filesFound = manifest.getConfiguration().getSources().stream()
+        final SortedSet<Path> uniquePaths = manifest.getConfiguration().getSources().stream()
                 .flatMap(source -> {
                     log.info("Listing files from backup source: {}", source);
                     return source.listMatchingFilePaths().stream();
                 })
-                .distinct()
-                .sorted(Comparator.comparing(Path::toAbsolutePath))
-                .parallel()
+                .collect(Collectors.toCollection(TreeSet::new));
+        log.info("Found {} unique files in backup sources. Parsing metadata...", uniquePaths.size());
+        this.filesFound = uniquePaths.parallelStream()
                 .map(path -> metadataParser.parse(path.toFile(), manifest.getConfiguration()))
                 .collect(Collectors.toList());
+        log.info("Parsed metadata of {} files in backup sources.", filesFound.size());
     }
 
     private void calculateBackupDelta() {

diff --git a/...e/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BaseBackupPipeline.java b/...e/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BaseBackupPipeline.java
@@ -154,7 +154,7 @@ private void archiveContentAndUpdateMetadata(
         archivedFileMetadata.setArchivedHash(source.getContentBoundary().getArchivedHash());
         if (!Objects.equals(archivedFileMetadata.getOriginalHash(), fileMetadata.getOriginalHash())) {
             log.warn("The hash changed between delta calculation and archival for: " + fileMetadata.getAbsolutePath()
-                    + "The archive might contain corrupt data for the file.");
+                    + " The archive might contain corrupt data for the file.");
         }
         //commit
         fileMetadata.setArchiveMetadataId(archivedFileMetadata.getId());

diff --git a/...c/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/ParallelBackupPipeline.java b/...c/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/ParallelBackupPipeline.java
@@ -105,7 +105,7 @@ private CompletableFuture<ArchivedFileMetadata> archiveContentAndUpdateMetadata(
             archivedFileMetadata.setArchivedHash(boundarySource.getContentBoundary().getArchivedHash());
             if (!Objects.equals(archivedFileMetadata.getOriginalHash(), fileMetadata.getOriginalHash())) {
                 log.warn("The hash changed between delta calculation and archival for: " + fileMetadata.getAbsolutePath()
-                        + "The archive might contain corrupt data for the file.");
+                        + " The archive might contain corrupt data for the file.");
             }
             //commit
             fileMetadata.setArchiveMetadataId(archivedFileMetadata.getId());

diff --git a/...barj-core/src/main/java/com/github/nagyesta/filebarj/core/common/ManifestManagerImpl.java b/...barj-core/src/main/java/com/github/nagyesta/filebarj/core/common/ManifestManagerImpl.java
@@ -188,7 +188,7 @@ private void populateFilesAndArchiveEntries(
                     remainingFiles.entrySet().stream()
                             .filter(entry -> manifest.getVersions().contains(entry.getValue().getBackupIncrement()))
                             .forEach(entry -> {
-                                //use the file metadata form the last manifest as that is the source of truth
+                                //use the file metadata from the last manifest as that is the source of truth
                                 files.computeIfAbsent(manifest.getFileNamePrefix(), prefix -> new HashMap<>())
                                         .put(entry.getKey().getId(), entry.getKey());
                                 //find the archived entry in the earlier manifests to be able to add the file name prefix

diff --git a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/model/enums/FileType.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/model/enums/FileType.java
@@ -36,7 +36,7 @@ public InputStream streamContent(final Path path) throws IOException {
      */
     SYMBOLIC_LINK(BasicFileAttributes::isSymbolicLink, true) {
         public InputStream streamContent(final Path path) throws IOException {
-            final var linkedPathAsString = Files.readSymbolicLink(path).toAbsolutePath().toString();
+            final var linkedPathAsString = Files.readSymbolicLink(path).toString();
             return new ByteArrayInputStream(linkedPathAsString.getBytes(StandardCharsets.UTF_8));
         }
     },

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
@@ -59,13 +59,14 @@ public RestoreController(
         final var manifests = manifestManager.load(backupDirectory, fileNamePrefix, kek, Integer.MAX_VALUE);
         log.info("Merging {} manifests", manifests.size());
         manifest = manifestManager.mergeForRestore(manifests);
+        log.info("Merge completed. Found {} files in backup.", manifest.allFilesReadOnly().size());
         filesFound = new HashMap<>();
         allEntries = new ArrayList<>();
+        final var archivedEntries = manifest.allArchivedEntriesReadOnly();
         manifest.allFilesReadOnly().values().stream()
                 .filter(metadata -> metadata.getStatus() != Change.DELETED)
                 .forEach(metadata -> {
                     if (metadata.getFileType() != FileType.DIRECTORY) {
-                        final var archivedEntries = manifest.allArchivedEntriesReadOnly();
                         final var archived = archivedEntries.get(metadata.getArchiveMetadataId());
                         filesFound.put(metadata, archived);
                     }

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
@@ -30,6 +30,7 @@
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
+import java.nio.file.LinkOption;
 import java.nio.file.Path;
 import java.security.PrivateKey;
 import java.util.*;
@@ -173,7 +174,7 @@ protected RestoreTargets getRestoreTargets() {
     }
 
     /**
-     * Removes the existing file and creates a symbolic link to point ot the desired target..
+     * Removes the existing file and creates a symbolic link to point ot the desired target.
      *
      * @param linkTarget   the link target
      * @param symbolicLink the link file we need to create
@@ -225,6 +226,7 @@ protected void createDirectory(@NotNull final Path path) throws IOException {
      * @param target  the target where we need to store the content
      */
     protected void restoreFileContent(@NotNull final InputStream content, @NotNull final Path target) {
+        createParentDirectoryAsFallbackIfMissing(target);
         try (var outputStream = new FileOutputStream(target.toFile());
              var bufferedStream = new BufferedOutputStream(outputStream);
              var countingStream = new CountingOutputStream(bufferedStream)) {
@@ -242,16 +244,27 @@ protected void restoreFileContent(@NotNull final InputStream content, @NotNull f
      * @throws IOException if an I/O error occurs
      */
     protected void deleteIfExists(@NotNull final Path currentFile) throws IOException {
-        if (!Files.exists(currentFile)) {
+        if (!Files.exists(currentFile, LinkOption.NOFOLLOW_LINKS)) {
             return;
         }
-        if (Files.isDirectory(currentFile)) {
+        if (Files.isDirectory(currentFile, LinkOption.NOFOLLOW_LINKS)) {
             FileUtils.deleteDirectory(currentFile.toFile());
         } else {
             Files.delete(currentFile);
         }
     }
 
+    private void createParentDirectoryAsFallbackIfMissing(@NotNull final Path target) {
+        try {
+            if (target.getParent() != null && !Files.exists(target.getParent())) {
+                log.warn("Creating missing parent directory: {}", target.getParent());
+                Files.createDirectories(target.getParent());
+            }
+        } catch (final IOException e) {
+            throw new ArchivalException("Failed to restore content: " + target, e);
+        }
+    }
+
     private void restoreContent(
             final int threads,
             @NotNull final Map<FileMetadata, ArchivedFileMetadata> filesWithContentChanges,
@@ -443,11 +456,18 @@ private void createSymbolicLinks(@NotNull final ConcurrentHashMap<FileMetadata,
 
     private boolean shouldCreateNewLink(final Path linkTarget, final Path to) throws IOException {
         var linkNeeded = true;
-        if (Files.exists(to) && Files.isSymbolicLink(to)) {
+        if (Files.exists(to, LinkOption.NOFOLLOW_LINKS) && Files.isSymbolicLink(to)) {
             final var currentTarget = IOUtils.toString(FileType.SYMBOLIC_LINK.streamContent(to), StandardCharsets.UTF_8);
             if (currentTarget.equals(linkTarget.toString())) {
+                log.debug("Found existing link: {} correctly pointing to: {}", to, currentTarget);
                 linkNeeded = false;
+            } else {
+                log.debug("Found existing link: {} pointing to: {} instead of: {}", to, currentTarget, linkTarget);
             }
+        } else if (!Files.exists(to, LinkOption.NOFOLLOW_LINKS)) {
+            log.debug("Link does not exist: {}", to);
+        } else if (!Files.isSymbolicLink(to)) {
+            log.debug("File exist, but is not symbolic link: {}", to);
         }
         return linkNeeded;
     }
@@ -466,7 +486,7 @@ private boolean skipIfNotInScope(
             archiveEntry.skipMetadata();
             return true;
         } catch (final IOException e) {
-            throw new ArchivalException("Failed to skip content and metadate.", e);
+            throw new ArchivalException("Failed to skip content and metadata.", e);
         }
     }
 
@@ -525,7 +545,7 @@ private void skipMetadata(@NotNull final SequentialBarjCargoArchiveEntry archive
         try {
             archiveEntry.skipMetadata();
         } catch (final IOException e) {
-            throw new ArchivalException("Failed to skip metadate.", e);
+            throw new ArchivalException("Failed to skip metadata.", e);
         }
     }