diff --git a/src/DiffEngineViewer.Tests/MoveSweepTests.cs b/src/DiffEngineViewer.Tests/MoveSweepTests.cs new file mode 100644 index 00000000..3634191a --- /dev/null +++ b/src/DiffEngineViewer.Tests/MoveSweepTests.cs @@ -0,0 +1,94 @@ +/// +/// Accepting a tracked move also removes the directory the received file sat in, when nothing is +/// left in it. That is a tidy-up after the fact, and the move it follows has already happened. +/// +public class MoveSweepTests : + IDisposable +{ + /// + /// The caller reads a throw from here as the move having failed, so it re-tracks the entry - + /// and the retry then fails with file not found, the temp file having been moved by the + /// attempt that "failed". Only IOException was caught, and a directory the parent will not + /// let go of raises UnauthorizedAccessException. + /// + [Test] + [RunOn(TUnit.Core.Enums.OS.Linux | TUnit.Core.Enums.OS.MacOs)] + public async Task A_directory_that_cannot_be_removed_does_not_fail_the_move() + { + // The received file is two deep, and it is the middle directory that cannot be removed. + // Everything the move itself touches - taking the file out of "received", and writing it + // into a directory of its own - stays permitted, so the only thing denied is the tidy-up + var locked = Path.Combine(root, "locked"); + var received = Path.Combine(locked, "received"); + Directory.CreateDirectory(received); + var temp = Path.Combine(received, "sample.received.txt"); + File.WriteAllText(temp, "the snapshot"); + var target = Path.Combine(root, "target"); + Directory.CreateDirectory(target); + var verified = Path.Combine(target, "sample.verified.txt"); + File.SetUnixFileMode(locked, UnixFileMode.UserRead | UnixFileMode.UserExecute); + + ViewerActions.Real.MoveFile(temp, verified); + + await Assert.That(File.Exists(verified)).IsTrue(); + // Still there, which is the point: it could not be removed and that did not matter + await Assert.That(Directory.Exists(received)).IsTrue(); + } + + [Test] + public async Task An_emptied_directory_is_removed() + { + var received = Path.Combine(root, "received"); + Directory.CreateDirectory(received); + var temp = Path.Combine(received, "sample.received.txt"); + File.WriteAllText(temp, "the snapshot"); + var target = Path.Combine(root, "sample.verified.txt"); + + ViewerActions.Real.MoveFile(temp, target); + + await Assert.That(File.Exists(target)).IsTrue(); + await Assert.That(Directory.Exists(received)).IsFalse(); + } + + /// + /// And one still holding something is left alone. + /// + [Test] + public async Task A_directory_with_anything_left_in_it_stays() + { + var received = Path.Combine(root, "received"); + Directory.CreateDirectory(received); + var temp = Path.Combine(received, "sample.received.txt"); + File.WriteAllText(temp, "the snapshot"); + File.WriteAllText(Path.Combine(received, "other.received.txt"), "another"); + var target = Path.Combine(root, "sample.verified.txt"); + + ViewerActions.Real.MoveFile(temp, target); + + await Assert.That(Directory.Exists(received)).IsTrue(); + } + + public MoveSweepTests() + { + root = Path.Combine(Path.GetTempPath(), $"MoveSweepTests_{Guid.NewGuid()}"); + Directory.CreateDirectory(root); + } + + public void Dispose() + { + // Whatever the test denied itself, given back, or the tree cannot be removed here either + if (!OperatingSystem.IsWindows()) + { + foreach (var directory in Directory.EnumerateDirectories(root, "*", SearchOption.AllDirectories)) + { + File.SetUnixFileMode( + directory, + UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + } + + Directory.Delete(root, true); + } + + readonly string root; +} diff --git a/src/DiffEngineViewer/ViewerActions.cs b/src/DiffEngineViewer/ViewerActions.cs index e0f7c5fc..375bb938 100644 --- a/src/DiffEngineViewer/ViewerActions.cs +++ b/src/DiffEngineViewer/ViewerActions.cs @@ -58,22 +58,39 @@ static void Missing(string temp, string target) => static void Move(string temp, string target) { File.Move(temp, target, true); + Sweep(Path.GetDirectoryName(temp)); + } - var directory = Path.GetDirectoryName(temp); - if (directory is null || - Directory.EnumerateFileSystemEntries(directory).Any()) + /// + /// The directory the received file sat in, if it is now empty. + /// + /// Nothing in here can fail the move. It is already done, and the caller reads a throw as the + /// move having failed: the entry goes back on the queue, and the retry fails with file not + /// found on a temp file that is no longer there. Only was covered, + /// so a directory whose parent will not have it removed - or whose permissions are not this + /// process's to change - reported a move that had succeeded as a failure. + /// + /// + static void Sweep(string? directory) + { + if (directory is null) { return; } try { + if (Directory.EnumerateFileSystemEntries(directory).Any()) + { + return; + } + Directory.Delete(directory); } - catch (IOException) + catch (Exception exception) + when (exception is IOException or UnauthorizedAccessException) { - // Raced by something writing into it. The move itself succeeded, which is what the - // caller is reporting on. + // Raced by something writing into it, or not ours to remove. } } }