Copy validation result .nupkg URL from processor (#373)

Progress on NuGet/Engineering#1190

Author: joelverhagen

src/NuGet.Services.Validation.Orchestrator/IValidationPackageFileService.cs

@@ -50,6 +50,14 @@ Task CopyValidationSetPackageToPackageFileAsync(
         /// <param name="normalizedVersion">The normalized package version.</param>
         Task CopyValidationPackageToPackageFileAsync(string id, string normalizedVersion);
 
+        /// <summary>
+        /// Copy a package URL to a location specific for the validation set.
+        /// </summary>
+        /// <param name="validationSet">The validation set.</param>
+        /// <param name="srcPackageUrl">The source package URL.</param>
+        /// <returns></returns>
+        Task CopyPackageUrlForValidationSetAsync(PackageValidationSet validationSet, string srcPackageUrl);
+
         /// <summary>
         /// Delete a package from a location specific for the validation set.
         /// </summary>

src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs

@@ -129,6 +129,23 @@ public Task<Uri> GetPackageForValidationSetReadUriAsync(PackageValidationSet val
             return _fileStorageService.GetFileReadUriAsync(CoreConstants.ValidationFolderName, fileName, endOfAccess);
         }
 
+        public Task CopyPackageUrlForValidationSetAsync(PackageValidationSet validationSet, string srcPackageUrl)
+        {
+            var destFileName = BuildValidationSetPackageFileName(validationSet);
+
+            _logger.LogInformation(
+                "Copying URL {SrcPackageUrl} to {DestFolderName}/{DestFileName}.",
+                srcPackageUrl,
+                CoreConstants.ValidationFolderName,
+                srcPackageUrl);
+
+            return _fileStorageService.CopyFileAsync(
+                new Uri(srcPackageUrl),
+                CoreConstants.ValidationFolderName,
+                destFileName,
+                AccessConditionWrapper.GenerateEmptyCondition());
+        }
+
         private Task<string> CopyFileAsync(
             string srcFolderName,
             string srcFileName,

src/NuGet.Services.Validation.Orchestrator/ValidationSetProcessor.cs

@@ -110,7 +110,7 @@ private async Task<bool> ProcessIncompleteValidations(PackageValidationSet valid
                     else
                     {
                         _logger.LogDebug(
-                            "Validation {ValidationType} for {PackageId} {PackageVersion} is still " +
+                            "Validation {ValidationType} for {PackageId} {PackageVersion} is already " +
                             "{ValidationStatus}, validation set {ValidationSetId}, {ValidationId}",
                             packageValidation.Type,
                             package.PackageRegistration.Id,

src/NuGet.Services.Validation.Orchestrator/ValidationStorageService.cs

@@ -13,20 +13,26 @@
 namespace NuGet.Services.Validation.Orchestrator
 {
     /// <summary>
-    /// Provides an access layer to the validation information stored in DB
+    /// Provides an access layer to the validation information stored in DB and blob storage.
     /// </summary>
     public class ValidationStorageService : IValidationStorageService
     {
         private readonly IValidationEntitiesContext _validationContext;
+        private readonly IValidationPackageFileService _packageFileService;
+        private readonly IValidatorProvider _validatorProvider;
         private readonly ITelemetryService _telemetryService;
         private readonly ILogger<ValidationStorageService> _logger;
 
         public ValidationStorageService(
             IValidationEntitiesContext validationContext,
+            IValidationPackageFileService packageFileService,
+            IValidatorProvider validatorProvider,
             ITelemetryService telemetryService,
             ILogger<ValidationStorageService> logger)
         {
             _validationContext = validationContext ?? throw new ArgumentNullException(nameof(validationContext));
+            _packageFileService = packageFileService ?? throw new ArgumentNullException(nameof(packageFileService));
+            _validatorProvider = validatorProvider ?? throw new ArgumentNullException(nameof(validatorProvider));
             _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
             _logger = logger ?? throw new ArgumentNullException(nameof(logger));
         }
@@ -58,12 +64,14 @@ public async Task<PackageValidationSet> CreateValidationSetAsync(PackageValidati
         public async Task MarkValidationStartedAsync(PackageValidation packageValidation, IValidationResult validationResult)
         {
             packageValidation = packageValidation ?? throw new ArgumentNullException(nameof(packageValidation));
+
             _logger.LogInformation("Marking validation {ValidationName} {ValidationId} {PackageId} {PackageVersion} as started with status {ValidationStatus}",
                 packageValidation.Type,
                 packageValidation.PackageValidationSet.ValidationTrackingId,
                 packageValidation.PackageValidationSet.PackageId,
                 packageValidation.PackageValidationSet.PackageNormalizedVersion,
                 validationResult.Status);
+
             if (validationResult.Status == ValidationStatus.NotStarted)
             {
                 throw new ArgumentOutOfRangeException(
@@ -74,20 +82,10 @@ public async Task MarkValidationStartedAsync(PackageValidation packageValidation
                     $"with status {ValidationStatus.NotStarted}");
             }
 
-            packageValidation.ValidationStatus = validationResult.Status;
-
-            // If the validation has completed, save the validation issues to the package's validation.
-            if (validationResult.Status != ValidationStatus.Incomplete)
-            {
-                AddValidationIssues(packageValidation, validationResult.Issues);
-            }
-
             var now = DateTime.UtcNow;
-            packageValidation.ValidationStatusTimestamp = now;
             packageValidation.Started = now;
-            await _validationContext.SaveChangesAsync();
 
-            TrackValidationStatus(packageValidation);
+            await SetValidationStatusAsync(packageValidation, validationResult, now);
         }
 
         public Task UpdateValidationSetAsync(PackageValidationSet packageValidationSet)
@@ -107,24 +105,56 @@ public Task UpdateValidationSetAsync(PackageValidationSet packageValidationSet)
         public async Task UpdateValidationStatusAsync(PackageValidation packageValidation, IValidationResult validationResult)
         {
             packageValidation = packageValidation ?? throw new ArgumentNullException(nameof(packageValidation));
+
+            if (packageValidation.ValidationStatus == validationResult.Status)
+            {
+                _logger.LogInformation("Validation {ValidationName} {ValidationId} {PackageId} {PackageVersion} already has status {ValidationStatus}",
+                    packageValidation.Type,
+                    packageValidation.PackageValidationSet.ValidationTrackingId,
+                    packageValidation.PackageValidationSet.PackageId,
+                    packageValidation.PackageValidationSet.PackageNormalizedVersion,
+                    validationResult.Status);
+
+                return;
+            }
+
             _logger.LogInformation("Updating the status of the validation {ValidationName} {ValidationId} {PackageId} {PackageVersion} to {ValidationStatus}",
                 packageValidation.Type,
                 packageValidation.PackageValidationSet.ValidationTrackingId,
                 packageValidation.PackageValidationSet.PackageId,
                 packageValidation.PackageValidationSet.PackageNormalizedVersion,
                 validationResult.Status);
 
-            if (packageValidation.ValidationStatus == validationResult.Status)
+            await SetValidationStatusAsync(packageValidation, validationResult, DateTime.UtcNow);
+        }
+
+        private async Task SetValidationStatusAsync(
+            PackageValidation packageValidation,
+            IValidationResult validationResult,
+            DateTime now)
+        {
+            if (validationResult.Status != ValidationStatus.Incomplete)
             {
-                return;
+                AddValidationIssues(packageValidation, validationResult.Issues);
             }
 
-            var previousValidationStatus = packageValidation.ValidationStatus;
+            if (validationResult.Status == ValidationStatus.Succeeded
+                && validationResult.NupkgUrl != null)
+            {
+                if (!_validatorProvider.IsProcessor(packageValidation.Type))
+                {
+                    throw new InvalidOperationException(
+                        $"The validator '{packageValidation.Type}' is not a processor but returned a .nupkg URL as " +
+                        $"part of the validation result.");
+                }
 
-            AddValidationIssues(packageValidation, validationResult.Issues);
+                await _packageFileService.CopyPackageUrlForValidationSetAsync(
+                    packageValidation.PackageValidationSet,
+                    validationResult.NupkgUrl);
+            }
 
             packageValidation.ValidationStatus = validationResult.Status;
-            packageValidation.ValidationStatusTimestamp = DateTime.UtcNow;
+            packageValidation.ValidationStatusTimestamp = now;
             await _validationContext.SaveChangesAsync();
 
             TrackValidationStatus(packageValidation);

src/Validation.Common.Job/Validation.Common.Job.csproj

@@ -84,7 +84,7 @@
       <Version>2.18.0</Version>
     </PackageReference>
     <PackageReference Include="NuGetGallery.Core">
-      <Version>4.4.4-dev-25609</Version>
+      <Version>4.4.4-dev-25804</Version>
     </PackageReference>
     <PackageReference Include="Serilog">
       <Version>2.5.0</Version>

tests/NuGet.Services.Validation.Orchestrator.Tests/ValidationPackageFileServiceFacts.cs

@@ -176,6 +176,23 @@ public async Task CopyValidationSetPackageToPackageFileAsync()
             _fileStorageService.Verify();
         }
 
+        [Fact]
+        public async Task CopyPackageUrlForValidationSetAsync()
+        {
+            _fileStorageService
+                .Setup(x => x.CopyFileAsync(
+                    _testUri,
+                    _validationContainerName,
+                    _validationSetPackageFileName,
+                    It.Is<IAccessCondition>(y => y.IfMatchETag == null && y.IfNoneMatchETag == null)))
+                .Returns(Task.CompletedTask)
+                .Verifiable();
+
+            await _target.CopyPackageUrlForValidationSetAsync(_validationSet, _testUri.AbsoluteUri);
+
+            _fileStorageService.Verify();
+        }
+
         [Fact]
         public async Task DeletePackageForValidationSetAsync()
         {