From 57bd5178f74c277b45c352dc67fcd63ed0f1a451 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 17 May 2017 21:40:17 +1000 Subject: [PATCH] Gets proper paging and filtering working for users --- .../Mappers/ExternalLoginMapper.cs | 35 +++ .../Persistence/Mappers/UserMapper.cs | 30 --- .../Repositories/UserRepository.cs | 200 +++++++++--------- src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Repositories/UserRepositoryTest.cs | 32 +++ .../Services/UserServiceTests.cs | 67 ++++++ 6 files changed, 236 insertions(+), 129 deletions(-) create mode 100644 src/Umbraco.Core/Persistence/Mappers/ExternalLoginMapper.cs diff --git a/src/Umbraco.Core/Persistence/Mappers/ExternalLoginMapper.cs b/src/Umbraco.Core/Persistence/Mappers/ExternalLoginMapper.cs new file mode 100644 index 0000000000..c4226797e2 --- /dev/null +++ b/src/Umbraco.Core/Persistence/Mappers/ExternalLoginMapper.cs @@ -0,0 +1,35 @@ +using System.Collections.Concurrent; +using Umbraco.Core.Models.Identity; +using Umbraco.Core.Models.Rdbms; + +namespace Umbraco.Core.Persistence.Mappers +{ + [MapperFor(typeof(IIdentityUserLogin))] + [MapperFor(typeof(IdentityUserLogin))] + public sealed class ExternalLoginMapper : BaseMapper + { + private static readonly ConcurrentDictionary PropertyInfoCacheInstance = new ConcurrentDictionary(); + public ExternalLoginMapper() + { + BuildMap(); + } + + #region Overrides of BaseMapper + + internal override ConcurrentDictionary PropertyInfoCache + { + get { return PropertyInfoCacheInstance; } + } + + internal override void BuildMap() + { + CacheMap(src => src.Id, dto => dto.Id); + CacheMap(src => src.CreateDate, dto => dto.CreateDate); + CacheMap(src => src.LoginProvider, dto => dto.LoginProvider); + CacheMap(src => src.ProviderKey, dto => dto.ProviderKey); + CacheMap(src => src.UserId, dto => dto.UserId); + } + + #endregion + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Mappers/UserMapper.cs b/src/Umbraco.Core/Persistence/Mappers/UserMapper.cs index baefd56db5..bf7ca9ed8f 100644 --- a/src/Umbraco.Core/Persistence/Mappers/UserMapper.cs +++ b/src/Umbraco.Core/Persistence/Mappers/UserMapper.cs @@ -1,41 +1,11 @@ using System; using System.Collections.Concurrent; using System.Linq.Expressions; -using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; namespace Umbraco.Core.Persistence.Mappers { - [MapperFor(typeof(IIdentityUserLogin))] - [MapperFor(typeof(IdentityUserLogin))] - public sealed class ExternalLoginMapper : BaseMapper - { - private static readonly ConcurrentDictionary PropertyInfoCacheInstance = new ConcurrentDictionary(); - public ExternalLoginMapper() - { - BuildMap(); - } - - #region Overrides of BaseMapper - - internal override ConcurrentDictionary PropertyInfoCache - { - get { return PropertyInfoCacheInstance; } - } - - internal override void BuildMap() - { - CacheMap(src => src.Id, dto => dto.Id); - CacheMap(src => src.CreateDate, dto => dto.CreateDate); - CacheMap(src => src.LoginProvider, dto => dto.LoginProvider); - CacheMap(src => src.ProviderKey, dto => dto.ProviderKey); - CacheMap(src => src.UserId, dto => dto.UserId); - } - - #endregion - } - [MapperFor(typeof(IUser))] [MapperFor(typeof(User))] public sealed class UserMapper : BaseMapper diff --git a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs index bcf1ca26a8..4ad7ecb390 100644 --- a/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/UserRepository.cs @@ -108,16 +108,21 @@ namespace Umbraco.Core.Persistence.Repositories /// /// private Sql GetQueryWithGroups() - { + { //base query includes user groups var sql = GetBaseQuery("umbracoUser.*, umbracoUserGroup.*, umbracoUserGroup2App.*"); + AddGroupLeftJoin(sql); + return sql; + } + + private void AddGroupLeftJoin(Sql sql) + { sql.LeftJoin(SqlSyntax) .On(SqlSyntax, dto => dto.UserId, dto => dto.Id) .LeftJoin(SqlSyntax) .On(SqlSyntax, dto => dto.Id, dto => dto.UserGroupId) .LeftJoin(SqlSyntax) - .On(SqlSyntax, dto => dto.UserGroupId, dto => dto.Id); - return sql; + .On(SqlSyntax, dto => dto.UserGroupId, dto => dto.Id); } private Sql GetBaseQuery(string columns) @@ -168,6 +173,24 @@ namespace Umbraco.Core.Persistence.Repositories var id = Convert.ToInt32(Database.Insert(userDto)); entity.Id = id; + if (entity.IsPropertyDirty("Groups")) + { + //lookup all assigned + var assigned = entity.Groups == null || entity.Groups.Any() == false + ? new List() + : Database.Fetch("SELECT * FROM umbracoUserGroup WHERE userGroupAlias IN (@aliases)", new { aliases = entity.Groups }); + + foreach (var groupDto in assigned) + { + var dto = new User2UserGroupDto + { + UserGroupId = groupDto.Id, + UserId = entity.Id + }; + Database.Insert(dto); + } + } + entity.ResetDirtyProperties(); } @@ -182,9 +205,7 @@ namespace Umbraco.Core.Persistence.Repositories } var userDto = userFactory.BuildDto(entity); - - var dirtyEntity = (ICanBeDirty)entity; - + //build list of columns to check for saving - we don't want to save the password if it hasn't changed! //List the columns to save, NOTE: would be nice to not have hard coded strings here but no real good way around that var colsToSave = new Dictionary() @@ -207,19 +228,19 @@ namespace Umbraco.Core.Persistence.Repositories //create list of properties that have changed var changedCols = colsToSave - .Where(col => dirtyEntity.IsPropertyDirty(col.Value)) + .Where(col => entity.IsPropertyDirty(col.Value)) .Select(col => col.Key) .ToList(); // DO NOT update the password if it has not changed or if it is null or empty - if (dirtyEntity.IsPropertyDirty("RawPasswordValue") && entity.RawPasswordValue.IsNullOrWhiteSpace() == false) + if (entity.IsPropertyDirty("RawPasswordValue") && entity.RawPasswordValue.IsNullOrWhiteSpace() == false) { changedCols.Add("userPassword"); //special case - when using ASP.Net identity the user manager will take care of updating the security stamp, however // when not using ASP.Net identity (i.e. old membership providers), we'll need to take care of updating this manually // so we can just detect if that property is dirty, if it's not we'll set it manually - if (dirtyEntity.IsPropertyDirty("SecurityStamp") == false) + if (entity.IsPropertyDirty("SecurityStamp") == false) { userDto.SecurityStampToken = entity.SecurityStamp = Guid.NewGuid().ToString(); changedCols.Add("securityStampToken"); @@ -231,24 +252,27 @@ namespace Umbraco.Core.Persistence.Repositories { Database.Update(userDto, changedCols); } - - //lookup all assigned - var assigned = entity.Groups == null || entity.Groups.Any() == false - ? new List() - : Database.Fetch("SELECT * FROM umbracoUserGroup WHERE userGroupAlias IN (@aliases)", new {aliases = entity.Groups}); - - //first delete all - //TODO: We could do this a nicer way instead of "Nuke and Pave" - Database.Delete("WHERE UserId = @UserId", new { UserId = entity.Id }); - foreach (var groupDto in assigned) - { - var dto = new User2UserGroupDto + if (entity.IsPropertyDirty("Groups")) + { + //lookup all assigned + var assigned = entity.Groups == null || entity.Groups.Any() == false + ? new List() + : Database.Fetch("SELECT * FROM umbracoUserGroup WHERE userGroupAlias IN (@aliases)", new { aliases = entity.Groups }); + + //first delete all + //TODO: We could do this a nicer way instead of "Nuke and Pave" + Database.Delete("WHERE UserId = @UserId", new { UserId = entity.Id }); + + foreach (var groupDto in assigned) { - UserGroupId = groupDto.Id, - UserId = entity.Id - }; - Database.Insert(dto); + var dto = new User2UserGroupDto + { + UserGroupId = groupDto.Id, + UserId = entity.Id + }; + Database.Insert(dto); + } } entity.ResetDirtyProperties(); @@ -372,9 +396,12 @@ namespace Umbraco.Core.Persistence.Repositories private IEnumerable GetPagedResultsByQuery(IQuery query, long pageIndex, int pageSize, out long totalRecords, string orderBy, Direction orderDirection, string[] userGroups = null, UserState? userState = null, IQuery filter = null) { if (string.IsNullOrWhiteSpace(orderBy)) throw new ArgumentException("Value cannot be null or whitespace.", "orderBy"); - - var filterSql = new Sql(); + + Sql filterSql = null; + if (filter != null || (userGroups != null && userGroups.Length > 0)) + filterSql = new Sql(); + if (filter != null) { foreach (var filterClaus in filter.GetWhereClauses()) @@ -382,100 +409,75 @@ namespace Umbraco.Core.Persistence.Repositories filterSql.Append(string.Format("AND ({0})", filterClaus.Item1), filterClaus.Item2); } } - Func> filterCallback = () => new Tuple(filterSql.SQL, filterSql.Arguments); - + if (userGroups != null && userGroups.Length > 0) + { + var subQuery = @"AND (umbracoUser.id IN (SELECT DISTINCT umbracoUser.id + FROM umbracoUser + INNER JOIN umbracoUser2UserGroup ON umbracoUser2UserGroup.userId = umbracoUser.id + INNER JOIN umbracoUserGroup ON umbracoUserGroup.id = umbracoUser2UserGroup.userGroupId + WHERE umbracoUserGroup.userGroupAlias IN (@userGroups)))"; + filterSql.Append(subQuery, new {userGroups= userGroups}); + } + // Get base query for returning IDs var sqlBaseIds = GetBaseQuery("id"); - // Get base query for returning all data - var sqlBaseFull = GetBaseQuery(false); - + if (query == null) query = new Query(); var translatorIds = new SqlTranslator(sqlBaseIds, query); - var sqlQueryIds = translatorIds.Translate(); - var translatorFull = new SqlTranslator(sqlBaseFull, query); - var sqlQueryFull = translatorFull.Translate(); - + var sqlQueryIds = translatorIds.Translate(); + //get sorted and filtered sql var sqlNodeIdsWithSort = GetSortedSqlForPagedResults( - GetFilteredSqlForPagedResults(sqlQueryIds, filterCallback), + GetFilteredSqlForPagedResults(sqlQueryIds, filterSql), orderDirection, orderBy); // Get page of results and total count - IEnumerable result; var pagedResult = Database.Page(pageIndex + 1, pageSize, sqlNodeIdsWithSort); - totalRecords = Convert.ToInt32(pagedResult.TotalItems); - - return Enumerable.Empty(); - - ////NOTE: We need to check the actual items returned, not the 'totalRecords', that is because if you request a page number - //// that doesn't actually have any data on it, the totalRecords will still indicate there are records but there are none in - //// the pageResult, then the GetAll will actually return ALL records in the db. - //if (pagedResult.Items.Any()) - //{ - // //Create the inner paged query that was used above to get the paged result, we'll use that as the inner sub query - // var args = sqlNodeIdsWithSort.Arguments; - // string sqlStringCount, sqlStringPage; - // Database.BuildPageQueries(pageIndex * pageSize, pageSize, sqlNodeIdsWithSort.SQL, ref args, out sqlStringCount, out sqlStringPage); - - // //We need to make this FULL query an inner join on the paged ID query - // var splitQuery = sqlQueryFull.SQL.Split(new[] { "WHERE " }, StringSplitOptions.None); - // var fullQueryWithPagedInnerJoin = new Sql(splitQuery[0]) - // .Append("INNER JOIN (") - // //join the paged query with the paged query arguments - // .Append(sqlStringPage, args) - // .Append(") temp ") - // .Append(string.Format("ON {0}.{1} = temp.{1}", nodeIdSelect.Item1, nodeIdSelect.Item2)) - // //add the original where clause back with the original arguments - // .Where(splitQuery[1], sqlQueryIds.Arguments); - - // //get sorted and filtered sql - // var fullQuery = GetSortedSqlForPagedResults( - // GetFilteredSqlForPagedResults(fullQueryWithPagedInnerJoin, defaultFilter), - // orderDirection, orderBy, orderBySystemField, nodeIdSelect); - - // return processQuery(fullQuery, new PagingSqlQuery(Database, sqlNodeIdsWithSort, pageIndex, pageSize)); - //} - //else - //{ - // result = Enumerable.Empty(); - //} - - //return result; + totalRecords = Convert.ToInt32(pagedResult.TotalItems); + //NOTE: We need to check the actual items returned, not the 'totalRecords', that is because if you request a page number + // that doesn't actually have any data on it, the totalRecords will still indicate there are records but there are none in + // the pageResult. + if (pagedResult.Items.Any()) + { + //Create the inner paged query that was used above to get the paged result, we'll use that as the inner sub query + var args = sqlNodeIdsWithSort.Arguments; + string sqlStringCount, sqlStringPage; + Database.BuildPageQueries(pageIndex * pageSize, pageSize, sqlNodeIdsWithSort.SQL, ref args, out sqlStringCount, out sqlStringPage); - //return GetPagedResultsByQuery(query, pageIndex, pageSize, out totalRecords, - // new Tuple("cmsMember", "nodeId"), - // (sqlFull, sqlIds) => ProcessQuery(sqlFull, sqlIds), orderBy, orderDirection, orderBySystemField, - // filterCallback); + var sqlQueryFull = GetBaseQuery("umbracoUser.*, umbracoUserGroup.*, umbracoUserGroup2App.*"); + + var fullQueryWithPagedInnerJoin = sqlQueryFull + .Append("INNER JOIN (") + //join the paged query with the paged query arguments + .Append(sqlStringPage, args) + .Append(") temp ") + .Append("ON umbracoUser.id = temp.id"); + AddGroupLeftJoin(fullQueryWithPagedInnerJoin); - //var sql = new Sql() - // .Select("umbracoUser.Id") - // .From(SqlSyntax); + //get sorted and filtered sql + var fullQuery = GetSortedSqlForPagedResults( + GetFilteredSqlForPagedResults(fullQueryWithPagedInnerJoin, filterSql), + orderDirection, orderBy); - //var idsQuery = query == null ? sql : new SqlTranslator(sql, query).Translate(); + var users = ConvertFromDtos(Database.Fetch(new UserGroupRelator().Map, fullQuery)) + .ToArray(); // important so we don't iterate twice, if we don't do this we can end up with null values in cache if we were caching. - //// need to ensure the order by is in brackets, see: https://github.com/toptensoftware/PetaPoco/issues/177 - //idsQuery.OrderBy("(" + orderBy + ")"); - //var page = Database.Page(pageIndex + 1, pageSize, idsQuery); - //totalRecords = Convert.ToInt32(page.TotalItems); + return users; + } - //if (totalRecords == 0) - // return Enumerable.Empty(); - - //// now get the actual users and ensure they are ordered properly (same clause) - //var ids = page.Items.ToArray(); - //return ids.Length == 0 ? Enumerable.Empty() : GetAll(ids).OrderBy(orderBy.Compile()); + return Enumerable.Empty(); } - private Sql GetFilteredSqlForPagedResults(Sql sql, Func> defaultFilter = null) + private Sql GetFilteredSqlForPagedResults(Sql sql, Sql filterSql) { Sql filteredSql; // Apply filter - if (defaultFilter != null) + if (filterSql != null) { - var filterResult = defaultFilter(); + var sqlFilter = " WHERE " + filterSql.SQL.TrimStart("AND "); //NOTE: this is certainly strange - NPoco handles this much better but we need to re-create the sql // instance a couple of times to get the parameter order correct, for some reason the first @@ -483,9 +485,9 @@ namespace Umbraco.Core.Persistence.Repositories // accordingly - so we re-create it again. In v8 we don't need to do this and it's already taken care of. filteredSql = new Sql(sql.SQL, sql.Arguments); - var args = filteredSql.Arguments.Concat(filterResult.Item2).ToArray(); + var args = filteredSql.Arguments.Concat(filterSql.Arguments).ToArray(); filteredSql = new Sql( - string.Format("{0} {1}", filteredSql.SQL, filterResult.Item1), + string.Format("{0} {1}", filteredSql.SQL, sqlFilter), args); filteredSql = new Sql(filteredSql.SQL, args); } diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index d3bc3e9f82..01329ab6eb 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -505,6 +505,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs index fc2b870eef..0ad8fe1708 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs @@ -58,6 +58,38 @@ namespace Umbraco.Tests.Persistence.Repositories } } + [Test] + public void Can_Perform_Add_With_Group() + { + var group = MockedUserGroup.CreateUserGroup(); + + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + using (var repository = CreateUserGroupRepository(unitOfWork)) + { + repository.AddOrUpdate(group); + unitOfWork.Commit(); + } + + using (var repository = CreateRepository(unitOfWork)) + { + IUser user = MockedUser.CreateUser(); + user.AddGroup(group.Alias); + + // Act + repository.AddOrUpdate(user); + unitOfWork.Commit(); + + user = repository.Get(user.Id); + + // Assert + Assert.That(user.HasIdentity, Is.True); + Assert.AreEqual(1, user.Groups.Count()); + Assert.AreEqual(group.Alias, user.Groups.ElementAt(0)); + } + } + [Test] public void Can_Perform_Multiple_Adds_On_UserRepository() { diff --git a/src/Umbraco.Tests/Services/UserServiceTests.cs b/src/Umbraco.Tests/Services/UserServiceTests.cs index 02a4f335cc..682b3695d7 100644 --- a/src/Umbraco.Tests/Services/UserServiceTests.cs +++ b/src/Umbraco.Tests/Services/UserServiceTests.cs @@ -12,6 +12,7 @@ using Umbraco.Core.Services; using Umbraco.Tests.TestHelpers; using Umbraco.Tests.TestHelpers.Entities; using umbraco.BusinessLogic.Actions; +using Umbraco.Core.Persistence.DatabaseModelDefinitions; namespace Umbraco.Tests.Services { @@ -309,6 +310,72 @@ namespace Umbraco.Tests.Services Assert.AreEqual("test0", found.Last().Username); } + [Test] + public void Get_All_Paged_Users_With_Filter() + { + var users = MockedUser.CreateMulipleUsers(10).ToArray(); + ServiceContext.UserService.Save(users); + + long totalRecs; + var found = ServiceContext.UserService.GetAll(0, 2, out totalRecs, "username", Direction.Ascending, filter: "test"); + + Assert.AreEqual(2, found.Count()); + Assert.AreEqual(10, totalRecs); + Assert.AreEqual("test0", found.First().Username); + Assert.AreEqual("test1", found.Last().Username); + } + + [Test] + public void Get_All_Paged_Users_For_Group() + { + var userGroup = MockedUserGroup.CreateUserGroup(); + ServiceContext.UserService.Save(userGroup); + + var users = MockedUser.CreateMulipleUsers(10).ToArray(); + for (var i = 0; i < 10;) + { + users[i].AddGroup(userGroup.Alias); + i = i + 2; + } + ServiceContext.UserService.Save(users); + + long totalRecs; + var found = ServiceContext.UserService.GetAll(0, 2, out totalRecs, "username", Direction.Ascending, userGroups: new[] {userGroup.Alias}); + + Assert.AreEqual(2, found.Count()); + Assert.AreEqual(5, totalRecs); + Assert.AreEqual("test0", found.First().Username); + Assert.AreEqual("test2", found.Last().Username); + } + + [Test] + public void Get_All_Paged_Users_For_Group_With_Filter() + { + var userGroup = MockedUserGroup.CreateUserGroup(); + ServiceContext.UserService.Save(userGroup); + + var users = MockedUser.CreateMulipleUsers(10).ToArray(); + for (var i = 0; i < 10;) + { + users[i].AddGroup(userGroup.Alias); + i = i + 2; + } + for (var i = 0; i < 10;) + { + users[i].Name = "blah" + users[i].Name; + i = i + 3; + } + ServiceContext.UserService.Save(users); + + long totalRecs; + var found = ServiceContext.UserService.GetAll(0, 2, out totalRecs, "username", Direction.Ascending, userGroups: new[] { userGroup.Alias }, filter: "blah"); + + Assert.AreEqual(2, found.Count()); + Assert.AreEqual(2, totalRecs); + Assert.AreEqual("test0", found.First().Username); + Assert.AreEqual("test6", found.Last().Username); + } + [Test] public void Count_All_Users() {