Skip to content

Commit 08de4c1

Browse files
authored
Better handling of version list when short version list is displayed. (#10697)
* Partial view for a version list. * Making sure Latest semver2 versions make it to the short version list. * Method and routes to load full version list. * Ordering by Key instead of Created. The order should largely be the same , but less DB resource spent.
1 parent c2dc7c4 commit 08de4c1

18 files changed

Lines changed: 385 additions & 158 deletions

File tree

src/GitHubVulnerabilities2Db/Gallery/ThrowingTelemetryService.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -408,5 +408,10 @@ public void TrackDownloadJsonTotalPackageIds(int totalPackageIds) {
408408
public void TrackDownloadJsonTotalPackageVersions(int totalPackageVersions) {
409409
throw new NotImplementedException();
410410
}
411+
412+
public void TrackFullVersionListLoadRequest()
413+
{
414+
throw new NotImplementedException();
415+
}
411416
}
412-
}
417+
}

src/NuGetGallery.Services/PackageManagement/IPackageService.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ IReadOnlyCollection<Package> FindPackagesById(
6666
/// <param name="includeSupportedFrameworks"></param>
6767
/// <param name="maxCount"></param>
6868
/// <returns></returns>
69-
IReadOnlyCollection<Package> FindLatestVersionsById(
69+
LatestPackageVersionsResult FindLatestVersionsById(
7070
string id,
7171
string includeVersion,
7272
bool includePackageRegistration,
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System.Collections.Generic;
5+
using NuGet.Services.Entities;
6+
7+
namespace NuGetGallery
8+
{
9+
public class LatestPackageVersionsResult
10+
{
11+
public IReadOnlyCollection<Package> Packages { get; set; }
12+
public bool HasMoreResults { get; set; }
13+
}
14+
}

src/NuGetGallery.Services/PackageManagement/PackageFilter.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public Package GetFiltered(IReadOnlyCollection<Package> packages, PackageFilterC
4646

4747
// The package still wasn't found so let's just get the latest version. This applies when
4848
// no version is specified, or when the version asked for (in context.Version) wasn't found
49-
result = result ?? _packageService.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, allowPrerelease: true);
49+
result = result ?? _packageService.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, allowPrerelease: true);
5050

5151
return result;
5252
}

src/NuGetGallery.Services/PackageManagement/PackageService.cs

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Collections.ObjectModel;
67
using System.Data.Entity;
78
using System.IO;
89
using System.Linq;
@@ -274,7 +275,7 @@ public IReadOnlyCollection<Package> FindPackagesById(
274275
return packages.ToList();
275276
}
276277

277-
public IReadOnlyCollection<Package> FindLatestVersionsById(
278+
public LatestPackageVersionsResult FindLatestVersionsById(
278279
string id,
279280
string includeVersion,
280281
bool includePackageRegistration,
@@ -301,10 +302,24 @@ public IReadOnlyCollection<Package> FindLatestVersionsById(
301302
includeDeprecations: includeDeprecations,
302303
includeDeprecationRelationships: false,
303304
includeSupportedFrameworks: includeSupportedFrameworks)
304-
.OrderByDescending(p => p.Created)
305-
.Take(maxCount)
305+
.OrderByDescending(p => p.IsLatestSemVer2 || p.IsLatestStableSemVer2)
306+
.ThenByDescending(p => p.Key)
307+
.Take(maxCount + 1)
306308
.ToList();
307309

310+
bool moreAvailable = packages.Count > maxCount;
311+
312+
if (moreAvailable)
313+
{
314+
// if we have list longer than requested, trim it, making sure we don't trim includeVersion if it happens to be last
315+
var removeAt = packages.Count - 1;
316+
if (!string.IsNullOrWhiteSpace(includeVersion) && packages[removeAt].NormalizedVersion == includeVersion)
317+
{
318+
--removeAt;
319+
}
320+
packages.RemoveAt(removeAt);
321+
}
322+
308323
if (!string.IsNullOrWhiteSpace(includeVersion) && !packages.Any(p => p.NormalizedVersion == includeVersion))
309324
{
310325
var requiredPackage = GetPackagesByIdQueryable(
@@ -329,7 +344,7 @@ public IReadOnlyCollection<Package> FindLatestVersionsById(
329344
}
330345
}
331346

332-
return packages;
347+
return new LatestPackageVersionsResult { Packages = packages, HasMoreResults = moreAvailable };
333348
}
334349

335350
public virtual Package FindPackageByIdAndVersion(

src/NuGetGallery.Services/Telemetry/ITelemetryService.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -432,5 +432,7 @@ void TrackABTestEvaluated(
432432

433433
IDisposable TrackSyncSqlConnectionCreationDuration();
434434
IDisposable TrackAsyncSqlConnectionCreationDuration();
435+
436+
void TrackFullVersionListLoadRequest();
435437
}
436-
}
438+
}

src/NuGetGallery.Services/Telemetry/TelemetryService.cs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) .NET Foundation. All rights reserved.
1+
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

44
using System;
@@ -97,6 +97,7 @@ public static class Events
9797
public const string InstanceUptime = "InstanceUptimeInDays";
9898
public const string ApiRequest = "ApiRequest";
9999
public const string CreateSqlConnectionDurationMs = "CreateSqlConnectionDurationMs";
100+
public const string FullVersionListRequest = "FullVersionListRequest";
100101
}
101102

102103
private readonly IDiagnosticsSource _diagnosticsSource;
@@ -1164,6 +1165,11 @@ public IDisposable TrackSyncSqlConnectionCreationDuration()
11641165
public IDisposable TrackAsyncSqlConnectionCreationDuration()
11651166
=> TrackSqlConnectionCreationDuration(Async);
11661167

1168+
public void TrackFullVersionListLoadRequest()
1169+
{
1170+
TrackMetric(Events.FullVersionListRequest, 1, _ => { });
1171+
}
1172+
11671173
private IDisposable TrackSqlConnectionCreationDuration(string kind)
11681174
{
11691175
return new DurationTracker(duration =>
@@ -1189,4 +1195,4 @@ private string BuildArrayProperty(IEnumerable<string> list)
11891195
return JsonConvert.SerializeObject(list);
11901196
}
11911197
}
1192-
}
1198+
}

src/NuGetGallery/App_Start/Routes.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,24 @@ public static void RegisterUIRoutes(RouteCollection routes, bool adminPanelEnabl
253253
action = nameof(PackagesController.AtomFeed)
254254
});
255255

256+
routes.MapRoute(
257+
RouteName.PackageVersionTableById,
258+
"packages/{id}/versiontable",
259+
new
260+
{
261+
controller = "Packages",
262+
action = nameof(PackagesController.GetAllPackageVersionsById)
263+
});
264+
265+
routes.MapRoute(
266+
RouteName.PackageVersionTableByIdVersion,
267+
"packages/{id}/{version}/versiontable",
268+
new
269+
{
270+
controller = "Packages",
271+
action = nameof(PackagesController.GetAllPackageVersions)
272+
});
273+
256274
routes.MapRoute(
257275
RouteName.PackageEnableLicenseReport,
258276
"packages/{id}/{version}/EnableLicenseReport",

src/NuGetGallery/Controllers/PackagesController.cs

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -928,14 +928,19 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
928928

929929
IReadOnlyCollection<Package> allVersions;
930930

931+
bool hasMoreVersions = false;
932+
931933
if (_featureFlagService.IsReducedVersionListsEnabled())
932934
{
933-
allVersions = _packageService.FindLatestVersionsById(id,
935+
LatestPackageVersionsResult latestVersions = _packageService.FindLatestVersionsById(id,
934936
includeVersion: normalized,
935937
includePackageRegistration: true,
936938
includeDeprecations: true,
937939
includeSupportedFrameworks: true,
938940
maxCount: 20);
941+
942+
allVersions = latestVersions.Packages;
943+
hasMoreVersions = latestVersions.HasMoreResults;
939944
}
940945
else
941946
{
@@ -1008,6 +1013,7 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
10081013
model.IsMarkdigMdSyntaxHighlightEnabled = _featureFlagService.IsMarkdigMdSyntaxHighlightEnabled();
10091014
model.IsMcpServerPackageDisplayEnabled = _featureFlagService.IsMcpServerPackageDisplayEnabled();
10101015
model.CanDisplayReadmeWarning = canDisplayReadmeWarning;
1016+
model.HasMoreVersions = hasMoreVersions;
10111017

10121018
if (model.IsComputeTargetFrameworkEnabled || model.IsDisplayTargetFrameworkEnabled)
10131019
{
@@ -2422,6 +2428,73 @@ public virtual Task<ActionResult> RejectPendingOwnershipRequest(string id, strin
24222428
return HandleOwnershipRequest(id, username, token, redirect: false, accept: false);
24232429
}
24242430

2431+
[HttpPost]
2432+
[ValidateAntiForgeryToken]
2433+
public ActionResult GetAllPackageVersionsById(string id)
2434+
{
2435+
return GetAllPackageVersions(id, version: null);
2436+
}
2437+
2438+
[HttpPost]
2439+
[ValidateAntiForgeryToken]
2440+
public ActionResult GetAllPackageVersions(string id, string version)
2441+
{
2442+
if (!_featureFlagService.IsReducedVersionListsEnabled())
2443+
{
2444+
return HttpNotFound();
2445+
}
2446+
2447+
if (string.IsNullOrWhiteSpace(id))
2448+
{
2449+
return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
2450+
}
2451+
2452+
_telemetryService.TrackFullVersionListLoadRequest();
2453+
2454+
string normalized = NuGetVersionFormatter.Normalize(version);
2455+
2456+
IReadOnlyCollection<Package> allVersions = _packageService.FindPackagesById(id,
2457+
includePackageRegistration: true,
2458+
includeDeprecations: true,
2459+
includeSupportedFrameworks: true);
2460+
2461+
var filterContext = new PackageFilterContext(RouteData?.Route, version);
2462+
var package = _packageFilter.GetFiltered(allVersions, filterContext);
2463+
2464+
// Validating packages should be hidden to everyone but the owners and admins.
2465+
var currentUser = GetCurrentUser();
2466+
if (package == null
2467+
|| ((package.PackageStatusKey == PackageStatus.Validating
2468+
|| package.PackageStatusKey == PackageStatus.FailedValidation)
2469+
&& ActionsRequiringPermissions.DisplayPrivatePackageMetadata.CheckPermissionsOnBehalfOfAnyAccount(currentUser, package) != PermissionsCheckResult.Allowed))
2470+
{
2471+
return HttpNotFound();
2472+
}
2473+
2474+
var isPackageDeprecationEnabled = _featureFlagService.IsManageDeprecationEnabled(currentUser, allVersions);
2475+
var packageKeyToDeprecation = isPackageDeprecationEnabled
2476+
? _deprecationService.GetDeprecationsById(id)
2477+
.GroupBy(d => d.PackageKey)
2478+
.ToDictionary(g => g.Key, g => g.First())
2479+
: null;
2480+
2481+
var isPackageVulnerabilitiesEnabled = _featureFlagService.IsDisplayVulnerabilitiesEnabled();
2482+
var packageKeyToVulnerabilities = isPackageVulnerabilitiesEnabled
2483+
? _vulnerabilitiesService.GetVulnerabilitiesById(id)
2484+
: null;
2485+
2486+
var model = _displayPackageViewModelFactory.Create(
2487+
package,
2488+
allVersions,
2489+
currentUser,
2490+
packageKeyToDeprecation,
2491+
packageKeyToVulnerabilities,
2492+
packageRenames: null, // not used in this view
2493+
readmeResult: null); // not used in this view
2494+
2495+
return PartialView("_DisplayPackageVersionTable", model);
2496+
}
2497+
24252498
private async Task<ActionResult> HandleOwnershipRequest(
24262499
string id,
24272500
string username,

src/NuGetGallery/NuGetGallery.csproj

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2327,6 +2327,9 @@
23272327
<ItemGroup>
23282328
<Content Include="Views\Users\TrustedPublishing.cshtml" />
23292329
</ItemGroup>
2330+
<ItemGroup>
2331+
<Content Include="Views\Packages\_DisplayPackageVersionTable.cshtml" />
2332+
</ItemGroup>
23302333
<PropertyGroup>
23312334
<VisualStudioVersion Condition="'$(VisualStudioVersion)' == ''">10.0</VisualStudioVersion>
23322335
<VSToolsPath Condition="'$(VSToolsPath)' == ''">$(MSBuildExtensionsPath32)\Microsoft\VisualStudio\v$(VisualStudioVersion)</VSToolsPath>

0 commit comments

Comments
 (0)