diff --git a/src/NuGetGallery.Core/NuGetGallery.Core.csproj b/src/NuGetGallery.Core/NuGetGallery.Core.csproj
index a6c30c559a..00b6654a50 100644
--- a/src/NuGetGallery.Core/NuGetGallery.Core.csproj
+++ b/src/NuGetGallery.Core/NuGetGallery.Core.csproj
@@ -161,6 +161,7 @@
+
diff --git a/src/NuGetGallery.Core/Services/PackageAlreadyExistsException.cs b/src/NuGetGallery.Core/Services/PackageAlreadyExistsException.cs
new file mode 100644
index 0000000000..6748c7d5a4
--- /dev/null
+++ b/src/NuGetGallery.Core/Services/PackageAlreadyExistsException.cs
@@ -0,0 +1,15 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+
+namespace NuGetGallery
+{
+ [Serializable]
+ public sealed class PackageAlreadyExistsException : Exception
+ {
+ public PackageAlreadyExistsException() { }
+ public PackageAlreadyExistsException(string message) : base(message) { }
+ public PackageAlreadyExistsException(string message, Exception inner) : base(message, inner) { }
+ }
+}
\ No newline at end of file
diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs
index 69ec973713..dccd155681 100644
--- a/src/NuGetGallery/Controllers/ApiController.cs
+++ b/src/NuGetGallery/Controllers/ApiController.cs
@@ -25,8 +25,6 @@
using NuGetGallery.Configuration;
using NuGetGallery.Filters;
using NuGetGallery.Infrastructure.Authentication;
-using NuGetGallery.Infrastructure.Mail;
-using NuGetGallery.Infrastructure.Mail.Messages;
using NuGetGallery.Packaging;
using NuGetGallery.Security;
using PackageIdValidator = NuGetGallery.Packaging.PackageIdValidator;
@@ -731,6 +729,10 @@ await AuditingService.SaveAuditRecordAsync(
{
return BadRequestForExceptionMessage(ex);
}
+ catch (PackageAlreadyExistsException ex)
+ {
+ return new HttpStatusCodeWithBodyResult(HttpStatusCode.Conflict, ex.Message);
+ }
catch (EntityException ex)
{
return BadRequestForExceptionMessage(ex);
diff --git a/src/NuGetGallery/Services/PackageService.cs b/src/NuGetGallery/Services/PackageService.cs
index 14656e9ff4..0e0f79b0c7 100644
--- a/src/NuGetGallery/Services/PackageService.cs
+++ b/src/NuGetGallery/Services/PackageService.cs
@@ -479,8 +479,8 @@ private Package CreatePackageFromNuGetPackage(PackageRegistration packageRegistr
if (package != null)
{
- throw new EntityException(
- "A package with identifier '{0}' and version '{1}' already exists.", packageRegistration.Id, package.Version);
+ throw new PackageAlreadyExistsException(
+ string.Format(Strings.PackageExistsAndCannotBeModified, packageRegistration.Id, package.Version));
}
package = new Package();
diff --git a/src/NuGetGallery/Services/PackageUploadService.cs b/src/NuGetGallery/Services/PackageUploadService.cs
index 1c7e643d00..1b840e0d73 100644
--- a/src/NuGetGallery/Services/PackageUploadService.cs
+++ b/src/NuGetGallery/Services/PackageUploadService.cs
@@ -3,6 +3,8 @@
using System;
using System.Collections.Generic;
+using System.Data.Entity.Infrastructure;
+using System.Data.SqlClient;
using System.IO;
using System.Linq;
using System.Threading;
@@ -427,7 +429,7 @@ await _packageFileService.DeleteValidationPackageFileAsync(
// commit all changes to database as an atomic transaction
await _entitiesContext.SaveChangesAsync();
}
- catch
+ catch (Exception ex)
{
// If saving to the DB fails for any reason we need to delete the package we just saved.
if (package.PackageStatusKey == PackageStatus.Validating)
@@ -443,10 +445,36 @@ await _packageFileService.DeletePackageFileAsync(
package.Version);
}
- throw;
+ return ReturnConflictOrThrow(ex);
}
return PackageCommitResult.Success;
}
+
+ private PackageCommitResult ReturnConflictOrThrow(Exception ex)
+ {
+ if (ex is DbUpdateConcurrencyException concurrencyEx)
+ {
+ return PackageCommitResult.Conflict;
+ }
+ else if (ex is DbUpdateException dbUpdateEx)
+ {
+ if (dbUpdateEx.InnerException?.InnerException != null)
+ {
+ if (dbUpdateEx.InnerException.InnerException is SqlException sqlException)
+ {
+ switch (sqlException.Number)
+ {
+ case 547: // Constraint check violation
+ case 2601: // Duplicated key row error
+ case 2627: // Unique constraint error
+ return PackageCommitResult.Conflict;
+ }
+ }
+ }
+ }
+
+ throw ex;
+ }
}
}
diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs
index b2a9ee0bfc..407d889ab2 100644
--- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs
+++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs
@@ -843,6 +843,52 @@ public async Task WillReturnConflictIfCommittingPackageReturnsConflict()
controller.MockEntitiesContext.VerifyCommitted(Times.Never());
}
+ [Fact]
+ public async Task WillReturnConflictIfGeneratePackageThrowsPackageAlreadyExistsException()
+ {
+ // Arrange
+ var packageId = "theId";
+ var nuGetPackage = TestPackage.CreateTestPackageStream(packageId, "1.0.42");
+
+ var currentUser = new User("currentUser") { Key = 1, EmailAddress = "currentUser@confirmed.com" };
+ var controller = new TestableApiController(GetConfigurationService());
+ controller.SetCurrentUser(currentUser);
+ controller.SetupPackageFromInputStream(nuGetPackage);
+
+ var owner = new User("owner") { Key = 2, EmailAddress = "org@confirmed.com" };
+
+ Expression> evaluateApiScope =
+ x => x.Evaluate(
+ currentUser,
+ It.IsAny>(),
+ ActionsRequiringPermissions.UploadNewPackageId,
+ It.Is((context) => context.PackageId == packageId),
+ NuGetScopes.PackagePush);
+
+ controller.MockApiScopeEvaluator
+ .Setup(evaluateApiScope)
+ .Returns(new ApiScopeEvaluationResult(owner, PermissionsCheckResult.Allowed, scopesAreValid: true));
+ controller
+ .MockPackageUploadService
+ .Setup(x => x.GeneratePackageAsync(It.IsAny(),
+ It.IsAny(),
+ It.IsAny(),
+ It.IsAny(),
+ It.IsAny()))
+ .Throws(new PackageAlreadyExistsException("Package exists"));
+
+ // Act
+ var result = await controller.CreatePackagePut();
+
+ // Assert
+ ResultAssert.IsStatusCode(result, HttpStatusCode.Conflict);
+ controller.MockPackageUploadService.Verify(x => x.GeneratePackageAsync(It.IsAny(),
+ It.IsAny(),
+ It.IsAny(),
+ It.IsAny(),
+ It.IsAny()), Times.Once);
+ }
+
[Fact]
public async Task WillReturnValidationWarnings()
{
diff --git a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs
index 0809dc8d01..5e73609a59 100644
--- a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs
+++ b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs
@@ -423,6 +423,26 @@ private async Task WillSaveTheCreatedPackageWhenThePackageRegistrationAlreadyExi
Assert.Same(packageRegistration.Packages.ElementAt(0), package);
}
+ [Fact]
+ private async Task WillThrowWhenThePackageRegistrationAndVersionAlreadyExists()
+ {
+ var currentUser = new User();
+ var packageId = "theId";
+ var packageVersion = "1.0.32";
+ var nugetPackage = PackageServiceUtility.CreateNuGetPackage(packageId, packageVersion);
+ var packageRegistration = new PackageRegistration
+ {
+ Id = packageId,
+ Owners = new HashSet { currentUser },
+ };
+ packageRegistration.Packages.Add(new Package() { Version = packageVersion });
+ var packageRegistrationRepository = new Mock>();
+ var service = CreateService(packageRegistrationRepository: packageRegistrationRepository, setup:
+ mockPackageService => { mockPackageService.Setup(x => x.FindPackageRegistrationById(It.IsAny())).Returns(packageRegistration); });
+
+ await Assert.ThrowsAsync(async () => await service.CreatePackageAsync(nugetPackage.Object, new PackageStreamMetadata(), currentUser, currentUser, isVerified: false));
+ }
+
[Fact]
private async Task WillThrowIfTheNuGetPackageIdIsLongerThanMaxPackageIdLength()
{
diff --git a/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs
index bf0e5e3f53..e9c2b7b2a2 100644
--- a/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs
+++ b/tests/NuGetGallery.Facts/Services/PackageUploadServiceFacts.cs
@@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
+using System.Data.Entity.Infrastructure;
using System.IO;
using System.Linq;
using System.Threading;
@@ -1037,6 +1038,27 @@ public async Task DeletesPackageIfDatabaseCommitFailsWhenAvailable()
Assert.Same(_unexpectedException, exception);
}
+ [Fact]
+ public async Task ReturnsConflictWhenDBCommitThrowsConcurrencyViolations()
+ {
+ _package.PackageStatusKey = PackageStatus.Available;
+ var ex = new DbUpdateConcurrencyException("whoops!");
+ _entitiesContext
+ .Setup(x => x.SaveChangesAsync())
+ .Throws(ex);
+
+ var result = await _target.CommitPackageAsync(_package, _packageFile);
+
+ _packageFileService.Verify(
+ x => x.DeletePackageFileAsync(Id, Version),
+ Times.Once);
+ _packageFileService.Verify(
+ x => x.DeletePackageFileAsync(It.IsAny(), It.IsAny()),
+ Times.Once);
+
+ Assert.Equal(PackageCommitResult.Conflict, result);
+ }
+
[Fact]
public async Task DeletesPackageIfDatabaseCommitFailsWhenValidating()
{