From 612412683527dad1cdc3dbd4df61969b4695d7b0 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 20 Apr 2016 15:50:38 +0200 Subject: [PATCH] Fixes the custom field sorting, no longer has a sub query for order by, now supports both mysql and sqlce, adds a unit test (should add more though) --- .../Repositories/ContentRepository.cs | 1 + .../Repositories/VersionableRepositoryBase.cs | 48 +++++++++------- .../SqlSyntax/ISqlSyntaxProvider.cs | 3 +- .../SqlSyntax/MySqlSyntaxProvider.cs | 5 +- .../SqlSyntax/SqlCeSyntaxProvider.cs | 1 - .../SqlSyntax/SqlSyntaxProviderBase.cs | 7 +-- .../Persistence/UmbracoDatabase.cs | 19 +++++-- .../Repositories/ContentRepositoryTest.cs | 57 +++++++++++++++++-- 8 files changed, 101 insertions(+), 40 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 270b209553..5f96cad114 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -105,6 +105,7 @@ namespace Umbraco.Core.Persistence.Repositories #region Overrides of PetaPocoRepositoryBase + protected override Sql GetBaseQuery(bool isCount) { var sqlx = string.Format("LEFT OUTER JOIN {0} {1} ON ({1}.{2}={0}.{2} AND {1}.{3}=1)", diff --git a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs index 934555ac6e..4b2befb3e3 100644 --- a/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/VersionableRepositoryBase.cs @@ -248,6 +248,7 @@ namespace Umbraco.Core.Persistence.Repositories private Sql GetSortedSqlForPagedResults(Sql sql, Direction orderDirection, string orderBy, bool orderBySystemField) { + //copy to var so that the original isn't changed var sortedSql = new Sql(sql.SQL, sql.Arguments); @@ -273,35 +274,40 @@ 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 ( - SELECT CASE - 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); + var innerJoinTempTable = string.Format(@"INNER JOIN ( + SELECT CASE + WHEN dataInt Is Not Null THEN {0} + WHEN dataDecimal Is Not Null THEN {1} + WHEN dataDate Is Not Null THEN {2} + ELSE {3} + 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 = @{4} AND cd.newest = 1) AS CustomPropData + ON CustomPropData.CustomPropValContentId = umbracoNode.id + ", sortedInt, sortedDecimal, sortedDate, sortedString, sortedSql.Arguments.Length); - sortedSql.Append(orderBySql, orderBy); + //insert this just above the LEFT OUTER JOIN + var newSql = sortedSql.SQL.Insert(sortedSql.SQL.IndexOf("LEFT OUTER JOIN"), innerJoinTempTable); + var newArgs = sortedSql.Arguments.ToList(); + newArgs.Add(orderBy); + + sortedSql = new Sql(newSql, newArgs.ToArray()); + + sortedSql.OrderBy("CustomPropData.CustomPropVal"); if (orderDirection == Direction.Descending) { sortedSql.Append(" DESC"); - } + } } - + return sortedSql; + } /// diff --git a/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs b/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs index c70b6a571a..e61b23c8ec 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/ISqlSyntaxProvider.cs @@ -69,8 +69,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 d4585f5aa1..7167a5af95 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/MySqlSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/MySqlSyntaxProvider.cs @@ -360,10 +360,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 1c6e1bba52..23e0a1bb87 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/SqlCeSyntaxProvider.cs @@ -208,7 +208,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 b7b58d929f..50417761aa 100644 --- a/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs +++ b/src/Umbraco.Core/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs @@ -537,9 +537,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 8fc1150409..a0490348f3 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 StackExchange.Profiling; using Umbraco.Core.Logging; @@ -128,15 +129,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); } public override void OnExecutedCommand(IDbCommand 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 2cdc40d6f6..d29b073df7 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; @@ -543,6 +544,41 @@ namespace Umbraco.Tests.Persistence.Repositories } } + [Test] + public void Can_Perform_GetPagedResultsByQuery_Sorting_On_Custom_Property() + { + // Arrange + var provider = new PetaPocoUnitOfWorkProvider(Logger); + var unitOfWork = provider.GetUnitOfWork(); + ContentTypeRepository contentTypeRepository; + using (var repository = CreateRepository(unitOfWork, out contentTypeRepository)) + { + // Act + var query = Query.Builder.Where(x => x.Name.Contains("Text")); + long totalRecords; + + try + { + 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 + { + DatabaseContext.Database.EnableSqlTrace = false; + DatabaseContext.Database.DisableSqlCount(); + } + } + } + [Test] public void Can_Perform_GetPagedResultsByQuery_ForFirstPage_On_ContentRepository() { @@ -555,12 +591,23 @@ namespace Umbraco.Tests.Persistence.Repositories // Act var query = Query.Builder.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(); + } } }