From 12f8150a6b822e4b13d1a004438375be73ac16c0 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sun, 30 Jun 2024 11:23:09 +0100 Subject: [PATCH 01/18] Move install time into mods Global kept for backward compatibility --- src/Core/ModInstaller.cs | 6 +- src/Core/ModManager.cs | 15 ++-- src/Core/State/InternalState.cs | 3 + src/Core/State/JsonFileStatePersistence.cs | 15 +++- tests/Core.Tests/ModManagerIntegrationTest.cs | 90 ++++++++++++------- 5 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 5f8fbf7..f2f85af 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -65,13 +65,8 @@ public void UninstallPackages( if (currentState.Mods.Any()) { eventHandler.UninstallStart(); - var skipCreatedAfter = SkipCreatedAfter(eventHandler, currentState.Time); var uninstallCallbacks = new ProcessingCallbacks { - Accept = gamePath => - { - return skipCreatedAfter(gamePath); - }, After = gamePath => { backupStrategy.RestoreBackup(gamePath.Full); @@ -95,6 +90,7 @@ public void UninstallPackages( installDir, filesLeft, uninstallCallbacks + .AndAccept(SkipCreatedAfter(eventHandler, modInstallationState.Time)) .AndAfter(_ => filesLeft.Remove(_.Relative)) .AndNotAccepted(_ => filesLeft.Remove(_.Relative)) ); diff --git a/src/Core/ModManager.cs b/src/Core/ModManager.cs index cf8fbde..f537f26 100644 --- a/src/Core/ModManager.cs +++ b/src/Core/ModManager.cs @@ -169,6 +169,7 @@ private bool RestoreOriginalState(IEventHandler eventHandler, CancellationToken else { modsLeft[modInstallation.PackageName] = new InternalModInstallationState( + Time: modsLeft[modInstallation.PackageName].Time, FsHash: modInstallation.PackageFsHash, Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, Files: modInstallation.InstalledFiles @@ -182,7 +183,8 @@ private bool RestoreOriginalState(IEventHandler eventHandler, CancellationToken { statePersistence.WriteState(new InternalState( Install: new( - Time: modsLeft.Any() ? previousInstallation.Time : null, + // TODO for state migration + Time: modsLeft.Values.Max(_ => _.Time), Mods: modsLeft ) )); @@ -208,10 +210,11 @@ private void InstallAllModFiles(IEventHandler eventHandler, CancellationToken ca modRepository.ListEnabledMods(), game.InstallationDirectory, modInstallation => installedFilesByMod.Add(modInstallation.PackageName, new( - FsHash: modInstallation.PackageFsHash, - Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, - Files: modInstallation.InstalledFiles - )), + Time: DateTime.UtcNow, + FsHash: modInstallation.PackageFsHash, + Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, + Files: modInstallation.InstalledFiles + )), eventHandler, cancellationToken); } @@ -219,7 +222,7 @@ private void InstallAllModFiles(IEventHandler eventHandler, CancellationToken ca { statePersistence.WriteState(new InternalState( Install: new( - Time: DateTime.UtcNow, + Time: installedFilesByMod.Values.Max(_ => _.Time), Mods: installedFilesByMod ) )); diff --git a/src/Core/State/InternalState.cs b/src/Core/State/InternalState.cs index b11e5ea..9b82701 100644 --- a/src/Core/State/InternalState.cs +++ b/src/Core/State/InternalState.cs @@ -12,6 +12,7 @@ InternalInstallationState Install }; public record InternalInstallationState( + // TODO: needed for backward compatibility DateTime? Time, IReadOnlyDictionary Mods ) @@ -23,6 +24,8 @@ IReadOnlyDictionary Mods }; public record InternalModInstallationState( + // TODO: nullable for backward compatibility + DateTime? Time, // Unknown when partially installed or upgrading from a previous version int? FsHash, // TODO: needed for backward compatibility diff --git a/src/Core/State/JsonFileStatePersistence.cs b/src/Core/State/JsonFileStatePersistence.cs index 7531bc3..3ddb50e 100644 --- a/src/Core/State/JsonFileStatePersistence.cs +++ b/src/Core/State/JsonFileStatePersistence.cs @@ -1,4 +1,5 @@ -using Newtonsoft.Json; +using Core.Utils; +using Newtonsoft.Json; namespace Core.State; @@ -28,7 +29,15 @@ public InternalState ReadState() if (File.Exists(stateFile)) { var contents = File.ReadAllText(stateFile); - return JsonConvert.DeserializeObject(contents); + var state = JsonConvert.DeserializeObject(contents); + // Fill mod install time if not present (for migration) + return state with + { + Install = state.Install with + { + Mods = state.Install.Mods.SelectValues(_ => _ with { Time = _.Time ?? state.Install.Time }) + } + }; } // Fallback to old state when new state is not present @@ -42,7 +51,7 @@ public InternalState ReadState() Time: installTime, Mods: oldState.AsEnumerable().ToDictionary( kv => kv.Key, - kv => new InternalModInstallationState(FsHash: null, Partial: false, Files: kv.Value) + kv => new InternalModInstallationState(Time: installTime, FsHash: null, Partial: false, Files: kv.Value) ) ) ); diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/ModManagerIntegrationTest.cs index 0dd0425..8ec9794 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/ModManagerIntegrationTest.cs @@ -3,6 +3,7 @@ using Core.IO; using Core.Mods; using Core.State; +using Core.Utils; using FluentAssertions; namespace Core.Tests; @@ -14,6 +15,9 @@ public class ModManagerIntegrationTest : AbstractFilesystemTest private const string DirAtRoot = "DirAtRoot"; private const string FileExcludedFromInstall = "Excluded"; + // Randomness ensures that at least some test runs will fail if it's used + private static readonly DateTime? ValueNotUsed = Random.Shared.Next() > 0 ? DateTime.MaxValue : DateTime.MinValue; + private static readonly TimeSpan TimeTolerance = TimeSpan.FromMilliseconds(100); private readonly DirectoryInfo gameDir; @@ -77,16 +81,16 @@ public void Uninstall_DeletesCreatedFilesAndDirectories() persistedState.InitState(new InternalState ( Install: new( - Time: null, + Time: ValueNotUsed, Mods: new Dictionary { ["A"] = new( - FsHash: null, Partial: false, Files: [ + Time: null, FsHash: null, Partial: false, Files: [ Path.Combine("X", "ModAFile"), Path.Combine("Y", "ModAFile") ]), ["B"] = new( - FsHash: null, Partial: false, Files: [ + Time: null, FsHash: null, Partial: false, Files: [ Path.Combine("X", "ModBFile") ]) } @@ -109,11 +113,11 @@ public void Uninstall_SkipsFilesCreatedAfterInstallation() persistedState.InitState(new InternalState ( Install: new( - Time: installationDateTime.ToUniversalTime(), + Time: ValueNotUsed, Mods: new Dictionary { [""] = new( - FsHash: null, Partial: false, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ "ModFile", "RecreatedFile", "AlreadyDeletedFile" @@ -138,20 +142,20 @@ public void Uninstall_StopsAfterAnyError() var installationDateTime = DateTime.Now.AddMinutes(1); persistedState.InitState(new InternalState( Install: new( - Time: installationDateTime.ToUniversalTime(), + Time: ValueNotUsed, Mods: new Dictionary { ["A"] = new( - FsHash: null, Partial: false, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ "ModAFile" ]), ["B"] = new( - FsHash: null, Partial: false, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ "ModBFile1", "ModBFile2" ]), ["C"] = new( - FsHash: null, Partial: false, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ "ModCFile" ]) } @@ -171,11 +175,11 @@ public void Uninstall_StopsAfterAnyError() Mods: new Dictionary { ["B"] = new( - FsHash: null, Partial: true, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: true, Files: [ "ModBFile2" ]), ["C"] = new( - FsHash: null, Partial: false, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ "ModCFile" ]) } @@ -188,11 +192,11 @@ public void Uninstall_RestoresBackups() { persistedState.InitState(new InternalState( Install: new( - Time: null, + Time: ValueNotUsed, Mods: new Dictionary { [""] = new( - FsHash: null, Partial: false, Files: [ + Time: null, FsHash: null, Partial: false, Files: [ "ModFile" ]) } @@ -214,11 +218,11 @@ public void Uninstall_SkipsRestoreIfModFileOverwritten() var installationDateTime = DateTime.Now.AddMinutes(1); persistedState.InitState(new InternalState( Install: new( - Time: installationDateTime.ToUniversalTime(), + Time: ValueNotUsed, Mods: new Dictionary { [""] = new( - FsHash: null, Partial: false, Files: [ + Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ "ModFile" ]) } @@ -267,7 +271,7 @@ public void Install_InstallsContentFromRootDirectories() persistedState.Should().HaveInstalled(new Dictionary { ["Package100"] = new( - FsHash: 100, Partial: false, Files: [ + Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: [ Path.Combine(DirAtRoot, "A"), Path.Combine(DirAtRoot, "B"), "C" @@ -292,7 +296,7 @@ public void Install_SkipsBlacklistedFiles() persistedState.Should().HaveInstalled(new Dictionary { ["Package100"] = new( - FsHash: 100, Partial: false, Files: [ + Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: [ Path.Combine(DirAtRoot, "B") ]), }); @@ -331,8 +335,8 @@ public void Install_GivesPriotiryToFilesLaterInTheModList() File.ReadAllText(GamePath(Path.Combine(DirAtRoot, "A"))).Should().Be("200"); persistedState.Should().HaveInstalled(new Dictionary { - ["Package100"] = new(FsHash: 100, Partial: false, Files: []), - ["Package200"] = new(FsHash: 200, Partial: false, Files: [ + ["Package100"] = new(Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: []), + ["Package200"] = new(Time: DateTime.UtcNow, FsHash: 200, Partial: false, Files: [ Path.Combine(DirAtRoot, "a") ]), }); @@ -352,7 +356,7 @@ public void Install_DuplicatesAreCaseInsensitive() persistedState.Should().HaveInstalled(new Dictionary { - ["Package100"] = new(FsHash: 100, Partial: false, Files: [ + ["Package100"] = new(Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: [ Path.Combine(DirAtRoot, "A") ]), }); @@ -389,11 +393,11 @@ public void Install_StopsAfterAnyError() Mods: new Dictionary { ["Package200"] = new( - FsHash: 200, Partial: true, Files: [ + Time: DateTime.UtcNow, FsHash: 200, Partial: true, Files: [ Path.Combine(DirAtRoot, "B1") ]), ["Package300"] = new( - FsHash: 300, Partial: false, Files: [ + Time: DateTime.UtcNow, FsHash: 300, Partial: false, Files: [ Path.Combine(DirAtRoot, "C") ]), } @@ -571,12 +575,12 @@ private string GamePath(string relativePath) => private class InMemoryStatePersistence : IStatePersistence { // Avoids bootfiles checks on uninstall - private static readonly InternalState SkipBootfilesCheck = new InternalState( + private static readonly InternalState SkipBootfilesCheck = new( Install: new( - Time: null, + Time: ValueNotUsed, Mods: new Dictionary { - ["INIT"] = new(FsHash: null, Partial: false, Files: []), + ["INIT"] = new(Time: null, FsHash: null, Partial: false, Files: []), } )); @@ -603,23 +607,47 @@ internal InMemoryStatePersistenceAssertions(InternalState? savedState) internal void Be(InternalState expected) { - savedState.Should().NotBeNull(); - // Not a great solution, but .NET doesn't natively provide support for mocking the clock - (savedState!.Install.Time ?? DateTime.MinValue).Should().BeCloseTo((expected.Install.Time ?? DateTime.MinValue), TimeTolerance); + var writtenState = WrittenState(); + ValidateDateTime(expected.Install.Time, writtenState.Install.Time); HaveInstalled(expected.Install.Mods); } internal void HaveInstalled(IReadOnlyDictionary expected) { - savedState.Should().NotBeNull(); - savedState?.Install.Mods.Should().BeEquivalentTo(expected); + var writtenState = WrittenState(); + var actualMods = writtenState.Install.Mods; + var expectedMods = expected.Select(mod => + { + var expectedTime = mod.Value.Time; + var actualTime = writtenState.Install.Mods.GetValueOrDefault(mod.Key)?.Time; + if (actualTime is null) + { + return mod; + } + ValidateDateTime(expectedTime, actualTime); + return new KeyValuePair(mod.Key, mod.Value with { Time = actualTime }); + }); + actualMods.Should().BeEquivalentTo(expectedMods); } internal void HaveInstalled(IEnumerable expected) { - savedState?.Install.Mods.Keys.Should().BeEquivalentTo(expected); + var writtenState = WrittenState(); + writtenState.Install.Mods.Keys.Should().BeEquivalentTo(expected); } + private InternalState WrittenState() + { + savedState.Should().NotBeNull("State was not written"); + return savedState!; + } + + /// + /// Not a great solution, but .NET doesn't natively provide support for mocking the clock! + /// + private void ValidateDateTime(DateTime? expected, DateTime? actual) => + (actual ?? DateTime.MinValue).Should().BeCloseTo((expected ?? DateTime.MinValue), TimeTolerance); + internal void BeEmpty() { savedState.Should().BeEquivalentTo(InternalState.Empty()); From 2f28eef810792df8679326a50b8aec8f064fa554 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Mon, 1 Jul 2024 13:00:17 +0100 Subject: [PATCH 02/18] Refactor ModInstaller to provide Apply --- src/Core/IModInstaller.cs | 4 +- src/Core/ModInstaller.cs | 24 +++-- src/Core/ModManager.cs | 98 +++++++------------ src/Core/Utils/DictionaryExtensions.cs | 10 +- .../Utils/DictionaryExtensionsTest.cs | 33 +++++++ .../Core.Tests/Utils/StringExtensionsTest.cs | 2 +- 6 files changed, 101 insertions(+), 70 deletions(-) create mode 100644 tests/Core.Tests/Utils/DictionaryExtensionsTest.cs diff --git a/src/Core/IModInstaller.cs b/src/Core/IModInstaller.cs index c0bf2f6..c7eac9c 100644 --- a/src/Core/IModInstaller.cs +++ b/src/Core/IModInstaller.cs @@ -2,8 +2,8 @@ using Core.State; namespace Core; + public interface IModInstaller { - void InstallPackages(IReadOnlyCollection packages, string installDir, Action afterInstall, ModInstaller.IEventHandler eventHandler, CancellationToken cancellationToken); - void UninstallPackages(InternalInstallationState currentState, string installDir, Action afterUninstall, ModInstaller.IEventHandler eventHandler, CancellationToken cancellationToken); + void Apply(IReadOnlyDictionary currentState, IReadOnlyCollection packages, string installDir, Action afterInstall, ModInstaller.IEventHandler eventHandler, CancellationToken cancellationToken); } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index f2f85af..196b2a9 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -55,14 +55,26 @@ public ModInstaller(IInstallationFactory installationFactory, ITempDir tempDir, backupStrategy = new SuffixBackupStrategy(); } - public void UninstallPackages( - InternalInstallationState currentState, + public void Apply( + IReadOnlyDictionary currentState, + IReadOnlyCollection packages, + string installDir, + Action afterCallback, + IEventHandler eventHandler, + CancellationToken cancellationToken) + { + UninstallPackages(currentState, installDir, afterCallback, eventHandler, cancellationToken); + InstallPackages(packages, installDir, afterCallback, eventHandler, cancellationToken); + } + + private void UninstallPackages( + IReadOnlyDictionary currentState, string installDir, Action afterUninstall, IEventHandler eventHandler, CancellationToken cancellationToken) { - if (currentState.Mods.Any()) + if (currentState.Any()) { eventHandler.UninstallStart(); var uninstallCallbacks = new ProcessingCallbacks @@ -76,7 +88,7 @@ public void UninstallPackages( backupStrategy.DeleteBackup(gamePath.Full); } }; - foreach (var (packageName, modInstallationState) in currentState.Mods) + foreach (var (packageName, modInstallationState) in currentState) { if (cancellationToken.IsCancellationRequested) { @@ -170,7 +182,7 @@ private static void DeleteEmptyDirectories(string dstRootPath, IEnumerable AncestorsUpTo(string root, string path) + private static List AncestorsUpTo(string root, string path) { var ancestors = new List(); for (var dir = Directory.GetParent(path); dir is not null && dir.FullName != root; dir = dir.Parent) @@ -180,7 +192,7 @@ private static IEnumerable AncestorsUpTo(string root, string path) return ancestors; } - public void InstallPackages( + private void InstallPackages( IReadOnlyCollection packages, string installDir, Action afterInstall, diff --git a/src/Core/ModManager.cs b/src/Core/ModManager.cs index f537f26..eec4eb5 100644 --- a/src/Core/ModManager.cs +++ b/src/Core/ModManager.cs @@ -1,8 +1,10 @@ +using System.Globalization; using Core.Games; using Core.IO; using Core.Mods; using Core.State; using Core.Utils; +using Microsoft.VisualBasic; using static Core.IModManager; namespace Core; @@ -138,59 +140,14 @@ public void InstallEnabledMods(IEventHandler eventHandler, CancellationToken can // Clean what left by a previous failed installation tempDir.Cleanup(); - if (RestoreOriginalState(eventHandler, cancellationToken)) - { - InstallAllModFiles(eventHandler, cancellationToken); - } + InstallMods(modRepository.ListEnabledMods(), eventHandler, cancellationToken); tempDir.Cleanup(); } public void UninstallAllMods(IEventHandler eventHandler, CancellationToken cancellationToken = default) { CheckGameNotRunning(); - RestoreOriginalState(eventHandler, cancellationToken); - } - - private bool RestoreOriginalState(IEventHandler eventHandler, CancellationToken cancellationToken) - { - var previousInstallation = statePersistence.ReadState().Install; - var modsLeft = new Dictionary(previousInstallation.Mods); - try - { - modInstaller.UninstallPackages( - previousInstallation, - game.InstallationDirectory, - modInstallation => - { - if (modInstallation.Installed == IInstallation.State.NotInstalled) - { - modsLeft.Remove(modInstallation.PackageName); - } - else - { - modsLeft[modInstallation.PackageName] = new InternalModInstallationState( - Time: modsLeft[modInstallation.PackageName].Time, - FsHash: modInstallation.PackageFsHash, - Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, - Files: modInstallation.InstalledFiles - ); - } - }, - eventHandler, - cancellationToken); - } - finally - { - statePersistence.WriteState(new InternalState( - Install: new( - // TODO for state migration - Time: modsLeft.Values.Max(_ => _.Time), - Mods: modsLeft - ) - )); - } - // Success if everything was uninstalled - return !modsLeft.Any(); + InstallMods(Array.Empty(), eventHandler, cancellationToken); } private void CheckGameNotRunning() @@ -201,29 +158,50 @@ private void CheckGameNotRunning() } } - private void InstallAllModFiles(IEventHandler eventHandler, CancellationToken cancellationToken) + private void InstallMods(IReadOnlyCollection packages, IEventHandler eventHandler, CancellationToken cancellationToken) { - var installedFilesByMod = new Dictionary(); + var previousState = statePersistence.ReadState().Install.Mods; + var currentState = new Dictionary(previousState); try { - modInstaller.InstallPackages( - modRepository.ListEnabledMods(), + modInstaller.Apply( + previousState, + packages, game.InstallationDirectory, - modInstallation => installedFilesByMod.Add(modInstallation.PackageName, new( - Time: DateTime.UtcNow, - FsHash: modInstallation.PackageFsHash, - Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, - Files: modInstallation.InstalledFiles - )), - eventHandler, + modInstallation => + { + switch (modInstallation.Installed) + { + case IInstallation.State.Installed: + case IInstallation.State.PartiallyInstalled: + currentState.Upsert(modInstallation.PackageName, + existing => new( + Time: existing.Time, + FsHash: existing.FsHash, + Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, + Files: modInstallation.InstalledFiles + ), + () => new( + Time: DateTime.UtcNow, + FsHash: modInstallation.PackageFsHash, + Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, + Files: modInstallation.InstalledFiles + )); + break; + case IInstallation.State.NotInstalled: + currentState.Remove(modInstallation.PackageName); + break; + } + }, + eventHandler, cancellationToken); } finally { statePersistence.WriteState(new InternalState( Install: new( - Time: installedFilesByMod.Values.Max(_ => _.Time), - Mods: installedFilesByMod + Time: currentState.Values.Max(_ => _.Time), + Mods: currentState ) )); } diff --git a/src/Core/Utils/DictionaryExtensions.cs b/src/Core/Utils/DictionaryExtensions.cs index aad2d8f..0e9b81c 100644 --- a/src/Core/Utils/DictionaryExtensions.cs +++ b/src/Core/Utils/DictionaryExtensions.cs @@ -1,4 +1,6 @@ -namespace Core.Utils; +using Core.Mods; + +namespace Core.Utils; public static class DictionaryExtensions { @@ -31,4 +33,10 @@ public static IReadOnlyDictionary SelectValues kv.Key, kv => f(kv.Key, kv.Value)); } + + public static void Upsert(this IDictionary dict, TKey key, Func updatedValue, Func insertedValue) + where TKey : notnull + { + dict[key] = dict.TryGetValue(key, out var existing) ? updatedValue(existing) : insertedValue(); + } } diff --git a/tests/Core.Tests/Utils/DictionaryExtensionsTest.cs b/tests/Core.Tests/Utils/DictionaryExtensionsTest.cs new file mode 100644 index 0000000..effcf0e --- /dev/null +++ b/tests/Core.Tests/Utils/DictionaryExtensionsTest.cs @@ -0,0 +1,33 @@ +using Core.Utils; +using FluentAssertions; + +namespace Core.Tests.Utils; + +[UnitTest] +public class DictionaryExtensionsTest +{ + private const int Key = 3; + + [Fact] + public void Upsert_AddsIfNotExisting() + { + var dict = new Dictionary(); + + dict.Upsert(Key, _ => throw new Exception("Should not have been called"), () => 42); + + dict[Key].Should().Be(42); + } + + [Fact] + public void Upsert_UpdatesIfExisting() + { + var dict = new Dictionary() + { + [Key] = 2 + }; + + dict.Upsert(Key, _ => _ + 40, () => throw new Exception("Should not have been called")); + + dict[Key].Should().Be(42); + } +} diff --git a/tests/Core.Tests/Utils/StringExtensionsTest.cs b/tests/Core.Tests/Utils/StringExtensionsTest.cs index c1752a9..0d12920 100644 --- a/tests/Core.Tests/Utils/StringExtensionsTest.cs +++ b/tests/Core.Tests/Utils/StringExtensionsTest.cs @@ -4,7 +4,7 @@ namespace Core.Tests.Utils; [UnitTest] -public class PostProcessorTest +public class StringExtensionsTest { [Fact] public void NormalizeWhitespaces_ReplacesWithSpaces() From 6837f93cc69b83b0260d48a74f7527a301d1705d Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sun, 7 Jul 2024 08:25:47 +0100 Subject: [PATCH 03/18] Minor cleanup --- src/Core/ModInstaller.cs | 19 +++++++++---------- src/Core/ModManager.cs | 2 +- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 196b2a9..c627c89 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -57,14 +57,14 @@ public ModInstaller(IInstallationFactory installationFactory, ITempDir tempDir, public void Apply( IReadOnlyDictionary currentState, - IReadOnlyCollection packages, + IReadOnlyCollection toInstall, string installDir, Action afterCallback, IEventHandler eventHandler, CancellationToken cancellationToken) { UninstallPackages(currentState, installDir, afterCallback, eventHandler, cancellationToken); - InstallPackages(packages, installDir, afterCallback, eventHandler, cancellationToken); + InstallPackages(toInstall, installDir, afterCallback, eventHandler, cancellationToken); } private void UninstallPackages( @@ -139,10 +139,9 @@ private void UninstallPackages( } } - private static void UninstallFiles(string dstPath, IEnumerable files, ProcessingCallbacks callbacks) + private static void UninstallFiles(string dstPath, IReadOnlyCollection filePaths, ProcessingCallbacks callbacks) { - var fileList = files.ToList(); // It must be enumerated twice - foreach (var relativePath in fileList) + foreach (var relativePath in filePaths) { var gamePath = new RootedPath(dstPath, relativePath); @@ -162,10 +161,10 @@ private static void UninstallFiles(string dstPath, IEnumerable files, Pr callbacks.After(gamePath); } - DeleteEmptyDirectories(dstPath, fileList); + DeleteEmptyDirectories(dstPath, filePaths); } - private static void DeleteEmptyDirectories(string dstRootPath, IEnumerable filePaths) + private static void DeleteEmptyDirectories(string dstRootPath, IReadOnlyCollection filePaths) { var dirs = filePaths .Select(file => Path.Combine(dstRootPath, file)) @@ -193,13 +192,13 @@ private static List AncestorsUpTo(string root, string path) } private void InstallPackages( - IReadOnlyCollection packages, + IReadOnlyCollection toInstall, string installDir, Action afterInstall, IEventHandler eventHandler, CancellationToken cancellationToken) { - var modPackages = packages.Where(_ => !BootfilesManager.IsBootFiles(_.PackageName)).Reverse(); + var modPackages = toInstall.Where(_ => !BootfilesManager.IsBootFiles(_.PackageName)).Reverse(); var modConfigs = new List(); var installedFiles = new HashSet(StringComparer.OrdinalIgnoreCase); @@ -247,7 +246,7 @@ private void InstallPackages( if (modConfigs.Where(_ => _.NotEmpty()).Any()) { eventHandler.PostProcessingStart(); - using var bootfilesMod = CreateBootfilesMod(packages, eventHandler); + using var bootfilesMod = CreateBootfilesMod(toInstall, eventHandler); try { bootfilesMod.Install(installDir, installCallbacks); diff --git a/src/Core/ModManager.cs b/src/Core/ModManager.cs index eec4eb5..9f8a9b6 100644 --- a/src/Core/ModManager.cs +++ b/src/Core/ModManager.cs @@ -193,7 +193,7 @@ private void InstallMods(IReadOnlyCollection packages, IEventHandler break; } }, - eventHandler, + eventHandler, cancellationToken); } finally From 5c657c743b5aeac9c50e1bb77996a74cb216697a Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sun, 7 Jul 2024 16:02:58 +0100 Subject: [PATCH 04/18] Removed unused contructor parameter --- src/Core/Init.cs | 2 +- src/Core/ModInstaller.cs | 4 +--- tests/Core.Tests/ModManagerIntegrationTest.cs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Core/Init.cs b/src/Core/Init.cs index a70f9d6..f50489b 100644 --- a/src/Core/Init.cs +++ b/src/Core/Init.cs @@ -18,7 +18,7 @@ public static IModManager CreateModManager(Config config) var modRepository = new ModRepository(modsDir); var installationFactory = new InstallationFactory(game, tempDir, config.ModInstall); var safeFileDelete = new WindowsRecyclingBin(); - var modInstaller = new ModInstaller(installationFactory, tempDir, config.ModInstall); + var modInstaller = new ModInstaller(installationFactory, config.ModInstall); return new ModManager(game, modRepository, modInstaller, statePersistence, safeFileDelete, tempDir); } } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index c627c89..8a84ab2 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -43,14 +43,12 @@ public interface IProgress } private readonly IInstallationFactory installationFactory; - private readonly ITempDir tempDir; private readonly Matcher filesToInstallMatcher; private readonly IBackupStrategy backupStrategy; - public ModInstaller(IInstallationFactory installationFactory, ITempDir tempDir, IConfig config) + public ModInstaller(IInstallationFactory installationFactory, IConfig config) { this.installationFactory = installationFactory; - this.tempDir = tempDir; filesToInstallMatcher = Matchers.ExcludingPatterns(config.ExcludedFromInstall); backupStrategy = new SuffixBackupStrategy(); } diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/ModManagerIntegrationTest.cs index 8ec9794..9f7bd83 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/ModManagerIntegrationTest.cs @@ -54,7 +54,7 @@ public ModManagerIntegrationTest() : base() modManager = new ModManager( gameMock.Object, modRepositoryMock.Object, - new ModInstaller(installationFactory, tempDir, modInstallConfig), + new ModInstaller(installationFactory, modInstallConfig), persistedState, safeFileDeleteMock.Object, tempDir); From a7cf2b272c6735b9a5af175b028a281eeea1012a Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sun, 7 Jul 2024 21:07:15 +0100 Subject: [PATCH 05/18] ModInstaller test harness Fixed missing install end --- src/Core/Init.cs | 6 +- src/Core/ModInstaller.cs | 5 +- .../Core.Tests/ModInstallerIntegrationTest.cs | 223 ++++++++++++++++++ tests/Core.Tests/ModManagerIntegrationTest.cs | 3 +- 4 files changed, 232 insertions(+), 5 deletions(-) create mode 100644 tests/Core.Tests/ModInstallerIntegrationTest.cs diff --git a/src/Core/Init.cs b/src/Core/Init.cs index f50489b..0c590c0 100644 --- a/src/Core/Init.cs +++ b/src/Core/Init.cs @@ -1,4 +1,5 @@ -using Core.Games; +using Core.Backup; +using Core.Games; using Core.IO; using Core.Mods; using Core.State; @@ -18,7 +19,8 @@ public static IModManager CreateModManager(Config config) var modRepository = new ModRepository(modsDir); var installationFactory = new InstallationFactory(game, tempDir, config.ModInstall); var safeFileDelete = new WindowsRecyclingBin(); - var modInstaller = new ModInstaller(installationFactory, config.ModInstall); + var backupStrategy = new SuffixBackupStrategy(); + var modInstaller = new ModInstaller(installationFactory, backupStrategy, config.ModInstall); return new ModManager(game, modRepository, modInstaller, statePersistence, safeFileDelete, tempDir); } } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 8a84ab2..346d2b4 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -46,11 +46,11 @@ public interface IProgress private readonly Matcher filesToInstallMatcher; private readonly IBackupStrategy backupStrategy; - public ModInstaller(IInstallationFactory installationFactory, IConfig config) + public ModInstaller(IInstallationFactory installationFactory, IBackupStrategy backupStrategy, IConfig config) { this.installationFactory = installationFactory; + this.backupStrategy = backupStrategy; filesToInstallMatcher = Matchers.ExcludingPatterns(config.ExcludedFromInstall); - backupStrategy = new SuffixBackupStrategy(); } public void Apply( @@ -260,6 +260,7 @@ private void InstallPackages( { eventHandler.PostProcessingNotRequired(); } + eventHandler.InstallEnd(); eventHandler.ProgressUpdate(progress.IncrementDone()); } else diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs new file mode 100644 index 0000000..33cfee6 --- /dev/null +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -0,0 +1,223 @@ +using Core.Backup; +using Core.Mods; +using Core.State; +using Core.Utils; +using FluentAssertions; + +namespace Core.Tests; + +public class ModInstallerIntegrationTest : AbstractFilesystemTest +{ + #region Initialisation + + private record InstallationResult( + int? PackageFsHash, + HashSet InstalledFiles, + IInstallation.State Installed + ) + { + internal InstallationResult(IInstallation installation) : this( + installation.PackageFsHash, + installation.InstalledFiles.ToHashSet(), + installation.Installed) { } + } + + private readonly Mock installationFactoryMock; + private readonly Mock backupStrategyMock; + private readonly Mock config; + private readonly ModInstaller modInstaller; + + private readonly Mock eventHandlerMock; + + private readonly Dictionary recordedState; + + public ModInstallerIntegrationTest() + { + installationFactoryMock = new Mock(); + backupStrategyMock = new Mock(); + config = new Mock(); + modInstaller = new ModInstaller( + installationFactoryMock.Object, + backupStrategyMock.Object, + config.Object); + eventHandlerMock = new Mock(); + recordedState = new(); + } + + #endregion + + [Fact] + public void Apply_NoMods() + { + modInstaller.Apply( + new Dictionary(), + [], + "", + RecordState, + eventHandlerMock.Object, + CancellationToken.None); + + recordedState.Should().BeEmpty(); + } + + [Fact] + public void Apply_UninstallsMods() + { + // TODO Introduce interface to delete file as part of restoring backup + CreateTestFile("AF"); + + modInstaller.Apply( + new Dictionary{ + ["A"] = new( + Time: null, + FsHash: 42, + Partial: false, + Files: ["AF"]) + }, + [], + testDir.FullName, + RecordState, + eventHandlerMock.Object, + CancellationToken.None); + + recordedState.Should().BeEquivalentTo(new Dictionary{ + ["A"] = new(42, [], IInstallation.State.NotInstalled), + }); + + // TODO see above + File.Exists(TestPath("AF")).Should().BeFalse(); + + backupStrategyMock.Verify(_ => _.RestoreBackup(TestPath("AF"))); + backupStrategyMock.VerifyNoOtherCalls(); + + eventHandlerMock.Verify(_ => _.UninstallStart()); + eventHandlerMock.Verify(_ => _.UninstallCurrent("A")); + eventHandlerMock.Verify(_ => _.UninstallEnd()); + eventHandlerMock.Verify(_ => _.InstallNoMods()); + eventHandlerMock.Verify(_ => _.ProgressUpdate(It.IsAny())); + eventHandlerMock.VerifyNoOtherCalls(); + } + + [Fact] + public void Apply_InstallsMods() + { + modInstaller.Apply( + new Dictionary(), + [ + PackageInstalling("A", 42, [ + "AF" + ]) + ], + testDir.FullName, + RecordState, + eventHandlerMock.Object, + CancellationToken.None); + + recordedState.Should().BeEquivalentTo(new Dictionary + { + ["A"] = new(42, ["AF"], IInstallation.State.Installed) + }); + + backupStrategyMock.Verify(_ => _.PerformBackup(TestPath("AF"))); + backupStrategyMock.Verify(_ => _.IsBackupFile(It.IsAny())); + backupStrategyMock.VerifyNoOtherCalls(); + + eventHandlerMock.Verify(_ => _.UninstallNoMods()); + eventHandlerMock.Verify(_ => _.InstallStart()); + eventHandlerMock.Verify(_ => _.InstallCurrent("A")); + eventHandlerMock.Verify(_ => _.PostProcessingNotRequired()); + eventHandlerMock.Verify(_ => _.InstallEnd()); + eventHandlerMock.Verify(_ => _.ProgressUpdate(It.IsAny())); + eventHandlerMock.VerifyNoOtherCalls(); + } + + [Fact] + public void Apply_UpdatesMods() + { + var endState = new Dictionary(); + modInstaller.Apply( + new Dictionary + { + ["A"] = new(Time: null, FsHash: 1, Partial: false, Files: [ + "AF", + "AF1", + ]) + }, + [ + PackageInstalling("A", 2, [ + "AF", + "AF2" + ]) + ], + testDir.FullName, + RecordState, + eventHandlerMock.Object, + CancellationToken.None); + + recordedState.Should().BeEquivalentTo(new Dictionary + { + ["A"] = new(2, ["AF", "AF2"], IInstallation.State.Installed) + }); + } + + #region Utility methods + + private ModPackage PackageInstalling(string name, int? fsHash, IReadOnlyCollection files) + { + var unusedPath = $@"Some\Unused\Path\{name}"; + var unusedEnabled = Random.Shared.NextDouble() < 0.5; + var package = new ModPackage(name, unusedPath, unusedEnabled, fsHash); + var installer = new StaticFilesInstaller(name, fsHash, new SubdirectoryTempDir(testDir.FullName), files); + installationFactoryMock.Setup(_ => _.ModInstaller(package)).Returns(installer); + return package; + } + + private class StaticFilesInstaller : BaseInstaller + { + private static readonly object NoContext = new(); + private readonly IReadOnlyCollection files; + + internal StaticFilesInstaller(string packageName, int? packageFsHash, ITempDir tempDir, IReadOnlyCollection files) : + base(packageName, packageFsHash, tempDir, Config()) + { + this.files = files; + } + + protected override void InstalAllFiles(InstallBody body) + { + foreach (var file in files) + { + body(file, NoContext); + } + } + + protected override void InstallFile(RootedPath destinationPath, object context) + { + // Do not install any file for real + } + + public override void Dispose() + { + } + + // Install everything from the root directory + + private static readonly string DirAtRoot = "X"; + + private static BaseInstaller.IConfig Config() + { + var mock = new Mock(); + mock.Setup(_ => _.DirsAtRoot).Returns([DirAtRoot]); + return mock.Object; + } + + protected override IEnumerable RelativeDirectoryPaths => [DirAtRoot]; + } + + private void RecordState(IInstallation state) + { + recordedState[state.PackageName] = new InstallationResult(state); + } + + #endregion +} diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/ModManagerIntegrationTest.cs index 9f7bd83..348b7a7 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/ModManagerIntegrationTest.cs @@ -1,4 +1,5 @@ using System.IO.Compression; +using Core.Backup; using Core.Games; using Core.IO; using Core.Mods; @@ -54,7 +55,7 @@ public ModManagerIntegrationTest() : base() modManager = new ModManager( gameMock.Object, modRepositoryMock.Object, - new ModInstaller(installationFactory, modInstallConfig), + new ModInstaller(installationFactory, new SuffixBackupStrategy(), modInstallConfig), persistedState, safeFileDeleteMock.Object, tempDir); From 404954ac547974d7accff1dcf5e31b85750bc796 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Thu, 18 Jul 2024 15:49:11 +0100 Subject: [PATCH 06/18] Moved backup file name check to backup call --- src/Core/Backup/IBackupStrategy.cs | 1 - src/Core/Backup/MoveFileBackupStrategy.cs | 25 ++++++--- src/Core/Backup/SuffixBackupStrategy.cs | 15 +++--- src/Core/ModInstaller.cs | 1 - .../Backup/MoveFileBackupStrategyTest.cs | 51 ++++++++++++------- .../Core.Tests/ModInstallerIntegrationTest.cs | 46 ++++++++++++++++- 6 files changed, 104 insertions(+), 35 deletions(-) diff --git a/src/Core/Backup/IBackupStrategy.cs b/src/Core/Backup/IBackupStrategy.cs index 314ef2e..b1c1788 100644 --- a/src/Core/Backup/IBackupStrategy.cs +++ b/src/Core/Backup/IBackupStrategy.cs @@ -5,5 +5,4 @@ public interface IBackupStrategy public void PerformBackup(string fullPath); public void RestoreBackup(string fullPath); public void DeleteBackup(string fullPath); - public bool IsBackupFile(string fullPath); } diff --git a/src/Core/Backup/MoveFileBackupStrategy.cs b/src/Core/Backup/MoveFileBackupStrategy.cs index 5993c1a..add60d8 100644 --- a/src/Core/Backup/MoveFileBackupStrategy.cs +++ b/src/Core/Backup/MoveFileBackupStrategy.cs @@ -2,26 +2,35 @@ namespace Core.Backup; -public class MoveFileBackupStrategy +public class MoveFileBackupStrategy : IBackupStrategy { + public interface IBackupFileNaming + { + public string ToBackup(string fullPath); + public bool IsBackup(string fullPath); + } private readonly IFileSystem fs; - private readonly Func generateBackupFilePath; + private readonly IBackupFileNaming backupFileNaming; - public MoveFileBackupStrategy(IFileSystem fs, Func generateBackupFilePath) + public MoveFileBackupStrategy(IFileSystem fs, IBackupFileNaming backupFileNaming) { this.fs = fs; - this.generateBackupFilePath = generateBackupFilePath; + this.backupFileNaming = backupFileNaming; } - public void PerformBackup(string fullPath) + public virtual void PerformBackup(string fullPath) { + if (backupFileNaming.IsBackup(fullPath)) + { + throw new InvalidOperationException("Installing a backup file is forbidden"); + } if (!fs.File.Exists(fullPath)) { return; } - var backupFilePath = generateBackupFilePath(fullPath); + var backupFilePath = backupFileNaming.ToBackup(fullPath); if (fs.File.Exists(backupFilePath)) { fs.File.Delete(fullPath); @@ -34,7 +43,7 @@ public void PerformBackup(string fullPath) public void RestoreBackup(string fullPath) { - var backupFilePath = generateBackupFilePath(fullPath); + var backupFilePath = backupFileNaming.ToBackup(fullPath); if (fs.File.Exists(backupFilePath)) { fs.File.Move(backupFilePath, fullPath); @@ -43,7 +52,7 @@ public void RestoreBackup(string fullPath) public void DeleteBackup(string fullPath) { - var backupFilePath = generateBackupFilePath(fullPath); + var backupFilePath = backupFileNaming.ToBackup(fullPath); if (fs.File.Exists(backupFilePath)) { fs.File.Delete(backupFilePath); diff --git a/src/Core/Backup/SuffixBackupStrategy.cs b/src/Core/Backup/SuffixBackupStrategy.cs index 0e7ae50..2283732 100644 --- a/src/Core/Backup/SuffixBackupStrategy.cs +++ b/src/Core/Backup/SuffixBackupStrategy.cs @@ -2,15 +2,18 @@ namespace Core.Backup; -public class SuffixBackupStrategy : MoveFileBackupStrategy, IBackupStrategy +public class SuffixBackupStrategy : MoveFileBackupStrategy { - public const string BackupSuffix = ".orig"; + private class BackupFileNaming : IBackupFileNaming + { + private const string BackupSuffix = ".orig"; + + public string ToBackup(string fullPath) => $"{fullPath}{BackupSuffix}"; + public bool IsBackup(string fullPath) => fullPath.EndsWith(BackupSuffix); + } public SuffixBackupStrategy() : - base(new FileSystem(), _ => $"{_}{BackupSuffix}") + base(new FileSystem(), new BackupFileNaming()) { } - - public bool IsBackupFile(string fullPath) => - fullPath.EndsWith(BackupSuffix); } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 346d2b4..03f7e57 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -204,7 +204,6 @@ private void InstallPackages( { Accept = gamePath => Whitelisted(gamePath) && - !backupStrategy.IsBackupFile(gamePath.Relative) && !installedFiles.Contains(gamePath.Relative), Before = gamePath => { diff --git a/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs b/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs index a0900ff..0c21ef7 100644 --- a/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs +++ b/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs @@ -1,6 +1,7 @@ using System.IO.Abstractions.TestingHelpers; using Core.Backup; using FluentAssertions; +using static Core.Backup.MoveFileBackupStrategy; namespace Core.Tests.Backup; @@ -9,19 +10,26 @@ public class MoveFileBackupStrategyTest { private static readonly string OriginalFile = "original"; private static readonly string OriginalContents = "something"; - private static readonly string BackupFile = GenerateBackupFilePath(OriginalFile); - private static string GenerateBackupFilePath(string fullPath) => $"b{fullPath}"; + private readonly Mock backupFileNamingMock; + + private MoveFileBackupStrategy.IBackupFileNaming BackupFileNaming => backupFileNamingMock.Object; + private string BackupFile => BackupFileNaming.ToBackup(OriginalFile); + + public MoveFileBackupStrategyTest() + { + backupFileNamingMock = new(); + backupFileNamingMock.Setup(_ => _.ToBackup(It.IsAny())).Returns(_ => $"b{_}"); + } [Fact] - public void BackupFile_MovesOriginalToBackup() + public void PerformBackup_MovesOriginalToBackup() { var fs = new MockFileSystem(new Dictionary { { OriginalFile, OriginalContents }, }); - - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.PerformBackup(OriginalFile); @@ -30,11 +38,23 @@ public void BackupFile_MovesOriginalToBackup() } [Fact] - public void BackupFile_SkipsBackupIfFileNotPresent() + public void PerformBackup_ErrorsIfNameIsBackupName() { var fs = new MockFileSystem(); + backupFileNamingMock.Setup(_ => _.IsBackup(OriginalFile)).Returns(true); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + sbs.Invoking(_ => _.PerformBackup(OriginalFile)) + .Should().Throw(); + + fs.FileExists(BackupFile).Should().BeFalse(); + } + + [Fact] + public void PerformBackup_SkipsBackupIfFileNotPresent() + { + var fs = new MockFileSystem(); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.PerformBackup(OriginalFile); @@ -42,7 +62,7 @@ public void BackupFile_SkipsBackupIfFileNotPresent() } [Fact] - public void BackupFile_KeepsExistingBackup() + public void PerformBackup_KeepsExistingBackup() { var oldBackupContents = "old backup"; var fs = new MockFileSystem(new Dictionary @@ -50,8 +70,7 @@ public void BackupFile_KeepsExistingBackup() { OriginalFile, OriginalContents }, { BackupFile, oldBackupContents }, }); - - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.PerformBackup(OriginalFile); @@ -66,8 +85,7 @@ public void RestoreBackup_MovesBackupToOriginal() { { BackupFile, OriginalContents}, }); - - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.RestoreBackup(OriginalFile); @@ -82,8 +100,7 @@ public void RestoreBackup_LeavesOriginalFileIfNoBackup() { { OriginalFile, "other contents" }, }); - - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.RestoreBackup(OriginalFile); @@ -98,8 +115,7 @@ public void RestoreBackup_ErrorsIfOriginalFileExists() { OriginalFile, "other contents" }, { BackupFile, OriginalContents}, }); - - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.Invoking(_ => _.RestoreBackup(OriginalFile)).Should().Throw(); @@ -114,8 +130,7 @@ public void DeleteBackup_RemovesBackupIfItExists() { { BackupFile, OriginalContents}, }); - - var sbs = new MoveFileBackupStrategy(fs, GenerateBackupFilePath); + var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); sbs.DeleteBackup(OriginalFile); diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index 33cfee6..ab11ab6 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -10,6 +10,8 @@ public class ModInstallerIntegrationTest : AbstractFilesystemTest { #region Initialisation + private class TestException : Exception {} + private record InstallationResult( int? PackageFsHash, HashSet InstalledFiles, @@ -98,6 +100,29 @@ public void Apply_UninstallsMods() eventHandlerMock.VerifyNoOtherCalls(); } + [Fact] + public void Apply_UninstallStopsIfBackupFails() + { + backupStrategyMock.Setup(_ => _.RestoreBackup(TestPath("Fail"))).Throws(); + + modInstaller.Invoking(_ => _.Apply( + new Dictionary + { + ["A"] = new( + Time: null, + FsHash: 42, + Partial: false, + Files: ["AF1", "Fail", "AF2"]) + }, + [], + testDir.FullName, + RecordState, + eventHandlerMock.Object, + CancellationToken.None)).Should().Throw(); + + recordedState["A"].InstalledFiles.Should().BeEquivalentTo(["Fail", "AF2"]); + } + [Fact] public void Apply_InstallsMods() { @@ -119,7 +144,6 @@ public void Apply_InstallsMods() }); backupStrategyMock.Verify(_ => _.PerformBackup(TestPath("AF"))); - backupStrategyMock.Verify(_ => _.IsBackupFile(It.IsAny())); backupStrategyMock.VerifyNoOtherCalls(); eventHandlerMock.Verify(_ => _.UninstallNoMods()); @@ -131,6 +155,26 @@ public void Apply_InstallsMods() eventHandlerMock.VerifyNoOtherCalls(); } + [Fact] + public void Apply_InstallStopsIfBackupFails() + { + backupStrategyMock.Setup(_ => _.PerformBackup(TestPath("Fail"))).Throws(); + + modInstaller.Invoking(_ => _.Apply( + new Dictionary(), + [ + PackageInstalling("A", 42, [ + "AF1", "Fail", "AF2" + ]) + ], + testDir.FullName, + RecordState, + eventHandlerMock.Object, + CancellationToken.None)).Should().Throw(); + + recordedState["A"].InstalledFiles.Should().BeEquivalentTo(["AF1"]); + } + [Fact] public void Apply_UpdatesMods() { From 1dc5cfb86f47e7a0699fa297bfdafe39325dc1ee Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sat, 20 Jul 2024 10:02:20 +0100 Subject: [PATCH 07/18] Simplified callback usage --- src/Core/ModInstaller.cs | 13 +++---------- src/Core/Mods/ProcessingCallbacks.cs | 9 +++++++++ 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 03f7e57..84ab0ce 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -77,14 +77,8 @@ private void UninstallPackages( eventHandler.UninstallStart(); var uninstallCallbacks = new ProcessingCallbacks { - After = gamePath => - { - backupStrategy.RestoreBackup(gamePath.Full); - }, - NotAccepted = gamePath => - { - backupStrategy.DeleteBackup(gamePath.Full); - } + After = gamePath => backupStrategy.RestoreBackup(gamePath.Full), + NotAccepted = gamePath => backupStrategy.DeleteBackup(gamePath.Full) }; foreach (var (packageName, modInstallationState) in currentState) { @@ -101,8 +95,7 @@ private void UninstallPackages( filesLeft, uninstallCallbacks .AndAccept(SkipCreatedAfter(eventHandler, modInstallationState.Time)) - .AndAfter(_ => filesLeft.Remove(_.Relative)) - .AndNotAccepted(_ => filesLeft.Remove(_.Relative)) + .AndFinally(_ => filesLeft.Remove(_.Relative)) ); } finally diff --git a/src/Core/Mods/ProcessingCallbacks.cs b/src/Core/Mods/ProcessingCallbacks.cs index 45559a2..a739a28 100644 --- a/src/Core/Mods/ProcessingCallbacks.cs +++ b/src/Core/Mods/ProcessingCallbacks.cs @@ -81,6 +81,15 @@ public ProcessingCallbacks AndNotAccepted(Action additional) => notAccepted = Combine(notAccepted, additional) }; + public ProcessingCallbacks AndFinally(Action additional) => + new() + { + accept = accept, + before = before, + after = Combine(after, additional), + notAccepted = Combine(notAccepted, additional) + }; + private static Predicate? Combine(Predicate? p1, Predicate p2) => p1 is null ? p2 : key => p1(key) && p2(key); From 6300289776905aa2f97dd3bd430ceea6015df2dd Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sun, 4 Aug 2024 07:47:06 +0100 Subject: [PATCH 08/18] Installer not Disposable --- src/Core/ModInstaller.cs | 9 ++------- src/Core/Mods/BaseInstaller.cs | 2 -- src/Core/Mods/GeneratedBootfilesInstaller.cs | 4 ---- src/Core/Mods/IInstaller.cs | 2 +- src/Core/Mods/ModArchiveInstaller.cs | 4 ---- src/Core/Mods/ModDirectoryInstaller.cs | 4 ---- tests/Core.Tests/ModInstallerIntegrationTest.cs | 4 ---- 7 files changed, 3 insertions(+), 26 deletions(-) diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 84ab0ce..f5dbda2 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -220,7 +220,7 @@ private void InstallPackages( break; } eventHandler.InstallCurrent(modPackage.PackageName); - using var mod = installationFactory.ModInstaller(modPackage); + var mod = installationFactory.ModInstaller(modPackage); try { var modConfig = mod.Install(installDir, installCallbacks); @@ -236,7 +236,7 @@ private void InstallPackages( if (modConfigs.Where(_ => _.NotEmpty()).Any()) { eventHandler.PostProcessingStart(); - using var bootfilesMod = CreateBootfilesMod(toInstall, eventHandler); + var bootfilesMod = CreateBootfilesMod(toInstall, eventHandler); try { bootfilesMod.Install(installDir, installCallbacks); @@ -349,10 +349,5 @@ public void PostProcessing(string dstPath, IReadOnlyList modConfi PostProcessor.AppendDrivelineRecords(dstPath, modConfigs.SelectMany(_ => _.DrivelineRecords)); postProcessingDone = true; } - - public void Dispose() - { - inner.Dispose(); - } } } diff --git a/src/Core/Mods/BaseInstaller.cs b/src/Core/Mods/BaseInstaller.cs index f1c711a..8a3d31c 100644 --- a/src/Core/Mods/BaseInstaller.cs +++ b/src/Core/Mods/BaseInstaller.cs @@ -172,8 +172,6 @@ private List FindDrivelineRecords() return recordBlocks; } - - public abstract void Dispose(); } public static class BaseInstaller diff --git a/src/Core/Mods/GeneratedBootfilesInstaller.cs b/src/Core/Mods/GeneratedBootfilesInstaller.cs index f8c36a3..74d8255 100644 --- a/src/Core/Mods/GeneratedBootfilesInstaller.cs +++ b/src/Core/Mods/GeneratedBootfilesInstaller.cs @@ -30,10 +30,6 @@ protected override void InstallFile(RootedPath destinationPath, FileInfo fileInf File.Move(fileInfo.FullName, destinationPath.Full); } - public override void Dispose() - { - } - #region Bootfiles Generation private void GenerateBootfiles() diff --git a/src/Core/Mods/IInstaller.cs b/src/Core/Mods/IInstaller.cs index b66a744..1b9171f 100644 --- a/src/Core/Mods/IInstaller.cs +++ b/src/Core/Mods/IInstaller.cs @@ -1,6 +1,6 @@ namespace Core.Mods; -public interface IInstaller : IInstallation, IDisposable +public interface IInstaller : IInstallation { ConfigEntries Install(string dstPath, ProcessingCallbacks callbacks); } diff --git a/src/Core/Mods/ModArchiveInstaller.cs b/src/Core/Mods/ModArchiveInstaller.cs index bcd9997..112e265 100644 --- a/src/Core/Mods/ModArchiveInstaller.cs +++ b/src/Core/Mods/ModArchiveInstaller.cs @@ -15,10 +15,6 @@ public ModArchiveInstaller(string packageName, int? packageFsHash, ITempDir temp this.archivePath = archivePath; } - public override void Dispose() - { - } - // LibArchive.Net is a mere wrapper around libarchive. It's better to avoid using // LINQ expressions as they can lead to or // being called out of order. diff --git a/src/Core/Mods/ModDirectoryInstaller.cs b/src/Core/Mods/ModDirectoryInstaller.cs index f427d5c..9fb824a 100644 --- a/src/Core/Mods/ModDirectoryInstaller.cs +++ b/src/Core/Mods/ModDirectoryInstaller.cs @@ -17,8 +17,4 @@ protected override void InstallFile(RootedPath destinationPath, FileInfo fileInf { File.Copy(fileInfo.FullName, destinationPath.Full); } - - public override void Dispose() - { - } } diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index ab11ab6..68ffee0 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -240,10 +240,6 @@ protected override void InstallFile(RootedPath destinationPath, object context) // Do not install any file for real } - public override void Dispose() - { - } - // Install everything from the root directory private static readonly string DirAtRoot = "X"; From c96c053cf6cb62c87324863923c2c57b30ab4116 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sun, 10 Nov 2024 07:30:38 +0000 Subject: [PATCH 09/18] Renamed saved state --- src/Core/IModInstaller.cs | 2 +- src/Core/ModInstaller.cs | 4 +- src/Core/ModManager.cs | 16 ++--- src/Core/State/IStatePersistence.cs | 4 +- src/Core/State/JsonFileStatePersistence.cs | 12 ++-- .../State/{InternalState.cs => SavedState.cs} | 23 ++++--- .../Core.Tests/ModInstallerIntegrationTest.cs | 12 ++-- tests/Core.Tests/ModManagerIntegrationTest.cs | 68 +++++++++---------- 8 files changed, 71 insertions(+), 70 deletions(-) rename src/Core/State/{InternalState.cs => SavedState.cs} (50%) diff --git a/src/Core/IModInstaller.cs b/src/Core/IModInstaller.cs index c7eac9c..5b00841 100644 --- a/src/Core/IModInstaller.cs +++ b/src/Core/IModInstaller.cs @@ -5,5 +5,5 @@ namespace Core; public interface IModInstaller { - void Apply(IReadOnlyDictionary currentState, IReadOnlyCollection packages, string installDir, Action afterInstall, ModInstaller.IEventHandler eventHandler, CancellationToken cancellationToken); + void Apply(IReadOnlyDictionary currentState, IReadOnlyCollection packages, string installDir, Action afterInstall, ModInstaller.IEventHandler eventHandler, CancellationToken cancellationToken); } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index f5dbda2..6d74bcc 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -54,7 +54,7 @@ public ModInstaller(IInstallationFactory installationFactory, IBackupStrategy ba } public void Apply( - IReadOnlyDictionary currentState, + IReadOnlyDictionary currentState, IReadOnlyCollection toInstall, string installDir, Action afterCallback, @@ -66,7 +66,7 @@ public void Apply( } private void UninstallPackages( - IReadOnlyDictionary currentState, + IReadOnlyDictionary currentState, string installDir, Action afterUninstall, IEventHandler eventHandler, diff --git a/src/Core/ModManager.cs b/src/Core/ModManager.cs index 9f8a9b6..2f22c95 100644 --- a/src/Core/ModManager.cs +++ b/src/Core/ModManager.cs @@ -1,10 +1,8 @@ -using System.Globalization; using Core.Games; using Core.IO; using Core.Mods; using Core.State; using Core.Utils; -using Microsoft.VisualBasic; using static Core.IModManager; namespace Core; @@ -53,7 +51,7 @@ public List FetchState() var availableModPackages = enabledModPackages.Merge(disabledModPackages); var bootfilesFailed = installedMods.Where(kv => BootfilesManager.IsBootFiles(kv.Key) && (kv.Value?.Partial ?? false)).Any(); - var isModInstalled = installedMods.SelectValues(modInstallationState => + var isModInstalled = installedMods.SelectValues(modInstallationState => modInstallationState is null ? false : ((modInstallationState.Partial || bootfilesFailed) ? null : true) ); var modsOutOfDate = installedMods.SelectValues((packageName, modInstallationState) => @@ -81,7 +79,7 @@ public List FetchState() }).ToList(); } - private static bool IsOutOfDate(ModPackage? modPackage, InternalModInstallationState? modInstallationState) + private static bool IsOutOfDate(ModPackage? modPackage, ModInstallationState? modInstallationState) { if (modPackage is null || modInstallationState is null) { @@ -140,14 +138,14 @@ public void InstallEnabledMods(IEventHandler eventHandler, CancellationToken can // Clean what left by a previous failed installation tempDir.Cleanup(); - InstallMods(modRepository.ListEnabledMods(), eventHandler, cancellationToken); + UpdateMods(modRepository.ListEnabledMods(), eventHandler, cancellationToken); tempDir.Cleanup(); } public void UninstallAllMods(IEventHandler eventHandler, CancellationToken cancellationToken = default) { CheckGameNotRunning(); - InstallMods(Array.Empty(), eventHandler, cancellationToken); + UpdateMods(Array.Empty(), eventHandler, cancellationToken); } private void CheckGameNotRunning() @@ -158,10 +156,10 @@ private void CheckGameNotRunning() } } - private void InstallMods(IReadOnlyCollection packages, IEventHandler eventHandler, CancellationToken cancellationToken) + private void UpdateMods(IReadOnlyCollection packages, IEventHandler eventHandler, CancellationToken cancellationToken) { var previousState = statePersistence.ReadState().Install.Mods; - var currentState = new Dictionary(previousState); + var currentState = new Dictionary(previousState); try { modInstaller.Apply( @@ -198,7 +196,7 @@ private void InstallMods(IReadOnlyCollection packages, IEventHandler } finally { - statePersistence.WriteState(new InternalState( + statePersistence.WriteState(new SavedState( Install: new( Time: currentState.Values.Max(_ => _.Time), Mods: currentState diff --git a/src/Core/State/IStatePersistence.cs b/src/Core/State/IStatePersistence.cs index ffea886..b87c7e2 100644 --- a/src/Core/State/IStatePersistence.cs +++ b/src/Core/State/IStatePersistence.cs @@ -2,6 +2,6 @@ public interface IStatePersistence { - public InternalState ReadState(); - public void WriteState(InternalState state); + public SavedState ReadState(); + public void WriteState(SavedState state); } diff --git a/src/Core/State/JsonFileStatePersistence.cs b/src/Core/State/JsonFileStatePersistence.cs index 3ddb50e..9bcc317 100644 --- a/src/Core/State/JsonFileStatePersistence.cs +++ b/src/Core/State/JsonFileStatePersistence.cs @@ -23,13 +23,13 @@ public JsonFileStatePersistence(string modsDir) oldStateFile = Path.Combine(modsDir, OldStateFileName); } - public InternalState ReadState() + public SavedState ReadState() { // Always favour new state if present if (File.Exists(stateFile)) { var contents = File.ReadAllText(stateFile); - var state = JsonConvert.DeserializeObject(contents); + var state = JsonConvert.DeserializeObject(contents); // Fill mod install time if not present (for migration) return state with { @@ -46,21 +46,21 @@ public InternalState ReadState() var contents = File.ReadAllText(oldStateFile); var oldState = JsonConvert.DeserializeObject>>(contents); var installTime = File.GetLastWriteTimeUtc(oldStateFile); - return new InternalState( + return new SavedState( Install: new( Time: installTime, Mods: oldState.AsEnumerable().ToDictionary( kv => kv.Key, - kv => new InternalModInstallationState(Time: installTime, FsHash: null, Partial: false, Files: kv.Value) + kv => new ModInstallationState(Time: installTime, FsHash: null, Partial: false, Files: kv.Value) ) ) ); } - return InternalState.Empty(); + return SavedState.Empty(); } - public void WriteState(InternalState state) + public void WriteState(SavedState state) { // Remove old state if upgrading from a previous version File.Delete(oldStateFile); diff --git a/src/Core/State/InternalState.cs b/src/Core/State/SavedState.cs similarity index 50% rename from src/Core/State/InternalState.cs rename to src/Core/State/SavedState.cs index 9b82701..f3fc549 100644 --- a/src/Core/State/InternalState.cs +++ b/src/Core/State/SavedState.cs @@ -2,28 +2,28 @@ namespace Core.State; -public record InternalState( - InternalInstallationState Install +public record SavedState( + InstallationState Install ) { - public static InternalState Empty() => new( - Install: InternalInstallationState.Empty() + public static SavedState Empty() => new( + Install: InstallationState.Empty() ); }; -public record InternalInstallationState( +public record InstallationState( // TODO: needed for backward compatibility DateTime? Time, - IReadOnlyDictionary Mods + IReadOnlyDictionary Mods ) { - public static InternalInstallationState Empty() => new( + public static InstallationState Empty() => new( Time: null, - Mods: ImmutableDictionary.Create() + Mods: ImmutableDictionary.Create() ); }; -public record InternalModInstallationState( +public record ModInstallationState( // TODO: nullable for backward compatibility DateTime? Time, // Unknown when partially installed or upgrading from a previous version @@ -32,4 +32,7 @@ public record InternalModInstallationState( // infer from null hash after the first install bool Partial, IReadOnlyCollection Files -); +) +{ + public static ModInstallationState Empty => new(null, null, false, Array.Empty()); +} diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index 68ffee0..3607afc 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -52,7 +52,7 @@ public ModInstallerIntegrationTest() public void Apply_NoMods() { modInstaller.Apply( - new Dictionary(), + new Dictionary(), [], "", RecordState, @@ -69,7 +69,7 @@ public void Apply_UninstallsMods() CreateTestFile("AF"); modInstaller.Apply( - new Dictionary{ + new Dictionary{ ["A"] = new( Time: null, FsHash: 42, @@ -106,7 +106,7 @@ public void Apply_UninstallStopsIfBackupFails() backupStrategyMock.Setup(_ => _.RestoreBackup(TestPath("Fail"))).Throws(); modInstaller.Invoking(_ => _.Apply( - new Dictionary + new Dictionary { ["A"] = new( Time: null, @@ -127,7 +127,7 @@ public void Apply_UninstallStopsIfBackupFails() public void Apply_InstallsMods() { modInstaller.Apply( - new Dictionary(), + new Dictionary(), [ PackageInstalling("A", 42, [ "AF" @@ -161,7 +161,7 @@ public void Apply_InstallStopsIfBackupFails() backupStrategyMock.Setup(_ => _.PerformBackup(TestPath("Fail"))).Throws(); modInstaller.Invoking(_ => _.Apply( - new Dictionary(), + new Dictionary(), [ PackageInstalling("A", 42, [ "AF1", "Fail", "AF2" @@ -180,7 +180,7 @@ public void Apply_UpdatesMods() { var endState = new Dictionary(); modInstaller.Apply( - new Dictionary + new Dictionary { ["A"] = new(Time: null, FsHash: 1, Partial: false, Files: [ "AF", diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/ModManagerIntegrationTest.cs index 348b7a7..159322b 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/ModManagerIntegrationTest.cs @@ -79,11 +79,11 @@ public void Uninstall_FailsIfGameRunning() [Fact] public void Uninstall_DeletesCreatedFilesAndDirectories() { - persistedState.InitState(new InternalState + persistedState.InitState(new SavedState ( Install: new( Time: ValueNotUsed, - Mods: new Dictionary + Mods: new Dictionary { ["A"] = new( Time: null, FsHash: null, Partial: false, Files: [ @@ -111,11 +111,11 @@ public void Uninstall_DeletesCreatedFilesAndDirectories() public void Uninstall_SkipsFilesCreatedAfterInstallation() { var installationDateTime = DateTime.Now.Subtract(TimeSpan.FromDays(1)); - persistedState.InitState(new InternalState + persistedState.InitState(new SavedState ( Install: new( Time: ValueNotUsed, - Mods: new Dictionary + Mods: new Dictionary { [""] = new( Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ @@ -141,10 +141,10 @@ public void Uninstall_StopsAfterAnyError() { // It must be after files are created var installationDateTime = DateTime.Now.AddMinutes(1); - persistedState.InitState(new InternalState( + persistedState.InitState(new SavedState( Install: new( Time: ValueNotUsed, - Mods: new Dictionary + Mods: new Dictionary { ["A"] = new( Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ @@ -170,10 +170,10 @@ public void Uninstall_StopsAfterAnyError() modManager.Invoking(_ => _.UninstallAllMods(eventHandlerMock.Object)) .Should().Throw(); - persistedState.Should().Be(new InternalState( - Install: new InternalInstallationState( + persistedState.Should().Be(new SavedState( + Install: new InstallationState( Time: installationDateTime.ToUniversalTime(), - Mods: new Dictionary + Mods: new Dictionary { ["B"] = new( Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: true, Files: [ @@ -191,10 +191,10 @@ public void Uninstall_StopsAfterAnyError() [Fact] public void Uninstall_RestoresBackups() { - persistedState.InitState(new InternalState( + persistedState.InitState(new SavedState( Install: new( Time: ValueNotUsed, - Mods: new Dictionary + Mods: new Dictionary { [""] = new( Time: null, FsHash: null, Partial: false, Files: [ @@ -217,10 +217,10 @@ public void Uninstall_SkipsRestoreIfModFileOverwritten() { // It must be after files are created var installationDateTime = DateTime.Now.AddMinutes(1); - persistedState.InitState(new InternalState( + persistedState.InitState(new SavedState( Install: new( Time: ValueNotUsed, - Mods: new Dictionary + Mods: new Dictionary { [""] = new( Time: installationDateTime.ToUniversalTime(), FsHash: null, Partial: false, Files: [ @@ -269,7 +269,7 @@ public void Install_InstallsContentFromRootDirectories() File.Exists(GamePath("C")).Should().BeTrue(); File.Exists(GamePath("D")).Should().BeFalse(); File.Exists(GamePath(Path.Combine("Baz", "D"))).Should().BeFalse(); - persistedState.Should().HaveInstalled(new Dictionary + persistedState.Should().HaveInstalled(new Dictionary { ["Package100"] = new( Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: [ @@ -294,7 +294,7 @@ public void Install_SkipsBlacklistedFiles() File.Exists(GamePath(Path.Combine("A", FileExcludedFromInstall))).Should().BeFalse(); File.Exists(GamePath(Path.Combine(DirAtRoot, "B"))).Should().BeTrue(); - persistedState.Should().HaveInstalled(new Dictionary + persistedState.Should().HaveInstalled(new Dictionary { ["Package100"] = new( Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: [ @@ -334,7 +334,7 @@ public void Install_GivesPriotiryToFilesLaterInTheModList() modManager.InstallEnabledMods(eventHandlerMock.Object); File.ReadAllText(GamePath(Path.Combine(DirAtRoot, "A"))).Should().Be("200"); - persistedState.Should().HaveInstalled(new Dictionary + persistedState.Should().HaveInstalled(new Dictionary { ["Package100"] = new(Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: []), ["Package200"] = new(Time: DateTime.UtcNow, FsHash: 200, Partial: false, Files: [ @@ -355,7 +355,7 @@ public void Install_DuplicatesAreCaseInsensitive() modManager.InstallEnabledMods(eventHandlerMock.Object); - persistedState.Should().HaveInstalled(new Dictionary + persistedState.Should().HaveInstalled(new Dictionary { ["Package100"] = new(Time: DateTime.UtcNow, FsHash: 100, Partial: false, Files: [ Path.Combine(DirAtRoot, "A") @@ -388,10 +388,10 @@ public void Install_StopsAfterAnyError() File.ReadAllText(GamePath(Path.Combine(DirAtRoot, "B1"))).Should().Be("200"); File.Exists(GamePath(Path.Combine(DirAtRoot, "B3"))).Should().BeFalse(); File.Exists(GamePath(Path.Combine(DirAtRoot, "A"))).Should().BeFalse(); - persistedState.Should().Be(new InternalState( - Install: new InternalInstallationState( + persistedState.Should().Be(new SavedState( + Install: new InstallationState( Time: DateTime.UtcNow, - Mods: new Dictionary + Mods: new Dictionary { ["Package200"] = new( Time: DateTime.UtcNow, FsHash: 200, Partial: true, Files: [ @@ -576,44 +576,44 @@ private string GamePath(string relativePath) => private class InMemoryStatePersistence : IStatePersistence { // Avoids bootfiles checks on uninstall - private static readonly InternalState SkipBootfilesCheck = new( + private static readonly SavedState SkipBootfilesCheck = new( Install: new( Time: ValueNotUsed, - Mods: new Dictionary + Mods: new Dictionary { ["INIT"] = new(Time: null, FsHash: null, Partial: false, Files: []), } )); - private InternalState initState = SkipBootfilesCheck; - private InternalState? savedState; + private SavedState initState = SkipBootfilesCheck; + private SavedState? savedState; - public void InitState(InternalState state) => initState = state; + public void InitState(SavedState state) => initState = state; - public InternalState ReadState() => savedState ?? initState; + public SavedState ReadState() => savedState ?? initState; - public void WriteState(InternalState state) => savedState = state; + public void WriteState(SavedState state) => savedState = state; internal InMemoryStatePersistenceAssertions Should() => new(savedState); } private class InMemoryStatePersistenceAssertions { - private readonly InternalState? savedState; + private readonly SavedState? savedState; - internal InMemoryStatePersistenceAssertions(InternalState? savedState) + internal InMemoryStatePersistenceAssertions(SavedState? savedState) { this.savedState = savedState; } - internal void Be(InternalState expected) + internal void Be(SavedState expected) { var writtenState = WrittenState(); ValidateDateTime(expected.Install.Time, writtenState.Install.Time); HaveInstalled(expected.Install.Mods); } - internal void HaveInstalled(IReadOnlyDictionary expected) + internal void HaveInstalled(IReadOnlyDictionary expected) { var writtenState = WrittenState(); var actualMods = writtenState.Install.Mods; @@ -626,7 +626,7 @@ internal void HaveInstalled(IReadOnlyDictionary(mod.Key, mod.Value with { Time = actualTime }); + return new KeyValuePair(mod.Key, mod.Value with { Time = actualTime }); }); actualMods.Should().BeEquivalentTo(expectedMods); } @@ -637,7 +637,7 @@ internal void HaveInstalled(IEnumerable expected) writtenState.Install.Mods.Keys.Should().BeEquivalentTo(expected); } - private InternalState WrittenState() + private SavedState WrittenState() { savedState.Should().NotBeNull("State was not written"); return savedState!; @@ -651,7 +651,7 @@ private void ValidateDateTime(DateTime? expected, DateTime? actual) => internal void BeEmpty() { - savedState.Should().BeEquivalentTo(InternalState.Empty()); + savedState.Should().BeEquivalentTo(SavedState.Empty()); } internal void HaveNotBeenWritten() From 19a609625f1b366519dde5331c8b9bfbf365ea9b Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Fri, 26 Jul 2024 06:12:25 +0100 Subject: [PATCH 10/18] Extracting some callbacks as backup strategy --- src/Core/Backup/IBackupStrategy.cs | 2 +- src/Core/Backup/IBackupStrategyProvider.cs | 8 ++ src/Core/Backup/MoveFileBackupStrategy.cs | 13 ++- src/Core/Backup/SkipUpdatedBackupStrategy.cs | 60 ++++++++++++ src/Core/Backup/SuffixBackupStrategy.cs | 3 +- src/Core/ModInstaller.cs | 68 +++++--------- src/Core/Mods/BaseInstaller.cs | 8 +- src/Core/Mods/IInstaller.cs | 6 +- .../Backup/MoveFileBackupStrategyTest.cs | 66 ++++++++----- .../Backup/SkipUpdatedBackupStrategyTest.cs | 94 +++++++++++++++++++ .../Core.Tests/ModInstallerIntegrationTest.cs | 10 +- tests/Core.Tests/ModManagerIntegrationTest.cs | 4 +- 12 files changed, 255 insertions(+), 87 deletions(-) create mode 100644 src/Core/Backup/IBackupStrategyProvider.cs create mode 100644 src/Core/Backup/SkipUpdatedBackupStrategy.cs create mode 100644 tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs diff --git a/src/Core/Backup/IBackupStrategy.cs b/src/Core/Backup/IBackupStrategy.cs index b1c1788..6442496 100644 --- a/src/Core/Backup/IBackupStrategy.cs +++ b/src/Core/Backup/IBackupStrategy.cs @@ -3,6 +3,6 @@ public interface IBackupStrategy { public void PerformBackup(string fullPath); - public void RestoreBackup(string fullPath); + public bool RestoreBackup(string fullPath); public void DeleteBackup(string fullPath); } diff --git a/src/Core/Backup/IBackupStrategyProvider.cs b/src/Core/Backup/IBackupStrategyProvider.cs new file mode 100644 index 0000000..2fd7f35 --- /dev/null +++ b/src/Core/Backup/IBackupStrategyProvider.cs @@ -0,0 +1,8 @@ +using Core.State; + +namespace Core.Backup; + +public interface IBackupStrategyProvider +{ + IBackupStrategy BackupStrategy(DateTime? installationTime); +} diff --git a/src/Core/Backup/MoveFileBackupStrategy.cs b/src/Core/Backup/MoveFileBackupStrategy.cs index add60d8..7974e7e 100644 --- a/src/Core/Backup/MoveFileBackupStrategy.cs +++ b/src/Core/Backup/MoveFileBackupStrategy.cs @@ -13,6 +13,11 @@ public interface IBackupFileNaming private readonly IFileSystem fs; private readonly IBackupFileNaming backupFileNaming; + public MoveFileBackupStrategy(IBackupFileNaming backupFileNaming) : + this(new FileSystem(), backupFileNaming) + { + } + public MoveFileBackupStrategy(IFileSystem fs, IBackupFileNaming backupFileNaming) { this.fs = fs; @@ -41,13 +46,19 @@ public virtual void PerformBackup(string fullPath) } } - public void RestoreBackup(string fullPath) + public bool RestoreBackup(string fullPath) { + if (fs.File.Exists(fullPath)) + { + fs.File.Delete(fullPath); + } var backupFilePath = backupFileNaming.ToBackup(fullPath); if (fs.File.Exists(backupFilePath)) { fs.File.Move(backupFilePath, fullPath); } + + return true; } public void DeleteBackup(string fullPath) diff --git a/src/Core/Backup/SkipUpdatedBackupStrategy.cs b/src/Core/Backup/SkipUpdatedBackupStrategy.cs new file mode 100644 index 0000000..de12587 --- /dev/null +++ b/src/Core/Backup/SkipUpdatedBackupStrategy.cs @@ -0,0 +1,60 @@ +using System.IO.Abstractions; + +namespace Core.Backup; + +/// +/// It avoids restoring backups when game files have been updated by Steam. +/// +internal class SkipUpdatedBackupStrategy : IBackupStrategy +{ + internal class Provider : IBackupStrategyProvider + { + private readonly IBackupStrategy defaultStrategy; + + public Provider(IBackupStrategy defaultStrategy) + { + this.defaultStrategy = defaultStrategy; + } + + public IBackupStrategy BackupStrategy(DateTime? installationTime) => + new SkipUpdatedBackupStrategy(defaultStrategy, installationTime); + } + + private readonly IFileSystem fs; + private readonly IBackupStrategy inner; + private readonly DateTime? backupTimeUtc; + + internal SkipUpdatedBackupStrategy(IBackupStrategy backupStrategy, DateTime? backupTimeUtc) : + this(new FileSystem(), backupStrategy, backupTimeUtc) + { + } + + internal SkipUpdatedBackupStrategy(IFileSystem fs, IBackupStrategy backupStrategy, DateTime? backupTimeUtc) + { + this.fs = fs; + inner = backupStrategy; + this.backupTimeUtc = backupTimeUtc; + } + + public void DeleteBackup(string fullPath) => + inner.DeleteBackup(fullPath); + + public void PerformBackup(string fullPath) => + inner.PerformBackup(fullPath); + + public bool RestoreBackup(string fullPath) + { + if (FileWasOverwritten(fullPath)) + { + inner.DeleteBackup(fullPath); + return false; + } + + return inner.RestoreBackup(fullPath); + } + + private bool FileWasOverwritten(string fullPath) => + backupTimeUtc is not null && + fs.File.Exists(fullPath) && + fs.File.GetCreationTimeUtc(fullPath) > backupTimeUtc; +} diff --git a/src/Core/Backup/SuffixBackupStrategy.cs b/src/Core/Backup/SuffixBackupStrategy.cs index 2283732..edb6f42 100644 --- a/src/Core/Backup/SuffixBackupStrategy.cs +++ b/src/Core/Backup/SuffixBackupStrategy.cs @@ -12,8 +12,7 @@ private class BackupFileNaming : IBackupFileNaming public bool IsBackup(string fullPath) => fullPath.EndsWith(BackupSuffix); } - public SuffixBackupStrategy() : - base(new FileSystem(), new BackupFileNaming()) + public SuffixBackupStrategy() : base(new BackupFileNaming()) { } } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 6d74bcc..d8f1930 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -43,13 +43,16 @@ public interface IProgress } private readonly IInstallationFactory installationFactory; + private readonly IBackupStrategyProvider backupStrategyProvider; private readonly Matcher filesToInstallMatcher; - private readonly IBackupStrategy backupStrategy; - public ModInstaller(IInstallationFactory installationFactory, IBackupStrategy backupStrategy, IConfig config) + public ModInstaller( + IInstallationFactory installationFactory, + IBackupStrategy backupStrategy, + IConfig config) { this.installationFactory = installationFactory; - this.backupStrategy = backupStrategy; + backupStrategyProvider = new SkipUpdatedBackupStrategy.Provider(backupStrategy); filesToInstallMatcher = Matchers.ExcludingPatterns(config.ExcludedFromInstall); } @@ -75,11 +78,6 @@ private void UninstallPackages( if (currentState.Any()) { eventHandler.UninstallStart(); - var uninstallCallbacks = new ProcessingCallbacks - { - After = gamePath => backupStrategy.RestoreBackup(gamePath.Full), - NotAccepted = gamePath => backupStrategy.DeleteBackup(gamePath.Full) - }; foreach (var (packageName, modInstallationState) in currentState) { if (cancellationToken.IsCancellationRequested) @@ -87,15 +85,16 @@ private void UninstallPackages( break; } eventHandler.UninstallCurrent(packageName); + var backupStrategy = backupStrategyProvider.BackupStrategy(modInstallationState.Time); var filesLeft = modInstallationState.Files.ToHashSet(StringComparer.OrdinalIgnoreCase); try { UninstallFiles( installDir, filesLeft, - uninstallCallbacks - .AndAccept(SkipCreatedAfter(eventHandler, modInstallationState.Time)) - .AndFinally(_ => filesLeft.Remove(_.Relative)) + backupStrategy, + eventHandler, + new ProcessingCallbacks().AndFinally(_ => filesLeft.Remove(_.Relative)) ); } finally @@ -130,7 +129,12 @@ private void UninstallPackages( } } - private static void UninstallFiles(string dstPath, IReadOnlyCollection filePaths, ProcessingCallbacks callbacks) + private void UninstallFiles( + string dstPath, + IReadOnlyCollection filePaths, + IBackupStrategy backupStrategy, + IEventHandler eventHandler, + ProcessingCallbacks callbacks) { foreach (var relativePath in filePaths) { @@ -138,16 +142,16 @@ private static void UninstallFiles(string dstPath, IReadOnlyCollection f if (!callbacks.Accept(gamePath)) { + backupStrategy.DeleteBackup(gamePath.Full); callbacks.NotAccepted(gamePath); continue; } callbacks.Before(gamePath); - // Delete will fail if the parent directory does not exist - if (File.Exists(gamePath.Full)) + if (!backupStrategy.RestoreBackup(gamePath.Full)) { - File.Delete(gamePath.Full); + eventHandler.UninstallSkipModified(gamePath.Full); } callbacks.After(gamePath); @@ -198,11 +202,7 @@ private void InstallPackages( Accept = gamePath => Whitelisted(gamePath) && !installedFiles.Contains(gamePath.Relative), - Before = gamePath => - { - backupStrategy.PerformBackup(gamePath.Full); - installedFiles.Add(gamePath.Relative); - }, + Before = gamePath => installedFiles.Add(gamePath.Relative), After = EnsureNotCreatedAfter(DateTime.UtcNow) }; @@ -220,10 +220,11 @@ private void InstallPackages( break; } eventHandler.InstallCurrent(modPackage.PackageName); + var backupStrategy = backupStrategyProvider.BackupStrategy(null); var mod = installationFactory.ModInstaller(modPackage); try { - var modConfig = mod.Install(installDir, installCallbacks); + var modConfig = mod.Install(installDir, backupStrategy, installCallbacks); modConfigs.Add(modConfig); } finally @@ -239,7 +240,8 @@ private void InstallPackages( var bootfilesMod = CreateBootfilesMod(toInstall, eventHandler); try { - bootfilesMod.Install(installDir, installCallbacks); + var backupStrategy = backupStrategyProvider.BackupStrategy(null); + bootfilesMod.Install(installDir, backupStrategy, installCallbacks); bootfilesMod.PostProcessing(installDir, modConfigs, eventHandler); } finally @@ -265,24 +267,6 @@ private void InstallPackages( private Predicate Whitelisted => gamePath => filesToInstallMatcher.Match(gamePath.Relative).HasMatches; - private static Predicate SkipCreatedAfter(IEventHandler eventHandler, DateTime? dateTimeUtc) - { - if (dateTimeUtc is null) - { - return _ => true; - } - - return gamePath => - { - var proceed = !File.Exists(gamePath.Full) || File.GetCreationTimeUtc(gamePath.Full) <= dateTimeUtc; - if (!proceed) - { - eventHandler.UninstallSkipModified(gamePath.Full); - } - return proceed; - }; - } - private static Action EnsureNotCreatedAfter(DateTime dateTimeUtc) => gamePath => { if (File.Exists(gamePath.Full) && File.GetCreationTimeUtc(gamePath.Full) > dateTimeUtc) @@ -333,9 +317,9 @@ public BootfilesMod(IInstaller inner) public int? PackageFsHash => inner.PackageFsHash; - public ConfigEntries Install(string dstPath, ProcessingCallbacks callbacks) + public ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, ProcessingCallbacks callbacks) { - inner.Install(dstPath, callbacks); + inner.Install(dstPath, backupStrategy, callbacks); return ConfigEntries.Empty; } diff --git a/src/Core/Mods/BaseInstaller.cs b/src/Core/Mods/BaseInstaller.cs index 8a3d31c..1e3200d 100644 --- a/src/Core/Mods/BaseInstaller.cs +++ b/src/Core/Mods/BaseInstaller.cs @@ -1,10 +1,11 @@ -using Core.Utils; +using Core.Backup; +using Core.Utils; using Microsoft.Extensions.FileSystemGlobbing; namespace Core.Mods; /// -/// +/// /// /// Type used by the implementation during the install loop. internal abstract class BaseInstaller : IInstaller @@ -30,7 +31,7 @@ internal BaseInstaller(string packageName, int? packageFsHash, ITempDir tempDir, filesToConfigureMatcher = Matchers.ExcludingPatterns(config.ExcludedFromConfig); } - public ConfigEntries Install(string dstPath, ProcessingCallbacks callbacks) + public ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, ProcessingCallbacks callbacks) { if (Installed != IInstallation.State.NotInstalled) { @@ -62,6 +63,7 @@ public ConfigEntries Install(string dstPath, ProcessingCallbacks cal if (callbacks.Accept(gamePath)) { callbacks.Before(gamePath); + backupStrategy.PerformBackup(gamePath.Full); if (!removeFile) { Directory.GetParent(gamePath.Full)?.Create(); diff --git a/src/Core/Mods/IInstaller.cs b/src/Core/Mods/IInstaller.cs index 1b9171f..79d20d8 100644 --- a/src/Core/Mods/IInstaller.cs +++ b/src/Core/Mods/IInstaller.cs @@ -1,6 +1,8 @@ -namespace Core.Mods; +using Core.Backup; + +namespace Core.Mods; public interface IInstaller : IInstallation { - ConfigEntries Install(string dstPath, ProcessingCallbacks callbacks); + ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, ProcessingCallbacks callbacks); } diff --git a/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs b/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs index 0c21ef7..b19dcc7 100644 --- a/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs +++ b/tests/Core.Tests/Backup/MoveFileBackupStrategyTest.cs @@ -1,15 +1,14 @@ using System.IO.Abstractions.TestingHelpers; using Core.Backup; using FluentAssertions; -using static Core.Backup.MoveFileBackupStrategy; namespace Core.Tests.Backup; [IntegrationTest] public class MoveFileBackupStrategyTest { - private static readonly string OriginalFile = "original"; - private static readonly string OriginalContents = "something"; + private const string OriginalFile = "original"; + private const string OriginalContents = "something"; private readonly Mock backupFileNamingMock; @@ -29,9 +28,9 @@ public void PerformBackup_MovesOriginalToBackup() { { OriginalFile, OriginalContents }, }); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.PerformBackup(OriginalFile); + mfbs.PerformBackup(OriginalFile); fs.FileExists(OriginalFile).Should().BeFalse(); fs.File.ReadAllText(BackupFile).Should().Be(OriginalContents); @@ -42,9 +41,9 @@ public void PerformBackup_ErrorsIfNameIsBackupName() { var fs = new MockFileSystem(); backupFileNamingMock.Setup(_ => _.IsBackup(OriginalFile)).Returns(true); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.Invoking(_ => _.PerformBackup(OriginalFile)) + mfbs.Invoking(_ => _.PerformBackup(OriginalFile)) .Should().Throw(); fs.FileExists(BackupFile).Should().BeFalse(); @@ -54,9 +53,9 @@ public void PerformBackup_ErrorsIfNameIsBackupName() public void PerformBackup_SkipsBackupIfFileNotPresent() { var fs = new MockFileSystem(); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.PerformBackup(OriginalFile); + mfbs.PerformBackup(OriginalFile); fs.FileExists(BackupFile).Should().BeFalse(); } @@ -70,9 +69,9 @@ public void PerformBackup_KeepsExistingBackup() { OriginalFile, OriginalContents }, { BackupFile, oldBackupContents }, }); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.PerformBackup(OriginalFile); + mfbs.PerformBackup(OriginalFile); fs.FileExists(OriginalFile).Should().BeFalse(); fs.File.ReadAllText(BackupFile).Should().Be(oldBackupContents); @@ -85,42 +84,57 @@ public void RestoreBackup_MovesBackupToOriginal() { { BackupFile, OriginalContents}, }); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.RestoreBackup(OriginalFile); + mfbs.RestoreBackup(OriginalFile); fs.File.ReadAllText(OriginalFile).Should().Be(OriginalContents); fs.FileExists(BackupFile).Should().BeFalse(); } [Fact] - public void RestoreBackup_LeavesOriginalFileIfNoBackup() + public void RestoreBackup_OverwritesOriginalFile() { var fs = new MockFileSystem(new Dictionary { { OriginalFile, "other contents" }, + { BackupFile, OriginalContents}, }); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.RestoreBackup(OriginalFile); + mfbs.RestoreBackup(OriginalFile); - fs.FileExists(OriginalFile).Should().BeTrue(); + fs.File.ReadAllText(OriginalFile).Should().Be(OriginalContents); + fs.FileExists(BackupFile).Should().BeFalse(); } [Fact] - public void RestoreBackup_ErrorsIfOriginalFileExists() + public void RestoreBackup_WhenNoOriginalFile() { var fs = new MockFileSystem(new Dictionary { - { OriginalFile, "other contents" }, { BackupFile, OriginalContents}, }); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.Invoking(_ => _.RestoreBackup(OriginalFile)).Should().Throw(); + mfbs.RestoreBackup(OriginalFile); - fs.File.ReadAllText(OriginalFile).Should().NotBe(OriginalContents); - fs.FileExists(BackupFile).Should().BeTrue(); + fs.File.ReadAllText(OriginalFile).Should().Be(OriginalContents); + fs.FileExists(BackupFile).Should().BeFalse(); + } + + [Fact] + public void RestoreBackup_DeletesOriginalFileIfNoBackup() + { + var fs = new MockFileSystem(new Dictionary + { + { OriginalFile, "other contents" }, + }); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + + mfbs.RestoreBackup(OriginalFile); + + fs.FileExists(OriginalFile).Should().BeFalse(); } [Fact] @@ -130,13 +144,13 @@ public void DeleteBackup_RemovesBackupIfItExists() { { BackupFile, OriginalContents}, }); - var sbs = new MoveFileBackupStrategy(fs, BackupFileNaming); + var mfbs = new MoveFileBackupStrategy(fs, BackupFileNaming); - sbs.DeleteBackup(OriginalFile); + mfbs.DeleteBackup(OriginalFile); fs.FileExists(OriginalFile).Should().BeFalse(); fs.FileExists(BackupFile).Should().BeFalse(); - sbs.DeleteBackup(OriginalFile); // Check that it does not error + mfbs.DeleteBackup(OriginalFile); // Check that it does not error } } diff --git a/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs b/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs new file mode 100644 index 0000000..0c04683 --- /dev/null +++ b/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs @@ -0,0 +1,94 @@ +using System.IO.Abstractions.TestingHelpers; +using Core.Backup; +using FluentAssertions; +using static Core.Backup.MoveFileBackupStrategy; + +namespace Core.Tests.Backup; + +[IntegrationTest] +public class SkipUpdatedBackupStrategyTest +{ + private const string OriginalFile = "original"; + + private readonly Mock innerStategyMock; + + public SkipUpdatedBackupStrategyTest() + { + innerStategyMock = new(); + } + + [Fact] + public void PerformBackup_ProxiesCallToInnerStategy() + { + var fs = new MockFileSystem(new Dictionary()); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, null); + + subs.PerformBackup(OriginalFile); + + innerStategyMock.Verify(_ => _.PerformBackup(OriginalFile)); + } + + [Fact] + public void DeleteBackup_ProxiesCallToInnerStategy() + { + var fs = new MockFileSystem(new Dictionary()); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, null); + + subs.DeleteBackup(OriginalFile); + + innerStategyMock.Verify(_ => _.DeleteBackup(OriginalFile)); + } + + [Fact] + public void RestoreBackup_ProxiesCallToInnerStategyIfNoBackupTime() + { + var fs = new MockFileSystem(new Dictionary()); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, null); + + subs.RestoreBackup(OriginalFile); + + innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile)); + } + + [Fact] + public void RestoreBackup_ProxiesCallToInnerStategyIfNoOriginalFile() + { + var fs = new MockFileSystem(new Dictionary()); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, DateTime.UtcNow); + + subs.RestoreBackup(OriginalFile); + + innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile)); + } + + [Fact] + public void RestoreBackup_DeletesBackupIfOverwritten() + { + var fileCreationTime = DateTime.UtcNow; + var backupTime = fileCreationTime.Subtract(TimeSpan.FromSeconds(1)); + var fs = new MockFileSystem(new Dictionary + { + { OriginalFile, new MockFileData("") { CreationTime = fileCreationTime } }, + }); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, backupTime); + + subs.RestoreBackup(OriginalFile); + + innerStategyMock.Verify(_ => _.DeleteBackup(OriginalFile)); + } + + [Fact] + public void RestoreBackup_ProxiesCallToInnerStategyIfNotOverwritten() + { + var backupTime = DateTime.UtcNow; + var fs = new MockFileSystem(new Dictionary + { + { OriginalFile, new MockFileData("") { CreationTime = backupTime } }, + }); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, backupTime); + + subs.RestoreBackup(OriginalFile); + + innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile)); + } +} diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index 3607afc..4a704ec 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -26,7 +26,6 @@ internal InstallationResult(IInstallation installation) : this( private readonly Mock installationFactoryMock; private readonly Mock backupStrategyMock; - private readonly Mock config; private readonly ModInstaller modInstaller; private readonly Mock eventHandlerMock; @@ -37,7 +36,7 @@ public ModInstallerIntegrationTest() { installationFactoryMock = new Mock(); backupStrategyMock = new Mock(); - config = new Mock(); + Mock config = new(); modInstaller = new ModInstaller( installationFactoryMock.Object, backupStrategyMock.Object, @@ -65,9 +64,6 @@ public void Apply_NoMods() [Fact] public void Apply_UninstallsMods() { - // TODO Introduce interface to delete file as part of restoring backup - CreateTestFile("AF"); - modInstaller.Apply( new Dictionary{ ["A"] = new( @@ -86,14 +82,12 @@ public void Apply_UninstallsMods() ["A"] = new(42, [], IInstallation.State.NotInstalled), }); - // TODO see above - File.Exists(TestPath("AF")).Should().BeFalse(); - backupStrategyMock.Verify(_ => _.RestoreBackup(TestPath("AF"))); backupStrategyMock.VerifyNoOtherCalls(); eventHandlerMock.Verify(_ => _.UninstallStart()); eventHandlerMock.Verify(_ => _.UninstallCurrent("A")); + eventHandlerMock.Verify(_ => _.UninstallSkipModified(TestPath("AF"))); eventHandlerMock.Verify(_ => _.UninstallEnd()); eventHandlerMock.Verify(_ => _.InstallNoMods()); eventHandlerMock.Verify(_ => _.ProgressUpdate(It.IsAny())); diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/ModManagerIntegrationTest.cs index 159322b..b23c770 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/ModManagerIntegrationTest.cs @@ -34,7 +34,7 @@ public class ModManagerIntegrationTest : AbstractFilesystemTest private readonly ModManager modManager; - public ModManagerIntegrationTest() : base() + public ModManagerIntegrationTest() { gameDir = testDir.CreateSubdirectory("Game"); modsDir = testDir.CreateSubdirectory("Packages"); @@ -131,7 +131,7 @@ public void Uninstall_SkipsFilesCreatedAfterInstallation() modManager.UninstallAllMods(eventHandlerMock.Object); - File.Exists(GamePath("ModFile")).Should().BeFalse(); + File.Exists(GamePath("ModFile")).Should().BeFalse(); // FIXME File.Exists(GamePath("RecreatedFile")).Should().BeTrue(); persistedState.Should().BeEmpty(); } From 6a9abeef4a1981168b421648c063930cfb4b9979 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sat, 28 Dec 2024 10:27:06 +0000 Subject: [PATCH 11/18] Inlined uninstallation --- src/Core/ModInstaller.cs | 47 +++++++++------------------------------- 1 file changed, 10 insertions(+), 37 deletions(-) diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index d8f1930..f63fd90 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -89,13 +89,16 @@ private void UninstallPackages( var filesLeft = modInstallationState.Files.ToHashSet(StringComparer.OrdinalIgnoreCase); try { - UninstallFiles( - installDir, - filesLeft, - backupStrategy, - eventHandler, - new ProcessingCallbacks().AndFinally(_ => filesLeft.Remove(_.Relative)) - ); + foreach (var relativePath in modInstallationState.Files) + { + var gamePath = new RootedPath(installDir, relativePath); + if (!backupStrategy.RestoreBackup(gamePath.Full)) + { + eventHandler.UninstallSkipModified(gamePath.Full); + } + filesLeft.Remove(gamePath.Relative); + } + DeleteEmptyDirectories(installDir, modInstallationState.Files); } finally { @@ -129,36 +132,6 @@ private void UninstallPackages( } } - private void UninstallFiles( - string dstPath, - IReadOnlyCollection filePaths, - IBackupStrategy backupStrategy, - IEventHandler eventHandler, - ProcessingCallbacks callbacks) - { - foreach (var relativePath in filePaths) - { - var gamePath = new RootedPath(dstPath, relativePath); - - if (!callbacks.Accept(gamePath)) - { - backupStrategy.DeleteBackup(gamePath.Full); - callbacks.NotAccepted(gamePath); - continue; - } - - callbacks.Before(gamePath); - - if (!backupStrategy.RestoreBackup(gamePath.Full)) - { - eventHandler.UninstallSkipModified(gamePath.Full); - } - - callbacks.After(gamePath); - } - DeleteEmptyDirectories(dstPath, filePaths); - } - private static void DeleteEmptyDirectories(string dstRootPath, IReadOnlyCollection filePaths) { var dirs = filePaths From d05c804e0b9703d50e2d14b10763ea5bcfcbf7ae Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Sat, 28 Dec 2024 10:45:51 +0000 Subject: [PATCH 12/18] Install first of multiple bootfiles Removes unnecessary complexity and error message --- src/Core/BaseEventLogger.cs | 8 ------- src/Core/ModInstaller.cs | 21 ++++++------------- tests/Core.Tests/ModManagerIntegrationTest.cs | 7 +++---- 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/Core/BaseEventLogger.cs b/src/Core/BaseEventLogger.cs index 3560bb6..3fcb333 100644 --- a/src/Core/BaseEventLogger.cs +++ b/src/Core/BaseEventLogger.cs @@ -27,14 +27,6 @@ public void PostProcessingStart() => LogMessage("Post-processing:"); public void ExtractingBootfiles(string? packageName) => LogMessage($"Extracting bootfiles from {packageName ?? "game"}"); - public void ExtractingBootfilesErrorMultiple(IReadOnlyCollection bootfilesPackageNames) - { - LogMessage("Multiple bootfiles found:"); - foreach (var packageName in bootfilesPackageNames) - { - LogMessage($"- {packageName}"); - } - } public void PostProcessingVehicles() => LogMessage("- Appending crd file entries"); public void PostProcessingTracks() => diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index f63fd90..468a69b 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -24,7 +24,6 @@ public interface IEventHandler : IProgress void PostProcessingNotRequired(); void PostProcessingStart(); void ExtractingBootfiles(string? packageName); - void ExtractingBootfilesErrorMultiple(IReadOnlyCollection bootfilesPackageNames); void PostProcessingVehicles(); void PostProcessingTracks(); void PostProcessingDrivelines(); @@ -250,22 +249,14 @@ private static Action EnsureNotCreatedAfter(DateTime dateTimeUtc) => private BootfilesMod CreateBootfilesMod(IReadOnlyCollection packages, IEventHandler eventHandler) { - var bootfilesPackages = packages - .Where(_ => BootfilesManager.IsBootFiles(_.PackageName)); - switch (bootfilesPackages.Count()) + var bootfilesPackage = packages.FirstOrDefault(p => BootfilesManager.IsBootFiles(p.PackageName)); + if (bootfilesPackage is null) { - case 0: - eventHandler.ExtractingBootfiles(null); - return new BootfilesMod(installationFactory.GeneratedBootfilesInstaller()); - case 1: - var modPackage = bootfilesPackages.First(); - eventHandler.ExtractingBootfiles(modPackage.PackageName); - return new BootfilesMod(installationFactory.ModInstaller(modPackage)); - default: - var bootfilesPackageNames = bootfilesPackages.Select(_ => _.PackageName).ToImmutableList(); - eventHandler.ExtractingBootfilesErrorMultiple(bootfilesPackageNames); - throw new Exception("Too many bootfiles found"); + eventHandler.ExtractingBootfiles(null); + return new BootfilesMod(installationFactory.GeneratedBootfilesInstaller()); } + eventHandler.ExtractingBootfiles(bootfilesPackage.PackageName); + return new BootfilesMod(installationFactory.ModInstaller(bootfilesPackage)); } private class BootfilesMod : IInstaller diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/ModManagerIntegrationTest.cs index b23c770..d6c4128 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/ModManagerIntegrationTest.cs @@ -510,7 +510,7 @@ public void Install_ExtractsBootfilesFromGameByDefault() } [Fact] - public void Install_RejectsMultipleCustomBootfiles() + public void Install_ChoosesFirstOfMultipleCustomBootfiles() { modRepositoryMock.Setup(_ => _.ListEnabledMods()).Returns([ CreateModArchive(100, [Path.Combine(DirAtRoot, "Foo.crd")]), @@ -518,10 +518,9 @@ public void Install_RejectsMultipleCustomBootfiles() CreateCustomBootfiles(901) ]); - modManager.Invoking(_ => _.InstallEnabledMods(eventHandlerMock.Object)) - .Should().Throw().WithMessage("*many bootfiles*"); + modManager.InstallEnabledMods(eventHandlerMock.Object); - persistedState.Should().HaveInstalled(["Package100"]); + persistedState.Should().HaveInstalled(["Package100", "__bootfiles900"]); } #region Utility methods From 79fb2d3b4e3eef0a48ca88f7d4b494e8d55fc65e Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Wed, 1 Jan 2025 08:05:12 +0000 Subject: [PATCH 13/18] Move Core public APIs into separate package --- src/CLI/ConsoleEventLogger.cs | 2 +- src/CLI/Program.cs | 2 +- src/Core/{ => API}/BaseEventLogger.cs | 2 +- src/Core/{ => API}/Config.cs | 2 +- src/Core/{ => API}/IModManager.cs | 2 +- src/Core/{ => API}/Init.cs | 2 +- src/Core/{ => API}/ModManager.cs | 4 ++-- src/Core/{ => API}/ModState.cs | 2 +- src/GUI/App.xaml.cs | 4 ++-- src/GUI/MainWindow.xaml.cs | 2 +- src/GUI/ModVM.cs | 1 + tests/Core.Tests/{ => API}/ModManagerIntegrationTest.cs | 8 ++++---- tests/Core.Tests/{ => Base}/AbstractFilesystemTest.cs | 2 +- tests/Core.Tests/ModInstallerIntegrationTest.cs | 1 + tests/Core.Tests/Mods/ModRepositoryIntegrationTest.cs | 1 + 15 files changed, 20 insertions(+), 17 deletions(-) rename src/Core/{ => API}/BaseEventLogger.cs (98%) rename src/Core/{ => API}/Config.cs (98%) rename src/Core/{ => API}/IModManager.cs (95%) rename src/Core/{ => API}/Init.cs (97%) rename src/Core/{ => API}/ModManager.cs (99%) rename src/Core/{ => API}/ModState.cs (87%) rename tests/Core.Tests/{ => API}/ModManagerIntegrationTest.cs (99%) rename tests/Core.Tests/{ => Base}/AbstractFilesystemTest.cs (97%) diff --git a/src/CLI/ConsoleEventLogger.cs b/src/CLI/ConsoleEventLogger.cs index 1f2de1a..b22b3e3 100644 --- a/src/CLI/ConsoleEventLogger.cs +++ b/src/CLI/ConsoleEventLogger.cs @@ -1,4 +1,4 @@ -using Core; +using Core.API; using Core.Utils; namespace AMS2CM.CLI; diff --git a/src/CLI/Program.cs b/src/CLI/Program.cs index d06c0f9..25390e5 100644 --- a/src/CLI/Program.cs +++ b/src/CLI/Program.cs @@ -1,5 +1,5 @@ using AMS2CM.CLI; -using Core; +using Core.API; try { diff --git a/src/Core/BaseEventLogger.cs b/src/Core/API/BaseEventLogger.cs similarity index 98% rename from src/Core/BaseEventLogger.cs rename to src/Core/API/BaseEventLogger.cs index 3fcb333..db31033 100644 --- a/src/Core/BaseEventLogger.cs +++ b/src/Core/API/BaseEventLogger.cs @@ -1,6 +1,6 @@ using Core.Utils; -namespace Core; +namespace Core.API; /// /// This class is here because of the CLI. Move it into the GUI once the CLI diff --git a/src/Core/Config.cs b/src/Core/API/Config.cs similarity index 98% rename from src/Core/Config.cs rename to src/Core/API/Config.cs index ed800d8..e859878 100644 --- a/src/Core/Config.cs +++ b/src/Core/API/Config.cs @@ -2,7 +2,7 @@ using Core.SoftwareUpdates; using Microsoft.Extensions.Configuration; -namespace Core; +namespace Core.API; public class Config { diff --git a/src/Core/IModManager.cs b/src/Core/API/IModManager.cs similarity index 95% rename from src/Core/IModManager.cs rename to src/Core/API/IModManager.cs index aa96e47..bb59be1 100644 --- a/src/Core/IModManager.cs +++ b/src/Core/API/IModManager.cs @@ -1,4 +1,4 @@ -namespace Core; +namespace Core.API; public interface IModManager { diff --git a/src/Core/Init.cs b/src/Core/API/Init.cs similarity index 97% rename from src/Core/Init.cs rename to src/Core/API/Init.cs index 0c590c0..a3b4770 100644 --- a/src/Core/Init.cs +++ b/src/Core/API/Init.cs @@ -4,7 +4,7 @@ using Core.Mods; using Core.State; -namespace Core; +namespace Core.API; public static class Init { diff --git a/src/Core/ModManager.cs b/src/Core/API/ModManager.cs similarity index 99% rename from src/Core/ModManager.cs rename to src/Core/API/ModManager.cs index 2f22c95..6d26848 100644 --- a/src/Core/ModManager.cs +++ b/src/Core/API/ModManager.cs @@ -3,9 +3,9 @@ using Core.Mods; using Core.State; using Core.Utils; -using static Core.IModManager; +using static Core.API.IModManager; -namespace Core; +namespace Core.API; internal class ModManager : IModManager { diff --git a/src/Core/ModState.cs b/src/Core/API/ModState.cs similarity index 87% rename from src/Core/ModState.cs rename to src/Core/API/ModState.cs index b475ac1..f1f75fb 100644 --- a/src/Core/ModState.cs +++ b/src/Core/API/ModState.cs @@ -1,4 +1,4 @@ -namespace Core; +namespace Core.API; public record ModState( string PackageName, diff --git a/src/GUI/App.xaml.cs b/src/GUI/App.xaml.cs index 8b490a6..2f635b7 100644 --- a/src/GUI/App.xaml.cs +++ b/src/GUI/App.xaml.cs @@ -1,7 +1,7 @@ -using Core; +using Core.API; using Core.SoftwareUpdates; using Microsoft.UI.Xaml; -using static Core.IModManager; +using static Core.API.IModManager; namespace AMS2CM.GUI; diff --git a/src/GUI/MainWindow.xaml.cs b/src/GUI/MainWindow.xaml.cs index 1289b09..bfafbaa 100644 --- a/src/GUI/MainWindow.xaml.cs +++ b/src/GUI/MainWindow.xaml.cs @@ -1,5 +1,5 @@ using System.Collections.ObjectModel; -using Core; +using Core.API; using Core.SoftwareUpdates; using Core.Utils; using Microsoft.UI.Xaml; diff --git a/src/GUI/ModVM.cs b/src/GUI/ModVM.cs index a351084..c54a6fb 100644 --- a/src/GUI/ModVM.cs +++ b/src/GUI/ModVM.cs @@ -1,5 +1,6 @@ using System.ComponentModel; using Core; +using Core.API; namespace AMS2CM.GUI; diff --git a/tests/Core.Tests/ModManagerIntegrationTest.cs b/tests/Core.Tests/API/ModManagerIntegrationTest.cs similarity index 99% rename from tests/Core.Tests/ModManagerIntegrationTest.cs rename to tests/Core.Tests/API/ModManagerIntegrationTest.cs index d6c4128..ab473f5 100644 --- a/tests/Core.Tests/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/API/ModManagerIntegrationTest.cs @@ -1,13 +1,14 @@ using System.IO.Compression; +using Core.API; using Core.Backup; using Core.Games; using Core.IO; using Core.Mods; using Core.State; -using Core.Utils; +using Core.Tests.Base; using FluentAssertions; -namespace Core.Tests; +namespace Core.Tests.API; public class ModManagerIntegrationTest : AbstractFilesystemTest { @@ -30,7 +31,6 @@ public class ModManagerIntegrationTest : AbstractFilesystemTest private readonly Mock eventHandlerMock = new(); private readonly InMemoryStatePersistence persistedState; - private readonly InstallationFactory installationFactory; private readonly ModManager modManager; @@ -47,7 +47,7 @@ public ModManagerIntegrationTest() DirsAtRoot = [DirAtRoot], ExcludedFromInstall = [$"**\\{FileExcludedFromInstall}"] }; - installationFactory = new InstallationFactory( + var installationFactory = new InstallationFactory( gameMock.Object, tempDir, modInstallConfig); diff --git a/tests/Core.Tests/AbstractFilesystemTest.cs b/tests/Core.Tests/Base/AbstractFilesystemTest.cs similarity index 97% rename from tests/Core.Tests/AbstractFilesystemTest.cs rename to tests/Core.Tests/Base/AbstractFilesystemTest.cs index 36457c3..e84e854 100644 --- a/tests/Core.Tests/AbstractFilesystemTest.cs +++ b/tests/Core.Tests/Base/AbstractFilesystemTest.cs @@ -1,4 +1,4 @@ -namespace Core.Tests; +namespace Core.Tests.Base; [IntegrationTest] public abstract class AbstractFilesystemTest : IDisposable diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index 4a704ec..17d90a0 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -1,6 +1,7 @@ using Core.Backup; using Core.Mods; using Core.State; +using Core.Tests.Base; using Core.Utils; using FluentAssertions; diff --git a/tests/Core.Tests/Mods/ModRepositoryIntegrationTest.cs b/tests/Core.Tests/Mods/ModRepositoryIntegrationTest.cs index e48c950..851569d 100644 --- a/tests/Core.Tests/Mods/ModRepositoryIntegrationTest.cs +++ b/tests/Core.Tests/Mods/ModRepositoryIntegrationTest.cs @@ -1,4 +1,5 @@ using Core.Mods; +using Core.Tests.Base; using FluentAssertions; namespace Core.Tests.Mods; From 0036043cca4b56f3451a606fe9521dd50bd92a6a Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Wed, 1 Jan 2025 08:10:39 +0000 Subject: [PATCH 14/18] Removed null check in version parsing Signature of System.Version.Parse is wrong! --- src/Core/SoftwareUpdates/GitHubUpdateChecker.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Core/SoftwareUpdates/GitHubUpdateChecker.cs b/src/Core/SoftwareUpdates/GitHubUpdateChecker.cs index 99edb01..306bdaf 100644 --- a/src/Core/SoftwareUpdates/GitHubUpdateChecker.cs +++ b/src/Core/SoftwareUpdates/GitHubUpdateChecker.cs @@ -25,9 +25,10 @@ public async Task CheckUpdateAvailable() var client = new GitHubClient(new ProductHeaderValue(config.GitHubClientApp)); var release = await client.Repository.Release.GetLatest(config.GitHubOwner, config.GitHubRepo); + // Note: Version.Parse breaks the contract and can return null! var latestVersion = Version.Parse(release.Name); var currentVersion = Version.Parse(GitVersionInformation.MajorMinorPatch); - return latestVersion is not null && currentVersion < latestVersion; + return currentVersion < latestVersion; } catch { From 89655b5714bf9d8d8b8c15af854dd1ed68e32ac2 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Wed, 1 Jan 2025 09:07:57 +0000 Subject: [PATCH 15/18] Move ensure created after to skip backup strategy --- src/Core/API/ModManager.cs | 15 +++--- src/Core/Backup/IBackupStrategyProvider.cs | 2 +- .../Backup/IInstallationBackupStrategy.cs | 11 +++++ src/Core/Backup/SkipUpdatedBackupStrategy.cs | 47 ++++++++++++------- src/Core/IModInstaller.cs | 8 +++- src/Core/ModInstaller.cs | 29 ++++-------- src/Core/Mods/BaseInstaller.cs | 10 ++-- src/Core/Mods/IInstaller.cs | 2 +- .../Backup/SkipUpdatedBackupStrategyTest.cs | 34 ++++++++++---- .../Core.Tests/ModInstallerIntegrationTest.cs | 2 +- 10 files changed, 98 insertions(+), 62 deletions(-) create mode 100644 src/Core/Backup/IInstallationBackupStrategy.cs diff --git a/src/Core/API/ModManager.cs b/src/Core/API/ModManager.cs index 6d26848..a6a61c9 100644 --- a/src/Core/API/ModManager.cs +++ b/src/Core/API/ModManager.cs @@ -173,14 +173,13 @@ private void UpdateMods(IReadOnlyCollection packages, IEventHandler case IInstallation.State.Installed: case IInstallation.State.PartiallyInstalled: currentState.Upsert(modInstallation.PackageName, - existing => new( - Time: existing.Time, - FsHash: existing.FsHash, - Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, - Files: modInstallation.InstalledFiles - ), - () => new( - Time: DateTime.UtcNow, + existing => existing with + { + Partial = modInstallation.Installed == IInstallation.State.PartiallyInstalled, + Files = modInstallation.InstalledFiles + }, + () => new ModInstallationState( + Time: DateTime.Now, FsHash: modInstallation.PackageFsHash, Partial: modInstallation.Installed == IInstallation.State.PartiallyInstalled, Files: modInstallation.InstalledFiles diff --git a/src/Core/Backup/IBackupStrategyProvider.cs b/src/Core/Backup/IBackupStrategyProvider.cs index 2fd7f35..df0daa2 100644 --- a/src/Core/Backup/IBackupStrategyProvider.cs +++ b/src/Core/Backup/IBackupStrategyProvider.cs @@ -4,5 +4,5 @@ namespace Core.Backup; public interface IBackupStrategyProvider { - IBackupStrategy BackupStrategy(DateTime? installationTime); + IInstallationBackupStrategy BackupStrategy(DateTime? backupTimeUtc); } diff --git a/src/Core/Backup/IInstallationBackupStrategy.cs b/src/Core/Backup/IInstallationBackupStrategy.cs new file mode 100644 index 0000000..499c666 --- /dev/null +++ b/src/Core/Backup/IInstallationBackupStrategy.cs @@ -0,0 +1,11 @@ +using Core.Mods; + +namespace Core.Backup; + +public interface IInstallationBackupStrategy +{ + public void PerformBackup(RootedPath path); + public bool RestoreBackup(RootedPath path); + public void DeleteBackup(RootedPath path); + public void AfterInstall(RootedPath path); +} diff --git a/src/Core/Backup/SkipUpdatedBackupStrategy.cs b/src/Core/Backup/SkipUpdatedBackupStrategy.cs index de12587..029f7f0 100644 --- a/src/Core/Backup/SkipUpdatedBackupStrategy.cs +++ b/src/Core/Backup/SkipUpdatedBackupStrategy.cs @@ -1,11 +1,12 @@ using System.IO.Abstractions; +using Core.Mods; namespace Core.Backup; /// /// It avoids restoring backups when game files have been updated by Steam. /// -internal class SkipUpdatedBackupStrategy : IBackupStrategy +internal class SkipUpdatedBackupStrategy : IInstallationBackupStrategy { internal class Provider : IBackupStrategyProvider { @@ -16,45 +17,59 @@ public Provider(IBackupStrategy defaultStrategy) this.defaultStrategy = defaultStrategy; } - public IBackupStrategy BackupStrategy(DateTime? installationTime) => - new SkipUpdatedBackupStrategy(defaultStrategy, installationTime); + public IInstallationBackupStrategy BackupStrategy(DateTime? backupTimeUtc) => + new SkipUpdatedBackupStrategy(defaultStrategy, backupTimeUtc); } private readonly IFileSystem fs; private readonly IBackupStrategy inner; private readonly DateTime? backupTimeUtc; - internal SkipUpdatedBackupStrategy(IBackupStrategy backupStrategy, DateTime? backupTimeUtc) : + private SkipUpdatedBackupStrategy( + IBackupStrategy backupStrategy, + DateTime? backupTimeUtc) : this(new FileSystem(), backupStrategy, backupTimeUtc) { } - internal SkipUpdatedBackupStrategy(IFileSystem fs, IBackupStrategy backupStrategy, DateTime? backupTimeUtc) + internal SkipUpdatedBackupStrategy( + IFileSystem fs, + IBackupStrategy backupStrategy, + DateTime? backupTimeUtc) { this.fs = fs; inner = backupStrategy; this.backupTimeUtc = backupTimeUtc; } - public void DeleteBackup(string fullPath) => - inner.DeleteBackup(fullPath); + public void DeleteBackup(RootedPath path) => + inner.DeleteBackup(path.Full); - public void PerformBackup(string fullPath) => - inner.PerformBackup(fullPath); + public void PerformBackup(RootedPath path) => + inner.PerformBackup(path.Full); - public bool RestoreBackup(string fullPath) + public bool RestoreBackup(RootedPath path) { - if (FileWasOverwritten(fullPath)) + if (FileWasOverwritten(path)) { - inner.DeleteBackup(fullPath); + inner.DeleteBackup(path.Full); return false; } - return inner.RestoreBackup(fullPath); + return inner.RestoreBackup(path.Full); } - private bool FileWasOverwritten(string fullPath) => + private bool FileWasOverwritten(RootedPath path) => backupTimeUtc is not null && - fs.File.Exists(fullPath) && - fs.File.GetCreationTimeUtc(fullPath) > backupTimeUtc; + fs.File.Exists(path.Full) && + fs.File.GetCreationTimeUtc(path.Full) > backupTimeUtc; + + public void AfterInstall(RootedPath path) + { + var now = DateTime.UtcNow; + if (fs.File.Exists(path.Full) && fs.File.GetCreationTimeUtc(path.Full) > now) + { + fs.File.SetCreationTimeUtc(path.Full, now); + } + } } diff --git a/src/Core/IModInstaller.cs b/src/Core/IModInstaller.cs index 5b00841..0ea4444 100644 --- a/src/Core/IModInstaller.cs +++ b/src/Core/IModInstaller.cs @@ -5,5 +5,11 @@ namespace Core; public interface IModInstaller { - void Apply(IReadOnlyDictionary currentState, IReadOnlyCollection packages, string installDir, Action afterInstall, ModInstaller.IEventHandler eventHandler, CancellationToken cancellationToken); + void Apply( + IReadOnlyDictionary currentState, + IReadOnlyCollection packages, + string installDir, + Action afterInstall, + ModInstaller.IEventHandler eventHandler, + CancellationToken cancellationToken); } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index 468a69b..f110d21 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -1,5 +1,4 @@ -using System.Collections.Immutable; -using Core.Backup; +using Core.Backup; using Core.Mods; using Core.State; using Core.Utils; @@ -91,9 +90,9 @@ private void UninstallPackages( foreach (var relativePath in modInstallationState.Files) { var gamePath = new RootedPath(installDir, relativePath); - if (!backupStrategy.RestoreBackup(gamePath.Full)) + if (!backupStrategy.RestoreBackup(gamePath)) { - eventHandler.UninstallSkipModified(gamePath.Full); + eventHandler.UninstallSkipModified(gamePath.Relative); } filesLeft.Remove(gamePath.Relative); } @@ -170,13 +169,11 @@ private void InstallPackages( var modConfigs = new List(); var installedFiles = new HashSet(StringComparer.OrdinalIgnoreCase); var installCallbacks = new ProcessingCallbacks - { - Accept = gamePath => - Whitelisted(gamePath) && - !installedFiles.Contains(gamePath.Relative), - Before = gamePath => installedFiles.Add(gamePath.Relative), - After = EnsureNotCreatedAfter(DateTime.UtcNow) - }; + { + Accept = gamePath => !installedFiles.Contains(gamePath.Relative), + Before = gamePath => installedFiles.Add(gamePath.Relative), + } + .AndAccept(gamePath => Whitelisted(gamePath)); // Increase by one in case bootfiles are needed and another one to show that something is happening var progress = new PercentOfTotal(modPackages.Count() + 2); @@ -239,14 +236,6 @@ private void InstallPackages( private Predicate Whitelisted => gamePath => filesToInstallMatcher.Match(gamePath.Relative).HasMatches; - private static Action EnsureNotCreatedAfter(DateTime dateTimeUtc) => gamePath => - { - if (File.Exists(gamePath.Full) && File.GetCreationTimeUtc(gamePath.Full) > dateTimeUtc) - { - File.SetCreationTimeUtc(gamePath.Full, dateTimeUtc); - } - }; - private BootfilesMod CreateBootfilesMod(IReadOnlyCollection packages, IEventHandler eventHandler) { var bootfilesPackage = packages.FirstOrDefault(p => BootfilesManager.IsBootFiles(p.PackageName)); @@ -281,7 +270,7 @@ public BootfilesMod(IInstaller inner) public int? PackageFsHash => inner.PackageFsHash; - public ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, ProcessingCallbacks callbacks) + public ConfigEntries Install(string dstPath, IInstallationBackupStrategy backupStrategy, ProcessingCallbacks callbacks) { inner.Install(dstPath, backupStrategy, callbacks); return ConfigEntries.Empty; diff --git a/src/Core/Mods/BaseInstaller.cs b/src/Core/Mods/BaseInstaller.cs index 1e3200d..b75ed94 100644 --- a/src/Core/Mods/BaseInstaller.cs +++ b/src/Core/Mods/BaseInstaller.cs @@ -31,7 +31,7 @@ internal BaseInstaller(string packageName, int? packageFsHash, ITempDir tempDir, filesToConfigureMatcher = Matchers.ExcludingPatterns(config.ExcludedFromConfig); } - public ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, ProcessingCallbacks callbacks) + public ConfigEntries Install(string dstPath, IInstallationBackupStrategy backupStrategy, ProcessingCallbacks callbacks) { if (Installed != IInstallation.State.NotInstalled) { @@ -63,13 +63,14 @@ public ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, Pro if (callbacks.Accept(gamePath)) { callbacks.Before(gamePath); - backupStrategy.PerformBackup(gamePath.Full); + backupStrategy.PerformBackup(gamePath); if (!removeFile) { Directory.GetParent(gamePath.Full)?.Create(); InstallFile(gamePath, context); } installedFiles.Add(gamePath.Relative); + backupStrategy.AfterInstall(gamePath); callbacks.After(gamePath); } else @@ -98,7 +99,7 @@ public ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, Pro protected abstract void InstallFile(RootedPath destinationPath, TPassthrough context); - protected static (string, bool) NeedsRemoving(string filePath) + private static (string, bool) NeedsRemoving(string filePath) { return filePath.EndsWith(BaseInstaller.RemoveFileSuffix) ? (filePath.RemoveSuffix(BaseInstaller.RemoveFileSuffix).Trim(), true) : @@ -108,8 +109,7 @@ protected static (string, bool) NeedsRemoving(string filePath) private ConfigEntries GenerateConfig() { var gameSupportedMod = FileEntriesToConfigure() - .Where(p => p.StartsWith(BaseInstaller.GameSupportedModDirectory)) - .Any(); + .Any(p => p.StartsWith(BaseInstaller.GameSupportedModDirectory)); return gameSupportedMod ? ConfigEntries.Empty : new(CrdFileEntries(), TrdFileEntries(), FindDrivelineRecords()); diff --git a/src/Core/Mods/IInstaller.cs b/src/Core/Mods/IInstaller.cs index 79d20d8..499e4cd 100644 --- a/src/Core/Mods/IInstaller.cs +++ b/src/Core/Mods/IInstaller.cs @@ -4,5 +4,5 @@ namespace Core.Mods; public interface IInstaller : IInstallation { - ConfigEntries Install(string dstPath, IBackupStrategy backupStrategy, ProcessingCallbacks callbacks); + ConfigEntries Install(string dstPath, IInstallationBackupStrategy backupStrategy, ProcessingCallbacks callbacks); } diff --git a/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs b/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs index 0c04683..1405ca9 100644 --- a/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs +++ b/tests/Core.Tests/Backup/SkipUpdatedBackupStrategyTest.cs @@ -1,5 +1,6 @@ using System.IO.Abstractions.TestingHelpers; using Core.Backup; +using Core.Mods; using FluentAssertions; using static Core.Backup.MoveFileBackupStrategy; @@ -8,7 +9,7 @@ namespace Core.Tests.Backup; [IntegrationTest] public class SkipUpdatedBackupStrategyTest { - private const string OriginalFile = "original"; + private readonly RootedPath OriginalFile = new("root", "original"); private readonly Mock innerStategyMock; @@ -25,7 +26,7 @@ public void PerformBackup_ProxiesCallToInnerStategy() subs.PerformBackup(OriginalFile); - innerStategyMock.Verify(_ => _.PerformBackup(OriginalFile)); + innerStategyMock.Verify(_ => _.PerformBackup(OriginalFile.Full)); } [Fact] @@ -36,7 +37,7 @@ public void DeleteBackup_ProxiesCallToInnerStategy() subs.DeleteBackup(OriginalFile); - innerStategyMock.Verify(_ => _.DeleteBackup(OriginalFile)); + innerStategyMock.Verify(_ => _.DeleteBackup(OriginalFile.Full)); } [Fact] @@ -47,7 +48,7 @@ public void RestoreBackup_ProxiesCallToInnerStategyIfNoBackupTime() subs.RestoreBackup(OriginalFile); - innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile)); + innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile.Full)); } [Fact] @@ -58,7 +59,7 @@ public void RestoreBackup_ProxiesCallToInnerStategyIfNoOriginalFile() subs.RestoreBackup(OriginalFile); - innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile)); + innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile.Full)); } [Fact] @@ -68,13 +69,13 @@ public void RestoreBackup_DeletesBackupIfOverwritten() var backupTime = fileCreationTime.Subtract(TimeSpan.FromSeconds(1)); var fs = new MockFileSystem(new Dictionary { - { OriginalFile, new MockFileData("") { CreationTime = fileCreationTime } }, + { OriginalFile.Full, new MockFileData("") { CreationTime = fileCreationTime } }, }); var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, backupTime); subs.RestoreBackup(OriginalFile); - innerStategyMock.Verify(_ => _.DeleteBackup(OriginalFile)); + innerStategyMock.Verify(_ => _.DeleteBackup(OriginalFile.Full)); } [Fact] @@ -83,12 +84,27 @@ public void RestoreBackup_ProxiesCallToInnerStategyIfNotOverwritten() var backupTime = DateTime.UtcNow; var fs = new MockFileSystem(new Dictionary { - { OriginalFile, new MockFileData("") { CreationTime = backupTime } }, + { OriginalFile.Full, new MockFileData("") { CreationTime = backupTime } }, }); var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, backupTime); subs.RestoreBackup(OriginalFile); - innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile)); + innerStategyMock.Verify(_ => _.RestoreBackup(OriginalFile.Full)); + } + + [Fact] + public void AfterInstall_EnduresDateInThePast() + { + var futureDate = DateTime.UtcNow.AddDays(1); + var fs = new MockFileSystem(new Dictionary + { + { OriginalFile.Full, new MockFileData("") { CreationTime = futureDate } }, + }); + var subs = new SkipUpdatedBackupStrategy(fs, innerStategyMock.Object, null); + + subs.AfterInstall(OriginalFile); + + fs.File.GetCreationTimeUtc(OriginalFile.Full).Should().BeOnOrBefore(DateTime.UtcNow); } } diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index 17d90a0..1811723 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -88,7 +88,7 @@ public void Apply_UninstallsMods() eventHandlerMock.Verify(_ => _.UninstallStart()); eventHandlerMock.Verify(_ => _.UninstallCurrent("A")); - eventHandlerMock.Verify(_ => _.UninstallSkipModified(TestPath("AF"))); + eventHandlerMock.Verify(_ => _.UninstallSkipModified("AF")); eventHandlerMock.Verify(_ => _.UninstallEnd()); eventHandlerMock.Verify(_ => _.InstallNoMods()); eventHandlerMock.Verify(_ => _.ProgressUpdate(It.IsAny())); From 8f1dc7c4b8c5f230704f4d99828ef152511db5a9 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Wed, 1 Jan 2025 09:14:56 +0000 Subject: [PATCH 16/18] Abstracted backup strategy provider --- src/Core/Backup/IBackupStrategyProvider.cs | 8 -------- src/Core/Backup/IModBackupStrategyProvider.cs | 8 ++++++++ src/Core/Backup/SkipUpdatedBackupStrategy.cs | 7 ++++--- src/Core/ModInstaller.cs | 10 +++++----- 4 files changed, 17 insertions(+), 16 deletions(-) delete mode 100644 src/Core/Backup/IBackupStrategyProvider.cs create mode 100644 src/Core/Backup/IModBackupStrategyProvider.cs diff --git a/src/Core/Backup/IBackupStrategyProvider.cs b/src/Core/Backup/IBackupStrategyProvider.cs deleted file mode 100644 index df0daa2..0000000 --- a/src/Core/Backup/IBackupStrategyProvider.cs +++ /dev/null @@ -1,8 +0,0 @@ -using Core.State; - -namespace Core.Backup; - -public interface IBackupStrategyProvider -{ - IInstallationBackupStrategy BackupStrategy(DateTime? backupTimeUtc); -} diff --git a/src/Core/Backup/IModBackupStrategyProvider.cs b/src/Core/Backup/IModBackupStrategyProvider.cs new file mode 100644 index 0000000..d9daf22 --- /dev/null +++ b/src/Core/Backup/IModBackupStrategyProvider.cs @@ -0,0 +1,8 @@ +using Core.State; + +namespace Core.Backup; + +public interface IModBackupStrategyProvider +{ + IInstallationBackupStrategy BackupStrategy(ModInstallationState? state); +} diff --git a/src/Core/Backup/SkipUpdatedBackupStrategy.cs b/src/Core/Backup/SkipUpdatedBackupStrategy.cs index 029f7f0..e6bfe02 100644 --- a/src/Core/Backup/SkipUpdatedBackupStrategy.cs +++ b/src/Core/Backup/SkipUpdatedBackupStrategy.cs @@ -1,5 +1,6 @@ using System.IO.Abstractions; using Core.Mods; +using Core.State; namespace Core.Backup; @@ -8,7 +9,7 @@ namespace Core.Backup; /// internal class SkipUpdatedBackupStrategy : IInstallationBackupStrategy { - internal class Provider : IBackupStrategyProvider + internal class Provider : IModBackupStrategyProvider { private readonly IBackupStrategy defaultStrategy; @@ -17,8 +18,8 @@ public Provider(IBackupStrategy defaultStrategy) this.defaultStrategy = defaultStrategy; } - public IInstallationBackupStrategy BackupStrategy(DateTime? backupTimeUtc) => - new SkipUpdatedBackupStrategy(defaultStrategy, backupTimeUtc); + public IInstallationBackupStrategy BackupStrategy(ModInstallationState? state) => + new SkipUpdatedBackupStrategy(defaultStrategy, state?.Time); } private readonly IFileSystem fs; diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index f110d21..e8b08b9 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -41,7 +41,7 @@ public interface IProgress } private readonly IInstallationFactory installationFactory; - private readonly IBackupStrategyProvider backupStrategyProvider; + private readonly IModBackupStrategyProvider modBackupStrategyProvider; private readonly Matcher filesToInstallMatcher; public ModInstaller( @@ -50,7 +50,7 @@ public ModInstaller( IConfig config) { this.installationFactory = installationFactory; - backupStrategyProvider = new SkipUpdatedBackupStrategy.Provider(backupStrategy); + modBackupStrategyProvider = new SkipUpdatedBackupStrategy.Provider(backupStrategy); filesToInstallMatcher = Matchers.ExcludingPatterns(config.ExcludedFromInstall); } @@ -83,7 +83,7 @@ private void UninstallPackages( break; } eventHandler.UninstallCurrent(packageName); - var backupStrategy = backupStrategyProvider.BackupStrategy(modInstallationState.Time); + var backupStrategy = modBackupStrategyProvider.BackupStrategy(modInstallationState); var filesLeft = modInstallationState.Files.ToHashSet(StringComparer.OrdinalIgnoreCase); try { @@ -189,7 +189,7 @@ private void InstallPackages( break; } eventHandler.InstallCurrent(modPackage.PackageName); - var backupStrategy = backupStrategyProvider.BackupStrategy(null); + var backupStrategy = modBackupStrategyProvider.BackupStrategy(null); var mod = installationFactory.ModInstaller(modPackage); try { @@ -209,7 +209,7 @@ private void InstallPackages( var bootfilesMod = CreateBootfilesMod(toInstall, eventHandler); try { - var backupStrategy = backupStrategyProvider.BackupStrategy(null); + var backupStrategy = modBackupStrategyProvider.BackupStrategy(null); bootfilesMod.Install(installDir, backupStrategy, installCallbacks); bootfilesMod.PostProcessing(installDir, modConfigs, eventHandler); } From ad841d22b5e36883ae7e6f39fae0b4896275240c Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Wed, 1 Jan 2025 09:21:31 +0000 Subject: [PATCH 17/18] Applied style suggestions to ModManager --- src/Core/ModInstaller.cs | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index e8b08b9..cdb75c8 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -1,4 +1,5 @@ -using Core.Backup; +using System.Collections.Immutable; +using Core.Backup; using Core.Mods; using Core.State; using Core.Utils; @@ -164,7 +165,7 @@ private void InstallPackages( IEventHandler eventHandler, CancellationToken cancellationToken) { - var modPackages = toInstall.Where(_ => !BootfilesManager.IsBootFiles(_.PackageName)).Reverse(); + var modPackages = toInstall.Where(p => !BootfilesManager.IsBootFiles(p.PackageName)).Reverse().ToImmutableArray(); var modConfigs = new List(); var installedFiles = new HashSet(StringComparer.OrdinalIgnoreCase); @@ -176,18 +177,14 @@ private void InstallPackages( .AndAccept(gamePath => Whitelisted(gamePath)); // Increase by one in case bootfiles are needed and another one to show that something is happening - var progress = new PercentOfTotal(modPackages.Count() + 2); + var progress = new PercentOfTotal(modPackages.Length + 2); if (modPackages.Any()) { eventHandler.InstallStart(); eventHandler.ProgressUpdate(progress.IncrementDone()); - foreach (var modPackage in modPackages) + foreach (var modPackage in modPackages.TakeWhile(_ => !cancellationToken.IsCancellationRequested)) { - if (cancellationToken.IsCancellationRequested) - { - break; - } eventHandler.InstallCurrent(modPackage.PackageName); var backupStrategy = modBackupStrategyProvider.BackupStrategy(null); var mod = installationFactory.ModInstaller(modPackage); @@ -203,7 +200,7 @@ private void InstallPackages( eventHandler.ProgressUpdate(progress.IncrementDone()); } - if (modConfigs.Where(_ => _.NotEmpty()).Any()) + if (modConfigs.Any(c => c.NotEmpty())) { eventHandler.PostProcessingStart(); var bootfilesMod = CreateBootfilesMod(toInstall, eventHandler); @@ -279,11 +276,11 @@ public ConfigEntries Install(string dstPath, IInstallationBackupStrategy backupS public void PostProcessing(string dstPath, IReadOnlyList modConfigs, IEventHandler eventHandler) { eventHandler.PostProcessingVehicles(); - PostProcessor.AppendCrdFileEntries(dstPath, modConfigs.SelectMany(_ => _.CrdFileEntries)); + PostProcessor.AppendCrdFileEntries(dstPath, modConfigs.SelectMany(c => c.CrdFileEntries)); eventHandler.PostProcessingTracks(); - PostProcessor.AppendTrdFileEntries(dstPath, modConfigs.SelectMany(_ => _.TrdFileEntries)); + PostProcessor.AppendTrdFileEntries(dstPath, modConfigs.SelectMany(c => c.TrdFileEntries)); eventHandler.PostProcessingDrivelines(); - PostProcessor.AppendDrivelineRecords(dstPath, modConfigs.SelectMany(_ => _.DrivelineRecords)); + PostProcessor.AppendDrivelineRecords(dstPath, modConfigs.SelectMany(c => c.DrivelineRecords)); postProcessingDone = true; } } From 0ffd1a871c5c11b10bb88259eccbee1d3284d078 Mon Sep 17 00:00:00 2001 From: Paolo Ambrosio Date: Wed, 1 Jan 2025 12:03:40 +0000 Subject: [PATCH 18/18] Moved whitelisting into base installer --- src/Core/API/Config.cs | 2 +- src/Core/API/Init.cs | 2 +- src/Core/ModInstaller.cs | 22 +++++-------------- src/Core/Mods/BaseInstaller.cs | 14 +++++++++++- .../API/ModManagerIntegrationTest.cs | 2 +- .../Core.Tests/ModInstallerIntegrationTest.cs | 4 +--- 6 files changed, 22 insertions(+), 24 deletions(-) diff --git a/src/Core/API/Config.cs b/src/Core/API/Config.cs index e859878..2645db0 100644 --- a/src/Core/API/Config.cs +++ b/src/Core/API/Config.cs @@ -34,7 +34,7 @@ public class GameConfig : Game.IConfig public string ProcessName { get; set; } = "Undefined"; } -public class ModInstallConfig : ModInstaller.IConfig, InstallationFactory.IConfig +public class ModInstallConfig : InstallationFactory.IConfig { public IEnumerable DirsAtRoot { get; set; } = Array.Empty(); public IEnumerable ExcludedFromInstall { get; set; } = Array.Empty(); diff --git a/src/Core/API/Init.cs b/src/Core/API/Init.cs index a3b4770..76fd535 100644 --- a/src/Core/API/Init.cs +++ b/src/Core/API/Init.cs @@ -20,7 +20,7 @@ public static IModManager CreateModManager(Config config) var installationFactory = new InstallationFactory(game, tempDir, config.ModInstall); var safeFileDelete = new WindowsRecyclingBin(); var backupStrategy = new SuffixBackupStrategy(); - var modInstaller = new ModInstaller(installationFactory, backupStrategy, config.ModInstall); + var modInstaller = new ModInstaller(installationFactory, backupStrategy); return new ModManager(game, modRepository, modInstaller, statePersistence, safeFileDelete, tempDir); } } diff --git a/src/Core/ModInstaller.cs b/src/Core/ModInstaller.cs index cdb75c8..f3a3591 100644 --- a/src/Core/ModInstaller.cs +++ b/src/Core/ModInstaller.cs @@ -9,11 +9,6 @@ namespace Core; public class ModInstaller : IModInstaller { - public interface IConfig - { - IEnumerable ExcludedFromInstall { get; } - } - public interface IEventHandler : IProgress { void InstallNoMods(); @@ -43,16 +38,13 @@ public interface IProgress private readonly IInstallationFactory installationFactory; private readonly IModBackupStrategyProvider modBackupStrategyProvider; - private readonly Matcher filesToInstallMatcher; public ModInstaller( IInstallationFactory installationFactory, - IBackupStrategy backupStrategy, - IConfig config) + IBackupStrategy backupStrategy) { this.installationFactory = installationFactory; modBackupStrategyProvider = new SkipUpdatedBackupStrategy.Provider(backupStrategy); - filesToInstallMatcher = Matchers.ExcludingPatterns(config.ExcludedFromInstall); } public void Apply( @@ -170,11 +162,10 @@ private void InstallPackages( var modConfigs = new List(); var installedFiles = new HashSet(StringComparer.OrdinalIgnoreCase); var installCallbacks = new ProcessingCallbacks - { - Accept = gamePath => !installedFiles.Contains(gamePath.Relative), - Before = gamePath => installedFiles.Add(gamePath.Relative), - } - .AndAccept(gamePath => Whitelisted(gamePath)); + { + Accept = gamePath => !installedFiles.Contains(gamePath.Relative), + Before = gamePath => installedFiles.Add(gamePath.Relative), + }; // Increase by one in case bootfiles are needed and another one to show that something is happening var progress = new PercentOfTotal(modPackages.Length + 2); @@ -230,9 +221,6 @@ private void InstallPackages( eventHandler.ProgressUpdate(progress.DoneAll()); } - private Predicate Whitelisted => - gamePath => filesToInstallMatcher.Match(gamePath.Relative).HasMatches; - private BootfilesMod CreateBootfilesMod(IReadOnlyCollection packages, IEventHandler eventHandler) { var bootfilesPackage = packages.FirstOrDefault(p => BootfilesManager.IsBootFiles(p.PackageName)); diff --git a/src/Core/Mods/BaseInstaller.cs b/src/Core/Mods/BaseInstaller.cs index b75ed94..0e6b209 100644 --- a/src/Core/Mods/BaseInstaller.cs +++ b/src/Core/Mods/BaseInstaller.cs @@ -19,6 +19,7 @@ internal abstract class BaseInstaller : IInstaller public IReadOnlyCollection InstalledFiles => installedFiles; private readonly IRootFinder rootFinder; + private readonly Matcher filesToInstallMatcher; private readonly Matcher filesToConfigureMatcher; private readonly List installedFiles = new(); @@ -28,6 +29,7 @@ internal BaseInstaller(string packageName, int? packageFsHash, ITempDir tempDir, PackageFsHash = packageFsHash; stagingDir = new DirectoryInfo(Path.Combine(tempDir.BasePath, packageName)); rootFinder = new ContainedDirsRootFinder(config.DirsAtRoot); + filesToInstallMatcher = Matchers.ExcludingPatterns(config.ExcludedFromInstall); filesToConfigureMatcher = Matchers.ExcludingPatterns(config.ExcludedFromConfig); } @@ -60,7 +62,8 @@ public ConfigEntries Install(string dstPath, IInstallationBackupStrategy backupS var (relativePath, removeFile) = NeedsRemoving(relativePathInMod); var gamePath = new RootedPath(dstPath, relativePath); - if (callbacks.Accept(gamePath)) + + if (Whitelisted(gamePath) && callbacks.Accept(gamePath)) { callbacks.Before(gamePath); backupStrategy.PerformBackup(gamePath); @@ -99,6 +102,9 @@ public ConfigEntries Install(string dstPath, IInstallationBackupStrategy backupS protected abstract void InstallFile(RootedPath destinationPath, TPassthrough context); + private bool Whitelisted(RootedPath path) => + filesToInstallMatcher.Match(path.Relative).HasMatches; + private static (string, bool) NeedsRemoving(string filePath) { return filePath.EndsWith(BaseInstaller.RemoveFileSuffix) ? @@ -184,6 +190,12 @@ IEnumerable DirsAtRoot { get; } + + IEnumerable ExcludedFromInstall + { + get; + } + IEnumerable ExcludedFromConfig { get; diff --git a/tests/Core.Tests/API/ModManagerIntegrationTest.cs b/tests/Core.Tests/API/ModManagerIntegrationTest.cs index ab473f5..1d656d4 100644 --- a/tests/Core.Tests/API/ModManagerIntegrationTest.cs +++ b/tests/Core.Tests/API/ModManagerIntegrationTest.cs @@ -55,7 +55,7 @@ public ModManagerIntegrationTest() modManager = new ModManager( gameMock.Object, modRepositoryMock.Object, - new ModInstaller(installationFactory, new SuffixBackupStrategy(), modInstallConfig), + new ModInstaller(installationFactory, new SuffixBackupStrategy()), persistedState, safeFileDeleteMock.Object, tempDir); diff --git a/tests/Core.Tests/ModInstallerIntegrationTest.cs b/tests/Core.Tests/ModInstallerIntegrationTest.cs index 1811723..2922a27 100644 --- a/tests/Core.Tests/ModInstallerIntegrationTest.cs +++ b/tests/Core.Tests/ModInstallerIntegrationTest.cs @@ -37,11 +37,9 @@ public ModInstallerIntegrationTest() { installationFactoryMock = new Mock(); backupStrategyMock = new Mock(); - Mock config = new(); modInstaller = new ModInstaller( installationFactoryMock.Object, - backupStrategyMock.Object, - config.Object); + backupStrategyMock.Object); eventHandlerMock = new Mock(); recordedState = new(); }