Skip to content

Added: Replace Library Items Atomically in Place & Other Library Refactorings #3199

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions src/NexusMods.Abstractions.Jobs/IJobMonitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ public interface IJobMonitor
/// Starts a job given the job definition and the code to run as part of the job.
/// </summary>
IJobTask<TJobType, TResultType> Begin<TJobType, TResultType>(TJobType job, Func<IJobContext<TJobType>, ValueTask<TResultType>> task)
where TJobType : IJobDefinition<TResultType>
where TJobType : IJobDefinition<TResultType>
where TResultType : notnull;


/// <summary>
/// Starts a job given the job definition.
/// </summary>
IJobTask<TJobType, TResultType> Begin<TJobType, TResultType>(TJobType job)
where TJobType : IJobDefinitionWithStart<TJobType, TResultType>
where TJobType : IJobDefinitionWithStart<TJobType, TResultType>
where TResultType : notnull;

/// <summary>
Expand Down
145 changes: 141 additions & 4 deletions src/NexusMods.Abstractions.Library/ILibraryService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
namespace NexusMods.Abstractions.Library;

/// <summary>
/// Represents the library.
/// Represents the library, this class provides access to various functionalities
/// that are accessible from within a 'library' related view.
/// </summary>
[PublicAPI]
public interface ILibraryService
Expand All @@ -28,30 +29,166 @@ public interface ILibraryService
/// </summary>
IJobTask<IAddLocalFile, LocalFile.ReadOnly> AddLocalFile(AbsolutePath absolutePath);

/// <summary>
/// Returns all loadouts that contain the given library item.
/// </summary>
/// <param name="libraryItem">The item to search for.</param>
/// <remarks>
/// The loadout and linked item to the current item.
/// </remarks>
IEnumerable<(Loadout.ReadOnly loadout, LibraryLinkedLoadoutItem.ReadOnly linkedItem)> LoadoutsWithLibraryItem(LibraryItem.ReadOnly libraryItem);

/// <summary>
/// Adds a library file.
/// </summary>
Task<LibraryFile.New> AddLibraryFile(ITransaction transaction, AbsolutePath source);

/// <summary>
/// Installs a library item into a target loadout.
/// To remove an item, use <see cref="RemoveLibraryItemFromLoadout(NexusMods.Abstractions.Loadouts.LibraryLinkedLoadoutItemId)"/>.
/// </summary>
/// <param name="libraryItem">The item to install.</param>
/// <param name="targetLoadout">The target loadout.</param>
/// <param name="parent">If specified the installed item will be placed in this group, otherwise it will default to the user's local collection</param>
/// <param name="installer">The Library will use this installer to install the item</param>
/// <param name="fallbackInstaller">Fallback installer instead of the default advanced installer</param>
IJobTask<IInstallLoadoutItemJob, LoadoutItemGroup.ReadOnly> InstallItem(
/// <param name="fallbackInstaller">The installer to use if the default installer fails</param>
/// <param name="transaction">The transaction to attach the installation to. Install is only completed when transaction is completed.</param>
/// <remarks>
/// Job returns a result with null <see cref="LoadoutItemGroup.ReadOnly"/> after
/// if supplied an external transaction via <paramref name="transaction"/>,
/// since it is the caller's responsibility to complete that transaction.
/// </remarks>
IJobTask<IInstallLoadoutItemJob, InstallLoadoutItemJobResult> InstallItem(
LibraryItem.ReadOnly libraryItem,
LoadoutId targetLoadout,
Optional<LoadoutItemGroupId> parent = default,
ILibraryItemInstaller? installer = null,
ILibraryItemInstaller? fallbackInstaller = null);
ILibraryItemInstaller? fallbackInstaller = null,
ITransaction? transaction = null);

/// <summary>
/// Removes a number of items from the library.
/// This will automatically unlink the loadouts from the items are part of.
/// </summary>
/// <param name="libraryItems">The items to remove from the library.</param>
/// <param name="gcRunMode">Defines how the garbage collector should be run</param>
Task RemoveItems(IEnumerable<LibraryItem.ReadOnly> libraryItems, GarbageCollectorRunMode gcRunMode = GarbageCollectorRunMode.RunAsynchronously);

/// <summary>
/// Unlinks a single item added by <see cref="InstallItem"/> function call from a loadout.
/// </summary>
/// <param name="itemId">The <see cref="LibraryLinkedLoadoutItem"/> to remove from the loadout.</param>
Task RemoveLibraryItemFromLoadout(LibraryLinkedLoadoutItemId itemId);

/// <summary>
/// Unlinks a number of items added by <see cref="InstallItem"/> function call from a loadout.
/// </summary>
/// <param name="itemIds">The <see cref="LibraryLinkedLoadoutItem"/>s to remove from the loadout.</param>
Task RemoveLibraryItemsFromLoadout(IEnumerable<LibraryLinkedLoadoutItemId> itemIds);

/// <summary>
/// Unlinks a single item added by <see cref="InstallItem"/> function call from a loadout.
/// </summary>
/// <param name="itemId">The <see cref="LibraryLinkedLoadoutItem"/>s to remove from the loadout.</param>
/// <param name="tx">Existing transaction to use</param>
void RemoveLibraryItemFromLoadout(LibraryLinkedLoadoutItemId itemId, ITransaction tx);

/// <summary>
/// Unlinks a number of items added by <see cref="InstallItem"/> function call from a loadout.
/// </summary>
/// <param name="itemIds">The <see cref="LibraryLinkedLoadoutItem"/>s to remove</param>
/// <param name="tx">Existing transaction to use</param>
void RemoveLibraryItemsFromLoadout(IEnumerable<LibraryLinkedLoadoutItemId> itemIds, ITransaction tx);

/// <summary>
/// Removes library items (originally installed via <see cref="InstallItem"/>) from all
/// loadouts using an existing transaction
/// </summary>
/// <param name="libraryItems">The library items to remove from the loadouts</param>
/// <param name="tx">Existing transaction to use</param>
void RemoveLibraryItemsFromAllLoadouts(IEnumerable<LibraryItem.ReadOnly> libraryItems, ITransaction tx);

/// <summary>
/// Removes library items (originally installed via <see cref="InstallItem"/>) from all
/// loadouts with automatic transaction
/// </summary>
/// <param name="libraryItems">The library items to remove from the loadouts</param>
Task RemoveLibraryItemsFromAllLoadouts(IEnumerable<LibraryItem.ReadOnly> libraryItems);

/// <summary>
/// Replaces all occurrences of a library item with a new version in all loadouts
/// </summary>
/// <param name="oldItem">The library item to be replaced</param>
/// <param name="newItem">The replacement library item</param>
/// <param name="options">Options regarding how to replace this library item.</param>
/// <param name="tx">The transaction to use</param>
/// <returns>
/// If an error occurs at any step of the way, this returns a 'fail' enum.
/// </returns>
ValueTask<LibraryItemReplacementResult> ReplaceLibraryItemInAllLoadouts(LibraryItem.ReadOnly oldItem, LibraryItem.ReadOnly newItem, ReplaceLibraryItemOptions options, ITransaction tx);

/// <summary>
/// Replaces all occurrences of a library item with a new version in all loadouts
/// </summary>
/// <param name="replacements">The replacements to perform</param>
/// <param name="options">Options regarding how to replace these library items.</param>
/// <param name="tx">The transaction to add this replace operation to.</param>
/// <returns>
/// If an error occurs at any step of the way, this returns a 'fail' enum.
/// </returns>
ValueTask<LibraryItemReplacementResult> ReplaceLibraryItemsInAllLoadouts(IEnumerable<(LibraryItem.ReadOnly oldItem, LibraryItem.ReadOnly newItem)> replacements, ReplaceLibraryItemsOptions options, ITransaction tx);

/// <summary>
/// Replaces all occurrences of a library item with a new version in all loadouts
/// </summary>
/// <param name="replacements">The replacements to perform</param>
/// <param name="options">Options regarding how to replace this library item.</param>
/// <returns>
/// If an error occurs at any step of the way, this returns a 'fail' enum.
/// </returns>
ValueTask<LibraryItemReplacementResult> ReplaceLibraryItemsInAllLoadouts(IEnumerable<(LibraryItem.ReadOnly oldItem, LibraryItem.ReadOnly newItem)> replacements, ReplaceLibraryItemsOptions options);
}

/// <summary>
/// Represents the result of a <see cref="ILibraryService.ReplaceLibraryItemInAllLoadouts"/> operation.
/// </summary>
public enum LibraryItemReplacementResult
{
/// <summary>
/// The operation was successful.
/// </summary>
Success,

/// <summary>
/// The operation failed (unknown reason).
/// </summary>
FailedUnknownReason,
}

/// <summary>
/// Options for the <see cref="ILibraryService.ReplaceLibraryItemInAllLoadouts(NexusMods.Abstractions.Library.Models.LibraryItem.ReadOnly,NexusMods.Abstractions.Library.Models.LibraryItem.ReadOnly,ReplaceLibraryItemOptions,NexusMods.MnemonicDB.Abstractions.ITransaction)"/>
/// API.
/// </summary>
public struct ReplaceLibraryItemOptions
{
/// <summary>
/// Ignores items in ReadOnly collections such as collections from Nexus Mods.
/// </summary>
public bool IgnoreReadOnlyCollections { get; set; }
}

/// <summary>
/// Options for the 'ReplaceLibraryItemsInAllLoadouts' API.
/// </summary>
public struct ReplaceLibraryItemsOptions
{
/// <summary>
/// Ignores items in ReadOnly collections such as collections from Nexus Mods.
/// </summary>
public bool IgnoreReadOnlyCollections { get; set; }

/// <summary>
/// Gets the <see cref="ReplaceLibraryItemOptions"/> for this <see cref="ReplaceLibraryItemsOptions"/>
/// </summary>
public ReplaceLibraryItemOptions ToReplaceLibraryItemOptions() => new() { IgnoreReadOnlyCollections = IgnoreReadOnlyCollections };
}
Comment on lines +76 to 194
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need all of these methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

The overloads with a tx allow you to do the operations as part of a larger transaction without committing.
The overloads without a create a transaction there and then, and do the changes on the spot.

RemoveLibraryItemsFromLoadout I added because the code for that was duplicated in multiple places throughout the app; so I lifted it out and deduplicated.

Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
using NexusMods.Abstractions.Library.Installers;
using NexusMods.Abstractions.Library.Models;
using NexusMods.Abstractions.Loadouts;
using NexusMods.MnemonicDB.Abstractions;

namespace NexusMods.Abstractions.Library.Jobs;

/// <summary>
/// A job that installs a library item to a loadout
/// </summary>
public interface IInstallLoadoutItemJob : IJobDefinition<LoadoutItemGroup.ReadOnly>
public interface IInstallLoadoutItemJob : IJobDefinition<InstallLoadoutItemJobResult>
{
/// <summary>
/// The library item to install
Expand All @@ -30,3 +31,15 @@ public interface IInstallLoadoutItemJob : IJobDefinition<LoadoutItemGroup.ReadOn
/// </summary>
public ILibraryItemInstaller? Installer { get; }
}

/// <summary>
/// The result of installing a loadout item via the <see cref="IInstallLoadoutItemJob"/>.
/// This struct holds a <see cref="LoadoutItemGroup"/> for the item which was just installed.
///
/// If the value is 'null' then the job was attached to an existing, external transaction
/// to be part of a larger atomic operation.
/// (Done by passing an <see cref="ITransaction"/> transaction to the job.)
/// This is because the value is not yet available; as the transaction
/// needs to be externally committed by the caller.
/// </summary>
public record struct InstallLoadoutItemJobResult(LoadoutItemGroup.ReadOnly? LoadoutItemGroup);
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,13 @@ public partial class LibraryLinkedLoadoutItem : IModelDefinition
/// The linked library item.
/// </summary>
public static readonly ReferenceAttribute<LibraryItem> LibraryItem = new(Namespace, nameof(LibraryItem)) { IsIndexed = true };

public readonly partial struct ReadOnly
{
/// <summary>
/// Tries converting this entity to a <see cref="LoadoutItem"/> entity,
/// if the entity is not a <see cref="LoadoutItem"/> entity, it returns false.
/// </summary>
public LoadoutItem.ReadOnly AsLoadoutItem() => new(Db, Id);
}
}
34 changes: 4 additions & 30 deletions src/NexusMods.App.UI/Pages/Library/LibraryItemRemover.cs
Original file line number Diff line number Diff line change
@@ -1,57 +1,31 @@
using Microsoft.Extensions.DependencyInjection;
using NexusMods.Abstractions.Library;
using NexusMods.Abstractions.Library.Models;
using NexusMods.Abstractions.Loadouts;
using NexusMods.App.UI.Overlays;
using NexusMods.App.UI.Overlays.LibraryDeleteConfirmation;
using NexusMods.MnemonicDB.Abstractions;
using NexusMods.MnemonicDB.Abstractions.IndexSegments;
using NexusMods.MnemonicDB.Abstractions.TxFunctions;
namespace NexusMods.App.UI.Pages.Library;

/// <summary>
/// Utility helper class for removing a set of library items from inside a ViewModel.
/// </summary>
/// <remarks>
/// This is here to help easier migration to new LoadoutItems based library UI
/// when the time comes.
/// This wraps the actual removal from loadouts and user facing UI confirmation
/// into a single operation, so you can call this from anywhere and not have to
/// worry too much about it.
/// </remarks>
public static class LibraryItemRemover
{
public static async Task RemoveAsync(
IConnection conn,
IOverlayController overlayController,
ILibraryService libraryService,
LibraryItem.ReadOnly[] toRemove,
CancellationToken cancellationToken = default)
LibraryItem.ReadOnly[] toRemove)
{
var warnings = LibraryItemDeleteWarningDetector.Process(conn, toRemove);
var alphaWarningViewModel = LibraryItemDeleteConfirmationViewModel.FromWarningDetector(warnings);
alphaWarningViewModel.Controller = overlayController;
var result = await overlayController.EnqueueAndWait(alphaWarningViewModel);
if (!result) return;

if (result)
{
// Note(sewer) Can the person reviewing this code let me know their opinion of
// whether this should be inlined into LibraryService or not?
var loadouts = Loadout.All(conn.Db).ToArray();
using var tx = conn.BeginTransaction();

// Note. A loadout may technically still be updated in the background via the CLI,
// However this is unlikelu, and the possibility of a concurrent update
// is always possible, as long as we show a blocking dialog to the user.
foreach (var itemInLoadout in warnings.ItemsInLoadouts)
{
foreach (var loadout in loadouts)
{
foreach (var loadoutItem in loadout.GetLoadoutItemsByLibraryItem(itemInLoadout))
tx.Delete(loadoutItem, recursive: true);
}
}

await tx.Commit();
await libraryService.RemoveItems(toRemove);
}
}
}
2 changes: 1 addition & 1 deletion src/NexusMods.App.UI/Pages/LibraryPage/LibraryViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ private async ValueTask RemoveSelectedItems(CancellationToken cancellationToken)
{
var db = _connection.Db;
var toRemove = GetSelectedIds().Select(id => LibraryItem.Load(db, id)).ToArray();
await LibraryItemRemover.RemoveAsync(_connection, _serviceProvider.GetRequiredService<IOverlayController>(), _libraryService, toRemove, cancellationToken);
await LibraryItemRemover.RemoveAsync(_connection, _serviceProvider.GetRequiredService<IOverlayController>(), _libraryService, toRemove);
}

private async ValueTask AddFilesFromDisk(IStorageProvider storageProvider, CancellationToken cancellationToken)
Expand Down
16 changes: 5 additions & 11 deletions src/NexusMods.App.UI/Pages/LoadoutPage/LoadoutViewModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@
using System.Reactive.Linq;
using Avalonia.Controls.Models.TreeDataGrid;
using DynamicData;
using DynamicData.Aggregation;
using DynamicData.Kernel;
using Microsoft.Extensions.DependencyInjection;
using NexusMods.Abstractions.Collections;
using NexusMods.Abstractions.Games;
using NexusMods.Abstractions.Library;
using NexusMods.Abstractions.Loadouts;
using NexusMods.Abstractions.Loadouts.Extensions;
using NexusMods.Abstractions.UI;
using NexusMods.App.UI.Controls;
using NexusMods.App.UI.Controls.Navigation;
using NexusMods.App.UI.Controls.Trees;
Expand All @@ -23,7 +22,6 @@
using NexusMods.Icons;
using NexusMods.MnemonicDB.Abstractions;
using NexusMods.MnemonicDB.Abstractions.ElementComparers;
using NexusMods.MnemonicDB.Abstractions.TxFunctions;
using ObservableCollections;
using OneOf;
using R3;
Expand All @@ -42,6 +40,7 @@ public class LoadoutViewModel : APageViewModel<ILoadoutViewModel>, ILoadoutViewM
public ReactiveCommand<Unit> CollectionToggleCommand { get; }

public LoadoutTreeDataGridAdapter Adapter { get; }
public ILibraryService _LibraryService;

[Reactive] public bool IsCollection { get; private set; }
[Reactive] public bool IsCollectionEnabled { get; private set; }
Expand All @@ -68,6 +67,7 @@ public LoadoutViewModel(

Adapter = new LoadoutTreeDataGridAdapter(serviceProvider, loadoutFilter);

_LibraryService = serviceProvider.GetRequiredService<ILibraryService>();
var connection = serviceProvider.GetRequiredService<IConnection>();

if (collectionGroupId.HasValue)
Expand Down Expand Up @@ -180,17 +180,11 @@ public LoadoutViewModel(
.SelectMany(static itemModel => GetLoadoutItemIds(itemModel))
.ToHashSet()
.Where(id => !IsRequired(id, connection))
.Select(x => (LibraryLinkedLoadoutItemId)x.Value)
.ToArray();

if (ids.Length == 0) return;
using var tx = connection.BeginTransaction();

foreach (var id in ids)
{
tx.Delete(id, recursive: true);
}

await tx.Commit();
await _LibraryService.RemoveLibraryItemsFromLoadout(ids);
},
awaitOperation: AwaitOperation.Sequential,
initialCanExecute: false,
Expand Down
3 changes: 2 additions & 1 deletion src/NexusMods.Collections/InstallCollectionDownloadJob.cs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ public static async ValueTask<InstallCollectionDownloadJob> Create(
}

var libraryFile = GetLibraryFile(Item, Connection.Db);
return await LibraryService.InstallItem(
var result = await LibraryService.InstallItem(
libraryFile.AsLibraryItem(),
TargetLoadout,
parent: Group.AsLoadoutItemGroup().LoadoutItemGroupId,
Expand All @@ -123,6 +123,7 @@ public static async ValueTask<InstallCollectionDownloadJob> Create(
// which installs unknown stuff into a "default folder"
fallbackInstaller: FallbackInstaller
);
return result.LoadoutItemGroup!.Value; // You can't attach external transaction in this context, so this is always valid.
}

private async Task<LoadoutItemGroup.ReadOnly> InstallBundledMod(CollectionDownloadBundled.ReadOnly download)
Expand Down
Loading
Loading