Skip to content

Commit 24d69cd

Browse files
ErikEJErikEJ
authored
Reduce database calls for Microsoft profile page from 21.056 to 2 (#10411)
1 parent 1de96f5 commit 24d69cd

8 files changed

Lines changed: 199 additions & 116 deletions

File tree

‎src/NuGetGallery.Services/PackageManagement/IPackageService.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ Package FilterLatestPackageBySuffix(IReadOnlyCollection<Package> packages,
103103

104104
IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted, bool includeVersions = false);
105105

106+
(IReadOnlyCollection<Package> Packages, long TotalDownloadCount, int PackageCount) FindPackagesByProfile(User user, int page, int pageSize);
107+
106108
IEnumerable<Package> FindPackagesByAnyMatchingOwner(User user, bool includeUnlisted, bool includeVersions = false);
107109

108110
IQueryable<PackageRegistration> FindPackageRegistrationsByOwner(User user);

‎src/NuGetGallery.Services/PackageManagement/PackageService.cs‎

Lines changed: 67 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,53 @@ private static Package FilterLatestPackageHelper(
428428
return package;
429429
}
430430

431+
public (IReadOnlyCollection<Package> Packages, long TotalDownloadCount, int PackageCount) FindPackagesByProfile(
432+
User user,
433+
int page,
434+
int pageSize)
435+
{
436+
if (user == null)
437+
{
438+
throw new ArgumentNullException(nameof(user));
439+
}
440+
441+
IQueryable<Package> packages = _packageRepository.GetAll()
442+
.Where(p => p.PackageRegistration.Owners.Any(o => o.Key == user.Key));
443+
444+
var packageSummary = packages
445+
.Where(p => p.IsLatestSemVer2)
446+
.Include(p => p.PackageRegistration)
447+
.GroupBy(r => 1)
448+
.Select(g => new
449+
{
450+
PackageCount = g.Sum(x => 1),
451+
DownloadCount = g.Sum(x => x.PackageRegistration.DownloadCount)
452+
})
453+
.FirstOrDefault();
454+
455+
var packageCount = packageSummary != null ? packageSummary.PackageCount : 0;
456+
var downloadCount = packageSummary != null ? packageSummary.DownloadCount : 0;
457+
458+
packages = packages
459+
.Where(p => p.Listed
460+
&& p.PackageStatusKey == PackageStatus.Available);
461+
462+
packages = GetLatestVersion(packages);
463+
464+
return (packages
465+
.OrderByDescending(p => p.PackageRegistration.DownloadCount)
466+
.ThenBy(p => p.PackageRegistration.Id)
467+
.Skip((page-1) * pageSize)
468+
.Take(pageSize)
469+
.Include(p => p.PackageRegistration.Owners)
470+
.Include(p => p.PackageRegistration.RequiredSigners)
471+
.Include(p => p.PackageRegistration.Packages.Select(p => p.SupportedFrameworks))
472+
.Include(p => p.PackageRegistration.Packages.Select(p => p.Deprecations))
473+
.ToList(),
474+
downloadCount,
475+
packageCount);
476+
}
477+
431478
public IEnumerable<Package> FindPackagesByOwner(User user, bool includeUnlisted, bool includeVersions = false)
432479
{
433480
return GetPackagesForOwners(new[] { user.Key }, includeUnlisted, includeVersions);
@@ -459,20 +506,7 @@ private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bo
459506

460507
if (!includeVersions)
461508
{
462-
// Do a best effort of retrieving the latest versions. Note that UpdateIsLatest has had concurrency issues
463-
// where sometimes packages no rows with IsLatest set. In this case, we'll just select the last inserted
464-
// row (descending [Key]) as opposed to reading all rows into memory and sorting on NuGetVersion.
465-
packages = packages
466-
.GroupBy(p => p.PackageRegistrationKey)
467-
.Select(g => g
468-
// order booleans desc so that true (1) comes first
469-
.OrderByDescending(p => p.IsLatestStableSemVer2)
470-
.ThenByDescending(p => p.IsLatestStable)
471-
.ThenByDescending(p => p.IsLatestSemVer2)
472-
.ThenByDescending(p => p.IsLatest)
473-
.ThenByDescending(p => p.Listed)
474-
.ThenByDescending(p => p.Key)
475-
.FirstOrDefault());
509+
packages = GetLatestVersion(packages);
476510
}
477511

478512
return packages
@@ -482,6 +516,25 @@ private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bo
482516
.ToList();
483517
}
484518

519+
private static IQueryable<Package> GetLatestVersion(IQueryable<Package> packages)
520+
{
521+
// Do a best effort of retrieving the latest versions. Note that UpdateIsLatest has had concurrency issues
522+
// where sometimes packages no rows with IsLatest set. In this case, we'll just select the last inserted
523+
// row (descending [Key]) as opposed to reading all rows into memory and sorting on NuGetVersion.
524+
packages = packages
525+
.GroupBy(p => p.PackageRegistrationKey)
526+
.Select(g => g
527+
// order booleans desc so that true (1) comes first
528+
.OrderByDescending(p => p.IsLatestStableSemVer2)
529+
.ThenByDescending(p => p.IsLatestStable)
530+
.ThenByDescending(p => p.IsLatestSemVer2)
531+
.ThenByDescending(p => p.IsLatest)
532+
.ThenByDescending(p => p.Listed)
533+
.ThenByDescending(p => p.Key)
534+
.FirstOrDefault());
535+
return packages;
536+
}
537+
485538
public IQueryable<PackageRegistration> FindPackageRegistrationsByOwner(User user)
486539
{
487540
return _packageRegistrationRepository.GetAll().Where(p => p.Owners.Any(o => o.Key == user.Key));

‎src/NuGetGallery/Controllers/UsersController.cs‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -751,17 +751,25 @@ public virtual ActionResult Profiles(string username, int page = 1)
751751
return HttpNotFound();
752752
}
753753

754-
var packages = PackageService.FindPackagesByOwner(user, includeUnlisted: false)
755-
.Where(p => p.PackageStatusKey == PackageStatus.Available)
756-
.OrderByDescending(p => p.PackageRegistration.DownloadCount)
754+
var packageResult = PackageService.FindPackagesByProfile(user, page, GalleryConstants.DefaultPackageListPageSize);
755+
756+
var packages = packageResult.Packages
757757
.Select(p =>
758758
{
759759
var viewModel = _listPackageItemViewModelFactory.Create(p, currentUser, false);
760760
viewModel.DownloadCount = p.PackageRegistration.DownloadCount;
761761
return viewModel;
762762
}).ToList();
763763

764-
var model = new UserProfileModel(user, currentUser, packages, page - 1, GalleryConstants.DefaultPackageListPageSize, Url);
764+
var model = new UserProfileModel(
765+
user,
766+
currentUser,
767+
packages,
768+
page - 1,
769+
GalleryConstants.DefaultPackageListPageSize,
770+
Url,
771+
packageResult.TotalDownloadCount,
772+
packageResult.PackageCount);
765773

766774
return View(model);
767775
}

‎src/NuGetGallery/ViewModels/UserProfileModel.cs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,28 +10,27 @@ namespace NuGetGallery
1010
{
1111
public class UserProfileModel
1212
{
13-
public UserProfileModel(User user, User currentUser, List<ListPackageItemViewModel> allPackages, int pageIndex, int pageSize, UrlHelper url)
13+
public UserProfileModel(User user, User currentUser, List<ListPackageItemViewModel> allPackages, int pageIndex, int pageSize, UrlHelper url, long totalDownloadCount, int packageCount)
1414
{
1515
User = user;
1616
Username = user.Username;
1717
EmailAddress = user.EmailAddress;
1818
UnconfirmedEmailAddress = user.UnconfirmedEmailAddress;
1919
IsLocked = user.IsLocked;
2020
AllPackages = allPackages;
21-
TotalPackages = allPackages.Count;
21+
TotalPackages = packageCount;
2222
PackagePage = pageIndex;
2323
PackagePageSize = pageSize;
2424

25-
TotalPackageDownloadCount = AllPackages.Sum(p => p.TotalDownloadCount);
25+
TotalPackageDownloadCount = totalDownloadCount;
2626

2727
PackagePageTotalCount = (TotalPackages + PackagePageSize - 1) / PackagePageSize;
2828

2929
var pager = new PreviousNextPagerViewModel<ListPackageItemViewModel>(allPackages, pageIndex, PackagePageTotalCount,
3030
page => url.User(user, page));
3131

3232
Pager = pager;
33-
PagedPackages = AllPackages.Skip(PackagePageSize * pageIndex)
34-
.Take(PackagePageSize).ToList();
33+
PagedPackages = AllPackages;
3534

3635
CanManageAccount = ActionsRequiringPermissions.ManageAccount.CheckPermissions(currentUser, user) == PermissionsCheckResult.Allowed;
3736
CanViewAccount = ActionsRequiringPermissions.ViewAccount.CheckPermissions(currentUser, user) == PermissionsCheckResult.Allowed;

‎src/NuGetGallery/Views/Users/Profiles.cshtml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@
2525
responsive: true)
2626
<div class="statistics">
2727
<div class="statistic">
28-
<div class="value">@Model.AllPackages.Count.ToNuGetNumberString()</div>
29-
<div class="description">@(Model.AllPackages.Count == 1 ? "Package" : "Packages")</div>
28+
<div class="value">@Model.TotalPackages.ToNuGetNumberString()</div>
29+
<div class="description">@(Model.TotalPackages == 1 ? "Package" : "Packages")</div>
3030
</div>
3131
<div class="statistic">
3232
<div class="value">@Model.TotalPackageDownloadCount.ToNuGetNumberString()</div>

‎tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs‎

Lines changed: 6 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,89 +1162,6 @@ public static IEnumerable<object[]> PossibleOwnershipScenarios_Data
11621162
}
11631163
}
11641164

1165-
[Theory]
1166-
[MemberData(nameof(PossibleOwnershipScenarios_Data))]
1167-
public void ReturnsSinglePackageAsExpected(User currentUser, User owner)
1168-
{
1169-
// Arrange
1170-
var username = "test";
1171-
1172-
var package = new Package
1173-
{
1174-
Version = "1.1.1",
1175-
1176-
PackageRegistration = new PackageRegistration
1177-
{
1178-
Id = "package",
1179-
Owners = new[] { owner },
1180-
DownloadCount = 150
1181-
},
1182-
1183-
DownloadCount = 100,
1184-
PackageStatusKey = PackageStatus.Available
1185-
};
1186-
var invalidatedPackage = new Package
1187-
{
1188-
Version = "1.0.0",
1189-
1190-
PackageRegistration = new PackageRegistration
1191-
{
1192-
Id = "packageFailedValidation",
1193-
Owners = new[] { owner },
1194-
DownloadCount = 0
1195-
},
1196-
1197-
DownloadCount = 0,
1198-
PackageStatusKey = PackageStatus.FailedValidation
1199-
};
1200-
var validatingPackage = new Package
1201-
{
1202-
Version = "1.0.0",
1203-
1204-
PackageRegistration = new PackageRegistration
1205-
{
1206-
Id = "packageValidating",
1207-
Owners = new[] { owner },
1208-
DownloadCount = 0
1209-
},
1210-
1211-
DownloadCount = 0,
1212-
PackageStatusKey = PackageStatus.Validating
1213-
};
1214-
var deletedPackage = new Package
1215-
{
1216-
Version = "1.0.0",
1217-
1218-
PackageRegistration = new PackageRegistration
1219-
{
1220-
Id = "packageDeleted",
1221-
Owners = new[] { owner },
1222-
DownloadCount = 0
1223-
},
1224-
1225-
DownloadCount = 0,
1226-
PackageStatusKey = PackageStatus.Deleted
1227-
};
1228-
1229-
GetMock<IUserService>()
1230-
.Setup(x => x.FindByUsername(username, false))
1231-
.Returns(owner);
1232-
1233-
GetMock<IPackageService>()
1234-
.Setup(x => x.FindPackagesByOwner(owner, false, false))
1235-
.Returns(new[] { package, invalidatedPackage, validatingPackage, deletedPackage });
1236-
1237-
var controller = GetController<UsersController>();
1238-
controller.SetCurrentUser(currentUser);
1239-
1240-
// Act
1241-
var result = controller.Profiles(username);
1242-
1243-
// Assert
1244-
var model = ResultAssert.IsView<UserProfileModel>(result);
1245-
AssertUserProfileModel(model, currentUser, owner, package);
1246-
}
1247-
12481165
[Theory]
12491166
[MemberData(nameof(PossibleOwnershipScenarios_Data))]
12501167
public void SortsPackagesByDownloadCount(User currentUser, User owner)
@@ -1284,9 +1201,11 @@ public void SortsPackagesByDownloadCount(User currentUser, User owner)
12841201
.Setup(x => x.FindByUsername(username, false))
12851202
.Returns(owner);
12861203

1204+
var packages = new List<Package> { package2, package1 };
1205+
12871206
GetMock<IPackageService>()
1288-
.Setup(x => x.FindPackagesByOwner(owner, false, false))
1289-
.Returns(new[] { package1, package2 });
1207+
.Setup(x => x.FindPackagesByProfile(owner, 1, GalleryConstants.DefaultPackageListPageSize))
1208+
.Returns((packages.AsReadOnly(), 350, 2));
12901209

12911210
var controller = GetController<UsersController>();
12921211
controller.SetCurrentUser(currentUser);
@@ -1327,8 +1246,8 @@ public void UsesProperIconUrl(User currentUser, User owner)
13271246
.Setup(x => x.FindByUsername(username, false))
13281247
.Returns(owner);
13291248
GetMock<IPackageService>()
1330-
.Setup(x => x.FindPackagesByOwner(owner, false, false))
1331-
.Returns(new[] { userPackage });
1249+
.Setup(x => x.FindPackagesByProfile(owner, 1, GalleryConstants.DefaultPackageListPageSize))
1250+
.Returns((userPackages.AsReadOnly(), 0, 0 ));
13321251

13331252
var controller = GetController<UsersController>();
13341253
controller.SetCurrentUser(currentUser);

0 commit comments

Comments
 (0)