yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #2866 from umbraco/temp-U4-11458-2

Browse Source 
Fixes SQL generation for paged results for both UserRepository and AuditRepository
This commit is contained in:
Sebastiaan Janssen
2018-09-25 14:27:27 +02:00
committed by GitHub
parent f9973f602c efbcce5261
commit b9f5ddab06
5 changed files with 214 additions and 78 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
src/Umbraco.Core/Persistence/Repositories/AuditRepository.cs
View File

@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text.RegularExpressions;
using Umbraco.Core.Logging;
using Umbraco.Core.Models;
using Umbraco.Core.Models.Membership;
@@ -47,43 +48,32 @@ namespace Umbraco.Core.Persistence.Repositories
var sql = GetBaseQuery(false);
if (query == null) query = new Query<IAuditItem>();
var queryHasWhereClause = query.GetWhereClauses().Any();
var translatorIds = new SqlTranslator<IAuditItem>(sql, query);
var translatedQuery = translatorIds.Translate();
var customFilterWheres = customFilter != null ? customFilter.GetWhereClauses().ToArray() : null;
var customFilterWheres = customFilter?.GetWhereClauses().ToArray();
var hasCustomFilter = customFilterWheres != null && customFilterWheres.Length > 0;
if (hasCustomFilter)
{
var filterSql = new Sql();
var first = true;
foreach (var filterClaus in customFilterWheres)
foreach (var filterClause in customFilterWheres)
{
if (first == false)
{
filterSql.Append(" AND ");
}
filterSql.Append(string.Format("({0})", filterClaus.Item1), filterClaus.Item2);
first = false;
filterSql.Append($"AND ({filterClause.Item1})", filterClause.Item2);
}
translatedQuery = GetFilteredSqlForPagedResults(translatedQuery, filterSql);
translatedQuery = GetFilteredSqlForPagedResults(translatedQuery, filterSql, queryHasWhereClause);
}
if (auditTypeFilter.Length > 0)
{
var filterSql = new Sql();
var first = true;
foreach (var filterClaus in auditTypeFilter)
foreach (var filterClause in auditTypeFilter)
{
if (first == false || hasCustomFilter)
{
filterSql.Append(" AND ");
}
filterSql.Append("(logHeader = @logHeader)", new {logHeader = filterClaus.ToString() });
first = false;
filterSql.Append("AND (logHeader = @logHeader)", new { logHeader = filterClause.ToString() });
}
translatedQuery = GetFilteredSqlForPagedResults(translatedQuery, filterSql);
translatedQuery = GetFilteredSqlForPagedResults(translatedQuery, filterSql, queryHasWhereClause || hasCustomFilter);
}
if (orderDirection == Direction.Descending)
@@ -99,7 +89,7 @@ namespace Umbraco.Core.Persistence.Repositories
dto => new AuditItem(dto.Id, dto.Comment, Enum<AuditType>.ParseOrNull(dto.Header) ?? AuditType.Custom, dto.UserId)).ToArray();
//Mapping the DateStamp
for (int i = 0; i < pages.Length; i++)
for (var i = 0; i < pages.Length; i++)
{
pages[i].CreateDate = pagedResult.Items[i].Datestamp;
}
@@ -169,14 +159,17 @@ namespace Umbraco.Core.Persistence.Repositories
}
#endregion
private Sql GetFilteredSqlForPagedResults(Sql sql, Sql filterSql)
private Sql GetFilteredSqlForPagedResults(Sql sql, Sql filterSql, bool hasWhereClause)
{
Sql filteredSql;
// Apply filter
if (filterSql != null)
{
var sqlFilter = " WHERE " + filterSql.SQL.TrimStart("AND ");
//ensure we don't append a WHERE if there is already one
var sqlFilter = hasWhereClause
? filterSql.SQL
: " 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

91
src/Umbraco.Core/Persistence/Repositories/UserRepository.cs
View File

@@ -4,6 +4,7 @@ using System.ComponentModel;
using System.Linq;
using System.Linq.Expressions;
using System.Text;
using System.Text.RegularExpressions;
using System.Web.Security;
using Newtonsoft.Json;
using Umbraco.Core.Configuration;
@@ -710,10 +711,10 @@ ORDER BY colName";
private IEnumerable<IUser> GetPagedResultsByQuery(IQuery<IUser> query, long pageIndex, int pageSize, out long totalRecords, string orderBy, Direction orderDirection,
string[] includeUserGroups = null,
string[] excludeUserGroups = null,
UserState[] userState = null,
private IEnumerable<IUser> GetPagedResultsByQuery(IQuery<IUser> query, long pageIndex, int pageSize, out long totalRecords, string orderBy, Direction orderDirection,
string[] includeUserGroups = null,
string[] excludeUserGroups = null,
UserState[] userState = null,
IQuery<IUser> customFilter = null)
{
if (string.IsNullOrWhiteSpace(orderBy)) throw new ArgumentException("Value cannot be null or whitespace.", "orderBy");
@@ -721,23 +722,23 @@ ORDER BY colName";
Sql filterSql = null;
var customFilterWheres = customFilter != null ? customFilter.GetWhereClauses().ToArray() : null;
var hasCustomFilter = customFilterWheres != null && customFilterWheres.Length > 0;
var hasCustomFilter = customFilterWheres != null && customFilterWheres.Length > 0;
if (hasCustomFilter
|| (includeUserGroups != null && includeUserGroups.Length > 0) || (excludeUserGroups != null && excludeUserGroups.Length > 0)
|| (userState != null && userState.Length > 0 && userState.Contains(UserState.All) == false))
filterSql = new Sql();
|| (includeUserGroups != null && includeUserGroups.Length > 0) || (excludeUserGroups != null && excludeUserGroups.Length > 0)
|| (userState != null && userState.Length > 0 && userState.Contains(UserState.All) == false))
filterSql = new Sql();
if (hasCustomFilter)
{
foreach (var filterClause in customFilterWheres)
{
filterSql.Append(string.Format("AND ({0})", filterClause.Item1), filterClause.Item2);
filterSql.Append($"AND ({filterClause.Item1})", filterClause.Item2);
}
}
}
if (includeUserGroups != null && includeUserGroups.Length > 0)
{
var subQuery = @"AND (umbracoUser.id IN (SELECT DISTINCT umbracoUser.id
const string 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
@@ -747,21 +748,21 @@ ORDER BY colName";
if (excludeUserGroups != null && excludeUserGroups.Length > 0)
{
var subQuery = @"AND (umbracoUser.id NOT IN (SELECT DISTINCT umbracoUser.id
const string subQuery = @"AND (umbracoUser.id NOT 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 = excludeUserGroups });
}
}
if (userState != null && userState.Length > 0)
{
{
//the "ALL" state doesn't require any filtering so we ignore that, if it exists in the list we don't do any filtering
if (userState.Contains(UserState.All) == false)
{
var sb = new StringBuilder("(");
var appended = false;
var appended = false;
if (userState.Contains(UserState.Active))
{
@@ -797,67 +798,81 @@ ORDER BY colName";
filterSql.Append("AND " + sb);
}
}
// Get base query for returning IDs
var sqlBaseIds = GetBaseQuery("id");
}
// Get base query for returning IDs
var sqlBaseIds = GetBaseQuery("id");
if (query == null) query = new Query<IUser>();
var queryHasWhereClause = query.GetWhereClauses().Any();
var translatorIds = new SqlTranslator<IUser>(sqlBaseIds, query);
var sqlQueryIds = translatorIds.Translate();
//get sorted and filtered sql
var sqlQueryIds = translatorIds.Translate();
var sqlBaseFull = GetBaseQuery("umbracoUser.*, umbracoUserGroup.*, umbracoUserGroup2App.*, umbracoUserStartNode.*");
var translatorFull = new SqlTranslator<IUser>(sqlBaseFull, query);
//get sorted and filtered sql
var sqlNodeIdsWithSort = GetSortedSqlForPagedResults(
GetFilteredSqlForPagedResults(sqlQueryIds, filterSql),
GetFilteredSqlForPagedResults(sqlQueryIds, filterSql, queryHasWhereClause),
orderDirection, orderBy);
// Get page of results and total count
var pagedResult = Database.Page<UserDto>(pageIndex + 1, pageSize, sqlNodeIdsWithSort);
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.
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<UserDto>(pageIndex * pageSize, pageSize, sqlNodeIdsWithSort.SQL, ref args, out sqlStringCount, out sqlStringPage);
var sqlQueryFull = translatorFull.Translate();
var sqlQueryFull = GetBaseQuery("umbracoUser.*, umbracoUserGroup.*, umbracoUserGroup2App.*, umbracoUserStartNode.*");
var fullQueryWithPagedInnerJoin = sqlQueryFull
.Append("INNER JOIN (")
//join the paged query with the paged query arguments
//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("ON umbracoUser.id = temp.id");
AddGroupLeftJoin(fullQueryWithPagedInnerJoin);
if (splitQuery.Length > 1)
{
//add the original where clause back with the original arguments
fullQueryWithPagedInnerJoin.Where(splitQuery[1], sqlQueryIds.Arguments);
}
//get sorted and filtered sql
var fullQuery = GetSortedSqlForPagedResults(
GetFilteredSqlForPagedResults(fullQueryWithPagedInnerJoin, filterSql),
GetFilteredSqlForPagedResults(fullQueryWithPagedInnerJoin, filterSql, queryHasWhereClause),
orderDirection, orderBy);
var users = ConvertFromDtos(Database.Fetch<UserDto, UserGroupDto, UserGroup2AppDto, UserStartNodeDto, UserDto>(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.
return users;
}
}
return Enumerable.Empty<IUser>();
}
private Sql GetFilteredSqlForPagedResults(Sql sql, Sql filterSql)
private Sql GetFilteredSqlForPagedResults(Sql sql, Sql filterSql, bool hasWhereClause)
{
Sql filteredSql;
// Apply filter
if (filterSql != null)
{
var sqlFilter = " WHERE " + filterSql.SQL.TrimStart("AND ");
//ensure we don't append a WHERE if there is already one
var sqlFilter = hasWhereClause
? filterSql.SQL
: " 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

5
src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs
View File

@@ -290,6 +290,9 @@ namespace Umbraco.Core.Persistence.Repositories
// Apply filter
if (defaultFilter != null)
{
//NOTE: It is assumed here that the `sql` already contains a WHERE clause, see UserRepository.GetFilteredSqlForPagedResults
// for an example of when it's not assumed there's already a WHERE clause
var filterResult = defaultFilter();
//NOTE: this is certainly strange - NPoco handles this much better but we need to re-create the sql
@@ -993,4 +996,4 @@ ORDER BY contentNodeId, versionId, propertytypeid
}
}
}
}
}

90
src/Umbraco.Tests/Persistence/Repositories/AuditRepositoryTest.cs
View File

@@ -66,6 +66,46 @@ namespace Umbraco.Tests.Persistence.Repositories
}
}
[Test]
public void Get_Paged_Items_By_User_Id_With_Query_And_Filter()
{
var provider = new PetaPocoUnitOfWorkProvider(Logger);
var unitOfWork = provider.GetUnitOfWork();
using (var repo = new AuditRepository(unitOfWork, CacheHelper, Logger, SqlSyntax))
{
for (int i = 0; i < 100; i++)
{
repo.AddOrUpdate(new AuditItem(i, string.Format("Content {0} created", i), AuditType.New, 0));
repo.AddOrUpdate(new AuditItem(i, string.Format("Content {0} published", i), AuditType.Publish, 0));
}
unitOfWork.Commit();
}
using (var repo = new AuditRepository(unitOfWork, CacheHelper, Logger, SqlSyntax))
{
var query = Query<IAuditItem>.Builder.Where(x => x.UserId == 0);
try
{
DatabaseContext.Database.EnableSqlTrace = true;
DatabaseContext.Database.EnableSqlCount();
var page = repo.GetPagedResultsByQuery(query, 0, 10, out var total, Direction.Descending,
new[] { AuditType.Publish },
Query<IAuditItem>.Builder.Where(x => x.UserId > -1));
Assert.AreEqual(10, page.Count());
Assert.AreEqual(100, total);
}
finally
{
DatabaseContext.Database.EnableSqlTrace = false;
DatabaseContext.Database.DisableSqlCount();
}
}
}
[Test]
public void Get_Paged_Items_With_AuditType_Filter()
{
@@ -83,14 +123,24 @@ namespace Umbraco.Tests.Persistence.Repositories
using (var repo = new AuditRepository(unitOfWork, CacheHelper, Logger, SqlSyntax))
{
long total;
var page = repo.GetPagedResultsByQuery(Query<IAuditItem>.Builder, 0, 9, out total, Direction.Descending,
new[] {AuditType.Publish}, null)
.ToArray();
try
{
DatabaseContext.Database.EnableSqlTrace = true;
DatabaseContext.Database.EnableSqlCount();
Assert.AreEqual(9, page.Length);
Assert.IsTrue(page.All(x => x.AuditType == AuditType.Publish));
Assert.AreEqual(100, total);
var page = repo.GetPagedResultsByQuery(Query<IAuditItem>.Builder, 0, 9, out var total, Direction.Descending,
new[] { AuditType.Publish }, null)
.ToArray();
Assert.AreEqual(9, page.Length);
Assert.IsTrue(page.All(x => x.AuditType == AuditType.Publish));
Assert.AreEqual(100, total);
}
finally
{
DatabaseContext.Database.EnableSqlTrace = false;
DatabaseContext.Database.DisableSqlCount();
}
}
}
@@ -111,15 +161,25 @@ namespace Umbraco.Tests.Persistence.Repositories
using (var repo = new AuditRepository(unitOfWork, CacheHelper, Logger, SqlSyntax))
{
long total;
var page = repo.GetPagedResultsByQuery(Query<IAuditItem>.Builder, 0, 8, out total, Direction.Descending,
null, Query<IAuditItem>.Builder.Where(item => item.Comment == "Content created"))
.ToArray();
try
{
DatabaseContext.Database.EnableSqlTrace = true;
DatabaseContext.Database.EnableSqlCount();
Assert.AreEqual(8, page.Length);
Assert.IsTrue(page.All(x => x.Comment == "Content created"));
Assert.AreEqual(100, total);
var page = repo.GetPagedResultsByQuery(Query<IAuditItem>.Builder, 0, 8, out var total, Direction.Descending,
null, Query<IAuditItem>.Builder.Where(item => item.Comment == "Content created"))
.ToArray();
Assert.AreEqual(8, page.Length);
Assert.IsTrue(page.All(x => x.Comment == "Content created"));
Assert.AreEqual(100, total);
}
finally
{
DatabaseContext.Database.EnableSqlTrace = false;
DatabaseContext.Database.DisableSqlCount();
}
}
}
}
}
}

69
src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs
View File

@@ -6,6 +6,7 @@ using Umbraco.Core.Configuration.UmbracoSettings;
using Umbraco.Core.IO;
using Umbraco.Core.Logging;
using Umbraco.Core.Models.Membership;
using Umbraco.Core.Persistence.DatabaseModelDefinitions;
using Umbraco.Core.Persistence.Querying;
using Umbraco.Core.Persistence.Repositories;
using Umbraco.Core.Persistence.UnitOfWork;
@@ -392,7 +393,71 @@ namespace Umbraco.Tests.Persistence.Repositories
var result = repository.Count(query);
// Assert
Assert.That(result, Is.GreaterThanOrEqualTo(2));
Assert.AreEqual(2, result);
}
}
[Test]
public void Can_Get_Paged_Results_By_Query_And_Filter_And_Groups()
{
// Arrange
var provider = new PetaPocoUnitOfWorkProvider(Logger);
var unitOfWork = provider.GetUnitOfWork();
using (var repository = CreateRepository(unitOfWork))
{
var users = CreateAndCommitMultipleUsers(repository, unitOfWork);
var query = Query<IUser>.Builder.Where(x => x.Username == "TestUser1" || x.Username == "TestUser2");
try
{
DatabaseContext.Database.EnableSqlTrace = true;
DatabaseContext.Database.EnableSqlCount();
// Act
var result = repository.GetPagedResultsByQuery(query, 0, 10, out var totalRecs, user => user.Id, Direction.Ascending,
excludeUserGroups: new[] { Constants.Security.TranslatorGroupAlias },
filter: Query<IUser>.Builder.Where(x => x.Id > -1));
// Assert
Assert.AreEqual(2, totalRecs);
}
finally
{
DatabaseContext.Database.EnableSqlTrace = false;
DatabaseContext.Database.DisableSqlCount();
}
}
}
[Test]
public void Can_Get_Paged_Results_With_Filter_And_Groups()
{
// Arrange
var provider = new PetaPocoUnitOfWorkProvider(Logger);
var unitOfWork = provider.GetUnitOfWork();
using (var repository = CreateRepository(unitOfWork))
{
var users = CreateAndCommitMultipleUsers(repository, unitOfWork);
try
{
DatabaseContext.Database.EnableSqlTrace = true;
DatabaseContext.Database.EnableSqlCount();
// Act
var result = repository.GetPagedResultsByQuery(null, 0, 10, out var totalRecs, user => user.Id, Direction.Ascending,
includeUserGroups: new[] { Constants.Security.AdminGroupAlias, Constants.Security.SensitiveDataGroupAlias },
excludeUserGroups: new[] { Constants.Security.TranslatorGroupAlias },
filter: Query<IUser>.Builder.Where(x => x.Id == 0));
// Assert
Assert.AreEqual(1, totalRecs);
}
finally
{
DatabaseContext.Database.EnableSqlTrace = false;
DatabaseContext.Database.DisableSqlCount();
}
}
}
@@ -439,4 +504,4 @@ namespace Umbraco.Tests.Persistence.Repositories
return new IUser[] { user1, user2, user3 };
}
}
}
}