Skip to content

Commit 42d7785

Browse files
authored
Fix DisplayPackage Perf regression (#6667)
1 parent e1d0485 commit 42d7785

3 files changed

Lines changed: 67 additions & 11 deletions

File tree

src/NuGetGallery/Controllers/PackagesController.cs

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,16 +1222,25 @@ public virtual ActionResult DeleteSymbols(string id, string version)
12221222

12231223
var model = new DeletePackageViewModel(package, currentUser, DeleteReasons);
12241224

1225+
// Fetch all the available symbols package for all the versions from the
1226+
// database since the DisplayPackageViewModel(base class for DeletePackageViewModel) does not
1227+
// set the `LatestSymbolsPackage` data on the model(since it is an unnecessary and expensive db
1228+
// query). It is fine to do this here when invoking delete page. Note: this could also potentially
1229+
// cause unbounded(high number) db calls based on the number of versions associated with a package.
1230+
var packageViewModelsForAllAvailableSymbolsPackage = package
1231+
.PackageRegistration
1232+
.Packages
1233+
.Where(p => p.PackageStatusKey != PackageStatus.Deleted)
1234+
.Select(p => p.LatestSymbolPackage())
1235+
.Where(sp => sp != null && sp.StatusKey == PackageStatus.Available)
1236+
.Select(sp => new PackageViewModel(sp.Package));
1237+
12251238
model.VersionSelectList = new SelectList(
1226-
model
1227-
.PackageVersions
1228-
.Where(p => !p.Deleted
1229-
&& p.LatestSymbolsPackage != null
1230-
&& p.LatestSymbolsPackage.StatusKey == PackageStatus.Available)
1231-
.Select(p => new
1239+
packageViewModelsForAllAvailableSymbolsPackage
1240+
.Select(pvm => new
12321241
{
1233-
text = p.NuGetVersion.ToFullString() + (p.LatestVersionSemVer2 ? " (Latest)" : string.Empty),
1234-
url = Url.DeleteSymbolsPackage(p)
1242+
text = pvm.NuGetVersion.ToFullString() + (pvm.LatestVersionSemVer2 ? " (Latest)" : string.Empty),
1243+
url = Url.DeleteSymbolsPackage(pvm)
12351244
}), "url", "text", Url.DeleteSymbolsPackage(model));
12361245

12371246
return View(model);

src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,6 @@ public DisplayPackageViewModel(Package package, User currentUser, string pushedB
5555

5656
PushedBy = pushedBy;
5757

58-
LatestSymbolsPackage = package.LatestSymbolPackage();
59-
6058
InitializeRepositoryMetadata(package.RepositoryUrl, package.RepositoryType);
6159

6260
if (PackageHelper.TryPrepareUrlForRendering(package.ProjectUrl, out string projectUrl))
@@ -65,7 +63,7 @@ public DisplayPackageViewModel(Package package, User currentUser, string pushedB
6563
}
6664

6765
var licenseExpression = package.LicenseExpression;
68-
if (!String.IsNullOrWhiteSpace(licenseExpression))
66+
if (!string.IsNullOrWhiteSpace(licenseExpression))
6967
{
7068
LicenseUrl = LicenseExpressionRedirectUrlHelper.GetLicenseExpressionRedirectUrl(licenseExpression);
7169
}

tests/NuGetGallery.Facts/ViewModels/DisplayPackageViewModelFacts.cs

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,55 @@ public void TheCtorSortsPackageVersionsProperly()
167167
Assert.Equal("1.0.10", packageVersions[0].Version);
168168
}
169169

170+
[Fact]
171+
public void TheCtorDoesNotPopulateLatestSymbolsPackageForHistory()
172+
{
173+
var package = new Package
174+
{
175+
Version = "1.0.0",
176+
Dependencies = Enumerable.Empty<PackageDependency>().ToList(),
177+
PackageRegistration = new PackageRegistration
178+
{
179+
Owners = Enumerable.Empty<User>().ToList(),
180+
}
181+
};
182+
183+
package.SymbolPackages.Add(new SymbolPackage()
184+
{
185+
Package = package,
186+
StatusKey = PackageStatus.Available
187+
});
188+
189+
package.PackageRegistration.Packages = new[]
190+
{
191+
new Package { Version = "1.0.0-alpha2", PackageRegistration = package.PackageRegistration },
192+
new Package { Version = "1.0.0", PackageRegistration = package.PackageRegistration },
193+
new Package { Version = "1.0.0-alpha", PackageRegistration = package.PackageRegistration },
194+
new Package { Version = "1.0.0-beta", PackageRegistration = package.PackageRegistration },
195+
new Package { Version = "1.0.2-beta", PackageRegistration = package.PackageRegistration },
196+
new Package { Version = "1.0.2", PackageRegistration = package.PackageRegistration },
197+
new Package { Version = "1.0.10", PackageRegistration = package.PackageRegistration }
198+
};
199+
200+
foreach (var packageVersion in package.PackageRegistration.Packages)
201+
{
202+
packageVersion.SymbolPackages.Add(new SymbolPackage()
203+
{
204+
Package = packageVersion,
205+
StatusKey = PackageStatus.Available
206+
});
207+
}
208+
209+
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
210+
211+
// Descending
212+
Assert.NotNull(viewModel.LatestSymbolsPackage);
213+
foreach (var version in viewModel.PackageVersions)
214+
{
215+
Assert.Null(version.LatestSymbolsPackage);
216+
}
217+
}
218+
170219
[Fact]
171220
public void TheCtorReturnsLatestSymbolPackageByDateCreated()
172221
{

0 commit comments

Comments
 (0)