From d7e61515de573044224fe4b743e7e3edace593fc Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 17 Oct 2018 09:59:16 +0200 Subject: [PATCH 01/12] Refactor TypeLoader --- src/Umbraco.Core/Composing/TypeLoader.cs | 46 +++++++----------------- src/Umbraco.Core/Runtime/CoreRuntime.cs | 6 ++-- 2 files changed, 15 insertions(+), 37 deletions(-) diff --git a/src/Umbraco.Core/Composing/TypeLoader.cs b/src/Umbraco.Core/Composing/TypeLoader.cs index f79c288e91..78b9943976 100644 --- a/src/Umbraco.Core/Composing/TypeLoader.cs +++ b/src/Umbraco.Core/Composing/TypeLoader.cs @@ -42,6 +42,17 @@ namespace Umbraco.Core.Composing private static LocalTempStorage _localTempStorage = LocalTempStorage.Unknown; private static string _fileBasePath; + /// + /// Initializes a new instance of the class. + /// + /// The application runtime cache. + /// + /// A profiling logger. + /// Used by LightInject. + public TypeLoader(IRuntimeCacheProvider runtimeCache, IGlobalSettings globalSettings, ProfilingLogger logger) + : this(runtimeCache, globalSettings, logger, true) + { } + /// /// Initializes a new instance of the class. /// @@ -49,7 +60,7 @@ namespace Umbraco.Core.Composing /// /// A profiling logger. /// Whether to detect changes using hashes. - internal TypeLoader(IRuntimeCacheProvider runtimeCache, IGlobalSettings globalSettings, ProfilingLogger logger, bool detectChanges = true) + internal TypeLoader(IRuntimeCacheProvider runtimeCache, IGlobalSettings globalSettings, ProfilingLogger logger, bool detectChanges) { _runtimeCache = runtimeCache ?? throw new ArgumentNullException(nameof(runtimeCache)); _globalSettings = globalSettings ?? throw new ArgumentNullException(nameof(globalSettings)); @@ -401,39 +412,6 @@ namespace Umbraco.Core.Composing return _fileBasePath; } - //private string GetFilePath(string extension) - //{ - // string path; - // switch (_globalSettings.LocalTempStorageLocation) - // { - // case LocalTempStorage.AspNetTemp: - // path = Path.Combine(HttpRuntime.CodegenDir, "UmbracoData", "umbraco-types." + extension); - // break; - // case LocalTempStorage.EnvironmentTemp: - // // include the appdomain hash is just a safety check, for example if a website is moved from worker A to worker B and then back - // // to worker A again, in theory the %temp% folder should already be empty but we really want to make sure that its not - // // utilizing an old path - assuming we cannot have SHA1 collisions on AppDomainAppId - // var appDomainHash = HttpRuntime.AppDomainAppId.ToSHA1(); - // var cachePath = Path.Combine(Environment.ExpandEnvironmentVariables("%temp%"), "UmbracoData", appDomainHash); - // path = Path.Combine(cachePath, "umbraco-types." + extension); - // break; - // case LocalTempStorage.Default: - // default: - // var tempFolder = IOHelper.MapPath("~/App_Data/TEMP/TypesCache"); - // path = Path.Combine(tempFolder, "umbraco-types." + NetworkHelper.FileSafeMachineName + "." + extension); - // break; - // } - - // // ensure that the folder exists - // var directory = Path.GetDirectoryName(path); - // if (directory == null) - // throw new InvalidOperationException($"Could not determine folder for file \"{path}\"."); - // if (Directory.Exists(directory) == false) - // Directory.CreateDirectory(directory); - - // return path; - //} - // internal for tests internal void WriteCache() { diff --git a/src/Umbraco.Core/Runtime/CoreRuntime.cs b/src/Umbraco.Core/Runtime/CoreRuntime.cs index cf2712974d..cc35543b66 100755 --- a/src/Umbraco.Core/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Core/Runtime/CoreRuntime.cs @@ -237,10 +237,10 @@ namespace Umbraco.Core.Runtime // and only these things - the rest should be composed in runtime components // register basic things + // logging, runtime state, configuration container.RegisterSingleton(); container.RegisterSingleton(); container.RegisterSingleton(); - container.RegisterFrom(); // register caches @@ -254,8 +254,8 @@ namespace Umbraco.Core.Runtime new IsolatedRuntimeCache(type => new DeepCloneRuntimeCacheProvider(new ObjectCacheRuntimeCacheProvider())))); container.RegisterSingleton(f => f.GetInstance().RuntimeCache); - // register the plugin manager - container.RegisterSingleton(f => new TypeLoader(f.GetInstance(), f.GetInstance(), f.GetInstance())); + // register the type loader + container.RegisterSingleton(); // register syntax providers - required by database factory container.Register("MySqlSyntaxProvider"); From e65470de8bec932be85fff9d19b62e32c0d24140 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 17 Oct 2018 09:59:55 +0200 Subject: [PATCH 02/12] Document SqlContext --- src/Umbraco.Core/Persistence/SqlContext.cs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/SqlContext.cs b/src/Umbraco.Core/Persistence/SqlContext.cs index feb92e0849..d11bce166f 100644 --- a/src/Umbraco.Core/Persistence/SqlContext.cs +++ b/src/Umbraco.Core/Persistence/SqlContext.cs @@ -30,10 +30,24 @@ namespace Umbraco.Core.Persistence Templates = new SqlTemplates(this); } - // fixme + /// + /// Initializes a new instance of the class. + /// + /// Initializes an empty context which must be fully initialized using the + /// method; this is done in + /// as soon as the factory is fully configured. internal SqlContext() { } + /// + /// Initializes this . + /// + /// The sql syntax provider. + /// The Poco data factory. + /// The database type. + /// The mappers. + /// Fully initializes an initially empty context; this is done in + /// as soon as the factory is fully configured. internal void Initialize(ISqlSyntaxProvider sqlSyntax, DatabaseType databaseType, IPocoDataFactory pocoDataFactory, IMapperCollection mappers = null) { // for tests From 11edbc86383c11776d93eb9f435f801f378d7207 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 17 Oct 2018 10:52:09 +0200 Subject: [PATCH 03/12] Remove obsolete post-migrations --- ...ediaXmlCacheForDeletedItemsAfterUpgrade.cs | 50 ------------- .../EnsureListViewDataTypeIsCreated.cs | 71 ------------------- .../OverwriteStylesheetFilesFromTempFiles.cs | 53 -------------- .../RebuildXmlCachesAfterUpgrade.cs | 57 --------------- src/Umbraco.Web/Umbraco.Web.csproj | 4 -- 5 files changed, 235 deletions(-) delete mode 100644 src/Umbraco.Web/Migrations/ClearMediaXmlCacheForDeletedItemsAfterUpgrade.cs delete mode 100644 src/Umbraco.Web/Migrations/EnsureListViewDataTypeIsCreated.cs delete mode 100644 src/Umbraco.Web/Migrations/OverwriteStylesheetFilesFromTempFiles.cs delete mode 100644 src/Umbraco.Web/Migrations/RebuildXmlCachesAfterUpgrade.cs diff --git a/src/Umbraco.Web/Migrations/ClearMediaXmlCacheForDeletedItemsAfterUpgrade.cs b/src/Umbraco.Web/Migrations/ClearMediaXmlCacheForDeletedItemsAfterUpgrade.cs deleted file mode 100644 index 991e77b4d0..0000000000 --- a/src/Umbraco.Web/Migrations/ClearMediaXmlCacheForDeletedItemsAfterUpgrade.cs +++ /dev/null @@ -1,50 +0,0 @@ -using Semver; -using Umbraco.Core; -using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; -using Umbraco.Core.Scoping; - -namespace Umbraco.Web.Migrations -{ - /// - /// This will execute after upgrading to remove any xml cache for media that are currently in the bin - /// - /// - /// This will execute for specific versions - - /// - /// * If current is less than or equal to 7.0.0 - /// - public class ClearMediaXmlCacheForDeletedItemsAfterUpgrade : IPostMigration - { - private readonly ILogger _logger; - - public ClearMediaXmlCacheForDeletedItemsAfterUpgrade(ILogger logger) - { - _logger = logger; - } - - public void Execute(string name, IScope scope, SemVersion originVersion, SemVersion targetVersion, ILogger logger) - { - if (name != Constants.System.UmbracoUpgradePlanName) return; - - var target70 = new SemVersion(7 /*, 0, 0*/); - - if (originVersion <= target70) - { - //This query is structured to work with MySql, SQLCE and SqlServer: - // http://issues.umbraco.org/issue/U4-3876 - - var syntax = scope.SqlContext.SqlSyntax; - - var sql = @"DELETE FROM cmsContentXml WHERE nodeId IN - (SELECT nodeId FROM (SELECT DISTINCT cmsContentXml.nodeId FROM cmsContentXml - INNER JOIN umbracoNode ON cmsContentXml.nodeId = umbracoNode.id - WHERE nodeObjectType = '" + Constants.ObjectTypes.Media + "' AND " + syntax.GetQuotedColumnName("path") + " LIKE '%-21%') x)"; - - var count = scope.Database.Execute(sql); - - _logger.Info("Cleared {Total} items from the media xml cache that were trashed and not meant to be there", count); - } - } - } -} diff --git a/src/Umbraco.Web/Migrations/EnsureListViewDataTypeIsCreated.cs b/src/Umbraco.Web/Migrations/EnsureListViewDataTypeIsCreated.cs deleted file mode 100644 index 2dbe6bd867..0000000000 --- a/src/Umbraco.Web/Migrations/EnsureListViewDataTypeIsCreated.cs +++ /dev/null @@ -1,71 +0,0 @@ -using System; -using NPoco; -using Semver; -using Umbraco.Core; -using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; -using Umbraco.Core.Persistence.Dtos; -using Umbraco.Core.Scoping; - -namespace Umbraco.Web.Migrations -{ - /// - /// Creates the built in list view data types - /// - public class EnsureDefaultListViewDataTypesCreated : IPostMigration - { - public void Execute(string name, IScope scope, SemVersion originVersion, SemVersion targetVersion, ILogger logger) - { - if (name != Constants.System.UmbracoUpgradePlanName) return; - - var target720 = new SemVersion(7, 2, 0); - - if (originVersion > target720) - return; - - var syntax = scope.SqlContext.SqlSyntax; - - try - { - //Turn on identity insert if db provider is not mysql - if (syntax.SupportsIdentityInsert()) - scope.Database.Execute(new Sql(string.Format("SET IDENTITY_INSERT {0} ON ", syntax.GetQuotedTableName("umbracoNode")))); - - if (scope.Database.Exists(Constants.DataTypes.DefaultContentListView)) - { - //If this already exists then just exit - return; - } - - scope.Database.Insert("umbracoNode", "id", false, new NodeDto { NodeId = Constants.DataTypes.DefaultContentListView, Trashed = false, ParentId = -1, UserId = -1, Level = 1, Path = "-1,-95", SortOrder = 2, UniqueId = new Guid("C0808DD3-8133-4E4B-8CE8-E2BEA84A96A4"), Text = Constants.Conventions.DataTypes.ListViewPrefix + "Content", NodeObjectType = Constants.ObjectTypes.DataType, CreateDate = DateTime.Now }); - scope.Database.Insert("umbracoNode", "id", false, new NodeDto { NodeId = Constants.DataTypes.DefaultMediaListView, Trashed = false, ParentId = -1, UserId = -1, Level = 1, Path = "-1,-96", SortOrder = 2, UniqueId = new Guid("3A0156C4-3B8C-4803-BDC1-6871FAA83FFF"), Text = Constants.Conventions.DataTypes.ListViewPrefix + "Media", NodeObjectType = Constants.ObjectTypes.DataType, CreateDate = DateTime.Now }); - scope.Database.Insert("umbracoNode", "id", false, new NodeDto { NodeId = Constants.DataTypes.DefaultMembersListView, Trashed = false, ParentId = -1, UserId = -1, Level = 1, Path = "-1,-97", SortOrder = 2, UniqueId = new Guid("AA2C52A0-CE87-4E65-A47C-7DF09358585D"), Text = Constants.Conventions.DataTypes.ListViewPrefix + "Members", NodeObjectType = Constants.ObjectTypes.DataType, CreateDate = DateTime.Now }); - } - finally - { - //Turn off identity insert if db provider is not mysql - if (syntax.SupportsIdentityInsert()) - scope.Database.Execute(new Sql(string.Format("SET IDENTITY_INSERT {0} OFF;", syntax.GetQuotedTableName("umbracoNode")))); - } - - try - { - //Turn on identity insert if db provider is not mysql - if (syntax.SupportsIdentityInsert()) - scope.Database.Execute(new Sql(string.Format("SET IDENTITY_INSERT {0} ON ", syntax.GetQuotedTableName(Constants.DatabaseSchema.Tables.DataType)))); - - const string memberListViewConfiguration = "{\"pageSize\":10,\"orderBy\":Name,\"orderDirection\":asc,includeProperties:[{\"alias\":\"email\",\"isSystem\":1},{\"alias\":\"username\",\"isSystem\":1},{\"alias\":\"updateDate\",\"header\":\"Last edited\",\"isSystem\":1}]}"; - - scope.Database.Insert(Constants.DatabaseSchema.Tables.DataType, "pk", false, new DataTypeDto { NodeId = Constants.DataTypes.DefaultContentListView, EditorAlias = Constants.PropertyEditors.Aliases.ListView, DbType = "Nvarchar" }); - scope.Database.Insert(Constants.DatabaseSchema.Tables.DataType, "pk", false, new DataTypeDto { NodeId = Constants.DataTypes.DefaultMediaListView, EditorAlias = Constants.PropertyEditors.Aliases.ListView, DbType = "Nvarchar" }); - scope.Database.Insert(Constants.DatabaseSchema.Tables.DataType, "pk", false, new DataTypeDto { NodeId = Constants.DataTypes.DefaultMembersListView, EditorAlias = Constants.PropertyEditors.Aliases.ListView, DbType = "Nvarchar", Configuration = memberListViewConfiguration }); - } - finally - { - //Turn off identity insert if db provider is not mysql - if (syntax.SupportsIdentityInsert()) - scope.Database.Execute(new Sql(string.Format("SET IDENTITY_INSERT {0} OFF;", syntax.GetQuotedTableName(Constants.DatabaseSchema.Tables.DataType)))); - } - } - } -} diff --git a/src/Umbraco.Web/Migrations/OverwriteStylesheetFilesFromTempFiles.cs b/src/Umbraco.Web/Migrations/OverwriteStylesheetFilesFromTempFiles.cs deleted file mode 100644 index 7a752fa562..0000000000 --- a/src/Umbraco.Web/Migrations/OverwriteStylesheetFilesFromTempFiles.cs +++ /dev/null @@ -1,53 +0,0 @@ -using System.IO; -using Semver; -using Umbraco.Core; -using Umbraco.Core.IO; -using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; -using Umbraco.Core.Migrations.Upgrade; -using Umbraco.Core.Scoping; - -namespace Umbraco.Web.Migrations -{ - - /// - /// When upgrading version 7.3 the migration MigrateStylesheetDataToFile will execute but we don't want to overwrite the developers - /// files during the migration since other parts of the migration might fail. So once the migration is complete, we'll then copy over the temp - /// files that this migration created over top of the developer's files. We'll also create a backup of their files. - /// - public sealed class OverwriteStylesheetFilesFromTempFiles : IPostMigration - { - public void Execute(string name, IScope scope, SemVersion originVersion, SemVersion targetVersion, ILogger logger) - { - if (name != Constants.System.UmbracoUpgradePlanName) return; - - var target73 = new SemVersion(7, 3, 0); - - if (originVersion <= target73) - { - var tempCssFolder = IOHelper.MapPath("~/App_Data/TEMP/CssMigration/"); - var cssFolder = IOHelper.MapPath("~/css"); - if (Directory.Exists(tempCssFolder)) - { - var files = Directory.GetFiles(tempCssFolder, "*.css", SearchOption.AllDirectories); - foreach (var file in files) - { - var relativePath = file.TrimStart(tempCssFolder).TrimStart("\\"); - var cssFilePath = Path.Combine(cssFolder, relativePath); - if (File.Exists(cssFilePath)) - { - //backup - var targetPath = Path.Combine(tempCssFolder, relativePath.EnsureEndsWith(".bak")); - logger.Info("CSS file is being backed up from {CssFilePath}, to {TargetPath} before being migrated to new format", cssFilePath, targetPath); - File.Copy(cssFilePath, targetPath, true); - } - - //ensure the sub folder eixts - Directory.CreateDirectory(Path.GetDirectoryName(cssFilePath)); - File.Copy(file, cssFilePath, true); - } - } - } - } - } -} diff --git a/src/Umbraco.Web/Migrations/RebuildXmlCachesAfterUpgrade.cs b/src/Umbraco.Web/Migrations/RebuildXmlCachesAfterUpgrade.cs deleted file mode 100644 index 2181eea367..0000000000 --- a/src/Umbraco.Web/Migrations/RebuildXmlCachesAfterUpgrade.cs +++ /dev/null @@ -1,57 +0,0 @@ -using System; -using Semver; -using Umbraco.Core; -using Umbraco.Core.Logging; -using Umbraco.Core.Migrations; -using Umbraco.Core.Scoping; -using Umbraco.Web.Composing; -using Umbraco.Web.PublishedCache.XmlPublishedCache; - -namespace Umbraco.Web.Migrations -{ - /// - /// Rebuilds the Xml caches after upgrading. - /// This will execute after upgrading to rebuild the xml cache - /// - /// - /// This cannot execute as part of a DB migration since it needs access to services and repositories. - /// Executes for: - /// - Media Xml : if current is less than, or equal to, 7.0.0 (superceeded by the next rule) - /// - Media & Content Xml : if current is less than, or equal to, 7.3.0 - because 7.3.0 adds .Key to cached items - /// - /// - public class RebuildXmlCachesAfterUpgrade : IPostMigration - { - public void Execute(string name, IScope scope, SemVersion originVersion, SemVersion targetVersion, ILogger logger) - { - if (name != Constants.System.UmbracoUpgradePlanName) return; - - var v730 = new SemVersion(new Version(7, 3, 0)); - - var doMedia = originVersion < v730; - var doContent = originVersion < v730; - - if (doMedia) - { - // fixme - maintain - for backward compatibility?! or replace with...?! - //var mediaService = (MediaService) ApplicationContext.Current.Services.MediaService; - //mediaService.RebuildXmlStructures(); - - var svc = Current.PublishedSnapshotService as PublishedSnapshotService; - svc?.RebuildMediaXml(); - - // note: not re-indexing medias? - } - - if (doContent) - { - // fixme - maintain - for backward compatibility?! or replace with...?! - //var contentService = (ContentService) ApplicationContext.Current.Services.ContentService; - //contentService.RebuildXmlStructures(); - - var svc = Current.PublishedSnapshotService as PublishedSnapshotService; - svc?.RebuildContentAndPreviewXml(); - } - } - } -} diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 4034dd2be6..699c985af3 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -835,9 +835,6 @@ - - - @@ -1005,7 +1002,6 @@ Resources.resx - From 7b3cded11544eb6c4401c2003a9e07dc3bfdf769 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 4 Jul 2018 14:44:59 +0200 Subject: [PATCH 04/12] Cleanup --- src/Umbraco.Core/Logging/Serilog/SerilogLogger.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Umbraco.Core/Logging/Serilog/SerilogLogger.cs b/src/Umbraco.Core/Logging/Serilog/SerilogLogger.cs index 6d3b2b28b3..c23a43a820 100644 --- a/src/Umbraco.Core/Logging/Serilog/SerilogLogger.cs +++ b/src/Umbraco.Core/Logging/Serilog/SerilogLogger.cs @@ -202,7 +202,6 @@ namespace Umbraco.Core.Logging.Serilog private static bool IsTimeoutThreadAbortException(Exception exception) { if (!(exception is ThreadAbortException abort)) return false; - if (abort.ExceptionState == null) return false; var stateType = abort.ExceptionState.GetType(); From 68ee1181cb0a983d60b0fa6049c4adeb3aad1a4b Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 4 Jul 2018 14:48:44 +0200 Subject: [PATCH 05/12] Execute true Sql in Migrations --- .../Expressions/Execute/ExecuteBuilder.cs | 19 +++++++++- .../ExecuteSqlStatementExpression.cs | 8 ++++ .../Expressions/Execute/IExecuteBuilder.cs | 9 ++++- .../Migrations/MigrationExpressionBase.cs | 26 ++++++++++++- .../Persistence/NPocoSqlExtensions.cs | 38 +++++++++++++++++-- .../Persistence/UmbracoDatabase.cs | 21 +++------- 6 files changed, 98 insertions(+), 23 deletions(-) diff --git a/src/Umbraco.Core/Migrations/Expressions/Execute/ExecuteBuilder.cs b/src/Umbraco.Core/Migrations/Expressions/Execute/ExecuteBuilder.cs index 0637d2e597..0ba2499c44 100644 --- a/src/Umbraco.Core/Migrations/Expressions/Execute/ExecuteBuilder.cs +++ b/src/Umbraco.Core/Migrations/Expressions/Execute/ExecuteBuilder.cs @@ -1,6 +1,7 @@ using NPoco; using Umbraco.Core.Migrations.Expressions.Common; using Umbraco.Core.Migrations.Expressions.Execute.Expressions; +using Umbraco.Core.Persistence; namespace Umbraco.Core.Migrations.Expressions.Execute { @@ -12,7 +13,16 @@ namespace Umbraco.Core.Migrations.Expressions.Execute { } /// - public void Do() => Expression.Execute(); + public void Do() + { + // slightly awkward, but doing it right would mean a *lot* + // of changes for MigrationExpressionBase + + if (Expression.SqlObject == null) + Expression.Execute(); + else + Expression.ExecuteSqlObject(); + } /// public IExecutableBuilder Sql(string sqlStatement) @@ -20,5 +30,12 @@ namespace Umbraco.Core.Migrations.Expressions.Execute Expression.SqlStatement = sqlStatement; return this; } + + /// + public IExecutableBuilder Sql(Sql sql) + { + Expression.SqlObject = sql; + return this; + } } } diff --git a/src/Umbraco.Core/Migrations/Expressions/Execute/Expressions/ExecuteSqlStatementExpression.cs b/src/Umbraco.Core/Migrations/Expressions/Execute/Expressions/ExecuteSqlStatementExpression.cs index b5c1fbdf6b..8b5da4f270 100644 --- a/src/Umbraco.Core/Migrations/Expressions/Execute/Expressions/ExecuteSqlStatementExpression.cs +++ b/src/Umbraco.Core/Migrations/Expressions/Execute/Expressions/ExecuteSqlStatementExpression.cs @@ -1,4 +1,5 @@ using NPoco; +using Umbraco.Core.Persistence; namespace Umbraco.Core.Migrations.Expressions.Execute.Expressions { @@ -10,6 +11,13 @@ namespace Umbraco.Core.Migrations.Expressions.Execute.Expressions public virtual string SqlStatement { get; set; } + public virtual Sql SqlObject { get; set; } + + public void ExecuteSqlObject() + { + Execute(SqlObject); + } + protected override string GetSql() { return SqlStatement; diff --git a/src/Umbraco.Core/Migrations/Expressions/Execute/IExecuteBuilder.cs b/src/Umbraco.Core/Migrations/Expressions/Execute/IExecuteBuilder.cs index 5747eb2c1a..7f575fd3f8 100644 --- a/src/Umbraco.Core/Migrations/Expressions/Execute/IExecuteBuilder.cs +++ b/src/Umbraco.Core/Migrations/Expressions/Execute/IExecuteBuilder.cs @@ -1,4 +1,6 @@ -using Umbraco.Core.Migrations.Expressions.Common; +using NPoco; +using Umbraco.Core.Migrations.Expressions.Common; +using Umbraco.Core.Persistence; namespace Umbraco.Core.Migrations.Expressions.Execute { @@ -12,5 +14,10 @@ namespace Umbraco.Core.Migrations.Expressions.Execute /// Specifies the Sql statement to execute. /// IExecutableBuilder Sql(string sqlStatement); + + /// + /// Specifies the Sql statement to execute. + /// + IExecutableBuilder Sql(Sql sql); } } diff --git a/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs b/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs index 6ac92a07aa..8b5d9cc78c 100644 --- a/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs +++ b/src/Umbraco.Core/Migrations/MigrationExpressionBase.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.IO; -using System.Linq; using System.Text; using NPoco; using Umbraco.Core.Logging; @@ -88,6 +87,31 @@ namespace Umbraco.Core.Migrations expression.Execute(); } + protected void Execute(Sql sql) + { + if (_executed) + throw new InvalidOperationException("This expression has already been executed."); + _executed = true; + + if (sql == null) + { + Logger.Info(GetType(), $"SQL [{Context.Index}]: "); + } + else + { + Logger.Info(GetType(), $"SQL [{Context.Index}]: {sql.ToText()}"); + Database.Execute(sql); + } + + Context.Index++; + + if (_expressions == null) + return; + + foreach (var expression in _expressions) + expression.Execute(); + } + private void ExecuteStatement(StringBuilder stmtBuilder) { var stmt = stmtBuilder.ToString(); diff --git a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs index a5ab62d25f..6f814a7174 100644 --- a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs +++ b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs @@ -76,7 +76,7 @@ namespace Umbraco.Core.Persistence var (s, a) = sql.SqlContext.VisitDto(predicate, alias); return sql.Where(s, a); } - + /// /// Appends a WHERE clause to the Sql statement. /// @@ -1117,10 +1117,40 @@ namespace Umbraco.Core.Persistence internal static void WriteToConsole(this Sql sql) { - Console.WriteLine(sql.SQL); + Console.Write(sql.ToText()); + } + + internal static string ToText(this Sql sql) + { + var text = new StringBuilder(); + sql.ToText(text); + return text.ToString(); + } + + internal static void ToText(this Sql sql, StringBuilder text) + { + ToText(sql.SQL, sql.Arguments, text); + } + + internal static void ToText(string sql, object[] arguments, StringBuilder text) + { + text.AppendLine(sql); + + if (arguments.Length == 0) + return; + + text.Append(" --"); + var i = 0; - foreach (var arg in sql.Arguments) - Console.WriteLine($" @{i++}: {arg}"); + foreach (var arg in arguments) + { + text.Append(" @"); + text.Append(i++); + text.Append(":"); + text.Append(arg); + } + + text.AppendLine(); } #endregion diff --git a/src/Umbraco.Core/Persistence/UmbracoDatabase.cs b/src/Umbraco.Core/Persistence/UmbracoDatabase.cs index 64e4c0adca..9ec043d684 100644 --- a/src/Umbraco.Core/Persistence/UmbracoDatabase.cs +++ b/src/Umbraco.Core/Persistence/UmbracoDatabase.cs @@ -228,24 +228,13 @@ namespace Umbraco.Core.Persistence private string CommandToString(string sql, object[] args) { - var sb = new StringBuilder(); + var text = new StringBuilder(); #if DEBUG_DATABASES - sb.Append(InstanceId); - sb.Append(": "); + text.Append(InstanceId); + text.Append(": "); #endif - sb.Append(sql); - if (args.Length > 0) - sb.Append(" --"); - var i = 0; - foreach (var arg in args) - { - sb.Append(" @"); - sb.Append(i++); - sb.Append(":"); - sb.Append(arg); - } - - return sb.ToString(); + NPocoSqlExtensions.ToText(sql, args, text); + return text.ToString(); } protected override void OnExecutedCommand(DbCommand cmd) From 478522a573964ccd737eb57c26951340ffb7416c Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 4 Dec 2018 17:01:33 +0100 Subject: [PATCH 06/12] Migrations TLC --- src/Umbraco.Core/Migrations/MigrationPlan.cs | 67 +++++----- .../Migrations/Upgrade/UmbracoPlan.cs | 116 ++++++++++-------- .../Migrations/Upgrade/UmbracoUpgrader.cs | 23 +++- .../Migrations/Upgrade/Upgrader.cs | 63 ++++++++-- .../Migrations/AdvancedMigrationTests.cs | 37 +++--- .../Migrations/MigrationPlanTests.cs | 36 +++--- .../Migrations/MigrationTests.cs | 31 +++-- .../Migrations/PostMigrationTests.cs | 15 +-- 8 files changed, 233 insertions(+), 155 deletions(-) diff --git a/src/Umbraco.Core/Migrations/MigrationPlan.cs b/src/Umbraco.Core/Migrations/MigrationPlan.cs index 5c999ad6ef..1324abc134 100644 --- a/src/Umbraco.Core/Migrations/MigrationPlan.cs +++ b/src/Umbraco.Core/Migrations/MigrationPlan.cs @@ -64,23 +64,8 @@ namespace Umbraco.Core.Migrations /// public string Name { get; } - /// - /// Adds an empty migration from source to target state. - /// - public MigrationPlan Add(string sourceState, string targetState) - => Add(sourceState, targetState); - - /// - /// Adds a migration from source to target state. - /// - public MigrationPlan Add(string sourceState, string targetState) - where TMigration : IMigration - => Add(sourceState, targetState, typeof(TMigration)); - - /// - /// Adds a migration from source to target state. - /// - public MigrationPlan Add(string sourceState, string targetState, Type migration) + // adds a transition + private MigrationPlan Add(string sourceState, string targetState, Type migration) { if (sourceState == null) throw new ArgumentNullException(nameof(sourceState)); if (string.IsNullOrWhiteSpace(targetState)) throw new ArgumentNullOrEmptyException(nameof(targetState)); @@ -113,26 +98,26 @@ namespace Umbraco.Core.Migrations } /// - /// Chains an empty migration from chain to target state. - /// - public MigrationPlan Chain(string targetState) - => Chain(targetState); + /// Adds a transition to a target state through an empty migration. + /// + public MigrationPlan To(string targetState) + => To(targetState); /// - /// Chains a migration from chain to target state. + /// Adds a transition to a target state through a migration. /// - public MigrationPlan Chain(string targetState) + public MigrationPlan To(string targetState) where TMigration : IMigration - => Chain(targetState, typeof(TMigration)); + => To(targetState, typeof(TMigration)); /// - /// Chains a migration from chain to target state. + /// Adds a transition to a target state through a migration. /// - public MigrationPlan Chain(string targetState, Type migration) + public MigrationPlan To(string targetState, Type migration) => Add(_prevState, targetState, migration); /// - /// Sets the chain state. + /// Sets the starting state. /// public MigrationPlan From(string sourceState) { @@ -141,19 +126,16 @@ namespace Umbraco.Core.Migrations } /// - /// Copies a chain. + /// Adds transitions to a target state by copying transitions from a start state to an end state. /// - /// Copies the chain going from startState to endState, with new states going from sourceState to targetState. - public MigrationPlan CopyChain(string sourceState, string startState, string endState, string targetState) + public MigrationPlan To(string targetState, string startState, string endState) { - if (sourceState == null) throw new ArgumentNullException(nameof(sourceState)); if (string.IsNullOrWhiteSpace(startState)) throw new ArgumentNullOrEmptyException(nameof(startState)); if (string.IsNullOrWhiteSpace(endState)) throw new ArgumentNullOrEmptyException(nameof(endState)); if (string.IsNullOrWhiteSpace(targetState)) throw new ArgumentNullOrEmptyException(nameof(targetState)); - if (sourceState == targetState) throw new ArgumentException("Source and target states cannot be identical."); + if (startState == endState) throw new ArgumentException("Start and end states cannot be identical."); - sourceState = sourceState.Trim(); startState = startState.Trim(); endState = endState.Trim(); targetState = targetState.Trim(); @@ -168,25 +150,36 @@ namespace Umbraco.Core.Migrations visited.Add(state); if (!_transitions.TryGetValue(state, out var transition)) - throw new InvalidOperationException($"There is no transition from state \"{sourceState}\"."); + throw new InvalidOperationException($"There is no transition from state \"{state}\"."); var newTargetState = transition.TargetState == endState ? targetState : Guid.NewGuid().ToString("B").ToUpper(); - Add(sourceState, newTargetState, transition.MigrationType); - sourceState = newTargetState; + To(newTargetState, transition.MigrationType); state = transition.TargetState; } return this; } + /// + /// Copies a chain. + /// + /// Copies the chain going from startState to endState, with new states going from sourceState to targetState. + [Obsolete("die", true)] + public MigrationPlan CopyChain(string sourceState, string startState, string endState, string targetState) + { + From(sourceState).To(targetState, startState, endState); + return this; + } + /// /// Copies a chain. /// /// Copies the chain going from startState to endState, with new states going from chain to targetState. + [Obsolete("die", true)] public MigrationPlan CopyChain(string startState, string endState, string targetState) - => CopyChain(_prevState, startState, endState, targetState); + => To(targetState, startState, endState); /// /// Gets the initial state. diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index ec49544976..e907ce09ab 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -85,65 +85,73 @@ namespace Umbraco.Core.Migrations.Upgrade // upgrades from 7 to 8, and then takes care of all eventual upgrades // From("{init-7.10.0}"); - Chain("{7C447271-CA3F-4A6A-A913-5D77015655CB}"); - Chain("{CBFF58A2-7B50-4F75-8E98-249920DB0F37}"); - Chain("{3D18920C-E84D-405C-A06A-B7CEE52FE5DD}"); - Chain("{FB0A5429-587E-4BD0-8A67-20F0E7E62FF7}"); - Chain("{F0C42457-6A3B-4912-A7EA-F27ED85A2092}"); - Chain("{8640C9E4-A1C0-4C59-99BB-609B4E604981}"); - Chain("{DD1B99AF-8106-4E00-BAC7-A43003EA07F8}"); - Chain("{9DF05B77-11D1-475C-A00A-B656AF7E0908}"); - Chain("{6FE3EF34-44A0-4992-B379-B40BC4EF1C4D}"); - Chain("{7F59355A-0EC9-4438-8157-EB517E6D2727}"); + To("{7C447271-CA3F-4A6A-A913-5D77015655CB}"); + To("{CBFF58A2-7B50-4F75-8E98-249920DB0F37}"); + To("{3D18920C-E84D-405C-A06A-B7CEE52FE5DD}"); + To("{FB0A5429-587E-4BD0-8A67-20F0E7E62FF7}"); + To("{F0C42457-6A3B-4912-A7EA-F27ED85A2092}"); + To("{8640C9E4-A1C0-4C59-99BB-609B4E604981}"); + To("{DD1B99AF-8106-4E00-BAC7-A43003EA07F8}"); + To("{9DF05B77-11D1-475C-A00A-B656AF7E0908}"); + To("{6FE3EF34-44A0-4992-B379-B40BC4EF1C4D}"); + To("{7F59355A-0EC9-4438-8157-EB517E6D2727}"); + //To("{941B2ABA-2D06-4E04-81F5-74224F1DB037}"); // AddVariationTables1 has been superseded by AddVariationTables2 + To("{76DF5CD7-A884-41A5-8DC6-7860D95B1DF5}"); - // AddVariationTables1 has been superceeded by AddVariationTables2 - //Chain("{941B2ABA-2D06-4E04-81F5-74224F1DB037}"); - Chain("{76DF5CD7-A884-41A5-8DC6-7860D95B1DF5}"); - // however, provide a path out of the old state - Add("{941B2ABA-2D06-4E04-81F5-74224F1DB037}", "{76DF5CD7-A884-41A5-8DC6-7860D95B1DF5}"); + // way out of the commented state + From("{941B2ABA-2D06-4E04-81F5-74224F1DB037}"); + To("{76DF5CD7-A884-41A5-8DC6-7860D95B1DF5}"); // resume at {76DF5CD7-A884-41A5-8DC6-7860D95B1DF5} ... - Chain("{A7540C58-171D-462A-91C5-7A9AA5CB8BFD}"); + To("{A7540C58-171D-462A-91C5-7A9AA5CB8BFD}"); + To("{3E44F712-E2E3-473A-AE49-5D7F8E67CE3F}"); // shannon added that one - let's keep it as the default path + //To("{65D6B71C-BDD5-4A2E-8D35-8896325E9151}"); // stephan added that one = merge conflict, remove + To("{4CACE351-C6B9-4F0C-A6BA-85A02BBD39E4}"); // but add it after shannon's, with a new target state - Chain("{3E44F712-E2E3-473A-AE49-5D7F8E67CE3F}"); // shannon added that one - let's keep it as the default path - //Chain("{65D6B71C-BDD5-4A2E-8D35-8896325E9151}"); // stephan added that one = merge conflict, remove, - Chain("{4CACE351-C6B9-4F0C-A6BA-85A02BBD39E4}"); // but add it after shannon's, with a new target state, - Add("{65D6B71C-BDD5-4A2E-8D35-8896325E9151}", "{4CACE351-C6B9-4F0C-A6BA-85A02BBD39E4}"); // and provide a path out of the conflict state + // way out of the commented state + From("{65D6B71C-BDD5-4A2E-8D35-8896325E9151}"); + To("{4CACE351-C6B9-4F0C-A6BA-85A02BBD39E4}"); // resume at {4CACE351-C6B9-4F0C-A6BA-85A02BBD39E4} ... - Chain("{1350617A-4930-4D61-852F-E3AA9E692173}"); - Chain("{39E5B1F7-A50B-437E-B768-1723AEC45B65}"); // from 7.12.0 - //Chain("{CF51B39B-9B9A-4740-BB7C-EAF606A7BFBF}"); // andy added that one = merge conflict, remove - Chain("{0541A62B-EF87-4CA2-8225-F0EB98ECCC9F}"); // from 7.12.0 - Chain("{EB34B5DC-BB87-4005-985E-D983EA496C38}"); // from 7.12.0 - Chain("{517CE9EA-36D7-472A-BF4B-A0D6FB1B8F89}"); // from 7.12.0 - Chain("{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}"); // from 7.12.0 - //Chain("{2C87AA47-D1BC-4ECB-8A73-2D8D1046C27F}"); // stephan added that one = merge conflict, remove + To("{1350617A-4930-4D61-852F-E3AA9E692173}"); + To("{39E5B1F7-A50B-437E-B768-1723AEC45B65}"); // from 7.12.0 + //To("{CF51B39B-9B9A-4740-BB7C-EAF606A7BFBF}"); // andy added that one = merge conflict, remove + To("{0541A62B-EF87-4CA2-8225-F0EB98ECCC9F}"); // from 7.12.0 + To("{EB34B5DC-BB87-4005-985E-D983EA496C38}"); // from 7.12.0 + To("{517CE9EA-36D7-472A-BF4B-A0D6FB1B8F89}"); // from 7.12.0 + To("{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}"); // from 7.12.0 + //To("{2C87AA47-D1BC-4ECB-8A73-2D8D1046C27F}"); // stephan added that one = merge conflict, remove - Chain("{8B14CEBD-EE47-4AAD-A841-93551D917F11}"); // add andy's after others, with a new target state - From("{CF51B39B-9B9A-4740-BB7C-EAF606A7BFBF}") // and provide a path out of andy's - .CopyChain("{39E5B1F7-A50B-437E-B768-1723AEC45B65}", "{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}", "{8B14CEBD-EE47-4AAD-A841-93551D917F11}"); // to next + To("{8B14CEBD-EE47-4AAD-A841-93551D917F11}"); // add andy's after others, with a new target state + + // way out of andy's + From("{CF51B39B-9B9A-4740-BB7C-EAF606A7BFBF}"); + To("{8B14CEBD-EE47-4AAD-A841-93551D917F11}", "{39E5B1F7-A50B-437E-B768-1723AEC45B65}", "{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}"); // resume at {8B14CEBD-EE47-4AAD-A841-93551D917F11} ... - Chain("{5F4597F4-A4E0-4AFE-90B5-6D2F896830EB}"); // add stephan's after others, with a new target state - From("{2C87AA47-D1BC-4ECB-8A73-2D8D1046C27F}") // and provide a path out of stephan's - .Chain("{5F4597F4-A4E0-4AFE-90B5-6D2F896830EB}"); // to next + To("{5F4597F4-A4E0-4AFE-90B5-6D2F896830EB}"); // add stephan's after others, with a new target state + + // way out of the commented state + From("{2C87AA47-D1BC-4ECB-8A73-2D8D1046C27F}"); + To("{5F4597F4-A4E0-4AFE-90B5-6D2F896830EB}"); // to next // resume at {5F4597F4-A4E0-4AFE-90B5-6D2F896830EB} ... - //Chain("{B19BF0F2-E1C6-4AEB-A146-BC559D97A2C6}"); - Chain("{290C18EE-B3DE-4769-84F1-1F467F3F76DA}"); - From("{B19BF0F2-E1C6-4AEB-A146-BC559D97A2C6}") - .Chain("{290C18EE-B3DE-4769-84F1-1F467F3F76DA}"); + //To("{B19BF0F2-E1C6-4AEB-A146-BC559D97A2C6}"); + To("{290C18EE-B3DE-4769-84F1-1F467F3F76DA}"); + + // way out of the commented state + From("{B19BF0F2-E1C6-4AEB-A146-BC559D97A2C6}"); + To("{290C18EE-B3DE-4769-84F1-1F467F3F76DA}"); // resume at {290C18EE-B3DE-4769-84F1-1F467F3F76DA}... - Chain("{6A2C7C1B-A9DB-4EA9-B6AB-78E7D5B722A7}"); - Chain("{77874C77-93E5-4488-A404-A630907CEEF0}"); - Chain("{8804D8E8-FE62-4E3A-B8A2-C047C2118C38}"); - Chain("{23275462-446E-44C7-8C2C-3B8C1127B07D}"); - Chain("{6B251841-3069-4AD5-8AE9-861F9523E8DA}"); - Chain("{EE429F1B-9B26-43CA-89F8-A86017C809A3}"); - Chain("{08919C4B-B431-449C-90EC-2B8445B5C6B1}"); - Chain("{7EB0254C-CB8B-4C75-B15B-D48C55B449EB}"); + To("{6A2C7C1B-A9DB-4EA9-B6AB-78E7D5B722A7}"); + To("{77874C77-93E5-4488-A404-A630907CEEF0}"); + To("{8804D8E8-FE62-4E3A-B8A2-C047C2118C38}"); + To("{23275462-446E-44C7-8C2C-3B8C1127B07D}"); + To("{6B251841-3069-4AD5-8AE9-861F9523E8DA}"); + To("{EE429F1B-9B26-43CA-89F8-A86017C809A3}"); + To("{08919C4B-B431-449C-90EC-2B8445B5C6B1}"); + To("{7EB0254C-CB8B-4C75-B15B-D48C55B449EB}"); //FINAL @@ -152,20 +160,20 @@ namespace Umbraco.Core.Migrations.Upgrade // and then, need to support upgrading from more recent 7.x // - From("{init-7.10.1}").Chain("{init-7.10.0}"); // same as 7.10.0 - From("{init-7.10.2}").Chain("{init-7.10.0}"); // same as 7.10.0 - From("{init-7.10.3}").Chain("{init-7.10.0}"); // same as 7.10.0 - From("{init-7.10.4}").Chain("{init-7.10.0}"); // same as 7.10.0 - From("{init-7.11.0}").Chain("{init-7.10.0}"); // same as 7.10.0 - From("{init-7.11.1}").Chain("{init-7.10.0}"); // same as 7.10.0 + From("{init-7.10.1}").To("{init-7.10.0}"); // same as 7.10.0 + From("{init-7.10.2}").To("{init-7.10.0}"); // same as 7.10.0 + From("{init-7.10.3}").To("{init-7.10.0}"); // same as 7.10.0 + From("{init-7.10.4}").To("{init-7.10.0}"); // same as 7.10.0 + From("{init-7.11.0}").To("{init-7.10.0}"); // same as 7.10.0 + From("{init-7.11.1}").To("{init-7.10.0}"); // same as 7.10.0 // 7.12.0 has migrations, define a custom chain which copies the chain // going from {init-7.10.0} to former final (1350617A) , and then goes straight to // main chain, skipping the migrations // From("{init-7.12.0}"); - // copy from copy to (former final) main chain - CopyChain("{init-7.10.0}", "{1350617A-4930-4D61-852F-E3AA9E692173}", "{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}"); + // target copy from copy to (former final) + To("{BBD99901-1545-40E4-8A5A-D7A675C7D2F2}", "{init-7.10.0}", "{1350617A-4930-4D61-852F-E3AA9E692173}"); } } } diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs index b24ad2a20e..b78aec9cd4 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs @@ -10,16 +10,20 @@ namespace Umbraco.Core.Migrations.Upgrade { public class UmbracoUpgrader : Upgrader { - public UmbracoUpgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, PostMigrationCollection postMigrations, ILogger logger) - : base(scopeProvider, migrationBuilder, keyValueService, postMigrations, logger) - { } + private readonly PostMigrationCollection _postMigrations; - protected override MigrationPlan GetPlan() + public UmbracoUpgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, PostMigrationCollection postMigrations, ILogger logger) + : base(scopeProvider, migrationBuilder, keyValueService, logger) + { + _postMigrations = postMigrations ?? throw new ArgumentNullException(nameof(postMigrations)); + } + + protected override MigrationPlan CreatePlan() { return new UmbracoPlan(MigrationBuilder, Logger); } - protected override (SemVersion, SemVersion) GetVersions() + protected (SemVersion, SemVersion) GetVersions() { // assume we have something in web.config that makes some sense if (!SemVersion.TryParse(ConfigurationManager.AppSettings["umbracoConfigurationStatus"], out var currentVersion)) @@ -27,5 +31,14 @@ namespace Umbraco.Core.Migrations.Upgrade return (currentVersion, UmbracoVersion.SemanticVersion); } + + public override void AfterMigrations(IScope scope) + { + // run post-migrations + var (originVersion, targetVersion) = GetVersions(); + + foreach (var postMigration in _postMigrations) + postMigration.Execute(Name, scope, originVersion, targetVersion, Logger); + } } } diff --git a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs index 974ed7b4f8..49f36a5099 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs @@ -1,47 +1,76 @@ using System; -using Semver; using Umbraco.Core.Logging; using Umbraco.Core.Scoping; using Umbraco.Core.Services; namespace Umbraco.Core.Migrations.Upgrade { + /// + /// Provides a base class for upgraders. + /// public abstract class Upgrader { private readonly IKeyValueService _keyValueService; - private readonly PostMigrationCollection _postMigrations; private MigrationPlan _plan; - protected Upgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, PostMigrationCollection postMigrations, ILogger logger) + /// + /// Initializes a new instance of the class. + /// + protected Upgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger) { ScopeProvider = scopeProvider ?? throw new ArgumentNullException(nameof(scopeProvider)); MigrationBuilder = migrationBuilder ?? throw new ArgumentNullException(nameof(migrationBuilder)); _keyValueService = keyValueService ?? throw new ArgumentNullException(nameof(keyValueService)); - _postMigrations = postMigrations ?? throw new ArgumentNullException(nameof(postMigrations)); Logger = logger ?? throw new ArgumentNullException(nameof(logger)); } + /// + /// Gets the name of the migration plan. + /// public string Name => Plan.Name; + /// + /// Gets the state value key corresponding to the migration plan. + /// public string StateValueKey => GetStateValueKey(Plan); + /// + /// Gets the scope provider. + /// protected IScopeProvider ScopeProvider { get; } + /// + /// Gets the migration builder. + /// protected IMigrationBuilder MigrationBuilder { get; } + /// + /// Gets the logger. + /// protected ILogger Logger { get; } - protected MigrationPlan Plan => _plan ?? (_plan = GetPlan()); + /// + /// Gets the migration plan. + /// + protected MigrationPlan Plan => _plan ?? (_plan = CreatePlan()); - protected abstract MigrationPlan GetPlan(); - protected abstract (SemVersion, SemVersion) GetVersions(); + /// + /// Creates the migration plan. + /// + /// + protected abstract MigrationPlan CreatePlan(); + /// + /// Executes. + /// public void Execute() { var plan = Plan; using (var scope = ScopeProvider.CreateScope()) { + BeforeMigrations(scope); + // read current state var currentState = _keyValueService.GetValue(StateValueKey); var forceState = false; @@ -63,15 +92,27 @@ namespace Umbraco.Core.Migrations.Upgrade else if (currentState != state) _keyValueService.SetValue(StateValueKey, currentState, state); - // run post-migrations - (var originVersion, var targetVersion) = GetVersions(); - foreach (var postMigration in _postMigrations) - postMigration.Execute(Name, scope, originVersion, targetVersion, Logger); + AfterMigrations(scope); scope.Complete(); } } + /// + /// Executes as part of the upgrade scope and before all migrations have executed. + /// + public virtual void BeforeMigrations(IScope scope) + { } + + /// + /// Executes as part of the upgrade scope and after all migrations have executed. + /// + public virtual void AfterMigrations(IScope scope) + { } + + /// + /// Gets the state value key for a migration plan. + /// public static string GetStateValueKey(MigrationPlan plan) => "Umbraco.Core.Upgrader.State+" + plan.Name; } } diff --git a/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs b/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs index 5505d7ecd7..f6dc3e1063 100644 --- a/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs @@ -34,9 +34,10 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), new PostMigrationCollection(Enumerable.Empty()), logger, + var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, new MigrationPlan("test", builder, logger) - .Add(string.Empty, "done")); + .From(string.Empty) + .To("done")); upgrader.Execute(); @@ -71,10 +72,11 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), new PostMigrationCollection(Enumerable.Empty()), logger, + var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, new MigrationPlan("test", builder, logger) - .Add(string.Empty, "a") - .Add("a", "done")); + .From(string.Empty) + .To("a") + .To("done")); upgrader.Execute(); scope.Complete(); @@ -106,11 +108,12 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), new PostMigrationCollection(Enumerable.Empty()), logger, + var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, new MigrationPlan("test", builder, logger) - .Add(string.Empty, "a") - .Add("a", "b") - .Add("b", "done")); + .From(string.Empty) + .To("a") + .To("b") + .To("done")); upgrader.Execute(); scope.Complete(); @@ -142,11 +145,12 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), new PostMigrationCollection(Enumerable.Empty()), logger, + var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, new MigrationPlan("test", builder, logger) - .Add(string.Empty, "a") - .Add("a", "b") - .Add("b", "done")); + .From(string.Empty) + .To("a") + .To("b") + .To("done")); upgrader.Execute(); scope.Complete(); @@ -176,10 +180,11 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), new PostMigrationCollection(Enumerable.Empty()), logger, + var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, new MigrationPlan("test", builder, logger) - .Add(string.Empty, "a") - .Add("a", "done")); + .From(string.Empty) + .To("a") + .To("done")); upgrader.Execute(); scope.Complete(); diff --git a/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs b/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs index ee1b2a56f5..b246472ee5 100644 --- a/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs @@ -51,8 +51,8 @@ namespace Umbraco.Tests.Migrations var plan = new MigrationPlan("default", migrationBuilder, logger) .From(string.Empty) - .Chain("{4A9A1A8F-0DA1-4BCF-AD06-C19D79152E35}") - .Chain("VERSION.33"); + .To("{4A9A1A8F-0DA1-4BCF-AD06-C19D79152E35}") + .To("VERSION.33"); var kvs = Mock.Of(); Mock.Get(kvs).Setup(x => x.GetValue(It.IsAny())).Returns(k => k == "Umbraco.Tests.MigrationPlan" ? string.Empty : null); @@ -82,9 +82,11 @@ namespace Umbraco.Tests.Migrations public void CanAddMigrations() { var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); - plan.Add(string.Empty, "aaa"); - plan.Add("aaa", "bbb"); - plan.Add("bbb", "ccc"); + plan + .From(string.Empty) + .To("aaa") + .To("bbb") + .To("ccc"); } [Test] @@ -93,7 +95,7 @@ namespace Umbraco.Tests.Migrations var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); Assert.Throws(() => { - plan.Add("aaa", "aaa"); + plan.From("aaa").To("aaa"); }); } @@ -101,10 +103,10 @@ namespace Umbraco.Tests.Migrations public void OnlyOneTransitionPerState() { var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); - plan.Add("aaa", "bbb"); + plan.From("aaa").To("bbb"); Assert.Throws(() => { - plan.Add("aaa", "ccc"); + plan.From("aaa").To("ccc"); }); } @@ -112,9 +114,12 @@ namespace Umbraco.Tests.Migrations public void CannotContainTwoMoreHeads() { var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); - plan.Add(string.Empty, "aaa"); - plan.Add("aaa", "bbb"); - plan.Add("ccc", "ddd"); + plan + .From(string.Empty) + .To("aaa") + .To("bbb") + .From("ccc") + .To("ddd"); Assert.Throws(() => plan.Validate()); } @@ -122,10 +127,11 @@ namespace Umbraco.Tests.Migrations public void CannotContainLoops() { var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); - plan.Add(string.Empty, "aaa"); - plan.Add("aaa", "bbb"); - plan.Add("bbb", "ccc"); - plan.Add("ccc", "aaa"); + plan + .From("aaa") + .To("bbb") + .To("ccc") + .To("aaa"); Assert.Throws(() => plan.Validate()); } diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs index d06cf2244e..4fc2928033 100644 --- a/src/Umbraco.Tests/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs @@ -20,23 +20,38 @@ namespace Umbraco.Tests.Migrations { private readonly MigrationPlan _plan; - public TestUpgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, PostMigrationCollection postMigrations, ILogger logger, MigrationPlan plan) - : base(scopeProvider, migrationBuilder, keyValueService, postMigrations, logger) + public TestUpgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, MigrationPlan plan) + : base(scopeProvider, migrationBuilder, keyValueService, logger) { _plan = plan; } - protected override MigrationPlan GetPlan() + protected override MigrationPlan CreatePlan() { return _plan; } - - protected override (SemVersion, SemVersion) GetVersions() - { - return (new SemVersion(0), new SemVersion(0)); - } } + public class TestUpgraderWithPostMigrations : TestUpgrader + { + private readonly PostMigrationCollection _postMigrations; + + public TestUpgraderWithPostMigrations(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, PostMigrationCollection postMigrations, MigrationPlan plan) + : base(scopeProvider, migrationBuilder, keyValueService, logger, plan) + { + _postMigrations = postMigrations; + } + + public override void AfterMigrations(IScope scope) + { + // run post-migrations + var originVersion = new SemVersion(0); + var targetVersion = new SemVersion(0); + + foreach (var postMigration in _postMigrations) + postMigration.Execute(Name, scope, originVersion, targetVersion, Logger); + } + } public class TestScopeProvider : IScopeProvider { diff --git a/src/Umbraco.Tests/Migrations/PostMigrationTests.cs b/src/Umbraco.Tests/Migrations/PostMigrationTests.cs index d94a1cc917..5b7ec004d5 100644 --- a/src/Umbraco.Tests/Migrations/PostMigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/PostMigrationTests.cs @@ -50,9 +50,8 @@ namespace Umbraco.Tests.Migrations var sqlContext = new SqlContext(new SqlCeSyntaxProvider(), DatabaseType.SQLCe, Mock.Of()); var scopeProvider = new MigrationTests.TestScopeProvider(scope) { SqlContext = sqlContext }; - var u1 = new MigrationTests.TestUpgrader(scopeProvider, builder, Mock.Of(), posts, logger, - new MigrationPlan("Test", builder, logger) - .Add(string.Empty, "done")); + var u1 = new MigrationTests.TestUpgraderWithPostMigrations(scopeProvider, builder, Mock.Of(), logger, posts, + new MigrationPlan("Test", builder, logger).From(string.Empty).To("done")); u1.Execute(); Assert.AreEqual(1, changed1.CountExecuted); @@ -94,17 +93,15 @@ namespace Umbraco.Tests.Migrations var sqlContext = new SqlContext(new SqlCeSyntaxProvider(), DatabaseType.SQLCe, Mock.Of()); var scopeProvider = new MigrationTests.TestScopeProvider(scope) { SqlContext = sqlContext }; - var u1 = new MigrationTests.TestUpgrader(scopeProvider, builder, Mock.Of(), posts, logger, - new MigrationPlan("Test1", builder, logger) - .Add(string.Empty, "done")); + var u1 = new MigrationTests.TestUpgraderWithPostMigrations(scopeProvider, builder, Mock.Of(), logger, posts, + new MigrationPlan("Test1", builder, logger).From(string.Empty).To("done")); u1.Execute(); Assert.AreEqual(1, changed1.CountExecuted); Assert.AreEqual(0, changed2.CountExecuted); - var u2 = new MigrationTests.TestUpgrader(scopeProvider, builder, Mock.Of(), posts, logger, - new MigrationPlan("Test2", builder, logger) - .Add(string.Empty, "done")); + var u2 = new MigrationTests.TestUpgraderWithPostMigrations(scopeProvider, builder, Mock.Of(), logger, posts, + new MigrationPlan("Test2", builder, logger).From(string.Empty).To("done")); u2.Execute(); Assert.AreEqual(1, changed1.CountExecuted); From 84e0d0571fdbb9b8ddf354bd8e9f088806859345 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 4 Jul 2018 15:07:09 +0200 Subject: [PATCH 07/12] Refactor migration plan and upgrader --- .../Migrations/Install/DatabaseBuilder.cs | 49 +------------ .../Migrations/Install/DatabaseDataCreator.cs | 6 +- src/Umbraco.Core/Migrations/MigrationPlan.cs | 48 ++++--------- .../Migrations/Upgrade/UmbracoPlan.cs | 8 --- .../Migrations/Upgrade/UmbracoUpgrader.cs | 40 +++++------ .../Migrations/Upgrade/Upgrader.cs | 70 ++++++------------- src/Umbraco.Core/Runtime/CoreRuntime.cs | 6 +- .../Migrations/AdvancedMigrationTests.cs | 30 ++++---- .../Migrations/MigrationPlanTests.cs | 19 +++-- .../Migrations/MigrationTests.cs | 20 ++++-- .../Migrations/PostMigrationTests.cs | 18 ++--- 11 files changed, 109 insertions(+), 205 deletions(-) diff --git a/src/Umbraco.Core/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Core/Migrations/Install/DatabaseBuilder.cs index 4986eeab02..ea818c60e3 100644 --- a/src/Umbraco.Core/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Core/Migrations/Install/DatabaseBuilder.cs @@ -534,56 +534,11 @@ namespace Umbraco.Core.Migrations.Install _logger.Info("Database upgrade started"); - //var database = scope.Database; - //var supportsCaseInsensitiveQueries = SqlSyntax.SupportsCaseInsensitiveQueries(database); - var message = GetResultMessageForMySql(); - // fixme - remove this code - //var schemaResult = ValidateDatabaseSchema(); - // - //var installedSchemaVersion = new SemVersion(schemaResult.DetermineInstalledVersion()); - //var installedMigrationVersion = schemaResult.DetermineInstalledVersionByMigrations(migrationEntryService); - //var targetVersion = UmbracoVersion.Current; - // - ////In some cases - like upgrading from 7.2.6 -> 7.3, there will be no migration information in the database and therefore it will - //// return a version of 0.0.0 and we don't necessarily want to run all migrations from 0 -> 7.3, so we'll just ensure that the - //// migrations are run for the target version - //if (installedMigrationVersion == new SemVersion(new Version(0, 0, 0)) && installedSchemaVersion > new SemVersion(new Version(0, 0, 0))) - //{ - // //set the installedMigrationVersion to be one less than the target so the latest migrations are guaranteed to execute - // installedMigrationVersion = new SemVersion(targetVersion.SubtractRevision()); - //} - // - ////Figure out what our current installed version is. If the web.config doesn't have a version listed, then we'll use the minimum - //// version detected between the schema installed and the migrations listed in the migration table. - //// If there is a version in the web.config, we'll take the minimum between the listed migration in the db and what - //// is declared in the web.config. - // - //var currentInstalledVersion = string.IsNullOrEmpty(GlobalSettings.ConfigurationStatus) - // //Take the minimum version between the detected schema version and the installed migration version - // ? new[] { installedSchemaVersion, installedMigrationVersion }.Min() - // //Take the minimum version between the installed migration version and the version specified in the config - // : new[] { SemVersion.Parse(GlobalSettings.ConfigurationStatus), installedMigrationVersion }.Min(); - // - ////Ok, another edge case here. If the current version is a pre-release, - //// then we want to ensure all migrations for the current release are executed. - //if (currentInstalledVersion.Prerelease.IsNullOrWhiteSpace() == false) - //{ - // currentInstalledVersion = new SemVersion(currentInstalledVersion.GetVersion().SubtractRevision()); - //} - // upgrade - var upgrader = new UmbracoUpgrader(_scopeProvider, _migrationBuilder, _keyValueService, _postMigrations, _logger); - upgrader.Execute(); - - // fixme remove this code - //var runner = new MigrationRunner(_scopeProvider, builder, migrationEntryService, _logger, currentInstalledVersion, UmbracoVersion.SemanticVersion, Constants.System.UmbracoMigrationName); - //var upgraded = runner.Execute(/*upgrade:true*/); - //if (upgraded == false) - //{ - // throw new ApplicationException("Upgrading failed, either an error occurred during the upgrade process or an event canceled the upgrade process, see log for full details"); - //} + var upgrader = new UmbracoUpgrader(); + upgrader.Execute(_scopeProvider, _migrationBuilder, _keyValueService, _logger, _postMigrations); message = message + "

Upgrade completed!

"; diff --git a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs index eb7cafcb01..f32ea1cb6f 100644 --- a/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs +++ b/src/Umbraco.Core/Migrations/Install/DatabaseDataCreator.cs @@ -321,9 +321,9 @@ namespace Umbraco.Core.Migrations.Install { // on install, initialize the umbraco migration plan with the final state - var plan = new UmbracoPlan(); - var stateValueKey = Upgrader.GetStateValueKey(plan); - var finalState = plan.FinalState; + var upgrader = new UmbracoUpgrader(); + var stateValueKey = upgrader.StateValueKey; + var finalState = upgrader.Plan.FinalState; _database.Insert(Constants.DatabaseSchema.Tables.KeyValue, "key", false, new KeyValueDto { Key = stateValueKey, Value = finalState, Updated = DateTime.Now }); } diff --git a/src/Umbraco.Core/Migrations/MigrationPlan.cs b/src/Umbraco.Core/Migrations/MigrationPlan.cs index 1324abc134..68402de85f 100644 --- a/src/Umbraco.Core/Migrations/MigrationPlan.cs +++ b/src/Umbraco.Core/Migrations/MigrationPlan.cs @@ -12,8 +12,6 @@ namespace Umbraco.Core.Migrations /// public class MigrationPlan { - private readonly IMigrationBuilder _migrationBuilder; - private readonly ILogger _logger; private readonly Dictionary _transitions = new Dictionary(); private string _prevState; @@ -23,8 +21,6 @@ namespace Umbraco.Core.Migrations /// Initializes a new instance of the class. /// /// The name of the plan. - /// The plan cannot be executed. Use this constructor e.g. when only validating the plan, - /// or trying to get its final state, without actually needing to execute it. public MigrationPlan(string name) { if (string.IsNullOrWhiteSpace(name)) throw new ArgumentNullOrEmptyException(nameof(name)); @@ -35,25 +31,6 @@ namespace Umbraco.Core.Migrations DefinePlan(); } - /// - /// Initializes a new instance of the class. - /// - /// The name of the plan. - /// A migration builder. - /// A logger. - /// The plan can be executed. - public MigrationPlan(string name, IMigrationBuilder migrationBuilder, ILogger logger) - { - if (string.IsNullOrWhiteSpace(name)) throw new ArgumentNullOrEmptyException(nameof(name)); - Name = name; - _migrationBuilder = migrationBuilder ?? throw new ArgumentNullException(nameof(migrationBuilder)); - _logger = logger ?? throw new ArgumentNullException(nameof(logger)); - - // ReSharper disable once VirtualMemberCallInConstructor - // (accepted) - DefinePlan(); - } - /// /// Defines the plan. /// @@ -253,43 +230,48 @@ namespace Umbraco.Core.Migrations /// /// A scope. /// The state to start execution at. + /// A migration builder. + /// A logger. /// The final state. /// The plan executes within the scope, which must then be completed. - public string Execute(IScope scope, string fromState) + public string Execute(IScope scope, string fromState, IMigrationBuilder migrationBuilder, ILogger logger) { Validate(); - if (_migrationBuilder == null || _logger == null) - throw new InvalidOperationException("Cannot execute a non-executing plan."); + if (migrationBuilder == null || logger == null) + throw new InvalidOperationException("Cannot execute a non-executable plan."); - _logger.Info("Starting '{MigrationName}'...", Name); + logger.Info("Starting '{MigrationName}'...", Name); var origState = fromState ?? string.Empty; - _logger.Info("At {OrigState}", string.IsNullOrWhiteSpace(origState) ? "origin": origState); + logger.Info("At {OrigState}", string.IsNullOrWhiteSpace(origState) ? "origin": origState); if (!_transitions.TryGetValue(origState, out var transition)) throw new Exception($"Unknown state \"{origState}\"."); - var context = new MigrationContext(scope.Database, _logger); + var context = new MigrationContext(scope.Database, logger); while (transition != null) { - var migration = _migrationBuilder.Build(transition.MigrationType, context); + var migration = migrationBuilder.Build(transition.MigrationType, context); migration.Migrate(); var nextState = transition.TargetState; origState = nextState; - _logger.Info("At {OrigState}", origState); + logger.Info("At {OrigState}", origState); if (!_transitions.TryGetValue(origState, out transition)) throw new Exception($"Unknown state \"{origState}\"."); } - _logger.Info("Done (pending scope completion)."); + logger.Info("Done (pending scope completion)."); + + // safety check + if (origState != _finalState) + throw new Exception($"Internal error, reached state {origState} which is not final state {_finalState}"); - // fixme - what about post-migrations? return origState; } diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index e907ce09ab..15a329d75a 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -2,7 +2,6 @@ using System.Configuration; using Semver; using Umbraco.Core.Configuration; -using Umbraco.Core.Logging; using Umbraco.Core.Migrations.Upgrade.V_7_12_0; using Umbraco.Core.Migrations.Upgrade.V_8_0_0; @@ -20,13 +19,6 @@ namespace Umbraco.Core.Migrations.Upgrade : base(Constants.System.UmbracoUpgradePlanName) { } - /// - /// Initializes a new instance of the class. - /// - public UmbracoPlan(IMigrationBuilder migrationBuilder, ILogger logger) - : base(Constants.System.UmbracoUpgradePlanName, migrationBuilder, logger) - { } - /// /// /// The default initial state in plans is string.Empty. diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs index b78aec9cd4..99c4821680 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs @@ -8,37 +8,37 @@ using Umbraco.Core.Services; namespace Umbraco.Core.Migrations.Upgrade { + /// + /// Represents the Umbraco upgrader. + /// public class UmbracoUpgrader : Upgrader { - private readonly PostMigrationCollection _postMigrations; + private PostMigrationCollection _postMigrations; - public UmbracoUpgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, PostMigrationCollection postMigrations, ILogger logger) - : base(scopeProvider, migrationBuilder, keyValueService, logger) + /// + protected override MigrationPlan CreatePlan() => new UmbracoPlan(); + + /// + /// Executes. + /// + public void Execute(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, PostMigrationCollection postMigrations) { - _postMigrations = postMigrations ?? throw new ArgumentNullException(nameof(postMigrations)); + _postMigrations = postMigrations; + Execute(scopeProvider, migrationBuilder, keyValueService, logger); } - protected override MigrationPlan CreatePlan() + /// + public override void AfterMigrations(IScope scope, ILogger logger) { - return new UmbracoPlan(MigrationBuilder, Logger); - } - - protected (SemVersion, SemVersion) GetVersions() - { - // assume we have something in web.config that makes some sense - if (!SemVersion.TryParse(ConfigurationManager.AppSettings["umbracoConfigurationStatus"], out var currentVersion)) + // assume we have something in web.config that makes some sense = the origin version + if (!SemVersion.TryParse(ConfigurationManager.AppSettings["umbracoConfigurationStatus"], out var originVersion)) throw new InvalidOperationException("Could not get current version from web.config umbracoConfigurationStatus appSetting."); - return (currentVersion, UmbracoVersion.SemanticVersion); - } - - public override void AfterMigrations(IScope scope) - { - // run post-migrations - var (originVersion, targetVersion) = GetVersions(); + // target version is the code version + var targetVersion = UmbracoVersion.SemanticVersion; foreach (var postMigration in _postMigrations) - postMigration.Execute(Name, scope, originVersion, targetVersion, Logger); + postMigration.Execute(Name, scope, originVersion, targetVersion, logger); } } } diff --git a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs index 49f36a5099..4b966a5d80 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs @@ -6,73 +6,49 @@ using Umbraco.Core.Services; namespace Umbraco.Core.Migrations.Upgrade { /// - /// Provides a base class for upgraders. + /// Provides an abstract base class for creating upgraders. /// public abstract class Upgrader { - private readonly IKeyValueService _keyValueService; private MigrationPlan _plan; - /// - /// Initializes a new instance of the class. - /// - protected Upgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger) - { - ScopeProvider = scopeProvider ?? throw new ArgumentNullException(nameof(scopeProvider)); - MigrationBuilder = migrationBuilder ?? throw new ArgumentNullException(nameof(migrationBuilder)); - _keyValueService = keyValueService ?? throw new ArgumentNullException(nameof(keyValueService)); - Logger = logger ?? throw new ArgumentNullException(nameof(logger)); - } - /// /// Gets the name of the migration plan. /// public string Name => Plan.Name; - /// - /// Gets the state value key corresponding to the migration plan. - /// - public string StateValueKey => GetStateValueKey(Plan); - - /// - /// Gets the scope provider. - /// - protected IScopeProvider ScopeProvider { get; } - - /// - /// Gets the migration builder. - /// - protected IMigrationBuilder MigrationBuilder { get; } - - /// - /// Gets the logger. - /// - protected ILogger Logger { get; } - /// /// Gets the migration plan. /// - protected MigrationPlan Plan => _plan ?? (_plan = CreatePlan()); + public MigrationPlan Plan => _plan ?? (_plan = CreatePlan()); /// /// Creates the migration plan. /// - /// protected abstract MigrationPlan CreatePlan(); + /// + /// Gets the key for the state value. + /// + public virtual string StateValueKey => "Umbraco.Core.Upgrader.State+" + Name; + /// /// Executes. /// - public void Execute() + /// A scope provider. + /// A migration builder. + /// A key-value service. + /// A logger. + public void Execute(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger) { var plan = Plan; - using (var scope = ScopeProvider.CreateScope()) + using (var scope = scopeProvider.CreateScope()) { - BeforeMigrations(scope); + BeforeMigrations(scope, logger); // read current state - var currentState = _keyValueService.GetValue(StateValueKey); + var currentState = keyValueService.GetValue(StateValueKey); var forceState = false; if (currentState == null) @@ -82,17 +58,17 @@ namespace Umbraco.Core.Migrations.Upgrade } // execute plan - var state = plan.Execute(scope, currentState); + var state = plan.Execute(scope, currentState, migrationBuilder, logger); if (string.IsNullOrWhiteSpace(state)) throw new Exception("Plan execution returned an invalid null or empty state."); // save new state if (forceState) - _keyValueService.SetValue(StateValueKey, state); + keyValueService.SetValue(StateValueKey, state); else if (currentState != state) - _keyValueService.SetValue(StateValueKey, currentState, state); + keyValueService.SetValue(StateValueKey, currentState, state); - AfterMigrations(scope); + AfterMigrations(scope, logger); scope.Complete(); } @@ -101,18 +77,14 @@ namespace Umbraco.Core.Migrations.Upgrade /// /// Executes as part of the upgrade scope and before all migrations have executed. /// - public virtual void BeforeMigrations(IScope scope) + public virtual void BeforeMigrations(IScope scope, ILogger logger) { } /// /// Executes as part of the upgrade scope and after all migrations have executed. /// - public virtual void AfterMigrations(IScope scope) + public virtual void AfterMigrations(IScope scope, ILogger logger) { } - /// - /// Gets the state value key for a migration plan. - /// - public static string GetStateValueKey(MigrationPlan plan) => "Umbraco.Core.Upgrader.State+" + plan.Name; } } diff --git a/src/Umbraco.Core/Runtime/CoreRuntime.cs b/src/Umbraco.Core/Runtime/CoreRuntime.cs index cc35543b66..dde91ad385 100755 --- a/src/Umbraco.Core/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Core/Runtime/CoreRuntime.cs @@ -389,14 +389,14 @@ namespace Umbraco.Core.Runtime protected virtual bool EnsureUmbracoUpgradeState(IUmbracoDatabaseFactory databaseFactory, ILogger logger) { - var umbracoPlan = new UmbracoPlan(); - var stateValueKey = Upgrader.GetStateValueKey(umbracoPlan); + var upgrader = new UmbracoUpgrader(); + var stateValueKey = upgrader.StateValueKey; // no scope, no service - just directly accessing the database using (var database = databaseFactory.CreateDatabase()) { _state.CurrentMigrationState = KeyValueService.GetValue(database, stateValueKey); - _state.FinalMigrationState = umbracoPlan.FinalState; + _state.FinalMigrationState = upgrader.Plan.FinalState; } logger.Debug("Final upgrade state is {FinalMigrationState}, database contains {DatabaseState}", _state.FinalMigrationState, _state.CurrentMigrationState ?? ""); diff --git a/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs b/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs index f6dc3e1063..07776fdee1 100644 --- a/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs @@ -34,12 +34,12 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, - new MigrationPlan("test", builder, logger) + var upgrader = new MigrationTests.TestUpgrader( + new MigrationPlan("test") .From(string.Empty) .To("done")); - upgrader.Execute(); + upgrader.Execute(ScopeProvider, builder, Mock.Of(), logger); var helper = new DatabaseSchemaCreator(scope.Database, logger); var exists = helper.TableExists("umbracoUser"); @@ -72,13 +72,13 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, - new MigrationPlan("test", builder, logger) + var upgrader = new MigrationTests.TestUpgrader( + new MigrationPlan("test") .From(string.Empty) .To("a") .To("done")); - upgrader.Execute(); + upgrader.Execute(ScopeProvider, builder, Mock.Of(), logger); scope.Complete(); } } @@ -108,14 +108,14 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, - new MigrationPlan("test", builder, logger) + var upgrader = new MigrationTests.TestUpgrader( + new MigrationPlan("test") .From(string.Empty) .To("a") .To("b") .To("done")); - upgrader.Execute(); + upgrader.Execute(ScopeProvider, builder, Mock.Of(), logger); scope.Complete(); } } @@ -145,14 +145,14 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, - new MigrationPlan("test", builder, logger) + var upgrader = new MigrationTests.TestUpgrader( + new MigrationPlan("test") .From(string.Empty) .To("a") .To("b") .To("done")); - upgrader.Execute(); + upgrader.Execute(ScopeProvider, builder, Mock.Of(), logger); scope.Complete(); } } @@ -180,13 +180,13 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader(ScopeProvider, builder, Mock.Of(), logger, - new MigrationPlan("test", builder, logger) + var upgrader = new MigrationTests.TestUpgrader( + new MigrationPlan("test") .From(string.Empty) .To("a") .To("done")); - upgrader.Execute(); + upgrader.Execute(ScopeProvider, builder, Mock.Of(), logger); scope.Complete(); } } diff --git a/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs b/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs index b246472ee5..d6ac7abff4 100644 --- a/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs @@ -46,10 +46,7 @@ namespace Umbraco.Tests.Migrations } }); - // fixme - NOT a migration collection builder, just a migration builder - // done, remove everywhere else, and delete migrationCollection stuff entirely - - var plan = new MigrationPlan("default", migrationBuilder, logger) + var plan = new MigrationPlan("default") .From(string.Empty) .To("{4A9A1A8F-0DA1-4BCF-AD06-C19D79152E35}") .To("VERSION.33"); @@ -64,7 +61,7 @@ namespace Umbraco.Tests.Migrations var sourceState = kvs.GetValue("Umbraco.Tests.MigrationPlan") ?? string.Empty; // execute plan - state = plan.Execute(s, sourceState); + state = plan.Execute(s, sourceState, migrationBuilder, logger); // save new state kvs.SetValue("Umbraco.Tests.MigrationPlan", sourceState, state); @@ -81,7 +78,7 @@ namespace Umbraco.Tests.Migrations [Test] public void CanAddMigrations() { - var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); + var plan = new MigrationPlan("default"); plan .From(string.Empty) .To("aaa") @@ -92,7 +89,7 @@ namespace Umbraco.Tests.Migrations [Test] public void CannotTransitionToSameState() { - var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); + var plan = new MigrationPlan("default"); Assert.Throws(() => { plan.From("aaa").To("aaa"); @@ -102,7 +99,7 @@ namespace Umbraco.Tests.Migrations [Test] public void OnlyOneTransitionPerState() { - var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); + var plan = new MigrationPlan("default"); plan.From("aaa").To("bbb"); Assert.Throws(() => { @@ -113,7 +110,7 @@ namespace Umbraco.Tests.Migrations [Test] public void CannotContainTwoMoreHeads() { - var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); + var plan = new MigrationPlan("default"); plan .From(string.Empty) .To("aaa") @@ -126,7 +123,7 @@ namespace Umbraco.Tests.Migrations [Test] public void CannotContainLoops() { - var plan = new MigrationPlan("default", Mock.Of(), Mock.Of()); + var plan = new MigrationPlan("default"); plan .From("aaa") .To("bbb") @@ -138,7 +135,7 @@ namespace Umbraco.Tests.Migrations [Test] public void ValidateUmbracoPlan() { - var plan = new UmbracoPlan(Mock.Of(), Mock.Of()); + var plan = new UmbracoPlan(); plan.Validate(); Console.WriteLine(plan.FinalState); Assert.IsFalse(string.IsNullOrWhiteSpace(plan.FinalState)); diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs index 4fc2928033..c539cd0c7d 100644 --- a/src/Umbraco.Tests/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs @@ -1,8 +1,10 @@ using System; +using System.Configuration; using System.Data; using Moq; using NUnit.Framework; using Semver; +using Umbraco.Core.Configuration; using Umbraco.Core.Events; using Umbraco.Core.Migrations; using Umbraco.Core.Migrations.Upgrade; @@ -20,8 +22,7 @@ namespace Umbraco.Tests.Migrations { private readonly MigrationPlan _plan; - public TestUpgrader(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, MigrationPlan plan) - : base(scopeProvider, migrationBuilder, keyValueService, logger) + public TestUpgrader(MigrationPlan plan) { _plan = plan; } @@ -34,22 +35,27 @@ namespace Umbraco.Tests.Migrations public class TestUpgraderWithPostMigrations : TestUpgrader { - private readonly PostMigrationCollection _postMigrations; + private PostMigrationCollection _postMigrations; - public TestUpgraderWithPostMigrations(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, PostMigrationCollection postMigrations, MigrationPlan plan) - : base(scopeProvider, migrationBuilder, keyValueService, logger, plan) + public TestUpgraderWithPostMigrations(MigrationPlan plan) + : base(plan) + { } + + public void Execute(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, PostMigrationCollection postMigrations) { _postMigrations = postMigrations; + Execute(scopeProvider, migrationBuilder, keyValueService, logger); } - public override void AfterMigrations(IScope scope) + public override void AfterMigrations(IScope scope, ILogger logger) { // run post-migrations var originVersion = new SemVersion(0); var targetVersion = new SemVersion(0); + // run post-migrations foreach (var postMigration in _postMigrations) - postMigration.Execute(Name, scope, originVersion, targetVersion, Logger); + postMigration.Execute(Name, scope, originVersion, targetVersion, logger); } } diff --git a/src/Umbraco.Tests/Migrations/PostMigrationTests.cs b/src/Umbraco.Tests/Migrations/PostMigrationTests.cs index 5b7ec004d5..95f1f8afac 100644 --- a/src/Umbraco.Tests/Migrations/PostMigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/PostMigrationTests.cs @@ -50,9 +50,9 @@ namespace Umbraco.Tests.Migrations var sqlContext = new SqlContext(new SqlCeSyntaxProvider(), DatabaseType.SQLCe, Mock.Of()); var scopeProvider = new MigrationTests.TestScopeProvider(scope) { SqlContext = sqlContext }; - var u1 = new MigrationTests.TestUpgraderWithPostMigrations(scopeProvider, builder, Mock.Of(), logger, posts, - new MigrationPlan("Test", builder, logger).From(string.Empty).To("done")); - u1.Execute(); + var u1 = new MigrationTests.TestUpgraderWithPostMigrations( + new MigrationPlan("Test").From(string.Empty).To("done")); + u1.Execute(scopeProvider, builder, Mock.Of(), logger, posts); Assert.AreEqual(1, changed1.CountExecuted); } @@ -93,16 +93,16 @@ namespace Umbraco.Tests.Migrations var sqlContext = new SqlContext(new SqlCeSyntaxProvider(), DatabaseType.SQLCe, Mock.Of()); var scopeProvider = new MigrationTests.TestScopeProvider(scope) { SqlContext = sqlContext }; - var u1 = new MigrationTests.TestUpgraderWithPostMigrations(scopeProvider, builder, Mock.Of(), logger, posts, - new MigrationPlan("Test1", builder, logger).From(string.Empty).To("done")); - u1.Execute(); + var u1 = new MigrationTests.TestUpgraderWithPostMigrations( + new MigrationPlan("Test1").From(string.Empty).To("done")); + u1.Execute(scopeProvider, builder, Mock.Of(), logger, posts); Assert.AreEqual(1, changed1.CountExecuted); Assert.AreEqual(0, changed2.CountExecuted); - var u2 = new MigrationTests.TestUpgraderWithPostMigrations(scopeProvider, builder, Mock.Of(), logger, posts, - new MigrationPlan("Test2", builder, logger).From(string.Empty).To("done")); - u2.Execute(); + var u2 = new MigrationTests.TestUpgraderWithPostMigrations( + new MigrationPlan("Test2").From(string.Empty).To("done")); + u2.Execute(scopeProvider, builder, Mock.Of(), logger, posts); Assert.AreEqual(1, changed1.CountExecuted); Assert.AreEqual(1, changed2.CountExecuted); From b086fd2f0bb083f6bf7e2b9ba1eedcde551f0801 Mon Sep 17 00:00:00 2001 From: Stephan Date: Wed, 4 Jul 2018 18:19:06 +0200 Subject: [PATCH 08/12] Cleanup the Umbraco migration plan --- src/Umbraco.Core/Migrations/MigrationPlan.cs | 49 +++++++++++++++++-- .../Migrations/Upgrade/Upgrader.cs | 5 ++ .../Migrations/MigrationPlanTests.cs | 36 ++++++++++++++ 3 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Core/Migrations/MigrationPlan.cs b/src/Umbraco.Core/Migrations/MigrationPlan.cs index 68402de85f..49edf6cc05 100644 --- a/src/Umbraco.Core/Migrations/MigrationPlan.cs +++ b/src/Umbraco.Core/Migrations/MigrationPlan.cs @@ -4,6 +4,7 @@ using System.Linq; using Umbraco.Core.Exceptions; using Umbraco.Core.Logging; using Umbraco.Core.Scoping; +using Type = System.Type; namespace Umbraco.Core.Migrations { @@ -31,6 +32,11 @@ namespace Umbraco.Core.Migrations DefinePlan(); } + /// + /// Gets the transitions. + /// + public IReadOnlyDictionary Transitions => _transitions; + /// /// Defines the plan. /// @@ -238,8 +244,8 @@ namespace Umbraco.Core.Migrations { Validate(); - if (migrationBuilder == null || logger == null) - throw new InvalidOperationException("Cannot execute a non-executable plan."); + if (migrationBuilder == null) throw new ArgumentNullException(nameof(migrationBuilder)); + if (logger == null) throw new ArgumentNullException(nameof(logger)); logger.Info("Starting '{MigrationName}'...", Name); @@ -275,10 +281,47 @@ namespace Umbraco.Core.Migrations return origState; } + /// + /// Follows a path (for tests and debugging). + /// + /// Does the same thing Execute does, but does not actually execute migrations. + internal string FollowPath(string fromState, string toState = null) + { + toState = toState.NullOrWhiteSpaceAsNull(); + + Validate(); + + var origState = fromState ?? string.Empty; + + if (!_transitions.TryGetValue(origState, out var transition)) + throw new Exception($"Unknown state \"{origState}\"."); + + while (transition != null) + { + var nextState = transition.TargetState; + origState = nextState; + + if (nextState == toState) + { + transition = null; + continue; + } + + if (!_transitions.TryGetValue(origState, out transition)) + throw new Exception($"Unknown state \"{origState}\"."); + } + + // safety check + if (origState != (toState ?? _finalState)) + throw new Exception($"Internal error, reached state {origState} which is not state {toState ?? _finalState}"); + + return origState; + } + /// /// Represents a plan transition. /// - private class Transition + public class Transition { /// /// Initializes a new instance of the class. diff --git a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs index 4b966a5d80..3795ed79af 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs @@ -41,6 +41,11 @@ namespace Umbraco.Core.Migrations.Upgrade /// A logger. public void Execute(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger) { + if (scopeProvider == null) throw new ArgumentNullException(nameof(scopeProvider)); + if (migrationBuilder == null) throw new ArgumentNullException(nameof(migrationBuilder)); + if (keyValueService == null) throw new ArgumentNullException(nameof(keyValueService)); + if (logger == null) throw new ArgumentNullException(nameof(logger)); + var plan = Plan; using (var scope = scopeProvider.CreateScope()) diff --git a/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs b/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs index d6ac7abff4..9aec42c252 100644 --- a/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationPlanTests.cs @@ -1,7 +1,9 @@ using System; +using System.Linq; using Moq; using NPoco; using NUnit.Framework; +using Umbraco.Core; using Umbraco.Core.Logging; using Umbraco.Core.Migrations; using Umbraco.Core.Migrations.Upgrade; @@ -141,6 +143,40 @@ namespace Umbraco.Tests.Migrations Assert.IsFalse(string.IsNullOrWhiteSpace(plan.FinalState)); } + [Test] + public void CanCopyChain() + { + var plan = new MigrationPlan("default"); + plan + .From(string.Empty) + .To("aaa") + .To("bbb") + .To("ccc") + .To("ddd") + .To("eee"); + + plan + .From("xxx") + .To("yyy", "bbb", "ddd") + .To("eee"); + + WritePlanToConsole(plan); + + plan.Validate(); + Assert.AreEqual("eee", plan.FollowPath("xxx")); + Assert.AreEqual("yyy", plan.FollowPath("xxx", "yyy")); + } + + private void WritePlanToConsole(MigrationPlan plan) + { + var final = plan.Transitions.First(x => x.Value == null).Key; + + Console.WriteLine("plan \"{0}\" to final state \"{1}\":", plan.Name, final); + foreach (var (_, transition) in plan.Transitions) + if (transition != null) + Console.WriteLine(transition); + } + public class DeleteRedirectUrlTable : MigrationBase { public DeleteRedirectUrlTable(IMigrationContext context) From b60cda2e1bfaa7ae4b04752067891df4a92718e4 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 4 Dec 2018 18:50:21 +0100 Subject: [PATCH 09/12] Add a missing null check --- src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs index 6f814a7174..ff5e1dec0e 100644 --- a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs +++ b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs @@ -1136,7 +1136,7 @@ namespace Umbraco.Core.Persistence { text.AppendLine(sql); - if (arguments.Length == 0) + if (arguments == null || arguments.Length == 0) return; text.Append(" --"); From 376203a4e10dcb99b5371f2cd9bbb3cb610fb20e Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 6 Dec 2018 07:48:26 +0100 Subject: [PATCH 10/12] Further simplify upgrader --- .../Migrations/Upgrade/UmbracoUpgrader.cs | 8 ++++++-- .../Migrations/Upgrade/Upgrader.cs | 19 ++++++++++--------- .../Migrations/AdvancedMigrationTests.cs | 11 ++++++----- .../Migrations/MigrationTests.cs | 19 +------------------ 4 files changed, 23 insertions(+), 34 deletions(-) diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs index 99c4821680..fa29e80a6b 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoUpgrader.cs @@ -15,8 +15,12 @@ namespace Umbraco.Core.Migrations.Upgrade { private PostMigrationCollection _postMigrations; - /// - protected override MigrationPlan CreatePlan() => new UmbracoPlan(); + /// + /// Initializes a new instance of the class. + /// + public UmbracoUpgrader() + : base(new UmbracoPlan()) + { } /// /// Executes. diff --git a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs index 3795ed79af..f6df52bc1e 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/Upgrader.cs @@ -6,11 +6,17 @@ using Umbraco.Core.Services; namespace Umbraco.Core.Migrations.Upgrade { /// - /// Provides an abstract base class for creating upgraders. + /// Represents an upgrader. /// - public abstract class Upgrader + public class Upgrader { - private MigrationPlan _plan; + /// + /// Initializes a new instance of the class. + /// + public Upgrader(MigrationPlan plan) + { + Plan = plan; + } /// /// Gets the name of the migration plan. @@ -20,12 +26,7 @@ namespace Umbraco.Core.Migrations.Upgrade /// /// Gets the migration plan. /// - public MigrationPlan Plan => _plan ?? (_plan = CreatePlan()); - - /// - /// Creates the migration plan. - /// - protected abstract MigrationPlan CreatePlan(); + public MigrationPlan Plan { get; } /// /// Gets the key for the state value. diff --git a/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs b/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs index 07776fdee1..5ff0dcffc1 100644 --- a/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/AdvancedMigrationTests.cs @@ -5,6 +5,7 @@ using NUnit.Framework; using Umbraco.Core.Logging; using Umbraco.Core.Migrations; using Umbraco.Core.Migrations.Install; +using Umbraco.Core.Migrations.Upgrade; using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Umbraco.Core.Persistence.Dtos; using Umbraco.Core.Services; @@ -34,7 +35,7 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader( + var upgrader = new Upgrader( new MigrationPlan("test") .From(string.Empty) .To("done")); @@ -72,7 +73,7 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader( + var upgrader = new Upgrader( new MigrationPlan("test") .From(string.Empty) .To("a") @@ -108,7 +109,7 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader( + var upgrader = new Upgrader( new MigrationPlan("test") .From(string.Empty) .To("a") @@ -145,7 +146,7 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader( + var upgrader = new Upgrader( new MigrationPlan("test") .From(string.Empty) .To("a") @@ -180,7 +181,7 @@ namespace Umbraco.Tests.Migrations using (var scope = ScopeProvider.CreateScope()) { - var upgrader = new MigrationTests.TestUpgrader( + var upgrader = new Upgrader( new MigrationPlan("test") .From(string.Empty) .To("a") diff --git a/src/Umbraco.Tests/Migrations/MigrationTests.cs b/src/Umbraco.Tests/Migrations/MigrationTests.cs index c539cd0c7d..8e84f88265 100644 --- a/src/Umbraco.Tests/Migrations/MigrationTests.cs +++ b/src/Umbraco.Tests/Migrations/MigrationTests.cs @@ -1,10 +1,8 @@ using System; -using System.Configuration; using System.Data; using Moq; using NUnit.Framework; using Semver; -using Umbraco.Core.Configuration; using Umbraco.Core.Events; using Umbraco.Core.Migrations; using Umbraco.Core.Migrations.Upgrade; @@ -18,22 +16,7 @@ namespace Umbraco.Tests.Migrations [TestFixture] public class MigrationTests { - public class TestUpgrader : Upgrader - { - private readonly MigrationPlan _plan; - - public TestUpgrader(MigrationPlan plan) - { - _plan = plan; - } - - protected override MigrationPlan CreatePlan() - { - return _plan; - } - } - - public class TestUpgraderWithPostMigrations : TestUpgrader + public class TestUpgraderWithPostMigrations : Upgrader { private PostMigrationCollection _postMigrations; From 54c352f1f96873f8fbd076e5baede56a28725ea3 Mon Sep 17 00:00:00 2001 From: Stephan Date: Thu, 6 Dec 2018 07:54:53 +0100 Subject: [PATCH 11/12] Kill sql.WriteToConsole --- src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs | 5 ----- src/Umbraco.Core/Persistence/SqlTemplate.cs | 5 +++-- .../Persistence/NPocoTests/NPocoFetchTests.cs | 2 -- .../NPocoTests/NPocoSqlExtensionsTests.cs | 1 - .../NPocoTests/NPocoSqlTemplateTests.cs | 14 ++++---------- .../Persistence/NPocoTests/NPocoSqlTests.cs | 2 -- .../Persistence/Querying/QueryBuilderTests.cs | 2 -- 7 files changed, 7 insertions(+), 24 deletions(-) diff --git a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs index ff5e1dec0e..9ce7bd9fee 100644 --- a/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs +++ b/src/Umbraco.Core/Persistence/NPocoSqlExtensions.cs @@ -1115,11 +1115,6 @@ namespace Umbraco.Core.Persistence return string.IsNullOrWhiteSpace(attr?.Name) ? column.Name : attr.Name; } - internal static void WriteToConsole(this Sql sql) - { - Console.Write(sql.ToText()); - } - internal static string ToText(this Sql sql) { var text = new StringBuilder(); diff --git a/src/Umbraco.Core/Persistence/SqlTemplate.cs b/src/Umbraco.Core/Persistence/SqlTemplate.cs index 7304f45e7f..e81da20f41 100644 --- a/src/Umbraco.Core/Persistence/SqlTemplate.cs +++ b/src/Umbraco.Core/Persistence/SqlTemplate.cs @@ -95,9 +95,10 @@ namespace Umbraco.Core.Persistence return new Sql(_sqlContext, isBuilt, _sql, args); } - internal void WriteToConsole() + internal string ToText() { - new Sql(_sqlContext, _sql, _args.Values.ToArray()).WriteToConsole(); + var sql = new Sql(_sqlContext, _sql, _args.Values.ToArray()); + return sql.ToText(); } /// diff --git a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoFetchTests.cs b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoFetchTests.cs index f33ee563b4..64bc825c3e 100644 --- a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoFetchTests.cs +++ b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoFetchTests.cs @@ -403,7 +403,6 @@ namespace Umbraco.Tests.Persistence.NPocoTests .From() .Where(x => x.Id == 1); - sql.WriteToConsole(); var dto = scope.Database.Fetch(sql).FirstOrDefault(); Assert.IsNotNull(dto); Assert.AreEqual("one", dto.Name); @@ -415,7 +414,6 @@ namespace Umbraco.Tests.Persistence.NPocoTests //Assert.AreEqual("one", dto.Name); var sql3 = new Sql(sql.SQL, 1); - sql.WriteToConsole(); dto = scope.Database.Fetch(sql3).FirstOrDefault(); Assert.IsNotNull(dto); Assert.AreEqual("one", dto.Name); diff --git a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlExtensionsTests.cs b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlExtensionsTests.cs index 458479b293..36b850d24d 100644 --- a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlExtensionsTests.cs +++ b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlExtensionsTests.cs @@ -117,7 +117,6 @@ INNER JOIN [dto2] ON [dto1].[id] = [dto2].[dto1id]".NoCrLf(), sql.SQL.NoCrLf()); var sql = Sql() .Update(u => u.Set(x => x.EditorAlias, "Umbraco.ColorPicker")) .Where(x => x.EditorAlias == "Umbraco.ColorPickerAlias"); - sql.WriteToConsole(); } [TableName("dto1")] diff --git a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTemplateTests.cs b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTemplateTests.cs index ac6df7897d..b24642e88b 100644 --- a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTemplateTests.cs +++ b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTemplateTests.cs @@ -27,15 +27,9 @@ namespace Umbraco.Tests.Persistence.NPocoTests .From("zbThing1") .Where("id=@id", new { id = SqlTemplate.Arg("id") })).Sql(new { id = 1 }); - sql.WriteToConsole(); - var sql2 = sqlTemplates.Get("xxx", x => throw new InvalidOperationException("Should be cached.")).Sql(1); - sql2.WriteToConsole(); - var sql3 = sqlTemplates.Get("xxx", x => throw new InvalidOperationException("Should be cached.")).Sql(new { id = 1 }); - - sql3.WriteToConsole(); } [Test] @@ -75,8 +69,8 @@ namespace Umbraco.Tests.Persistence.NPocoTests Assert.AreEqual(1, sql.Arguments.Length); Assert.AreEqual(123, sql.Arguments[0]); - Assert.Throws(() => template.Sql(new { xvalue = 123 }).WriteToConsole()); - Assert.Throws(() => template.Sql(new { value = 123, xvalue = 456 }).WriteToConsole()); + Assert.Throws(() => template.Sql(new { xvalue = 123 })); + Assert.Throws(() => template.Sql(new { value = 123, xvalue = 456 })); var i = 666; @@ -121,8 +115,8 @@ namespace Umbraco.Tests.Persistence.NPocoTests Assert.AreEqual(1, sql.Arguments.Length); Assert.AreEqual(123, sql.Arguments[0]); - Assert.Throws(() => template.Sql(new { j = 123 }).WriteToConsole()); - Assert.Throws(() => template.Sql(new { i = 123, j = 456 }).WriteToConsole()); + Assert.Throws(() => template.Sql(new { j = 123 })); + Assert.Throws(() => template.Sql(new { i = 123, j = 456 })); // now with more arguments diff --git a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTests.cs b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTests.cs index 6fa2af74cf..a04984eb64 100644 --- a/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTests.cs +++ b/src/Umbraco.Tests/Persistence/NPocoTests/NPocoSqlTests.cs @@ -305,12 +305,10 @@ namespace Umbraco.Tests.Persistence.NPocoTests .From() .Where(x => x.SessionId == sessionId); - sql.WriteToConsole(); Assert.AreEqual("SELECT * FROM [umbracoUserLogin] WHERE (([umbracoUserLogin].[sessionId] = @0))", sql.SQL.NoCrLf()); sql = sql.ForUpdate(); - sql.WriteToConsole(); Assert.AreEqual("SELECT * FROM [umbracoUserLogin] WITH (UPDLOCK) WHERE (([umbracoUserLogin].[sessionId] = @0))", sql.SQL.NoCrLf()); } } diff --git a/src/Umbraco.Tests/Persistence/Querying/QueryBuilderTests.cs b/src/Umbraco.Tests/Persistence/Querying/QueryBuilderTests.cs index a9928046d7..ca6b4cd5f0 100644 --- a/src/Umbraco.Tests/Persistence/Querying/QueryBuilderTests.cs +++ b/src/Umbraco.Tests/Persistence/Querying/QueryBuilderTests.cs @@ -107,8 +107,6 @@ namespace Umbraco.Tests.Persistence.Querying var translator = new SqlTranslator(sql, query); var result = translator.Translate(); - result.WriteToConsole(); - Assert.AreEqual("-1,1046,1076,1089%", result.Arguments[0]); Assert.AreEqual(1046, result.Arguments[1]); Assert.AreEqual(true, result.Arguments[2]); From a74052d647f7f08614ef6fd56d77694480630a72 Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 11 Dec 2018 10:26:58 +0100 Subject: [PATCH 12/12] Further simplify upgrader --- src/Umbraco.Core/Migrations/MigrationPlan.cs | 28 ------------------- .../Migrations/Upgrade/UmbracoPlan.cs | 8 ++++-- 2 files changed, 5 insertions(+), 31 deletions(-) diff --git a/src/Umbraco.Core/Migrations/MigrationPlan.cs b/src/Umbraco.Core/Migrations/MigrationPlan.cs index 49edf6cc05..2e2bc8b661 100644 --- a/src/Umbraco.Core/Migrations/MigrationPlan.cs +++ b/src/Umbraco.Core/Migrations/MigrationPlan.cs @@ -26,10 +26,6 @@ namespace Umbraco.Core.Migrations { if (string.IsNullOrWhiteSpace(name)) throw new ArgumentNullOrEmptyException(nameof(name)); Name = name; - - // ReSharper disable once VirtualMemberCallInConstructor - // (accepted) - DefinePlan(); } /// @@ -37,11 +33,6 @@ namespace Umbraco.Core.Migrations /// public IReadOnlyDictionary Transitions => _transitions; - /// - /// Defines the plan. - /// - protected virtual void DefinePlan() { } - /// /// Gets the name of the plan. /// @@ -145,25 +136,6 @@ namespace Umbraco.Core.Migrations return this; } - /// - /// Copies a chain. - /// - /// Copies the chain going from startState to endState, with new states going from sourceState to targetState. - [Obsolete("die", true)] - public MigrationPlan CopyChain(string sourceState, string startState, string endState, string targetState) - { - From(sourceState).To(targetState, startState, endState); - return this; - } - - /// - /// Copies a chain. - /// - /// Copies the chain going from startState to endState, with new states going from chain to targetState. - [Obsolete("die", true)] - public MigrationPlan CopyChain(string startState, string endState, string targetState) - => To(targetState, startState, endState); - /// /// Gets the initial state. /// diff --git a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs index 15a329d75a..c5e24210c0 100644 --- a/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Core/Migrations/Upgrade/UmbracoPlan.cs @@ -17,7 +17,9 @@ namespace Umbraco.Core.Migrations.Upgrade /// public UmbracoPlan() : base(Constants.System.UmbracoUpgradePlanName) - { } + { + DefinePlan(); + } /// /// @@ -53,8 +55,8 @@ namespace Umbraco.Core.Migrations.Upgrade } } - /// - protected override void DefinePlan() + // define the plan + protected void DefinePlan() { // MODIFYING THE PLAN //