Merge branch 'dev' into dev-pkgdeprecation

xavierdecoster · xavierdecoster · commit 05e70ea1070f · 2019-04-29T22:24:23.000+02:00
diff --git a/src/NuGet.Services.Entities/AccountDelete.cs b/src/NuGet.Services.Entities/AccountDelete.cs
@@ -28,13 +28,19 @@ public class AccountDelete
         public User DeletedAccount { get; set; }
 
         /// <summary>
-        /// The User(admin) key that executed the delete action.
+        /// The User key that executed the delete action.
         /// </summary>
-        public int DeletedByKey { get; set; }
+        /// <remarks>
+        /// <c>null</c> if the user was deleted.
+        /// </remarks>
+        public int? DeletedByKey { get; set; }
 
         /// <summary>
-        /// The User(admin) that executed the delete action.
+        /// The User that executed the delete action.
         /// </summary>
+        /// <remarks>
+        /// <c>null</c> if the user was deleted.
+        /// </remarks>
         public User DeletedBy { get; set; }
 
          /// <summary>
diff --git a/src/NuGet.Services.Entities/Package.cs b/src/NuGet.Services.Entities/Package.cs
@@ -217,9 +217,11 @@ public bool HasReadMe
         public string MinClientVersion { get; set; }
 
         /// <summary>
-        /// The logged in user when this package version was created.
-        /// NULL for older packages.
+        /// The user that uploaded this package or <c>null</c> if the user was deleted.
         /// </summary>
+        /// <remarks>
+        /// Packages uploaded before this field was added have <c>null</c>.
+        /// </remarks>
         public User User { get; set; }
         public int? UserKey { get; set; }
 
diff --git a/src/NuGet.Services.Entities/PackageDelete.cs b/src/NuGet.Services.Entities/PackageDelete.cs
@@ -20,9 +20,15 @@ public PackageDelete()
         [Required]
         public DateTime DeletedOn { get; set; }
 
-        [Required]
+        /// <remarks>
+        /// <c>null</c> if the user was deleted.
+        /// </remarks>
         public User DeletedBy { get; set; }
-        public int DeletedByKey { get; set; }
+
+        /// <remarks>
+        /// <c>null</c> if the user was deleted.
+        /// </remarks>
+        public int? DeletedByKey { get; set; }
 
         [Required]
         public string Reason { get; set; }
diff --git a/src/NuGet.Services.Entities/PackageDeprecation.cs b/src/NuGet.Services.Entities/PackageDeprecation.cs
@@ -69,6 +69,9 @@ public PackageDeprecation()
         /// <summary>
         /// Gets or sets the user that executed the package deprecation.
         /// </summary>
+        /// <remarks>
+        /// This field will be <c>null</c> if the user that deprecated the package has been deleted.
+        /// </remarks>
         public virtual User DeprecatedByUser { get; set; }
 
         /// <summary>
diff --git a/src/NuGetGallery.Core/Entities/EntitiesContext.cs b/src/NuGetGallery.Core/Entities/EntitiesContext.cs
@@ -49,6 +49,7 @@ public EntitiesContext(DbConnection connection, bool readOnly)
         }
 
         public bool ReadOnly { get; private set; }
+        public DbSet<Package> Packages { get; set; }
         public DbSet<PackageRegistration> PackageRegistrations { get; set; }
         public DbSet<Credential> Credentials { get; set; }
         public DbSet<Scope> Scopes { get; set; }
@@ -353,7 +354,7 @@ protected override void OnModelCreating(DbModelBuilder modelBuilder)
                 .HasRequired(a => a.DeletedAccount);
 
             modelBuilder.Entity<AccountDelete>()
-                .HasRequired(a => a.DeletedBy)
+                .HasOptional(a => a.DeletedBy)
                 .WithMany()
                 .WillCascadeOnDelete(false);
 
diff --git a/src/NuGetGallery.Core/Entities/IEntitiesContext.cs b/src/NuGetGallery.Core/Entities/IEntitiesContext.cs
@@ -10,6 +10,7 @@ namespace NuGetGallery
     public interface IEntitiesContext
     {
         DbSet<Certificate> Certificates { get; set; }
+        DbSet<Package> Packages { get; set; }
         DbSet<PackageRegistration> PackageRegistrations { get; set; }
         DbSet<Credential> Credentials { get; set; }
         DbSet<Scope> Scopes { get; set; }
diff --git a/src/NuGetGallery/Migrations/201904252152567_DeletedByOptional.Designer.cs b/src/NuGetGallery/Migrations/201904252152567_DeletedByOptional.Designer.cs
diff --git a/src/NuGetGallery/Migrations/201904252152567_DeletedByOptional.cs b/src/NuGetGallery/Migrations/201904252152567_DeletedByOptional.cs
@@ -0,0 +1,32 @@
+namespace NuGetGallery.Migrations
+{
+    using System;
+    using System.Data.Entity.Migrations;
+    
+    public partial class DeletedByOptional : DbMigration
+    {
+        public override void Up()
+        {
+            DropForeignKey("dbo.PackageDeletes", "DeletedByKey", "dbo.Users");
+            DropIndex("dbo.PackageDeletes", new[] { "DeletedByKey" });
+            DropIndex("dbo.AccountDeletes", new[] { "DeletedByKey" });
+            AlterColumn("dbo.PackageDeletes", "DeletedByKey", c => c.Int());
+            AlterColumn("dbo.AccountDeletes", "DeletedByKey", c => c.Int());
+            CreateIndex("dbo.PackageDeletes", "DeletedByKey");
+            CreateIndex("dbo.AccountDeletes", "DeletedByKey");
+            AddForeignKey("dbo.PackageDeletes", "DeletedByKey", "dbo.Users", "Key");
+        }
+        
+        public override void Down()
+        {
+            DropForeignKey("dbo.PackageDeletes", "DeletedByKey", "dbo.Users");
+            DropIndex("dbo.AccountDeletes", new[] { "DeletedByKey" });
+            DropIndex("dbo.PackageDeletes", new[] { "DeletedByKey" });
+            AlterColumn("dbo.AccountDeletes", "DeletedByKey", c => c.Int(nullable: false));
+            AlterColumn("dbo.PackageDeletes", "DeletedByKey", c => c.Int(nullable: false));
+            CreateIndex("dbo.AccountDeletes", "DeletedByKey");
+            CreateIndex("dbo.PackageDeletes", "DeletedByKey");
+            AddForeignKey("dbo.PackageDeletes", "DeletedByKey", "dbo.Users", "Key", cascadeDelete: true);
+        }
+    }
+}
diff --git a/src/NuGetGallery/Migrations/201904252152567_DeletedByOptional.resx b/src/NuGetGallery/Migrations/201904252152567_DeletedByOptional.resx
diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj
@@ -242,6 +242,14 @@
     <Compile Include="Migrations\201904251233185_RemoveVulnerabilityDataFromPackageDeprecationModel.Designer.cs">
       <DependentUpon>201904251233185_RemoveVulnerabilityDataFromPackageDeprecationModel.cs</DependentUpon>
     </Compile>
+    <Compile Include="Migrations\201904252152567_DeletedByOptional.cs" />
+    <Compile Include="Migrations\201904252152567_DeletedByOptional.Designer.cs">
+      <DependentUpon>201904252152567_DeletedByOptional.cs</DependentUpon>
+    </Compile>
+    <Compile Include="Queries\AutocompleteCveIdQueryResults.cs" />
+    <Compile Include="Queries\AutocompleteCweIdQueryResults.cs" />
+    <Compile Include="Queries\AutocompleteCveIdQueryResult.cs" />
+    <Compile Include="Queries\AutocompleteCweIdQueryResult.cs" />
     <Compile Include="Services\IPackageDeprecationService.cs" />
     <Compile Include="Queries\AutocompleteDatabasePackageIdsQuery.cs" />
     <Compile Include="Queries\AutocompleteDatabasePackageVersionsQuery.cs" />
@@ -1715,6 +1723,8 @@
     </EmbeddedResource>
     <EmbeddedResource Include="Migrations\201904251233185_RemoveVulnerabilityDataFromPackageDeprecationModel.resx">
       <DependentUpon>201904251233185_RemoveVulnerabilityDataFromPackageDeprecationModel.cs</DependentUpon>
+    <EmbeddedResource Include="Migrations\201904252152567_DeletedByOptional.resx">
+      <DependentUpon>201904252152567_DeletedByOptional.cs</DependentUpon>
     </EmbeddedResource>
     <EmbeddedResource Include="OData\QueryAllowed\Data\apiv1packages.json" />
     <EmbeddedResource Include="OData\QueryAllowed\Data\apiv1search.json" />
diff --git a/src/NuGetGallery/Services/DeleteAccountService.cs b/src/NuGetGallery/Services/DeleteAccountService.cs
@@ -3,7 +3,6 @@
 
 using System;
 using System.Collections.Generic;
-using System.Data.Entity;
 using System.Globalization;
 using System.Linq;
 using System.Threading.Tasks;
@@ -20,12 +19,14 @@ namespace NuGetGallery
     public class DeleteAccountService : IDeleteAccountService
     {
         private readonly IEntityRepository<AccountDelete> _accountDeleteRepository;
+        private readonly IEntityRepository<PackageDelete> _packageDeleteRepository;
         private readonly IEntitiesContext _entitiesContext;
         private readonly IPackageService _packageService;
         private readonly IPackageOwnershipManagementService _packageOwnershipManagementService;
         private readonly IReservedNamespaceService _reservedNamespaceService;
         private readonly ISecurityPolicyService _securityPolicyService;
         private readonly AuthenticationService _authService;
+        private readonly IEntityRepository<PackageDeprecation> _deprecationRepository;
         private readonly IEntityRepository<User> _userRepository;
         private readonly IEntityRepository<Scope> _scopeRepository;
         private readonly ISupportRequestService _supportRequestService;
@@ -35,6 +36,8 @@ public class DeleteAccountService : IDeleteAccountService
 
         public DeleteAccountService(
             IEntityRepository<AccountDelete> accountDeleteRepository,
+            IEntityRepository<PackageDelete> packageDeleteRepository,
+            IEntityRepository<PackageDeprecation> deprecationRepository,
             IEntityRepository<User> userRepository,
             IEntityRepository<Scope> scopeRepository,
             IEntitiesContext entitiesContext,
@@ -49,6 +52,8 @@ public DeleteAccountService(
             ITelemetryService telemetryService)
         {
             _accountDeleteRepository = accountDeleteRepository ?? throw new ArgumentNullException(nameof(accountDeleteRepository));
+            _packageDeleteRepository = packageDeleteRepository ?? throw new ArgumentNullException(nameof(packageDeleteRepository));
+            _deprecationRepository = deprecationRepository ?? throw new ArgumentNullException(nameof(deprecationRepository));
             _userRepository = userRepository ?? throw new ArgumentNullException(nameof(userRepository));
             _scopeRepository = scopeRepository ?? throw new ArgumentNullException(nameof(scopeRepository));
             _entitiesContext = entitiesContext ?? throw new ArgumentNullException(nameof(entitiesContext));
@@ -109,6 +114,10 @@ private async Task DeleteAccountImplAsync(User userToBeDeleted, User userToExecu
             await RemoveSecurityPolicies(userToBeDeleted);
             await RemoveUserCredentials(userToBeDeleted);
             await RemovePackageOwnershipRequests(userToBeDeleted);
+            ResetPackagesAndAccountsDeletedBy(userToBeDeleted);
+
+            RemovePackagePushedBy(userToBeDeleted);
+            RemovePackageDeprecatedBy(userToBeDeleted);
 
             var organizationToBeDeleted = userToBeDeleted as Organization;
             if (organizationToBeDeleted != null)
@@ -209,6 +218,17 @@ private async Task RemovePackageOwnership(User user, User requestingUser, Accoun
             }
         }
 
+        private void RemovePackagePushedBy(User user)
+        {
+            foreach (var package in _entitiesContext
+                .Packages
+                .Where(p => p.UserKey == user.Key)
+                .ToList())
+            {
+                package.User = null;
+            }
+        }
+
         private List<Package> GetPackagesOwnedByUser(User user)
         {
             return _packageService
@@ -218,15 +238,32 @@ private List<Package> GetPackagesOwnedByUser(User user)
 
         private async Task RemovePackageOwnershipRequests(User user)
         {
-            var requests = _packageOwnershipManagementService
+            var toRequests = _packageOwnershipManagementService
                 .GetPackageOwnershipRequests(newOwner: user)
                 .ToList();
 
+            var fromRequests = _packageOwnershipManagementService
+                .GetPackageOwnershipRequests(requestingOwner: user)
+                .ToList();
+
+            var requests = toRequests.Concat(fromRequests).ToList();
+
             foreach (var request in requests)
             {
                 await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(request.PackageRegistration, request.NewOwner, commitChanges: false);
             }
         }
+
+        private void RemovePackageDeprecatedBy(User user)
+        {
+            foreach (var deprecation in _deprecationRepository
+                .GetAll()
+                .Where(d => d.DeprecatedByUserKey == user.Key)
+                .ToList())
+            {
+                deprecation.DeprecatedByUser = null;
+            }
+        }
         
         private async Task RemoveMemberships(User user, User requestingUser, AccountDeletionOrphanPackagePolicy orphanPackagePolicy)
         {
@@ -285,6 +322,25 @@ private void RemoveMembers(Organization organization)
             }
         }
 
+        private void ResetPackagesAndAccountsDeletedBy(User user)
+        {
+            foreach (var deletedPackage in _packageDeleteRepository
+                .GetAll()
+                .Where(d => d.DeletedByKey == user.Key)
+                .ToList())
+            {
+                deletedPackage.DeletedBy = null;
+            }
+
+            foreach (var deletedAccount in _accountDeleteRepository
+                .GetAll()
+                .Where(d => d.DeletedByKey == user.Key)
+                .ToList())
+            {
+                deletedAccount.DeletedBy = null;
+            }
+        }
+
         private void RemoveUserDataInUserTable(User user)
         {
             user.SetAccountAsDeleted();
diff --git a/src/NuGetGallery/Services/PackageService.cs b/src/NuGetGallery/Services/PackageService.cs
@@ -368,6 +368,7 @@ public async Task RemovePackageOwnerAsync(PackageRegistration package, User user
         {
             // To support the delete account scenario, the admin can delete the last owner of a package.
             package.Owners.Remove(user);
+
             if (commitChanges)
             {
                 await _packageRepository.CommitChangesAsync();
diff --git a/tests/NuGetGallery.Facts/Services/DeleteAccountServiceFacts.cs b/tests/NuGetGallery.Facts/Services/DeleteAccountServiceFacts.cs
diff --git a/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/PackageServiceFacts.cs

Original file line number	Diff line number	Diff line change
`@@ -10,6 +10,7 @@ namespace NuGetGallery`
`10`	`10`	`public interface IEntitiesContext`
`11`	`11`	`{`
`12`	`12`	`DbSet<Certificate> Certificates { get; set; }`
	`13`	`+ DbSet<Package> Packages { get; set; }`
`13`	`14`	`DbSet<PackageRegistration> PackageRegistrations { get; set; }`
`14`	`15`	`DbSet<Credential> Credentials { get; set; }`
`15`	`16`	`DbSet<Scope> Scopes { get; set; }`