Skip to content

Commit 91a4d51

Browse files
author
Scott Bommarito
authored
DisplayPackageViewModel.PackageVersions should not be enumerated multiple times (#6855)
1 parent 576938d commit 91a4d51

5 files changed

Lines changed: 56 additions & 46 deletions

File tree

‎src/NuGetGallery/Controllers/PackagesController.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -658,14 +658,8 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
658658
{
659659
return HttpNotFound();
660660
}
661-
662-
var packageHistory = package
663-
.PackageRegistration
664-
.Packages
665-
.ToList()
666-
.OrderByDescending(p => new NuGetVersion(p.Version));
667-
668-
var model = new DisplayPackageViewModel(package, currentUser, packageHistory);
661+
662+
var model = new DisplayPackageViewModel(package, currentUser);
669663

670664
model.ValidatingTooLong = _validationService.IsValidatingTooLong(package);
671665
model.PackageValidationIssues = _validationService.GetLatestPackageValidationIssues(package);

‎src/NuGetGallery/ViewModels/DeletePackageViewModel.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace NuGetGallery
1313
public class DeletePackageViewModel : DisplayPackageViewModel
1414
{
1515
public DeletePackageViewModel(Package package, User currentUser, IReadOnlyList<ReportPackageReason> reasons)
16-
: base(package, currentUser, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)))
16+
: base(package, currentUser)
1717
{
1818
DeletePackagesRequest = new DeletePackagesRequest
1919
{

‎src/NuGetGallery/ViewModels/DisplayPackageViewModel.cs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace NuGetGallery
1313
{
1414
public class DisplayPackageViewModel : ListPackageItemViewModel
1515
{
16-
public DisplayPackageViewModel(Package package, User currentUser, IOrderedEnumerable<Package> packageHistory)
16+
public DisplayPackageViewModel(Package package, User currentUser)
1717
: this(package, currentUser, (string)null)
1818
{
1919
HasSemVer2Version = NuGetVersion.IsSemVer2;
@@ -23,7 +23,13 @@ public DisplayPackageViewModel(Package package, User currentUser, IOrderedEnumer
2323
.Any(p => (p.HasUpperBound && p.MaxVersion.IsSemVer2) || (p.HasLowerBound && p.MinVersion.IsSemVer2));
2424

2525
Dependencies = new DependencySetsViewModel(package.Dependencies);
26-
PackageVersions = packageHistory.Select(p => new DisplayPackageViewModel(p, currentUser, GetPushedBy(p, currentUser)));
26+
27+
var packageHistory = package
28+
.PackageRegistration
29+
.Packages
30+
.OrderByDescending(p => new NuGetVersion(p.Version))
31+
.ToList();
32+
PackageVersions = packageHistory.Select(p => new DisplayPackageViewModel(p, currentUser, GetPushedBy(p, currentUser))).ToList();
2733

2834
PushedBy = GetPushedBy(package, currentUser);
2935
PackageFileSize = package.PackageFileSize;
@@ -42,7 +48,7 @@ public DisplayPackageViewModel(Package package, User currentUser, IOrderedEnumer
4248
}
4349
}
4450

45-
public DisplayPackageViewModel(Package package, User currentUser, string pushedBy)
51+
private DisplayPackageViewModel(Package package, User currentUser, string pushedBy)
4652
: base(package, currentUser)
4753
{
4854
Copyright = package.Copyright;
@@ -81,7 +87,7 @@ public DisplayPackageViewModel(Package package, User currentUser, string pushedB
8187
public IReadOnlyList<ValidationIssue> PackageValidationIssues { get; set; }
8288
public IReadOnlyList<ValidationIssue> SymbolsPackageValidationIssues { get; set; }
8389
public DependencySetsViewModel Dependencies { get; set; }
84-
public IEnumerable<DisplayPackageViewModel> PackageVersions { get; set; }
90+
public IReadOnlyList<DisplayPackageViewModel> PackageVersions { get; set; }
8591
public string Copyright { get; set; }
8692
public string ReadMeHtml { get; set; }
8793
public DateTime? LastEdited { get; set; }

‎src/NuGetGallery/ViewModels/ManagePackageViewModel.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,7 @@ namespace NuGetGallery
1313
public class ManagePackageViewModel : DisplayPackageViewModel
1414
{
1515
public ManagePackageViewModel(Package package, User currentUser, IReadOnlyList<ReportPackageReason> reasons, UrlHelper url, string readMe)
16-
: base(
17-
package,
18-
currentUser,
19-
package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)))
16+
: base(package, currentUser)
2017
{
2118
IsCurrentUserAnAdmin = currentUser != null && currentUser.IsAdministrator;
2219

‎tests/NuGetGallery.Facts/ViewModels/DisplayPackageViewModelFacts.cs‎

Lines changed: 42 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,14 @@ public void ItDeterminesRepositoryKind(string repoUrl, string repoType, Reposito
5050
Version = "1.0.0",
5151
RepositoryUrl = repoUrl,
5252
RepositoryType = repoType,
53+
PackageRegistration = new PackageRegistration
54+
{
55+
Owners = Enumerable.Empty<User>().ToList(),
56+
Packages = Enumerable.Empty<Package>().ToList()
57+
}
5358
};
5459

55-
var model = new DisplayPackageViewModel(package, null, "test");
60+
var model = new DisplayPackageViewModel(package, null);
5661
Assert.Equal(expectedKind, model.RepositoryType);
5762
Assert.Equal(expectedUrl, model.RepositoryUrl);
5863
}
@@ -86,10 +91,15 @@ public void ItInitializesProjectUrl(string projectUrl, string expected)
8691
var package = new Package
8792
{
8893
Version = "1.0.0",
89-
ProjectUrl = projectUrl
94+
ProjectUrl = projectUrl,
95+
PackageRegistration = new PackageRegistration
96+
{
97+
Owners = Enumerable.Empty<User>().ToList(),
98+
Packages = Enumerable.Empty<Package>().ToList()
99+
}
90100
};
91101

92-
var model = new DisplayPackageViewModel(package, null, "test");
102+
var model = new DisplayPackageViewModel(package, null);
93103
Assert.Equal(expected, model.ProjectUrl);
94104
}
95105

@@ -109,10 +119,15 @@ public void ItInitializesLicenseUrl(string licenseUrl, string expected)
109119
var package = new Package
110120
{
111121
Version = "1.0.0",
112-
LicenseUrl = licenseUrl
122+
LicenseUrl = licenseUrl,
123+
PackageRegistration = new PackageRegistration
124+
{
125+
Owners = Enumerable.Empty<User>().ToList(),
126+
Packages = Enumerable.Empty<Package>().ToList()
127+
}
113128
};
114129

115-
var model = new DisplayPackageViewModel(package, null, "test");
130+
var model = new DisplayPackageViewModel(package, null);
116131
Assert.Equal(expected, model.LicenseUrl);
117132
}
118133

@@ -124,9 +139,14 @@ public void LicenseNamesAreParsedByCommas()
124139
LicenseUrl = "https://mylicense.com",
125140
Version = "1.0.0",
126141
LicenseNames = "l1,l2, l3 ,l4 , l5 ",
142+
PackageRegistration = new PackageRegistration
143+
{
144+
Owners = Enumerable.Empty<User>().ToList(),
145+
Packages = Enumerable.Empty<Package>().ToList()
146+
}
127147
};
128148

129-
var packageViewModel = new DisplayPackageViewModel(package, currentUser: null, pushedBy: "test");
149+
var packageViewModel = new DisplayPackageViewModel(package, currentUser: null);
130150
Assert.Equal(new string[] { "l1", "l2", "l3", "l4", "l5" }, packageViewModel.LicenseNames);
131151
}
132152

@@ -155,7 +175,7 @@ public void TheCtorSortsPackageVersionsProperly()
155175
new Package { Version = "1.0.10", PackageRegistration = package.PackageRegistration }
156176
};
157177

158-
var packageVersions = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)))
178+
var packageVersions = new DisplayPackageViewModel(package, null)
159179
.PackageVersions.ToList();
160180

161181
// Descending
@@ -207,7 +227,7 @@ public void TheCtorDoesNotPopulateLatestSymbolsPackageForHistory()
207227
});
208228
}
209229

210-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
230+
var viewModel = new DisplayPackageViewModel(package, null);
211231

212232
// Descending
213233
Assert.NotNull(viewModel.LatestSymbolsPackage);
@@ -245,7 +265,7 @@ public void TheCtorReturnsLatestSymbolPackageByDateCreated()
245265

246266
package.SymbolPackages = symbolPackageList;
247267

248-
var viewModel = new DisplayPackageViewModel(package, null, packageHistory: Enumerable.Empty<Package>().OrderBy(x => 1));
268+
var viewModel = new DisplayPackageViewModel(package, null);
249269

250270
Assert.Equal(symbolPackageList[0], viewModel.LatestSymbolsPackage);
251271
}
@@ -282,10 +302,8 @@ public void AvgDownloadsPerDayConsidersOldestPackageVersionInHistory()
282302
new Package { Version = "2.0.1", PackageRegistration = packageRegistration, DownloadCount = 140, Created = utcNow.AddDays(-3) }
283303
};
284304

285-
var packageHistory = packageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version));
286-
287305
// Act
288-
var viewModel = new DisplayPackageViewModel(package, null, packageHistory);
306+
var viewModel = new DisplayPackageViewModel(package, null);
289307

290308
// Assert
291309
Assert.Equal(daysSinceFirstPackageCreated, viewModel.TotalDaysSinceCreated);
@@ -314,7 +332,7 @@ public void DownloadsPerDayLabelShowsLessThanOneWhenAverageBelowOne()
314332

315333
package.PackageRegistration.Packages = new[] { package };
316334

317-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
335+
var viewModel = new DisplayPackageViewModel(package, null);
318336

319337
// Act
320338
var label = viewModel.DownloadsPerDayLabel;
@@ -346,7 +364,7 @@ public void DownloadsPerDayLabelShowsOneWhenAverageBetweenOneAndOnePointFive(int
346364

347365
package.PackageRegistration.Packages = new[] { package };
348366

349-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
367+
var viewModel = new DisplayPackageViewModel(package, null);
350368

351369
// Act
352370
var label = viewModel.DownloadsPerDayLabel;
@@ -394,7 +412,7 @@ public void HasNewerPrereleaseReturnsTrueWhenNewerPrereleaseAvailable(
394412

395413
package.PackageRegistration.Packages = new[] { package, otherPackage };
396414

397-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
415+
var viewModel = new DisplayPackageViewModel(package, null);
398416

399417
// Act
400418
var hasNewerPrerelease = viewModel.HasNewerPrerelease;
@@ -441,7 +459,7 @@ public void HasNewerReleaseReturnsTrueWhenNewerReleaseAvailable(
441459

442460
package.PackageRegistration.Packages = new[] { package, otherPackage };
443461

444-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
462+
var viewModel = new DisplayPackageViewModel(package, null);
445463

446464
// Act
447465
var hasNewerRelease = viewModel.HasNewerRelease;
@@ -480,7 +498,7 @@ public void HasNewerPrereleaseDoesNotConsiderUnlistedVersions()
480498

481499
package.PackageRegistration.Packages = new[] { package, otherPackage };
482500

483-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
501+
var viewModel = new DisplayPackageViewModel(package, null);
484502

485503
// Act
486504
var hasNewerPrerelease = viewModel.HasNewerPrerelease;
@@ -520,7 +538,7 @@ public void HasNewerReleaseDoesNotConsiderUnlistedVersions()
520538

521539
package.PackageRegistration.Packages = new[] { package, otherPackage };
522540

523-
var viewModel = new DisplayPackageViewModel(package, null, package.PackageRegistration.Packages.OrderByDescending(p => new NuGetVersion(p.Version)));
541+
var viewModel = new DisplayPackageViewModel(package, null);
524542

525543
// Act
526544
var hasNewerRelease = viewModel.HasNewerRelease;
@@ -536,10 +554,9 @@ public void HasSemVer2DependencyIsFalseWhenInvalidDependencyVersionRange(string
536554
{
537555
// Arrange
538556
var package = CreateTestPackage("1.0.0", dependencyVersion: versionSpec);
539-
var history = package.PackageRegistration.Packages.OrderByDescending(p => p.Version);
540557

541558
// Act
542-
var viewModel = new DisplayPackageViewModel(package, null, history);
559+
var viewModel = new DisplayPackageViewModel(package, null);
543560

544561
// Assert
545562
Assert.False(viewModel.HasSemVer2Dependency);
@@ -553,10 +570,9 @@ public void HasSemVer2DependencyWhenSemVer2DependencyVersionSpec(string versionS
553570
{
554571
// Arrange
555572
var package = CreateTestPackage("1.0.0", dependencyVersion: versionSpec);
556-
var history = package.PackageRegistration.Packages.OrderByDescending(p => p.Version);
557573

558574
// Act
559-
var viewModel = new DisplayPackageViewModel(package, null, history);
575+
var viewModel = new DisplayPackageViewModel(package, null);
560576

561577
// Assert
562578
Assert.True(viewModel.HasSemVer2Dependency);
@@ -570,10 +586,9 @@ public void HasSemVer2DependencyIsFalseWhenNonSemVer2DependencyVersionSpec(strin
570586
{
571587
// Arrange
572588
var package = CreateTestPackage("1.0.0", dependencyVersion: versionSpec);
573-
var history = package.PackageRegistration.Packages.OrderByDescending(p => p.Version);
574589

575590
// Act
576-
var viewModel = new DisplayPackageViewModel(package, null, history);
591+
var viewModel = new DisplayPackageViewModel(package, null);
577592

578593
// Assert
579594
Assert.False(viewModel.HasSemVer2Dependency);
@@ -587,10 +602,9 @@ public void HasSemVer2VersionIsFalseWhenNonSemVer2Version(string version)
587602
{
588603
// Arrange
589604
var package = CreateTestPackage(version);
590-
var history = package.PackageRegistration.Packages.OrderByDescending(p => p.Version);
591605

592606
// Act
593-
var viewModel = new DisplayPackageViewModel(package, null, history);
607+
var viewModel = new DisplayPackageViewModel(package, null);
594608

595609
// Assert
596610
Assert.False(viewModel.HasSemVer2Version);
@@ -604,10 +618,9 @@ public void HasSemVer2VersionIsTrueWhenSemVer2Version(string version)
604618
{
605619
// Arrange
606620
var package = CreateTestPackage(version);
607-
var history = package.PackageRegistration.Packages.OrderByDescending(p => p.Version);
608621

609622
// Act
610-
var viewModel = new DisplayPackageViewModel(package, null, history);
623+
var viewModel = new DisplayPackageViewModel(package, null);
611624

612625
// Assert
613626
Assert.True(viewModel.HasSemVer2Version);
@@ -717,7 +730,7 @@ public static IEnumerable<object[]> Data
717730
[MemberData(nameof(Data))]
718731
public void ReturnsExpectedUser(Package package, User currentUser, string expected)
719732
{
720-
var model = new DisplayPackageViewModel(package, currentUser, new[] { package }.OrderByDescending(p => new NuGetVersion(p.Version)));
733+
var model = new DisplayPackageViewModel(package, currentUser);
721734

722735
Assert.Equal(expected, model.PushedBy);
723736
}

0 commit comments

Comments
 (0)