[Symbol server]Address UI fixes and fix re-upload on failed validatio…

…n issues for Symbol Server (#6627)
NuGet · Nov 6, 2018 · 2cf936f · 2cf936f
1 parent a053203
commit 2cf936f
Show file tree

Hide file tree

Showing 3 changed files with 80 additions and 8 deletions.
diff --git a/src/NuGetGallery/Services/SymbolPackageUploadService.cs b/src/NuGetGallery/Services/SymbolPackageUploadService.cs
@@ -146,6 +146,7 @@ public async Task<PackageCommitResult> 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<PackageCommitResult> 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
@@ -275,9 +275,8 @@
                 {
                     @ViewHelpers.AlertWarning(
                         @<text>
-                            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. 
+                            <strong>The symbols for this package have not been indexed yet.</strong> 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. <a href="https://aka.ms/NuGetSymbolsPackageValidation">Read more</a>.
                         </text>
                      )
@@ -721,10 +720,13 @@
                         <a href="@Url.RevalidatePackage(Model)">Revalidate package</a>
                     </li>
 
-                    <li>
-                        <i class="ms-Icon ms-Icon--Refresh" aria-hidden="true"></i>
-                        <a href="@Url.RevalidateSymbolsPackage(Model)">Revalidate symbols</a>
-                    </li>
+                    if (Model.LatestSymbolsPackage != null)
+                    {
+                        <li>
+                            <i class="ms-Icon ms-Icon--Refresh" aria-hidden="true"></i>
+                            <a href="@Url.RevalidateSymbolsPackage(Model)">Revalidate symbols</a>
+                        </li>
+                    }
                 }
 
                 @if (User.IsAdministrator() && Config.Current.AsynchronousPackageValidationEnabled)

diff --git 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<Stream>(), It.IsAny<bool>()), Times.Once);
             }
 
+            [Fact]
+            public async Task WillDeleteFailedValidationSnupkg()
+            {
+                // Arrange
+                var symbolPackageService = new Mock<ISymbolPackageService>();
+                symbolPackageService
+                    .Setup(x => x.CreateSymbolPackage(It.IsAny<Package>(), It.IsAny<PackageStreamMetadata>()))
+                    .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<ISymbolPackageFileService>();
+                symbolPackageFileService
+                    .Setup(x => x.DoesValidationPackageFileExistAsync(It.IsAny<Package>()))
+                    .ReturnsAsync(true)
+                    .Verifiable();
+                symbolPackageFileService
+                    .Setup(x => x.DeleteValidationPackageFileAsync(It.IsAny<string>(), It.IsAny<string>()))
+                    .Completes()
+                    .Verifiable();
+                symbolPackageFileService
+                    .Setup(x => x.SaveValidationPackageFileAsync(It.IsAny<Package>(), It.IsAny<Stream>()))
+                    .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()
             {