From c02bfb593019c415271cd2b9fd64ffe8723b238b Mon Sep 17 00:00:00 2001 From: Mark McDowall Date: Mon, 22 May 2023 22:10:43 -0700 Subject: [PATCH] Fixed: Don't rollback file move if destination already exists (cherry picked from commit f05405fe1ce4c78a8c75e27920c863c5b83686bd) (cherry picked from commit 8ab040f612ee04dac4813a08cdeaddd446a64dc9) --- .../DiskTests/DiskTransferServiceFixture.cs | 20 +++++++++++++++++++ src/NzbDrone.Common/Disk/DiskProviderBase.cs | 5 +++++ .../Disk/DiskTransferService.cs | 8 ++++++-- .../Disk/FileAlreadyExistsException.cs | 15 ++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 src/NzbDrone.Common/Disk/FileAlreadyExistsException.cs diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 8f29b3d15..4fca6ca40 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -351,6 +351,26 @@ public void should_rollback_move_on_partial_if_source_remains() .Verify(v => v.DeleteFile(_targetPath), Times.Once()); } + [Test] + public void should_not_rollback_move_on_partial_if_destination_already_exists() + { + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_targetPath, true, 900); + }); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _targetPath, false)) + .Throws(new FileAlreadyExistsException("File already exists", _targetPath)); + + Assert.Throws(() => Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move)); + + Mocker.GetMock() + .Verify(v => v.DeleteFile(_targetPath), Times.Never()); + } + [Test] public void should_log_error_if_rollback_partialmove_fails() { diff --git a/src/NzbDrone.Common/Disk/DiskProviderBase.cs b/src/NzbDrone.Common/Disk/DiskProviderBase.cs index 5e9228236..521cb61df 100644 --- a/src/NzbDrone.Common/Disk/DiskProviderBase.cs +++ b/src/NzbDrone.Common/Disk/DiskProviderBase.cs @@ -264,6 +264,11 @@ public void MoveFolder(string source, string destination, bool overwrite = false protected virtual void MoveFileInternal(string source, string destination) { + if (File.Exists(destination)) + { + throw new FileAlreadyExistsException("File already exists", destination); + } + File.Move(source, destination); } diff --git a/src/NzbDrone.Common/Disk/DiskTransferService.cs b/src/NzbDrone.Common/Disk/DiskTransferService.cs index 83d815348..fb7d93f48 100644 --- a/src/NzbDrone.Common/Disk/DiskTransferService.cs +++ b/src/NzbDrone.Common/Disk/DiskTransferService.cs @@ -500,9 +500,13 @@ private void TryMoveFileVerified(string sourcePath, string targetPath, long orig throw new IOException(string.Format("File move incomplete, data loss may have occurred. [{0}] was {1} bytes long instead of the expected {2}.", targetPath, targetSize, originalSize)); } } - catch + catch (Exception ex) { - RollbackPartialMove(sourcePath, targetPath); + if (ex is not FileAlreadyExistsException) + { + RollbackPartialMove(sourcePath, targetPath); + } + throw; } } diff --git a/src/NzbDrone.Common/Disk/FileAlreadyExistsException.cs b/src/NzbDrone.Common/Disk/FileAlreadyExistsException.cs new file mode 100644 index 000000000..69acb4cd7 --- /dev/null +++ b/src/NzbDrone.Common/Disk/FileAlreadyExistsException.cs @@ -0,0 +1,15 @@ +using System; + +namespace NzbDrone.Common.Disk +{ + public class FileAlreadyExistsException : Exception + { + public string Filename { get; set; } + + public FileAlreadyExistsException(string message, string filename) + : base(message) + { + Filename = filename; + } + } +}