Skip to content

Commit f14275f

Browse files
author
Scott Bommarito
authored
Server-side performance fixes for Manage Packages page (#6752)
1 parent a308141 commit f14275f

2 files changed

Lines changed: 38 additions & 32 deletions

File tree

src/NuGetGallery/Services/ActionRequiringEntityPermissions.cs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,35 +55,43 @@ public PermissionsCheckResult CheckPermissions(IPrincipal currentPrincipal, User
5555

5656
public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, TEntity entity)
5757
{
58-
return CheckPermissionsOnBehalfOfAnyAccount(currentUser, entity, out var accountsAllowedOnBehalfOf);
58+
return CheckPermissionsOnBehalfOfAnyAccount(currentUser, entity, exitEarly: true, accountsAllowedOnBehalfOf: out var _);
5959
}
6060

6161
public PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, TEntity entity, out IEnumerable<User> accountsAllowedOnBehalfOf)
62+
{
63+
return CheckPermissionsOnBehalfOfAnyAccount(currentUser, entity, exitEarly: false, accountsAllowedOnBehalfOf: out accountsAllowedOnBehalfOf);
64+
}
65+
66+
private PermissionsCheckResult CheckPermissionsOnBehalfOfAnyAccount(User currentUser, TEntity entity, bool exitEarly, out IEnumerable<User> accountsAllowedOnBehalfOf)
6267
{
6368
accountsAllowedOnBehalfOf = new List<User>();
6469

65-
var possibleAccountsOnBehalfOf =
66-
new[] { currentUser }
67-
.Concat(GetOwners(entity));
70+
var possibleAccountsOnBehalfOf = new List<User>
71+
{
72+
currentUser
73+
};
74+
75+
possibleAccountsOnBehalfOf.AddRange(GetOwners(entity));
6876

6977
if (currentUser != null)
7078
{
71-
possibleAccountsOnBehalfOf =
72-
possibleAccountsOnBehalfOf
73-
.Concat(currentUser.Organizations.Select(o => o.Organization));
79+
possibleAccountsOnBehalfOf.AddRange(currentUser.Organizations.Select(o => o.Organization));
7480
}
7581

76-
possibleAccountsOnBehalfOf = possibleAccountsOnBehalfOf.Distinct(new UserEqualityComparer());
77-
7882
var aggregateResult = PermissionsCheckResult.Unknown;
7983

80-
foreach (var accountOnBehalfOf in possibleAccountsOnBehalfOf)
84+
foreach (var accountOnBehalfOf in possibleAccountsOnBehalfOf.Distinct(new UserEqualityComparer()))
8185
{
8286
var result = CheckPermissions(currentUser, accountOnBehalfOf, entity);
8387
aggregateResult = ChoosePermissionsCheckResult(aggregateResult, result);
8488
if (result == PermissionsCheckResult.Allowed)
8589
{
8690
(accountsAllowedOnBehalfOf as List<User>).Add(accountOnBehalfOf);
91+
if (exitEarly)
92+
{
93+
break;
94+
}
8795
}
8896
}
8997

@@ -115,4 +123,4 @@ private PermissionsCheckResult ChoosePermissionsCheckResult(PermissionsCheckResu
115123
return new[] { current, next }.Max();
116124
}
117125
}
118-
}
126+
}

src/NuGetGallery/Services/PackageService.cs

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -255,30 +255,28 @@ private IEnumerable<Package> GetPackagesForOwners(IEnumerable<int> ownerKeys, bo
255255
packages = packages.Where(p => p.Listed);
256256
}
257257

258-
if (includeVersions)
259-
{
260-
return packages
261-
.Include(p => p.PackageRegistration)
262-
.Include(p => p.PackageRegistration.Owners)
263-
.ToList();
264-
}
265-
266-
// Do a best effort of retrieving the latest version. Note that UpdateIsLatest has had concurrency issues
267-
// where sometimes packages no rows with IsLatest set. In this case, we'll just select the last inserted
268-
// row (descending [Key]) as opposed to reading all rows into memory and sorting on NuGetVersion.
258+
if (!includeVersions)
259+
{
260+
// Do a best effort of retrieving the latest version. Note that UpdateIsLatest has had concurrency issues
261+
// where sometimes packages no rows with IsLatest set. In this case, we'll just select the last inserted
262+
// row (descending [Key]) as opposed to reading all rows into memory and sorting on NuGetVersion.
263+
packages = packages
264+
.GroupBy(p => p.PackageRegistrationKey)
265+
.Select(g => g
266+
// order booleans desc so that true (1) comes first
267+
.OrderByDescending(p => p.IsLatestStableSemVer2)
268+
.ThenByDescending(p => p.IsLatestStable)
269+
.ThenByDescending(p => p.IsLatestSemVer2)
270+
.ThenByDescending(p => p.IsLatest)
271+
.ThenByDescending(p => p.Listed)
272+
.ThenByDescending(p => p.Key)
273+
.FirstOrDefault());
274+
}
275+
269276
return packages
270-
.GroupBy(p => p.PackageRegistrationKey)
271-
.Select(g => g
272-
// order booleans desc so that true (1) comes first
273-
.OrderByDescending(p => p.IsLatestStableSemVer2)
274-
.ThenByDescending(p => p.IsLatestStable)
275-
.ThenByDescending(p => p.IsLatestSemVer2)
276-
.ThenByDescending(p => p.IsLatest)
277-
.ThenByDescending(p => p.Listed)
278-
.ThenByDescending(p => p.Key)
279-
.FirstOrDefault())
280277
.Include(p => p.PackageRegistration)
281278
.Include(p => p.PackageRegistration.Owners)
279+
.Include(p => p.PackageRegistration.RequiredSigners)
282280
.ToList();
283281
}
284282

0 commit comments

Comments
 (0)