From d33fa2c4eee4d9d5652fff761afa2175fe408a87 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 1 May 2024 09:45:57 -0700 Subject: [PATCH 1/4] Cleanup Migrations --- .../OrchardCore.AdminDashboard/Migrations.cs | 4 +- .../Indexing/SQL/Migrations.cs | 1 + .../OrchardCore.ContentFields/Migrations.cs | 2 +- .../Widgets/WidgetMigrations.cs | 5 +- .../OrchardCore.Html/Migrations.cs | 4 +- .../OrchardCore.Media/Migrations.cs | 8 +- .../OrchardCore.Menu/Migrations.cs | 4 +- .../OrchardCore.PublishLater/Migrations.cs | 2 +- .../OrchardCore.Search.Lucene/Migrations.cs | 12 +-- .../Migration/DataMigrationManager.cs | 90 ++++++++----------- .../Migration/Records/DataMigrationRecord.cs | 2 +- 11 files changed, 62 insertions(+), 72 deletions(-) diff --git a/src/OrchardCore.Modules/OrchardCore.AdminDashboard/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.AdminDashboard/Migrations.cs index 2074fd88558..2c04c2e08e4 100644 --- a/src/OrchardCore.Modules/OrchardCore.AdminDashboard/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.AdminDashboard/Migrations.cs @@ -14,7 +14,9 @@ public class Migrations : DataMigration private readonly IContentDefinitionManager _contentDefinitionManager; private readonly IRecipeMigrator _recipeMigrator; - public Migrations(IContentDefinitionManager contentDefinitionManager, IRecipeMigrator recipeMigrator) + public Migrations( + IContentDefinitionManager contentDefinitionManager, + IRecipeMigrator recipeMigrator) { _contentDefinitionManager = contentDefinitionManager; _recipeMigrator = recipeMigrator; diff --git a/src/OrchardCore.Modules/OrchardCore.ContentFields/Indexing/SQL/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.ContentFields/Indexing/SQL/Migrations.cs index a06d99c46b8..19ae0d9a242 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentFields/Indexing/SQL/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentFields/Indexing/SQL/Migrations.cs @@ -12,6 +12,7 @@ namespace OrchardCore.ContentFields.Indexing.SQL public class Migrations : DataMigration { private readonly ILogger _logger; + public Migrations(ILogger logger) { _logger = logger; diff --git a/src/OrchardCore.Modules/OrchardCore.ContentFields/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.ContentFields/Migrations.cs index 5a49971612d..1a45ce0eb1d 100644 --- a/src/OrchardCore.Modules/OrchardCore.ContentFields/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.ContentFields/Migrations.cs @@ -23,7 +23,7 @@ public Migrations( } // New installations don't need to be upgraded, but because there is no initial migration record, - // 'UpgradeAsync' is called in a new 'CreateAsync' but only if the feature was already installed. + // 'UpgradeAsync()' is called in a new 'CreateAsync()' but only if the feature was already installed. public async Task CreateAsync() { if (_shellDescriptor.WasFeatureAlreadyInstalled("OrchardCore.ContentFields")) diff --git a/src/OrchardCore.Modules/OrchardCore.Facebook/Widgets/WidgetMigrations.cs b/src/OrchardCore.Modules/OrchardCore.Facebook/Widgets/WidgetMigrations.cs index 8c876a063f4..f36c0845fac 100644 --- a/src/OrchardCore.Modules/OrchardCore.Facebook/Widgets/WidgetMigrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Facebook/Widgets/WidgetMigrations.cs @@ -15,7 +15,9 @@ public class WidgetMigrations : DataMigration private readonly IRecipeMigrator _recipeMigrator; private readonly IContentDefinitionManager _contentDefinitionManager; - public WidgetMigrations(IRecipeMigrator recipeMigrator, IContentDefinitionManager contentDefinitionManager) + public WidgetMigrations( + IRecipeMigrator recipeMigrator, + IContentDefinitionManager contentDefinitionManager) { _recipeMigrator = recipeMigrator; _contentDefinitionManager = contentDefinitionManager; @@ -28,6 +30,7 @@ await _contentDefinitionManager.AlterPartDefinitionAsync(nameof(FacebookPluginPa .WithDescription("Provides a Facebook plugin part to create Facebook social plugin widgets.")); await _recipeMigrator.ExecuteAsync($"Widgets/migration{RecipesConstants.RecipeExtension}", this); + return 1; } } diff --git a/src/OrchardCore.Modules/OrchardCore.Html/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.Html/Migrations.cs index 72c8004bbad..85d712b40d0 100644 --- a/src/OrchardCore.Modules/OrchardCore.Html/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Html/Migrations.cs @@ -54,7 +54,7 @@ public int UpdateFrom2() } // This code can be removed in a later version. - public async Task UpdateFrom3() + public async Task UpdateFrom3Async() { // Update content type definitions foreach (var contentType in await _contentDefinitionManager.LoadTypeDefinitionsAsync()) @@ -131,7 +131,7 @@ static bool UpdateBody(JsonNode content) } // This code can be removed in a later version. - public async Task UpdateFrom4() + public async Task UpdateFrom4Async() { // For backwards compatibility with liquid filters we disable html sanitization on existing field definitions. foreach (var contentType in await _contentDefinitionManager.LoadTypeDefinitionsAsync()) diff --git a/src/OrchardCore.Modules/OrchardCore.Media/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.Media/Migrations.cs index f1e1f87a8c2..8f365881c0e 100644 --- a/src/OrchardCore.Modules/OrchardCore.Media/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Media/Migrations.cs @@ -22,7 +22,7 @@ public Migrations( } // New installations don't need to be upgraded, but because there is no initial migration record, - // 'UpgradeAsync' is called in a new 'CreateAsync' but only if the feature was already installed. + // 'UpgradeAsync()' is called in a new 'CreateAsync()' but only if the feature was already installed. public async Task CreateAsync() { if (_shellDescriptor.WasFeatureAlreadyInstalled("OrchardCore.Media")) @@ -35,9 +35,7 @@ public async Task CreateAsync() } // Upgrade an existing installation. - private async Task UpgradeAsync() - { - await _contentDefinitionManager.MigrateFieldSettingsAsync(); - } + private Task UpgradeAsync() + => _contentDefinitionManager.MigrateFieldSettingsAsync(); } } diff --git a/src/OrchardCore.Modules/OrchardCore.Menu/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.Menu/Migrations.cs index 9c8165fbafd..e928e11e4af 100644 --- a/src/OrchardCore.Modules/OrchardCore.Menu/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Menu/Migrations.cs @@ -15,7 +15,9 @@ public class Migrations : DataMigration private readonly IRecipeMigrator _recipeMigrator; private readonly ISession _session; - public Migrations(IRecipeMigrator recipeMigrator, ISession session) + public Migrations( + IRecipeMigrator recipeMigrator, + ISession session) { _recipeMigrator = recipeMigrator; _session = session; diff --git a/src/OrchardCore.Modules/OrchardCore.PublishLater/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.PublishLater/Migrations.cs index cf361cb78f9..f227c53faa6 100644 --- a/src/OrchardCore.Modules/OrchardCore.PublishLater/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.PublishLater/Migrations.cs @@ -46,7 +46,7 @@ await SchemaBuilder.AlterIndexTableAsync(table => table // This code can be removed in a later version. public async Task UpdateFrom1Async() { - // The 'ScheduledPublishUtc' column and related index are kept on existing databases, + // The 'ScheduledPublishDateTimeUtc' column and related index are kept on existing databases, // this because dropping an index and altering a column don't work on all providers. await SchemaBuilder.AlterIndexTableAsync(table => table diff --git a/src/OrchardCore.Modules/OrchardCore.Search.Lucene/Migrations.cs b/src/OrchardCore.Modules/OrchardCore.Search.Lucene/Migrations.cs index 9ba4139ff17..691f4397ba3 100644 --- a/src/OrchardCore.Modules/OrchardCore.Search.Lucene/Migrations.cs +++ b/src/OrchardCore.Modules/OrchardCore.Search.Lucene/Migrations.cs @@ -20,14 +20,16 @@ public class Migrations : DataMigration private readonly IContentDefinitionManager _contentDefinitionManager; private readonly ShellDescriptor _shellDescriptor; - public Migrations(IContentDefinitionManager contentDefinitionManager, ShellDescriptor shellDescriptor) + public Migrations( + IContentDefinitionManager contentDefinitionManager, + ShellDescriptor shellDescriptor) { _contentDefinitionManager = contentDefinitionManager; _shellDescriptor = shellDescriptor; } // New installations don't need to be upgraded, but because there is no initial migration record, - // 'UpgradeAsync' is called in a new 'CreateAsync' but only if the feature was already installed. + // 'UpgradeAsync()' is called in a new 'CreateAsync()' but only if the feature was already installed. public async Task CreateAsync() { if (_shellDescriptor.WasFeatureAlreadyInstalled("OrchardCore.Search.Lucene")) @@ -190,10 +192,8 @@ await _contentDefinitionManager.AlterPartDefinitionAsync(partDefinition.Name, pa try { - if (logger.IsEnabled(LogLevel.Debug)) - { - logger.LogDebug("Updating Lucene indices settings and queries"); - } + logger.LogDebug("Updating Lucene indices settings and queries"); + var quotedTableName = dialect.QuoteForTableName(table, session.Store.Configuration.Schema); var quotedContentColumnName = dialect.QuoteForColumnName("Content"); var quotedTypeColumnName = dialect.QuoteForColumnName("Type"); diff --git a/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs b/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs index 3a8530abaa4..8dcbcb5589d 100644 --- a/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs +++ b/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs @@ -17,8 +17,8 @@ namespace OrchardCore.Data.Migration /// public class DataMigrationManager : IDataMigrationManager { - const string _updateFromPrefix = "UpdateFrom"; - const string _asyncSuffix = "Async"; + private const string _updateFromPrefix = "UpdateFrom"; + private const string _asyncSuffix = "Async"; private readonly IEnumerable _dataMigrations; private readonly ISession _session; @@ -86,7 +86,7 @@ public async Task> GetFeaturesThatNeedUpdateAsync() return CreateUpgradeLookupTable(dataMigration).ContainsKey(record.Version.Value); } - return GetCreateMethod(dataMigration) != null; + return GetMethod(dataMigration, "Create") != null; }); return outOfDateMigrations.Select(m => _typeFeatureProvider.GetFeatureForDependency(m.GetType()).Id).ToArray(); @@ -107,11 +107,11 @@ public async Task Uninstall(string feature) // get current version for this migration var dataMigrationRecord = await GetDataMigrationRecordAsync(tempMigration); - var uninstallMethod = GetUninstallMethod(migration); + var uninstallMethod = GetMethod(migration, "Uninstall"); if (uninstallMethod != null) { - await InvokeMethod(uninstallMethod, migration); + await InvokeMethodAsync(uninstallMethod, migration); } if (dataMigrationRecord == null) @@ -123,6 +123,23 @@ public async Task Uninstall(string feature) } } + public async Task UpdateAllFeaturesAsync() + { + var featuresThatNeedUpdate = await GetFeaturesThatNeedUpdateAsync(); + + foreach (var featureId in featuresThatNeedUpdate) + { + try + { + await UpdateAsync(featureId); + } + catch (Exception ex) when (!ex.IsFatal()) + { + _logger.LogError(ex, "Could not run migrations automatically on '{FeatureName}'", featureId); + } + } + } + public async Task UpdateAsync(IEnumerable featureIds) { foreach (var featureId in featureIds) @@ -186,7 +203,7 @@ public async Task UpdateAsync(string featureId) if (current == 0) { // Try to get a Create method. - var createMethod = GetCreateMethod(migration); + var createMethod = GetMethod(migration, "Create"); if (createMethod == null) { @@ -194,7 +211,7 @@ public async Task UpdateAsync(string featureId) continue; } - current = await InvokeMethod(createMethod, migration); + current = await InvokeMethodAsync(createMethod, migration); } var lookupTable = CreateUpgradeLookupTable(migration); @@ -203,7 +220,7 @@ public async Task UpdateAsync(string featureId) { _logger.LogInformation("Applying migration for '{FeatureName}' from version {Version}.", featureId, current); - current = await InvokeMethod(methodInfo, migration); + current = await InvokeMethodAsync(methodInfo, migration); } // If current is 0, it means no upgrade/create method was found or succeeded. @@ -228,7 +245,7 @@ public async Task UpdateAsync(string featureId) } } - private static async Task InvokeMethod(MethodInfo method, IDataMigration migration) + private static async Task InvokeMethodAsync(MethodInfo method, IDataMigration migration) { if (method.ReturnType == typeof(Task)) { @@ -271,15 +288,15 @@ private static Dictionary CreateUpgradeLookupTable(IDataMigrati return dataMigration .GetType() .GetMethods(BindingFlags.Public | BindingFlags.Instance) - .Select(GetUpdateMethod) + .Select(GetUpdateFromMethod) .Where(tuple => tuple != null) .ToDictionary(tuple => tuple.Item1, tuple => tuple.Item2); } - private static Tuple GetUpdateMethod(MethodInfo methodInfo) + private static Tuple GetUpdateFromMethod(MethodInfo methodInfo) { - if (methodInfo.Name.StartsWith(_updateFromPrefix, StringComparison.Ordinal) - && (methodInfo.ReturnType == typeof(int) || methodInfo.ReturnType == typeof(Task))) + if (methodInfo.Name.StartsWith(_updateFromPrefix, StringComparison.Ordinal) && + (methodInfo.ReturnType == typeof(int) || methodInfo.ReturnType == typeof(Task))) { var version = methodInfo.Name.EndsWith(_asyncSuffix, StringComparison.Ordinal) ? methodInfo.Name.Substring(_updateFromPrefix.Length, methodInfo.Name.Length - _updateFromPrefix.Length - _asyncSuffix.Length) @@ -295,37 +312,21 @@ private static Tuple GetUpdateMethod(MethodInfo methodInfo) } /// - /// Returns the Create or CreateAsync method from a data migration class if it's found. + /// Returns the method from a data migration class that match the given name if found. /// - private static MethodInfo GetCreateMethod(IDataMigration dataMigration) + private static MethodInfo GetMethod(IDataMigration dataMigration, string name) { - var methodInfo = dataMigration.GetType().GetMethod("Create", BindingFlags.Public | BindingFlags.Instance); - if (methodInfo != null && (methodInfo.ReturnType == typeof(int) || methodInfo.ReturnType == typeof(Task))) - { - return methodInfo; - } - - methodInfo = dataMigration.GetType().GetMethod("CreateAsync", BindingFlags.Public | BindingFlags.Instance); - if (methodInfo != null && methodInfo.ReturnType == typeof(Task)) - { - return methodInfo; - } - - return null; - } + // First try to find a method that match the given name. (Ex. Create()) + var methodInfo = dataMigration.GetType().GetMethod(name, BindingFlags.Public | BindingFlags.Instance); - /// - /// Returns the Uninstall method from a data migration class if it's found. - /// - private static MethodInfo GetUninstallMethod(IDataMigration dataMigration) - { - var methodInfo = dataMigration.GetType().GetMethod("Uninstall", BindingFlags.Public | BindingFlags.Instance); if (methodInfo != null && (methodInfo.ReturnType == typeof(int) || methodInfo.ReturnType == typeof(Task))) { return methodInfo; } - methodInfo = dataMigration.GetType().GetMethod("UninstallAsync", BindingFlags.Public | BindingFlags.Instance); + // At this point, try to find a method that match the given name and ends with Async. (Ex. CreateAsync()) + methodInfo = dataMigration.GetType().GetMethod(name + _asyncSuffix, BindingFlags.Public | BindingFlags.Instance); + if (methodInfo != null && methodInfo.ReturnType == typeof(Task)) { return methodInfo; @@ -333,22 +334,5 @@ private static MethodInfo GetUninstallMethod(IDataMigration dataMigration) return null; } - - public async Task UpdateAllFeaturesAsync() - { - var featuresThatNeedUpdate = await GetFeaturesThatNeedUpdateAsync(); - - foreach (var featureId in featuresThatNeedUpdate) - { - try - { - await UpdateAsync(featureId); - } - catch (Exception ex) when (!ex.IsFatal()) - { - _logger.LogError(ex, "Could not run migrations automatically on '{FeatureName}'", featureId); - } - } - } } } diff --git a/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs b/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs index b63e8a21342..e8dbb0b4b6e 100644 --- a/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs +++ b/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs @@ -16,7 +16,7 @@ public DataMigrationRecord() } /// - /// Gete or sets the record Id. + /// Get or sets the record Id. /// public long Id { get; set; } From 657bbbb052be016a2bdb691f3f8d175362f17a44 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 1 May 2024 09:49:13 -0700 Subject: [PATCH 2/4] Update src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs --- .../OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs b/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs index 8dcbcb5589d..8c3c09cc00d 100644 --- a/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs +++ b/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs @@ -312,7 +312,7 @@ private static Tuple GetUpdateFromMethod(MethodInfo methodInfo) } /// - /// Returns the method from a data migration class that match the given name if found. + /// Returns the method from a data migration class that matches the given name if found. /// private static MethodInfo GetMethod(IDataMigration dataMigration, string name) { From 12ad55807a9368a78c043039c8c3d4d4318f878b Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 1 May 2024 09:49:31 -0700 Subject: [PATCH 3/4] Update src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs --- .../OrchardCore.Data/Migration/Records/DataMigrationRecord.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs b/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs index e8dbb0b4b6e..69a633d4616 100644 --- a/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs +++ b/src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs @@ -16,7 +16,7 @@ public DataMigrationRecord() } /// - /// Get or sets the record Id. + /// Gets or sets the record Id. /// public long Id { get; set; } From 032f66645bac1b28528aae72e9074c7a01a043f4 Mon Sep 17 00:00:00 2001 From: Mike Alhayek Date: Wed, 1 May 2024 09:52:27 -0700 Subject: [PATCH 4/4] Update src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs --- .../OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs b/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs index 8c3c09cc00d..a9123fbeebc 100644 --- a/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs +++ b/src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs @@ -324,7 +324,7 @@ private static MethodInfo GetMethod(IDataMigration dataMigration, string name) return methodInfo; } - // At this point, try to find a method that match the given name and ends with Async. (Ex. CreateAsync()) + // At this point, try to find a method that matches the given name and ends with Async. (Ex. CreateAsync()) methodInfo = dataMigration.GetType().GetMethod(name + _asyncSuffix, BindingFlags.Public | BindingFlags.Instance); if (methodInfo != null && methodInfo.ReturnType == typeof(Task))