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.

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

Downloader rework #481

Merged
merged 16 commits into from
Jul 26, 2023
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
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
4 changes: 2 additions & 2 deletions .github/workflows/clean_environment_tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ on:
branches: [ main ]
paths:
- ".github/workflows/clean_environment_tests.yaml"
- "./src/**"
- "./tests/**"
- "src/**"
- "tests/**"
- "**.props"
- "**.targets"
types: [ opened, synchronize, reopened, ready_for_review ]
Expand Down

Large diffs are not rendered by default.

101 changes: 101 additions & 0 deletions src/Networking/NexusMods.Networking.HttpDownloader/DTOs/ChunkState.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
using System.Text.Json.Serialization;
using JetBrains.Annotations;
using NexusMods.Paths;

namespace NexusMods.Networking.HttpDownloader.DTOs;


/// <summary>
/// Individual state of a chunk of a download
/// </summary>
public class ChunkState
{
/// <summary>
/// Create a new chunk state with the given size and offset
/// </summary>
/// <param name="start"></param>
/// <param name="size"></param>
/// <param name="initChunk"></param>
/// <returns></returns>
public static ChunkState Create(Size start, Size size)
{
return new ChunkState
{
Completed = Size.Zero,
Read = Size.Zero,
Size = size,
Offset = start,
};
}

/// <summary>
/// The offset of the chunk within the output file
/// </summary>
public Size Offset { get; init; } = Size.Zero;

/// <summary>
/// The size of the chunk
/// </summary>
public Size Size { get; set; }

/// <summary>
/// The number of bytes read from the network into the chunk
/// </summary>
[JsonIgnore]
public Size Read { get; set; } = Size.Zero;

/// <summary>
/// The number of bytes written to the output file
/// </summary>
public Size Completed { get; set; } = Size.Zero;

/// <summary>
/// The source of the chunk, (the download information)
/// </summary>
[JsonIgnore]
public Source? Source { get; set; }

[PublicAPI]
public string SourceUrl => Source?.Request?.RequestUri?.AbsoluteUri ?? "No URL";

/// <summary>
/// Token used to cancel the download of the chunk
/// </summary>
[JsonIgnore]
public CancellationTokenSource? Cancel { get; set; }

/// <summary>
/// The time the download of the chunk started
/// </summary>
[JsonIgnore]
public DateTime Started { get; set; }

/// <summary>
/// The number of bytes per second being read from the network
/// </summary>
public Bandwidth BytesPerSecond => Read / (DateTime.Now - Started);

/// <summary>
/// True if the chunk has been read completely
/// </summary>
[JsonIgnore]
public bool IsReadComplete => Read == Size;

/// <summary>
/// True if the chunk has been written completely
/// </summary>
[JsonIgnore]
public bool IsWriteComplete => Completed == Size;

/// <summary>
/// True if the chunk has been read and written completely
/// </summary>
[JsonIgnore]
public bool IsComplete => IsReadComplete && IsWriteComplete;

/// <summary>
/// Returns the number of bytes remaining to be read
/// </summary>
[JsonIgnore]
public Size RemainingToRead => Size - Read;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
using System.Text.Json.Serialization;
using NexusMods.Paths;

namespace NexusMods.Networking.HttpDownloader.DTOs;

/// <summary>
/// Overall state of a download
/// </summary>
public class DownloadState
halgari marked this conversation as resolved.
Show resolved Hide resolved
{

/// <summary>
/// This is incremented by 1 every time the state is loaded and the download is resumed
/// </summary>
public int ResumeCount { get; set; }

/// <summary>
/// Total size of the download
/// </summary>
public Size TotalSize { get; set; }

/// <summary>
/// A list of the chunks that make up the download
/// </summary>
public List<ChunkState> Chunks { get; set; } = new();

/// <summary>
/// The destination of the download on the local file system
/// </summary>
[JsonIgnore]
public AbsolutePath Destination;

/// <summary>
/// The sources of the download (the download information, aka URL mirrors)
/// </summary>
[JsonIgnore]
public Source[]? Sources;

/// <summary>
/// Returns true if any chunk is incomplete
/// </summary>
[JsonIgnore]
public bool HasIncompleteChunk
{
get {
return Chunks.Any(chunk => !chunk.IsComplete);
}
}


/// <summary>
/// The file where the download state is stored
/// </summary>
[JsonIgnore]
public AbsolutePath StateFilePath => GetStateFilePath(Destination);

/// <summary>
/// The file where the download is stored while it is in progress
/// </summary>
[JsonIgnore]
public AbsolutePath TempFilePath => GetTempFilePath(Destination);

/// <summary>
/// Based on the destination, get the path to the state file
/// </summary>
/// <param name="destination"></param>
/// <returns></returns>
public static AbsolutePath GetStateFilePath(AbsolutePath destination) => destination.ReplaceExtension(new Extension(".progress"));

/// <summary>
/// Based on the destination, get the path to the temp file
/// </summary>
/// <param name="destination"></param>
/// <returns></returns>
public static AbsolutePath GetTempFilePath(AbsolutePath destination) => destination.ReplaceExtension(new Extension(".downloading"));


/// <summary>
/// Gets chunks that are not completely downloaded and written to disk
/// </summary>
/// <returns></returns>
[JsonIgnore]
public IEnumerable<ChunkState> UnfinishedChunks => Chunks.Where(chunk => !chunk.IsComplete);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using NexusMods.Paths;

namespace NexusMods.Networking.HttpDownloader.DTOs;

/// <summary>
/// Defines the features of a download server.
/// </summary>
class ServerFeatures
{
/// <summary>
/// Not technically a feature, servers report the size of the download as part of this request
/// </summary>
public Size? DownloadSize { get; init; }

public bool SupportsResume { get; init; }

public static async Task<ServerFeatures> Request(HttpClient client, HttpRequestMessage message, CancellationToken token)
{
message.Method = HttpMethod.Head;
using var response = await client.SendAsync(message, HttpCompletionOption.ResponseHeadersRead, token);
if (!response.IsSuccessStatusCode)
{
throw new HttpRequestException($"Server responded with {response.StatusCode}");
}

var features = new ServerFeatures
{
SupportsResume = response.Headers.AcceptRanges.Contains("bytes"),
DownloadSize = response.Content.Headers.ContentLength == null ?
null :
Size.FromLong(response.Content.Headers.ContentLength.Value)
};
return features;
}
}
22 changes: 22 additions & 0 deletions src/Networking/NexusMods.Networking.HttpDownloader/DTOs/Source.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
namespace NexusMods.Networking.HttpDownloader.DTOs;

/// <summary>
/// Information about a download source (URL mirror)
/// </summary>
public class Source
{
/// <summary>
/// Request for the download
/// </summary>
public HttpRequestMessage? Request { get; init; }

/// <summary>
/// The priority of the source, slower sources should have a lower priority
/// </summary>
public int Priority { get; set; }

public override string ToString()
{
return Request?.RequestUri?.AbsoluteUri ?? "No URL";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
using System.Buffers;
using NexusMods.Paths;

namespace NexusMods.Networking.HttpDownloader.DTOs;

/// <summary>
/// Used to contain information that will be sent to the write queue.
/// </summary>
struct WriteOrder
{
public Size Offset;
public Memory<byte> Data;
public IMemoryOwner<byte> Owner;
public ChunkState Chunk;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
namespace NexusMods.Networking.HttpDownloader;

public static class HttpRequestMessageExtensions
{
/// <summary>
/// Copies the request message so that it can be sent again.
/// </summary>
/// <param name="input"></param>
/// <returns></returns>
public static HttpRequestMessage Copy(this HttpRequestMessage input)
{
var newRequest = new HttpRequestMessage(input.Method, input.RequestUri);
foreach (var option in input.Options)
{
newRequest.Options.Set(new HttpRequestOptionsKey<object?>(option.Key), option.Value);
}

foreach (var header in input.Headers)
{
newRequest.Headers.TryAddWithoutValidation(header.Key, header.Value);
}

return newRequest;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ namespace NexusMods.Networking.HttpDownloader;
[PublicAPI]
public interface IHttpDownloaderSettings
{
/// <summary>
/// Number of chunks to use while downloading. More chunks might mean larger server load.
/// </summary>
public int ChunkCount { get; }

/// <summary>
/// How many blocks can be queued to be written to disk. If this is too low, temporary disk slowdown could lead to
/// worse download speed as the disk throttles the download threads. If this is high, consistently slow disk
Expand All @@ -38,9 +33,6 @@ public interface IHttpDownloaderSettings
[PublicAPI]
public class HttpDownloaderSettings : IHttpDownloaderSettings
{
/// <inheritdoc />
public int ChunkCount { get; set; } = 4;

/// <inheritdoc />
public int WriteQueueLength { get; set; } = 16;

Expand All @@ -55,7 +47,6 @@ public class HttpDownloaderSettings : IHttpDownloaderSettings
/// </summary>
public void Sanitize()
{
ChunkCount = ChunkCount <= 0 ? 4 : ChunkCount;
WriteQueueLength = WriteQueueLength <= 0 ? 16 : WriteQueueLength;
MinCancelAge = MinCancelAge <= 0 ? 500 : MinCancelAge;
CancelSpeedFraction = CancelSpeedFraction <= 0 ? 0.66 : CancelSpeedFraction;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,6 @@ public async Task<Hash> DownloadAsync(IReadOnlyList<HttpRequestMessage> sources,
// Note: If download fails, job will be reported as 'failed', and will not participate in throughput calculations.
state.Jobs.Add(job);

// Integrate local download.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the corresponding removal in AdvancedHttpDownloader break certain tests such as the CLI test for DownloadAndInstallMod (DownloadAndInstallMod -> DownloadModFromUrl).

The problem specifically is that the systems under test will have IHttpDownloader injected into them; and the DI mappings for that interface isn't remappable on the fly. We need:

  • Static URL for these tests.
  • Test data be a mod NMA can currently recognise/install.

There's multiple possible approaches here; I have no particular preference as to which we use.
Here are some.

A. Restore the code for downloading from local file:// URIs.
This is a simple approach, and this code will only ever be used by the tests that require them; while the app gains support for a new URI scheme.

While it's not strictly part of HTTP to support this scheme, it's commonly supported in browsers and web technologies.

B. Host the data at a static location.

Either on the Nexus, the same way we have http://miami.nexus-cdn.com/100M and friends.
Or make a GitHub Release on a dummy repo and upload the files as assets; these have stable URLs.
https://github.com/{{organisation}}/{{repo}}/releases/download/{{tag}}/{{fileName}}.

C. Extend LocalHttpServer to serve files from LocalHost (this machine).

Alternative solution that requires we bootstrap the server in all tests that run on local files.
Not trivial to do in 5 minutes unless you know the APIs, but even if you don't some AI wouldn't hurt there.

I have no preference for any of the choices. Just note that if you go with B/C, or another option, also delete LocalFileDownloader as it will be dead code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the tests are broken, so I'll fix that, I forgot I removed that code. But in general I don't like inserting overrides into classes where DI can do the trick instead.

var hash = await LocalFileDownloader.TryDownloadLocal(job, source, destination);
if (hash != null)
return hash.Value;

var response = await _client.SendAsync(source, HttpCompletionOption.ResponseHeadersRead, token);
if (!response.IsSuccessStatusCode)
{
Expand Down
3 changes: 3 additions & 0 deletions src/NexusMods.DataModel/Loadouts/LoadoutRegistry.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Collections.ObjectModel;
using System.Reactive.Disposables;
using System.Reactive.Linq;
using DynamicData;
Expand Down Expand Up @@ -267,6 +268,8 @@ public IEnumerable<Loadout> AllLoadouts()
.Select(id => Get(id)!);
}



/// <summary>
/// An observable of all the revisions of a given LoadoutId
/// </summary>
Expand Down
Loading
Loading