From a816de1e800b7755a6658687923a7af4f23c987d Mon Sep 17 00:00:00 2001 From: Joel Bennett Date: Wed, 4 Mar 2015 01:36:11 -0500 Subject: [PATCH] (GH-143) Ensure NuGetList streams output Remove ToList() in the calls to NuGetList.GetPackages() and return an IEnumerable to ensure that calls to List stream output instead of performing blocking operations to return results from searching package feeds. --- .../commands/ChocolateyPinCommandSpecs.cs | 12 ++++---- .../commands/ChocolateyPinCommand.cs | 5 ++-- .../infrastructure.app/nuget/NugetList.cs | 12 ++++---- .../services/ChocolateyPackageService.cs | 8 +++--- .../services/INugetService.cs | 3 +- .../services/NugetService.cs | 28 +++++++------------ 6 files changed, 31 insertions(+), 37 deletions(-) diff --git a/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs b/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs index 21d5121208..95ae4330cd 100644 --- a/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs +++ b/src/chocolatey.tests/infrastructure.app/commands/ChocolateyPinCommandSpecs.cs @@ -337,10 +337,12 @@ public override void Context() configuration.Sources = ApplicationParameters.PackagesLocation; configuration.ListCommand.LocalOnly = true; configuration.AllVersions = true; - var packageResults = new ConcurrentDictionary(); - packageResults.GetOrAdd("regular", new PackageResult(package.Object, null)); - packageResults.GetOrAdd("pinned", new PackageResult(pinnedPackage.Object, null)); - nugetService.Setup(n => n.list_run(It.IsAny(), false)).Returns(packageResults); + var packageResults = new [] + { + new PackageResult(package.Object, null), + new PackageResult(pinnedPackage.Object, null) + }; + nugetService.Setup(n => n.list_run(It.IsAny(), true)).Returns(packageResults); configuration.PinCommand.Command = PinCommandType.list; } @@ -412,7 +414,7 @@ public void should_call_nuget_service_list_run_when_command_is_list() configuration.PinCommand.Command = PinCommandType.list; command.run(configuration); - nugetService.Verify(n => n.list_run(It.IsAny(), false), Times.Once); + nugetService.Verify(n => n.list_run(It.IsAny(), true), Times.Once); } [Pending("NuGet is killing me with extension methods. Need to find proper item to mock out to return the package object.")] diff --git a/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs b/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs index 180d233c6f..11ab93045f 100644 --- a/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs +++ b/src/chocolatey/infrastructure.app/commands/ChocolateyPinCommand.cs @@ -139,10 +139,9 @@ public void run(ChocolateyConfiguration configuration) public void list_pins(IPackageManager packageManager, ChocolateyConfiguration config) { - var localPackages = _nugetService.list_run(config, logResults: false); - foreach (var pkg in localPackages.or_empty_list_if_null()) + foreach (var pkg in _nugetService.list_run(config, logResults: true)) { - var pkgInfo = _packageInfoService.get_package_information(pkg.Value.Package); + var pkgInfo = _packageInfoService.get_package_information(pkg.Package); if (pkgInfo != null && pkgInfo.IsPinned) { this.Log().Info(() => "{0}|{1}".format_with(pkgInfo.Package.Id, pkgInfo.Package.Version)); diff --git a/src/chocolatey/infrastructure.app/nuget/NugetList.cs b/src/chocolatey/infrastructure.app/nuget/NugetList.cs index 957cc6e04a..58ab1dc6a8 100644 --- a/src/chocolatey/infrastructure.app/nuget/NugetList.cs +++ b/src/chocolatey/infrastructure.app/nuget/NugetList.cs @@ -22,7 +22,7 @@ namespace chocolatey.infrastructure.app.nuget // ReSharper disable InconsistentNaming - public sealed class NugetList + public static class NugetList { public static IEnumerable GetPackages(ChocolateyConfiguration configuration, ILogger nugetLogger) { @@ -31,7 +31,7 @@ public static IEnumerable GetPackages(ChocolateyConfiguration configur if (configuration.AllVersions) { - return results.Where(PackageExtensions.IsListed).OrderBy(p => p.Id).ToList(); + return results.Where(PackageExtensions.IsListed).OrderBy(p => p.Id); } if (configuration.Prerelease && packageRepository.SupportsPrereleasePackages) @@ -44,10 +44,10 @@ public static IEnumerable GetPackages(ChocolateyConfiguration configur } return results.OrderBy(p => p.Id) - .AsEnumerable() - .Where(PackageExtensions.IsListed) - .Where(p => configuration.Prerelease || p.IsReleaseVersion()) - .distinct_last(PackageEqualityComparer.Id, PackageComparer.Version).ToList(); + .AsEnumerable() + .Where(PackageExtensions.IsListed) + .Where(p => configuration.Prerelease || p.IsReleaseVersion()) + .distinct_last(PackageEqualityComparer.Id, PackageComparer.Version); } } diff --git a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs index 02108c33e7..1c781faac0 100644 --- a/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs +++ b/src/chocolatey/infrastructure.app/services/ChocolateyPackageService.cs @@ -89,7 +89,7 @@ public void list_run(ChocolateyConfiguration config, bool logResults) var list = _nugetService.list_run(config, logResults: true); if (config.RegularOutput) { - this.Log().Warn(() => @"{0} packages {1}.".format_with(list.Count, config.ListCommand.LocalOnly ? "installed" : "found")); + this.Log().Warn(() => @"{0} packages {1}.".format_with(list.Count(), config.ListCommand.LocalOnly ? "installed" : "found")); if (config.ListCommand.LocalOnly && config.ListCommand.IncludeRegistryPrograms) { @@ -99,14 +99,14 @@ public void list_run(ChocolateyConfiguration config, bool logResults) } } - private void report_registry_programs(ChocolateyConfiguration config, ConcurrentDictionary list) + private void report_registry_programs(ChocolateyConfiguration config, IEnumerable list) { var itemsToRemoveFromMachine = new List(); foreach (var packageResult in list) { - if (packageResult.Value != null && packageResult.Value.Package != null) + if (packageResult != null && packageResult.Package != null) { - var pkginfo = _packageInfoService.get_package_information(packageResult.Value.Package); + var pkginfo = _packageInfoService.get_package_information(packageResult.Package); if (pkginfo.RegistrySnapshot == null) { continue; diff --git a/src/chocolatey/infrastructure.app/services/INugetService.cs b/src/chocolatey/infrastructure.app/services/INugetService.cs index 63d8ac92bb..29371dbc57 100644 --- a/src/chocolatey/infrastructure.app/services/INugetService.cs +++ b/src/chocolatey/infrastructure.app/services/INugetService.cs @@ -17,6 +17,7 @@ namespace chocolatey.infrastructure.app.services { using System; using System.Collections.Concurrent; + using System.Collections.Generic; using configuration; using results; @@ -34,7 +35,7 @@ public interface INugetService /// The configuration. /// Should results be logged? /// - ConcurrentDictionary list_run(ChocolateyConfiguration config, bool logResults); + IEnumerable list_run(ChocolateyConfiguration config, bool logResults); /// /// Run pack in noop mode. diff --git a/src/chocolatey/infrastructure.app/services/NugetService.cs b/src/chocolatey/infrastructure.app/services/NugetService.cs index 63da94e3ef..d581eacf13 100644 --- a/src/chocolatey/infrastructure.app/services/NugetService.cs +++ b/src/chocolatey/infrastructure.app/services/NugetService.cs @@ -74,40 +74,34 @@ public void list_noop(ChocolateyConfiguration config) )); } - public ConcurrentDictionary list_run(ChocolateyConfiguration config, bool logResults = true) + public IEnumerable list_run(ChocolateyConfiguration config, bool logResults) { - var packageResults = new ConcurrentDictionary(); - if (config.RegularOutput) this.Log().Debug(() => "Running list with the following filter = '{0}'".format_with(config.Input)); - - var packages = NugetList.GetPackages(config, _nugetLogger).ToList(); - if (config.RegularOutput) this.Log().Debug(() => "--- Start of List ---"); - foreach (var package in packages.or_empty_list_if_null()) + foreach (var package in NugetList.GetPackages(config, _nugetLogger)) { + var pkg = package; // for lamda access if (logResults) { if (config.RegularOutput) { - this.Log().Info(config.Verbose ? ChocolateyLoggers.Important : ChocolateyLoggers.Normal, () => "{0} {1}".format_with(package.Id, package.Version.to_string())); - if (config.Verbose) this.Log().Info(() => " {0}{1} Description: {2}{1} Tags: {3}{1} Number of Downloads: {4}{1}".format_with(package.Title.escape_curly_braces(), Environment.NewLine, package.Description.escape_curly_braces(), package.Tags.escape_curly_braces(), package.DownloadCount <= 0 ? "n/a" : package.DownloadCount.to_string())); - // Maintainer(s):{3}{1} | package.Owners.join(", ") - null at the moment + this.Log().Info(config.Verbose ? ChocolateyLoggers.Important : ChocolateyLoggers.Normal, () => "{0} {1}".format_with(pkg.Id, pkg.Version.to_string())); + if (config.Verbose) this.Log().Info(() => " {0}{1} Description: {2}{1} Tags: {3}{1} Number of Downloads: {4}{1}".format_with(pkg.Title.escape_curly_braces(), Environment.NewLine, pkg.Description.escape_curly_braces(), pkg.Tags.escape_curly_braces(), pkg.DownloadCount <= 0 ? "n/a" : pkg.DownloadCount.to_string())); + // Maintainer(s):{3}{1} | pkg.Owners.join(", ") - null at the moment } else { - this.Log().Info(config.Verbose ? ChocolateyLoggers.Important : ChocolateyLoggers.Normal, () => "{0}|{1}".format_with(package.Id, package.Version.to_string())); + this.Log().Info(config.Verbose ? ChocolateyLoggers.Important : ChocolateyLoggers.Normal, () => "{0}|{1}".format_with(pkg.Id, pkg.Version.to_string())); } } else { - this.Log().Debug(() => "{0} {1}".format_with(package.Id, package.Version.to_string())); + this.Log().Debug(() => "{0} {1}".format_with(pkg.Id, pkg.Version.to_string())); } - packageResults.GetOrAdd(package.Id, new PackageResult(package, null)); + yield return new PackageResult(pkg, null); } if (config.RegularOutput) this.Log().Debug(() => "--- End of List ---"); - - return packageResults; } public void pack_noop(ChocolateyConfiguration config) @@ -1034,13 +1028,11 @@ private void set_package_names_if_all_is_specified(ChocolateyConfiguration confi var input = config.Input; config.Input = string.Empty; - var localPackages = list_run(config, logResults: false); - + config.PackageNames = list_run(config, false).Select(p => p.Name).@join(ApplicationParameters.PackageNamesSeparator); config.Input = input; config.Noop = noop; config.Prerelease = pre; config.Sources = sources; - config.PackageNames = string.Join(ApplicationParameters.PackageNamesSeparator, localPackages.Select((p) => p.Key).or_empty_list_if_null()); if (customAction != null) customAction.Invoke(); }