diff --git a/src/Umbraco.Infrastructure/CompatibilitySuppressions.xml b/src/Umbraco.Infrastructure/CompatibilitySuppressions.xml index f48f361244..6062ef76da 100644 --- a/src/Umbraco.Infrastructure/CompatibilitySuppressions.xml +++ b/src/Umbraco.Infrastructure/CompatibilitySuppressions.xml @@ -1,5 +1,33 @@  + + CP0001 + T:Umbraco.Cms.Infrastructure.Migrations.PostMigrations.ClearCsrfCookies + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0001 + T:Umbraco.Cms.Infrastructure.Migrations.PostMigrations.DeleteLogViewerQueryFile + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0001 + T:Umbraco.Cms.Infrastructure.Migrations.PostMigrations.RebuildPublishedSnapshot + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Core.Migrations.IMigrationPlanExecutor.Execute(Umbraco.Cms.Infrastructure.Migrations.MigrationPlan,System.String) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + CP0002 M:Umbraco.Cms.Core.Models.Blocks.BlockGridLayoutItem.get_ForceLeft @@ -28,6 +56,62 @@ lib/net7.0/Umbraco.Infrastructure.dll true + + CP0002 + M:Umbraco.Cms.Infrastructure.Install.PackageMigrationRunner.#ctor(Umbraco.Cms.Core.Logging.IProfilingLogger,Umbraco.Cms.Core.Scoping.ICoreScopeProvider,Umbraco.Cms.Core.Packaging.PendingPackageMigrations,Umbraco.Cms.Core.Packaging.PackageMigrationPlanCollection,Umbraco.Cms.Core.Migrations.IMigrationPlanExecutor,Umbraco.Cms.Core.Services.IKeyValueService,Umbraco.Cms.Core.Events.IEventAggregator) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.IMigrationContext.AddPostMigration``1 + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.Install.DatabaseBuilder.#ctor(Umbraco.Cms.Core.Scoping.ICoreScopeProvider,Umbraco.Cms.Infrastructure.Scoping.IScopeAccessor,Umbraco.Cms.Infrastructure.Persistence.IUmbracoDatabaseFactory,Umbraco.Cms.Core.Services.IRuntimeState,Microsoft.Extensions.Logging.ILoggerFactory,Umbraco.Cms.Core.Services.IKeyValueService,Umbraco.Cms.Infrastructure.Persistence.IDbProviderFactoryCreator,Umbraco.Cms.Core.Configuration.IConfigManipulator,Microsoft.Extensions.Options.IOptionsMonitor{Umbraco.Cms.Core.Configuration.Models.GlobalSettings},Microsoft.Extensions.Options.IOptionsMonitor{Umbraco.Cms.Core.Configuration.Models.ConnectionStrings},Umbraco.Cms.Core.Migrations.IMigrationPlanExecutor,Umbraco.Cms.Infrastructure.Migrations.Install.DatabaseSchemaCreatorFactory,System.Collections.Generic.IEnumerable{Umbraco.Cms.Infrastructure.Persistence.IDatabaseProviderMetadata}) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.MigrationPlan.AddPostMigration``1 + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.MigrationPlan.get_PostMigrationTypes + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.MigrationPlanExecutor.#ctor(Umbraco.Cms.Core.Scoping.ICoreScopeProvider,Umbraco.Cms.Infrastructure.Scoping.IScopeAccessor,Microsoft.Extensions.Logging.ILoggerFactory,Umbraco.Cms.Infrastructure.Migrations.IMigrationBuilder) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.MigrationPlanExecutor.Execute(Umbraco.Cms.Infrastructure.Migrations.MigrationPlan,System.String) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + + + CP0002 + M:Umbraco.Cms.Infrastructure.Migrations.Upgrade.Upgrader.Execute(Umbraco.Cms.Core.Migrations.IMigrationPlanExecutor,Umbraco.Cms.Core.Scoping.IScopeProvider,Umbraco.Cms.Core.Services.IKeyValueService) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + CP0006 M:Umbraco.Cms.Core.Deploy.IGridCellValueConnector.GetValue(Umbraco.Cms.Core.Models.GridValue.GridControl,System.Collections.Generic.ICollection{Umbraco.Cms.Core.Deploy.ArtifactDependency},Umbraco.Cms.Core.Deploy.IContextCache) @@ -42,4 +126,11 @@ lib/net7.0/Umbraco.Infrastructure.dll true + + CP0006 + M:Umbraco.Cms.Core.Migrations.IMigrationPlanExecutor.ExecutePlan(Umbraco.Cms.Infrastructure.Migrations.MigrationPlan,System.String) + lib/net7.0/Umbraco.Infrastructure.dll + lib/net7.0/Umbraco.Infrastructure.dll + true + \ No newline at end of file diff --git a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Installer.cs b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Installer.cs index c3aa291fb7..c63f76b6d8 100644 --- a/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Installer.cs +++ b/src/Umbraco.Infrastructure/DependencyInjection/UmbracoBuilder.Installer.cs @@ -7,6 +7,8 @@ using Umbraco.Cms.Core.Install.Models; using Umbraco.Cms.Core.Telemetry; using Umbraco.Cms.Infrastructure.Install; using Umbraco.Cms.Infrastructure.Install.InstallSteps; +using Umbraco.Cms.Infrastructure.Migrations.Notifications; +using Umbraco.Cms.Infrastructure.Migrations.PostMigrations; namespace Umbraco.Cms.Infrastructure.DependencyInjection; @@ -38,6 +40,8 @@ public static partial class UmbracoBuilderExtensions builder.Services.AddTransient(); + // Add post migration notification handlers + builder.AddNotificationHandler(); return builder; } } diff --git a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs index 4039533fa1..38ad9e178d 100644 --- a/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs +++ b/src/Umbraco.Infrastructure/Install/InstallSteps/DatabaseUpgradeStep.cs @@ -48,7 +48,6 @@ namespace Umbraco.Cms.Infrastructure.Install.InstallSteps _logger.LogInformation("Running 'Upgrade' service"); var plan = new UmbracoPlan(_umbracoVersion); - plan.AddPostMigration(); // needed when running installer (back-office) DatabaseBuilder.Result? result = _databaseBuilder.UpgradeSchemaAndData(plan); diff --git a/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs b/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs index db054b2dc5..9acedf6797 100644 --- a/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs +++ b/src/Umbraco.Infrastructure/Install/PackageMigrationRunner.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Logging; @@ -18,6 +19,7 @@ namespace Umbraco.Cms.Infrastructure.Install; public class PackageMigrationRunner { private readonly IEventAggregator _eventAggregator; + private readonly ILogger _logger; private readonly IKeyValueService _keyValueService; private readonly IMigrationPlanExecutor _migrationPlanExecutor; private readonly Dictionary _packageMigrationPlans; @@ -32,7 +34,8 @@ public class PackageMigrationRunner PackageMigrationPlanCollection packageMigrationPlans, IMigrationPlanExecutor migrationPlanExecutor, IKeyValueService keyValueService, - IEventAggregator eventAggregator) + IEventAggregator eventAggregator, + ILogger logger) { _profilingLogger = profilingLogger; _scopeProvider = scopeProvider; @@ -40,6 +43,7 @@ public class PackageMigrationRunner _migrationPlanExecutor = migrationPlanExecutor; _keyValueService = keyValueService; _eventAggregator = eventAggregator; + _logger = logger; _packageMigrationPlans = packageMigrationPlans.ToDictionary(x => x.Name); } @@ -71,30 +75,27 @@ public class PackageMigrationRunner /// If any plan fails it will throw an exception. public IEnumerable RunPackagePlans(IEnumerable plansToRun) { - var results = new List(); + List results = new(); - // 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 (ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true)) + // We don't create an explicit scope, the Upgrader will handle that depending on the migration type. + // We also run ALL the package migration to completion, even if one fails, my package should not fail if someone else's package does + foreach (var migrationName in plansToRun) { - foreach (var migrationName in plansToRun) + if (_packageMigrationPlans.TryGetValue(migrationName, out PackageMigrationPlan? plan) is false) { - if (!_packageMigrationPlans.TryGetValue(migrationName, out PackageMigrationPlan? plan)) - { - throw new InvalidOperationException("Cannot find package migration plan " + migrationName); - } + // If we can't find the migration plan for a package we'll just log a message and continue. + _logger.LogError("Package migration failed for {migrationName}, was unable to find the migration plan", migrationName); + continue; + } - using (_profilingLogger.TraceDuration( - "Starting unattended package migration for " + migrationName, - "Unattended upgrade completed for " + migrationName)) - { - var upgrader = new Upgrader(plan); + using (_profilingLogger.TraceDuration( + "Starting unattended package migration for " + migrationName, + "Unattended upgrade completed for " + migrationName)) + { + Upgrader upgrader = new(plan); - // This may throw, if so the transaction will be rolled back - results.Add(upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService)); - } + // This may throw, if so the transaction will be rolled back + results.Add(upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService)); } } diff --git a/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs b/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs index 2e99770874..0298939b8f 100644 --- a/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/ExecutedMigrationPlan.cs @@ -9,9 +9,51 @@ public class ExecutedMigrationPlan FinalState = finalState ?? throw new ArgumentNullException(nameof(finalState)); } - public MigrationPlan Plan { get; } + public ExecutedMigrationPlan( + MigrationPlan plan, + string initialState, + string finalState, + bool successful, + IReadOnlyList completedTransitions) + { + Plan = plan; + InitialState = initialState ?? throw new ArgumentNullException(nameof(initialState)); + FinalState = finalState ?? throw new ArgumentNullException(nameof(finalState)); + Successful = successful; + CompletedTransitions = completedTransitions; + } - public string InitialState { get; } + public ExecutedMigrationPlan() + { + } - public string FinalState { get; } + /// + /// The Migration plan itself. + /// + public required MigrationPlan Plan { get; init; } + + /// + /// The initial state the plan started from, is null if the plan started from the beginning. + /// + public required string InitialState { get; init; } + + /// + /// The final state after the migrations has ran. + /// + public required string FinalState { get; init; } + + /// + /// Determines if the migration plan was a success, that is that all migrations ran successfully. + /// + public required bool Successful { get; init; } + + /// + /// The exception that caused the plan to fail. + /// + public Exception? Exception { get; init; } + + /// + /// A collection of all the succeeded transition. + /// + public required IReadOnlyList CompletedTransitions { get; init; } } diff --git a/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs index 6af5af23a3..0001b3381d 100644 --- a/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/IMigrationContext.cs @@ -37,10 +37,4 @@ public interface IMigrationContext /// Gets or sets a value indicating whether an expression is being built. /// bool BuildingExpression { get; set; } - - /// - /// Adds a post-migration. - /// - void AddPostMigration() - where TMigration : MigrationBase; } diff --git a/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs index 552ca21b5e..f9f87f9e00 100644 --- a/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/IMigrationPlanExecutor.cs @@ -4,5 +4,5 @@ namespace Umbraco.Cms.Core.Migrations; public interface IMigrationPlanExecutor { - string Execute(MigrationPlan plan, string fromState); + ExecutedMigrationPlan ExecutePlan(MigrationPlan plan, string fromState); } diff --git a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs index bd3fc6849d..f27d806b21 100644 --- a/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs +++ b/src/Umbraco.Infrastructure/Migrations/Install/DatabaseBuilder.cs @@ -4,11 +4,13 @@ using Microsoft.Extensions.Options; using Umbraco.Cms.Core; using Umbraco.Cms.Core.Configuration; using Umbraco.Cms.Core.Configuration.Models; +using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Install; using Umbraco.Cms.Core.Install.Models; using Umbraco.Cms.Core.Migrations; using Umbraco.Cms.Core.Scoping; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Migrations.Notifications; using Umbraco.Cms.Infrastructure.Migrations.Upgrade; using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Persistence.Dtos; @@ -35,6 +37,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install private readonly IMigrationPlanExecutor _migrationPlanExecutor; private readonly DatabaseSchemaCreatorFactory _databaseSchemaCreatorFactory; private readonly IEnumerable _databaseProviderMetadata; + private readonly IEventAggregator _aggregator; private DatabaseSchemaResult? _databaseSchemaValidationResult; @@ -54,7 +57,8 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install IOptionsMonitor connectionStrings, IMigrationPlanExecutor migrationPlanExecutor, DatabaseSchemaCreatorFactory databaseSchemaCreatorFactory, - IEnumerable databaseProviderMetadata) + IEnumerable databaseProviderMetadata, + IEventAggregator aggregator) { _scopeProvider = scopeProvider; _scopeAccessor = scopeAccessor; @@ -69,6 +73,7 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install _migrationPlanExecutor = migrationPlanExecutor; _databaseSchemaCreatorFactory = databaseSchemaCreatorFactory; _databaseProviderMetadata = databaseProviderMetadata; + _aggregator = aggregator; } #region Status @@ -329,12 +334,17 @@ namespace Umbraco.Cms.Infrastructure.Migrations.Install // upgrade var upgrader = new Upgrader(plan); - upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService); + ExecutedMigrationPlan result = upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService); + + _aggregator.Publish(new UmbracoPlanExecutedNotification { ExecutedPlan = result }); + + // The migration may have failed, it this is the case, we throw the exception now that we've taken care of business. + if (result.Successful is false && result.Exception is not null) + { + return HandleInstallException(result.Exception); + } var message = "

Upgrade completed!

"; - - //now that everything is done, we need to determine the version of SQL server that is executing - _logger.LogInformation("Database configuration status: {DbConfigStatus}", message); return new Result { Message = message, Success = true, Percentage = "100" }; diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs b/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs index a4c4c0c99a..7b9bb1551c 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationBase.cs @@ -85,6 +85,11 @@ public abstract partial class MigrationBase : IDiscoverable /// public IUpdateBuilder Update => BeginBuild(new UpdateBuilder(Context)); + /// + /// If this is set to true, the published cache will be rebuild upon successful completion of the migration. + /// + public bool RebuildCache { get; set; } + /// /// Runs the migration. /// diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs index eaf2eb4f4d..4b1900c27c 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationContext.cs @@ -8,8 +8,6 @@ namespace Umbraco.Cms.Infrastructure.Migrations; /// internal class MigrationContext : IMigrationContext { - private readonly List _postMigrations = new(); - /// /// Initializes a new instance of the class. /// @@ -18,12 +16,8 @@ internal class MigrationContext : IMigrationContext Plan = plan; Database = database ?? throw new ArgumentNullException(nameof(database)); Logger = logger ?? throw new ArgumentNullException(nameof(logger)); - _postMigrations.AddRange(plan.PostMigrationTypes); } - // this is only internally exposed - public IReadOnlyList PostMigrations => _postMigrations; - /// public ILogger Logger { get; } @@ -40,11 +34,4 @@ internal class MigrationContext : IMigrationContext /// public bool BuildingExpression { get; set; } - - /// - public void AddPostMigration() - where TMigration : MigrationBase => - - // just adding - will be de-duplicated when executing - _postMigrations.Add(typeof(TMigration)); } diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs index 68b870bb7c..3be0a01a4f 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlan.cs @@ -7,7 +7,6 @@ namespace Umbraco.Cms.Infrastructure.Migrations; /// public class MigrationPlan { - private readonly List _postMigrationTypes = new(); private readonly Dictionary _transitions = new(StringComparer.InvariantCultureIgnoreCase); private string? _finalState; @@ -45,8 +44,6 @@ public class MigrationPlan /// public IReadOnlyDictionary Transitions => _transitions; - public IReadOnlyList PostMigrationTypes => _postMigrationTypes; - /// /// Gets the name of the plan. /// @@ -293,20 +290,6 @@ public class MigrationPlan return this; } - /// - /// Adds a post-migration to the plan. - /// - public virtual MigrationPlan AddPostMigration() - where TMigration : MigrationBase - { - // TODO: Post migrations are obsolete/irrelevant. Notifications should be used instead. - // The only place we use this is to clear cookies in the installer which could be done - // via notification. Then we can clean up all the code related to post migrations which is - // not insignificant. - _postMigrationTypes.Add(typeof(TMigration)); - return this; - } - /// /// Creates a random, unique state. /// diff --git a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs index caf498132e..da746ac360 100644 --- a/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs +++ b/src/Umbraco.Infrastructure/Migrations/MigrationPlanExecutor.cs @@ -1,118 +1,214 @@ using Microsoft.Extensions.Logging; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Migrations; +using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Scoping; +using Umbraco.Cms.Infrastructure.Persistence; using Umbraco.Cms.Infrastructure.Scoping; +using Umbraco.Extensions; namespace Umbraco.Cms.Infrastructure.Migrations; +/* + * This is what runs our migration plans. + * It's important to note that this was altered to allow for partial migration completions. + * The need for this became apparent when we added support for SQLite. + * The main issue being how SQLites handles altering the schema. + * Long story short, SQLite doesn't support altering columns, + * or adding non-nullable columns with non-trivial types, for instance GUIDs. + * + * The recommended workaround for this is to add a new table, migrate the data, and delete the old one, + * however this causes issues with foreign keys. The recommended approach for this is to disable foreign keys entirely + * when doing the migration. However, foreign keys MUST be disabled outside a transaction. + * This was impossible with our previous migration system since it ALWAYS ran all migrations in a single transaction + * meaning that the transaction will always have been started when your migration is run, so you can't disable FKs. + * + * To now support this the UnscopedMigrationBase was added, which allows you to create a migration with no transaction. + * But this requires each migration to run in its own scope, + * meaning we can't roll back the entire plan, but only a single migration. + * + * Hence the need for partial migration completions. + */ + public class MigrationPlanExecutor : IMigrationPlanExecutor { private readonly ILogger _logger; private readonly ILoggerFactory _loggerFactory; private readonly IMigrationBuilder _migrationBuilder; + private readonly IUmbracoDatabaseFactory _databaseFactory; + private readonly IPublishedSnapshotService _publishedSnapshotService; + private readonly DistributedCache _distributedCache; private readonly IScopeAccessor _scopeAccessor; private readonly ICoreScopeProvider _scopeProvider; + private bool _rebuildCache; public MigrationPlanExecutor( ICoreScopeProvider scopeProvider, IScopeAccessor scopeAccessor, ILoggerFactory loggerFactory, - IMigrationBuilder migrationBuilder) + IMigrationBuilder migrationBuilder, + IUmbracoDatabaseFactory databaseFactory, + IPublishedSnapshotService publishedSnapshotService, + DistributedCache distributedCache) { _scopeProvider = scopeProvider; _scopeAccessor = scopeAccessor; _loggerFactory = loggerFactory; _migrationBuilder = migrationBuilder; + _databaseFactory = databaseFactory; + _publishedSnapshotService = publishedSnapshotService; + _distributedCache = distributedCache; _logger = _loggerFactory.CreateLogger(); } /// /// Executes the plan. /// - /// A scope. + /// The migration plan to be executes. /// 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) + /// ExecutedMigrationPlan containing information about the plan execution, such as completion state and the steps that ran. + /// + /// 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. + /// + public ExecutedMigrationPlan ExecutePlan(MigrationPlan plan, string fromState) { plan.Validate(); - _logger.LogInformation("Starting '{MigrationName}'...", plan.Name); + ExecutedMigrationPlan result = RunMigrationPlan(plan, fromState); - fromState ??= string.Empty; + // If any completed migration requires us to rebuild cache we'll do that. + if (_rebuildCache) + { + RebuildCache(); + } + + return result; + } + + private ExecutedMigrationPlan RunMigrationPlan(MigrationPlan plan, string fromState) + { + _logger.LogInformation("Starting '{MigrationName}'...", plan.Name); var nextState = fromState; _logger.LogInformation("At {OrigState}", string.IsNullOrWhiteSpace(nextState) ? "origin" : nextState); - if (!plan.Transitions.TryGetValue(nextState, out MigrationPlan.Transition? transition)) + if (plan.Transitions.TryGetValue(nextState, out MigrationPlan.Transition? transition) is false) { plan.ThrowOnUnknownInitialState(nextState); } - using (ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true)) + List completedTransitions = new(); + + while (transition is not null) { - // 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 - // executed is listening to service notifications to perform some persistence logic, - // that packages notification handlers may explode because that package isn't fully installed yet. - using (scope.Notifications.Suppress()) + _logger.LogInformation("Execute {MigrationType}", transition.MigrationType.Name); + + try { - var context = new MigrationContext(plan, _scopeAccessor.AmbientScope?.Database, - _loggerFactory.CreateLogger()); - - while (transition != null) + if (transition.MigrationType.IsAssignableTo(typeof(UnscopedMigrationBase))) { - _logger.LogInformation("Execute {MigrationType}", transition.MigrationType.Name); - - MigrationBase migration = _migrationBuilder.Build(transition.MigrationType, context); - migration.Run(); - - nextState = transition.TargetState; - - _logger.LogInformation("At {OrigState}", nextState); - - // 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(nextState, out transition)) - { - throw new InvalidOperationException($"Unknown state \"{nextState}\"."); - } + RunUnscopedMigration(transition.MigrationType, plan); } - - // prepare and de-duplicate post-migrations, only keeping the 1st occurence - var temp = new HashSet(); - IEnumerable postMigrationTypes = context.PostMigrations - .Where(x => !temp.Contains(x)) - .Select(x => - { - temp.Add(x); - return x; - }); - - // run post-migrations - foreach (Type postMigrationType in postMigrationTypes) + else { - _logger.LogInformation($"PostMigration: {postMigrationType.FullName}."); - MigrationBase postMigration = _migrationBuilder.Build(postMigrationType, context); - postMigration.Run(); + RunScopedMigration(transition.MigrationType, plan); } } + catch (Exception exception) + { + _logger.LogError("Plan failed at step {TargetState}", transition.TargetState); + // We have to always return something, so whatever running this has a chance to save the state we got to. + return new ExecutedMigrationPlan + { + Successful = false, + Exception = exception, + InitialState = fromState, + FinalState = transition.SourceState, + CompletedTransitions = completedTransitions, + Plan = plan, + }; + } + + // 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; + + _logger.LogInformation("At {OrigState}", nextState); + + // this should never happen as the plan has been validated - this is just a paranoid safety test + // If it does something is wrong and we'll fail the execution and return an error. + if (plan.Transitions.TryGetValue(nextState, out transition) is false) + { + return new ExecutedMigrationPlan + { + Successful = false, + Exception = new InvalidOperationException($"Unknown state \"{nextState}\"."), + InitialState = fromState, + // We were unable to get the next transition, and we never executed it, so final state is source state. + FinalState = completedTransitions.Last().TargetState, + CompletedTransitions = completedTransitions, + Plan = plan, + }; + } } - _logger.LogInformation("Done (pending scope completion)."); + _logger.LogInformation("Done"); - // 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 (nextState != finalState) + return new ExecutedMigrationPlan { - throw new InvalidOperationException( - $"Internal error, reached state {nextState} which is not final state {finalState}"); - } + Successful = true, + InitialState = fromState, + FinalState = transition?.TargetState ?? completedTransitions.Last().TargetState, + CompletedTransitions = completedTransitions, + Plan = plan, + }; + } - return nextState; + private void RunUnscopedMigration(Type migrationType, MigrationPlan plan) + { + using IUmbracoDatabase database = _databaseFactory.CreateDatabase(); + var context = new MigrationContext(plan, database, _loggerFactory.CreateLogger()); + + RunMigration(migrationType, context); + } + + private void RunScopedMigration(Type migrationType, 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 + // executed is listening to service notifications to perform some persistence logic, + // that packages notification handlers may explode because that package isn't fully installed yet. + using ICoreScope scope = _scopeProvider.CreateCoreScope(); + using (scope.Notifications.Suppress()) + { + var context = new MigrationContext( + plan, + _scopeAccessor.AmbientScope?.Database, + _loggerFactory.CreateLogger()); + + RunMigration(migrationType, context); + + scope.Complete(); + } + } + + private void RunMigration(Type migrationType, MigrationContext context) + { + MigrationBase migration = _migrationBuilder.Build(migrationType, context); + migration.Run(); + + // If the migration requires clearing the cache set the flag, this will automatically only happen if it succeeds + // Otherwise it'll error out before and return. + if (migration.RebuildCache) + { + _rebuildCache = true; + } + } + + private void RebuildCache() + { + _publishedSnapshotService.RebuildAll(); + _distributedCache.RefreshAllPublishedSnapshot(); } } diff --git a/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs b/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs index 22c7e0710d..a227318405 100644 --- a/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs +++ b/src/Umbraco.Infrastructure/Migrations/Notifications/MigrationPlansExecutedNotification.cs @@ -3,8 +3,16 @@ using Umbraco.Cms.Core.Notifications; namespace Umbraco.Cms.Infrastructure.Migrations.Notifications; /// -/// Published when one or more migration plans have been successfully executed. +/// Published when one or more migration plans has been executed. /// +/// +/// +/// Each migration plan may or may not have succeeded, signaled by the Successful property. +/// +/// +/// A failed migration plan may have partially completed, in which case the successful transition are located in the CompletedTransitions collection. +/// +/// public class MigrationPlansExecutedNotification : INotification { public MigrationPlansExecutedNotification(IReadOnlyList executedPlans) diff --git a/src/Umbraco.Infrastructure/Migrations/Notifications/UmbracoPlanExecutedNotification.cs b/src/Umbraco.Infrastructure/Migrations/Notifications/UmbracoPlanExecutedNotification.cs new file mode 100644 index 0000000000..3410af134b --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/Notifications/UmbracoPlanExecutedNotification.cs @@ -0,0 +1,19 @@ +using Umbraco.Cms.Core.Notifications; + +namespace Umbraco.Cms.Infrastructure.Migrations.Notifications; + +/// +/// Published the umbraco migration plan has been executed. +/// +/// +/// +/// The migration plan may or may not have succeeded, signaled by the Successful property. +/// +/// +/// A failed migration plan may have partially completed, in which case the successful transition are located in the CompletedTransitions collection. +/// +/// +public class UmbracoPlanExecutedNotification : INotification +{ + public required ExecutedMigrationPlan ExecutedPlan { get; init; } +} diff --git a/src/Umbraco.Infrastructure/Migrations/PostMigrations/ClearCsrfCookieHandler.cs b/src/Umbraco.Infrastructure/Migrations/PostMigrations/ClearCsrfCookieHandler.cs new file mode 100644 index 0000000000..fc796d66b8 --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/PostMigrations/ClearCsrfCookieHandler.cs @@ -0,0 +1,28 @@ +using Umbraco.Cms.Core; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Web; +using Umbraco.Cms.Infrastructure.Migrations.Notifications; + +namespace Umbraco.Cms.Infrastructure.Migrations.PostMigrations; + +public class ClearCsrfCookieHandler : INotificationHandler +{ + private readonly ICookieManager _cookieManager; + + public ClearCsrfCookieHandler(ICookieManager cookieManager) + { + _cookieManager = cookieManager; + } + + public void Handle(UmbracoPlanExecutedNotification notification) + { + // We'll only clear the cookie if the migration actually succeeded. + if (notification.ExecutedPlan.Successful is false) + { + return; + } + + _cookieManager.ExpireCookie(Constants.Web.AngularCookieName); + _cookieManager.ExpireCookie(Constants.Web.CsrfValidationCookieName); + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/PostMigrations/ClearCsrfCookies.cs b/src/Umbraco.Infrastructure/Migrations/PostMigrations/ClearCsrfCookies.cs deleted file mode 100644 index c991d35f01..0000000000 --- a/src/Umbraco.Infrastructure/Migrations/PostMigrations/ClearCsrfCookies.cs +++ /dev/null @@ -1,21 +0,0 @@ -using Umbraco.Cms.Core; -using Umbraco.Cms.Core.Web; - -namespace Umbraco.Cms.Infrastructure.Migrations.PostMigrations; - -/// -/// Clears Csrf tokens. -/// -public class ClearCsrfCookies : MigrationBase -{ - private readonly ICookieManager _cookieManager; - - public ClearCsrfCookies(IMigrationContext context, ICookieManager cookieManager) - : base(context) => _cookieManager = cookieManager; - - protected override void Migrate() - { - _cookieManager.ExpireCookie(Constants.Web.AngularCookieName); - _cookieManager.ExpireCookie(Constants.Web.CsrfValidationCookieName); - } -} diff --git a/src/Umbraco.Infrastructure/Migrations/PostMigrations/DeleteLogViewerQueryFile.cs b/src/Umbraco.Infrastructure/Migrations/PostMigrations/DeleteLogViewerQueryFile.cs deleted file mode 100644 index a75b01edaf..0000000000 --- a/src/Umbraco.Infrastructure/Migrations/PostMigrations/DeleteLogViewerQueryFile.cs +++ /dev/null @@ -1,30 +0,0 @@ -using Umbraco.Cms.Core.Hosting; - -// using Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_9_0_0; -namespace Umbraco.Cms.Infrastructure.Migrations.PostMigrations; - -/// -/// Deletes the old file that saved log queries -/// -public class DeleteLogViewerQueryFile : MigrationBase -{ - private readonly IHostingEnvironment _hostingEnvironment; - - /// - /// Initializes a new instance of the class. - /// - public DeleteLogViewerQueryFile(IMigrationContext context, IHostingEnvironment hostingEnvironment) - : base(context) => - _hostingEnvironment = hostingEnvironment; - - /// - protected override void Migrate() - { - // var logViewerQueryFile = MigrateLogViewerQueriesFromFileToDb.GetLogViewerQueryFile(_hostingEnvironment); - // - // if(File.Exists(logViewerQueryFile)) - // { - // File.Delete(logViewerQueryFile); - // } - } -} diff --git a/src/Umbraco.Infrastructure/Migrations/PostMigrations/RebuildPublishedSnapshot.cs b/src/Umbraco.Infrastructure/Migrations/PostMigrations/RebuildPublishedSnapshot.cs deleted file mode 100644 index f8f81acd7b..0000000000 --- a/src/Umbraco.Infrastructure/Migrations/PostMigrations/RebuildPublishedSnapshot.cs +++ /dev/null @@ -1,19 +0,0 @@ -namespace Umbraco.Cms.Infrastructure.Migrations.PostMigrations; - -/// -/// Rebuilds the published snapshot. -/// -public class RebuildPublishedSnapshot : MigrationBase -{ - private readonly IPublishedSnapshotRebuilder _rebuilder; - - /// - /// Initializes a new instance of the class. - /// - public RebuildPublishedSnapshot(IMigrationContext context, IPublishedSnapshotRebuilder rebuilder) - : base(context) - => _rebuilder = rebuilder; - - /// - protected override void Migrate() => _rebuilder.Rebuild(); -} diff --git a/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs b/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs new file mode 100644 index 0000000000..79a7f2e8ac --- /dev/null +++ b/src/Umbraco.Infrastructure/Migrations/UnscopedMigrationBase.cs @@ -0,0 +1,14 @@ +namespace Umbraco.Cms.Infrastructure.Migrations; + +/// +/// Base class for creating a migration that does not have a scope provided for it +/// +/// +/// This is just a marker class, and has all the same functionality as the underlying MigrationBase +/// +public abstract class UnscopedMigrationBase : MigrationBase +{ + protected UnscopedMigrationBase(IMigrationContext context) : base(context) + { + } +} diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs index e9babb7d46..359dc6db21 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/UmbracoPlan.cs @@ -43,7 +43,10 @@ public class UmbracoPlan : MigrationPlan /// /// Defines the plan. /// - protected void DefinePlan() + /// + /// This is virtual for testing purposes. + /// + protected virtual void DefinePlan() { // MODIFYING THE PLAN // diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs index e67ed8da43..05ddb12e39 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/Upgrader.cs @@ -31,20 +31,15 @@ public class Upgrader public virtual string StateValueKey => Constants.Conventions.Migrations.KeyValuePrefix + Name; /// - /// Executes. - /// - /// A scope provider. - /// A key-value service. - [Obsolete("Please use the Execute method that accepts an Umbraco.Cms.Core.Scoping.ICoreScopeProvider instead.")] - public ExecutedMigrationPlan Execute(IMigrationPlanExecutor migrationPlanExecutor, IScopeProvider scopeProvider, IKeyValueService keyValueService) - => Execute(migrationPlanExecutor, (ICoreScopeProvider)scopeProvider, keyValueService); - - /// /// Executes. /// + /// /// A scope provider. /// A key-value service. - public ExecutedMigrationPlan Execute(IMigrationPlanExecutor migrationPlanExecutor, ICoreScopeProvider scopeProvider, + /// + public ExecutedMigrationPlan Execute( + IMigrationPlanExecutor migrationPlanExecutor, + ICoreScopeProvider scopeProvider, IKeyValueService keyValueService) { if (scopeProvider == null) @@ -57,38 +52,49 @@ public class Upgrader throw new ArgumentNullException(nameof(keyValueService)); } - using (ICoreScope scope = scopeProvider.CreateCoreScope()) + var initialState = GetInitialState(scopeProvider, keyValueService); + + ExecutedMigrationPlan result = migrationPlanExecutor.ExecutePlan(Plan, initialState); + + if (string.IsNullOrWhiteSpace(result.FinalState) || result.FinalState == result.InitialState) { - // read current state - var currentState = keyValueService.GetValue(StateValueKey); - var forceState = false; - - if (currentState == null || Plan.IgnoreCurrentState) - { - currentState = Plan.InitialState; - forceState = true; - } - - // execute plan - var state = migrationPlanExecutor.Execute(Plan, currentState); - if (string.IsNullOrWhiteSpace(state)) + // 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."); } - // save new state - if (forceState) - { - keyValueService.SetValue(StateValueKey, state); - } - else if (currentState != state) - { - keyValueService.SetValue(StateValueKey, currentState, state); - } - - scope.Complete(); - - return new ExecutedMigrationPlan(Plan, currentState, 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 + 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; + } + + private string GetInitialState(ICoreScopeProvider scopeProvider, IKeyValueService keyValueService) + { + using ICoreScope scope = scopeProvider.CreateCoreScope(); + var currentState = keyValueService.GetValue(StateValueKey); + scope.Complete(); + + if (currentState is null || Plan.IgnoreCurrentState) + { + return Plan.InitialState; + } + + return currentState; + } + + private void SetState(string state, ICoreScopeProvider scopeProvider, IKeyValueService keyValueService) + { + using ICoreScope scope = scopeProvider.CreateCoreScope(); + keyValueService.SetValue(StateValueKey, state); + scope.Complete(); } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/MigrateDataTypeConfigurations.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/MigrateDataTypeConfigurations.cs index 199cef5f39..3037497fe3 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/MigrateDataTypeConfigurations.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_13_0_0/MigrateDataTypeConfigurations.cs @@ -32,7 +32,6 @@ public class MigrateDataTypeConfigurations : MigrationBase List dataTypeDtos = Database.Fetch(sql); - var refreshCache = false; foreach (DataTypeDto dataTypeDto in dataTypeDtos) { Dictionary configurationData = dataTypeDto.Configuration.IsNullOrWhiteSpace() @@ -64,14 +63,9 @@ public class MigrateDataTypeConfigurations : MigrationBase { dataTypeDto.Configuration = serializer.Serialize(configurationData); Database.Update(dataTypeDto); - refreshCache = true; + RebuildCache = true; } } - - if (refreshCache) - { - Context.AddPostMigration(); - } } // convert the stored keys "minimum" and "maximum" to the expected keys "min" and "max for multiple textstrings diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/DropDownPropertyEditorsMigration.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/DropDownPropertyEditorsMigration.cs index b8c438924d..d8ca98e454 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/DropDownPropertyEditorsMigration.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/DropDownPropertyEditorsMigration.cs @@ -45,10 +45,7 @@ public class DropDownPropertyEditorsMigration : PropertyEditorsMigrationBase // if some data types have been updated directly in the database (editing DataTypeDto and/or PropertyDataDto), // bypassing the services, then we need to rebuild the cache entirely, including the umbracoContentNu table - if (refreshCache) - { - Context.AddPostMigration(); - } + // This has been removed since this migration should be deleted. } private bool Migrate(IEnumerable dataTypes) diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/RadioAndCheckboxPropertyEditorsMigration.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/RadioAndCheckboxPropertyEditorsMigration.cs index 1d0c66e788..851ea698df 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/RadioAndCheckboxPropertyEditorsMigration.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_0/RadioAndCheckboxPropertyEditorsMigration.cs @@ -50,10 +50,7 @@ public class RadioAndCheckboxPropertyEditorsMigration : PropertyEditorsMigration // if some data types have been updated directly in the database (editing DataTypeDto and/or PropertyDataDto), // bypassing the services, then we need to rebuild the cache entirely, including the umbracoContentNu table - if (refreshCache) - { - Context.AddPostMigration(); - } + // This has been removed since this migration should be deleted. } private bool Migrate(IEnumerable dataTypes, bool isMultiple) diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_1/ChangeNuCacheJsonFormat.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_1/ChangeNuCacheJsonFormat.cs index 83d97d9331..b9dcdfa395 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_1/ChangeNuCacheJsonFormat.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_8_0_1/ChangeNuCacheJsonFormat.cs @@ -1,3 +1,4 @@ +using HtmlAgilityPack; using Umbraco.Cms.Infrastructure.Migrations.PostMigrations; namespace Umbraco.Cms.Infrastructure.Migrations.Upgrade.V_8_0_1; @@ -10,8 +11,8 @@ public class ChangeNuCacheJsonFormat : MigrationBase { } - protected override void Migrate() => - - // nothing - just adding the post-migration - Context.AddPostMigration(); + protected override void Migrate() + { + // This has been removed since post migrations are no longer a thing, and this migration should be deleted. + } } diff --git a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MigrateLogViewerQueriesFromFileToDb.cs b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MigrateLogViewerQueriesFromFileToDb.cs index b9e6123c15..68b34ae8fb 100644 --- a/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MigrateLogViewerQueriesFromFileToDb.cs +++ b/src/Umbraco.Infrastructure/Migrations/Upgrade/V_9_0_0/MigrateLogViewerQueriesFromFileToDb.cs @@ -101,7 +101,5 @@ public class MigrateLogViewerQueriesFromFileToDb : MigrationBase } Database.InsertBulk(logQueriesInFile!); - - Context.AddPostMigration(); } } diff --git a/src/Umbraco.New.Cms.Infrastructure/Installer/Steps/DatabaseUpgradeStep.cs b/src/Umbraco.New.Cms.Infrastructure/Installer/Steps/DatabaseUpgradeStep.cs index 83cae8d80b..d0763f58dd 100644 --- a/src/Umbraco.New.Cms.Infrastructure/Installer/Steps/DatabaseUpgradeStep.cs +++ b/src/Umbraco.New.Cms.Infrastructure/Installer/Steps/DatabaseUpgradeStep.cs @@ -42,7 +42,7 @@ public class DatabaseUpgradeStep : IInstallStep, IUpgradeStep _logger.LogInformation("Running 'Upgrade' service"); var plan = new UmbracoPlan(_umbracoVersion); - plan.AddPostMigration(); // needed when running installer (back-office) + // TODO: Clear CSRF cookies with notification. DatabaseBuilder.Result? result = _databaseBuilder.UpgradeSchemaAndData(plan); diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs new file mode 100644 index 0000000000..6fadae55d0 --- /dev/null +++ b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Migrations/PartialMigrationsTests.cs @@ -0,0 +1,310 @@ +using Microsoft.Extensions.DependencyInjection; +using NPoco; +using NUnit.Framework; +using Umbraco.Cms.Core.Configuration; +using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.Exceptions; +using Umbraco.Cms.Core.Migrations; +using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Infrastructure.Migrations; +using Umbraco.Cms.Infrastructure.Migrations.Install; +using Umbraco.Cms.Infrastructure.Migrations.Notifications; +using Umbraco.Cms.Infrastructure.Migrations.Upgrade; +using Umbraco.Cms.Infrastructure.Persistence; +using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations; +using Umbraco.Cms.Infrastructure.Scoping; +using Umbraco.Cms.Tests.Common.Testing; +using Umbraco.Cms.Tests.Integration.Testing; + +namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Migrations; + +// These tests depend on the key-value table, so we need a schema to run these tests. +[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)] +[TestFixture] +public class PartialMigrationsTests : UmbracoIntegrationTest +{ + public const string TableName = "testTable"; + public const string ColumnName = "testColumn"; + + private IMigrationPlanExecutor MigrationPlanExecutor => GetRequiredService(); + + private IKeyValueService KeyValueService => GetRequiredService(); + + [TearDown] + public void ResetMigration() + { + ErrorMigration.ShouldExplode = true; + UmbracoPlanExecutedTestNotificationHandler.HandleNotification = null; + } + + protected override void ConfigureTestServices(IServiceCollection services) + => services.AddNotificationHandler(); + + [Test] + public void CanRerunPartiallyCompletedMigration() + { + var plan = new MigrationPlan("test") + .From(string.Empty) + .To("a") + .To("b") + .To("c"); + + var upgrader = new Upgrader(plan); + + var result = upgrader.Execute(MigrationPlanExecutor, ScopeProvider, KeyValueService); + + Assert.Multiple(() => + { + Assert.IsFalse(result.Successful); + Assert.AreEqual(string.Empty, result.InitialState); + Assert.AreEqual("a", result.FinalState); + Assert.AreEqual(1, result.CompletedTransitions.Count); + Assert.IsNotNull(result.Exception); + + // Ensure that the partial success is saved in the keyvalue service so next plan execution starts correctly. + using var scope = ScopeProvider.CreateScope(autoComplete: true); + Assert.AreEqual("a", KeyValueService.GetValue(upgrader.StateValueKey)); + // Ensure that the changes from the first migration is persisted + Assert.IsTrue(scope.Database.HasTable(TableName)); + // But that the final migration wasn't run + Assert.IsFalse(ColumnExists(TableName, ColumnName, scope)); + }); + + // Now let's simulate that someone came along and fixed the broken migration and we'll now try and rerun + ErrorMigration.ShouldExplode = false; + upgrader = new Upgrader(plan); + result = upgrader.Execute(MigrationPlanExecutor, ScopeProvider, KeyValueService); + + Assert.Multiple(() => + { + Assert.AreEqual("a", result.InitialState); + Assert.IsTrue(result.Successful); + Assert.IsNull(result.Exception); + Assert.AreEqual(2, result.CompletedTransitions.Count); + Assert.AreEqual("c", result.FinalState); + + // Ensure that everything got updated in the database. + using var scope = ScopeProvider.CreateScope(autoComplete: true); + Assert.AreEqual("c", KeyValueService.GetValue(upgrader.StateValueKey)); + Assert.IsTrue(scope.Database.HasTable(TableName)); + Assert.IsTrue(ColumnExists(TableName, ColumnName, scope)); + }); + } + + [Test] + public void StateIsOnlySavedIfAMigrationSucceeds() + { + var plan = new MigrationPlan("test") + .From(string.Empty) + .To("a") + .To("b"); + + var upgrader = new Upgrader(plan); + var result = upgrader.Execute(MigrationPlanExecutor, ScopeProvider, KeyValueService); + + Assert.Multiple(() => + { + Assert.IsFalse(result.Successful); + Assert.IsNotNull(result.Exception); + Assert.IsInstanceOf(result.Exception); + Assert.IsEmpty(result.CompletedTransitions); + Assert.AreEqual(string.Empty, result.InitialState); + Assert.AreEqual(string.Empty, result.FinalState); + + using var scope = ScopeProvider.CreateCoreScope(); + Assert.IsNull(KeyValueService.GetValue(upgrader.StateValueKey)); + }); + } + + [Test] + public void ScopesAreCreatedIfNecessary() + { + // The migrations have assert to esnure scopes + var plan = new MigrationPlan("test") + .From(string.Empty) + .To("a") + .To("b"); + + var upgrader = new Upgrader(plan); + var result = upgrader.Execute(MigrationPlanExecutor, ScopeProvider, KeyValueService); + + Assert.IsTrue(result.Successful); + Assert.AreEqual(2, result.CompletedTransitions.Count); + Assert.AreEqual("b", result.FinalState); + } + + [Test] + [TestCase(true)] + [TestCase(false)] + public void UmbracoPlanExecutedNotificationIsAlwaysPublished(bool shouldSucceed) + { + var notificationPublished = false; + ErrorMigration.ShouldExplode = shouldSucceed is false; + + UmbracoPlanExecutedTestNotificationHandler.HandleNotification += notification => + { + notificationPublished = true; + Assert.Multiple(() => + { + var executedPlan = notification.ExecutedPlan; + + if (shouldSucceed) + { + Assert.IsTrue(executedPlan.Successful); + Assert.IsNull(executedPlan.Exception); + Assert.AreEqual("c", executedPlan.FinalState); + Assert.AreEqual(3, executedPlan.CompletedTransitions.Count); + } + else + { + Assert.IsFalse(executedPlan.Successful); + Assert.IsNotNull(executedPlan.Exception); + Assert.IsInstanceOf(executedPlan.Exception); + Assert.AreEqual("a", executedPlan.FinalState); + Assert.AreEqual(1, executedPlan.CompletedTransitions.Count); + } + }); + }; + + // We have to use the DatabaseBuilder otherwise the notification isn't published + var databaseBuilder = GetRequiredService(); + var plan = new TestUmbracoPlan(null!); + databaseBuilder.UpgradeSchemaAndData(plan); + + Assert.IsTrue(notificationPublished); + } + + private bool ColumnExists(string tableName, string columnName, IScope scope) => + scope.Database.SqlContext.SqlSyntax.GetColumnsInSchema(scope.Database) + .Any(x => x.TableName.Equals(tableName) && x.ColumnName.Equals(columnName)); +} + + +// This is just some basic migrations to test the migration plans... +internal class ErrorMigration : MigrationBase +{ + // Used to determine if an exception should be thrown, used to test re-running migrations + public static bool ShouldExplode { get; set; } = true; + + public ErrorMigration(IMigrationContext context) : base(context) + { + } + + protected override void Migrate() + { + if (ShouldExplode) + { + throw new PanicException(); + } + } +} + +internal class CreateTableMigration : MigrationBase +{ + public CreateTableMigration(IMigrationContext context) : base(context) + { + } + + protected override void Migrate() => Create.Table().Do(); +} + +internal class AddColumnMigration : MigrationBase +{ + public AddColumnMigration(IMigrationContext context) : base(context) + { + } + + protected override void Migrate() => Create + .Column(PartialMigrationsTests.ColumnName) + .OnTable(PartialMigrationsTests.TableName) + .AsString() + .Do(); +} + +internal class AssertScopeUnscopedTestMigration : UnscopedMigrationBase +{ + private readonly IScopeProvider _scopeProvider; + private readonly IScopeAccessor _scopeAccessor; + + public AssertScopeUnscopedTestMigration( + IMigrationContext context, + IScopeProvider scopeProvider, + IScopeAccessor scopeAccessor) : base(context) + { + _scopeProvider = scopeProvider; + _scopeAccessor = scopeAccessor; + } + + protected override void Migrate() + { + // Since this is a scopeless migration both ambient scope and the parent scope should be null + Assert.IsNull(_scopeAccessor.AmbientScope); + + using var scope = _scopeProvider.CreateScope(); + Assert.IsNull(((Scope)scope).ParentScope); + } +} + +internal class AsserScopeScopedTestMigration : MigrationBase +{ + private readonly IScopeProvider _scopeProvider; + private readonly IScopeAccessor _scopeAccessor; + + public AsserScopeScopedTestMigration( + IMigrationContext context, + IScopeProvider scopeProvider, + IScopeAccessor scopeAccessor) : base(context) + { + _scopeProvider = scopeProvider; + _scopeAccessor = scopeAccessor; + } + + protected override void Migrate() + { + Assert.IsNotNull(_scopeAccessor.AmbientScope); + + using var scope = _scopeProvider.CreateScope(); + + Assert.IsNotNull(((Scope)scope).ParentScope); + } +} + +[TableName(PartialMigrationsTests.TableName)] +[PrimaryKey("id", AutoIncrement = true)] +internal class TestDto +{ + [Column("id")] + [PrimaryKeyColumn(Name = "PK_testTable")] + public int Id { get; set; } +} + +internal class UmbracoPlanExecutedTestNotificationHandler : INotificationHandler +{ + public static Action? HandleNotification { get; set; } + + public void Handle(UmbracoPlanExecutedNotification notification) => HandleNotification?.Invoke(notification); +} + + +/// +/// This is a fake UmbracoPlan used for testing of the DatabaseBuilder, this overrides everything to be of type +/// UmbracoPlan but behave like a normal migration plan. +/// +internal class TestUmbracoPlan : UmbracoPlan +{ + public TestUmbracoPlan(IUmbracoVersion umbracoVersion) : base(umbracoVersion) + { + } + + public override string InitialState => string.Empty; + + public override bool IgnoreCurrentState => true; + + protected override void DefinePlan() + { + From(InitialState); + To("a"); + To("b"); + To("c"); + } +} \ 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 8349974ebd..59d6539b4f 100644 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs +++ b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/MigrationPlanTests.cs @@ -9,9 +9,12 @@ using Microsoft.Extensions.Options; using Moq; using NPoco; using NUnit.Framework; +using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Configuration.Models; using Umbraco.Cms.Core.Events; +using Umbraco.Cms.Core.PublishedCache; using Umbraco.Cms.Core.Services; +using Umbraco.Cms.Core.Sync; using Umbraco.Cms.Infrastructure.Migrations; using Umbraco.Cms.Infrastructure.Migrations.Upgrade; using Umbraco.Cms.Infrastructure.Persistence; @@ -37,6 +40,11 @@ public class MigrationPlanTests .Setup(x => x.Database) .Returns(database); + var databaseFactory = Mock.Of(); + Mock.Get(databaseFactory) + .Setup(x => x.CreateDatabase()) + .Returns(database); + var sqlContext = new SqlContext( new SqlServerSyntaxProvider(Options.Create(new GlobalSettings())), DatabaseType.SQLCe, @@ -59,7 +67,11 @@ public class MigrationPlanTests } }); - var executor = new MigrationPlanExecutor(scopeProvider, scopeProvider, loggerFactory, migrationBuilder); + var distributedCache = new DistributedCache( + Mock.Of(), + new CacheRefresherCollection(() => Enumerable.Empty())); + + var executor = new MigrationPlanExecutor(scopeProvider, scopeProvider, loggerFactory, migrationBuilder, databaseFactory, Mock.Of(), distributedCache); var plan = new MigrationPlan("default") .From(string.Empty) @@ -77,7 +89,8 @@ public class MigrationPlanTests var sourceState = kvs.GetValue("Umbraco.Tests.MigrationPlan") ?? string.Empty; // execute plan - state = executor.Execute(plan, sourceState); + var result = executor.ExecutePlan(plan, sourceState); + state = result.FinalState; // save new state kvs.SetValue("Umbraco.Tests.MigrationPlan", sourceState, state); diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs deleted file mode 100644 index fdae9cda06..0000000000 --- a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/Migrations/PostMigrationTests.cs +++ /dev/null @@ -1,164 +0,0 @@ -// Copyright (c) Umbraco. -// See LICENSE for more details. - -using System; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Options; -using Moq; -using NPoco; -using NUnit.Framework; -using Umbraco.Cms.Core.Configuration.Models; -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; -using Umbraco.Cms.Infrastructure.Migrations.Upgrade; -using Umbraco.Cms.Infrastructure.Persistence; -using Umbraco.Cms.Infrastructure.Scoping; -using Umbraco.Cms.Persistence.SqlServer.Services; -using Umbraco.Cms.Tests.Common.TestHelpers; -using IScope = Umbraco.Cms.Infrastructure.Scoping.IScope; - -namespace Umbraco.Cms.Tests.UnitTests.Umbraco.Infrastructure.Migrations; - -[TestFixture] -public class PostMigrationTests -{ - private static readonly ILoggerFactory s_loggerFactory = NullLoggerFactory.Instance; - - private IMigrationPlanExecutor GetMigrationPlanExecutor( - ICoreScopeProvider scopeProvider, - IScopeAccessor scopeAccessor, - IMigrationBuilder builder) - => new MigrationPlanExecutor(scopeProvider, scopeAccessor, s_loggerFactory, builder); - - [Test] - public void ExecutesPlanPostMigration() - { - var builder = Mock.Of(); - Mock.Get(builder) - .Setup(x => x.Build(It.IsAny(), It.IsAny())) - .Returns((t, c) => - { - switch (t.Name) - { - case nameof(NoopMigration): - return new NoopMigration(c); - case nameof(TestPostMigration): - return new TestPostMigration(c); - default: - throw new NotSupportedException(); - } - }); - - var database = new TestDatabase(); - var scope = Mock.Of(x => x.Notifications == Mock.Of()); - Mock.Get(scope) - .Setup(x => x.Database) - .Returns(database); - - var sqlContext = new SqlContext( - new SqlServerSyntaxProvider(Options.Create(new GlobalSettings())), - DatabaseType.SQLCe, - Mock.Of()); - var scopeProvider = new MigrationTests.TestScopeProvider(scope) { SqlContext = sqlContext }; - - var plan = new MigrationPlan("Test") - .From(string.Empty).To("done"); - - plan.AddPostMigration(); - TestPostMigration.MigrateCount = 0; - - var upgrader = new Upgrader(plan); - var executor = GetMigrationPlanExecutor(scopeProvider, scopeProvider, builder); - upgrader.Execute( - executor, - scopeProvider, - Mock.Of()); - - Assert.AreEqual(1, TestPostMigration.MigrateCount); - } - - [Test] - public void MigrationCanAddPostMigration() - { - var builder = Mock.Of(); - Mock.Get(builder) - .Setup(x => x.Build(It.IsAny(), It.IsAny())) - .Returns((t, c) => - { - switch (t.Name) - { - case nameof(NoopMigration): - return new NoopMigration(c); - case nameof(TestMigration): - return new TestMigration(c); - case nameof(TestPostMigration): - return new TestPostMigration(c); - default: - throw new NotSupportedException(); - } - }); - - var database = new TestDatabase(); - var scope = Mock.Of(x => x.Notifications == Mock.Of()); - Mock.Get(scope) - .Setup(x => x.Database) - .Returns(database); - - var sqlContext = new SqlContext( - new SqlServerSyntaxProvider(Options.Create(new GlobalSettings())), - DatabaseType.SQLCe, - Mock.Of()); - var scopeProvider = new MigrationTests.TestScopeProvider(scope) { SqlContext = sqlContext }; - - var plan = new MigrationPlan("Test") - .From(string.Empty).To("done"); - - TestMigration.MigrateCount = 0; - TestPostMigration.MigrateCount = 0; - - new MigrationContext(plan, database, s_loggerFactory.CreateLogger()); - - var upgrader = new Upgrader(plan); - var executor = GetMigrationPlanExecutor(scopeProvider, scopeProvider, builder); - upgrader.Execute( - executor, - scopeProvider, - Mock.Of()); - - Assert.AreEqual(1, TestMigration.MigrateCount); - Assert.AreEqual(1, TestPostMigration.MigrateCount); - } - - public class TestMigration : MigrationBase - { - public TestMigration(IMigrationContext context) - : base(context) - { - } - - public static int MigrateCount { get; set; } - - protected override void Migrate() - { - MigrateCount++; - - Context.AddPostMigration(); - } - } - - public class TestPostMigration : MigrationBase - { - public TestPostMigration(IMigrationContext context) - : base(context) - { - } - - public static int MigrateCount { get; set; } - - protected override void Migrate() => MigrateCount++; - } -}