diff --git a/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs b/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs index 904bf68623..dc2a5570a6 100644 --- a/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs +++ b/src/Umbraco.Examine.Lucene/UmbracoContentIndex.cs @@ -86,7 +86,7 @@ namespace Umbraco.Examine || !validator.ValidateProtectedContent(path, v.Category)) ? 0 : 1; - }); + }).ToList(); var hasDeletes = false; var hasUpdates = false; @@ -105,7 +105,7 @@ namespace Umbraco.Examine { hasUpdates = true; //these are the valid ones, so just index them all at once - base.PerformIndexItems(group, onComplete); + base.PerformIndexItems(group.ToList(), onComplete); } } diff --git a/src/Umbraco.Infrastructure/Search/ExamineComponent.cs b/src/Umbraco.Infrastructure/Search/ExamineComponent.cs index 037981d8b4..35a8804ecb 100644 --- a/src/Umbraco.Infrastructure/Search/ExamineComponent.cs +++ b/src/Umbraco.Infrastructure/Search/ExamineComponent.cs @@ -583,6 +583,26 @@ namespace Umbraco.Web.Search public static void Execute(ExamineComponent examineComponent, IContent content, bool isPublished) { + // TODO: This is ugly, it is going to build the value set 3x and for 2 of those times it will be the same value + // set. We can do better. + + // TODO: We are .ToList() ing each of the calls to GetValueSets here. This is currently required but isn't the way + // that this was intended to work. Ideally, the IEnumerable package gets passed to Examine and it is only iterated/executed + // when the indexing takes place which would occur on a background thread. This is problematic with how the ContentValueSetBuilder + // in combination with UmbracoContentIndex.PerformIndexItems works because (at least what I've come to believe) we are using yield + // return in the GetValueSets call in combination with trying to lazily resolve the enumerable but because we GroupBy in + // UmbracoContentIndex.PerformIndexItems it's eagerly executed but then lazily executed again on the background thread and I believe + // that in doing this when the call is made to _userService.GetProfilesById it's trying to resolve a scope from an AsyncLocal instance + // that has already been disposed higher up it's chain. I 'think' to how the eager/lazy enumeration happens with yield return that it's + // capturing a scope/AsyncLocal instance that it shouldn't really be using. + + // TODO: We don't want these value sets to be eagerly built in this thread since this is most likely going to be a request thread. + // This is why the lazy execution of the Enumerable had the intended affect of executing only when requested on the background thread. + // This could still be acheived: Either we have a custom Enumerable/Enumerator to do this, or we simply call the below code + // on a background thread... which would be much easier! + + // TODO: I think this is an issue in v8 too! + foreach (var index in examineComponent._examineManager.Indexes.OfType() //filter the indexers .Where(x => isPublished || !x.PublishedValuesOnly) @@ -593,7 +613,7 @@ namespace Umbraco.Web.Search ? examineComponent._publishedContentValueSetBuilder : (IValueSetBuilder)examineComponent._contentValueSetBuilder; - index.IndexItems(builder.GetValueSets(content)); + index.IndexItems(builder.GetValueSets(content).ToList()); } } } 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.Infrastructure/Sync/DatabaseServerMessenger.cs b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs index c915013162..2f6bb61e42 100644 --- a/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs +++ b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs @@ -63,6 +63,13 @@ namespace Umbraco.Core.Sync _lastPruned = _lastSync = DateTime.UtcNow; _syncIdle = new ManualResetEvent(true); _distCacheFilePath = new Lazy(() => GetDistCacheFilePath(hostingEnvironment)); + + // See notes on LocalIdentity + LocalIdentity = NetworkHelper.MachineName // eg DOMAIN\SERVER + + "/" + _hostingEnvironment.ApplicationId // eg /LM/S3SVC/11/ROOT + + " [P" + Process.GetCurrentProcess().Id // eg 1234 + + "/D" + AppDomain.CurrentDomain.Id // eg 22 + + "] " + Guid.NewGuid().ToString("N").ToUpper(); // make it truly unique } protected ILogger Logger { get; } @@ -526,11 +533,7 @@ namespace Umbraco.Core.Sync /// Practically, all we really need is the guid, the other infos are here for information /// and debugging purposes. /// - protected string LocalIdentity => NetworkHelper.MachineName // eg DOMAIN\SERVER - + "/" + _hostingEnvironment.ApplicationId // eg /LM/S3SVC/11/ROOT - + " [P" + Process.GetCurrentProcess().Id // eg 1234 - + "/D" + AppDomain.CurrentDomain.Id // eg 22 - + "] " + Guid.NewGuid().ToString("N").ToUpper(); // make it truly unique + protected string LocalIdentity { get; } private string GetDistCacheFilePath(IHostingEnvironment hostingEnvironment) { 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)));