Restore: eliminate unnecessary call to DependencyGraphSpec.GetClosure…

…(...) (NuGet#3256)

Fix NuGet/Home#9042.

src/NuGet.Core/NuGet.Commands/RestoreCommand/RequestFactory/DependencyGraphSpecRequestProvider.cs

@@ -72,16 +72,21 @@ private IReadOnlyList<RestoreSummaryRequest> GetRequestsFromItems(RestoreArgs re
                 // Parallel.Foreach has an optimization for Arrays, so calling .ToArray() is better and adds almost no overhead
                 Parallel.ForEach(dgFile.Restore.ToArray(), parallelOptions, projectNameToRestore =>
                 {
-                    var closure = dgFile.GetClosure(projectNameToRestore);
-
-                    var projectDependencyGraphSpec = dgFile.WithProjectClosure(projectNameToRestore);
+                    IReadOnlyList<PackageSpec> closure = dgFile.GetClosure(projectNameToRestore);
+                    DependencyGraphSpec projectDependencyGraphSpec = dgFile.CreateFromClosure(projectNameToRestore, closure);
 
                     var externalClosure = new HashSet<ExternalProjectReference>(closure.Select(GetExternalProject));
 
-                    var rootProject = externalClosure.Single(p =>
+                    ExternalProjectReference rootProject = externalClosure.Single(p =>
                         StringComparer.Ordinal.Equals(projectNameToRestore, p.UniqueName));
 
-                    var request = Create(projectNameToRestore, rootProject, externalClosure, restoreContext, projectDgSpec: projectDependencyGraphSpec, settingsLoadingContext: settingsLoadingContext);
+                    RestoreSummaryRequest request = Create(
+                        projectNameToRestore,
+                        rootProject,
+                        externalClosure,
+                        restoreContext,
+                        projectDependencyGraphSpec,
+                        settingsLoadingContext);
 
                     if (request.Request.ProjectStyle == ProjectStyle.DotnetCliTool)
                     {
@@ -94,8 +99,9 @@ private IReadOnlyList<RestoreSummaryRequest> GetRequestsFromItems(RestoreArgs re
                     }
                 });
             }
+
             // Filter out duplicate tool restore requests
-            foreach (var subSetRequest in ToolRestoreUtility.GetSubSetRequests(toolRequests))
+            foreach (RestoreSummaryRequest subSetRequest in ToolRestoreUtility.GetSubSetRequests(toolRequests))
             {
                 requests.Add(subSetRequest);
             }
@@ -105,11 +111,9 @@ private IReadOnlyList<RestoreSummaryRequest> GetRequestsFromItems(RestoreArgs re
 
         public static IEnumerable<ExternalProjectReference> GetExternalClosure(DependencyGraphSpec dgFile, string projectNameToRestore)
         {
-            var closure = dgFile.GetClosure(projectNameToRestore);
-
-            var externalClosure = closure.Select(GetExternalProject);
-            return externalClosure;
+            IReadOnlyList<PackageSpec> closure = dgFile.GetClosure(projectNameToRestore);
 
+            return closure.Select(GetExternalProject);
         }
 
         private static ExternalProjectReference GetExternalProject(PackageSpec rootProject)
@@ -152,7 +156,7 @@ private RestoreSummaryRequest Create(
                 sources,
                 restoreArgs.CacheContext,
                 restoreArgs.Log);
-            
+
             var rootPath = Path.GetDirectoryName(project.PackageSpec.FilePath);
 
             // Create request
@@ -170,7 +174,7 @@ private RestoreSummaryRequest Create(
                 DependencyGraphSpec = projectDgSpec,
                 MSBuildProjectExtensionsPath = projectPackageSpec.RestoreMetadata.OutputPath
             };
-            
+
             var restoreLegacyPackagesDirectory = project.PackageSpec?.RestoreMetadata?.LegacyPackagesDirectory
                 ?? DefaultRestoreLegacyPackagesDirectory;
             request.IsLowercasePackagesDirectory = !restoreLegacyPackagesDirectory;
@@ -200,14 +204,6 @@ private string GetPackagesPath(RestoreArgs restoreArgs, PackageSpec project)
             return project.RestoreMetadata.PackagesPath;
         }
 
-        private void UpdateSources(ProjectRestoreMetadata project, List<SourceRepository> sources)
-        {
-            project.Sources.Clear();
-            foreach (var source in sources)
-            {
-                project.Sources.Add(source.PackageSource);
-            }
-        }
         /// <summary>
         /// Return all references for a given project path.
         /// References is modified by this method.

src/NuGet.Core/NuGet.ProjectModel/DependencyGraphSpec.cs

@@ -144,6 +144,39 @@ public DependencyGraphSpec WithProjectClosure(string projectUniqueName)
             return projectDependencyGraphSpec;
         }
 
+        /// <summary>
+        /// Creates a new <see cref="DependencyGraphSpec" /> from a project name and project closure.
+        /// </summary>
+        /// <param name="projectUniqueName">The project's unique name.</param>
+        /// <param name="closure">The project's closure</param>
+        /// <returns>A <see cref="DependencyGraphSpec" />.</returns>
+        /// <exception cref="ArgumentException">Thrown if <paramref name="projectUniqueName" />
+        /// is <c>null</c> or an empty string.</exception>
+        /// <exception cref="ArgumentNullException">Thrown if <paramref name="closure" /> is <c>null</c>.</exception>
+        public DependencyGraphSpec CreateFromClosure(string projectUniqueName, IReadOnlyList<PackageSpec> closure)
+        {
+            if (string.IsNullOrEmpty(projectUniqueName))
+            {
+                throw new ArgumentException(Strings.ArgumentNullOrEmpty, nameof(projectUniqueName));
+            }
+
+            if (closure == null)
+            {
+                throw new ArgumentNullException(nameof(closure));
+            }
+
+            var dgSpec = new DependencyGraphSpec();
+
+            dgSpec.AddRestore(projectUniqueName);
+
+            foreach (PackageSpec packageSpec in closure)
+            {
+                dgSpec.AddProject(_isReadOnly ? packageSpec : packageSpec.Clone());
+            }
+
+            return dgSpec;
+        }
+
         /// <summary>
         /// Retrieve the full project closure including the root project itself.
         /// </summary>

test/NuGet.Core.Tests/NuGet.ProjectModel.Test/DependencyGraphSpecTests.cs

@@ -618,6 +618,78 @@ public void AddProject_WhenRestoreMetadataIsNull_AddsProject()
                 actualResult => Assert.Same(expectedResult, actualResult));
         }
 
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        public void CreateFromClosure_WhenProjectUniqueNameIsNullOrEmpty_Throws(string projectUniqueName)
+        {
+            IReadOnlyList<PackageSpec> closure = new[] { new PackageSpec() };
+            var dgSpec = new DependencyGraphSpec();
+
+            ArgumentException exception = Assert.Throws<ArgumentException>(
+                () => dgSpec.CreateFromClosure(projectUniqueName, closure));
+
+            Assert.Equal("projectUniqueName", exception.ParamName);
+        }
+
+        [Fact]
+        public void CreateFromClosure_WhenClosureIsNull_Throws()
+        {
+            var dgSpec = new DependencyGraphSpec();
+
+            ArgumentNullException exception = Assert.Throws<ArgumentNullException>(
+                () => dgSpec.CreateFromClosure(PackageSpecName, closure: null));
+
+            Assert.Equal("closure", exception.ParamName);
+        }
+
+        [Fact]
+        public void CreateFromClosure_WhenReadOnlyIsTrue_ReturnsSameClosure()
+        {
+            var expectedResult = new PackageSpec()
+            {
+                RestoreMetadata = new ProjectRestoreMetadata()
+            };
+            IReadOnlyList<PackageSpec> closure = new[] { expectedResult };
+            var dgSpec = new DependencyGraphSpec(isReadOnly: true);
+
+            DependencyGraphSpec newDgSpec = dgSpec.CreateFromClosure(PackageSpecName, closure);
+
+            Assert.Collection(
+                newDgSpec.Restore,
+                actualResult => Assert.Equal(PackageSpecName, actualResult));
+
+            Assert.Collection(
+                newDgSpec.Projects,
+                actualResult => Assert.Same(expectedResult, actualResult));
+        }
+
+        [Fact]
+        public void CreateFromClosure_WhenReadOnlyIsFalse_ReturnsClonedClosure()
+        {
+            var expectedResult = new PackageSpec()
+            {
+                IsDefaultVersion = false,
+                RestoreMetadata = new ProjectRestoreMetadata()
+            };
+            IReadOnlyList<PackageSpec> closure = new[] { expectedResult };
+            var dgSpec = new DependencyGraphSpec(isReadOnly: false);
+
+            DependencyGraphSpec newDgSpec = dgSpec.CreateFromClosure(PackageSpecName, closure);
+
+            Assert.Collection(
+                newDgSpec.Restore,
+                actualResult => Assert.Equal(PackageSpecName, actualResult));
+
+            Assert.Collection(
+                newDgSpec.Projects,
+                actualResult =>
+                {
+                    Assert.True(expectedResult.Equals(actualResult));
+                    Assert.NotSame(expectedResult, actualResult);
+                });
+        }
+
         private static DependencyGraphSpec CreateDependencyGraphSpec()
         {
             var dgSpec = new DependencyGraphSpec();