Revert email retry (#6290)

* Revert "Don't throw when SmtpUri is not set (#6278)"

This reverts commit e60e75f.

* Revert "Fix logging exceptions from background thread (#6273)"

This reverts commit d1610bb.

* Revert "Improve email sending (#6154)"

This reverts commit 2616a99.

Author: zivkan

src/NuGetGallery.Core/Services/CoreMessageService.cs

@@ -2,13 +2,11 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Collections.ObjectModel;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using System.Net.Mail;
 using System.Text;
-using System.Threading.Tasks;
 using AnglicanGeek.MarkdownMailer;
 using NuGet.Services.Validation;
 using NuGet.Services.Validation.Issues;
@@ -17,11 +15,9 @@ namespace NuGetGallery.Services
 {
     public class CoreMessageService : ICoreMessageService
     {
-        private static readonly ReadOnlyCollection<TimeSpan> RetryDelays = Array.AsReadOnly(new[] {
-            TimeSpan.FromSeconds(0.1),
-            TimeSpan.FromSeconds(1),
-            TimeSpan.FromSeconds(10)
-        });
+        protected CoreMessageService()
+        {
+        }
 
         public CoreMessageService(IMailSender mailSender, ICoreMessageServiceConfiguration coreConfiguration)
         {
@@ -32,7 +28,7 @@ public CoreMessageService(IMailSender mailSender, ICoreMessageServiceConfigurati
         public IMailSender MailSender { get; protected set; }
         public ICoreMessageServiceConfiguration CoreConfiguration { get; protected set; }
 
-        public async Task SendPackageAddedNoticeAsync(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl)
+        public void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl)
         {
             string subject = $"[{CoreConfiguration.GalleryOwner.DisplayName}] Package published - {package.PackageRegistration.Id} {package.Version}";
             string body = $@"The package [{package.PackageRegistration.Id} {package.Version}]({packageUrl}) was recently published on {CoreConfiguration.GalleryOwner.DisplayName} by {package.User.Username}. If this was not intended, please [contact support]({packageSupportUrl}).
@@ -53,12 +49,12 @@ [change your email notification settings]({emailSettingsUrl}).
 
                 if (mailMessage.To.Any())
                 {
-                    await SendMessageAsync(mailMessage);
+                    SendMessage(mailMessage);
                 }
             }
         }
 
-        public async Task SendPackageValidationFailedNoticeAsync(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl)
+        public void SendPackageValidationFailedNotice(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl)
         {
             var validationIssues = validationSet.GetValidationIssues();
 
@@ -99,7 +95,7 @@ public async Task SendPackageValidationFailedNoticeAsync(Package package, Packag
 
                 if (mailMessage.To.Any())
                 {
-                    await SendMessageAsync(mailMessage);
+                    SendMessage(mailMessage, copySender: false);
                 }
             }
         }
@@ -135,7 +131,7 @@ private static string ParseValidationIssue(ValidationIssue validationIssue, stri
             }
         }
 
-        public async Task SendValidationTakingTooLongNoticeAsync(Package package, string packageUrl)
+        public void SendValidationTakingTooLongNotice(Package package, string packageUrl)
         {
             string subject = "[{0}] Package validation taking longer than expected - {1} {2}";
             string body = "It is taking longer than expected for your package [{1} {2}]({3}) to get published.\n\n" +
@@ -167,7 +163,7 @@ public async Task SendValidationTakingTooLongNoticeAsync(Package package, string
 
                 if (mailMessage.To.Any())
                 {
-                    await SendMessageAsync(mailMessage);
+                    SendMessage(mailMessage, copySender: false);
                 }
             }
         }
@@ -197,54 +193,30 @@ protected static void AddOwnersSubscribedToPackagePushedNotification(PackageRegi
             }
         }
 
-        protected virtual async Task SendMessageAsync(MailMessage mailMessage)
+        protected void SendMessage(MailMessage mailMessage)
         {
-            int attempt = 0;
-            bool success = false;
-            while (!success)
-            {
-                try
-                {
-                    await AttemptSendMessageAsync(mailMessage, attempt + 1);
-                    success = true;
-                }
-                catch (SmtpException)
-                {
-                    if (attempt < RetryDelays.Count)
-                    {
-                        await Task.Delay(RetryDelays[attempt]);
-                        attempt++;
-                    }
-                    else
-                    {
-                        throw;
-                    }
-                }
-            }
+            SendMessage(mailMessage, copySender: false);
         }
 
-        protected virtual Task AttemptSendMessageAsync(MailMessage mailMessage, int attemptNumber)
+        virtual protected void SendMessage(MailMessage mailMessage, bool copySender)
         {
-            // AnglicanGeek.MarkdownMailer doesn't have an async overload
             MailSender.Send(mailMessage);
-            return Task.CompletedTask;
-        }
-
-        protected async Task SendMessageToSenderAsync(MailMessage mailMessage)
-        {
-            using (var senderCopy = new MailMessage(
-                CoreConfiguration.GalleryOwner,
-                mailMessage.ReplyToList.First()))
+            if (copySender)
             {
-                senderCopy.Subject = mailMessage.Subject + " [Sender Copy]";
-                senderCopy.Body = string.Format(
-                        CultureInfo.CurrentCulture,
-                        "You sent the following message via {0}: {1}{1}{2}",
-                        CoreConfiguration.GalleryOwner.DisplayName,
-                        Environment.NewLine,
-                        mailMessage.Body);
+                var senderCopy = new MailMessage(
+                    CoreConfiguration.GalleryOwner,
+                    mailMessage.ReplyToList.First())
+                {
+                    Subject = mailMessage.Subject + " [Sender Copy]",
+                    Body = string.Format(
+                            CultureInfo.CurrentCulture,
+                            "You sent the following message via {0}: {1}{1}{2}",
+                            CoreConfiguration.GalleryOwner.DisplayName,
+                            Environment.NewLine,
+                            mailMessage.Body),
+                };
                 senderCopy.ReplyToList.Add(mailMessage.ReplyToList.First());
-                await SendMessageAsync(senderCopy);
+                MailSender.Send(senderCopy);
             }
         }
     }

src/NuGetGallery.Core/Services/ICoreMessageService.cs

@@ -2,14 +2,13 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using NuGet.Services.Validation;
-using System.Threading.Tasks;
 
 namespace NuGetGallery.Services
 {
     public interface ICoreMessageService
     {
-        Task SendPackageAddedNoticeAsync(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl);
-        Task SendPackageValidationFailedNoticeAsync(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl);
-        Task SendValidationTakingTooLongNoticeAsync(Package package, string packageUrl);
+        void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl);
+        void SendPackageValidationFailedNotice(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl);
+        void SendValidationTakingTooLongNotice(Package package, string packageUrl);
     }
 }

src/NuGetGallery/App_Start/DefaultDependenciesModule.cs

@@ -35,7 +35,6 @@
 using NuGetGallery.Infrastructure.Authentication;
 using NuGetGallery.Infrastructure.Lucene;
 using NuGetGallery.Security;
-using NuGetGallery.Services;
 using SecretReaderFactory = NuGetGallery.Configuration.SecretReader.SecretReaderFactory;
 
 namespace NuGetGallery
@@ -357,7 +356,7 @@ protected override void Load(ContainerBuilder builder)
                 .As<IMailSender>()
                 .InstancePerLifetimeScope();
 
-            builder.RegisterType<BackgroundMessageService>()
+            builder.RegisterType<MessageService>()
                 .AsSelf()
                 .As<IMessageService>()
                 .InstancePerLifetimeScope();

src/NuGetGallery/Controllers/AccountsController.cs

@@ -92,7 +92,7 @@ public virtual ActionResult ConfirmationRequired(string accountName = null)
         [HttpPost]
         [ActionName("ConfirmationRequired")]
         [ValidateAntiForgeryToken]
-        public virtual async Task<ActionResult> ConfirmationRequiredPost(string accountName = null)
+        public virtual ActionResult ConfirmationRequiredPost(string accountName = null)
         {
             var account = GetAccount(accountName);
 
@@ -108,7 +108,7 @@ public virtual async Task<ActionResult> ConfirmationRequiredPost(string accountN
             ConfirmationViewModel model;
             if (!alreadyConfirmed)
             {
-                await SendNewAccountEmailAsync(account);
+                SendNewAccountEmail(account);
 
                 model = new ConfirmationViewModel(account)
                 {
@@ -122,7 +122,7 @@ public virtual async Task<ActionResult> ConfirmationRequiredPost(string accountN
             return View(model);
         }
 
-        protected abstract Task SendNewAccountEmailAsync(User account);
+        protected abstract void SendNewAccountEmail(User account);
 
         [UIAuthorize(allowDiscontinuedLogins: true)]
         public virtual async Task<ActionResult> Confirm(string accountName, string token)
@@ -163,7 +163,7 @@ public virtual async Task<ActionResult> Confirm(string accountName, string token
                 // Change notice not required for new accounts.
                 if (model.SuccessfulConfirmation && !model.ConfirmingNewAccount)
                 {
-                    await MessageService.SendEmailChangeNoticeToPreviousEmailAddressAsync(account, existingEmail);
+                    MessageService.SendEmailChangeNoticeToPreviousEmailAddress(account, existingEmail);
 
                     string returnUrl = HttpContext.GetConfirmationReturnUrl();
                     if (!String.IsNullOrEmpty(returnUrl))
@@ -254,13 +254,13 @@ public virtual async Task<ActionResult> ChangeEmail(TAccountViewModel model)
 
             if (account.Confirmed)
             {
-                await SendEmailChangedConfirmationNoticeAsync(account);
+                SendEmailChangedConfirmationNotice(account);
             }
 
             return RedirectToAction(AccountAction);
         }
 
-        protected abstract Task SendEmailChangedConfirmationNoticeAsync(User account);
+        protected abstract void SendEmailChangedConfirmationNotice(User account);
 
         [HttpPost]
         [UIAuthorize]

src/NuGetGallery/Controllers/ApiController.cs

@@ -699,7 +699,7 @@ await AuditingService.SaveAuditRecordAsync(
                             if (!(ConfigurationService.Current.AsynchronousPackageValidationEnabled && ConfigurationService.Current.BlockingAsynchronousPackageValidationEnabled))
                             {
                                 // Notify user of push unless async validation in blocking mode is used
-                                await MessageService.SendPackageAddedNoticeAsync(package,
+                                MessageService.SendPackageAddedNotice(package,
                                     Url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false),
                                     Url.ReportPackage(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false),
                                     Url.AccountSettings(relativeUrl: false));

src/NuGetGallery/Controllers/AuthenticationController.cs

@@ -287,7 +287,7 @@ public virtual async Task<ActionResult> Register(LogOnViewModel model, string re
             // Send a new account email
             if (NuGetContext.Config.Current.ConfirmEmailAddresses && !string.IsNullOrEmpty(user.User.UnconfirmedEmailAddress))
             {
-                await _messageService.SendNewAccountEmailAsync(
+                _messageService.SendNewAccountEmail(
                     user.User,
                     Url.ConfirmEmail(
                         user.User.Username,
@@ -325,7 +325,7 @@ public virtual ActionResult LogOff(string returnUrl)
 
         [HttpPost]
         [ValidateAntiForgeryToken]
-        public virtual async Task<JsonResult> SignInAssistance(string username, string providedEmailAddress)
+        public virtual JsonResult SignInAssistance(string username, string providedEmailAddress)
         {
             // If provided email address is empty or null, return the result with a formatted
             // email address, otherwise send sign-in assistance email to the associated mail address.
@@ -352,7 +352,7 @@ public virtual async Task<JsonResult> SignInAssistance(string username, string p
                     else
                     {
                         var externalCredentials = user.Credentials.Where(cred => cred.IsExternal());
-                        await _messageService.SendSigninAssistanceEmailAsync(new MailAddress(email, user.Username), externalCredentials);
+                        _messageService.SendSigninAssistanceEmail(new MailAddress(email, user.Username), externalCredentials);
                         return Json(new { success = true });
                     }
                 }
@@ -674,7 +674,7 @@ private async Task<LoginUserDetails> AssociateCredential(AuthenticatedUser user)
             await RemovePasswordCredential(user.User);
 
             // Notify the user of the change
-            await _messageService.SendCredentialAddedNoticeAsync(user.User, _authService.DescribeCredential(result.Credential));
+            _messageService.SendCredentialAddedNotice(user.User, _authService.DescribeCredential(result.Credential));
 
             return new LoginUserDetails
             {

src/NuGetGallery/Controllers/JsonApiController.cs

@@ -119,7 +119,7 @@ public async Task<JsonResult> AddPackageOwner(string id, string username, string
 
                     foreach (var owner in model.Package.Owners)
                     {
-                        await _messageService.SendPackageOwnerAddedNoticeAsync(owner, model.User, model.Package, packageUrl);
+                        _messageService.SendPackageOwnerAddedNotice(owner, model.User, model.Package, packageUrl);
                     }
                 }
                 else
@@ -147,12 +147,12 @@ public async Task<JsonResult> AddPackageOwner(string id, string username, string
                         model.User.Username,
                         relativeUrl: false);
 
-                    await _messageService.SendPackageOwnerRequestAsync(model.CurrentUser, model.User, model.Package, packageUrl,
+                    _messageService.SendPackageOwnerRequest(model.CurrentUser, model.User, model.Package, packageUrl,
                         confirmationUrl, rejectionUrl, encodedMessage, policyMessage: string.Empty);
 
                     foreach (var owner in model.Package.Owners)
                     {
-                        await _messageService.SendPackageOwnerRequestInitiatedNoticeAsync(model.CurrentUser, owner, model.User, model.Package, cancellationUrl);
+                        _messageService.SendPackageOwnerRequestInitiatedNotice(model.CurrentUser, owner, model.User, model.Package, cancellationUrl);
                     }
                 }
 
@@ -190,12 +190,12 @@ public async Task<JsonResult> RemovePackageOwner(string id, string username)
                         throw new InvalidOperationException("You can't remove the only owner from a package.");
                     }
                     await _packageOwnershipManagementService.RemovePackageOwnerAsync(model.Package, model.CurrentUser, model.User, commitAsTransaction:true);
-                    await _messageService.SendPackageOwnerRemovedNoticeAsync(model.CurrentUser, model.User, model.Package);
+                    _messageService.SendPackageOwnerRemovedNotice(model.CurrentUser, model.User, model.Package);
                 }
                 else
                 {
                     await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(model.Package, model.User);
-                    await _messageService.SendPackageOwnerRequestCancellationNoticeAsync(model.CurrentUser, model.User, model.Package);
+                    _messageService.SendPackageOwnerRequestCancellationNotice(model.CurrentUser, model.User, model.Package);
                 }
 
                 return Json(new { success = true });