From 8ed3a95a56313e2fef2ea7377032586741683ae2 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Sun, 28 Oct 2018 21:18:53 +0100 Subject: [PATCH] Fix #3204 and do some refactoring to avoid duplicate code --- .../Services/NotificationService.cs | 60 ++++--------------- 1 file changed, 10 insertions(+), 50 deletions(-) diff --git a/src/Umbraco.Core/Services/NotificationService.cs b/src/Umbraco.Core/Services/NotificationService.cs index cd94fd7ca7..07d6e4e9e5 100644 --- a/src/Umbraco.Core/Services/NotificationService.cs +++ b/src/Umbraco.Core/Services/NotificationService.cs @@ -58,54 +58,7 @@ namespace Umbraco.Core.Services if (entity is IContent == false) throw new NotSupportedException(); - var content = (IContent) entity; - - // lazily get previous version - IContentBase prevVersion = null; - - // do not load *all* users in memory at once - // do not load notifications *per user* (N+1 select) - // cannot load users & notifications in 1 query (combination btw User2AppDto and User2NodeNotifyDto) - // => get batches of users, get all their notifications in 1 query - // re. users: - // users being (dis)approved = not an issue, filtered in memory not in SQL - // users being modified or created = not an issue, ordering by ID, as long as we don't *insert* low IDs - // users being deleted = not an issue for GetNextUsers - var id = 0; - var nodeIds = content.Path.Split(',').Select(int.Parse).ToArray(); - const int pagesz = 400; // load batches of 400 users - do - { - // users are returned ordered by id, notifications are returned ordered by user id - var users = ((UserService) _userService).GetNextUsers(id, pagesz).Where(x => x.IsApproved).ToList(); - var notifications = GetUsersNotifications(users.Select(x => x.Id), action, nodeIds, Constants.ObjectTypes.DocumentGuid).ToList(); - if (notifications.Count == 0) break; - - var i = 0; - foreach (var user in users) - { - // continue if there's no notification for this user - if (notifications[i].UserId != user.Id) continue; // next user - - // lazy load prev version - if (prevVersion == null) - { - prevVersion = GetPreviousVersion(entity.Id); - } - - // queue notification - var req = CreateNotificationRequest(operatingUser, user, content, prevVersion, actionName, http, createSubject, createBody); - Enqueue(req); - - // skip other notifications for this user - while (i < notifications.Count && notifications[i++].UserId == user.Id) ; - if (i >= notifications.Count) break; // break if no more notifications - } - - // load more users if any - id = users.Count == pagesz ? users.Last().Id + 1 : -1; - - } while (id > 0); + SendNotifications(operatingUser, new[] { (IContent)entity }, action, actionName, http, createSubject, createBody); } /// @@ -154,7 +107,14 @@ namespace Umbraco.Core.Services // lazily get versions var prevVersionDictionary = new Dictionary(); - // see notes above + // do not load *all* users in memory at once + // do not load notifications *per user* (N+1 select) + // cannot load users & notifications in 1 query (combination btw User2AppDto and User2NodeNotifyDto) + // => get batches of users, get all their notifications in 1 query + // re. users: + // users being (dis)approved = not an issue, filtered in memory not in SQL + // users being modified or created = not an issue, ordering by ID, as long as we don't *insert* low IDs + // users being deleted = not an issue for GetNextUsers var id = 0; const int pagesz = 400; // load batches of 400 users do @@ -656,4 +616,4 @@ namespace Umbraco.Core.Services #endregion } -} \ No newline at end of file +}