Move downloads out of Temp and clean up after cancel and complete

src/Networking/NexusMods.Networking.Downloaders/DownloadService.cs

@@ -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;
@@ -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;
@@ -25,6 +24,9 @@ public class DownloadService : IDownloadService, IDisposable, IHostedService
 {
     /// <inheritdoc />
     public ReadOnlyObservableCollection<IDownloadTask> Downloads => _downloadsCollection;
+
+    /// <inheritdoc />
+    public AbsolutePath OngoingDownloadsDirectory { get; private set; }
     private ReadOnlyObservableCollection<IDownloadTask> _downloadsCollection = ReadOnlyObservableCollection<IDownloadTask>.Empty;
 
     private readonly SourceCache<IDownloadTask, EntityId> _downloads = new(t => t.PersistentState.Id);
@@ -35,13 +37,24 @@ public class DownloadService : IDownloadService, IDisposable, IHostedService
     private readonly CompositeDisposable _disposables;
     private readonly IFileStore _fileStore;
 
-    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;
+        OngoingDownloadsDirectory = settingsManager.Get<DownloadSettings>().OngoingDownloadLocation.ToPath(fs);
+        if (!OngoingDownloadsDirectory.DirectoryExists())
+        {
+            OngoingDownloadsDirectory.CreateDirectory();
+        }
     }
 
     internal IEnumerable<IDownloadTask> GetItemsToResume()
@@ -206,11 +219,9 @@ public async Task StopAsync(CancellationToken cancellationToken)
     {
         var suspendingTasks = _downloads.Items
             .Where(dl => dl.PersistentState.Status == DownloadTaskStatus.Downloading)
-            // 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);
     }
+
 }

src/Networking/NexusMods.Networking.Downloaders/DownloadSettings.cs

@@ -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 record 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);
+    }
+}

src/Networking/NexusMods.Networking.Downloaders/Interfaces/IDownloadService.cs

@@ -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>

src/Networking/NexusMods.Networking.Downloaders/Interfaces/IDownloadTask.cs

@@ -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.
@@ -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,
 

src/Networking/NexusMods.Networking.Downloaders/Services.cs

@@ -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;
@@ -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>()

src/Networking/NexusMods.Networking.Downloaders/Tasks/ADownloadTask.cs

@@ -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;
 
@@ -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>
@@ -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;
     }
@@ -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);
@@ -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 />
@@ -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)
         {

src/Networking/NexusMods.Networking.HttpDownloader/AdvancedHttpDownloader.cs

@@ -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);
         }
 
@@ -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)
@@ -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);

src/Networking/NexusMods.Networking.HttpDownloader/DTOs/DownloadState.cs

@@ -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
@@ -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>

src/NexusMods.App.UI/Controls/DownloadGrid/Columns/DownloadStatus/DownloadStatusDesignViewModel.cs

@@ -71,7 +71,6 @@ public static string FormatStatus(DownloadTaskStatus state, long totalBytes, lon
             DownloadTaskStatus.Idle => Language.DownloadStatusDesignViewModel_FormatStatus_Queued,
             DownloadTaskStatus.Paused => Language.DownloadStatusDesignViewModel_FormatStatus_Paused,
             DownloadTaskStatus.Downloading => Language.DownloadStatusDesignViewModel_FormatStatus_Downloading,
-            DownloadTaskStatus.Installing => Language.DownloadStatusDesignViewModel_FormatStatus_Installing,
             DownloadTaskStatus.Completed => Language.DownloadStatusDesignViewModel_FormatStatus_Complete,
             DownloadTaskStatus.Analyzing => Language.DownloadStatusDesignViewModel_FormatStatus_Analyzing,
             DownloadTaskStatus.Cancelled => Language.DownloadStatusDesignViewModel_FormatStatus_Cancelled,