diff --git a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs index 1b30efbc7..8ebaef6fb 100644 --- a/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs +++ b/src/NzbDrone.Common.Test/DiskTests/DiskTransferServiceFixture.cs @@ -34,7 +34,7 @@ namespace NzbDrone.Common.Test.DiskTests { MonoOnly(); - Subject.VerificationMode.Should().Be(DiskTransferVerificationMode.Transactional); + Subject.VerificationMode.Should().Be(DiskTransferVerificationMode.TryTransactional); } [Test] @@ -199,9 +199,6 @@ namespace NzbDrone.Common.Test.DiskTests Mocker.GetMock() .Verify(v => v.CopyFile(_sourcePath, _targetPath, false), Times.Once()); - - Mocker.GetMock() - .Verify(v => v.GetFileSize(It.IsAny()), Times.Never()); } [Test] @@ -213,9 +210,6 @@ namespace NzbDrone.Common.Test.DiskTests Mocker.GetMock() .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Once()); - - Mocker.GetMock() - .Verify(v => v.GetFileSize(It.IsAny()), Times.Never()); } [Test] @@ -377,6 +371,52 @@ namespace NzbDrone.Common.Test.DiskTests ExceptionVerification.ExpectedErrors(1); } + [Test] + public void mode_transactional_should_move_and_verify_if_same_folder() + { + Subject.VerificationMode = DiskTransferVerificationMode.Transactional; + + var targetPath = _sourcePath + ".test"; + + var result = Subject.TransferFile(_sourcePath, targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.TryCreateHardLink(_sourcePath, _backupPath), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.CopyFile(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_sourcePath, targetPath, false), Times.Once()); + } + + [Test] + public void mode_trytransactional_should_revert_to_verifyonly_if_hardlink_fails() + { + Subject.VerificationMode = DiskTransferVerificationMode.TryTransactional; + + WithFailedHardlink(); + + Mocker.GetMock() + .Setup(v => v.MoveFile(_sourcePath, _targetPath, false)) + .Callback(() => + { + WithExistingFile(_sourcePath, false); + WithExistingFile(_targetPath, true); + }); + + var result = Subject.TransferFile(_sourcePath, _targetPath, TransferMode.Move); + + Mocker.GetMock() + .Verify(v => v.TryCreateHardLink(_sourcePath, _backupPath), Times.Once()); + + Mocker.GetMock() + .Verify(v => v.CopyFile(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never()); + + Mocker.GetMock() + .Verify(v => v.MoveFile(_sourcePath, _targetPath, false), Times.Once()); + } + [Test] public void mode_transactional_should_delete_old_backup_on_move() { diff --git a/src/NzbDrone.Common/Disk/DiskTransferService.cs b/src/NzbDrone.Common/Disk/DiskTransferService.cs index 130bed7f5..d670b5fdb 100644 --- a/src/NzbDrone.Common/Disk/DiskTransferService.cs +++ b/src/NzbDrone.Common/Disk/DiskTransferService.cs @@ -21,6 +21,7 @@ namespace NzbDrone.Common.Disk { None, VerifyOnly, + TryTransactional, Transactional } @@ -40,7 +41,7 @@ namespace NzbDrone.Common.Disk // TODO: Atm we haven't seen partial transfers on windows so we disable verified transfer. // (If enabled in the future, be sure to check specifically for ReFS, which doesn't support hardlinks.) - VerificationMode = OsInfo.IsWindows ? DiskTransferVerificationMode.VerifyOnly : DiskTransferVerificationMode.Transactional; + VerificationMode = OsInfo.IsWindows ? DiskTransferVerificationMode.VerifyOnly : DiskTransferVerificationMode.TryTransactional; } public TransferMode TransferFolder(string sourcePath, string targetPath, TransferMode mode, bool verified = true) @@ -48,11 +49,6 @@ namespace NzbDrone.Common.Disk Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); - if (VerificationMode != DiskTransferVerificationMode.Transactional) - { - verified = false; - } - if (!_diskProvider.FolderExists(targetPath)) { _diskProvider.CreateFolder(targetPath); @@ -85,12 +81,14 @@ namespace NzbDrone.Common.Disk Ensure.That(sourcePath, () => sourcePath).IsValidPath(); Ensure.That(targetPath, () => targetPath).IsValidPath(); - if (VerificationMode != DiskTransferVerificationMode.Transactional) + if (VerificationMode != DiskTransferVerificationMode.Transactional && VerificationMode != DiskTransferVerificationMode.TryTransactional) { verified = false; } _logger.Debug("{0} [{1}] > [{2}]", mode, sourcePath, targetPath); + + var originalSize = _diskProvider.GetFileSize(sourcePath); if (sourcePath == targetPath) { @@ -127,6 +125,15 @@ namespace NzbDrone.Common.Disk return TransferMode.None; } + if (sourcePath.GetParentPath() == targetPath.GetParentPath()) + { + if (mode.HasFlag(TransferMode.Move)) + { + TryMoveFileVerified(sourcePath, targetPath, originalSize); + return TransferMode.Move; + } + } + if (sourcePath.IsParentPath(targetPath)) { throw new IOException(string.Format("Destination cannot be a child of the source [{0}] => [{1}]", sourcePath, targetPath)); @@ -151,7 +158,7 @@ namespace NzbDrone.Common.Disk { if (mode.HasFlag(TransferMode.Copy)) { - if (TryCopyFile(sourcePath, targetPath)) + if (TryCopyFileTransactional(sourcePath, targetPath, originalSize)) { return TransferMode.Copy; } @@ -159,7 +166,7 @@ namespace NzbDrone.Common.Disk if (mode.HasFlag(TransferMode.Move)) { - if (TryMoveFile(sourcePath, targetPath)) + if (TryMoveFileTransactional(sourcePath, targetPath, originalSize)) { return TransferMode.Move; } @@ -167,50 +174,18 @@ namespace NzbDrone.Common.Disk throw new IOException(string.Format("Failed to completely transfer [{0}] to [{1}], aborting.", sourcePath, targetPath)); } - else if (VerificationMode == DiskTransferVerificationMode.VerifyOnly) + else if (VerificationMode != DiskTransferVerificationMode.None) { - var originalSize = _diskProvider.GetFileSize(sourcePath); - if (mode.HasFlag(TransferMode.Copy)) { - try - { - _diskProvider.CopyFile(sourcePath, targetPath); - - var targetSize = _diskProvider.GetFileSize(targetPath); - if (targetSize != originalSize) - { - throw new IOException(string.Format("File copy incomplete. [{0}] was {1} bytes long instead of {2} bytes.", targetPath, targetSize, originalSize)); - } - - return TransferMode.Copy; - } - catch - { - RollbackCopy(sourcePath, targetPath); - throw; - } + TryCopyFileVerified(sourcePath, targetPath, originalSize); + return TransferMode.Copy; } if (mode.HasFlag(TransferMode.Move)) { - try - { - _diskProvider.MoveFile(sourcePath, targetPath); - - var targetSize = _diskProvider.GetFileSize(targetPath); - if (targetSize != originalSize) - { - throw new IOException(string.Format("File copy incomplete, data loss may have occured. [{0}] was {1} bytes long instead of the expected {2}.", targetPath, targetSize, originalSize)); - } - - return TransferMode.Move; - } - catch - { - RollbackPartialMove(sourcePath, targetPath); - throw; - } + TryMoveFileVerified(sourcePath, targetPath, originalSize); + return TransferMode.Move; } } else @@ -310,10 +285,8 @@ namespace NzbDrone.Common.Disk Thread.Sleep(3000); } - private bool TryCopyFile(string sourcePath, string targetPath) + private bool TryCopyFileTransactional(string sourcePath, string targetPath, long originalSize) { - var originalSize = _diskProvider.GetFileSize(sourcePath); - var tempTargetPath = targetPath + ".partial~"; if (_diskProvider.FileExists(tempTargetPath)) @@ -367,10 +340,8 @@ namespace NzbDrone.Common.Disk return false; } - private bool TryMoveFile(string sourcePath, string targetPath) + private bool TryMoveFileTransactional(string sourcePath, string targetPath, long originalSize) { - var originalSize = _diskProvider.GetFileSize(sourcePath); - var backupPath = sourcePath + ".backup~"; var tempTargetPath = targetPath + ".partial~"; @@ -423,16 +394,63 @@ namespace NzbDrone.Common.Disk } } - _logger.Trace("Hardlink move failed, reverting to copy."); - if (TryCopyFile(sourcePath, targetPath)) + if (VerificationMode == DiskTransferVerificationMode.Transactional) { - _logger.Trace("Copy succeeded, deleting source."); - _diskProvider.DeleteFile(sourcePath); + _logger.Trace("Hardlink move failed, reverting to copy."); + if (TryCopyFileTransactional(sourcePath, targetPath, originalSize)) + { + _logger.Trace("Copy succeeded, deleting source."); + _diskProvider.DeleteFile(sourcePath); + return true; + } + } + else + { + _logger.Trace("Hardlink move failed, reverting to move."); + TryMoveFileVerified(sourcePath, targetPath, originalSize); return true; } - _logger.Trace("Copy failed."); + _logger.Trace("Move failed."); return false; } + + private void TryCopyFileVerified(string sourcePath, string targetPath, long originalSize) + { + try + { + _diskProvider.CopyFile(sourcePath, targetPath); + + var targetSize = _diskProvider.GetFileSize(targetPath); + if (targetSize != originalSize) + { + throw new IOException(string.Format("File copy incomplete. [{0}] was {1} bytes long instead of {2} bytes.", targetPath, targetSize, originalSize)); + } + } + catch + { + RollbackCopy(sourcePath, targetPath); + throw; + } + } + + private void TryMoveFileVerified(string sourcePath, string targetPath, long originalSize) + { + try + { + _diskProvider.MoveFile(sourcePath, targetPath); + + var targetSize = _diskProvider.GetFileSize(targetPath); + if (targetSize != originalSize) + { + 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 + { + RollbackPartialMove(sourcePath, targetPath); + throw; + } + } } }