Optimize the push package API (#8371)

Previously, the functional tests on PROD failed repeatedly. The functional tests push many packages in parallel, which caused an expensive SQL query to time out. This optimizes the push package API by avoiding a costly SQL query on the "hot" path.

Addresses #8368

Author: loic-sharma

src/NuGetGallery.Services/PackageManagement/IPackageService.cs

@@ -6,6 +6,7 @@
 using System.Threading.Tasks;
 using NuGet.Packaging;
 using NuGet.Services.Entities;
+using NuGet.Versioning;
 using NuGetGallery.Packaging;
 
 namespace NuGetGallery
@@ -155,5 +156,13 @@ Package FilterLatestPackageBySuffix(IReadOnlyCollection<Package> packages,
         /// <exception cref="ArgumentNullException">Thrown if <paramref name="registration" />
         /// is <c>null</c>.</exception>
         Task SetRequiredSignerAsync(PackageRegistration registration, User signer, bool commitChanges = true);
+
+        /// <summary>
+        /// Get a package's status, or <c>null</c> if the package does not exist.
+        /// </summary>
+        /// <param name="packageId">The package's ID.</param>
+        /// <param name="packageVersion">The package's version.</param>
+        /// <returns>The package's status, or <c>null</c> is the package does not exist.</returns>
+        PackageStatus? GetPackageStatus(string packageId, NuGetVersion packageVersion);
     }
 }

src/NuGetGallery.Services/PackageManagement/PackageService.cs

@@ -945,5 +945,20 @@ public async Task SetRequiredSignerAsync(PackageRegistration registration, User
                 _telemetryService.TrackRequiredSignerSet(registration.Id);
             }
         }
+
+        public PackageStatus? GetPackageStatus(string packageId, NuGetVersion packageVersion)
+        {
+            var normalizedVersion = packageVersion.ToNormalizedString();
+
+            // Note the casting to a nullable enum in the "Select". This is required to
+            // return "null" if there are no rows. Otherwise, "FirstOrDefault" would return
+            // "PackageStatus.Available" as the default value.
+            return _packageRepository
+                .GetAll()
+                .Where(p => p.PackageRegistration.Id == packageId)
+                .Where(p => p.Version == normalizedVersion)
+                .Select(p => (PackageStatus?)p.PackageStatusKey)
+                .FirstOrDefault();
+        }
     }
 }

src/NuGetGallery/Controllers/ApiController.cs

@@ -657,11 +657,17 @@ await AuditingService.SaveAuditRecordAsync(
                                         string.Format(CultureInfo.CurrentCulture, Strings.PackageIsLocked, packageRegistration.Id));
                                 }
 
-                                var existingPackage = PackageService.FindPackageByIdAndVersionStrict(id, version.ToStringSafe());
-                                if (existingPackage != null)
+                                // A package can only be reuploaded if it never passed validation.
+                                var existingStatus = PackageService.GetPackageStatus(id, version);
+                                if (existingStatus != null)
                                 {
-                                    if (existingPackage.PackageStatusKey == PackageStatus.FailedValidation)
+                                    if (existingStatus == PackageStatus.FailedValidation)
                                     {
+                                        // Allow this new upload to replace the existing package.
+                                        // We avoided loading the full package entity until now as it is
+                                        // a relatively expensive operation and this path is uncommon.
+                                        var existingPackage = PackageService.FindPackageByIdAndVersionStrict(id, version.ToStringSafe());
+
                                         TelemetryService.TrackPackageReupload(existingPackage);
 
                                         await PackageDeleteService.HardDeletePackagesAsync(

tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs

@@ -850,7 +850,13 @@ public async Task WillReturnConflictIfAPackageWithTheIdAndSameNormalizedVersionA
                 var controller = new TestableApiController(GetConfigurationService());
                 controller.SetCurrentUser(new User() { EmailAddress = "[email protected]" });
                 controller.MockPackageService.Setup(x => x.FindPackageRegistrationById(id)).Returns(packageRegistration);
-                controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersionStrict(id, version)).Returns(conflictingPackage);
+                controller
+                    .MockPackageService
+                    .Setup(x => x.GetPackageStatus(
+                        id,
+                        It.Is<NuGetVersion>(v => v.ToNormalizedString() == version)))
+                    .Returns(status);
+
                 controller.SetupPackageFromInputStream(nuGetPackage);
 
                 // Act
@@ -897,8 +903,16 @@ public async Task WillAllowReuploadingPackageIfFailedValidation()
 
                 var controller = new TestableApiController(GetConfigurationService());
                 controller.SetCurrentUser(currentUser);
+
                 controller.MockPackageService.Setup(x => x.FindPackageRegistrationById(id)).Returns(packageRegistration);
                 controller.MockPackageService.Setup(x => x.FindPackageByIdAndVersionStrict(id, version)).Returns(conflictingPackage);
+                controller
+                    .MockPackageService
+                    .Setup(x => x.GetPackageStatus(
+                        id,
+                        It.Is<NuGetVersion>(v => v.ToNormalizedString() == version)))
+                    .Returns(PackageStatus.FailedValidation);
+
                 controller.SetupPackageFromInputStream(nuGetPackage);
 
                 // Act

tests/NuGetGallery.FunctionalTests/PackageCreation/ApiPushTests.cs

@@ -35,9 +35,7 @@ public async Task DuplicatePushesAreRejectedAndNotDeleted()
             // Arrange
             var packageId = $"{nameof(DuplicatePushesAreRejectedAndNotDeleted)}.{Guid.NewGuid():N}";
 
-            // TODO: Increase this back to 10.
-            // See: https://github.com/NuGet/NuGetGallery/issues/8368
-            int pushVersionCount = 1;
+            int pushVersionCount = 10;
             var duplicatePushTasks = new List<Task>();
             for (var duplicateTaskIndex = 0; duplicateTaskIndex < pushVersionCount; duplicateTaskIndex++)
             {