Make PackageSigningValidator into an IProcessor (#389)

Progress on NuGet/Engineering#1207

Author: joelverhagen

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

@@ -23,6 +23,7 @@
 using NuGet.Jobs.Validation.Common;
 using NuGet.Jobs.Validation.PackageSigning.Messages;
 using NuGet.Jobs.Validation.PackageSigning.Storage;
+using NuGet.Jobs.Validation.Storage;
 using NuGet.Services.Configuration;
 using NuGet.Services.KeyVault;
 using NuGet.Services.Logging;
@@ -203,6 +204,7 @@ private void ConfigureJobServices(IServiceCollection services, IConfigurationRoo
             services.AddTransient<IBrokeredMessageSerializer<SignatureValidationMessage>, SignatureValidationMessageSerializer>();
             services.AddTransient<IBrokeredMessageSerializer<CertificateValidationMessage>, CertificateValidationMessageSerializer>();
             services.AddTransient<IValidatorStateService, ValidatorStateService>();
+            services.AddTransient<ISimpleCloudBlobProvider, SimpleCloudBlobProvider>();
             services.AddTransient<PackageSigningValidator>();
             services.AddTransient<MailSenderConfiguration>(serviceProvider =>
             {

src/NuGet.Services.Validation.Orchestrator/PackageSigning/ProcessSignature/PackageSigningValidator.cs

@@ -6,26 +6,29 @@
 using System.Threading.Tasks;
 using Microsoft.Extensions.Logging;
 using NuGet.Jobs.Validation.PackageSigning.Storage;
-using NuGet.Services.Validation.Orchestrator;
+using NuGet.Jobs.Validation.Storage;
 using NuGet.Services.Validation.Orchestrator.Telemetry;
 
 namespace NuGet.Services.Validation.PackageSigning.ProcessSignature
 {
-    public class PackageSigningValidator : BaseValidator, IValidator
+    public class PackageSigningValidator : IProcessor
     {
         private readonly IValidatorStateService _validatorStateService;
         private readonly IProcessSignatureEnqueuer _signatureVerificationEnqueuer;
+        private readonly ISimpleCloudBlobProvider _blobProvider;
         private readonly ITelemetryService _telemetryService;
         private readonly ILogger<PackageSigningValidator> _logger;
 
         public PackageSigningValidator(
             IValidatorStateService validatorStateService,
             IProcessSignatureEnqueuer signatureVerificationEnqueuer,
+            ISimpleCloudBlobProvider blobProvider,
             ITelemetryService telemetryService,
             ILogger<PackageSigningValidator> logger)
         {
             _validatorStateService = validatorStateService ?? throw new ArgumentNullException(nameof(validatorStateService));
             _signatureVerificationEnqueuer = signatureVerificationEnqueuer ?? throw new ArgumentNullException(nameof(signatureVerificationEnqueuer));
+            _blobProvider = blobProvider ?? throw new ArgumentNullException(nameof(blobProvider));
             _telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService));
             _logger = logger ?? throw new ArgumentNullException(nameof(logger));
         }
@@ -44,6 +47,25 @@ public async Task<IValidationResult> StartAsync(IValidationRequest request)
             return validatorStatus.ToValidationResult();
         }
 
+        public async Task CleanUpAsync(IValidationRequest request)
+        {
+            var validatorStatus = await _validatorStateService.GetStatusAsync(request);
+
+            if (validatorStatus.NupkgUrl == null)
+            {
+                return;
+            }
+
+            _logger.LogInformation(
+                "Cleaning up the .nupkg URL for validation ID {ValidationId} ({PackageId} {PackageVersion}).",
+                request.ValidationId,
+                request.PackageId,
+                request.PackageVersion);
+
+            var blob = _blobProvider.GetBlobFromUrl(validatorStatus.NupkgUrl);
+            await blob.DeleteIfExistsAsync();
+        }
+
         private async Task<ValidatorStatus> StartInternalAsync(IValidationRequest request)
         {
             // Check that this is the first validation for this specific request.

src/NuGet.Services.Validation.Orchestrator/settings.json

@@ -4,7 +4,7 @@
       {
         "name": "VcsValidator",
         "TrackAfter": "1:00:00:00",
-        "requiredValidations": [],
+        "requiredValidations": [ "PackageSigningValidator" ],
         "ShouldStart": true,
         "FailureBehavior": "MustSucceed"
       },

src/Validation.PackageSigning.ProcessSignature/Validation.PackageSigning.ProcessSignature.csproj

@@ -76,10 +76,6 @@
       <Project>{fa87d075-a934-4443-8d0b-5db32640b6d7}</Project>
       <Name>Validation.Common.Job</Name>
     </ProjectReference>
-    <ProjectReference Include="..\Validation.Common\Validation.Common.csproj">
-      <Project>{2539DDF3-0CC5-4A03-B5F9-39B47744A7BD}</Project>
-      <Name>Validation.Common</Name>
-    </ProjectReference>
     <ProjectReference Include="..\Validation.PackageSigning.Core\Validation.PackageSigning.Core.csproj">
       <Project>{91c060da-736f-4da9-a57f-cb3ac0e6cb10}</Project>
       <Name>Validation.PackageSigning.Core</Name>

tests/NuGet.Services.Validation.Orchestrator.Tests/PackageSigning/ProcessSignature/PackageSigningValidatorFacts.cs

@@ -8,9 +8,12 @@
 using Microsoft.Extensions.Logging;
 using Moq;
 using NuGet.Jobs.Validation.PackageSigning.Storage;
+using NuGet.Jobs.Validation.Storage;
 using NuGet.Services.Validation.Orchestrator.Telemetry;
 using NuGet.Services.Validation.PackageSigning.ProcessSignature;
+using NuGetGallery;
 using Xunit;
+using Xunit.Abstractions;
 
 namespace NuGet.Services.Validation.PackageSigning
 {
@@ -32,6 +35,10 @@ public class TheGetStatusMethod : FactsBase
                 ValidationStatus.Succeeded,
             };
 
+            public TheGetStatusMethod(ITestOutputHelper output) : base(output)
+            {
+            }
+
             [Theory]
             [MemberData(nameof(PossibleValidationStatuses))]
             public async Task ReturnsPersistedStatus(ValidationStatus status)
@@ -107,6 +114,10 @@ public class TheStartValidationAsyncMethod : FactsBase
                 ValidationStatus.Succeeded,
             };
 
+            public TheStartValidationAsyncMethod(ITestOutputHelper output) : base(output)
+            {
+            }
+
             [Theory]
             [MemberData(nameof(StartedValidationStatuses))]
             public async Task ReturnsPersistedStatusesIfValidationAlreadyStarted(ValidationStatus status)
@@ -204,21 +215,68 @@ public async Task StartsValidationIfNotStarted()
             public static IEnumerable<object[]> StartedValidationStatuses => startedValidationStatuses.Select(s => new object[] { s });
         }
 
+        public class TheCleanUpAsyncMethod : FactsBase
+        {
+            private readonly ValidatorStatus _validatorStatus;
+            private readonly Mock<ISimpleCloudBlob> _blob;
+
+            public TheCleanUpAsyncMethod(ITestOutputHelper output) : base(output)
+            {
+                _validatorStatus = new ValidatorStatus();
+                _validatorStateService
+                    .Setup(x => x.GetStatusAsync(It.IsAny<IValidationRequest>()))
+                    .ReturnsAsync(() => _validatorStatus);
+
+                _blob = new Mock<ISimpleCloudBlob>(MockBehavior.Strict);
+                _blobProvider
+                    .Setup(x => x.GetBlobFromUrl(It.IsAny<string>()))
+                    .Returns(() => _blob.Object);
+            }
+
+            [Fact]
+            public async Task DeletesNothingWhenThereIsNoNupkgUrl()
+            {
+                await _target.CleanUpAsync(_validationRequest.Object);
+
+                _validatorStateService.Verify(x => x.GetStatusAsync(_validationRequest.Object), Times.Once);
+                _blobProvider.Verify(x => x.GetBlobFromUrl (It.IsAny<string>()), Times.Never);
+            }
+
+            [Fact]
+            public async Task DeletesTheBlobWhenThereIsNupkgUrl()
+            {
+                var nupkgUrl = "http://example/packages/nuget.versioning.4.6.0.nupkg";
+                _validatorStatus.NupkgUrl = nupkgUrl;
+                _blob
+                    .Setup(x => x.DeleteIfExistsAsync())
+                    .Returns(Task.CompletedTask);
+
+                await _target.CleanUpAsync(_validationRequest.Object);
+
+                _validatorStateService.Verify(x => x.GetStatusAsync(_validationRequest.Object), Times.Once);
+                _blobProvider.Verify(x => x.GetBlobFromUrl(nupkgUrl), Times.Once);
+                _blob.Verify(x => x.DeleteIfExistsAsync(), Times.Once);
+            }
+        }
+
         public abstract class FactsBase
         {
             protected readonly Mock<IValidatorStateService> _validatorStateService;
             protected readonly Mock<IProcessSignatureEnqueuer> _packageSignatureVerifier;
+            protected readonly Mock<ISimpleCloudBlobProvider> _blobProvider;
             protected readonly Mock<ITelemetryService> _telemetryService;
-            protected readonly Mock<ILogger<PackageSigningValidator>> _logger;
+            protected readonly ILogger<PackageSigningValidator> _logger;
             protected readonly Mock<IValidationRequest> _validationRequest;
             protected readonly PackageSigningValidator _target;
 
-            public FactsBase()
+            public FactsBase(ITestOutputHelper output)
             {
                 _validatorStateService = new Mock<IValidatorStateService>();
                 _packageSignatureVerifier = new Mock<IProcessSignatureEnqueuer>();
+                _blobProvider = new Mock<ISimpleCloudBlobProvider>();
                 _telemetryService = new Mock<ITelemetryService>();
-                _logger = new Mock<ILogger<PackageSigningValidator>>();
+                var loggerFactory = new LoggerFactory().AddXunit(output);
+                _logger = loggerFactory.CreateLogger<PackageSigningValidator>();
 
                 _validationRequest = new Mock<IValidationRequest>();
                 _validationRequest.Setup(x => x.NupkgUrl).Returns(NupkgUrl);
@@ -228,10 +286,11 @@ public FactsBase()
                 _validationRequest.Setup(x => x.ValidationId).Returns(ValidationId);
 
                 _target = new PackageSigningValidator(
-                        _validatorStateService.Object,
-                        _packageSignatureVerifier.Object,
-                        _telemetryService.Object,
-                        _logger.Object);
+                    _validatorStateService.Object,
+                    _packageSignatureVerifier.Object,
+                    _blobProvider.Object,
+                    _telemetryService.Object,
+                    _logger);
             }
         }
     }

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

@@ -85,7 +85,7 @@ public class GetValidator : BaseFacts
             /// </summary>
             [Theory]
             [InlineData("VcsValidator", false)]
-            [InlineData("PackageSigningValidator", false)]
+            [InlineData("PackageSigningValidator", true)]
             [InlineData("PackageCertificatesValidator", false)]
             public void KnownValidatorsDoNotChangeNames(string validatorName, bool isProcessor)
             {