From d679d725621f6c3f1f4b1cd5dbc076df903952ad Mon Sep 17 00:00:00 2001 From: Xavier Decoster Date: Mon, 8 May 2017 14:15:12 +0200 Subject: [PATCH 1/2] Fix bug in IsLatest(Stable)SemVer2 --- .../Lucene/LuceneIndexingService.cs | 1 - src/NuGetGallery/Services/PackageService.cs | 2 +- .../Services/PackageServiceFacts.cs | 261 ++++++++++++------ 3 files changed, 185 insertions(+), 79 deletions(-) diff --git a/src/NuGetGallery/Infrastructure/Lucene/LuceneIndexingService.cs b/src/NuGetGallery/Infrastructure/Lucene/LuceneIndexingService.cs index fd963f35b7..4c2346904c 100644 --- a/src/NuGetGallery/Infrastructure/Lucene/LuceneIndexingService.cs +++ b/src/NuGetGallery/Infrastructure/Lucene/LuceneIndexingService.cs @@ -10,7 +10,6 @@ using System.Linq; using System.Threading.Tasks; using Lucene.Net.Index; -using Lucene.Net.Store; using NuGetGallery.Configuration; using NuGetGallery.Diagnostics; using WebBackgrounder; diff --git a/src/NuGetGallery/Services/PackageService.cs b/src/NuGetGallery/Services/PackageService.cs index 6574578ada..6c412e802b 100644 --- a/src/NuGetGallery/Services/PackageService.cs +++ b/src/NuGetGallery/Services/PackageService.cs @@ -847,7 +847,7 @@ public async Task UpdateIsLatestAsync(PackageRegistration packageRegistration, b // If the newest uploaded package is a prerelease package, we need to find an older package that is // a release version and set it to IsLatest. var latestSemVer2ReleasePackage = FindPackage( - packageRegistration.Packages.Where(p => !p.IsPrerelease && !p.Deleted && p.Listed && p.SemVerLevelKey == SemVerLevelKey.SemVer2)); + packageRegistration.Packages.Where(p => !p.IsPrerelease && !p.Deleted && p.Listed && (p.SemVerLevelKey == SemVerLevelKey.SemVer2 || p.SemVerLevelKey == SemVerLevelKey.Unknown))); if (latestSemVer2ReleasePackage != null) { diff --git a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs index 89d4782226..56c770e093 100644 --- a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs @@ -173,7 +173,7 @@ public async Task RemovesRelatedPendingOwnerRequest() repository.VerifyAll(); } - + [Fact] public async Task WritesAnAuditRecord() { @@ -496,7 +496,7 @@ public async Task UpdateIndexIfCommitChangesIsTrue() var currentUser = new User(); // Act - var package = await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), currentUser, commitChanges: true); + var package = await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), currentUser, commitChanges: true); // Assert indexingService.Verify(); @@ -627,10 +627,10 @@ private async Task WillSaveTheCreatedPackageWhenThePackageRegistrationAlreadyExi { var currentUser = new User(); var packageRegistration = new PackageRegistration - { - Id = "theId", - Owners = new HashSet { currentUser }, - }; + { + Id = "theId", + Owners = new HashSet { currentUser }, + }; var packageRegistrationRepository = new Mock>(); var service = CreateService(packageRegistrationRepository: packageRegistrationRepository, setup: mockPackageService => { mockPackageService.Setup(x => x.FindPackageRegistrationById(It.IsAny())).Returns(packageRegistration); }); @@ -647,10 +647,10 @@ private async Task WillThrowIfThePackageRegistrationAlreadyExistsAndTheCurrentUs { var currentUser = new User(); var packageRegistration = new PackageRegistration - { - Id = "theId", - Owners = new HashSet() - }; + { + Id = "theId", + Owners = new HashSet() + }; var packageRegistrationRepository = new Mock>(); var service = CreateService(packageRegistrationRepository: packageRegistrationRepository, setup: mockPackageService => { mockPackageService.Setup(x => x.FindPackageRegistrationById(It.IsAny())).Returns(packageRegistration); }); @@ -733,7 +733,7 @@ private async Task WillThrowIfTheNuGetPackageAuthorsIsLongerThan4000() private async Task WillThrowIfTheNuGetPackageCopyrightIsLongerThan4000() { var service = CreateService(); - var nugetPackage = CreateNuGetPackage(copyright: "theCopyright".PadRight(4001, '_')); + var nugetPackage = CreateNuGetPackage(copyright: "theCopyright".PadRight(4001, '_')); var ex = await Assert.ThrowsAsync(async () => await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null)); @@ -837,7 +837,7 @@ public async Task WillThrowIfTheNuGetPackageDescriptionIsNull() private async Task WillThrowIfTheNuGetPackageDescriptionIsLongerThan4000() { var service = CreateService(); - var nugetPackage = CreateNuGetPackage(description: "theDescription".PadRight(4001, '_')); + var nugetPackage = CreateNuGetPackage(description: "theDescription".PadRight(4001, '_')); var ex = await Assert.ThrowsAsync(async () => await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null)); @@ -892,7 +892,7 @@ private async Task WillThrowIfTheNuGetPackageSummaryIsLongerThan4000() private async Task WillThrowIfTheNuGetPackageTagsIsLongerThan4000() { var service = CreateService(); - var nugetPackage = CreateNuGetPackage(tags: "theTags".PadRight(4001, '_')); + var nugetPackage = CreateNuGetPackage(tags: "theTags".PadRight(4001, '_')); var ex = await Assert.ThrowsAsync(async () => await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), null)); @@ -946,7 +946,7 @@ private async Task WillSaveSupportedFrameworks() Assert.Equal("net40", package.SupportedFrameworks.First().TargetFramework); Assert.Equal("net35", package.SupportedFrameworks.ElementAt(1).TargetFramework); } - + [Fact] private async Task WillNotSaveAnySupportedFrameworksWhenThereIsAnAnyTargetFramework() { @@ -1072,9 +1072,15 @@ public async Task WillUpdateIsLatest1() // Assert Assert.True(package10A.IsLatest); + Assert.True(package10A.IsLatestSemVer2); Assert.False(package10A.IsLatestStable); + Assert.False(package10A.IsLatestStableSemVer2); + Assert.False(package09.IsLatest); + Assert.False(package09.IsLatestSemVer2); Assert.True(package09.IsLatestStable); + Assert.True(package09.IsLatestStableSemVer2); + packageRepository.Verify(); } @@ -1100,11 +1106,112 @@ public async Task WillUpdateIsLatest2() // Assert Assert.True(package100.IsLatest); + Assert.True(package100.IsLatestSemVer2); + Assert.True(package100.IsLatestStable); + Assert.True(package100.IsLatestStableSemVer2); + + Assert.False(package10A.IsLatest); + Assert.False(package10A.IsLatestSemVer2); + Assert.False(package10A.IsLatestStable); + Assert.False(package10A.IsLatestStableSemVer2); + + Assert.False(package09.IsLatest); + Assert.False(package09.IsLatestSemVer2); + Assert.False(package09.IsLatestStable); + Assert.False(package09.IsLatestStableSemVer2); + + packageRepository.Verify(); + } + + [Fact] + public async Task WillUpdateIsLatest3() + { + // Arrange + var packages = new HashSet(); + var packageRegistration = new PackageRegistration { Packages = packages }; + var semVer2Package = new Package { PackageRegistration = packageRegistration, Version = "1.0.1-alpha.1", IsPrerelease = true, SemVerLevelKey = SemVerLevelKey.SemVer2 }; + packages.Add(semVer2Package); + var package100 = new Package { PackageRegistration = packageRegistration, Version = "1.0.0" }; + packages.Add(package100); + var package10A = new Package { PackageRegistration = packageRegistration, Version = "1.0.0-a", IsPrerelease = true }; + packages.Add(package10A); + var package09 = new Package { PackageRegistration = packageRegistration, Version = "0.9.0" }; + packages.Add(package09); + var packageRepository = new Mock>(MockBehavior.Strict); + packageRepository.Setup(r => r.CommitChangesAsync()) + .Returns(Task.CompletedTask).Verifiable(); + var service = CreateService(packageRepository: packageRepository); + + // Act + await service.UpdateIsLatestAsync(packageRegistration, true); + + // Assert + Assert.True(semVer2Package.IsLatestSemVer2); + Assert.False(semVer2Package.IsLatestStableSemVer2); + Assert.False(semVer2Package.IsLatest); + Assert.False(semVer2Package.IsLatestStable); + + Assert.True(package100.IsLatest); + Assert.False(package100.IsLatestSemVer2); + Assert.True(package100.IsLatestStable); + Assert.True(package100.IsLatestStableSemVer2); + + Assert.False(package10A.IsLatest); + Assert.False(package10A.IsLatestSemVer2); + Assert.False(package10A.IsLatestStable); + Assert.False(package10A.IsLatestStableSemVer2); + + Assert.False(package09.IsLatest); + Assert.False(package09.IsLatestSemVer2); + Assert.False(package09.IsLatestStable); + Assert.False(package09.IsLatestStableSemVer2); + + packageRepository.Verify(); + } + + [Fact] + public async Task WillUpdateIsLatest4() + { + // Arrange + var packages = new HashSet(); + var packageRegistration = new PackageRegistration { Packages = packages }; + var semVer2Package = new Package { PackageRegistration = packageRegistration, Version = "1.0.1+metadata", SemVerLevelKey = SemVerLevelKey.SemVer2 }; + packages.Add(semVer2Package); + var package100 = new Package { PackageRegistration = packageRegistration, Version = "1.0.0" }; + packages.Add(package100); + var package10A = new Package { PackageRegistration = packageRegistration, Version = "1.0.0-a", IsPrerelease = true }; + packages.Add(package10A); + var package09 = new Package { PackageRegistration = packageRegistration, Version = "0.9.0" }; + packages.Add(package09); + var packageRepository = new Mock>(MockBehavior.Strict); + packageRepository.Setup(r => r.CommitChangesAsync()) + .Returns(Task.CompletedTask).Verifiable(); + var service = CreateService(packageRepository: packageRepository); + + // Act + await service.UpdateIsLatestAsync(packageRegistration, true); + + // Assert + Assert.True(semVer2Package.IsLatestSemVer2); + Assert.True(semVer2Package.IsLatestStableSemVer2); + Assert.False(semVer2Package.IsLatest); + Assert.False(semVer2Package.IsLatestStable); + + Assert.True(package100.IsLatest); + Assert.False(package100.IsLatestSemVer2); Assert.True(package100.IsLatestStable); + Assert.False(package100.IsLatestStableSemVer2); + Assert.False(package10A.IsLatest); + Assert.False(package10A.IsLatestSemVer2); Assert.False(package10A.IsLatestStable); + Assert.False(package10A.IsLatestStableSemVer2); + Assert.False(package09.IsLatest); + Assert.False(package09.IsLatestSemVer2); Assert.False(package09.IsLatestStable); + Assert.False(package09.IsLatestStableSemVer2); + packageRepository.Verify(); } } @@ -1160,7 +1267,7 @@ public void ReturnsTheLatestStableVersionIfAvailable(string semVerLevel) Assert.NotNull(result); Assert.Equal("1.0", result.Version); } - + [Fact] public void ReturnsTheLatestStableSemVer2VersionIfAvailable() { @@ -1550,12 +1657,12 @@ public async Task WritesAnAuditRecord() var packageRepository = new Mock>(); var auditingService = new TestAuditingService(); var service = CreateService( - packageRepository: packageRepository, + packageRepository: packageRepository, auditingService: auditingService); // Act await service.MarkPackageListedAsync(package); - + // Assert Assert.True(auditingService.WroteRecord(ar => ar.Action == AuditedPackageAction.List @@ -1678,14 +1785,14 @@ public class ThePublishPackageMethod public async Task WillSetThePublishedDateOnThePackageBeingPublished() { var package = new Package + { + Version = "1.0.42", + PackageRegistration = new PackageRegistration { - Version = "1.0.42", - PackageRegistration = new PackageRegistration - { - Id = "theId", - Packages = new HashSet() - } - }; + Id = "theId", + Packages = new HashSet() + } + }; package.PackageRegistration.Packages.Add(package); var packageRepository = new Mock>(); var service = CreateService(packageRepository: packageRepository, setup: @@ -1724,14 +1831,14 @@ public async Task WillSetThePublishedDateOnThePackageBeingPublishedWithOverload( public async Task WillSetUpdateIsLatestStableOnThePackageWhenItIsTheLatestVersionWithOverload() { var package = new Package + { + Version = "1.0.42", + PackageRegistration = new PackageRegistration { - Version = "1.0.42", - PackageRegistration = new PackageRegistration - { - Id = "theId", - Packages = new HashSet() - } - }; + Id = "theId", + Packages = new HashSet() + } + }; package.PackageRegistration.Packages.Add(package); var packageRepository = new Mock>(); @@ -1770,22 +1877,22 @@ public async Task WillSetUpdateIsLatestStableOnThePackageWhenItIsTheLatestVersio public async Task WillNotSetUpdateIsLatestStableOnThePackageWhenItIsNotTheLatestVersionWithOverload() { var package = new Package + { + Version = "1.0.42", + PackageRegistration = new PackageRegistration { - Version = "1.0.42", - PackageRegistration = new PackageRegistration - { - Id = "theId", - Packages = new HashSet() - } - }; + Id = "theId", + Packages = new HashSet() + } + }; package.PackageRegistration.Packages.Add(package); package.PackageRegistration.Packages.Add( new Package - { - Version = "2.0", - PackageRegistration = package.PackageRegistration, - Published = DateTime.UtcNow - }); + { + Version = "2.0", + PackageRegistration = package.PackageRegistration, + Published = DateTime.UtcNow + }); var packageRepository = new Mock>(); var service = CreateService(packageRepository: packageRepository, setup: mockPackageService => { mockPackageService.Setup(x => x.FindPackageByIdAndVersionStrict(It.IsAny(), It.IsAny())).Returns(package); }); @@ -1799,23 +1906,23 @@ public async Task WillNotSetUpdateIsLatestStableOnThePackageWhenItIsNotTheLatest public async Task PublishPackageUpdatesIsAbsoluteLatestForPrereleasePackage() { var package = new Package + { + Version = "1.0.42-alpha", + Published = DateTime.UtcNow, + PackageRegistration = new PackageRegistration { - Version = "1.0.42-alpha", - Published = DateTime.UtcNow, - PackageRegistration = new PackageRegistration - { - Id = "theId", - Packages = new HashSet() - }, - IsPrerelease = true, - }; + Id = "theId", + Packages = new HashSet() + }, + IsPrerelease = true, + }; package.PackageRegistration.Packages.Add(package); var package39 = new Package - { - Version = "1.0.39", - PackageRegistration = package.PackageRegistration, - Published = DateTime.UtcNow.AddDays(-1) - }; + { + Version = "1.0.39", + PackageRegistration = package.PackageRegistration, + Published = DateTime.UtcNow.AddDays(-1) + }; package.PackageRegistration.Packages.Add(package39); var packageRepository = new Mock>(); var service = CreateService(packageRepository: packageRepository, setup: @@ -1866,24 +1973,24 @@ public async Task PublishPackageUpdatesIsAbsoluteLatestForPrereleasePackageWithO public async Task SetUpdateDoesNotSetIsLatestStableForAnyIfAllPackagesArePrerelease() { var package = new Package + { + Version = "1.0.42-alpha", + Published = DateTime.UtcNow, + IsPrerelease = true, + PackageRegistration = new PackageRegistration { - Version = "1.0.42-alpha", - Published = DateTime.UtcNow, - IsPrerelease = true, - PackageRegistration = new PackageRegistration - { - Id = "theId", - Packages = new HashSet() - } - }; + Id = "theId", + Packages = new HashSet() + } + }; package.PackageRegistration.Packages.Add(package); var package39 = new Package - { - Version = "1.0.39-beta", - PackageRegistration = package.PackageRegistration, - Published = DateTime.UtcNow.AddDays(-1), - IsPrerelease = true - }; + { + Version = "1.0.39-beta", + PackageRegistration = package.PackageRegistration, + Published = DateTime.UtcNow.AddDays(-1), + IsPrerelease = true + }; package.PackageRegistration.Packages.Add(package39); var packageRepository = new Mock>(); var service = CreateService(packageRepository: packageRepository, setup: @@ -1984,11 +2091,11 @@ await Assert.ThrowsAsync( public async Task RemovesPendingPackageOwner() { var packageOwnerRequest = new PackageOwnerRequest - { - PackageRegistrationKey = 1, - RequestingOwnerKey = 99, - NewOwnerKey = 200 - }; + { + PackageRegistrationKey = 1, + RequestingOwnerKey = 99, + NewOwnerKey = 200 + }; var packageOwnerRequestRepository = new Mock>(); packageOwnerRequestRepository.Setup(r => r.GetAll()).Returns(new[] { packageOwnerRequest }.AsQueryable()); packageOwnerRequestRepository.Setup(r => r.DeleteOnCommit(packageOwnerRequest)).Verifiable(); From 868c37a5dd6ced8b5d1a3794fdf64e03fb697314 Mon Sep 17 00:00:00 2001 From: Xavier Decoster Date: Mon, 8 May 2017 18:29:47 +0200 Subject: [PATCH 2/2] Test method renamed to be more descriptive --- tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs index 56c770e093..547a839d78 100644 --- a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs @@ -1053,7 +1053,7 @@ public async Task CommitIfCommitChangesIsTrue() } [Fact] - public async Task WillUpdateIsLatest1() + public async Task UpdateIsLatestScenarioForPrereleaseAsAbsoluteLatest() { // Arrange var packages = new HashSet(); @@ -1085,7 +1085,7 @@ public async Task WillUpdateIsLatest1() } [Fact] - public async Task WillUpdateIsLatest2() + public async Task UpdateIsLatestScenarioForStableAsAbsoluteLatest() { // Arrange var packages = new HashSet(); @@ -1124,7 +1124,7 @@ public async Task WillUpdateIsLatest2() } [Fact] - public async Task WillUpdateIsLatest3() + public async Task UpdateIsLatestScenarioForSemVer2PrereleaseAsAbsoluteLatest() { // Arrange var packages = new HashSet(); @@ -1170,7 +1170,7 @@ public async Task WillUpdateIsLatest3() } [Fact] - public async Task WillUpdateIsLatest4() + public async Task UpdateIsLatestScenarioForSemVer2StableAsAbsoluteLatest() { // Arrange var packages = new HashSet();