diff --git a/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs index 5e0766755a..cf68617315 100644 --- a/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs @@ -44,4 +44,8 @@ public interface IMigrationContext [Obsolete("This will be removed in the V13, and replaced with a RebuildCache flag on the MigrationBase")] void AddPostMigration() where TMigration : MigrationBase; + + bool IsCompleted { get; } + + void Complete(); } diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs index f753707770..4af60ece42 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs @@ -1,4 +1,6 @@ using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence; namespace Umbraco.Cms.Infrastructure.Migrations; @@ -8,13 +10,15 @@ namespace Umbraco.Cms.Infrastructure.Migrations; /// internal class MigrationContext : IMigrationContext { + private readonly Action? _onCompleteAction; private readonly List _postMigrations = new(); /// /// Initializes a new instance of the class. /// - public MigrationContext(MigrationPlan plan, IUmbracoDatabase? database, ILogger logger) + public MigrationContext(MigrationPlan plan, IUmbracoDatabase? database, ILogger logger, Action? onCompleteAction = null) { + _onCompleteAction = onCompleteAction; Plan = plan; Database = database ?? throw new ArgumentNullException(nameof(database)); Logger = logger ?? throw new ArgumentNullException(nameof(logger)); @@ -41,6 +45,19 @@ internal class MigrationContext : IMigrationContext /// public bool BuildingExpression { get; set; } + public bool IsCompleted { get; private set; } = false; + + public void Complete() + { + if (IsCompleted) + { + return; + } + + _onCompleteAction?.Invoke(); + + IsCompleted = true; + } /// [Obsolete("This will be removed in the V13, and replaced with a RebuildCache flag on the MigrationBase, and a UmbracoPlanExecutedNotification.")] diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs index a3646ed5c9..31a9a3fa5f 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -1,10 +1,12 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.DependencyInjection; using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Scoping; using Umbraco.Extensions; @@ -39,6 +41,7 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor private readonly IMigrationBuilder _migrationBuilder; private readonly IUmbracoDatabaseFactory _databaseFactory; private readonly IPublishedSnapshotService _publishedSnapshotService; + private readonly IKeyValueService _keyValueService; private readonly DistributedCache _distributedCache; private readonly IScopeAccessor _scopeAccessor; private readonly ICoreScopeProvider _scopeProvider; @@ -51,7 +54,8 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor IMigrationBuilder migrationBuilder, IUmbracoDatabaseFactory databaseFactory, IPublishedSnapshotService publishedSnapshotService, - DistributedCache distributedCache) + DistributedCache distributedCache, + IKeyValueService keyValueService) { _scopeProvider = scopeProvider; _scopeAccessor = scopeAccessor; @@ -59,11 +63,33 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor _migrationBuilder = migrationBuilder; _databaseFactory = databaseFactory; _publishedSnapshotService = publishedSnapshotService; + _keyValueService = keyValueService; _distributedCache = distributedCache; _logger = _loggerFactory.CreateLogger(); } - [Obsolete("Use constructor with 7 parameters")] + [Obsolete("Use non-obsolete constructor. This will be removed in Umbraco 15.")] + public MigrationPlanExecutor( + ICoreScopeProvider scopeProvider, + IScopeAccessor scopeAccessor, + ILoggerFactory loggerFactory, + IMigrationBuilder migrationBuilder, + IUmbracoDatabaseFactory databaseFactory, + IPublishedSnapshotService publishedSnapshotService, + DistributedCache distributedCache) + : this( + scopeProvider, + scopeAccessor, + loggerFactory, + migrationBuilder, + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService()) + { + } + + [Obsolete("Use non-obsolete constructor. This will be removed in Umbraco 15.")] public MigrationPlanExecutor( ICoreScopeProvider scopeProvider, IScopeAccessor scopeAccessor, @@ -76,7 +102,9 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor migrationBuilder, StaticServiceProvider.Instance.GetRequiredService(), StaticServiceProvider.Instance.GetRequiredService(), - StaticServiceProvider.Instance.GetRequiredService()) + StaticServiceProvider.Instance.GetRequiredService(), + StaticServiceProvider.Instance.GetRequiredService() + ) { } @@ -92,7 +120,6 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor /// Each migration in the plan, may or may not run in a scope depending on the type of plan. /// A plan can complete partially, the changes of each completed migration will be saved. /// - [Obsolete("This will return an ExecutedMigrationPlan in V13")] public ExecutedMigrationPlan ExecutePlan(MigrationPlan plan, string fromState) { plan.Validate(); @@ -104,6 +131,7 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor // If any completed migration requires us to rebuild cache we'll do that. if (_rebuildCache) { + _logger.LogInformation("Starts rebuilding the cache. This can be a long running operation"); RebuildCache(); } @@ -160,11 +188,11 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor { if (transition.MigrationType.IsAssignableTo(typeof(UnscopedMigrationBase))) { - executedMigrationContexts.Add(RunUnscopedMigration(transition.MigrationType, plan)); + executedMigrationContexts.Add(RunUnscopedMigration(transition, plan)); } else { - executedMigrationContexts.Add(RunScopedMigration(transition.MigrationType, plan)); + executedMigrationContexts.Add(RunScopedMigration(transition, plan)); } } catch (Exception exception) @@ -184,6 +212,13 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor }; } + + IEnumerable nonCompletedMigrationsContexts = executedMigrationContexts.Where(x => x.IsCompleted is false); + if (nonCompletedMigrationsContexts.Any()) + { + throw new InvalidOperationException($"Migration ({transition.MigrationType.FullName}) has been executed without indicated it was completed correctly."); + } + // The plan migration (transition), completed, so we'll add this to our list so we can return this at some point. completedTransitions.Add(transition); nextState = transition.TargetState; @@ -233,17 +268,22 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor }; } - private MigrationContext RunUnscopedMigration(Type migrationType, MigrationPlan plan) + private MigrationContext RunUnscopedMigration(MigrationPlan.Transition transition, MigrationPlan plan) { using IUmbracoDatabase database = _databaseFactory.CreateDatabase(); - var context = new MigrationContext(plan, database, _loggerFactory.CreateLogger()); + var context = new MigrationContext(plan, database, _loggerFactory.CreateLogger(), () => OnComplete(plan, transition.TargetState)); - RunMigration(migrationType, context); + RunMigration(transition.MigrationType, context); return context; } - private MigrationContext RunScopedMigration(Type migrationType, MigrationPlan plan) + private void OnComplete(MigrationPlan plan, string targetState) + { + _keyValueService.SetValue(Constants.Conventions.Migrations.KeyValuePrefix + plan.Name, targetState); + } + + private MigrationContext RunScopedMigration(MigrationPlan.Transition transition, MigrationPlan plan) { // We want to suppress scope (service, etc...) notifications during a migration plan // execution. This is because if a package that doesn't have their migration plan @@ -255,9 +295,13 @@ public class MigrationPlanExecutor : IMigrationPlanExecutor var context = new MigrationContext( plan, _scopeAccessor.AmbientScope?.Database, - _loggerFactory.CreateLogger()); + _loggerFactory.CreateLogger(), + () => OnComplete(plan, transition.TargetState)); - RunMigration(migrationType, context); + RunMigration(transition.MigrationType, context); + + // Ensure we mark the context as complete before the scope completes + context.Complete(); scope.Complete(); diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs index 35c04e08e0..b73967f400 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs @@ -70,10 +70,6 @@ public class Upgrader return result; } - // We always save the final state of the migration plan, this is because a partial success is possible - // So we still want to save the place we got to in the database- - SetState(result.FinalState, scopeProvider, keyValueService); - return result; } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUserGroups.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUserGroups.cs index bc4b287eae..ad8e5091ea 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUserGroups.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUserGroups.cs @@ -57,6 +57,8 @@ public class AddGuidsToUserGroups : UnscopedMigrationBase Database.Update(userGroup); } + Context.Complete(); + scope.Complete(); } @@ -87,6 +89,8 @@ public class AddGuidsToUserGroups : UnscopedMigrationBase // Now that keys are disabled and we have a transaction, we'll do our migration. MigrateColumnSqlite(); + + Context.Complete(); scope.Complete(); } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs index 0995acfeeb..e5edce8e87 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddGuidsToUsers.cs @@ -33,11 +33,13 @@ internal class AddGuidsToUsers : UnscopedMigrationBase if (DatabaseType != DatabaseType.SQLite) { MigrateSqlServer(); + Context.Complete(); scope.Complete(); return; } MigrateSqlite(); + Context.Complete(); scope.Complete(); } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddListViewKeysToDocumentTypes.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddListViewKeysToDocumentTypes.cs index de54432c73..452160d25b 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddListViewKeysToDocumentTypes.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/AddListViewKeysToDocumentTypes.cs @@ -31,11 +31,13 @@ public class AddListViewKeysToDocumentTypes : UnscopedMigrationBase if (DatabaseType != DatabaseType.SQLite) { MigrateSqlServer(); + Context.Complete(); scope.Complete(); return; } MigrateSqlite(); + Context.Complete(); scope.Complete(); } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/MigrateTours.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/MigrateTours.cs index fedfd43c51..302d49e763 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/MigrateTours.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_14_0_0/MigrateTours.cs @@ -79,11 +79,13 @@ internal class MigrateTours : UnscopedMigrationBase if (DatabaseType != DatabaseType.SQLite) { MigrateUserTableSqlServer(); + Context.Complete(); scope.Complete(); return; } MigrateUserTableSqlite(); + Context.Complete(); scope.Complete(); } diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs index 792f40ac2e..084efb2371 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/AdvancedMigrationTests.cs @@ -2,16 +2,22 @@ // See LICENSE for more details. using System.Linq; +using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Migrations; +using Umbraco.Cms.Core.PublishedCache; +using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; using Umbraco.Cms.Infrastructure.Migrations; using Umbraco.Cms.Infrastructure.Migrations.Install; using Umbraco.Cms.Infrastructure.Migrations.Upgrade; +using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.Dtos; +using Umbraco.Cms.Infrastructure.Scoping; using Umbraco.Cms.Tests.Common.Testing; using Umbraco.Cms.Tests.Integration.Testing; @@ -23,7 +29,22 @@ public class AdvancedMigrationTests : UmbracoIntegrationTest { private IUmbracoVersion UmbracoVersion => GetRequiredService(); private IEventAggregator EventAggregator => GetRequiredService(); - private IMigrationPlanExecutor MigrationPlanExecutor => GetRequiredService(); + private ICoreScopeProvider CoreScopeProvider => GetRequiredService(); + private IScopeAccessor ScopeAccessor => GetRequiredService(); + private ILoggerFactory LoggerFactory => GetRequiredService(); + private IMigrationBuilder MigrationBuilder => GetRequiredService(); + private IUmbracoDatabaseFactory UmbracoDatabaseFactory => GetRequiredService(); + private IPublishedSnapshotService PublishedSnapshotService => GetRequiredService(); + private DistributedCache DistributedCache => GetRequiredService(); + private IMigrationPlanExecutor MigrationPlanExecutor => new MigrationPlanExecutor( + CoreScopeProvider, + ScopeAccessor, + LoggerFactory, + MigrationBuilder, + UmbracoDatabaseFactory, + PublishedSnapshotService, + DistributedCache, + Mock.Of()); [Test] public void CreateTableOfTDto() diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/Expressions/Create/Expressions/CreateTableExpressionTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/Expressions/Create/Expressions/CreateTableExpressionTests.cs index ff508c8f32..9f44122f20 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/Expressions/Create/Expressions/CreateTableExpressionTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/Expressions/Create/Expressions/CreateTableExpressionTests.cs @@ -40,7 +40,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations.Expres .WithColumn("bar").AsInt32().PrimaryKey("PK_foo") .Do(); - // (TableName, ColumnName, ConstraintName) + // (TableName, ColumnName, ConstraintName) var constraint = database.SqlContext.SqlSyntax.GetConstraintsPerColumn(database).Single(); Assert.Multiple(() => @@ -92,7 +92,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations.Expres .ForeignKey("MY_SUPER_COOL_FK", "foo", "bar") .Do(); - // (TableName, ColumnName, ConstraintName) + // (TableName, ColumnName, ConstraintName) var constraint = database.SqlContext.SqlSyntax .GetConstraintsPerColumn(database) .Single(x => x.Item3 == "MY_SUPER_COOL_FK"); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs index cb857befda..1fb0e78417 100644 --- a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs @@ -263,6 +263,8 @@ internal class AssertScopeUnscopedTestMigration : UnscopedMigrationBase using var scope = _scopeProvider.CreateScope(); Assert.IsNull(((Scope)scope).ParentScope); + + Context.Complete(); } } @@ -354,4 +356,4 @@ internal class SimpleMigrationStep : MigrationBase : base(context) => _logger = logger; protected override void Migrate() => _logger.LogDebug("Here be migration"); -} \ No newline at end of file +} diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs index 8956fe4315..fd279b5b3a 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs @@ -76,7 +76,7 @@ public class MigrationPlanTests loggerFactory, migrationBuilder, databaseFactory, - Mock.Of(), distributedCache); + Mock.Of(), distributedCache, Mock.Of()); var plan = new MigrationPlan("default") .From(string.Empty)