Only commit once in DeleteAccountService.DeleteAccountAsync (#7034)

Author: Scott Bommarito

src/NuGetGallery/Areas/Admin/Controllers/ReservedNamespaceController.cs

@@ -101,7 +101,7 @@ public async Task<JsonResult> RemoveOwner(ReservedNamespace prefix, string owner
         {
             try
             {
-                await _reservedNamespaceService.DeleteOwnerFromReservedNamespaceAsync(prefix.Value, owner, commitAsTransaction:true);
+                await _reservedNamespaceService.DeleteOwnerFromReservedNamespaceAsync(prefix.Value, owner, commitChanges: true);
                 return Json(new { success = true, message = string.Format(Strings.ReservedNamespace_OwnerRemoved, owner, prefix.Value) });
             }
             catch (Exception ex) when (ex is InvalidOperationException || ex is ArgumentException)

src/NuGetGallery/Authentication/AuthenticationService.cs

@@ -599,12 +599,16 @@ public virtual CredentialViewModel DescribeCredential(Credential credential)
             return credentialViewModel;
         }
 
-        public virtual async Task RemoveCredential(User user, Credential cred)
+        public virtual async Task RemoveCredential(User user, Credential cred, bool commitChanges = true)
         {
             await Auditing.SaveAuditRecordAsync(new UserAuditRecord(user, AuditedUserAction.RemoveCredential, cred));
             user.Credentials.Remove(cred);
             Entities.Credentials.Remove(cred);
-            await Entities.SaveChangesAsync();
+
+            if (commitChanges)
+            {
+                await Entities.SaveChangesAsync();
+            }
         }
 
         public virtual async Task EditCredentialScopes(User user, Credential cred, ICollection<Scope> newScopes)

src/NuGetGallery/Controllers/JsonApiController.cs

@@ -203,7 +203,7 @@ public async Task<JsonResult> RemovePackageOwner(string id, string username)
                         return Json(new { success = false, message = "You can't remove the only owner from a package." }, JsonRequestBehavior.AllowGet);
                     }
 
-                    await _packageOwnershipManagementService.RemovePackageOwnerAsync(model.Package, model.CurrentUser, model.User, commitAsTransaction: true);
+                    await _packageOwnershipManagementService.RemovePackageOwnerAsync(model.Package, model.CurrentUser, model.User, commitChanges: true);
 
                     var emailMessage = new PackageOwnerRemovedMessage(_appConfiguration, model.CurrentUser, model.User, model.Package);
                     await _messageService.SendMessageAsync(emailMessage);

src/NuGetGallery/Controllers/OrganizationsController.cs

@@ -367,7 +367,7 @@ public override async Task<ActionResult> RequestAccountDeletion(string accountNa
                 return RedirectToAction(nameof(DeleteRequest));
             }
 
-            var result = await DeleteAccountService.DeleteAccountAsync(account, currentUser, commitAsTransaction: true);
+            var result = await DeleteAccountService.DeleteAccountAsync(account, currentUser);
 
             if (result.Success)
             {

src/NuGetGallery/Controllers/UsersController.cs

@@ -333,8 +333,7 @@ public override async Task<ActionResult> RequestAccountDeletion(string accountNa
                 // Unconfirmed users can be deleted immediately without creating a support request.
                 DeleteUserAccountStatus accountDeleteStatus = await _deleteAccountService.DeleteAccountAsync(userToBeDeleted: user,
                     userToExecuteTheDelete: user,
-                    orphanPackagePolicy: AccountDeletionOrphanPackagePolicy.UnlistOrphans,
-                    commitAsTransaction: true);
+                    orphanPackagePolicy: AccountDeletionOrphanPackagePolicy.UnlistOrphans);
                 if (!accountDeleteStatus.Success)
                 {
                     TempData["RequestFailedMessage"] = Strings.AccountSelfDelete_Fail;
@@ -393,8 +392,7 @@ public virtual async Task<ActionResult> Delete(DeleteAccountAsAdminViewModel mod
                 var status = await _deleteAccountService.DeleteAccountAsync(
                     userToBeDeleted: user,
                     userToExecuteTheDelete: admin,
-                    orphanPackagePolicy: model.ShouldUnlist ? AccountDeletionOrphanPackagePolicy.UnlistOrphans : AccountDeletionOrphanPackagePolicy.KeepOrphans,
-                    commitAsTransaction: true);
+                    orphanPackagePolicy: model.ShouldUnlist ? AccountDeletionOrphanPackagePolicy.UnlistOrphans : AccountDeletionOrphanPackagePolicy.KeepOrphans);
                 return View("DeleteUserAccountStatus", status);
             }
         }

src/NuGetGallery/Security/ISecurityPolicyService.cs

@@ -41,12 +41,12 @@ public interface ISecurityPolicyService
         /// <summary>
         /// Unsubscribe a user from one or more security policies.
         /// </summary>
-        Task UnsubscribeAsync(User user, string subscriptionName);
+        Task UnsubscribeAsync(User user, string subscriptionName, bool commitChanges = true);
 
         /// <summary>
         /// Unsubscribe a user from one or more security policies.
         /// </summary>
-        Task UnsubscribeAsync(User user, IUserSecurityPolicySubscription subscription);
+        Task UnsubscribeAsync(User user, IUserSecurityPolicySubscription subscription, bool commitChanges = true);
 
         /// <summary>
         /// Evaluate any security policies that may apply to the current context.

src/NuGetGallery/Security/SecurityPolicyService.cs

@@ -426,7 +426,7 @@ await Auditing.SaveAuditRecordAsync(
         /// <summary>
         /// Unsubscribe a user from one or more security policies.
         /// </summary>
-        public Task UnsubscribeAsync(User user, string subscriptionName)
+        public Task UnsubscribeAsync(User user, string subscriptionName, bool commitChanges = true)
         {
             if (string.IsNullOrEmpty(subscriptionName))
             {
@@ -439,13 +439,13 @@ public Task UnsubscribeAsync(User user, string subscriptionName)
                 throw new NotSupportedException($"Subscription '{subscriptionName}' not found.");
             }
 
-            return UnsubscribeAsync(user, subscription);
+            return UnsubscribeAsync(user, subscription, commitChanges);
         }
 
         /// <summary>
         /// Unsubscribe a user from one or more security policies.
         /// </summary>
-        public async Task UnsubscribeAsync(User user, IUserSecurityPolicySubscription subscription)
+        public async Task UnsubscribeAsync(User user, IUserSecurityPolicySubscription subscription, bool commitChanges = true)
         {
             if (user == null)
             {
@@ -471,7 +471,10 @@ public async Task UnsubscribeAsync(User user, IUserSecurityPolicySubscription su
                 await Auditing.SaveAuditRecordAsync(
                     new UserAuditRecord(user, AuditedUserAction.UnsubscribeFromPolicies, subscription.Policies));
 
-                await EntitiesContext.SaveChangesAsync();
+                if (commitChanges)
+                {
+                    await EntitiesContext.SaveChangesAsync();
+                }
 
                 Diagnostics.Information($"User is now unsubscribed from '{subscription.SubscriptionName}'.");
             }