Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] Fix several bugs with 'EngineSettings.UsePartialFiles' #608

Merged
merged 2 commits into from
Jan 27, 2023
Merged

[core] Fix several bugs with 'EngineSettings.UsePartialFiles' #608

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f3dece
[core] Fix several bugs with 'EngineSettings.UsePartialFiles'
alanmcgovern Jan 25, 2023
3daf4ef
Fix the old test
alanmcgovern Jan 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion src/MonoTorrent.Client/MonoTorrent.Client/ClientEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,11 @@ public static async Task<ClientEngine> RestoreStateAsync (ReadOnlyMemory<byte> b
TorrentFileInfo torrentFile;
torrentFile = (TorrentFileInfo) manager.Files.Single (t => t.Path == ((BEncodedString) file[nameof (torrentFile.Path)]).Text);
torrentFile.Priority = (Priority) Enum.Parse (typeof (Priority), file[nameof (torrentFile.Priority)].ToString ()!);
torrentFile.FullPath = ((BEncodedString) file[nameof (torrentFile.FullPath)]).Text;
torrentFile.UpdatePaths ((
newPath: ((BEncodedString) file[nameof (torrentFile.FullPath)]).Text,
downloadCompletePath: ((BEncodedString) file[nameof (torrentFile.DownloadCompleteFullPath)]).Text,
downloadIncompletePath: ((BEncodedString) file[nameof (torrentFile.DownloadIncompleteFullPath)]).Text
));
}
} else {
var magnetLink = MagnetLink.Parse (torrent[nameof (manager.MagnetLink)].ToString ()!);
Expand Down Expand Up @@ -127,6 +131,8 @@ public async Task<byte[]> SaveStateAsync ()
dict[nameof (t.Files)] = new BEncodedList (t.Files.Select (file =>
new BEncodedDictionary {
{ nameof(file.FullPath), (BEncodedString) file.FullPath },
{ nameof(file.DownloadCompleteFullPath), (BEncodedString) file.DownloadCompleteFullPath },
{ nameof(file.DownloadIncompleteFullPath), (BEncodedString) file.DownloadIncompleteFullPath },
{ nameof(file.Path), (BEncodedString) file.Path },
{ nameof(file.Priority), (BEncodedString) file.Priority.ToString () },
}
Expand Down
23 changes: 12 additions & 11 deletions src/MonoTorrent.Client/MonoTorrent.Client/Managers/DiskManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,27 +455,28 @@ public async Task FlushAsync (ITorrentManagerInfo manager, int startIndex, int e
}
}

internal Task MoveFileAsync (ITorrentManagerFile file, string newPath)
=> MoveFileAsync ((TorrentFileInfo) file, newPath);
internal Task MoveFileAsync (ITorrentManagerFile file, (string newPath, string downloadComplete, string downloadIncomplete) paths)
=> MoveFileAsync ((TorrentFileInfo) file, paths);

internal Task MoveFileAsync (TorrentFileInfo file, string newPath)
=> MoveFileAsync (file, newPath, false);
internal Task MoveFileAsync (TorrentFileInfo file, (string newPath, string downloadComplete, string downloadIncomplete) paths)
=> MoveFileAsync (file, paths, false);

internal async Task MoveFilesAsync (IList<ITorrentManagerFile> files, string newRoot, bool overwrite)
{
foreach (TorrentFileInfo file in files)
await MoveFileAsync (file, Path.Combine (newRoot, file.Path), overwrite);
foreach (TorrentFileInfo file in files) {
var paths = TorrentFileInfo.GetNewPaths (Path.GetFullPath (Path.Combine (newRoot, file.Path)), Settings.UsePartialFiles, file.Path == file.DownloadCompleteFullPath);
await MoveFileAsync (file, paths, overwrite);
}
}

async Task MoveFileAsync (TorrentFileInfo file, string newPath, bool overwrite)
async Task MoveFileAsync (TorrentFileInfo file, (string newPath, string downloadCompletePath, string downloadIncompletePath) paths, bool overwrite)
{
await IOLoop;

newPath = Path.GetFullPath (newPath);
if (newPath != file.FullPath && await Cache.Writer.ExistsAsync (file)) {
await Cache.Writer.MoveAsync (file, newPath, false);
if (paths.newPath != file.FullPath && await Cache.Writer.ExistsAsync (file)) {
await Cache.Writer.MoveAsync (file, paths.newPath, false);
}
file.FullPath = newPath;
file.UpdatePaths (paths);
}

internal async ReusableTask<bool> ReadAsync (ITorrentManagerInfo manager, BlockInfo request, Memory<byte> buffer)
Expand Down
23 changes: 20 additions & 3 deletions src/MonoTorrent.Client/MonoTorrent.Client/Managers/TorrentFileInfo.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,19 @@

using System;
using System.Linq;
using System.Runtime;

namespace MonoTorrent.Client
{
class TorrentFileInfo : ITorrentManagerFile
{
public static string IncompleteFileSuffix => ".!mt";

public string DownloadCompleteFullPath { get; set; }
public string DownloadCompleteFullPath { get; private set; }

public string DownloadIncompleteFullPath { get; set; }
public string DownloadIncompleteFullPath { get; private set; }

public string FullPath { get; set; }
public string FullPath { get; private set; }

ITorrentFile TorrentFile { get; }

Expand Down Expand Up @@ -109,5 +110,21 @@ internal static string PathAndFileNameEscape (string path)
filename = filename.Replace ($"{illegal}", $"_{Convert.ToString (illegal, 16)}_");
return System.IO.Path.Combine (dir, filename);
}

internal static (string path, string completePath, string incompletePath) GetNewPaths (string newPath, bool usePartialFiles, bool isComplete)
{
var downloadCompleteFullPath = newPath;
var downloadIncompleteFullPath = downloadCompleteFullPath + (usePartialFiles ? TorrentFileInfo.IncompleteFileSuffix : "");
newPath = isComplete ? downloadCompleteFullPath : downloadIncompleteFullPath;

return (newPath, downloadCompleteFullPath, downloadIncompleteFullPath);
}

internal void UpdatePaths ((string newPath, string downloadCompletePath, string downloadIncompletePath) paths)
{
FullPath = paths.newPath;
DownloadCompleteFullPath = paths.downloadCompletePath;
DownloadIncompleteFullPath = paths.downloadIncompletePath;
}
}
}
22 changes: 12 additions & 10 deletions src/MonoTorrent.Client/MonoTorrent.Client/Managers/TorrentManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,8 @@ public async Task MoveFileAsync (ITorrentManagerFile file, string path)
throw new TorrentException ("Cannot move files when the torrent is active");

try {
await Engine!.DiskManager.MoveFileAsync (file, path);
var paths = TorrentFileInfo.GetNewPaths (Path.GetFullPath (path), Engine.Settings.UsePartialFiles, file.Path == file.DownloadCompleteFullPath);
await Engine!.DiskManager.MoveFileAsync (file, paths);
} catch (Exception ex) {
TrySetError (Reason.WriteFailure, ex);
throw;
Expand Down Expand Up @@ -661,8 +662,11 @@ internal void SetMetadata (Torrent torrent)
// All files marked as 'Normal' priority by default so 'PartialProgressSelector'
// should be set to 'true' for each piece as all files are being downloaded.
Files = Torrent.Files.Select (file => {
var downloadCompleteFullPath = Path.Combine (ContainingDirectory, TorrentFileInfo.PathAndFileNameEscape (file.Path));
var downloadIncompleteFullPath = downloadCompleteFullPath + TorrentFileInfo.IncompleteFileSuffix;

// Generate the paths when 'UsePartialFiles' is enabled.
var paths = TorrentFileInfo.GetNewPaths (Path.Combine (ContainingDirectory, TorrentFileInfo.PathAndFileNameEscape (file.Path)), usePartialFiles: true, isComplete: true);
var downloadCompleteFullPath = paths.completePath;
var downloadIncompleteFullPath = paths.incompletePath;

// FIXME: Is this the best place to futz with actually moving files?
if (!Engine!.Settings.UsePartialFiles) {
Expand All @@ -673,10 +677,8 @@ internal void SetMetadata (Torrent torrent)
}

var currentPath = File.Exists (downloadCompleteFullPath) ? downloadCompleteFullPath : downloadIncompleteFullPath;
var torrentFileInfo = new TorrentFileInfo (file, currentPath) {
DownloadCompleteFullPath = downloadCompleteFullPath,
DownloadIncompleteFullPath = downloadIncompleteFullPath
};
var torrentFileInfo = new TorrentFileInfo (file, currentPath);
torrentFileInfo.UpdatePaths ((currentPath, downloadCompleteFullPath, downloadIncompleteFullPath));
if (file.Length == 0)
torrentFileInfo.BitField[0] = true;
return torrentFileInfo;
Expand Down Expand Up @@ -991,7 +993,7 @@ async void InvokePieceHashedAsync ()
internal async ReusableTask UpdateUsePartialFiles (bool usePartialFiles)
{
foreach (TorrentFileInfo file in Files)
file.DownloadIncompleteFullPath = file.DownloadCompleteFullPath + (usePartialFiles ? TorrentFileInfo.IncompleteFileSuffix : "");
file.UpdatePaths ((file.FullPath, file.DownloadCompleteFullPath, file.DownloadCompleteFullPath + (usePartialFiles ? TorrentFileInfo.IncompleteFileSuffix : "")));
await RefreshPartialDownloadFilePaths (0, Files.Count);
}

Expand All @@ -1002,10 +1004,10 @@ internal async ReusableTask RefreshPartialDownloadFilePaths (int fileStartIndex,
for (int i = fileStartIndex; i < fileStartIndex + count; i++) {
if (files[i].BitField.AllTrue && files[i].FullPath != files[i].DownloadCompleteFullPath) {
tasks ??= new List<Task> ();
tasks.Add (Engine!.DiskManager.MoveFileAsync (files[i], files[i].DownloadCompleteFullPath));
tasks.Add (Engine!.DiskManager.MoveFileAsync (files[i], (files[i].DownloadCompleteFullPath, files[i].DownloadCompleteFullPath, files[i].DownloadIncompleteFullPath)));
} else if (!files[i].BitField.AllTrue && files[i].FullPath != files[i].DownloadIncompleteFullPath) {
tasks ??= new List<Task> ();
tasks.Add (Engine!.DiskManager.MoveFileAsync (files[i], files[i].DownloadIncompleteFullPath));
tasks.Add (Engine!.DiskManager.MoveFileAsync (files[i], (files[i].DownloadIncompleteFullPath, files[i].DownloadCompleteFullPath, files[i].DownloadIncompleteFullPath)));
}
}
if (tasks != null)
Expand Down
20 changes: 11 additions & 9 deletions src/MonoTorrent.PieceWriter/MonoTorrent.PieceWriter/DiskWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,17 +142,19 @@ public async ReusableTask MoveAsync (ITorrentManagerFile file, string newPath, b
if (file is null)
throw new ArgumentNullException (nameof (file));

if (Streams.TryGetValue (file, out AllStreams? data)) {
using var releaser = await data.Locker.EnterAsync ();
await CloseAllAsync (data);
if (!Streams.TryGetValue (file, out AllStreams? data))
Streams[file] = data = new AllStreams ();

if (File.Exists (file.FullPath)) {
if (overwrite)
File.Delete (newPath);
using var releaser = await data.Locker.EnterAsync ();
await CloseAllAsync (data).ConfigureAwait (false);

Directory.CreateDirectory (Path.GetDirectoryName (newPath)!);
File.Move (file.FullPath, newPath);
}
await new ThreadSwitcher ();
if (File.Exists (file.FullPath)) {
if (overwrite)
File.Delete (newPath);

Directory.CreateDirectory (Path.GetDirectoryName (newPath)!);
File.Move (file.FullPath, newPath);
}
}

Expand Down
76 changes: 76 additions & 0 deletions src/Tests/Tests.MonoTorrent.Client/MonoTorrent.Client/ClientEngineTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,44 @@ public async Task ContainingDirectory_PathBusting ()
Assert.ThrowsAsync<ArgumentException> (() => rig.Engine.AddAsync (editor.ToTorrent (), "path", new TorrentSettings ()));
}

[Test]
[TestCase (true)]
[TestCase (false)]
public async Task UsePartialFiles_InitiallyOff_ChangeFullPath_ToggleOn (bool createFile)
{
var pieceLength = Constants.BlockSize * 4;
var engine = new ClientEngine (EngineSettingsBuilder.CreateForTests (usePartialFiles: false));
var torrent = TestRig.CreateMultiFileTorrent (TorrentFile.Create (pieceLength, Constants.BlockSize, Constants.BlockSize * 2, Constants.BlockSize * 3), pieceLength, out BEncoding.BEncodedDictionary _);

using var tempDir = TempDir.Create ();
var manager = await engine.AddAsync (torrent, tempDir.Path);
Assert.AreEqual (manager.Files[0].DownloadCompleteFullPath, manager.Files[0].DownloadIncompleteFullPath);

var newPath = Path.GetFullPath (Path.Combine (tempDir.Path, "new_full_path.fake"));

await manager.MoveFileAsync (manager.Files[0], newPath);
Assert.AreEqual (newPath, manager.Files[0].FullPath);
Assert.AreEqual (newPath, manager.Files[0].DownloadCompleteFullPath);
Assert.AreEqual (newPath, manager.Files[0].DownloadIncompleteFullPath);

if (createFile)
File.Create (manager.Files[0].FullPath).Dispose ();

var settings = new EngineSettingsBuilder (engine.Settings) { UsePartialFiles = true }.ToSettings ();
await engine.UpdateSettingsAsync (settings);
Assert.AreEqual (newPath + TorrentFileInfo.IncompleteFileSuffix, manager.Files[0].FullPath);
Assert.AreEqual (newPath, manager.Files[0].DownloadCompleteFullPath);
Assert.AreEqual (newPath + TorrentFileInfo.IncompleteFileSuffix, manager.Files[0].DownloadIncompleteFullPath);

if (createFile) {
Assert.IsFalse (File.Exists (newPath));
Assert.IsTrue (File.Exists (newPath + TorrentFileInfo.IncompleteFileSuffix));
} else {
Assert.IsFalse (File.Exists (newPath));
Assert.IsFalse (File.Exists (newPath + TorrentFileInfo.IncompleteFileSuffix));
}
}

[Test]
public async Task UsePartialFiles_InitiallyOff_ToggleOn ()
{
Expand Down Expand Up @@ -221,6 +259,44 @@ public async Task UsePartialFiles_InitiallyOn_ToggleOff ()
Assert.AreEqual (manager.Files[0].DownloadCompleteFullPath, manager.Files[0].DownloadIncompleteFullPath);
}

[Test]
[TestCase (true)]
[TestCase (false)]
public async Task UsePartialFiles_InitiallyOn_ChangeFullPath_ToggleOff (bool createFile)
{
var pieceLength = Constants.BlockSize * 4;
var engine = new ClientEngine (EngineSettingsBuilder.CreateForTests (usePartialFiles: true));
var torrent = TestRig.CreateMultiFileTorrent (TorrentFile.Create (pieceLength, Constants.BlockSize, Constants.BlockSize * 2, Constants.BlockSize * 3), pieceLength, out BEncoding.BEncodedDictionary _);

using var tempDir = TempDir.Create ();
var manager = await engine.AddAsync (torrent, tempDir.Path);
Assert.AreNotEqual (manager.Files[0].DownloadCompleteFullPath, manager.Files[0].DownloadIncompleteFullPath);

var newPath = Path.GetFullPath (Path.Combine (tempDir.Path, "new_full_path.fake"));

await manager.MoveFileAsync (manager.Files[0], newPath);
Assert.AreEqual (newPath + TorrentFileInfo.IncompleteFileSuffix, manager.Files[0].FullPath);
Assert.AreEqual (newPath, manager.Files[0].DownloadCompleteFullPath);
Assert.AreEqual (newPath + TorrentFileInfo.IncompleteFileSuffix, manager.Files[0].DownloadIncompleteFullPath);

if (createFile)
File.Create (manager.Files[0].FullPath).Dispose ();

var settings = new EngineSettingsBuilder (engine.Settings) { UsePartialFiles = false }.ToSettings ();
await engine.UpdateSettingsAsync (settings);
Assert.AreEqual (newPath, manager.Files[0].FullPath);
Assert.AreEqual (newPath, manager.Files[0].DownloadCompleteFullPath);
Assert.AreEqual (newPath, manager.Files[0].DownloadIncompleteFullPath);

if (createFile) {
Assert.IsFalse (File.Exists (newPath + TorrentFileInfo.IncompleteFileSuffix));
Assert.IsTrue (File.Exists (newPath));
} else {
Assert.IsFalse (File.Exists (newPath));
Assert.IsFalse (File.Exists (newPath + TorrentFileInfo.IncompleteFileSuffix));
}
}

[Test]
public void DownloadMetadata_Cancelled ()
{
Expand Down
2 changes: 1 addition & 1 deletion src/Tests/Tests.MonoTorrent.Client/MonoTorrent.Client/DiskManagerExceptionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ public void CloseFail ()
public void MoveFileFail ()
{
writer.move = true;
Assert.ThrowsAsync<Exception> (() => diskManager.MoveFileAsync ((TorrentFileInfo) data.Files[0], "root"));
Assert.ThrowsAsync<Exception> (() => diskManager.MoveFileAsync ((TorrentFileInfo) data.Files[0], ("root", "bar", "baz")));
}

[Test]
Expand Down
11 changes: 7 additions & 4 deletions src/Tests/Tests.MonoTorrent.Client/MonoTorrent.Client/DiskManagerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,8 +338,11 @@ public async Task MoveFile_ConvertsToFullPath ()
var file = TorrentFileInfo.Create (Constants.BlockSize, 123456).Single ();
Assert.IsFalse (File.Exists (file.FullPath));

await manager.MoveFileAsync (file, "NewPath");
Assert.AreEqual (Path.GetFullPath ("NewPath"), file.FullPath);
var newFullPath = Path.GetFullPath ("NewFullPath");
await manager.MoveFileAsync (file, (newFullPath, newFullPath, newFullPath));
Assert.AreEqual (newFullPath, file.FullPath);
Assert.AreEqual (newFullPath, file.DownloadCompleteFullPath);
Assert.AreEqual (newFullPath, file.DownloadIncompleteFullPath);
Assert.IsFalse (File.Exists (file.FullPath));
}

Expand All @@ -353,7 +356,7 @@ public async Task MoveFile_SamePath ()
using var writer = new TestPieceWriter ();
using var manager = new DiskManager (new EngineSettings (), Factories.Default, writer);

await manager.MoveFileAsync (file, file.FullPath);
await manager.MoveFileAsync (file, (file.FullPath, file.FullPath, file.FullPath + TorrentFileInfo.IncompleteFileSuffix));
Assert.IsTrue (File.Exists (file.FullPath));
}

Expand All @@ -368,7 +371,7 @@ public async Task MoveFile_TargetDirectoryDoesNotExist ()
using var manager = new DiskManager (new EngineSettings (), Factories.Default, writer);

var fullPath = Path.Combine (tmp.Path, "New", "Path", "file.txt");
await manager.MoveFileAsync (file, fullPath);
await manager.MoveFileAsync (file, (fullPath, fullPath, fullPath + TorrentFileInfo.IncompleteFileSuffix));
Assert.AreEqual (fullPath, file.FullPath);
}

Expand Down