Fix concurrency/consistency issues in Gallery API push

[shishirx34]

Fixes https://github.com/NuGet/Engineering/issues/1817. Mostly affects the functional test failures, kind of hard to test but I will deploy this change to dev and run functional tests couple of times to verify.

[joelverhagen]

        var package = packageRegistration.Packages.SingleOrDefault(pv => pv.Version == packageMetadata.Version.OriginalVersion);

I don't understand why we even need this line. Isn't this checked at the beginning of the control and again (by the DB) on commit?

Said another way, we already check it in memory here:

NuGetGallery/src/NuGetGallery/Controllers/ApiController.cs

Lines 599 to 602 in e582941

	return new HttpStatusCodeWithBodyResult(
	HttpStatusCode.Conflict,
	string.Format(CultureInfo.CurrentCulture, Strings.PackageExistsAndCannotBeModified,
	id, version.ToNormalizedStringSafe()));

And the DB check it with a unique constraint on (PackageRegistrationKey, NormalizedVersion) on the packages table. What does this one add?

---

Refers to: src/NuGetGallery/Services/PackageService.cs:478 in bee9def. [](commit_id = bee9def, deletion_comment = False)

[joelverhagen]

🕐

[shishirx34]

@joelverhagen - technically we do not check for the conflict on DB commit, rather in the CommitPackageAsync we check if the file exists in validation container in which case we return a Conflict. But even before that we queue up the validation into the pipeline(which eventually will deadletter or thrown away). If StartValidationAsync were to mark package as available then on DB commit unique constraint will still throw an EntityException which we could possibly handle(not handled in the code today) to return conflict(that could be another approach).

NuGetGallery/src/NuGetGallery/Services/PackageUploadService.cs

Line 352 in e582941

public async Task<PackageCommitResult> CommitPackageAsync(Package package, Stream packageFile)

I do not see any harm in handling it the current way in PR, given that it could avoid unnecessary queuing of validations and the check doesn't result in any performance issues(fail fast).

[shishirx34]

I added the concurrency and sql constraint violations check for completion.