From c5a17408d5df58be66cc69e0a0a0a1b791d0f37c Mon Sep 17 00:00:00 2001 From: "Erik A. Brandstadmoen" Date: Mon, 22 Jul 2024 23:12:13 +0200 Subject: [PATCH] Bug #557: Fixed new (similar) bug if grate schemas/tables exist with different casing for PostgreSQL and MariaDB> (#560) There was a regression, a similar bug to #245 which was introduced when using SQL scripts to handle the grate table structure versioning (grate the grate), in release 1.7.0. This should fix it. Fixes #557 --- src/grate.core/Migration/GrateMigrator.cs | 10 +- .../up/00_02_fix_version_table_casing.sql | 2 + .../up/00_03_fix_scriptsrun_table_casing.sql | 2 + ...0_03_fix_scriptsrunerrors_table_casing.sql | 2 + .../up/00_01_fix_schema_casing.sql | 7 + .../up/00_02_fix_version_table_casing.sql | 7 + .../up/00_03_fix_scriptsrun_table_casing.sql | 7 + ...0_03_fix_scriptsrunerrors_table_casing.sql | 7 + ...n_Grate_structure_is_not_latest_version.cs | 6 +- ...n_Grate_structure_is_not_latest_version.cs | 160 +++++++++++++++++- 10 files changed, 199 insertions(+), 11 deletions(-) create mode 100644 src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql create mode 100644 src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql create mode 100644 src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql create mode 100644 src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_01_fix_schema_casing.sql create mode 100644 src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql create mode 100644 src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql create mode 100644 src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql diff --git a/src/grate.core/Migration/GrateMigrator.cs b/src/grate.core/Migration/GrateMigrator.cs index 741435a3..acccddfc 100644 --- a/src/grate.core/Migration/GrateMigrator.cs +++ b/src/grate.core/Migration/GrateMigrator.cs @@ -610,7 +610,10 @@ private async Task GetBootstrapInternalGrateConfiguration(st [ "ScriptsRunTable=GrateScriptsRun", "ScriptsRunErrorsTable=GrateScriptsRunErrors", - "VersionTable=GrateVersion" + "VersionTable=GrateVersion", + "ScriptsRunTableLowerCase=grateversion", + "ScriptsRunErrorsTableLowerCase=gratescriptrunerrors", + "VersionTableLowerCase=grateversion" ], DeferWritingToRunTables = true, Environment = GrateEnvironment.InternalBootstrap, @@ -664,7 +667,10 @@ private async Task GetInternalGrateConfiguration(string inte UserTokens = [ $"ScriptsRunTable={thisConfig.ScriptsRunTableName}", $"ScriptsRunErrorsTable={thisConfig.ScriptsRunErrorsTableName}", - $"VersionTable={thisConfig.VersionTableName}" + $"VersionTable={thisConfig.VersionTableName}", + $"ScriptsRunTableLowerCase={thisConfig.ScriptsRunTableName.ToLower()}", + $"ScriptsRunErrorsTableLowerCase={thisConfig.ScriptsRunErrorsTableName.ToLower()}", + $"VersionTableLowerCase={thisConfig.VersionTableName.ToLower()}" ], Environment = GrateEnvironment.Internal diff --git a/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql b/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql new file mode 100644 index 00000000..4aa8a755 --- /dev/null +++ b/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql @@ -0,0 +1,2 @@ +ALTER TABLE IF EXISTS `{{SchemaName}}_{{VersionTableLowerCase}}` +RENAME TO `{{SchemaName}}_{{VersionTable}}` \ No newline at end of file diff --git a/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql b/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql new file mode 100644 index 00000000..5cb2ba83 --- /dev/null +++ b/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql @@ -0,0 +1,2 @@ +ALTER TABLE IF EXISTS `{{SchemaName}}_{{ScriptsRunTableLowerCase}}` +RENAME TO `{{SchemaName}}_{{ScriptsRunTable}}` \ No newline at end of file diff --git a/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql b/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql new file mode 100644 index 00000000..be103f71 --- /dev/null +++ b/src/grate.mariadb/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql @@ -0,0 +1,2 @@ +ALTER TABLE IF EXISTS `{{SchemaName}}_{{ScriptsRunErrorsTableLowerCase}}` +RENAME TO `{{SchemaName}}_{{ScriptsRunErrorsTable}}` \ No newline at end of file diff --git a/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_01_fix_schema_casing.sql b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_01_fix_schema_casing.sql new file mode 100644 index 00000000..226d46ac --- /dev/null +++ b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_01_fix_schema_casing.sql @@ -0,0 +1,7 @@ +DO LANGUAGE plpgsql +$$ +BEGIN + ALTER SCHEMA {{SchemaName}} RENAME TO "{{SchemaName}}"; +EXCEPTION WHEN duplicate_schema or invalid_schema_name THEN +END; +$$; \ No newline at end of file diff --git a/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql new file mode 100644 index 00000000..0346bb23 --- /dev/null +++ b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_02_fix_version_table_casing.sql @@ -0,0 +1,7 @@ +DO LANGUAGE plpgsql +$$ +BEGIN + ALTER TABLE "{{SchemaName}}"."{{VersionTableLowerCase}}" RENAME TO "{{VersionTable}}"; +EXCEPTION WHEN undefined_table or duplicate_table THEN +END; +$$; \ No newline at end of file diff --git a/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql new file mode 100644 index 00000000..6edc2368 --- /dev/null +++ b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrun_table_casing.sql @@ -0,0 +1,7 @@ +DO LANGUAGE plpgsql +$$ +BEGIN + ALTER TABLE "{{SchemaName}}"."{{ScriptsRunTableLowerCase}}" RENAME TO "{{ScriptsRunTable}}"; +EXCEPTION WHEN undefined_table or duplicate_table THEN +END; +$$; \ No newline at end of file diff --git a/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql new file mode 100644 index 00000000..999b30d6 --- /dev/null +++ b/src/grate.postgresql/Bootstrapping/Sql/GrateStructure/up/00_03_fix_scriptsrunerrors_table_casing.sql @@ -0,0 +1,7 @@ +DO LANGUAGE plpgsql +$$ +BEGIN + ALTER TABLE "{{SchemaName}}"."{{ScriptsRunErrorsTableLowerCase}}" RENAME TO "{{ScriptsRunErrorsTable}}"; +EXCEPTION WHEN undefined_table or duplicate_table THEN +END; +$$; \ No newline at end of file diff --git a/unittests/Sqlite/Bootstrapping/When_Grate_structure_is_not_latest_version.cs b/unittests/Sqlite/Bootstrapping/When_Grate_structure_is_not_latest_version.cs index e5766d89..b9b62f55 100644 --- a/unittests/Sqlite/Bootstrapping/When_Grate_structure_is_not_latest_version.cs +++ b/unittests/Sqlite/Bootstrapping/When_Grate_structure_is_not_latest_version.cs @@ -1,4 +1,5 @@ -using Sqlite.TestInfrastructure; +using grate.Configuration; +using Sqlite.TestInfrastructure; using TestCommon.TestInfrastructure; namespace Sqlite.Bootstrapping; @@ -9,7 +10,6 @@ namespace Sqlite.Bootstrapping; public class When_Grate_structure_is_not_latest_version(SqliteGrateTestContext context, ITestOutputHelper testOutput) : TestCommon.Generic.Bootstrapping.When_Grate_structure_is_not_latest_version(context, testOutput) { - [Fact(Skip = "Not able to apply logic to DDL statements for Sqlite")] - public override Task The_latest_version_is_applied() => Task.CompletedTask; + public override Task The_latest_version_is_applied(string versionTableName) => Task.CompletedTask; } diff --git a/unittests/TestCommon/Generic/Bootstrapping/When_Grate_structure_is_not_latest_version.cs b/unittests/TestCommon/Generic/Bootstrapping/When_Grate_structure_is_not_latest_version.cs index 06b99995..ae9e1d84 100644 --- a/unittests/TestCommon/Generic/Bootstrapping/When_Grate_structure_is_not_latest_version.cs +++ b/unittests/TestCommon/Generic/Bootstrapping/When_Grate_structure_is_not_latest_version.cs @@ -12,19 +12,25 @@ namespace TestCommon.Generic.Bootstrapping; // ReSharper disable once UnusedType.Global +// ReSharper disable once InconsistentNaming public abstract class When_Grate_structure_is_not_latest_version(IGrateTestContext context, ITestOutputHelper testOutput) : MigrationsScriptsBase(context, testOutput) { - [Fact] - public virtual async Task The_latest_version_is_applied() + [Theory] + [MemberData(nameof(VersionTableWithDifferentCasings))] + public virtual async Task The_latest_version_is_applied(string versionTableName) { var db = TestConfig.RandomDatabase(); var parent = CreateRandomTempDirectory(); - // This will create the version table without the status column + // This will create the version table without the status column, with lower-case + var cfg = Context.DefaultConfiguration with + { + VersionTableName = versionTableName.ToLower() + }; - var config = GrateConfigurationBuilder.Create(Context.DefaultConfiguration) + var config = GrateConfigurationBuilder.Create(cfg) .WithConnectionString(Context.ConnectionString(db)) .WithFolders(FoldersConfiguration.Default()) .WithSqlFilesDirectory(parent) @@ -69,7 +75,6 @@ public virtual async Task The_latest_version_is_applied() conn.Close(); - // Check that the status column is not there var tableWithSchema = Context.Syntax.TableWithSchema(config.SchemaName, config.VersionTableName); var selectSql = $"SELECT * FROM {tableWithSchema}"; @@ -82,13 +87,24 @@ public virtual async Task The_latest_version_is_applied() TryClose(conn); columns.Should().NotContain("status".ToUpper()); + // Reset the config to use the actual column casings, to make sure that the status column is added even if the + // already "existing" tables (which we just created above, with varying casings), are updated, even if the + // casing of the default configuration is different from the standard one + var actualConfig = config with + { + VersionTableName = versionTableName + }; + // Run the migration - await using (var migrator = Context.Migrator.WithConfiguration(config)) + await using (var migrator = Context.Migrator.WithConfiguration(actualConfig)) { await RunMigration(migrator); } // Check that the status column has been added + tableWithSchema = Context.Syntax.TableWithSchema(actualConfig.SchemaName, actualConfig.VersionTableName); + selectSql = $"SELECT * FROM {tableWithSchema}"; + conn = Context.CreateDbConnection(db); reader = await conn.ExecuteReaderAsync(selectSql); @@ -99,8 +115,140 @@ public virtual async Task The_latest_version_is_applied() //await Context.DropDatabase(db); } + + + [Theory] + [MemberData(nameof(TablesWithDifferentCasings))] + public virtual async Task The_table_name_casings_are_converted_if_needed( + string scriptsRun, string scriptsRunErrors, string version) + { + var db = TestConfig.RandomDatabase(); + var parent = CreateRandomTempDirectory(); + + // This will create the ScriptsRun, ScriptsRunErrors and Version tables + // with lower-case version of the supplied casing + var cfg = Context.DefaultConfiguration with + { + VersionTableName = version.ToLower(), + ScriptsRunTableName = scriptsRun.ToLower(), + ScriptsRunErrorsTableName = scriptsRunErrors.ToLower(), + }; + + var config = GrateConfigurationBuilder.Create(cfg) + .WithConnectionString(Context.ConnectionString(db)) + .WithFolders(FoldersConfiguration.Default()) + .WithSqlFilesDirectory(parent) + .Build(); + + // Create database + var password = Context.AdminConnectionString + .Split(";", TrimEntries | RemoveEmptyEntries) + .SingleOrDefault(entry => entry.StartsWith("Password") || entry.StartsWith("Pwd"))? + .Split("=", TrimEntries | RemoveEmptyEntries) + .Last(); + + var createDatabaseSql = Context.Syntax.CreateDatabase(db, password); + using (var adminConn = Context.CreateAdminDbConnection()) + { + await adminConn.ExecuteAsync(createDatabaseSql); + } + + var conn = Context.CreateDbConnection(db); + // Manually create the script tables, with modified casing + var resources = TestInfrastructure.Bootstrapping.GetBootstrapScripts(this.Context.DatabaseType, "Baseline"); + foreach (var resource in resources) + { + var resourceText = await TestInfrastructure.Bootstrapping.GetContent(this.Context.DatabaseType.Assembly, resource); + + resourceText = resourceText.Replace("{{ScriptsRunTable}}", config.ScriptsRunTableName); + resourceText = resourceText.Replace("{{ScriptsRunErrorsTable}}", config.ScriptsRunErrorsTableName); + resourceText = resourceText.Replace("{{VersionTable}}", config.VersionTableName); + resourceText = resourceText.Replace("{{SchemaName}}", config.SchemaName); + + await conn.ExecuteAsync(resourceText); + } + + conn.Close(); + + // Check that the table exists in the DB, with lower-case + // - Selecting from the table with a non-existing table name would throw an exception + await CheckThatTableExists(config.VersionTableName, config.SchemaName); + await CheckThatTableExists(config.ScriptsRunTableName, config.SchemaName); + await CheckThatTableExists(config.ScriptsRunErrorsTableName, config.SchemaName); + + // Reset the config to use the supplied column names, and run the migration. Then, select from the version table + // with the supplied table name, and see that it has been converted to the default casing (if needed by the DB provider), + // i.e., we can still select from it. + var actualConfig = config with + { + VersionTableName = version, + ScriptsRunTableName = scriptsRun, + ScriptsRunErrorsTableName = scriptsRunErrors, + }; + + // Run the migration + await using (var migrator = Context.Migrator.WithConfiguration(actualConfig)) + { + await RunMigration(migrator); + } + + // Check that the tables can be selected from, using the supplied table names. + await CheckThatTableExists(actualConfig.VersionTableName, config.SchemaName); + await CheckThatTableExists(actualConfig.ScriptsRunTableName, config.SchemaName); + await CheckThatTableExists(actualConfig.ScriptsRunErrorsTableName, config.SchemaName); + + return; + + async Task CheckThatTableExists(string tableName, string schemaName) + { + var tableWithSchema = Context.Syntax.TableWithSchema(schemaName, tableName); + var selectSql = $"SELECT * FROM {tableWithSchema}"; + + conn = Context.CreateDbConnection(db); + var reader = await conn.ExecuteReaderAsync(selectSql); + + var columns = GetColumns(reader); + TryClose(conn); + columns.Should().HaveCountGreaterThan(2); + } + } + + public static TheoryData VersionTableWithDifferentCasings() + { + var def = GrateConfiguration.Default; + return new TheoryData + { + def.VersionTableName, + def.VersionTableName.ToLower(), + def.VersionTableName.ToUpper() + }; + } + + public static TheoryData TablesWithDifferentCasings() + { + var def = GrateConfiguration.Default; + + var all = from scriptsRun in DifferentPermutations(def.ScriptsRunTableName) + from scriptsRunErrors in DifferentPermutations(def.ScriptsRunErrorsTableName) + from versionTableName in DifferentPermutations(def.VersionTableName) + select (scriptsRun, scriptsRunErrors, versionTableName); + + var data = new TheoryData(); + foreach (var (run, errors, version) in all) + { + data.Add(run, errors, version); + } + return data; + } + private static IEnumerable DifferentPermutations(string original) => + [ + original, + original.ToLower(), + //original.ToUpper(), + new string(original.ToCharArray().Select(c => Random.Shared.Next(2) == 0 ? c : Char.ToUpper(c)).ToArray()) + ]; private async Task RunMigration(IGrateMigrator migrator) {