Skip to content

Commit c8619f9

Browse files
authored
Close legacy dry-run, empty-run and callback-context safety gaps
Defer legacy runner history access until navigation and use IMigrationHistory for read-only reads; constructors and DryRun no longer initialize history. Empty latest-version runs return before history access. Restore migration context around post-commit callbacks and clear it even when a callback fails. Update README scope semantics to match the explicitly requested effective-scope behavior. Validation: solution build, Unit 70 passed, SQLite 145 passed with one pre-existing default-removal skip handled in the provider layer. New regressions cover legacy navigation, empty assemblies, callback context, durable history and callback failure cleanup. Addresses reviews 4071830715, 4071830792 and 4071830843; documents the deliberate scope policy discussed in 4071742900.
1 parent 3fa1ac3 commit c8619f9

6 files changed

Lines changed: 85 additions & 16 deletions

File tree

‎README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,9 +180,9 @@ billingMigrator.MigrateToLastVersion();
180180

181181
Important details:
182182

183-
- A scope partitions **history**, not migration discovery. Passing a mixed assembly does not automatically filter it by scope.
184-
- Leave `MigrationAttribute.Scope` unset to inherit the provider scope. If you override it, it must align with the history the runner reads.
185-
- Duplicate versions are checked across the runner's whole loaded migration set. Separate scopes do not permit duplicate versions within one runner.
183+
- In the upgrade source, explicit scopes filter discovery; unscoped migrations inherit the runner scope. A scope partitions history, not database objects.
184+
- Leave `MigrationAttribute.Scope` unset to inherit the provider scope; set it to select a migration for one specific scope.
185+
- Duplicate versions are checked within the effective scope. Duplicate versions in distinct explicit scopes are independent.
186186
- Scopes do not isolate tables or data. Module migrations still need compatible table names and coordinated schema ownership.
187187

188188
See [ProviderFactory](src/Migrator/ProviderFactory.cs), [MigrationLoader](src/Migrator/MigrationLoader.cs) and [history implementation](src/Migrator/Providers/TransformationProvider.cs).

‎src/Migrator.Tests/RunnerSafetyTests.cs‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,47 @@ public class RunnerSafetyTests
2424
public override void Up() => throw new InvalidOperationException("original");
2525
public override void Down() => throw new InvalidOperationException("original");
2626
}
27+
[Migration(4, Ignore = true)] public class CallbackContext : One
28+
{
29+
public override void AfterUp()
30+
{
31+
if (!ReferenceEquals(((TransformationProvider)Database).CurrentMigration, this)) throw new InvalidOperationException("Missing callback context.");
32+
Database.ExecuteNonQuery("INSERT INTO Example VALUES (1)");
33+
}
34+
}
35+
[Migration(5, Ignore = true)] public class FailedCallbackContext : CallbackContext
36+
{ public override void AfterUp() { base.AfterUp(); throw new InvalidOperationException("Callback failed."); } }
37+
38+
[Test, Category("SQLite")] public void LegacyDryRunConstructionAndNavigationAreReadOnly()
39+
{
40+
using var connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True"); connection.Open();
41+
using var provider = ProviderFactory.Create(ProviderTypes.SQLite, connection, null);
42+
var legacy = new MigrateAnywhere(new List<long> { 1 }, provider, new Logger(false)) { DryRun = true };
43+
Assert.That(legacy.Current, Is.Zero);
44+
Assert.That(legacy.AppliedVersions, Is.Empty);
45+
Assert.That(legacy.Continue(1), Is.True);
46+
legacy.Migrate(new One { Database = provider });
47+
Assert.That(provider.TableExists("SchemaInfo"), Is.False);
48+
Assert.That(provider.TableExists("Example"), Is.False);
49+
Assert.That(((DotNetProjects.Migrator.Providers.Impl.SQLite.SQLiteTransformationProvider)provider).IsPragmaForeignKeysOn(), Is.True);
50+
}
51+
[Test, Category("SQLite")] public void EmptyLatestRunDoesNotCreateHistory()
52+
{
53+
using var provider = ProviderFactory.Create(ProviderTypes.SQLite, "Data Source=:memory:", null);
54+
new DotNetProjects.Migrator.Migrator(provider, false, Array.Empty<Type>()).MigrateToLastVersion();
55+
Assert.That(provider.TableExists("SchemaInfo"), Is.False);
56+
}
57+
[TestCase(false), TestCase(true), Category("SQLite")]
58+
public void PostCommitCallbackHasContextAndAlwaysClearsIt(bool fail)
59+
{
60+
using var provider = ProviderFactory.Create(ProviderTypes.SQLite, "Data Source=:memory:", null);
61+
var runner = new DotNetProjects.Migrator.Migrator(provider, false, fail ? typeof(FailedCallbackContext) : typeof(CallbackContext));
62+
if (fail) Assert.That(Assert.Throws<InvalidOperationException>(runner.MigrateToLastVersion).Message, Is.EqualTo("Callback failed."));
63+
else runner.MigrateToLastVersion();
64+
Assert.That(((TransformationProvider)provider).CurrentMigration, Is.Null);
65+
Assert.That(Convert.ToInt64(provider.ExecuteScalar("SELECT COUNT(*) FROM Example")), Is.EqualTo(1));
66+
Assert.That(provider.AppliedMigrations, Has.Count.EqualTo(1));
67+
}
2768
[Test] public void CustomProvidersRetainExplicitMigrationScope()
2869
{
2970
var provider = Substitute.For<ITransformationProvider>();

‎src/Migrator/BaseMigrate.cs‎

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
using System;
12
using System.Collections.Generic;
3+
using System.Linq;
24
using DotNetProjects.Migrator.Framework;
35

46
namespace DotNetProjects.Migrator;
@@ -11,24 +13,39 @@ public abstract class BaseMigrate
1113
protected bool _dryrun;
1214
protected ILogger _logger;
1315
protected List<long> _original;
16+
private bool _initialized;
1417

1518
protected BaseMigrate(List<long> availableMigrations, ITransformationProvider provider, ILogger logger)
1619
{
1720
_provider = provider;
18-
_availableMigrations = availableMigrations;
19-
_original = new List<long>(_provider.AppliedMigrations.ToArray()); //clone
21+
_availableMigrations = availableMigrations.OrderBy(version => version).ToList();
2022
_logger = logger;
2123
}
2224

25+
protected IReadOnlyList<long> ReadHistory()
26+
{
27+
if (_provider is IMigrationHistory history) return history.ReadAppliedMigrations();
28+
if (DryRun) throw new NotSupportedException("Legacy dry-run requires IMigrationHistory on custom providers.");
29+
return _provider.AppliedMigrations;
30+
}
31+
32+
private void InitializeHistory()
33+
{
34+
if (_initialized) return;
35+
_original = new List<long>(ReadHistory());
36+
_current = _original.DefaultIfEmpty(0).Max();
37+
_initialized = true;
38+
}
39+
2340
public List<long> AppliedVersions
2441
{
25-
get { return _original; }
42+
get { InitializeHistory(); return _original; }
2643
}
2744

2845
public virtual long Current
2946
{
30-
get { return _current; }
31-
protected set { _current = value; }
47+
get { InitializeHistory(); return _current; }
48+
protected set { InitializeHistory(); _current = value; }
3249
}
3350

3451
public virtual bool DryRun
@@ -61,12 +78,13 @@ public void Iterate()
6178
/// <returns>The migration number of the next available Migration.</returns>
6279
protected long NextMigration()
6380
{
81+
if (_availableMigrations.Count == 0) return 0;
6482
// Start searching at the current index
6583
var migrationSearch = _availableMigrations.IndexOf(Current) + 1;
6684

6785
// See if we can find a migration that matches the requirement
6886
while (migrationSearch < _availableMigrations.Count
69-
&& _provider.AppliedMigrations.Contains(_availableMigrations[migrationSearch]))
87+
&& ReadHistory().Contains(_availableMigrations[migrationSearch]))
7088
{
7189
migrationSearch++;
7290
}
@@ -93,7 +111,7 @@ protected long PreviousMigration()
93111

94112
// See if we can find a migration that matches the requirement
95113
while (migrationSearch > -1
96-
&& !_provider.AppliedMigrations.Contains(_availableMigrations[migrationSearch]))
114+
&& !ReadHistory().Contains(_availableMigrations[migrationSearch]))
97115
{
98116
migrationSearch--;
99117
}

‎src/Migrator/MigrateAnywhere.cs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,6 @@ public class MigrateAnywhere : BaseMigrate
1717
public MigrateAnywhere(List<long> availableMigrations, ITransformationProvider provider, ILogger logger)
1818
: base(availableMigrations, provider, logger)
1919
{
20-
_current = 0;
21-
if (provider.AppliedMigrations.Count > 0)
22-
{
23-
_current = provider.AppliedMigrations.Max();
24-
}
2520
_goForward = false;
2621
}
2722

@@ -47,6 +42,7 @@ public override long Previous
4742

4843
public override bool Continue(long version)
4944
{
45+
if (_availableMigrations.Count == 0) return false;
5046
// If we're going backwards and our current is less than the target,
5147
// reverse direction. Also, start over at zero to make sure we catch
5248
// any merged migrations that are less than the current target.

‎src/Migrator/MigrationExecution.cs‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,15 @@ internal static void Execute(ITransformationProvider provider, IMigration migrat
5353
}
5454
}
5555
// These callbacks intentionally run after commit; failure cannot be rolled back.
56-
if (step.IsUp) migration.AfterUp(); else migration.AfterDown();
56+
After(provider, migration, step.IsUp);
5757
}
58+
internal static void After(ITransformationProvider provider, IMigration migration, bool up)
59+
{
60+
var concrete = provider as TransformationProvider;
61+
var previous = concrete?.CurrentMigration;
62+
if (concrete != null) concrete.CurrentMigration = migration;
63+
try { if (up) migration.AfterUp(); else migration.AfterDown(); }
64+
finally { if (concrete != null) concrete.CurrentMigration = previous; }
65+
}
66+
5867
}

‎src/Migrator/Migrator.cs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,11 @@ public long? LastAppliedMigrationVersion
182182
/// </summary>
183183
public void MigrateToLastVersion()
184184
{
185+
if (_migrationLoader.GetAvailableMigrations().Count == 0)
186+
{
187+
Logger.Warn("No migrations found for the effective scope.");
188+
return;
189+
}
185190
MigrateTo(_migrationLoader.LastVersion);
186191
}
187192

0 commit comments

Comments
 (0)