From 0c9f4f69001b826e517a0f7ecd3879753a525848 Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Tue, 10 Jul 2018 13:25:05 +0200 Subject: [PATCH 1/8] Improve email sending * Retry failed sends, to improve reliability * Send App Insights telemetry, so we have visibility into the extent of the problem (plus the quantity of messages sent) * NuGetGallery to send emails in the background, so slow/retried emails don't slow HTTP responses --- .../Services/CoreMessageService.cs | 75 ++++++++++++------- .../App_Start/DefaultDependenciesModule.cs | 3 +- src/NuGetGallery/NuGetGallery.csproj | 1 + .../Services/BackgroundMessageService.cs | 37 +++++++++ src/NuGetGallery/Services/ITelemetryClient.cs | 9 +++ src/NuGetGallery/Services/MessageService.cs | 38 ++++++---- .../Services/TelemetryClientWrapper.cs | 19 +++++ .../Services/MessageServiceFacts.cs | 5 +- 8 files changed, 143 insertions(+), 44 deletions(-) create mode 100644 src/NuGetGallery/Services/BackgroundMessageService.cs diff --git a/src/NuGetGallery.Core/Services/CoreMessageService.cs b/src/NuGetGallery.Core/Services/CoreMessageService.cs index a0581375a5..a195a4fc37 100644 --- a/src/NuGetGallery.Core/Services/CoreMessageService.cs +++ b/src/NuGetGallery.Core/Services/CoreMessageService.cs @@ -2,10 +2,12 @@ // 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.Globalization; using System.Linq; using System.Net.Mail; using System.Text; +using System.Threading; using AnglicanGeek.MarkdownMailer; using NuGet.Services.Validation; using NuGet.Services.Validation.Issues; @@ -14,9 +16,11 @@ namespace NuGetGallery.Services { public class CoreMessageService : ICoreMessageService { - protected CoreMessageService() - { - } + private static readonly ReadOnlyCollection RetryDelays = Array.AsReadOnly(new[] { + TimeSpan.FromSeconds(0.1), + TimeSpan.FromSeconds(1), + TimeSpan.FromSeconds(10) + }); public CoreMessageService(IMailSender mailSender, ICoreMessageServiceConfiguration coreConfiguration) { @@ -48,7 +52,7 @@ [change your email notification settings]({emailSettingsUrl}). if (mailMessage.To.Any()) { - SendMessage(mailMessage, copySender: false); + SendMessage(mailMessage); } } } @@ -94,7 +98,7 @@ public void SendPackageValidationFailedNotice(Package package, PackageValidation if (mailMessage.To.Any()) { - SendMessage(mailMessage, copySender: false); + SendMessage(mailMessage); } } } @@ -162,7 +166,7 @@ public void SendValidationTakingTooLongNotice(Package package, string packageUrl if (mailMessage.To.Any()) { - SendMessage(mailMessage, copySender: false); + SendMessage(mailMessage); } } } @@ -192,31 +196,52 @@ protected static void AddOwnersSubscribedToPackagePushedNotification(PackageRegi } } - protected void SendMessage(MailMessage mailMessage) + protected virtual void SendMessage(MailMessage mailMessage) { - SendMessage(mailMessage, copySender: false); + int attempt = 0; + for (;;) + { + try + { + AttemptSendMessage(mailMessage); + break; + } + catch (SmtpException) + { + if (attempt < RetryDelays.Count) + { + Thread.Sleep(RetryDelays[attempt]); + attempt++; + } + else + { + throw; + } + } + } } - virtual protected void SendMessage(MailMessage mailMessage, bool copySender) + protected virtual void AttemptSendMessage(MailMessage mailMessage) { MailSender.Send(mailMessage); - if (copySender) + } + + protected void SendMessageToSender(MailMessage mailMessage) + { + var senderCopy = new MailMessage( + CoreConfiguration.GalleryOwner, + mailMessage.ReplyToList.First()) { - 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()); - MailSender.Send(senderCopy); - } + 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()); + SendMessage(senderCopy); } } } diff --git a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs index a9d5058c23..db23744416 100644 --- a/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs +++ b/src/NuGetGallery/App_Start/DefaultDependenciesModule.cs @@ -35,6 +35,7 @@ using NuGetGallery.Infrastructure.Authentication; using NuGetGallery.Infrastructure.Lucene; using NuGetGallery.Security; +using NuGetGallery.Services; using SecretReaderFactory = NuGetGallery.Configuration.SecretReader.SecretReaderFactory; namespace NuGetGallery @@ -338,7 +339,7 @@ protected override void Load(ContainerBuilder builder) .As() .InstancePerLifetimeScope(); - builder.RegisterType() + builder.RegisterType() .AsSelf() .As() .InstancePerLifetimeScope(); diff --git a/src/NuGetGallery/NuGetGallery.csproj b/src/NuGetGallery/NuGetGallery.csproj index 90c8e9a9b5..7597aac33e 100644 --- a/src/NuGetGallery/NuGetGallery.csproj +++ b/src/NuGetGallery/NuGetGallery.csproj @@ -361,6 +361,7 @@ + diff --git a/src/NuGetGallery/Services/BackgroundMessageService.cs b/src/NuGetGallery/Services/BackgroundMessageService.cs new file mode 100644 index 0000000000..d6a4fc52de --- /dev/null +++ b/src/NuGetGallery/Services/BackgroundMessageService.cs @@ -0,0 +1,37 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Net.Mail; +using System.Threading.Tasks; +using AnglicanGeek.MarkdownMailer; +using NuGetGallery.Configuration; + +namespace NuGetGallery.Services +{ + public class BackgroundMessageService : MessageService + { + public BackgroundMessageService(IMailSender mailSender, IAppConfiguration config, ITelemetryClient telemetryClient) + :base(mailSender, config, telemetryClient) + { + } + + protected override void SendMessage(MailMessage mailMessage) + { + // Send email as background task, as we don't want to delay the HTTP response. + // Particularly when sending email fails and needs to be retried with a delay. + Task.Run(() => + { + try + { + base.SendMessage(mailMessage); + } + catch (Exception ex) + { + // Log but swallow the exception. + QuietLog.LogHandledException(ex); + } + }); + } + } +} \ No newline at end of file diff --git a/src/NuGetGallery/Services/ITelemetryClient.cs b/src/NuGetGallery/Services/ITelemetryClient.cs index 99c7ff37f7..f6ed2502ee 100644 --- a/src/NuGetGallery/Services/ITelemetryClient.cs +++ b/src/NuGetGallery/Services/ITelemetryClient.cs @@ -14,5 +14,14 @@ public interface ITelemetryClient void TrackMetric(string metricName, double value, IDictionary properties = null); void TrackException(Exception exception, IDictionary properties = null, IDictionary metrics = null); + + void TrackDependency(string dependencyTypeName, + string target, + string dependencyName, + string data, + DateTimeOffset startTime, + TimeSpan duration, + string resultCode, + bool success); } } diff --git a/src/NuGetGallery/Services/MessageService.cs b/src/NuGetGallery/Services/MessageService.cs index 0ed979707e..45f17d392f 100644 --- a/src/NuGetGallery/Services/MessageService.cs +++ b/src/NuGetGallery/Services/MessageService.cs @@ -3,10 +3,12 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Globalization; using System.Linq; using System.Net.Mail; using System.Text; +using System.Threading.Tasks; using System.Web; using AnglicanGeek.MarkdownMailer; using NuGetGallery.Configuration; @@ -16,15 +18,16 @@ namespace NuGetGallery { public class MessageService : CoreMessageService, IMessageService { - protected MessageService() - { - } - - public MessageService(IMailSender mailSender, IAppConfiguration config) + public MessageService(IMailSender mailSender, IAppConfiguration config, ITelemetryClient telemetryClient) : base(mailSender, config) { + this.telemetryClient = telemetryClient ?? throw new ArgumentNullException(nameof(telemetryClient)); + smtpUri = config.SmtpUri?.Host ?? throw new ArgumentNullException(nameof(config) + "." + nameof(config.SmtpUri)); } + private readonly ITelemetryClient telemetryClient; + private readonly string smtpUri; + public IAppConfiguration Config { get { return (IAppConfiguration)CoreConfiguration; } @@ -168,7 +171,11 @@ [change your email notification settings]({7}). if (mailMessage.To.Any()) { - SendMessage(mailMessage, copySender); + SendMessage(mailMessage); + if (copySender) + { + SendMessageToSender(mailMessage); + } } } } @@ -1014,21 +1021,20 @@ private bool AddAddressesForAccountManagementToEmail(MailMessage mailMessage, Us return AddAddressesWithPermissionToEmail(mailMessage, user, ActionsRequiringPermissions.ManageAccount); } - protected override void SendMessage(MailMessage mailMessage, bool copySender) + protected override void AttemptSendMessage(MailMessage mailMessage) { + bool success = false; + DateTimeOffset now = DateTimeOffset.UtcNow; + Stopwatch sw = Stopwatch.StartNew(); try { - base.SendMessage(mailMessage, copySender); - } - catch (InvalidOperationException ex) - { - // Log but swallow the exception - QuietLog.LogHandledException(ex); + base.AttemptSendMessage(mailMessage); + success = true; } - catch (SmtpException ex) + finally { - // Log but swallow the exception - QuietLog.LogHandledException(ex); + sw.Stop(); + telemetryClient.TrackDependency("SMTP", smtpUri, "SendMessage", null, now, sw.Elapsed, null, success); } } diff --git a/src/NuGetGallery/Services/TelemetryClientWrapper.cs b/src/NuGetGallery/Services/TelemetryClientWrapper.cs index d5d7f1fdbe..5525680983 100644 --- a/src/NuGetGallery/Services/TelemetryClientWrapper.cs +++ b/src/NuGetGallery/Services/TelemetryClientWrapper.cs @@ -52,5 +52,24 @@ public void TrackMetric(string metricName, double value, IDictionary().Object) { configurationService.Current.GalleryOwner = TestGalleryOwner; configurationService.Current.GalleryNoReplyAddress = TestGalleryNoReplyAddress; - Config = configurationService.Current; - MailSender = MockMailSender = new TestMailSender(); + MockMailSender = (TestMailSender)MailSender; } public Mock MockAuthService { get; protected set; } @@ -2018,6 +2018,7 @@ private TestableMessageService(IGalleryConfigurationService configurationService public static TestableMessageService Create(IGalleryConfigurationService configurationService) { + configurationService.Current.SmtpUri = new Uri("smtp://fake.mail.server"); return new TestableMessageService(configurationService); } } From 9ff2af9509c3e4b7dabcd22e688c6a3f9dcd6db1 Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Thu, 12 Jul 2018 14:03:34 +0200 Subject: [PATCH 2/8] Change infinite loop with break to loop with condition --- src/NuGetGallery.Core/Services/CoreMessageService.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CoreMessageService.cs b/src/NuGetGallery.Core/Services/CoreMessageService.cs index a195a4fc37..97c8c7cde3 100644 --- a/src/NuGetGallery.Core/Services/CoreMessageService.cs +++ b/src/NuGetGallery.Core/Services/CoreMessageService.cs @@ -199,12 +199,13 @@ protected static void AddOwnersSubscribedToPackagePushedNotification(PackageRegi protected virtual void SendMessage(MailMessage mailMessage) { int attempt = 0; - for (;;) + bool success = false; + while (!success) { try { AttemptSendMessage(mailMessage); - break; + success = true; } catch (SmtpException) { From 53bf1187b61320a799683742eaf4d426cafe4c50 Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Tue, 31 Jul 2018 16:42:05 -0700 Subject: [PATCH 3/8] Make CoreMessageService, and all usage, async --- .../Services/CoreMessageService.cs | 42 +-- .../Services/ICoreMessageService.cs | 7 +- .../Controllers/AccountsController.cs | 2 +- src/NuGetGallery/Controllers/ApiController.cs | 2 +- .../Controllers/AuthenticationController.cs | 6 +- .../Controllers/JsonApiController.cs | 10 +- .../Controllers/OrganizationsController.cs | 18 +- .../Controllers/PackagesController.cs | 16 +- .../Controllers/PagesController.cs | 2 +- .../Controllers/UsersController.cs | 28 +- .../Services/BackgroundMessageService.cs | 8 +- src/NuGetGallery/Services/IMessageService.cs | 63 ++--- src/NuGetGallery/Services/MessageService.cs | 146 +++++----- .../Controllers/AccountsControllerFacts.cs | 21 +- .../Controllers/ApiControllerFacts.cs | 2 +- .../AuthenticationControllerFacts.cs | 18 +- .../Controllers/JsonApiControllerFacts.cs | 13 +- .../OrganizationsControllerFacts.cs | 34 +-- .../Controllers/PackagesControllerFacts.cs | 32 +-- .../Controllers/PagesControllerFacts.cs | 2 +- .../Controllers/UsersControllerFacts.cs | 56 ++-- .../Services/MessageServiceFacts.cs | 249 +++++++++--------- 22 files changed, 398 insertions(+), 379 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CoreMessageService.cs b/src/NuGetGallery.Core/Services/CoreMessageService.cs index 97c8c7cde3..50626e2dfe 100644 --- a/src/NuGetGallery.Core/Services/CoreMessageService.cs +++ b/src/NuGetGallery.Core/Services/CoreMessageService.cs @@ -7,7 +7,7 @@ using System.Linq; using System.Net.Mail; using System.Text; -using System.Threading; +using System.Threading.Tasks; using AnglicanGeek.MarkdownMailer; using NuGet.Services.Validation; using NuGet.Services.Validation.Issues; @@ -31,7 +31,7 @@ public CoreMessageService(IMailSender mailSender, ICoreMessageServiceConfigurati public IMailSender MailSender { get; protected set; } public ICoreMessageServiceConfiguration CoreConfiguration { get; protected set; } - public void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl) + public async Task SendPackageAddedNoticeAsync(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}). @@ -52,12 +52,12 @@ [change your email notification settings]({emailSettingsUrl}). if (mailMessage.To.Any()) { - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); } } } - public void SendPackageValidationFailedNotice(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl) + public async Task SendPackageValidationFailedNoticeAsync(Package package, PackageValidationSet validationSet, string packageUrl, string packageSupportUrl, string announcementsUrl, string twitterUrl) { var validationIssues = validationSet.GetValidationIssues(); @@ -98,7 +98,7 @@ public void SendPackageValidationFailedNotice(Package package, PackageValidation if (mailMessage.To.Any()) { - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); } } } @@ -134,7 +134,7 @@ private static string ParseValidationIssue(ValidationIssue validationIssue, stri } } - public void SendValidationTakingTooLongNotice(Package package, string packageUrl) + public async Task SendValidationTakingTooLongNoticeAsync(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" + @@ -166,7 +166,7 @@ public void SendValidationTakingTooLongNotice(Package package, string packageUrl if (mailMessage.To.Any()) { - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); } } } @@ -196,7 +196,7 @@ protected static void AddOwnersSubscribedToPackagePushedNotification(PackageRegi } } - protected virtual void SendMessage(MailMessage mailMessage) + protected virtual async Task SendMessageAsync(MailMessage mailMessage) { int attempt = 0; bool success = false; @@ -204,14 +204,14 @@ protected virtual void SendMessage(MailMessage mailMessage) { try { - AttemptSendMessage(mailMessage); + await AttemptSendMessageAsync(mailMessage); success = true; } catch (SmtpException) { if (attempt < RetryDelays.Count) { - Thread.Sleep(RetryDelays[attempt]); + await Task.Delay(RetryDelays[attempt]); attempt++; } else @@ -222,27 +222,29 @@ protected virtual void SendMessage(MailMessage mailMessage) } } - protected virtual void AttemptSendMessage(MailMessage mailMessage) + protected virtual Task AttemptSendMessageAsync(MailMessage mailMessage) { + // AnglicanGeek.MarkdownMailer doesn't have an async overload MailSender.Send(mailMessage); + return Task.CompletedTask; } - protected void SendMessageToSender(MailMessage mailMessage) + protected async Task SendMessageToSenderAsync(MailMessage mailMessage) { - var senderCopy = new MailMessage( + using (var senderCopy = new MailMessage( CoreConfiguration.GalleryOwner, - mailMessage.ReplyToList.First()) + mailMessage.ReplyToList.First())) { - Subject = mailMessage.Subject + " [Sender Copy]", - Body = string.Format( + 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), - }; - senderCopy.ReplyToList.Add(mailMessage.ReplyToList.First()); - SendMessage(senderCopy); + mailMessage.Body); + senderCopy.ReplyToList.Add(mailMessage.ReplyToList.First()); + await SendMessageAsync(senderCopy); + } } } } diff --git a/src/NuGetGallery.Core/Services/ICoreMessageService.cs b/src/NuGetGallery.Core/Services/ICoreMessageService.cs index 2f81dafcad..25dfa773b9 100644 --- a/src/NuGetGallery.Core/Services/ICoreMessageService.cs +++ b/src/NuGetGallery.Core/Services/ICoreMessageService.cs @@ -2,13 +2,14 @@ // 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 { - 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); + 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); } } \ No newline at end of file diff --git a/src/NuGetGallery/Controllers/AccountsController.cs b/src/NuGetGallery/Controllers/AccountsController.cs index 6fed67835b..ffd5630ff0 100644 --- a/src/NuGetGallery/Controllers/AccountsController.cs +++ b/src/NuGetGallery/Controllers/AccountsController.cs @@ -163,7 +163,7 @@ public virtual async Task Confirm(string accountName, string token // Change notice not required for new accounts. if (model.SuccessfulConfirmation && !model.ConfirmingNewAccount) { - MessageService.SendEmailChangeNoticeToPreviousEmailAddress(account, existingEmail); + await MessageService.SendEmailChangeNoticeToPreviousEmailAddressAsync(account, existingEmail); string returnUrl = HttpContext.GetConfirmationReturnUrl(); if (!String.IsNullOrEmpty(returnUrl)) diff --git a/src/NuGetGallery/Controllers/ApiController.cs b/src/NuGetGallery/Controllers/ApiController.cs index dd2b207fdd..06f897b130 100644 --- a/src/NuGetGallery/Controllers/ApiController.cs +++ b/src/NuGetGallery/Controllers/ApiController.cs @@ -521,7 +521,7 @@ await AuditingService.SaveAuditRecordAsync( if (!(ConfigurationService.Current.AsynchronousPackageValidationEnabled && ConfigurationService.Current.BlockingAsynchronousPackageValidationEnabled)) { // Notify user of push unless async validation in blocking mode is used - MessageService.SendPackageAddedNotice(package, + await MessageService.SendPackageAddedNoticeAsync(package, Url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false), Url.ReportPackage(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false), Url.AccountSettings(relativeUrl: false)); diff --git a/src/NuGetGallery/Controllers/AuthenticationController.cs b/src/NuGetGallery/Controllers/AuthenticationController.cs index 0218a76257..c2568b47b1 100644 --- a/src/NuGetGallery/Controllers/AuthenticationController.cs +++ b/src/NuGetGallery/Controllers/AuthenticationController.cs @@ -287,7 +287,7 @@ public virtual async Task Register(LogOnViewModel model, string re // Send a new account email if (NuGetContext.Config.Current.ConfirmEmailAddresses && !string.IsNullOrEmpty(user.User.UnconfirmedEmailAddress)) { - _messageService.SendNewAccountEmail( + await _messageService.SendNewAccountEmailAsync( user.User, Url.ConfirmEmail( user.User.Username, @@ -352,7 +352,7 @@ public virtual JsonResult SignInAssistance(string username, string providedEmail else { var externalCredentials = user.Credentials.Where(cred => cred.IsExternal()); - _messageService.SendSigninAssistanceEmail(new MailAddress(email, user.Username), externalCredentials); + _messageService.SendSigninAssistanceEmailAsync(new MailAddress(email, user.Username), externalCredentials); return Json(new { success = true }); } } @@ -674,7 +674,7 @@ private async Task AssociateCredential(AuthenticatedUser user) await RemovePasswordCredential(user.User); // Notify the user of the change - _messageService.SendCredentialAddedNotice(user.User, _authService.DescribeCredential(result.Credential)); + await _messageService.SendCredentialAddedNoticeAsync(user.User, _authService.DescribeCredential(result.Credential)); return new LoginUserDetails { diff --git a/src/NuGetGallery/Controllers/JsonApiController.cs b/src/NuGetGallery/Controllers/JsonApiController.cs index c9fb270c53..248444737e 100644 --- a/src/NuGetGallery/Controllers/JsonApiController.cs +++ b/src/NuGetGallery/Controllers/JsonApiController.cs @@ -119,7 +119,7 @@ public async Task AddPackageOwner(string id, string username, string foreach (var owner in model.Package.Owners) { - _messageService.SendPackageOwnerAddedNotice(owner, model.User, model.Package, packageUrl); + await _messageService.SendPackageOwnerAddedNoticeAsync(owner, model.User, model.Package, packageUrl); } } else @@ -147,12 +147,12 @@ public async Task AddPackageOwner(string id, string username, string model.User.Username, relativeUrl: false); - _messageService.SendPackageOwnerRequest(model.CurrentUser, model.User, model.Package, packageUrl, + await _messageService.SendPackageOwnerRequestAsync(model.CurrentUser, model.User, model.Package, packageUrl, confirmationUrl, rejectionUrl, encodedMessage, policyMessage: string.Empty); foreach (var owner in model.Package.Owners) { - _messageService.SendPackageOwnerRequestInitiatedNotice(model.CurrentUser, owner, model.User, model.Package, cancellationUrl); + await _messageService.SendPackageOwnerRequestInitiatedNoticeAsync(model.CurrentUser, owner, model.User, model.Package, cancellationUrl); } } @@ -190,12 +190,12 @@ public async Task 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); - _messageService.SendPackageOwnerRemovedNotice(model.CurrentUser, model.User, model.Package); + await _messageService.SendPackageOwnerRemovedNoticeAsync(model.CurrentUser, model.User, model.Package); } else { await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(model.Package, model.User); - _messageService.SendPackageOwnerRequestCancellationNotice(model.CurrentUser, model.User, model.Package); + await _messageService.SendPackageOwnerRequestCancellationNoticeAsync(model.CurrentUser, model.User, model.Package); } return Json(new { success = true }); diff --git a/src/NuGetGallery/Controllers/OrganizationsController.cs b/src/NuGetGallery/Controllers/OrganizationsController.cs index d007a4ea2b..7d22657e05 100644 --- a/src/NuGetGallery/Controllers/OrganizationsController.cs +++ b/src/NuGetGallery/Controllers/OrganizationsController.cs @@ -57,13 +57,13 @@ protected override void SendNewAccountEmail(User account) { var confirmationUrl = Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendNewAccountEmail(account, confirmationUrl); + MessageService.SendNewAccountEmailAsync(account, confirmationUrl); } protected override void SendEmailChangedConfirmationNotice(User account) { var confirmationUrl = Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendEmailChangeConfirmationNotice(account, confirmationUrl); + MessageService.SendEmailChangeConfirmationNoticeAsync(account, confirmationUrl); } [HttpGet] @@ -134,8 +134,8 @@ public async Task AddMember(string accountName, string memberName, b var rejectUrl = Url.RejectOrganizationMembershipRequest(request, relativeUrl: false); var cancelUrl = Url.CancelOrganizationMembershipRequest(memberName, relativeUrl: false); - MessageService.SendOrganizationMembershipRequest(account, request.NewMember, currentUser, request.IsAdmin, profileUrl, confirmUrl, rejectUrl); - MessageService.SendOrganizationMembershipRequestInitiatedNotice(account, currentUser, request.NewMember, request.IsAdmin, cancelUrl); + await MessageService.SendOrganizationMembershipRequestAsync(account, request.NewMember, currentUser, request.IsAdmin, profileUrl, confirmUrl, rejectUrl); + await MessageService.SendOrganizationMembershipRequestInitiatedNoticeAsync(account, currentUser, request.NewMember, request.IsAdmin, cancelUrl); return Json(new OrganizationMemberViewModel(request)); } @@ -159,7 +159,7 @@ public async Task ConfirmMemberRequest(string accountName, string try { var member = await UserService.AddMemberAsync(account, GetCurrentUser().Username, confirmationToken); - MessageService.SendOrganizationMemberUpdatedNotice(account, member); + await MessageService.SendOrganizationMemberUpdatedNoticeAsync(account, member); TempData["Message"] = String.Format(CultureInfo.CurrentCulture, Strings.AddMember_Success, account.Username); @@ -188,7 +188,7 @@ public async Task RejectMemberRequest(string accountName, string c { var member = GetCurrentUser(); await UserService.RejectMembershipRequestAsync(account, member.Username, confirmationToken); - MessageService.SendOrganizationMembershipRequestRejectedNotice(account, member); + await MessageService.SendOrganizationMembershipRequestRejectedNoticeAsync(account, member); return HandleOrganizationMembershipRequestView(new HandleOrganizationMembershipRequestModel(false, account)); } @@ -221,7 +221,7 @@ public async Task CancelMemberRequest(string accountName, string mem try { var removedUser = await UserService.CancelMembershipRequestAsync(account, memberName); - MessageService.SendOrganizationMembershipRequestCancelledNotice(account, removedUser); + await MessageService.SendOrganizationMembershipRequestCancelledNoticeAsync(account, removedUser); return Json(Strings.CancelMemberRequest_Success); } catch (EntityException e) @@ -252,7 +252,7 @@ public async Task UpdateMember(string accountName, string memberName try { var membership = await UserService.UpdateMemberAsync(account, memberName, isAdmin); - MessageService.SendOrganizationMemberUpdatedNotice(account, membership); + await MessageService.SendOrganizationMemberUpdatedNoticeAsync(account, membership); return Json(new OrganizationMemberViewModel(membership)); } @@ -287,7 +287,7 @@ public async Task DeleteMember(string accountName, string memberName try { var removedMember = await UserService.DeleteMemberAsync(account, memberName); - MessageService.SendOrganizationMemberRemovedNotice(account, removedMember); + await MessageService.SendOrganizationMemberRemovedNoticeAsync(account, removedMember); return Json(Strings.DeleteMember_Success); } catch (EntityException e) diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs index 83aa2eebad..300062e3b2 100644 --- a/src/NuGetGallery/Controllers/PackagesController.cs +++ b/src/NuGetGallery/Controllers/PackagesController.cs @@ -752,7 +752,7 @@ public virtual async Task ReportAbuse(string id, string version, R await _supportRequestService.AddNewSupportRequestAsync(subject, reportForm.Message, requestorEmailAddress, reason, user, package); - _messageService.ReportAbuse(request); + await _messageService.ReportAbuseAsync(request); TempData["Message"] = "Your abuse report has been sent to the gallery operators."; @@ -893,7 +893,7 @@ private void NotifyReportMyPackageSupportRequest(ReportMyPackageViewModel report CopySender = reportForm.CopySender }; - _messageService.ReportMyPackage(request); + _messageService.ReportMyPackageAsync(request); TempData["Message"] = Strings.SupportRequestSentTransientMessage; } @@ -931,7 +931,7 @@ await _supportRequestService.UpdateIssueAsync( comment: null, editedBy: user.Username); - _messageService.SendPackageDeletedNotice( + await _messageService.SendPackageDeletedNoticeAsync( package, Url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false), Url.ReportPackage(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false)); @@ -990,7 +990,7 @@ public virtual ActionResult ContactOwners(string id, string version, ContactOwne var user = GetCurrentUser(); var fromAddress = new MailAddress(user.EmailAddress, user.Username); - _messageService.SendContactOwnersMessage( + _messageService.SendContactOwnersMessageAsync( fromAddress, package, Url.Package(package, false), @@ -1366,7 +1366,7 @@ private async Task HandleOwnershipRequest(string id, string userna await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(package, user); - _messageService.SendPackageOwnerRequestRejectionNotice(requestingUser, user, package); + await _messageService.SendPackageOwnerRequestRejectionNoticeAsync(requestingUser, user, package); return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, user.Username, ConfirmOwnershipResult.Rejected)); } @@ -1408,7 +1408,7 @@ public virtual async Task CancelPendingOwnershipRequest(string id, await _packageOwnershipManagementService.DeletePackageOwnershipRequestAsync(package, pendingUser); - _messageService.SendPackageOwnerRequestCancellationNotice(requestingUser, pendingUser, package); + await _messageService.SendPackageOwnerRequestCancellationNoticeAsync(requestingUser, pendingUser, package); return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, pendingUsername, ConfirmOwnershipResult.Cancelled)); } @@ -1425,7 +1425,7 @@ private void SendAddPackageOwnerNotification(PackageRegistration package, User n // Notify existing owners var notNewOwners = package.Owners.Where(notNewOwner).ToList(); - notNewOwners.ForEach(owner => _messageService.SendPackageOwnerAddedNotice(owner, newOwner, package, packageUrl)); + notNewOwners.ForEach(owner => _messageService.SendPackageOwnerAddedNoticeAsync(owner, newOwner, package, packageUrl)); } internal virtual async Task Edit(string id, string version, bool? listed, Func urlFactory) @@ -1685,7 +1685,7 @@ await _auditingService.SaveAuditRecordAsync( if (!(_config.AsynchronousPackageValidationEnabled && _config.BlockingAsynchronousPackageValidationEnabled)) { // notify user unless async validation in blocking mode is used - _messageService.SendPackageAddedNotice(package, + await _messageService.SendPackageAddedNoticeAsync(package, Url.Package(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false), Url.ReportPackage(package.PackageRegistration.Id, package.NormalizedVersion, relativeUrl: false), Url.AccountSettings(relativeUrl: false)); diff --git a/src/NuGetGallery/Controllers/PagesController.cs b/src/NuGetGallery/Controllers/PagesController.cs index d10103cd4c..76072a8bc4 100644 --- a/src/NuGetGallery/Controllers/PagesController.cs +++ b/src/NuGetGallery/Controllers/PagesController.cs @@ -94,7 +94,7 @@ public virtual async Task Contact(ContactSupportViewModel contactF var subject = $"Support Request for user '{user.Username}'"; await _supportRequestService.AddNewSupportRequestAsync(subject, contactForm.Message, user.EmailAddress, "Other", user); - _messageService.SendContactSupportEmail(request); + await _messageService.SendContactSupportEmailAsync(request); ModelState.Clear(); diff --git a/src/NuGetGallery/Controllers/UsersController.cs b/src/NuGetGallery/Controllers/UsersController.cs index 1006534acf..00eef77f6f 100644 --- a/src/NuGetGallery/Controllers/UsersController.cs +++ b/src/NuGetGallery/Controllers/UsersController.cs @@ -74,13 +74,13 @@ protected override void SendNewAccountEmail(User account) { var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendNewAccountEmail(account, confirmationUrl); + MessageService.SendNewAccountEmailAsync(account, confirmationUrl); } protected override void SendEmailChangedConfirmationNotice(User account) { var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendEmailChangeConfirmationNotice(account, confirmationUrl); + MessageService.SendEmailChangeConfirmationNoticeAsync(account, confirmationUrl); } protected override User GetAccount(string accountName) @@ -158,16 +158,16 @@ public virtual async Task TransformToOrganization(TransformAccount if (existingTransformRequestUser != null) { - MessageService.SendOrganizationTransformRequestCancelledNotice(accountToTransform, existingTransformRequestUser); + await MessageService.SendOrganizationTransformRequestCancelledNoticeAsync(accountToTransform, existingTransformRequestUser); } var returnUrl = Url.ConfirmTransformAccount(accountToTransform); var confirmUrl = Url.ConfirmTransformAccount(accountToTransform, relativeUrl: false); var rejectUrl = Url.RejectTransformAccount(accountToTransform, relativeUrl: false); - MessageService.SendOrganizationTransformRequest(accountToTransform, adminUser, Url.User(accountToTransform, relativeUrl: false), confirmUrl, rejectUrl); + await MessageService.SendOrganizationTransformRequestAsync(accountToTransform, adminUser, Url.User(accountToTransform, relativeUrl: false), confirmUrl, rejectUrl); var cancelUrl = Url.CancelTransformAccount(accountToTransform, relativeUrl: false); - MessageService.SendOrganizationTransformInitiatedNotice(accountToTransform, adminUser, cancelUrl); + await MessageService.SendOrganizationTransformInitiatedNoticeAsync(accountToTransform, adminUser, cancelUrl); TelemetryService.TrackOrganizationTransformInitiated(accountToTransform); @@ -206,7 +206,7 @@ public virtual async Task ConfirmTransformToOrganization(string ac return TransformToOrganizationFailed(errorReason); } - MessageService.SendOrganizationTransformRequestAcceptedNotice(accountToTransform, adminUser); + await MessageService.SendOrganizationTransformRequestAcceptedNoticeAsync(accountToTransform, adminUser); TelemetryService.TrackOrganizationTransformCompleted(accountToTransform); @@ -234,7 +234,7 @@ public virtual async Task RejectTransformToOrganization(string acc { if (await UserService.RejectTransformUserToOrganizationRequest(accountToTransform, adminUser, token)) { - MessageService.SendOrganizationTransformRequestRejectedNotice(accountToTransform, adminUser); + await MessageService.SendOrganizationTransformRequestRejectedNoticeAsync(accountToTransform, adminUser); TelemetryService.TrackOrganizationTransformDeclined(accountToTransform); @@ -262,7 +262,7 @@ public virtual async Task CancelTransformToOrganization(string tok if (await UserService.CancelTransformUserToOrganizationRequest(accountToTransform, token)) { - MessageService.SendOrganizationTransformRequestCancelledNotice(accountToTransform, adminUser); + await MessageService.SendOrganizationTransformRequestCancelledNoticeAsync(accountToTransform, adminUser); TelemetryService.TrackOrganizationTransformCancelled(accountToTransform); @@ -314,7 +314,7 @@ public override async Task RequestAccountDeletion(string accountNa var isSupportRequestCreated = await _supportRequestService.TryAddDeleteSupportRequestAsync(user); if (isSupportRequestCreated) { - MessageService.SendAccountDeleteNotice(user); + await MessageService.SendAccountDeleteNoticeAsync(user); } else { @@ -597,7 +597,7 @@ public virtual async Task ResetPassword(string username, string to if (credential != null && !forgot) { // Setting a password, so notify the user - MessageService.SendCredentialAddedNotice(credential.User, AuthenticationService.DescribeCredential(credential)); + await MessageService.SendCredentialAddedNoticeAsync(credential.User, AuthenticationService.DescribeCredential(credential)); } return RedirectToAction("PasswordChanged"); @@ -831,7 +831,7 @@ public virtual async Task GenerateApiKey(string description, string var newCredentialViewModel = await GenerateApiKeyInternal(description, resolvedScopes, expiration); - MessageService.SendCredentialAddedNotice(GetCurrentUser(), newCredentialViewModel); + await MessageService.SendCredentialAddedNoticeAsync(GetCurrentUser(), newCredentialViewModel); return Json(new ApiKeyViewModel(newCredentialViewModel)); } @@ -987,7 +987,7 @@ private async Task RemoveApiKeyCredential(User user, Credential cred await AuthenticationService.RemoveCredential(user, cred); // Notify the user of the change - MessageService.SendCredentialRemovedNotice(user, AuthenticationService.DescribeCredential(cred)); + await MessageService.SendCredentialRemovedNoticeAsync(user, AuthenticationService.DescribeCredential(cred)); return Json(Strings.CredentialRemoved); } @@ -1017,7 +1017,7 @@ private async Task RemoveCredentialInternal(User user, Credential } // Notify the user of the change - MessageService.SendCredentialRemovedNotice(user, AuthenticationService.DescribeCredential(cred)); + await MessageService.SendCredentialRemovedNoticeAsync(user, AuthenticationService.DescribeCredential(cred)); TempData["Message"] = message; } @@ -1065,7 +1065,7 @@ private ActionResult SendPasswordResetEmail(User user, bool forgotPassword) user.PasswordResetToken, forgotPassword, relativeUrl: false); - MessageService.SendPasswordResetInstructions(user, resetPasswordUrl, forgotPassword); + MessageService.SendPasswordResetInstructionsAsync(user, resetPasswordUrl, forgotPassword); return RedirectToAction(actionName: "PasswordSent", controllerName: "Users"); } diff --git a/src/NuGetGallery/Services/BackgroundMessageService.cs b/src/NuGetGallery/Services/BackgroundMessageService.cs index d6a4fc52de..98ff700991 100644 --- a/src/NuGetGallery/Services/BackgroundMessageService.cs +++ b/src/NuGetGallery/Services/BackgroundMessageService.cs @@ -16,15 +16,15 @@ public BackgroundMessageService(IMailSender mailSender, IAppConfiguration config { } - protected override void SendMessage(MailMessage mailMessage) + protected override Task SendMessageAsync(MailMessage mailMessage) { // Send email as background task, as we don't want to delay the HTTP response. // Particularly when sending email fails and needs to be retried with a delay. - Task.Run(() => + Task.Run(async () => { try { - base.SendMessage(mailMessage); + await base.SendMessageAsync(mailMessage); } catch (Exception ex) { @@ -32,6 +32,8 @@ protected override void SendMessage(MailMessage mailMessage) QuietLog.LogHandledException(ex); } }); + + return Task.CompletedTask; } } } \ No newline at end of file diff --git a/src/NuGetGallery/Services/IMessageService.cs b/src/NuGetGallery/Services/IMessageService.cs index b971bb9c4f..acb684d4d5 100644 --- a/src/NuGetGallery/Services/IMessageService.cs +++ b/src/NuGetGallery/Services/IMessageService.cs @@ -3,42 +3,43 @@ using System.Collections.Generic; using System.Net.Mail; +using System.Threading.Tasks; using NuGetGallery.Services; namespace NuGetGallery { public interface IMessageService { - void SendContactOwnersMessage(MailAddress fromAddress, Package package, string packageUrl, string message, string emailSettingsUrl, bool copyFromAddress); - void ReportAbuse(ReportPackageRequest report); - void ReportMyPackage(ReportPackageRequest report); - void SendNewAccountEmail(User newUser, string confirmationUrl); - void SendEmailChangeConfirmationNotice(User user, string confirmationUrl); - void SendPasswordResetInstructions(User user, string resetPasswordUrl, bool forgotPassword); - void SendEmailChangeNoticeToPreviousEmailAddress(User user, string oldEmailAddress); - void SendPackageOwnerRequest(User fromUser, User toUser, PackageRegistration package, string packageUrl, string confirmationUrl, string rejectionUrl, string message, string policyMessage); - void SendPackageOwnerRequestInitiatedNotice(User requestingOwner, User receivingOwner, User newOwner, PackageRegistration package, string cancellationUrl); - void SendPackageOwnerRequestRejectionNotice(User requestingOwner, User newOwner, PackageRegistration package); - void SendPackageOwnerRequestCancellationNotice(User requestingOwner, User newOwner, PackageRegistration package); - void SendPackageOwnerAddedNotice(User toUser, User newOwner, PackageRegistration package, string packageUrl); - void SendPackageOwnerRemovedNotice(User fromUser, User toUser, PackageRegistration package); - void SendCredentialRemovedNotice(User user, CredentialViewModel removedCredentialViewModel); - void SendCredentialAddedNotice(User user, CredentialViewModel addedCredentialViewModel); - void SendContactSupportEmail(ContactSupportRequest request); - void SendPackageAddedNotice(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl); - void SendAccountDeleteNotice(User user); - void SendPackageDeletedNotice(Package package, string packageUrl, string packageSupportUrl); - void SendSigninAssistanceEmail(MailAddress emailAddress, IEnumerable credentials); - void SendOrganizationTransformRequest(User accountToTransform, User adminUser, string profileUrl, string confirmationUrl, string rejectionUrl); - void SendOrganizationTransformInitiatedNotice(User accountToTransform, User adminUser, string cancellationUrl); - void SendOrganizationTransformRequestAcceptedNotice(User accountToTransform, User adminUser); - void SendOrganizationTransformRequestRejectedNotice(User accountToTransform, User adminUser); - void SendOrganizationTransformRequestCancelledNotice(User accountToTransform, User adminUser); - void SendOrganizationMembershipRequest(Organization organization, User newUser, User adminUser, bool isAdmin, string profileUrl, string confirmationUrl, string rejectionUrl); - void SendOrganizationMembershipRequestInitiatedNotice(Organization organization, User requestingUser, User pendingUser, bool isAdmin, string cancellationUrl); - void SendOrganizationMembershipRequestRejectedNotice(Organization organization, User pendingUser); - void SendOrganizationMembershipRequestCancelledNotice(Organization organization, User pendingUser); - void SendOrganizationMemberUpdatedNotice(Organization organization, Membership membership); - void SendOrganizationMemberRemovedNotice(Organization organization, User removedUser); + Task SendContactOwnersMessageAsync(MailAddress fromAddress, Package package, string packageUrl, string message, string emailSettingsUrl, bool copyFromAddress); + Task ReportAbuseAsync(ReportPackageRequest report); + Task ReportMyPackageAsync(ReportPackageRequest report); + Task SendNewAccountEmailAsync(User newUser, string confirmationUrl); + Task SendEmailChangeConfirmationNoticeAsync(User user, string confirmationUrl); + Task SendPasswordResetInstructionsAsync(User user, string resetPasswordUrl, bool forgotPassword); + Task SendEmailChangeNoticeToPreviousEmailAddressAsync(User user, string oldEmailAddress); + Task SendPackageOwnerRequestAsync(User fromUser, User toUser, PackageRegistration package, string packageUrl, string confirmationUrl, string rejectionUrl, string message, string policyMessage); + Task SendPackageOwnerRequestInitiatedNoticeAsync(User requestingOwner, User receivingOwner, User newOwner, PackageRegistration package, string cancellationUrl); + Task SendPackageOwnerRequestRejectionNoticeAsync(User requestingOwner, User newOwner, PackageRegistration package); + Task SendPackageOwnerRequestCancellationNoticeAsync(User requestingOwner, User newOwner, PackageRegistration package); + Task SendPackageOwnerAddedNoticeAsync(User toUser, User newOwner, PackageRegistration package, string packageUrl); + Task SendPackageOwnerRemovedNoticeAsync(User fromUser, User toUser, PackageRegistration package); + Task SendCredentialRemovedNoticeAsync(User user, CredentialViewModel removedCredentialViewModel); + Task SendCredentialAddedNoticeAsync(User user, CredentialViewModel addedCredentialViewModel); + Task SendContactSupportEmailAsync(ContactSupportRequest request); + Task SendPackageAddedNoticeAsync(Package package, string packageUrl, string packageSupportUrl, string emailSettingsUrl); + Task SendAccountDeleteNoticeAsync(User user); + Task SendPackageDeletedNoticeAsync(Package package, string packageUrl, string packageSupportUrl); + Task SendSigninAssistanceEmailAsync(MailAddress emailAddress, IEnumerable credentials); + Task SendOrganizationTransformRequestAsync(User accountToTransform, User adminUser, string profileUrl, string confirmationUrl, string rejectionUrl); + Task SendOrganizationTransformInitiatedNoticeAsync(User accountToTransform, User adminUser, string cancellationUrl); + Task SendOrganizationTransformRequestAcceptedNoticeAsync(User accountToTransform, User adminUser); + Task SendOrganizationTransformRequestRejectedNoticeAsync(User accountToTransform, User adminUser); + Task SendOrganizationTransformRequestCancelledNoticeAsync(User accountToTransform, User adminUser); + Task SendOrganizationMembershipRequestAsync(Organization organization, User newUser, User adminUser, bool isAdmin, string profileUrl, string confirmationUrl, string rejectionUrl); + Task SendOrganizationMembershipRequestInitiatedNoticeAsync(Organization organization, User requestingUser, User pendingUser, bool isAdmin, string cancellationUrl); + Task SendOrganizationMembershipRequestRejectedNoticeAsync(Organization organization, User pendingUser); + Task SendOrganizationMembershipRequestCancelledNoticeAsync(Organization organization, User pendingUser); + Task SendOrganizationMemberUpdatedNoticeAsync(Organization organization, Membership membership); + Task SendOrganizationMemberRemovedNoticeAsync(Organization organization, User removedUser); } } \ No newline at end of file diff --git a/src/NuGetGallery/Services/MessageService.cs b/src/NuGetGallery/Services/MessageService.cs index 45f17d392f..009f9ca2eb 100644 --- a/src/NuGetGallery/Services/MessageService.cs +++ b/src/NuGetGallery/Services/MessageService.cs @@ -34,7 +34,7 @@ public IAppConfiguration Config set { CoreConfiguration = value; } } - public void ReportAbuse(ReportPackageRequest request) + public async Task ReportAbuseAsync(ReportPackageRequest request) { string subject = "[{GalleryOwnerName}] Support Request for '{Id}' version {Version} (Reason: {Reason})"; subject = request.FillIn(subject, Config); @@ -82,11 +82,11 @@ public void ReportAbuse(ReportPackageRequest request) // CCing helps to create a thread of email that can be augmented by the sending user mailMessage.CC.Add(request.FromAddress); } - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); } } - public void ReportMyPackage(ReportPackageRequest request) + public async Task ReportMyPackageAsync(ReportPackageRequest request) { string subject = "[{GalleryOwnerName}] Owner Support Request for '{Id}' version {Version} (Reason: {Reason})"; subject = request.FillIn(subject, Config); @@ -129,11 +129,11 @@ public void ReportMyPackage(ReportPackageRequest request) // CCing helps to create a thread of email that can be augmented by the sending user mailMessage.CC.Add(request.FromAddress); } - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); } } - public void SendContactOwnersMessage(MailAddress fromAddress, Package package, string packageUrl, string message, string emailSettingsUrl, bool copySender) + public async Task SendContactOwnersMessageAsync(MailAddress fromAddress, Package package, string packageUrl, string message, string emailSettingsUrl, bool copySender) { string subject = "[{0}] Message for owners of the package '{1}'"; string body = @"_User {0} <{1}> sends the following message to the owners of Package '[{2} {3}]({4})'._ @@ -171,16 +171,16 @@ [change your email notification settings]({7}). if (mailMessage.To.Any()) { - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); if (copySender) { - SendMessageToSender(mailMessage); + await SendMessageToSenderAsync(mailMessage); } } } } - public void SendNewAccountEmail(User newUser, string confirmationUrl) + public async Task SendNewAccountEmailAsync(User newUser, string confirmationUrl) { var isOrganization = newUser is Organization; @@ -201,11 +201,11 @@ We can't wait to see what packages you'll upload. mailMessage.From = Config.GalleryNoReplyAddress; mailMessage.To.Add(newUser.ToMailAddress()); - SendMessage(mailMessage); + await SendMessageAsync(mailMessage); } } - public void SendSigninAssistanceEmail(MailAddress emailAddress, IEnumerable credentials) + public async Task SendSigninAssistanceEmailAsync(MailAddress emailAddress, IEnumerable credentials) { string body = @"Hi there, @@ -242,12 +242,12 @@ public void SendSigninAssistanceEmail(MailAddress emailAddress, IEnumerable() - .Verify(m => m.SendEmailChangeConfirmationNotice(It.IsAny(), It.IsAny()), + .Verify(m => m.SendEmailChangeConfirmationNoticeAsync(It.IsAny(), It.IsAny()), Times.Once); } @@ -268,7 +268,7 @@ public virtual async Task WhenNewEmailIsDifferentAndWasUnconfirmed_SavesChanges( ResultAssert.IsRedirectToRoute(result, new { action = controller.AccountAction }); GetMock() - .Verify(m => m.SendEmailChangeConfirmationNotice(It.IsAny(), It.IsAny()), + .Verify(m => m.SendEmailChangeConfirmationNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } @@ -291,7 +291,7 @@ protected virtual Task InvokeChangeEmail( controller.SetCurrentUser(getCurrentUser(Fakes)); var messageService = GetMock(); - messageService.Setup(m => m.SendEmailChangeConfirmationNotice(It.IsAny(), It.IsAny())) + messageService.Setup(m => m.SendEmailChangeConfirmationNoticeAsync(It.IsAny(), It.IsAny())) .Verifiable(); var userService = GetMock(); @@ -474,7 +474,7 @@ public virtual void WhenAlreadyConfirmed_DoesNotSendEmail(Func getC // Assert var mailService = GetMock(); - mailService.Verify(m => m.SendNewAccountEmail(It.IsAny(), It.IsAny()), Times.Never); + mailService.Verify(m => m.SendNewAccountEmailAsync(It.IsAny(), It.IsAny()), Times.Never); var model = ResultAssert.IsView(result); Assert.False(model.SentEmail); @@ -500,7 +500,7 @@ public virtual void WhenIsNotConfirmed_SendsEmail(Func getCurrentUs // Assert var mailService = GetMock(); - mailService.Verify(m => m.SendNewAccountEmail(It.IsAny(), confirmationUrl), Times.Once); + mailService.Verify(m => m.SendNewAccountEmailAsync(It.IsAny(), confirmationUrl), Times.Once); var model = ResultAssert.IsView(result); Assert.True(model.SentEmail); @@ -519,7 +519,7 @@ protected virtual ActionResult InvokeConfirmationRequiredPost( .Returns(account as User); GetMock() - .Setup(m => m.SendNewAccountEmail( + .Setup(m => m.SendNewAccountEmailAsync( account, string.IsNullOrEmpty(confirmationUrl) ? It.IsAny() : confirmationUrl)) .Verifiable(); @@ -594,7 +594,7 @@ public virtual async Task WhenAlreadyConfirmed_DoesNotConfirmEmailAddress(Func(); - mailService.Verify(m => m.SendEmailChangeNoticeToPreviousEmailAddress( + mailService.Verify(m => m.SendEmailChangeNoticeToPreviousEmailAddressAsync( It.IsAny(), It.IsAny()), Times.Never); @@ -627,7 +627,7 @@ public virtual async Task WhenIsNotConfirmedAndNoExistingEmail_ConfirmsEmailAddr Times.Once); var mailService = GetMock(); - mailService.Verify(m => m.SendEmailChangeNoticeToPreviousEmailAddress( + mailService.Verify(m => m.SendEmailChangeNoticeToPreviousEmailAddressAsync( It.IsAny(), It.IsAny()), Times.Never); @@ -659,7 +659,7 @@ public virtual async Task WhenIsNotConfirmedAndHasExistingEmail_ConfirmsEmailAdd Times.Once); var mailService = GetMock(); - mailService.Verify(m => m.SendEmailChangeNoticeToPreviousEmailAddress( + mailService.Verify(m => m.SendEmailChangeNoticeToPreviousEmailAddressAsync( It.IsAny(), It.IsAny()), Times.Once); @@ -730,9 +730,10 @@ protected virtual Task InvokeConfirm( } GetMock() - .Setup(m => m.SendEmailChangeNoticeToPreviousEmailAddress( + .Setup(m => m.SendEmailChangeNoticeToPreviousEmailAddressAsync( It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask) .Verifiable(); // Act diff --git a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs index fd6279a9cf..ee3a086e36 100644 --- a/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs @@ -326,7 +326,7 @@ public async Task CreatePackageWillSendPackageAddedNotice(bool asyncValidationEn // Assert controller.MockMessageService - .Verify(ms => ms.SendPackageAddedNotice(package, It.IsAny(), It.IsAny(), It.IsAny()), + .Verify(ms => ms.SendPackageAddedNoticeAsync(package, It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(callExpected ? 1 : 0)); } diff --git a/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs index 539021f2fc..5ee5d92277 100644 --- a/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs @@ -197,7 +197,7 @@ public void SendsNotificationForAssistance() .Returns(existingUser); var messageServiceMock = GetMock(); messageServiceMock - .Setup(m => m.SendSigninAssistanceEmail(It.IsAny(), It.IsAny>())) + .Setup(m => m.SendSigninAssistanceEmailAsync(It.IsAny(), It.IsAny>())) .Verifiable(); var controller = GetController(); @@ -396,7 +396,7 @@ public async Task WhenAttemptingToLinkExternalToExistingAccountWithNoExternalAcc .Verify(x => x.RemoveCredential(user, passwordCredential)); GetMock() - .Verify(x => x.SendCredentialAddedNotice(It.IsAny(), It.IsAny())); + .Verify(x => x.SendCredentialAddedNoticeAsync(It.IsAny(), It.IsAny())); } public async Task WhenAttemptingToLinkExternalToAccountWithExistingExternals_RejectsLinking() @@ -491,7 +491,7 @@ public async Task WhenAttemptingToLinkExternalToAdminUserWithExistingExternals_A .Verify(x => x.CreateSessionAsync(controller.OwinContext, authUser, false)); GetMock() - .Verify(x => x.SendCredentialAddedNotice(authUser.User, credentialViewModel)); + .Verify(x => x.SendCredentialAddedNoticeAsync(authUser.User, credentialViewModel)); } [Fact] @@ -548,8 +548,9 @@ public async Task GivenValidExternalAuth_ItLinksCredentialSendsEmailAndLogsIn() .Completes() .Verifiable(); GetMock() - .Setup(x => x.SendCredentialAddedNotice(authUser.User, + .Setup(x => x.SendCredentialAddedNoticeAsync(authUser.User, It.Is(c => c.Type == CredentialTypes.External.MicrosoftAccount))) + .Returns(Task.CompletedTask) .Verifiable(); var controller = GetController(); @@ -617,8 +618,9 @@ public async Task GivenAdminLogsInWithValidExternalAuth_ItChallengesWhenNotUsing .Verifiable(); GetMock() - .Setup(x => x.SendCredentialAddedNotice(authUser.User, + .Setup(x => x.SendCredentialAddedNoticeAsync(authUser.User, It.Is(c => c.Type == CredentialTypes.External.Prefix + providerUsedForLogin))) + .Returns(Task.CompletedTask) .Verifiable(); EnableAllAuthenticators(Get()); @@ -749,7 +751,7 @@ public async Task WillCreateAndLogInTheUserWhenNotLinking() GetMock().VerifyAll(); GetMock() - .Verify(x => x.SendNewAccountEmail( + .Verify(x => x.SendNewAccountEmailAsync( authUser.User, TestUtility.GallerySiteRootHttps + "account/confirm/" + authUser.User.Username + "/" + authUser.User.EmailConfirmationToken)); ResultAssert.IsSafeRedirectTo(result, "/theReturnUrl"); @@ -795,7 +797,7 @@ public async Task WillNotSendConfirmationEmailWhenConfirmEmailAddressesIsOff() // Assert GetMock() - .Verify(x => x.SendNewAccountEmail( + .Verify(x => x.SendNewAccountEmailAsync( It.IsAny(), It.IsAny()), Times.Never()); } @@ -885,7 +887,7 @@ public async Task GivenValidExternalAuth_ItCreatesAccountAndLinksCredential() authenticationServiceMock.VerifyAll(); GetMock() - .Verify(x => x.SendNewAccountEmail( + .Verify(x => x.SendNewAccountEmailAsync( authUser.User, TestUtility.GallerySiteRootHttps + "account/confirm/" + authUser.User.Username + "/" + authUser.User.EmailConfirmationToken)); diff --git a/tests/NuGetGallery.Facts/Controllers/JsonApiControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/JsonApiControllerFacts.cs index c72b300488..ed2a009e03 100644 --- a/tests/NuGetGallery.Facts/Controllers/JsonApiControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/JsonApiControllerFacts.cs @@ -365,7 +365,7 @@ public async Task CreatesPackageOwnerRequestSendsEmailAndReturnsPendingState(Fun .Verifiable(); messageServiceMock - .Setup(m => m.SendPackageOwnerRequest( + .Setup(m => m.SendPackageOwnerRequestAsync( currentUser, userToAdd, fakes.Package, @@ -374,17 +374,19 @@ public async Task CreatesPackageOwnerRequestSendsEmailAndReturnsPendingState(Fun TestUtility.GallerySiteRootHttps + $"packages/FakePackage/owners/{userToAdd.Username}/reject/confirmation-code", "Hello World! Html Encoded <3", "")) + .Returns(Task.CompletedTask) .Verifiable(); foreach (var owner in fakes.Package.Owners) { messageServiceMock - .Setup(m => m.SendPackageOwnerRequestInitiatedNotice( + .Setup(m => m.SendPackageOwnerRequestInitiatedNoticeAsync( currentUser, owner, userToAdd, fakes.Package, It.IsAny())) + .Returns(Task.CompletedTask) .Verifiable(); } } @@ -398,11 +400,12 @@ public async Task CreatesPackageOwnerRequestSendsEmailAndReturnsPendingState(Fun foreach (var owner in fakes.Package.Owners) { messageServiceMock - .Setup(m => m.SendPackageOwnerAddedNotice( + .Setup(m => m.SendPackageOwnerAddedNoticeAsync( owner, userToAdd, fakes.Package, It.IsAny())) + .Returns(Task.CompletedTask) .Verifiable(); } } @@ -540,7 +543,7 @@ public async Task RemovesPackageOwnerRequest(Func getCurrentUser, F packageOwnershipManagementService.Verify(x => x.DeletePackageOwnershipRequestAsync(package, requestedUser)); GetMock() - .Verify(x => x.SendPackageOwnerRequestCancellationNotice(currentUser, requestedUser, package)); + .Verify(x => x.SendPackageOwnerRequestCancellationNoticeAsync(currentUser, requestedUser, package)); } [Theory] @@ -571,7 +574,7 @@ public async Task RemovesExistingOwner(Func getCurrentUser, Func x.RemovePackageOwnerAsync(package, currentUser, userToRemove, It.IsAny())); GetMock() - .Verify(x => x.SendPackageOwnerRemovedNotice(currentUser, userToRemove, package)); + .Verify(x => x.SendPackageOwnerRemovedNoticeAsync(currentUser, userToRemove, package)); } } diff --git a/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs index 0c81c73903..ae85c7dccb 100644 --- a/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs @@ -381,7 +381,7 @@ public async Task WhenAddOrganizationSucceeds_RedirectsToManageOrganization() new { accountName = org.Username, action = nameof(OrganizationsController.ManageOrganization) }); messageService.Verify( - x => x.SendNewAccountEmail( + x => x.SendNewAccountEmailAsync( org, It.Is(s => s.Contains(token))), Times.Once()); @@ -499,7 +499,7 @@ public async Task WhenMembershipRequestCreated_ReturnsSuccess(Func GetMock().Verify(s => s.AddMembershipRequestAsync(account, defaultMemberName, isAdmin), Times.Once); GetMock() - .Verify(s => s.SendOrganizationMembershipRequest( + .Verify(s => s.SendOrganizationMembershipRequestAsync( account, It.Is(u => u.Username == defaultMemberName), controller.GetCurrentUser(), @@ -508,7 +508,7 @@ public async Task WhenMembershipRequestCreated_ReturnsSuccess(Func It.IsAny(), It.IsAny())); GetMock() - .Verify(s => s.SendOrganizationMembershipRequestInitiatedNotice( + .Verify(s => s.SendOrganizationMembershipRequestInitiatedNoticeAsync( account, controller.GetCurrentUser(), It.Is(u => u.Username == defaultMemberName), @@ -571,7 +571,7 @@ public async Task WhenAccountMissingReturns404(bool isAdmin) ResultAssert.IsStatusCode(result, HttpStatusCode.NotFound); GetMock().Verify(s => s.AddMemberAsync(It.IsAny(), Fakes.User.Username, defaultConfirmationToken), Times.Never); - GetMock().Verify(s => s.SendOrganizationMemberUpdatedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMemberUpdatedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -597,7 +597,7 @@ public async Task WhenEntityException_ReturnsNonSuccess(bool isAdmin) Assert.False(model.Successful); GetMock().Verify(s => s.AddMemberAsync(account, Fakes.User.Username, defaultConfirmationToken), Times.Once); - GetMock().Verify(s => s.SendOrganizationMemberUpdatedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMemberUpdatedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -623,7 +623,7 @@ public async Task WhenMembershipRequestCreated_ReturnsSuccess(bool isAdmin) GetMock().Verify(s => s.AddMemberAsync(account, Fakes.User.Username, defaultConfirmationToken), Times.Once); GetMock() - .Verify(s => s.SendOrganizationMemberUpdatedNotice( + .Verify(s => s.SendOrganizationMemberUpdatedNoticeAsync( account, It.Is(m => Fakes.User.Username == m.Member.Username && m.Organization == account && m.IsAdmin == isAdmin)), Times.Once); } @@ -686,7 +686,7 @@ public async Task WhenAccountMissingReturns404(bool isAdmin) ResultAssert.IsStatusCode(result, HttpStatusCode.NotFound); GetMock().Verify(s => s.RejectMembershipRequestAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - GetMock().Verify(s => s.SendOrganizationMembershipRequestRejectedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMembershipRequestRejectedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -712,7 +712,7 @@ public async Task WhenEntityException_ReturnsNonSuccess(bool isAdmin) Assert.False(model.Successful); GetMock().Verify(s => s.RejectMembershipRequestAsync(account, Fakes.User.Username, defaultConfirmationToken), Times.Once); - GetMock().Verify(s => s.SendOrganizationMembershipRequestRejectedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMembershipRequestRejectedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -735,7 +735,7 @@ public async Task WhenMembershipRequestCreated_ReturnsSuccess(bool isAdmin) Assert.True(model.Successful); GetMock().Verify(s => s.RejectMembershipRequestAsync(account, Fakes.User.Username, defaultConfirmationToken), Times.Once); - GetMock().Verify(s => s.SendOrganizationMembershipRequestRejectedNotice(account, Fakes.User), Times.Once); + GetMock().Verify(s => s.SendOrganizationMembershipRequestRejectedNoticeAsync(account, Fakes.User), Times.Once); } private Task InvokeRejectMember( @@ -883,7 +883,7 @@ public async Task WhenMembershipCreated_ReturnsSuccess(Func getCurr GetMock().Verify(s => s.UpdateMemberAsync(account, defaultMemberName, isAdmin), Times.Once); GetMock() - .Verify(s => s.SendOrganizationMemberUpdatedNotice( + .Verify(s => s.SendOrganizationMemberUpdatedNoticeAsync( account, It.Is(m => m.Organization == account && m.Member.Username == defaultMemberName && m.IsAdmin == isAdmin))); } @@ -962,7 +962,7 @@ public async Task WithNonOrganizationAdmin_ReturnsForbidden(Func ge Assert.Equal(Strings.Unauthorized, result.Data); GetMock().Verify(s => s.DeleteMemberAsync(It.IsAny(), It.IsAny()), Times.Never); - GetMock().Verify(s => s.SendOrganizationMemberRemovedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMemberRemovedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -983,7 +983,7 @@ public async Task WhenOrganizationIsUnconfirmed_ReturnsNonSuccess(Func().Verify(s => s.DeleteMemberAsync(It.IsAny(), It.IsAny()), Times.Never); - GetMock().Verify(s => s.SendOrganizationMemberRemovedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMemberRemovedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -1003,7 +1003,7 @@ public async Task WhenEntityException_ReturnsNonSuccess(Func getCur Assert.Equal("error", result.Data); GetMock().Verify(s => s.DeleteMemberAsync(account, defaultMemberName), Times.Once); - GetMock().Verify(s => s.SendOrganizationMemberRemovedNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMemberRemovedNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -1025,7 +1025,7 @@ public async Task WhenDeletingAsAdmin_ReturnsSuccess(Func getCurren GetMock() .Verify(s => s.DeleteMemberAsync(account, defaultMemberName), Times.Once); GetMock() - .Verify(s => s.SendOrganizationMemberRemovedNotice(account, It.Is(u => u.Username == defaultMemberName)), Times.Once); + .Verify(s => s.SendOrganizationMemberRemovedNoticeAsync(account, It.Is(u => u.Username == defaultMemberName)), Times.Once); } [Fact] @@ -1116,7 +1116,7 @@ public async Task WithNonOrganizationAdmin_ReturnsForbidden(Func ge Assert.Equal(Strings.Unauthorized, result.Data); GetMock().Verify(s => s.CancelMembershipRequestAsync(It.IsAny(), It.IsAny()), Times.Never); - GetMock().Verify(s => s.SendOrganizationMembershipRequestCancelledNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMembershipRequestCancelledNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -1136,7 +1136,7 @@ public async Task WhenEntityException_ReturnsNonSuccess(Func getCur Assert.Equal("error", result.Data); GetMock().Verify(s => s.CancelMembershipRequestAsync(account, defaultMemberName), Times.Once); - GetMock().Verify(s => s.SendOrganizationMembershipRequestCancelledNotice(It.IsAny(), It.IsAny()), Times.Never); + GetMock().Verify(s => s.SendOrganizationMembershipRequestCancelledNoticeAsync(It.IsAny(), It.IsAny()), Times.Never); } [Theory] @@ -1156,7 +1156,7 @@ public async Task WhenSuccess_ReturnsSuccess(Func getCurrentUser) Assert.Equal(Strings.CancelMemberRequest_Success, result.Data); GetMock().Verify(s => s.CancelMembershipRequestAsync(account, defaultMemberName), Times.Once); - GetMock().Verify(s => s.SendOrganizationMembershipRequestCancelledNotice(account, It.Is(u => u.Username == defaultMemberName)), Times.Once); + GetMock().Verify(s => s.SendOrganizationMembershipRequestCancelledNoticeAsync(account, It.Is(u => u.Username == defaultMemberName)), Times.Once); } private Task InvokeCancelMemberRequestMember( diff --git a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs index 0dc0d115e1..90f88bcdf8 100644 --- a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs @@ -889,7 +889,7 @@ private static Expression> Packag private static Expression> MessageServiceForConfirmOwnershipRequestExpression(PackageOwnerRequest request) { - return messageService => messageService.SendPackageOwnerAddedNotice( + return messageService => messageService.SendPackageOwnerAddedNoticeAsync( request.RequestingOwner, request.NewOwner, request.PackageRegistration, @@ -898,7 +898,7 @@ private static Expression> MessageServiceForConfirmOwner private static Expression> MessageServiceForRejectOwnershipRequestExpression(PackageOwnerRequest request) { - return messageService => messageService.SendPackageOwnerRequestRejectionNotice(request.RequestingOwner, request.NewOwner, request.PackageRegistration); + return messageService => messageService.SendPackageOwnerRequestRejectionNoticeAsync(request.RequestingOwner, request.NewOwner, request.PackageRegistration); } public static IEnumerable ReturnsSuccessIfTokenIsValid_Data @@ -1164,7 +1164,7 @@ public async Task ReturnsCancelledIfPackageOwnershipRequestExists(User currentUs Assert.Equal(packageId, model.PackageId); packageService.Verify(); packageOwnershipManagementRequestService.Verify(); - messageService.Verify(m => m.SendPackageOwnerRequestCancellationNotice(userA, userB, package)); + messageService.Verify(m => m.SendPackageOwnerRequestCancellationNoticeAsync(userA, userB, package)); } } } @@ -1376,7 +1376,7 @@ public void HtmlEncodesMessageContent() var messageService = new Mock(); string sentMessage = null; messageService.Setup( - s => s.SendContactOwnersMessage( + s => s.SendContactOwnersMessageAsync( It.IsAny(), It.IsAny(), It.IsAny(), @@ -1424,7 +1424,7 @@ public void CallsSendContactOwnersMessageWithUserInfo() var messageService = new Mock(); messageService.Setup( - s => s.SendContactOwnersMessage( + s => s.SendContactOwnersMessageAsync( It.IsAny(), It.IsAny(), It.IsAny(), @@ -2594,7 +2594,7 @@ public async Task FormSendsMessageToGalleryOwnerWithEmailOnlyWhenUnauthenticated Assert.NotNull(result); messageService.Verify( - s => s.ReportAbuse( + s => s.ReportAbuseAsync( It.Is( r => r.FromAddress.Address == ReporterEmailAddress && r.Package == package @@ -2628,7 +2628,7 @@ public async Task FormSendsMessageToGalleryOwnerWithUserInfoWhenAuthenticated(Us Assert.NotNull(result); messageService.Verify( - s => s.ReportAbuse( + s => s.ReportAbuseAsync( It.Is( r => r.Message == EncodedMessage && r.FromAddress.Address == currentUser.EmailAddress @@ -2642,7 +2642,7 @@ public Task GetReportAbuseFormResult(User currentUser, User owner, { messageService = new Mock(); messageService.Setup( - s => s.ReportAbuse(It.Is(r => r.Message == UnencodedMessage))); + s => s.ReportAbuseAsync(It.Is(r => r.Message == UnencodedMessage))); package = new Package { PackageRegistration = new PackageRegistration { Id = PackageId, Owners = new[] { owner } }, @@ -2881,7 +2881,7 @@ public async Task HtmlEncodesMessageContent(User currentUser, User owner) ReportPackageRequest reportRequest = null; _messageService - .Setup(s => s.ReportMyPackage(It.IsAny())) + .Setup(s => s.ReportMyPackageAsync(It.IsAny())) .Callback(r => reportRequest = r); // Act @@ -3007,7 +3007,7 @@ public async Task AllowsPackageDelete(User currentUser, User owner) currentUser.Username), Times.Once); _messageService.Verify( - x => x.SendPackageDeletedNotice( + x => x.SendPackageDeletedNoticeAsync( _package, It.IsAny(), It.IsAny()), @@ -3015,7 +3015,7 @@ public async Task AllowsPackageDelete(User currentUser, User owner) Assert.Equal(Strings.UserPackageDeleteCompleteTransientMessage, _controller.TempData["Message"]); _messageService.Verify( - x => x.ReportMyPackage(It.IsAny()), + x => x.ReportMyPackageAsync(It.IsAny()), Times.Never); } @@ -3067,13 +3067,13 @@ public async Task TreatsDeleteFailureAsNormalRequest(User currentUser, User owne It.IsAny()), Times.Never); _messageService.Verify( - x => x.SendPackageDeletedNotice( + x => x.SendPackageDeletedNoticeAsync( It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); _messageService.Verify( - x => x.ReportMyPackage(It.IsAny()), + x => x.ReportMyPackageAsync(It.IsAny()), Times.Once); } @@ -3233,7 +3233,7 @@ public async Task DoesNotRequireDeleteDecision(User currentUser, User owner, Rep It.IsAny()), Times.Never); _messageService.Verify( - x => x.ReportMyPackage(It.IsAny()), + x => x.ReportMyPackageAsync(It.IsAny()), Times.Once); } @@ -3273,7 +3273,7 @@ public async Task IgnoresDeleteRequestWhenNotAllowed(User currentUser, User owne It.IsAny()), Times.Never); _messageService.Verify( - x => x.ReportMyPackage(It.IsAny()), + x => x.ReportMyPackageAsync(It.IsAny()), Times.Once); } } @@ -5075,7 +5075,7 @@ public async Task WillSendPackageAddedNotice(bool asyncValidationEnabled, bool b // Assert fakeMessageService - .Verify(ms => ms.SendPackageAddedNotice(fakePackage, It.IsAny(), It.IsAny(), It.IsAny()), + .Verify(ms => ms.SendPackageAddedNoticeAsync(fakePackage, It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(callExpected ? 1 : 0)); } } diff --git a/tests/NuGetGallery.Facts/Controllers/PagesControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/PagesControllerFacts.cs index 16c4f46b1c..91841165fc 100644 --- a/tests/NuGetGallery.Facts/Controllers/PagesControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/PagesControllerFacts.cs @@ -43,7 +43,7 @@ public async Task HtmlEncodesTheSupportContactEmail() // assert: the HTML encoded message was passed to the service GetMock() - .Verify(m => m.SendContactSupportEmail( + .Verify(m => m.SendContactSupportEmailAsync( It.Is(c => c.Message == expectedMessage && c.SubjectLine == expectedSubjectLine))); diff --git a/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs index 6d80ab7319..0f2d7666bb 100644 --- a/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs @@ -258,7 +258,7 @@ public async Task SendsEmailWithPasswordResetUrl() PasswordResetTokenExpirationDate = DateTime.UtcNow.AddHours(Constants.PasswordResetTokenExpirationHours) }; GetMock() - .Setup(s => s.SendPasswordResetInstructions(user, resetUrl, true)); + .Setup(s => s.SendPasswordResetInstructionsAsync(user, resetUrl, true)); GetMock() .Setup(s => s.FindByEmailAddress("user")) .Returns(user); @@ -271,7 +271,7 @@ public async Task SendsEmailWithPasswordResetUrl() await controller.ForgotPassword(model); GetMock() - .Verify(s => s.SendPasswordResetInstructions(user, resetUrl, true)); + .Verify(s => s.SendPasswordResetInstructionsAsync(user, resetUrl, true)); } [Fact] @@ -447,7 +447,7 @@ public async Task SendsPasswordAddedMessageWhenForgotFalse() await controller.ResetPassword("user", "token", model, forgot: false); GetMock() - .Verify(m => m.SendCredentialAddedNotice(cred.User, + .Verify(m => m.SendCredentialAddedNoticeAsync(cred.User, It.Is(c => c.Type == cred.Type))); } @@ -952,7 +952,7 @@ public async Task SendsNotificationMailToUser() expirationInDays: 90); GetMock() - .Verify(m => m.SendCredentialAddedNotice(user, It.IsAny())); + .Verify(m => m.SendCredentialAddedNoticeAsync(user, It.IsAny())); } } @@ -1309,7 +1309,7 @@ public async Task GivenDisabledPasswordLogin_RemovesCredentialAndSendsNotice() .Verifiable(); GetMock() .Setup(m => - m.SendCredentialRemovedNotice( + m.SendCredentialRemovedNoticeAsync( user, It.Is(c => c.Type == CredentialTypes.External.MicrosoftAccount))) .Verifiable(); @@ -1378,8 +1378,12 @@ public async Task GivenNoOldPassword_ItSendsAPasswordSetEmail() string actualConfirmUrl = null; GetMock() - .Setup(a => a.SendPasswordResetInstructions(user, It.IsAny(), false)) - .Callback((_, url, __) => actualConfirmUrl = url) + .Setup(a => a.SendPasswordResetInstructionsAsync(user, It.IsAny(), false)) + .Returns((_, url, __) => + { + actualConfirmUrl = url; + return Task.CompletedTask; + }) .Verifiable(); var controller = GetController(); @@ -1510,9 +1514,10 @@ public async Task GivenValidRequest_ItRemovesCredAndSendsNotificationToUser() .Completes() .Verifiable(); GetMock() - .Setup(m => m.SendCredentialRemovedNotice( + .Setup(m => m.SendCredentialRemovedNoticeAsync( user, It.Is(c => c.Type == cred.Type))) + .Returns(Task.CompletedTask) .Verifiable(); var controller = GetController(); @@ -1616,9 +1621,10 @@ public async Task GivenValidRequest_ItRemovesCredAndSendsNotificationToUser() .Verifiable(); GetMock() .Setup(m => - m.SendCredentialRemovedNotice( + m.SendCredentialRemovedNoticeAsync( user, It.Is(c => c.Type == CredentialTypes.External.MicrosoftAccount))) + .Returns(Task.CompletedTask) .Verifiable(); var controller = GetController(); @@ -2305,7 +2311,7 @@ public async Task SucceedsIfSupportRequestIsAdded(bool successOnSentRequest) Assert.Equal(!successOnSentRequest, tempData); GetMock() .Verify( - stub => stub.SendAccountDeleteNotice(testUser), + stub => stub.SendAccountDeleteNoticeAsync(testUser), successOnSentRequest ? Times.Once() : Times.Never()); } @@ -2433,7 +2439,7 @@ public async Task WhenCanTransformReturnsFalse_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequest( + m.SendOrganizationTransformRequestAsync( It.IsAny(), It.IsAny(), It.IsAny(), @@ -2443,7 +2449,7 @@ public async Task WhenCanTransformReturnsFalse_ShowsError() GetMock() .Verify( - m => m.SendOrganizationTransformInitiatedNotice( + m => m.SendOrganizationTransformInitiatedNoticeAsync( It.IsAny(), It.IsAny(), It.IsAny()), @@ -2478,7 +2484,7 @@ public async Task WhenAdminIsNotFound_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequest( + m.SendOrganizationTransformRequestAsync( It.IsAny(), It.IsAny(), It.IsAny(), @@ -2488,7 +2494,7 @@ public async Task WhenAdminIsNotFound_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformInitiatedNotice( + m.SendOrganizationTransformInitiatedNoticeAsync( It.IsAny(), It.IsAny(), It.IsAny()), @@ -2517,7 +2523,7 @@ public async Task WhenValid_CreatesRequestAndRedirects() Assert.IsType(result); GetMock() - .Verify(m => m.SendOrganizationTransformRequest( + .Verify(m => m.SendOrganizationTransformRequestAsync( It.IsAny(), It.IsAny(), It.IsAny(), @@ -2525,7 +2531,7 @@ public async Task WhenValid_CreatesRequestAndRedirects() It.IsAny())); GetMock() - .Verify(m => m.SendOrganizationTransformInitiatedNotice( + .Verify(m => m.SendOrganizationTransformInitiatedNoticeAsync( It.IsAny(), It.IsAny(), It.IsAny())); @@ -2559,7 +2565,7 @@ public async Task WhenAccountToTransformIsNotFound_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequestAcceptedNotice( + m.SendOrganizationTransformRequestAcceptedNoticeAsync( It.IsAny(), It.IsAny()), Times.Never()); @@ -2590,7 +2596,7 @@ public async Task WhenCanTransformReturnsFalse_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequestAcceptedNotice( + m.SendOrganizationTransformRequestAcceptedNoticeAsync( It.IsAny(), It.IsAny()), Times.Never()); @@ -2622,7 +2628,7 @@ public async Task WhenUserServiceReturnsFalse_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequestAcceptedNotice( + m.SendOrganizationTransformRequestAcceptedNoticeAsync( It.IsAny(), It.IsAny()), Times.Never()); @@ -2649,7 +2655,7 @@ public async Task WhenUserServiceReturnsSuccess_Redirects() GetMock() .Verify(m => - m.SendOrganizationTransformRequestAcceptedNotice( + m.SendOrganizationTransformRequestAcceptedNoticeAsync( It.IsAny(), It.IsAny())); @@ -2711,7 +2717,7 @@ public async Task WhenAccountToTransformIsNotFound_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequestRejectedNotice( + m.SendOrganizationTransformRequestRejectedNoticeAsync( It.IsAny(), It.IsAny()), Times.Never()); @@ -2738,7 +2744,7 @@ public async Task WhenUserServiceReturnsFalse_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequestRejectedNotice( + m.SendOrganizationTransformRequestRejectedNoticeAsync( It.IsAny(), It.IsAny()), Times.Never()); @@ -2767,7 +2773,7 @@ public async Task WhenUserServiceReturnsSuccess_Redirects() GetMock() .Verify(m => - m.SendOrganizationTransformRequestRejectedNotice( + m.SendOrganizationTransformRequestRejectedNoticeAsync( It.IsAny(), It.IsAny())); @@ -2819,7 +2825,7 @@ public async Task WhenUserServiceReturnsFalse_ShowsError() GetMock() .Verify(m => - m.SendOrganizationTransformRequestCancelledNotice( + m.SendOrganizationTransformRequestCancelledNoticeAsync( It.IsAny(), It.IsAny()), Times.Never()); @@ -2847,7 +2853,7 @@ public async Task WhenUserServiceReturnsSuccess_Redirects() GetMock() .Verify(m => - m.SendOrganizationTransformRequestCancelledNotice( + m.SendOrganizationTransformRequestCancelledNoticeAsync( It.IsAny(), It.IsAny())); diff --git a/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs index 164bedbaa7..c74d31384c 100644 --- a/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs @@ -16,6 +16,7 @@ using NuGet.Versioning; using NuGet.Services.Validation; using NuGet.Services.Validation.Issues; +using System.Threading.Tasks; namespace NuGetGallery { @@ -28,7 +29,7 @@ public class TheReportAbuseMethod : TestContainer { [Fact] - public void WillSendEmailToGalleryOwner() + public async Task WillSendEmailToGalleryOwner() { // Arrange var from = new MailAddress("legit@example.com", "too"); @@ -41,7 +42,7 @@ public void WillSendEmailToGalleryOwner() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.ReportAbuse( + await messageService.ReportAbuseAsync( new ReportPackageRequest { AlreadyContactedOwners = true, @@ -69,7 +70,7 @@ public void WillSendEmailToGalleryOwner() } [Fact] - public void WillCopySenderIfAsked() + public async Task WillCopySenderIfAsked() { var from = new MailAddress("legit@example.com", "too"); var package = new Package @@ -96,7 +97,7 @@ public void WillCopySenderIfAsked() Url = TestUtility.MockUrlHelper(), CopySender = true, }; - messageService.ReportAbuse(reportPackageRequest); + await messageService.ReportAbuseAsync(reportPackageRequest); var message = messageService.MockMailSender.Sent.Single(); Assert.Equal(TestGalleryOwner, message.To.Single()); @@ -111,7 +112,7 @@ public class TheReportMyPackageMethod : TestContainer { [Fact] - public void WillSendEmailToGalleryOwner() + public async Task WillSendEmailToGalleryOwner() { var from = new MailAddress("legit@example.com", "too"); var owner = new User @@ -131,7 +132,7 @@ public void WillSendEmailToGalleryOwner() var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.ReportMyPackage( + await messageService.ReportMyPackageAsync( new ReportPackageRequest { FromAddress = from, @@ -157,7 +158,7 @@ public void WillSendEmailToGalleryOwner() } [Fact] - public void WillCopySenderIfAsked() + public async Task WillCopySenderIfAsked() { var from = new MailAddress("legit@example.com", "too"); var package = new Package @@ -184,7 +185,7 @@ public void WillCopySenderIfAsked() Url = TestUtility.MockUrlHelper(), CopySender = true, }; - messageService.ReportMyPackage(reportPackageRequest); + await messageService.ReportMyPackageAsync(reportPackageRequest); var message = messageService.MockMailSender.Sent.Single(); Assert.Equal(TestGalleryOwner, message.To.Single()); @@ -199,7 +200,7 @@ public class TheSendContactOwnersMessageMethod : TestContainer { [Fact] - public void WillCopySenderIfAsked() + public async Task WillCopySenderIfAsked() { // arrange var packageId = "smangit"; @@ -227,7 +228,7 @@ public void WillCopySenderIfAsked() var messageService = TestableMessageService.Create(GetConfigurationService()); // act - messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", true); + await messageService.SendContactOwnersMessageAsync(from, package, "http://someurl/", "Test message", "http://someotherurl/", true); var messages = messageService.MockMailSender.Sent; // assert @@ -244,7 +245,7 @@ public void WillCopySenderIfAsked() } [Fact] - public void WillSendEmailToAllOwners() + public async Task WillSendEmailToAllOwners() { var id = "smangit"; var version = "1.0.0"; @@ -270,7 +271,7 @@ public void WillSendEmailToAllOwners() var messageService = TestableMessageService.Create(GetConfigurationService()); var packageUrl = "http://packageUrl/"; - messageService.SendContactOwnersMessage(from, package, packageUrl, "Test message", "http://emailSettingsUrl/", false); + await messageService.SendContactOwnersMessageAsync(from, package, packageUrl, "Test message", "http://emailSettingsUrl/", false); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(owner1Email, message.To[0].Address); @@ -285,7 +286,7 @@ public void WillSendEmailToAllOwners() } [Fact] - public void WillNotSendEmailToOwnerThatOptsOut() + public async Task WillNotSendEmailToOwnerThatOptsOut() { // arrange var packageId = "smangit"; @@ -312,7 +313,7 @@ public void WillNotSendEmailToOwnerThatOptsOut() var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", false); + await messageService.SendContactOwnersMessageAsync(from, package, "http://someurl/", "Test message", "http://someotherurl/", false); var message = messageService.MockMailSender.Sent.Last(); // assert @@ -321,7 +322,7 @@ public void WillNotSendEmailToOwnerThatOptsOut() } [Fact] - public void WillNotAttemptToSendIfNoOwnersAllow() + public async Task WillNotAttemptToSendIfNoOwnersAllow() { // arrange var packageId = "smangit"; @@ -347,14 +348,14 @@ public void WillNotAttemptToSendIfNoOwnersAllow() }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", false); + await messageService.SendContactOwnersMessageAsync(from, package, "http://someurl/", "Test message", "http://someotherurl/", false); // assert Assert.Empty(messageService.MockMailSender.Sent); } [Fact] - public void WillNotCopySenderIfNoOwnersAllow() + public async Task WillNotCopySenderIfNoOwnersAllow() { // arrange var packageId = "smangit"; @@ -380,7 +381,7 @@ public void WillNotCopySenderIfNoOwnersAllow() }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendContactOwnersMessage(from, package, "http://someurl/", "Test message", "http://someotherurl/", false); + await messageService.SendContactOwnersMessageAsync(from, package, "http://someurl/", "Test message", "http://someotherurl/", false); // assert Assert.Empty(messageService.MockMailSender.Sent); @@ -393,14 +394,14 @@ public class TheSendNewAccountEmailMethod [Theory] [InlineData(false)] [InlineData(true)] - public void WillSendEmailToNewUser(bool isOrganization) + public async Task WillSendEmailToNewUser(bool isOrganization) { var unconfirmedEmailAddress = "unconfirmed@unconfirmed.com"; var user = isOrganization ? new Organization("organization") : new User("user"); user.UnconfirmedEmailAddress = unconfirmedEmailAddress; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendNewAccountEmail(user, "http://example.com/confirmation-token-url"); + await messageService.SendNewAccountEmailAsync(user, "http://example.com/confirmation-token-url"); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(unconfirmedEmailAddress, message.To[0].Address); @@ -417,7 +418,7 @@ public class TheSendEmailChangeConfirmationNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void WillSendEmail(bool isOrganization) + public async Task WillSendEmail(bool isOrganization) { var unconfirmedEmailAddress = "unconfirmed@unconfirmed.com"; var user = isOrganization ? new Organization("organization") : new User("user"); @@ -425,7 +426,7 @@ public void WillSendEmail(bool isOrganization) var tokenUrl = "http://example.com/confirmation-token-url"; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendEmailChangeConfirmationNotice(user, tokenUrl); + await messageService.SendEmailChangeConfirmationNoticeAsync(user, tokenUrl); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.UnconfirmedEmailAddress, message.To[0].Address); @@ -441,7 +442,7 @@ public class TheSendEmailChangeNoticeToPreviousEmailAddressMethod [Theory] [InlineData(false)] [InlineData(true)] - public void WillSendEmail(bool isOrganization) + public async Task WillSendEmail(bool isOrganization) { var newEmail = "new@email.com"; var user = isOrganization ? new Organization("organization") : new User("user"); @@ -449,7 +450,7 @@ public void WillSendEmail(bool isOrganization) var oldEmail = "old@email.com"; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendEmailChangeNoticeToPreviousEmailAddress(user, oldEmail); + await messageService.SendEmailChangeNoticeToPreviousEmailAddressAsync(user, oldEmail); var message = messageService.MockMailSender.Sent.Last(); var accountString = isOrganization ? "organization" : "account"; @@ -467,7 +468,7 @@ public class TheSendPackageOwnerRequestMethod [Theory] [InlineData(false)] [InlineData(true)] - public void SendsPackageOwnerRequestConfirmationUrl(bool isOrganization) + public async Task SendsPackageOwnerRequestConfirmationUrl(bool isOrganization) { var to = isOrganization ? GetOrganizationWithRecipients() : new User(); to.Username = "Noob"; @@ -482,7 +483,7 @@ public void SendsPackageOwnerRequestConfirmationUrl(bool isOrganization) const string userMessage = "Hello World!"; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequest(from, to, package, packageUrl, confirmationUrl, rejectionUrl, userMessage, string.Empty); + await messageService.SendPackageOwnerRequestAsync(from, to, package, packageUrl, confirmationUrl, rejectionUrl, userMessage, string.Empty); var message = messageService.MockMailSender.Sent.Last(); var yourString = isOrganization ? "your organization" : "you"; @@ -508,7 +509,7 @@ public void SendsPackageOwnerRequestConfirmationUrl(bool isOrganization) [Theory] [InlineData(false)] [InlineData(true)] - public void SendsPackageOwnerRequestConfirmationUrlWithoutUserMessage(bool isOrganization) + public async Task SendsPackageOwnerRequestConfirmationUrlWithoutUserMessage(bool isOrganization) { var to = isOrganization ? GetOrganizationWithRecipients() : new User(); to.Username = "Noob"; @@ -521,7 +522,7 @@ public void SendsPackageOwnerRequestConfirmationUrlWithoutUserMessage(bool isOrg const string rejectionUrl = "http://example.com/rejection-token-url"; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequest(from, to, package, packageUrl, confirmationUrl, rejectionUrl, string.Empty, string.Empty); + await messageService.SendPackageOwnerRequestAsync(from, to, package, packageUrl, confirmationUrl, rejectionUrl, string.Empty, string.Empty); var message = messageService.MockMailSender.Sent.Last(); Assert.DoesNotContain("The user 'Existing' added the following message for you", message.Body); @@ -530,7 +531,7 @@ public void SendsPackageOwnerRequestConfirmationUrlWithoutUserMessage(bool isOrg [Theory] [InlineData(false)] [InlineData(true)] - public void DoesNotSendRequestIfUserDoesNotAllowEmails(bool isOrganization) + public async Task DoesNotSendRequestIfUserDoesNotAllowEmails(bool isOrganization) { var to = isOrganization ? GetOrganizationWithoutRecipients() : new User(); to.Username = "Noob"; @@ -543,7 +544,7 @@ public void DoesNotSendRequestIfUserDoesNotAllowEmails(bool isOrganization) const string rejectionUrl = "http://example.com/rejection-token-url"; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequest(from, to, package, packageUrl, confirmationUrl, rejectionUrl, string.Empty, string.Empty); + await messageService.SendPackageOwnerRequestAsync(from, to, package, packageUrl, confirmationUrl, rejectionUrl, string.Empty, string.Empty); Assert.Empty(messageService.MockMailSender.Sent); } @@ -553,7 +554,7 @@ public class TheSendPackageOwnerRequestInitiatedNoticeMethod : TestContainer { [Fact] - public void SendsNotice() + public async Task SendsNotice() { var requestingOwner = new User("Existing") { EmailAddress = "existing-owner@example.com" }; var receivingOwner = new User("Receiving") @@ -574,7 +575,7 @@ public void SendsNotice() }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequestInitiatedNotice(requestingOwner, receivingOwner, newOwner, package, cancelUrl); + await messageService.SendPackageOwnerRequestInitiatedNoticeAsync(requestingOwner, receivingOwner, newOwner, package, cancelUrl); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(receivingOwner.EmailAddress, message.To[0].Address); @@ -586,7 +587,7 @@ public void SendsNotice() } [Fact] - public void DoesNotSendNoticeIfUserDoesNotAllowEmails() + public async Task DoesNotSendNoticeIfUserDoesNotAllowEmails() { var requestingOwner = new User("Existing") { EmailAddress = "existing-owner@example.com" }; var receivingOwner = new User("Receiving") @@ -607,7 +608,7 @@ public void DoesNotSendNoticeIfUserDoesNotAllowEmails() }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequestInitiatedNotice(requestingOwner, receivingOwner, newOwner, package, cancelUrl); + await messageService.SendPackageOwnerRequestInitiatedNoticeAsync(requestingOwner, receivingOwner, newOwner, package, cancelUrl); Assert.Empty(messageService.MockMailSender.Sent); } @@ -619,7 +620,7 @@ public class TheSendPackageOwnerRequestRejectionNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void SendsNotice(bool isOrganization) + public async Task SendsNotice(bool isOrganization) { var requestingOwner = isOrganization ? GetOrganizationWithRecipients() : new User(); requestingOwner.Username = "Existing"; @@ -636,7 +637,7 @@ public void SendsNotice(bool isOrganization) }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequestRejectionNotice(requestingOwner, newOwner, package); + await messageService.SendPackageOwnerRequestRejectionNoticeAsync(requestingOwner, newOwner, package); var message = messageService.MockMailSender.Sent.Last(); var yourString = isOrganization ? "your organization's" : "your"; @@ -658,7 +659,7 @@ public void SendsNotice(bool isOrganization) [Theory] [InlineData(false)] [InlineData(true)] - public void DoesNotSendNoticeIfUserDoesNotAllowEmails(bool isOrganization) + public async Task DoesNotSendNoticeIfUserDoesNotAllowEmails(bool isOrganization) { var requestingOwner = isOrganization ? GetOrganizationWithoutRecipients() : new User(); requestingOwner.Username = "Existing"; @@ -675,7 +676,7 @@ public void DoesNotSendNoticeIfUserDoesNotAllowEmails(bool isOrganization) }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequestRejectionNotice(requestingOwner, newOwner, package); + await messageService.SendPackageOwnerRequestRejectionNoticeAsync(requestingOwner, newOwner, package); Assert.Empty(messageService.MockMailSender.Sent); } @@ -687,7 +688,7 @@ public class TheSendPackageOwnerRequestCancellationNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void SendsNotice(bool isOrganization) + public async Task SendsNotice(bool isOrganization) { var requestingOwner = new User { Username = "Existing", EmailAddress = "existing-owner@example.com" }; var newOwner = isOrganization ? GetOrganizationWithRecipients() : new User(); @@ -697,7 +698,7 @@ public void SendsNotice(bool isOrganization) var package = new PackageRegistration { Id = "CoolStuff" }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequestCancellationNotice(requestingOwner, newOwner, package); + await messageService.SendPackageOwnerRequestCancellationNoticeAsync(requestingOwner, newOwner, package); var message = messageService.MockMailSender.Sent.Last(); var yourString = isOrganization ? "your organization" : "you"; @@ -719,7 +720,7 @@ public void SendsNotice(bool isOrganization) [Theory] [InlineData(false)] [InlineData(true)] - public void DoesNotSendNoticeIfUserDoesNotAllowEmails(bool isOrganization) + public async Task DoesNotSendNoticeIfUserDoesNotAllowEmails(bool isOrganization) { var requestingOwner = new User { Username = "Existing", EmailAddress = "existing-owner@example.com" }; @@ -737,7 +738,7 @@ public void DoesNotSendNoticeIfUserDoesNotAllowEmails(bool isOrganization) }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRequestCancellationNotice(requestingOwner, newOwner, package); + await messageService.SendPackageOwnerRequestCancellationNoticeAsync(requestingOwner, newOwner, package); Assert.Empty(messageService.MockMailSender.Sent); } @@ -749,7 +750,7 @@ public class TheSendPackageOwnerAddedNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void SendsPackageOwnerAddedNotice(bool isOrganization) + public async Task SendsPackageOwnerAddedNotice(bool isOrganization) { // Arrange var toUser = isOrganization ? GetOrganizationWithRecipients() : new User(); @@ -762,7 +763,7 @@ public void SendsPackageOwnerAddedNotice(bool isOrganization) var packageUrl = "packageUrl"; // Act - messageService.SendPackageOwnerAddedNotice(toUser, newUser, package, packageUrl); + await messageService.SendPackageOwnerAddedNoticeAsync(toUser, newUser, package, packageUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -782,7 +783,7 @@ public void SendsPackageOwnerAddedNotice(bool isOrganization) [Theory] [InlineData(false)] [InlineData(true)] - public void DoesNotSendPackageOwnerAddedNoticeIfUserDoesNotAllowEmails(bool isOrganization) + public async Task DoesNotSendPackageOwnerAddedNoticeIfUserDoesNotAllowEmails(bool isOrganization) { // Arrange var toUser = isOrganization ? GetOrganizationWithoutRecipients() : new User(); @@ -794,7 +795,7 @@ public void DoesNotSendPackageOwnerAddedNoticeIfUserDoesNotAllowEmails(bool isOr var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendPackageOwnerAddedNotice(toUser, newUser, package, "packageUrl"); + await messageService.SendPackageOwnerAddedNoticeAsync(toUser, newUser, package, "packageUrl"); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -807,7 +808,7 @@ public class TheSendPackageOwnerRemovedNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void SendsPackageOwnerRemovedNotice(bool isOrganization) + public async Task SendsPackageOwnerRemovedNotice(bool isOrganization) { var to = isOrganization ? GetOrganizationWithRecipients() : new User(); to.Username = "Noob"; @@ -817,7 +818,7 @@ public void SendsPackageOwnerRemovedNotice(bool isOrganization) var package = new PackageRegistration { Id = "CoolStuff" }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRemovedNotice(from, to, package); + await messageService.SendPackageOwnerRemovedNoticeAsync(from, to, package); var message = messageService.MockMailSender.Sent.Last(); if (isOrganization) @@ -837,7 +838,7 @@ public void SendsPackageOwnerRemovedNotice(bool isOrganization) [Theory] [InlineData(false)] [InlineData(true)] - public void DoesNotSendRemovedNoticeIfUserDoesNotAllowEmails(bool isOrganization) + public async Task DoesNotSendRemovedNoticeIfUserDoesNotAllowEmails(bool isOrganization) { var to = isOrganization ? GetOrganizationWithoutRecipients() : new User(); to.Username = "Noob"; @@ -847,7 +848,7 @@ public void DoesNotSendRemovedNoticeIfUserDoesNotAllowEmails(bool isOrganization var package = new PackageRegistration { Id = "CoolStuff" }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageOwnerRemovedNotice(from, to, package); + await messageService.SendPackageOwnerRemovedNoticeAsync(from, to, package); Assert.Empty(messageService.MockMailSender.Sent); } @@ -862,12 +863,12 @@ public class TheSendResetPasswordInstructionsMethod : TestContainer { [Fact] - public void WillSendInstructions() + public async Task WillSendInstructions() { var user = new User { EmailAddress = "legit@example.com", Username = "too" }; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPasswordResetInstructions(user, "http://example.com/pwd-reset-token-url", true); + await messageService.SendPasswordResetInstructionsAsync(user, "http://example.com/pwd-reset-token-url", true); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal("legit@example.com", message.To[0].Address); @@ -889,7 +890,7 @@ public TheSendCredentialRemovedNoticeMethod() } [Fact] - public void UsesProviderNounToDescribeCredentialIfPresent() + public async Task UsesProviderNounToDescribeCredentialIfPresent() { var user = new User { EmailAddress = "legit@example.com", Username = "foo" }; var cred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "abc123", "Test User"); @@ -897,7 +898,7 @@ public void UsesProviderNounToDescribeCredentialIfPresent() var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendCredentialRemovedNotice(user, _authenticationService.DescribeCredential(cred)); + await messageService.SendCredentialRemovedNoticeAsync(user, _authenticationService.DescribeCredential(cred)); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.ToMailAddress(), message.To[0]); @@ -907,13 +908,13 @@ public void UsesProviderNounToDescribeCredentialIfPresent() } [Fact] - public void UsesTypeCaptionToDescribeCredentialIfNoProviderNounPresent() + public async Task UsesTypeCaptionToDescribeCredentialIfNoProviderNounPresent() { var user = new User { EmailAddress = "legit@example.com", Username = "foo" }; var cred = new CredentialBuilder().CreatePasswordCredential("bogus"); var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendCredentialRemovedNotice(user, _authenticationService.DescribeCredential(cred)); + await messageService.SendCredentialRemovedNoticeAsync(user, _authenticationService.DescribeCredential(cred)); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.ToMailAddress(), message.To[0]); @@ -923,7 +924,7 @@ public void UsesTypeCaptionToDescribeCredentialIfNoProviderNounPresent() } [Fact] - public void ApiKeyRemovedMessageIsCorrect() + public async Task ApiKeyRemovedMessageIsCorrect() { var user = new User { EmailAddress = "legit@example.com", Username = "foo" }; var cred = TestCredentialHelper.CreateV2ApiKey(Guid.NewGuid(), TimeSpan.FromDays(1)).WithDefaultScopes(); @@ -931,7 +932,7 @@ public void ApiKeyRemovedMessageIsCorrect() cred.User = user; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendCredentialRemovedNotice(user, _authenticationService.DescribeCredential(cred)); + await messageService.SendCredentialRemovedNoticeAsync(user, _authenticationService.DescribeCredential(cred)); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.ToMailAddress(), message.To[0]); @@ -952,14 +953,14 @@ public TheSendCredentialAddedNoticeMethod() } [Fact] - public void UsesProviderNounToDescribeCredentialIfPresent() + public async Task UsesProviderNounToDescribeCredentialIfPresent() { var user = new User { EmailAddress = "legit@example.com", Username = "foo" }; var cred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "abc123", "Test User"); const string MicrosoftAccountCredentialName = "Microsoft account"; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendCredentialAddedNotice(user, _authenticationService.DescribeCredential(cred)); + await messageService.SendCredentialAddedNoticeAsync(user, _authenticationService.DescribeCredential(cred)); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.ToMailAddress(), message.To[0]); @@ -969,13 +970,13 @@ public void UsesProviderNounToDescribeCredentialIfPresent() } [Fact] - public void UsesTypeCaptionToDescribeCredentialIfNoProviderNounPresent() + public async Task UsesTypeCaptionToDescribeCredentialIfNoProviderNounPresent() { var user = new User { EmailAddress = "legit@example.com", Username = "foo" }; var cred = new CredentialBuilder().CreatePasswordCredential("bogus"); var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendCredentialAddedNotice(user, _authenticationService.DescribeCredential(cred)); + await messageService.SendCredentialAddedNoticeAsync(user, _authenticationService.DescribeCredential(cred)); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.ToMailAddress(), message.To[0]); @@ -985,7 +986,7 @@ public void UsesTypeCaptionToDescribeCredentialIfNoProviderNounPresent() } [Fact] - public void ApiKeyAddedMessageIsCorrect() + public async Task ApiKeyAddedMessageIsCorrect() { var user = new User { EmailAddress = "legit@example.com", Username = "foo" }; var cred = TestCredentialHelper.CreateV2ApiKey(Guid.NewGuid(), TimeSpan.FromDays(1)).WithDefaultScopes(); @@ -993,7 +994,7 @@ public void ApiKeyAddedMessageIsCorrect() cred.User = user; var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendCredentialAddedNotice(user, _authenticationService.DescribeCredential(cred)); + await messageService.SendCredentialAddedNoticeAsync(user, _authenticationService.DescribeCredential(cred)); var message = messageService.MockMailSender.Sent.Last(); Assert.Equal(user.ToMailAddress(), message.To[0]); @@ -1013,7 +1014,7 @@ public class TheSendPackageAddedNoticeMethod [InlineData("1.2.3+metadata")] [InlineData("1.2.3-alpha+metadata")] [InlineData("1.2.3-alpha.1+metadata")] - public void WillSendEmailToAllOwners(string version) + public async Task WillSendEmailToAllOwners(string version) { // Arrange var nugetVersion = new NuGetVersion(version); @@ -1039,7 +1040,7 @@ public void WillSendEmailToAllOwners(string version) var packageUrl = $"https://localhost/packages/{packageRegistration.Id}/{nugetVersion.ToNormalizedString()}"; var supportUrl = $"https://localhost/packages/{packageRegistration.Id}/{nugetVersion.ToNormalizedString()}/ReportMyPackage"; var emailSettingsUrl = "https://localhost/account"; - messageService.SendPackageAddedNotice(package, packageUrl, supportUrl, emailSettingsUrl); + await messageService.SendPackageAddedNoticeAsync(package, packageUrl, supportUrl, emailSettingsUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1053,7 +1054,7 @@ public void WillSendEmailToAllOwners(string version) } [Fact] - public void WillNotSendEmailToOwnerThatOptsOut() + public async Task WillNotSendEmailToOwnerThatOptsOut() { // Arrange var packageRegistration = new PackageRegistration @@ -1075,7 +1076,7 @@ public void WillNotSendEmailToOwnerThatOptsOut() // Act var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageAddedNotice(package, "http://dummy1", "http://dummy2", "http://dummy3"); + await messageService.SendPackageAddedNoticeAsync(package, "http://dummy1", "http://dummy2", "http://dummy3"); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1085,7 +1086,7 @@ public void WillNotSendEmailToOwnerThatOptsOut() } [Fact] - public void WillNotAttemptToSendIfNoOwnersAllow() + public async Task WillNotAttemptToSendIfNoOwnersAllow() { // Arrange var packageRegistration = new PackageRegistration @@ -1107,7 +1108,7 @@ public void WillNotAttemptToSendIfNoOwnersAllow() // Act var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendPackageAddedNotice(package, "http://dummy1", "http://dummy2", "http://dummy3"); + await messageService.SendPackageAddedNoticeAsync(package, "http://dummy1", "http://dummy2", "http://dummy3"); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1152,7 +1153,7 @@ public static IEnumerable WillSendEmailToAllOwners_Data [Theory] [MemberData(nameof(WillSendEmailToAllOwners_Data))] - public void WillSendEmailToAllOwners(ValidationIssue validationIssue, bool user1PushAllowed, bool user2PushAllowed, bool user1EmailAllowed, bool user2EmailAllowed) + public async Task WillSendEmailToAllOwners(ValidationIssue validationIssue, bool user1PushAllowed, bool user2PushAllowed, bool user1EmailAllowed, bool user2EmailAllowed) { // Arrange var packageRegistration = new PackageRegistration @@ -1196,7 +1197,7 @@ public void WillSendEmailToAllOwners(ValidationIssue validationIssue, bool user1 var supportUrl = $"https://supportUrl"; var announcementsUrl = "https://announcementsUrl"; var twitterUrl = "https://twitterUrl"; - messageService.SendPackageValidationFailedNotice(package, packageValidationSet, packageUrl, supportUrl, announcementsUrl, twitterUrl); + await messageService.SendPackageValidationFailedNoticeAsync(package, packageValidationSet, packageUrl, supportUrl, announcementsUrl, twitterUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1260,7 +1261,7 @@ public class TheSendValidationTakingTooLongNoticeMethod [InlineData("1.2.3+metadata")] [InlineData("1.2.3-alpha+metadata")] [InlineData("1.2.3-alpha.1+metadata")] - public void WillSendEmailToAllOwners(string version) + public async Task WillSendEmailToAllOwners(string version) { // Arrange var nugetVersion = new NuGetVersion(version); @@ -1283,7 +1284,7 @@ public void WillSendEmailToAllOwners(string version) // Act var messageService = TestableMessageService.Create(GetConfigurationService()); var packageUrl = $"https://localhost/packages/{packageRegistration.Id}/{nugetVersion.ToNormalizedString()}"; - messageService.SendValidationTakingTooLongNotice(package, packageUrl); + await messageService.SendValidationTakingTooLongNoticeAsync(package, packageUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1307,7 +1308,7 @@ public static IEnumerable EmailSettingsCombinations [Theory] [MemberData(nameof(EmailSettingsCombinations))] - public void WillHonorPushSettings(bool user1PushAllowed, bool user2PushAllowed, bool user1EmailAllowed, bool user2EmailAllowed) + public async Task WillHonorPushSettings(bool user1PushAllowed, bool user2PushAllowed, bool user1EmailAllowed, bool user2EmailAllowed) { // Arrange var packageRegistration = new PackageRegistration @@ -1331,7 +1332,7 @@ public void WillHonorPushSettings(bool user1PushAllowed, bool user2PushAllowed, // Act var messageService = TestableMessageService.Create(GetConfigurationService()); - messageService.SendValidationTakingTooLongNotice(package, "http://dummy1"); + await messageService.SendValidationTakingTooLongNoticeAsync(package, "http://dummy1"); // Assert var message = messageService.MockMailSender.Sent.LastOrDefault(); @@ -1359,7 +1360,7 @@ public class TheSendPackageDeletedNoticeMethod : TestContainer { [Fact] - public void WillSendEmailToAllOwners() + public async Task WillSendEmailToAllOwners() { // Arrange var nugetVersion = new NuGetVersion("3.1.0"); @@ -1384,7 +1385,7 @@ public void WillSendEmailToAllOwners() var supportUrl = $"https://localhost/packages/{packageRegistration.Id}/{nugetVersion.ToNormalizedString()}/ReportMyPackage"; // Act - messageService.SendPackageDeletedNotice(package, packageUrl, supportUrl); + await messageService.SendPackageDeletedNoticeAsync(package, packageUrl, supportUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1401,7 +1402,7 @@ public class TheSendOrganizationTransformRequestMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com" }; @@ -1413,7 +1414,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequest(accountToTransform, adminUser, profileUrl, confirmationUrl, rejectionUrl); + await messageService.SendOrganizationTransformRequestAsync(accountToTransform, adminUser, profileUrl, confirmationUrl, rejectionUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1428,7 +1429,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com" }; @@ -1440,7 +1441,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequest(accountToTransform, adminUser, profileUrl, confirmationUrl, rejectionUrl); + await messageService.SendOrganizationTransformRequestAsync(accountToTransform, adminUser, profileUrl, confirmationUrl, rejectionUrl); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1451,7 +1452,7 @@ public class TheSendOrganizationTransformInitiatedNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com", EmailAllowed = true }; @@ -1461,7 +1462,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformInitiatedNotice(accountToTransform, adminUser, cancelUrl); + await messageService.SendOrganizationTransformInitiatedNoticeAsync(accountToTransform, adminUser, cancelUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1476,7 +1477,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com", EmailAllowed = false }; @@ -1486,7 +1487,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformInitiatedNotice(accountToTransform, adminUser, cancelUrl); + await messageService.SendOrganizationTransformInitiatedNoticeAsync(accountToTransform, adminUser, cancelUrl); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1497,7 +1498,7 @@ public class TheSendOrganizationTransformRequestAcceptedNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com", EmailAllowed = true }; @@ -1506,7 +1507,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequestAcceptedNotice(accountToTransform, adminUser); + await messageService.SendOrganizationTransformRequestAcceptedNoticeAsync(accountToTransform, adminUser); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1519,7 +1520,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com", EmailAllowed = false }; @@ -1528,7 +1529,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequestAcceptedNotice(accountToTransform, adminUser); + await messageService.SendOrganizationTransformRequestAcceptedNoticeAsync(accountToTransform, adminUser); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1539,7 +1540,7 @@ public class TheSendOrganizationTransformRequestRejectedNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com", EmailAllowed = true }; @@ -1548,7 +1549,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequestRejectedNotice(accountToTransform, adminUser); + await messageService.SendOrganizationTransformRequestRejectedNoticeAsync(accountToTransform, adminUser); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1561,7 +1562,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com", EmailAllowed = false }; @@ -1570,7 +1571,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequestRejectedNotice(accountToTransform, adminUser); + await messageService.SendOrganizationTransformRequestRejectedNoticeAsync(accountToTransform, adminUser); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1581,7 +1582,7 @@ public class TheSendOrganizationTransformRequestCancelledNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com" }; @@ -1590,7 +1591,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequestCancelledNotice(accountToTransform, adminUser); + await messageService.SendOrganizationTransformRequestCancelledNoticeAsync(accountToTransform, adminUser); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1603,7 +1604,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var accountToTransform = new User("bumblebee") { EmailAddress = "bumblebee@transformers.com" }; @@ -1612,7 +1613,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationTransformRequestCancelledNotice(accountToTransform, adminUser); + await messageService.SendOrganizationTransformRequestCancelledNoticeAsync(accountToTransform, adminUser); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1625,7 +1626,7 @@ public class TheSendOrganizationMembershipRequestMethod [Theory] [InlineData(false)] [InlineData(true)] - public void WillSendEmailIfEmailAllowed(bool isAdmin) + public async Task WillSendEmailIfEmailAllowed(bool isAdmin) { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com" }; @@ -1638,7 +1639,7 @@ public void WillSendEmailIfEmailAllowed(bool isAdmin) var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequest(organization, newUser, adminUser, isAdmin, profileUrl, confirmationUrl, rejectionUrl); + await messageService.SendOrganizationMembershipRequestAsync(organization, newUser, adminUser, isAdmin, profileUrl, confirmationUrl, rejectionUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1657,7 +1658,7 @@ public void WillSendEmailIfEmailAllowed(bool isAdmin) [Theory] [InlineData(false)] [InlineData(true)] - public void WillNotSendEmailIfEmailNotAllowed(bool isAdmin) + public async Task WillNotSendEmailIfEmailNotAllowed(bool isAdmin) { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com" }; @@ -1670,7 +1671,7 @@ public void WillNotSendEmailIfEmailNotAllowed(bool isAdmin) var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequest(organization, newUser, adminUser, isAdmin, profileUrl, confirmationUrl, rejectionUrl); + await messageService.SendOrganizationMembershipRequestAsync(organization, newUser, adminUser, isAdmin, profileUrl, confirmationUrl, rejectionUrl); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1683,7 +1684,7 @@ public class TheSendOrganizationMembershipRequestInitiatedNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void WillSendEmailIfEmailAllowed(bool isAdmin) + public async Task WillSendEmailIfEmailAllowed(bool isAdmin) { // Arrange var organization = GetOrganizationWithRecipients(); @@ -1694,7 +1695,7 @@ public void WillSendEmailIfEmailAllowed(bool isAdmin) var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequestInitiatedNotice(organization, requestingUser, pendingUser, isAdmin, cancelUrl); + await messageService.SendOrganizationMembershipRequestInitiatedNoticeAsync(organization, requestingUser, pendingUser, isAdmin, cancelUrl); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1710,7 +1711,7 @@ public void WillSendEmailIfEmailAllowed(bool isAdmin) [Theory] [InlineData(false)] [InlineData(true)] - public void WillNotSendEmailIfEmailNotAllowed(bool isAdmin) + public async Task WillNotSendEmailIfEmailNotAllowed(bool isAdmin) { // Arrange var organization = GetOrganizationWithoutRecipients(); @@ -1721,7 +1722,7 @@ public void WillNotSendEmailIfEmailNotAllowed(bool isAdmin) var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequestInitiatedNotice(organization, requestingUser, pendingUser, isAdmin, cancelUrl); + await messageService.SendOrganizationMembershipRequestInitiatedNoticeAsync(organization, requestingUser, pendingUser, isAdmin, cancelUrl); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1732,7 +1733,7 @@ public class TheSendOrganizationMembershipRequestRejectedNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var organization = GetOrganizationWithRecipients(); @@ -1741,7 +1742,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequestRejectedNotice(organization, pendingUser); + await messageService.SendOrganizationMembershipRequestRejectedNoticeAsync(organization, pendingUser); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1755,7 +1756,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var organization = GetOrganizationWithoutRecipients(); @@ -1764,7 +1765,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequestRejectedNotice(organization, pendingUser); + await messageService.SendOrganizationMembershipRequestRejectedNoticeAsync(organization, pendingUser); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1775,7 +1776,7 @@ public class TheSendOrganizationMembershipRequestCancelledNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com" }; @@ -1784,7 +1785,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequestCancelledNotice(organization, pendingUser); + await messageService.SendOrganizationMembershipRequestCancelledNoticeAsync(organization, pendingUser); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1797,7 +1798,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var accountToTransform = new Organization("transformers") { EmailAddress = "transformers@transformers.com" }; @@ -1806,7 +1807,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMembershipRequestCancelledNotice(accountToTransform, pendingUser); + await messageService.SendOrganizationMembershipRequestCancelledNoticeAsync(accountToTransform, pendingUser); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1819,7 +1820,7 @@ public class TheSendOrganizationMemberUpdatedNoticeMethod [Theory] [InlineData(false)] [InlineData(true)] - public void WillSendEmailIfEmailAllowed(bool isAdmin) + public async Task WillSendEmailIfEmailAllowed(bool isAdmin) { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com", EmailAllowed = true }; @@ -1829,7 +1830,7 @@ public void WillSendEmailIfEmailAllowed(bool isAdmin) var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMemberUpdatedNotice(organization, membership); + await messageService.SendOrganizationMemberUpdatedNoticeAsync(organization, membership); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1845,7 +1846,7 @@ public void WillSendEmailIfEmailAllowed(bool isAdmin) [Theory] [InlineData(false)] [InlineData(true)] - public void WillNotSendEmailIfEmailNotAllowed(bool isAdmin) + public async Task WillNotSendEmailIfEmailNotAllowed(bool isAdmin) { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com", EmailAllowed = false }; @@ -1855,7 +1856,7 @@ public void WillNotSendEmailIfEmailNotAllowed(bool isAdmin) var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMemberUpdatedNotice(organization, membership); + await messageService.SendOrganizationMemberUpdatedNoticeAsync(organization, membership); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1866,7 +1867,7 @@ public class TheSendOrganizationMemberRemovedNoticeMethod : TestContainer { [Fact] - public void WillSendEmailIfEmailAllowed() + public async Task WillSendEmailIfEmailAllowed() { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com", EmailAllowed = true }; @@ -1875,7 +1876,7 @@ public void WillSendEmailIfEmailAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMemberRemovedNotice(organization, removedUser); + await messageService.SendOrganizationMemberRemovedNoticeAsync(organization, removedUser); // Assert var message = messageService.MockMailSender.Sent.Last(); @@ -1888,7 +1889,7 @@ public void WillSendEmailIfEmailAllowed() } [Fact] - public void WillNotSendEmailIfEmailNotAllowed() + public async Task WillNotSendEmailIfEmailNotAllowed() { // Arrange var organization = new Organization("transformers") { EmailAddress = "transformers@transformers.com", EmailAllowed = false }; @@ -1897,7 +1898,7 @@ public void WillNotSendEmailIfEmailNotAllowed() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendOrganizationMemberRemovedNotice(organization, member); + await messageService.SendOrganizationMemberRemovedNoticeAsync(organization, member); // Assert Assert.Empty(messageService.MockMailSender.Sent); @@ -1908,7 +1909,7 @@ public class TheSendAccountDeleteNoticeMethod : TestContainer { [Fact] - public void VerifyTheMessageBody() + public async Task VerifyTheMessageBody() { // Arrange var userName = "deleteduser"; @@ -1918,7 +1919,7 @@ public void VerifyTheMessageBody() var messageService = TestableMessageService.Create(GetConfigurationService()); // Act - messageService.SendAccountDeleteNotice(userToDelete); + await messageService.SendAccountDeleteNoticeAsync(userToDelete); // Assert var message = messageService.MockMailSender.Sent.Last(); From 7a0b0a20933692a3b3ce2d853a53fb62dd780644 Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Wed, 1 Aug 2018 11:39:15 -0700 Subject: [PATCH 4/8] Track send email through telemetry service --- src/NuGetGallery/Services/BackgroundMessageService.cs | 4 ++-- src/NuGetGallery/Services/ITelemetryService.cs | 9 +++++++++ src/NuGetGallery/Services/MessageService.cs | 10 +++++----- src/NuGetGallery/Services/TelemetryService.cs | 5 +++++ .../NuGetGallery.Facts/Services/MessageServiceFacts.cs | 2 +- 5 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/NuGetGallery/Services/BackgroundMessageService.cs b/src/NuGetGallery/Services/BackgroundMessageService.cs index 98ff700991..fc733d89e6 100644 --- a/src/NuGetGallery/Services/BackgroundMessageService.cs +++ b/src/NuGetGallery/Services/BackgroundMessageService.cs @@ -11,8 +11,8 @@ namespace NuGetGallery.Services { public class BackgroundMessageService : MessageService { - public BackgroundMessageService(IMailSender mailSender, IAppConfiguration config, ITelemetryClient telemetryClient) - :base(mailSender, config, telemetryClient) + public BackgroundMessageService(IMailSender mailSender, IAppConfiguration config, ITelemetryService telemetryService) + :base(mailSender, config, telemetryService) { } diff --git a/src/NuGetGallery/Services/ITelemetryService.cs b/src/NuGetGallery/Services/ITelemetryService.cs index 5516657395..990690f26b 100644 --- a/src/NuGetGallery/Services/ITelemetryService.cs +++ b/src/NuGetGallery/Services/ITelemetryService.cs @@ -136,5 +136,14 @@ public interface ITelemetryService /// /// The requesting the delete. void TrackRequestForAccountDeletion(User user); + + /// + /// A telemetry event emitted when an email is sent. + /// + /// URI to the SMTP server + /// The start time of when sending the email is attempted. + /// The duration of how long the send took. + /// + void TrackSendEmail(string smtpUri, DateTimeOffset startTime, TimeSpan duration, bool success); } } \ No newline at end of file diff --git a/src/NuGetGallery/Services/MessageService.cs b/src/NuGetGallery/Services/MessageService.cs index 009f9ca2eb..49892ca6df 100644 --- a/src/NuGetGallery/Services/MessageService.cs +++ b/src/NuGetGallery/Services/MessageService.cs @@ -18,14 +18,14 @@ namespace NuGetGallery { public class MessageService : CoreMessageService, IMessageService { - public MessageService(IMailSender mailSender, IAppConfiguration config, ITelemetryClient telemetryClient) + public MessageService(IMailSender mailSender, IAppConfiguration config, ITelemetryService telemetryService) : base(mailSender, config) { - this.telemetryClient = telemetryClient ?? throw new ArgumentNullException(nameof(telemetryClient)); + this.telemetryService = telemetryService ?? throw new ArgumentNullException(nameof(telemetryService)); smtpUri = config.SmtpUri?.Host ?? throw new ArgumentNullException(nameof(config) + "." + nameof(config.SmtpUri)); } - private readonly ITelemetryClient telemetryClient; + private readonly ITelemetryService telemetryService; private readonly string smtpUri; public IAppConfiguration Config @@ -1024,7 +1024,7 @@ private bool AddAddressesForAccountManagementToEmail(MailMessage mailMessage, Us protected override async Task AttemptSendMessageAsync(MailMessage mailMessage) { bool success = false; - DateTimeOffset now = DateTimeOffset.UtcNow; + DateTimeOffset startTime = DateTimeOffset.UtcNow; Stopwatch sw = Stopwatch.StartNew(); try { @@ -1034,7 +1034,7 @@ protected override async Task AttemptSendMessageAsync(MailMessage mailMessage) finally { sw.Stop(); - telemetryClient.TrackDependency("SMTP", smtpUri, "SendMessage", null, now, sw.Elapsed, null, success); + telemetryService.TrackSendEmail(smtpUri, startTime, sw.Elapsed, success); } } diff --git a/src/NuGetGallery/Services/TelemetryService.cs b/src/NuGetGallery/Services/TelemetryService.cs index 7afcb7e72e..bb58dac047 100644 --- a/src/NuGetGallery/Services/TelemetryService.cs +++ b/src/NuGetGallery/Services/TelemetryService.cs @@ -590,6 +590,11 @@ public void TrackRequestForAccountDeletion(User user) }); } + public void TrackSendEmail(string smtpUri, DateTimeOffset startTime, TimeSpan duration, bool success) + { + _telemetryClient.TrackDependency("SMTP", smtpUri, "SendMessage", null, startTime, duration, null, success); + } + /// /// We use instead of /// diff --git a/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs b/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs index c74d31384c..a4cf6a3df9 100644 --- a/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs +++ b/tests/NuGetGallery.Facts/Services/MessageServiceFacts.cs @@ -2006,7 +2006,7 @@ public class TestableMessageService : MessageService { private TestableMessageService(IGalleryConfigurationService configurationService) - : base(new TestMailSender(), configurationService.Current, new Mock().Object) + : base(new TestMailSender(), configurationService.Current, new Mock().Object) { configurationService.Current.GalleryOwner = TestGalleryOwner; configurationService.Current.GalleryNoReplyAddress = TestGalleryNoReplyAddress; From 3b07214e5622844979290a8522c106508cd67e8e Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Wed, 1 Aug 2018 15:33:18 -0700 Subject: [PATCH 5/8] Clone MailMessage in BackgroundMessageService, to avoid using disposed message --- .../Services/BackgroundMessageService.cs | 54 ++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/src/NuGetGallery/Services/BackgroundMessageService.cs b/src/NuGetGallery/Services/BackgroundMessageService.cs index fc733d89e6..1cd3851918 100644 --- a/src/NuGetGallery/Services/BackgroundMessageService.cs +++ b/src/NuGetGallery/Services/BackgroundMessageService.cs @@ -20,20 +20,72 @@ protected override Task SendMessageAsync(MailMessage mailMessage) { // Send email as background task, as we don't want to delay the HTTP response. // Particularly when sending email fails and needs to be retried with a delay. + // MailMessage is IDisposable, so first clone the message, to ensure if the + // caller disposes it, the message is available until the async task is complete. + + var messageCopy = CloneMessage(mailMessage); + Task.Run(async () => { try { - await base.SendMessageAsync(mailMessage); + await base.SendMessageAsync(messageCopy); } catch (Exception ex) { // Log but swallow the exception. QuietLog.LogHandledException(ex); } + finally + { + messageCopy.Dispose(); + } }); return Task.CompletedTask; } + + private MailMessage CloneMessage(MailMessage mailMessage) + { + string from = mailMessage.From.ToString(); + string to = mailMessage.To.ToString(); + + MailMessage copy = new MailMessage(from, to, mailMessage.Subject, mailMessage.Body); + + copy.IsBodyHtml = mailMessage.IsBodyHtml; + copy.BodyTransferEncoding = mailMessage.BodyTransferEncoding; + copy.BodyEncoding = mailMessage.BodyEncoding; + copy.HeadersEncoding = mailMessage.HeadersEncoding; + foreach (System.Collections.Specialized.NameValueCollection header in mailMessage.Headers) + { + copy.Headers.Add(header); + } + copy.SubjectEncoding = mailMessage.SubjectEncoding; + copy.DeliveryNotificationOptions = mailMessage.DeliveryNotificationOptions; + foreach (var cc in mailMessage.CC) + { + copy.CC.Add(cc); + } + foreach(var attachment in mailMessage.Attachments) + { + copy.Attachments.Add(attachment); + } + foreach (var bcc in mailMessage.Bcc) + { + copy.Bcc.Add(bcc); + } + foreach (var replyTo in mailMessage.ReplyToList) + { + copy.ReplyToList.Add(replyTo); + } + copy.Sender = mailMessage.Sender; + copy.Priority = mailMessage.Priority; + foreach (var view in mailMessage.AlternateViews) + { + copy.AlternateViews.Add(view); + } + + return copy; + } } } \ No newline at end of file From b59b62081aa66bee1eb70190d53be6c90d2a8eba Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Wed, 1 Aug 2018 16:00:03 -0700 Subject: [PATCH 6/8] record attempt number for send email telemetry --- src/NuGetGallery.Core/Services/CoreMessageService.cs | 4 ++-- src/NuGetGallery/Services/ITelemetryClient.cs | 3 ++- src/NuGetGallery/Services/ITelemetryService.cs | 5 +++-- src/NuGetGallery/Services/MessageService.cs | 6 +++--- src/NuGetGallery/Services/TelemetryClientWrapper.cs | 12 ++++++++++-- src/NuGetGallery/Services/TelemetryService.cs | 8 ++++++-- 6 files changed, 26 insertions(+), 12 deletions(-) diff --git a/src/NuGetGallery.Core/Services/CoreMessageService.cs b/src/NuGetGallery.Core/Services/CoreMessageService.cs index 50626e2dfe..4fb3f5bcc4 100644 --- a/src/NuGetGallery.Core/Services/CoreMessageService.cs +++ b/src/NuGetGallery.Core/Services/CoreMessageService.cs @@ -204,7 +204,7 @@ protected virtual async Task SendMessageAsync(MailMessage mailMessage) { try { - await AttemptSendMessageAsync(mailMessage); + await AttemptSendMessageAsync(mailMessage, attempt + 1); success = true; } catch (SmtpException) @@ -222,7 +222,7 @@ protected virtual async Task SendMessageAsync(MailMessage mailMessage) } } - protected virtual Task AttemptSendMessageAsync(MailMessage mailMessage) + protected virtual Task AttemptSendMessageAsync(MailMessage mailMessage, int attemptNumber) { // AnglicanGeek.MarkdownMailer doesn't have an async overload MailSender.Send(mailMessage); diff --git a/src/NuGetGallery/Services/ITelemetryClient.cs b/src/NuGetGallery/Services/ITelemetryClient.cs index f6ed2502ee..bdaf36a540 100644 --- a/src/NuGetGallery/Services/ITelemetryClient.cs +++ b/src/NuGetGallery/Services/ITelemetryClient.cs @@ -22,6 +22,7 @@ void TrackDependency(string dependencyTypeName, DateTimeOffset startTime, TimeSpan duration, string resultCode, - bool success); + bool success, + IDictionary properties); } } diff --git a/src/NuGetGallery/Services/ITelemetryService.cs b/src/NuGetGallery/Services/ITelemetryService.cs index 990690f26b..7a5bdef870 100644 --- a/src/NuGetGallery/Services/ITelemetryService.cs +++ b/src/NuGetGallery/Services/ITelemetryService.cs @@ -143,7 +143,8 @@ public interface ITelemetryService /// URI to the SMTP server /// The start time of when sending the email is attempted. /// The duration of how long the send took. - /// - void TrackSendEmail(string smtpUri, DateTimeOffset startTime, TimeSpan duration, bool success); + /// Whether sending the email was successful. + /// The number of attempts the message has tried to be sent. + void TrackSendEmail(string smtpUri, DateTimeOffset startTime, TimeSpan duration, bool success, int attemptNumber); } } \ No newline at end of file diff --git a/src/NuGetGallery/Services/MessageService.cs b/src/NuGetGallery/Services/MessageService.cs index 49892ca6df..72053df165 100644 --- a/src/NuGetGallery/Services/MessageService.cs +++ b/src/NuGetGallery/Services/MessageService.cs @@ -1021,20 +1021,20 @@ private bool AddAddressesForAccountManagementToEmail(MailMessage mailMessage, Us return AddAddressesWithPermissionToEmail(mailMessage, user, ActionsRequiringPermissions.ManageAccount); } - protected override async Task AttemptSendMessageAsync(MailMessage mailMessage) + protected override async Task AttemptSendMessageAsync(MailMessage mailMessage, int attemptNumber) { bool success = false; DateTimeOffset startTime = DateTimeOffset.UtcNow; Stopwatch sw = Stopwatch.StartNew(); try { - await base.AttemptSendMessageAsync(mailMessage); + await base.AttemptSendMessageAsync(mailMessage, attemptNumber); success = true; } finally { sw.Stop(); - telemetryService.TrackSendEmail(smtpUri, startTime, sw.Elapsed, success); + telemetryService.TrackSendEmail(smtpUri, startTime, sw.Elapsed, success, attemptNumber); } } diff --git a/src/NuGetGallery/Services/TelemetryClientWrapper.cs b/src/NuGetGallery/Services/TelemetryClientWrapper.cs index 5525680983..e817cba55e 100644 --- a/src/NuGetGallery/Services/TelemetryClientWrapper.cs +++ b/src/NuGetGallery/Services/TelemetryClientWrapper.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using Microsoft.ApplicationInsights; +using Microsoft.ApplicationInsights.DataContracts; namespace NuGetGallery { @@ -60,11 +61,18 @@ public void TrackDependency(string dependencyTypeName, DateTimeOffset startTime, TimeSpan duration, string resultCode, - bool success) + bool success, + IDictionary properties) { try { - UnderlyingClient.TrackDependency(dependencyTypeName, target, dependencyName, data, startTime, duration, resultCode, success); + var telemetry = new DependencyTelemetry(dependencyTypeName, target, dependencyName, data, startTime, duration, resultCode, success); + foreach (var property in properties) + { + telemetry.Properties.Add(property); + } + + UnderlyingClient.TrackDependency(telemetry); } catch { diff --git a/src/NuGetGallery/Services/TelemetryService.cs b/src/NuGetGallery/Services/TelemetryService.cs index 06836ba74a..1bfb9efef7 100644 --- a/src/NuGetGallery/Services/TelemetryService.cs +++ b/src/NuGetGallery/Services/TelemetryService.cs @@ -594,9 +594,13 @@ public void TrackRequestForAccountDeletion(User user) }); } - public void TrackSendEmail(string smtpUri, DateTimeOffset startTime, TimeSpan duration, bool success) + public void TrackSendEmail(string smtpUri, DateTimeOffset startTime, TimeSpan duration, bool success, int attemptNumber) { - _telemetryClient.TrackDependency("SMTP", smtpUri, "SendMessage", null, startTime, duration, null, success); + var properties = new Dictionary + { + { "attempt", attemptNumber.ToString() } + }; + _telemetryClient.TrackDependency("SMTP", smtpUri, "SendMessage", null, startTime, duration, null, success, properties); } /// From 7918915401581f8f2e29deda297b1d3f2f8f969f Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Fri, 3 Aug 2018 09:36:03 -0700 Subject: [PATCH 7/8] await all new async methods --- .../Controllers/AccountsController.cs | 10 ++++----- .../Controllers/AuthenticationController.cs | 4 ++-- .../Controllers/OrganizationsController.cs | 10 ++++----- .../Controllers/PackagesController.cs | 10 ++++----- .../Controllers/UsersController.cs | 16 +++++++------- .../Controllers/AccountsControllerFacts.cs | 13 +++++++----- .../AuthenticationControllerFacts.cs | 21 ++++++++++--------- .../OrganizationsControllerFacts.cs | 4 ++-- .../Controllers/PackagesControllerFacts.cs | 17 ++++++++------- .../Controllers/UsersControllerFacts.cs | 3 ++- 10 files changed, 58 insertions(+), 50 deletions(-) diff --git a/src/NuGetGallery/Controllers/AccountsController.cs b/src/NuGetGallery/Controllers/AccountsController.cs index ffd5630ff0..27be140c32 100644 --- a/src/NuGetGallery/Controllers/AccountsController.cs +++ b/src/NuGetGallery/Controllers/AccountsController.cs @@ -92,7 +92,7 @@ public virtual ActionResult ConfirmationRequired(string accountName = null) [HttpPost] [ActionName("ConfirmationRequired")] [ValidateAntiForgeryToken] - public virtual ActionResult ConfirmationRequiredPost(string accountName = null) + public virtual async Task ConfirmationRequiredPost(string accountName = null) { var account = GetAccount(accountName); @@ -108,7 +108,7 @@ public virtual ActionResult ConfirmationRequiredPost(string accountName = null) ConfirmationViewModel model; if (!alreadyConfirmed) { - SendNewAccountEmail(account); + await SendNewAccountEmailAsync(account); model = new ConfirmationViewModel(account) { @@ -122,7 +122,7 @@ public virtual ActionResult ConfirmationRequiredPost(string accountName = null) return View(model); } - protected abstract void SendNewAccountEmail(User account); + protected abstract Task SendNewAccountEmailAsync(User account); [UIAuthorize(allowDiscontinuedLogins: true)] public virtual async Task Confirm(string accountName, string token) @@ -254,13 +254,13 @@ public virtual async Task ChangeEmail(TAccountViewModel model) if (account.Confirmed) { - SendEmailChangedConfirmationNotice(account); + await SendEmailChangedConfirmationNoticeAsync(account); } return RedirectToAction(AccountAction); } - protected abstract void SendEmailChangedConfirmationNotice(User account); + protected abstract Task SendEmailChangedConfirmationNoticeAsync(User account); [HttpPost] [UIAuthorize] diff --git a/src/NuGetGallery/Controllers/AuthenticationController.cs b/src/NuGetGallery/Controllers/AuthenticationController.cs index c2568b47b1..4cf2b03841 100644 --- a/src/NuGetGallery/Controllers/AuthenticationController.cs +++ b/src/NuGetGallery/Controllers/AuthenticationController.cs @@ -325,7 +325,7 @@ public virtual ActionResult LogOff(string returnUrl) [HttpPost] [ValidateAntiForgeryToken] - public virtual JsonResult SignInAssistance(string username, string providedEmailAddress) + public virtual async Task 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 JsonResult SignInAssistance(string username, string providedEmail else { var externalCredentials = user.Credentials.Where(cred => cred.IsExternal()); - _messageService.SendSigninAssistanceEmailAsync(new MailAddress(email, user.Username), externalCredentials); + await _messageService.SendSigninAssistanceEmailAsync(new MailAddress(email, user.Username), externalCredentials); return Json(new { success = true }); } } diff --git a/src/NuGetGallery/Controllers/OrganizationsController.cs b/src/NuGetGallery/Controllers/OrganizationsController.cs index 7d22657e05..c5ef85db72 100644 --- a/src/NuGetGallery/Controllers/OrganizationsController.cs +++ b/src/NuGetGallery/Controllers/OrganizationsController.cs @@ -53,17 +53,17 @@ public OrganizationsController( EmailUpdateCancelled = Strings.OrganizationEmailUpdateCancelled }; - protected override void SendNewAccountEmail(User account) + protected override Task SendNewAccountEmailAsync(User account) { var confirmationUrl = Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendNewAccountEmailAsync(account, confirmationUrl); + return MessageService.SendNewAccountEmailAsync(account, confirmationUrl); } - protected override void SendEmailChangedConfirmationNotice(User account) + protected override Task SendEmailChangedConfirmationNoticeAsync(User account) { var confirmationUrl = Url.ConfirmOrganizationEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendEmailChangeConfirmationNoticeAsync(account, confirmationUrl); + return MessageService.SendEmailChangeConfirmationNoticeAsync(account, confirmationUrl); } [HttpGet] @@ -85,7 +85,7 @@ public async Task Add(AddOrganizationViewModel model) try { var organization = await UserService.AddOrganizationAsync(organizationName, organizationEmailAddress, adminUser); - SendNewAccountEmail(organization); + await SendNewAccountEmailAsync(organization); TelemetryService.TrackOrganizationAdded(organization); return RedirectToAction(nameof(ManageOrganization), new { accountName = organization.Username }); } diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs index 827009a4dc..278cca143d 100644 --- a/src/NuGetGallery/Controllers/PackagesController.cs +++ b/src/NuGetGallery/Controllers/PackagesController.cs @@ -811,7 +811,7 @@ public virtual async Task ReportMyPackage(string id, string versio if (!deleted) { - NotifyReportMyPackageSupportRequest(reportForm, package, user, from); + await NotifyReportMyPackageSupportRequestAsync(reportForm, package, user, from); } return Redirect(Url.Package(package.PackageRegistration.Id, package.NormalizedVersion)); @@ -879,7 +879,7 @@ private async Task ValidateReportMyPackageViewModel(ReportMyPackag return null; } - private void NotifyReportMyPackageSupportRequest(ReportMyPackageViewModel reportForm, Package package, User user, MailAddress from) + private async Task NotifyReportMyPackageSupportRequestAsync(ReportMyPackageViewModel reportForm, Package package, User user, MailAddress from) { var request = new ReportPackageRequest { @@ -892,7 +892,7 @@ private void NotifyReportMyPackageSupportRequest(ReportMyPackageViewModel report CopySender = reportForm.CopySender }; - _messageService.ReportMyPackageAsync(request); + await _messageService.ReportMyPackageAsync(request); TempData["Message"] = Strings.SupportRequestSentTransientMessage; } @@ -971,7 +971,7 @@ public virtual ActionResult ContactOwners(string id, string version) [ValidateAntiForgeryToken] [ValidateRecaptchaResponse] [RequiresAccountConfirmation("contact package owners")] - public virtual ActionResult ContactOwners(string id, string version, ContactOwnersViewModel contactForm) + public virtual async Task ContactOwners(string id, string version, ContactOwnersViewModel contactForm) { // Html Encode the message contactForm.Message = System.Web.HttpUtility.HtmlEncode(contactForm.Message); @@ -989,7 +989,7 @@ public virtual ActionResult ContactOwners(string id, string version, ContactOwne var user = GetCurrentUser(); var fromAddress = new MailAddress(user.EmailAddress, user.Username); - _messageService.SendContactOwnersMessageAsync( + await _messageService.SendContactOwnersMessageAsync( fromAddress, package, Url.Package(package, false), diff --git a/src/NuGetGallery/Controllers/UsersController.cs b/src/NuGetGallery/Controllers/UsersController.cs index 00eef77f6f..ab357566d8 100644 --- a/src/NuGetGallery/Controllers/UsersController.cs +++ b/src/NuGetGallery/Controllers/UsersController.cs @@ -70,17 +70,17 @@ public UsersController( EmailUpdateCancelled = Strings.UserEmailUpdateCancelled }; - protected override void SendNewAccountEmail(User account) + protected override Task SendNewAccountEmailAsync(User account) { var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendNewAccountEmailAsync(account, confirmationUrl); + return MessageService.SendNewAccountEmailAsync(account, confirmationUrl); } - protected override void SendEmailChangedConfirmationNotice(User account) + protected override Task SendEmailChangedConfirmationNoticeAsync(User account) { var confirmationUrl = Url.ConfirmEmail(account.Username, account.EmailConfirmationToken, relativeUrl: false); - MessageService.SendEmailChangeConfirmationNoticeAsync(account, confirmationUrl); + return MessageService.SendEmailChangeConfirmationNoticeAsync(account, confirmationUrl); } protected override User GetAccount(string accountName) @@ -527,7 +527,7 @@ public virtual async Task ForgotPassword(ForgotPasswordViewModel m ModelState.AddModelError(string.Empty, Strings.CouldNotFindAnyoneWithThatUsernameOrEmail); break; case PasswordResetResultType.Success: - return SendPasswordResetEmail(result.User, forgotPassword: true); + return await SendPasswordResetEmailAsync(result.User, forgotPassword: true); default: throw new NotImplementedException($"The password reset result type '{result.Type}' is not supported."); } @@ -646,7 +646,7 @@ public virtual async Task ChangePassword(UserAccountViewModel mode return AccountView(user, model); } - return SendPasswordResetEmail(user, forgotPassword: false); + return await SendPasswordResetEmailAsync(user, forgotPassword: false); } else { @@ -1058,14 +1058,14 @@ private static int CountLoginCredentials(User user) c.Type.StartsWith(CredentialTypes.External.Prefix, StringComparison.OrdinalIgnoreCase)); } - private ActionResult SendPasswordResetEmail(User user, bool forgotPassword) + private async Task SendPasswordResetEmailAsync(User user, bool forgotPassword) { var resetPasswordUrl = Url.ResetEmailOrPassword( user.Username, user.PasswordResetToken, forgotPassword, relativeUrl: false); - MessageService.SendPasswordResetInstructionsAsync(user, resetPasswordUrl, forgotPassword); + await MessageService.SendPasswordResetInstructionsAsync(user, resetPasswordUrl, forgotPassword); return RedirectToAction(actionName: "PasswordSent", controllerName: "Users"); } diff --git a/tests/NuGetGallery.Facts/Controllers/AccountsControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/AccountsControllerFacts.cs index 7358b2c7cd..f0c093f1af 100644 --- a/tests/NuGetGallery.Facts/Controllers/AccountsControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/AccountsControllerFacts.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading.Tasks; using System.Web.Mvc; +using Autofac; using Moq; using NuGetGallery.Framework; using Xunit; @@ -292,6 +293,7 @@ protected virtual Task InvokeChangeEmail( var messageService = GetMock(); messageService.Setup(m => m.SendEmailChangeConfirmationNoticeAsync(It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask) .Verifiable(); var userService = GetMock(); @@ -463,14 +465,14 @@ public abstract class TheConfirmationRequiredPostBaseAction : AccountsController { [Theory] [MemberData(AllowedCurrentUsersDataName)] - public virtual void WhenAlreadyConfirmed_DoesNotSendEmail(Func getCurrentUser) + public virtual async Task WhenAlreadyConfirmed_DoesNotSendEmail(Func getCurrentUser) { // Arrange var controller = GetController(); var account = GetAccount(controller); // Act - var result = InvokeConfirmationRequiredPost(controller, account, getCurrentUser); + var result = await InvokeConfirmationRequiredPostAsync(controller, account, getCurrentUser); // Assert var mailService = GetMock(); @@ -482,7 +484,7 @@ public virtual void WhenAlreadyConfirmed_DoesNotSendEmail(Func getC [Theory] [MemberData(AllowedCurrentUsersDataName)] - public virtual void WhenIsNotConfirmed_SendsEmail(Func getCurrentUser) + public virtual async Task WhenIsNotConfirmed_SendsEmail(Func getCurrentUser) { // Arrange var controller = GetController(); @@ -496,7 +498,7 @@ public virtual void WhenIsNotConfirmed_SendsEmail(Func getCurrentUs var confirmationUrl = (account is Organization) ? TestUtility.GallerySiteRootHttps + $"organization/{account.Username}/Confirm?token=confirmation" : TestUtility.GallerySiteRootHttps + $"account/confirm/{account.Username}/confirmation"; - var result = InvokeConfirmationRequiredPost(controller, account, getCurrentUser, confirmationUrl); + var result = await InvokeConfirmationRequiredPostAsync(controller, account, getCurrentUser, confirmationUrl); // Assert var mailService = GetMock(); @@ -506,7 +508,7 @@ public virtual void WhenIsNotConfirmed_SendsEmail(Func getCurrentUs Assert.True(model.SentEmail); } - protected virtual ActionResult InvokeConfirmationRequiredPost( + protected virtual Task InvokeConfirmationRequiredPostAsync( TAccountsController controller, TUser account, Func getCurrentUser, @@ -522,6 +524,7 @@ protected virtual ActionResult InvokeConfirmationRequiredPost( .Setup(m => m.SendNewAccountEmailAsync( account, string.IsNullOrEmpty(confirmationUrl) ? It.IsAny() : confirmationUrl)) + .Returns(Task.CompletedTask) .Verifiable(); // Act diff --git a/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs index 5ee5d92277..66fbd62387 100644 --- a/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/AuthenticationControllerFacts.cs @@ -107,11 +107,11 @@ public void WillNotRedirectToTheReturnUrlWhenReturnUrlContainsAccount() public class TheSigninAssistanceAction : TestContainer { [Fact] - public void NullUsernameReturnsFalse() + public async Task NullUsernameReturnsFalse() { var controller = GetController(); - var result = controller.SignInAssistance(username: null, providedEmailAddress: null); + var result = await controller.SignInAssistance(username: null, providedEmailAddress: null); dynamic data = result.Data; Assert.False(data.success); } @@ -121,7 +121,7 @@ public void NullUsernameReturnsFalse() [InlineData("rm@address.com", "r**********m@address.com")] [InlineData("r@address.com", "r**********@address.com")] [InlineData("random.very.long.address@address.com", "r**********s@address.com")] - public void NullProvidedEmailReturnsFormattedEmail(string email, string expectedEmail) + public async Task NullProvidedEmailReturnsFormattedEmail(string email, string expectedEmail) { var cred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "blorg", identity: "John Doe "); var existingUser = new User("existingUser") { EmailAddress = email, Credentials = new[] { cred } }; @@ -133,7 +133,7 @@ public void NullProvidedEmailReturnsFormattedEmail(string email, string expected var controller = GetController(); - var result = controller.SignInAssistance(username: "existingUser", providedEmailAddress: null); + var result = await controller.SignInAssistance(username: "existingUser", providedEmailAddress: null); dynamic data = result.Data; Assert.True(data.success); Assert.Equal(expectedEmail, data.EmailAddress); @@ -144,7 +144,7 @@ public void NullProvidedEmailReturnsFormattedEmail(string email, string expected [InlineData("rm@address.com", "r**********m@address.com")] [InlineData("r@address.com", "r**********@address.com")] [InlineData("random.very.long.address@address.com", "r**********s@address.com")] - public void NullProvidedEmailReturnsFormattedEmailForUnconfirmedAccount(string email, string expectedEmail) + public async Task NullProvidedEmailReturnsFormattedEmailForUnconfirmedAccount(string email, string expectedEmail) { var cred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "blorg", identity: "John Doe "); var existingUser = new User("existingUser") { UnconfirmedEmailAddress = email, Credentials = new[] { cred } }; @@ -156,7 +156,7 @@ public void NullProvidedEmailReturnsFormattedEmailForUnconfirmedAccount(string e var controller = GetController(); - var result = controller.SignInAssistance(username: "existingUser", providedEmailAddress: null); + var result = await controller.SignInAssistance(username: "existingUser", providedEmailAddress: null); dynamic data = result.Data; Assert.True(data.success); Assert.Equal(expectedEmail, data.EmailAddress); @@ -166,7 +166,7 @@ public void NullProvidedEmailReturnsFormattedEmailForUnconfirmedAccount(string e [InlineData("blarg")] [InlineData("wrong@email")] [InlineData("nonmatching@emailaddress.com")] - public void InvalidProvidedEmailReturnsFalse(string providedEmail) + public async Task InvalidProvidedEmailReturnsFalse(string providedEmail) { var cred = new CredentialBuilder().CreateExternalCredential("MicrosoftAccount", "blorg", identity: "existing@example.com"); var existingUser = new User("existingUser") { EmailAddress = "existing@example.com", Credentials = new[] { cred } }; @@ -178,13 +178,13 @@ public void InvalidProvidedEmailReturnsFalse(string providedEmail) var controller = GetController(); - var result = controller.SignInAssistance(username: "existingUser", providedEmailAddress: providedEmail); + var result = await controller.SignInAssistance(username: "existingUser", providedEmailAddress: providedEmail); dynamic data = result.Data; Assert.False(data.success); } [Fact] - public void SendsNotificationForAssistance() + public async Task SendsNotificationForAssistance() { var email = "existing@example.com"; var fakes = Get(); @@ -198,11 +198,12 @@ public void SendsNotificationForAssistance() var messageServiceMock = GetMock(); messageServiceMock .Setup(m => m.SendSigninAssistanceEmailAsync(It.IsAny(), It.IsAny>())) + .Returns(Task.CompletedTask) .Verifiable(); var controller = GetController(); - var result = controller.SignInAssistance(username: "existingUser", providedEmailAddress: email); + var result = await controller.SignInAssistance(username: "existingUser", providedEmailAddress: email); dynamic data = result.Data; Assert.True(data.success); messageServiceMock.Verify(); diff --git a/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs index ae85c7dccb..80ad7fb521 100644 --- a/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/OrganizationsControllerFacts.cs @@ -257,14 +257,14 @@ public static IEnumerable WithNonOrganizationAdmin_ReturnsForbidden_Da [Theory] [MemberData(nameof(WithNonOrganizationAdmin_ReturnsForbidden_Data))] - public void WithNonOrganizationAdmin_ReturnsForbidden(Func getCurrentUser) + public async Task WithNonOrganizationAdmin_ReturnsForbidden(Func getCurrentUser) { // Arrange var controller = GetController(); var account = GetAccount(controller); // Act - var result = InvokeConfirmationRequiredPost(controller, account, getCurrentUser) as HttpStatusCodeResult; + var result = await InvokeConfirmationRequiredPostAsync(controller, account, getCurrentUser) as HttpStatusCodeResult; // Assert Assert.NotNull(result); diff --git a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs index 55939754de..49d862136b 100644 --- a/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs @@ -1364,7 +1364,7 @@ public void OnlyShowsOwnersWhoAllowReceivingEmails() } [Fact] - public void HtmlEncodesMessageContent() + public async Task HtmlEncodesMessageContent() { // arrange var packageId = "factory"; @@ -1387,7 +1387,8 @@ public void HtmlEncodesMessageContent() { sentPackageUrl = packageUrl; sentMessage = msg; - }); + }) + .Returns(Task.CompletedTask); var package = new Package { PackageRegistration = new PackageRegistration {Id = packageId}, @@ -1408,14 +1409,14 @@ public void HtmlEncodesMessageContent() }; // act - var result = controller.ContactOwners(packageId, packageVersion, model) as RedirectToRouteResult; + var result = await controller.ContactOwners(packageId, packageVersion, model) as RedirectToRouteResult; Assert.Equal(encodedMessage, sentMessage); Assert.Equal(controller.Url.Package(package, false), sentPackageUrl); } [Fact] - public void CallsSendContactOwnersMessageWithUserInfo() + public async Task CallsSendContactOwnersMessageWithUserInfo() { // arrange var packageId = "factory"; @@ -1429,7 +1430,8 @@ public void CallsSendContactOwnersMessageWithUserInfo() It.IsAny(), It.IsAny(), message, - It.IsAny(), false)); + It.IsAny(), false)) + .Returns(Task.CompletedTask); var package = new Package { PackageRegistration = new PackageRegistration { Id = packageId }, @@ -1450,7 +1452,7 @@ public void CallsSendContactOwnersMessageWithUserInfo() }; // act - var result = controller.ContactOwners(packageId, packageVersion, model) as RedirectToRouteResult; + var result = await controller.ContactOwners(packageId, packageVersion, model) as RedirectToRouteResult; // assert Assert.NotNull(result); @@ -2882,7 +2884,8 @@ public async Task HtmlEncodesMessageContent(User currentUser, User owner) ReportPackageRequest reportRequest = null; _messageService .Setup(s => s.ReportMyPackageAsync(It.IsAny())) - .Callback(r => reportRequest = r); + .Callback(r => reportRequest = r) + .Returns(Task.CompletedTask); // Act await _controller.ReportMyPackage( diff --git a/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs b/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs index 0f2d7666bb..a8e9f87680 100644 --- a/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs +++ b/tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs @@ -258,7 +258,8 @@ public async Task SendsEmailWithPasswordResetUrl() PasswordResetTokenExpirationDate = DateTime.UtcNow.AddHours(Constants.PasswordResetTokenExpirationHours) }; GetMock() - .Setup(s => s.SendPasswordResetInstructionsAsync(user, resetUrl, true)); + .Setup(s => s.SendPasswordResetInstructionsAsync(user, resetUrl, true)) + .Returns(Task.CompletedTask); GetMock() .Setup(s => s.FindByEmailAddress("user")) .Returns(user); From 164856e02689676e0e4072962fe93a5ecf0ddfb0 Mon Sep 17 00:00:00 2001 From: Andy Zivkovic Date: Fri, 3 Aug 2018 15:51:14 -0700 Subject: [PATCH 8/8] IEnumerable<>.ForEach doesn't work with async --- src/NuGetGallery/Controllers/PackagesController.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/NuGetGallery/Controllers/PackagesController.cs b/src/NuGetGallery/Controllers/PackagesController.cs index 278cca143d..67235e5d3a 100644 --- a/src/NuGetGallery/Controllers/PackagesController.cs +++ b/src/NuGetGallery/Controllers/PackagesController.cs @@ -1355,7 +1355,7 @@ private async Task HandleOwnershipRequest(string id, string userna { await _packageOwnershipManagementService.AddPackageOwnerAsync(package, user); - SendAddPackageOwnerNotification(package, user); + await SendAddPackageOwnerNotificationAsync(package, user); return View("ConfirmOwner", new PackageOwnerConfirmationModel(id, user.Username, ConfirmOwnershipResult.Success)); } @@ -1417,14 +1417,15 @@ public virtual async Task CancelPendingOwnershipRequest(string id, /// /// Package to which owner was added. /// Owner added. - private void SendAddPackageOwnerNotification(PackageRegistration package, User newOwner) + private Task SendAddPackageOwnerNotificationAsync(PackageRegistration package, User newOwner) { var packageUrl = Url.Package(package.Id, version: null, relativeUrl: false); Func notNewOwner = o => !o.Username.Equals(newOwner.Username, StringComparison.OrdinalIgnoreCase); // Notify existing owners var notNewOwners = package.Owners.Where(notNewOwner).ToList(); - notNewOwners.ForEach(owner => _messageService.SendPackageOwnerAddedNoticeAsync(owner, newOwner, package, packageUrl)); + var tasks = notNewOwners.Select(owner => _messageService.SendPackageOwnerAddedNoticeAsync(owner, newOwner, package, packageUrl)); + return Task.WhenAll(tasks); } internal virtual async Task Edit(string id, string version, bool? listed, Func urlFactory)