Remove ipc code #797

halgari · 2023-11-29T16:27:04Z

This PR makes the following changes:

Removes all IPC message and job systems.
Replaces the IPC message queue with a simple Rx based pub/sub system. IMessageConsumer and IMessageProducer remain, but they are backed by Rx
All serialization for the message queue is removed, nothing needs to be serialized so the serialization code is dead
RateLimiting library has been removed, it was mostly unused, and rate limiting should be done by other parts of the app (like the downloader) or by the user (by not installing 1000 mods at one time). We can revisit this in the future.
Ratelimiting jobs have been replaced with Activities
Activities are simple markers for "work being done"
- Completely contained behind abstractions
- Projects only need NexusMods.Abstractions to participate in activites
- The write-side IActivityFactory and the read side IActivityMonitor are separated via interfaces, only import what you need
- All rate limiting code is gone so code can simply say .SetProgress to update the progress
- Typed versions exist for activities that are based on Size or other such units
- The IActivityMontior exposes a IObservable<ActivityReport> that can be used to get updates on the activity
- The activity reports contain throughput information, estimated completion times, etc.
Reworks the app startup for a "Slim mode". In this mode, only the code needed to startup the SingleProcess code and kick off the starting of the main process is loaded in the DI system. Improves startup time a little.
Telemetry only loads in the "normal mode" not "slim". Same for the datamodel, game logic, etc.

Resolves #792

…-code

codecov · 2023-11-29T19:22:20Z

Codecov Report

Merging #797 (0bc6269) into main (75e7bf5) will decrease coverage by 1.12%.
The diff coverage is 68.53%.

❗ Current head 0bc6269 differs from pull request most recent head b1d2214. Consider uploading reports for the commit b1d2214 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #797      +/-   ##
==========================================
- Coverage   63.01%   61.89%   -1.12%     
==========================================
  Files         586      578       -8     
  Lines       18125    17427     -698     
  Branches     1277     1214      -63     
==========================================
- Hits        11422    10787     -635     
+ Misses       6361     6319      -42     
+ Partials      342      321      -21

Flag	Coverage Δ
Linux	`61.18% <68.53%> (-1.11%)`	⬇️
Windows	`61.03% <68.53%> (-1.14%)`	⬇️
clean_environment_tests	`61.88% <68.53%> (-1.13%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...t/NexusMods.FileExtractor/Extractors/IExtractor.cs	`100.00% <100.00%> (ø)`
...hiveManagement/NexusMods.FileExtractor/Services.cs	`100.00% <ø> (ø)`
...D.UI/FooterStepper/FooterStepperDesignViewModel.cs	`0.00% <ø> (ø)`
...s.FOMOD.UI/FooterStepper/FooterStepperViewModel.cs	`0.00% <ø> (ø)`
...ames/NexusMods.Games.FOMOD.UI/GuidedInstallerUi.cs	`16.09% <ø> (ø)`
...ames.FOMOD.UI/Step/GuidedInstallerStepViewModel.cs	`0.00% <ø> (ø)`
.../NexusMods.Games.FOMOD/CoreDelegates/UiDelegate.cs	`5.71% <ø> (ø)`
...etworking.HttpDownloader/AdvancedHttpDownloader.cs	`78.02% <100.00%> (+0.63%)`	⬆️
...sMods.Networking.HttpDownloader/IHttpDownloader.cs	`50.00% <100.00%> (+50.00%)`	⬆️
...ng/NexusMods.Networking.HttpDownloader/Services.cs	`100.00% <100.00%> (ø)`
... and 43 more

... and 8 files with indirect coverage changes

erri120 · 2023-11-30T13:29:36Z

src/NexusMods.Abstractions/DateTime/IDateTimeProvider.cs

+namespace NexusMods.Abstractions.DateTime;
+
+/// <summary>
+/// A provider for the current time, useful for overriding in tests.
+/// </summary>
+public interface IDateTimeProvider
+{
+    /// <summary>
+    /// Gets the current time in UTC.
+    /// </summary>
+    /// <returns></returns>
+    System.DateTime GetCurrentTimeUtc();
+}


What's the benefit of using this over the new TimeProvider API added in .NET 8?

Also, this appears to not be used anywhere, but it probably should be.

github-actions · 2023-11-30T13:41:21Z

This PR conflicts with main. You need to rebase the PR before it can be merged.

src/NexusMods.Abstractions/Activities/IActivitySource.cs

src/NexusMods.Abstractions/Activities/ActivityGroup.cs

src/NexusMods.Abstractions/Activities/ActivityId.cs

src/NexusMods.DataModel/Activities/Activity.cs

src/NexusMods.DataModel/NexusMods.DataModel.csproj

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

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

erri120 · 2023-11-30T14:52:54Z

src/NexusMods.Abstractions/Activities/ActivityReport.cs

+/// <summary>
+/// A typed report for an activity, these are immutable and emitted by the <see cref="IActivityMonitor"/>.
+/// </summary>
+/// <typeparam name="T"></typeparam>
+public class ActivityReport<T> : ActivityReport
+where T : IDivisionOperators<T, double, T>
+{
+    /// <summary>
+    /// The maximum value for the activity, if any.
+    /// </summary>
+    public T? Max { get; init; }
+
+    /// <summary>
+    /// The current value for the activity, if any.
+    /// </summary>
+    public T? Current { get; init; }
+
+    /// <summary>
+    /// If the activity has any progress this will return the amount of progress per second
+    /// </summary>
+    public T? Throughput => Current != null && Elapsed.TotalSeconds > 0 ?
+        Current / Elapsed.TotalSeconds : default;
+}


We can restrict T to be a value type T : struct and use Optional<T> from DynamicData instead of T?.

src/NexusMods.Abstractions/Activities/ActivityReport.cs

erri120

All good besides the T? for value types in the activity types. We should use Optional<T> instead, as it's much easier to use.

# Conflicts: # src/NexusMods.App/Services.cs # src/NexusMods.DataModel/ArchiveInstaller.cs

github-actions · 2023-11-30T16:25:54Z

This PR doesn't conflict with main anymore. It can be merged after all status checks have passed and it has been reviewed.

github-actions · 2023-11-30T16:32:31Z

This PR conflicts with main. You need to rebase the PR before it can be merged.

# Conflicts: # src/NexusMods.App/Program.cs

github-actions · 2023-11-30T16:37:54Z

This PR doesn't conflict with main anymore. It can be merged after all status checks have passed and it has been reviewed.

codecov-commenter · 2023-11-30T16:45:10Z

Codecov Report

Merging #797 (440d581) into main (753a9b1) will decrease coverage by 1.02%.
The diff coverage is 69.79%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #797      +/-   ##
==========================================
- Coverage   62.14%   61.12%   -1.02%     
==========================================
  Files         594      586       -8     
  Lines       18656    17965     -691     
  Branches     1334     1273      -61     
==========================================
- Hits        11593    10981     -612     
+ Misses       6710     6660      -50     
+ Partials      353      324      -29

Flag	Coverage Δ
Linux	`60.43% <69.53%> (-1.06%)`	⬇️
Windows	`60.28% <69.79%> (-1.06%)`	⬇️
clean_environment_tests	`61.10% <69.79%> (-1.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
...t/NexusMods.FileExtractor/Extractors/IExtractor.cs	`100.00% <100.00%> (ø)`
...hiveManagement/NexusMods.FileExtractor/Services.cs	`100.00% <ø> (ø)`
...D.UI/FooterStepper/FooterStepperDesignViewModel.cs	`0.00% <ø> (ø)`
...s.FOMOD.UI/FooterStepper/FooterStepperViewModel.cs	`0.00% <ø> (ø)`
...ames/NexusMods.Games.FOMOD.UI/GuidedInstallerUi.cs	`16.09% <ø> (ø)`
...ames.FOMOD.UI/Step/GuidedInstallerStepViewModel.cs	`0.00% <ø> (ø)`
.../NexusMods.Games.FOMOD/CoreDelegates/UiDelegate.cs	`5.71% <ø> (ø)`
...etworking.HttpDownloader/AdvancedHttpDownloader.cs	`78.02% <100.00%> (+0.63%)`	⬆️
...sMods.Networking.HttpDownloader/IHttpDownloader.cs	`50.00% <100.00%> (+50.00%)`	⬆️
...ng/NexusMods.Networking.HttpDownloader/Services.cs	`100.00% <100.00%> (ø)`
... and 43 more

... and 8 files with indirect coverage changes

halgari added 10 commits November 27, 2023 13:33

WIP removed IPC code and replacing IResource with Activities

e0ae6b0

More changes caused by removing the rate limiter

1b9633b

Remove more dead code

bb850e3

WIP

9d0ee2e

Lots more refactoring

c86a5a6

Latest work, likely will need to revert some of this

359b0b2

Somehow this whole thing builds

5d96334

A few tests pass

89f9dcb

Fix a few broken tests

0083fbc

All the tests pass? What is this madness?

6e2b02e

halgari added the status-dont-merge label Nov 29, 2023

halgari self-assigned this Nov 29, 2023

halgari added 6 commits November 29, 2023 09:27

Merge branch 'main' into remove-ipc-code

37eb1d2

Code cleanup

2d9898d

Latest cleanup

c110a29

Merge remote-tracking branch 'origin/remove-ipc-code' into remove-ipc…

08d2fb6

…-code

Remove unused imports

0bc6269

Make some internal classes internal

a97360a

Add some debugging code, found when I was verifying OAuth still worked

b1d2214

halgari added this to the v0.3 milestone Nov 29, 2023

halgari requested a review from a team November 29, 2023 19:34