From 2cf936fe74ccf11f10c404d8609b90a03f7d869b Mon Sep 17 00:00:00 2001 From: Shishir H Date: Mon, 5 Nov 2018 17:11:00 -0800 Subject: [PATCH] [Symbol server]Address UI fixes and fix re-upload on failed validation issues for Symbol Server (#6627) --- .../Services/SymbolPackageUploadService.cs | 19 ++++++- .../Views/Packages/DisplayPackage.cshtml | 16 +++--- .../SymbolPackageUploadServiceFacts.cs | 53 +++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/NuGetGallery/Services/SymbolPackageUploadService.cs b/src/NuGetGallery/Services/SymbolPackageUploadService.cs index 1dbaf994da..24f0c58747 100644 --- a/src/NuGetGallery/Services/SymbolPackageUploadService.cs +++ b/src/NuGetGallery/Services/SymbolPackageUploadService.cs @@ -146,6 +146,7 @@ public async Task CreateAndUploadSymbolsPackage(Package pac Stream symbolPackageFile = symbolPackageStream.AsSeekableStream(); + var previousSymbolsPackage = package.LatestSymbolPackage(); var symbolPackage = _symbolPackageService.CreateSymbolPackage(package, packageStreamMetadata); await _validationService.StartValidationAsync(symbolPackage); @@ -161,6 +162,16 @@ public async Task CreateAndUploadSymbolsPackage(Package pac { if (symbolPackage.StatusKey == PackageStatus.Validating) { + // If the last uploaded symbol package has failed validation, it will leave the snupkg in the + // validations container. We could possibly overwrite it, but that might introduce a concurrency + // issue on multiple snupkg uploads with a prior failed validation. The best thing to do would be + // to delete the failed validation snupkg from validations container and then proceed with normal + // upload. + if (previousSymbolsPackage != null && previousSymbolsPackage.StatusKey == PackageStatus.FailedValidation) + { + await DeleteSymbolsPackageAsync(previousSymbolsPackage); + } + await _symbolPackageFileService.SaveValidationPackageFileAsync(symbolPackage.Package, symbolPackageFile); } else if (symbolPackage.StatusKey == PackageStatus.Available) @@ -239,7 +250,13 @@ public async Task DeleteSymbolsPackageAsync(SymbolPackage symbolPackage) throw new ArgumentNullException(nameof(symbolPackage)); } - if (await _symbolPackageFileService.DoesPackageFileExistAsync(symbolPackage.Package)) + if (symbolPackage.StatusKey == PackageStatus.FailedValidation + && await _symbolPackageFileService.DoesValidationPackageFileExistAsync(symbolPackage.Package)) + { + await _symbolPackageFileService.DeleteValidationPackageFileAsync(symbolPackage.Id, symbolPackage.Version); + } + else if (symbolPackage.StatusKey == PackageStatus.Available + && await _symbolPackageFileService.DoesPackageFileExistAsync(symbolPackage.Package)) { await _symbolPackageFileService.DeletePackageFileAsync(symbolPackage.Id, symbolPackage.Version); } diff --git a/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml b/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml index 6e615a174c..9968291441 100644 --- a/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml +++ b/src/NuGetGallery/Views/Packages/DisplayPackage.cshtml @@ -275,9 +275,8 @@ { @ViewHelpers.AlertWarning( @ - The symbols for this package have not been indexed yet. They are not available - for download from the NuGet.org symbol server. Symbols will be indexed and will - be available for download after both validation and indexing are complete. + The symbols for this package have not been indexed yet. They are not available + for download from the NuGet.org symbol server. Symbols will be available for download after both validation and indexing are complete. Symbols validation and indexing may take up to an hour. Read more. ) @@ -721,10 +720,13 @@ Revalidate package -
  • - - Revalidate symbols -
    • + if (Model.LatestSymbolsPackage != null) + { +
  • + + Revalidate symbols +
    • + } } @if (User.IsAdministrator() && Config.Current.AsynchronousPackageValidationEnabled) diff --git a/tests/NuGetGallery.Facts/Services/SymbolPackageUploadServiceFacts.cs b/tests/NuGetGallery.Facts/Services/SymbolPackageUploadServiceFacts.cs index 7a60eccb20..4c48e8f8f6 100644 --- a/tests/NuGetGallery.Facts/Services/SymbolPackageUploadServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/SymbolPackageUploadServiceFacts.cs @@ -264,6 +264,59 @@ public async Task WillReturnConflictIfFileExistsInContainer() symbolPackageFileService.Verify(x => x.SavePackageFileAsync(package, It.IsAny(), It.IsAny()), Times.Once); } + [Fact] + public async Task WillDeleteFailedValidationSnupkg() + { + // Arrange + var symbolPackageService = new Mock(); + symbolPackageService + .Setup(x => x.CreateSymbolPackage(It.IsAny(), It.IsAny())) + .Returns((Package package1, PackageStreamMetadata streamMetadata) => + { + var symbolPackage = new SymbolPackage() + { + Package = package1, + PackageKey = package1.Key, + Created = DateTime.UtcNow, + StatusKey = PackageStatus.Validating + }; + + return symbolPackage; + }) + .Verifiable(); + + var symbolPackageFileService = new Mock(); + symbolPackageFileService + .Setup(x => x.DoesValidationPackageFileExistAsync(It.IsAny())) + .ReturnsAsync(true) + .Verifiable(); + symbolPackageFileService + .Setup(x => x.DeleteValidationPackageFileAsync(It.IsAny(), It.IsAny())) + .Completes() + .Verifiable(); + symbolPackageFileService + .Setup(x => x.SaveValidationPackageFileAsync(It.IsAny(), It.IsAny())) + .Completes() + .Verifiable(); + + var service = CreateService(symbolPackageService: symbolPackageService, symbolPackageFileService: symbolPackageFileService); + var package = new Package() { PackageRegistration = new PackageRegistration() { Id = "theId" }, Version = "1.0.23" }; + package.SymbolPackages.Add(new SymbolPackage() + { + StatusKey = PackageStatus.FailedValidation, + Key = 1232, + Package = package + }); + + // Act + var result = await service.CreateAndUploadSymbolsPackage(package, new MemoryStream()); + + // Assert + Assert.NotNull(result); + Assert.Equal(PackageCommitResult.Success, result); + symbolPackageFileService.VerifyAll(); + } + [Fact] public async Task WillDeleteSavedFileAndThrowWhenDBWriteFails() {