From f149ca9c763135f3a7aab1e4b6dc2d56d5ff4455 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 20 Apr 2016 12:50:52 +0200 Subject: [PATCH] Fixes SqlCe and MySQL sorting on custom properties for list view, this also makes the query work much better and there is no longer a requirement for a sub-query in the order by statement. --- .../Repositories/VersionableRepositoryBase.cs | 43 ++++++------- .../SqlSyntax/ISqlSyntaxProvider.cs | 3 +- .../SqlSyntax/MySqlSyntaxProvider.cs | 5 +- .../SqlSyntax/SqlCeSyntaxProvider.cs | 1 - .../SqlSyntax/SqlSyntaxProviderBase.cs | 7 +-- .../Persistence/UmbracoDatabase.cs | 19 ++++-- .../Repositories/ContentRepositoryTest.cs | 60 +++++++++++++++++-- 7 files changed, 99 insertions(+), 39 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs index b32336e9b2..62d7259065 100644 --- a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs @@ -303,34 +303,37 @@ namespace Umbraco.Core.Persistence.Repositories // from most recent content version for the given order by field var sortedInt = string.Format(SqlSyntax.ConvertIntegerToOrderableString, "dataInt"); var sortedDate = string.Format(SqlSyntax.ConvertDateToOrderableString, "dataDate"); - var sortedString = string.Format(SqlSyntax.IsNull, "dataNvarchar", "''"); + var sortedString = string.Format("COALESCE({0},'')", "dataNvarchar"); var sortedDecimal = string.Format(SqlSyntax.ConvertDecimalToOrderableString, "dataDecimal"); - var orderBySql = string.Format(@"ORDER BY ( + var innerJoinTempTable = string.Format(@"INNER JOIN ( SELECT CASE - WHEN dataInt Is Not Null THEN {0} + WHEN dataInt Is Not Null THEN {0} WHEN dataDecimal Is Not Null THEN {1} WHEN dataDate Is Not Null THEN {2} ELSE {3} - END - FROM cmsContent c - INNER JOIN cmsContentVersion cv ON cv.ContentId = c.nodeId AND VersionDate = ( - SELECT Max(VersionDate) - FROM cmsContentVersion - WHERE ContentId = c.nodeId - ) - INNER JOIN cmsPropertyData cpd ON cpd.contentNodeId = c.nodeId - AND cpd.versionId = cv.VersionId - INNER JOIN cmsPropertyType cpt ON cpt.Id = cpd.propertytypeId - WHERE c.nodeId = umbracoNode.Id and cpt.Alias = @0)", sortedInt, sortedDecimal, sortedDate, sortedString); + END AS CustomPropVal, + cd.nodeId AS CustomPropValContentId + FROM cmsDocument cd + INNER JOIN cmsPropertyData cpd ON cpd.contentNodeId = cd.nodeId AND cpd.versionId = cd.versionId + INNER JOIN cmsPropertyType cpt ON cpt.Id = cpd.propertytypeId + WHERE cpt.Alias = @2 AND cd.newest = 1) AS CustomPropData + ON CustomPropData.CustomPropValContentId = umbracoNode.id +", sortedInt, sortedDecimal, sortedDate, sortedString); - throw new NotImplementedException("FIX ME!"); + //insert this just above the LEFT OUTER JOIN + var newSql = psql.SQL.Insert(psql.SQL.IndexOf("LEFT OUTER JOIN"), innerJoinTempTable); + var newArgs = psql.Arguments.ToList(); + newArgs.Add(orderBy); - //sortedSql.Append(orderBySql, orderBy); - //if (orderDirection == Direction.Descending) - //{ - // sortedSql.Append(" DESC"); - //} + psql = new Sql(psql.SqlContext, newSql, newArgs.ToArray()); + + psql.OrderBy("CustomPropData.CustomPropVal"); + //psql.Append(innerJoinTempTable, orderBy); + if (orderDirection == Direction.Descending) + { + psql.Append(" DESC"); + } } } diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs b/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs index 33197dced8..fc26bcd7c9 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs @@ -70,8 +70,7 @@ namespace Umbraco.Core.Persistence.SqlSyntax bool SupportsClustered(); bool SupportsIdentityInsert(); bool? SupportsCaseInsensitiveQueries(Database db); - - string IsNull { get; } + string ConvertIntegerToOrderableString { get; } string ConvertDateToOrderableString { get; } string ConvertDecimalToOrderableString { get; } diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/MySqlSyntaxProvider.cs b/src/Umbraco.Core/Persistence/SqlSyntax/MySqlSyntaxProvider.cs index f9a193d731..462f601323 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/MySqlSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/MySqlSyntaxProvider.cs @@ -361,10 +361,9 @@ ORDER BY TABLE_NAME, INDEX_NAME", public override string DropIndex { get { return "DROP INDEX {0} ON {1}"; } } public override string RenameColumn { get { return "ALTER TABLE {0} CHANGE {1} {2}"; } } - public override string IsNull { get { return "IFNULL({0},{1})"; } } - public override string ConvertIntegerToOrderableString { get { return "LPAD({0}, 8, '0')"; } } + public override string ConvertIntegerToOrderableString { get { return "LPAD(FORMAT({0}, 0), 8, '0')"; } } public override string ConvertDateToOrderableString { get { return "DATE_FORMAT({0}, '%Y%m%d')"; } } - public override string ConvertDecimalToOrderableString { get { return "LPAD({0}, 25, '0')"; } } + public override string ConvertDecimalToOrderableString { get { return "LPAD(FORMAT({0}, 9), 20, '0')"; } } public override bool? SupportsCaseInsensitiveQueries(Database db) { diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs b/src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs index 41b88abec9..4a3eb2e4a8 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs @@ -209,7 +209,6 @@ ORDER BY TABLE_NAME, INDEX_NAME"); public override string DropIndex { get { return "DROP INDEX {1}.{0}"; } } - } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs b/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs index 36aad20b29..a992cdd231 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs @@ -538,9 +538,8 @@ namespace Umbraco.Core.Persistence.SqlSyntax public virtual string DeleteConstraint { get { return "ALTER TABLE {0} DROP CONSTRAINT {1}"; } } public virtual string CreateForeignKeyConstraint { get { return "ALTER TABLE {0} ADD CONSTRAINT {1} FOREIGN KEY ({2}) REFERENCES {3} ({4}){5}{6}"; } } - public virtual string IsNull { get { return "ISNULL({0},{1})"; } } - public virtual string ConvertIntegerToOrderableString { get { return "RIGHT('00000000' + CAST({0} AS varchar(8)),8)"; } } - public virtual string ConvertDateToOrderableString { get { return "CONVERT(varchar, {0}, 102)"; } } - public virtual string ConvertDecimalToOrderableString { get { return "RIGHT('0000000000000000000000000' + CAST({0} AS varchar(25)),25)"; } } + public virtual string ConvertIntegerToOrderableString { get { return "REPLACE(STR({0}, 8), SPACE(1), '0')"; } } + public virtual string ConvertDateToOrderableString { get { return "CONVERT(nvarchar, {0}, 102)"; } } + public virtual string ConvertDecimalToOrderableString { get { return "REPLACE(STR({0}, 20, 9), SPACE(1), '0')"; } } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/UmbracoDatabase.cs b/src/Umbraco.Core/Persistence/UmbracoDatabase.cs index eaf637df4b..451fbf36d7 100644 --- a/src/Umbraco.Core/Persistence/UmbracoDatabase.cs +++ b/src/Umbraco.Core/Persistence/UmbracoDatabase.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Data; using System.Data.Common; using System.Linq; +using System.Text; using NPoco; using StackExchange.Profiling; using Umbraco.Core.Logging; @@ -115,15 +116,25 @@ namespace Umbraco.Core.Persistence // if no timeout is specified, and the connection has a longer timeout, use it if (OneTimeCommandTimeout == 0 && CommandTimeout == 0 && cmd.Connection.ConnectionTimeout > 30) cmd.CommandTimeout = cmd.Connection.ConnectionTimeout; + + if (EnableSqlTrace) + { + var sb = new StringBuilder(); + sb.Append(cmd.CommandText); + foreach (DbParameter p in cmd.Parameters) + { + sb.Append(" - "); + sb.Append(p.Value); + } + + _logger.Debug(sb.ToString()); + } + base.OnExecutingCommand(cmd); } protected override void OnExecutedCommand(DbCommand cmd) { - if (EnableSqlTrace) - { - _logger.Debug(cmd.CommandText); - } if (_enableCount) { SqlCount++; diff --git a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs index 65c5187120..a6a26c203d 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/ContentRepositoryTest.cs @@ -1,6 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Text.RegularExpressions; using System.Xml.Linq; using Moq; using NUnit.Framework; @@ -556,6 +557,44 @@ namespace Umbraco.Tests.Persistence.Repositories Assert.AreEqual("[alias2]", matches[1].Groups[2].Value); } + [Test] + public void Can_Perform_GetPagedResultsByQuery_Sorting_On_Custom_Property() + { + // Arrange + var provider = new NPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = repository.Query.Where(x => x.Name.Contains("Text")); + long totalRecords; + + //var origRegex = NPoco.PagingHelper.rxOrderBy; + try + { + //NPoco.PagingHelper.rxOrderBy = new Regex("(?!.*(?:\\s+FROM[\\s\\(]+))ORDER\\s+BY\\s+([\\w\\.\\[\\]\\(\\) \"`,'@\\s\\+\\=]+)(?!.*\\))", RegexOptions.IgnoreCase | RegexOptions.Compiled); + DatabaseContext.Database.EnableSqlTrace = true; + DatabaseContext.Database.EnableSqlCount(); + + var result = repository.GetPagedResultsByQuery(query, 0, 2, out totalRecords, "title", Direction.Ascending, false); + + Assert.AreEqual(3, totalRecords); + Assert.AreEqual(2, result.Count()); + + result = repository.GetPagedResultsByQuery(query, 1, 2, out totalRecords, "title", Direction.Ascending, false); + + Assert.AreEqual(1, result.Count()); + } + finally + { + //NPoco.PagingHelper.rxOrderBy = origRegex; + DatabaseContext.Database.EnableSqlTrace = false; + DatabaseContext.Database.DisableSqlCount(); + } + } + } + [Test] public void Can_Perform_GetPagedResultsByQuery_ForFirstPage_On_ContentRepository() { @@ -568,12 +607,23 @@ namespace Umbraco.Tests.Persistence.Repositories // Act var query = repository.Query.Where(x => x.Level == 2); long totalRecords; - var result = repository.GetPagedResultsByQuery(query, 0, 1, out totalRecords, "Name", Direction.Ascending, true); - // Assert - Assert.That(totalRecords, Is.GreaterThanOrEqualTo(2)); - Assert.That(result.Count(), Is.EqualTo(1)); - Assert.That(result.First().Name, Is.EqualTo("Text Page 1")); + try + { + DatabaseContext.Database.EnableSqlTrace = true; + DatabaseContext.Database.EnableSqlCount(); + var result = repository.GetPagedResultsByQuery(query, 0, 1, out totalRecords, "Name", Direction.Ascending, true); + + // Assert + Assert.That(totalRecords, Is.GreaterThanOrEqualTo(2)); + Assert.That(result.Count(), Is.EqualTo(1)); + Assert.That(result.First().Name, Is.EqualTo("Text Page 1")); + } + finally + { + DatabaseContext.Database.EnableSqlTrace = false; + DatabaseContext.Database.DisableSqlCount(); + } } }