Delete uploaded package from blob storage if saving changes to DB fai…

…ls (#3469)
NuGet · Jan 12, 2017 · 9216b03 · 9216b03
1 parent 89d78f0
commit 9216b03
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 3 deletions.
diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs
@@ -358,7 +358,16 @@ private async Task<ActionResult> CreatePackageInternal()
                             }
                         }
 
-                        await EntitiesContext.SaveChangesAsync();
+                        try
+                        {
+                            await EntitiesContext.SaveChangesAsync();
+                        }
+                        catch
+                        {
+                            // If saving to the DB fails for any reason, we need to delete the package we just saved.
+                            await PackageFileService.DeletePackageFileAsync(nuspec.GetId(), nuspec.GetVersion().ToNormalizedString());
+                            throw;
+                        }
 
                         IndexingService.UpdatePackage(package);
 

diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs
@@ -1180,8 +1180,17 @@ public virtual async Task<ActionResult> VerifyPackage(VerifyPackageRequest formD
                     return new RedirectResult(Url.VerifyPackage());
                 }
 
-                // commit all changes to database as an atomic transaction
-                await _entitiesContext.SaveChangesAsync();
+                try
+                {
+                    // commit all changes to database as an atomic transaction
+                    await _entitiesContext.SaveChangesAsync();
+                }
+                catch
+                {
+                    // If saving to the DB fails for any reason we need to delete the package we just saved.
+                    await _packageFileService.DeletePackageFileAsync(packageMetadata.Id, packageMetadata.Version.ToNormalizedString());
+                    throw;
+                }
 
                 // tell Lucene to update index for the new package
                 _indexingService.UpdateIndex();

diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs
@@ -118,6 +118,50 @@ public async Task CreatePackageWillSavePackageFileToFileStorage()
                 controller.MockPackageFileService.Verify();
             }
 
+            [Fact]
+            public async Task WillDeletePackageFileFromBlobStorageIfSavingDbChangesFails()
+            {
+                // Arrange
+                var user = new User() { EmailAddress = "confirmed@email.com" };
+                var packageId = "theId";
+                var packageVersion = "1.0.42";
+                var packageRegistration = new PackageRegistration();
+                packageRegistration.Id = packageId;
+                packageRegistration.Owners.Add(user);
+                var package = new Package();
+                package.PackageRegistration = packageRegistration;
+                package.Version = "1.0.42";
+                packageRegistration.Packages.Add(package);
+
+                var controller = new TestableApiController();
+                controller.SetCurrentUser(user);
+                controller.MockPackageFileService.Setup(
+                        p => p.SavePackageFileAsync(It.IsAny<Package>(), It.IsAny<Stream>()))
+                    .Returns(Task.CompletedTask).Verifiable();
+                controller.MockPackageFileService.Setup(
+                        p =>
+                            p.DeletePackageFileAsync(packageId,
+                                packageVersion))
+                    .Returns(Task.CompletedTask).Verifiable();
+                controller.MockPackageService.Setup(p => p.FindPackageRegistrationById(It.IsAny<string>()))
+                    .Returns(packageRegistration);
+                controller.MockPackageService.Setup(
+                        p =>
+                            p.CreatePackageAsync(It.IsAny<PackageArchiveReader>(), It.IsAny<PackageStreamMetadata>(),
+                                It.IsAny<User>(), false))
+                    .Returns(Task.FromResult(package));
+                controller.MockEntitiesContext.Setup(e => e.SaveChangesAsync()).Throws<Exception>();
+
+                var nuGetPackage = TestPackage.CreateTestPackageStream(packageId, "1.0.42");
+                controller.SetupPackageFromInputStream(nuGetPackage);
+
+                // Act
+                await Assert.ThrowsAsync<Exception>(async () => await controller.CreatePackagePut());
+
+                // Assert
+                controller.MockPackageFileService.Verify();
+            }
+
             [Fact]
             public async Task WritesAnAuditRecord()
             {

diff --git a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs
@@ -1436,6 +1436,47 @@ public async Task WillSavePackageToFileStorage()
                 }
             }
 
+            [Fact]
+            public async Task WillDeletePackageFileFromBlobStorageIfSavingDbChangesFails()
+            {
+                // Arrange
+                var packageId = "theId";
+                var packageVersion = "1.0.0";
+                var fakeUploadFileService = new Mock<IUploadFileService>();
+                using (var fakeFileStream = new MemoryStream())
+                {
+                    fakeUploadFileService.Setup(x => x.GetUploadFileAsync(TestUtility.FakeUser.Key)).Returns(Task.FromResult<Stream>(fakeFileStream));
+                    fakeUploadFileService.Setup(x => x.DeleteUploadFileAsync(TestUtility.FakeUser.Key)).Returns(Task.FromResult(0));
+                    var fakePackageService = new Mock<IPackageService>();
+                    var fakePackage = new Package { PackageRegistration = new PackageRegistration { Id = packageId }, Version = packageVersion };
+                    fakePackageService.Setup(x => x.CreatePackageAsync(It.IsAny<PackageArchiveReader>(), It.IsAny<PackageStreamMetadata>(), It.IsAny<User>(), It.IsAny<bool>()))
+                        .Returns(Task.FromResult(fakePackage));
+                    var fakeNuGetPackage = TestPackage.CreateTestPackageStream(packageId, packageVersion);
+
+                    var fakePackageFileService = new Mock<IPackageFileService>();
+                    fakePackageFileService.Setup(x => x.SavePackageFileAsync(fakePackage, It.IsAny<Stream>())).Returns(Task.CompletedTask).Verifiable();
+                    fakePackageFileService.Setup(x => x.DeletePackageFileAsync(packageId, packageVersion)).Returns(Task.CompletedTask).Verifiable();
+
+                    var fakeEntitiesContext = new Mock<IEntitiesContext>();
+                    fakeEntitiesContext.Setup(e => e.SaveChangesAsync()).Throws<Exception>();
+
+                    var controller = CreateController(
+                        packageService: fakePackageService,
+                        uploadFileService: fakeUploadFileService,
+                        fakeNuGetPackage: fakeNuGetPackage,
+                        packageFileService: fakePackageFileService,
+                        entitiesContext: fakeEntitiesContext);
+                    controller.SetCurrentUser(TestUtility.FakeUser);
+
+                    // Act
+                    await Assert.ThrowsAsync<Exception>(async () => await controller.VerifyPackage(new VerifyPackageRequest() { Listed = true, Edit = null }));
+
+                    // Assert
+                    fakePackageService.Verify(x => x.CreatePackageAsync(It.IsAny<PackageArchiveReader>(), It.IsAny<PackageStreamMetadata>(), TestUtility.FakeUser, false));
+                    fakePackageFileService.Verify();
+                }
+            }
+
             [Fact]
             public async Task WillShowViewWithMessageIfSavingPackageBlobFails()
             {