Refuse to re-apply an edited already-applied migration (closes #29)

MigrationOps.Core.Tests/AddPlanEntryTests.cs

@@ -0,0 +1,131 @@
+using MigrationOps.Core.MigrationFramework.Services;
+using MigrationOps.Core.Models;
+
+namespace MigrationOps.Core.Tests
+{
+    // AddPlanEntry is the single place that turns a MigrationFileStatus diff into the status
+    // the dry-run report / --verify gate acts on. These tests exist mainly to lock in the one
+    // rule that matters most: a drifted (edited) migration must classify as Changed, while a
+    // drifted database object script (proc/view/etc., meant to be re-applied) classifies as
+    // WouldApply — mixing those up would either block legitimate object redeploys or let an
+    // edited migration slip through as an ordinary pending apply.
+    public class AddPlanEntryTests
+    {
+        private static readonly HashSet<string> NoUnresolvedReported = new(StringComparer.OrdinalIgnoreCase);
+
+        [Fact]
+        public void AlreadyAppliedStatusMapsToAlreadyApplied()
+        {
+            var plan = new DryRunPlan();
+            var status = new MigrationFileStatus
+            {
+                FileName = "Foo.sql",
+                Tags = new List<string> { "Db1" },
+                IsApplied = true,
+                CurrentChecksum = "abc"
+            };
+
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.Migration, "Db1", "Foo.sql", NoUnresolvedReported);
+
+            var entry = Assert.Single(plan.Entries);
+            Assert.Equal(PlanEntryStatus.AlreadyApplied, entry.Status);
+        }
+
+        [Fact]
+        public void DriftedMigrationMapsToChangedNotWouldApply()
+        {
+            using var dir = new TempDirectory();
+            var filePath = dir.WriteFile("Foo.sql", "SELECT 1;");
+            var plan = new DryRunPlan();
+            var status = new MigrationFileStatus
+            {
+                FileName = "Foo.sql",
+                Tags = new List<string> { "Db1" },
+                HasDrift = true,
+                RecordedChecksum = "old",
+                CurrentChecksum = "new"
+            };
+
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.Migration, "Db1", filePath, NoUnresolvedReported);
+
+            var entry = Assert.Single(plan.Entries);
+            Assert.Equal(PlanEntryStatus.Changed, entry.Status);
+            Assert.Contains("recorded", entry.Detail);
+            Assert.NotNull(entry.ScriptText);
+        }
+
+        [Fact]
+        public void DriftedDatabaseObjectScriptMapsToWouldApplyNotChanged()
+        {
+            using var dir = new TempDirectory();
+            var filePath = dir.WriteFile("Foo.sql", "CREATE OR ALTER VIEW dbo.V AS SELECT 1;");
+            var plan = new DryRunPlan();
+            var status = new MigrationFileStatus
+            {
+                FileName = "Foo.sql",
+                Tags = new List<string> { "Db1" },
+                HasDrift = true,
+                RecordedChecksum = "old",
+                CurrentChecksum = "new"
+            };
+
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.DatabaseObject, "Db1", filePath, NoUnresolvedReported);
+
+            var entry = Assert.Single(plan.Entries);
+            Assert.Equal(PlanEntryStatus.WouldApply, entry.Status);
+            Assert.Contains("updated", entry.Detail);
+        }
+
+        [Fact]
+        public void NewPendingFileMapsToWouldApply()
+        {
+            using var dir = new TempDirectory();
+            var filePath = dir.WriteFile("Foo.sql", "SELECT 1;");
+            var plan = new DryRunPlan();
+            var status = new MigrationFileStatus
+            {
+                FileName = "Foo.sql",
+                Tags = new List<string> { "Db1" },
+                CurrentChecksum = "new"
+            };
+
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.Migration, "Db1", filePath, NoUnresolvedReported);
+
+            var entry = Assert.Single(plan.Entries);
+            Assert.Equal(PlanEntryStatus.WouldApply, entry.Status);
+            Assert.Contains("new", entry.Detail);
+        }
+
+        [Fact]
+        public void ChecksumMissingMapsToValidationError()
+        {
+            var plan = new DryRunPlan();
+            var status = new MigrationFileStatus
+            {
+                FileName = "Foo.sql",
+                Tags = new List<string> { "Db1" },
+                ChecksumMissing = true
+            };
+
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.Migration, "Db1", "Foo.sql", NoUnresolvedReported);
+
+            var entry = Assert.Single(plan.Entries);
+            Assert.Equal(PlanEntryStatus.ValidationError, entry.Status);
+        }
+
+        [Fact]
+        public void TaglessFileIsReportedOnceAcrossMultipleDatabasesAsUnresolved()
+        {
+            var plan = new DryRunPlan();
+            var status = new MigrationFileStatus { FileName = "Foo.sql", ValidationError = "no tags" };
+            var reported = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
+
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.Migration, "Db1", "Foo.sql", reported);
+            MigrationService.AddPlanEntry(plan, status, ScriptKind.Migration, "Db2", "Foo.sql", reported);
+
+            var entry = Assert.Single(plan.Entries);
+            Assert.Equal(PlanEntryStatus.ValidationError, entry.Status);
+            Assert.Equal("(unresolved)", entry.Database);
+        }
+    }
+}

MigrationOps.Core.Tests/DetectEditedMigrationTests.cs

@@ -0,0 +1,41 @@
+using MigrationOps.Core.MigrationFramework.Services;
+
+namespace MigrationOps.Core.Tests
+{
+    // Covers the guard added for #29: editing an already-applied migration must be refused by
+    // `apply`, not just flagged by dry-run. DetectEditedMigration is the pure decision behind
+    // that guard - given the checksum of the migration's last *successful* apply (or null if
+    // it has never successfully applied) and the checksum of the file on disk, it decides
+    // whether it's safe to proceed.
+    public class DetectEditedMigrationTests
+    {
+        [Fact]
+        public void AllowsAnUnchangedAlreadyAppliedMigration()
+        {
+            var error = MigrationService.DetectEditedMigration("20260101-001-Foo.sql", recordedChecksum: "abc", currentChecksum: "abc");
+
+            Assert.Null(error);
+        }
+
+        [Fact]
+        public void RejectsAMigrationEditedAfterItWasApplied()
+        {
+            var error = MigrationService.DetectEditedMigration("20260101-001-Foo.sql", recordedChecksum: "old-checksum", currentChecksum: "new-checksum");
+
+            Assert.NotNull(error);
+            Assert.Contains("20260101-001-Foo.sql", error);
+            Assert.Contains("immutable", error);
+        }
+
+        [Fact]
+        public void AllowsAMigrationThatHasNeverSuccessfullyApplied()
+        {
+            // recordedChecksum is null both for a brand-new file and for one whose only history
+            // row is a failed attempt (Success = 0) - GetLatestSuccessfulMigrationChecksum only
+            // looks at Success = 1 rows, so a failed-then-fixed migration is allowed to re-run.
+            var error = MigrationService.DetectEditedMigration("20260101-001-Foo.sql", recordedChecksum: null, currentChecksum: "whatever");
+
+            Assert.Null(error);
+        }
+    }
+}

MigrationOps.Core.Tests/DetermineDatabaseFromTagsTests.cs

@@ -0,0 +1,56 @@
+using MigrationOps.Core.MigrationFramework.Services;
+
+namespace MigrationOps.Core.Tests
+{
+    public class DetermineDatabaseFromTagsTests
+    {
+        // DetermineDatabaseFromTags reads the "Databases" section of configuration, so each
+        // test points a MigrationService at a throwaway dbconfig.json with just the database
+        // names it needs (connection strings are never used by this method).
+        private static MigrationService CreateServiceWithDatabases(params string[] databaseNames)
+        {
+            var entries = string.Join(",", databaseNames.Select(n => $"\"{n}\": {{ \"ConnectionString\": \"unused\" }}"));
+            var json = $"{{ \"Databases\": {{ {entries} }} }}";
+
+            using var configFile = new TempFile(json, ".json");
+            return new MigrationService(configFile.Path);
+        }
+
+        [Fact]
+        public void ReturnsMatchingDatabaseName()
+        {
+            var service = CreateServiceWithDatabases("Db1", "Db2");
+
+            Assert.Equal("Db1", service.DetermineDatabaseFromTags(new List<string> { "Db1" }));
+        }
+
+        [Fact]
+        public void MatchesConfiguredDatabaseCaseInsensitively()
+        {
+            var service = CreateServiceWithDatabases("Db1");
+
+            // Note: the match is case-insensitive, but the method returns the tag as written
+            // in the file, not the configured key's casing. That's safe downstream only
+            // because IConfiguration's own indexer (used by GetConnectionString) is itself
+            // case-insensitive - this test documents current behavior rather than asserting
+            // it is the ideal API.
+            Assert.Equal("db1", service.DetermineDatabaseFromTags(new List<string> { "db1" }));
+        }
+
+        [Fact]
+        public void ReturnsFirstTagThatMatchesAConfiguredDatabase()
+        {
+            var service = CreateServiceWithDatabases("Db1", "Db2");
+
+            Assert.Equal("Db2", service.DetermineDatabaseFromTags(new List<string> { "UnknownTag", "Db2" }));
+        }
+
+        [Fact]
+        public void ThrowsWhenNoTagMatchesAConfiguredDatabase()
+        {
+            var service = CreateServiceWithDatabases("Db1", "Db2");
+
+            Assert.Throws<InvalidOperationException>(() => service.DetermineDatabaseFromTags(new List<string> { "Db3" }));
+        }
+    }
+}

MigrationOps.Core.Tests/EnsureCreateOrAlterStatementTests.cs

@@ -0,0 +1,69 @@
+using MigrationOps.Core.MigrationFramework.Services;
+
+namespace MigrationOps.Core.Tests
+{
+    public class EnsureCreateOrAlterStatementTests
+    {
+        [Fact]
+        public void AllowsCreateOrAlterAfterHeaderComments()
+        {
+            var script = "-- Tags: Db1\n-- Checksum: abc\nCREATE OR ALTER PROCEDURE dbo.Foo AS SELECT 1;";
+
+            var exception = Record.Exception(() => MigrationService.EnsureCreateOrAlterStatement(script, "Foo.sql"));
+
+            Assert.Null(exception);
+        }
+
+        [Fact]
+        public void AllowsBlankLinesBeforeTheStatement()
+        {
+            var script = "-- Tags: Db1\n\n\nCREATE OR ALTER VIEW dbo.V AS SELECT 1;";
+
+            var exception = Record.Exception(() => MigrationService.EnsureCreateOrAlterStatement(script, "V.sql"));
+
+            Assert.Null(exception);
+        }
+
+        [Fact]
+        public void IsCaseInsensitive()
+        {
+            var script = "-- Tags: Db1\ncreate or alter function dbo.F() returns int as begin return 1 end";
+
+            var exception = Record.Exception(() => MigrationService.EnsureCreateOrAlterStatement(script, "F.sql"));
+
+            Assert.Null(exception);
+        }
+
+        [Fact]
+        public void ThrowsWhenFirstStatementIsPlainCreate()
+        {
+            var script = "-- Tags: Db1\nCREATE PROCEDURE dbo.Foo AS SELECT 1;";
+
+            var ex = Assert.Throws<InvalidOperationException>(
+                () => MigrationService.EnsureCreateOrAlterStatement(script, "Foo.sql"));
+
+            Assert.Contains("Foo.sql", ex.Message);
+            Assert.Contains("CREATE OR ALTER", ex.Message);
+        }
+
+        [Fact]
+        public void ThrowsForEmptyScript()
+        {
+            var ex = Assert.Throws<InvalidOperationException>(
+                () => MigrationService.EnsureCreateOrAlterStatement("", "Empty.sql"));
+
+            Assert.Contains("empty", ex.Message);
+        }
+
+        [Fact]
+        public void ThrowsWhenScriptIsOnlyHeaderComments()
+        {
+            var script = "-- Tags: Db1\n-- Checksum: abc\n-- just a trailing comment, no statement";
+
+            var ex = Assert.Throws<InvalidOperationException>(
+                () => MigrationService.EnsureCreateOrAlterStatement(script, "OnlyComments.sql"));
+
+            Assert.Contains("empty", ex.Message);
+        }
+    }
+}

MigrationOps.Core.Tests/ExtractChecksumFromScriptTests.cs

@@ -0,0 +1,55 @@
+using MigrationOps.Core.MigrationFramework.Services;
+
+namespace MigrationOps.Core.Tests
+{
+    public class ExtractChecksumFromScriptTests
+    {
+        private readonly MigrationService _service = TestMigrationService.Create();
+
+        [Fact]
+        public void ExtractsChecksumValue()
+        {
+            var script = "-- Tags: Db1\n-- Checksum: abc123\nSELECT 1;";
+
+            Assert.Equal("abc123", _service.ExtractChecksumFromScript(script));
+        }
+
+        [Fact]
+        public void TrimsWhitespaceAroundChecksum()
+        {
+            var script = "-- Checksum:   abc123   \nSELECT 1;";
+
+            Assert.Equal("abc123", _service.ExtractChecksumFromScript(script));
+        }
+
+        [Fact]
+        public void HandlesWindowsLineEndings()
+        {
+            var script = "-- Tags: Db1\r\n-- Checksum: winval\r\nSELECT 1;";
+
+            Assert.Equal("winval", _service.ExtractChecksumFromScript(script));
+        }
+
+        [Fact]
+        public void HandlesUnixLineEndings()
+        {
+            var script = "-- Tags: Db1\n-- Checksum: unixval\nSELECT 1;";
+
+            Assert.Equal("unixval", _service.ExtractChecksumFromScript(script));
+        }
+
+        [Fact]
+        public void ThrowsWhenNoChecksumLinePresent()
+        {
+            var script = "-- Tags: Db1\nSELECT 1;";
+
+            Assert.Throws<InvalidOperationException>(() => _service.ExtractChecksumFromScript(script));
+        }
+
+        [Fact]
+        public void ThrowsForEmptyScript()
+        {
+            Assert.Throws<InvalidOperationException>(() => _service.ExtractChecksumFromScript(""));
+        }
+    }
+}