diff --git a/src/Umbraco.Core/Persistence/Repositories/IUserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/IUserRepository.cs index 7c2a7e266458..a8863f135034 100644 --- a/src/Umbraco.Core/Persistence/Repositories/IUserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/IUserRepository.cs @@ -109,8 +109,23 @@ IEnumerable GetPagedResultsByQuery( void ClearLoginSession(Guid sessionId); + /// + /// Gets a page of users, ordered by Id and starting from the provided Id. + /// + /// The user Id to start retrieving users from. + /// The number of users to return. + /// A page of instances. + [Obsolete("No longer used in Umbraco. Scheduled for removal in Umbraco 18.")] IEnumerable GetNextUsers(int id, int count); + /// + /// Gets a page of approved users, ordered by Id and starting from the provided Id. + /// + /// The user Id to start retrieving users from. + /// The number of users to return. + /// A page of instances. + IEnumerable GetNextApprovedUsers(int id, int count) => Enumerable.Empty(); + /// /// Invalidates sessions for users that aren't associated with the current collection of providers. /// diff --git a/src/Umbraco.Core/Services/IUserService.cs b/src/Umbraco.Core/Services/IUserService.cs index eb69f51ff568..df0965fcde80 100644 --- a/src/Umbraco.Core/Services/IUserService.cs +++ b/src/Umbraco.Core/Services/IUserService.cs @@ -231,8 +231,23 @@ IEnumerable GetAll( /// IEnumerable GetAllNotInGroup(int groupId); + /// + /// Gets a page of users, ordered by Id and starting from the provided Id. + /// + /// The user Id to start retrieving users from. + /// The number of users to return. + /// A page of instances. + [Obsolete("No longer used in Umbraco. Scheduled for removal in Umbraco 18.")] IEnumerable GetNextUsers(int id, int count); + /// + /// Gets a page of approved users, ordered by Id and starting from the provided Id. + /// + /// The user Id to start retrieving users from. + /// The number of users to return. + /// A page of instances. + IEnumerable GetNextApprovedUsers(int id, int count) => Enumerable.Empty(); + /// /// Invalidates sessions for users that aren't associated with the current collection of providers. /// diff --git a/src/Umbraco.Core/Services/NotificationService.cs b/src/Umbraco.Core/Services/NotificationService.cs index 336435d97ad5..1c370fe5f0fc 100644 --- a/src/Umbraco.Core/Services/NotificationService.cs +++ b/src/Umbraco.Core/Services/NotificationService.cs @@ -96,7 +96,7 @@ public void SendNotifications( // see notes above var id = Constants.Security.SuperUserId; - const int pagesz = 400; // load batches of 400 users + const int UserBatchSize = 400; // load batches of 400 users do { var notifications = GetUsersNotifications(new List(), action, Enumerable.Empty(), Constants.ObjectTypes.Document)?.ToList(); @@ -106,10 +106,10 @@ public void SendNotifications( } // users are returned ordered by id, notifications are returned ordered by user id - var users = _userService.GetNextUsers(id, pagesz).Where(x => x.IsApproved).ToList(); - foreach (IUser user in users) + var approvedUsers = _userService.GetNextApprovedUsers(id, UserBatchSize).ToList(); + foreach (IUser approvedUser in approvedUsers) { - Notification[] userNotifications = notifications.Where(n => n.UserId == user.Id).ToArray(); + Notification[] userNotifications = notifications.Where(n => n.UserId == approvedUser.Id).ToArray(); foreach (Notification notification in userNotifications) { // notifications are inherited down the tree - find the topmost entity @@ -130,14 +130,14 @@ public void SendNotifications( } // queue notification - NotificationRequest req = CreateNotificationRequest(operatingUser, user, entityForNotification, prevVersionDictionary[entityForNotification.Id], actionName, siteUri, createSubject, createBody); + NotificationRequest req = CreateNotificationRequest(operatingUser, approvedUser, entityForNotification, prevVersionDictionary[entityForNotification.Id], actionName, siteUri, createSubject, createBody); Enqueue(req); break; } } // load more users if any - id = users.Count == pagesz ? users.Last().Id + 1 : -1; + id = approvedUsers.Count == UserBatchSize ? approvedUsers.Last().Id + 1 : -1; } while (id > 0); } diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs index 625dd8e77816..8723917742c5 100644 --- a/src/Umbraco.Core/Services/UserService.cs +++ b/src/Umbraco.Core/Services/UserService.cs @@ -712,6 +712,7 @@ public IEnumerable GetAll(long pageIndex, int pageSize, out long totalRec } } + /// public IEnumerable GetNextUsers(int id, int count) { using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) @@ -720,6 +721,15 @@ public IEnumerable GetNextUsers(int id, int count) } } + /// + public IEnumerable GetNextApprovedUsers(int id, int count) + { + using (ICoreScope scope = ScopeProvider.CreateCoreScope(autoComplete: true)) + { + return _userRepository.GetNextApprovedUsers(id, count); + } + } + /// public void InvalidateSessionsForRemovedProviders(IEnumerable currentLoginProviders) { diff --git a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs index 1cda2e5259ce..385735911209 100644 --- a/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs +++ b/src/Umbraco.Infrastructure/Persistence/Repositories/Implement/UserRepository.cs @@ -1053,13 +1053,25 @@ private Sql ApplySort(Sql sql, Expression GetNextUsers(int id, int count) + /// + public IEnumerable GetNextUsers(int id, int count) => PerformGetNextUsers(id, false, count); + + /// + public IEnumerable GetNextApprovedUsers(int id, int count) => PerformGetNextUsers(id, true, count); + + private IEnumerable PerformGetNextUsers(int id, bool approvedOnly, int count) { Sql idsQuery = SqlContext.Sql() .Select(x => x.Id) .From() - .Where(x => x.Id >= id) - .OrderBy(x => x.Id); + .Where(x => x.Id >= id); + + if (approvedOnly) + { + idsQuery = idsQuery.Where(x => x.Disabled == false); + } + + idsQuery = idsQuery.OrderBy(x => x.Id); // first page is index 1, not zero var ids = Database.Page(1, count, idsQuery).Items.ToArray(); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs index 8884622bf2b8..c3d5ebef5413 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/UserServiceTests.cs @@ -967,6 +967,73 @@ public void Can_Get_Assigned_StartNodes_For_User() } } + [Test] + public void Can_Get_Next_Users_In_Batches() + { + var users = UserBuilder.CreateMulipleUsers(10).ToArray(); + UserService.Save(users); + + var userBatch1 = UserService.GetNextUsers(Constants.Security.SuperUserId, 3); + var userBatch2 = UserService.GetNextUsers(1, 6); + var userBatch3 = UserService.GetNextUsers(4, 5); + var userBatch4 = UserService.GetNextUsers(9, 5); + var allUsers = UserService.GetNextUsers(Constants.Security.SuperUserId, int.MaxValue); + + Assert.AreEqual(3, userBatch1.Count()); + Assert.AreEqual(Constants.Security.SuperUserId, userBatch1.First().Id); + Assert.AreEqual(2, userBatch1.Last().Id); + + Assert.AreEqual(6, userBatch2.Count()); + Assert.AreEqual(1, userBatch2.First().Id); + Assert.AreEqual(6, userBatch2.Last().Id); + + Assert.AreEqual(5, userBatch3.Count()); + Assert.AreEqual(4, userBatch3.First().Id); + Assert.AreEqual(8, userBatch3.Last().Id); + + Assert.AreEqual(2, userBatch4.Count()); + Assert.AreEqual(9, userBatch4.First().Id); + Assert.AreEqual(10, userBatch4.Last().Id); + + Assert.AreEqual(11, allUsers.Count()); + } + + [Test] + public void Can_Get_Next_Approved_Users_In_Batches() + { + var users = UserBuilder.CreateMulipleUsers(10).ToArray(); + for (int i = 0; i < users.Length; i++) + { + users[i].IsApproved = !(i == 0 || i == 6); // Setup all users as approved except for a couple. + } + + UserService.Save(users); + + var userBatch1 = UserService.GetNextApprovedUsers(Constants.Security.SuperUserId, 3); + var userBatch2 = UserService.GetNextApprovedUsers(1, 6); + var userBatch3 = UserService.GetNextApprovedUsers(4, 5); + var userBatch4 = UserService.GetNextApprovedUsers(9, 5); + var allApprovedUsers = UserService.GetNextApprovedUsers(Constants.Security.SuperUserId, int.MaxValue); + + Assert.AreEqual(3, userBatch1.Count()); + Assert.AreEqual(Constants.Security.SuperUserId, userBatch1.First().Id); + Assert.AreEqual(3, userBatch1.Last().Id); + + Assert.AreEqual(6, userBatch2.Count()); + Assert.AreEqual(2, userBatch2.First().Id); + Assert.AreEqual(8, userBatch2.Last().Id); + + Assert.AreEqual(5, userBatch3.Count()); + Assert.AreEqual(4, userBatch3.First().Id); + Assert.AreEqual(9, userBatch3.Last().Id); + + Assert.AreEqual(2, userBatch4.Count()); + Assert.AreEqual(9, userBatch4.First().Id); + Assert.AreEqual(10, userBatch4.Last().Id); + + Assert.AreEqual(9, allApprovedUsers.Count()); + } + private Content[] BuildContentItems(int numberToCreate) { var template = TemplateBuilder.CreateTextPageTemplate();