From 6b2a1e9950400e81f97a6c4189955ab0010797d7 Mon Sep 17 00:00:00 2001 From: "Erik A. Brandstadmoen" Date: Wed, 24 Jul 2024 15:37:08 +0200 Subject: [PATCH] Bug #553: Baseline creates orphan version_id in ScriptsRun (#567) When running in baseline mode, no SQL is actually run, so there was not inserted any entry into the Version table. However, this should be inserted in baseline mode, as this makes sense business-wise, to register the baseline as a version of the database (even with no changes) Fixes #553 --- src/grate.core/Migration/GrateMigrator.cs | 12 ++- ...Grate_internal_structure_does_not_exist.cs | 39 ++++++++++ .../Versioning_The_Database.cs | 75 +++++++++++++++++++ 3 files changed, 119 insertions(+), 7 deletions(-) diff --git a/src/grate.core/Migration/GrateMigrator.cs b/src/grate.core/Migration/GrateMigrator.cs index 066a764f..05446d03 100644 --- a/src/grate.core/Migration/GrateMigrator.cs +++ b/src/grate.core/Migration/GrateMigrator.cs @@ -125,9 +125,7 @@ public async Task Migrate() // Run these first without a transaction, to make sure the tables are created even on a potential rollback await CreateGrateStructure(dbMigrator); - string? newVersion; - long versionId; - (versionId, newVersion) = await VersionTheDatabase(dbMigrator); + var (versionId, newVersion) = await VersionTheDatabase(dbMigrator); Separator('='); _logger.LogInformation("Migration Scripts"); @@ -211,17 +209,17 @@ public async Task Migrate() // TODO: Clean up the try - catch here - it's only used in initial bootstrapping if the // TODO: version table _does_ exist, but it doesn't have the version table. // TODO: This will be just once for each of legacy RoundhousE databases migrated to grate. - try { //If we get here this means no exceptions are thrown above, so we can conclude the migration was successful! - if (anySqlRun) + if (anySqlRun || config.Baseline) { await DbMigrator.Database.ChangeVersionStatus(MigrationStatus.Finished, versionId); } else { - // as we have an issue with the versioning table, we need to delete the version record + // Delete the version record if no actual migration was performed, to avoid having a lot of "no-op" + // migrations if grate is run a lot without any changes await DbMigrator.Database.DeleteVersionRecord(versionId); } } @@ -637,7 +635,7 @@ private async Task GetInternalGrateConfiguration(string inte // If they do, check if the tables are already logged as run in their own tables. // If they are, the tables are already known to grate. If they are not, the tables are created by RoundhousE, or // an earlier version of grate, and we need to run in baseline mode, to register the scripts as run, without - // actually running them. + // actually running them. var baseline = internalFolderName == "Baseline" && await this.Database.VersionTableExists() && !(await this.Database.GrateInternalTablesAreProperlyLogged()); diff --git a/unittests/TestCommon/Generic/Bootstrapping/When_Grate_internal_structure_does_not_exist.cs b/unittests/TestCommon/Generic/Bootstrapping/When_Grate_internal_structure_does_not_exist.cs index ded46bda..ebe3a2f1 100644 --- a/unittests/TestCommon/Generic/Bootstrapping/When_Grate_internal_structure_does_not_exist.cs +++ b/unittests/TestCommon/Generic/Bootstrapping/When_Grate_internal_structure_does_not_exist.cs @@ -124,6 +124,45 @@ public async Task Logs_internal_scripts_run_in_own_structure(string name) //await Context.DropDatabase(db); } + + [Fact] + [Trait("Category", "Versioning")] + public async Task A_version_with_the_current_application_version_is_inserted_into_the_GrateVersion_table() + { + var db = TestConfig.RandomDatabase(); + var dbVersion = "1.2.3.1"; + + var parent = CreateRandomTempDirectory(); + var knownFolders = Folders.Default; + CreateDummySql(parent, knownFolders[Up]); + + var config = GrateConfigurationBuilder.Create(Context.DefaultConfiguration) + .WithConnectionString(Context.ConnectionString(db)) + .WithFolders(knownFolders) + .WithSqlFilesDirectory(parent) + .WithVersion(dbVersion) + .Build(); + + await using (var migrator = Context.Migrator.WithConfiguration(config)) + { + await migrator.Migrate(); + } + + IEnumerable<(string version, string status)> entries; + string sql = $"SELECT version, status FROM {Context.Syntax.TableWithSchema("grate", "GrateVersion")}"; + + using (var conn = Context.CreateDbConnection(db)) + { + entries = (await conn.QueryAsync<(string version, string status)>(sql)).ToArray(); + } + + // There can be multiple internal migrations, one baseline, and some adjustments. So multiple entries are OK here + entries.Should().HaveCountGreaterThanOrEqualTo(1); + var version = entries.First(); + version.version.Should().Be(ApplicationInfo.Version); + version.status.Should().Be(MigrationStatus.Finished); + } + // This was failing, because the tokens were already replaced before the internal scripts were run, // and the tokens were lazily initialized. So we had to clear the tokens when running the internal scripts. diff --git a/unittests/TestCommon/Generic/Running_MigrationScripts/Versioning_The_Database.cs b/unittests/TestCommon/Generic/Running_MigrationScripts/Versioning_The_Database.cs index deb9fe52..d2eed771 100644 --- a/unittests/TestCommon/Generic/Running_MigrationScripts/Versioning_The_Database.cs +++ b/unittests/TestCommon/Generic/Running_MigrationScripts/Versioning_The_Database.cs @@ -119,6 +119,81 @@ public async Task Creates_a_new_version_with_status_InProgress() //await Context.DropDatabase(db); } + + [Fact] + [Trait("Category", "Versioning")] + public async Task Migrating_creates_a_new_version_record() + { + var db = TestConfig.RandomDatabase(); + var dbVersion = "1.2.3.1"; + + var parent = CreateRandomTempDirectory(); + var knownFolders = Folders.Default; + CreateDummySql(parent, knownFolders[Up]); + + var config = GrateConfigurationBuilder.Create(Context.DefaultConfiguration) + .WithConnectionString(Context.ConnectionString(db)) + .WithFolders(knownFolders) + .WithSqlFilesDirectory(parent) + .WithVersion(dbVersion) + .Build(); + + await using (var migrator = Context.Migrator.WithConfiguration(config)) + { + await migrator.Migrate(); + } + + IEnumerable<(string version, string status)> entries; + string sql = $"SELECT version, status FROM {Context.Syntax.TableWithSchema("grate", "Version")}"; + + using (var conn = Context.CreateDbConnection(db)) + { + entries = (await conn.QueryAsync<(string version, string status)>(sql)).ToArray(); + } + + entries.Should().HaveCount(1); + var version = entries.Single(); + version.version.Should().Be(dbVersion); + version.status.Should().Be(MigrationStatus.Finished); + } + + [Fact] + [Trait("Category", "Versioning")] + public async Task Migrating_creates_a_new_version_record_even_in_baseline_mode() + { + var db = TestConfig.RandomDatabase(); + var dbVersion = "1.2.3.4"; + + var parent = CreateRandomTempDirectory(); + var knownFolders = Folders.Default; + CreateDummySql(parent, knownFolders[Up]); + + var config = GrateConfigurationBuilder.Create(Context.DefaultConfiguration) + .WithConnectionString(Context.ConnectionString(db)) + .WithFolders(knownFolders) + .WithSqlFilesDirectory(parent) + .WithVersion(dbVersion) + .Baseline() + .Build(); + + await using (var migrator = Context.Migrator.WithConfiguration(config)) + { + await migrator.Migrate(); + } + + IEnumerable<(string version, string status)> entries; + string sql = $"SELECT version, status FROM {Context.Syntax.TableWithSchema("grate", "Version")}"; + + using (var conn = Context.CreateDbConnection(db)) + { + entries = (await conn.QueryAsync<(string version, string status)>(sql)).ToArray(); + } + + entries.Should().HaveCount(1); + var version = entries.Single(); + version.version.Should().Be(dbVersion); + version.status.Should().Be(MigrationStatus.Finished); + } [Fact] [Trait("Category", "Versioning")]