[core] Fix some issues with TorrentManager.Move*Async

Moving a single file now works the same as moving all files. Destination directory is created (if needed), and files which do not exist are handled correctly.
alanmcgovern · May 4, 2021 · b28da78 · b28da78
1 parent 17d6eec
commit b28da78
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 16 deletions.
diff --git a/src/MonoTorrent.Tests/Client/DiskManagerExceptionTests.cs b/src/MonoTorrent.Tests/Client/DiskManagerExceptionTests.cs
@@ -162,7 +162,7 @@ public void MoveFileFail ()
         public void MoveFilesFail ()
         {
             writer.move = true;
-            Assert.ThrowsAsync<Exception> (() => diskManager.MoveFilesAsync (data, "root", true));
+            Assert.ThrowsAsync<Exception> (() => diskManager.MoveFilesAsync (data.Files, "root", true));
         }
 
         [Test]

diff --git a/src/MonoTorrent.Tests/Client/DiskManagerTests.cs b/src/MonoTorrent.Tests/Client/DiskManagerTests.cs
@@ -29,6 +29,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.IO;
 using System.Linq;
 using System.Security.Cryptography;
 using System.Threading.Tasks;
@@ -278,6 +279,104 @@ public async Task ExceedWriteRate ()
             }
         }
 
+        [Test]
+        public async Task MoveFile_ConvertsToFullPath()
+        {
+
+            using var writer = new DiskWriter ((file, access) => null);
+            using var manager = new DiskManager (new EngineSettings (), writer);
+
+            var file = TorrentFileInfo.Create (Piece.BlockSize, 123456).Single ();
+            Assert.IsFalse (File.Exists (file.FullPath));
+
+            await manager.MoveFileAsync (file, "NewPath");
+            Assert.AreEqual (Path.GetFullPath ("NewPath"), file.FullPath);
+            Assert.IsFalse (File.Exists (file.FullPath));
+        }
+
+        [Test]
+        public async Task MoveFile_SamePath ()
+        {
+            using var tmp = TempDir.Create ();
+            var file = TorrentFileInfo.Create (Piece.BlockSize, ("file.txt", 123456, Path.Combine (tmp.Path, "orig.txt"))).Single ();
+            File.OpenWrite (file.FullPath).Close ();
+
+            using var writer = new DiskWriter ((file, access) => null);
+            using var manager = new DiskManager (new EngineSettings (), writer);
+
+            await manager.MoveFileAsync (file, file.FullPath);
+            Assert.IsTrue (File.Exists (file.FullPath));
+        }
+
+        [Test]
+        public async Task MoveFile_TargetDirectoryDoesNotExist ()
+        {
+            using var tmp = TempDir.Create ();
+            var file = TorrentFileInfo.Create (Piece.BlockSize, ("file.txt", 123456, Path.Combine (tmp.Path, "orig.txt"))).Single ();
+            File.OpenWrite (file.FullPath).Close ();
+
+            using var writer = new DiskWriter ((file, access) => null);
+            using var manager = new DiskManager (new EngineSettings (), writer);
+
+            var fullPath = Path.Combine (tmp.Path, "New", "Path", "file.txt");
+            await manager.MoveFileAsync (file, fullPath);
+            Assert.AreEqual (fullPath, file.FullPath);
+            Assert.IsTrue (File.Exists (file.FullPath));
+        }
+
+        [Test]
+        public async Task MoveFiles_DoNotOverwrite ()
+        {
+            using var tmp = TempDir.Create ();
+            using var newRoot = TempDir.Create ();
+
+            var file = TorrentFileInfo.Create (Piece.BlockSize, ("file.txt", 123456, Path.Combine (tmp.Path, "sub_dir", "orig.txt"))).Single ();
+            Directory.CreateDirectory (Path.GetDirectoryName (file.FullPath));
+            File.OpenWrite (file.FullPath).Close ();
+
+            using var writer = new DiskWriter ((file, access) => null);
+            using var manager = new DiskManager (new EngineSettings (), writer);
+
+            await manager.MoveFilesAsync (new[] { file }, newRoot.Path, false);
+            Assert.AreEqual (Path.Combine (newRoot.Path, file.Path), file.FullPath);
+            Assert.IsTrue (File.Exists (file.FullPath));
+        }
+
+        [Test]
+        public async Task MoveFiles_Overwrite ()
+        {
+            using var tmp = TempDir.Create ();
+            using var newRoot = TempDir.Create ();
+
+            var file = TorrentFileInfo.Create (Piece.BlockSize, ("file.txt", 123456, Path.Combine (tmp.Path, "sub_dir", "orig.txt"))).Single ();
+            Directory.CreateDirectory (Path.GetDirectoryName (file.FullPath));
+            File.OpenWrite (file.FullPath).Close ();
+
+            using var writer = new DiskWriter ((file, access) => null);
+            using var manager = new DiskManager (new EngineSettings (), writer);
+
+            await manager.MoveFilesAsync (new[] { file }, newRoot.Path, true);
+            Assert.AreEqual (Path.Combine (newRoot.Path, file.Path), file.FullPath);
+            Assert.IsTrue (File.Exists (file.FullPath));
+        }
+
+        [Test]
+        public async Task MoveFiles_Overwrite_SameDir ()
+        {
+            using var tmp = TempDir.Create ();
+
+            var file = TorrentFileInfo.Create (Piece.BlockSize, (Path.Combine ("sub_dir", "orig.txt"), 123456, Path.Combine (tmp.Path, "sub_dir", "orig.txt"))).Single ();
+            Directory.CreateDirectory (Path.GetDirectoryName (file.FullPath));
+            File.OpenWrite (file.FullPath).Close ();
+
+            using var writer = new DiskWriter ((file, access) => null);
+            using var manager = new DiskManager (new EngineSettings (), writer);
+
+            await manager.MoveFilesAsync (new[] { file }, tmp.Path, true);
+            Assert.AreEqual (Path.Combine (tmp.Path, file.Path), file.FullPath);
+            Assert.IsTrue (File.Exists (file.FullPath));
+        }
+
         [Test]
         public void ReadPastTheEnd ()
         {

diff --git a/src/MonoTorrent/MonoTorrent.Client.PieceWriters/DiskWriter.cs b/src/MonoTorrent/MonoTorrent.Client.PieceWriters/DiskWriter.cs
@@ -95,7 +95,10 @@ public async ReusableTask MoveAsync (ITorrentFileInfo file, string newPath, bool
 
             if (overwrite)
                 File.Delete (newPath);
-            File.Move (file.FullPath, newPath);
+            if (File.Exists (file.FullPath)) {
+                Directory.CreateDirectory (Path.GetDirectoryName (newPath));
+                File.Move (file.FullPath, newPath);
+            }
         }
 
         public async ReusableTask<int> ReadAsync (ITorrentFileInfo file, long offset, byte[] buffer, int bufferOffset, int count)

diff --git a/src/MonoTorrent/MonoTorrent.Client/Managers/DiskManager.cs b/src/MonoTorrent/MonoTorrent.Client/Managers/DiskManager.cs
@@ -342,26 +342,24 @@ public async Task FlushAsync (ITorrentData manager, int startIndex, int endIndex
             }
         }
 
-        internal async Task MoveFileAsync (TorrentFileInfo file, string newPath)
-        {
-            await IOLoop;
+        internal Task MoveFileAsync (TorrentFileInfo file, string newPath)
+            => MoveFileAsync (file, newPath, false);
 
-            newPath = Path.GetFullPath (newPath);
-            await Cache.Writer.MoveAsync (file, newPath, false);
-            file.FullPath = newPath;
+        internal async Task MoveFilesAsync (IList<ITorrentFileInfo> files, string newRoot, bool overwrite)
+        {
+            foreach (TorrentFileInfo file in files)
+                await MoveFileAsync (file, Path.Combine (newRoot, file.Path), overwrite);
         }
 
-        internal async Task MoveFilesAsync (ITorrentData manager, string newRoot, bool overwrite)
+        async Task MoveFileAsync (TorrentFileInfo file, string newPath, bool overwrite)
         {
             await IOLoop;
 
-            foreach (TorrentFileInfo file in manager.Files) {
-                string newPath = Path.Combine (newRoot, file.Path);
-                if (await Cache.Writer.ExistsAsync (file)) {
-                    await Cache.Writer.MoveAsync (file, newPath, overwrite);
-                }
-                file.FullPath = newPath;
+            newPath = Path.GetFullPath (newPath);
+            if (newPath != file.FullPath && await Cache.Writer.ExistsAsync (file)) {
+                await Cache.Writer.MoveAsync (file, newPath, false);
             }
+            file.FullPath = newPath;
         }
 
         internal async ReusableTask<bool> ReadAsync (ITorrentData manager, BlockInfo request, byte[] buffer)

diff --git a/src/MonoTorrent/MonoTorrent.Client/Managers/TorrentManager.cs b/src/MonoTorrent/MonoTorrent.Client/Managers/TorrentManager.cs
@@ -565,6 +565,7 @@ public async Task MoveFileAsync (ITorrentFileInfo file, string path)
                 await Engine.DiskManager.MoveFileAsync ((TorrentFileInfo)file, path);
             } catch (Exception ex) {
                 TrySetError (Reason.WriteFailure, ex);
+                throw;
             }
         }
 
@@ -577,7 +578,7 @@ public async Task MoveFilesAsync (string newRoot, bool overWriteExisting)
                 throw new TorrentException ("Cannot move files when the torrent is active");
 
             try {
-                await Engine.DiskManager.MoveFilesAsync (this, newRoot, overWriteExisting);
+                await Engine.DiskManager.MoveFilesAsync (Files, newRoot, overWriteExisting);
                 SavePath = newRoot;
             } catch (Exception ex) {
                 TrySetError (Reason.WriteFailure, ex);