diff --git a/src/Umbraco.Infrastructure/Services/Implement/NotificationService.cs b/src/Umbraco.Infrastructure/Services/Implement/NotificationService.cs index d6c30b24c6..cf53fb34cd 100644 --- a/src/Umbraco.Infrastructure/Services/Implement/NotificationService.cs +++ b/src/Umbraco.Infrastructure/Services/Implement/NotificationService.cs @@ -3,10 +3,10 @@ using System.Collections.Concurrent; using System.Collections.Generic; using System.Globalization; using System.Linq; +using System.Net; using System.Net.Mail; using System.Text; using System.Threading; -using Umbraco.Core.Composing; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.IO; @@ -16,7 +16,6 @@ using Umbraco.Core.Models.Entities; using Umbraco.Core.Models.Membership; using Umbraco.Core.Persistence.Repositories; using Umbraco.Core.Scoping; -using Umbraco.Core.Strings; namespace Umbraco.Core.Services.Implement { @@ -29,15 +28,17 @@ namespace Umbraco.Core.Services.Implement private readonly INotificationsRepository _notificationsRepository; private readonly IGlobalSettings _globalSettings; private readonly IContentSettings _contentSettings; + private readonly IEmailSender _emailSender; private readonly ILogger _logger; private readonly IIOHelper _ioHelper; public NotificationService(IScopeProvider provider, IUserService userService, IContentService contentService, ILocalizationService localizationService, - ILogger logger, IIOHelper ioHelper, INotificationsRepository notificationsRepository, IGlobalSettings globalSettings, IContentSettings contentSettings) + ILogger logger, IIOHelper ioHelper, INotificationsRepository notificationsRepository, IGlobalSettings globalSettings, IContentSettings contentSettings, IEmailSender emailSender) { _notificationsRepository = notificationsRepository; _globalSettings = globalSettings; _contentSettings = contentSettings; + _emailSender = emailSender; _uowProvider = provider ?? throw new ArgumentNullException(nameof(provider)); _userService = userService ?? throw new ArgumentNullException(nameof(userService)); _contentService = contentService ?? throw new ArgumentNullException(nameof(contentService)); @@ -405,8 +406,9 @@ namespace Umbraco.Core.Services.Implement string.Concat(siteUri.Authority, _ioHelper.ResolveUrl(_globalSettings.UmbracoPath)), summary.ToString()); + var fromMail = _contentSettings.NotificationEmailAddress ?? _globalSettings.SmtpSettings.From; // create the mail message - var mail = new MailMessage(_contentSettings.NotificationEmailAddress, mailingUser.Email); + var mail = new MailMessage(fromMail, mailingUser.Email); // populate the message @@ -508,51 +510,38 @@ namespace Umbraco.Core.Services.Implement { ThreadPool.QueueUserWorkItem(state => { - var s = new SmtpClient(); - try + _logger.Debug("Begin processing notifications."); + while (true) { - _logger.Debug("Begin processing notifications."); - while (true) + NotificationRequest request; + while (notificationRequests.TryTake(out request, 8 * 1000)) // stay on for 8s { - NotificationRequest request; - while (notificationRequests.TryTake(out request, 8 * 1000)) // stay on for 8s + try { - try - { - if (Sendmail != null) Sendmail(s, request.Mail, _logger); else s.Send(request.Mail); - _logger.Debug("Notification '{Action}' sent to {Username} ({Email})", request.Action, request.UserName, request.Email); - } - catch (Exception ex) - { - _logger.Error(ex, "An error occurred sending notification"); - s.Dispose(); - s = new SmtpClient(); - } - finally - { - request.Mail.Dispose(); - } + _emailSender.SendAsync(request.Mail).GetAwaiter().GetResult(); + _logger.Debug("Notification '{Action}' sent to {Username} ({Email})", request.Action, request.UserName, request.Email); } - lock (Locker) + catch (Exception ex) { - if (notificationRequests.Count > 0) continue; // last chance - _running = false; // going down - break; + _logger.Error(ex, "An error occurred sending notification"); + } + finally + { + request.Mail.Dispose(); } } + lock (Locker) + { + if (notificationRequests.Count > 0) continue; // last chance + _running = false; // going down + break; + } } - finally - { - s.Dispose(); - } + _logger.Debug("Done processing notifications."); }); } - // for tests - internal static Action Sendmail; - //= (_, msg, logger) => logger.Debug("Email " + msg.To.ToString()); - #endregion } } diff --git a/src/Umbraco.Tests/TestHelpers/TestHelper.cs b/src/Umbraco.Tests/TestHelpers/TestHelper.cs index 5512f50254..9aeb668518 100644 --- a/src/Umbraco.Tests/TestHelpers/TestHelper.cs +++ b/src/Umbraco.Tests/TestHelpers/TestHelper.cs @@ -42,6 +42,8 @@ namespace Umbraco.Tests.TestHelpers public static class TestHelper { private static readonly TestHelperInternal _testHelperInternal = new TestHelperInternal(); + private static IEmailSender _emailSender; + private class TestHelperInternal : TestHelperBase { public TestHelperInternal() : base(typeof(TestHelperInternal).Assembly) @@ -103,6 +105,8 @@ namespace Umbraco.Tests.TestHelpers public static IWebRoutingSettings WebRoutingSettings => _testHelperInternal.WebRoutingSettings; + public static IEmailSender EmailSender { get; } = new EmailSender(SettingsForTests.GenerateMockGlobalSettings()); + /// /// Some test files are copied to the /bin (/bin/debug) on build, this is a utility to return their physical path based on a virtual path name diff --git a/src/Umbraco.Tests/TestHelpers/TestObjects.cs b/src/Umbraco.Tests/TestHelpers/TestObjects.cs index 7e8914f78e..44f6be8e7d 100644 --- a/src/Umbraco.Tests/TestHelpers/TestObjects.cs +++ b/src/Umbraco.Tests/TestHelpers/TestObjects.cs @@ -161,7 +161,7 @@ namespace Umbraco.Tests.TestHelpers var dataTypeService = GetLazyService(factory, c => new DataTypeService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), ioHelper, localizedTextService.Value, localizationService.Value, TestHelper.ShortStringHelper)); var propertyValidationService = new Lazy(() => new PropertyValidationService(propertyEditorCollection, dataTypeService.Value, localizedTextService.Value)); var contentService = GetLazyService(factory, c => new ContentService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c), propertyValidationService, TestHelper.ShortStringHelper)); - var notificationService = GetLazyService(factory, c => new NotificationService(scopeProvider, userService.Value, contentService.Value, localizationService.Value, logger, ioHelper, GetRepo(c), globalSettings, contentSettings)); + var notificationService = GetLazyService(factory, c => new NotificationService(scopeProvider, userService.Value, contentService.Value, localizationService.Value, logger, ioHelper, GetRepo(c), globalSettings, contentSettings, TestHelper.EmailSender)); var serverRegistrationService = GetLazyService(factory, c => new ServerRegistrationService(scopeProvider, logger, eventMessagesFactory, GetRepo(c), TestHelper.GetHostingEnvironment())); var memberGroupService = GetLazyService(factory, c => new MemberGroupService(scopeProvider, logger, eventMessagesFactory, GetRepo(c))); var memberService = GetLazyService(factory, c => new MemberService(scopeProvider, logger, eventMessagesFactory, memberGroupService.Value, GetRepo(c), GetRepo(c), GetRepo(c), GetRepo(c)));