From 95af144ae9517bb3389406b660487e732cde2c67 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 24 Oct 2016 13:08:35 +0200 Subject: [PATCH 01/18] Migrate locks to their own umbracoLocks table --- src/Umbraco.Core/Constants-System.cs | 3 -- src/Umbraco.Core/Models/Rdbms/LockDto.cs | 29 ++++++++++++++ .../Persistence/Constants-Locks.cs | 11 ++++++ .../Persistence/DatabasenodeLockExtensions.cs | 4 +- .../Migrations/Initial/BaseDataCreation.cs | 12 +++++- .../Initial/DatabaseSchemaCreation.cs | 3 +- .../AddLockObjects.cs | 39 +++++++++++++++++++ .../AddLockTable.cs | 32 +++++++++++++++ .../EnsureServersLockObject.cs | 2 +- .../AddServerRegistrationColumnsAndLock.cs | 2 +- .../Services/ServerRegistrationService.cs | 2 +- src/Umbraco.Core/Umbraco.Core.csproj | 4 ++ 12 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 src/Umbraco.Core/Models/Rdbms/LockDto.cs create mode 100644 src/Umbraco.Core/Persistence/Constants-Locks.cs create mode 100644 src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs create mode 100644 src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockTable.cs diff --git a/src/Umbraco.Core/Constants-System.cs b/src/Umbraco.Core/Constants-System.cs index 4a30db9cd8..bc86d1717f 100644 --- a/src/Umbraco.Core/Constants-System.cs +++ b/src/Umbraco.Core/Constants-System.cs @@ -25,9 +25,6 @@ public const int DefaultContentListViewDataTypeId = -95; public const int DefaultMediaListViewDataTypeId = -96; public const int DefaultMembersListViewDataTypeId = -97; - - // identifiers for lock objects - public const int ServersLock = -331; } public static class DatabaseProviders diff --git a/src/Umbraco.Core/Models/Rdbms/LockDto.cs b/src/Umbraco.Core/Models/Rdbms/LockDto.cs new file mode 100644 index 0000000000..b543ce4241 --- /dev/null +++ b/src/Umbraco.Core/Models/Rdbms/LockDto.cs @@ -0,0 +1,29 @@ +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.DatabaseAnnotations; + +namespace Umbraco.Core.Models.Rdbms +{ + [TableName("umbracoLock")] + [PrimaryKey("id")] + [ExplicitColumns] + internal class LockDto + { + public LockDto() + { + Value = 1; + } + + [Column("id")] + [PrimaryKeyColumn(Name = "PK_umbracoLock")] + public int Id { get; set; } + + [Column("value")] + [NullSetting(NullSetting = NullSettings.NotNull)] + public int Value { get; set; } + + [Column("name")] + [NullSetting(NullSetting = NullSettings.NotNull)] + [Length(64)] + public string Name { get; set; } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Constants-Locks.cs b/src/Umbraco.Core/Persistence/Constants-Locks.cs new file mode 100644 index 0000000000..49c6f933fb --- /dev/null +++ b/src/Umbraco.Core/Persistence/Constants-Locks.cs @@ -0,0 +1,11 @@ +// ReSharper disable once CheckNamespace +namespace Umbraco.Core +{ + static partial class Constants + { + public static class Locks + { + public const int Servers = -331; + } + } +} diff --git a/src/Umbraco.Core/Persistence/DatabasenodeLockExtensions.cs b/src/Umbraco.Core/Persistence/DatabasenodeLockExtensions.cs index 3e6d245416..5ef29aa951 100644 --- a/src/Umbraco.Core/Persistence/DatabasenodeLockExtensions.cs +++ b/src/Umbraco.Core/Persistence/DatabasenodeLockExtensions.cs @@ -24,7 +24,7 @@ namespace Umbraco.Core.Persistence { ValidateDatabase(database); - database.Execute("UPDATE umbracoNode SET sortOrder = (CASE WHEN (sortOrder=1) THEN -1 ELSE 1 END) WHERE id=@id", + database.Execute("UPDATE umbracoLock SET value = (CASE WHEN (value=1) THEN -1 ELSE 1 END) WHERE id=@id", new { @id = nodeId }); } @@ -36,7 +36,7 @@ namespace Umbraco.Core.Persistence { ValidateDatabase(database); - database.ExecuteScalar("SELECT sortOrder FROM umbracoNode WHERE id=@id", + database.ExecuteScalar("SELECT value FROM umbracoLock WHERE id=@id", new { @id = nodeId }); } } diff --git a/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs b/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs index 9570024b09..60fd37a357 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Initial/BaseDataCreation.cs @@ -33,7 +33,12 @@ namespace Umbraco.Core.Persistence.Migrations.Initial CreateUmbracNodeData(); } - if(tableName.Equals("cmsContentType")) + if (tableName.Equals("umbracoLock")) + { + CreateUmbracoLockData(); + } + + if (tableName.Equals("cmsContentType")) { CreateCmsContentTypeData(); } @@ -141,9 +146,12 @@ namespace Umbraco.Core.Persistence.Migrations.Initial //_database.Insert("umbracoNode", "id", false, new NodeDto { NodeId = 1038, Trashed = false, ParentId = -1, UserId = 0, Level = 1, Path = "-1,1038", SortOrder = 2, UniqueId = new Guid("1251c96c-185c-4e9b-93f4-b48205573cbd"), Text = "Simple Editor", NodeObjectType = new Guid(Constants.ObjectTypes.DataType), CreateDate = DateTime.Now }); //_database.Insert("umbracoNode", "id", false, new NodeDto { NodeId = 1042, Trashed = false, ParentId = -1, UserId = 0, Level = 1, Path = "-1,1042", SortOrder = 2, UniqueId = new Guid("0a452bd5-83f9-4bc3-8403-1286e13fb77e"), Text = "Macro Container", NodeObjectType = new Guid(Constants.ObjectTypes.DataType), CreateDate = DateTime.Now }); + } + private void CreateUmbracoLockData() + { // all lock objects - _database.Insert("umbracoNode", "id", false, new NodeDto { NodeId = Constants.System.ServersLock, Trashed = false, ParentId = -1, UserId = 0, Level = 1, Path = "-1," + Constants.System.ServersLock, SortOrder = 1, UniqueId = new Guid("0AF5E610-A310-4B6F-925F-E928D5416AF7"), Text = "LOCK: Servers", NodeObjectType = Constants.ObjectTypes.LockObjectGuid, CreateDate = DateTime.Now }); + _database.Insert("umbracoLock", "id", false, new LockDto { Id = Constants.Locks.Servers, Name = "Servers" }); } private void CreateCmsContentTypeData() diff --git a/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs b/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs index 423c847c47..edac819474 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Initial/DatabaseSchemaCreation.cs @@ -84,7 +84,8 @@ namespace Umbraco.Core.Persistence.Migrations.Initial {45, typeof (MigrationDto)}, {46, typeof (UmbracoDeployChecksumDto)}, {47, typeof (UmbracoDeployDependencyDto)}, - {48, typeof (RedirectUrlDto) } + {48, typeof (RedirectUrlDto) }, + {49, typeof (LockDto) } }; #endregion diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs new file mode 100644 index 0000000000..d5c377fba1 --- /dev/null +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs @@ -0,0 +1,39 @@ +using Umbraco.Core.Configuration; +using Umbraco.Core.Logging; +using Umbraco.Core.Models.Rdbms; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFiveFive +{ + [Migration("7.5.5", 101, GlobalSettings.UmbracoMigrationName)] + public class AddLockObjects : MigrationBase + { + public AddLockObjects(ISqlSyntaxProvider sqlSyntax, ILogger logger) + : base(sqlSyntax, logger) + { } + + public override void Up() + { + EnsureLockObject(Constants.Locks.Servers, "Servers"); + } + + public override void Down() + { + // not implemented + } + + private void EnsureLockObject(int id, string name) + { + Execute.Code(db => + { + var exists = db.Exists(id); + if (exists) return string.Empty; + // be safe: delete old umbracoNode lock objects if any + db.Execute("DELETE FROM umbracoNode WHERE id=@id;", new { id }); + // then create umbracoLock object + db.Execute("INSERT umbracoLock (id, name, value) VALUES (@id, '@name', 1);", new { id, name }); + return string.Empty; + }); + } + } +} diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockTable.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockTable.cs new file mode 100644 index 0000000000..5dc1720a2a --- /dev/null +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockTable.cs @@ -0,0 +1,32 @@ +using System.Linq; +using Umbraco.Core.Configuration; +using Umbraco.Core.Logging; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFiveFive +{ + [Migration("7.5.5", 100, GlobalSettings.UmbracoMigrationName)] + public class AddLockTable : MigrationBase + { + public AddLockTable(ISqlSyntaxProvider sqlSyntax, ILogger logger) + : base(sqlSyntax, logger) + { } + + public override void Up() + { + var tables = SqlSyntax.GetTablesInSchema(Context.Database).ToArray(); + if (tables.InvariantContains("umbracoLock") == false) + { + Create.Table("umbracoLock") + .WithColumn("id").AsInt32().PrimaryKey("PK_umbracoLock") + .WithColumn("value").AsInt32().NotNullable() + .WithColumn("name").AsString(64).NotNullable(); + } + } + + public override void Down() + { + // not implemented + } + } +} diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/EnsureServersLockObject.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/EnsureServersLockObject.cs index 6f9d74e5db..1bfef5ef6d 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/EnsureServersLockObject.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveZero/EnsureServersLockObject.cs @@ -22,7 +22,7 @@ namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFiveZer // for some reason it was not, so it was created during migrations but not during // new installs, so for ppl that upgrade, make sure they have it - EnsureLockObject(Constants.System.ServersLock, "0AF5E610-A310-4B6F-925F-E928D5416AF7", "LOCK: Servers"); + EnsureLockObject(Constants.Locks.Servers, "0AF5E610-A310-4B6F-925F-E928D5416AF7", "LOCK: Servers"); } public override void Down() diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddServerRegistrationColumnsAndLock.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddServerRegistrationColumnsAndLock.cs index 118dc1fc06..6a14e408a9 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddServerRegistrationColumnsAndLock.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenThreeZero/AddServerRegistrationColumnsAndLock.cs @@ -23,7 +23,7 @@ namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenThreeZe Create.Column("isMaster").OnTable("umbracoServer").AsBoolean().NotNullable().WithDefaultValue(0); } - EnsureLockObject(Constants.System.ServersLock, "0AF5E610-A310-4B6F-925F-E928D5416AF7", "LOCK: Servers"); + EnsureLockObject(Constants.Locks.Servers, "0AF5E610-A310-4B6F-925F-E928D5416AF7", "LOCK: Servers"); } public override void Down() diff --git a/src/Umbraco.Core/Services/ServerRegistrationService.cs b/src/Umbraco.Core/Services/ServerRegistrationService.cs index 5a4a48b7aa..b2d5ca855e 100644 --- a/src/Umbraco.Core/Services/ServerRegistrationService.cs +++ b/src/Umbraco.Core/Services/ServerRegistrationService.cs @@ -20,7 +20,7 @@ namespace Umbraco.Core.Services private readonly static string CurrentServerIdentityValue = NetworkHelper.MachineName // eg DOMAIN\SERVER + "/" + HttpRuntime.AppDomainAppId; // eg /LM/S3SVC/11/ROOT - private static readonly int[] LockingRepositoryIds = { Constants.System.ServersLock }; + private static readonly int[] LockingRepositoryIds = { Constants.Locks.Servers }; private ServerRole _currentServerRole = ServerRole.Unknown; private readonly LockingRepository _lrepo; diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 3dd960db06..7efbe99296 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -406,6 +406,7 @@ + @@ -415,6 +416,7 @@ + @@ -428,6 +430,8 @@ + + From c58cffe4a2dc32b99a1394776bd0c7e4b0d810fa Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 24 Oct 2016 14:46:04 +0200 Subject: [PATCH 02/18] Add tests for locks --- src/Umbraco.Tests/Persistence/LocksTests.cs | 300 ++++++++++++++++++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + 2 files changed, 301 insertions(+) create mode 100644 src/Umbraco.Tests/Persistence/LocksTests.cs diff --git a/src/Umbraco.Tests/Persistence/LocksTests.cs b/src/Umbraco.Tests/Persistence/LocksTests.cs new file mode 100644 index 0000000000..e973b14614 --- /dev/null +++ b/src/Umbraco.Tests/Persistence/LocksTests.cs @@ -0,0 +1,300 @@ +using System; +using System.Collections.Generic; +using System.Data; +using System.Data.SqlServerCe; +using System.Threading; +using NUnit.Framework; +using Umbraco.Core; +using Umbraco.Core.Events; +using Umbraco.Core.Models.Rdbms; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.SqlSyntax; +using Umbraco.Core.Persistence.UnitOfWork; +using Umbraco.Core.Publishing; +using Umbraco.Core.Services; +using Umbraco.Tests.Services; +using Umbraco.Tests.TestHelpers; +using Ignore = NUnit.Framework.IgnoreAttribute; + +namespace Umbraco.Tests.Persistence +{ + [DatabaseTestBehavior(DatabaseBehavior.NewDbFileAndSchemaPerTest)] + [TestFixture] + [Ignore("Takes too much time.")] + public class LocksTests : BaseDatabaseFactoryTest + { + private ThreadSafetyServiceTest.PerThreadPetaPocoUnitOfWorkProvider _uowProvider; + private ThreadSafetyServiceTest.PerThreadDatabaseFactory _dbFactory; + + [SetUp] + public override void Initialize() + { + base.Initialize(); + + //we need to use our own custom IDatabaseFactory for the DatabaseContext because we MUST ensure that + //a Database instance is created per thread, whereas the default implementation which will work in an HttpContext + //threading environment, or a single apartment threading environment will not work for this test because + //it is multi-threaded. + _dbFactory = new ThreadSafetyServiceTest.PerThreadDatabaseFactory(Logger); + //overwrite the local object + ApplicationContext.DatabaseContext = new DatabaseContext(_dbFactory, Logger, new SqlCeSyntaxProvider(), Constants.DatabaseProviders.SqlCe); + + //disable cache + var cacheHelper = CacheHelper.CreateDisabledCacheHelper(); + + //here we are going to override the ServiceContext because normally with our test cases we use a + //global Database object but this is NOT how it should work in the web world or in any multi threaded scenario. + //we need a new Database object for each thread. + var repositoryFactory = new RepositoryFactory(cacheHelper, Logger, SqlSyntax, SettingsForTests.GenerateMockSettings()); + _uowProvider = new ThreadSafetyServiceTest.PerThreadPetaPocoUnitOfWorkProvider(_dbFactory); + var evtMsgs = new TransientMessagesFactory(); + ApplicationContext.Services = new ServiceContext( + repositoryFactory, + _uowProvider, + new FileUnitOfWorkProvider(), + new PublishingStrategy(evtMsgs, Logger), + cacheHelper, + Logger, + evtMsgs); + + // create a few lock objects + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.Execute("SET IDENTITY_INSERT umbracoLock ON"); + database.Insert("umbracoLock", "id", false, new LockDto { Id = 1, Name = "Lock.1" }); + database.Insert("umbracoLock", "id", false, new LockDto { Id = 2, Name = "Lock.2" }); + database.Insert("umbracoLock", "id", false, new LockDto { Id = 3, Name = "Lock.3" }); + database.Execute("SET IDENTITY_INSERT umbracoLock OFF"); + database.CompleteTransaction(); + } + catch + { + database.AbortTransaction(); + } + } + + [TearDown] + public override void TearDown() + { + //dispose! + _dbFactory.Dispose(); + _uowProvider.Dispose(); + + base.TearDown(); + } + + [Test] + public void Test() + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.AcquireLockNodeReadLock(Constants.Locks.Servers); + } + finally + { + database.CompleteTransaction(); + } + } + + [Test] + public void ConcurrentReadersTest() + { + var threads = new List(); + var locker = new object(); + var acquired = 0; + var maxAcquired = 0; + for (var i = 0; i < 5; i++) + { + threads.Add(new Thread(() => + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.AcquireLockNodeReadLock(Constants.Locks.Servers); + lock (locker) + { + acquired++; + } + Thread.Sleep(500); + lock (locker) + { + if (maxAcquired < acquired) maxAcquired = acquired; + } + Thread.Sleep(500); + lock (locker) + { + acquired--; + } + } + finally + { + database.CompleteTransaction(); + } + })); + } + foreach (var thread in threads) thread.Start(); + foreach (var thread in threads) thread.Join(); + Assert.AreEqual(5, maxAcquired); + } + + [Test] + public void ConcurrentWritersTest() + { + var threads = new List(); + var locker = new object(); + var acquired = 0; + var maxAcquired = 0; + for (var i = 0; i < 5; i++) + { + threads.Add(new Thread(() => + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.AcquireLockNodeWriteLock(Constants.Locks.Servers); + lock (locker) + { + acquired++; + } + Thread.Sleep(500); + lock (locker) + { + if (maxAcquired < acquired) maxAcquired = acquired; + } + Thread.Sleep(500); + lock (locker) + { + acquired--; + } + } + finally + { + database.CompleteTransaction(); + } + })); + } + foreach (var thread in threads) thread.Start(); + foreach (var thread in threads) thread.Join(); + Assert.AreEqual(1, maxAcquired); + } + + [Test] + public void DeadLockTest() + { + Exception e1 = null, e2 = null; + + var thread1 = new Thread(() => + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.AcquireLockNodeWriteLock(1); + Thread.Sleep(1000); + database.AcquireLockNodeWriteLock(2); + Thread.Sleep(1000); + } + catch (Exception e) + { + e1 = e; + } + finally + { + database.CompleteTransaction(); + } + }); + var thread2 = new Thread(() => + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.AcquireLockNodeWriteLock(2); + Thread.Sleep(1000); + database.AcquireLockNodeWriteLock(1); + Thread.Sleep(1000); + } + catch (Exception e) + { + e2 = e; + } + finally + { + database.CompleteTransaction(); + } + }); + thread1.Start(); + thread2.Start(); + thread1.Join(); + thread2.Join(); + Assert.IsNotNull(e1); + Assert.IsNotNull(e2); + Assert.IsInstanceOf(e1); + Assert.IsInstanceOf(e2); + } + + [Test] + public void NoDeadLockTest() + { + Exception e1 = null, e2 = null; + + var thread1 = new Thread(() => + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + database.AcquireLockNodeWriteLock(1); + var info = database.Query("SELECT * FROM sys.lock_information;"); + Console.WriteLine("LOCKS:"); + foreach (var row in info) + Console.WriteLine($"> {row.request_spid} {row.resource_type} {row.resource_description} {row.request_mode} {row.resource_table} {row.resource_table_id} {row.request_status}"); + Thread.Sleep(6000); + } + catch (Exception e) + { + e1 = e; + } + finally + { + database.CompleteTransaction(); + } + }); + var thread2 = new Thread(() => + { + var database = DatabaseContext.Database; + database.BeginTransaction(IsolationLevel.RepeatableRead); + try + { + Thread.Sleep(1000); + database.AcquireLockNodeWriteLock(2); + var info = database.Query("SELECT * FROM sys.lock_information;"); + Console.WriteLine("LOCKS:"); + foreach (var row in info) + Console.WriteLine($"> {row.request_spid} {row.resource_type} {row.resource_description} {row.request_mode} {row.resource_table} {row.resource_table_id} {row.request_status}"); + Thread.Sleep(1000); + } + catch (Exception e) + { + e2 = e; + } + finally + { + database.CompleteTransaction(); + } + }); + thread1.Start(); + thread2.Start(); + thread1.Join(); + thread2.Join(); + Assert.IsNull(e1); + Assert.IsNull(e2); + } + } +} diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index 042fd6ff6f..25ed563e2d 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -176,6 +176,7 @@ + From 21e2c35ab0994db141ba8ef2173cf2fc4ce6cad2 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 27 Oct 2016 17:36:08 +0200 Subject: [PATCH 03/18] U4-9111 Benchmark BulkCopy changes from U4-9107 --- .../Persistence/Mappers/BaseMapper.cs | 16 +- .../Persistence/Mappers/ContentMapper.cs | 6 + .../Querying/BaseExpressionHelper.cs | 426 ++++++++++++------ .../Persistence/Querying/IQuery.cs | 23 - .../Querying/ModelToSqlExpressionHelper.cs | 57 ++- .../Querying/PocoToSqlExpressionHelper.cs | 47 +- .../Persistence/Querying/Query.cs | 1 + .../Persistence/Querying/QueryExtensions.cs | 27 ++ src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../ModelToSqlExpressionHelperBenchmarks.cs | 105 +++++ src/Umbraco.Tests.Benchmarks/Program.cs | 3 +- .../Umbraco.Tests.Benchmarks.csproj | 6 +- .../Persistence/Querying/ExpressionTests.cs | 52 ++- 13 files changed, 554 insertions(+), 216 deletions(-) create mode 100644 src/Umbraco.Core/Persistence/Querying/QueryExtensions.cs create mode 100644 src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs diff --git a/src/Umbraco.Core/Persistence/Mappers/BaseMapper.cs b/src/Umbraco.Core/Persistence/Mappers/BaseMapper.cs index ea7c8ab8f9..40ec415b30 100644 --- a/src/Umbraco.Core/Persistence/Mappers/BaseMapper.cs +++ b/src/Umbraco.Core/Persistence/Mappers/BaseMapper.cs @@ -8,7 +8,17 @@ namespace Umbraco.Core.Persistence.Mappers { public abstract class BaseMapper { - + private readonly ISqlSyntaxProvider _sqlSyntax; + + protected BaseMapper() : this(SqlSyntaxContext.SqlSyntaxProvider) + { + } + + protected BaseMapper(ISqlSyntaxProvider sqlSyntax) + { + _sqlSyntax = sqlSyntax; + } + internal abstract ConcurrentDictionary PropertyInfoCache { get; } internal abstract void BuildMap(); @@ -58,8 +68,8 @@ namespace Umbraco.Core.Persistence.Mappers string columnName = columnAttribute.Name; string columnMap = string.Format("{0}.{1}", - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedTableName(tableName), - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName(columnName)); + _sqlSyntax.GetQuotedTableName(tableName), + _sqlSyntax.GetQuotedColumnName(columnName)); return columnMap; } } diff --git a/src/Umbraco.Core/Persistence/Mappers/ContentMapper.cs b/src/Umbraco.Core/Persistence/Mappers/ContentMapper.cs index 1cc29cf959..5db0d1d3b0 100644 --- a/src/Umbraco.Core/Persistence/Mappers/ContentMapper.cs +++ b/src/Umbraco.Core/Persistence/Mappers/ContentMapper.cs @@ -3,6 +3,7 @@ using System.Collections.Concurrent; using System.Linq.Expressions; using Umbraco.Core.Models; using Umbraco.Core.Models.Rdbms; +using Umbraco.Core.Persistence.SqlSyntax; namespace Umbraco.Core.Persistence.Mappers { @@ -16,6 +17,11 @@ namespace Umbraco.Core.Persistence.Mappers { private static readonly ConcurrentDictionary PropertyInfoCacheInstance = new ConcurrentDictionary(); + public ContentMapper(ISqlSyntaxProvider sqlSyntax) : base(sqlSyntax) + { + + } + //NOTE: its an internal class but the ctor must be public since we're using Activator.CreateInstance to create it // otherwise that would fail because there is no public constructor. public ContentMapper() diff --git a/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs index 0960acc9e5..4bbd86d66b 100644 --- a/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Globalization; @@ -10,22 +11,98 @@ using Umbraco.Core.Persistence.SqlSyntax; namespace Umbraco.Core.Persistence.Querying { - internal abstract class BaseExpressionHelper : BaseExpressionHelper + + /// + /// This is used to determine if the expression result is cached and therefore only the SQL parameters will be extracted + /// + /// + /// This saves some performance overhead since the SQL string itself does not get generated because it already exists. + /// + internal class CachedExpression : Expression { + public CachedExpression() + { + CompiledOutput = null; + } + + public Expression InnerExpression { get; private set; } + + /// + /// The compiled SQL statement output + /// + public string CompiledOutput { get; private set; } + + public bool IsCompiled + { + get { return CompiledOutput.IsNullOrWhiteSpace() == false; } + } + + public void Compile(string output) + { + if (IsCompiled) + throw new InvalidOperationException("Cached expression is already compiled"); + + CompiledOutput = output; + } + + public void Wrap(Expression exp) + { + InnerExpression = exp; + } + } + + /// + /// An expression tree parser to create SQL statements and SQL parameters based on a given strongly typed expression + /// + /// + /// Logic that is shared with the expression helpers. This object stores state, it cannot be re-used to parse an expression. + /// + internal abstract class BaseExpressionHelper + { + protected BaseExpressionHelper(ISqlSyntaxProvider sqlSyntax) + { + SqlSyntax = sqlSyntax; + } + + /// + /// Indicates that the SQL statement has already been compiled, so Visiting will just generate the Sql Parameters + /// + protected bool IsCompiled { get; set; } + + protected ISqlSyntaxProvider SqlSyntax { get; private set; } + + protected List SqlParameters = new List(); + protected abstract string VisitMemberAccess(MemberExpression m); protected internal virtual string Visit(Expression exp) { + //set the flag if it is already compiled + var compiledExp = exp as CachedExpression; + if (compiledExp != null) + { + if (compiledExp.IsCompiled) + { + IsCompiled = true; + } + exp = compiledExp.InnerExpression; + } if (exp == null) return string.Empty; + + string result; + switch (exp.NodeType) { case ExpressionType.Lambda: - return VisitLambda(exp as LambdaExpression); + result = VisitLambda(exp as LambdaExpression); + break; case ExpressionType.MemberAccess: - return VisitMemberAccess(exp as MemberExpression); + result = VisitMemberAccess(exp as MemberExpression); + break; case ExpressionType.Constant: - return VisitConstant(exp as ConstantExpression); + result = VisitConstant(exp as ConstantExpression); + break; case ExpressionType.Add: case ExpressionType.AddChecked: case ExpressionType.Subtract: @@ -49,7 +126,8 @@ namespace Umbraco.Core.Persistence.Querying case ExpressionType.RightShift: case ExpressionType.LeftShift: case ExpressionType.ExclusiveOr: - return VisitBinary(exp as BinaryExpression); + result = VisitBinary(exp as BinaryExpression); + break; case ExpressionType.Negate: case ExpressionType.NegateChecked: case ExpressionType.Not: @@ -58,19 +136,37 @@ namespace Umbraco.Core.Persistence.Querying case ExpressionType.ArrayLength: case ExpressionType.Quote: case ExpressionType.TypeAs: - return VisitUnary(exp as UnaryExpression); + result = VisitUnary(exp as UnaryExpression); + break; case ExpressionType.Parameter: - return VisitParameter(exp as ParameterExpression); + result = VisitParameter(exp as ParameterExpression); + break; case ExpressionType.Call: - return VisitMethodCall(exp as MethodCallExpression); + result = VisitMethodCall(exp as MethodCallExpression); + break; case ExpressionType.New: - return VisitNew(exp as NewExpression); + result = VisitNew(exp as NewExpression); + break; case ExpressionType.NewArrayInit: case ExpressionType.NewArrayBounds: - return VisitNewArray(exp as NewArrayExpression); + result = VisitNewArray(exp as NewArrayExpression); + break; default: - return exp.ToString(); + result = exp.ToString(); + break; } + + if (compiledExp != null) + { + if (compiledExp.IsCompiled == false) + { + compiledExp.Compile(result); + } + return compiledExp.CompiledOutput; + } + + return result; + } protected virtual string VisitLambda(LambdaExpression lambda) @@ -79,14 +175,20 @@ namespace Umbraco.Core.Persistence.Querying { var m = lambda.Body as MemberExpression; - if (m.Expression != null) + if (m != null && m.Expression != null) { //This deals with members that are boolean (i.e. x => IsTrashed ) string r = VisitMemberAccess(m); - SqlParameters.Add(true); - return string.Format("{0} = @{1}", r, SqlParameters.Count - 1); - //return string.Format("{0}={1}", r, GetQuotedTrueValue()); + SqlParameters.Add(true); + + //don't execute if compiled + if (IsCompiled == false) + { + return string.Format("{0} = @{1}", r, SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; } } @@ -95,8 +197,10 @@ namespace Umbraco.Core.Persistence.Querying protected virtual string VisitBinary(BinaryExpression b) { - string left, right; - var operand = BindOperant(b.NodeType); + var left = string.Empty; + var right = string.Empty; + + var operand = BindOperant(b.NodeType); if (operand == "AND" || operand == "OR") { MemberExpression m = b.Left as MemberExpression; @@ -105,9 +209,12 @@ namespace Umbraco.Core.Persistence.Querying string r = VisitMemberAccess(m); SqlParameters.Add(1); - left = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); - //left = string.Format("{0}={1}", r, GetQuotedTrueValue()); + //don't execute if compiled + if (IsCompiled == false) + { + left = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); + } } else { @@ -119,9 +226,12 @@ namespace Umbraco.Core.Persistence.Querying string r = VisitMemberAccess(m); SqlParameters.Add(1); - right = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); - //right = string.Format("{0}={1}", r, GetQuotedTrueValue()); + //don't execute if compiled + if (IsCompiled == false) + { + right = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); + } } else { @@ -132,14 +242,14 @@ namespace Umbraco.Core.Persistence.Querying { // deal with (x == true|false) - most common var constRight = b.Right as ConstantExpression; - if (constRight != null && constRight.Type == typeof (bool)) - return ((bool) constRight.Value) ? VisitNotNot(b.Left) : VisitNot(b.Left); + if (constRight != null && constRight.Type == typeof(bool)) + return ((bool)constRight.Value) ? VisitNotNot(b.Left) : VisitNot(b.Left); right = Visit(b.Right); // deal with (true|false == x) - why not var constLeft = b.Left as ConstantExpression; - if (constLeft != null && constLeft.Type == typeof (bool)) - return ((bool) constLeft.Value) ? VisitNotNot(b.Right) : VisitNot(b.Right); + if (constLeft != null && constLeft.Type == typeof(bool)) + return ((bool)constLeft.Value) ? VisitNotNot(b.Right) : VisitNot(b.Right); left = Visit(b.Left); } else if (operand == "<>") @@ -147,13 +257,13 @@ namespace Umbraco.Core.Persistence.Querying // deal with (x != true|false) - most common var constRight = b.Right as ConstantExpression; if (constRight != null && constRight.Type == typeof(bool)) - return ((bool) constRight.Value) ? VisitNot(b.Left) : VisitNotNot(b.Left); + return ((bool)constRight.Value) ? VisitNot(b.Left) : VisitNotNot(b.Left); right = Visit(b.Right); // deal with (true|false != x) - why not var constLeft = b.Left as ConstantExpression; if (constLeft != null && constLeft.Type == typeof(bool)) - return ((bool) constLeft.Value) ? VisitNot(b.Right) : VisitNotNot(b.Right); + return ((bool)constLeft.Value) ? VisitNot(b.Right) : VisitNotNot(b.Right); left = Visit(b.Left); } else @@ -178,9 +288,21 @@ namespace Umbraco.Core.Persistence.Querying { case "MOD": case "COALESCE": - return string.Format("{0}({1},{2})", operand, left, right); + //don't execute if compiled + if (IsCompiled == false) + { + return string.Format("{0}({1},{2})", operand, left, right); + } + //already compiled, return + return string.Empty; default: - return "(" + left + " " + operand + " " + right + ")"; + //don't execute if compiled + if (IsCompiled == false) + { + return string.Concat("(", left, " ", operand, " ", right, ")"); + } + //already compiled, return + return string.Empty; } } @@ -213,22 +335,33 @@ namespace Umbraco.Core.Persistence.Querying object o = getter(); SqlParameters.Add(o); - return string.Format("@{0}", SqlParameters.Count - 1); - //return GetQuotedValue(o, o.GetType()); + //don't execute if compiled + if (IsCompiled == false) + { + return string.Format("@{0}", SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; } catch (InvalidOperationException) - { - // FieldName ? - List exprs = VisitExpressionList(nex.Arguments); - var r = new StringBuilder(); - foreach (Object e in exprs) + { + //don't execute if compiled + if (IsCompiled == false) { - r.AppendFormat("{0}{1}", - r.Length > 0 ? "," : "", - e); + // FieldName ? + List exprs = VisitExpressionList(nex.Arguments); + var r = new StringBuilder(); + foreach (Object e in exprs) + { + r.AppendFormat("{0}{1}", + r.Length > 0 ? "," : "", + e); + } + return r.ToString(); } - return r.ToString(); + //already compiled, return + return string.Empty; } } @@ -244,14 +377,14 @@ namespace Umbraco.Core.Persistence.Querying return "null"; SqlParameters.Add(c.Value); - return string.Format("@{0}", SqlParameters.Count - 1); - //if (c.Value is bool) - //{ - // object o = GetQuotedValue(c.Value, c.Value.GetType()); - // return string.Format("({0}={1})", GetQuotedTrueValue(), o); - //} - //return GetQuotedValue(c.Value, c.Value.GetType()); + //don't execute if compiled + if (IsCompiled == false) + { + return string.Format("@{0}", SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; } protected virtual string VisitUnary(UnaryExpression u) @@ -277,10 +410,22 @@ namespace Umbraco.Core.Persistence.Querying case ExpressionType.MemberAccess: // false property , i.e. x => !Trashed SqlParameters.Add(true); - return string.Format("NOT ({0} = @{1})", o, SqlParameters.Count - 1); + //don't execute if compiled + if (IsCompiled == false) + { + return string.Format("NOT ({0} = @{1})", o, SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; default: - // could be anything else, such as: x => !x.Path.StartsWith("-20") - return "NOT (" + o + ")"; + //don't execute if compiled + if (IsCompiled == false) + { + // could be anything else, such as: x => !x.Path.StartsWith("-20") + return string.Concat("NOT (", o, ")"); + } + //already compiled, return + return string.Empty; } } @@ -293,7 +438,14 @@ namespace Umbraco.Core.Persistence.Querying case ExpressionType.MemberAccess: // true property, i.e. x => Trashed SqlParameters.Add(true); - return string.Format("({0} = @{1})", o, SqlParameters.Count - 1); + + //don't execute if compiled + if (IsCompiled == false) + { + return string.Format("({0} = @{1})", o, SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; default: // could be anything else, such as: x => x.Path.StartsWith("-20") return o; @@ -302,15 +454,22 @@ namespace Umbraco.Core.Persistence.Querying protected virtual string VisitNewArray(NewArrayExpression na) { - List exprs = VisitExpressionList(na.Expressions); - var r = new StringBuilder(); - foreach (Object e in exprs) - { - r.Append(r.Length > 0 ? "," + e : e); - } - return r.ToString(); + //don't execute if compiled + if (IsCompiled == false) + { + var r = new StringBuilder(); + foreach (Object e in exprs) + { + r.Append(r.Length > 0 ? "," + e : e); + } + + return r.ToString(); + } + //already compiled, return + return string.Empty; + } protected virtual List VisitNewArrayFromExpressionList(NewArrayExpression na) @@ -375,19 +534,31 @@ namespace Umbraco.Core.Persistence.Querying var objectForMethod = m.Object ?? m.Arguments[0]; var visitedObjectForMethod = Visit(objectForMethod); - var methodArgs = m.Object == null - ? m.Arguments.Skip(1).ToArray() + var methodArgs = m.Object == null + ? m.Arguments.Skip(1).ToArray() : m.Arguments.ToArray(); switch (m.Method.Name) { case "ToString": SqlParameters.Add(objectForMethod.ToString()); - return string.Format("@{0}", SqlParameters.Count - 1); + //don't execute if compiled + if (IsCompiled == false) + return string.Format("@{0}", SqlParameters.Count - 1); + //already compiled, return + return string.Empty; case "ToUpper": - return string.Format("upper({0})", visitedObjectForMethod); + //don't execute if compiled + if (IsCompiled == false) + return string.Format("upper({0})", visitedObjectForMethod); + //already compiled, return + return string.Empty; case "ToLower": - return string.Format("lower({0})", visitedObjectForMethod); + //don't execute if compiled + if (IsCompiled == false) + return string.Format("lower({0})", visitedObjectForMethod); + //already compiled, return + return string.Empty; case "SqlWildcard": case "StartsWith": case "EndsWith": @@ -401,7 +572,7 @@ namespace Umbraco.Core.Persistence.Querying case "InvariantEndsWith": case "InvariantContains": case "InvariantEquals": - + string compareValue; if (methodArgs[0].NodeType != ExpressionType.Constant) @@ -488,7 +659,12 @@ namespace Umbraco.Core.Persistence.Querying SqlParameters.Add(RemoveQuote(replaceValue)); - return string.Format("replace({0}, @{1}, @{2})", visitedObjectForMethod, SqlParameters.Count - 2, SqlParameters.Count - 1); + //don't execute if compiled + if (IsCompiled == false) + return string.Format("replace({0}, @{1}, @{2})", visitedObjectForMethod, SqlParameters.Count - 2, SqlParameters.Count - 1); + //already compiled, return + return string.Empty; + //case "Substring": // var startIndex = Int32.Parse(args[0].ToString()) + 1; // if (args.Count == 2) @@ -555,77 +731,42 @@ namespace Umbraco.Core.Persistence.Querying throw new ArgumentOutOfRangeException("No logic supported for " + m.Method.Name); - //var s2 = new StringBuilder(); - //foreach (Object e in args) - //{ - // s2.AppendFormat(",{0}", GetQuotedValue(e, e.GetType())); - //} - //return string.Format("{0}({1}{2})", m.Method.Name, r, s2.ToString()); + //var s2 = new StringBuilder(); + //foreach (Object e in args) + //{ + // s2.AppendFormat(",{0}", GetQuotedValue(e, e.GetType())); + //} + //return string.Format("{0}({1}{2})", m.Method.Name, r, s2.ToString()); } } public virtual string GetQuotedTableName(string tableName) { + //already compiled, return + if (IsCompiled) + return tableName; + return string.Format("\"{0}\"", tableName); } public virtual string GetQuotedColumnName(string columnName) { + //already compiled, return + if (IsCompiled) + return columnName; + return string.Format("\"{0}\"", columnName); } public virtual string GetQuotedName(string name) { + //already compiled, return + if (IsCompiled) + return name; + return string.Format("\"{0}\"", name); } - //private string GetQuotedTrueValue() - //{ - // return GetQuotedValue(true, typeof(bool)); - //} - - //private string GetQuotedFalseValue() - //{ - // return GetQuotedValue(false, typeof(bool)); - //} - - //public virtual string GetQuotedValue(object value, Type fieldType) - //{ - // return GetQuotedValue(value, fieldType, EscapeParam, ShouldQuoteValue); - //} - - //private string GetTrueExpression() - //{ - // object o = GetQuotedTrueValue(); - // return string.Format("({0}={1})", o, o); - //} - - //private string GetFalseExpression() - //{ - - // return string.Format("({0}={1})", - // GetQuotedTrueValue(), - // GetQuotedFalseValue()); - //} - - //private bool IsTrueExpression(string exp) - //{ - // return (exp == GetTrueExpression()); - //} - - //private bool IsFalseExpression(string exp) - //{ - // return (exp == GetFalseExpression()); - //} - } - - /// - /// Logic that is shared with the expression helpers - /// - internal class BaseExpressionHelper - { - protected List SqlParameters = new List(); - public object[] GetSqlParameters() { return SqlParameters.ToArray(); @@ -637,25 +778,45 @@ namespace Umbraco.Core.Persistence.Querying { case "SqlWildcard": SqlParameters.Add(RemoveQuote(val)); - return SqlSyntaxContext.SqlSyntaxProvider.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //don't execute if compiled + if (IsCompiled == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; case "Equals": SqlParameters.Add(RemoveQuote(val)); - return SqlSyntaxContext.SqlSyntaxProvider.GetStringColumnEqualComparison(col, SqlParameters.Count - 1, columnType); + //don't execute if compiled + if (IsCompiled == false) + return SqlSyntax.GetStringColumnEqualComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; case "StartsWith": SqlParameters.Add(string.Format("{0}{1}", RemoveQuote(val), - SqlSyntaxContext.SqlSyntaxProvider.GetWildcardPlaceholder())); - return SqlSyntaxContext.SqlSyntaxProvider.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + SqlSyntax.GetWildcardPlaceholder())); + //don't execute if compiled + if (IsCompiled == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; case "EndsWith": SqlParameters.Add(string.Format("{0}{1}", - SqlSyntaxContext.SqlSyntaxProvider.GetWildcardPlaceholder(), + SqlSyntax.GetWildcardPlaceholder(), RemoveQuote(val))); - return SqlSyntaxContext.SqlSyntaxProvider.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //don't execute if compiled + if (IsCompiled == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; case "Contains": SqlParameters.Add(string.Format("{0}{1}{0}", - SqlSyntaxContext.SqlSyntaxProvider.GetWildcardPlaceholder(), + SqlSyntax.GetWildcardPlaceholder(), RemoveQuote(val))); - return SqlSyntaxContext.SqlSyntaxProvider.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //don't execute if compiled + if (IsCompiled == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; case "InvariantEquals": case "SqlEquals": //recurse @@ -730,7 +891,7 @@ namespace Umbraco.Core.Persistence.Querying { return paramValue == null ? string.Empty - : SqlSyntaxContext.SqlSyntaxProvider.EscapeString(paramValue.ToString()); + : SqlSyntax.EscapeString(paramValue.ToString()); } public virtual bool ShouldQuoteValue(Type fieldType) @@ -740,16 +901,9 @@ namespace Umbraco.Core.Persistence.Querying protected virtual string RemoveQuote(string exp) { - //if (exp.StartsWith("'") && exp.EndsWith("'")) - //{ - // exp = exp.Remove(0, 1); - // exp = exp.Remove(exp.Length - 1, 1); - //} - //return exp; - if ((exp.StartsWith("\"") || exp.StartsWith("`") || exp.StartsWith("'")) - && - (exp.EndsWith("\"") || exp.EndsWith("`") || exp.EndsWith("'"))) + && + (exp.EndsWith("\"") || exp.EndsWith("`") || exp.EndsWith("'"))) { exp = exp.Remove(0, 1); exp = exp.Remove(exp.Length - 1, 1); diff --git a/src/Umbraco.Core/Persistence/Querying/IQuery.cs b/src/Umbraco.Core/Persistence/Querying/IQuery.cs index b158943cb4..ae986baddc 100644 --- a/src/Umbraco.Core/Persistence/Querying/IQuery.cs +++ b/src/Umbraco.Core/Persistence/Querying/IQuery.cs @@ -1,31 +1,8 @@ using System; -using System.Collections.Generic; using System.Linq.Expressions; namespace Umbraco.Core.Persistence.Querying { - /// - /// SD: This is a horrible hack but unless we break compatibility with anyone who's actually implemented IQuery{T} there's not much we can do. - /// The IQuery{T} interface is useless without having a GetWhereClauses method and cannot be used for tests. - /// We have to wait till v8 to make this change I suppose. - /// - internal static class QueryExtensions - { - /// - /// Returns all translated where clauses and their sql parameters - /// - /// - public static IEnumerable> GetWhereClauses(this IQuery query) - { - var q = query as Query; - if (q == null) - { - throw new NotSupportedException(typeof(IQuery) + " cannot be cast to " + typeof(Query)); - } - return q.GetWhereClauses(); - } - } - /// /// Represents a query for building Linq translatable SQL queries /// diff --git a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs index a0ccfaa070..7e4466b529 100644 --- a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs @@ -11,14 +11,24 @@ using Umbraco.Core.Persistence.SqlSyntax; namespace Umbraco.Core.Persistence.Querying { - internal class ModelToSqlExpressionHelper : BaseExpressionHelper + /// + /// An expression tree parser to create SQL statements and SQL parameters based on a given strongly typed expression based on Umbraco's business logic Models + /// + /// + /// This object stores state, it cannot be re-used to parse an expression + /// + internal class ModelToSqlExpressionHelper : BaseExpressionHelper { private readonly BaseMapper _mapper; - public ModelToSqlExpressionHelper() + public ModelToSqlExpressionHelper(ISqlSyntaxProvider sqlSyntax, BaseMapper mapper) : base(sqlSyntax) + { + _mapper = mapper; + } + + public ModelToSqlExpressionHelper() : this(SqlSyntaxContext.SqlSyntaxProvider, MappingResolver.Current.ResolveMapperByType(typeof(T))) { - _mapper = MappingResolver.Current.ResolveMapperByType(typeof(T)); } protected override string VisitMemberAccess(MemberExpression m) @@ -27,18 +37,30 @@ namespace Umbraco.Core.Persistence.Querying m.Expression.NodeType == ExpressionType.Parameter && m.Expression.Type == typeof(T)) { - var field = _mapper.Map(m.Member.Name, true); - if (field.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); - return field; + //don't execute if compiled + if (IsCompiled == false) + { + var field = _mapper.Map(m.Member.Name, true); + if (field.IsNullOrWhiteSpace()) + throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; } if (m.Expression != null && m.Expression.NodeType == ExpressionType.Convert) { - var field = _mapper.Map(m.Member.Name, true); - if (field.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); - return field; + //don't execute if compiled + if (IsCompiled == false) + { + var field = _mapper.Map(m.Member.Name, true); + if (field.IsNullOrWhiteSpace()) + throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; } var member = Expression.Convert(m, typeof(object)); @@ -47,16 +69,13 @@ namespace Umbraco.Core.Persistence.Querying object o = getter(); SqlParameters.Add(o); - return string.Format("@{0}", SqlParameters.Count - 1); - //return GetQuotedValue(o, o != null ? o.GetType() : null); + //don't execute if compiled + if (IsCompiled == false) + return string.Format("@{0}", SqlParameters.Count - 1); + //already compiled, return + return string.Empty; } - - //protected bool IsFieldName(string quotedExp) - //{ - // //Not entirely sure this is reliable, but its better then simply returning true - // return quotedExp.LastIndexOf("'", StringComparison.InvariantCultureIgnoreCase) + 1 != quotedExp.Length; - //} } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs index bbdb7a5509..cbc3aa8f05 100644 --- a/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs @@ -9,11 +9,18 @@ using Umbraco.Core.Persistence.SqlSyntax; namespace Umbraco.Core.Persistence.Querying { - internal class PocoToSqlExpressionHelper : BaseExpressionHelper + + /// + /// An expression tree parser to create SQL statements and SQL parameters based on a given strongly typed expression based on Umbraco's PetaPoco dto Models + /// + /// + /// This object stores state, it cannot be re-used to parse an expression + /// + internal class PocoToSqlExpressionHelper : BaseExpressionHelper { private readonly Database.PocoData _pd; - public PocoToSqlExpressionHelper() + public PocoToSqlExpressionHelper() : base(SqlSyntaxContext.SqlSyntaxProvider) { _pd = new Database.PocoData(typeof(T)); } @@ -24,14 +31,26 @@ namespace Umbraco.Core.Persistence.Querying m.Expression.NodeType == ExpressionType.Parameter && m.Expression.Type == typeof(T)) { - string field = GetFieldName(_pd, m.Member.Name); - return field; + //don't execute if compiled + if (IsCompiled == false) + { + string field = GetFieldName(_pd, m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; } if (m.Expression != null && m.Expression.NodeType == ExpressionType.Convert) { - string field = GetFieldName(_pd, m.Member.Name); - return field; + //don't execute if compiled + if (IsCompiled == false) + { + string field = GetFieldName(_pd, m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; } var member = Expression.Convert(m, typeof(object)); @@ -40,23 +59,21 @@ namespace Umbraco.Core.Persistence.Querying object o = getter(); SqlParameters.Add(o); - return string.Format("@{0}", SqlParameters.Count - 1); - - //return GetQuotedValue(o, o != null ? o.GetType() : null); + //don't execute if compiled + if (IsCompiled == false) + return string.Format("@{0}", SqlParameters.Count - 1); + //already compiled, return + return string.Empty; } protected virtual string GetFieldName(Database.PocoData pocoData, string name) { var column = pocoData.Columns.FirstOrDefault(x => x.Value.PropertyInfo.Name == name); return string.Format("{0}.{1}", - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedTableName(pocoData.TableInfo.TableName), - SqlSyntaxContext.SqlSyntaxProvider.GetQuotedColumnName(column.Value.ColumnName)); + SqlSyntax.GetQuotedTableName(pocoData.TableInfo.TableName), + SqlSyntax.GetQuotedColumnName(column.Value.ColumnName)); } - //protected bool IsFieldName(string quotedExp) - //{ - // return true; - //} } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Querying/Query.cs b/src/Umbraco.Core/Persistence/Querying/Query.cs index 4dd268268f..1a270cec4b 100644 --- a/src/Umbraco.Core/Persistence/Querying/Query.cs +++ b/src/Umbraco.Core/Persistence/Querying/Query.cs @@ -30,6 +30,7 @@ namespace Umbraco.Core.Persistence.Querying { if (predicate != null) { + //TODO: This should have an SqlSyntax object passed in, this ctor is relying on a singleton var expressionHelper = new ModelToSqlExpressionHelper(); string whereExpression = expressionHelper.Visit(predicate); diff --git a/src/Umbraco.Core/Persistence/Querying/QueryExtensions.cs b/src/Umbraco.Core/Persistence/Querying/QueryExtensions.cs new file mode 100644 index 0000000000..20c3409a40 --- /dev/null +++ b/src/Umbraco.Core/Persistence/Querying/QueryExtensions.cs @@ -0,0 +1,27 @@ +using System; +using System.Collections.Generic; + +namespace Umbraco.Core.Persistence.Querying +{ + /// + /// SD: This is a horrible hack but unless we break compatibility with anyone who's actually implemented IQuery{T} there's not much we can do. + /// The IQuery{T} interface is useless without having a GetWhereClauses method and cannot be used for tests. + /// We have to wait till v8 to make this change I suppose. + /// + internal static class QueryExtensions + { + /// + /// Returns all translated where clauses and their sql parameters + /// + /// + public static IEnumerable> GetWhereClauses(this IQuery query) + { + var q = query as Query; + if (q == null) + { + throw new NotSupportedException(typeof(IQuery) + " cannot be cast to " + typeof(Query)); + } + return q.GetWhereClauses(); + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c28cea24cd..36894801aa 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -474,6 +474,7 @@ + diff --git a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs new file mode 100644 index 0000000000..b492d73829 --- /dev/null +++ b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Generic; +using System.Data.SqlServerCe; +using System.Diagnostics; +using System.IO; +using System.Linq.Expressions; +using System.Threading; +using System.Xml; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Columns; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Diagnosers; +using BenchmarkDotNet.Diagnostics.Windows; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Loggers; +using BenchmarkDotNet.Reports; +using BenchmarkDotNet.Running; +using BenchmarkDotNet.Validators; +using Umbraco.Core; +using Umbraco.Core.Logging; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Rdbms; +using Umbraco.Core.Persistence; +using Umbraco.Core.Persistence.Mappers; +using Umbraco.Core.Persistence.Migrations.Initial; +using Umbraco.Core.Persistence.Querying; +using Umbraco.Core.Persistence.SqlSyntax; +using Umbraco.Tests.TestHelpers; +using ILogger = Umbraco.Core.Logging.ILogger; + +namespace Umbraco.Tests.Benchmarks +{ + [Config(typeof(Config))] + public class ModelToSqlExpressionHelperBenchmarks + { + private class Config : ManualConfig + { + public Config() + { + Add(new MemoryDiagnoser()); + } + } + + public ModelToSqlExpressionHelperBenchmarks() + { + _contentMapper = new ContentMapper(_syntaxProvider); + _contentMapper.BuildMap(); + _cachedExpression = new CachedExpression(); + } + + private readonly ISqlSyntaxProvider _syntaxProvider = new SqlCeSyntaxProvider(); + private readonly BaseMapper _contentMapper; + private readonly CachedExpression _cachedExpression; + //private static readonly Expression> TemplatePredicate = content => + // content.Path.StartsWith(string.Empty) && content.Published && (content.ContentTypeId == 0 || content.ContentTypeId == 0); + + [Benchmark(Baseline = true)] + public void WithNonCached() + { + for (int i = 0; i < 100; i++) + { + var a = i; + var b = i*10; + Expression> predicate = content => + content.Path.StartsWith("-1") && content.Published && (content.ContentTypeId == a || content.ContentTypeId == b); + + var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(_syntaxProvider, _contentMapper); + var result = modelToSqlExpressionHelper.Visit(predicate); + } + + } + + [Benchmark] + public void WithSQL() + { + var subQuery = new Sql() + //.Select("umbracoNode.id as nodeId") + .From(_syntaxProvider) + .InnerJoin(_syntaxProvider) + .On(_syntaxProvider, left => left.NodeId, right => right.NodeId) + .WhereIn(dto => dto.ContentTypeId, contentTypeIds, SqlSyntax) + .Where(x => x.NodeObjectType == NodeObjectTypeId); + } + + [Benchmark()] + public void WithCachedExpression() + { + for (int i = 0; i < 100; i++) + { + var a = i; + var b = i * 10; + Expression> predicate = content => + content.Path.StartsWith("-1") && content.Published && (content.ContentTypeId == a || content.ContentTypeId == b); + + var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(_syntaxProvider, _contentMapper); + + //wrap it! + _cachedExpression.Wrap(predicate); + + var result = modelToSqlExpressionHelper.Visit(_cachedExpression); + } + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests.Benchmarks/Program.cs b/src/Umbraco.Tests.Benchmarks/Program.cs index 37c1ccd853..eaac2abc0d 100644 --- a/src/Umbraco.Tests.Benchmarks/Program.cs +++ b/src/Umbraco.Tests.Benchmarks/Program.cs @@ -11,7 +11,8 @@ namespace Umbraco.Tests.Benchmarks { static void Main(string[] args) { - var summary = BenchmarkRunner.Run(); + var summary = BenchmarkRunner.Run(); + //var summary = BenchmarkRunner.Run(); Console.ReadLine(); } diff --git a/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj b/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj index 66033ba08e..f6ef93fb20 100644 --- a/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj +++ b/src/Umbraco.Tests.Benchmarks/Umbraco.Tests.Benchmarks.csproj @@ -96,6 +96,7 @@ + @@ -103,16 +104,11 @@ - - - - - {31785bc3-256c-4613-b2f5-a1b0bdded8c1} diff --git a/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs b/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs index 4529343811..6834968460 100644 --- a/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs +++ b/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs @@ -16,19 +16,45 @@ namespace Umbraco.Tests.Persistence.Querying [TestFixture] public class ExpressionTests : BaseUsingSqlCeSyntax { - // [Test] - // public void Can_Query_With_Content_Type_Alias() - // { - // //Arrange - // Expression> predicate = content => content.ContentType.Alias == "Test"; - // var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); - // var result = modelToSqlExpressionHelper.Visit(predicate); + // [Test] + // public void Can_Query_With_Content_Type_Alias() + // { + // //Arrange + // Expression> predicate = content => content.ContentType.Alias == "Test"; + // var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + // var result = modelToSqlExpressionHelper.Visit(predicate); - // Debug.Print("Model to Sql ExpressionHelper: \n" + result); + // Debug.Print("Model to Sql ExpressionHelper: \n" + result); - // Assert.AreEqual("[cmsContentType].[alias] = @0", result); - // Assert.AreEqual("Test", modelToSqlExpressionHelper.GetSqlParameters()[0]); - // } + // Assert.AreEqual("[cmsContentType].[alias] = @0", result); + // Assert.AreEqual("Test", modelToSqlExpressionHelper.GetSqlParameters()[0]); + // } + + [Test] + public void CachedExpression_Can_Verify_Path_StartsWith_Predicate_In_Same_Result() + { + //Arrange + + //use a single cached expression for multiple expressions and ensure the correct output + // is done for both of them. + var cachedExpression = new CachedExpression(); + + + Expression> predicate1 = content => content.Path.StartsWith("-1"); + cachedExpression.Wrap(predicate1); + var modelToSqlExpressionHelper1 = new ModelToSqlExpressionHelper(); + var result1 = modelToSqlExpressionHelper1.Visit(cachedExpression); + Assert.AreEqual("upper([umbracoNode].[path]) LIKE upper(@0)", result1); + Assert.AreEqual("-1%", modelToSqlExpressionHelper1.GetSqlParameters()[0]); + + Expression> predicate2 = content => content.Path.StartsWith("-1,123,97"); + cachedExpression.Wrap(predicate2); + var modelToSqlExpressionHelper2 = new ModelToSqlExpressionHelper(); + var result2 = modelToSqlExpressionHelper2.Visit(cachedExpression); + Assert.AreEqual("upper([umbracoNode].[path]) LIKE upper(@0)", result2); + Assert.AreEqual("-1,123,97%", modelToSqlExpressionHelper2.GetSqlParameters()[0]); + + } [Test] public void Can_Verify_Path_StartsWith_Predicate_In_Same_Result() @@ -37,9 +63,7 @@ namespace Umbraco.Tests.Persistence.Querying Expression> predicate = content => content.Path.StartsWith("-1"); var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); var result = modelToSqlExpressionHelper.Visit(predicate); - - Debug.Print("Model to Sql ExpressionHelper: \n" + result); - + Assert.AreEqual("upper([umbracoNode].[path]) LIKE upper(@0)", result); Assert.AreEqual("-1%", modelToSqlExpressionHelper.GetSqlParameters()[0]); } From 9717d03a75579457711552445ac676dec8ed466d Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 27 Oct 2016 17:38:04 +0200 Subject: [PATCH 04/18] oops, didn't mean to commit this --- .../ModelToSqlExpressionHelperBenchmarks.cs | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs index b492d73829..922badc33b 100644 --- a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs @@ -70,17 +70,7 @@ namespace Umbraco.Tests.Benchmarks } - [Benchmark] - public void WithSQL() - { - var subQuery = new Sql() - //.Select("umbracoNode.id as nodeId") - .From(_syntaxProvider) - .InnerJoin(_syntaxProvider) - .On(_syntaxProvider, left => left.NodeId, right => right.NodeId) - .WhereIn(dto => dto.ContentTypeId, contentTypeIds, SqlSyntax) - .Where(x => x.NodeObjectType == NodeObjectTypeId); - } + [Benchmark()] public void WithCachedExpression() From 3f609fdf90c1625489ea70310d0999d5c99e57f6 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 27 Oct 2016 17:50:52 +0200 Subject: [PATCH 05/18] Moves a few queries to re-used query instances --- .../Persistence/Repositories/ContentRepository.cs | 9 +++++++-- .../Persistence/Repositories/RepositoryBase.cs | 9 +++++++-- src/Umbraco.Core/Services/ContentService.cs | 9 +++++++-- src/Umbraco.Core/Services/EntityService.cs | 9 +++++++-- .../ModelToSqlExpressionHelperBenchmarks.cs | 4 +--- src/UmbracoExamine/UmbracoContentIndexer.cs | 9 ++++++--- 6 files changed, 35 insertions(+), 14 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs index 84caeb2a3e..d989edebb8 100644 --- a/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs +++ b/src/Umbraco.Core/Persistence/Repositories/ContentRepository.cs @@ -104,6 +104,12 @@ namespace Umbraco.Core.Persistence.Repositories #endregion + #region Static Queries + + private readonly IQuery _publishedQuery = Query.Builder.Where(x => x.Published == true); + + #endregion + #region Overrides of PetaPocoRepositoryBase @@ -207,8 +213,7 @@ namespace Umbraco.Core.Persistence.Repositories //now insert the data, again if something fails here, the whole transaction is reversed if (contentTypeIds == null) { - var query = Query.Builder.Where(x => x.Published == true); - RebuildXmlStructuresProcessQuery(serializer, query, tr, groupSize); + RebuildXmlStructuresProcessQuery(serializer, _publishedQuery, tr, groupSize); } else { diff --git a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs index 5534a9ea40..9379d14af8 100644 --- a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs @@ -83,6 +83,12 @@ namespace Umbraco.Core.Persistence.Repositories } + #region Static Queries + + private readonly IQuery _hasIdQuery = Query.Builder.Where(x => x.Id != 0); + + #endregion + protected virtual TId GetEntityId(TEntity entity) { return (TId)(object)entity.Id; @@ -112,8 +118,7 @@ namespace Umbraco.Core.Persistence.Repositories new RepositoryCachePolicyOptions(() => { //Get count of all entities of current type (TEntity) to ensure cached result is correct - var query = Query.Builder.Where(x => x.Id != 0); - return PerformCount(query); + return PerformCount(_hasIdQuery); }))); } } diff --git a/src/Umbraco.Core/Services/ContentService.cs b/src/Umbraco.Core/Services/ContentService.cs index 38b7aec9d7..6223e5933b 100644 --- a/src/Umbraco.Core/Services/ContentService.cs +++ b/src/Umbraco.Core/Services/ContentService.cs @@ -56,6 +56,12 @@ namespace Umbraco.Core.Services _userService = userService; } + #region Static Queries + + private readonly IQuery _notTrashedQuery = Query.Builder.Where(x => x.Trashed == false); + + #endregion + public int CountPublished(string contentTypeAlias = null) { var uow = UowProvider.GetUnitOfWork(); @@ -789,8 +795,7 @@ namespace Umbraco.Core.Services { using (var repository = RepositoryFactory.CreateContentRepository(UowProvider.GetUnitOfWork())) { - var query = Query.Builder.Where(x => x.Trashed == false); - return repository.GetByPublishedVersion(query); + return repository.GetByPublishedVersion(_notTrashedQuery); } } diff --git a/src/Umbraco.Core/Services/EntityService.cs b/src/Umbraco.Core/Services/EntityService.cs index 257416d054..a6b84704fc 100644 --- a/src/Umbraco.Core/Services/EntityService.cs +++ b/src/Umbraco.Core/Services/EntityService.cs @@ -62,6 +62,12 @@ namespace Umbraco.Core.Services } + #region Static Queries + + private readonly IQuery _rootEntityQuery = Query.Builder.Where(x => x.ParentId == -1); + + #endregion + /// /// Returns the integer id for a given GUID /// @@ -389,8 +395,7 @@ namespace Umbraco.Core.Services var objectTypeId = umbracoObjectType.GetGuid(); using (var repository = RepositoryFactory.CreateEntityRepository(UowProvider.GetUnitOfWork())) { - var query = Query.Builder.Where(x => x.ParentId == -1); - var entities = repository.GetByQuery(query, objectTypeId); + var entities = repository.GetByQuery(_rootEntityQuery, objectTypeId); return entities; } diff --git a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs index 922badc33b..2b1561012b 100644 --- a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs @@ -51,9 +51,7 @@ namespace Umbraco.Tests.Benchmarks private readonly ISqlSyntaxProvider _syntaxProvider = new SqlCeSyntaxProvider(); private readonly BaseMapper _contentMapper; private readonly CachedExpression _cachedExpression; - //private static readonly Expression> TemplatePredicate = content => - // content.Path.StartsWith(string.Empty) && content.Published && (content.ContentTypeId == 0 || content.ContentTypeId == 0); - + [Benchmark(Baseline = true)] public void WithNonCached() { diff --git a/src/UmbracoExamine/UmbracoContentIndexer.cs b/src/UmbracoExamine/UmbracoContentIndexer.cs index 4fcf51deae..f69e7e59bb 100644 --- a/src/UmbracoExamine/UmbracoContentIndexer.cs +++ b/src/UmbracoExamine/UmbracoContentIndexer.cs @@ -347,6 +347,11 @@ namespace UmbracoExamine #region Protected + /// + /// This is a static query, it's parameters don't change so store statically + /// + private static readonly IQuery PublishedQuery = Query.Builder.Where(x => x.Published == true); + protected override void PerformIndexAll(string type) { const int pageSize = 10000; @@ -376,9 +381,7 @@ namespace UmbracoExamine else { //add the published filter - var qry = Query.Builder.Where(x => x.Published == true); - - descendants = _contentService.GetPagedDescendants(contentParentId, pageIndex, pageSize, out total, "Path", Direction.Ascending, true, qry); + descendants = _contentService.GetPagedDescendants(contentParentId, pageIndex, pageSize, out total, "Path", Direction.Ascending, true, PublishedQuery); } //if specific types are declared we need to post filter them From e2d8a2808708138a4b9949d259eec418e2813da7 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 27 Oct 2016 18:45:42 +0200 Subject: [PATCH 06/18] ensures the generic base class static query instance is lazily created --- src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs | 2 +- .../Persistence/Repositories/RepositoryBase.cs | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs b/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs index ea44964219..6909c77744 100644 --- a/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs +++ b/src/Umbraco.Core/Persistence/Mappers/MappingResolver.cs @@ -43,7 +43,7 @@ namespace Umbraco.Core.Persistence.Mappers { return byAttribute.Result; } - throw new Exception("Invalid Type: A Mapper could not be resolved based on the passed in Type"); + throw new Exception("Invalid Type: A Mapper could not be resolved based on the passed in Type " + type); }); } diff --git a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs index 9379d14af8..41946d48d4 100644 --- a/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs +++ b/src/Umbraco.Core/Persistence/Repositories/RepositoryBase.cs @@ -85,7 +85,7 @@ namespace Umbraco.Core.Persistence.Repositories #region Static Queries - private readonly IQuery _hasIdQuery = Query.Builder.Where(x => x.Id != 0); + private IQuery _hasIdQuery; #endregion @@ -117,6 +117,12 @@ namespace Umbraco.Core.Persistence.Repositories RuntimeCache, new RepositoryCachePolicyOptions(() => { + //create it once if it is needed (no need for locking here) + if (_hasIdQuery == null) + { + _hasIdQuery = Query.Builder.Where(x => x.Id != 0); + } + //Get count of all entities of current type (TEntity) to ensure cached result is correct return PerformCount(_hasIdQuery); }))); From 958392f09b193a972e071033207574dc94387c97 Mon Sep 17 00:00:00 2001 From: Stephan Date: Fri, 28 Oct 2016 16:30:20 +0200 Subject: [PATCH 07/18] U4-9105 - bit of cleanup --- .../Persistence/PetaPocoSqlExtensions.cs | 2 +- ...sionHelper.cs => ExpressionVisitorBase.cs} | 1841 ++++++++--------- ...lper.cs => ModelToSqlExpressionVisitor.cs} | 153 +- ...elper.cs => PocoToSqlExpressionVisitor.cs} | 150 +- .../Persistence/Querying/Query.cs | 2 +- src/Umbraco.Core/Umbraco.Core.csproj | 6 +- .../ModelToSqlExpressionHelperBenchmarks.cs | 4 +- .../Persistence/Querying/ExpressionTests.cs | 20 +- 8 files changed, 1077 insertions(+), 1101 deletions(-) rename src/Umbraco.Core/Persistence/Querying/{BaseExpressionHelper.cs => ExpressionVisitorBase.cs} (81%) rename src/Umbraco.Core/Persistence/Querying/{ModelToSqlExpressionHelper.cs => ModelToSqlExpressionVisitor.cs} (65%) rename src/Umbraco.Core/Persistence/Querying/{PocoToSqlExpressionHelper.cs => PocoToSqlExpressionVisitor.cs} (73%) diff --git a/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs b/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs index f5c0e0e616..d2fa98ef12 100644 --- a/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs +++ b/src/Umbraco.Core/Persistence/PetaPocoSqlExtensions.cs @@ -29,7 +29,7 @@ namespace Umbraco.Core.Persistence public static Sql Where(this Sql sql, Expression> predicate) { - var expresionist = new PocoToSqlExpressionHelper(); + var expresionist = new PocoToSqlExpressionVisitor(); var whereExpression = expresionist.Visit(predicate); return sql.Where(whereExpression, expresionist.GetSqlParameters()); } diff --git a/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs similarity index 81% rename from src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs rename to src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs index 4bbd86d66b..678ceb1d8e 100644 --- a/src/Umbraco.Core/Persistence/Querying/BaseExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/ExpressionVisitorBase.cs @@ -1,927 +1,916 @@ -using System; -using System.Collections; -using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Globalization; -using System.Linq; -using System.Linq.Expressions; -using System.Text; -using Umbraco.Core.Persistence.SqlSyntax; - -namespace Umbraco.Core.Persistence.Querying -{ - - /// - /// This is used to determine if the expression result is cached and therefore only the SQL parameters will be extracted - /// - /// - /// This saves some performance overhead since the SQL string itself does not get generated because it already exists. - /// - internal class CachedExpression : Expression - { - public CachedExpression() - { - CompiledOutput = null; - } - - public Expression InnerExpression { get; private set; } - - /// - /// The compiled SQL statement output - /// - public string CompiledOutput { get; private set; } - - public bool IsCompiled - { - get { return CompiledOutput.IsNullOrWhiteSpace() == false; } - } - - public void Compile(string output) - { - if (IsCompiled) - throw new InvalidOperationException("Cached expression is already compiled"); - - CompiledOutput = output; - } - - public void Wrap(Expression exp) - { - InnerExpression = exp; - } - } - - /// - /// An expression tree parser to create SQL statements and SQL parameters based on a given strongly typed expression - /// - /// - /// Logic that is shared with the expression helpers. This object stores state, it cannot be re-used to parse an expression. - /// - internal abstract class BaseExpressionHelper - { - protected BaseExpressionHelper(ISqlSyntaxProvider sqlSyntax) - { - SqlSyntax = sqlSyntax; - } - - /// - /// Indicates that the SQL statement has already been compiled, so Visiting will just generate the Sql Parameters - /// - protected bool IsCompiled { get; set; } - - protected ISqlSyntaxProvider SqlSyntax { get; private set; } - - protected List SqlParameters = new List(); - - protected abstract string VisitMemberAccess(MemberExpression m); - - protected internal virtual string Visit(Expression exp) - { - //set the flag if it is already compiled - var compiledExp = exp as CachedExpression; - if (compiledExp != null) - { - if (compiledExp.IsCompiled) - { - IsCompiled = true; - } - exp = compiledExp.InnerExpression; - } - - if (exp == null) return string.Empty; - - string result; - - switch (exp.NodeType) - { - case ExpressionType.Lambda: - result = VisitLambda(exp as LambdaExpression); - break; - case ExpressionType.MemberAccess: - result = VisitMemberAccess(exp as MemberExpression); - break; - case ExpressionType.Constant: - result = VisitConstant(exp as ConstantExpression); - break; - case ExpressionType.Add: - case ExpressionType.AddChecked: - case ExpressionType.Subtract: - case ExpressionType.SubtractChecked: - case ExpressionType.Multiply: - case ExpressionType.MultiplyChecked: - case ExpressionType.Divide: - case ExpressionType.Modulo: - case ExpressionType.And: - case ExpressionType.AndAlso: - case ExpressionType.Or: - case ExpressionType.OrElse: - case ExpressionType.LessThan: - case ExpressionType.LessThanOrEqual: - case ExpressionType.GreaterThan: - case ExpressionType.GreaterThanOrEqual: - case ExpressionType.Equal: - case ExpressionType.NotEqual: - case ExpressionType.Coalesce: - case ExpressionType.ArrayIndex: - case ExpressionType.RightShift: - case ExpressionType.LeftShift: - case ExpressionType.ExclusiveOr: - result = VisitBinary(exp as BinaryExpression); - break; - case ExpressionType.Negate: - case ExpressionType.NegateChecked: - case ExpressionType.Not: - case ExpressionType.Convert: - case ExpressionType.ConvertChecked: - case ExpressionType.ArrayLength: - case ExpressionType.Quote: - case ExpressionType.TypeAs: - result = VisitUnary(exp as UnaryExpression); - break; - case ExpressionType.Parameter: - result = VisitParameter(exp as ParameterExpression); - break; - case ExpressionType.Call: - result = VisitMethodCall(exp as MethodCallExpression); - break; - case ExpressionType.New: - result = VisitNew(exp as NewExpression); - break; - case ExpressionType.NewArrayInit: - case ExpressionType.NewArrayBounds: - result = VisitNewArray(exp as NewArrayExpression); - break; - default: - result = exp.ToString(); - break; - } - - if (compiledExp != null) - { - if (compiledExp.IsCompiled == false) - { - compiledExp.Compile(result); - } - return compiledExp.CompiledOutput; - } - - return result; - - } - - protected virtual string VisitLambda(LambdaExpression lambda) - { - if (lambda.Body.NodeType == ExpressionType.MemberAccess) - { - var m = lambda.Body as MemberExpression; - - if (m != null && m.Expression != null) - { - //This deals with members that are boolean (i.e. x => IsTrashed ) - string r = VisitMemberAccess(m); - - SqlParameters.Add(true); - - //don't execute if compiled - if (IsCompiled == false) - { - return string.Format("{0} = @{1}", r, SqlParameters.Count - 1); - } - //already compiled, return - return string.Empty; - } - - } - return Visit(lambda.Body); - } - - protected virtual string VisitBinary(BinaryExpression b) - { - var left = string.Empty; - var right = string.Empty; - - var operand = BindOperant(b.NodeType); - if (operand == "AND" || operand == "OR") - { - MemberExpression m = b.Left as MemberExpression; - if (m != null && m.Expression != null) - { - string r = VisitMemberAccess(m); - - SqlParameters.Add(1); - - //don't execute if compiled - if (IsCompiled == false) - { - left = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); - } - } - else - { - left = Visit(b.Left); - } - m = b.Right as MemberExpression; - if (m != null && m.Expression != null) - { - string r = VisitMemberAccess(m); - - SqlParameters.Add(1); - - //don't execute if compiled - if (IsCompiled == false) - { - right = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); - } - } - else - { - right = Visit(b.Right); - } - } - else if (operand == "=") - { - // deal with (x == true|false) - most common - var constRight = b.Right as ConstantExpression; - if (constRight != null && constRight.Type == typeof(bool)) - return ((bool)constRight.Value) ? VisitNotNot(b.Left) : VisitNot(b.Left); - right = Visit(b.Right); - - // deal with (true|false == x) - why not - var constLeft = b.Left as ConstantExpression; - if (constLeft != null && constLeft.Type == typeof(bool)) - return ((bool)constLeft.Value) ? VisitNotNot(b.Right) : VisitNot(b.Right); - left = Visit(b.Left); - } - else if (operand == "<>") - { - // deal with (x != true|false) - most common - var constRight = b.Right as ConstantExpression; - if (constRight != null && constRight.Type == typeof(bool)) - return ((bool)constRight.Value) ? VisitNot(b.Left) : VisitNotNot(b.Left); - right = Visit(b.Right); - - // deal with (true|false != x) - why not - var constLeft = b.Left as ConstantExpression; - if (constLeft != null && constLeft.Type == typeof(bool)) - return ((bool)constLeft.Value) ? VisitNot(b.Right) : VisitNotNot(b.Right); - left = Visit(b.Left); - } - else - { - left = Visit(b.Left); - right = Visit(b.Right); - } - - if (operand == "=" && right == "null") operand = "is"; - else if (operand == "<>" && right == "null") operand = "is not"; - else if (operand == "=" || operand == "<>") - { - //if (IsTrueExpression(right)) right = GetQuotedTrueValue(); - //else if (IsFalseExpression(right)) right = GetQuotedFalseValue(); - - //if (IsTrueExpression(left)) left = GetQuotedTrueValue(); - //else if (IsFalseExpression(left)) left = GetQuotedFalseValue(); - - } - - switch (operand) - { - case "MOD": - case "COALESCE": - //don't execute if compiled - if (IsCompiled == false) - { - return string.Format("{0}({1},{2})", operand, left, right); - } - //already compiled, return - return string.Empty; - default: - //don't execute if compiled - if (IsCompiled == false) - { - return string.Concat("(", left, " ", operand, " ", right, ")"); - } - //already compiled, return - return string.Empty; - } - } - - protected virtual List VisitExpressionList(ReadOnlyCollection original) - { - var list = new List(); - for (int i = 0, n = original.Count; i < n; i++) - { - if (original[i].NodeType == ExpressionType.NewArrayInit || - original[i].NodeType == ExpressionType.NewArrayBounds) - { - - list.AddRange(VisitNewArrayFromExpressionList(original[i] as NewArrayExpression)); - } - else - list.Add(Visit(original[i])); - - } - return list; - } - - protected virtual string VisitNew(NewExpression nex) - { - // TODO : check ! - var member = Expression.Convert(nex, typeof(object)); - var lambda = Expression.Lambda>(member); - try - { - var getter = lambda.Compile(); - object o = getter(); - - SqlParameters.Add(o); - - //don't execute if compiled - if (IsCompiled == false) - { - return string.Format("@{0}", SqlParameters.Count - 1); - } - //already compiled, return - return string.Empty; - } - catch (InvalidOperationException) - { - //don't execute if compiled - if (IsCompiled == false) - { - // FieldName ? - List exprs = VisitExpressionList(nex.Arguments); - var r = new StringBuilder(); - foreach (Object e in exprs) - { - r.AppendFormat("{0}{1}", - r.Length > 0 ? "," : "", - e); - } - return r.ToString(); - } - //already compiled, return - return string.Empty; - } - - } - - protected virtual string VisitParameter(ParameterExpression p) - { - return p.Name; - } - - protected virtual string VisitConstant(ConstantExpression c) - { - if (c.Value == null) - return "null"; - - SqlParameters.Add(c.Value); - - //don't execute if compiled - if (IsCompiled == false) - { - return string.Format("@{0}", SqlParameters.Count - 1); - } - //already compiled, return - return string.Empty; - } - - protected virtual string VisitUnary(UnaryExpression u) - { - switch (u.NodeType) - { - case ExpressionType.Not: - return VisitNot(u.Operand); - default: - return Visit(u.Operand); - } - } - - private string VisitNot(Expression exp) - { - var o = Visit(exp); - - // use a "NOT (...)" syntax instead of "<>" since we don't know whether "<>" works in all sql servers - // also, x.StartsWith(...) translates to "x LIKE '...%'" which we cannot "<>" and have to "NOT (...") - - switch (exp.NodeType) - { - case ExpressionType.MemberAccess: - // false property , i.e. x => !Trashed - SqlParameters.Add(true); - //don't execute if compiled - if (IsCompiled == false) - { - return string.Format("NOT ({0} = @{1})", o, SqlParameters.Count - 1); - } - //already compiled, return - return string.Empty; - default: - //don't execute if compiled - if (IsCompiled == false) - { - // could be anything else, such as: x => !x.Path.StartsWith("-20") - return string.Concat("NOT (", o, ")"); - } - //already compiled, return - return string.Empty; - } - } - - private string VisitNotNot(Expression exp) - { - var o = Visit(exp); - - switch (exp.NodeType) - { - case ExpressionType.MemberAccess: - // true property, i.e. x => Trashed - SqlParameters.Add(true); - - //don't execute if compiled - if (IsCompiled == false) - { - return string.Format("({0} = @{1})", o, SqlParameters.Count - 1); - } - //already compiled, return - return string.Empty; - default: - // could be anything else, such as: x => x.Path.StartsWith("-20") - return o; - } - } - - protected virtual string VisitNewArray(NewArrayExpression na) - { - List exprs = VisitExpressionList(na.Expressions); - - //don't execute if compiled - if (IsCompiled == false) - { - var r = new StringBuilder(); - foreach (Object e in exprs) - { - r.Append(r.Length > 0 ? "," + e : e); - } - - return r.ToString(); - } - //already compiled, return - return string.Empty; - - } - - protected virtual List VisitNewArrayFromExpressionList(NewArrayExpression na) - { - - List exprs = VisitExpressionList(na.Expressions); - return exprs; - } - - protected virtual string BindOperant(ExpressionType e) - { - - switch (e) - { - case ExpressionType.Equal: - return "="; - case ExpressionType.NotEqual: - return "<>"; - case ExpressionType.GreaterThan: - return ">"; - case ExpressionType.GreaterThanOrEqual: - return ">="; - case ExpressionType.LessThan: - return "<"; - case ExpressionType.LessThanOrEqual: - return "<="; - case ExpressionType.AndAlso: - return "AND"; - case ExpressionType.OrElse: - return "OR"; - case ExpressionType.Add: - return "+"; - case ExpressionType.Subtract: - return "-"; - case ExpressionType.Multiply: - return "*"; - case ExpressionType.Divide: - return "/"; - case ExpressionType.Modulo: - return "MOD"; - case ExpressionType.Coalesce: - return "COALESCE"; - default: - return e.ToString(); - } - } - - protected virtual string VisitMethodCall(MethodCallExpression m) - { - //Here's what happens with a MethodCallExpression: - // If a method is called that contains a single argument, - // then m.Object is the object on the left hand side of the method call, example: - // x.Path.StartsWith(content.Path) - // m.Object = x.Path - // and m.Arguments.Length == 1, therefor m.Arguments[0] == content.Path - // If a method is called that contains multiple arguments, then m.Object == null and the - // m.Arguments collection contains the left hand side of the method call, example: - // x.Path.SqlStartsWith(content.Path, TextColumnType.NVarchar) - // m.Object == null - // m.Arguments.Length == 3, therefor, m.Arguments[0] == x.Path, m.Arguments[1] == content.Path, m.Arguments[2] == TextColumnType.NVarchar - // So, we need to cater for these scenarios. - - var objectForMethod = m.Object ?? m.Arguments[0]; - var visitedObjectForMethod = Visit(objectForMethod); - var methodArgs = m.Object == null - ? m.Arguments.Skip(1).ToArray() - : m.Arguments.ToArray(); - - switch (m.Method.Name) - { - case "ToString": - SqlParameters.Add(objectForMethod.ToString()); - //don't execute if compiled - if (IsCompiled == false) - return string.Format("@{0}", SqlParameters.Count - 1); - //already compiled, return - return string.Empty; - case "ToUpper": - //don't execute if compiled - if (IsCompiled == false) - return string.Format("upper({0})", visitedObjectForMethod); - //already compiled, return - return string.Empty; - case "ToLower": - //don't execute if compiled - if (IsCompiled == false) - return string.Format("lower({0})", visitedObjectForMethod); - //already compiled, return - return string.Empty; - case "SqlWildcard": - case "StartsWith": - case "EndsWith": - case "Contains": - case "Equals": - case "SqlStartsWith": - case "SqlEndsWith": - case "SqlContains": - case "SqlEquals": - case "InvariantStartsWith": - case "InvariantEndsWith": - case "InvariantContains": - case "InvariantEquals": - - string compareValue; - - if (methodArgs[0].NodeType != ExpressionType.Constant) - { - //This occurs when we are getting a value from a non constant such as: x => x.Path.StartsWith(content.Path) - // So we'll go get the value: - var member = Expression.Convert(methodArgs[0], typeof(object)); - var lambda = Expression.Lambda>(member); - var getter = lambda.Compile(); - compareValue = getter().ToString(); - } - else - { - compareValue = methodArgs[0].ToString(); - } - - //special case, if it is 'Contains' and the member that Contains is being called on is not a string, then - // we should be doing an 'In' clause - but we currently do not support this - if (methodArgs[0].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[0].Type)) - { - throw new NotSupportedException("An array Contains method is not supported"); - } - - //default column type - var colType = TextColumnType.NVarchar; - - //then check if the col type argument has been passed to the current method (this will be the case for methods like - // SqlContains and other Sql methods) - if (methodArgs.Length > 1) - { - var colTypeArg = methodArgs.FirstOrDefault(x => x is ConstantExpression && x.Type == typeof(TextColumnType)); - if (colTypeArg != null) - { - colType = (TextColumnType)((ConstantExpression)colTypeArg).Value; - } - } - - return HandleStringComparison(visitedObjectForMethod, compareValue, m.Method.Name, colType); - - case "Replace": - string searchValue; - - if (methodArgs[0].NodeType != ExpressionType.Constant) - { - //This occurs when we are getting a value from a non constant such as: x => x.Path.StartsWith(content.Path) - // So we'll go get the value: - var member = Expression.Convert(methodArgs[0], typeof(object)); - var lambda = Expression.Lambda>(member); - var getter = lambda.Compile(); - searchValue = getter().ToString(); - } - else - { - searchValue = methodArgs[0].ToString(); - } - - if (methodArgs[0].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[0].Type)) - { - throw new NotSupportedException("An array Contains method is not supported"); - } - - string replaceValue; - - if (methodArgs[1].NodeType != ExpressionType.Constant) - { - //This occurs when we are getting a value from a non constant such as: x => x.Path.StartsWith(content.Path) - // So we'll go get the value: - var member = Expression.Convert(methodArgs[1], typeof(object)); - var lambda = Expression.Lambda>(member); - var getter = lambda.Compile(); - replaceValue = getter().ToString(); - } - else - { - replaceValue = methodArgs[1].ToString(); - } - - if (methodArgs[1].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[1].Type)) - { - throw new NotSupportedException("An array Contains method is not supported"); - } - - SqlParameters.Add(RemoveQuote(searchValue)); - - SqlParameters.Add(RemoveQuote(replaceValue)); - - //don't execute if compiled - if (IsCompiled == false) - return string.Format("replace({0}, @{1}, @{2})", visitedObjectForMethod, SqlParameters.Count - 2, SqlParameters.Count - 1); - //already compiled, return - return string.Empty; - - //case "Substring": - // var startIndex = Int32.Parse(args[0].ToString()) + 1; - // if (args.Count == 2) - // { - // var length = Int32.Parse(args[1].ToString()); - // return string.Format("substring({0} from {1} for {2})", - // r, - // startIndex, - // length); - // } - // else - // return string.Format("substring({0} from {1})", - // r, - // startIndex); - //case "Round": - //case "Floor": - //case "Ceiling": - //case "Coalesce": - //case "Abs": - //case "Sum": - // return string.Format("{0}({1}{2})", - // m.Method.Name, - // r, - // args.Count == 1 ? string.Format(",{0}", args[0]) : ""); - //case "Concat": - // var s = new StringBuilder(); - // foreach (Object e in args) - // { - // s.AppendFormat(" || {0}", e); - // } - // return string.Format("{0}{1}", r, s); - - //case "In": - - // var member = Expression.Convert(m.Arguments[0], typeof(object)); - // var lambda = Expression.Lambda>(member); - // var getter = lambda.Compile(); - - // var inArgs = (object[])getter(); - - // var sIn = new StringBuilder(); - // foreach (var e in inArgs) - // { - // SqlParameters.Add(e); - - // sIn.AppendFormat("{0}{1}", - // sIn.Length > 0 ? "," : "", - // string.Format("@{0}", SqlParameters.Count - 1)); - - // //sIn.AppendFormat("{0}{1}", - // // sIn.Length > 0 ? "," : "", - // // GetQuotedValue(e, e.GetType())); - // } - - // return string.Format("{0} {1} ({2})", r, m.Method.Name, sIn.ToString()); - //case "Desc": - // return string.Format("{0} DESC", r); - //case "Alias": - //case "As": - // return string.Format("{0} As {1}", r, - // GetQuotedColumnName(RemoveQuoteFromAlias(RemoveQuote(args[0].ToString())))); - - default: - - throw new ArgumentOutOfRangeException("No logic supported for " + m.Method.Name); - - //var s2 = new StringBuilder(); - //foreach (Object e in args) - //{ - // s2.AppendFormat(",{0}", GetQuotedValue(e, e.GetType())); - //} - //return string.Format("{0}({1}{2})", m.Method.Name, r, s2.ToString()); - } - } - - public virtual string GetQuotedTableName(string tableName) - { - //already compiled, return - if (IsCompiled) - return tableName; - - return string.Format("\"{0}\"", tableName); - } - - public virtual string GetQuotedColumnName(string columnName) - { - //already compiled, return - if (IsCompiled) - return columnName; - - return string.Format("\"{0}\"", columnName); - } - - public virtual string GetQuotedName(string name) - { - //already compiled, return - if (IsCompiled) - return name; - - return string.Format("\"{0}\"", name); - } - - public object[] GetSqlParameters() - { - return SqlParameters.ToArray(); - } - - protected string HandleStringComparison(string col, string val, string verb, TextColumnType columnType) - { - switch (verb) - { - case "SqlWildcard": - SqlParameters.Add(RemoveQuote(val)); - //don't execute if compiled - if (IsCompiled == false) - return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); - //already compiled, return - return string.Empty; - case "Equals": - SqlParameters.Add(RemoveQuote(val)); - //don't execute if compiled - if (IsCompiled == false) - return SqlSyntax.GetStringColumnEqualComparison(col, SqlParameters.Count - 1, columnType); - //already compiled, return - return string.Empty; - case "StartsWith": - SqlParameters.Add(string.Format("{0}{1}", - RemoveQuote(val), - SqlSyntax.GetWildcardPlaceholder())); - //don't execute if compiled - if (IsCompiled == false) - return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); - //already compiled, return - return string.Empty; - case "EndsWith": - SqlParameters.Add(string.Format("{0}{1}", - SqlSyntax.GetWildcardPlaceholder(), - RemoveQuote(val))); - //don't execute if compiled - if (IsCompiled == false) - return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); - //already compiled, return - return string.Empty; - case "Contains": - SqlParameters.Add(string.Format("{0}{1}{0}", - SqlSyntax.GetWildcardPlaceholder(), - RemoveQuote(val))); - //don't execute if compiled - if (IsCompiled == false) - return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); - //already compiled, return - return string.Empty; - case "InvariantEquals": - case "SqlEquals": - //recurse - return HandleStringComparison(col, val, "Equals", columnType); - case "InvariantStartsWith": - case "SqlStartsWith": - //recurse - return HandleStringComparison(col, val, "StartsWith", columnType); - case "InvariantEndsWith": - case "SqlEndsWith": - //recurse - return HandleStringComparison(col, val, "EndsWith", columnType); - case "InvariantContains": - case "SqlContains": - //recurse - return HandleStringComparison(col, val, "Contains", columnType); - default: - throw new ArgumentOutOfRangeException("verb"); - } - } - - //public virtual string GetQuotedValue(object value, Type fieldType, Func escapeCallback = null, Func shouldQuoteCallback = null) - //{ - // if (value == null) return "NULL"; - - // if (escapeCallback == null) - // { - // escapeCallback = EscapeParam; - // } - // if (shouldQuoteCallback == null) - // { - // shouldQuoteCallback = ShouldQuoteValue; - // } - - // if (!fieldType.UnderlyingSystemType.IsValueType && fieldType != typeof(string)) - // { - // //if (TypeSerializer.CanCreateFromString(fieldType)) - // //{ - // // return "'" + escapeCallback(TypeSerializer.SerializeToString(value)) + "'"; - // //} - - // throw new NotSupportedException( - // string.Format("Property of type: {0} is not supported", fieldType.FullName)); - // } - - // if (fieldType == typeof(int)) - // return ((int)value).ToString(CultureInfo.InvariantCulture); - - // if (fieldType == typeof(float)) - // return ((float)value).ToString(CultureInfo.InvariantCulture); - - // if (fieldType == typeof(double)) - // return ((double)value).ToString(CultureInfo.InvariantCulture); - - // if (fieldType == typeof(decimal)) - // return ((decimal)value).ToString(CultureInfo.InvariantCulture); - - // if (fieldType == typeof(DateTime)) - // { - // return "'" + escapeCallback(((DateTime)value).ToIsoString()) + "'"; - // } - - // if (fieldType == typeof(bool)) - // return ((bool)value) ? Convert.ToString(1, CultureInfo.InvariantCulture) : Convert.ToString(0, CultureInfo.InvariantCulture); - - // return shouldQuoteCallback(fieldType) - // ? "'" + escapeCallback(value) + "'" - // : value.ToString(); - //} - - public virtual string EscapeParam(object paramValue) - { - return paramValue == null - ? string.Empty - : SqlSyntax.EscapeString(paramValue.ToString()); - } - - public virtual bool ShouldQuoteValue(Type fieldType) - { - return true; - } - - protected virtual string RemoveQuote(string exp) - { - if ((exp.StartsWith("\"") || exp.StartsWith("`") || exp.StartsWith("'")) - && - (exp.EndsWith("\"") || exp.EndsWith("`") || exp.EndsWith("'"))) - { - exp = exp.Remove(0, 1); - exp = exp.Remove(exp.Length - 1, 1); - } - return exp; - } - - //protected virtual string RemoveQuoteFromAlias(string exp) - //{ - - // if ((exp.StartsWith("\"") || exp.StartsWith("`") || exp.StartsWith("'")) - // && - // (exp.EndsWith("\"") || exp.EndsWith("`") || exp.EndsWith("'"))) - // { - // exp = exp.Remove(0, 1); - // exp = exp.Remove(exp.Length - 1, 1); - // } - // return exp; - //} - } +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.Linq; +using System.Linq.Expressions; +using System.Text; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Querying +{ + /// + /// Represents an expression which caches the visitor's result. + /// + internal class CachedExpression : Expression + { + private string _visitResult; + + /// + /// Gets or sets the inner Expression. + /// + public Expression InnerExpression { get; private set; } + + /// + /// Gets or sets the compiled SQL statement output. + /// + public string VisitResult + { + get { return _visitResult; } + set + { + if (Visited) + throw new InvalidOperationException("Cached expression has already been visited."); + _visitResult = value; + Visited = true; + } + } + + /// + /// Gets or sets a value indicating whether the cache Expression has been compiled already. + /// + public bool Visited { get; private set; } + + /// + /// Replaces the inner expression. + /// + /// expression. + /// The new expression is assumed to have different parameter but produce the same SQL statement. + public void Wrap(Expression expression) + { + InnerExpression = expression; + } + } + + /// + /// An expression tree parser to create SQL statements and SQL parameters based on a strongly typed expression. + /// + /// This object is stateful and cannot be re-used to parse an expression. + internal abstract class ExpressionVisitorBase + { + protected ExpressionVisitorBase(ISqlSyntaxProvider sqlSyntax) + { + SqlSyntax = sqlSyntax; + } + + /// + /// Gets or sets a value indicating whether the visited expression has been visited already, + /// in which case visiting will just populate the SQL parameters. + /// + protected bool Visited { get; set; } + + /// + /// Gets or sets the SQL syntax provider for the current database. + /// + protected ISqlSyntaxProvider SqlSyntax { get; private set; } + + /// + /// Gets the list of SQL parameters. + /// + protected readonly List SqlParameters = new List(); + + /// + /// Gets the SQL parameters. + /// + /// + public object[] GetSqlParameters() + { + return SqlParameters.ToArray(); + } + + /// + /// Visits the expression and produces the corresponding SQL statement. + /// + /// The expression + /// The SQL statement corresponding to the expression. + /// Also populates the SQL parameters. + public virtual string Visit(Expression expression) + { + // if the expression is a CachedExpression, + // visit the inner expression if not already visited + var cachedExpression = expression as CachedExpression; + if (cachedExpression != null) + { + Visited = cachedExpression.Visited; + expression = cachedExpression.InnerExpression; + } + + if (expression == null) return string.Empty; + + string result; + + switch (expression.NodeType) + { + case ExpressionType.Lambda: + result = VisitLambda(expression as LambdaExpression); + break; + case ExpressionType.MemberAccess: + result = VisitMemberAccess(expression as MemberExpression); + break; + case ExpressionType.Constant: + result = VisitConstant(expression as ConstantExpression); + break; + case ExpressionType.Add: + case ExpressionType.AddChecked: + case ExpressionType.Subtract: + case ExpressionType.SubtractChecked: + case ExpressionType.Multiply: + case ExpressionType.MultiplyChecked: + case ExpressionType.Divide: + case ExpressionType.Modulo: + case ExpressionType.And: + case ExpressionType.AndAlso: + case ExpressionType.Or: + case ExpressionType.OrElse: + case ExpressionType.LessThan: + case ExpressionType.LessThanOrEqual: + case ExpressionType.GreaterThan: + case ExpressionType.GreaterThanOrEqual: + case ExpressionType.Equal: + case ExpressionType.NotEqual: + case ExpressionType.Coalesce: + case ExpressionType.ArrayIndex: + case ExpressionType.RightShift: + case ExpressionType.LeftShift: + case ExpressionType.ExclusiveOr: + result = VisitBinary(expression as BinaryExpression); + break; + case ExpressionType.Negate: + case ExpressionType.NegateChecked: + case ExpressionType.Not: + case ExpressionType.Convert: + case ExpressionType.ConvertChecked: + case ExpressionType.ArrayLength: + case ExpressionType.Quote: + case ExpressionType.TypeAs: + result = VisitUnary(expression as UnaryExpression); + break; + case ExpressionType.Parameter: + result = VisitParameter(expression as ParameterExpression); + break; + case ExpressionType.Call: + result = VisitMethodCall(expression as MethodCallExpression); + break; + case ExpressionType.New: + result = VisitNew(expression as NewExpression); + break; + case ExpressionType.NewArrayInit: + case ExpressionType.NewArrayBounds: + result = VisitNewArray(expression as NewArrayExpression); + break; + default: + result = expression.ToString(); + break; + } + + // if the expression is a CachedExpression, + // and is not already compiled, assign the result + if (cachedExpression != null) + { + if (cachedExpression.Visited == false) + cachedExpression.VisitResult = result; + result = cachedExpression.VisitResult; + } + + return result; + } + + protected abstract string VisitMemberAccess(MemberExpression m); + + protected virtual string VisitLambda(LambdaExpression lambda) + { + if (lambda.Body.NodeType == ExpressionType.MemberAccess) + { + var m = lambda.Body as MemberExpression; + + if (m != null && m.Expression != null) + { + //This deals with members that are boolean (i.e. x => IsTrashed ) + var r = VisitMemberAccess(m); + + SqlParameters.Add(true); + + return Visited ? string.Empty : string.Format("{0} = @{1}", r, SqlParameters.Count - 1); + } + + } + return Visit(lambda.Body); + } + + protected virtual string VisitBinary(BinaryExpression b) + { + var left = string.Empty; + var right = string.Empty; + + var operand = BindOperant(b.NodeType); + if (operand == "AND" || operand == "OR") + { + var m = b.Left as MemberExpression; + if (m != null && m.Expression != null) + { + string r = VisitMemberAccess(m); + + SqlParameters.Add(1); + + //don't execute if compiled + if (Visited == false) + { + left = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); + } + } + else + { + left = Visit(b.Left); + } + m = b.Right as MemberExpression; + if (m != null && m.Expression != null) + { + var r = VisitMemberAccess(m); + + SqlParameters.Add(1); + + //don't execute if compiled + if (Visited == false) + { + right = string.Format("{0} = @{1}", r, SqlParameters.Count - 1); + } + } + else + { + right = Visit(b.Right); + } + } + else if (operand == "=") + { + // deal with (x == true|false) - most common + var constRight = b.Right as ConstantExpression; + if (constRight != null && constRight.Type == typeof(bool)) + return (bool)constRight.Value ? VisitNotNot(b.Left) : VisitNot(b.Left); + right = Visit(b.Right); + + // deal with (true|false == x) - why not + var constLeft = b.Left as ConstantExpression; + if (constLeft != null && constLeft.Type == typeof(bool)) + return (bool)constLeft.Value ? VisitNotNot(b.Right) : VisitNot(b.Right); + left = Visit(b.Left); + } + else if (operand == "<>") + { + // deal with (x != true|false) - most common + var constRight = b.Right as ConstantExpression; + if (constRight != null && constRight.Type == typeof(bool)) + return (bool)constRight.Value ? VisitNot(b.Left) : VisitNotNot(b.Left); + right = Visit(b.Right); + + // deal with (true|false != x) - why not + var constLeft = b.Left as ConstantExpression; + if (constLeft != null && constLeft.Type == typeof(bool)) + return (bool)constLeft.Value ? VisitNot(b.Right) : VisitNotNot(b.Right); + left = Visit(b.Left); + } + else + { + left = Visit(b.Left); + right = Visit(b.Right); + } + + if (operand == "=" && right == "null") operand = "is"; + else if (operand == "<>" && right == "null") operand = "is not"; + else if (operand == "=" || operand == "<>") + { + //if (IsTrueExpression(right)) right = GetQuotedTrueValue(); + //else if (IsFalseExpression(right)) right = GetQuotedFalseValue(); + + //if (IsTrueExpression(left)) left = GetQuotedTrueValue(); + //else if (IsFalseExpression(left)) left = GetQuotedFalseValue(); + + } + + switch (operand) + { + case "MOD": + case "COALESCE": + //don't execute if compiled + if (Visited == false) + { + return string.Format("{0}({1},{2})", operand, left, right); + } + //already compiled, return + return string.Empty; + default: + //don't execute if compiled + if (Visited == false) + { + return string.Concat("(", left, " ", operand, " ", right, ")"); + } + //already compiled, return + return string.Empty; + } + } + + protected virtual List VisitExpressionList(ReadOnlyCollection original) + { + var list = new List(); + for (int i = 0, n = original.Count; i < n; i++) + { + if (original[i].NodeType == ExpressionType.NewArrayInit || + original[i].NodeType == ExpressionType.NewArrayBounds) + { + list.AddRange(VisitNewArrayFromExpressionList(original[i] as NewArrayExpression)); + } + else + { + list.Add(Visit(original[i])); + } + } + return list; + } + + protected virtual string VisitNew(NewExpression nex) + { + // TODO : check ! + var member = Expression.Convert(nex, typeof(object)); + var lambda = Expression.Lambda>(member); + try + { + var getter = lambda.Compile(); + var o = getter(); + + SqlParameters.Add(o); + + //don't execute if compiled + if (Visited == false) + { + return string.Format("@{0}", SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; + } + catch (InvalidOperationException) + { + //don't execute if compiled + if (Visited == false) + { + // FieldName ? + List exprs = VisitExpressionList(nex.Arguments); + var r = new StringBuilder(); + foreach (var e in exprs) + { + r.AppendFormat("{0}{1}", + r.Length > 0 ? "," : "", + e); + } + return r.ToString(); + } + //already compiled, return + return string.Empty; + } + } + + protected virtual string VisitParameter(ParameterExpression p) + { + return p.Name; + } + + protected virtual string VisitConstant(ConstantExpression c) + { + if (c.Value == null) + return "null"; + + SqlParameters.Add(c.Value); + + //don't execute if compiled + if (Visited == false) + { + return string.Format("@{0}", SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; + } + + protected virtual string VisitUnary(UnaryExpression u) + { + switch (u.NodeType) + { + case ExpressionType.Not: + return VisitNot(u.Operand); + default: + return Visit(u.Operand); + } + } + + private string VisitNot(Expression exp) + { + var o = Visit(exp); + + // use a "NOT (...)" syntax instead of "<>" since we don't know whether "<>" works in all sql servers + // also, x.StartsWith(...) translates to "x LIKE '...%'" which we cannot "<>" and have to "NOT (...") + + switch (exp.NodeType) + { + case ExpressionType.MemberAccess: + // false property , i.e. x => !Trashed + SqlParameters.Add(true); + //don't execute if compiled + if (Visited == false) + { + return string.Format("NOT ({0} = @{1})", o, SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; + default: + //don't execute if compiled + if (Visited == false) + { + // could be anything else, such as: x => !x.Path.StartsWith("-20") + return string.Concat("NOT (", o, ")"); + } + //already compiled, return + return string.Empty; + } + } + + private string VisitNotNot(Expression exp) + { + var o = Visit(exp); + + switch (exp.NodeType) + { + case ExpressionType.MemberAccess: + // true property, i.e. x => Trashed + SqlParameters.Add(true); + + //don't execute if compiled + if (Visited == false) + { + return string.Format("({0} = @{1})", o, SqlParameters.Count - 1); + } + //already compiled, return + return string.Empty; + default: + // could be anything else, such as: x => x.Path.StartsWith("-20") + return o; + } + } + + protected virtual string VisitNewArray(NewArrayExpression na) + { + var exprs = VisitExpressionList(na.Expressions); + + //don't execute if compiled + if (Visited == false) + { + var r = new StringBuilder(); + foreach (var e in exprs) + { + r.Append(r.Length > 0 ? "," + e : e); + } + + return r.ToString(); + } + //already compiled, return + return string.Empty; + } + + protected virtual List VisitNewArrayFromExpressionList(NewArrayExpression na) + { + var exprs = VisitExpressionList(na.Expressions); + return exprs; + } + + protected virtual string BindOperant(ExpressionType e) + { + switch (e) + { + case ExpressionType.Equal: + return "="; + case ExpressionType.NotEqual: + return "<>"; + case ExpressionType.GreaterThan: + return ">"; + case ExpressionType.GreaterThanOrEqual: + return ">="; + case ExpressionType.LessThan: + return "<"; + case ExpressionType.LessThanOrEqual: + return "<="; + case ExpressionType.AndAlso: + return "AND"; + case ExpressionType.OrElse: + return "OR"; + case ExpressionType.Add: + return "+"; + case ExpressionType.Subtract: + return "-"; + case ExpressionType.Multiply: + return "*"; + case ExpressionType.Divide: + return "/"; + case ExpressionType.Modulo: + return "MOD"; + case ExpressionType.Coalesce: + return "COALESCE"; + default: + return e.ToString(); + } + } + + protected virtual string VisitMethodCall(MethodCallExpression m) + { + //Here's what happens with a MethodCallExpression: + // If a method is called that contains a single argument, + // then m.Object is the object on the left hand side of the method call, example: + // x.Path.StartsWith(content.Path) + // m.Object = x.Path + // and m.Arguments.Length == 1, therefor m.Arguments[0] == content.Path + // If a method is called that contains multiple arguments, then m.Object == null and the + // m.Arguments collection contains the left hand side of the method call, example: + // x.Path.SqlStartsWith(content.Path, TextColumnType.NVarchar) + // m.Object == null + // m.Arguments.Length == 3, therefor, m.Arguments[0] == x.Path, m.Arguments[1] == content.Path, m.Arguments[2] == TextColumnType.NVarchar + // So, we need to cater for these scenarios. + + var objectForMethod = m.Object ?? m.Arguments[0]; + var visitedObjectForMethod = Visit(objectForMethod); + var methodArgs = m.Object == null + ? m.Arguments.Skip(1).ToArray() + : m.Arguments.ToArray(); + + switch (m.Method.Name) + { + case "ToString": + SqlParameters.Add(objectForMethod.ToString()); + //don't execute if compiled + if (Visited == false) + return string.Format("@{0}", SqlParameters.Count - 1); + //already compiled, return + return string.Empty; + case "ToUpper": + //don't execute if compiled + if (Visited == false) + return string.Format("upper({0})", visitedObjectForMethod); + //already compiled, return + return string.Empty; + case "ToLower": + //don't execute if compiled + if (Visited == false) + return string.Format("lower({0})", visitedObjectForMethod); + //already compiled, return + return string.Empty; + case "SqlWildcard": + case "StartsWith": + case "EndsWith": + case "Contains": + case "Equals": + case "SqlStartsWith": + case "SqlEndsWith": + case "SqlContains": + case "SqlEquals": + case "InvariantStartsWith": + case "InvariantEndsWith": + case "InvariantContains": + case "InvariantEquals": + + string compareValue; + + if (methodArgs[0].NodeType != ExpressionType.Constant) + { + //This occurs when we are getting a value from a non constant such as: x => x.Path.StartsWith(content.Path) + // So we'll go get the value: + var member = Expression.Convert(methodArgs[0], typeof(object)); + var lambda = Expression.Lambda>(member); + var getter = lambda.Compile(); + compareValue = getter().ToString(); + } + else + { + compareValue = methodArgs[0].ToString(); + } + + //special case, if it is 'Contains' and the member that Contains is being called on is not a string, then + // we should be doing an 'In' clause - but we currently do not support this + if (methodArgs[0].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[0].Type)) + { + throw new NotSupportedException("An array Contains method is not supported"); + } + + //default column type + var colType = TextColumnType.NVarchar; + + //then check if the col type argument has been passed to the current method (this will be the case for methods like + // SqlContains and other Sql methods) + if (methodArgs.Length > 1) + { + var colTypeArg = methodArgs.FirstOrDefault(x => x is ConstantExpression && x.Type == typeof(TextColumnType)); + if (colTypeArg != null) + { + colType = (TextColumnType)((ConstantExpression)colTypeArg).Value; + } + } + + return HandleStringComparison(visitedObjectForMethod, compareValue, m.Method.Name, colType); + + case "Replace": + string searchValue; + + if (methodArgs[0].NodeType != ExpressionType.Constant) + { + //This occurs when we are getting a value from a non constant such as: x => x.Path.StartsWith(content.Path) + // So we'll go get the value: + var member = Expression.Convert(methodArgs[0], typeof(object)); + var lambda = Expression.Lambda>(member); + var getter = lambda.Compile(); + searchValue = getter().ToString(); + } + else + { + searchValue = methodArgs[0].ToString(); + } + + if (methodArgs[0].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[0].Type)) + { + throw new NotSupportedException("An array Contains method is not supported"); + } + + string replaceValue; + + if (methodArgs[1].NodeType != ExpressionType.Constant) + { + //This occurs when we are getting a value from a non constant such as: x => x.Path.StartsWith(content.Path) + // So we'll go get the value: + var member = Expression.Convert(methodArgs[1], typeof(object)); + var lambda = Expression.Lambda>(member); + var getter = lambda.Compile(); + replaceValue = getter().ToString(); + } + else + { + replaceValue = methodArgs[1].ToString(); + } + + if (methodArgs[1].Type != typeof(string) && TypeHelper.IsTypeAssignableFrom(methodArgs[1].Type)) + { + throw new NotSupportedException("An array Contains method is not supported"); + } + + SqlParameters.Add(RemoveQuote(searchValue)); + + SqlParameters.Add(RemoveQuote(replaceValue)); + + //don't execute if compiled + if (Visited == false) + return string.Format("replace({0}, @{1}, @{2})", visitedObjectForMethod, SqlParameters.Count - 2, SqlParameters.Count - 1); + //already compiled, return + return string.Empty; + + //case "Substring": + // var startIndex = Int32.Parse(args[0].ToString()) + 1; + // if (args.Count == 2) + // { + // var length = Int32.Parse(args[1].ToString()); + // return string.Format("substring({0} from {1} for {2})", + // r, + // startIndex, + // length); + // } + // else + // return string.Format("substring({0} from {1})", + // r, + // startIndex); + //case "Round": + //case "Floor": + //case "Ceiling": + //case "Coalesce": + //case "Abs": + //case "Sum": + // return string.Format("{0}({1}{2})", + // m.Method.Name, + // r, + // args.Count == 1 ? string.Format(",{0}", args[0]) : ""); + //case "Concat": + // var s = new StringBuilder(); + // foreach (Object e in args) + // { + // s.AppendFormat(" || {0}", e); + // } + // return string.Format("{0}{1}", r, s); + + //case "In": + + // var member = Expression.Convert(m.Arguments[0], typeof(object)); + // var lambda = Expression.Lambda>(member); + // var getter = lambda.Compile(); + + // var inArgs = (object[])getter(); + + // var sIn = new StringBuilder(); + // foreach (var e in inArgs) + // { + // SqlParameters.Add(e); + + // sIn.AppendFormat("{0}{1}", + // sIn.Length > 0 ? "," : "", + // string.Format("@{0}", SqlParameters.Count - 1)); + + // //sIn.AppendFormat("{0}{1}", + // // sIn.Length > 0 ? "," : "", + // // GetQuotedValue(e, e.GetType())); + // } + + // return string.Format("{0} {1} ({2})", r, m.Method.Name, sIn.ToString()); + //case "Desc": + // return string.Format("{0} DESC", r); + //case "Alias": + //case "As": + // return string.Format("{0} As {1}", r, + // GetQuotedColumnName(RemoveQuoteFromAlias(RemoveQuote(args[0].ToString())))); + + default: + + throw new ArgumentOutOfRangeException("No logic supported for " + m.Method.Name); + + //var s2 = new StringBuilder(); + //foreach (Object e in args) + //{ + // s2.AppendFormat(",{0}", GetQuotedValue(e, e.GetType())); + //} + //return string.Format("{0}({1}{2})", m.Method.Name, r, s2.ToString()); + } + } + + public virtual string GetQuotedTableName(string tableName) + { + return Visited ? tableName : string.Format("\"{0}\"", tableName); + } + + public virtual string GetQuotedColumnName(string columnName) + { + return Visited ? columnName : string.Format("\"{0}\"", columnName); + } + + public virtual string GetQuotedName(string name) + { + return Visited ? name : string.Format("\"{0}\"", name); + } + + protected string HandleStringComparison(string col, string val, string verb, TextColumnType columnType) + { + switch (verb) + { + case "SqlWildcard": + SqlParameters.Add(RemoveQuote(val)); + //don't execute if compiled + if (Visited == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; + case "Equals": + SqlParameters.Add(RemoveQuote(val)); + //don't execute if compiled + if (Visited == false) + return SqlSyntax.GetStringColumnEqualComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; + case "StartsWith": + SqlParameters.Add(string.Format("{0}{1}", + RemoveQuote(val), + SqlSyntax.GetWildcardPlaceholder())); + //don't execute if compiled + if (Visited == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; + case "EndsWith": + SqlParameters.Add(string.Format("{0}{1}", + SqlSyntax.GetWildcardPlaceholder(), + RemoveQuote(val))); + //don't execute if compiled + if (Visited == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; + case "Contains": + SqlParameters.Add(string.Format("{0}{1}{0}", + SqlSyntax.GetWildcardPlaceholder(), + RemoveQuote(val))); + //don't execute if compiled + if (Visited == false) + return SqlSyntax.GetStringColumnWildcardComparison(col, SqlParameters.Count - 1, columnType); + //already compiled, return + return string.Empty; + case "InvariantEquals": + case "SqlEquals": + //recurse + return HandleStringComparison(col, val, "Equals", columnType); + case "InvariantStartsWith": + case "SqlStartsWith": + //recurse + return HandleStringComparison(col, val, "StartsWith", columnType); + case "InvariantEndsWith": + case "SqlEndsWith": + //recurse + return HandleStringComparison(col, val, "EndsWith", columnType); + case "InvariantContains": + case "SqlContains": + //recurse + return HandleStringComparison(col, val, "Contains", columnType); + default: + throw new ArgumentOutOfRangeException("verb"); + } + } + + //public virtual string GetQuotedValue(object value, Type fieldType, Func escapeCallback = null, Func shouldQuoteCallback = null) + //{ + // if (value == null) return "NULL"; + + // if (escapeCallback == null) + // { + // escapeCallback = EscapeParam; + // } + // if (shouldQuoteCallback == null) + // { + // shouldQuoteCallback = ShouldQuoteValue; + // } + + // if (!fieldType.UnderlyingSystemType.IsValueType && fieldType != typeof(string)) + // { + // //if (TypeSerializer.CanCreateFromString(fieldType)) + // //{ + // // return "'" + escapeCallback(TypeSerializer.SerializeToString(value)) + "'"; + // //} + + // throw new NotSupportedException( + // string.Format("Property of type: {0} is not supported", fieldType.FullName)); + // } + + // if (fieldType == typeof(int)) + // return ((int)value).ToString(CultureInfo.InvariantCulture); + + // if (fieldType == typeof(float)) + // return ((float)value).ToString(CultureInfo.InvariantCulture); + + // if (fieldType == typeof(double)) + // return ((double)value).ToString(CultureInfo.InvariantCulture); + + // if (fieldType == typeof(decimal)) + // return ((decimal)value).ToString(CultureInfo.InvariantCulture); + + // if (fieldType == typeof(DateTime)) + // { + // return "'" + escapeCallback(((DateTime)value).ToIsoString()) + "'"; + // } + + // if (fieldType == typeof(bool)) + // return ((bool)value) ? Convert.ToString(1, CultureInfo.InvariantCulture) : Convert.ToString(0, CultureInfo.InvariantCulture); + + // return shouldQuoteCallback(fieldType) + // ? "'" + escapeCallback(value) + "'" + // : value.ToString(); + //} + + public virtual string EscapeParam(object paramValue) + { + return paramValue == null ? string.Empty : SqlSyntax.EscapeString(paramValue.ToString()); + } + + public virtual bool ShouldQuoteValue(Type fieldType) + { + return true; + } + + protected virtual string RemoveQuote(string exp) + { + if ((exp.StartsWith("\"") || exp.StartsWith("`") || exp.StartsWith("'")) + && + (exp.EndsWith("\"") || exp.EndsWith("`") || exp.EndsWith("'"))) + { + exp = exp.Remove(0, 1); + exp = exp.Remove(exp.Length - 1, 1); + } + return exp; + } + + //protected virtual string RemoveQuoteFromAlias(string expression) + //{ + + // if ((expression.StartsWith("\"") || expression.StartsWith("`") || expression.StartsWith("'")) + // && + // (expression.EndsWith("\"") || expression.EndsWith("`") || expression.EndsWith("'"))) + // { + // expression = expression.Remove(0, 1); + // expression = expression.Remove(expression.Length - 1, 1); + // } + // return expression; + //} + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs similarity index 65% rename from src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs rename to src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs index 7e4466b529..b265a5b587 100644 --- a/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/ModelToSqlExpressionVisitor.cs @@ -1,81 +1,74 @@ -using System; -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Diagnostics; -using System.Globalization; -using System.Linq; -using System.Linq.Expressions; -using System.Text; -using Umbraco.Core.Persistence.Mappers; -using Umbraco.Core.Persistence.SqlSyntax; - -namespace Umbraco.Core.Persistence.Querying -{ - /// - /// An expression tree parser to create SQL statements and SQL parameters based on a given strongly typed expression based on Umbraco's business logic Models - /// - /// - /// This object stores state, it cannot be re-used to parse an expression - /// - internal class ModelToSqlExpressionHelper : BaseExpressionHelper - { - - private readonly BaseMapper _mapper; - - public ModelToSqlExpressionHelper(ISqlSyntaxProvider sqlSyntax, BaseMapper mapper) : base(sqlSyntax) - { - _mapper = mapper; - } - - public ModelToSqlExpressionHelper() : this(SqlSyntaxContext.SqlSyntaxProvider, MappingResolver.Current.ResolveMapperByType(typeof(T))) - { - } - - protected override string VisitMemberAccess(MemberExpression m) - { - if (m.Expression != null && - m.Expression.NodeType == ExpressionType.Parameter - && m.Expression.Type == typeof(T)) - { - //don't execute if compiled - if (IsCompiled == false) - { - var field = _mapper.Map(m.Member.Name, true); - if (field.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); - return field; - } - //already compiled, return - return string.Empty; - } - - if (m.Expression != null && m.Expression.NodeType == ExpressionType.Convert) - { - //don't execute if compiled - if (IsCompiled == false) - { - var field = _mapper.Map(m.Member.Name, true); - if (field.IsNullOrWhiteSpace()) - throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); - return field; - } - //already compiled, return - return string.Empty; - } - - var member = Expression.Convert(m, typeof(object)); - var lambda = Expression.Lambda>(member); - var getter = lambda.Compile(); - object o = getter(); - - SqlParameters.Add(o); - - //don't execute if compiled - if (IsCompiled == false) - return string.Format("@{0}", SqlParameters.Count - 1); - //already compiled, return - return string.Empty; - - } - } +using System; +using System.Linq.Expressions; +using Umbraco.Core.Persistence.Mappers; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Querying +{ + /// + /// An expression tree parser to create SQL statements and SQL parameters based on a strongly typed expression, + /// based on Umbraco's business logic models. + /// + /// This object is stateful and cannot be re-used to parse an expression. + internal class ModelToSqlExpressionVisitor : ExpressionVisitorBase + { + private readonly BaseMapper _mapper; + + public ModelToSqlExpressionVisitor(ISqlSyntaxProvider sqlSyntax, BaseMapper mapper) + : base(sqlSyntax) + { + _mapper = mapper; + } + + public ModelToSqlExpressionVisitor() + : this(SqlSyntaxContext.SqlSyntaxProvider, MappingResolver.Current.ResolveMapperByType(typeof(T))) + { } + + protected override string VisitMemberAccess(MemberExpression m) + { + if (m.Expression != null && + m.Expression.NodeType == ExpressionType.Parameter + && m.Expression.Type == typeof(T)) + { + //don't execute if compiled + if (Visited == false) + { + var field = _mapper.Map(m.Member.Name, true); + if (field.IsNullOrWhiteSpace()) + throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; + } + + if (m.Expression != null && m.Expression.NodeType == ExpressionType.Convert) + { + //don't execute if compiled + if (Visited == false) + { + var field = _mapper.Map(m.Member.Name, true); + if (field.IsNullOrWhiteSpace()) + throw new InvalidOperationException("The mapper returned an empty field for the member name: " + m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; + } + + var member = Expression.Convert(m, typeof(object)); + var lambda = Expression.Lambda>(member); + var getter = lambda.Compile(); + var o = getter(); + + SqlParameters.Add(o); + + //don't execute if compiled + if (Visited == false) + return string.Format("@{0}", SqlParameters.Count - 1); + //already compiled, return + return string.Empty; + + } + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs b/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionVisitor.cs similarity index 73% rename from src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs rename to src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionVisitor.cs index cbc3aa8f05..6b527296df 100644 --- a/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionHelper.cs +++ b/src/Umbraco.Core/Persistence/Querying/PocoToSqlExpressionVisitor.cs @@ -1,79 +1,73 @@ -using System; -using System.Collections; -using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Linq; -using System.Linq.Expressions; -using System.Text; -using Umbraco.Core.Persistence.SqlSyntax; - -namespace Umbraco.Core.Persistence.Querying -{ - - /// - /// An expression tree parser to create SQL statements and SQL parameters based on a given strongly typed expression based on Umbraco's PetaPoco dto Models - /// - /// - /// This object stores state, it cannot be re-used to parse an expression - /// - internal class PocoToSqlExpressionHelper : BaseExpressionHelper - { - private readonly Database.PocoData _pd; - - public PocoToSqlExpressionHelper() : base(SqlSyntaxContext.SqlSyntaxProvider) - { - _pd = new Database.PocoData(typeof(T)); - } - - protected override string VisitMemberAccess(MemberExpression m) - { - if (m.Expression != null && - m.Expression.NodeType == ExpressionType.Parameter - && m.Expression.Type == typeof(T)) - { - //don't execute if compiled - if (IsCompiled == false) - { - string field = GetFieldName(_pd, m.Member.Name); - return field; - } - //already compiled, return - return string.Empty; - } - - if (m.Expression != null && m.Expression.NodeType == ExpressionType.Convert) - { - //don't execute if compiled - if (IsCompiled == false) - { - string field = GetFieldName(_pd, m.Member.Name); - return field; - } - //already compiled, return - return string.Empty; - } - - var member = Expression.Convert(m, typeof(object)); - var lambda = Expression.Lambda>(member); - var getter = lambda.Compile(); - object o = getter(); - - SqlParameters.Add(o); - - //don't execute if compiled - if (IsCompiled == false) - return string.Format("@{0}", SqlParameters.Count - 1); - //already compiled, return - return string.Empty; - } - - protected virtual string GetFieldName(Database.PocoData pocoData, string name) - { - var column = pocoData.Columns.FirstOrDefault(x => x.Value.PropertyInfo.Name == name); - return string.Format("{0}.{1}", - SqlSyntax.GetQuotedTableName(pocoData.TableInfo.TableName), - SqlSyntax.GetQuotedColumnName(column.Value.ColumnName)); - } - - } +using System; +using System.Linq; +using System.Linq.Expressions; +using Umbraco.Core.Persistence.SqlSyntax; + +namespace Umbraco.Core.Persistence.Querying +{ + /// + /// An expression tree parser to create SQL statements and SQL parameters based on a strongly typed expression, + /// based on Umbraco's DTOs. + /// + /// This object is stateful and cannot be re-used to parse an expression. + internal class PocoToSqlExpressionVisitor : ExpressionVisitorBase + { + private readonly Database.PocoData _pd; + + public PocoToSqlExpressionVisitor() + : base(SqlSyntaxContext.SqlSyntaxProvider) + { + _pd = new Database.PocoData(typeof(T)); + } + + protected override string VisitMemberAccess(MemberExpression m) + { + if (m.Expression != null && + m.Expression.NodeType == ExpressionType.Parameter + && m.Expression.Type == typeof(T)) + { + //don't execute if compiled + if (Visited == false) + { + string field = GetFieldName(_pd, m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; + } + + if (m.Expression != null && m.Expression.NodeType == ExpressionType.Convert) + { + //don't execute if compiled + if (Visited == false) + { + string field = GetFieldName(_pd, m.Member.Name); + return field; + } + //already compiled, return + return string.Empty; + } + + var member = Expression.Convert(m, typeof(object)); + var lambda = Expression.Lambda>(member); + var getter = lambda.Compile(); + var o = getter(); + + SqlParameters.Add(o); + + //don't execute if compiled + if (Visited == false) + return string.Format("@{0}", SqlParameters.Count - 1); + //already compiled, return + return string.Empty; + } + + protected virtual string GetFieldName(Database.PocoData pocoData, string name) + { + var column = pocoData.Columns.FirstOrDefault(x => x.Value.PropertyInfo.Name == name); + return string.Format("{0}.{1}", + SqlSyntax.GetQuotedTableName(pocoData.TableInfo.TableName), + SqlSyntax.GetQuotedColumnName(column.Value.ColumnName)); + } + } } \ No newline at end of file diff --git a/src/Umbraco.Core/Persistence/Querying/Query.cs b/src/Umbraco.Core/Persistence/Querying/Query.cs index 1a270cec4b..6213ca5ed6 100644 --- a/src/Umbraco.Core/Persistence/Querying/Query.cs +++ b/src/Umbraco.Core/Persistence/Querying/Query.cs @@ -31,7 +31,7 @@ namespace Umbraco.Core.Persistence.Querying if (predicate != null) { //TODO: This should have an SqlSyntax object passed in, this ctor is relying on a singleton - var expressionHelper = new ModelToSqlExpressionHelper(); + var expressionHelper = new ModelToSqlExpressionVisitor(); string whereExpression = expressionHelper.Visit(predicate); _wheres.Add(new Tuple(whereExpression, expressionHelper.GetSqlParameters())); diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index 36894801aa..bc1aa0c8f6 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -1016,10 +1016,10 @@ - - + + - + diff --git a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs index 2b1561012b..03aa3206ee 100644 --- a/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs +++ b/src/Umbraco.Tests.Benchmarks/ModelToSqlExpressionHelperBenchmarks.cs @@ -62,7 +62,7 @@ namespace Umbraco.Tests.Benchmarks Expression> predicate = content => content.Path.StartsWith("-1") && content.Published && (content.ContentTypeId == a || content.ContentTypeId == b); - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(_syntaxProvider, _contentMapper); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(_syntaxProvider, _contentMapper); var result = modelToSqlExpressionHelper.Visit(predicate); } @@ -80,7 +80,7 @@ namespace Umbraco.Tests.Benchmarks Expression> predicate = content => content.Path.StartsWith("-1") && content.Published && (content.ContentTypeId == a || content.ContentTypeId == b); - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(_syntaxProvider, _contentMapper); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(_syntaxProvider, _contentMapper); //wrap it! _cachedExpression.Wrap(predicate); diff --git a/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs b/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs index 6834968460..65a660146e 100644 --- a/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs +++ b/src/Umbraco.Tests/Persistence/Querying/ExpressionTests.cs @@ -21,7 +21,7 @@ namespace Umbraco.Tests.Persistence.Querying // { // //Arrange // Expression> predicate = content => content.ContentType.Alias == "Test"; - // var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + // var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); // var result = modelToSqlExpressionHelper.Visit(predicate); // Debug.Print("Model to Sql ExpressionHelper: \n" + result); @@ -42,14 +42,14 @@ namespace Umbraco.Tests.Persistence.Querying Expression> predicate1 = content => content.Path.StartsWith("-1"); cachedExpression.Wrap(predicate1); - var modelToSqlExpressionHelper1 = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper1 = new ModelToSqlExpressionVisitor(); var result1 = modelToSqlExpressionHelper1.Visit(cachedExpression); Assert.AreEqual("upper([umbracoNode].[path]) LIKE upper(@0)", result1); Assert.AreEqual("-1%", modelToSqlExpressionHelper1.GetSqlParameters()[0]); Expression> predicate2 = content => content.Path.StartsWith("-1,123,97"); cachedExpression.Wrap(predicate2); - var modelToSqlExpressionHelper2 = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper2 = new ModelToSqlExpressionVisitor(); var result2 = modelToSqlExpressionHelper2.Visit(cachedExpression); Assert.AreEqual("upper([umbracoNode].[path]) LIKE upper(@0)", result2); Assert.AreEqual("-1,123,97%", modelToSqlExpressionHelper2.GetSqlParameters()[0]); @@ -61,7 +61,7 @@ namespace Umbraco.Tests.Persistence.Querying { //Arrange Expression> predicate = content => content.Path.StartsWith("-1"); - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Assert.AreEqual("upper([umbracoNode].[path]) LIKE upper(@0)", result); @@ -73,7 +73,7 @@ namespace Umbraco.Tests.Persistence.Querying { //Arrange Expression> predicate = content => content.ParentId == -1; - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Debug.Print("Model to Sql ExpressionHelper: \n" + result); @@ -86,7 +86,7 @@ namespace Umbraco.Tests.Persistence.Querying public void Equals_Operator_For_Value_Gets_Escaped() { Expression> predicate = user => user.Username == "hello@world.com"; - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Debug.Print("Model to Sql ExpressionHelper: \n" + result); @@ -99,7 +99,7 @@ namespace Umbraco.Tests.Persistence.Querying public void Equals_Method_For_Value_Gets_Escaped() { Expression> predicate = user => user.Username.Equals("hello@world.com"); - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Debug.Print("Model to Sql ExpressionHelper: \n" + result); @@ -115,7 +115,7 @@ namespace Umbraco.Tests.Persistence.Querying SqlSyntaxContext.SqlSyntaxProvider = new MySqlSyntaxProvider(Mock.Of()); Expression> predicate = user => user.Username.Equals("mydomain\\myuser"); - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Debug.Print("Model to Sql ExpressionHelper: \n" + result); @@ -132,7 +132,7 @@ namespace Umbraco.Tests.Persistence.Querying SqlSyntaxContext.SqlSyntaxProvider = new MySqlSyntaxProvider(Mock.Of()); Expression> predicate = user => user.Login.StartsWith("mydomain\\myuser"); - var modelToSqlExpressionHelper = new PocoToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new PocoToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Debug.Print("Poco to Sql ExpressionHelper: \n" + result); @@ -145,7 +145,7 @@ namespace Umbraco.Tests.Persistence.Querying public void Sql_Replace_Mapped() { Expression> predicate = user => user.Username.Replace("@world", "@test") == "hello@test.com"; - var modelToSqlExpressionHelper = new ModelToSqlExpressionHelper(); + var modelToSqlExpressionHelper = new ModelToSqlExpressionVisitor(); var result = modelToSqlExpressionHelper.Visit(predicate); Debug.Print("Model to Sql ExpressionHelper: \n" + result); From 69b442a31a931a7d7390698c368ca1f377505fb1 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 31 Oct 2016 11:02:00 +0100 Subject: [PATCH 08/18] Fix - PublicAccessEntry rules clearing --- src/Umbraco.Core/Models/PublicAccessEntry.cs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/Umbraco.Core/Models/PublicAccessEntry.cs b/src/Umbraco.Core/Models/PublicAccessEntry.cs index 4af6f9536a..1ed15a8fb4 100644 --- a/src/Umbraco.Core/Models/PublicAccessEntry.cs +++ b/src/Umbraco.Core/Models/PublicAccessEntry.cs @@ -25,7 +25,7 @@ namespace Umbraco.Core.Models NoAccessNodeId = noAccessNode.Id; _protectedNodeId = protectedNode.Id; - _ruleCollection = new ObservableCollection(ruleCollection); + _ruleCollection = new ObservableCollection(ruleCollection); _ruleCollection.CollectionChanged += _ruleCollection_CollectionChanged; } @@ -81,7 +81,7 @@ namespace Umbraco.Core.Models internal IEnumerable RemovedRules { get { return _removedRules; } - } + } public IEnumerable Rules { @@ -107,10 +107,7 @@ namespace Umbraco.Core.Models public void ClearRules() { - for (var i = _ruleCollection.Count - 1; i >= 0; i--) - { - RemoveRule(_ruleCollection[i]); - } + _ruleCollection.Clear(); } [DataMember] @@ -126,7 +123,7 @@ namespace Umbraco.Core.Models get { return _noAccessNodeId; } set { SetPropertyValueAndDetectChanges(value, ref _noAccessNodeId, Ps.Value.NoAccessNodeIdSelector); } } - + [DataMember] public int ProtectedNodeId { From c77174059d41ffc69cbbbe940b2d508bf4b8096a Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 31 Oct 2016 11:06:17 +0100 Subject: [PATCH 09/18] Backport MainDom cleanup from 7.6 --- src/Umbraco.Core/MainDom.cs | 68 +++++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/MainDom.cs b/src/Umbraco.Core/MainDom.cs index fb8ad06999..8e7efc5171 100644 --- a/src/Umbraco.Core/MainDom.cs +++ b/src/Umbraco.Core/MainDom.cs @@ -1,18 +1,21 @@ using System; using System.Collections.Generic; -using System.Diagnostics; -using System.IO.MemoryMappedFiles; -using System.Text; using System.Threading; -using System.Threading.Tasks; using System.Web.Hosting; using Umbraco.Core.Logging; -using Umbraco.Core.ObjectResolution; namespace Umbraco.Core { - // represents the main domain - class MainDom : IRegisteredObject + /// + /// Represents the main AppDomain running for a given application. + /// + /// + /// There can be only one "main" AppDomain running for a given application at a time. + /// When an AppDomain starts, it tries to acquire the main domain status. + /// When an AppDomain stops (eg the application is restarting) it should release the main domain status. + /// It is possible to register against the MainDom and be notified when it is released. + /// + internal class MainDom : IRegisteredObject { #region Vars @@ -34,16 +37,26 @@ namespace Umbraco.Core private volatile bool _signaled; // we have been signaled // actions to run before releasing the main domain - private readonly SortedList _callbacks = new SortedList(); + private readonly SortedList _callbacks = new SortedList(new WeightComparer()); private const int LockTimeoutMilliseconds = 90000; // (1.5 * 60 * 1000) == 1 min 30 seconds + private class WeightComparer : IComparer + { + public int Compare(int x, int y) + { + var result = x.CompareTo(y); + // return "equal" as "greater than" + return result == 0 ? 1 : result; + } + } + #endregion #region Ctor // initializes a new instance of MainDom - public MainDom(ILogger logger) + internal MainDom(ILogger logger) { _logger = logger; @@ -52,22 +65,47 @@ namespace Umbraco.Core if (HostingEnvironment.ApplicationID != null) appId = HostingEnvironment.ApplicationID.ReplaceNonAlphanumericChars(string.Empty); - var lockName = "UMBRACO-" + appId + "-MAINDOM-LCK"; + // combining with the physical path because if running on eg IIS Express, + // two sites could have the same appId even though they are different. + // + // now what could still collide is... two sites, running in two different processes + // and having the same appId, and running on the same app physical path + // + // we *cannot* use the process ID here because when an AppPool restarts it is + // a new process for the same application path + + var appPath = HostingEnvironment.ApplicationPhysicalPath; + var hash = (appId + ":::" + appPath).ToSHA1(); + + var lockName = "UMBRACO-" + hash + "-MAINDOM-LCK"; _asyncLock = new AsyncLock(lockName); - var eventName = "UMBRACO-" + appId + "-MAINDOM-EVT"; + var eventName = "UMBRACO-" + hash + "-MAINDOM-EVT"; _signal = new EventWaitHandle(false, EventResetMode.AutoReset, eventName); } #endregion - // register a main domain consumer + /// + /// Registers a resource that requires the current AppDomain to be the main domain to function. + /// + /// An action to execute before the AppDomain releases the main domain status. + /// An optional weight (lower goes first). + /// A value indicating whether it was possible to register. public bool Register(Action release, int weight = 100) { return Register(null, release, weight); } - // register a main domain consumer + /// + /// Registers a resource that requires the current AppDomain to be the main domain to function. + /// + /// An action to execute when registering. + /// An action to execute before the AppDomain releases the main domain status. + /// An optional weight (lower goes first). + /// A value indicating whether it was possible to register. + /// If registering is successful, then the action + /// is guaranteed to execute before the AppDomain releases the main domain status. public bool Register(Action install, Action release, int weight = 100) { lock (_locko) @@ -123,7 +161,7 @@ namespace Umbraco.Core } // acquires the main domain - public bool Acquire() + internal bool Acquire() { lock (_locko) // we don't want the hosting environment to interfere by signaling { @@ -174,7 +212,7 @@ namespace Umbraco.Core } // IRegisteredObject - public void Stop(bool immediate) + void IRegisteredObject.Stop(bool immediate) { try { From 59ace3d8814e74ece37de3d207b039edd608ee02 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 31 Oct 2016 11:28:28 +0100 Subject: [PATCH 10/18] Backport SafeCallContext, DefaultDatabaseFactory fixes from 7.6 --- .../Packaging/PackageBinaryInspector.cs | 50 ++--- .../Persistence/DefaultDatabaseFactory.cs | 186 +++++++++++++----- src/Umbraco.Core/SafeCallContext.cs | 94 +++++++++ src/Umbraco.Core/Umbraco.Core.csproj | 1 + .../Persistence/DatabaseContextTests.cs | 6 +- 5 files changed, 260 insertions(+), 77 deletions(-) create mode 100644 src/Umbraco.Core/SafeCallContext.cs diff --git a/src/Umbraco.Core/Packaging/PackageBinaryInspector.cs b/src/Umbraco.Core/Packaging/PackageBinaryInspector.cs index 8852475543..116b5ee952 100644 --- a/src/Umbraco.Core/Packaging/PackageBinaryInspector.cs +++ b/src/Umbraco.Core/Packaging/PackageBinaryInspector.cs @@ -29,20 +29,24 @@ namespace Umbraco.Core.Packaging /// public static IEnumerable ScanAssembliesForTypeReference(IEnumerable assemblys, out string[] errorReport) { - var appDomain = GetTempAppDomain(); - var type = typeof(PackageBinaryInspector); - try + // beware! when toying with domains, use a safe call context! + using (new SafeCallContext()) { - var value = (PackageBinaryInspector)appDomain.CreateInstanceAndUnwrap( - type.Assembly.FullName, - type.FullName); - // do NOT turn PerformScan into static (even if ReSharper says so)! - var result = value.PerformScan(assemblys.ToArray(), out errorReport); - return result; - } - finally - { - AppDomain.Unload(appDomain); + var appDomain = GetTempAppDomain(); + var type = typeof(PackageBinaryInspector); + try + { + var value = (PackageBinaryInspector) appDomain.CreateInstanceAndUnwrap( + type.Assembly.FullName, + type.FullName); + // do NOT turn PerformScan into static (even if ReSharper says so)! + var result = value.PerformScan(assemblys.ToArray(), out errorReport); + return result; + } + finally + { + AppDomain.Unload(appDomain); + } } } @@ -78,7 +82,7 @@ namespace Umbraco.Core.Packaging /// /// Performs the assembly scanning /// - /// + /// /// /// /// @@ -107,7 +111,7 @@ namespace Umbraco.Core.Packaging /// /// Performs the assembly scanning /// - /// + /// /// /// /// @@ -154,7 +158,7 @@ namespace Umbraco.Core.Packaging //get the list of assembly names to compare below var loadedNames = loaded.Select(x => x.GetName().Name).ToArray(); - + //Then load each referenced assembly into the context foreach (var a in loaded) { @@ -170,7 +174,7 @@ namespace Umbraco.Core.Packaging } catch (FileNotFoundException) { - //if an exception occurs it means that a referenced assembly could not be found + //if an exception occurs it means that a referenced assembly could not be found errors.Add( string.Concat("This package references the assembly '", assemblyName.Name, @@ -179,7 +183,7 @@ namespace Umbraco.Core.Packaging } catch (Exception ex) { - //if an exception occurs it means that a referenced assembly could not be found + //if an exception occurs it means that a referenced assembly could not be found errors.Add( string.Concat("This package could not be verified for compatibility. An error occurred while loading a referenced assembly '", assemblyName.Name, @@ -197,7 +201,7 @@ namespace Umbraco.Core.Packaging { //now we need to see if they contain any type 'T' var reflectedAssembly = a; - + try { var found = reflectedAssembly.GetExportedTypes() @@ -210,8 +214,8 @@ namespace Umbraco.Core.Packaging } catch (Exception ex) { - //This is a hack that nobody can seem to get around, I've read everything and it seems that - // this is quite a common thing when loading types into reflection only load context, so + //This is a hack that nobody can seem to get around, I've read everything and it seems that + // this is quite a common thing when loading types into reflection only load context, so // we're just going to ignore this specific one for now var typeLoadEx = ex as TypeLoadException; if (typeLoadEx != null) @@ -232,7 +236,7 @@ namespace Umbraco.Core.Packaging LogHelper.Error("An error occurred scanning package assemblies", ex); } } - + } errorReport = errors.ToArray(); @@ -252,7 +256,7 @@ namespace Umbraco.Core.Packaging var contractType = contractAssemblyLoadFrom.GetExportedTypes() .FirstOrDefault(x => x.FullName == typeof(T).FullName && x.Assembly.FullName == typeof(T).Assembly.FullName); - + if (contractType == null) { throw new InvalidOperationException("Could not find type " + typeof(T) + " in the LoadFrom assemblies"); diff --git a/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs b/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs index 658b8ebabe..c96b442e6e 100644 --- a/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs +++ b/src/Umbraco.Core/Persistence/DefaultDatabaseFactory.cs @@ -1,6 +1,6 @@ using System; +using System.Runtime.Remoting.Messaging; using System.Web; -using Umbraco.Core.Configuration; using Umbraco.Core.Logging; namespace Umbraco.Core.Persistence @@ -19,13 +19,24 @@ namespace Umbraco.Core.Persistence private readonly ILogger _logger; public string ConnectionString { get; private set; } public string ProviderName { get; private set; } - - //very important to have ThreadStatic: - // see: http://issues.umbraco.org/issue/U4-2172 - [ThreadStatic] - private static volatile UmbracoDatabase _nonHttpInstance; - private static readonly object Locker = new object(); + // NO! see notes in v8 HybridAccessorBase + //[ThreadStatic] + //private static volatile UmbracoDatabase _nonHttpInstance; + + private const string ItemKey = "Umbraco.Core.Persistence.DefaultDatabaseFactory"; + + private static UmbracoDatabase NonContextValue + { + get { return (UmbracoDatabase) CallContext.LogicalGetData(ItemKey); } + set + { + if (value == null) CallContext.FreeNamedDataSlot(ItemKey); + else CallContext.LogicalSetData(ItemKey, value); + } + } + + private static readonly object Locker = new object(); /// /// Constructor accepting custom connection string @@ -36,7 +47,10 @@ namespace Umbraco.Core.Persistence { if (logger == null) throw new ArgumentNullException("logger"); Mandate.ParameterNotNullOrEmpty(connectionStringName, "connectionStringName"); - _connectionStringName = connectionStringName; + + //if (NonContextValue != null) throw new Exception("NonContextValue is not null."); + + _connectionStringName = connectionStringName; _logger = logger; } @@ -51,65 +65,133 @@ namespace Umbraco.Core.Persistence if (logger == null) throw new ArgumentNullException("logger"); Mandate.ParameterNotNullOrEmpty(connectionString, "connectionString"); Mandate.ParameterNotNullOrEmpty(providerName, "providerName"); - ConnectionString = connectionString; + + //if (NonContextValue != null) throw new Exception("NonContextValue is not null."); + + ConnectionString = connectionString; ProviderName = providerName; _logger = logger; } public UmbracoDatabase CreateDatabase() { - //no http context, create the singleton global object - if (HttpContext.Current == null) - { - if (_nonHttpInstance == null) - { - lock (Locker) - { - //double check - if (_nonHttpInstance == null) - { - _nonHttpInstance = string.IsNullOrEmpty(ConnectionString) == false && string.IsNullOrEmpty(ProviderName) == false - ? new UmbracoDatabase(ConnectionString, ProviderName, _logger) - : new UmbracoDatabase(_connectionStringName, _logger); - } - } - } - return _nonHttpInstance; - } + // no http context, create the call context object + // NOTHING is going to track the object and it is the responsibility of the caller to release it! + // using the ReleaseDatabase method. + if (HttpContext.Current == null) + { + LogHelper.Debug("Get NON http [T" + Environment.CurrentManagedThreadId + "]"); + var value = NonContextValue; + if (value != null) return value; + lock (Locker) + { + value = NonContextValue; + if (value != null) return value; - //we have an http context, so only create one per request - if (HttpContext.Current.Items.Contains(typeof(DefaultDatabaseFactory)) == false) - { - HttpContext.Current.Items.Add(typeof (DefaultDatabaseFactory), - string.IsNullOrEmpty(ConnectionString) == false && string.IsNullOrEmpty(ProviderName) == false + LogHelper.Debug("Create NON http [T" + Environment.CurrentManagedThreadId + "]"); + NonContextValue = value = string.IsNullOrEmpty(ConnectionString) == false && string.IsNullOrEmpty(ProviderName) == false + ? new UmbracoDatabase(ConnectionString, ProviderName, _logger) + : new UmbracoDatabase(_connectionStringName, _logger); + + return value; + } + } + + // we have an http context, so only create one per request. + // UmbracoDatabase is marked IDisposeOnRequestEnd and therefore will be disposed when + // UmbracoModule attempts to dispose the relevant HttpContext items. so we DO dispose + // connections at the end of each request. no need to call ReleaseDatabase. + LogHelper.Debug("Get http [T" + Environment.CurrentManagedThreadId + "]"); + if (HttpContext.Current.Items.Contains(typeof(DefaultDatabaseFactory)) == false) + { + LogHelper.Debug("Create http [T" + Environment.CurrentManagedThreadId + "]"); + HttpContext.Current.Items.Add(typeof(DefaultDatabaseFactory), + string.IsNullOrEmpty(ConnectionString) == false && string.IsNullOrEmpty(ProviderName) == false ? new UmbracoDatabase(ConnectionString, ProviderName, _logger) : new UmbracoDatabase(_connectionStringName, _logger)); - } - return (UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]; - } + } + return (UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]; + } - protected override void DisposeResources() + // releases the "context" database + public void ReleaseDatabase() + { + if (HttpContext.Current == null) + { + var value = NonContextValue; + if (value != null) value.Dispose(); + NonContextValue = null; + } + else + { + var db = (UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]; + if (db != null) db.Dispose(); + HttpContext.Current.Items.Remove(typeof(DefaultDatabaseFactory)); + } + } + + protected override void DisposeResources() { - if (HttpContext.Current == null) - { - _nonHttpInstance.Dispose(); - } - else - { - if (HttpContext.Current.Items.Contains(typeof(DefaultDatabaseFactory))) - { - ((UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]).Dispose(); - } - } + ReleaseDatabase(); } // during tests, the thread static var can leak between tests // this method provides a way to force-reset the variable internal void ResetForTests() { - if (_nonHttpInstance == null) return; - _nonHttpInstance.Dispose(); - _nonHttpInstance = null; - } - } + var value = NonContextValue; + if (value != null) value.Dispose(); + NonContextValue = null; + } + + #region SafeCallContext + + // see notes in SafeCallContext - need to do this since we are using + // the logical call context... + + static DefaultDatabaseFactory() + { + SafeCallContext.Register(DetachDatabase, AttachDatabase); + } + + // detaches the current database + // ie returns the database and remove it from whatever is "context" + private static UmbracoDatabase DetachDatabase() + { + if (HttpContext.Current == null) + { + var db = NonContextValue; + NonContextValue = null; + return db; + } + else + { + var db = (UmbracoDatabase)HttpContext.Current.Items[typeof(DefaultDatabaseFactory)]; + HttpContext.Current.Items.Remove(typeof(DefaultDatabaseFactory)); + return db; + } + } + + // attach a current database + // ie assign it to whatever is "context" + // throws if there already is a database + private static void AttachDatabase(object o) + { + var database = o as UmbracoDatabase; + if (o != null && database == null) throw new ArgumentException("Not an UmbracoDatabase.", "o"); + + if (HttpContext.Current == null) + { + if (NonContextValue != null) throw new InvalidOperationException(); + if (database != null) NonContextValue = database; + } + else + { + if (HttpContext.Current.Items[typeof(DefaultDatabaseFactory)] != null) throw new InvalidOperationException(); + if (database != null) HttpContext.Current.Items[typeof(DefaultDatabaseFactory)] = database; + } + } + + #endregion + } } \ No newline at end of file diff --git a/src/Umbraco.Core/SafeCallContext.cs b/src/Umbraco.Core/SafeCallContext.cs new file mode 100644 index 0000000000..5ed41d389f --- /dev/null +++ b/src/Umbraco.Core/SafeCallContext.cs @@ -0,0 +1,94 @@ +using System; +using System.Linq; +using System.Collections.Generic; + +namespace Umbraco.Core +{ + internal class SafeCallContext : IDisposable + { + private static readonly List> EnterFuncs = new List>(); + private static readonly List> ExitActions = new List>(); + private static int _count; + private readonly List _objects; + private bool _disposed; + + public static void Register(Func enterFunc, Action exitAction) + { + if (enterFunc == null) throw new ArgumentNullException("enterFunc"); + if (exitAction == null) throw new ArgumentNullException("exitAction"); + + lock (EnterFuncs) + { + if (_count > 0) throw new InvalidOperationException("Cannot register while some SafeCallContext instances exist."); + EnterFuncs.Add(enterFunc); + ExitActions.Add(exitAction); + } + } + + // tried to make the UmbracoDatabase serializable but then it leaks to weird places + // in ReSharper and so on, where Umbraco.Core is not available. Tried to serialize + // as an object instead but then it comes *back* deserialized into the original context + // as an object and of course it breaks everything. Cannot prevent this from flowing, + // and ExecutionContext.SuppressFlow() works for threads but not domains. and we'll + // have the same issue with anything that toys with logical call context... + // + // so this class lets anything that uses the logical call context register itself, + // providing two methods: + // - an enter func that removes and returns whatever is in the logical call context + // - an exit action that restores the value into the logical call context + // whenever a SafeCallContext instance is created, it uses these methods to capture + // and clear the logical call context, and restore it when disposed. + // + // in addition, a static Clear method is provided - which uses the enter funcs to + // remove everything from logical call context - not to be used when the app runs, + // but can be useful during tests + // + // note + // see System.Transactions + // they are using a conditional weak table to store the data, and what they store in + // LLC is the key - which is just an empty MarshalByRefObject that is created with + // the transaction scope - that way, they can "clear current data" provided that + // they have the key - but they need to hold onto a ref to the scope... not ok for us + + public static void Clear() + { + lock (EnterFuncs) + { + foreach (var enter in EnterFuncs) + enter(); + } + } + + public SafeCallContext() + { + lock (EnterFuncs) + { + _count++; + _objects = EnterFuncs.Select(x => x()).ToList(); + } + } + + public void Dispose() + { + if (_disposed) throw new ObjectDisposedException("this"); + _disposed = true; + lock (EnterFuncs) + { + for (var i = 0; i < ExitActions.Count; i++) + ExitActions[i](_objects[i]); + _count--; + } + } + + // for unit tests ONLY + internal static void Reset() + { + lock (EnterFuncs) + { + if (_count > 0) throw new InvalidOperationException("Cannot reset while some SafeCallContext instances exist."); + EnterFuncs.Clear(); + ExitActions.Clear(); + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index c28cea24cd..9e323b0c7e 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -504,6 +504,7 @@ + diff --git a/src/Umbraco.Tests/Persistence/DatabaseContextTests.cs b/src/Umbraco.Tests/Persistence/DatabaseContextTests.cs index 94f0010201..a9b016c2b6 100644 --- a/src/Umbraco.Tests/Persistence/DatabaseContextTests.cs +++ b/src/Umbraco.Tests/Persistence/DatabaseContextTests.cs @@ -23,6 +23,8 @@ namespace Umbraco.Tests.Persistence [SetUp] public void Setup() { + SafeCallContext.Clear(); + _dbContext = new DatabaseContext( new DefaultDatabaseFactory(Core.Configuration.GlobalSettings.UmbracoConnectionName, Mock.Of()), Mock.Of(), new SqlCeSyntaxProvider(), Constants.DatabaseProviders.SqlCe); @@ -34,7 +36,7 @@ namespace Umbraco.Tests.Persistence { DatabaseContext = _dbContext, IsReady = true - }; + }; } [TearDown] @@ -99,7 +101,7 @@ namespace Umbraco.Tests.Persistence var appCtx = new ApplicationContext( new DatabaseContext(Mock.Of(), Mock.Of(), Mock.Of(), "test"), - new ServiceContext(migrationEntryService: Mock.Of()), + new ServiceContext(migrationEntryService: Mock.Of()), CacheHelper.CreateDisabledCacheHelper(), new ProfilingLogger(Mock.Of(), Mock.Of())); From 07f4da9b007c5d40909f83577bf9b5c8b7e110f2 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 2 Nov 2016 09:42:20 +0100 Subject: [PATCH 11/18] Cleanup WebProfiler from 8.x --- .../Profiling/StartupWebProfilerProvider.cs | 159 ------------------ src/Umbraco.Web/Profiling/WebProfiler.cs | 72 ++++---- .../Profiling/WebProfilerProvider.cs | 121 +++++++++++++ src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 4 files changed, 154 insertions(+), 200 deletions(-) delete mode 100644 src/Umbraco.Web/Profiling/StartupWebProfilerProvider.cs create mode 100644 src/Umbraco.Web/Profiling/WebProfilerProvider.cs diff --git a/src/Umbraco.Web/Profiling/StartupWebProfilerProvider.cs b/src/Umbraco.Web/Profiling/StartupWebProfilerProvider.cs deleted file mode 100644 index 72a398f17f..0000000000 --- a/src/Umbraco.Web/Profiling/StartupWebProfilerProvider.cs +++ /dev/null @@ -1,159 +0,0 @@ -using System.Threading; -using System.Web; -using StackExchange.Profiling; -using Umbraco.Core; - -namespace Umbraco.Web.Profiling -{ - /// - /// This is a custom MiniProfiler WebRequestProfilerProvider (which is generally the default) that allows - /// us to profile items during app startup - before an HttpRequest is created - /// - /// - /// Once the boot phase is changed to StartupPhase.Request then the base class (default) provider will handle all - /// profiling data and this sub class no longer performs any logic. - /// - internal class StartupWebProfilerProvider : WebRequestProfilerProvider - { - public StartupWebProfilerProvider() - { - _startupPhase = StartupPhase.Boot; - //create the startup profiler - _startupProfiler = new MiniProfiler("http://localhost/umbraco-startup", ProfileLevel.Verbose) - { - Name = "StartupProfiler" - }; - } - - private MiniProfiler _startupProfiler; - private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(); - - /// - /// Used to determine which phase the boot process is in - /// - private enum StartupPhase - { - None = 0, - Boot = 1, - Request = 2 - } - - private volatile StartupPhase _startupPhase; - - /// - /// Executed once the application boot process is complete and changes the phase to Request - /// - public void BootComplete() - { - using (new ReadLock(_locker)) - { - if (_startupPhase != StartupPhase.Boot) return; - } - - using (var l = new UpgradeableReadLock(_locker)) - { - if (_startupPhase == StartupPhase.Boot) - { - l.UpgradeToWriteLock(); - _startupPhase = StartupPhase.Request; - } - } - } - - /// - /// Executed when a profiling operation is completed - /// - /// - /// - /// This checks if the bootup phase is None, if so, it just calls the base class, otherwise it checks - /// if a profiler is active (i.e. in startup), then sets the phase to None so that the base class will be used - /// for all subsequent calls. - /// - public override void Stop(bool discardResults) - { - using (new ReadLock(_locker)) - { - if (_startupPhase == StartupPhase.None) - { - base.Stop(discardResults); - return; - } - } - - using (var l = new UpgradeableReadLock(_locker)) - { - if (_startupPhase > 0 && base.GetCurrentProfiler() == null) - { - l.UpgradeToWriteLock(); - - _startupPhase = StartupPhase.None; - - //This is required to pass the mini profiling context from before a request - // to the current startup request. - if (HttpContext.Current != null) - { - HttpContext.Current.Items[":mini-profiler:"] = _startupProfiler; - base.Stop(discardResults); - _startupProfiler = null; - } - } - else - { - base.Stop(discardResults); - } - } - } - - /// - /// Executed when a profiling operation is started - /// - /// - /// - /// - /// This checks if the startup phase is not None, if this is the case and the current profiler is NULL - /// then this sets the startup profiler to be active. Otherwise it just calls the base class Start method. - /// - public override MiniProfiler Start(ProfileLevel level) - { - using (new ReadLock(_locker)) - { - if (_startupPhase > 0 && base.GetCurrentProfiler() == null) - { - SetProfilerActive(_startupProfiler); - return _startupProfiler; - } - - return base.Start(level); - } - } - - /// - /// This returns the current profiler - /// - /// - /// - /// If the boot phase is not None, then this will return the startup profiler (this), otherwise - /// returns the base class - /// - public override MiniProfiler GetCurrentProfiler() - { - using (new ReadLock(_locker)) - { - if (_startupPhase > 0) - { - try - { - var current = base.GetCurrentProfiler(); - if (current == null) return _startupProfiler; - } - catch - { - return _startupProfiler; - } - } - - return base.GetCurrentProfiler(); - } - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/Profiling/WebProfiler.cs b/src/Umbraco.Web/Profiling/WebProfiler.cs index a1998c8761..62d69019d6 100644 --- a/src/Umbraco.Web/Profiling/WebProfiler.cs +++ b/src/Umbraco.Web/Profiling/WebProfiler.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using System.Web; using StackExchange.Profiling; using StackExchange.Profiling.SqlFormatters; @@ -14,22 +15,23 @@ namespace Umbraco.Web.Profiling /// internal class WebProfiler : IProfiler { - private StartupWebProfilerProvider _startupWebProfilerProvider; + private const string BootRequestItemKey = "Umbraco.Web.Profiling.WebProfiler__isBootRequest"; + private WebProfilerProvider _provider; + private int _first; /// /// Constructor /// internal WebProfiler() { - //setup some defaults + // create our own provider, which can provide a profiler even during boot + // MiniProfiler's default cannot because there's no HttpRequest in HttpContext + _provider = new WebProfilerProvider(); + + // settings MiniProfiler.Settings.SqlFormatter = new SqlServerFormatter(); MiniProfiler.Settings.StackMaxLength = 5000; - - //At this point we know that we've been constructed during app startup, there won't be an HttpRequest in the HttpContext - // since it hasn't started yet. So we need to do some hacking to enable profiling during startup. - _startupWebProfilerProvider = new StartupWebProfilerProvider(); - //this should always be the case during startup, we'll need to set a custom profiler provider - MiniProfiler.Settings.ProfilerProvider = _startupWebProfilerProvider; + MiniProfiler.Settings.ProfilerProvider = _provider; //Binds to application events to enable the MiniProfiler with a real HttpRequest UmbracoApplicationBase.ApplicationInit += UmbracoApplicationApplicationInit; @@ -57,43 +59,33 @@ namespace Umbraco.Web.Profiling } } - /// - /// Handle the begin request event - /// - /// - /// - void UmbracoApplicationEndRequest(object sender, EventArgs e) - { - if (_startupWebProfilerProvider != null) - { - Stop(); - _startupWebProfilerProvider = null; - } - else if (CanPerformProfilingAction(sender)) - { - Stop(); - } - } - - /// - /// Handle the end request event - /// - /// - /// void UmbracoApplicationBeginRequest(object sender, EventArgs e) { - if (_startupWebProfilerProvider != null) + // if this is the first request, notify our own provider that this request is the boot request + var first = Interlocked.Exchange(ref _first, 1) == 0; + if (first) { - _startupWebProfilerProvider.BootComplete(); + _provider.BeginBootRequest(); + ((HttpApplication)sender).Context.Items[BootRequestItemKey] = true; + // and no need to start anything, profiler is already there } - - if (CanPerformProfilingAction(sender)) - { + // else start a profiler, the normal way + else if (ShouldProfile(sender)) Start(); - } } - private bool CanPerformProfilingAction(object sender) + void UmbracoApplicationEndRequest(object sender, EventArgs e) + { + // if this is the boot request, or if we should profile this request, stop + // (the boot request is always profiled, no matter what) + var isBootRequest = ((HttpApplication)sender).Context.Items[BootRequestItemKey] != null; // fixme perfs + if (isBootRequest) + _provider.EndBootRequest(); + if (isBootRequest || ShouldProfile(sender)) + Stop(); + } + + private bool ShouldProfile(object sender) { if (GlobalSettings.DebugMode == false) return false; @@ -108,10 +100,10 @@ namespace Umbraco.Web.Profiling return false; if (string.IsNullOrEmpty(request.Result.QueryString["umbDebug"])) - return true; + return false; if (request.Result.Url.IsBackOfficeRequest(HttpRuntime.AppDomainAppVirtualPath)) - return true; + return false; return true; } diff --git a/src/Umbraco.Web/Profiling/WebProfilerProvider.cs b/src/Umbraco.Web/Profiling/WebProfilerProvider.cs new file mode 100644 index 0000000000..ffd1871ecc --- /dev/null +++ b/src/Umbraco.Web/Profiling/WebProfilerProvider.cs @@ -0,0 +1,121 @@ +using System; +using System.Threading; +using System.Web; +using StackExchange.Profiling; + +namespace Umbraco.Web.Profiling +{ + /// + /// This is a custom MiniProfiler WebRequestProfilerProvider (which is generally the default) that allows + /// us to profile items during app startup - before an HttpRequest is created + /// + /// + /// Once the boot phase is changed to StartupPhase.Request then the base class (default) provider will handle all + /// profiling data and this sub class no longer performs any logic. + /// + internal class WebProfilerProvider : WebRequestProfilerProvider + { + private readonly ReaderWriterLockSlim _locker = new ReaderWriterLockSlim(); + private MiniProfiler _startupProfiler; + private int _first; + private volatile BootPhase _bootPhase; + + public WebProfilerProvider() + { + // booting... + _bootPhase = BootPhase.Boot; + } + + /// + /// Indicates the boot phase. + /// + private enum BootPhase + { + Boot = 0, // boot phase (before the 1st request even begins) + BootRequest = 1, // request boot phase (during the 1st request) + Booted = 2 // done booting + } + + public void BeginBootRequest() + { + _locker.EnterWriteLock(); + try + { + if (_bootPhase != BootPhase.Boot) + throw new InvalidOperationException("Invalid boot phase."); + _bootPhase = BootPhase.BootRequest; + + // assign the profiler to be the current MiniProfiler for the request + // is's already active, starting and all + HttpContext.Current.Items[":mini-profiler:"] = _startupProfiler; + } + finally + { + _locker.ExitWriteLock(); + } + } + + public void EndBootRequest() + { + _locker.EnterWriteLock(); + try + { + if (_bootPhase != BootPhase.BootRequest) + throw new InvalidOperationException("Invalid boot phase."); + _bootPhase = BootPhase.Booted; + + _startupProfiler = null; + } + finally + { + _locker.ExitWriteLock(); + } + } + + /// + /// Executed when a profiling operation is started + /// + /// + /// + /// + /// This checks if the startup phase is not None, if this is the case and the current profiler is NULL + /// then this sets the startup profiler to be active. Otherwise it just calls the base class Start method. + /// + public override MiniProfiler Start(ProfileLevel level) + { + var first = Interlocked.Exchange(ref _first, 1) == 0; + if (first == false) return base.Start(level); + + _startupProfiler = new MiniProfiler("http://localhost/umbraco-startup") { Name = "StartupProfiler" }; + SetProfilerActive(_startupProfiler); + return _startupProfiler; + } + + /// + /// This returns the current profiler + /// + /// + /// + /// If the boot phase is not None, then this will return the startup profiler (this), otherwise + /// returns the base class + /// + public override MiniProfiler GetCurrentProfiler() + { + // if not booting then just use base (fast) + // no lock, _bootPhase is volatile + if (_bootPhase == BootPhase.Booted) + return base.GetCurrentProfiler(); + + // else + try + { + var current = base.GetCurrentProfiler(); + return current ?? _startupProfiler; + } + catch + { + return _startupProfiler; + } + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 7fb022e848..527d96dcea 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -382,7 +382,7 @@ - + From 8bb069e996a452b2dd4fa016bca32c3ec6d933cc Mon Sep 17 00:00:00 2001 From: Claus Date: Tue, 8 Nov 2016 09:55:24 +0100 Subject: [PATCH 12/18] U4-9134 XSS security issue in the grid exposing xss clean method on templateutilities. making the clean xss string extensions public instead of internal. ensuring the included grid renderers clean for xss. ensuring the included grid editors using html.raw with value directly, cleans for xss. --- src/Umbraco.Core/StringExtensions.cs | 2 +- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 2 + .../Partials/Grid/Bootstrap2-Fluid.cshtml | 31 ++-- .../Views/Partials/Grid/Bootstrap2.cshtml | 31 ++-- .../Partials/Grid/Bootstrap3-Fluid.cshtml | 32 ++-- .../Views/Partials/Grid/Bootstrap3.cshtml | 31 ++-- .../Views/Partials/Grid/Editors/Base.cshtml | 1 - .../Views/Partials/Grid/Editors/Embed.cshtml | 1 - .../Views/Partials/Grid/Editors/Macro.cshtml | 2 - .../Views/Partials/Grid/Editors/Media.cshtml | 1 - .../Partials/Grid/Editors/TextString.cshtml | 5 +- .../Templates/TemplateUtilities.cs | 143 +++++++++--------- 12 files changed, 164 insertions(+), 118 deletions(-) diff --git a/src/Umbraco.Core/StringExtensions.cs b/src/Umbraco.Core/StringExtensions.cs index 036b5b979f..27de06c371 100644 --- a/src/Umbraco.Core/StringExtensions.cs +++ b/src/Umbraco.Core/StringExtensions.cs @@ -184,7 +184,7 @@ namespace Umbraco.Core /// /// /// - internal static string CleanForXss(this string input, params char[] ignoreFromClean) + public static string CleanForXss(this string input, params char[] ignoreFromClean) { //remove any html input = input.StripHtml(); diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 6f2728a241..512c8177a9 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -1973,6 +1973,8 @@ + + Web.Template.config diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml index 446a82f510..65b9b8abc7 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml @@ -64,21 +64,32 @@ JObject cfg = contentItem.config; if(cfg != null) - foreach (JProperty property in cfg.Properties()) { - attrs.Add(property.Name + "='" + property.Value.ToString() + "'"); + foreach (JProperty property in cfg.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + attrs.Add(property.Name + "='" + propertyValue + "'"); + } } - + JObject style = contentItem.styles; - if (style != null) { - var cssVals = new List(); - foreach (JProperty property in style.Properties()) - cssVals.Add(property.Name + ":" + property.Value.ToString() + ";"); + if (style != null) { + var cssVals = new List(); + foreach (JProperty property in style.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + cssVals.Add(property.Name + ":" + propertyValue + ";"); + } + } - if (cssVals.Any()) - attrs.Add("style='" + string.Join(" ", cssVals) + "'"); + if (cssVals.Any()) + attrs.Add("style='" + string.Join(" ", cssVals) + "'"); } - + return new MvcHtmlString(string.Join(" ", attrs)); } } \ No newline at end of file diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml index 6bc730e1f8..37e3c84dad 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml @@ -64,21 +64,32 @@ JObject cfg = contentItem.config; if(cfg != null) - foreach (JProperty property in cfg.Properties()) { - attrs.Add(property.Name + "=\"" + property.Value.ToString() + "\""); + foreach (JProperty property in cfg.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + attrs.Add(property.Name + "=\"" + propertyValue + "\""); + } } - + JObject style = contentItem.styles; - if (style != null) { - var cssVals = new List(); - foreach (JProperty property in style.Properties()) - cssVals.Add(property.Name + ":" + property.Value.ToString() + ";"); + if (style != null) { + var cssVals = new List(); + foreach (JProperty property in style.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + cssVals.Add(property.Name + ":" + propertyValue + ";"); + } + } - if (cssVals.Any()) - attrs.Add("style=\"" + string.Join(" ", cssVals) + "\""); + if (cssVals.Any()) + attrs.Add("style=\"" + string.Join(" ", cssVals) + "\""); } - + return new MvcHtmlString(string.Join(" ", attrs)); } } \ No newline at end of file diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml index 1244821d7e..45be239245 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml @@ -5,6 +5,7 @@ @* Razor helpers located at the bottom of this file *@ + @if (Model != null && Model.sections != null) { var oneColumn = ((System.Collections.ICollection)Model.sections).Count == 1; @@ -59,21 +60,32 @@ JObject cfg = contentItem.config; if(cfg != null) - foreach (JProperty property in cfg.Properties()) { - attrs.Add(property.Name + "='" + property.Value.ToString() + "'"); + foreach (JProperty property in cfg.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + attrs.Add(property.Name + "='" + propertyValue + "'"); + } } - + JObject style = contentItem.styles; - if (style != null) { - var cssVals = new List(); - foreach (JProperty property in style.Properties()) - cssVals.Add(property.Name + ":" + property.Value.ToString() + ";"); + if (style != null) { + var cssVals = new List(); + foreach (JProperty property in style.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + cssVals.Add(property.Name + ":" + propertyValue + ";"); + } + } - if (cssVals.Any()) - attrs.Add("style='" + string.Join(" ", cssVals) + "'"); + if (cssVals.Any()) + attrs.Add("style='" + string.Join(" ", cssVals) + "'"); } - + return new MvcHtmlString(string.Join(" ", attrs)); } } \ No newline at end of file diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml index f76028d296..afadd3d93e 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml @@ -64,21 +64,32 @@ JObject cfg = contentItem.config; if(cfg != null) - foreach (JProperty property in cfg.Properties()) { - attrs.Add(property.Name + "=\"" + property.Value.ToString() + "\""); + foreach (JProperty property in cfg.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + attrs.Add(property.Name + "=\"" + propertyValue +"\""); + } } - + JObject style = contentItem.styles; - if (style != null) { - var cssVals = new List(); - foreach (JProperty property in style.Properties()) - cssVals.Add(property.Name + ":" + property.Value.ToString() + ";"); + if (style != null) { + var cssVals = new List(); + foreach (JProperty property in style.Properties()) + { + var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); + if (string.IsNullOrWhiteSpace(propertyValue) == false) + { + cssVals.Add(property.Name + ":" + propertyValue + ";"); + } + } - if (cssVals.Any()) - attrs.Add("style=\"" + string.Join(" ", cssVals) + "\""); + if (cssVals.Any()) + attrs.Add("style=\"" + string.Join(" ", cssVals) + "\""); } - + return new MvcHtmlString(string.Join(" ", attrs)); } } \ No newline at end of file diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml index a86c04819a..ffb7603048 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Base.cshtml @@ -1,5 +1,4 @@ @model dynamic -@using Umbraco.Web.Templates @functions { public static string EditorView(dynamic contentItem) diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml index 4fd66ddb90..c27be6bcdf 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Embed.cshtml @@ -1,3 +1,2 @@ @model dynamic -@using Umbraco.Web.Templates @Html.Raw(Model.value) diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml index e0822808d8..ed08bb2484 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Macro.cshtml @@ -1,6 +1,4 @@ @inherits UmbracoViewPage -@using Umbraco.Web.Templates - @if (Model.value != null) { diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml index f5dfc6459c..5b5adbdc7d 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/Media.cshtml @@ -1,5 +1,4 @@ @model dynamic -@using Umbraco.Web.Templates @if (Model.value != null) { diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml index a031c658a9..4a15201997 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml @@ -4,10 +4,9 @@ @if (Model.editor.config.markup != null) { string markup = Model.editor.config.markup.ToString(); - var UmbracoHelper = new UmbracoHelper(UmbracoContext.Current); - markup = markup.Replace("#value#", UmbracoHelper.ReplaceLineBreaksForHtml(Model.value.ToString())); + markup = markup.Replace("#value#", UmbracoHelper.ReplaceLineBreaksForHtml(TemplateUtilities.CleanForXss(Model.value.ToString()))); markup = markup.Replace("#style#", Model.editor.config.style.ToString()); @@ -17,6 +16,6 @@ else { -
@Model.value
+
@TemplateUtilities.CleanForXss(Model.value.ToString())
 } diff --git a/src/Umbraco.Web/Templates/TemplateUtilities.cs b/src/Umbraco.Web/Templates/TemplateUtilities.cs index 881cb563c5..c56d7b5b8a 100644 --- a/src/Umbraco.Web/Templates/TemplateUtilities.cs +++ b/src/Umbraco.Web/Templates/TemplateUtilities.cs @@ -7,17 +7,17 @@ using Umbraco.Core.Logging; namespace Umbraco.Web.Templates { - //NOTE: I realize there is only one class in this namespace but I'm pretty positive that there will be more classes in - //this namespace once we start migrating and cleaning up more code. + //NOTE: I realize there is only one class in this namespace but I'm pretty positive that there will be more classes in + //this namespace once we start migrating and cleaning up more code. - /// - /// Utility class used for templates - /// - public static class TemplateUtilities - { + /// + /// Utility class used for templates + /// + public static class TemplateUtilities + { //TODO: Pass in an Umbraco context!!!!!!!! Don't rely on the singleton so things are more testable internal static string ParseInternalLinks(string text, bool preview) - { + { // save and set for url provider var inPreviewMode = UmbracoContext.Current.InPreviewMode; UmbracoContext.Current.InPreviewMode = preview; @@ -33,79 +33,84 @@ namespace Umbraco.Web.Templates } return text; - } + } - /// - /// Parses the string looking for the {localLink} syntax and updates them to their correct links. - /// - /// - /// - public static string ParseInternalLinks(string text) - { + /// + /// Parses the string looking for the {localLink} syntax and updates them to their correct links. + /// + /// + /// + public static string ParseInternalLinks(string text) + { //TODO: Pass in an Umbraco context!!!!!!!! Don't rely on the singleton so things are more testable, better yet, pass in urlprovider, routing context, separately - //don't attempt to proceed without a context as we cannot lookup urls without one - if (UmbracoContext.Current == null || UmbracoContext.Current.RoutingContext == null) - { - return text; - } + //don't attempt to proceed without a context as we cannot lookup urls without one + if (UmbracoContext.Current == null || UmbracoContext.Current.RoutingContext == null) + { + return text; + } - var urlProvider = UmbracoContext.Current.UrlProvider; + var urlProvider = UmbracoContext.Current.UrlProvider; - // Parse internal links - var tags = Regex.Matches(text, @"href=""[/]?(?:\{|\%7B)localLink:([0-9]+)(?:\}|\%7D)", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); - foreach (Match tag in tags) - if (tag.Groups.Count > 0) - { - var id = tag.Groups[1].Value; //.Remove(tag.Groups[1].Value.Length - 1, 1); - var newLink = urlProvider.GetUrl(int.Parse(id)); - text = text.Replace(tag.Value, "href=\"" + newLink); - } + // Parse internal links + var tags = Regex.Matches(text, @"href=""[/]?(?:\{|\%7B)localLink:([0-9]+)(?:\}|\%7D)", RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); + foreach (Match tag in tags) + if (tag.Groups.Count > 0) + { + var id = tag.Groups[1].Value; //.Remove(tag.Groups[1].Value.Length - 1, 1); + var newLink = urlProvider.GetUrl(int.Parse(id)); + text = text.Replace(tag.Value, "href=\"" + newLink); + } return text; - } + } - // static compiled regex for faster performance - private readonly static Regex ResolveUrlPattern = new Regex("(=[\"\']?)(\\W?\\~(?:.(?![\"\']?\\s+(?:\\S+)=|[>\"\']))+.)[\"\']?", RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); + // static compiled regex for faster performance + private readonly static Regex ResolveUrlPattern = new Regex("(=[\"\']?)(\\W?\\~(?:.(?![\"\']?\\s+(?:\\S+)=|[>\"\']))+.)[\"\']?", + RegexOptions.Compiled | RegexOptions.IgnoreCase | RegexOptions.IgnorePatternWhitespace); - /// - /// The RegEx matches any HTML attribute values that start with a tilde (~), those that match are passed to ResolveUrl to replace the tilde with the application path. - /// - /// - /// - /// - /// When used with a Virtual-Directory set-up, this would resolve all URLs correctly. - /// The recommendation is that the "ResolveUrlsFromTextString" option (in umbracoSettings.config) is set to false for non-Virtual-Directory installs. - /// - public static string ResolveUrlsFromTextString(string text) - { + /// + /// The RegEx matches any HTML attribute values that start with a tilde (~), those that match are passed to ResolveUrl to replace the tilde with the application path. + /// + /// + /// + /// + /// When used with a Virtual-Directory set-up, this would resolve all URLs correctly. + /// The recommendation is that the "ResolveUrlsFromTextString" option (in umbracoSettings.config) is set to false for non-Virtual-Directory installs. + /// + public static string ResolveUrlsFromTextString(string text) + { if (UmbracoConfig.For.UmbracoSettings().Content.ResolveUrlsFromTextString == false) return text; - using (var timer = DisposableTimer.DebugDuration(typeof(IOHelper), "ResolveUrlsFromTextString starting", "ResolveUrlsFromTextString complete")) - { - // find all relative urls (ie. urls that contain ~) - var tags = ResolveUrlPattern.Matches(text); - LogHelper.Debug(typeof(IOHelper), "After regex: " + timer.Stopwatch.ElapsedMilliseconds + " matched: " + tags.Count); - foreach (Match tag in tags) - { - var url = ""; - if (tag.Groups[1].Success) - url = tag.Groups[1].Value; + using (var timer = DisposableTimer.DebugDuration(typeof(IOHelper), "ResolveUrlsFromTextString starting", "ResolveUrlsFromTextString complete")) + { + // find all relative urls (ie. urls that contain ~) + var tags = ResolveUrlPattern.Matches(text); + LogHelper.Debug(typeof(IOHelper), "After regex: " + timer.Stopwatch.ElapsedMilliseconds + " matched: " + tags.Count); + foreach (Match tag in tags) + { + var url = ""; + if (tag.Groups[1].Success) + url = tag.Groups[1].Value; - // The richtext editor inserts a slash in front of the url. That's why we need this little fix - // if (url.StartsWith("/")) - // text = text.Replace(url, ResolveUrl(url.Substring(1))); - // else - if (String.IsNullOrEmpty(url) == false) - { - var resolvedUrl = (url.Substring(0, 1) == "/") ? IOHelper.ResolveUrl(url.Substring(1)) : IOHelper.ResolveUrl(url); - text = text.Replace(url, resolvedUrl); - } - } - } + // The richtext editor inserts a slash in front of the url. That's why we need this little fix + // if (url.StartsWith("/")) + // text = text.Replace(url, ResolveUrl(url.Substring(1))); + // else + if (String.IsNullOrEmpty(url) == false) + { + var resolvedUrl = (url.Substring(0, 1) == "/") ? IOHelper.ResolveUrl(url.Substring(1)) : IOHelper.ResolveUrl(url); + text = text.Replace(url, resolvedUrl); + } + } + } - return text; - } + return text; + } - } + public static string CleanForXss(string text, params char[] ignoreFromClean) + { + return text.CleanForXss(ignoreFromClean); + } + } } From 3deda7efabf436d905cad69b6d7a7c164bd31de9 Mon Sep 17 00:00:00 2001 From: Claus Date: Wed, 9 Nov 2016 13:09:31 +0100 Subject: [PATCH 13/18] U4-7833 Changing doc type results in invalid cache, lucene indexes, etc... For now: add messages in the UI to tell people they need to publish the items again and rebuilt their indexes. --- src/Umbraco.Web.UI/umbraco/config/lang/da.xml | 3 ++- src/Umbraco.Web.UI/umbraco/config/lang/en.xml | 2 ++ src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml | 1 + src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx | 4 +++- src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs | 3 +++ .../umbraco/dialogs/ChangeDocType.aspx.designer.cs | 9 +++++++++ 6 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/da.xml b/src/Umbraco.Web.UI/umbraco/config/lang/da.xml index b0452fb631..b9996db11c 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/da.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/da.xml @@ -109,9 +109,10 @@ Ny skabelon Ny type ingen + Du bør genopbygge dine Examine indekser via 'Examine Management' dashboarded i udvikler sektionen for at sikre at denne ændring registreres i hele dit website. Indhold Vælg ny dokumenttype - Dokumenttypen på detvalgte indhold blev skiftet til [new type], og følgende egenskaber blev overført: + Dokumenttypen på det valgte indhold blev skiftet til [new type], og følgende egenskaber blev overført: til Overførsel af egenskaber kunne ikke fuldføres, da en eller flere egenskaber er indstillet til at blive overført mere end én gang. Kun andre dokumenttyper, der er gyldige på denne placering, vises. diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/umbraco/config/lang/en.xml index bdc2647563..317c044f9d 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en.xml @@ -100,6 +100,7 @@ Show styles Insert table Generate models + Save and generate models To change the document type for the selected content, first select from the list of valid types for this location. @@ -114,6 +115,7 @@ New Template New Type none + Please make sure to rebuild your Examine indexes using the 'Examine Management' dashboard in the Developer section, to ensure this change is applied everywhere. Content Select New Document Type The document type of the selected content has been successfully changed to [new type] and the following properties mapped: diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml index 7544cacc0c..7a39c59406 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml @@ -115,6 +115,7 @@ New Template New Type none + Please make sure to rebuild your Examine indexes using the 'Examine Management' dashboard in the Developer section, to ensure this change is applied everywhere. Content Select New Document Type The document type of the selected content has been successfully changed to [new type] and the following properties mapped: diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx index 77deab2709..a11c5cab49 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx +++ b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx @@ -97,7 +97,9 @@

- <%=umbraco.ui.Text("defaultdialogs", "closeThisWindow") %> + +

+ <%=umbraco.ui.Text("defaultdialogs", "closeThisWindow") %>

diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs index 0db3b6c36e..5d9243f994 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs +++ b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs @@ -265,10 +265,13 @@ namespace Umbraco.Web.UI.Umbraco.Dialogs { ContentPublishedMessage.Text = global::umbraco.ui.Text("changeDocType", "contentRepublished"); ContentPublishedMessage.Visible = true; + RebuildIndexesMessage.Text = global::umbraco.ui.Text("changeDocType", "rebuildIndexes"); + RebuildIndexesMessage.Visible = true; } else { ContentPublishedMessage.Visible = false; + RebuildIndexesMessage.Visible = false; } SuccessPlaceholder.Visible = true; SaveAndCancelPlaceholder.Visible = false; diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs index b774a3ff79..5fa673836a 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs +++ b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs @@ -165,6 +165,15 @@ namespace Umbraco.Web.UI.Umbraco.Dialogs { /// protected global::System.Web.UI.WebControls.Literal ContentPublishedMessage; + /// + /// RebuildIndexesMessage control. + /// + /// + /// Auto-generated field. + /// To modify move field declaration from designer file to code-behind file. + /// + protected global::System.Web.UI.WebControls.Literal RebuildIndexesMessage; + /// /// ValidationPlaceholder control. /// From df12f19c53b7e4bf0ae1df89e212579a4bfdde24 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 14 Nov 2016 16:08:21 +0100 Subject: [PATCH 14/18] U4-9115 - fix issue --- .../Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs index d5c377fba1..c56ae0e2f1 100644 --- a/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs +++ b/src/Umbraco.Core/Persistence/Migrations/Upgrades/TargetVersionSevenFiveFive/AddLockObjects.cs @@ -31,7 +31,7 @@ namespace Umbraco.Core.Persistence.Migrations.Upgrades.TargetVersionSevenFiveFiv // be safe: delete old umbracoNode lock objects if any db.Execute("DELETE FROM umbracoNode WHERE id=@id;", new { id }); // then create umbracoLock object - db.Execute("INSERT umbracoLock (id, name, value) VALUES (@id, '@name', 1);", new { id, name }); + db.Execute("INSERT umbracoLock (id, name, value) VALUES (@id, @name, 1);", new { id, name }); return string.Empty; }); } From 740abb14265541891bf7f89f629b1c5cb7199821 Mon Sep 17 00:00:00 2001 From: Sebastiaan Janssen Date: Mon, 14 Nov 2016 16:17:58 +0100 Subject: [PATCH 15/18] We don't support C# 6 yet --- src/Umbraco.Tests/Persistence/LocksTests.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Tests/Persistence/LocksTests.cs b/src/Umbraco.Tests/Persistence/LocksTests.cs index e973b14614..624a94f39f 100644 --- a/src/Umbraco.Tests/Persistence/LocksTests.cs +++ b/src/Umbraco.Tests/Persistence/LocksTests.cs @@ -254,7 +254,9 @@ namespace Umbraco.Tests.Persistence var info = database.Query("SELECT * FROM sys.lock_information;"); Console.WriteLine("LOCKS:"); foreach (var row in info) - Console.WriteLine($"> {row.request_spid} {row.resource_type} {row.resource_description} {row.request_mode} {row.resource_table} {row.resource_table_id} {row.request_status}"); + Console.WriteLine(string.Format("> {0} {1} {2} {3} {4} {5} {6}", row.request_spid, + row.resource_type, row.resource_description, row.request_mode, row.resource_table, + row.resource_table_id, row.request_status)); Thread.Sleep(6000); } catch (Exception e) @@ -277,7 +279,9 @@ namespace Umbraco.Tests.Persistence var info = database.Query("SELECT * FROM sys.lock_information;"); Console.WriteLine("LOCKS:"); foreach (var row in info) - Console.WriteLine($"> {row.request_spid} {row.resource_type} {row.resource_description} {row.request_mode} {row.resource_table} {row.resource_table_id} {row.request_status}"); + Console.WriteLine(string.Format("> {0} {1} {2} {3} {4} {5} {6}", row.request_spid, + row.resource_type, row.resource_description, row.request_mode, row.resource_table, + row.resource_table_id, row.request_status)); Thread.Sleep(1000); } catch (Exception e) From 2de465e8f397f4cd87550984ea2648e4477e0d8c Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 14 Nov 2016 17:39:28 +0100 Subject: [PATCH 16/18] Ensures that indexes are rebuild for items that have had their content type's or property type's aliases changed, also ensures that the content refresher kicks in if a property type alias has changed (this wasn't previously being done) --- src/Umbraco.Core/Models/ContentTypeBase.cs | 2 +- .../Cache/ContentTypeCacheRefresher.cs | 73 +++++++++-------- src/Umbraco.Web/Search/ExamineEvents.cs | 80 ++++++++++++++++++- 3 files changed, 114 insertions(+), 41 deletions(-) diff --git a/src/Umbraco.Core/Models/ContentTypeBase.cs b/src/Umbraco.Core/Models/ContentTypeBase.cs index 2982713e5a..88476f946d 100644 --- a/src/Umbraco.Core/Models/ContentTypeBase.cs +++ b/src/Umbraco.Core/Models/ContentTypeBase.cs @@ -326,7 +326,7 @@ namespace Umbraco.Core.Models } } - /// + /// /// A boolean flag indicating if a property type has been removed from this instance. /// /// diff --git a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs index 44a6efe9ff..246571d479 100644 --- a/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs +++ b/src/Umbraco.Web/Cache/ContentTypeCacheRefresher.cs @@ -32,7 +32,7 @@ namespace Umbraco.Web.Cache /// /// /// - private static JsonPayload[] DeserializeFromJsonPayload(string json) + internal static JsonPayload[] DeserializeFromJsonPayload(string json) { var serializer = new JavaScriptSerializer(); var jsonObject = serializer.Deserialize(json); @@ -45,30 +45,28 @@ namespace Umbraco.Web.Cache /// /// if the item was deleted /// - private static JsonPayload FromContentType(IContentTypeBase contentType, bool isDeleted = false) + internal static JsonPayload FromContentType(IContentTypeBase contentType, bool isDeleted = false) { var payload = new JsonPayload - { - Alias = contentType.Alias, - Id = contentType.Id, - PropertyTypeIds = contentType.PropertyTypes.Select(x => x.Id).ToArray(), - //either IContentType or IMediaType or IMemberType - Type = (contentType is IContentType) - ? typeof(IContentType).Name - : (contentType is IMediaType) + { + Alias = contentType.Alias, + Id = contentType.Id, + PropertyTypeIds = contentType.PropertyTypes.Select(x => x.Id).ToArray(), + //either IContentType or IMediaType or IMemberType + Type = (contentType is IContentType) + ? typeof(IContentType).Name + : (contentType is IMediaType) ? typeof(IMediaType).Name : typeof(IMemberType).Name, - DescendantPayloads = contentType.Descendants().Select(x => FromContentType(x)).ToArray(), - WasDeleted = isDeleted - }; - //here we need to check if the alias of the content type changed or if one of the properties was removed. - var dirty = contentType as IRememberBeingDirty; - if (dirty != null) - { - payload.PropertyRemoved = dirty.WasPropertyDirty("HasPropertyTypeBeenRemoved"); - payload.AliasChanged = dirty.WasPropertyDirty("Alias"); - payload.IsNew = dirty.WasPropertyDirty("HasIdentity"); - } + DescendantPayloads = contentType.Descendants().Select(x => FromContentType(x)).ToArray(), + WasDeleted = isDeleted, + PropertyRemoved = contentType.WasPropertyDirty("HasPropertyTypeBeenRemoved"), + AliasChanged = contentType.WasPropertyDirty("Alias"), + PropertyTypeAliasChanged = contentType.PropertyTypes.Any(x => x.WasPropertyDirty("Alias")), + IsNew = contentType.WasPropertyDirty("HasIdentity") + }; + + return payload; } @@ -90,7 +88,7 @@ namespace Umbraco.Web.Cache #region Sub classes - private class JsonPayload + internal class JsonPayload { public JsonPayload() { @@ -103,6 +101,7 @@ namespace Umbraco.Web.Cache public string Type { get; set; } public bool AliasChanged { get; set; } public bool PropertyRemoved { get; set; } + public bool PropertyTypeAliasChanged { get; set; } public JsonPayload[] DescendantPayloads { get; set; } public bool WasDeleted { get; set; } public bool IsNew { get; set; } @@ -190,21 +189,21 @@ namespace Umbraco.Web.Cache ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheByKeySearch(CacheKeys.IdToKeyCacheKey); ApplicationContext.Current.ApplicationCache.RuntimeCache.ClearCacheByKeySearch(CacheKeys.KeyToIdCacheKey); - payloads.ForEach(payload => + foreach (var payload in payloads) + { + //clear the cache for each item + ClearContentTypeCache(payload); + + //we only need to do this for IContentType NOT for IMediaType, we don't want to refresh the whole cache. + //if the item was deleted or the alias changed or property removed then we need to refresh the content. + //and, don't refresh the cache if it is new. + if (payload.Type == typeof(IContentType).Name + && payload.IsNew == false + && (payload.WasDeleted || payload.AliasChanged || payload.PropertyRemoved || payload.PropertyTypeAliasChanged)) { - //clear the cache for each item - ClearContentTypeCache(payload); - - //we only need to do this for IContentType NOT for IMediaType, we don't want to refresh the whole cache. - //if the item was deleted or the alias changed or property removed then we need to refresh the content. - //and, don't refresh the cache if it is new. - if (payload.Type == typeof(IContentType).Name - && !payload.IsNew - && (payload.WasDeleted || payload.AliasChanged || payload.PropertyRemoved)) - { - needsContentRefresh = true; - } - }); + needsContentRefresh = true; + } + } //need to refresh the xml content cache if required if (needsContentRefresh) @@ -237,7 +236,7 @@ namespace Umbraco.Web.Cache //cache if only a media type has changed. //we don't want to update the routes cache if all of the content types here are new. if (payloads.Any(x => x.Type == typeof(IContentType).Name) - && !payloads.All(x => x.IsNew)) //if they are all new then don't proceed + && payloads.All(x => x.IsNew) == false) //if they are all new then don't proceed { // SD: we need to clear the routes cache here! // diff --git a/src/Umbraco.Web/Search/ExamineEvents.cs b/src/Umbraco.Web/Search/ExamineEvents.cs index 20701fe330..d02850bffe 100644 --- a/src/Umbraco.Web/Search/ExamineEvents.cs +++ b/src/Umbraco.Web/Search/ExamineEvents.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Xml; @@ -67,12 +68,12 @@ namespace Umbraco.Web.Search /// /// This is used to refresh content indexers IndexData based on the DataService whenever a content type is changed since - /// properties may have been added/removed + /// properties may have been added/removed, then we need to re-index any required data if aliases have been changed /// /// /// /// - /// See: http://issues.umbraco.org/issue/U4-4798 + /// See: http://issues.umbraco.org/issue/U4-4798, http://issues.umbraco.org/issue/U4-7833 /// static void ContentTypeCacheRefresherCacheUpdated(ContentTypeCacheRefresher sender, CacheRefresherEventArgs e) { @@ -81,6 +82,79 @@ namespace Umbraco.Web.Search { provider.RefreshIndexerDataFromDataService(); } + + if (e.MessageType == MessageType.RefreshByJson) + { + var contentTypesChanged = new HashSet(); + var mediaTypesChanged = new HashSet(); + var memberTypesChanged = new HashSet(); + + var payloads = ContentTypeCacheRefresher.DeserializeFromJsonPayload(e.MessageObject.ToString()); + foreach (var payload in payloads) + { + if (payload.IsNew == false + && (payload.WasDeleted || payload.AliasChanged || payload.PropertyRemoved || payload.PropertyTypeAliasChanged)) + { + //if we get here it means that some aliases have changed and the indexes for those particular doc types will need to be updated + if (payload.Type == typeof(IContentType).Name) + { + //if it is content + contentTypesChanged.Add(payload.Alias); + } + else if (payload.Type == typeof(IMediaType).Name) + { + //if it is media + mediaTypesChanged.Add(payload.Alias); + } + else if (payload.Type == typeof(IMemberType).Name) + { + //if it is members + memberTypesChanged.Add(payload.Alias); + } + } + } + + //TODO: We need to update Examine to support re-indexing multiple items at once instead of one by one which will speed up + // the re-indexing process, we don't want to revert to rebuilding the whole thing! + + if (contentTypesChanged.Count > 0) + { + foreach (var alias in contentTypesChanged) + { + var ctType = ApplicationContext.Current.Services.ContentTypeService.GetContentType(alias); + var contentItems = ApplicationContext.Current.Services.ContentService.GetContentOfContentType(ctType.Id); + foreach (var contentItem in contentItems) + { + ReIndexForContent(contentItem, contentItem.HasPublishedVersion && contentItem.Trashed == false); + } + } + } + if (mediaTypesChanged.Count > 0) + { + foreach (var alias in mediaTypesChanged) + { + var ctType = ApplicationContext.Current.Services.ContentTypeService.GetMediaType(alias); + var mediaItems = ApplicationContext.Current.Services.MediaService.GetMediaOfMediaType(ctType.Id); + foreach (var mediaItem in mediaItems) + { + ReIndexForMedia(mediaItem, mediaItem.Trashed == false); + } + } + } + if (memberTypesChanged.Count > 0) + { + foreach (var alias in memberTypesChanged) + { + var ctType = ApplicationContext.Current.Services.MemberTypeService.Get(alias); + var memberItems = ApplicationContext.Current.Services.MemberService.GetMembersByMemberType(ctType.Id); + foreach (var memberItem in memberItems) + { + ReIndexForMember(memberItem); + } + } + } + } + } static void MemberCacheRefresherCacheUpdated(MemberCacheRefresher sender, CacheRefresherEventArgs e) @@ -432,7 +506,7 @@ namespace Umbraco.Web.Search //add an icon attribute to get indexed xml.Add(new XAttribute("icon", sender.ContentType.Icon)); - ExamineManager.Instance.ReIndexNode( + ExamineManager.Instance.ReIndexNode( xml, IndexTypes.Content, ExamineManager.Instance.IndexProviderCollection.OfType() From 213f02cbc6b7f30f94ab12d3e122e0f28947b537 Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 14 Nov 2016 17:39:46 +0100 Subject: [PATCH 17/18] Revert "U4-7833 Changing doc type results in invalid cache, lucene indexes, etc..." This reverts commit 3deda7efabf436d905cad69b6d7a7c164bd31de9. --- src/Umbraco.Web.UI/umbraco/config/lang/da.xml | 3 +-- src/Umbraco.Web.UI/umbraco/config/lang/en.xml | 2 -- src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml | 1 - src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx | 4 +--- src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs | 3 --- .../umbraco/dialogs/ChangeDocType.aspx.designer.cs | 9 --------- 6 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/da.xml b/src/Umbraco.Web.UI/umbraco/config/lang/da.xml index b9996db11c..b0452fb631 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/da.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/da.xml @@ -109,10 +109,9 @@ Ny skabelon Ny type ingen - Du bør genopbygge dine Examine indekser via 'Examine Management' dashboarded i udvikler sektionen for at sikre at denne ændring registreres i hele dit website. Indhold Vælg ny dokumenttype - Dokumenttypen på det valgte indhold blev skiftet til [new type], og følgende egenskaber blev overført: + Dokumenttypen på detvalgte indhold blev skiftet til [new type], og følgende egenskaber blev overført: til Overførsel af egenskaber kunne ikke fuldføres, da en eller flere egenskaber er indstillet til at blive overført mere end én gang. Kun andre dokumenttyper, der er gyldige på denne placering, vises. diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/en.xml b/src/Umbraco.Web.UI/umbraco/config/lang/en.xml index 317c044f9d..bdc2647563 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en.xml @@ -100,7 +100,6 @@ Show styles Insert table Generate models - Save and generate models To change the document type for the selected content, first select from the list of valid types for this location. @@ -115,7 +114,6 @@ New Template New Type none - Please make sure to rebuild your Examine indexes using the 'Examine Management' dashboard in the Developer section, to ensure this change is applied everywhere. Content Select New Document Type The document type of the selected content has been successfully changed to [new type] and the following properties mapped: diff --git a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml index 7a39c59406..7544cacc0c 100644 --- a/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml +++ b/src/Umbraco.Web.UI/umbraco/config/lang/en_us.xml @@ -115,7 +115,6 @@ New Template New Type none - Please make sure to rebuild your Examine indexes using the 'Examine Management' dashboard in the Developer section, to ensure this change is applied everywhere. Content Select New Document Type The document type of the selected content has been successfully changed to [new type] and the following properties mapped: diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx index a11c5cab49..77deab2709 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx +++ b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx @@ -97,9 +97,7 @@

- -

- <%=umbraco.ui.Text("defaultdialogs", "closeThisWindow") %> + <%=umbraco.ui.Text("defaultdialogs", "closeThisWindow") %>

diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs index 5d9243f994..0db3b6c36e 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs +++ b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.cs @@ -265,13 +265,10 @@ namespace Umbraco.Web.UI.Umbraco.Dialogs { ContentPublishedMessage.Text = global::umbraco.ui.Text("changeDocType", "contentRepublished"); ContentPublishedMessage.Visible = true; - RebuildIndexesMessage.Text = global::umbraco.ui.Text("changeDocType", "rebuildIndexes"); - RebuildIndexesMessage.Visible = true; } else { ContentPublishedMessage.Visible = false; - RebuildIndexesMessage.Visible = false; } SuccessPlaceholder.Visible = true; SaveAndCancelPlaceholder.Visible = false; diff --git a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs index 5fa673836a..b774a3ff79 100644 --- a/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs +++ b/src/Umbraco.Web.UI/umbraco/dialogs/ChangeDocType.aspx.designer.cs @@ -165,15 +165,6 @@ namespace Umbraco.Web.UI.Umbraco.Dialogs { /// protected global::System.Web.UI.WebControls.Literal ContentPublishedMessage; - /// - /// RebuildIndexesMessage control. - /// - /// - /// Auto-generated field. - /// To modify move field declaration from designer file to code-behind file. - /// - protected global::System.Web.UI.WebControls.Literal RebuildIndexesMessage; - /// /// ValidationPlaceholder control. /// From 6f39439e3dffd915e885f6186f19b8e3d3436fdf Mon Sep 17 00:00:00 2001 From: Claus Date: Tue, 15 Nov 2016 12:10:36 +0100 Subject: [PATCH 18/18] removing string.empty checks for attributes. removing the redundant clean for outputting model.value in TextString.cshtml. --- .../Views/Partials/Grid/Bootstrap2-Fluid.cshtml | 5 +---- src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml | 5 +---- .../Views/Partials/Grid/Bootstrap3-Fluid.cshtml | 5 +---- src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml | 5 +---- .../Views/Partials/Grid/Editors/TextString.cshtml | 2 +- 5 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml index 65b9b8abc7..f6b93139ce 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2-Fluid.cshtml @@ -67,10 +67,7 @@ foreach (JProperty property in cfg.Properties()) { var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); - if (string.IsNullOrWhiteSpace(propertyValue) == false) - { - attrs.Add(property.Name + "='" + propertyValue + "'"); - } + attrs.Add(property.Name + "=\"" + propertyValue + "\""); } JObject style = contentItem.styles; diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml index 37e3c84dad..c5fabe2abf 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap2.cshtml @@ -67,10 +67,7 @@ foreach (JProperty property in cfg.Properties()) { var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); - if (string.IsNullOrWhiteSpace(propertyValue) == false) - { - attrs.Add(property.Name + "=\"" + propertyValue + "\""); - } + attrs.Add(property.Name + "=\"" + propertyValue + "\""); } JObject style = contentItem.styles; diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml index 45be239245..b7e8ef34fb 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3-Fluid.cshtml @@ -63,10 +63,7 @@ foreach (JProperty property in cfg.Properties()) { var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); - if (string.IsNullOrWhiteSpace(propertyValue) == false) - { - attrs.Add(property.Name + "='" + propertyValue + "'"); - } + attrs.Add(property.Name + "=\"" + propertyValue + "\""); } JObject style = contentItem.styles; diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml index afadd3d93e..3a4fa3b8e2 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Bootstrap3.cshtml @@ -67,10 +67,7 @@ foreach (JProperty property in cfg.Properties()) { var propertyValue = TemplateUtilities.CleanForXss(property.Value.ToString()); - if (string.IsNullOrWhiteSpace(propertyValue) == false) - { - attrs.Add(property.Name + "=\"" + propertyValue +"\""); - } + attrs.Add(property.Name + "=\"" + propertyValue + "\""); } JObject style = contentItem.styles; diff --git a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml index 4a15201997..5a570efdb5 100644 --- a/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml +++ b/src/Umbraco.Web.UI/Views/Partials/Grid/Editors/TextString.cshtml @@ -16,6 +16,6 @@ else { -
@TemplateUtilities.CleanForXss(Model.value.ToString())
+
@Model.value
 }