From 4e910b5ab956fb75281b4bbc1321272f868f751c Mon Sep 17 00:00:00 2001 From: Esta Nagy Date: Mon, 1 Jan 2024 22:36:11 +0100 Subject: [PATCH] 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 --- .../backup/pipeline/BackupController.java | 9 +++--- .../backup/pipeline/BaseBackupPipeline.java | 2 +- .../pipeline/ParallelBackupPipeline.java | 2 +- .../core/common/ManifestManagerImpl.java | 2 +- .../filebarj/core/model/enums/FileType.java | 2 +- .../restore/pipeline/RestoreController.java | 3 +- .../restore/pipeline/RestorePipeline.java | 32 +++++++++++++++---- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BackupController.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BackupController.java index cc4e452..487c417 100644 --- a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BackupController.java +++ b/file-barj-core/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 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/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BaseBackupPipeline.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BaseBackupPipeline.java index 4ce136c..3c246ca 100644 --- a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/BaseBackupPipeline.java +++ b/file-barj-core/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/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/ParallelBackupPipeline.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/ParallelBackupPipeline.java index ce7298a..64d8d91 100644 --- a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/ParallelBackupPipeline.java +++ b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/backup/pipeline/ParallelBackupPipeline.java @@ -105,7 +105,7 @@ private CompletableFuture 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/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/common/ManifestManagerImpl.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/common/ManifestManagerImpl.java index 912e471..b7ae899 100644 --- a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/common/ManifestManagerImpl.java +++ b/file-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 index a5af8cb..074805c 100644 --- 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/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestoreController.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestoreController.java index 9a529f2..f5170a3 100644 --- a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestoreController.java +++ b/file-barj-core/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/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestorePipeline.java b/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestorePipeline.java index 7ab5b23..a6dea50 100644 --- a/file-barj-core/src/main/java/com/github/nagyesta/filebarj/core/restore/pipeline/RestorePipeline.java +++ b/file-barj-core/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 filesWithContentChanges, @@ -443,11 +456,18 @@ private void createSymbolicLinks(@NotNull final ConcurrentHashMap