Merge pull request #205 from wokket/fix/#204-broken-dryrun

fix #204: Ensure --baseline in combination with a --dryrun doesn't create the grate schema objects

grate.unittests/Generic/Running_MigrationScripts/Everytime_scripts.cs

@@ -42,23 +42,16 @@ public async Task Are_not_run_in_dryrun()
 
         var knownFolders = KnownFolders.In(CreateRandomTempDirectory());
         CreateDummySql(knownFolders.Permissions);
-            
+
         var config = Context.GetConfiguration(db, knownFolders) with
         {
             DryRun = true, // this is important!
         };
 
-        await using (var migrator = Context.GetMigrator(config))
-        {
-            await migrator.Migrate();
-        }
-
-        string sql = $"SELECT script_name FROM {Context.Syntax.TableWithSchema("grate", "ScriptsRun")}";
-
-        await using var conn = Context.CreateDbConnection(db);
-        var scripts = (await conn.QueryAsync<string>(sql)).ToArray();
-        scripts.Should().BeEmpty();
-
+        await using var migrator = Context.GetMigrator(config);
+        await migrator.Migrate();
+        // this helper takes into account whether the grate versioning table exists or not.
+        Assert.False(await migrator.DbMigrator.Database.HasRun("1_jalla.sql"));
     }
 
     [Test]
@@ -100,13 +93,13 @@ public async Task Are_not_run_in_baseline()
     {
         var db = TestConfig.RandomDatabase();
 
-        var knownFolders = KnownFolders.In(CreateRandomTempDirectory()); 
-            
+        var knownFolders = KnownFolders.In(CreateRandomTempDirectory());
+
         var config = Context.GetConfiguration(db, knownFolders) with
         {
             Baseline = true, // this is important!
         };
-           
+
         var path = knownFolders?.Views?.Path ?? throw new Exception("Config Fail");
 
         WriteSql(path, "view.sql", "create view grate as select '1' as col");
@@ -124,9 +117,6 @@ public async Task Are_not_run_in_baseline()
 
         // but doesn't exist
         Assert.ThrowsAsync(Context.DbExceptionType, async () => await conn.QueryAsync<string>("select * from grate"));
-
-
-
     }
 
     private void CreateEveryTimeScriptFile(MigrationsFolder? folder)

grate.unittests/Generic/Running_MigrationScripts/Versioning_The_Database.cs

@@ -1,4 +1,7 @@
-using System.Threading.Tasks;
+using System.Linq;
+using System.Threading.Tasks;
+using System.Transactions;
+using Dapper;
 using FluentAssertions;
 using FluentAssertions.Execution;
 using grate.Configuration;
@@ -21,7 +24,7 @@ public async Task Returns_The_New_Version_Id()
 
         var knownFolders = KnownFolders.In(CreateRandomTempDirectory());
         CreateDummySql(knownFolders.Sprocs);
-            
+
         await using (migrator = Context.GetMigrator(db, knownFolders))
         {
             await migrator.Migrate();
@@ -31,11 +34,32 @@ public async Task Returns_The_New_Version_Id()
                 // Version again
                 var version = await migrator.DbMigrator.VersionTheDatabase("1.2.3.4");
                 version.Should().Be(2);
-                
+
                 // And again
                 version = await migrator.DbMigrator.VersionTheDatabase("1.2.3.4");
                 version.Should().Be(3);
             }
         }
     }
+
+    [Test]
+    public async Task Does_Not_Create_Versions_When_Dryrun()
+    {
+        //for bug #204 - when running --baseline and --dryrun on a new db it shouldn't create the grate schema's etc
+        var db = TestConfig.RandomDatabase();
+        var knownFolders = KnownFolders.In(CreateRandomTempDirectory());
+
+        CreateDummySql(knownFolders.Sprocs); // make sure there's something that could be logged...
+
+        var grateConfig = Context.GetConfiguration(db, knownFolders) with
+        {
+            Baseline = true, // don't run the sql
+            DryRun = true // and don't actually _touch_ the DB in any way
+        };
+
+        await using var migrator = Context.GetMigrator(grateConfig);
+        await migrator.Migrate(); // shouldn't touch anything because of --dryrun
+        var addedTable = await migrator.DbMigrator.Database.VersionTableExists();
+        Assert.False(addedTable); // we didn't even add the grate infrastructure
+    }
 }

grate/Configuration/GrateConfiguration.cs

@@ -113,6 +113,7 @@ public string? AdminConnectionString
 
     /// <summary>
     /// If true we need to log what we would have done, but NOT run any SQL
+    /// including writing to the grate versioning schema
     /// </summary>
     public bool DryRun { get; init; }
 

grate/Migration/AnsiSqlDatabase.cs

@@ -255,9 +255,9 @@ protected virtual async Task CreateVersionTable()
 
     protected async Task<bool> ScriptsRunTableExists() => await TableExists(SchemaName, "ScriptsRun");
     protected async Task<bool> ScriptsRunErrorsTableExists() => await TableExists(SchemaName, "ScriptsRunErrors");
-    protected async Task<bool> VersionTableExists() => await TableExists(SchemaName, "Version");
+    public async Task<bool> VersionTableExists() => await TableExists(SchemaName, "Version");
 
-    private async Task<bool> TableExists(string schemaName, string tableName)
+    public async Task<bool> TableExists(string schemaName, string tableName)
     {
         var fullTableName = SupportsSchemas ? tableName : _syntax.TableWithSchema(schemaName, tableName);
         var tableSchema = SupportsSchemas ? schemaName : DatabaseName;
@@ -288,9 +288,17 @@ protected virtual string ExistsSql(string tableSchema, string fullTableName)
 
     public async Task<string> GetCurrentVersion()
     {
-        var sql = CurrentVersionSql;
-        var res = await ExecuteScalarAsync<string>(Connection, sql);
-        return res ?? "0.0.0.0";
+        try
+        {
+            var sql = CurrentVersionSql;
+            var res = await ExecuteScalarAsync<string>(Connection, sql);
+            return res ?? "0.0.0.0";
+        }
+        catch (Exception ex)
+        {
+            Logger.LogDebug(ex, "An error occurred getting the current database version, new database + --dryrun?");
+            return "0.0.0.0";
+        }
     }
 
     public virtual async Task<long> VersionTheDatabase(string newVersion)
@@ -354,13 +362,24 @@ private class ScriptsRunCacheItem
 
     private async Task<IDictionary<string, string>> GetAllScriptsRun()
     {
-        var sql = $@"
+
+        try
+        {
+
+            var sql = $@"
 SELECT script_name, text_hash
 FROM {ScriptsRunTable} sr
 WHERE id = (SELECT MAX(id) FROM {ScriptsRunTable} sr2 WHERE sr2.script_name = sr.script_name)
 ";
-        var results = await Connection.QueryAsync<ScriptsRunCacheItem>(sql);
-        return results.ToDictionary(item => item.script_name, item => item.text_hash);
+            var results = await Connection.QueryAsync<ScriptsRunCacheItem>(sql);
+            return results.ToDictionary(item => item.script_name, item => item.text_hash);
+
+        }
+        catch (Exception ex) when (Config?.DryRun ?? throw new InvalidOperationException("No configuration available."))
+        {
+            Logger.LogDebug(ex, "Ignoring error getting ScriptsRun when in --dryrun, probable missing table");
+            return new Dictionary<string, string>(); // return empty set if nothing has ever been run
+        }
     }
 
     private async Task<IDictionary<string, string>> GetScriptsRunCache() => _scriptsRunCache ??= await GetAllScriptsRun();
@@ -391,12 +410,20 @@ public async Task<bool> HasRun(string scriptName)
             return true;
         }
 
-        var hasRunSql = Parameterize($@"
+        try
+        {
+            var hasRunSql = Parameterize($@"
 SELECT 1 FROM  {ScriptsRunTable}
 WHERE script_name = @scriptName");
 
-        var run = await ExecuteScalarAsync<bool?>(Connection, hasRunSql, new { scriptName });
-        return run ?? false;
+            var run = await ExecuteScalarAsync<bool?>(Connection, hasRunSql, new { scriptName });
+            return run ?? false;
+        }
+        catch (Exception ex) when (Config?.DryRun ?? throw new InvalidOperationException("No configuration available"))
+        {
+            Logger.LogDebug(ex, "Ignoring exception in dryrun, missing table?");
+            return false;
+        }
     }
 
     protected virtual object Bool(bool source) => source;

grate/Migration/DbMigrator.cs

@@ -52,7 +52,19 @@ public Task DropDatabase()
 
     public Task RunSupportTasks() => Database.RunSupportTasks();
     public Task<string> GetCurrentVersion() => Database.GetCurrentVersion();
-    public Task<long> VersionTheDatabase(string newVersion) => Database.VersionTheDatabase(newVersion);
+    public Task<long> VersionTheDatabase(string newVersion)
+    {
+        if (Configuration.DryRun)
+        {
+            _logger.LogDebug("Skipping writing database version row due to --dryrun");
+            return Task.FromResult(-1L);
+        }
+        else
+        {
+            return Database.VersionTheDatabase(newVersion);
+        }
+    }
+
     public Task OpenAdminConnection() => Database.OpenAdminConnection();
     public Task CloseAdminConnection() => Database.CloseAdminConnection();
 
@@ -235,7 +247,7 @@ private async Task RunTheActualSql(
             {
                 Database.Rollback();
                 Transaction.Current?.Dispose();
-                    
+
                 await RecordScriptInScriptsRunErrorsTable(scriptName, sql, statement, ex.Message, versionId);
 
                 await Database.CloseConnection();
@@ -279,8 +291,16 @@ private Task RecordScriptInScriptsRunTable(string scriptName, string sql, Migrat
         var hash = _hashGenerator.Hash(sql);
         var sqlToStore = Configuration.DoNotStoreScriptsRunText ? null : sql;
 
-        _logger.LogTrace("Recording {ScriptName} script ran on {ServerName} - {DatabaseName}.", scriptName, Database.ServerName, Database.DatabaseName);
-        return Database.InsertScriptRun(scriptName, sqlToStore, hash, migrationType == MigrationType.Once, versionId);
+        if (Configuration.DryRun)
+        {
+            _logger.LogTrace("Skipping recording {ScriptName} script ran on {ServerName} - {DatabaseName}, --dryrun prevents sql writes", scriptName, Database.ServerName, Database.DatabaseName);
+            return Task.CompletedTask;
+        }
+        else
+        {
+            _logger.LogTrace("Recording {ScriptName} script ran on {ServerName} - {DatabaseName}.", scriptName, Database.ServerName, Database.DatabaseName);
+            return Database.InsertScriptRun(scriptName, sqlToStore, hash, migrationType == MigrationType.Once, versionId);
+        }
     }
 
     private Task RecordScriptInScriptsRunErrorsTable(string scriptName, string sql, string errorSql, string errorMessage, long versionId)
@@ -295,4 +315,4 @@ public async ValueTask DisposeAsync()
         await Database.DisposeAsync();
         GC.SuppressFinalize(this);
     }
-}
+}

grate/Migration/GrateMigrator.cs

@@ -170,7 +170,15 @@ private async Task CreateGrateStructure(IDbMigrator dbMigrator)
         _logger.LogInformation("Grate Structure");
         Separator('=');
 
-        await dbMigrator.RunSupportTasks();
+        if (dbMigrator.Configuration.DryRun)
+        {
+            _logger.LogInformation("Skipping creation of versioning structures due to --dryrun");
+        }
+        else
+        {
+            await dbMigrator.RunSupportTasks();
+        }
+
         await dbMigrator.CloseConnection();
     }
 
@@ -283,8 +291,8 @@ private void CopyToChangeDropFolder(DirectoryInfo migrationRoot, FileSystemInfo
 
         File.Copy(file.FullName, destinationFile);
     }
-        
-// ReSharper disable once TemplateIsNotCompileTimeConstantProblem
+
+    // ReSharper disable once TemplateIsNotCompileTimeConstantProblem
 #pragma warning disable CA2254 // Template should be a static expression.
     private void Separator(char c) => _logger.LogInformation(new string(c, 80));
 #pragma warning restore CA2254 // Template should be a static expression

grate/Migration/IDatabase.cs

@@ -40,4 +40,5 @@ public interface IDatabase : IAsyncDisposable
     Task<bool> HasRun(string scriptName);
     Task InsertScriptRun(string scriptName, string? sql, string hash, bool runOnce, long versionId);
     Task InsertScriptRunError(string scriptName, string? sql, string errorSql, string errorMessage, long versionId);
+    Task<bool> VersionTableExists();
 }