From ae36c58d0dd50eea20c0da84100832ac846c0203 Mon Sep 17 00:00:00 2001 From: anurse Date: Tue, 30 Jul 2013 15:20:39 -0700 Subject: [PATCH] Fix #1260 by updating the Id casing on each upload. --- Facts/Controllers/ApiControllerFacts.cs | 45 +++++++++++++++-- Facts/Controllers/PackagesControllerFacts.cs | 51 +++++++++++++++++++- Website/Controllers/ApiController.cs | 13 ++++- Website/Controllers/PackagesController.cs | 9 +++- 4 files changed, 110 insertions(+), 8 deletions(-) diff --git a/Facts/Controllers/ApiControllerFacts.cs b/Facts/Controllers/ApiControllerFacts.cs index ee503bb1a0..513a5b501e 100644 --- a/Facts/Controllers/ApiControllerFacts.cs +++ b/Facts/Controllers/ApiControllerFacts.cs @@ -91,6 +91,11 @@ public async Task CreatePackageWillSavePackageFileToFileStorage() nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42")); controller.SetupPackageFromInputStream(nuGetPackage); + controller + .MockPackageService + .Setup(p => p.CreatePackage(nuGetPackage.Object, fakeUser, false)) + .Returns(new Package() { PackageRegistration = new PackageRegistration() { Id = "theId" } }); + // Act await controller.CreatePackagePut(guid); @@ -192,7 +197,33 @@ public void WillCreateAPackageFromTheNuGetPackage() controller.CreatePackagePut(Guid.NewGuid().ToString()); - controller.MockPackageService.Verify(x => x.CreatePackage(nuGetPackage.Object, It.IsAny(), true)); + controller.MockPackageService.Verify(x => x.CreatePackage(nuGetPackage.Object, It.IsAny(), false)); + } + + [Fact] + public void WillUpdateThePackageRegistrationIdIfTheCasingIsDifferent() + { + var nuGetPackage = new Mock(); + nuGetPackage.Setup(x => x.Metadata.Id).Returns("theId"); + nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42")); + + var controller = new TestableApiController(); + controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(new User()); + controller.SetupPackageFromInputStream(nuGetPackage); + + var package = new Package() { PackageRegistration = new PackageRegistration() { Id = "TheID" } }; + controller + .MockPackageService + .Setup(x => x.CreatePackage(nuGetPackage.Object, It.IsAny(), false)) + .Returns(package) + .Verifiable(); + + + controller.CreatePackagePut(Guid.NewGuid().ToString()); + + Assert.Equal("theId", package.PackageRegistration.Id); + controller.MockEntitiesContext.Verify(e => e.SaveChanges()); + controller.MockPackageService.VerifyAll(); } [Fact] @@ -208,7 +239,7 @@ public void WillCreateAPackageWithTheUserMatchingTheApiKey() controller.CreatePackagePut(Guid.NewGuid().ToString()); - controller.MockPackageService.Verify(x => x.CreatePackage(It.IsAny(), matchingUser, true)); + controller.MockPackageService.Verify(x => x.CreatePackage(It.IsAny(), matchingUser, false)); } [Fact] @@ -220,12 +251,18 @@ public void CreatePackageRefreshesNuGetExeIfCommandLinePackageIsUploaded() nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("1.0.42")); var matchingUser = new User(); var controller = new TestableApiController(); - controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), true)) + controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), false)) .Returns(new Package { IsLatestStable = true }); controller.MockNuGetExeDownloaderService.Setup(s => s.UpdateExecutableAsync(nuGetPackage.Object)).Verifiable(); controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(matchingUser); controller.SetupPackageFromInputStream(nuGetPackage); + controller + .MockPackageService + .Setup(p => p.CreatePackage(nuGetPackage.Object, matchingUser, false)) + .Returns(new Package() { IsLatestStable = true, PackageRegistration = new PackageRegistration() { Id = "NuGet.CommandLine" } }); + + // Act controller.CreatePackagePut(Guid.NewGuid().ToString()); @@ -242,7 +279,7 @@ public void CreatePackageDoesNotRefreshNuGetExeIfItIsNotLatestStable() nuGetPackage.Setup(x => x.Metadata.Version).Returns(new SemanticVersion("2.0.0-alpha")); var matchingUser = new User(); var controller = new TestableApiController(); - controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), true)) + controller.MockPackageService.Setup(p => p.CreatePackage(nuGetPackage.Object, It.IsAny(), false)) .Returns(new Package { IsLatest = true, IsLatestStable = false }); controller.MockNuGetExeDownloaderService.Setup(s => s.UpdateExecutableAsync(nuGetPackage.Object)).Verifiable(); controller.MockUserService.Setup(x => x.FindByApiKey(It.IsAny())).Returns(matchingUser); diff --git a/Facts/Controllers/PackagesControllerFacts.cs b/Facts/Controllers/PackagesControllerFacts.cs index a4c9fc86c0..dafed10f59 100644 --- a/Facts/Controllers/PackagesControllerFacts.cs +++ b/Facts/Controllers/PackagesControllerFacts.cs @@ -1077,6 +1077,7 @@ public async Task WillCreateThePackage() fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns( new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" }); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var controller = CreateController( packageService: fakePackageService, @@ -1108,6 +1109,7 @@ public async Task WillSavePackageToFileStorage() var fakePackage = new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" }; fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakePackage); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var fakePackageFileService = new Mock(); fakePackageFileService.Setup(x => x.SavePackageFileAsync(fakePackage, It.IsAny())).Returns(Task.FromResult(0)).Verifiable(); @@ -1145,6 +1147,7 @@ public async Task WillUpdateIndexingService() var fakePackage = new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" }; fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakePackage); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var fakePackageFileService = new Mock(); fakePackageFileService.Setup(x => x.SavePackageFileAsync(fakePackage, It.IsAny())).Returns(Task.FromResult(0)).Verifiable(); @@ -1185,6 +1188,7 @@ public async Task WillSaveChangesToEntitiesContext() var fakePackage = new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" }; fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakePackage); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var entitiesContext = new Mock(); entitiesContext.Setup(e => e.SaveChanges()).Verifiable(); @@ -1224,6 +1228,7 @@ public async Task WillNotCommitChangesToPackageService() fakePackageService.Setup(x => x.PublishPackage(fakePackage, false)); fakePackageService.Setup(x => x.MarkPackageUnlisted(fakePackage, false)); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var controller = CreateController( packageService: fakePackageService, @@ -1257,6 +1262,7 @@ public async Task WillPublishThePackage() var fakePackageService = new Mock(); fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakePackage); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var controller = CreateController( packageService: fakePackageService, uploadFileService: fakeUploadFileService, @@ -1270,6 +1276,40 @@ public async Task WillPublishThePackage() fakeFileStream.Dispose(); } + [Fact] + public async Task WillUpdateTheIdOfTheRegistrationIfTheCasingChanges() + { + var fakeCurrentUser = new User { Key = 42 }; + var fakeUserService = new Mock(); + fakeUserService.Setup(x => x.FindByUsername(It.IsAny())).Returns(fakeCurrentUser); + var fakeIdentity = new Mock(); + fakeIdentity.Setup(x => x.Name).Returns("theUsername"); + var fakeUploadFileService = new Mock(); + var fakeFileStream = new MemoryStream(); + fakeUploadFileService.Setup(x => x.GetUploadFileAsync(42)).Returns(Task.FromResult(fakeFileStream)); + fakeUploadFileService.Setup(x => x.DeleteUploadFileAsync(42)).Returns(Task.FromResult(0)); + var fakePackage = new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" }; + var fakePackageService = new Mock(); + fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(fakePackage); + var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata.Id).Returns("TheID"); + var fakeEntitiesContext = new Mock(); + var controller = CreateController( + packageService: fakePackageService, + uploadFileService: fakeUploadFileService, + userService: fakeUserService, + fakeIdentity: fakeIdentity, + fakeNuGetPackage: fakeNuGetPackage, + entitiesContext: fakeEntitiesContext); + + await controller.VerifyPackage(null); + + fakePackageService.Verify(x => x.PublishPackage(fakePackage, false), Times.Once()); + Assert.Equal("TheID", fakePackage.PackageRegistration.Id); + fakeEntitiesContext.Verify(e => e.SaveChanges()); + fakeFileStream.Dispose(); + } + [Fact] public async Task WillMarkThePackageUnlistedWhenListedArgumentIsFalse() { @@ -1318,6 +1358,7 @@ public async Task WillNotMarkThePackageUnlistedWhenListedArgumentIsNullorTrue(bo fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns( new Package { PackageRegistration = new PackageRegistration { Id = "theId" }, Version = "theVersion" }); var fakeNuGetPackage = new Mock(); + fakeNuGetPackage.Setup(p => p.Metadata).Returns(new Mock().Object); var controller = CreateController( packageService: fakePackageService, uploadFileService: fakeUploadFileService, @@ -1472,12 +1513,15 @@ public async Task WillExtractNuGetExe() fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(commandLinePackage); var nugetExeDownloader = new Mock(MockBehavior.Strict); nugetExeDownloader.Setup(d => d.UpdateExecutableAsync(It.IsAny())).Returns(Task.FromResult(0)).Verifiable(); + var fakeNupkg = new Mock(); + fakeNupkg.Setup(p => p.Metadata.Id).Returns(commandLinePackage.PackageRegistration.Id); var controller = CreateController( packageService: fakePackageService, uploadFileService: fakeUploadFileService, fakeIdentity: fakeIdentity, userService: fakeUserService, - downloaderService: nugetExeDownloader); + downloaderService: nugetExeDownloader, + fakeNuGetPackage: fakeNupkg); // Act await controller.VerifyPackage(false); @@ -1507,12 +1551,15 @@ public async Task WillNotExtractNuGetExeIfIsNotLatestStable() }; fakePackageService.Setup(x => x.CreatePackage(It.IsAny(), It.IsAny(), It.IsAny())).Returns(commandLinePackage); var nugetExeDownloader = new Mock(MockBehavior.Strict); + var fakeNupkg = new Mock(); + fakeNupkg.Setup(p => p.Metadata.Id).Returns(commandLinePackage.PackageRegistration.Id); var controller = CreateController( packageService: fakePackageService, uploadFileService: fakeUploadFileService, fakeIdentity: fakeIdentity, userService: fakeUserService, - downloaderService: nugetExeDownloader); + downloaderService: nugetExeDownloader, + fakeNuGetPackage: fakeNupkg); // Act await controller.VerifyPackage(false); diff --git a/Website/Controllers/ApiController.cs b/Website/Controllers/ApiController.cs index 61aba21307..cc6612960d 100644 --- a/Website/Controllers/ApiController.cs +++ b/Website/Controllers/ApiController.cs @@ -220,6 +220,7 @@ private async Task CreatePackageInternal(string apiKey) { // Ensure that the user can push packages for this partialId. var packageRegistration = PackageService.FindPackageRegistrationById(packageToPush.Metadata.Id); + if (packageRegistration != null) { if (!packageRegistration.IsOwner(user)) @@ -244,12 +245,22 @@ private async Task CreatePackageInternal(string apiKey) } } - var package = PackageService.CreatePackage(packageToPush, user, commitChanges: true); + var package = PackageService.CreatePackage(packageToPush, user, commitChanges: false); + + // Update the package registration's ID if it doesn't already match. + if (packageToPush != null && packageToPush.Metadata != null) // A little extra defensiveness makes the tests simpler :) + { + package.PackageRegistration.Id = packageToPush.Metadata.Id; + } + using (Stream uploadStream = packageToPush.GetStream()) { await PackageFileService.SavePackageFileAsync(package, uploadStream); } + EntitiesContext.SaveChanges(); + IndexingService.UpdateIndex(); + if ( packageToPush.Metadata.Id.Equals(Constants.NuGetCommandLinePackageId, StringComparison.OrdinalIgnoreCase) && package.IsLatestStable) diff --git a/Website/Controllers/PackagesController.cs b/Website/Controllers/PackagesController.cs index 51254bbbbd..c6bf76834d 100644 --- a/Website/Controllers/PackagesController.cs +++ b/Website/Controllers/PackagesController.cs @@ -622,6 +622,12 @@ public virtual async Task VerifyPackage(bool? listed) package = _packageService.CreatePackage(nugetPackage, currentUser, commitChanges: false); Debug.Assert(package.PackageRegistration != null); + // Update the package registration's ID if it doesn't already match. + if (nugetPackage.Metadata != null) // A little extra defensiveness makes the tests simpler :) + { + package.PackageRegistration.Id = nugetPackage.Metadata.Id; + } + _packageService.PublishPackage(package, commitChanges: false); if (listed == false) @@ -642,7 +648,8 @@ public virtual async Task VerifyPackage(bool? listed) _indexingService.UpdateIndex(); // If we're pushing a new stable version of NuGet.CommandLine, update the extracted executable. - if (package.PackageRegistration.Id.Equals(Constants.NuGetCommandLinePackageId, StringComparison.OrdinalIgnoreCase) && + if (!String.IsNullOrEmpty(package.PackageRegistration.Id) && + package.PackageRegistration.Id.Equals(Constants.NuGetCommandLinePackageId, StringComparison.OrdinalIgnoreCase) && package.IsLatestStable) { await _nugetExeDownloaderService.UpdateExecutableAsync(nugetPackage);