Cleanup Migrations

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;

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<Migrations> logger)
         {
             _logger = logger;

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<int> CreateAsync()
         {
             if (_shellDescriptor.WasFeatureAlreadyInstalled("OrchardCore.ContentFields"))

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;
         }
     }

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<int> UpdateFrom3()
+        public async Task<int> 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<int> UpdateFrom4()
+        public async Task<int> UpdateFrom4Async()
         {
             // For backwards compatibility with liquid filters we disable html sanitization on existing field definitions.
             foreach (var contentType in await _contentDefinitionManager.LoadTypeDefinitionsAsync())

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<int> CreateAsync()
         {
             if (_shellDescriptor.WasFeatureAlreadyInstalled("OrchardCore.Media"))
@@ -35,9 +35,7 @@ public async Task<int> CreateAsync()
         }
 
         // Upgrade an existing installation.
-        private async Task UpgradeAsync()
-        {
-            await _contentDefinitionManager.MigrateFieldSettingsAsync<MediaField, MediaFieldSettings>();
-        }
+        private Task UpgradeAsync()
+            => _contentDefinitionManager.MigrateFieldSettingsAsync<MediaField, MediaFieldSettings>();
     }
 }

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;

src/OrchardCore.Modules/OrchardCore.PublishLater/Migrations.cs

@@ -46,7 +46,7 @@ await SchemaBuilder.AlterIndexTableAsync<PublishLaterPartIndex>(table => table
     // This code can be removed in a later version.
     public async Task<int> 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<PublishLaterPartIndex>(table => table

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<int> 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");

src/OrchardCore/OrchardCore.Data.YesSql/Migration/DataMigrationManager.cs

@@ -17,8 +17,8 @@ namespace OrchardCore.Data.Migration
     /// </summary>
     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<IDataMigration> _dataMigrations;
         private readonly ISession _session;
@@ -86,7 +86,7 @@ public async Task<IEnumerable<string>> 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<string> featureIds)
         {
             foreach (var featureId in featureIds)
@@ -186,15 +203,15 @@ 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)
                         {
                             _logger.LogWarning("The migration '{name}' for '{FeatureName}' does not contain a proper Create or CreateAsync method.", migration.GetType().FullName, 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<int> InvokeMethod(MethodInfo method, IDataMigration migration)
+        private static async Task<int> InvokeMethodAsync(MethodInfo method, IDataMigration migration)
         {
             if (method.ReturnType == typeof(Task<int>))
             {
@@ -271,15 +288,15 @@ private static Dictionary<int, MethodInfo> 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<int, MethodInfo> GetUpdateMethod(MethodInfo methodInfo)
+        private static Tuple<int, MethodInfo> GetUpdateFromMethod(MethodInfo methodInfo)
         {
-            if (methodInfo.Name.StartsWith(_updateFromPrefix, StringComparison.Ordinal)
-                && (methodInfo.ReturnType == typeof(int) || methodInfo.ReturnType == typeof(Task<int>)))
+            if (methodInfo.Name.StartsWith(_updateFromPrefix, StringComparison.Ordinal) &&
+                (methodInfo.ReturnType == typeof(int) || methodInfo.ReturnType == typeof(Task<int>)))
             {
                 var version = methodInfo.Name.EndsWith(_asyncSuffix, StringComparison.Ordinal)
                     ? methodInfo.Name.Substring(_updateFromPrefix.Length, methodInfo.Name.Length - _updateFromPrefix.Length - _asyncSuffix.Length)
@@ -295,60 +312,27 @@ private static Tuple<int, MethodInfo> GetUpdateMethod(MethodInfo methodInfo)
         }
 
         /// <summary>
-        /// 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.
         /// </summary>
-        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<int>)))
-            {
-                return methodInfo;
-            }
-
-            methodInfo = dataMigration.GetType().GetMethod("CreateAsync", BindingFlags.Public | BindingFlags.Instance);
-            if (methodInfo != null && methodInfo.ReturnType == typeof(Task<int>))
-            {
-                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);
 
-        /// <summary>
-        /// Returns the Uninstall method from a data migration class if it's found.
-        /// </summary>
-        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<int>)))
             {
                 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<int>))
             {
                 return methodInfo;
             }
 
             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);
-                }
-            }
-        }
     }
 }

src/OrchardCore/OrchardCore.Data/Migration/Records/DataMigrationRecord.cs

@@ -16,7 +16,7 @@ public DataMigrationRecord()
         }
 
         /// <summary>
-        /// Gete or sets the record Id.
+        /// Get or sets the record Id.
         /// </summary>
         public long Id { get; set; }