Unlisting a package when deprecating it should update LastEdited (#6955)

Author: Scott Bommarito

src/NuGetGallery/Services/PackageDeprecationService.cs

@@ -15,15 +15,21 @@ public class PackageDeprecationService : IPackageDeprecationService
         private readonly IEntityRepository<PackageDeprecation> _deprecationRepository;
         private readonly IEntityRepository<Cve> _cveRepository;
         private readonly IEntityRepository<Cwe> _cweRepository;
+        private readonly IPackageService _packageService;
+        private readonly IIndexingService _indexingService;
 
         public PackageDeprecationService(
            IEntityRepository<PackageDeprecation> deprecationRepository,
            IEntityRepository<Cve> cveRepository,
-           IEntityRepository<Cwe> cweRepository)
+           IEntityRepository<Cwe> cweRepository,
+           IPackageService packageService,
+           IIndexingService indexingService)
         {
             _deprecationRepository = deprecationRepository ?? throw new ArgumentNullException(nameof(deprecationRepository));
             _cveRepository = cveRepository ?? throw new ArgumentNullException(nameof(cveRepository));
             _cweRepository = cweRepository ?? throw new ArgumentNullException(nameof(cweRepository));
+            _packageService = packageService ?? throw new ArgumentNullException(nameof(packageService));
+            _indexingService = indexingService ?? throw new ArgumentNullException(nameof(indexingService));
         }
 
         public async Task UpdateDeprecation(
@@ -101,7 +107,7 @@ public async Task UpdateDeprecation(
 
                     if (shouldUnlist)
                     {
-                        package.Listed = false;
+                        await _packageService.MarkPackageUnlistedAsync(package, commitChanges: false);
                     }
                 }
             }
@@ -116,6 +122,12 @@ public async Task UpdateDeprecation(
             }
 
             await _deprecationRepository.CommitChangesAsync();
+
+            // Update the indexing of the packages we updated the deprecation information of.
+            foreach (var package in packages)
+            {
+                _indexingService.UpdatePackage(package);
+            }
         }
 
         public async Task<IReadOnlyCollection<Cve>> GetOrCreateCvesByIdAsync(IEnumerable<string> ids, bool commitChanges)

tests/NuGetGallery.Facts/Services/PackageDeprecationServiceFacts.cs

@@ -77,8 +77,10 @@ await Assert.ThrowsAsync<ArgumentNullException>(() =>
                         false));
             }
 
-            [Fact]
-            public async Task DeletesExistingDeprecationsIfStatusNotDeprecated()
+            [Theory]
+            [InlineData(false)]
+            [InlineData(true)]
+            public async Task DeletesExistingDeprecationsIfStatusNotDeprecated(bool shouldUnlist)
             {
                 // Arrange
                 var packageWithDeprecation1 = new Package
@@ -114,6 +116,20 @@ public async Task DeletesExistingDeprecationsIfStatusNotDeprecated()
                     .Completes()
                     .Verifiable();
 
+                var packageService = GetMock<IPackageService>();
+                var indexingService = GetMock<IIndexingService>();
+                foreach (var package in packages)
+                {
+                    // When deleting deprecations, packages should not be unlisted because the option is hidden in the UI.
+                    packageService
+                        .Setup(x => x.MarkPackageUnlistedAsync(package, false))
+                        .Throws<InvalidOperationException>();
+
+                    indexingService
+                        .Setup(x => x.UpdatePackage(package))
+                        .Verifiable();
+                }
+
                 var service = Get<PackageDeprecationService>();
 
                 // Act
@@ -126,10 +142,11 @@ await service.UpdateDeprecation(
                     null,
                     null,
                     null,
-                    false);
+                    shouldUnlist);
 
                 // Assert
                 deprecationRepository.Verify();
+                indexingService.Verify();
 
                 foreach (var package in packages)
                 {
@@ -143,22 +160,27 @@ await service.UpdateDeprecation(
             public async Task ReplacesExistingDeprecations(bool shouldUnlist)
             {
                 // Arrange
-                var unlistedPackageWithoutDeprecation = new Package();
+                var lastEdited = new DateTime(2019, 3, 4);
+
+                var unlistedPackageWithoutDeprecation = new Package
+                {
+                    LastEdited = lastEdited
+                };
 
                 var packageWithDeprecation1 = new Package
                 {
-                    Listed = true,
-                    Deprecations = new List<PackageDeprecation> { new PackageDeprecation() }
+                    Deprecations = new List<PackageDeprecation> { new PackageDeprecation() },
+                    LastEdited = lastEdited
                 };
 
                 var packageWithoutDeprecation1 = new Package
                 {
-                    Listed = true,
+                    LastEdited = lastEdited
                 };
 
                 var packageWithDeprecation2 = new Package
                 {
-                    Listed = true,
+                    LastEdited = lastEdited,
                     Deprecations = new List<PackageDeprecation>
                     {
                         new PackageDeprecation
@@ -176,12 +198,12 @@ public async Task ReplacesExistingDeprecations(bool shouldUnlist)
 
                 var packageWithoutDeprecation2 = new Package
                 {
-                    Listed = true,
+                    LastEdited = lastEdited
                 };
 
                 var packageWithDeprecation3 = new Package
                 {
-                    Listed = true,
+                    LastEdited = lastEdited,
                     Deprecations = new List<PackageDeprecation>
                     {
                         new PackageDeprecation
@@ -221,6 +243,30 @@ public async Task ReplacesExistingDeprecations(bool shouldUnlist)
                     .Completes()
                     .Verifiable();
 
+                var packageService = GetMock<IPackageService>();
+                var indexingService = GetMock<IIndexingService>();
+                foreach (var package in packages)
+                {
+                    var unlistPackageSetup = packageService
+                        .Setup(x => x.MarkPackageUnlistedAsync(package, false));
+
+                    if (shouldUnlist)
+                    {
+                        unlistPackageSetup
+                            .Completes()
+                            .Verifiable();
+                    }
+                    else
+                    {
+                        unlistPackageSetup
+                            .Throws<InvalidOperationException>();
+                    }
+
+                    indexingService
+                        .Setup(x => x.UpdatePackage(package))
+                        .Verifiable();
+                }
+
                 var service = Get<PackageDeprecationService>();
 
                 var status = (PackageDeprecationStatus)99;
@@ -272,6 +318,8 @@ await service.UpdateDeprecation(
 
                 // Assert
                 deprecationRepository.Verify();
+                packageService.Verify();
+                indexingService.Verify();
 
                 foreach (var package in packages)
                 {
@@ -283,15 +331,6 @@ await service.UpdateDeprecation(
                     Assert.Equal(alternatePackageRegistration, deprecation.AlternatePackageRegistration);
                     Assert.Equal(alternatePackage, deprecation.AlternatePackage);
                     Assert.Equal(customMessage, deprecation.CustomMessage);
-                    
-                    if (shouldUnlist)
-                    {
-                        Assert.False(package.Listed);
-                    }
-                    else if (package != unlistedPackageWithoutDeprecation)
-                    {
-                        Assert.True(package.Listed);
-                    }
                 }
             }
         }