From eb732f03b012c1387bc4e3fbd66397de2fdd3814 Mon Sep 17 00:00:00 2001 From: Jimmy Lewis Date: Mon, 8 Apr 2024 16:20:07 -0700 Subject: [PATCH 1/6] Introduce installation goal state The existing LibraryInstallationState abstraction is not flexible about mapping files to their source on an individual basis. It presumes that all files installed by a library retain the same folder hierarchy as the library they originate from. In order to allow more flexibility, this change adds a new abstraction to wrap all installed files to their individual source files. This eliminates the prior assumption, allowing for flexibility to install files to a different folder hierarchy than their originating library structure. This flexibility also allows an important new concept: each destination file must be unique (to avoid collisions), but the sources are not so constrained. A single source file may be installed to multiple destinations; by creating a mapping of destination-to-source, we can easily allow this. This change prepares for a new feature to allow more granular mappings of files within a library. --- .../LibraryInstallationGoalState.cs | 56 +++++++ src/LibraryManager/Providers/BaseProvider.cs | 153 ++++++++++++------ .../Providers/BaseProviderTest.cs | 124 ++++++++++++++ 3 files changed, 285 insertions(+), 48 deletions(-) create mode 100644 src/LibraryManager.Contracts/LibraryInstallationGoalState.cs create mode 100644 test/LibraryManager.Test/Providers/BaseProviderTest.cs diff --git a/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs b/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs new file mode 100644 index 00000000..cc347951 --- /dev/null +++ b/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs @@ -0,0 +1,56 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Generic; +using System.IO; + +namespace Microsoft.Web.LibraryManager.Contracts +{ + /// + /// Represents a goal state of deployed files mapped to their sources from the local cache + /// + public class LibraryInstallationGoalState + { + /// + /// Initialize a new goal state from the desired installation state. + /// + public LibraryInstallationGoalState(ILibraryInstallationState installationState) + { + InstallationState = installationState; + } + + /// + /// The ILibraryInstallationState that this goal state was computed from. + /// + public ILibraryInstallationState InstallationState { get; } + + /// + /// Mapping from destination file to source file + /// + public IDictionary InstalledFiles { get; } = new Dictionary(); + + /// + /// Returns whether the goal is in an achieved state - that is, all files are up to date. + /// + /// + /// This is intended to serve as a fast check compared to restoring the files. + /// If there isn't a faster way to verify that a file is up to date, this method should + /// return false to indicate that a restore can't be skipped. + /// + public bool IsAchieved() + { + foreach (KeyValuePair kvp in InstalledFiles) + { + var destinationFile = new FileInfo(kvp.Key); + var cacheFile = new FileInfo(kvp.Value); + + if (!destinationFile.Exists || !cacheFile.Exists || !FileHelpers.AreFilesUpToDate(destinationFile, cacheFile)) + { + return false; + } + } + + return true; + } + } +} diff --git a/src/LibraryManager/Providers/BaseProvider.cs b/src/LibraryManager/Providers/BaseProvider.cs index 03beaf6b..53ef971a 100644 --- a/src/LibraryManager/Providers/BaseProvider.cs +++ b/src/LibraryManager/Providers/BaseProvider.cs @@ -16,7 +16,7 @@ namespace Microsoft.Web.LibraryManager.Providers { /// - /// Default implenentation for a provider, since most provider implementations are very similar. + /// Default implementation for a provider, since most provider implementations are very similar. /// internal abstract class BaseProvider : IProvider { @@ -60,30 +60,61 @@ public virtual async Task InstallAsync(ILibraryInstalla return LibraryOperationResult.FromCancelled(desiredState); } - //Expand the files property if needed - ILibraryOperationResult updateResult = await UpdateStateAsync(desiredState, cancellationToken); - if (!updateResult.Success) - { - return updateResult; - } + ILibraryCatalog catalog = GetCatalog(); + ILibrary library = await catalog.GetLibraryAsync(desiredState.Name, desiredState.Version, cancellationToken).ConfigureAwait(false); - desiredState = updateResult.InstallationState; + LibraryInstallationGoalState goalState = GenerateGoalState(desiredState, library); - // Refresh cache if needed - ILibraryOperationResult cacheUpdateResult = await RefreshCacheAsync(desiredState, cancellationToken); - if (!cacheUpdateResult.Success) + if (!IsSourceCacheReady(goalState)) { - return cacheUpdateResult; + ILibraryOperationResult updateCacheResult = await RefreshCacheAsync(desiredState, library, cancellationToken); + if (!updateCacheResult.Success) + { + return updateCacheResult; + } } - // Check if Library is already up to date - if (IsLibraryUpToDate(desiredState)) + if (goalState.IsAchieved()) { return LibraryOperationResult.FromUpToDate(desiredState); } - // Write files to destination - return await WriteToFilesAsync(desiredState, cancellationToken); + return await InstallFiles(goalState, cancellationToken); + + } + + private async Task InstallFiles(LibraryInstallationGoalState goalState, CancellationToken cancellationToken) + { + try + { + foreach (KeyValuePair kvp in goalState.InstalledFiles) + { + if (cancellationToken.IsCancellationRequested) + { + return LibraryOperationResult.FromCancelled(goalState.InstallationState); + } + + string sourcePath = kvp.Value; + string destinationPath = kvp.Key; + bool writeOk = await HostInteraction.CopyFileAsync(sourcePath, destinationPath, cancellationToken); + + if (!writeOk) + { + return new LibraryOperationResult(goalState.InstallationState, PredefinedErrors.CouldNotWriteFile(destinationPath)); + } + } + } + catch (UnauthorizedAccessException) + { + return new LibraryOperationResult(goalState.InstallationState, PredefinedErrors.PathOutsideWorkingDirectory()); + } + catch (Exception ex) + { + HostInteraction.Logger.Log(ex.ToString(), LogLevel.Error); + return new LibraryOperationResult(goalState.InstallationState, PredefinedErrors.UnknownException()); + } + + return LibraryOperationResult.FromSuccess(goalState.InstallationState); } /// @@ -165,6 +196,50 @@ public virtual async Task UpdateStateAsync(ILibraryInst #endregion + public LibraryInstallationGoalState GenerateGoalState(ILibraryInstallationState desiredState, ILibrary library) + { + var goalState = new LibraryInstallationGoalState(desiredState); + IEnumerable outFiles; + if (desiredState.Files == null || desiredState.Files.Count == 0) + { + outFiles = library.Files.Keys; + } + else + { + outFiles = FileGlobbingUtility.ExpandFileGlobs(desiredState.Files, library.Files.Keys); + } + + foreach (string outFile in outFiles) + { + // strip the source prefix + string destinationFile = Path.Combine(HostInteraction.WorkingDirectory, desiredState.DestinationPath, outFile); + + // don't forget to include the cache folder in the path + string sourceFile = GetCachedFileLocalPath(desiredState, outFile); + + // TODO: make goalState immutable + // map destination back to the library-relative file it originated from + goalState.InstalledFiles.Add(destinationFile, sourceFile); + } + + return goalState; + } + + public bool IsSourceCacheReady(LibraryInstallationGoalState goalState) + { + foreach (KeyValuePair item in goalState.InstalledFiles) + { + string cachePath = GetCachedFileLocalPath(goalState.InstallationState, item.Value); + // TODO: use abstraction for filesystem ops + if (!File.Exists(cachePath)) + { + return false; + } + } + + return true; + } + protected virtual ILibraryOperationResult CheckForInvalidFiles(ILibraryInstallationState desiredState, string libraryId, ILibrary library) { IReadOnlyList invalidFiles = library.GetInvalidFiles(desiredState.Files); @@ -239,36 +314,7 @@ protected async Task WriteToFilesAsync(ILibraryInstalla /// private string GetCachedFileLocalPath(ILibraryInstallationState state, string sourceFile) { - return Path.Combine(CacheFolder, state.Name, state.Version, sourceFile); - } - - private bool IsLibraryUpToDate(ILibraryInstallationState state) - { - try - { - if (!string.IsNullOrEmpty(state.Name) && !string.IsNullOrEmpty(state.Version)) - { - string cacheDir = Path.Combine(CacheFolder, state.Name, state.Version); - string destinationDir = Path.Combine(HostInteraction.WorkingDirectory, state.DestinationPath); - - foreach (string sourceFile in state.Files) - { - var destinationFile = new FileInfo(Path.Combine(destinationDir, sourceFile).Replace('\\', '/')); - var cacheFile = new FileInfo(Path.Combine(cacheDir, sourceFile).Replace('\\', '/')); - - if (!destinationFile.Exists || !cacheFile.Exists || !FileHelpers.AreFilesUpToDate(destinationFile, cacheFile)) - { - return false; - } - } - } - } - catch - { - return false; - } - - return true; + return Path.Combine(CacheFolder, state.Name, state.Version, sourceFile.Trim('/')); } /// @@ -277,7 +323,7 @@ private bool IsLibraryUpToDate(ILibraryInstallationState state) /// /// /// - private async Task RefreshCacheAsync(ILibraryInstallationState state, CancellationToken cancellationToken) + private async Task RefreshCacheAsync(ILibraryInstallationState state, ILibrary library, CancellationToken cancellationToken) { if (cancellationToken.IsCancellationRequested) { @@ -288,8 +334,19 @@ private async Task RefreshCacheAsync(ILibraryInstallati try { + IEnumerable filesToCache; + // expand "files" to concrete files in the library + if (state.Files == null || state.Files.Count == 0) + { + filesToCache = library.Files.Keys; + } + else + { + filesToCache = FileGlobbingUtility.ExpandFileGlobs(state.Files, library.Files.Keys); + } + var librariesMetadata = new HashSet(); - foreach (string sourceFile in state.Files) + foreach (string sourceFile in filesToCache) { string cacheFile = Path.Combine(libraryDir, sourceFile); string url = GetDownloadUrl(state, sourceFile); diff --git a/test/LibraryManager.Test/Providers/BaseProviderTest.cs b/test/LibraryManager.Test/Providers/BaseProviderTest.cs new file mode 100644 index 00000000..fd760b8a --- /dev/null +++ b/test/LibraryManager.Test/Providers/BaseProviderTest.cs @@ -0,0 +1,124 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.IO; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Microsoft.Web.LibraryManager.Cache; +using Microsoft.Web.LibraryManager.Contracts; +using Microsoft.Web.LibraryManager.Providers; +using Microsoft.Web.LibraryManager.Resources; + +namespace Microsoft.Web.LibraryManager.Test.Providers +{ + [TestClass] + public class BaseProviderTest + { + private IHostInteraction _hostInteraction; + private ILibrary _library; + + public BaseProviderTest() + { + _hostInteraction = new Mocks.HostInteraction() + { + CacheDirectory = "C:\\cache", + WorkingDirectory = "C:\\project", + }; + + _library = new Mocks.Library() + { + Name = "test", + Version = "1.0", + ProviderId = "TestProvider", + Files = new Dictionary() + { + { "file1.txt", true }, + { "file2.txt", false }, + { "folder/file3.txt", false }, + }, + }; + + } + + [TestMethod] + public void GenerateGoalState_NoFileMapping_SpecifyFilesAtLibraryLevel() + { + ILibraryInstallationState installState = new LibraryInstallationState + { + Name = "test", + Version = "1.0", + ProviderId = "TestProvider", + DestinationPath = "lib/test", + Files = ["folder/*.txt"], + }; + BaseProvider provider = new TestProvider(_hostInteraction, cacheService: null); + string expectedDestinationFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/folder/file3.txt")); + string expectedSourceFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/folder/file3.txt")); + + LibraryInstallationGoalState goalState = provider.GenerateGoalState(installState, _library); + + Assert.IsNotNull(goalState); + Assert.AreEqual(1, goalState.InstalledFiles.Count); + Assert.IsTrue(goalState.InstalledFiles.TryGetValue(expectedDestinationFile1, out string file1)); + Assert.AreEqual(expectedSourceFile1, file1); + } + + [TestMethod] + public void GenerateGoalState_NoFileMapping_NoFilesAtLibraryLevel() + { + ILibraryInstallationState installState = new LibraryInstallationState + { + Name = "test", + Version = "1.0", + ProviderId = "TestProvider", + DestinationPath = "lib/test", + }; + BaseProvider provider = new TestProvider(_hostInteraction, cacheService: null); + string expectedDestinationFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/file1.txt")); + string expectedSourceFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/file1.txt")); + string expectedDestinationFile2 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/file2.txt")); + string expectedSourceFile2 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/file2.txt")); + string expectedDestinationFile3 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/folder/file3.txt")); + string expectedSourceFile3 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/folder/file3.txt")); + + LibraryInstallationGoalState goalState = provider.GenerateGoalState(installState, _library); + + Assert.IsNotNull(goalState); + Assert.AreEqual(3, goalState.InstalledFiles.Count); + Assert.IsTrue(goalState.InstalledFiles.TryGetValue(expectedDestinationFile1, out string file1)); + Assert.AreEqual(expectedSourceFile1, file1); + Assert.IsTrue(goalState.InstalledFiles.TryGetValue(expectedDestinationFile2, out string file2)); + Assert.AreEqual(expectedSourceFile2, file2); + Assert.IsTrue(goalState.InstalledFiles.TryGetValue(expectedDestinationFile3, out string file3)); + Assert.AreEqual(expectedSourceFile3, file3); + } + + private class TestProvider : BaseProvider + { + public TestProvider(IHostInteraction hostInteraction, CacheService cacheService) + : base(hostInteraction, cacheService) + { + } + + public override string Id => nameof(TestProvider); + + public override string LibraryIdHintText => Text.CdnjsLibraryIdHintText; + + public override ILibraryCatalog GetCatalog() + { + throw new NotImplementedException(); + } + + public override string GetSuggestedDestination(ILibrary library) + { + throw new NotImplementedException(); + } + + protected override string GetDownloadUrl(ILibraryInstallationState state, string sourceFile) + { + throw new NotImplementedException(); + } + } + } +} From 4b79145702000118b4452bdb5b324b7ea0224593 Mon Sep 17 00:00:00 2001 From: Jimmy Lewis Date: Tue, 16 Apr 2024 17:52:07 -0700 Subject: [PATCH 2/6] Use OperationGoalState for intermediate steps in Install and GenerateGoalState --- src/LibraryManager.Contracts/FileHelpers.cs | 8 +- .../PredefinedErrors.cs | 10 ++- .../Resources/Text.Designer.cs | 9 +++ .../Resources/Text.resx | 57 +++++++------- src/LibraryManager/Providers/BaseProvider.cs | 75 +++++++++++++++++-- .../Providers/Cdnjs/CdnjsProviderTest.cs | 12 ++- .../JsDelivr/JsDelivrProviderTest.cs | 12 ++- .../Providers/Unpkg/UnpkgProviderTest.cs | 12 ++- 8 files changed, 149 insertions(+), 46 deletions(-) diff --git a/src/LibraryManager.Contracts/FileHelpers.cs b/src/LibraryManager.Contracts/FileHelpers.cs index 7092f875..d95932b0 100644 --- a/src/LibraryManager.Contracts/FileHelpers.cs +++ b/src/LibraryManager.Contracts/FileHelpers.cs @@ -374,7 +374,13 @@ public static bool IsUnderRootDirectory(string filePath, string rootDirectory) && normalizedFilePath.StartsWith(normalizedRootDirectory, StringComparison.OrdinalIgnoreCase); } - internal static string NormalizePath(string path) + /// + /// Normalizes the path string so it can be easily compared. + /// + /// + /// Result will be lowercase and have any trailing slashes removed. + /// + public static string NormalizePath(string path) { if (string.IsNullOrEmpty(path)) { diff --git a/src/LibraryManager.Contracts/PredefinedErrors.cs b/src/LibraryManager.Contracts/PredefinedErrors.cs index 71873e9a..cfcb4107 100644 --- a/src/LibraryManager.Contracts/PredefinedErrors.cs +++ b/src/LibraryManager.Contracts/PredefinedErrors.cs @@ -17,11 +17,11 @@ namespace Microsoft.Web.LibraryManager.Contracts public static class PredefinedErrors { /// - /// Represents an unhandled exception that occured in the provider. + /// Represents an unhandled exception that occurred in the provider. /// /// /// An should never throw and this error - /// should be used as when catching generic exeptions. + /// should be used as when catching generic exceptions. /// /// The error code LIB000 public static IError UnknownException() @@ -198,6 +198,12 @@ public static IError DuplicateLibrariesInManifest(string duplicateLibrary) public static IError FileNameMustNotBeEmpty(string libraryId) => new Error("LIB020", string.Format(Text.ErrorFilePathIsEmpty, libraryId)); + /// + /// A library mapping does not have a destination specified + /// + public static IError DestinationNotSpecified(string libraryId) + => new Error("LIB021", string.Format(Text.ErrorDestinationNotSpecified, libraryId)); + /// /// The manifest must specify a version /// diff --git a/src/LibraryManager.Contracts/Resources/Text.Designer.cs b/src/LibraryManager.Contracts/Resources/Text.Designer.cs index 4ca1b150..5da2ffcb 100644 --- a/src/LibraryManager.Contracts/Resources/Text.Designer.cs +++ b/src/LibraryManager.Contracts/Resources/Text.Designer.cs @@ -87,6 +87,15 @@ internal class Text { } } + /// + /// Looks up a localized string similar to The "{0}" library is missing a destination.. + /// + internal static string ErrorDestinationNotSpecified { + get { + return ResourceManager.GetString("ErrorDestinationNotSpecified", resourceCulture); + } + } + /// /// Looks up a localized string similar to The "{0}" destination file path has invalid characters. /// diff --git a/src/LibraryManager.Contracts/Resources/Text.resx b/src/LibraryManager.Contracts/Resources/Text.resx index 4ed3c263..3c0d9e51 100644 --- a/src/LibraryManager.Contracts/Resources/Text.resx +++ b/src/LibraryManager.Contracts/Resources/Text.resx @@ -1,17 +1,17 @@  - @@ -187,6 +187,9 @@ Valid files are {2} The library "{0}" cannot specify a file with an empty name + + The "{0}" library is missing a destination. + The Library Manager manifest must specify a version. diff --git a/src/LibraryManager/Providers/BaseProvider.cs b/src/LibraryManager/Providers/BaseProvider.cs index 53ef971a..14d6e63c 100644 --- a/src/LibraryManager/Providers/BaseProvider.cs +++ b/src/LibraryManager/Providers/BaseProvider.cs @@ -60,10 +60,25 @@ public virtual async Task InstallAsync(ILibraryInstalla return LibraryOperationResult.FromCancelled(desiredState); } - ILibraryCatalog catalog = GetCatalog(); - ILibrary library = await catalog.GetLibraryAsync(desiredState.Name, desiredState.Version, cancellationToken).ConfigureAwait(false); + OperationResult getLibrary = await GetLibraryForInstallationState(desiredState, cancellationToken).ConfigureAwait(false); + if (!getLibrary.Success) + { + return new LibraryOperationResult(desiredState, [.. getLibrary.Errors]) + { + Cancelled = getLibrary.Cancelled, + }; + } + + OperationResult getGoalState = GenerateGoalState(desiredState, getLibrary.Result); + if (!getGoalState.Success) + { + return new LibraryOperationResult(desiredState, [.. getGoalState.Errors]) + { + Cancelled = getGoalState.Cancelled, + }; + } - LibraryInstallationGoalState goalState = GenerateGoalState(desiredState, library); + LibraryInstallationGoalState goalState = getGoalState.Result; if (!IsSourceCacheReady(goalState)) { @@ -83,8 +98,30 @@ public virtual async Task InstallAsync(ILibraryInstalla } - private async Task InstallFiles(LibraryInstallationGoalState goalState, CancellationToken cancellationToken) + private async Task> GetLibraryForInstallationState(ILibraryInstallationState desiredState, CancellationToken cancellationToken) + { + ILibrary library; + try + { + ILibraryCatalog catalog = GetCatalog(); + library = await catalog.GetLibraryAsync(desiredState.Name, desiredState.Version, cancellationToken).ConfigureAwait(false); + } + catch (InvalidLibraryException) + { + string libraryId = LibraryNamingScheme.GetLibraryId(desiredState.Name, desiredState.Version); + return OperationResult.FromError(PredefinedErrors.UnableToResolveSource(libraryId, desiredState.ProviderId)); + } + catch (Exception ex) { + HostInteraction.Logger.Log(ex.ToString(), LogLevel.Error); + return OperationResult.FromError(PredefinedErrors.UnknownException()); + } + + return OperationResult.FromSuccess(library); + } + + private async Task InstallFiles(LibraryInstallationGoalState goalState, CancellationToken cancellationToken) + { try { foreach (KeyValuePair kvp in goalState.InstalledFiles) @@ -196,9 +233,16 @@ public virtual async Task UpdateStateAsync(ILibraryInst #endregion - public LibraryInstallationGoalState GenerateGoalState(ILibraryInstallationState desiredState, ILibrary library) + private OperationResult GenerateGoalState(ILibraryInstallationState desiredState, ILibrary library) { var goalState = new LibraryInstallationGoalState(desiredState); + List errors = null; + + if (string.IsNullOrEmpty(desiredState.DestinationPath)) + { + return OperationResult.FromError(PredefinedErrors.DestinationNotSpecified(desiredState.Name)); + } + IEnumerable outFiles; if (desiredState.Files == null || desiredState.Files.Count == 0) { @@ -209,20 +253,39 @@ public LibraryInstallationGoalState GenerateGoalState(ILibraryInstallationState outFiles = FileGlobbingUtility.ExpandFileGlobs(desiredState.Files, library.Files.Keys); } + if (library.GetInvalidFiles(outFiles.ToList()) is IReadOnlyList invalidFiles + && invalidFiles.Count > 0) + { + errors ??= []; + errors.Add(PredefinedErrors.InvalidFilesInLibrary(desiredState.Name, invalidFiles, library.Files.Keys)); + } + foreach (string outFile in outFiles) { // strip the source prefix string destinationFile = Path.Combine(HostInteraction.WorkingDirectory, desiredState.DestinationPath, outFile); + if (!FileHelpers.IsUnderRootDirectory(destinationFile, HostInteraction.WorkingDirectory)) + { + errors ??= []; + errors.Add(PredefinedErrors.PathOutsideWorkingDirectory()); + } + destinationFile = FileHelpers.NormalizePath(destinationFile); // don't forget to include the cache folder in the path string sourceFile = GetCachedFileLocalPath(desiredState, outFile); + sourceFile = FileHelpers.NormalizePath(sourceFile); // TODO: make goalState immutable // map destination back to the library-relative file it originated from goalState.InstalledFiles.Add(destinationFile, sourceFile); } - return goalState; + if (errors is not null) + { + return OperationResult.FromErrors([.. errors]); + } + + return OperationResult.FromSuccess(goalState); } public bool IsSourceCacheReady(LibraryInstallationGoalState goalState) diff --git a/test/LibraryManager.Test/Providers/Cdnjs/CdnjsProviderTest.cs b/test/LibraryManager.Test/Providers/Cdnjs/CdnjsProviderTest.cs index e051d8ed..8bb33056 100644 --- a/test/LibraryManager.Test/Providers/Cdnjs/CdnjsProviderTest.cs +++ b/test/LibraryManager.Test/Providers/Cdnjs/CdnjsProviderTest.cs @@ -100,8 +100,7 @@ public async Task InstallAsync_NoPathDefined() ILibraryOperationResult result = await _provider.InstallAsync(desiredState, CancellationToken.None).ConfigureAwait(false); Assert.IsFalse(result.Success); - // Unknown exception. We no longer validate ILibraryState at the provider level - Assert.AreEqual("LIB000", result.Errors[0].Code); + Assert.AreEqual("LIB021", result.Errors[0].Code); } [TestMethod] @@ -148,11 +147,16 @@ public async Task InstallAsync_WithGlobPatterns_CorrectlyInstallsAllMatchingFile Files = new[] { "*.js", "!*.min.js" }, }; + // Verify expansion of Files + OperationResult getGoalState = await _provider.GetInstallationGoalStateAsync(desiredState, CancellationToken.None); + Assert.IsTrue(getGoalState.Success); + LibraryInstallationGoalState goalState = getGoalState.Result; + Assert.AreEqual(1, goalState.InstalledFiles.Count); + Assert.AreEqual("jquery.js", Path.GetFileName(goalState.InstalledFiles.Keys.First())); + // Install library ILibraryOperationResult result = await _provider.InstallAsync(desiredState, CancellationToken.None).ConfigureAwait(false); Assert.IsTrue(result.Success); - Assert.IsTrue(result.InstallationState.Files.Count == 1); // jquery.min.js file was excluded - Assert.AreEqual("jquery.js", result.InstallationState.Files.First()); } [TestMethod] diff --git a/test/LibraryManager.Test/Providers/JsDelivr/JsDelivrProviderTest.cs b/test/LibraryManager.Test/Providers/JsDelivr/JsDelivrProviderTest.cs index c1ca3623..8f19e162 100644 --- a/test/LibraryManager.Test/Providers/JsDelivr/JsDelivrProviderTest.cs +++ b/test/LibraryManager.Test/Providers/JsDelivr/JsDelivrProviderTest.cs @@ -100,8 +100,7 @@ public async Task InstallAsync_NoPathDefined() ILibraryOperationResult result = await _provider.InstallAsync(desiredState, CancellationToken.None).ConfigureAwait(false); Assert.IsFalse(result.Success); - // Unknown exception. We no longer validate ILibraryState at the provider level - Assert.AreEqual("LIB000", result.Errors[0].Code); + Assert.AreEqual("LIB021", result.Errors[0].Code); } [TestMethod] @@ -148,10 +147,17 @@ public async Task InstallAsync_WithGlobPatterns_CorrectlyInstallsAllMatchingFile Files = new[] { "dist/*.js", "!dist/*min*" }, }; + // Verify expansion of Files + OperationResult getGoalState = await _provider.GetInstallationGoalStateAsync(desiredState, CancellationToken.None); + Assert.IsTrue(getGoalState.Success); + LibraryInstallationGoalState goalState = getGoalState.Result; + // Remove the project folder and "/lib/" from the file paths + List installedFiles = goalState.InstalledFiles.Keys.Select(f => f.Substring(_projectFolder.Length + 5).Replace("\\", "/")).ToList(); + CollectionAssert.AreEquivalent(new[] { "dist/core.js", "dist/jquery.js", "dist/jquery.slim.js" }, installedFiles); + // Install library ILibraryOperationResult result = await _provider.InstallAsync(desiredState, CancellationToken.None).ConfigureAwait(false); Assert.IsTrue(result.Success); - CollectionAssert.AreEquivalent(new[] { "dist/core.js", "dist/jquery.js", "dist/jquery.slim.js" }, result.InstallationState.Files.ToList()); } [TestMethod] diff --git a/test/LibraryManager.Test/Providers/Unpkg/UnpkgProviderTest.cs b/test/LibraryManager.Test/Providers/Unpkg/UnpkgProviderTest.cs index d8636757..9947089a 100644 --- a/test/LibraryManager.Test/Providers/Unpkg/UnpkgProviderTest.cs +++ b/test/LibraryManager.Test/Providers/Unpkg/UnpkgProviderTest.cs @@ -99,8 +99,7 @@ public async Task InstallAsync_NoPathDefined() ILibraryOperationResult result = await _provider.InstallAsync(desiredState, CancellationToken.None).ConfigureAwait(false); Assert.IsFalse(result.Success); - // Unknown exception. We no longer validate ILibraryState at the provider level - Assert.AreEqual("LIB000", result.Errors[0].Code); + Assert.AreEqual("LIB021", result.Errors[0].Code); } [TestMethod] @@ -147,10 +146,17 @@ public async Task InstallAsync_WithGlobPatterns_CorrectlyInstallsAllMatchingFile Files = new[] { "dist/*.js", "!dist/*min*" }, }; + // Verify expansion of Files + OperationResult getGoalState = await _provider.GetInstallationGoalStateAsync(desiredState, CancellationToken.None); + Assert.IsTrue(getGoalState.Success); + LibraryInstallationGoalState goalState = getGoalState.Result; + // Remove the project folder and "/lib/" from the file paths + List installedFiles = goalState.InstalledFiles.Keys.Select(f => f.Substring(_projectFolder.Length + 5).Replace("\\", "/")).ToList(); + CollectionAssert.AreEquivalent(new[] { "dist/core.js", "dist/jquery.js", "dist/jquery.slim.js" }, installedFiles); + // Install library ILibraryOperationResult result = await _provider.InstallAsync(desiredState, CancellationToken.None).ConfigureAwait(false); Assert.IsTrue(result.Success); - CollectionAssert.AreEquivalent(new[] { "dist/core.js", "dist/jquery.js", "dist/jquery.slim.js" }, result.InstallationState.Files.ToList()); } [TestMethod] From b845a91b1544492f60802e0b603025b84ebff07a Mon Sep 17 00:00:00 2001 From: Jimmy Lewis Date: Tue, 16 Apr 2024 18:34:57 -0700 Subject: [PATCH 3/6] Change BaseProvider to use a public API for getting goal state. Also updated Manifest to use the goal state for the uninstall scenario. --- src/LibraryManager.Contracts/IProvider.cs | 5 ++ src/LibraryManager/Manifest.cs | 58 ++++++++----------- src/LibraryManager/Providers/BaseProvider.cs | 14 ++++- test/LibraryManager.Mocks/Provider.cs | 13 +++++ .../Providers/BaseProviderTest.cs | 35 +++++++---- .../Shared/LibraryCommandServiceTest.cs | 16 +++-- 6 files changed, 89 insertions(+), 52 deletions(-) diff --git a/src/LibraryManager.Contracts/IProvider.cs b/src/LibraryManager.Contracts/IProvider.cs index 8a43d551..b6cd90d7 100644 --- a/src/LibraryManager.Contracts/IProvider.cs +++ b/src/LibraryManager.Contracts/IProvider.cs @@ -66,5 +66,10 @@ public interface IProvider /// /// string GetSuggestedDestination(ILibrary library); + + /// + /// Gets the goal state of the library installation. Does not imply actual installation. + /// + Task> GetInstallationGoalStateAsync(ILibraryInstallationState installationState, CancellationToken cancellationToken); } } diff --git a/src/LibraryManager/Manifest.cs b/src/LibraryManager/Manifest.cs index 179a1b59..a8556214 100644 --- a/src/LibraryManager/Manifest.cs +++ b/src/LibraryManager/Manifest.cs @@ -204,8 +204,6 @@ public Manifest Clone() string destination, CancellationToken cancellationToken) { - ILibraryOperationResult result; - var desiredState = new LibraryInstallationState() { Name = libraryName, @@ -236,14 +234,14 @@ public Manifest Clone() return conflictResults; } - result = await provider.InstallAsync(desiredState, cancellationToken).ConfigureAwait(false); + ILibraryOperationResult installResult = await provider.InstallAsync(desiredState, cancellationToken); - if (result.Success) + if (installResult.Success) { AddLibrary(desiredState); } - return result; + return installResult; } private ILibraryInstallationState SetDefaultProviderIfNeeded(LibraryInstallationState desiredState) @@ -510,7 +508,7 @@ private async Task> GetAllManifestFilesWithVersionsA return allFiles.SelectMany(f => f).Distinct(); } - return new List(); + return new List(); } private async Task> GetFilesWithVersionsAsync(ILibraryInstallationState state) @@ -567,41 +565,31 @@ private async Task> GetFilesWithVersionsAsync(ILibra try { IProvider provider = _dependencies.GetProvider(state.ProviderId); - ILibraryOperationResult updatedStateResult = await provider.UpdateStateAsync(state, CancellationToken.None).ConfigureAwait(false); - - if (updatedStateResult.Success) + OperationResult getGoalState = await provider.GetInstallationGoalStateAsync(state, cancellationToken).ConfigureAwait(false); + if (!getGoalState.Success) { - List filesToDelete = new List(); - state = updatedStateResult.InstallationState; - - foreach (string file in state.Files) + return new LibraryOperationResult(state, [.. getGoalState.Errors]) { - var url = new Uri(file, UriKind.RelativeOrAbsolute); - - if (!url.IsAbsoluteUri) - { - string relativePath = Path.Combine(state.DestinationPath, file).Replace('\\', '/'); - filesToDelete.Add(relativePath); - } - } + Cancelled = getGoalState.Cancelled, + }; + } - bool success = true; - if (deleteFilesFunction != null) - { - success = await deleteFilesFunction.Invoke(filesToDelete).ConfigureAwait(false); - } + LibraryInstallationGoalState goalState = getGoalState.Result; - if (success) - { - return LibraryOperationResult.FromSuccess(updatedStateResult.InstallationState); - } - else - { - return LibraryOperationResult.FromError(PredefinedErrors.CouldNotDeleteLibrary(libraryId)); - } + bool success = true; + if (deleteFilesFunction != null) + { + success = await deleteFilesFunction.Invoke(goalState.InstalledFiles.Keys).ConfigureAwait(false); } - return updatedStateResult; + if (success) + { + return LibraryOperationResult.FromSuccess(goalState.InstallationState); + } + else + { + return LibraryOperationResult.FromError(PredefinedErrors.CouldNotDeleteLibrary(libraryId)); + } } catch (OperationCanceledException) { diff --git a/src/LibraryManager/Providers/BaseProvider.cs b/src/LibraryManager/Providers/BaseProvider.cs index 14d6e63c..40025f95 100644 --- a/src/LibraryManager/Providers/BaseProvider.cs +++ b/src/LibraryManager/Providers/BaseProvider.cs @@ -82,7 +82,7 @@ public virtual async Task InstallAsync(ILibraryInstalla if (!IsSourceCacheReady(goalState)) { - ILibraryOperationResult updateCacheResult = await RefreshCacheAsync(desiredState, library, cancellationToken); + ILibraryOperationResult updateCacheResult = await RefreshCacheAsync(desiredState, getLibrary.Result, cancellationToken); if (!updateCacheResult.Success) { return updateCacheResult; @@ -231,6 +231,18 @@ public virtual async Task UpdateStateAsync(ILibraryInst return LibraryOperationResult.FromSuccess(desiredState); } + public async Task> GetInstallationGoalStateAsync(ILibraryInstallationState desiredState, CancellationToken cancellationToken) + { + // get the library from the catalog + OperationResult getLibrary = await GetLibraryForInstallationState(desiredState, cancellationToken).ConfigureAwait(false); + if (!getLibrary.Success) + { + return OperationResult.FromErrors([.. getLibrary.Errors]); + } + + return GenerateGoalState(desiredState, getLibrary.Result); + } + #endregion private OperationResult GenerateGoalState(ILibraryInstallationState desiredState, ILibrary library) diff --git a/test/LibraryManager.Mocks/Provider.cs b/test/LibraryManager.Mocks/Provider.cs index 41f99918..6f0d1cc4 100644 --- a/test/LibraryManager.Mocks/Provider.cs +++ b/test/LibraryManager.Mocks/Provider.cs @@ -58,6 +58,11 @@ public Provider(IHostInteraction hostInteraction) /// public virtual ILibraryOperationResult Result { get; set; } + /// + /// Gets or sets the goal state to return from + /// + public LibraryInstallationGoalState GoalState { get; set; } + /// /// Indicates whether libraries with versions are supported. /// @@ -105,5 +110,13 @@ public string GetSuggestedDestination(ILibrary library) { return library?.Name; } + + /// + /// Returns a stubbed value + /// + public Task> GetInstallationGoalStateAsync(ILibraryInstallationState installationState, CancellationToken cancellationToken) + { + return Task.FromResult(OperationResult.FromSuccess(GoalState)); + } } } diff --git a/test/LibraryManager.Test/Providers/BaseProviderTest.cs b/test/LibraryManager.Test/Providers/BaseProviderTest.cs index fd760b8a..91368d37 100644 --- a/test/LibraryManager.Test/Providers/BaseProviderTest.cs +++ b/test/LibraryManager.Test/Providers/BaseProviderTest.cs @@ -4,6 +4,8 @@ using System; using System.Collections.Generic; using System.IO; +using System.Threading; +using System.Threading.Tasks; using Microsoft.VisualStudio.TestTools.UnitTesting; using Microsoft.Web.LibraryManager.Cache; using Microsoft.Web.LibraryManager.Contracts; @@ -17,6 +19,7 @@ public class BaseProviderTest { private IHostInteraction _hostInteraction; private ILibrary _library; + private readonly Mocks.LibraryCatalog _catalog; public BaseProviderTest() { @@ -39,10 +42,12 @@ public BaseProviderTest() }, }; + _catalog = new Mocks.LibraryCatalog() + .AddLibrary(_library); } [TestMethod] - public void GenerateGoalState_NoFileMapping_SpecifyFilesAtLibraryLevel() + public async Task GenerateGoalState_NoFileMapping_SpecifyFilesAtLibraryLevel() { ILibraryInstallationState installState = new LibraryInstallationState { @@ -52,12 +57,17 @@ public void GenerateGoalState_NoFileMapping_SpecifyFilesAtLibraryLevel() DestinationPath = "lib/test", Files = ["folder/*.txt"], }; - BaseProvider provider = new TestProvider(_hostInteraction, cacheService: null); + BaseProvider provider = new TestProvider(_hostInteraction, cacheService: null) + { + Catalog = _catalog, + }; string expectedDestinationFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/folder/file3.txt")); string expectedSourceFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/folder/file3.txt")); - LibraryInstallationGoalState goalState = provider.GenerateGoalState(installState, _library); + OperationResult getGoalState = await provider.GetInstallationGoalStateAsync(installState, CancellationToken.None); + Assert.IsTrue(getGoalState.Success); + LibraryInstallationGoalState goalState = getGoalState.Result; Assert.IsNotNull(goalState); Assert.AreEqual(1, goalState.InstalledFiles.Count); Assert.IsTrue(goalState.InstalledFiles.TryGetValue(expectedDestinationFile1, out string file1)); @@ -65,7 +75,7 @@ public void GenerateGoalState_NoFileMapping_SpecifyFilesAtLibraryLevel() } [TestMethod] - public void GenerateGoalState_NoFileMapping_NoFilesAtLibraryLevel() + public async Task GenerateGoalState_NoFileMapping_NoFilesAtLibraryLevel() { ILibraryInstallationState installState = new LibraryInstallationState { @@ -74,7 +84,10 @@ public void GenerateGoalState_NoFileMapping_NoFilesAtLibraryLevel() ProviderId = "TestProvider", DestinationPath = "lib/test", }; - BaseProvider provider = new TestProvider(_hostInteraction, cacheService: null); + BaseProvider provider = new TestProvider(_hostInteraction, cacheService: null) + { + Catalog = _catalog, + }; string expectedDestinationFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/file1.txt")); string expectedSourceFile1 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/file1.txt")); string expectedDestinationFile2 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/file2.txt")); @@ -82,7 +95,10 @@ public void GenerateGoalState_NoFileMapping_NoFilesAtLibraryLevel() string expectedDestinationFile3 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.WorkingDirectory, "lib/test/folder/file3.txt")); string expectedSourceFile3 = FileHelpers.NormalizePath(Path.Combine(provider.HostInteraction.CacheDirectory, "TestProvider/test/1.0/folder/file3.txt")); - LibraryInstallationGoalState goalState = provider.GenerateGoalState(installState, _library); + OperationResult getGoalState = await provider.GetInstallationGoalStateAsync(installState, CancellationToken.None); + + Assert.IsTrue(getGoalState.Success); + LibraryInstallationGoalState goalState = getGoalState.Result; Assert.IsNotNull(goalState); Assert.AreEqual(3, goalState.InstalledFiles.Count); @@ -105,10 +121,9 @@ public TestProvider(IHostInteraction hostInteraction, CacheService cacheService) public override string LibraryIdHintText => Text.CdnjsLibraryIdHintText; - public override ILibraryCatalog GetCatalog() - { - throw new NotImplementedException(); - } + public ILibraryCatalog Catalog { get; set; } + + public override ILibraryCatalog GetCatalog() => Catalog; public override string GetSuggestedDestination(ILibrary library) { diff --git a/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs b/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs index 62e306fe..3713b70c 100644 --- a/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs +++ b/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs @@ -28,6 +28,14 @@ public async Task UninstallAsync_DeletesFilesFromDisk() var mockTaskStatusCenterService = new Mock(); mockTaskStatusCenterService.Setup(m => m.CreateTaskHandlerAsync(It.IsAny())) .Returns(Task.FromResult(new Mock().Object)); + var testInstallationState = new LibraryInstallationState + { + ProviderId = "testProvider", + Files = new[] { "test.js" }, + DestinationPath = "testDestination", + }; + var testGoalState = new LibraryInstallationGoalState(testInstallationState); + testGoalState.InstalledFiles.Add(Path.Combine(mockInteraction.WorkingDirectory, "testDestination", "test.js"), Path.Combine(mockInteraction.WorkingDirectory, "test.js")); var mockDependencies = new Dependencies(mockInteraction, new IProvider[] { new Mocks.Provider(mockInteraction) @@ -36,13 +44,9 @@ public async Task UninstallAsync_DeletesFilesFromDisk() Catalog = new Mocks.LibraryCatalog(), Result = new LibraryOperationResult { - InstallationState = new LibraryInstallationState - { - ProviderId = "testProvider", - Files = new [] { "test.js" }, - DestinationPath = "testDestination", - } + InstallationState = testInstallationState }, + GoalState = testGoalState, SupportsLibraryVersions = true, } }); From c0de1fc51d2c299c29d031166dbcad6f4d1d0912 Mon Sep 17 00:00:00 2001 From: Jimmy Lewis Date: Tue, 16 Apr 2024 00:33:45 -0700 Subject: [PATCH 4/6] Check some special cases for URI paths instead of local file paths --- src/LibraryManager.Contracts/FileHelpers.cs | 15 +++++++++++++++ .../LibraryInstallationGoalState.cs | 7 +++++++ 2 files changed, 22 insertions(+) diff --git a/src/LibraryManager.Contracts/FileHelpers.cs b/src/LibraryManager.Contracts/FileHelpers.cs index d95932b0..3d58e5d8 100644 --- a/src/LibraryManager.Contracts/FileHelpers.cs +++ b/src/LibraryManager.Contracts/FileHelpers.cs @@ -387,6 +387,12 @@ public static string NormalizePath(string path) return path; } + // If the path is a URI, we don't want to normalize it + if (IsHttpUri(path)) + { + return path; + } + // net451 does not have the OSPlatform apis to determine if the OS is windows or not. // This also does not handle the fact that MacOS can be configured to be either sensitive or insenstive // to the casing. @@ -400,5 +406,14 @@ public static string NormalizePath(string path) return Path.GetFullPath(path).TrimEnd(Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar); } + + /// + /// Determines if the path is an HTTP or HTTPS Uri + /// + public static bool IsHttpUri(string path) + { + return Uri.TryCreate(path, UriKind.Absolute, out Uri uri) + && (uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps); + } } } diff --git a/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs b/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs index cc347951..b9e14779 100644 --- a/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs +++ b/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs @@ -41,6 +41,13 @@ public bool IsAchieved() { foreach (KeyValuePair kvp in InstalledFiles) { + // If the source file is a remote Uri, we have no way to determine if it matches the installed file. + // So we will always reinstall the library in this case. + if (FileHelpers.IsHttpUri(kvp.Value)) + { + return false; + } + var destinationFile = new FileInfo(kvp.Key); var cacheFile = new FileInfo(kvp.Value); From 4e665b3cc146cd2e76c9b86e8f08fbb449bf1aa1 Mon Sep 17 00:00:00 2001 From: Jimmy Lewis Date: Wed, 17 Apr 2024 00:21:18 -0700 Subject: [PATCH 5/6] Expose logic for finding local cache path Needed so that FileSystemProvider can override the default behavior, since it doesn't use the same cache logic as other providers. --- src/LibraryManager/Providers/BaseProvider.cs | 2 +- .../FileSystem/FileSystemProvider.cs | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/LibraryManager/Providers/BaseProvider.cs b/src/LibraryManager/Providers/BaseProvider.cs index 40025f95..8c8806b0 100644 --- a/src/LibraryManager/Providers/BaseProvider.cs +++ b/src/LibraryManager/Providers/BaseProvider.cs @@ -387,7 +387,7 @@ protected async Task WriteToFilesAsync(ILibraryInstalla /// Gets the expected local path for a file from the file cache /// /// - private string GetCachedFileLocalPath(ILibraryInstallationState state, string sourceFile) + protected virtual string GetCachedFileLocalPath(ILibraryInstallationState state, string sourceFile) { return Path.Combine(CacheFolder, state.Name, state.Version, sourceFile.Trim('/')); } diff --git a/src/LibraryManager/Providers/FileSystem/FileSystemProvider.cs b/src/LibraryManager/Providers/FileSystem/FileSystemProvider.cs index 51afa5c6..27317660 100644 --- a/src/LibraryManager/Providers/FileSystem/FileSystemProvider.cs +++ b/src/LibraryManager/Providers/FileSystem/FileSystemProvider.cs @@ -206,5 +206,28 @@ protected override string GetDownloadUrl(ILibraryInstallationState state, string { throw new NotSupportedException(); } + + protected override string GetCachedFileLocalPath(ILibraryInstallationState state, string sourceFile) + { + // FileSystemProvider pulls files directly, no caching. So here we need to build a full + // path or URI to the file. + + // For HTTP files, the state.Name is the full URL to a single file + if (FileHelpers.IsHttpUri(state.Name)) + { + return state.Name; + } + + // For other filesystem libraries, the state.Name may be a either a file or folder + // TODO: abstract file system + if (File.Exists(state.Name)) + { + return state.Name; + } + + // as a fallback, assume state.Name is a directory. If this path doesn't exist, it will + // be handled elsewhere. + return Path.Combine(state.Name, sourceFile); + } } } From d53986ed65db75963f796fe643f45bd5d8056ef1 Mon Sep 17 00:00:00 2001 From: Jimmy Lewis Date: Fri, 26 Apr 2024 09:04:36 -0700 Subject: [PATCH 6/6] make goalstate immutable --- .../LibraryInstallationGoalState.cs | 5 +++-- src/LibraryManager/Providers/BaseProvider.cs | 6 +++--- .../Shared/LibraryCommandServiceTest.cs | 8 ++++++-- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs b/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs index b9e14779..3c6df96f 100644 --- a/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs +++ b/src/LibraryManager.Contracts/LibraryInstallationGoalState.cs @@ -14,9 +14,10 @@ public class LibraryInstallationGoalState /// /// Initialize a new goal state from the desired installation state. /// - public LibraryInstallationGoalState(ILibraryInstallationState installationState) + public LibraryInstallationGoalState(ILibraryInstallationState installationState, Dictionary installedFiles) { InstallationState = installationState; + InstalledFiles = installedFiles; } /// @@ -27,7 +28,7 @@ public LibraryInstallationGoalState(ILibraryInstallationState installationState) /// /// Mapping from destination file to source file /// - public IDictionary InstalledFiles { get; } = new Dictionary(); + public IDictionary InstalledFiles { get; } /// /// Returns whether the goal is in an achieved state - that is, all files are up to date. diff --git a/src/LibraryManager/Providers/BaseProvider.cs b/src/LibraryManager/Providers/BaseProvider.cs index 8c8806b0..60955bd1 100644 --- a/src/LibraryManager/Providers/BaseProvider.cs +++ b/src/LibraryManager/Providers/BaseProvider.cs @@ -247,7 +247,6 @@ public async Task> GetInstallation private OperationResult GenerateGoalState(ILibraryInstallationState desiredState, ILibrary library) { - var goalState = new LibraryInstallationGoalState(desiredState); List errors = null; if (string.IsNullOrEmpty(desiredState.DestinationPath)) @@ -265,6 +264,7 @@ private OperationResult GenerateGoalState(ILibrary outFiles = FileGlobbingUtility.ExpandFileGlobs(desiredState.Files, library.Files.Keys); } + Dictionary installFiles = new(); if (library.GetInvalidFiles(outFiles.ToList()) is IReadOnlyList invalidFiles && invalidFiles.Count > 0) { @@ -287,9 +287,8 @@ private OperationResult GenerateGoalState(ILibrary string sourceFile = GetCachedFileLocalPath(desiredState, outFile); sourceFile = FileHelpers.NormalizePath(sourceFile); - // TODO: make goalState immutable // map destination back to the library-relative file it originated from - goalState.InstalledFiles.Add(destinationFile, sourceFile); + installFiles.Add(destinationFile, sourceFile); } if (errors is not null) @@ -297,6 +296,7 @@ private OperationResult GenerateGoalState(ILibrary return OperationResult.FromErrors([.. errors]); } + var goalState = new LibraryInstallationGoalState(desiredState, installFiles); return OperationResult.FromSuccess(goalState); } diff --git a/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs b/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs index 3713b70c..990a6de8 100644 --- a/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs +++ b/test/Microsoft.Web.LibraryManager.Vsix.Test/Shared/LibraryCommandServiceTest.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +using System.Collections.Generic; using System.IO; using System.Text; using System.Threading; @@ -34,8 +35,11 @@ public async Task UninstallAsync_DeletesFilesFromDisk() Files = new[] { "test.js" }, DestinationPath = "testDestination", }; - var testGoalState = new LibraryInstallationGoalState(testInstallationState); - testGoalState.InstalledFiles.Add(Path.Combine(mockInteraction.WorkingDirectory, "testDestination", "test.js"), Path.Combine(mockInteraction.WorkingDirectory, "test.js")); + Dictionary installedFiles = new() + { + { Path.Combine(mockInteraction.WorkingDirectory, "testDestination", "test.js"), Path.Combine(mockInteraction.WorkingDirectory, "test.js")} + }; + var testGoalState = new LibraryInstallationGoalState(testInstallationState, installedFiles); var mockDependencies = new Dependencies(mockInteraction, new IProvider[] { new Mocks.Provider(mockInteraction)