Skip to content

Commit

Permalink
Move downloads out of Temp and clean up after cancel and complete.
Browse files Browse the repository at this point in the history
  • Loading branch information
Al12rs committed Jul 1, 2024
1 parent cdfcc2d commit e07ca73
Show file tree
Hide file tree
Showing 12 changed files with 174 additions and 55 deletions.
33 changes: 28 additions & 5 deletions src/Networking/NexusMods.Networking.Downloaders/DownloadService.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
using System.Collections.ObjectModel;
using System.ComponentModel;
using DynamicData;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using NexusMods.Abstractions.MnemonicDB.Attributes.Extensions;
using NexusMods.Abstractions.NexusWebApi.Types;
using NexusMods.MnemonicDB.Abstractions;
using NexusMods.Networking.Downloaders.Interfaces;
Expand All @@ -14,8 +14,7 @@
using Microsoft.Extensions.Hosting;
using NexusMods.Abstractions.Activities;
using NexusMods.Abstractions.IO;
using NexusMods.MnemonicDB.Abstractions.DatomIterators;
using NexusMods.MnemonicDB.Abstractions.Query;
using NexusMods.Abstractions.Settings;
using NexusMods.Paths;

namespace NexusMods.Networking.Downloaders;
Expand All @@ -25,6 +24,9 @@ public class DownloadService : IDownloadService, IDisposable, IHostedService
{
/// <inheritdoc />
public ReadOnlyObservableCollection<IDownloadTask> Downloads => _downloadsCollection;

/// <inheritdoc />
public AbsolutePath OngoingDownloadsDirectory => _downloadDirectory;
private ReadOnlyObservableCollection<IDownloadTask> _downloadsCollection = ReadOnlyObservableCollection<IDownloadTask>.Empty;

private readonly SourceCache<IDownloadTask, EntityId> _downloads = new(t => t.PersistentState.Id);
Expand All @@ -34,14 +36,26 @@ public class DownloadService : IDownloadService, IDisposable, IHostedService
private bool _isDisposed;
private readonly CompositeDisposable _disposables;
private readonly IFileStore _fileStore;
private AbsolutePath _downloadDirectory;

public DownloadService(ILogger<DownloadService> logger, IServiceProvider provider, IFileStore fileStore, IConnection conn)
public DownloadService(
ILogger<DownloadService> logger,
IServiceProvider provider,
IFileStore fileStore,
IConnection conn,
IFileSystem fs,
ISettingsManager settingsManager)
{
_logger = logger;
_provider = provider;
_conn = conn;
_disposables = new CompositeDisposable();
_fileStore = fileStore;
_downloadDirectory = settingsManager.Get<DownloadSettings>().OngoingDownloadLocation.ToPath(fs);
if (!_downloadDirectory.DirectoryExists())
{
_downloadDirectory.CreateDirectory();
}
}

internal IEnumerable<IDownloadTask> GetItemsToResume()
Expand Down Expand Up @@ -209,8 +223,17 @@ public async Task StopAsync(CancellationToken cancellationToken)
// TODO(Al12rs): should Suspend() instead, but only after moving ongoing dl files outside Temp folder,
// that is otherwise cleaned up on application close, causing exceptions due to file in use,
// on top of discarding all the progress.
.Select(dl => dl.Cancel());
.Select(dl => dl.Suspend());

await Task.WhenAll(suspendingTasks);
}

/// <summary>
/// Set a custom downloadDirectory, for tests only.
/// Directory should already exist.
[EditorBrowsable(EditorBrowsableState.Never)]
internal void SetDownloadDirectory(AbsolutePath path)
{
_downloadDirectory = path;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
using JetBrains.Annotations;
using Microsoft.Extensions.DependencyInjection;
using NexusMods.Abstractions.Settings;
using NexusMods.Paths;

namespace NexusMods.Networking.Downloaders;

/// <summary>
/// Settings for downloads.
/// </summary>
[PublicAPI]
public class DownloadSettings : ISettings
{

/// <summary>
/// Base directory where files generated during ongoing download operations are located.
/// </summary>
/// <remarks>
/// Should not be placed in Temp or similar directories,
/// as download files may need to persist across application and system restarts.
/// </remarks>
public ConfigurablePath OngoingDownloadLocation { get; set; }


/// <inheritdoc/>
public static ISettingsBuilder Configure(ISettingsBuilder settingsBuilder)
{
return settingsBuilder
.ConfigureDefault(CreateDefault)
.ConfigureStorageBackend<DownloadSettings>(builder => builder.UseJson());
}

/// <summary>
/// Create default value.
/// </summary>
public static DownloadSettings CreateDefault(IServiceProvider serviceProvider)
{
var os = serviceProvider.GetRequiredService<IFileSystem>().OS;

return new DownloadSettings
{
OngoingDownloadLocation = GetStandardDownloadPath(os),
};
}

private static ConfigurablePath GetStandardDownloadPath(IOSInformation os)
{
var basePath = os.MatchPlatform(
onWindows: () => KnownPath.LocalApplicationDataDirectory,
onLinux: () => KnownPath.XDG_DATA_HOME,
onOSX: () => KnownPath.LocalApplicationDataDirectory
);
// NOTE: OSX ".App" is apparently special, using _ instead of . to prevent weirdness
var baseDirectoryName = os.IsOSX ? "NexusMods_App/" : "NexusMods.App/";
var downloadsSubPath = baseDirectoryName + "Downloads/Ongoing";

return new ConfigurablePath(basePath, downloadsSubPath);
}

/// <summary>
/// Absolute path to the standard downloads' folder.
/// </summary>
public static AbsolutePath GetStandardDownloadsFolder(IFileSystem fs)
{
return GetStandardDownloadPath(fs.OS).ToPath(fs);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ public interface IDownloadService
/// </summary>
ReadOnlyObservableCollection<IDownloadTask> Downloads { get; }

/// <summary>
/// The base directory for ongoing downloads
/// </summary>
AbsolutePath OngoingDownloadsDirectory { get; }

/// <summary>
/// Adds a task that will download from a NXM link.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ public interface IDownloadTask
DownloaderState.ReadOnly PersistentState { get; }

/// <summary>
/// The download location of the task.
/// Path of the ongoing download file
/// </summary>
public AbsolutePath DownloadLocation { get; }
public AbsolutePath DownloadPath { get; }

/// <summary>
/// Calculates the download speed of the current job.
Expand Down Expand Up @@ -93,12 +93,7 @@ public enum DownloadTaskStatus : byte
Downloading,

/// <summary>
/// The mod is being archived (and possibly installed) to a loadout.
/// </summary>
Installing,

/// <summary>
/// The task has ran to completion.
/// The task has run to completion.
/// </summary>
Completed,

Expand Down
2 changes: 2 additions & 0 deletions src/Networking/NexusMods.Networking.Downloaders/Services.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.Extensions.DependencyInjection;
using NexusMods.Abstractions.Settings;
using NexusMods.MnemonicDB.Abstractions;
using NexusMods.Networking.Downloaders.Interfaces;
using NexusMods.Networking.Downloaders.Tasks;
Expand All @@ -18,6 +19,7 @@ public static class Services
public static IServiceCollection AddDownloaders(this IServiceCollection services)
{
return services.AddSingleton<DownloadService>()
.AddSettings<DownloadSettings>()
.AddHostedService<DownloadService>(sp=> sp.GetRequiredService<DownloadService>())
.AddSingleton<IDownloadService>(sp=> sp.GetRequiredService<DownloadService>())
.AddTransient<NxmDownloadTask>()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,13 @@
using System.ComponentModel;
using System.Runtime.CompilerServices;
using DynamicData.Kernel;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using NexusMods.Abstractions.Activities;
using NexusMods.Abstractions.FileStore;
using NexusMods.Abstractions.FileStore.Downloads;
using NexusMods.Abstractions.HttpDownloader;
using NexusMods.Abstractions.IO;
using NexusMods.DataModel;
using NexusMods.MnemonicDB.Abstractions;
using NexusMods.Networking.Downloaders.Interfaces;
using NexusMods.Networking.Downloaders.Tasks.State;
using NexusMods.Paths;
using NexusMods.Paths.Utilities;
using ReactiveUI;
using ReactiveUI.Fody.Helpers;

Expand All @@ -30,33 +25,33 @@ public abstract class ADownloadTask : ReactiveObject, IDownloadTask
/// </summary>
protected HttpDownloaderState? TransientState = null!;
protected ILogger<ADownloadTask> Logger;
protected TemporaryFileManager TemporaryFileManager;
protected HttpClient HttpClient;
protected IHttpDownloader HttpDownloader;
protected CancellationTokenSource CancellationTokenSource;
protected TemporaryPath _downloadLocation = default!;
protected AbsolutePath _downloadPath = default!;
protected IFileSystem FileSystem;
protected IFileOriginRegistry FileOriginRegistry;
private DownloaderState.ReadOnly _persistentState;

private IDownloadService _downloadService;

protected ADownloadTask(IServiceProvider provider)
{
Connection = provider.GetRequiredService<IConnection>();
Logger = provider.GetRequiredService<ILogger<ADownloadTask>>();
TemporaryFileManager = provider.GetRequiredService<TemporaryFileManager>();
HttpClient = provider.GetRequiredService<HttpClient>();
HttpDownloader = provider.GetRequiredService<IHttpDownloader>();
CancellationTokenSource = new CancellationTokenSource();
ActivityFactory = provider.GetRequiredService<IActivityFactory>();
FileSystem = provider.GetRequiredService<IFileSystem>();
FileOriginRegistry = provider.GetRequiredService<IFileOriginRegistry>();
_downloadService = provider.GetRequiredService<IDownloadService>();
}

public void Init(DownloaderState.ReadOnly state)
{
PersistentState = state;
Downloaded = state.Downloaded;
_downloadLocation = new TemporaryPath(FileSystem, FileSystem.FromUnsanitizedFullPath(state.DownloadPath), false);
_downloadPath = FileSystem.FromUnsanitizedFullPath(state.DownloadPath);
}

/// <summary>
Expand All @@ -73,13 +68,19 @@ public void RefreshState()
/// </summary>
protected EntityId Create(ITransaction tx)
{
_downloadLocation = TemporaryFileManager.CreateFile();
// Add a subfolder for the download task
var guid = Guid.NewGuid().ToString();
var downloadSubfolder = _downloadService.OngoingDownloadsDirectory.Combine(guid);
downloadSubfolder.CreateDirectory();

_downloadPath = downloadSubfolder.Combine(guid).AppendExtension(KnownExtensions.Tmp);

var state = new DownloaderState.New(tx)
{
FriendlyName = "<Unknown>",
Status = DownloadTaskStatus.Idle,
Downloaded = Size.Zero,
DownloadPath = DownloadLocation.ToString(),
DownloadPath = DownloadPath.ToString(),
};
return state.Id;
}
Expand Down Expand Up @@ -151,7 +152,7 @@ protected async Task MarkComplete()
[Reactive]
public DownloaderState.ReadOnly PersistentState { get; protected set; }

public AbsolutePath DownloadLocation => _downloadLocation;
public AbsolutePath DownloadPath => _downloadPath;


[Reactive] public Bandwidth Bandwidth { get; set; } = Bandwidth.From(0);
Expand All @@ -173,6 +174,10 @@ public async Task Cancel()
try { await CancellationTokenSource.CancelAsync(); }
catch (Exception) { /* ignored */ }
await SetStatus(DownloadTaskStatus.Cancelled);

// Cleanup the download directory (this could actually still be in use by the downloader,
// since CancelAsync is not guaranteed to wait for exception handlers to finish handling the cancellation)
CleanupDownloadFiles();
}

/// <inheritdoc />
Expand All @@ -193,24 +198,39 @@ public async Task Resume()
await SetStatus(DownloadTaskStatus.Downloading);
TransientState = new HttpDownloaderState
{
Activity = ActivityFactory.Create<Size>(IHttpDownloader.Group, "Downloading {FileName}", DownloadLocation),
Activity = ActivityFactory.Create<Size>(IHttpDownloader.Group, "Downloading {FileName}", DownloadPath),
};
_ = StartActivityUpdater();

Logger.LogDebug("Dispatching download task for {Name}", PersistentState.FriendlyName);
await Download(DownloadLocation, CancellationTokenSource.Token);
try
{
await Download(DownloadPath, CancellationTokenSource.Token);
}
catch (OperationCanceledException e)
{
return;
}

UpdateActivity();
await SetStatus(DownloadTaskStatus.Analyzing);
Logger.LogInformation("Finished download of {Name} starting analysis", PersistentState.FriendlyName);
await AnalyzeFile();
await MarkComplete();
CleanupDownloadFiles();
}

// Delete the download task subfolder and all files within it.
private void CleanupDownloadFiles()
{
DownloadPath.Parent.DeleteDirectory(recursive: true);
}

private async Task AnalyzeFile()
{
try
{
await FileOriginRegistry.RegisterDownload(DownloadLocation, PersistentState.Id, PersistentState.FriendlyName);
await FileOriginRegistry.RegisterDownload(DownloadPath, PersistentState.Id, PersistentState.FriendlyName);
}
catch (Exception ex)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,12 +163,12 @@ private async Task<Hash> DownloadWithoutResume(IReadOnlyList<HttpRequestMessage>

private static async Task<Hash> FinalizeDownload(DownloadState state, CancellationToken cancel)
{
var tempPath = state.TempFilePath;
var progressPath = state.ProgressFilePath;

if (state.HasIncompleteChunk) return Hash.Zero;

state.StateFilePath.Delete();
File.Move(tempPath.ToString(), state.Destination.ToString(), true);
File.Move(progressPath.ToString(), state.Destination.ToString(), true);
return await state.Destination.XxHash64Async(token: cancel);
}

Expand All @@ -177,8 +177,8 @@ private static async Task<Hash> FinalizeDownload(DownloadState state, Cancellati
private async Task FileWriterTask(DownloadState state, ChannelReader<WriteOrder> writes,
IActivitySource<Size> job, CancellationToken cancel)
{
var tempPath = state.TempFilePath;
await using var file = tempPath.Open(FileMode.OpenOrCreate, FileAccess.Write);
var progressPath = state.ProgressFilePath;
await using var file = progressPath.Open(FileMode.OpenOrCreate, FileAccess.Write);
file.SetLength((long)(ulong)state.TotalSize);

while (true)
Expand Down Expand Up @@ -471,7 +471,7 @@ private async Task<DownloadState> InitiateState(AbsolutePath destination, Size s
{
DownloadState? state = null;
var stateFilePath = DownloadState.GetStateFilePath(destination);
if (stateFilePath.FileExists && DownloadState.GetTempFilePath(destination).FileExists)
if (stateFilePath.FileExists && DownloadState.GetProgressFilePath(destination).FileExists)
{
_logger.LogInformation("Resuming prior download {FilePath}", destination);
state = await DeserializeDownloadState(stateFilePath, cancel);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ public Size CompletedSize
/// The file where the download is stored while it is in progress
/// </summary>
[JsonIgnore]
public AbsolutePath TempFilePath => GetTempFilePath(Destination);
public AbsolutePath ProgressFilePath => GetProgressFilePath(Destination);

/// <summary>
/// Based on the destination, get the path to the state file
Expand All @@ -83,7 +83,7 @@ public Size CompletedSize
/// </summary>
/// <param name="destination"></param>
/// <returns></returns>
public static AbsolutePath GetTempFilePath(AbsolutePath destination) => destination.ReplaceExtension(new Extension(".downloading"));
public static AbsolutePath GetProgressFilePath(AbsolutePath destination) => destination.ReplaceExtension(new Extension(".downloading"));


/// <summary>
Expand Down
Loading

0 comments on commit e07ca73

Please sign in to comment.