Change Manifest.InstallLibraryAsync to return a consolidated result (#737)

This was designed to return many results, but that doesn't make a lot of sense when installing a single library.  Also, the existing code paths only lever returned an enumerable of one item, which is wasteful.

src/LibraryManager/Manifest.cs

@@ -196,7 +196,7 @@ namespace Microsoft.Web.LibraryManager
         /// <param name="destination"></param>
         /// <param name="cancellationToken"></param>
         /// <returns></returns>
-        public async Task<IEnumerable<ILibraryOperationResult>> InstallLibraryAsync(
+        public async Task<ILibraryOperationResult> InstallLibraryAsync(
             string libraryName,
             string version,
             string providerId,
@@ -208,7 +208,7 @@ namespace Microsoft.Web.LibraryManager
 
             var desiredState = new LibraryInstallationState()
             {
-                Name =  libraryName,
+                Name = libraryName,
                 Version = version,
                 Files = files,
                 ProviderId = providerId,
@@ -220,18 +220,18 @@ namespace Microsoft.Web.LibraryManager
             ILibraryOperationResult validationResult = await desiredState.IsValidAsync(_dependencies);
             if (!validationResult.Success)
             {
-                return new [] { validationResult };
+                return validationResult;
             }
 
             IProvider provider = _dependencies.GetProvider(desiredState.ProviderId);
             if (provider == null)
             {
-                return new [] { new LibraryOperationResult(desiredState, new IError[] { PredefinedErrors.ProviderUnknown(desiredState.ProviderId) })};
+                return new LibraryOperationResult(desiredState, new IError[] { PredefinedErrors.ProviderUnknown(desiredState.ProviderId) });
             }
 
-            IEnumerable<ILibraryOperationResult> conflictResults = await CheckLibraryForConflictsAsync(desiredState, cancellationToken).ConfigureAwait(false);
+            ILibraryOperationResult conflictResults = await CheckLibraryForConflictsAsync(desiredState, cancellationToken).ConfigureAwait(false);
 
-            if (!conflictResults.All(r => r.Success))
+            if (!conflictResults.Success)
             {
                 return conflictResults;
             }
@@ -243,7 +243,7 @@ namespace Microsoft.Web.LibraryManager
                 AddLibrary(desiredState);
             }
 
-            return new [] { result };
+            return result;
         }
 
         private ILibraryInstallationState SetDefaultProviderIfNeeded(LibraryInstallationState desiredState)
@@ -261,14 +261,15 @@ namespace Microsoft.Web.LibraryManager
             return desiredState;
         }
 
-        private async Task<IEnumerable<ILibraryOperationResult>> CheckLibraryForConflictsAsync(ILibraryInstallationState desiredState, CancellationToken cancellationToken)
+        private async Task<ILibraryOperationResult> CheckLibraryForConflictsAsync(ILibraryInstallationState desiredState, CancellationToken cancellationToken)
         {
             var libraries = new List<ILibraryInstallationState>(Libraries);
             libraries.Add(desiredState);
 
             IEnumerable<ILibraryOperationResult> fileConflicts = await LibrariesValidator.GetLibrariesErrorsAsync(libraries, _dependencies, DefaultDestination, DefaultProvider, cancellationToken).ConfigureAwait(false);
 
-            return fileConflicts;
+            // consolidate all the errors in one result
+            return new LibraryOperationResult(desiredState, fileConflicts.SelectMany(r => r.Errors).ToArray());
         }
 
         /// <summary>

src/libman/Commands/InstallCommand.cs

@@ -132,15 +132,15 @@ namespace Microsoft.Web.LibraryManager.Tools.Commands
                 destinationToUse = InstallDestination;
             }
 
-            IEnumerable<ILibraryOperationResult> results = await _manifest.InstallLibraryAsync(
-                                                                library.Name,
-                                                                library.Version,
-                                                                providerIdToUse,
-                                                                files,
-                                                                destinationToUse,
-                                                                CancellationToken.None);
+            ILibraryOperationResult result = await _manifest.InstallLibraryAsync(
+                library.Name,
+                library.Version,
+                providerIdToUse,
+                files,
+                destinationToUse,
+                CancellationToken.None);
 
-            if (results.All(r => r.Success))
+            if (result.Success)
             {
                 await _manifest.SaveAsync(Settings.ManifestFileName, CancellationToken.None);
                 Logger.Log(string.Format(Resources.Text.InstalledLibrary, libraryId, InstallDestination), LogLevel.Operation);
@@ -149,15 +149,12 @@ namespace Microsoft.Web.LibraryManager.Tools.Commands
             {
                 bool isFileConflicts = false;
                 Logger.Log(string.Format(Resources.Text.InstallLibraryFailed, libraryId), LogLevel.Error);
-                foreach (ILibraryOperationResult result in results)
+                foreach (IError error in result.Errors)
                 {
-                    foreach (IError error in result.Errors)
+                    Logger.Log(string.Format("[{0}]: {1}", error.Code, error.Message), LogLevel.Error);
+                    if (error.Code == PredefinedErrors.ConflictingFilesInManifest("", new List<string>()).Code)
                     {
-                        Logger.Log(string.Format("[{0}]: {1}", error.Code, error.Message), LogLevel.Error);
-                        if(error.Code == PredefinedErrors.ConflictingFilesInManifest("", new List<string>()).Code)
-                        {
-                            isFileConflicts = true;
-                        }
+                        isFileConflicts = true;
                     }
                 }
 

test/LibraryManager.Test/ManifestTest.cs

@@ -366,55 +366,54 @@ namespace Microsoft.Web.LibraryManager.Test
             var manifest = Manifest.FromJson("{}", _dependencies);
 
             // Null LibraryId
-            IEnumerable<ILibraryOperationResult> results = await manifest.InstallLibraryAsync(null, null,"cdnjs", null, "wwwroot", CancellationToken.None);
-            Assert.IsFalse(results.First().Success);
-            Assert.AreEqual(1, results.First().Errors.Count);
-            Assert.AreEqual("LIB006", results.First().Errors[0].Code);
+            ILibraryOperationResult result = await manifest.InstallLibraryAsync(null, null,"cdnjs", null, "wwwroot", CancellationToken.None);
+            Assert.IsFalse(result.Success);
+            Assert.AreEqual(1, result.Errors.Count);
+            Assert.AreEqual("LIB006", result.Errors[0].Code);
 
             // Empty ProviderId
-            results = await manifest.InstallLibraryAsync("jquery", "3.2.1", "", null, "wwwroot", CancellationToken.None);
-            Assert.IsFalse(results.First().Success);
-            Assert.AreEqual(1, results.First().Errors.Count);
-            Assert.AreEqual("LIB007", results.First().Errors[0].Code);
+            result = await manifest.InstallLibraryAsync("jquery", "3.2.1", "", null, "wwwroot", CancellationToken.None);
+            Assert.IsFalse(result.Success);
+            Assert.AreEqual(1, result.Errors.Count);
+            Assert.AreEqual("LIB007", result.Errors[0].Code);
 
             // Null destination
-            results = await manifest.InstallLibraryAsync("jquery", "3.2.1", "cdnjs", null, null, CancellationToken.None);
+            result = await manifest.InstallLibraryAsync("jquery", "3.2.1", "cdnjs", null, null, CancellationToken.None);
 
-            Assert.IsFalse(results.First().Success);
-            Assert.AreEqual(1, results.First().Errors.Count);
-            Assert.AreEqual("LIB005", results.First().Errors[0].Code);
+            Assert.IsFalse(result.Success);
+            Assert.AreEqual(1, result.Errors.Count);
+            Assert.AreEqual("LIB005", result.Errors[0].Code);
 
 
             // Valid Options all files.
-            results = await manifest.InstallLibraryAsync("jquery", "3.3.1", "cdnjs", null, "wwwroot", CancellationToken.None);
+            result = await manifest.InstallLibraryAsync("jquery", "3.3.1", "cdnjs", null, "wwwroot", CancellationToken.None);
 
-            Assert.IsTrue(results.First().Success);
-            Assert.AreEqual("wwwroot", results.First().InstallationState.DestinationPath);
-            Assert.AreEqual("jquery", results.First().InstallationState.Name);
-            Assert.AreEqual("3.3.1", results.First().InstallationState.Version);
-            Assert.AreEqual("cdnjs", results.First().InstallationState.ProviderId);
-            Assert.IsNotNull(results.First().InstallationState.Files);
+            Assert.IsTrue(result.Success);
+            Assert.AreEqual("wwwroot", result.InstallationState.DestinationPath);
+            Assert.AreEqual("jquery", result.InstallationState.Name);
+            Assert.AreEqual("3.3.1", result.InstallationState.Version);
+            Assert.AreEqual("cdnjs", result.InstallationState.ProviderId);
 
             // Valid parameters and files.
             var files = new List<string>() { "jquery.min.js" };
-            results = await manifest.InstallLibraryAsync("jquery", "2.2.0", "cdnjs", files, "wwwroot2", CancellationToken.None);
-            Assert.IsFalse(results.First().Success);
-            Assert.AreEqual(1, results.First().Errors.Count);
-            Assert.AreEqual("LIB019", results.First().Errors[0].Code);
+            result = await manifest.InstallLibraryAsync("jquery", "2.2.0", "cdnjs", files, "wwwroot2", CancellationToken.None);
+            Assert.IsFalse(result.Success);
+            Assert.AreEqual(1, result.Errors.Count);
+            Assert.AreEqual("LIB019", result.Errors[0].Code);
 
             // Valid parameters invalid files
             files.Add("abc.js");
-            results = await manifest.InstallLibraryAsync("twitter-bootstrap", "4.1.1", "cdnjs", files, "wwwroot3", CancellationToken.None);
-            Assert.IsFalse(results.First().Success);
-            Assert.AreEqual(1, results.First().Errors.Count);
-            Assert.AreEqual("LIB018", results.First().Errors[0].Code);
+            result = await manifest.InstallLibraryAsync("twitter-bootstrap", "4.1.1", "cdnjs", files, "wwwroot3", CancellationToken.None);
+            Assert.IsFalse(result.Success);
+            Assert.AreEqual(1, result.Errors.Count);
+            Assert.AreEqual("LIB018", result.Errors[0].Code);
         }
 
         [TestMethod]
         public async Task InstallLibraryAsync_SetsDefaultProvider()
         {
             var manifest = Manifest.FromJson(_emptyLibmanJson, _dependencies);
-            IEnumerable<ILibraryOperationResult> results = await manifest.InstallLibraryAsync("jquery", "3.2.1", "cdnjs", null, "wwwroot", CancellationToken.None);
+            _ = await manifest.InstallLibraryAsync("jquery", "3.2.1", "cdnjs", null, "wwwroot", CancellationToken.None);
 
             Assert.AreEqual("cdnjs", manifest.DefaultProvider);
             var libraryState = manifest.Libraries.First() as LibraryInstallationState;