From 79c810726f7580c3938aaae1d72a4e52865e91a3 Mon Sep 17 00:00:00 2001 From: cmanu Date: Wed, 24 Oct 2018 11:13:25 -0700 Subject: [PATCH] Integrate public symbols etag. (#592) Add the etag support for symbols and update the email services to align with the gallery changes. --- .../CoreMessageServiceConfiguration.cs | 4 +- .../IValidationPackageFileService.cs | 8 + .../Job.cs | 10 +- ...et.Services.Validation.Orchestrator.csproj | 2 +- .../PackageStatusProcessor.cs | 14 +- .../Services/MessageServiceConfiguration.cs | 15 +- .../Services/PackageMessageService.cs | 38 ++- .../Services/SymbolsMessageService.cs | 36 ++- .../SymbolsStatusProcessor.cs | 73 ++++++ .../ValidationPackageFileService.cs | 9 + .../ValidationSetProvider.cs | 16 +- .../ValidationSymbolFileService.cs | 55 ---- .../Validation.Common.Job.csproj | 2 +- .../Validation.Common.Job.nuspec | 2 +- .../ISymbolsValidatorService.cs | 2 +- src/Validation.Symbols/ITelemetryService.cs | 2 +- ...vices.Validation.Orchestrator.Tests.csproj | 1 + .../Services/MessageServiceFacts.cs | 25 +- .../Services/PackageEntityServiceFacts.cs | 2 +- .../Services/SymbolsMessageServiceFacts.cs | 27 +- .../Symbol/SymbolsStatusProcessorFacts.cs | 234 ++++++++++++++++++ 21 files changed, 457 insertions(+), 120 deletions(-) create mode 100644 src/NuGet.Services.Validation.Orchestrator/SymbolsStatusProcessor.cs delete mode 100644 src/NuGet.Services.Validation.Orchestrator/ValidationSymbolFileService.cs create mode 100644 tests/NuGet.Services.Validation.Orchestrator.Tests/Symbol/SymbolsStatusProcessorFacts.cs diff --git a/src/NuGet.Services.Validation.Orchestrator/Configuration/CoreMessageServiceConfiguration.cs b/src/NuGet.Services.Validation.Orchestrator/Configuration/CoreMessageServiceConfiguration.cs index b3d90e78a..84953d9a3 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Configuration/CoreMessageServiceConfiguration.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Configuration/CoreMessageServiceConfiguration.cs @@ -4,11 +4,11 @@ using System; using System.Net.Mail; using Microsoft.Extensions.Options; -using NuGetGallery.Services; +using NuGetGallery.Infrastructure.Mail; namespace NuGet.Services.Validation.Orchestrator { - public class CoreMessageServiceConfiguration : ICoreMessageServiceConfiguration + public class CoreMessageServiceConfiguration : IMessageServiceConfiguration { public CoreMessageServiceConfiguration(IOptionsSnapshot emailConfigurationAccessor) { diff --git a/src/NuGet.Services.Validation.Orchestrator/IValidationPackageFileService.cs b/src/NuGet.Services.Validation.Orchestrator/IValidationPackageFileService.cs index e6137c4e2..c636a2e7d 100644 --- a/src/NuGet.Services.Validation.Orchestrator/IValidationPackageFileService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/IValidationPackageFileService.cs @@ -123,5 +123,13 @@ Task CopyValidationSetPackageToPackageFileAsync( /// Thrown if the blob has changed between /// successive read and write operations. Task UpdatePackageBlobMetadataAsync(PackageValidationSet validationSet); + + /// + /// Reads the ETag for the package in the public container. + /// + /// A validationSet that will identify the package that will have its blob metadata updated. + /// A task that represents the asynchronous operation. + /// The result is the etag of the package blob or null if the package does not exists. + Task GetPublicPackageBlobETagOrNullAsync(PackageValidationSet validationSet); } } \ No newline at end of file diff --git a/src/NuGet.Services.Validation.Orchestrator/Job.cs b/src/NuGet.Services.Validation.Orchestrator/Job.cs index 919d2ec0c..f8c38ada8 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Job.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Job.cs @@ -40,7 +40,7 @@ using NuGet.Services.Validation.PackageSigning.ValidateCertificate; using NuGet.Services.Validation.Vcs; using NuGetGallery.Diagnostics; -using NuGetGallery.Services; +using NuGetGallery.Infrastructure.Mail; namespace NuGet.Services.Validation.Orchestrator { @@ -280,8 +280,8 @@ private void ConfigureJobServices(IServiceCollection services, IConfigurationRoo ? (IMailSender)new DiskMailSender() : (IMailSender)new MailSender(mailSenderConfiguration); }); - services.AddTransient(); - services.AddTransient(); + services.AddTransient(); + services.AddTransient(); services.AddTransient, PackageMessageService>(); services.AddTransient(); services.AddTransient(); @@ -581,7 +581,7 @@ private static void ConfigureFileServices(IServiceCollection services, IConfigur break; case ValidatingType.SymbolPackage: services.AddTransient(); - services.AddTransient(); + services.AddTransient(); break; default: throw new NotImplementedException($"Unknown type: {validatingType}"); @@ -595,7 +595,7 @@ private static void ConfigureOrchestratorSymbolTypes(IServiceCollection services services.AddTransient(); services.AddTransient, SymbolCriteriaEvaluator>(); services.AddTransient, ValidationOutcomeProcessor>(); - services.AddTransient, EntityStatusProcessor>(); + services.AddTransient, SymbolsStatusProcessor>(); services.AddTransient, ValidationSetProvider>(); services.AddTransient, SymbolsPackageMessageService>(); services.AddTransient, SymbolsValidatorMessageSerializer>(); diff --git a/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj b/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj index 39e1a907e..83eecdeb0 100644 --- a/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj +++ b/src/NuGet.Services.Validation.Orchestrator/NuGet.Services.Validation.Orchestrator.csproj @@ -53,6 +53,7 @@ + @@ -107,7 +108,6 @@ - diff --git a/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs index 944480749..40092f252 100644 --- a/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs +++ b/src/NuGet.Services.Validation.Orchestrator/PackageStatusProcessor.cs @@ -13,11 +13,11 @@ namespace NuGet.Services.Validation.Orchestrator { public class EntityStatusProcessor : IStatusProcessor where T : class, IEntity { - private readonly IEntityService _galleryPackageService; - private readonly IValidationFileService _packageFileService; - private readonly IValidatorProvider _validatorProvider; - private readonly ITelemetryService _telemetryService; - private readonly ILogger> _logger; + protected readonly IEntityService _galleryPackageService; + protected readonly IValidationFileService _packageFileService; + protected readonly IValidatorProvider _validatorProvider; + protected readonly ITelemetryService _telemetryService; + protected readonly ILogger> _logger; public EntityStatusProcessor( IEntityService galleryPackageService, @@ -76,7 +76,7 @@ public Task SetStatusAsync( } } - private async Task MakePackageFailedValidationAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + protected virtual async Task MakePackageFailedValidationAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) { var fromStatus = validatingEntity.Status; @@ -88,7 +88,7 @@ private async Task MakePackageFailedValidationAsync(IValidatingEntity validat } } - private async Task MakePackageAvailableAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + protected virtual async Task MakePackageAvailableAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) { // 1) Operate on blob storage. var copied = await UpdatePublicPackageAsync(validationSet); diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/MessageServiceConfiguration.cs b/src/NuGet.Services.Validation.Orchestrator/Services/MessageServiceConfiguration.cs index 72e1ead35..fc342701d 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Services/MessageServiceConfiguration.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Services/MessageServiceConfiguration.cs @@ -2,11 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Net.Mail; using Microsoft.Extensions.Options; +using NuGetGallery.Infrastructure.Mail; namespace NuGet.Services.Validation.Orchestrator { - public class MessageServiceConfiguration + public class MessageServiceConfiguration : IMessageServiceConfiguration { public EmailConfiguration EmailConfiguration { get; } @@ -17,6 +19,7 @@ public MessageServiceConfiguration(IOptionsSnapshot emailCon throw new ArgumentNullException(nameof(emailConfigurationAccessor)); } EmailConfiguration = emailConfigurationAccessor.Value ?? throw new ArgumentException("Value cannot be null", nameof(emailConfigurationAccessor)); + if (string.IsNullOrWhiteSpace(EmailConfiguration.PackageUrlTemplate)) { throw new ArgumentException($"{nameof(emailConfigurationAccessor.Value)}.{nameof(EmailConfiguration.PackageUrlTemplate)} cannot be empty", nameof(emailConfigurationAccessor)); @@ -33,9 +36,19 @@ public MessageServiceConfiguration(IOptionsSnapshot emailCon { throw new ArgumentException($"{nameof(emailConfigurationAccessor.Value)}.{nameof(EmailConfiguration.EmailSettingsUrl)} must be an absolute Url", nameof(emailConfigurationAccessor)); } + + GalleryOwner = new MailAddress(EmailConfiguration.GalleryOwner); + GalleryNoReplyAddress = new MailAddress(EmailConfiguration.GalleryNoReplyAddress); } public string GalleryPackageUrl(string packageId, string packageNormalizedVersion) => string.Format(EmailConfiguration.PackageUrlTemplate, packageId, packageNormalizedVersion); public string PackageSupportUrl(string packageId, string packageNormalizedVersion) => string.Format(EmailConfiguration.PackageSupportTemplate, packageId, packageNormalizedVersion); + + public MailAddress GalleryOwner { get; set; } + + /// + /// Gets the gallery e-mail from name and email address + /// + public MailAddress GalleryNoReplyAddress { get; set; } } } diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/PackageMessageService.cs b/src/NuGet.Services.Validation.Orchestrator/Services/PackageMessageService.cs index eaa6c8716..152c640f1 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Services/PackageMessageService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Services/PackageMessageService.cs @@ -6,23 +6,24 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using NuGetGallery; -using NuGetGallery.Services; +using NuGetGallery.Infrastructure.Mail; +using NuGetGallery.Infrastructure.Mail.Messages; namespace NuGet.Services.Validation.Orchestrator { public class PackageMessageService : IMessageService { - private readonly ICoreMessageService _coreMessageService; + private readonly IMessageService _messageService; private readonly ILogger _logger; private readonly MessageServiceConfiguration _serviceConfiguration; public PackageMessageService( - ICoreMessageService coreMessageService, + IMessageService messageService, IOptionsSnapshot emailConfigurationAccessor, ILogger logger) { _serviceConfiguration = new MessageServiceConfiguration(emailConfigurationAccessor); - _coreMessageService = coreMessageService ?? throw new ArgumentNullException(nameof(coreMessageService)); + _messageService = messageService ?? throw new ArgumentNullException(nameof(messageService)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -32,8 +33,15 @@ public async Task SendPublishedMessageAsync(Package package) var galleryPackageUrl = _serviceConfiguration.GalleryPackageUrl(package.PackageRegistration.Id, package.NormalizedVersion); var packageSupportUrl = _serviceConfiguration.PackageSupportUrl(package.PackageRegistration.Id, package.NormalizedVersion); - - await _coreMessageService.SendPackageAddedNoticeAsync(package, galleryPackageUrl, packageSupportUrl, _serviceConfiguration.EmailConfiguration.EmailSettingsUrl); + var packageAddedMessage = new PackageAddedMessage( + _serviceConfiguration, + package, + galleryPackageUrl, + packageSupportUrl, + _serviceConfiguration.EmailConfiguration.EmailSettingsUrl, + Array.Empty()); + + await _messageService.SendMessageAsync(packageAddedMessage); } public async Task SendValidationFailedMessageAsync(Package package, PackageValidationSet validationSet) @@ -44,14 +52,28 @@ public async Task SendValidationFailedMessageAsync(Package package, PackageValid var galleryPackageUrl = _serviceConfiguration.GalleryPackageUrl(package.PackageRegistration.Id, package.NormalizedVersion); var packageSupportUrl = _serviceConfiguration.PackageSupportUrl(package.PackageRegistration.Id, package.NormalizedVersion); - await _coreMessageService.SendPackageValidationFailedNoticeAsync(package, validationSet, galleryPackageUrl, packageSupportUrl, _serviceConfiguration.EmailConfiguration.AnnouncementsUrl, _serviceConfiguration.EmailConfiguration.TwitterUrl); + var packageValidationFailedMessage = new PackageValidationFailedMessage( + _serviceConfiguration, + package, + validationSet, + galleryPackageUrl, + packageSupportUrl, + _serviceConfiguration.EmailConfiguration.AnnouncementsUrl, + _serviceConfiguration.EmailConfiguration.TwitterUrl); + + await _messageService.SendMessageAsync(packageValidationFailedMessage); } public async Task SendValidationTakingTooLongMessageAsync(Package package) { package = package ?? throw new ArgumentNullException(nameof(package)); - await _coreMessageService.SendValidationTakingTooLongNoticeAsync(package, _serviceConfiguration.GalleryPackageUrl(package.PackageRegistration.Id, package.NormalizedVersion)); + var packageValidationTakingTooLongMessage = new PackageValidationTakingTooLongMessage( + _serviceConfiguration, + package, + _serviceConfiguration.GalleryPackageUrl(package.PackageRegistration.Id, package.NormalizedVersion)); + + await _messageService.SendMessageAsync(packageValidationTakingTooLongMessage); } } } diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/SymbolsMessageService.cs b/src/NuGet.Services.Validation.Orchestrator/Services/SymbolsMessageService.cs index 57d74ea44..de57ff76a 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Services/SymbolsMessageService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Services/SymbolsMessageService.cs @@ -6,23 +6,24 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using NuGetGallery; -using NuGetGallery.Services; +using NuGetGallery.Infrastructure.Mail; +using NuGetGallery.Infrastructure.Mail.Messages; namespace NuGet.Services.Validation.Orchestrator { public class SymbolsPackageMessageService : IMessageService { - private readonly ICoreMessageService _coreMessageService; + private readonly IMessageService _messageService; private readonly ILogger _logger; private readonly MessageServiceConfiguration _serviceConfiguration; public SymbolsPackageMessageService( - ICoreMessageService coreMessageService, + IMessageService messageService, IOptionsSnapshot emailConfigurationAccessor, ILogger logger) { _serviceConfiguration = new MessageServiceConfiguration(emailConfigurationAccessor); - _coreMessageService = coreMessageService ?? throw new ArgumentNullException(nameof(coreMessageService)); + _messageService = messageService ?? throw new ArgumentNullException(nameof(messageService)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } @@ -35,8 +36,14 @@ public async Task SendPublishedMessageAsync(SymbolPackage symbolPackage) var galleryPackageUrl = _serviceConfiguration.GalleryPackageUrl(symbolPackage.Id, symbolPackage.Package.NormalizedVersion); var packageSupportUrl = _serviceConfiguration.PackageSupportUrl(symbolPackage.Id, symbolPackage.Package.NormalizedVersion); - - await _coreMessageService.SendSymbolPackageAddedNoticeAsync(symbolPackage, galleryPackageUrl, packageSupportUrl, _serviceConfiguration.EmailConfiguration.EmailSettingsUrl); + var symbolPackageAddedMessage = new SymbolPackageAddedMessage( + _serviceConfiguration, + symbolPackage, + galleryPackageUrl, + packageSupportUrl, + _serviceConfiguration.EmailConfiguration.EmailSettingsUrl, + Array.Empty()); + await _messageService.SendMessageAsync(symbolPackageAddedMessage); } public async Task SendValidationFailedMessageAsync(SymbolPackage symbolPackage, PackageValidationSet validationSet) @@ -50,7 +57,16 @@ public async Task SendValidationFailedMessageAsync(SymbolPackage symbolPackage, var galleryPackageUrl = _serviceConfiguration.GalleryPackageUrl(symbolPackage.Id, symbolPackage.Package.NormalizedVersion); var packageSupportUrl = _serviceConfiguration.PackageSupportUrl(symbolPackage.Id, symbolPackage.Package.NormalizedVersion); - await _coreMessageService.SendSymbolPackageValidationFailedNoticeAsync(symbolPackage, validationSet, galleryPackageUrl, packageSupportUrl, _serviceConfiguration.EmailConfiguration.AnnouncementsUrl, _serviceConfiguration.EmailConfiguration.TwitterUrl); + var symbolPackageValidationFailedMessage = new SymbolPackageValidationFailedMessage( + _serviceConfiguration, + symbolPackage, + validationSet, + galleryPackageUrl, + packageSupportUrl, + _serviceConfiguration.EmailConfiguration.AnnouncementsUrl, + _serviceConfiguration.EmailConfiguration.TwitterUrl); + + await _messageService.SendMessageAsync(symbolPackageValidationFailedMessage); } public async Task SendValidationTakingTooLongMessageAsync(SymbolPackage symbolPackage) @@ -59,8 +75,12 @@ public async Task SendValidationTakingTooLongMessageAsync(SymbolPackage symbolPa { throw new ArgumentNullException(nameof(symbolPackage)); } + var symbolPackageValidationTakingTooLongMessage = new SymbolPackageValidationTakingTooLongMessage( + _serviceConfiguration, + symbolPackage, + _serviceConfiguration.GalleryPackageUrl(symbolPackage.Package.PackageRegistration.Id, symbolPackage.Package.NormalizedVersion)); - await _coreMessageService.SendValidationTakingTooLongNoticeAsync(symbolPackage, _serviceConfiguration.GalleryPackageUrl(symbolPackage.Id, symbolPackage.Package.NormalizedVersion)); + await _messageService.SendMessageAsync(symbolPackageValidationTakingTooLongMessage); } } } diff --git a/src/NuGet.Services.Validation.Orchestrator/SymbolsStatusProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/SymbolsStatusProcessor.cs new file mode 100644 index 000000000..ee1485304 --- /dev/null +++ b/src/NuGet.Services.Validation.Orchestrator/SymbolsStatusProcessor.cs @@ -0,0 +1,73 @@ +// 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.Threading.Tasks; +using Microsoft.Extensions.Logging; +using NuGet.Services.Validation.Orchestrator.Telemetry; +using NuGetGallery; + +namespace NuGet.Services.Validation.Orchestrator +{ + public class SymbolsStatusProcessor : EntityStatusProcessor + { + public SymbolsStatusProcessor( + IEntityService galleryPackageService, + IValidationFileService packageFileService, + IValidatorProvider validatorProvider, + ITelemetryService telemetryService, + ILogger> logger) + : base(galleryPackageService, packageFileService, validatorProvider, telemetryService, logger) + { + } + + protected override async Task MakePackageAvailableAsync(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + { + if(!CanProceedToMakePackageAvailable(validatingEntity, validationSet)) + { + _logger.LogInformation("SymbolsPackage PackageId {PackageId} PackageVersion {PackageVersion} Status {Status} was not made available again.", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validatingEntity.Status); + return; + } + await base.MakePackageAvailableAsync(validatingEntity, validationSet); + } + + /// + /// Proceed to change the state only if: + /// 1.the current symbol entity is in a failed state and there is not an existent symbol push already started by the user. This state can happen on revalidation only. + /// or + /// 2. the current validation is in validating state + /// If the symbols validation would have processors as validators the copy should be done on other states as well. + /// + /// The that is under current revalidation. + /// The validation set for the current validation. + /// True if the package should be made available (copied to the public container, db updated etc.) + public bool CanProceedToMakePackageAvailable(IValidatingEntity validatingEntity, PackageValidationSet validationSet) + { + // _galleryPackageService.FindPackageByIdAndVersionStrict will return the symbol entity that is in Validating state or null if + // there not any symbols entity in validating state. + var validatingSymbolsEntity = _galleryPackageService.FindPackageByIdAndVersionStrict(validationSet.PackageId, validationSet.PackageNormalizedVersion); + + // If the current entity is in validating mode a new symbolPush is not allowed, so it is safe to copy. + var aNewEntityInValidatingStateExists = validatingSymbolsEntity != null; + + var proceed = validatingEntity.Status == PackageStatus.Validating || (!aNewEntityInValidatingStateExists && validatingEntity.Status == PackageStatus.FailedValidation); + _logger.LogInformation("Proceed to make symbols available check: " + + "PackageId {PackageId} " + + "PackageVersion {PackageVersion} " + + "ValidationTrackingId {ValidationTrackingId} " + + "CurrentValidating entity status {CurrentEntityStatus}" + + "ANewEntityInValidatingStateExists {ANewEntityInValidatingStateExists}" + + "Proceed {Proceed}", + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId, + validatingEntity.Status, + aNewEntityInValidatingStateExists, + proceed + ); + return proceed; + } + } +} diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs index e5f75f54f..f6b916d8b 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/ValidationPackageFileService.cs @@ -266,6 +266,15 @@ await _fileStorageService.SetMetadataAsync( return streamMetadata; } + public async Task GetPublicPackageBlobETagOrNullAsync(PackageValidationSet validationSet) + { + var fileName = BuildFileName(validationSet, + _fileMetadataService.FileSavePathTemplate, + _fileMetadataService.FileExtension); + + return await _fileStorageService.GetETagOrNullAsync(_fileMetadataService.FileFolderName, fileName); + } + private Task CopyFileAsync( string srcFolderName, string srcFileName, diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationSetProvider.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationSetProvider.cs index cae24512a..12b00bcbd 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationSetProvider.cs +++ b/src/NuGet.Services.Validation.Orchestrator/ValidationSetProvider.cs @@ -69,9 +69,19 @@ public async Task TryGetOrCreateValidationSetAsync(Package { await _packageFileService.CopyValidationPackageForValidationSetAsync(validationSet); - // This indicates that the package in the packages container is expected to not exist (i.e. it has - // has no etag at all). - validationSet.PackageETag = null; + // A symbols package for the same id and version can be re-submitted. + // When this happens a new validation is submitted. After validation the new symbols package will overwrite the old symbols package. + // Because of this when a new validation for a symbols package is received it can already exist a symbols package in the public symbols container. + if (validatingEntity.ValidatingType == ValidatingType.SymbolPackage) + { + validationSet.PackageETag = await _packageFileService.GetPublicPackageBlobETagOrNullAsync(validationSet); + } + else + { + // This indicates that the package in the packages container is expected to not exist (i.e. it has + // has no etag at all). + validationSet.PackageETag = null; + } } // If there are any processors in the validation set, back up the original. We back up from the diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationSymbolFileService.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationSymbolFileService.cs deleted file mode 100644 index 6de6863fd..000000000 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationSymbolFileService.cs +++ /dev/null @@ -1,55 +0,0 @@ -// 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; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using NuGet.Jobs.Validation; -using NuGet.Services.Validation.Orchestrator.Telemetry; -using NuGetGallery; - -namespace NuGet.Services.Validation.Orchestrator -{ - /// - /// The save operations for symbols need to allow overwrite. - /// Extend the ValidationFileService and overwrite the Copy methods. - /// - public class ValidationSymbolFileService : ValidationFileService - { - public ValidationSymbolFileService( - ICoreFileStorageService fileStorageService, - IFileDownloader fileDownloader, - ITelemetryService telemetryService, - ILogger logger, - IFileMetadataService fileMetadataService) : base(fileStorageService, fileDownloader, telemetryService, logger, fileMetadataService) - { - } - - public override async Task CopyValidationPackageToPackageFileAsync(PackageValidationSet validationSet) - { - var packageUri = await GetPackageForValidationSetReadUriAsync( - validationSet, - DateTimeOffset.UtcNow.Add(AccessDuration)); - - Package packageFromValidationSet = new Package() - { - PackageRegistration = new PackageRegistration() { Id = validationSet.PackageId }, - NormalizedVersion = validationSet.PackageNormalizedVersion, - Key = validationSet.PackageKey - }; - - using (var packageStream = await _fileDownloader.DownloadAsync(packageUri, CancellationToken.None)) - { - await SavePackageFileAsync(packageFromValidationSet, packageStream, overwrite: true); - } - } - - public override async Task CopyValidationSetPackageToPackageFileAsync( - PackageValidationSet validationSet, - IAccessCondition destAccessCondition) - { - await CopyValidationPackageToPackageFileAsync(validationSet); - } - } -} \ No newline at end of file diff --git a/src/Validation.Common.Job/Validation.Common.Job.csproj b/src/Validation.Common.Job/Validation.Common.Job.csproj index 49175a8b1..3ae2596e1 100644 --- a/src/Validation.Common.Job/Validation.Common.Job.csproj +++ b/src/Validation.Common.Job/Validation.Common.Job.csproj @@ -112,7 +112,7 @@ 2.29.0 - 4.4.4-master-41290 + 4.4.4-dev-42523 2.5.0 diff --git a/src/Validation.Common.Job/Validation.Common.Job.nuspec b/src/Validation.Common.Job/Validation.Common.Job.nuspec index cd9c45ab8..0c007762b 100644 --- a/src/Validation.Common.Job/Validation.Common.Job.nuspec +++ b/src/Validation.Common.Job/Validation.Common.Job.nuspec @@ -23,7 +23,7 @@ - + diff --git a/src/Validation.Symbols/ISymbolsValidatorService.cs b/src/Validation.Symbols/ISymbolsValidatorService.cs index 4dc9dbea6..7c565d4b7 100644 --- a/src/Validation.Symbols/ISymbolsValidatorService.cs +++ b/src/Validation.Symbols/ISymbolsValidatorService.cs @@ -13,7 +13,7 @@ public interface ISymbolsValidatorService /// /// Validates the symbol package. /// - /// The regarding to the symbols pacakge to be validated.. + /// The regarding to the symbols package to be validated.. /// Cancellation token. /// The validation result. Task ValidateSymbolsAsync(SymbolsValidatorMessage message, CancellationToken token); diff --git a/src/Validation.Symbols/ITelemetryService.cs b/src/Validation.Symbols/ITelemetryService.cs index cc11e3194..a9ed22b82 100644 --- a/src/Validation.Symbols/ITelemetryService.cs +++ b/src/Validation.Symbols/ITelemetryService.cs @@ -34,7 +34,7 @@ public interface ITelemetryService : ISubscriptionProcessorTelemetryService /// /// Tracks the status of the validation. /// - /// The pacakge id. + /// The package id. /// The package normalized version. /// The validation result. /// Information about the issue id failed or empty if passed.. diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/NuGet.Services.Validation.Orchestrator.Tests.csproj b/tests/NuGet.Services.Validation.Orchestrator.Tests/NuGet.Services.Validation.Orchestrator.Tests.csproj index 590e3022f..b6795d9f4 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/NuGet.Services.Validation.Orchestrator.Tests.csproj +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/NuGet.Services.Validation.Orchestrator.Tests.csproj @@ -62,6 +62,7 @@ + diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/MessageServiceFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/MessageServiceFacts.cs index 8785d3335..dcd22218e 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/MessageServiceFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/MessageServiceFacts.cs @@ -9,7 +9,8 @@ using Microsoft.Extensions.Options; using Moq; using NuGetGallery; -using NuGetGallery.Services; +using NuGetGallery.Infrastructure.Mail; +using NuGetGallery.Infrastructure.Mail.Messages; using Xunit; namespace NuGet.Services.Validation.Orchestrator.Tests @@ -20,7 +21,7 @@ public class MessageServiceFacts public void ConstructorThrowsWhenCoreMessageServiceIsNull() { var ex = Assert.Throws(() => new PackageMessageService(null, EmailConfigurationAccessorMock.Object, LoggerMock.Object)); - Assert.Equal("coreMessageService", ex.ParamName); + Assert.Equal("messageService", ex.ParamName); } [Fact] @@ -103,18 +104,15 @@ public void ConstructorThrowsWhenEmailSettingsUrlIsNotProperUrl() [Fact] public void SendPackagePublishedEmailMethodCallsCoreMessageService() { - var expectedPackageUrl = string.Format(EmailConfiguration.PackageUrlTemplate, Package.PackageRegistration.Id, Package.NormalizedVersion); - var expectedSupportUrl = string.Format(EmailConfiguration.PackageSupportTemplate, Package.PackageRegistration.Id, Package.NormalizedVersion); - var service = new PackageMessageService(CoreMessageServiceMock.Object, EmailConfigurationAccessorMock.Object, LoggerMock.Object); var ex = Record.Exception(() => service.SendPublishedMessageAsync(Package).Wait()); Assert.Null(ex); CoreMessageServiceMock - .Verify(cms => cms.SendPackageAddedNoticeAsync(Package, expectedPackageUrl, expectedSupportUrl, ValidSettingsUrl, It.IsAny>()), Times.Once()); + .Verify(cms => cms.SendMessageAsync(It.Is(arg=>arg.Package == Package), It.IsAny(), It.IsAny()), Times.Once()); CoreMessageServiceMock - .Verify(cms => cms.SendPackageAddedNoticeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once()); + .Verify(cms => cms.SendMessageAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } [Fact] @@ -128,18 +126,13 @@ public async Task SendPackagePublishedEmailThrowsWhenPackageIsNull() [Fact] public void SendPackageValidationFailedMessageCallsCoreMessageService() { - var expectedPackageUrl = string.Format(EmailConfiguration.PackageUrlTemplate, Package.PackageRegistration.Id, Package.NormalizedVersion); - var expectedSupportUrl = string.Format(EmailConfiguration.PackageSupportTemplate, Package.PackageRegistration.Id, Package.NormalizedVersion); - var service = new PackageMessageService(CoreMessageServiceMock.Object, EmailConfigurationAccessorMock.Object, LoggerMock.Object); var ex = Record.Exception(() => service.SendValidationFailedMessageAsync(Package, ValidationSet).Wait()); Assert.Null(ex); CoreMessageServiceMock - .Verify(cms => cms.SendPackageValidationFailedNoticeAsync(Package, ValidationSet, expectedPackageUrl, expectedSupportUrl, EmailConfiguration.AnnouncementsUrl, EmailConfiguration.TwitterUrl), Times.Once()); - CoreMessageServiceMock - .Verify(cms => cms.SendPackageValidationFailedNoticeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + .Verify(cms => cms.SendMessageAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } [Fact] @@ -166,7 +159,9 @@ public MessageServiceFacts() PackageSupportTemplate = "https://example.com/packageSupport/{0}/{1}", EmailSettingsUrl = ValidSettingsUrl, AnnouncementsUrl = "https://announcements.com", - TwitterUrl = "https://twitter.com/nuget" + TwitterUrl = "https://twitter.com/nuget", + GalleryNoReplyAddress = "NuGet Gallery ", + GalleryOwner = "NuGet Gallery " }; EmailConfigurationAccessorMock @@ -184,7 +179,7 @@ public MessageServiceFacts() ValidationSet = new PackageValidationSet(); } - private Mock CoreMessageServiceMock { get; set; } = new Mock(); + private Mock CoreMessageServiceMock { get; set; } = new Mock(); private Mock> EmailConfigurationAccessorMock { get; set; } = new Mock>(); private Mock> LoggerMock { get; set; } = new Mock>(); private Package Package { get; set; } diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs index c02e576cd..fb377f27c 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs @@ -21,7 +21,7 @@ public class PackageEntityServiceFacts public class TheFindPackageByIdAndVersionStrictMethod { [Fact] - public void ReturnsNullIfPacakgeDoesNotExist() + public void ReturnsNullIfPackageDoesNotExist() { // Arrange var mockCorePackageService = new Mock(); diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolsMessageServiceFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolsMessageServiceFacts.cs index 2c052c02c..387683329 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolsMessageServiceFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolsMessageServiceFacts.cs @@ -8,7 +8,8 @@ using Microsoft.Extensions.Options; using Moq; using NuGetGallery; -using NuGetGallery.Services; +using NuGetGallery.Infrastructure.Mail; +using NuGetGallery.Infrastructure.Mail.Messages; using Xunit; namespace NuGet.Services.Validation.Orchestrator.Tests @@ -19,7 +20,7 @@ public class SymbolsMessageServiceFacts public void ConstructorThrowsWhenCoreMessageServiceIsNull() { var ex = Assert.Throws(() => new SymbolsPackageMessageService(null, EmailConfigurationAccessorMock.Object, LoggerMock.Object)); - Assert.Equal("coreMessageService", ex.ParamName); + Assert.Equal("messageService", ex.ParamName); } [Fact] @@ -48,9 +49,7 @@ public void SendPackagePublishedEmailMethodCallsCoreMessageService() Assert.Null(ex); CoreMessageServiceMock - .Verify(cms => cms.SendSymbolPackageAddedNoticeAsync(SymbolPackage, expectedPackageUrl, expectedSupportUrl, ValidSettingsUrl, It.IsAny>()), Times.Once()); - CoreMessageServiceMock - .Verify(cms => cms.SendSymbolPackageAddedNoticeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once()); + .Verify(cms => cms.SendMessageAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } [Fact] @@ -73,9 +72,7 @@ public void SendPackageValidationFailedMessageCallsCoreMessageService() Assert.Null(ex); CoreMessageServiceMock - .Verify(cms => cms.SendSymbolPackageValidationFailedNoticeAsync(SymbolPackage, ValidationSet, expectedPackageUrl, expectedSupportUrl, EmailConfiguration.AnnouncementsUrl, EmailConfiguration.TwitterUrl), Times.Once()); - CoreMessageServiceMock - .Verify(cms => cms.SendSymbolPackageValidationFailedNoticeAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); + .Verify(cms => cms.SendMessageAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once()); } [Fact] @@ -103,7 +100,9 @@ public SymbolsMessageServiceFacts() PackageSupportTemplate = "https://example.com/packageSupport/{0}/{1}", EmailSettingsUrl = ValidSettingsUrl, AnnouncementsUrl = "https://announcements.com", - TwitterUrl = "https://twitter.com/nuget" + TwitterUrl = "https://twitter.com/nuget", + GalleryNoReplyAddress = "NuGet Gallery ", + GalleryOwner = "NuGet Gallery " }; EmailConfigurationAccessorMock @@ -125,11 +124,19 @@ public SymbolsMessageServiceFacts() ValidationSet = new PackageValidationSet(); } - private Mock CoreMessageServiceMock { get; set; } = new Mock(); + private Mock CoreMessageServiceMock { get; set; } = new Mock(); private Mock> EmailConfigurationAccessorMock { get; set; } = new Mock>(); private Mock> LoggerMock { get; set; } = new Mock>(); private SymbolPackage SymbolPackage { get; set; } private PackageValidationSet ValidationSet { get; set; } private EmailConfiguration EmailConfiguration { get; set; } + + private MessageServiceConfiguration ServiceConfiguration + { + get + { + return new MessageServiceConfiguration(EmailConfigurationAccessorMock.Object); + } + } } } diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/Symbol/SymbolsStatusProcessorFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/Symbol/SymbolsStatusProcessorFacts.cs new file mode 100644 index 000000000..7f9528c86 --- /dev/null +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/Symbol/SymbolsStatusProcessorFacts.cs @@ -0,0 +1,234 @@ +// 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; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; + +using Moq; +using NuGet.Jobs.Validation; +using NuGet.Jobs.Validation.Storage; +using NuGet.Services.Validation.Orchestrator.Telemetry; +using NuGet.Services.Validation.Symbols; + +using Xunit; +using Xunit.Abstractions; + + +using NuGetGallery; +using NuGetGallery.Packaging; + + +namespace NuGet.Services.Validation.Orchestrator.Tests.Symbol +{ + public class SymbolsStatusProcessorFacts + { + public class TheProceedToMakePackageAvailable : BaseFacts + { + [Fact] + public void ItShouldNotProceedWhenFromAvailableState() + { + // Arrange + IValidatingEntity validatingSymbolPackage = null; + SymbolsPackageServiceMock.Setup(sp => sp.FindPackageByIdAndVersionStrict(PackageId, PackageVersion)) + .Returns(validatingSymbolPackage); + + var validationSet = new PackageValidationSet + { + PackageId = AvailableSymbolPackage.Id, + PackageNormalizedVersion = AvailableSymbolPackage.Version, + PackageKey = AvailableSymbolPackage.Key, + PackageValidations = new List + { + new PackageValidation { Type = "SomeValidator" }, + } + }; + + // Act + bool result = Target.CanProceedToMakePackageAvailable(AvailableSymbolPackageValidatingEntity, validationSet); + Target.SetStatusAsync(AvailableSymbolPackageValidatingEntity, validationSet, PackageStatus.Available); + + // Assert + Assert.False(result); + SymbolPackageFileServiceMock.Verify(spfs => spfs.DoesValidationSetPackageExistAsync(It.IsAny()), Times.Never); + SymbolPackageFileServiceMock.Verify(spfs => spfs.CopyValidationSetPackageToPackageFileAsync(It.IsAny(), It.IsAny()), Times.Never); + SymbolPackageFileServiceMock.Verify(spfs => spfs.CopyValidationPackageToPackageFileAsync(It.IsAny()), Times.Never); + SymbolsPackageServiceMock.Verify(sps => sps.UpdateStatusAsync(It.IsAny < SymbolPackage>(), It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public void ItShouldNotProceedWhenFromFailedStateWithValidationInProgress() + { + // Arrange + SymbolsPackageServiceMock.Setup(sp => sp.FindPackageByIdAndVersionStrict(PackageId, PackageVersion)) + .Returns(ValidatingSymbolPackageValidatingEntity); + + var validationSet = new PackageValidationSet + { + PackageId = FailedSymbolPackage.Id, + PackageNormalizedVersion = FailedSymbolPackage.Version, + PackageKey = FailedSymbolPackageValidatingEntity.Key, + PackageValidations = new List + { + new PackageValidation { Type = "SomeValidator" }, + } + }; + + // Act + bool result = Target.CanProceedToMakePackageAvailable(FailedSymbolPackageValidatingEntity, validationSet); + Target.SetStatusAsync(FailedSymbolPackageValidatingEntity, validationSet, PackageStatus.Available); + + // Assert + Assert.False(result); + SymbolPackageFileServiceMock.Verify(spfs => spfs.DoesValidationSetPackageExistAsync(It.IsAny()), Times.Never); + SymbolPackageFileServiceMock.Verify(spfs => spfs.CopyValidationSetPackageToPackageFileAsync(It.IsAny(), It.IsAny()), Times.Never); + SymbolPackageFileServiceMock.Verify(spfs => spfs.CopyValidationPackageToPackageFileAsync(It.IsAny()), Times.Never); + SymbolsPackageServiceMock.Verify(sps => sps.UpdateStatusAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + [Fact] + public void ItShouldProceedWhenFromFailedStateWithNoValidationInProgress() + { + // Arrange + IValidatingEntity validatingSymbolPackage = null; + SymbolsPackageServiceMock.Setup(sp => sp.FindPackageByIdAndVersionStrict(PackageId, PackageVersion)) + .Returns(validatingSymbolPackage); + + var validationSet = new PackageValidationSet + { + PackageId = FailedSymbolPackage.Id, + PackageNormalizedVersion = FailedSymbolPackage.Version, + PackageKey = FailedSymbolPackageValidatingEntity.Key, + PackageValidations = new List + { + new PackageValidation { Type = "SomeValidator" }, + } + }; + + // Act + bool result = Target.CanProceedToMakePackageAvailable(FailedSymbolPackageValidatingEntity, validationSet); + Target.SetStatusAsync(FailedSymbolPackageValidatingEntity, validationSet, PackageStatus.Available); + + // Assert + Assert.True(result); + SymbolPackageFileServiceMock.Verify(spfs => spfs.DoesValidationSetPackageExistAsync(It.IsAny()), Times.Once); + } + + [Fact] + public void ItShouldProceedWhenFromValidatingState() + { + // Arrange + SymbolsPackageServiceMock.Setup(sp => sp.FindPackageByIdAndVersionStrict(PackageId, PackageVersion)) + .Returns(ValidatingSymbolPackageValidatingEntity); + + var validationSet = new PackageValidationSet + { + PackageId = ValidatingSymbolPackage.Id, + PackageNormalizedVersion = ValidatingSymbolPackage.Version, + PackageKey = ValidatingSymbolPackage.Key, + PackageValidations = new List + { + new PackageValidation { Type = "SomeValidator" }, + } + }; + + // Act + bool result = Target.CanProceedToMakePackageAvailable(ValidatingSymbolPackageValidatingEntity, validationSet); + Target.SetStatusAsync(ValidatingSymbolPackageValidatingEntity, validationSet, PackageStatus.Available); + + // Assert + Assert.True(result); + SymbolPackageFileServiceMock.Verify(spfs => spfs.DoesValidationSetPackageExistAsync(It.IsAny()), Times.Once); + } + } + + public class BaseFacts + { + public const string PackageId = "SomeId"; + public const string PackageVersion = "1.1.1"; + public const int PackageKey = 100; + + public BaseFacts() + { + + Package = new Package + { + PackageRegistration = new PackageRegistration() + { + Id = PackageId + }, + PackageStatusKey = PackageStatus.Available, + Version = PackageVersion, + NormalizedVersion = PackageVersion, + Key = PackageKey + }; + + AvailableSymbolPackage = new SymbolPackage + { + Key = 1, + Package = Package, + PackageKey = PackageKey, + StatusKey = PackageStatus.Available + }; + + FailedSymbolPackage = new SymbolPackage + { + Key = 2, + Package = Package, + PackageKey = PackageKey, + StatusKey = PackageStatus.FailedValidation + }; + + ValidatingSymbolPackage = new SymbolPackage + { + Key = 3, + Package = Package, + PackageKey = 100, + StatusKey = PackageStatus.Validating + }; + + SymbolsPackageServiceMock = new Mock>(); + SymbolPackageFileServiceMock = new Mock(); + ValidatorProviderMock = new Mock(); + TelemetryServiceMock = new Mock(); + LoggerMock = new Mock>>(); + + var streamMetadata = new PackageStreamMetadata() + { + Size = 1, + Hash = "hash", + HashAlgorithm = CoreConstants.Sha512HashAlgorithmId + }; + + Target = new SymbolsStatusProcessor( + SymbolsPackageServiceMock.Object, + SymbolPackageFileServiceMock.Object, + ValidatorProviderMock.Object, + TelemetryServiceMock.Object, + LoggerMock.Object); + + AvailableSymbolPackageValidatingEntity = new SymbolPackageValidatingEntity(AvailableSymbolPackage); + FailedSymbolPackageValidatingEntity = new SymbolPackageValidatingEntity(FailedSymbolPackage); + ValidatingSymbolPackageValidatingEntity = new SymbolPackageValidatingEntity(ValidatingSymbolPackage); + } + + public Package Package { get; } + public SymbolPackage AvailableSymbolPackage { get; } + public SymbolPackage FailedSymbolPackage { get; } + public SymbolPackage ValidatingSymbolPackage { get; } + public Mock> SymbolsPackageServiceMock { get; } + public Mock SymbolPackageFileServiceMock { get; } + public Mock ValidatorProviderMock { get; } + public Mock TelemetryServiceMock { get; } + public Mock>> LoggerMock { get; } + public SymbolsStatusProcessor Target { get; } + public SymbolPackageValidatingEntity AvailableSymbolPackageValidatingEntity { get; } + public SymbolPackageValidatingEntity FailedSymbolPackageValidatingEntity { get; } + public SymbolPackageValidatingEntity ValidatingSymbolPackageValidatingEntity { get; } + + } + + } +}