From 8ede33f6ba651b7f0d1b569a603c36e8a9d3ca3f Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 18 Aug 2021 12:01:56 -0600 Subject: [PATCH] Updates how package migrations are run. Only publish a single notification. --- .../Notifications/MigrationPlanExecuted.cs | 19 --- ...edPackageMigrationsExecutedNotification.cs | 19 --- src/Umbraco.Core/Umbraco.Core.csproj | 1 - .../InstallSteps/DatabaseUpgradeStep.cs | 6 +- .../Install/PackageMigrationRunner.cs | 108 ++++++++++++++++++ .../Install/UnattendedUpgrader.cs | 95 +++------------ .../Migrations/ExecutedMigrationPlan.cs | 18 +++ .../Migrations/IMigrationExpression.cs | 2 +- .../Migrations/IMigrationPlanExecutor.cs | 2 +- .../Migrations/Install/DatabaseBuilder.cs | 4 +- .../Migrations/MigrationPlanExecutor.cs | 12 +- .../MigrationPlansExecutedNotification.cs | 18 +++ .../Migrations/Upgrade/Upgrader.cs | 11 +- .../Migrations/AdvancedMigrationTests.cs | 20 ++-- .../Migrations/MigrationPlanTests.cs | 6 +- .../Migrations/PostMigrationTests.cs | 10 +- .../Controllers/PackageController.cs | 37 +++--- 17 files changed, 210 insertions(+), 178 deletions(-) delete mode 100644 src/Umbraco.Core/Notifications/MigrationPlanExecuted.cs delete mode 100644 src/Umbraco.Core/Notifications/UnattendedPackageMigrationsExecutedNotification.cs create mode 100644 src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs create mode 100644 src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs create mode 100644 src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs diff --git a/src/Umbraco.Core/Notifications/MigrationPlanExecuted.cs b/src/Umbraco.Core/Notifications/MigrationPlanExecuted.cs deleted file mode 100644 index 9db7b0cf61..0000000000 --- a/src/Umbraco.Core/Notifications/MigrationPlanExecuted.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace Umbraco.Cms.Core.Notifications -{ - /// - /// Published when a migration plan has been successfully executed. - /// - public class MigrationPlanExecuted : INotification - { - public MigrationPlanExecuted(string migrationPlanName, string initialState, string finalState) - { - MigrationPlanName = migrationPlanName; - InitialState = initialState; - FinalState = finalState; - } - - public string MigrationPlanName { get; } - public string InitialState { get; } - public string FinalState { get; } - } -} diff --git a/src/Umbraco.Core/Notifications/UnattendedPackageMigrationsExecutedNotification.cs b/src/Umbraco.Core/Notifications/UnattendedPackageMigrationsExecutedNotification.cs deleted file mode 100644 index 42768f6539..0000000000 --- a/src/Umbraco.Core/Notifications/UnattendedPackageMigrationsExecutedNotification.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System.Collections.Generic; - -namespace Umbraco.Cms.Core.Notifications -{ - - /// - /// Published when unattended package migrations have been successfully executed - /// - public class UnattendedPackageMigrationsExecutedNotification : INotification - { - public UnattendedPackageMigrationsExecutedNotification(IReadOnlyList packageMigrations) - => PackageMigrations = packageMigrations; - - /// - /// The list of package migration names that have been executed. - /// - public IReadOnlyList PackageMigrations { get; } - } -} diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj index f43ac533a7..f5f0c4c705 100644 --- a/src/Umbraco.Core/Umbraco.Core.csproj +++ b/src/Umbraco.Core/Umbraco.Core.csproj @@ -61,6 +61,5 @@ - diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs index 3fe5493c37..aba2cdd9f4 100644 --- a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs +++ b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs @@ -40,7 +40,7 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps _connectionStrings = connectionStrings.Value ?? throw new ArgumentNullException(nameof(connectionStrings)); } - public override async Task ExecuteAsync(object model) + public override Task ExecuteAsync(object model) { var installSteps = InstallStatusTracker.GetStatus().ToArray(); var previousStep = installSteps.Single(x => x.Name == "DatabaseInstall"); @@ -53,7 +53,7 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps var plan = new UmbracoPlan(_umbracoVersion); plan.AddPostMigration(); // needed when running installer (back-office) - var result = await _databaseBuilder.UpgradeSchemaAndDataAsync(plan); + var result = _databaseBuilder.UpgradeSchemaAndData(plan); if (result.Success == false) { @@ -61,7 +61,7 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps } } - return null; + return Task.FromResult((InstallSetupResult)null); } public override bool RequiresExecution(object model) diff --git a/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs b/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs new file mode 100644 index 0000000000..54448c68c0 --- /dev/null +++ b/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs @@ -0,0 +1,108 @@ +using System; +using System.Linq; +using System.Collections.Generic; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Logging; +using Umbraco.Cms.Core.Packaging; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Migrations.Upgrade; +using Umbraco.Extensions; +using Umbraco.Cms.Core.Migrations; +using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Infrastructure.Migrations; +using Umbraco.Cms.Infrastructure.Migrations.Notifications; +using Umbraco.Cms.Core; + +namespace Umbraco.Cms.Infrastructure.Install +{ + /// + /// Runs the package migration plans + /// + public class PackageMigrationRunner + { + private readonly IProfilingLogger _profilingLogger; + private readonly IScopeProvider _scopeProvider; + private readonly PendingPackageMigrations _pendingPackageMigrations; + private readonly IMigrationPlanExecutor _migrationPlanExecutor; + private readonly IKeyValueService _keyValueService; + private readonly IEventAggregator _eventAggregator; + private readonly Dictionary _packageMigrationPlans; + + public PackageMigrationRunner( + IProfilingLogger profilingLogger, + IScopeProvider scopeProvider, + PendingPackageMigrations pendingPackageMigrations, + PackageMigrationPlanCollection packageMigrationPlans, + IMigrationPlanExecutor migrationPlanExecutor, + IKeyValueService keyValueService, + IEventAggregator eventAggregator) + { + _profilingLogger = profilingLogger; + _scopeProvider = scopeProvider; + _pendingPackageMigrations = pendingPackageMigrations; + _migrationPlanExecutor = migrationPlanExecutor; + _keyValueService = keyValueService; + _eventAggregator = eventAggregator; + _packageMigrationPlans = packageMigrationPlans.ToDictionary(x => x.Name); + } + + /// + /// Runs all migration plans for a package name if any are pending. + /// + /// + /// + public IEnumerable RunPackageMigrationsIfPending(string packageName) + { + IReadOnlyDictionary keyValues = _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix); + IReadOnlyList pendingMigrations = _pendingPackageMigrations.GetPendingPackageMigrations(keyValues); + + IEnumerable packagePlans = _packageMigrationPlans.Values + .Where(x => x.PackageName.InvariantEquals(packageName)) + .Where(x => pendingMigrations.Contains(x.Name)) + .Select(x => x.Name); + + return RunPackagePlans(packagePlans); + } + + /// + /// Runs the all specified package migration plans and publishes a + /// if all are successful. + /// + /// + /// + /// If any plan fails it will throw an exception. + public IEnumerable RunPackagePlans(IEnumerable plansToRun) + { + var results = new List(); + + // Create an explicit scope around all package migrations so they are + // all executed in a single transaction. If one package migration fails, + // none of them will be committed. This is intended behavior so we can + // ensure when we publish the success notification that is is done when they all succeed. + using (IScope scope = _scopeProvider.CreateScope(autoComplete: true)) + { + foreach (var migrationName in plansToRun) + { + if (!_packageMigrationPlans.TryGetValue(migrationName, out PackageMigrationPlan plan)) + { + throw new InvalidOperationException("Cannot find package migration plan " + migrationName); + } + + using (_profilingLogger.TraceDuration( + "Starting unattended package migration for " + migrationName, + "Unattended upgrade completed for " + migrationName)) + { + var upgrader = new Upgrader(plan); + // This may throw, if so the transaction will be rolled back + results.Add(upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService)); + } + } + } + + var executedPlansNotification = new MigrationPlansExecutedNotification(results); + _eventAggregator.Publish(executedPlansNotification); + + return results; + } + } +} diff --git a/src/Umbraco.Infrastructure/Install/UnattendedUpgrader.cs b/src/Umbraco.Infrastructure/Install/UnattendedUpgrader.cs index a5c1b75218..5f5c8f16a8 100644 --- a/src/Umbraco.Infrastructure/Install/UnattendedUpgrader.cs +++ b/src/Umbraco.Infrastructure/Install/UnattendedUpgrader.cs @@ -1,5 +1,4 @@ using System; -using System.Linq; using System.Collections.Generic; using System.Threading; using System.Threading.Tasks; @@ -8,15 +7,13 @@ using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Exceptions; using Umbraco.Cms.Core.Logging; using Umbraco.Cms.Core.Notifications; -using Umbraco.Cms.Core.Packaging; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Migrations.Install; using Umbraco.Cms.Infrastructure.Migrations.Upgrade; using Umbraco.Cms.Infrastructure.Runtime; using Umbraco.Extensions; -using Umbraco.Cms.Core.Migrations; -using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core; +using Umbraco.Cms.Infrastructure.Migrations; namespace Umbraco.Cms.Infrastructure.Install { @@ -30,35 +27,23 @@ namespace Umbraco.Cms.Infrastructure.Install private readonly IUmbracoVersion _umbracoVersion; private readonly DatabaseBuilder _databaseBuilder; private readonly IRuntimeState _runtimeState; - private readonly PackageMigrationPlanCollection _packageMigrationPlans; - private readonly IMigrationPlanExecutor _migrationPlanExecutor; - private readonly IScopeProvider _scopeProvider; - private readonly IKeyValueService _keyValueService; - private readonly IEventAggregator _eventAggregator; + private readonly PackageMigrationRunner _packageMigrationRunner; public UnattendedUpgrader( IProfilingLogger profilingLogger, IUmbracoVersion umbracoVersion, DatabaseBuilder databaseBuilder, IRuntimeState runtimeState, - PackageMigrationPlanCollection packageMigrationPlans, - IMigrationPlanExecutor migrationPlanExecutor, - IScopeProvider scopeProvider, - IKeyValueService keyValueService, - IEventAggregator eventAggregator) + PackageMigrationRunner packageMigrationRunner) { _profilingLogger = profilingLogger ?? throw new ArgumentNullException(nameof(profilingLogger)); _umbracoVersion = umbracoVersion ?? throw new ArgumentNullException(nameof(umbracoVersion)); _databaseBuilder = databaseBuilder ?? throw new ArgumentNullException(nameof(databaseBuilder)); _runtimeState = runtimeState ?? throw new ArgumentNullException(nameof(runtimeState)); - _packageMigrationPlans = packageMigrationPlans; - _migrationPlanExecutor = migrationPlanExecutor; - _scopeProvider = scopeProvider; - _keyValueService = keyValueService; - _eventAggregator = eventAggregator; + _packageMigrationRunner = packageMigrationRunner; } - public async Task HandleAsync(RuntimeUnattendedUpgradeNotification notification, CancellationToken cancellationToken) + public Task HandleAsync(RuntimeUnattendedUpgradeNotification notification, CancellationToken cancellationToken) { if (_runtimeState.RunUnattendedBootLogic()) { @@ -71,7 +56,7 @@ namespace Umbraco.Cms.Infrastructure.Install "Starting unattended upgrade.", "Unattended upgrade completed.")) { - DatabaseBuilder.Result result = await _databaseBuilder.UpgradeSchemaAndDataAsync(plan); + DatabaseBuilder.Result result = _databaseBuilder.UpgradeSchemaAndData(plan); if (result.Success == false) { var innerException = new UnattendedInstallException("An error occurred while running the unattended upgrade.\n" + result.Message); @@ -95,74 +80,30 @@ namespace Umbraco.Cms.Infrastructure.Install throw new InvalidOperationException("No pending migrations found but the runtime level reason is " + Core.RuntimeLevelReason.UpgradePackageMigrations); } - var exceptions = new List(); - var packageMigrationsPlans = _packageMigrationPlans.ToDictionary(x => x.Name); - - // Create an explicit scope around all package migrations so they are - // all executed in a single transaction. - using (IScope scope = _scopeProvider.CreateScope(autoComplete: true)) + try { - foreach (var migrationName in pendingMigrations) - { - if (!packageMigrationsPlans.TryGetValue(migrationName, out PackageMigrationPlan plan)) - { - throw new InvalidOperationException("Cannot find package migration plan " + migrationName); - } - - using (_profilingLogger.TraceDuration( - "Starting unattended package migration for " + migrationName, - "Unattended upgrade completed for " + migrationName)) - { - var upgrader = new Upgrader(plan); - - try - { - await upgrader.ExecuteAsync(_migrationPlanExecutor, _scopeProvider, _keyValueService); - } - catch (Exception ex) - { - exceptions.Add(new UnattendedInstallException("Unattended package migration failed for " + migrationName, ex)); - } - } - } - } - - if (exceptions.Count > 0) - { - notification.UnattendedUpgradeResult = RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors; - SetRuntimeErrors(exceptions); - } - else - { - var packageMigrationsExecutedNotification = new UnattendedPackageMigrationsExecutedNotification(pendingMigrations); - await _eventAggregator.PublishAsync(packageMigrationsExecutedNotification); - + IEnumerable result = _packageMigrationRunner.RunPackagePlans(pendingMigrations); notification.UnattendedUpgradeResult = RuntimeUnattendedUpgradeNotification.UpgradeResult.PackageMigrationComplete; } + catch (Exception ex ) + { + SetRuntimeError(ex); + notification.UnattendedUpgradeResult = RuntimeUnattendedUpgradeNotification.UpgradeResult.HasErrors; + } } break; default: throw new InvalidOperationException("Invalid reason " + _runtimeState.Reason); } } + + return Task.CompletedTask; } - private void SetRuntimeErrors(List exception) - { - Exception innerException; - if (exception.Count == 1) - { - innerException = exception[0]; - } - else - { - innerException = new AggregateException(exception); - } - - _runtimeState.Configure( + private void SetRuntimeError(Exception exception) + => _runtimeState.Configure( RuntimeLevel.BootFailed, RuntimeLevelReason.BootFailedOnException, - innerException); - } + exception); } } diff --git a/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs b/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs new file mode 100644 index 0000000000..9979da1e40 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs @@ -0,0 +1,18 @@ +using System; + +namespace Umbraco.Cms.Infrastructure.Migrations +{ + public class ExecutedMigrationPlan + { + public ExecutedMigrationPlan(MigrationPlan plan, string initialState, string finalState) + { + Plan = plan; + InitialState = initialState ?? throw new ArgumentNullException(nameof(initialState)); + FinalState = finalState ?? throw new ArgumentNullException(nameof(finalState)); + } + + public MigrationPlan Plan { get; } + public string InitialState { get; } + public string FinalState { get; } + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/IMigrationExpression.cs b/src/Umbraco.Infrastructure/Migrations/IMigrationExpression.cs index 30d3385632..3a5a4649fe 100644 --- a/src/Umbraco.Infrastructure/Migrations/IMigrationExpression.cs +++ b/src/Umbraco.Infrastructure/Migrations/IMigrationExpression.cs @@ -1,4 +1,4 @@ -namespace Umbraco.Cms.Infrastructure.Migrations +namespace Umbraco.Cms.Infrastructure.Migrations { /// /// Marker interface for migration expressions diff --git a/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs index 47de12a3f2..41a831360a 100644 --- a/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs @@ -5,6 +5,6 @@ namespace Umbraco.Cms.Core.Migrations { public interface IMigrationPlanExecutor { - Task ExecuteAsync(MigrationPlan plan, string fromState); + string Execute(MigrationPlan plan, string fromState); } } diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index 6110279442..b7437f4c2d 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -413,7 +413,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install /// configured and it is possible to connect to the database. /// Runs whichever migrations need to run. /// - public async Task UpgradeSchemaAndDataAsync(UmbracoPlan plan) + public Result UpgradeSchemaAndData(UmbracoPlan plan) { try { @@ -427,7 +427,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install // upgrade var upgrader = new Upgrader(plan); - await upgrader.ExecuteAsync(_migrationPlanExecutor, _scopeProvider, _keyValueService); + 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 index b4a7616bc8..09cddbc20b 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -1,11 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Threading.Tasks; using Microsoft.Extensions.Logging; -using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Migrations; -using Umbraco.Cms.Core.Notifications; using Umbraco.Cms.Core.Scoping; using Umbraco.Extensions; using Type = System.Type; @@ -17,19 +14,16 @@ namespace Umbraco.Cms.Infrastructure.Migrations private readonly IScopeProvider _scopeProvider; private readonly ILoggerFactory _loggerFactory; private readonly IMigrationBuilder _migrationBuilder; - private readonly IEventAggregator _eventAggregator; private readonly ILogger _logger; public MigrationPlanExecutor( IScopeProvider scopeProvider, ILoggerFactory loggerFactory, - IMigrationBuilder migrationBuilder, - IEventAggregator eventAggregator) + IMigrationBuilder migrationBuilder) { _scopeProvider = scopeProvider; _loggerFactory = loggerFactory; _migrationBuilder = migrationBuilder; - _eventAggregator = eventAggregator; _logger = _loggerFactory.CreateLogger(); } @@ -43,7 +37,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations /// /// The final state. /// The plan executes within the scope, which must then be completed. - public async Task ExecuteAsync(MigrationPlan plan, string fromState) + public string Execute(MigrationPlan plan, string fromState) { plan.Validate(); @@ -102,8 +96,6 @@ namespace Umbraco.Cms.Infrastructure.Migrations postMigration.Run(); } } - - await _eventAggregator.PublishAsync(new MigrationPlanExecuted(plan.Name, fromState, nextState)); } _logger.LogInformation("Done (pending scope completion)."); diff --git a/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs b/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs new file mode 100644 index 0000000000..50ee5c3582 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs @@ -0,0 +1,18 @@ +using System.Collections.Generic; +using Umbraco.Cms.Core.Notifications; + +namespace Umbraco.Cms.Infrastructure.Migrations.Notifications +{ + /// + /// Published when one or more migration plans have been successfully executed. + /// + public class MigrationPlansExecutedNotification : INotification + { + public MigrationPlansExecutedNotification(IReadOnlyList executedPlans) + => ExecutedPlans = executedPlans; + + public IReadOnlyList ExecutedPlans { get; } + + + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs index 44b9f3c708..afc4fdec81 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs @@ -1,5 +1,4 @@ using System; -using System.Threading.Tasks; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Scoping; @@ -37,7 +36,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade /// /// A scope provider. /// A key-value service. - public async Task ExecuteAsync(IMigrationPlanExecutor migrationPlanExecutor, IScopeProvider scopeProvider, IKeyValueService keyValueService) + public ExecutedMigrationPlan Execute(IMigrationPlanExecutor migrationPlanExecutor, IScopeProvider scopeProvider, IKeyValueService keyValueService) { if (scopeProvider == null) throw new ArgumentNullException(nameof(scopeProvider)); if (keyValueService == null) throw new ArgumentNullException(nameof(keyValueService)); @@ -52,13 +51,13 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade { currentState = Plan.InitialState; forceState = true; - } + } // execute plan - var state = await migrationPlanExecutor.ExecuteAsync(Plan, currentState); + var state = migrationPlanExecutor.Execute(Plan, currentState); if (string.IsNullOrWhiteSpace(state)) { - throw new Exception("Plan execution returned an invalid null or empty state."); + throw new InvalidOperationException("Plan execution returned an invalid null or empty state."); } // save new state @@ -72,6 +71,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade } scope.Complete(); + + return new ExecutedMigrationPlan(Plan, currentState, state); } } } diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs index 0de597d61f..40dbad176e 100644 --- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs +++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs @@ -32,7 +32,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations private IMigrationPlanExecutor MigrationPlanExecutor => GetRequiredService(); [Test] - public async Task CreateTableOfTDto() + public void CreateTableOfTDto() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -54,7 +54,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .From(string.Empty) .To("done")); - await upgrader.ExecuteAsync(MigrationPlanExecutor, ScopeProvider, Mock.Of()); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); var helper = new DatabaseSchemaCreator(scope.Database, LoggerFactory.CreateLogger(), LoggerFactory, UmbracoVersion, EventAggregator); bool exists = helper.TableExists("umbracoUser"); @@ -65,7 +65,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations } [Test] - public async Task DeleteKeysAndIndexesOfTDto() + public void DeleteKeysAndIndexesOfTDto() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -91,13 +91,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("a") .To("done")); - await upgrader.ExecuteAsync(MigrationPlanExecutor, ScopeProvider, Mock.Of()); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } [Test] - public async Task CreateKeysAndIndexesOfTDto() + public void CreateKeysAndIndexesOfTDto() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -126,13 +126,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("b") .To("done")); - await upgrader.ExecuteAsync(MigrationPlanExecutor, ScopeProvider, Mock.Of()); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } [Test] - public async Task CreateKeysAndIndexes() + public void CreateKeysAndIndexes() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -161,13 +161,13 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("b") .To("done")); - await upgrader.ExecuteAsync(MigrationPlanExecutor, ScopeProvider, Mock.Of()); + upgrader.Execute(MigrationPlanExecutor, ScopeProvider, Mock.Of()); scope.Complete(); } } [Test] - public async Task CreateColumn() + public void CreateColumn() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -193,7 +193,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations .To("a") .To("done")); - await upgrader.ExecuteAsync(MigrationPlanExecutor, ScopeProvider, Mock.Of()); + 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 8ee093372e..08bcee255e 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs @@ -29,7 +29,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations public class MigrationPlanTests { [Test] - public async Task CanExecute() + public void CanExecute() { NullLoggerFactory loggerFactory = NullLoggerFactory.Instance; @@ -58,7 +58,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations } }); - var executor = new MigrationPlanExecutor(scopeProvider, loggerFactory, migrationBuilder, Mock.Of()); + var executor = new MigrationPlanExecutor(scopeProvider, loggerFactory, migrationBuilder); MigrationPlan plan = new MigrationPlan("default") .From(string.Empty) @@ -76,7 +76,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations var sourceState = kvs.GetValue("Umbraco.Tests.MigrationPlan") ?? string.Empty; // execute plan - state = await executor.ExecuteAsync(plan, sourceState); + 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 805473cd7f..a61de49ee5 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs @@ -27,10 +27,10 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations { private static readonly ILoggerFactory s_loggerFactory = NullLoggerFactory.Instance; private IMigrationPlanExecutor GetMigrationPlanExecutor(IScopeProvider scopeProvider, IMigrationBuilder builder) - => new MigrationPlanExecutor(scopeProvider, s_loggerFactory, builder, Mock.Of()); + => new MigrationPlanExecutor(scopeProvider, s_loggerFactory, builder); [Test] - public async Task ExecutesPlanPostMigration() + public void ExecutesPlanPostMigration() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -68,7 +68,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations var upgrader = new Upgrader(plan); IMigrationPlanExecutor executor = GetMigrationPlanExecutor(scopeProvider, builder); - await upgrader.ExecuteAsync( + upgrader.Execute( executor, scopeProvider, Mock.Of()); @@ -77,7 +77,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations } [Test] - public async Task MigrationCanAddPostMigration() + public void MigrationCanAddPostMigration() { IMigrationBuilder builder = Mock.Of(); Mock.Get(builder) @@ -119,7 +119,7 @@ namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations var upgrader = new Upgrader(plan); IMigrationPlanExecutor executor = GetMigrationPlanExecutor(scopeProvider, builder); - await upgrader.ExecuteAsync( + upgrader.Execute( executor, scopeProvider, Mock.Of()); diff --git a/src/Umbraco.Web.BackOffice/Controllers/PackageController.cs b/src/Umbraco.Web.BackOffice/Controllers/PackageController.cs index 2e5bb9c93d..f8815a7086 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/PackageController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/PackageController.cs @@ -23,6 +23,7 @@ using Umbraco.Cms.Core.Scoping; using Microsoft.Extensions.Logging; using System.Numerics; using System.Threading.Tasks; +using Umbraco.Cms.Infrastructure.Install; namespace Umbraco.Cms.Web.BackOffice.Controllers { @@ -39,6 +40,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers private readonly PendingPackageMigrations _pendingPackageMigrations; private readonly PackageMigrationPlanCollection _packageMigrationPlans; private readonly IMigrationPlanExecutor _migrationPlanExecutor; + private readonly PackageMigrationRunner _packageMigrationRunner; private readonly IScopeProvider _scopeProvider; private readonly ILogger _logger; @@ -49,6 +51,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers PendingPackageMigrations pendingPackageMigrations, PackageMigrationPlanCollection packageMigrationPlans, IMigrationPlanExecutor migrationPlanExecutor, + PackageMigrationRunner packageMigrationRunner, IScopeProvider scopeProvider, ILogger logger) { @@ -58,6 +61,7 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers _pendingPackageMigrations = pendingPackageMigrations; _packageMigrationPlans = packageMigrationPlans; _migrationPlanExecutor = migrationPlanExecutor; + _packageMigrationRunner = packageMigrationRunner; _scopeProvider = scopeProvider; _logger = logger; } @@ -118,35 +122,24 @@ namespace Umbraco.Cms.Web.BackOffice.Controllers } [HttpPost] - public async Task>> RunMigrations([FromQuery]string packageName) + public ActionResult> RunMigrations([FromQuery]string packageName) { - IReadOnlyDictionary keyValues = _keyValueService.FindByKeyPrefix(Constants.Conventions.Migrations.KeyValuePrefix); - IReadOnlyList pendingMigrations = _pendingPackageMigrations.GetPendingPackageMigrations(keyValues); - foreach(PackageMigrationPlan plan in _packageMigrationPlans.Where(x => x.PackageName.InvariantEquals(packageName))) + try { - if (pendingMigrations.Contains(plan.Name)) - { - var upgrader = new Upgrader(plan); - - try - { - await upgrader.ExecuteAsync(_migrationPlanExecutor, _scopeProvider, _keyValueService); - } - catch (Exception ex) - { - _logger.LogError(ex, "Package migration failed on package {Package} for plan {Plan}", packageName, plan.Name); - - return ValidationErrorResult.CreateNotificationValidationErrorResult( - $"Package migration failed on package {packageName} for plan {plan.Name} with error: {ex.Message}. Check log for full details."); - } - } + _packageMigrationRunner.RunPackageMigrationsIfPending(packageName); + return _packagingService.GetAllInstalledPackages().ToList(); } + catch (Exception ex) + { + _logger.LogError(ex, "Package migration failed on package {Package}", packageName); - return _packagingService.GetAllInstalledPackages().ToList(); + return ValidationErrorResult.CreateNotificationValidationErrorResult( + $"Package migration failed on package {packageName} with error: {ex.Message}. Check log for full details."); + } } [HttpGet] - public IActionResult DownloadCreatedPackage(int id) + public IActionResult DownloadCreatedPackage(int id) { var package = _packagingService.GetCreatedPackageById(id); if (package == null)