diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs index 8c7e1c2f9a..1023789012 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -175,11 +175,24 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor _logger.LogInformation("Done"); + // final state is set to either the transition target state + // or the final completed transition target state if transition is null + // or the original migration state, if no transitions completed + string finalState = fromState; + if (transition is not null) + { + finalState = transition.TargetState; + } + else if (completedTransitions.Any()) + { + finalState = completedTransitions.Last().TargetState; + } + return new ExecutedMigrationPlan { Successful = true, InitialState = fromState, - FinalState = transition?.TargetState ?? completedTransitions.Last().TargetState, + FinalState = finalState, CompletedTransitions = completedTransitions, Plan = plan, }; diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs index 05ddb12e39..35c04e08e0 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs @@ -36,37 +36,37 @@ public class Upgrader /// /// A scope provider. /// A key-value service. - /// public ExecutedMigrationPlan Execute( IMigrationPlanExecutor migrationPlanExecutor, ICoreScopeProvider scopeProvider, IKeyValueService keyValueService) { - if (scopeProvider == null) + if (scopeProvider is null) { throw new ArgumentNullException(nameof(scopeProvider)); } - if (keyValueService == null) + if (keyValueService is null) { throw new ArgumentNullException(nameof(keyValueService)); } - var initialState = GetInitialState(scopeProvider, keyValueService); + string initialState = GetInitialState(scopeProvider, keyValueService); ExecutedMigrationPlan result = migrationPlanExecutor.ExecutePlan(Plan, initialState); - if (string.IsNullOrWhiteSpace(result.FinalState) || result.FinalState == result.InitialState) + // This should never happen, if the final state comes back as null or equal to the initial state + // it means that no transitions was successful, which means it cannot be a successful migration + if (result.Successful && string.IsNullOrWhiteSpace(result.FinalState)) { - // This should never happen, if the final state comes back as null or equal to the initial state - // it means that no transitions was successful, which means it cannot be a successful migration - if (result.Successful) - { - throw new InvalidOperationException("Plan execution returned an invalid null or empty state."); - } + throw new InvalidOperationException("Plan execution returned an invalid null or empty state."); + } - // Otherwise it just means that our migration failed on the first step, which is fine. - // We will skip saving the state since we it's still the same + // Otherwise it just means that our migration failed on the first step, which is fine, + // or there were no pending transitions so nothing changed. + // We will skip saving the state since we it's still the same + if (result.FinalState == result.InitialState) + { return result; } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs index 6fadae55d0..cb857befda 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using NPoco; using NUnit.Framework; using Umbraco.Cms.Core.Configuration; @@ -91,6 +92,26 @@ public class PartialMigrationsTests : UmbracoIntegrationTest }); } + [Test] + public void CanRunMigrationTwice() + { + Upgrader? upgrader = new(new SimpleMigrationPlan()); + Upgrader? upgrader2 = new(new SimpleMigrationPlan()); + var result = upgrader.Execute(MigrationPlanExecutor, ScopeProvider, KeyValueService); + var result2 = upgrader2.Execute(MigrationPlanExecutor, ScopeProvider, KeyValueService); + + Assert.Multiple(() => + { + Assert.True(result.Successful); + Assert.AreEqual("SimpleMigrationPlan_InitialState", result.InitialState); + Assert.AreEqual("SimpleMigrationStep", result.FinalState); + Assert.AreEqual(1, result.CompletedTransitions.Count); + Assert.IsNull(result.Exception); + Assert.True(result2.Successful); + Assert.IsNull(result2.Exception); + }); + } + [Test] public void StateIsOnlySavedIfAMigrationSucceeds() { @@ -307,4 +328,30 @@ internal class TestUmbracoPlan : UmbracoPlan To("b"); To("c"); } +} + +internal class SimpleMigrationPlan : MigrationPlan +{ + public SimpleMigrationPlan() + : base("SimpleMigrationPlan") => DefinePlan(); + + public override string InitialState => "SimpleMigrationPlan_InitialState"; + + private void DefinePlan() + { + MigrationPlan plan = From(InitialState) + .To(nameof(SimpleMigrationStep)); + } +} + +internal class SimpleMigrationStep : MigrationBase +{ + private readonly ILogger _logger; + + public SimpleMigrationStep( + IMigrationContext context, + ILogger logger) + : base(context) => _logger = logger; + + protected override void Migrate() => _logger.LogDebug("Here be migration"); } \ No newline at end of file