diff --git a/FileDeduplicator.Test/DeduplicatorTests.cs b/FileDeduplicator.Test/DeduplicatorTests.cs
index 89b7a34..dea0d86 100644
--- a/FileDeduplicator.Test/DeduplicatorTests.cs
+++ b/FileDeduplicator.Test/DeduplicatorTests.cs
@@ -232,6 +232,149 @@ public void ReclaimedBytesCountsOnlyTheDeletedCopies()
Assert.AreEqual(200, result.BytesReclaimed);
}
+ ///
+ /// Grouping happens before an interactive confirmation prompt of unbounded length, so a file
+ /// can stop being a duplicate before the delete pass runs. Such a file must be preserved and
+ /// reported, never deleted on the strength of the stale hash.
+ ///
+ [TestMethod]
+ public void AFileThatChangedAfterGroupingIsPreservedAndReported()
+ {
+ // Arrange -- three copies; a.txt is the keeper, bb.txt and ccc.txt are up for deletion
+ using TempTree tree = new();
+ AbsoluteFilePath keeper = tree.Write("a.txt", "shared");
+ AbsoluteFilePath changed = tree.Write("bb.txt", "shared");
+ AbsoluteFilePath stillDuplicate = tree.Write("ccc.txt", "shared");
+ IReadOnlyList duplicates = Duplicates(FileHasher.HashFiles([keeper, changed, stillDuplicate]));
+
+ // Arrange -- bb.txt is rewritten during the confirmation pause, as a sync client or an
+ // editor autosave would do
+ _ = tree.Write("bb.txt", "no longer the same content");
+
+ // Act
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(duplicates);
+
+ // Assert
+ Assert.IsTrue(TempTree.Exists(changed), "A file that is no longer a duplicate must not be deleted.");
+ Assert.IsTrue(TempTree.Exists(keeper), "The keeper must survive.");
+ Assert.IsFalse(TempTree.Exists(stillDuplicate), "A copy that is still identical should still be deleted.");
+ Assert.AreEqual(1, result.DeletedCount);
+ Assert.ContainsSingle(result.SkippedFiles);
+ Assert.AreEqual(changed, result.SkippedFiles[0].Path);
+ Assert.Contains("changed", result.SkippedFiles[0].Reason);
+ Assert.IsEmpty(result.Errors);
+ }
+
+ ///
+ /// If the copy being kept changed during the pause, deleting the rest would destroy the only
+ /// remaining copies of the grouped content, so the whole group must be left alone.
+ ///
+ [TestMethod]
+ public void AChangedKeeperPreservesTheWholeGroup()
+ {
+ // Arrange
+ using TempTree tree = new();
+ AbsoluteFilePath keeper = tree.Write("a.txt", "shared");
+ AbsoluteFilePath copyOne = tree.Write("bb.txt", "shared");
+ AbsoluteFilePath copyTwo = tree.Write("ccc.txt", "shared");
+ IReadOnlyList duplicates = Duplicates(FileHasher.HashFiles([keeper, copyOne, copyTwo]));
+
+ // Arrange -- the keeper is rewritten during the confirmation pause
+ _ = tree.Write("a.txt", "the keeper was overwritten");
+
+ // Act
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(duplicates);
+
+ // Assert
+ Assert.AreEqual(0, result.DeletedCount);
+ Assert.AreEqual(0, result.BytesReclaimed);
+ Assert.HasCount(2, result.SkippedFiles);
+ Assert.IsTrue(TempTree.Exists(copyOne), "The grouped content must survive somewhere.");
+ Assert.IsTrue(TempTree.Exists(copyTwo), "The grouped content must survive somewhere.");
+ }
+
+ ///
+ /// A file that disappears before the delete pass must be reported as skipped rather than
+ /// counted among the deletions, so the reclaimed total stays honest.
+ ///
+ [TestMethod]
+ public void AFileThatVanishedBeforeDeletionIsReportedNotCounted()
+ {
+ // Arrange
+ using TempTree tree = new();
+ AbsoluteFilePath keeper = tree.Write("a.txt", "shared");
+ AbsoluteFilePath vanishing = tree.Write("bb.txt", "shared");
+ IReadOnlyList duplicates = Duplicates(FileHasher.HashFiles([keeper, vanishing]));
+
+ // Arrange -- something else removes it during the confirmation pause
+ File.Delete(vanishing.WeakString);
+
+ // Act
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(duplicates);
+
+ // Assert
+ Assert.AreEqual(0, result.DeletedCount);
+ Assert.AreEqual(0, result.BytesReclaimed);
+ Assert.ContainsSingle(result.SkippedFiles);
+ Assert.AreEqual(vanishing, result.SkippedFiles[0].Path);
+ Assert.IsTrue(TempTree.Exists(keeper), "The keeper must survive.");
+ }
+
+ ///
+ /// A path whose file was replaced by a directory of the same name cannot be re-read, so it
+ /// must be skipped. Attempting the delete instead would raise
+ /// , which the delete path does not catch.
+ ///
+ [TestMethod]
+ public void APathReplacedByADirectoryIsSkippedRatherThanDeleted()
+ {
+ // Arrange
+ using TempTree tree = new();
+ AbsoluteFilePath keeper = tree.Write("a.txt", "shared");
+ AbsoluteFilePath replaced = tree.Write("bb.txt", "shared");
+ IReadOnlyList duplicates = Duplicates(FileHasher.HashFiles([keeper, replaced]));
+
+ // Arrange -- during the confirmation pause the file becomes a directory of the same name
+ File.Delete(replaced.WeakString);
+ _ = Directory.CreateDirectory(replaced.WeakString);
+
+ // Act
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(duplicates);
+
+ // Assert
+ Assert.AreEqual(0, result.DeletedCount);
+ Assert.ContainsSingle(result.SkippedFiles);
+ Assert.AreEqual(replaced, result.SkippedFiles[0].Path);
+ Assert.IsTrue(Directory.Exists(replaced.WeakString), "The directory now at that path must be left alone.");
+ Assert.IsTrue(TempTree.Exists(keeper), "The keeper must survive.");
+ }
+
+ ///
+ /// The re-verification must not degrade into skipping everything: untouched duplicates are
+ /// still deleted, and nothing is reported as preserved.
+ ///
+ [TestMethod]
+ public void UntouchedDuplicatesAreDeletedWithNothingSkipped()
+ {
+ // Arrange
+ using TempTree tree = new();
+ Dictionary hashes = FileHasher.HashFiles(
+ [
+ tree.Write("a.txt", "shared"),
+ tree.Write("bb.txt", "shared"),
+ tree.Write("ccc.txt", "shared"),
+ ]);
+ IReadOnlyList duplicates = Duplicates(hashes);
+
+ // Act
+ DeduplicationResult result = Deduplicator.DeleteDuplicates(duplicates);
+
+ // Assert
+ Assert.AreEqual(2, result.DeletedCount);
+ Assert.IsEmpty(result.SkippedFiles);
+ Assert.IsEmpty(result.Errors);
+ }
+
///
/// Deleting nothing must report nothing, rather than throwing on an empty group list.
///
@@ -245,5 +388,6 @@ public void DeletingAnEmptyGroupListIsANoOp()
Assert.AreEqual(0, result.DeletedCount);
Assert.AreEqual(0, result.BytesReclaimed);
Assert.IsEmpty(result.Errors);
+ Assert.IsEmpty(result.SkippedFiles);
}
}
diff --git a/FileDeduplicator/Deduplicator.cs b/FileDeduplicator/Deduplicator.cs
index ea3164e..3975caa 100644
--- a/FileDeduplicator/Deduplicator.cs
+++ b/FileDeduplicator/Deduplicator.cs
@@ -41,15 +41,30 @@ internal static DeduplicationResult DeleteDuplicates(IReadOnlyList errors = [];
+ List skipped = [];
foreach (DuplicateGroup group in duplicateGroups)
{
AbsoluteFilePath keeper = SelectFileToKeep(group.Files);
- foreach (AbsoluteFilePath file in group.Files)
+ // The grouping was computed before the confirmation prompt, which is an interactive
+ // pause of unbounded length. If the copy being kept no longer holds the group's
+ // content, deleting the others would destroy the only remaining copies of it, so the
+ // whole group is left alone.
+ if (!StillMatchesGroup(keeper, group.Hash, out string? keeperReason))
{
- if (file == keeper)
+ SkipWholeGroup(group, keeper, keeperReason, skipped);
+ continue;
+ }
+
+ foreach (AbsoluteFilePath file in group.Files.Where(f => f != keeper))
+ {
+ // Re-hash immediately before deleting: a file that changed since the scan is no
+ // longer a duplicate, and deleting it would be irreversible loss of content that
+ // exists nowhere else.
+ if (!StillMatchesGroup(file, group.Hash, out string? reason))
{
+ Skip(file, reason, skipped);
continue;
}
@@ -70,7 +85,63 @@ internal static DeduplicationResult DeleteDuplicates(IReadOnlyList
+ /// Preserves every copy in a group, because the copy that would have been kept no longer
+ /// holds the group's content.
+ ///
+ /// The group to leave on disk.
+ /// The copy that would have been kept.
+ /// What the keeper did, phrased to follow its name.
+ /// The list to record the preserved files on.
+ private static void SkipWholeGroup(DuplicateGroup group, AbsoluteFilePath keeper, string? keeperReason, List skipped)
+ {
+ foreach (AbsoluteFilePath file in group.Files.Where(f => f != keeper))
+ {
+ Skip(file, $"the copy being kept ({keeper}) {keeperReason}", skipped);
+ }
+ }
+
+ ///
+ /// Re-reads a file and reports whether it still holds the content its duplicate group was
+ /// formed from.
+ ///
+ /// The file to re-hash.
+ /// The hash the group was formed from.
+ /// When the answer is no, why -- phrased to follow the file's name.
+ /// if the file still hashes to .
+ private static bool StillMatchesGroup(AbsoluteFilePath file, string groupHash, out string? reason)
+ {
+ try
+ {
+ if (string.Equals(FileHasher.ComputeHash(file), groupHash, StringComparison.Ordinal))
+ {
+ reason = null;
+ return true;
+ }
+
+ reason = "changed since it was scanned, so it is no longer a duplicate";
+ return false;
+ }
+ catch (IOException ex)
+ {
+ reason = $"could not be re-read to confirm it is still a duplicate: {ex.Message}";
+ return false;
+ }
+ catch (UnauthorizedAccessException ex)
+ {
+ reason = $"could not be re-read to confirm it is still a duplicate: {ex.Message}";
+ return false;
+ }
+ }
+
+ private static void Skip(AbsoluteFilePath file, string? reason, List skipped)
+ {
+ SkippedFile skip = new(file, reason ?? "could not be confirmed as a duplicate");
+ skipped.Add(skip);
+ Console.WriteLine($" Skipped: {file} -- {skip.Reason}");
}
}
@@ -81,9 +152,24 @@ internal sealed class DuplicateGroup(string hash, List files)
internal long FileSize { get; } = new FileInfo(files[0].WeakString).Length;
}
-internal sealed class DeduplicationResult(int deletedCount, long bytesReclaimed, List errors)
+internal sealed class DeduplicationResult(int deletedCount, long bytesReclaimed, List errors, List skippedFiles)
{
internal int DeletedCount { get; } = deletedCount;
internal long BytesReclaimed { get; } = bytesReclaimed;
internal IReadOnlyList Errors { get; } = errors;
+
+ ///
+ /// Gets the files that were proposed for deletion but left on disk because they could no
+ /// longer be confirmed as duplicates.
+ ///
+ internal IReadOnlyList SkippedFiles { get; } = skippedFiles;
+}
+
+///
+/// A file that was preserved instead of deleted, and why.
+///
+internal sealed class SkippedFile(AbsoluteFilePath path, string reason)
+{
+ internal AbsoluteFilePath Path { get; } = path;
+ internal string Reason { get; } = reason;
}
diff --git a/FileDeduplicator/Verbs/Deduplicate.cs b/FileDeduplicator/Verbs/Deduplicate.cs
index 5910c43..de3e770 100644
--- a/FileDeduplicator/Verbs/Deduplicate.cs
+++ b/FileDeduplicator/Verbs/Deduplicate.cs
@@ -83,6 +83,16 @@ internal override void Run(Deduplicate options)
Console.WriteLine($"Deleted {result.DeletedCount} file(s).");
Console.WriteLine($"Reclaimed {FormatBytes(result.BytesReclaimed)} of disk space.");
+ if (result.SkippedFiles.Count > 0)
+ {
+ Console.WriteLine($"Preserved {result.SkippedFiles.Count} file(s) that could no longer be confirmed as duplicates:");
+
+ foreach (SkippedFile skipped in result.SkippedFiles)
+ {
+ Console.WriteLine($" {skipped.Path} -- {skipped.Reason}");
+ }
+ }
+
if (result.Errors.Count > 0)
{
Console.WriteLine($"Encountered {result.Errors.Count} error(s) during deletion.");