From 8eb0f45cf214ae30dd7af1a5cf690e5257f9f669 Mon Sep 17 00:00:00 2001 From: Shannon Deminick Date: Wed, 9 Jun 2021 16:18:15 +1000 Subject: [PATCH] Refactor MigrationPlan to separate the executor from the plan itself. (#10417) * Refactor MigrationPlan to separate the executor from the plan itself. * cleanup --- .../Migrations/IMigrationPlanExecutor.cs | 7 + .../Migrations/MergeBuilder.cs | 5 +- .../Migrations/MigrationPlan.cs | 142 +++++------------- .../Packaging/PackageMigrationPlan.cs | 14 ++ .../UmbracoBuilder.CoreServices.cs | 2 + .../Migrations/Install/DatabaseBuilder.cs | 9 +- .../Migrations/MigrationPlanExecutor.cs | 105 +++++++++++++ .../Migrations/Upgrade/UmbracoPlan.cs | 3 +- .../Migrations/Upgrade/Upgrader.cs | 36 ++--- src/Umbraco.Infrastructure/RuntimeState.cs | 16 -- .../Migrations/AdvancedMigrationTests.cs | 14 +- .../Migrations/MigrationPlanTests.cs | 6 +- .../Migrations/PostMigrationTests.cs | 18 +-- 13 files changed, 207 insertions(+), 170 deletions(-) create mode 100644 src/Umbraco.Core/Migrations/IMigrationPlanExecutor.cs rename src/{Umbraco.Infrastructure => Umbraco.Core}/Migrations/MergeBuilder.cs (96%) rename src/{Umbraco.Infrastructure => Umbraco.Core}/Migrations/MigrationPlan.cs (68%) create mode 100644 src/Umbraco.Core/Packaging/PackageMigrationPlan.cs create mode 100644 src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs diff --git a/src/Umbraco.Core/Migrations/IMigrationPlanExecutor.cs b/src/Umbraco.Core/Migrations/IMigrationPlanExecutor.cs new file mode 100644 index 0000000000..0ba8bc1ecd --- /dev/null +++ b/src/Umbraco.Core/Migrations/IMigrationPlanExecutor.cs @@ -0,0 +1,7 @@ +namespace Umbraco.Cms.Core.Migrations +{ + public interface IMigrationPlanExecutor + { + string Execute(MigrationPlan plan, string fromState); + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/MergeBuilder.cs b/src/Umbraco.Core/Migrations/MergeBuilder.cs similarity index 96% rename from src/Umbraco.Infrastructure/Migrations/MergeBuilder.cs rename to src/Umbraco.Core/Migrations/MergeBuilder.cs index 4385fd54b8..a00a774db4 100644 --- a/src/Umbraco.Infrastructure/Migrations/MergeBuilder.cs +++ b/src/Umbraco.Core/Migrations/MergeBuilder.cs @@ -1,8 +1,7 @@ -using System; +using System; using System.Collections.Generic; -using Umbraco.Cms.Core.Migrations; -namespace Umbraco.Cms.Infrastructure.Migrations +namespace Umbraco.Cms.Core.Migrations { /// /// Represents a migration plan builder for merges. diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs b/src/Umbraco.Core/Migrations/MigrationPlan.cs similarity index 68% rename from src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs rename to src/Umbraco.Core/Migrations/MigrationPlan.cs index 3bde224640..19bd551075 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs +++ b/src/Umbraco.Core/Migrations/MigrationPlan.cs @@ -1,14 +1,12 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; -using Microsoft.Extensions.Logging; -using Umbraco.Cms.Core.Migrations; -using Umbraco.Cms.Core.Scoping; using Umbraco.Extensions; using Type = System.Type; -namespace Umbraco.Cms.Infrastructure.Migrations +namespace Umbraco.Cms.Core.Migrations { + /// /// Represents a migration plan. /// @@ -26,8 +24,10 @@ namespace Umbraco.Cms.Infrastructure.Migrations /// The name of the plan. public MigrationPlan(string name) { - if (name == null) throw new ArgumentNullException(nameof(name)); - if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(name)); + if (name == null) + throw new ArgumentNullException(nameof(name)); + if (string.IsNullOrWhiteSpace(name)) + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(name)); Name = name; } @@ -37,6 +37,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations /// public IReadOnlyDictionary Transitions => _transitions; + public IReadOnlyList PostMigrationTypes => _postMigrationTypes; + /// /// Gets the name of the plan. /// @@ -45,12 +47,18 @@ namespace Umbraco.Cms.Infrastructure.Migrations // adds a transition private MigrationPlan Add(string sourceState, string targetState, Type migration) { - if (sourceState == null) throw new ArgumentNullException(nameof(sourceState)); - if (targetState == null) throw new ArgumentNullException(nameof(targetState)); - if (string.IsNullOrWhiteSpace(targetState)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(targetState)); - if (sourceState == targetState) throw new ArgumentException("Source and target state cannot be identical."); - if (migration == null) throw new ArgumentNullException(nameof(migration)); - if (!migration.Implements()) throw new ArgumentException($"Type {migration.Name} does not implement IMigration.", nameof(migration)); + if (sourceState == null) + throw new ArgumentNullException(nameof(sourceState)); + if (targetState == null) + throw new ArgumentNullException(nameof(targetState)); + if (string.IsNullOrWhiteSpace(targetState)) + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(targetState)); + if (sourceState == targetState) + throw new ArgumentException("Source and target state cannot be identical."); + if (migration == null) + throw new ArgumentNullException(nameof(migration)); + if (!migration.Implements()) + throw new ArgumentException($"Type {migration.Name} does not implement IMigration.", nameof(migration)); sourceState = sourceState.Trim(); targetState = targetState.Trim(); @@ -112,7 +120,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations /// The previous target state, which we need to recover from through . /// The new target state. public MigrationPlan ToWithReplace(string recoverState, string targetState) - where TMigrationNew: IMigration + where TMigrationNew : IMigration where TMigrationRecover : IMigration { To(targetState); @@ -139,13 +147,20 @@ namespace Umbraco.Cms.Infrastructure.Migrations /// public MigrationPlan ToWithClone(string startState, string endState, string targetState) { - if (startState == null) throw new ArgumentNullException(nameof(startState)); - if (string.IsNullOrWhiteSpace(startState)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(startState)); - if (endState == null) throw new ArgumentNullException(nameof(endState)); - if (string.IsNullOrWhiteSpace(endState)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(endState)); - if (targetState == null) throw new ArgumentNullException(nameof(targetState)); - if (string.IsNullOrWhiteSpace(targetState)) throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(targetState)); - if (startState == endState) throw new ArgumentException("Start and end states cannot be identical."); + if (startState == null) + throw new ArgumentNullException(nameof(startState)); + if (string.IsNullOrWhiteSpace(startState)) + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(startState)); + if (endState == null) + throw new ArgumentNullException(nameof(endState)); + if (string.IsNullOrWhiteSpace(endState)) + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(endState)); + if (targetState == null) + throw new ArgumentNullException(nameof(targetState)); + if (string.IsNullOrWhiteSpace(targetState)) + throw new ArgumentException("Value can't be empty or consist only of white-space characters.", nameof(targetState)); + if (startState == endState) + throw new ArgumentException("Start and end states cannot be identical."); startState = startState.Trim(); endState = endState.Trim(); @@ -173,15 +188,6 @@ namespace Umbraco.Cms.Infrastructure.Migrations return this; } - /// - /// Prepares post-migrations. - /// - /// - /// This can be overriden to filter, complement, and/or re-order post-migrations. - /// - protected virtual IEnumerable PreparePostMigrations(IEnumerable types) - => types; - /// /// Adds a post-migration to the plan. /// @@ -254,7 +260,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations var verified = new List(); foreach (var transition in _transitions.Values) { - if (transition == null || verified.Contains(transition.SourceState)) continue; + if (transition == null || verified.Contains(transition.SourceState)) + continue; var visited = new List { transition.SourceState }; var nextTransition = _transitions[transition.TargetState]; @@ -275,82 +282,11 @@ namespace Umbraco.Cms.Infrastructure.Migrations /// /// Throws an exception when the initial state is unknown. /// - protected virtual void ThrowOnUnknownInitialState(string state) + public virtual void ThrowOnUnknownInitialState(string state) { throw new InvalidOperationException($"The migration plan does not support migrating from state \"{state}\"."); } - /// - /// Executes the plan. - /// - /// 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, IMigrationBuilder migrationBuilder, ILogger logger, ILoggerFactory loggerFactory) - { - Validate(); - - if (migrationBuilder == null) throw new ArgumentNullException(nameof(migrationBuilder)); - if (logger == null) throw new ArgumentNullException(nameof(logger)); - - logger.LogInformation("Starting '{MigrationName}'...", Name); - - var origState = fromState ?? string.Empty; - - logger.LogInformation("At {OrigState}", string.IsNullOrWhiteSpace(origState) ? "origin": origState); - - if (!_transitions.TryGetValue(origState, out var transition)) - ThrowOnUnknownInitialState(origState); - - var context = new MigrationContext(scope.Database, loggerFactory.CreateLogger()); - context.PostMigrations.AddRange(_postMigrationTypes); - - while (transition != null) - { - logger.LogInformation("Execute {MigrationType}", transition.MigrationType.Name); - - var migration = migrationBuilder.Build(transition.MigrationType, context); - migration.Migrate(); - - var nextState = transition.TargetState; - origState = nextState; - - logger.LogInformation("At {OrigState}", origState); - - // throw a raw exception here: this should never happen as the plan has - // been validated - this is just a paranoid safety test - if (!_transitions.TryGetValue(origState, out transition)) - throw new InvalidOperationException($"Unknown state \"{origState}\"."); - } - - // prepare and de-duplicate post-migrations, only keeping the 1st occurence - var temp = new HashSet(); - var postMigrationTypes = PreparePostMigrations(context.PostMigrations) - .Where(x => !temp.Contains(x)) - .Select(x => { temp.Add(x); return x; }); - - // run post-migrations - foreach (var postMigrationType in postMigrationTypes) - { - logger.LogInformation($"PostMigration: {postMigrationType.FullName}."); - var postMigration = migrationBuilder.Build(postMigrationType, context); - postMigration.Migrate(); - } - - logger.LogInformation("Done (pending scope completion)."); - - // safety check - again, this should never happen as the plan has been validated, - // and this is just a paranoid safety test - if (origState != _finalState) - throw new InvalidOperationException($"Internal error, reached state {origState} which is not final state {_finalState}"); - - return origState; - } - /// /// Follows a path (for tests and debugging). /// diff --git a/src/Umbraco.Core/Packaging/PackageMigrationPlan.cs b/src/Umbraco.Core/Packaging/PackageMigrationPlan.cs new file mode 100644 index 0000000000..114aea8f24 --- /dev/null +++ b/src/Umbraco.Core/Packaging/PackageMigrationPlan.cs @@ -0,0 +1,14 @@ +using System; +using System.Collections.Generic; +using System.Text; +using Umbraco.Cms.Core.Migrations; + +namespace Umbraco.Cms.Core.Packaging +{ + public abstract class PackageMigrationPlan : MigrationPlan + { + protected PackageMigrationPlan(string name) : base(name) + { + } + } +} diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs index 14457e9687..8e230c4963 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.CoreServices.cs @@ -14,6 +14,7 @@ using Umbraco.Cms.Core.Logging.Serilog.Enrichers; using Umbraco.Cms.Core.Mail; using Umbraco.Cms.Core.Manifest; using Umbraco.Cms.Core.Media; +using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Models.PublishedContent; using Umbraco.Cms.Core.Packaging; using Umbraco.Cms.Core.PropertyEditors; @@ -103,6 +104,7 @@ namespace Umbraco.Cms.Infrastructure.DependencyInjection builder.Services.AddUnique(factory => new DefaultShortStringHelper(new DefaultShortStringHelperConfig().WithDefault(factory.GetRequiredService>().Value))); + builder.Services.AddUnique(); builder.Services.AddUnique(factory => new MigrationBuilder(factory)); builder.Services.AddUnique(); diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index 0da54785f2..a9ab97573a 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -4,6 +4,7 @@ using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Hosting; +using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Migrations.Upgrade; @@ -28,6 +29,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install private readonly ILoggerFactory _loggerFactory; private readonly IDbProviderFactoryCreator _dbProviderFactoryCreator; private readonly IConfigManipulator _configManipulator; + private readonly IMigrationPlanExecutor _migrationPlanExecutor; private readonly DatabaseSchemaCreatorFactory _databaseSchemaCreatorFactory; private DatabaseSchemaResult _databaseSchemaValidationResult; @@ -39,25 +41,26 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install IScopeProvider scopeProvider, IUmbracoDatabaseFactory databaseFactory, IRuntimeState runtime, - ILogger logger, ILoggerFactory loggerFactory, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, IHostingEnvironment hostingEnvironment, IDbProviderFactoryCreator dbProviderFactoryCreator, IConfigManipulator configManipulator, + IMigrationPlanExecutor migrationPlanExecutor, DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory) { _scopeProvider = scopeProvider; _databaseFactory = databaseFactory; _runtime = runtime; - _logger = logger; + _logger = loggerFactory.CreateLogger(); _loggerFactory = loggerFactory; _migrationBuilder = migrationBuilder; _keyValueService = keyValueService; _hostingEnvironment = hostingEnvironment; _dbProviderFactoryCreator = dbProviderFactoryCreator; _configManipulator = configManipulator; + _migrationPlanExecutor = migrationPlanExecutor; _databaseSchemaCreatorFactory = databaseSchemaCreatorFactory; } @@ -423,7 +426,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install // upgrade var upgrader = new Upgrader(plan); - upgrader.Execute(_scopeProvider, _migrationBuilder, _keyValueService, _loggerFactory.CreateLogger(), _loggerFactory); + upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService); var message = "

Upgrade completed!

"; diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs new file mode 100644 index 0000000000..8c372ebe72 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -0,0 +1,105 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Migrations; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Extensions; +using Type = System.Type; + +namespace Umbraco.Cms.Infrastructure.Migrations +{ + public class MigrationPlanExecutor : IMigrationPlanExecutor + { + private readonly IScopeProvider _scopeProvider; + private readonly ILoggerFactory _loggerFactory; + private readonly IMigrationBuilder _migrationBuilder; + private readonly ILogger _logger; + + public MigrationPlanExecutor(IScopeProvider scopeProvider, ILoggerFactory loggerFactory, IMigrationBuilder migrationBuilder) + { + _scopeProvider = scopeProvider; + _loggerFactory = loggerFactory; + _migrationBuilder = migrationBuilder; + _logger = _loggerFactory.CreateLogger(); + } + + /// + /// Executes the plan. + /// + /// 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(MigrationPlan plan, string fromState) + { + plan.Validate(); + + _logger.LogInformation("Starting '{MigrationName}'...", plan.Name); + + var origState = fromState ?? string.Empty; + + _logger.LogInformation("At {OrigState}", string.IsNullOrWhiteSpace(origState) ? "origin" : origState); + + if (!plan.Transitions.TryGetValue(origState, out MigrationPlan.Transition transition)) + { + plan.ThrowOnUnknownInitialState(origState); + } + + using (IScope scope = _scopeProvider.CreateScope(autoComplete: true)) + { + var context = new MigrationContext(scope.Database, _loggerFactory.CreateLogger()); + context.PostMigrations.AddRange(plan.PostMigrationTypes); + + while (transition != null) + { + _logger.LogInformation("Execute {MigrationType}", transition.MigrationType.Name); + + var migration = _migrationBuilder.Build(transition.MigrationType, context); + migration.Migrate(); + + var nextState = transition.TargetState; + origState = nextState; + + _logger.LogInformation("At {OrigState}", origState); + + // throw a raw exception here: this should never happen as the plan has + // been validated - this is just a paranoid safety test + if (!plan.Transitions.TryGetValue(origState, out transition)) + { + throw new InvalidOperationException($"Unknown state \"{origState}\"."); + } + } + + // prepare and de-duplicate post-migrations, only keeping the 1st occurence + var temp = new HashSet(); + var postMigrationTypes = context.PostMigrations + .Where(x => !temp.Contains(x)) + .Select(x => { temp.Add(x); return x; }); + + // run post-migrations + foreach (var postMigrationType in postMigrationTypes) + { + _logger.LogInformation($"PostMigration: {postMigrationType.FullName}."); + var postMigration = _migrationBuilder.Build(postMigrationType, context); + postMigration.Migrate(); + } + } + + _logger.LogInformation("Done (pending scope completion)."); + + // safety check - again, this should never happen as the plan has been validated, + // and this is just a paranoid safety test + var finalState = plan.FinalState; + if (origState != finalState) + { + throw new InvalidOperationException($"Internal error, reached state {origState} which is not final state {finalState}"); + } + + return origState; + } + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index 12d8a4afd2..be031d8667 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -1,5 +1,6 @@ using System; using Umbraco.Cms.Core.Configuration; +using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Semver; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.Common; using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_0_0; @@ -85,7 +86,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade } } - protected override void ThrowOnUnknownInitialState(string state) + public override void ThrowOnUnknownInitialState(string state) { if (TryGetInitStateVersion(state, out var initVersion)) { diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs index 58b7f9e07b..8b9060a45d 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs @@ -1,5 +1,5 @@ -using System; -using Microsoft.Extensions.Logging; +using System; +using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; @@ -37,23 +37,16 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade /// Executes. /// /// A scope provider. - /// A migration builder. /// A key-value service. - /// A logger. - /// A logger factory - public void Execute(IScopeProvider scopeProvider, IMigrationBuilder migrationBuilder, IKeyValueService keyValueService, ILogger logger, ILoggerFactory loggerFactory) + public void Execute(IMigrationPlanExecutor migrationPlanExecutor, IScopeProvider scopeProvider, IKeyValueService keyValueService) { 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()) { - BeforeMigrations(scope, logger); - // read current state var currentState = keyValueService.GetValue(StateValueKey); var forceState = false; @@ -65,33 +58,24 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade } // execute plan - var state = plan.Execute(scope, currentState, migrationBuilder, loggerFactory.CreateLogger(), loggerFactory); + var state = migrationPlanExecutor.Execute(plan, currentState); 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); + } else if (currentState != state) + { keyValueService.SetValue(StateValueKey, currentState, state); - - AfterMigrations(scope, logger); + } scope.Complete(); } } - - /// - /// Executes as part of the upgrade scope and before all migrations have executed. - /// - 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, ILogger logger) - { } - } } diff --git a/src/Umbraco.Infrastructure/RuntimeState.cs b/src/Umbraco.Infrastructure/RuntimeState.cs index 894536e4e7..bef9adb76d 100644 --- a/src/Umbraco.Infrastructure/RuntimeState.cs +++ b/src/Umbraco.Infrastructure/RuntimeState.cs @@ -263,22 +263,6 @@ namespace Umbraco.Cms.Core } } - private bool EnsureUmbracoUpgradeState(IUmbracoDatabaseFactory databaseFactory, ILogger logger) - { - var upgrader = new Upgrader(new UmbracoPlan(_umbracoVersion)); - var stateValueKey = upgrader.StateValueKey; - - // no scope, no service - just directly accessing the database - using (var database = databaseFactory.CreateDatabase()) - { - CurrentMigrationState = database.GetFromKeyValueTable(stateValueKey); - FinalMigrationState = upgrader.Plan.FinalState; - } - - logger.LogDebug("Final upgrade state is {FinalMigrationState}, database contains {DatabaseState}", FinalMigrationState, CurrentMigrationState ?? ""); - - return CurrentMigrationState == FinalMigrationState; - } private bool DoesUmbracoRequireUpgrade(IUmbracoDatabase database) { var upgrader = new Upgrader(new UmbracoPlan(_umbracoVersion)); diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs index 5ecf2f5503..27f3ac2c48 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs @@ -9,6 +9,7 @@ using Moq; using NUnit.Framework; using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Migrations; @@ -25,10 +26,9 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations [UmbracoTest(Database = UmbracoTestOptions.Database.NewEmptyPerTest)] public class AdvancedMigrationTests : UmbracoIntegrationTest { - private readonly ILoggerFactory _loggerFactory = NullLoggerFactory.Instance; - private IUmbracoVersion UmbracoVersion => GetRequiredService(); private IEventAggregator EventAggregator => GetRequiredService(); + private IMigrationPlanExecutor MigrationPlanExecutor => GetRequiredService(); [Test] public void CreateTableOfTDto() @@ -53,7 +53,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .From(string.Empty) .To("done")); - upgrader.Execute(ScopeProvider, builder, Mock.Of(), _loggerFactory.CreateLogger(), _loggerFactory); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); var helper = new DatabaseSchemaCreator(scope.Database, LoggerFactory.CreateLogger(), LoggerFactory, UmbracoVersion, EventAggregator); bool exists = helper.TableExists("umbracoUser"); @@ -90,7 +90,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("a") .To("done")); - upgrader.Execute(ScopeProvider, builder, Mock.Of(), _loggerFactory.CreateLogger(), _loggerFactory); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } @@ -125,7 +125,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("b") .To("done")); - upgrader.Execute(ScopeProvider, builder, Mock.Of(), _loggerFactory.CreateLogger(), _loggerFactory); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } @@ -160,7 +160,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("b") .To("done")); - upgrader.Execute(ScopeProvider, builder, Mock.Of(), _loggerFactory.CreateLogger(), _loggerFactory); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } @@ -192,7 +192,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("a") .To("done")); - upgrader.Execute(ScopeProvider, builder, Mock.Of(), _loggerFactory.CreateLogger(), _loggerFactory); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs index c6090a7bc2..35fcf88f2e 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs @@ -57,6 +57,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations } }); + var executor = new MigrationPlanExecutor(scopeProvider, loggerFactory, migrationBuilder); + MigrationPlan plan = new MigrationPlan("default") .From(string.Empty) .To("{4A9A1A8F-0DA1-4BCF-AD06-C19D79152E35}") @@ -72,8 +74,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations // read current state var sourceState = kvs.GetValue("Umbraco.Tests.MigrationPlan") ?? string.Empty; - // execute plan - state = plan.Execute(s, sourceState, migrationBuilder, loggerFactory.CreateLogger(), loggerFactory); + // execute plan + state = executor.Execute(plan, sourceState); // save new state kvs.SetValue("Umbraco.Tests.MigrationPlan", sourceState, state); diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs index 4b1e2aa526..3952dea607 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs @@ -1,4 +1,4 @@ -// Copyright (c) Umbraco. +// Copyright (c) Umbraco. // See LICENSE for more details. using System; @@ -24,6 +24,8 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations public class PostMigrationTests { private static readonly ILoggerFactory s_loggerFactory = NullLoggerFactory.Instance; + private IMigrationPlanExecutor GetMigrationPlanExecutor(IScopeProvider scopeProvider, IMigrationBuilder builder) + => new MigrationPlanExecutor(scopeProvider, s_loggerFactory, builder); [Test] public void ExecutesPlanPostMigration() @@ -63,12 +65,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations TestPostMigration.MigrateCount = 0; var upgrader = new Upgrader(plan); + IMigrationPlanExecutor executor = GetMigrationPlanExecutor(scopeProvider, builder); upgrader.Execute( + executor, scopeProvider, - builder, - Mock.Of(), - s_loggerFactory.CreateLogger(), - s_loggerFactory); + Mock.Of()); Assert.AreEqual(1, TestPostMigration.MigrateCount); } @@ -115,12 +116,11 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations new MigrationContext(database, s_loggerFactory.CreateLogger()); var upgrader = new Upgrader(plan); + IMigrationPlanExecutor executor = GetMigrationPlanExecutor(scopeProvider, builder); upgrader.Execute( + executor, scopeProvider, - builder, - Mock.Of(), - s_loggerFactory.CreateLogger(), - s_loggerFactory); + Mock.Of()); Assert.AreEqual(1, TestMigration.MigrateCount); Assert.AreEqual(1, TestPostMigration.MigrateCount);