Move remaining ownership emails from the Controller layer to the service layer

src/AccountDeleter/Providers/AccountDeleteUrlHelper.cs

@@ -7,9 +7,24 @@ namespace NuGetGallery.AccountDeleter
 {
     public class AccountDeleteUrlHelper : IUrlHelper
     {
+        public string ConfirmPendingOwnershipRequest(string id, string username, string confirmationCode, bool relativeUrl)
+        {
+            throw new NotImplementedException();
+        }
+
+        public string ManagePackageOwnership(string id, bool relativeUrl)
+        {
+            throw new NotImplementedException();
+        }
+
         public string Package(string id, string version, bool relativeUrl)
         {
             throw new NotImplementedException();
         }
+
+        public string RejectPendingOwnershipRequest(string id, string username, string confirmationCode, bool relativeUrl)
+        {
+            throw new NotImplementedException();
+        }
     }
 }

src/NuGetGallery.Services/NuGetGallery.Services.csproj

@@ -155,6 +155,11 @@
     <Compile Include="Helpers\UploadHelper.cs" />
     <Compile Include="Mail\GalleryEmailRecipientsUtility.cs" />
     <Compile Include="Mail\Messages\PackageOwnerAddedMessage.cs" />
+    <Compile Include="Mail\Messages\PackageOwnerRemovedMessage.cs" />
+    <Compile Include="Mail\Messages\PackageOwnershipRequestCanceledMessage.cs" />
+    <Compile Include="Mail\Messages\PackageOwnershipRequestDeclinedMessage.cs" />
+    <Compile Include="Mail\Messages\PackageOwnershipRequestInitiatedMessage.cs" />
+    <Compile Include="Mail\Messages\PackageOwnershipRequestMessage.cs" />
     <Compile Include="Models\LuceneIndexLocation.cs" />
     <Compile Include="Models\PackageDependent.cs" />
     <Compile Include="Models\PackageDependents.cs" />

src/NuGetGallery.Services/PackageManagement/IPackageOwnershipManagementService.cs

@@ -28,6 +28,16 @@ public interface IPackageOwnershipManagementService
         /// <param name="user">The user to add as an owner to the package.</param>
         Task AddPackageOwnerAsync(PackageRegistration packageRegistration, User user, bool commitChanges = true);
 
+        /// <summary>
+        /// Add the pending ownership request and then sends notification messages to the new and existing owners.
+        /// Immediately commits the changes to the database. Same behavior as <see cref="AddPackageOwnershipRequestAsync(PackageRegistration, User, User)"/>
+        /// with the addition of sending messages.
+        /// </summary>
+        /// <param name="packageRegistration">The package registration that has pending ownership request.</param>
+        /// <param name="requestingOwner">The user to requesting to add the pending owner.</param>
+        /// <param name="newOwner">The user to be added for from pending ownership.</param>
+        Task<PackageOwnerRequest> AddPackageOwnershipRequestWithMessagesAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner, string message);
+
         /// <summary>
         /// Add the pending ownership request.
         /// </summary>
@@ -36,6 +46,16 @@ public interface IPackageOwnershipManagementService
         /// <param name="newOwner">The user to be added for from pending ownership.</param>
         Task<PackageOwnerRequest> AddPackageOwnershipRequestAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner);
 
+        /// <summary>
+        /// Remove the user as from the list of owners of the package and then sends notification messages. Immediately
+        /// commits the changes to the database. Same behavior as <see cref="RemovePackageOwnerAsync(PackageRegistration, User, User, bool)" />
+        /// with the addition of sending messages.
+        /// </summary>
+        /// <param name="packageRegistration">The package registration that is intended to get ownership.</param>
+        /// <param name="requestingUser">The user requesting to remove an owner from the package.</param>
+        /// <param name="userToBeRemoved">The user to remove as an owner from the package.</param>
+        Task RemovePackageOwnerWithMessagesAsync(PackageRegistration packageRegistration, User requestingUser, User userToBeRemoved);
+
         /// <summary>
         /// Remove the user as from the list of owners of the package. Also remove the package registration
         /// from the reserved namespaces owned by this user if the Id matches any of the reserved prefixes
@@ -47,6 +67,28 @@ public interface IPackageOwnershipManagementService
         /// <param name="commitChanges">Whether or not to commit the changes.</param>
         Task RemovePackageOwnerAsync(PackageRegistration packageRegistration, User requestingUser, User userToBeRemoved, bool commitChanges);
 
+        /// <summary>
+        /// Remove the pending ownership request and then send the "cancel" notification messages. This should be used when
+        /// the sender (<paramref name="requestingOwner"/>) deletes the request. Immediately commits the changes
+        /// to the database. Same behavior as <see cref="DeletePackageOwnershipRequestAsync(PackageRegistration, User, bool)"/>
+        /// with the addition of sending messages.
+        /// </summary>
+        /// <param name="packageRegistration">The package registration that has pending ownership request.</param>
+        /// <param name="requestingOwner">The user that originally sent the ownership request.</param>
+        /// <param name="newOwner">The user to be removed from pending ownership.</param>
+        Task CancelPackageOwnershipRequestWithMessagesAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner);
+
+        /// <summary>
+        /// Remove the pending ownership request and then send the "decline" notification messages. This should be used when
+        /// the recipient (<paramref name="newOwner"/>) deletes the request. Immediately commits the changes
+        /// to the database. Same behavior as <see cref="DeletePackageOwnershipRequestAsync(PackageRegistration, User, bool)"/>
+        /// with the addition of sending messages.
+        /// </summary>
+        /// <param name="packageRegistration">The package registration that has pending ownership request.</param>
+        /// <param name="requestingOwner">The user that originally sent the ownership request.</param>
+        /// <param name="newOwner">The user to be removed from pending ownership.</param>
+        Task DeclinePackageOwnershipRequestWithMessagesAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner);
+
         /// <summary>
         /// Remove the pending ownership request.
         /// </summary>

src/NuGetGallery.Services/PackageManagement/PackageOwnershipManagementService.cs

@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Linq;
 using System.Threading.Tasks;
+using System.Web;
 using NuGet.Services.Entities;
 using NuGet.Services.Messaging.Email;
 using NuGetGallery.Auditing;
@@ -131,8 +132,101 @@ private async Task AddPackageOwnerTask(PackageRegistration packageRegistration,
             await DeletePackageOwnershipRequestAsync(packageRegistration, user, commitChanges, saveAudit: false);
         }
 
+        public async Task<PackageOwnerRequest> AddPackageOwnershipRequestWithMessagesAsync(
+            PackageRegistration packageRegistration,
+            User requestingOwner,
+            User newOwner,
+            string message)
+        {
+            if (packageRegistration == null)
+            {
+                throw new ArgumentNullException(nameof(packageRegistration));
+            }
+
+            if (requestingOwner == null)
+            {
+                throw new ArgumentNullException(nameof(requestingOwner));
+            }
+
+            if (newOwner == null)
+            {
+                throw new ArgumentNullException(nameof(newOwner));
+            }
+
+            var encodedMessage = HttpUtility.HtmlEncode(message ?? string.Empty);
+
+            var packageUrl = _urlHelper.Package(packageRegistration.Id, version: null, relativeUrl: false);
+
+            var ownerRequest = await AddPackageOwnershipRequestAsync(
+                packageRegistration, requestingOwner, newOwner);
+
+            var confirmationUrl = _urlHelper.ConfirmPendingOwnershipRequest(
+                packageRegistration.Id,
+                newOwner.Username,
+                ownerRequest.ConfirmationCode,
+                relativeUrl: false);
+
+            var rejectionUrl = _urlHelper.RejectPendingOwnershipRequest(
+                packageRegistration.Id,
+                newOwner.Username,
+                ownerRequest.ConfirmationCode,
+                relativeUrl: false);
+
+            var manageUrl = _urlHelper.ManagePackageOwnership(
+                packageRegistration.Id,
+                relativeUrl: false);
+
+            var packageOwnershipRequestMessage = new PackageOwnershipRequestMessage(
+                _appConfiguration,
+                requestingOwner,
+                newOwner,
+                packageRegistration,
+                packageUrl,
+                confirmationUrl,
+                rejectionUrl,
+                encodedMessage,
+                string.Empty);
+
+            // Accumulate the tasks so that they are sent in parallel and as many messages as possible are sent even if
+            // one fails (i.e. throws an exception).
+            var messageTasks = new List<Task>();
+            messageTasks.Add(_messageService.SendMessageAsync(packageOwnershipRequestMessage));
+
+            foreach (var owner in packageRegistration.Owners)
+            {
+                var emailMessage = new PackageOwnershipRequestInitiatedMessage(
+                    _appConfiguration,
+                    requestingOwner,
+                    owner,
+                    newOwner,
+                    packageRegistration,
+                    manageUrl);
+
+                messageTasks.Add(_messageService.SendMessageAsync(emailMessage));
+            }
+
+            await Task.WhenAll(messageTasks);
+
+            return ownerRequest;
+        }
+
         public async Task<PackageOwnerRequest> AddPackageOwnershipRequestAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner)
         {
+            if (packageRegistration == null)
+            {
+                throw new ArgumentNullException(nameof(packageRegistration));
+            }
+
+            if (requestingOwner == null)
+            {
+                throw new ArgumentNullException(nameof(requestingOwner));
+            }
+
+            if (newOwner == null)
+            {
+                throw new ArgumentNullException(nameof(newOwner));
+            }
+
             var request = await _packageOwnerRequestService.AddPackageOwnershipRequest(packageRegistration, requestingOwner, newOwner);
 
             await _auditingService.SaveAuditRecordAsync(PackageRegistrationAuditRecord.CreateForAddOwnershipRequest(
@@ -153,6 +247,14 @@ public IEnumerable<PackageOwnerRequest> GetPackageOwnershipRequests(PackageRegis
             return _packageOwnerRequestService.GetPackageOwnershipRequests(package, requestingOwner, newOwner);
         }
 
+        public async Task RemovePackageOwnerWithMessagesAsync(PackageRegistration packageRegistration, User requestingOwner, User ownerToBeRemoved)
+        {
+            await RemovePackageOwnerAsync(packageRegistration, requestingOwner, ownerToBeRemoved);
+
+            var emailMessage = new PackageOwnerRemovedMessage(_appConfiguration, requestingOwner, ownerToBeRemoved, packageRegistration);
+            await _messageService.SendMessageAsync(emailMessage);
+        }
+
         public async Task RemovePackageOwnerAsync(PackageRegistration packageRegistration, User requestingOwner, User ownerToBeRemoved, bool commitChanges = true)
         {
             if (packageRegistration == null)
@@ -223,6 +325,32 @@ private async Task RemovePackageOwnerImplAsync(PackageRegistration packageRegist
             }
         }
 
+        public async Task CancelPackageOwnershipRequestWithMessagesAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner)
+        {
+            if (requestingOwner == null)
+            {
+                throw new ArgumentNullException(nameof(requestingOwner));
+            }
+
+            await DeletePackageOwnershipRequestAsync(packageRegistration, newOwner);
+
+            var emailMessage = new PackageOwnershipRequestCanceledMessage(_appConfiguration, requestingOwner, newOwner, packageRegistration);
+            await _messageService.SendMessageAsync(emailMessage);
+        }
+
+        public async Task DeclinePackageOwnershipRequestWithMessagesAsync(PackageRegistration packageRegistration, User requestingOwner, User newOwner)
+        {
+            if (requestingOwner == null)
+            {
+                throw new ArgumentNullException(nameof(requestingOwner));
+            }
+
+            await DeletePackageOwnershipRequestAsync(packageRegistration, newOwner);
+
+            var emailMessage = new PackageOwnershipRequestDeclinedMessage(_appConfiguration, requestingOwner, newOwner, packageRegistration);
+            await _messageService.SendMessageAsync(emailMessage);
+        }
+
         public async Task DeletePackageOwnershipRequestAsync(PackageRegistration packageRegistration, User newOwner, bool commitChanges = true)
         {
             await DeletePackageOwnershipRequestAsync(packageRegistration, newOwner, commitChanges, saveAudit: true);

src/NuGetGallery.Services/Providers/IUrlHelper.cs

@@ -19,5 +19,33 @@ public interface IUrlHelper
         /// <param name="relativeUrl">True to return a relative URL, false to return an absolute URL.</param>
         /// <returns>The relative or absolute URL as a string.</returns>
         string Package(string id, string version, bool relativeUrl);
+
+        /// <summary>
+        /// Produces a URL to the package ownership request confirmation page.
+        /// </summary>
+        /// <param name="id">The package ID for the ownership request.</param>
+        /// <param name="username">The username of the ownership request recipient (new owner).</param>
+        /// <param name="confirmationCode">The confirmation code (secret) associated with the request.</param>
+        /// <param name="relativeUrl">True to return a relative URL, false to return an absolute URL.</param>
+        /// <returns>The relative or absolute URL as a string.</returns>
+        string ConfirmPendingOwnershipRequest(string id, string username, string confirmationCode, bool relativeUrl);
+
+        /// <summary>
+        /// Produces a URL to the package ownership request rejection page.
+        /// </summary>
+        /// <param name="id">The package ID for the ownership request.</param>
+        /// <param name="username">The username of the ownership request recipient (new owner).</param>
+        /// <param name="confirmationCode">The confirmation code (secret) associated with the request.</param>
+        /// <param name="relativeUrl">True to return a relative URL, false to return an absolute URL.</param>
+        /// <returns>The relative or absolute URL as a string.</returns>
+        string RejectPendingOwnershipRequest(string id, string username, string confirmationCode, bool relativeUrl);
+
+        /// <summary>
+        /// Produces a URL to manage the ownership of an existing package.
+        /// </summary>
+        /// <param name="id">The package ID to manage ownership for.</param>
+        /// <param name="relativeUrl">True to return a relative URL, false to return an absolute URL.</param>
+        /// <returns>The relative or absolute URL as a string.</returns>
+        string ManagePackageOwnership(string id, bool relativeUrl);
     }
 }

src/NuGetGallery/Controllers/JsonApiController.cs

@@ -140,52 +140,11 @@ public async Task<JsonResult> AddPackageOwner(AddPackageOwnerViewModel addOwnerD
                 }
                 else
                 {
-                    var encodedMessage = HttpUtility.HtmlEncode(message);
-
-                    var ownerRequest = await _packageOwnershipManagementService.AddPackageOwnershipRequestAsync(
-                        model.Package, model.CurrentUser, model.User);
-
-                    var confirmationUrl = Url.ConfirmPendingOwnershipRequest(
-                        model.Package.Id,
-                        model.User.Username,
-                        ownerRequest.ConfirmationCode,
-                        relativeUrl: false);
-
-                    var rejectionUrl = Url.RejectPendingOwnershipRequest(
-                        model.Package.Id,
-                        model.User.Username,
-                        ownerRequest.ConfirmationCode,
-                        relativeUrl: false);
-
-                    var manageUrl = Url.ManagePackageOwnership(
-                        model.Package.Id,
-                        relativeUrl: false);
-
-                    var packageOwnershipRequestMessage = new PackageOwnershipRequestMessage(
-                        _appConfiguration,
+                    await _packageOwnershipManagementService.AddPackageOwnershipRequestWithMessagesAsync(
+                        model.Package,
                         model.CurrentUser,
                         model.User,
-                        model.Package,
-                        packageUrl,
-                        confirmationUrl,
-                        rejectionUrl,
-                        encodedMessage,
-                        string.Empty);
-
-                    await _messageService.SendMessageAsync(packageOwnershipRequestMessage);
-
-                    foreach (var owner in model.Package.Owners)
-                    {
-                        var emailMessage = new PackageOwnershipRequestInitiatedMessage(
-                            _appConfiguration,
-                            model.CurrentUser,
-                            owner,
-                            model.User,
-                            model.Package,
-                            manageUrl);
-
-                        await _messageService.SendMessageAsync(emailMessage);
-                    }
+                        message);
                 }
 
                 return Json(new
@@ -226,17 +185,11 @@ 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, commitChanges: true);
-
-                    var emailMessage = new PackageOwnerRemovedMessage(_appConfiguration, model.CurrentUser, model.User, model.Package);
-                    await _messageService.SendMessageAsync(emailMessage);
+                    await _packageOwnershipManagementService.RemovePackageOwnerWithMessagesAsync(model.Package, model.CurrentUser, model.User);
                 }
                 else
                 {
-                    await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(model.Package, model.User);
-
-                    var emailMessage = new PackageOwnershipRequestCanceledMessage(_appConfiguration, model.CurrentUser, model.User, model.Package);
-                    await _messageService.SendMessageAsync(emailMessage);
+                    await _packageOwnershipManagementService.CancelPackageOwnershipRequestWithMessagesAsync(model.Package, model.CurrentUser, model.User);
                 }
 
                 return Json(new { success = true });

src/NuGetGallery/Controllers/PackagesController.cs

@@ -2339,12 +2339,7 @@ public virtual Task<ActionResult> RejectPendingOwnershipRequest(string id, strin
             }
             else
             {
-                var requestingUser = request.RequestingOwner;
-
-                await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(package, user);
-
-                var emailMessage = new PackageOwnershipRequestDeclinedMessage(_config, requestingUser, user, package);
-                await _messageService.SendMessageAsync(emailMessage);
+                await _packageOwnershipManagementService.DeclinePackageOwnershipRequestWithMessagesAsync(package, request.RequestingOwner, user);
 
                 return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, user.Username, ConfirmOwnershipResult.Rejected));
             }