Skip to content

Commit eea4ac7

Browse files
author
Scott Bommarito
authored
DisplayPackage, Manage, and DeleteSymbols pages should find exact version case-insensitively (#7161)
1 parent 9980106 commit eea4ac7

5 files changed

Lines changed: 166 additions & 12 deletions

File tree

src/NuGetGallery/Controllers/PackagesController.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -720,7 +720,7 @@ public virtual async Task<ActionResult> DisplayPackage(string id, string version
720720
}
721721
else
722722
{
723-
package = packages.SingleOrDefault(p => p.NormalizedVersion == NuGetVersionFormatter.Normalize(version));
723+
package = _packageService.FilterExactPackage(packages, version);
724724
}
725725
}
726726

@@ -1462,7 +1462,7 @@ public virtual async Task<ActionResult> Manage(string id, string version = null)
14621462
if (version != null)
14631463
{
14641464
// Try to find the exact version if it was specified.
1465-
package = packages.SingleOrDefault(p => p.NormalizedVersion == NuGetVersionFormatter.Normalize(version));
1465+
package = _packageService.FilterExactPackage(packages, version);
14661466
}
14671467

14681468
if (package == null)
@@ -1506,7 +1506,7 @@ public virtual ActionResult DeleteSymbols(string id, string version)
15061506
if (version != null)
15071507
{
15081508
// Try to find the exact version if it was specified.
1509-
package = packages.SingleOrDefault(p => p.NormalizedVersion == NuGetVersionFormatter.Normalize(version));
1509+
package = _packageService.FilterExactPackage(packages, version);
15101510
}
15111511

15121512
if (package == null)

src/NuGetGallery/Services/IPackageService.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,29 @@ Package FindPackageByIdAndVersion(
4040
int? semVerLevelKey = null,
4141
bool allowPrerelease = true);
4242

43+
/// <summary>
44+
/// Filters a list of packages for an exact version.
45+
/// </summary>
46+
/// <param name="packages">The list of packages to search.</param>
47+
/// <param name="version">The version to search for. This version is normalized and compared case-insensitively.</param>
48+
/// <returns>
49+
/// Returns the package in <paramref name="packages"/> with the exact version.
50+
/// Returns null if no such package exists.
51+
/// </returns>
52+
Package FilterExactPackage(
53+
IReadOnlyCollection<Package> packages,
54+
string version);
55+
56+
/// <summary>
57+
/// Filters a list of packages and returns the latest that follows some criteria.
58+
/// </summary>
59+
/// <param name="packages">The list of packages to search.</param>
60+
/// <param name="semVerLevelKey">The SemVer-level key that determines the SemVer filter to be applied.</param>
61+
/// <param name="allowPrerelease"><c>True</c> indicating pre-release packages are allowed, otherwise <c>false</c>.</param>
62+
/// <returns>
63+
/// Returns the latest package in <paramref name="packages"/> that follows the criteria.
64+
/// Returns null if no such package exists.
65+
/// </returns>
4366
Package FilterLatestPackage(
4467
IReadOnlyCollection<Package> packages,
4568
int? semVerLevelKey = SemVerLevelKey.SemVer2,

src/NuGetGallery/Services/PackageService.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,17 @@ public virtual Package FindPackageByIdAndVersion(
183183
return package;
184184
}
185185

186+
public virtual Package FilterExactPackage(
187+
IReadOnlyCollection<Package> packages,
188+
string version)
189+
{
190+
var normalizedVersion = NuGetVersionFormatter.Normalize(version);
191+
return packages.SingleOrDefault(p => string.Equals(
192+
p.NormalizedVersion,
193+
normalizedVersion,
194+
StringComparison.OrdinalIgnoreCase));
195+
}
196+
186197
public virtual Package FilterLatestPackage(
187198
IReadOnlyCollection<Package> packages,
188199
int? semVerLevelKey = SemVerLevelKey.SemVer2,

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

Lines changed: 64 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -376,11 +376,17 @@ public async Task GivenANonExistentPackageIt404s()
376376
GetConfigurationService(),
377377
packageService: packageService);
378378

379+
var version = "1.1.1";
380+
var packages = new Package[0];
379381
packageService.Setup(p => p.FindPackagesById("Foo", PackageDeprecationFieldsToInclude.Deprecation))
380-
.Returns(new Package[0]);
382+
.Returns(packages);
383+
384+
packageService
385+
.Setup(p => p.FilterExactPackage(packages, version))
386+
.Returns((Package)null);
381387

382388
// Act
383-
var result = await controller.DisplayPackage("Foo", "1.1.1");
389+
var result = await controller.DisplayPackage("Foo", version);
384390

385391
// Assert
386392
ResultAssert.IsNotFound(result);
@@ -523,6 +529,10 @@ private async Task<ActionResult> GetActionResultForPackageStatusAsync(
523529
.Setup(p => p.FindPackagesById(id, PackageDeprecationFieldsToInclude.Deprecation))
524530
.Returns(packages);
525531

532+
packageService
533+
.Setup(p => p.FilterExactPackage(packages, version))
534+
.Returns(package);
535+
526536
var getDeprecationByPackageSetup = deprecationService
527537
.Setup(x => x.GetDeprecationByPackage(package));
528538

@@ -646,6 +656,10 @@ private async Task CheckValidPackage(User currentUser, User owner)
646656
.Setup(p => p.FindPackagesById(id, PackageDeprecationFieldsToInclude.Deprecation))
647657
.Returns(packages);
648658

659+
packageService
660+
.Setup(p => p.FilterExactPackage(packages, normalizedVersion))
661+
.Returns(package);
662+
649663
deprecationService
650664
.Setup(x => x.GetDeprecationByPackage(package))
651665
.Verifiable();
@@ -994,6 +1008,7 @@ public async Task GetsValidationIssues()
9941008
var packages = new[] { package };
9951009
packageService.Setup(p => p.FindPackagesById("Foo", PackageDeprecationFieldsToInclude.Deprecation))
9961010
.Returns(packages);
1011+
9971012
packageService.Setup(p => p.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
9981013
.Returns(package);
9991014

@@ -2691,12 +2706,18 @@ protected override Mock<IPackageService> SetupPackageService(bool isPackageMissi
26912706
.Returns(packages)
26922707
.Verifiable();
26932708

2694-
if (isPackageMissing)
2695-
{
2696-
packageService
2697-
.Setup(p => p.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
2698-
.Returns((Package)null);
2699-
}
2709+
packageService
2710+
.Setup(p => p.FilterExactPackage(packages, It.Is<string>(s => s == Package.Version || s == Package.NormalizedVersion)))
2711+
.Returns(isPackageMissing ? null : Package)
2712+
.Verifiable();
2713+
2714+
packageService
2715+
.Setup(svc => svc.FilterExactPackage(packages, It.Is<string>(s => s != Package.Version && s != Package.NormalizedVersion)))
2716+
.Returns((Package)null);
2717+
2718+
packageService
2719+
.Setup(svc => svc.FilterLatestPackage(packages, It.IsAny<int>(), It.IsAny<bool>()))
2720+
.Returns((Package)null);
27002721

27012722
return packageService;
27022723
}
@@ -2713,6 +2734,14 @@ protected override Mock<IPackageService> SetupPackageService(bool isPackageMissi
27132734
.Returns(packages)
27142735
.Verifiable();
27152736

2737+
packageService
2738+
.Setup(p => p.FilterExactPackage(packages, It.Is<string>(s => s == Package.Version || s == Package.NormalizedVersion)))
2739+
.Returns(isPackageMissing ? null : Package);
2740+
2741+
packageService
2742+
.Setup(svc => svc.FilterExactPackage(packages, It.Is<string>(s => s != Package.Version && s != Package.NormalizedVersion)))
2743+
.Returns((Package)null);
2744+
27162745
packageService
27172746
.Setup(p => p.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
27182747
.Returns(isPackageMissing ? null : Package)
@@ -2790,6 +2819,11 @@ public void Returns404IfPackageNotFound()
27902819
.Setup(x => x.FindPackagesById(_packageRegistration.Id, PackageDeprecationFieldsToInclude.None))
27912820
.Returns(new Package[0]);
27922821

2822+
packageService
2823+
.Setup(p => p.FilterExactPackage(It.IsAny<IReadOnlyCollection<Package>>(), It.IsAny<string>()))
2824+
.Returns((Package)null)
2825+
.Verifiable();
2826+
27932827
var controller = CreateController(
27942828
GetConfigurationService(),
27952829
packageService: packageService);
@@ -2929,11 +2963,24 @@ public class TheDeleteSymbolsMethodWithExactVersion : TheDeleteSymbolsMethod
29292963
protected override Mock<IPackageService> CreatePackageService()
29302964
{
29312965
var packageService = new Mock<IPackageService>(MockBehavior.Strict);
2966+
var packages = _packageRegistration.Packages.ToList();
29322967
packageService
29332968
.Setup(svc => svc.FindPackagesById(_packageId, PackageDeprecationFieldsToInclude.None))
2934-
.Returns(_packageRegistration.Packages.ToList())
2969+
.Returns(packages)
29352970
.Verifiable();
29362971

2972+
packageService
2973+
.Setup(p => p.FilterExactPackage(packages, It.Is<string>(s => s == _package.Version || s == _package.NormalizedVersion)))
2974+
.Returns(_package);
2975+
2976+
packageService
2977+
.Setup(svc => svc.FilterExactPackage(packages, It.Is<string>(s => s != _package.Version && s != _package.NormalizedVersion)))
2978+
.Returns((Package)null);
2979+
2980+
packageService
2981+
.Setup(svc => svc.FilterLatestPackage(packages, It.IsAny<int>(), It.IsAny<bool>()))
2982+
.Returns((Package)null);
2983+
29372984
return packageService;
29382985
}
29392986

@@ -2954,6 +3001,14 @@ protected override Mock<IPackageService> CreatePackageService()
29543001
.Returns(packages)
29553002
.Verifiable();
29563003

3004+
packageService
3005+
.Setup(p => p.FilterExactPackage(packages, It.Is<string>(s => s == _package.Version || s == _package.NormalizedVersion)))
3006+
.Returns(_package);
3007+
3008+
packageService
3009+
.Setup(svc => svc.FilterExactPackage(packages, It.Is<string>(s => s != _package.Version && s != _package.NormalizedVersion)))
3010+
.Returns((Package)null);
3011+
29573012
packageService
29583013
.Setup(svc => svc.FilterLatestPackage(packages, SemVerLevelKey.SemVer2, true))
29593014
.Returns(_package)

tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -864,6 +864,71 @@ private async Task WillThrowIfTheRepositoryUrlIsLongerThan4000()
864864
}
865865
}
866866

867+
public class TheFilterExactPackageMethod
868+
{
869+
[Fact]
870+
public void ThrowsIfPackagesNull()
871+
{
872+
Assert.Throws<ArgumentNullException>(() => InvokeMethod(null, null));
873+
}
874+
875+
[Theory]
876+
[InlineData(null)]
877+
[InlineData("1.0.0")]
878+
public void ReturnsNullIfEmptyList(string version)
879+
{
880+
Assert.Equal(null, InvokeMethod(new Package[0], version));
881+
}
882+
883+
[Theory]
884+
[InlineData(null)]
885+
[InlineData("1.0.0-afakepackage")]
886+
public void ReturnsNullIfMissing(string version)
887+
{
888+
var packages = new[]
889+
{
890+
CreateTestPackage("1.0.0"),
891+
CreateTestPackage("2.0.0")
892+
};
893+
894+
Assert.Equal(null, InvokeMethod(packages, version));
895+
}
896+
897+
/// <remarks>
898+
/// The method should compare the normalized version of the package and be case-insensitive.
899+
/// </remarks>
900+
[Theory]
901+
[InlineData("1.0.0-a")]
902+
[InlineData("1.0.0-A")]
903+
[InlineData("1.0.0-a+metadata")]
904+
[InlineData("1.0.0-A+metadata")]
905+
public void ReturnsVersionIfExists(string version)
906+
{
907+
var package = CreateTestPackage("1.0.0-a");
908+
var packages = new[]
909+
{
910+
package,
911+
CreateTestPackage("2.0.0")
912+
};
913+
914+
Assert.Equal(package, InvokeMethod(packages, version));
915+
}
916+
917+
private Package InvokeMethod(IReadOnlyCollection<Package> packages, string version)
918+
{
919+
var service = CreateService();
920+
return service.FilterExactPackage(packages, version);
921+
}
922+
923+
private Package CreateTestPackage(string normalizedVersion)
924+
{
925+
return new Package
926+
{
927+
NormalizedVersion = normalizedVersion
928+
};
929+
}
930+
}
931+
867932
public class TheFilterLatestPackageMethod
868933
{
869934
protected const string Id = "theId";

0 commit comments

Comments
 (0)