Merge pull request #3471 from josh-cooley/bug/reduce-dependency-fetch

(#3451) Use availablePackages in GetPackageDependencies

src/chocolatey.tests.integration/scenarios/UpgradeScenarios.cs

@@ -2946,6 +2946,10 @@ public void Should_not_have_warning_package_result()
             }
         }
 
+        /// <summary>
+        /// This test suite tests upgrading isdependency from 1.0.0 while hasdependency is pinned to a version that does not allow
+        /// for isdependency to upgrade to the latest available in the source.
+        /// </summary>
         public class When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version : ScenariosBase
         {
             public override void Context()

src/chocolatey.tests/infrastructure.app/nuget/NugetCommonSpecs.cs

@@ -17,15 +17,21 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Threading.Tasks;
+using System.Threading;
+using Chocolatey.NuGet.Frameworks;
 using chocolatey.infrastructure.app;
 using chocolatey.infrastructure.app.configuration;
 using chocolatey.infrastructure.app.nuget;
 using chocolatey.infrastructure.filesystem;
 using Moq;
 using NuGet.Common;
+using NuGet.Configuration;
 using NuGet.Packaging;
+using NuGet.Packaging.Core;
 using NuGet.Protocol;
 using NuGet.Protocol.Core.Types;
+using NuGet.Versioning;
 using FluentAssertions;
 
 namespace chocolatey.tests.infrastructure.app.nuget
@@ -179,5 +185,86 @@ public void Should_set_user_agent_string()
                 UserAgent.UserAgentString.Should().StartWith(expectedUserAgentString);
             }
         }
+
+        private class When_gets_package_dependencies : TinySpec
+        {
+            private Func<Task> _because;
+            private readonly Mock<SourceCacheContext> _sourceCacheContext = new Mock<SourceCacheContext>();
+            private readonly Mock<ILogger> _nugetLogger = new Mock<ILogger>();
+            private readonly List<NuGetEndpointResources> _nuGetEndpointResources = new List<NuGetEndpointResources>();
+            private readonly HashSet<SourcePackageDependencyInfo> _sourcePackageDependencyInfos = new HashSet<SourcePackageDependencyInfo>();
+            private readonly HashSet<PackageDependency> _packageDependencies = new HashSet<PackageDependency>();
+            private readonly Mock<SourceRepository> _sourceRepository = new Mock<SourceRepository>();
+            private readonly Mock<DependencyInfoResource> _dependencyInfoResource = new Mock<DependencyInfoResource>();
+            private PackageSource _packageSource;
+            private ChocolateyConfiguration _configuration;
+
+            public override void Context()
+            {
+                _configuration = new ChocolateyConfiguration();
+                _sourceCacheContext.ResetCalls();
+                _nugetLogger.ResetCalls();
+                _sourceRepository.ResetCalls();
+                _nuGetEndpointResources.Clear();
+                _sourcePackageDependencyInfos.Clear();
+                _packageDependencies.Clear();
+                _sourceRepository.Setup(r => r.GetResource<DependencyInfoResource>(It.IsAny<SourceCacheContext>())).Returns(_dependencyInfoResource.Object);
+                _packageSource = new PackageSource("C:\\packages");
+                _sourceRepository.SetupGet(r => r.PackageSource).Returns(_packageSource);
+
+                var chocolateySourceCacheContext = new ChocolateySourceCacheContext(_configuration);
+                _nuGetEndpointResources.Add(NuGetEndpointResources.GetResourcesBySource(_sourceRepository.Object, chocolateySourceCacheContext));
+            }
+
+            public override void Because()
+            {
+                _because = () => NugetCommon.GetPackageDependencies(new PackageIdentity("a", new NuGetVersion(1, 0, 1000)), NuGetFramework.AnyFramework,
+                    _sourceCacheContext.Object, _nugetLogger.Object, _nuGetEndpointResources, _sourcePackageDependencyInfos, _packageDependencies, _configuration);
+            }
+
+            [Fact]
+            public async Task Should_request_dependencies_once()
+            {
+                Context();
+
+                var adeps = new[] { new PackageDependency("b", new VersionRange(new NuGetVersion(1, 0, 1000), true, new NuGetVersion(2, 0, 0), false)), new PackageDependency("c", new VersionRange(new NuGetVersion(1, 0, 1), true, new NuGetVersion(2, 0, 0), false)) };
+                var adepInfo = new SourcePackageDependencyInfo("a", new NuGetVersion(1, 0, 1000), adeps, true, _sourceRepository.Object);
+                _dependencyInfoResource.Setup(r => r.ResolvePackage(It.Is<PackageIdentity>(pid => pid.Id == "a" && pid.Version == new NuGetVersion(1, 0, 1000)), It.IsAny<NuGetFramework>(), It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(adepInfo);
+                var bdeps = new[] { new PackageDependency("d", new VersionRange(new NuGetVersion(1, 0, 1000), true, new NuGetVersion(2, 0, 0), false)) };
+                var bdepInfo = new[] { new SourcePackageDependencyInfo("b", new NuGetVersion(1, 0, 1000), bdeps, true, _sourceRepository.Object) };
+                _dependencyInfoResource.Setup(r => r.ResolvePackages("b", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(bdepInfo);
+                var cdeps = new[] { new PackageDependency("d", new VersionRange(new NuGetVersion(1, 0, 1), true, new NuGetVersion(2, 0, 0), false)) };
+                var cdepInfo = new[] { new SourcePackageDependencyInfo("c", new NuGetVersion(1, 0, 1), cdeps, true, _sourceRepository.Object) };
+                _dependencyInfoResource.Setup(r => r.ResolvePackages("c", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(cdepInfo);
+                var ddepInfo = GetDependencies("d", "e", "1.0.0", 1001, "2.0.0");
+                _dependencyInfoResource.Setup(r => r.ResolvePackages("d", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(ddepInfo);
+                var edepInfo = GetDependencies("e", null, "1.0.0", 1001, null);
+                _dependencyInfoResource.Setup(r => r.ResolvePackages("e", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>())).ReturnsAsync(edepInfo);
+
+                await _because();
+
+                _dependencyInfoResource.Verify(r => r.ResolvePackage(It.Is<PackageIdentity>(pid => pid.Id == "a" && pid.Version == new NuGetVersion(1, 0, 1000)), It.IsAny<NuGetFramework>(), It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
+                _dependencyInfoResource.Verify(r => r.ResolvePackages("b", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
+                _dependencyInfoResource.Verify(r => r.ResolvePackages("c", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
+                _dependencyInfoResource.Verify(r => r.ResolvePackages("d", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
+                _dependencyInfoResource.Verify(r => r.ResolvePackages("e", false, NuGetFramework.AnyFramework, It.IsAny<SourceCacheContext>(), It.IsAny<ILogger>(), It.IsAny<CancellationToken>()), Times.Once());
+            }
+
+            private IEnumerable<SourcePackageDependencyInfo> GetDependencies(string packageId, string dependencyId, string lowerRangeStart, int count, string upperRange)
+            {
+                var dependencyInfo = new List<SourcePackageDependencyInfo>();
+                var startVersion = NuGetVersion.Parse(lowerRangeStart);
+                var upperVersion = !string.IsNullOrEmpty(upperRange) ? NuGetVersion.Parse(upperRange) : null;
+                for (var i = startVersion.Patch; i < startVersion.Patch + count; i++)
+                {
+                    var packageDependency = !string.IsNullOrEmpty(dependencyId)
+                        ? new[] { new PackageDependency(dependencyId, new VersionRange(new NuGetVersion(startVersion.Major, startVersion.Minor, i), true, upperVersion, false)) }
+                        : Array.Empty<PackageDependency>();
+                    dependencyInfo.Add(new SourcePackageDependencyInfo(packageId, new NuGetVersion(startVersion.Major, startVersion.Minor, i), packageDependency, true, _sourceRepository.Object));
+                }
+
+                return dependencyInfo;
+            }
+        }
     }
 }

src/chocolatey/infrastructure.app/nuget/NugetCommon.cs

@@ -451,16 +451,27 @@ public static async Task GetPackageDependencies(string packageId,
 
             //if (availablePackages.Contains(packageID)) return;
 
-            var dependencyInfoResources = resources.DependencyInfoResources();
-
-            foreach (var dependencyInfoResource in dependencyInfoResources)
+            foreach (var resource in resources)
             {
-                IEnumerable<SourcePackageDependencyInfo> dependencyInfos = Array.Empty<SourcePackageDependencyInfo>();
+                var dependencyInfoResource = resource.DependencyInfoResource;
+
+                if (dependencyInfoResource is null)
+                {
+                    // We can't lookup any dependencies using this resource.
+                    continue;
+                }
+
+                // check if we already have packages matching the constraints in the list
+                IEnumerable<SourcePackageDependencyInfo> dependencyInfos = availablePackages.Where(
+                    p => string.Equals(p.Id, packageId, StringComparison.OrdinalIgnoreCase) && p.HasVersion && versionRange.Satisfies(p.Version) && p.Source == resource.Source).ToList();
 
                 try
                 {
-                    dependencyInfos = await dependencyInfoResource.ResolvePackages(
-                        packageId, configuration.Prerelease, framework, cacheContext, logger, CancellationToken.None);
+                    if (!dependencyInfos.Any())
+                    {
+                        dependencyInfos = await dependencyInfoResource.ResolvePackages(
+                            packageId, configuration.Prerelease, framework, cacheContext, logger, CancellationToken.None);
+                    }
                 }
                 catch (AggregateException ex) when (!(ex.InnerException is null))
                 {

src/chocolatey/infrastructure.app/services/NugetService.cs

@@ -1327,17 +1327,17 @@ public virtual ConcurrentDictionary<string, PackageResult> Upgrade(ChocolateyCon
                                     null));
 
                         // For an initial attempt at finding a package resolution solution, check for all parent packages (i.e. locally installed packages
-                        // that take a dependency on the package that is currently being upgraded) and find the depdendencies associated with these packages.
-                        // NOTE: All the latest availble package version, as well as the specifically requested package version (if applicable) will be
+                        // that take a dependency on the package that is currently being upgraded) and find the dependencies associated with these packages.
+                        // NOTE: All the latest available package version, as well as the specifically requested package version (if applicable) will be
                         // searched for.  If this don't provide enough information to obtain a solution, then a follow up query in the catch block for this
                         // section of the code will be completed.
                         var parentInfos = new HashSet<SourcePackageDependencyInfo>(PackageIdentityComparer.Default);
                         NugetCommon.GetPackageParents(availablePackage.Identity.Id, parentInfos, localPackagesDependencyInfos).GetAwaiter().GetResult();
                         foreach (var parentPackage in parentInfos)
                         {
-                            if (version != null)
+                            if (parentPackage.HasVersion)
                             {
-                                var requestedPackageDependency = NugetList.FindPackage(parentPackage.Id, config, _nugetLogger, (SourceCacheContext)sourceCacheContext, remoteEndpoints, version);
+                                var requestedPackageDependency = NugetList.FindPackage(parentPackage.Id, config, _nugetLogger, (SourceCacheContext)sourceCacheContext, remoteEndpoints, parentPackage.Version);
 
                                 if (requestedPackageDependency != null)
                                 {

tests/helpers/common-helpers.psm1

@@ -18,6 +18,10 @@ $script:LicenseType = $null
 $script:chocolateyTestLocation = $null
 $script:originalChocolateyInstall = $env:ChocolateyInstall
 
+# Set the Progress Preference to SilentlyContinue. We do not need progress bars in our tests.
+# And they are known to greatly hinder performance on Windows PowerShell.
+$global:ProgressPreference = 'SilentlyContinue'
+
 Get-ChildItem -Path $PSScriptRoot\common -Filter *.ps1 -Recurse | ForEach-Object { . $_.FullName }
 
 # Prepare information that will be useful for troubleshooting.

tests/pester-tests/commands/choco-install.Tests.ps1

@@ -2046,6 +2046,30 @@ To install a local, or remote file, you may use:
         }
     }
 
+    Context 'Installing package with large number of dependency versions' {
+        BeforeAll {
+            Restore-ChocolateyInstallSnapshot -SetWorkDir
+
+            # demo-projects is a zip file that contains 5 packages with over 2000 versions between them.
+            # They can be freshly generated by running the GeneratePackages.ps1 script inside of it with
+            # PowerShell 7+. This zip file is placed in the repository as it is expected to not change,
+            # and makes this test simpler. If this zip file requires regular changes, we would want to
+            # remove it from the repository and store it elsewhere that we can retrieve it during CI testing.
+            Expand-Archive $PSScriptRoot/demo-projects.zip -DestinationPath $PWD
+            $Output = Invoke-Choco install package-a -s 'a,b,c' --confirm
+        }
+
+        It "Exits with Success (0)" {
+            $Output.ExitCode | Should -Be 0 -Because $Output.String
+        }
+
+        It "Completes quickly" {
+            # 30 seconds was chosen for this timeout because the benchmark of Chocolatey was that this would take about 15 seconds.
+            # Prior to the change that this is testing, this execution would be measured in minutes with the fastest being 7 minutes.
+            $Output.Duration | Should -BeLessOrEqual '00:00:30' -Because $Output.String
+        }
+    }
+
     # This needs to be the last test in this block, to ensure NuGet configurations aren't being created.
     # Any tests after this block are expected to generate the configuration as they're explicitly using the NuGet CLI
     Test-NuGetPaths

tests/pester-tests/commands/choco-upgrade.Tests.ps1

@@ -695,6 +695,47 @@ To upgrade a local, or remote file, you may use:
         }
     }
 
+    # This test is taken from UpgradeScenarios.cs from the integration test suite:
+    # When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version
+    # Further details about this test can be found there.
+    Context 'Upgrading a dependency with pinned parent and depends on a range less than upgrade version' {
+        BeforeAll {
+            $DependentPackageName = 'isdependency'
+            Restore-ChocolateyInstallSnapshot
+            $Setup = @(
+                Invoke-Choco install $DependentPackageName --version 1.0.0 --confirm
+                Invoke-Choco install hasdependency --pin --version 1.0.0 --confirm
+            )
+            $Output = Invoke-Choco upgrade $DependentPackageName --confirm
+            $Packages = (Invoke-Choco list -r).Lines | ConvertFrom-ChocolateyOutput -Command List
+        }
+
+        AfterAll {
+            Remove-ChocolateyInstallSnapshot
+        }
+
+        It "Exits with Success (0)" {
+            $Output.ExitCode | Should -Be 0 -Because (@($Setup.String; $Output.String) -join [System.Environment]::NewLine)
+        }
+
+        It "Should report upgraded successfully" {
+            $Output.Lines | Should -Contain "Chocolatey upgraded 1/1 packages." -Because (@($Setup.String; $Output.String) -join [System.Environment]::NewLine)
+        }
+
+        It "Should report package conflicts" {
+            $message = "[NuGet] One or more unresolved package dependency constraints detected in the Chocolatey lib folder. All dependency constraints must be resolved to add or update packages. If these packages are being updated this message may be ignored, if not the following error(s) may be blocking the current package operation: 'hasdependency 1.0.0 constraint: isdependency (>= 1.0.0 && < 2.0.0)'"
+            $Output.Lines | Should -Contain $message -Because (@($Setup.String; $Output.String) -join [System.Environment]::NewLine)
+        }
+
+        It "Should upgrade the dependent package to the highest version in the range" {
+            ($Packages | Where-Object Name -eq $DependentPackageName).Version | Should -Be '1.1.0' -Because (@($Setup.String; $Output.String) -join [System.Environment]::NewLine)
+        }
+
+        It "Should not upgrade the exact version dependency package" {
+            ($Packages | Where-Object Name -eq 'isexactversiondependency').Version | Should -Be '1.0.0' -Because (@($Setup.String; $Output.String) -join [System.Environment]::NewLine)
+        }
+    }
+
     # This needs to be (almost) the last test in this block, to ensure NuGet configurations aren't being created.
     # Any tests after this block are expected to generate the configuration as they're explicitly using the NuGet CLI
     Test-NuGetPaths