Revert "Improve email sending (#6154)"

This reverts commit 2616a99.

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 });