diff --git a/src/NuGet.Services.Revalidate/Job.cs b/src/NuGet.Services.Revalidate/Job.cs index 70155403a..9c7b5c027 100644 --- a/src/NuGet.Services.Revalidate/Job.cs +++ b/src/NuGet.Services.Revalidate/Job.cs @@ -32,6 +32,7 @@ public class Job : ValidationJobBase private const string RebuildPreinstalledSetArgumentName = "RebuildPreinstalledSet"; private const string InitializeArgumentName = "Initialize"; private const string VerifyInitializationArgumentName = "VerifyInitialization"; + private const string JobConfigurationSectionName = "RevalidateJob"; private static readonly TimeSpan RetryLaterSleepDuration = TimeSpan.FromMinutes(5); @@ -112,6 +113,7 @@ await scope.ServiceProvider protected override void ConfigureJobServices(IServiceCollection services, IConfigurationRoot configurationRoot) { services.Configure(configurationRoot.GetSection(JobConfigurationSectionName)); + services.AddSingleton(provider => provider.GetRequiredService>().Value); services.AddSingleton(provider => provider.GetRequiredService>().Value.Initialization); services.AddSingleton(provider => provider.GetRequiredService>().Value.Health); diff --git a/src/NuGet.Services.Revalidate/Settings/dev.json b/src/NuGet.Services.Revalidate/Settings/dev.json index 7f4c60e58..31ec1022a 100644 --- a/src/NuGet.Services.Revalidate/Settings/dev.json +++ b/src/NuGet.Services.Revalidate/Settings/dev.json @@ -16,8 +16,8 @@ "ComponentPath": "NuGet/Package Publishing" }, - "MinPackageEventRate": 120, - "MaxPackageEventRate": 400, + "MinPackageEventRate": "#{Jobs.nuget.services.revalidation.MinPackageEventRate}", + "MaxPackageEventRate": "#{Jobs.nuget.services.revalidation.MaxPackageEventRate}", "RetryLaterSleep": "00:00:30", diff --git a/src/NuGet.Services.Revalidate/Settings/int.json b/src/NuGet.Services.Revalidate/Settings/int.json index 8fafcf4dd..ee4719b91 100644 --- a/src/NuGet.Services.Revalidate/Settings/int.json +++ b/src/NuGet.Services.Revalidate/Settings/int.json @@ -16,8 +16,8 @@ "ComponentPath": "NuGet/Package Publishing" }, - "MinPackageEventRate": 120, - "MaxPackageEventRate": 400, + "MinPackageEventRate": "#{Jobs.nuget.services.revalidation.MinPackageEventRate}", + "MaxPackageEventRate": "#{Jobs.nuget.services.revalidation.MaxPackageEventRate}", "RetryLaterSleep": "00:00:30", diff --git a/src/NuGet.Services.Revalidate/Settings/prod.json b/src/NuGet.Services.Revalidate/Settings/prod.json index 6dc0c5ef5..da5f0c238 100644 --- a/src/NuGet.Services.Revalidate/Settings/prod.json +++ b/src/NuGet.Services.Revalidate/Settings/prod.json @@ -16,8 +16,8 @@ "ComponentPath": "NuGet/Package Publishing" }, - "MinPackageEventRate": 120, - "MaxPackageEventRate": 400, + "MinPackageEventRate": "#{Jobs.nuget.services.revalidation.MinPackageEventRate}", + "MaxPackageEventRate": "#{Jobs.nuget.services.revalidation.MaxPackageEventRate}", "RetryLaterSleep": "00:00:30", diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/IEntityService.cs b/src/NuGet.Services.Validation.Orchestrator/Services/IEntityService.cs index 74867128f..3d5d3fede 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Services/IEntityService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Services/IEntityService.cs @@ -17,9 +17,15 @@ public interface IEntityService where T : class, IEntity /// /// The id . /// The version. - /// + /// The entity. IValidatingEntity FindPackageByIdAndVersionStrict(string id, string version); + /// + /// Find the entity based on the key. + /// + /// The entity. + IValidatingEntity FindPackageByKey(int key); + /// /// Update the status of the entity. /// @@ -35,7 +41,7 @@ public interface IEntityService where T : class, IEntity /// The entity. /// The metadata. /// True if the changes will be commited to the database. - /// + /// A that can be used to await for the operation completion. Task UpdateMetadataAsync(T entity, object metadata, bool commitChanges = true); } } diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/PackageEntityService.cs b/src/NuGet.Services.Validation.Orchestrator/Services/PackageEntityService.cs index 7784643f0..4e3fdec08 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Services/PackageEntityService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Services/PackageEntityService.cs @@ -1,6 +1,9 @@ // 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.Linq; +using System.Data.Entity; using System.Threading.Tasks; using NuGetGallery; using NuGetGallery.Packaging; @@ -13,10 +16,12 @@ namespace NuGet.Services.Validation.Orchestrator public class PackageEntityService : IEntityService { private ICorePackageService _galleryEntityService; + private IEntityRepository _packageRepository; - public PackageEntityService(ICorePackageService galleryEntityService) + public PackageEntityService(ICorePackageService galleryEntityService, IEntityRepository packageRepository) { - _galleryEntityService = galleryEntityService; + _galleryEntityService = galleryEntityService ?? throw new ArgumentNullException(nameof(galleryEntityService)); + _packageRepository = packageRepository ?? throw new ArgumentNullException(nameof(packageRepository)); } public IValidatingEntity FindPackageByIdAndVersionStrict(string id, string version) @@ -25,6 +30,18 @@ public IValidatingEntity FindPackageByIdAndVersionStrict(string id, str return p == null ? null : new PackageValidatingEntity(p); } + public IValidatingEntity FindPackageByKey(int key) + { + var package = _packageRepository + .GetAll() + .Include(p => p.LicenseReports) + .Include(p => p.PackageRegistration) + .Include(p => p.User) + .Include(p => p.SymbolPackages) + .SingleOrDefault(p => p.Key == key); + return package == null ? null : new PackageValidatingEntity(package); + } + public async Task UpdateStatusAsync(Package entity, PackageStatus newStatus, bool commitChanges = true) { await _galleryEntityService.UpdatePackageStatusAsync(entity, newStatus, commitChanges); diff --git a/src/NuGet.Services.Validation.Orchestrator/Services/SymbolEntityService.cs b/src/NuGet.Services.Validation.Orchestrator/Services/SymbolEntityService.cs index f93d1593f..f4364f936 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Services/SymbolEntityService.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Services/SymbolEntityService.cs @@ -13,10 +13,13 @@ namespace NuGet.Services.Validation.Orchestrator /// public class SymbolEntityService : IEntityService { - ICoreSymbolPackageService _galleryEntityService; - public SymbolEntityService(ICoreSymbolPackageService galleryEntityService) + private ICoreSymbolPackageService _galleryEntityService; + private IEntityRepository _symbolsPackageRepository; + + public SymbolEntityService(ICoreSymbolPackageService galleryEntityService, IEntityRepository symbolsPackageRepository) { _galleryEntityService = galleryEntityService ?? throw new ArgumentNullException(nameof(galleryEntityService)); + _symbolsPackageRepository = symbolsPackageRepository ?? throw new ArgumentNullException(nameof(symbolsPackageRepository)); } /// @@ -35,6 +38,14 @@ public IValidatingEntity FindPackageByIdAndVersionStrict(string i return symbolPackage == null ? null : new SymbolPackageValidatingEntity(symbolPackage); } + public IValidatingEntity FindPackageByKey(int key) + { + var symbolPackage = _symbolsPackageRepository + .GetAll() + .SingleOrDefault(p => p.Key == key); + return symbolPackage == null ? null : new SymbolPackageValidatingEntity(symbolPackage); + } + public async Task UpdateStatusAsync(SymbolPackage entity, PackageStatus newStatus, bool commitChanges = true) { if(entity == null) diff --git a/src/NuGet.Services.Validation.Orchestrator/SymbolValidationMessageHandler.cs b/src/NuGet.Services.Validation.Orchestrator/SymbolValidationMessageHandler.cs index a3d1cfbf2..abf4e0454 100644 --- a/src/NuGet.Services.Validation.Orchestrator/SymbolValidationMessageHandler.cs +++ b/src/NuGet.Services.Validation.Orchestrator/SymbolValidationMessageHandler.cs @@ -73,7 +73,11 @@ public async Task HandleAsync(PackageValidationMessageData message) message.PackageNormalizedVersion, message.ValidationTrackingId)) { - var symbolPackageEntity = _gallerySymbolService.FindPackageByIdAndVersionStrict(message.PackageId, message.PackageNormalizedVersion); + // When a message is sent from the Gallery with validation of a new entity, the EntityKey will be null because the message is sent to the service bus before the entity is persisted in the DB + // However when a revalidation happens or when the message is re-sent by the orchestrator the message will contain the key. In this case the key is used to find the entity to validate. + var symbolPackageEntity = message.EntityKey.HasValue + ? _gallerySymbolService.FindPackageByKey(message.EntityKey.Value) + : _gallerySymbolService.FindPackageByIdAndVersionStrict(message.PackageId, message.PackageNormalizedVersion); if (symbolPackageEntity == null) { @@ -94,9 +98,10 @@ public async Task HandleAsync(PackageValidationMessageData message) } else { - _logger.LogInformation("Could not find symbols for package {PackageId} {PackageNormalizedVersion} in DB, retrying", + _logger.LogInformation("Could not find symbols for package {PackageId} {PackageNormalizedVersion} {Key} in DB, retrying", message.PackageId, - message.PackageNormalizedVersion); + message.PackageNormalizedVersion, + message.EntityKey.HasValue); return false; } @@ -114,6 +119,7 @@ public async Task HandleAsync(PackageValidationMessageData message) } var processorStats = await _validationSetProcessor.ProcessValidationsAsync(validationSet); + // As part of the processing the validation outcome the orchestrator will send itself a message if validation are still being processed. await _validationOutcomeProcessor.ProcessValidationOutcomeAsync(validationSet, symbolPackageEntity, processorStats); } diff --git a/src/NuGet.Services.Validation.Orchestrator/Symbols/SymbolScanValidator.cs b/src/NuGet.Services.Validation.Orchestrator/Symbols/SymbolScanValidator.cs index 8f1c22232..ebb7956e4 100644 --- a/src/NuGet.Services.Validation.Orchestrator/Symbols/SymbolScanValidator.cs +++ b/src/NuGet.Services.Validation.Orchestrator/Symbols/SymbolScanValidator.cs @@ -113,7 +113,7 @@ private bool ShouldSkipScan(IValidationRequest request) { var symbolPackage = _symbolPackageService .FindSymbolPackagesByIdAndVersion(request.PackageId,request.PackageVersion) - .FirstOrDefault(sp => sp.StatusKey == PackageStatus.Validating); + .FirstOrDefault(sp => sp.Key == request.PackageKey); if (symbolPackage == null) { diff --git a/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs b/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs index 4ec2d2222..a4348655b 100644 --- a/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs +++ b/src/NuGet.Services.Validation.Orchestrator/ValidationOutcomeProcessor.cs @@ -264,7 +264,12 @@ private async Task ScheduleCheckIfNotTimedOut(PackageValidationSet validationSet // Schedule another check if we haven't reached the validation set timeout yet. if (validationSetDuration <= _validationConfiguration.TimeoutValidationSetAfter) { - var messageData = new PackageValidationMessageData(validationSet.PackageId, validationSet.PackageNormalizedVersion, validationSet.ValidationTrackingId, validationSet.ValidatingType); + var messageData = new PackageValidationMessageData( + validationSet.PackageId, + validationSet.PackageNormalizedVersion, + validationSet.ValidationTrackingId, + validationSet.ValidatingType, + entityKey: validationSet.PackageKey); var postponeUntil = DateTimeOffset.UtcNow + _validationConfiguration.ValidationMessageRecheckPeriod; await _validationEnqueuer.StartValidationAsync(messageData, postponeUntil); diff --git a/src/Validation.Symbols/ISymbolsFileService.cs b/src/Validation.Symbols/ISymbolsFileService.cs index 6365414cb..fc779f867 100644 --- a/src/Validation.Symbols/ISymbolsFileService.cs +++ b/src/Validation.Symbols/ISymbolsFileService.cs @@ -21,10 +21,9 @@ public interface ISymbolsFileService /// /// Downloads the snupkg file. /// - /// The package id. - /// The package normalized version. + /// The uri of the snupkg. /// Cancellation token. - /// The nupkg stream. - Task DownloadSnupkgFileAsync(string packageId, string packageNormalizedVersion, CancellationToken cancellationToken); + /// The snupkg stream. + Task DownloadSnupkgFileAsync(string snupkgUri, CancellationToken cancellationToken); } } diff --git a/src/Validation.Symbols/ISymbolsValidatorService.cs b/src/Validation.Symbols/ISymbolsValidatorService.cs index 275a31c40..4dc9dbea6 100644 --- a/src/Validation.Symbols/ISymbolsValidatorService.cs +++ b/src/Validation.Symbols/ISymbolsValidatorService.cs @@ -1,10 +1,9 @@ // 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.Collections.Generic; -using System.IO; using System.Threading; using System.Threading.Tasks; +using NuGet.Jobs.Validation.Symbols.Core; using NuGet.Services.Validation; namespace Validation.Symbols @@ -12,12 +11,11 @@ namespace Validation.Symbols public interface ISymbolsValidatorService { /// - /// Validates the symbols against the PE files. + /// Validates the symbol package. /// - /// The package Id. - /// The package normalized version. - /// A cancellation token to be used for cancellation of the async execution. - /// - Task ValidateSymbolsAsync(string packageId, string packageNormalizedVersion, CancellationToken token); + /// The regarding to the symbols pacakge to be validated.. + /// Cancellation token. + /// The validation result. + Task ValidateSymbolsAsync(SymbolsValidatorMessage message, CancellationToken token); } } diff --git a/src/Validation.Symbols/Job.cs b/src/Validation.Symbols/Job.cs index 4b86d9de1..7370b9fb9 100644 --- a/src/Validation.Symbols/Job.cs +++ b/src/Validation.Symbols/Job.cs @@ -40,11 +40,7 @@ protected override void ConfigureJobServices(IServiceCollection services, IConfi configurationAccessor.Value.ValidationPackageConnectionString, readAccessGeoRedundant: false), c.GetRequiredService()); - var symbolValidationStorageService = new CloudBlobCoreFileStorageService(new CloudBlobClientWrapper( - configurationAccessor.Value.ValidationSymbolsConnectionString, - readAccessGeoRedundant: false), c.GetRequiredService()); - - return new SymbolsFileService(packageStorageService, packageValidationStorageService, symbolValidationStorageService); + return new SymbolsFileService(packageStorageService, packageValidationStorageService, c.GetRequiredService()); }); services.AddSingleton(new TelemetryClient()); } diff --git a/src/Validation.Symbols/Settings/dev.json b/src/Validation.Symbols/Settings/dev.json index 348d1e4d7..2c0b657b1 100644 --- a/src/Validation.Symbols/Settings/dev.json +++ b/src/Validation.Symbols/Settings/dev.json @@ -13,6 +13,7 @@ "PackageConnectionString": "DefaultEndpointsProtocol=https;AccountName=nugetdevlegacy;AccountKey=$$Dev-NuGetDevLegacyStorage-Key$$", "ValidationSymbolsConnectionString": "DefaultEndpointsProtocol=https;AccountName=nugetdevlegacy;AccountKey=$$Dev-NuGetDevLegacyStorage-Key$$" }, + "PackageDownloadTimeout": "00:10:00", "KeyVault_VaultName": "#{Deployment.Azure.KeyVault.VaultName}", "KeyVault_ClientId": "#{Deployment.Azure.KeyVault.ClientId}", "KeyVault_CertificateThumbprint": "#{Deployment.Azure.KeyVault.CertificateThumbprint}", diff --git a/src/Validation.Symbols/Settings/int.json b/src/Validation.Symbols/Settings/int.json index e7d1c6c9c..5052b91f0 100644 --- a/src/Validation.Symbols/Settings/int.json +++ b/src/Validation.Symbols/Settings/int.json @@ -13,7 +13,7 @@ "PackageConnectionString": "DefaultEndpointsProtocol=https;AccountName=nugetint0;AccountKey=$$Int-NuGetInt0Storage-Key$$", "ValidationSymbolsConnectionString": "DefaultEndpointsProtocol=https;AccountName=nugetint0;AccountKey=$$Int-NuGetInt0Storage-Key$$" }, - + "PackageDownloadTimeout": "00:10:00", "KeyVault_VaultName": "#{Deployment.Azure.KeyVault.VaultName}", "KeyVault_ClientId": "#{Deployment.Azure.KeyVault.ClientId}", "KeyVault_CertificateThumbprint": "#{Deployment.Azure.KeyVault.CertificateThumbprint}", diff --git a/src/Validation.Symbols/Settings/prod.json b/src/Validation.Symbols/Settings/prod.json index c37b3ca8f..cb2d2542a 100644 --- a/src/Validation.Symbols/Settings/prod.json +++ b/src/Validation.Symbols/Settings/prod.json @@ -13,7 +13,7 @@ "PackageConnectionString": "DefaultEndpointsProtocol=https;AccountName=nugetgallery;AccountKey=$$Prod-NuGetGalleryStorage-Key$$", "ValidationSymbolsConnectionString": "DefaultEndpointsProtocol=https;AccountName=nugetgallery;AccountKey=$$Prod-NuGetGalleryStorage-Key$$" }, - + "PackageDownloadTimeout": "00:10:00", "KeyVault_VaultName": "#{Deployment.Azure.KeyVault.VaultName}", "KeyVault_ClientId": "#{Deployment.Azure.KeyVault.ClientId}", "KeyVault_CertificateThumbprint": "#{Deployment.Azure.KeyVault.CertificateThumbprint}", diff --git a/src/Validation.Symbols/SymbolsFileService.cs b/src/Validation.Symbols/SymbolsFileService.cs index 6a296567f..0c3f12b05 100644 --- a/src/Validation.Symbols/SymbolsFileService.cs +++ b/src/Validation.Symbols/SymbolsFileService.cs @@ -6,6 +6,7 @@ using System.Threading.Tasks; using System.Threading; using NuGetGallery; +using NuGet.Jobs.Validation; namespace Validation.Symbols { @@ -13,12 +14,12 @@ public class SymbolsFileService : ISymbolsFileService { private CorePackageFileService _packageFileService; private CorePackageFileService _packageValidationFileService; - private CorePackageFileService _symbolValidationFileService; + private IFileDownloader _fileDownloader; public SymbolsFileService( ICoreFileStorageService packageStorageService, ICoreFileStorageService packageValidationStorageService, - ICoreFileStorageService symbolValidationStorageService) + IFileDownloader fileDownloader) { if (packageStorageService == null) { @@ -28,32 +29,14 @@ public SymbolsFileService( { throw new ArgumentNullException(nameof(packageValidationStorageService)); } - if (symbolValidationStorageService == null) - { - throw new ArgumentNullException(nameof(symbolValidationStorageService)); - } _packageFileService = new CorePackageFileService(packageStorageService, new PackageFileMetadataService()); _packageValidationFileService = new CorePackageFileService(packageValidationStorageService, new PackageFileMetadataService()); - _symbolValidationFileService = new CorePackageFileService(symbolValidationStorageService, new SymbolPackageFileMetadataService()); + _fileDownloader = fileDownloader ?? throw new ArgumentNullException(nameof(fileDownloader)); } - public async Task DownloadSnupkgFileAsync(string packageId, string packageNormalizedVersion, CancellationToken cancellationToken) + public async Task DownloadSnupkgFileAsync(string snupkgUri, CancellationToken cancellationToken) { - var package = new Package() - { - NormalizedVersion = packageNormalizedVersion, - PackageRegistration = new PackageRegistration() - { - Id = packageId - } - }; - - if (await _symbolValidationFileService.DoesValidationPackageFileExistAsync(package)) - { - return await _symbolValidationFileService.DownloadValidationPackageFileAsync(package); - } - - throw new FileNotFoundException(string.Format("Symbols package {0} {1} not found in the validation container.", packageId, packageNormalizedVersion)); + return await _fileDownloader.DownloadAsync(new Uri(snupkgUri), cancellationToken); } public async Task DownloadNupkgFileAsync(string packageId, string packageNormalizedVersion, CancellationToken cancellationToken) diff --git a/src/Validation.Symbols/SymbolsValidatorMessageHandler.cs b/src/Validation.Symbols/SymbolsValidatorMessageHandler.cs index 1de5ec477..29b816aee 100644 --- a/src/Validation.Symbols/SymbolsValidatorMessageHandler.cs +++ b/src/Validation.Symbols/SymbolsValidatorMessageHandler.cs @@ -86,7 +86,7 @@ public async Task HandleAsync(SymbolsValidatorMessage message) return true; } - var validationResult = await _symbolValidatorService.ValidateSymbolsAsync(message.PackageId, message.PackageNormalizedVersion, CancellationToken.None); + var validationResult = await _symbolValidatorService.ValidateSymbolsAsync(message, CancellationToken.None); if (validationResult.Status == ValidationStatus.Failed || validationResult.Status == ValidationStatus.Succeeded) { diff --git a/src/Validation.Symbols/SymbolsValidatorService.cs b/src/Validation.Symbols/SymbolsValidatorService.cs index b7d2c5043..b5c81ccbc 100644 --- a/src/Validation.Symbols/SymbolsValidatorService.cs +++ b/src/Validation.Symbols/SymbolsValidatorService.cs @@ -42,60 +42,62 @@ public SymbolsValidatorService( _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } - public async Task ValidateSymbolsAsync(string packageId, string packageNormalizedVersion, CancellationToken token) + public async Task ValidateSymbolsAsync(SymbolsValidatorMessage message, CancellationToken token) { _logger.LogInformation("{ValidatorName} :Start ValidateSymbolsAsync. PackageId: {packageId} PackageNormalizedVersion: {packageNormalizedVersion}", ValidatorName.SymbolsValidator, - packageId, - packageNormalizedVersion); + message.PackageId, + message.PackageNormalizedVersion); - Stream snupkgstream; - Stream nupkgstream; - try - { - snupkgstream = await _symbolFileService.DownloadSnupkgFileAsync(packageId, packageNormalizedVersion, token); - } - catch(FileNotFoundException) - { - _telemetryService.TrackSymbolsPackageNotFoundEvent(packageId, packageNormalizedVersion); - return ValidationResult.Failed; - } try { - nupkgstream = await _symbolFileService.DownloadNupkgFileAsync(packageId, packageNormalizedVersion, token); - } - catch (FileNotFoundException) - { - _telemetryService.TrackPackageNotFoundEvent(packageId, packageNormalizedVersion); - return ValidationResult.Failed; - } - var pdbs = _zipArchiveService.ReadFilesFromZipStream(snupkgstream, SymbolExtension); - var pes = _zipArchiveService.ReadFilesFromZipStream(nupkgstream, PEExtensions); - - using (_telemetryService.TrackSymbolValidationDurationEvent(packageId, packageNormalizedVersion, pdbs.Count)) - { - if (!SymbolsHaveMatchingPEFiles(pdbs, pes)) - { - _telemetryService.TrackSymbolsValidationResultEvent(packageId, packageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound)); - return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound); - } - var targetDirectory = Settings.GetWorkingDirectory(); - try + using (Stream snupkgstream = await _symbolFileService.DownloadSnupkgFileAsync(message.SnupkgUrl, token)) { - _logger.LogInformation("Extracting symbols to {TargetDirectory}", targetDirectory); - var symbolFiles = _zipArchiveService.ExtractFilesFromZipStream(snupkgstream, targetDirectory, SymbolExtension); + try + { + using (Stream nupkgstream = await _symbolFileService.DownloadNupkgFileAsync(message.PackageId, message.PackageNormalizedVersion, token)) + { + var pdbs = _zipArchiveService.ReadFilesFromZipStream(snupkgstream, SymbolExtension); + var pes = _zipArchiveService.ReadFilesFromZipStream(nupkgstream, PEExtensions); + + using (_telemetryService.TrackSymbolValidationDurationEvent(message.PackageId, message.PackageNormalizedVersion, pdbs.Count)) + { + if (!SymbolsHaveMatchingPEFiles(pdbs, pes)) + { + _telemetryService.TrackSymbolsValidationResultEvent(message.PackageId, message.PackageNormalizedVersion, ValidationStatus.Failed, nameof(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound)); + return ValidationResult.FailedWithIssues(ValidationIssue.SymbolErrorCode_MatchingPortablePDBNotFound); + } + var targetDirectory = Settings.GetWorkingDirectory(); + try + { + _logger.LogInformation("Extracting symbols to {TargetDirectory}", targetDirectory); + var symbolFiles = _zipArchiveService.ExtractFilesFromZipStream(snupkgstream, targetDirectory, SymbolExtension); - _logger.LogInformation("Extracting dlls to {TargetDirectory}", targetDirectory); - _zipArchiveService.ExtractFilesFromZipStream(nupkgstream, targetDirectory, PEExtensions, symbolFiles); + _logger.LogInformation("Extracting dlls to {TargetDirectory}", targetDirectory); + _zipArchiveService.ExtractFilesFromZipStream(nupkgstream, targetDirectory, PEExtensions, symbolFiles); - var status = ValidateSymbolMatching(targetDirectory, packageId, packageNormalizedVersion); - return status; - } - finally - { - TryCleanWorkingDirectoryForSeconds(targetDirectory, packageId, packageNormalizedVersion, _cleanWorkingDirectoryTimeSpan); + var status = ValidateSymbolMatching(targetDirectory, message.PackageId, message.PackageNormalizedVersion); + return status; + } + finally + { + TryCleanWorkingDirectoryForSeconds(targetDirectory, message.PackageId, message.PackageNormalizedVersion, _cleanWorkingDirectoryTimeSpan); + } + } + } + } + catch (FileNotFoundException) + { + _telemetryService.TrackPackageNotFoundEvent(message.PackageId, message.PackageNormalizedVersion); + return ValidationResult.Failed; + } } } + catch(InvalidOperationException) + { + _telemetryService.TrackSymbolsPackageNotFoundEvent(message.PackageId, message.PackageNormalizedVersion); + return ValidationResult.Failed; + } } private void TryCleanWorkingDirectoryForSeconds(string workingDirectory, string packageId, string packageNormalizedVersion, TimeSpan seconds) diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs index 961af525f..58829bca8 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/PackageStatusProcessorFacts.cs @@ -101,7 +101,8 @@ public async Task DoesNotSetPackageStreamMetadataIfNotChanged() var stream = new MemoryStream(Encoding.ASCII.GetBytes(content)); var corePackageService = new Mock(); - var entityPackageService = new PackageEntityService(corePackageService.Object); + var mockPackageEntityRepository = new Mock>(); + var entityPackageService = new PackageEntityService(corePackageService.Object, mockPackageEntityRepository.Object); PackageFileServiceMock .Setup(x => x.DownloadPackageFileToDiskAsync(ValidationSet)) diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs index 992209cad..c02e576cd 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/PackageEntityServiceFacts.cs @@ -25,8 +25,9 @@ public void ReturnsNullIfPacakgeDoesNotExist() { // Arrange var mockCorePackageService = new Mock(); + var mockIPackageEntityRepository = new Mock>(); mockCorePackageService.Setup(c => c.FindPackageByIdAndVersionStrict(It.IsAny(), It.IsAny())).Returns((Package)null); - var service = new PackageEntityService(mockCorePackageService.Object); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.FindPackageByIdAndVersionStrict("Id", "version"); @@ -41,8 +42,9 @@ public void ReturnsThePackageEntityAsIValidatingEntity() var package = CreatePackage(); var mockCorePackageService = new Mock(); + var mockIPackageEntityRepository = new Mock>(); mockCorePackageService.Setup(c => c.FindPackageByIdAndVersionStrict(PackageId, PackageNormalizedVersion)).Returns(package); - var service = new PackageEntityService(mockCorePackageService.Object); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.FindPackageByIdAndVersionStrict(PackageId, PackageNormalizedVersion); @@ -63,8 +65,9 @@ public void InvokeTheICorePackageServices() // Arrange var package = CreatePackage(); var mockCorePackageService = new Mock(); + var mockIPackageEntityRepository = new Mock>(); mockCorePackageService.Setup(c => c.UpdatePackageStatusAsync(It.IsAny(), It.IsAny(), true)).Returns(Task.CompletedTask); - var service = new PackageEntityService(mockCorePackageService.Object); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.UpdateStatusAsync(package, PackageStatus.Available, true); @@ -82,7 +85,8 @@ public void NullMetadataIsNoop() // Arrange var package = CreatePackage(); var mockCorePackageService = new Mock(); - var service = new PackageEntityService(mockCorePackageService.Object); + var mockIPackageEntityRepository = new Mock>(); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.UpdateMetadataAsync(package, null, true); @@ -97,7 +101,8 @@ public void IncorrectMetadataTypeIsNoop() // Arrange var package = CreatePackage(); var mockCorePackageService = new Mock(); - var service = new PackageEntityService(mockCorePackageService.Object); + var mockIPackageEntityRepository = new Mock>(); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.UpdateMetadataAsync(package, "NotMetadata", true); @@ -122,7 +127,8 @@ public void IdenticalMetadataChangesIsNoop() }; var mockCorePackageService = new Mock(); - var service = new PackageEntityService(mockCorePackageService.Object); + var mockIPackageEntityRepository = new Mock>(); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.UpdateMetadataAsync(package, metadata, true); @@ -147,8 +153,9 @@ public void MetadataChangeInvokeICoreService() }; var mockCorePackageService = new Mock(); + var mockIPackageEntityRepository = new Mock>(); mockCorePackageService.Setup(c => c.UpdatePackageStreamMetadataAsync(package, metadata, true)).Returns(Task.CompletedTask); - var service = new PackageEntityService(mockCorePackageService.Object); + var service = new PackageEntityService(mockCorePackageService.Object, mockIPackageEntityRepository.Object); // Act & Assert var result = service.UpdateMetadataAsync(package, metadata, true); diff --git a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolEntityServiceFacts.cs b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolEntityServiceFacts.cs index c248a4be7..b42e66e86 100644 --- a/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolEntityServiceFacts.cs +++ b/tests/NuGet.Services.Validation.Orchestrator.Tests/Services/SymbolEntityServiceFacts.cs @@ -266,13 +266,15 @@ public async Task UpdateStatusAsyncMethodDoesNotUpdatePreviosAvailableStatusOnFa public abstract class FactsBase { protected readonly Mock _coreSymbolPackageService; - + protected readonly Mock> _symbolEntityRepository; + protected readonly SymbolEntityService _target; public FactsBase(ITestOutputHelper output) { _coreSymbolPackageService = new Mock(); - _target = new SymbolEntityService(_coreSymbolPackageService.Object); + _symbolEntityRepository = new Mock>(); + _target = new SymbolEntityService(_coreSymbolPackageService.Object, _symbolEntityRepository.Object); } } } diff --git a/tests/Validation.Symbols.Tests/SymbolsFileServiceTests.cs b/tests/Validation.Symbols.Tests/SymbolsFileServiceTests.cs index f177da248..d52b7f4ca 100644 --- a/tests/Validation.Symbols.Tests/SymbolsFileServiceTests.cs +++ b/tests/Validation.Symbols.Tests/SymbolsFileServiceTests.cs @@ -6,6 +6,7 @@ using System.Threading; using System.Threading.Tasks; using NuGetGallery; +using NuGet.Jobs.Validation; using Moq; using Xunit; @@ -14,7 +15,7 @@ namespace Validation.Symbols.Tests public class SymbolsFileServiceTests { private Mock _packageStorageService; - private Mock _symbolsValidationStorageService; + private Mock _fileDownloader; private Mock _packageValidationStorageService; private const string PackageId = "Pack"; private const string PackageNormalizedVersion = "1.2.3"; @@ -22,7 +23,7 @@ public class SymbolsFileServiceTests public SymbolsFileServiceTests() { _packageStorageService = new Mock(); - _symbolsValidationStorageService = new Mock(); + _fileDownloader = new Mock(); _packageValidationStorageService = new Mock(); } @@ -30,8 +31,8 @@ public SymbolsFileServiceTests() public void ConstructorNullCheck() { // Arrange + Act + Assert - Assert.Throws(() => new SymbolsFileService(null, _packageValidationStorageService.Object, _symbolsValidationStorageService.Object)); - Assert.Throws(() => new SymbolsFileService(_packageStorageService.Object, null, _symbolsValidationStorageService.Object)); + Assert.Throws(() => new SymbolsFileService(null, _packageValidationStorageService.Object, _fileDownloader.Object)); + Assert.Throws(() => new SymbolsFileService(_packageStorageService.Object, null, _fileDownloader.Object)); Assert.Throws(() => new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, null)); } @@ -39,21 +40,20 @@ public void ConstructorNullCheck() public async Task DownloadSnupkgFileAsyncCallsDownloadValidationPackageFileAsync() { // Arrange - var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _symbolsValidationStorageService.Object); - _symbolsValidationStorageService.Setup(svss => svss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(true); - // Act - var stream = await service.DownloadSnupkgFileAsync(PackageId, PackageNormalizedVersion, CancellationToken.None); - - // Assert - _symbolsValidationStorageService.Verify(svss => svss.GetFileAsync(It.IsAny(), It.IsAny()), Times.Once); + var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _fileDownloader.Object); + _fileDownloader.Setup(svss => svss.DownloadAsync(It.IsAny(), It.IsAny())).ThrowsAsync(new InvalidOperationException("Boo")); + + // Act + Assert + var exception = await Assert.ThrowsAsync( async () => await service.DownloadSnupkgFileAsync("https://dummy.snupkg", CancellationToken.None)); + Assert.Equal("Boo", exception.Message); } [Fact] public async Task DownloadNupkgFileAsyncSearchPackageValidationAfterPackageFolder() { // Arrange - var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _symbolsValidationStorageService.Object); + var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _fileDownloader.Object); _packageStorageService.Setup(pss => pss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(false); _packageValidationStorageService.Setup(pvss => pvss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(true); @@ -68,7 +68,7 @@ public async Task DownloadNupkgFileAsyncSearchPackageValidationAfterPackageFolde public async Task DownloadNupkgFileAsyncUsesPackageFolderIfFound() { // Arrange - var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _symbolsValidationStorageService.Object); + var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _fileDownloader.Object); _packageStorageService.Setup(pss => pss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(true); _packageValidationStorageService.Setup(pvss => pvss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(true); @@ -84,7 +84,7 @@ public async Task DownloadNupkgFileAsyncUsesPackageFolderIfFound() public void DownloadNupkgFileAsyncThowsIfNotFound() { // Arrange - var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _symbolsValidationStorageService.Object); + var service = new SymbolsFileService(_packageStorageService.Object, _packageValidationStorageService.Object, _fileDownloader.Object); _packageStorageService.Setup(pss => pss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(false); _packageValidationStorageService.Setup(pvss => pvss.FileExistsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(false); diff --git a/tests/Validation.Symbols.Tests/SymbolsValidatorMessageHandlerTests.cs b/tests/Validation.Symbols.Tests/SymbolsValidatorMessageHandlerTests.cs index 121e033fb..bb4c56321 100644 --- a/tests/Validation.Symbols.Tests/SymbolsValidatorMessageHandlerTests.cs +++ b/tests/Validation.Symbols.Tests/SymbolsValidatorMessageHandlerTests.cs @@ -113,7 +113,7 @@ public async Task IncompleteStateIsProcessed() { State = ValidationStatus.Incomplete }; _validatorStateService.Setup(s => s.GetStatusAsync(It.IsAny())).ReturnsAsync(status); - _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(ValidationResult.Incomplete); + _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(ValidationResult.Incomplete); var handler = new SymbolsValidatorMessageHandler(_logger.Object, _symbolService.Object, _validatorStateService.Object); @@ -122,7 +122,7 @@ public async Task IncompleteStateIsProcessed() // Assert Assert.False(result); - _symbolService.Verify(ss => ss.ValidateSymbolsAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + _symbolService.Verify(ss => ss.ValidateSymbolsAsync(It.IsAny(), It.IsAny()), Times.Once); } [Fact] @@ -134,7 +134,7 @@ public async Task IncompleteStateIsProcessedAndSavedOnSuccess() _validatorStateService.Setup(s => s.GetStatusAsync(It.IsAny())).ReturnsAsync(status); _validatorStateService.Setup(s => s.SaveStatusAsync(It.IsAny())).ReturnsAsync(SaveStatusResult.Success); - _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny(), It.IsAny())).ReturnsAsync(ValidationResult.Succeeded); + _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny())).ReturnsAsync(ValidationResult.Succeeded); var handler = new SymbolsValidatorMessageHandler(_logger.Object, _symbolService.Object, _validatorStateService.Object); @@ -156,7 +156,7 @@ public async Task IncompleteStateIsProcessedAndSavedOnFailed() _validatorStateService.Setup(s => s.GetStatusAsync(It.IsAny())).ReturnsAsync(status); _validatorStateService.Setup(s => s.SaveStatusAsync(It.IsAny())).ReturnsAsync(SaveStatusResult.Success); - _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny(), It.IsAny())). + _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny())). ReturnsAsync(ValidationResult.FailedWithIssues(ValidationIssue.Unknown)); var handler = new SymbolsValidatorMessageHandler(_logger.Object, _symbolService.Object, _validatorStateService.Object); @@ -180,7 +180,7 @@ public async Task ReturnsFalseIfFailToSaveInDB() _validatorStateService.Setup(s => s.GetStatusAsync(It.IsAny())).ReturnsAsync(status); _validatorStateService.Setup(s => s.SaveStatusAsync(It.IsAny())).ReturnsAsync(SaveStatusResult.StaleStatus); - _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny(), It.IsAny())). + _symbolService.Setup(s => s.ValidateSymbolsAsync(It.IsAny(), It.IsAny())). ReturnsAsync(ValidationResult.Succeeded); var handler = new SymbolsValidatorMessageHandler(_logger.Object, _symbolService.Object, _validatorStateService.Object); diff --git a/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs b/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs index df8e19eb7..4bb07f459 100644 --- a/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs +++ b/tests/Validation.Symbols.Tests/SymbolsValidatorServiceTests.cs @@ -18,6 +18,7 @@ public class SymbolsValidatorServiceTests { private const string PackageId = "Pack"; private const string PackageNormalizedVersion = "1.2.3"; + private static readonly SymbolsValidatorMessage Message = new SymbolsValidatorMessage(new Guid(), 1, PackageId, PackageNormalizedVersion, "https://dummy.snupkg"); public sealed class TheValidateSymbolsAsyncMethod : FactBase { @@ -26,13 +27,13 @@ public async Task ValidateSymbolsAsyncWillFailIfSnupkgNotFound() { // Arrange _symbolsFileService. - Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny(), It.IsAny())). - ThrowsAsync(new FileNotFoundException("Snupkg not found")); + Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny())). + ThrowsAsync(new InvalidOperationException("Snupkg not found")); var service = new SymbolsValidatorService(_symbolsFileService.Object, _zipService.Object, _telemetryService.Object, _logger.Object); // Act - var result = await service.ValidateSymbolsAsync(PackageId, PackageNormalizedVersion, CancellationToken.None); + var result = await service.ValidateSymbolsAsync(Message, CancellationToken.None); // Assert Assert.Equal(ValidationResult.Failed.Status, result.Status); @@ -47,13 +48,13 @@ public async Task ValidateSymbolsAsyncWillFailIfNupkgNotFound() ThrowsAsync(new FileNotFoundException("Nupkg not found")); _symbolsFileService. - Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny(), It.IsAny())). + Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny())). ReturnsAsync(new MemoryStream()); var service = new SymbolsValidatorService(_symbolsFileService.Object, _zipService.Object, _telemetryService.Object, _logger.Object); // Act - var result = await service.ValidateSymbolsAsync(PackageId, PackageNormalizedVersion, CancellationToken.None); + var result = await service.ValidateSymbolsAsync(Message, CancellationToken.None); // Assert Assert.Equal(ValidationResult.Failed.Status, result.Status); @@ -69,7 +70,7 @@ public async Task ValidateSymbolsAsyncWillFailIfTheSnupkgNotSubsetOfNupkg() ReturnsAsync(new MemoryStream()); _symbolsFileService. - Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny(), It.IsAny())). + Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny())). ReturnsAsync(new MemoryStream()); _zipService.Setup(s => s.ReadFilesFromZipStream(It.IsAny(), It.IsAny())).Returns(new List() @@ -81,7 +82,7 @@ public async Task ValidateSymbolsAsyncWillFailIfTheSnupkgNotSubsetOfNupkg() var service = new SymbolsValidatorService(_symbolsFileService.Object, _zipService.Object, _telemetryService.Object, _logger.Object); // Act - var result = await service.ValidateSymbolsAsync(PackageId, PackageNormalizedVersion, CancellationToken.None); + var result = await service.ValidateSymbolsAsync(Message, CancellationToken.None); // Assert Assert.Equal(ValidationResult.Failed.Status, result.Status); @@ -97,7 +98,7 @@ public async Task ValidateSymbolsAsyncWillSucceedOnCorrectMatch() ReturnsAsync(new MemoryStream()); _symbolsFileService. - Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny(), It.IsAny())). + Setup(sfs => sfs.DownloadSnupkgFileAsync(It.IsAny(), It.IsAny())). ReturnsAsync(new MemoryStream()); _zipService.Setup(s => s.ReadFilesFromZipStream(It.IsAny(), It.IsAny())).Returns(new List() @@ -109,7 +110,7 @@ public async Task ValidateSymbolsAsyncWillSucceedOnCorrectMatch() var service = new TestSymbolsValidatorService(_symbolsFileService.Object, _zipService.Object, _telemetryService.Object, _logger.Object); // Act - var result = await service.ValidateSymbolsAsync(PackageId, PackageNormalizedVersion, CancellationToken.None); + var result = await service.ValidateSymbolsAsync(Message, CancellationToken.None); // Assert Assert.Equal(ValidationResult.Succeeded.Status, result.Status);