Stage 1 of Package Reference Compatibility Validation #346
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,109 @@ | ||
| // 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.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.Extensions.Logging; | ||
| using NuGet.Common; | ||
| using NuGet.Jobs.Validation; | ||
| using NuGet.Jobs.Validation.PackageSigning.Storage; | ||
| using Validation.PackageCompatibility.Core.Messages; | ||
| using Validation.PackageCompatibility.Core.Storage; | ||
|
|
||
| namespace NuGet.Services.Validation.PackageCompatibility | ||
| { | ||
| /// <summary> | ||
| /// Configuration for initializing the <see cref="PackageCompatibilityValidator"/>. | ||
| /// </summary> | ||
| public class PackageCompatibilityValidator : IValidator | ||
| { | ||
| private IValidatorStateService _validatorStateService; | ||
| private IPackageCompatibilityService _packageCompatibilityService; | ||
| private readonly ILogger<PackageCompatibilityValidator> _logger; | ||
|
|
||
| private IPackageDownloader _packageDownloader; | ||
|
|
||
| public PackageCompatibilityValidator( | ||
| IValidatorStateService validatorStateService, | ||
| IPackageCompatibilityService packageCompatibilityService, | ||
| IPackageDownloader packageDownloader, | ||
| ILogger<PackageCompatibilityValidator> logger) | ||
| { | ||
| _validatorStateService = validatorStateService; | ||
|
|
||
| _packageCompatibilityService = packageCompatibilityService; | ||
| _packageDownloader = packageDownloader; | ||
| _logger = logger; | ||
| } | ||
|
|
||
| public async Task<IValidationResult> GetResultAsync(IValidationRequest request) | ||
| { | ||
| var validatorStatus = await _validatorStateService.GetStatusAsync(request); | ||
|
|
||
| return validatorStatus.ToValidationResult(); | ||
| } | ||
|
|
||
| public async Task<IValidationResult> StartAsync(IValidationRequest request) | ||
| { | ||
| try | ||
| { | ||
| var validatorStatus = await _validatorStateService.GetStatusAsync(request); | ||
|
|
||
| if (validatorStatus.State != ValidationStatus.NotStarted) | ||
| { | ||
| _logger.LogWarning( | ||
| "Package Compatibility validation with validationId {ValidationId} ({PackageId} {PackageVersion}) has already started.", | ||
| request.ValidationId, | ||
| request.PackageId, | ||
| request.PackageVersion); | ||
|
|
||
| return validatorStatus.ToValidationResult(); | ||
| } | ||
|
|
||
| try | ||
| { | ||
| await Validate(request, CancellationToken.None); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| _logger.LogWarning(0, e, "Validation failed in the validator for the following ValidationId {ValidationId}", request.ValidationId); | ||
| } | ||
|
|
||
| // Treat every validation as succeeded, as we don't want to block | ||
| validatorStatus.State = ValidationStatus.Succeeded; | ||
|
|
||
| await _validatorStateService.SaveStatusAsync(validatorStatus); | ||
|
|
||
| return validatorStatus.ToValidationResult(); | ||
| } | ||
| catch (Exception e) | ||
| { | ||
| _logger.LogWarning(0, e, "Validation failed for the following ValidationId {ValidationId}", request.ValidationId); | ||
| } | ||
|
|
||
| return new ValidationResult(ValidationStatus.Succeeded); | ||
| } | ||
|
|
||
| private async Task Validate(IValidationRequest request, CancellationToken cancellationToken) | ||
| { | ||
| using (var packageStream = await _packageDownloader.DownloadAsync(new Uri(request.NupkgUrl), cancellationToken)) | ||
| using (var package = new Packaging.PackageArchiveReader(packageStream)) | ||
| { | ||
| var warnings = new List<PackLogMessage>(); | ||
|
|
||
| foreach (var rule in Packaging.Rules.DefaultPackageRuleSet.Rules) | ||
| { | ||
| warnings.AddRange(rule.Validate(package)); | ||
| } | ||
|
|
||
| await _packageCompatibilityService.SetPackageCompatibilityState(request.ValidationId, warnings); | ||
| } | ||
| } | ||
|
|
||
| public Task CleanUpAsync(IValidationRequest request) | ||
| { | ||
| return Task.CompletedTask; | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| // 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 NuGet.Common; | ||
| using System; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Text; | ||
| using System.Threading.Tasks; | ||
|
|
||
| namespace Validation.PackageCompatibility.Core.Storage | ||
| { | ||
| public interface IPackageCompatibilityService | ||
| { | ||
| Task SetPackageCompatibilityState( | ||
| Guid validationId, | ||
| IEnumerable<PackLogMessage> messages); | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| // 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; | ||
|
|
||
| namespace Validation.PackageCompatibility.Core.Messages | ||
| { | ||
| public class PackageCompatibilityValidationMessage | ||
| { | ||
| public PackageCompatibilityValidationMessage(string packageId, string packageVersion, Uri nupkgUri, Guid validationId) | ||
|
There was a problem hiding this comment.
Is this just used for the PackageCompatibilityValidator.Validate method? A public class seems like over kill, maybe a private class in PackageCompatibilityValidator would be better. Also, are all of these parameters used? It seems like PackageId is not. There was a problem hiding this comment. In phase 2, this validation will run in it's own proc, and it'll be used there for logging and better visibility, analogous to the way the package id/version are used in the signing validation. |
||
| { | ||
| if (validationId == Guid.Empty) | ||
| { | ||
| throw new ArgumentOutOfRangeException(nameof(validationId)); | ||
| } | ||
| ValidationId = validationId; | ||
| PackageId = packageId ?? throw new ArgumentNullException(nameof(packageId)); | ||
| PackageVersion = packageVersion ?? throw new ArgumentNullException(nameof(packageVersion)); | ||
| NupkgUri = nupkgUri ?? throw new ArgumentNullException(nameof(nupkgUri)); | ||
| } | ||
|
|
||
| public string PackageId { get; } | ||
| public string PackageVersion { get; } | ||
| public Uri NupkgUri { get; } | ||
| public Guid ValidationId { get; } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validators that are outside of the
NuGet.Services.Validation.Orchestratorproject are not supported today. This is a low-priority "bug" in the Orchestrator that we will fix in the future. The type that extendsIValidatorwill need to be moved, the implementation can be kept here.See this:
NuGet.Jobs/src/NuGet.Services.Validation.Orchestrator/ValidatorProvider.cs
Line 27 in 9722ac9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's useful feedback.
This is currently in the Orchestrator project though, just making sure you noticed that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. I can't read :)