From a607a752c9a35ca0b8010e92b6ac9eb26a123ac0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Sat, 14 Nov 2026 11:53:18 +0100 Subject: [PATCH] fix: a rename in place whose name is held when it runs is a rebuild A contributor's change in place runs after EF Core's operations. An index EF Core created under the name a constraint gave up ran while the constraint still held it, and the migration failed with 42P07; two constraints on two tables of one schema swapping names failed the same way. The planner now compares the name a rename gives up and the name it takes, through NameClaims, with what holds a name at that moment: EF Core's objects as the target has them, the declared objects as EF Core's operations leave them but for those dropped before or lost, and the earlier changes in place under their new names. A change that collides is rebuilt: dropped before EF Core's operations and created after them, with its extension ensured. The contributor's statements for it never reach the migration. A name EF Core gives up is free by then, as the server confirms. Closes #99 Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 8 + .../Internal/SchemaKitStrings.cs | 5 + .../Migrations/Internal/NameClaims.cs | 46 +++-- .../Internal/SchemaOperationPlanner.cs | 106 ++++++++++- .../Sql/SchemaSql.cs | 3 + .../Harness/Models.cs | 140 +++++++++++++++ .../Integration/PgExclusionLiveTests.cs | 90 ++++++++++ .../PgExclusionMigrationTests.cs | 147 ++++++++++++++++ .../Harness/SampleContract.cs | 5 +- .../Migrations/RenameInPlaceTests.cs | 166 ++++++++++++++++++ 10 files changed, 703 insertions(+), 13 deletions(-) create mode 100644 test/CodoMetis.EFCore.SchemaKit.Tests/Migrations/RenameInPlaceTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 7a83d09..c1d09aa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,6 +224,14 @@ contributor's code, and a hole takes only `Identifier`, `TableRef`, `QualifiedNa - **Order:** drops, EF Core's operations untouched, in-place statements, creates; contributors by placement, and a `BeforeTables` contributor the other way round (created before, dropped after, never rebuilt). +- **A rename in place whose name is held when it runs is a rebuild** (`KeepInPlace`, #99). A + change in place runs after EF Core's operations: an index EF Core creates under the name a + constraint gives up fails with 42P07. Both names are compared, through `NameClaims`, with what + holds a name at that moment: EF Core's objects as the target has them, the declared objects as + EF Core's operations leave them but for those dropped before or lost, and the earlier changes in + place under their new names. A name EF Core gives up is free by then, and the server agrees. The + contributor's statements go into a builder of their own and reach the migration only if the + change stays in place. It is a point fix; the ledger over ordered actions is #115. - **A declaration whose owner has no contributor is refused,** on either side. #22 adds the explicit way out. - **Which model is the consumer's is told by reference, never by where it stands in a diff** diff --git a/src/CodoMetis.EFCore.SchemaKit/Infrastructure/Internal/SchemaKitStrings.cs b/src/CodoMetis.EFCore.SchemaKit/Infrastructure/Internal/SchemaKitStrings.cs index 99d6f4c..d421d7b 100644 --- a/src/CodoMetis.EFCore.SchemaKit/Infrastructure/Internal/SchemaKitStrings.cs +++ b/src/CodoMetis.EFCore.SchemaKit/Infrastructure/Internal/SchemaKitStrings.cs @@ -285,6 +285,11 @@ public static string RebuildBeforeTables(Type contributor, QualifiedName name) => $"'{contributor.FullName}'.Plan answered Rebuild for '{name}', and its placement is BeforeTables: tables are " + "made of its objects, so one cannot be dropped and created again around them. Answer InPlace, or change the object under another name."; + public static string RenameBeforeTablesCollides(Type contributor, QualifiedName from, QualifiedName to) + => $"'{contributor.FullName}'.Plan answered InPlace for '{from}' becoming '{to}', and the migration has something else " + + "hold one of the two names while the change runs. An object of another kind would be rebuilt around it, and this " + + "one's placement is BeforeTables: tables are made of it. Move the rename to a migration of its own."; + public static string UnknownOperation(Type operation) => $"The migration holds an operation of type '{operation.FullName}', which SchemaKit does not know. What it does " + "to the schema objects this model declares cannot be told, and one of them could be dropped or left broken " + diff --git a/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/NameClaims.cs b/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/NameClaims.cs index 2e6f2e6..9f8960a 100644 --- a/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/NameClaims.cs +++ b/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/NameClaims.cs @@ -21,7 +21,12 @@ namespace CodoMetis.EFCore.SchemaKit.Migrations.Internal; internal static class NameClaims { /// A name taken, by whom, and how that party's names compare. - private sealed record Taken(NameClaim Claim, string By, object Party, bool Ours, SchemaNameComparison Comparison); + /// The name, and the namespace it is taken in. + /// The party, as a message names it. + /// The party itself: two claims of one party never collide with each other. + /// Whether the party is a declaration, not one of EF Core's own objects. + /// How the party's names compare. + internal sealed record Taken(NameClaim Claim, string By, object Party, bool Ours, SchemaNameComparison Comparison); /// Throws with every collision between the declared objects, and between one of them and one of EF Core's. public static void Require( @@ -31,11 +36,7 @@ public static void Require( { List taken = []; foreach (var (contributor, descriptor, entityType, declaration) in declared) - { - var by = $"{declaration.Owner} {declaration.Kind} '{descriptor.Name}' on {entityType.ShortName()}"; - foreach (var claim in contributor.Claims(descriptor).Distinct()) - taken.Add(new Taken(claim, by, descriptor, Ours: true, declaration.NameComparison)); - } + taken.AddRange(Of(contributor, descriptor, entityType, declaration, party: descriptor)); var ours = taken.Count; taken.AddRange(OfTheModel(model, dialect)); @@ -47,10 +48,7 @@ public static void Require( for (var j = i + 1; j < taken.Count; j++) { var (first, second) = (taken[i], taken[j]); - var comparison = first.Comparison == SchemaNameComparison.IgnoreCase || second.Comparison == SchemaNameComparison.IgnoreCase - ? StringComparison.OrdinalIgnoreCase - : StringComparison.Ordinal; - if (!Same(first.Claim, second.Claim, comparison, dialect.DefaultSchema)) continue; + if (!Collide(first, second, dialect)) continue; // Two objects that collide in two namespaces, as two exclusion constraints of one // name do, are one finding. @@ -63,8 +61,34 @@ public static void Require( if (collisions.Count > 0) throw new SchemaValidationException(collisions); } + /// What a declared object takes, as its contributor says. + /// The object's contributor, which says what it claims. + /// The object. + /// The entity type it is declared on, for the message. + /// Its declaration, for the message and how its names compare. + /// What stands for the object: claims of one party do not collide with each other. + public static IEnumerable Of(ISchemaContributor contributor, ISchemaDescriptor descriptor, IReadOnlyEntityType entityType, ISchemaObject declaration, object party) + { + var by = $"{declaration.Owner} {declaration.Kind} '{descriptor.Name}' on {entityType.ShortName()}"; + return contributor.Claims(descriptor).Distinct().Select(claim => new Taken(claim, by, party, Ours: true, declaration.NameComparison)); + } + + /// + /// Whether two parties' claims are one name in one namespace: under the stricter comparison of + /// the two, unless either party's names compare without regard to case. + /// + public static bool Collide(Taken first, Taken second, ISchemaDialect dialect) + { + if (ReferenceEquals(first.Party, second.Party)) return false; + + var comparison = first.Comparison == SchemaNameComparison.IgnoreCase || second.Comparison == SchemaNameComparison.IgnoreCase + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + return Same(first.Claim, second.Claim, comparison, dialect.DefaultSchema); + } + /// What EF Core's own objects take, each asked of the dialect. - private static IEnumerable OfTheModel(IRelationalModel model, ISchemaDialect dialect) + public static IEnumerable OfTheModel(IRelationalModel model, ISchemaDialect dialect) { foreach (var table in model.Tables) { diff --git a/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/SchemaOperationPlanner.cs b/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/SchemaOperationPlanner.cs index 0af44ca..4b49fca 100644 --- a/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/SchemaOperationPlanner.cs +++ b/src/CodoMetis.EFCore.SchemaKit/Migrations/Internal/SchemaOperationPlanner.cs @@ -135,6 +135,7 @@ public SchemaOperationPlan Plan(IRelationalModel? source, IRelationalModel? targ changed.AddRange(Pair(from, to, (s, t) => s.SameButForTheNameAs(t))); var inPlace = new SchemaSql(Dialect); + List<(Resolved Was, Resolved Becomes, SchemaSql Statements)> inPlaceChanges = []; HashSet dropped = new(from, ReferenceEqualityComparer.Instance); HashSet created = new(to, ReferenceEqualityComparer.Instance); @@ -172,10 +173,16 @@ public SchemaOperationPlan Plan(IRelationalModel? source, IRelationalModel? targ continue; } - switch (was.Contributor.Plan(was.Descriptor, becomes.Descriptor, inPlace)) + // The answer's statements are kept only if the change stays in place: the planner can + // still turn it into a rebuild below, and then they must not reach the migration. + var statements = new SchemaSql(Dialect); + switch (was.Contributor.Plan(was.Descriptor, becomes.Descriptor, statements)) { case SchemaChange.None: + break; + case SchemaChange.InPlace: + inPlaceChanges.Add((was, becomes, statements)); break; case SchemaChange.Rebuild: @@ -190,6 +197,11 @@ public SchemaOperationPlan Plan(IRelationalModel? source, IRelationalModel? targ } } + // A change in place runs after EF Core's operations, where a name it gives up or takes may + // be held by something else; such a change is rebuilt instead (#99). + if (inPlaceChanges.Count > 0) + KeepInPlace(inPlaceChanges, sourceOrder, dropped, created, target!, inPlace); + List drops = [.. sourceOrder.Where(dropped.Contains)]; List creates = [.. targetOrder.Where(created.Contains)]; @@ -227,6 +239,98 @@ public SchemaOperationPlan Plan(IRelationalModel? source, IRelationalModel? targ return new SchemaOperationPlan(before.Operations, [.. inPlace.Operations, .. after.Operations]); } + /// + /// Writes the changes in place whose names are free when they run, in the order they run, and + /// makes each of the others a rebuild (#99). + /// + /// + /// + /// A change in place runs after EF Core's operations and before the creates. When it renames an + /// object, the name it gives up and the name it takes are compared, through the claims and the + /// namespace rules of , with what holds a name at that moment: EF Core's + /// objects as the target has them, since all of EF Core's operations have run; every declared + /// object of the source as EF Core's operations leave it, but for those dropped before them or + /// lost with them (what tables are made of is dropped last, so it is still there); and the + /// changes in place written before this one, under their new names. A name EF Core takes is the + /// failure of #99: CREATE INDEX under the name a constraint gives up runs while the + /// constraint still holds it. A name another rename still holds is the same failure among + /// declarations, which is how two constraints on two tables of one schema swapping names + /// failed. + /// + /// + /// A change that collides is rebuilt: the drop frees the old name before EF Core's operations, + /// the create takes the new one after them and after every change in place, and the create + /// ensures its extensions like any other. No temporary name is used. A name EF Core gives up is + /// no collision: its operation has run, which is why a rename into the name of an index EF Core + /// drops or renames in the same migration stays a rename, as the server confirms. + /// + /// + /// This is a point fix. It reasons about one moment, the changes in place, with everything else + /// in its phase as a whole: a name an object's create takes against EF Core's operations, a + /// BeforeTables create against an EnsureSchema, and an order among creates or among + /// drops are not looked at here. The general answer is a ledger over the ordered actions (#115). + /// + /// + private void KeepInPlace( + List<(Resolved Was, Resolved Becomes, SchemaSql Statements)> changes, + List sources, + HashSet dropped, + HashSet created, + IRelationalModel target, + SchemaSql inPlace) + { + List? held = null; + foreach (var (was, becomes, statements) in changes) + { + List gives = [.. Claims(was)]; + List takes = [.. NameClaims.Of(becomes.Contributor, becomes.Descriptor, becomes.EntityType, becomes.Declaration, party: was)]; + + // A change that names nothing anew holds what it held: nothing to compare. + if (gives.Select(taken => taken.Claim).ToHashSet().SetEquals(takes.Select(taken => taken.Claim))) + { + inPlace.Append(statements); + continue; + } + + held ??= Held(sources, dropped, target); + if (gives.Concat(takes).Any(claim => held.Any(other => NameClaims.Collide(claim, other, Dialect)))) + { + if (was.Contributor.Placement.IsBeforeTables) + throw new InvalidOperationException(SchemaKitStrings.RenameBeforeTablesCollides(was.Contributor.GetType(), was.Descriptor.Name, becomes.Descriptor.Name)); + + dropped.Add(was); + created.Add(becomes); + held.RemoveAll(taken => ReferenceEquals(taken.Party, was)); + continue; + } + + inPlace.Append(statements); + held.RemoveAll(taken => ReferenceEquals(taken.Party, was)); + held.AddRange(takes); + } + } + + /// What holds a name when the changes in place begin: EF Core's objects as the target has them, and the declared objects still there. + private List Held(List sources, HashSet dropped, IRelationalModel target) + { + List held = [.. NameClaims.OfTheModel(target, Dialect)]; + foreach (var source in sources) + { + // Gone with an operation of EF Core's, or dropped before them. What tables are made of + // is dropped after everything else, so it holds its name throughout. + if (source.Impact is Impact.Lost) continue; + if (dropped.Contains(source) && !source.Contributor.Placement.IsBeforeTables) continue; + + held.AddRange(Claims(source)); + } + + return held; + } + + /// What an object of the source takes as EF Core's operations leave it: under the new names of its table and columns, and its own name still. + private static IEnumerable Claims(Resolved source) + => NameClaims.Of(source.Contributor, source.Descriptor, source.EntityType, source.Declaration, party: source); + /// /// An object of the source side as EF Core's operations leave it. The contributor is asked /// about each change that touches what the object depends on, with the object as it is now; diff --git a/src/CodoMetis.EFCore.SchemaKit/Sql/SchemaSql.cs b/src/CodoMetis.EFCore.SchemaKit/Sql/SchemaSql.cs index 5740ced..1abc26d 100644 --- a/src/CodoMetis.EFCore.SchemaKit/Sql/SchemaSql.cs +++ b/src/CodoMetis.EFCore.SchemaKit/Sql/SchemaSql.cs @@ -31,6 +31,9 @@ public sealed partial class SchemaSql /// The statements so far, as the operations a migration carries. internal IReadOnlyList Operations => _operations; + /// Adds another builder's statements, in their order: a contributor's answer the planner keeps. + internal void Append(SchemaSql other) => _operations.AddRange(other._operations); + /// /// Adds one statement. The provider's statement terminator is appended; do not write one. /// diff --git a/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Harness/Models.cs b/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Harness/Models.cs index 422f3f7..7dc4678 100644 --- a/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Harness/Models.cs +++ b/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Harness/Models.cs @@ -530,4 +530,144 @@ public static void Build(ModelBuilder modelBuilder) .DeclarePostgres(pg => [pg.Exclusion(pg.Equal(x => x.RoomId).Overlaps(x => x.Period), name: TwoSchemas.Shared)]); } } + + // ── A name a rename frees or takes in the migration that renames (#99) ── + + /// The default name has, before renames a column it is built from. + public const string UnnamedName = "EX_role_grants_GranteeId_lowerNote_Period"; + + /// , and EF Core creates an index under the name the constraint gives up. + public sealed class RenamedAndItsOldNameReused : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + var grants = Grants(modelBuilder); + grants.HasIndex(x => x.RoleId).HasDatabaseName(Name); + grants.DeclarePostgres(_ => [ActivePeriod with { Name = "ex_renamed" }]); + } + } + + /// , and EF Core creates an index under the default name the constraint gives up. + public sealed class UnnamedRenamedAroundAndItsOldNameReused : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + var grants = Grants(modelBuilder); + grants.Property(x => x.GranteeId).HasColumnName("grantee"); + grants.HasIndex(x => x.RoleId).HasDatabaseName(UnnamedName); + grants.DeclarePostgres(pg => [pg.Exclusion(pg.Equal(x => x.GranteeId).Equal(x => x.Note!.ToLower()).Overlaps(x => x.Period))]); + } + } + + /// beside an index of EF Core's that has the name gives the constraint. + public sealed class SchedulingBesideAnIndexNamedLikeItsNextName : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + var grants = Grants(modelBuilder); + grants.HasIndex(x => x.RoleId).HasDatabaseName("ex_renamed"); + grants.DeclarePostgres(_ => [ActivePeriod]); + } + } + + /// The constraint takes the name EF Core's index gives up, which EF Core renames. + public sealed class RenamedIntoTheNameAnIndexGaveUp : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + var grants = Grants(modelBuilder); + grants.HasIndex(x => x.RoleId).HasDatabaseName("ix_role"); + grants.DeclarePostgres(_ => [ActivePeriod with { Name = "ex_renamed" }]); + } + } + + /// + /// while EF Core moves its table to another schema, and an index of EF + /// Core's in the schema the table leaves takes the constraint's old name. The constraint's + /// index has moved with its table by the time the index is created: nothing collides. + /// + public sealed class RenamedOnAMovedTableAndItsOldNameReusedWhereItWas : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().ToTable("role_grants", "ops").DeclarePostgres(_ => [ActivePeriod with { Name = "ex_renamed" }]); + modelBuilder.Entity().ToTable("bookings").HasIndex(x => x.RoomId).HasDatabaseName(Name); + } + } + + /// The same move, and the index in the schema the table moves to: there the old name is still held. + public sealed class RenamedOnAMovedTableAndItsOldNameReusedWhereItGoes : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().ToTable("role_grants", "ops").DeclarePostgres(_ => [ActivePeriod with { Name = "ex_renamed" }]); + modelBuilder.Entity().ToTable("bookings", "ops").HasIndex(x => x.RoomId).HasDatabaseName(Name); + } + } + + /// + /// The same table rename, and an index of EF Core's on the renamed table under the + /// constraint's old name: the index and the constraint's index are relations of one schema, + /// whatever their tables are called. + /// + public sealed class RenamedOnARenamedTableAndItsOldNameReused : ITestModel + { + public static void Build(ModelBuilder modelBuilder) + { + var grants = modelBuilder.Entity().ToTable("grants"); + grants.HasIndex(x => x.RoleId).HasDatabaseName(Name); + grants.DeclarePostgres(_ => [ActivePeriod with { Name = "ex_renamed" }]); + } + } + + private static PgExclusion OnGrants(PgSchema pg, string name) => pg.Exclusion(pg.Overlaps(x => x.Period), name: name); + + private static PgExclusion OnGrantsByRole(PgSchema pg, string name) => pg.Exclusion(pg.Equal(x => x.RoleId).Overlaps(x => x.Period), name: name); + + private static PgExclusion OnBookings(PgSchema pg, string name) => pg.Exclusion(pg.Overlaps(x => x.Period), name: name); + + /// Two tables of one schema, a constraint on each: their indexes are relations of that schema. + public sealed class TwoTables : ITestModel where TNames : INames + { + public static void Build(ModelBuilder modelBuilder) + { + Grants(modelBuilder).DeclarePostgres(pg => [OnGrants(pg, TNames.First)]); + modelBuilder.Entity().ToTable("bookings").DeclarePostgres(pg => [OnBookings(pg, TNames.Second)]); + } + } + + /// Two constraints on one table. + public sealed class OneTable : ITestModel where TNames : INames + { + public static void Build(ModelBuilder modelBuilder) + => Grants(modelBuilder).DeclarePostgres(pg => [OnGrants(pg, TNames.First), OnGrantsByRole(pg, TNames.Second)]); + } + + public interface INames + { + static abstract string First { get; } + + static abstract string Second { get; } + } + + public sealed class AB : INames + { + public static string First => "ex_a"; + + public static string Second => "ex_b"; + } + + public sealed class BA : INames + { + public static string First => "ex_b"; + + public static string Second => "ex_a"; + } + + public sealed class BC : INames + { + public static string First => "ex_b"; + + public static string Second => "ex_c"; + } } diff --git a/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Integration/PgExclusionLiveTests.cs b/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Integration/PgExclusionLiveTests.cs index 51d8e40..e1ee230 100644 --- a/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Integration/PgExclusionLiveTests.cs +++ b/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/Integration/PgExclusionLiveTests.cs @@ -555,4 +555,94 @@ public void A_name_that_is_taken_fails_and_replaces_nothing() new[] { """public.role_grants | ex_role_grant_active_period | index ex_role_grant_active_period | EXCLUDE USING gist ("Period" WITH &&)""" }, database.Exclusions()); } + + // ── A name a rename frees or takes in the same migration (#99) ── + // + // A rename in place runs after EF Core's operations. Each pair here is applied both ways: one + // direction frees a name EF Core takes, the other takes a name EF Core gives up. + + /// EF Core creates an index under the name the constraint gives up: the rename becomes a drop before and an add after. + [TestMethod] + public void An_index_takes_the_name_a_renamed_constraint_gives_up() + => AssertMigratesTo(); + + /// + /// The other way: the constraint takes the name of an index EF Core drops. EF Core's drop runs + /// first, so the rename finds the name free, and the constraint stays the one it was. + /// + [TestMethod] + public void A_renamed_constraint_takes_the_name_of_an_index_that_is_dropped() + { + AssertMigratesTo(); + + using var database = new LiveDatabase(); + database.Create(); + var oid = database.ConstraintOid("ex_renamed"); + database.Migrate(); + Assert.AreEqual(oid, database.ConstraintOid(Models.Name)); + } + + [TestMethod] + public void An_index_takes_the_default_name_a_column_rename_gives_up() + => AssertMigratesTo(); + + [TestMethod] + public void A_column_rename_gives_a_constraint_the_default_name_of_an_index_that_is_dropped() + => AssertMigratesTo(); + + /// The constraint takes the name of an index EF Core renames: EF Core's rename runs first. + [TestMethod] + public void A_renamed_constraint_takes_the_name_of_an_index_that_is_renamed() + { + AssertMigratesTo(); + + using var database = new LiveDatabase(); + database.Create(); + var oid = database.ConstraintOid(Models.Name); + database.Migrate(); + Assert.AreEqual(oid, database.ConstraintOid("ex_renamed")); + } + + /// EF Core renames an index into the name a renamed constraint gives up. + [TestMethod] + public void An_index_is_renamed_into_the_name_a_renamed_constraint_gives_up() + => AssertMigratesTo(); + + /// The names are compared as EF Core's table rename leaves them: an index on the renamed table takes the old name. + [TestMethod] + public void A_table_rename_does_not_hide_a_collision() + { + AssertMigratesTo(); + AssertMigratesTo(); + } + + /// + /// The schemas are compared as EF Core's move leaves them: the old name is free in the schema + /// the table leaves, so the rename stays one, and held in the one it goes to. Up only: a move + /// back to a table without a schema does not reach the server on Npgsql (#105). + /// + [TestMethod] + public void A_move_to_another_schema_neither_invents_a_collision_nor_hides_one() + { + AssertMigratesTo(); + AssertMigratesTo(); + } + + /// Two constraints on two tables of a schema swap names, and a rename chain runs through them, both ways. + [TestMethod] + public void Constraints_on_two_tables_swap_names_and_pass_one_on() + { + AssertMigratesTo, Models.TwoTables>(); + AssertMigratesTo, Models.TwoTables>(); + AssertMigratesTo, Models.TwoTables>(); + } + + /// The same on one table, where the review found today's order sound: it stays so. + [TestMethod] + public void Constraints_on_one_table_swap_names_and_pass_one_on() + { + AssertMigratesTo, Models.OneTable>(); + AssertMigratesTo, Models.OneTable>(); + AssertMigratesTo, Models.OneTable>(); + } } diff --git a/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/PgExclusionMigrationTests.cs b/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/PgExclusionMigrationTests.cs index c9a3064..7a5049a 100644 --- a/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/PgExclusionMigrationTests.cs +++ b/test/CodoMetis.EFCore.SchemaKit.PostgreSQL.Tests/PgExclusionMigrationTests.cs @@ -455,4 +455,151 @@ public void A_drop_names_the_table_with_its_schema() => CollectionAssert.AreEqual( new[] { """ALTER TABLE grants.role_grants DROP CONSTRAINT ex_no_overlap;""" }, Contexts.Diff().Shape()); + + // ── A name a rename frees or takes in the same migration (#99) ── + + private const string AddRenamed = + """ALTER TABLE role_grants ADD CONSTRAINT ex_renamed EXCLUDE USING gist ("GranteeId" WITH =, "RoleId" WITH =, "Period" WITH &&) WHERE ("RevokedOn" IS NULL) DEFERRABLE INITIALLY DEFERRED;"""; + + /// + /// EF Core creates an index under the name the constraint gives up. A rename runs after + /// EF Core's operations, where the name is still the constraint's; a drop runs before them and + /// an add after. The add brings its extension like any other. + /// + [TestMethod] + public void A_rename_whose_old_name_an_index_takes_is_a_rebuild() + => CollectionAssert.AreEqual( + new[] { Extension, Drop, "CreateIndexOperation", AddRenamed }, + Contexts.Diff().Shape()); + + /// The same with a default name that a column rename changes. + [TestMethod] + public void A_default_name_a_column_rename_gives_up_to_an_index_is_a_rebuild() + => CollectionAssert.AreEqual( + new[] + { + Extension, + """ALTER TABLE role_grants DROP CONSTRAINT "EX_role_grants_GranteeId_lowerNote_Period";""", + "RenameColumnOperation", + "CreateIndexOperation", + """ALTER TABLE role_grants ADD CONSTRAINT "EX_role_grants_grantee_lowerNote_Period" EXCLUDE USING gist (grantee WITH =, (lower("Note")) WITH =, "Period" WITH &&);""", + }, + Contexts.Diff().Shape()); + + /// + /// The other way round: the constraint takes the name of an index EF Core drops or renames. + /// EF Core's operation runs first, so the name is free and the rename stays one statement. + /// + [TestMethod] + public void A_rename_into_a_name_an_index_gives_up_stays_a_rename() + { + CollectionAssert.AreEqual( + new[] { "DropIndexOperation", $"ALTER TABLE role_grants RENAME CONSTRAINT ex_renamed TO {Models.Name};" }, + Contexts.Diff().Shape()); + CollectionAssert.AreEqual( + new[] { "DropIndexOperation", "RenameColumnOperation", """ALTER TABLE role_grants RENAME CONSTRAINT "EX_role_grants_grantee_lowerNote_Period" TO "EX_role_grants_GranteeId_lowerNote_Period";""" }, + Contexts.Diff().Shape()); + CollectionAssert.AreEqual( + new[] { "RenameIndexOperation", $"ALTER TABLE role_grants RENAME CONSTRAINT {Models.Name} TO ex_renamed;" }, + Contexts.Diff().Shape()); + } + + /// EF Core renames an index into the name the constraint gives up: as a create would, it needs the name free. + [TestMethod] + public void A_rename_whose_old_name_an_index_is_renamed_into_is_a_rebuild() + => CollectionAssert.AreEqual( + new[] { Extension, "ALTER TABLE role_grants DROP CONSTRAINT ex_renamed;", "RenameIndexOperation", Add }, + Contexts.Diff().Shape()); + + /// Compared as EF Core's table rename leaves them, the index on the renamed table and the constraint's old name collide. + [TestMethod] + public void A_table_rename_does_not_hide_a_collision() + => CollectionAssert.AreEqual( + new[] + { + Extension, Drop, "DropPrimaryKeyOperation", "RenameTableOperation", "AddPrimaryKeyOperation", "CreateIndexOperation", + AddRenamed.Replace("ALTER TABLE role_grants", "ALTER TABLE grants"), + }, + Contexts.Diff().Shape()); + + /// + /// The table moves to another schema and its constraint's index with it, before EF Core creates + /// an index under the old name: where the table was, the name is free; where it goes, it is not. + /// + [TestMethod] + public void A_move_to_another_schema_is_compared_where_the_table_is_by_then() + { + CollectionAssert.AreEqual( + new[] + { + "EnsureSchemaOperation", "RenameTableOperation", "CreateTableOperation", "CreateIndexOperation", + $"ALTER TABLE ops.role_grants RENAME CONSTRAINT {Models.Name} TO ex_renamed;", + }, + Contexts.Diff().Shape()); + CollectionAssert.AreEqual( + new[] + { + Extension, Drop, "EnsureSchemaOperation", "RenameTableOperation", "CreateTableOperation", "CreateIndexOperation", + AddRenamed.Replace("ALTER TABLE role_grants", "ALTER TABLE ops.role_grants"), + }, + Contexts.Diff().Shape()); + } + + /// + /// Two constraints on two tables of a schema swap names. Renamed in turn, the first would take + /// a name the second still holds; so the second is rebuilt around the first's rename. + /// + [TestMethod] + public void Constraints_on_two_tables_that_swap_names_are_renamed_once_and_rebuilt_once() + => CollectionAssert.AreEqual( + new[] + { + "ALTER TABLE bookings DROP CONSTRAINT ex_b;", + "ALTER TABLE role_grants RENAME CONSTRAINT ex_a TO ex_b;", + """ALTER TABLE bookings ADD CONSTRAINT ex_a EXCLUDE USING gist ("Period" WITH &&);""", + }, + Contexts.Diff, Models.TwoTables>().Shape()); + + /// A rename chain whose renames run in an order that frees each name first stays renames; the other order rebuilds the one that would wait. + [TestMethod] + public void A_rename_chain_over_two_tables_is_renamed_where_the_order_allows() + { + CollectionAssert.AreEqual( + new[] { "ALTER TABLE bookings RENAME CONSTRAINT ex_b TO ex_c;", "ALTER TABLE role_grants RENAME CONSTRAINT ex_a TO ex_b;" }, + Contexts.Diff, Models.TwoTables>().Shape()); + CollectionAssert.AreEqual( + new[] + { + "ALTER TABLE bookings DROP CONSTRAINT ex_c;", + "ALTER TABLE role_grants RENAME CONSTRAINT ex_b TO ex_a;", + """ALTER TABLE bookings ADD CONSTRAINT ex_b EXCLUDE USING gist ("Period" WITH &&);""", + }, + Contexts.Diff, Models.TwoTables>().Shape()); + } + + /// On one table a name is matched before a rename is: a swap and a chain are rebuilds already, as before #99. + [TestMethod] + public void Constraints_on_one_table_that_swap_names_or_pass_one_on_are_rebuilt_as_before() + { + CollectionAssert.AreEqual( + new[] + { + Extension, + "ALTER TABLE role_grants DROP CONSTRAINT ex_a;", + "ALTER TABLE role_grants DROP CONSTRAINT ex_b;", + """ALTER TABLE role_grants ADD CONSTRAINT ex_b EXCLUDE USING gist ("Period" WITH &&);""", + """ALTER TABLE role_grants ADD CONSTRAINT ex_a EXCLUDE USING gist ("RoleId" WITH =, "Period" WITH &&);""", + }, + Contexts.Diff, Models.OneTable>().Shape()); + CollectionAssert.AreEqual( + new[] + { + Extension, + "ALTER TABLE role_grants DROP CONSTRAINT ex_a;", + "ALTER TABLE role_grants DROP CONSTRAINT ex_b;", + """ALTER TABLE role_grants ADD CONSTRAINT ex_b EXCLUDE USING gist ("Period" WITH &&);""", + """ALTER TABLE role_grants ADD CONSTRAINT ex_c EXCLUDE USING gist ("RoleId" WITH =, "Period" WITH &&);""", + }, + Contexts.Diff, Models.OneTable>().Shape()); + } } diff --git a/test/CodoMetis.EFCore.SchemaKit.Tests/Harness/SampleContract.cs b/test/CodoMetis.EFCore.SchemaKit.Tests/Harness/SampleContract.cs index 3c48d68..90675f2 100644 --- a/test/CodoMetis.EFCore.SchemaKit.Tests/Harness/SampleContract.cs +++ b/test/CodoMetis.EFCore.SchemaKit.Tests/Harness/SampleContract.cs @@ -318,7 +318,10 @@ protected override SchemaChange Plan(TestDomainDescriptor from, TestDomainDescri protected override Impact ImpactOf(TestDomainDescriptor descriptor, BaseChange change) => ImpactOverride?.Invoke(change) ?? new Impact.Unaffected(); - protected override IEnumerable Claims(TestDomainDescriptor descriptor) => []; + /// What answers in place of nothing, for the tests that need a domain to hold a name. + public Func>? ClaimsOverride { get; set; } + + protected override IEnumerable Claims(TestDomainDescriptor descriptor) => ClaimsOverride?.Invoke(descriptor) ?? []; } /// A second contributor for the owner has. diff --git a/test/CodoMetis.EFCore.SchemaKit.Tests/Migrations/RenameInPlaceTests.cs b/test/CodoMetis.EFCore.SchemaKit.Tests/Migrations/RenameInPlaceTests.cs new file mode 100644 index 0000000..a1aede9 --- /dev/null +++ b/test/CodoMetis.EFCore.SchemaKit.Tests/Migrations/RenameInPlaceTests.cs @@ -0,0 +1,166 @@ +using CodoMetis.EFCore.SchemaKit.Contributors.Internal; +using CodoMetis.EFCore.SchemaKit.Tests.Harness; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Migrations; + +namespace CodoMetis.EFCore.SchemaKit.Tests.Migrations; + +/// Models in which a declared object is renamed and EF Core takes or gives up one of its two names in the same migration. +public static class RenameModels +{ + private static readonly TestGuard.Unique Name = new([new GuardKey("Name")]) { Name = "ux_name" }; + + /// The sample guard's name is taken among the relations of the schema, as a table's is. + public sealed class RenamedAndItsOldNameATable : IPlanModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Metadata.Declare([Name with { Name = "ux_name2" }]); + modelBuilder.Entity().ToTable("ux_name"); + } + } + + public sealed class GuardedBesideATableNamedLikeItsNextName : IPlanModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Metadata.Declare([Name]); + modelBuilder.Entity().ToTable("ux_name2"); + } + } + + /// Another guard holds the name the rename takes, on another table. + public sealed class GuardedBesideAGuardNamedLikeItsNextName : IPlanModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Metadata.Declare([Name]); + modelBuilder.Entity().Metadata.Declare([new TestGuard.Unique([new GuardKey("Label")]) { Name = "ux_name2" }]); + } + } + + /// , and the table the other guard was on stays without it. + public sealed class RenamedBesideAnUnguardedGadget : IPlanModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Metadata.Declare([Name with { Name = "ux_name2" }]); + modelBuilder.Entity(); + } + } + + /// + /// The guard on gives its name up to the one on , and + /// EF Core alters the column it is on. + /// + public sealed class RenamedIntoTheNameOfAGuardOnAnAlteredColumn : IPlanModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Metadata.Declare([Name with { Name = "ux_name2" }]); + var gadget = modelBuilder.Entity(); + gadget.Property(x => x.Label).HasMaxLength(40); + gadget.Metadata.Declare([new TestGuard.Unique([new GuardKey("Label")]) { Name = "ux_gadget" }]); + } + } + + /// A domain is made before the tables. Renamed while a new table takes its old name, it cannot be rebuilt around the table. + public sealed class DomainRenamedAndItsOldNameATable : IPlanModel + { + public static void Build(ModelBuilder modelBuilder) + { + modelBuilder.Entity().Metadata.Declare([new TestDomain("integer") { Name = "dom2" }]); + modelBuilder.Entity().ToTable("dom"); + } + } +} + +/// +/// A change in place runs after EF Core's operations. A rename whose old name EF Core takes in the +/// same migration, or whose new name something still holds then, is rebuilt: the drop runs before +/// EF Core's operations and the create after them (#99). The rule is the kernel's, so it holds on +/// every provider, through the claims of NameClaims. +/// +[TestClass] +[DoNotParallelize] +public sealed class RenameInPlaceTests +{ + /// Each statement by its first two words, each operation of EF Core's by its type. + private static List Diff(Provider provider, Action? arrange = null) + where TFrom : IPlanModel + where TTo : IPlanModel + { + using var from = SchemaOperationPlannerTests.Context(provider); + using var to = SchemaOperationPlannerTests.Context(provider); + arrange?.Invoke(to); + + return [.. SchemaOperationPlannerTests.Describe(to.GetService().GetDifferences(SchemaOperationPlannerTests.Model(from), SchemaOperationPlannerTests.Model(to))) + .Select(described => string.Join(' ', described.Split(' ').Take(2)))]; + } + + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer), DataRow(Provider.Sqlite)] + public void A_rename_without_a_collision_stays_in_place(Provider provider) + => CollectionAssert.AreEqual(new[] { "ALTER INDEX" }, Diff(provider)); + + /// The rename's statement is the contributor's answer, written into the builder it was handed: it does not reach the migration. + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer), DataRow(Provider.Sqlite)] + public void A_rename_whose_old_name_EF_Core_takes_is_a_drop_before_and_a_create_after(Provider provider) + => CollectionAssert.AreEqual( + new[] { "DROP INDEX", "CreateTableOperation", "CREATE UNIQUE" }, + Diff(provider)); + + /// EF Core's drop runs before the rename: the name is free by then. + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer), DataRow(Provider.Sqlite)] + public void A_rename_into_a_name_EF_Core_gives_up_stays_in_place(Provider provider) + => CollectionAssert.AreEqual( + new[] { "DropTableOperation", "ALTER INDEX" }, + Diff(provider)); + + /// And the other way, where EF Core creates the table under the name the rename takes: a create of EF Core's needs the name free. + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer), DataRow(Provider.Sqlite)] + public void A_rename_into_a_name_EF_Core_takes_first_is_a_rebuild_too(Provider provider) + => CollectionAssert.AreEqual( + new[] { "DROP INDEX", "CreateTableOperation", "CREATE UNIQUE" }, + Diff(provider)); + + /// A declared object no longer declared is dropped before EF Core's operations, so its name is free when the rename runs. + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer), DataRow(Provider.Sqlite)] + public void A_rename_into_a_name_a_dropped_declaration_gives_up_stays_in_place(Provider provider) + => CollectionAssert.AreEqual( + new[] { "DROP INDEX", "ALTER INDEX" }, + Diff(provider)); + + /// + /// A declared object the database loses with an operation of EF Core's is gone by the time the + /// rename runs, and is created again after it under its new name. Not on SQLite, which rebuilds + /// the table for an altered column: that is another change, and a create after a rebuild is not + /// placed there yet. + /// + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer)] + public void A_rename_into_a_name_a_lost_declaration_gives_up_stays_in_place(Provider provider) + { + var shape = Diff(provider, to => + ((TestGuardContributor)to.GetService().Find("Test.Guard")!).ImpactOverride = + change => change is BaseChange.ColumnAltered ? new Impact.Lost() : new Impact.Follows()); + + Assert.AreEqual("ALTER INDEX", shape[^2], string.Join(" | ", shape)); + Assert.AreEqual("CREATE UNIQUE", shape[^1], string.Join(" | ", shape)); + CollectionAssert.DoesNotContain(shape, "DROP INDEX"); + } + + [TestMethod, DataRow(Provider.Npgsql), DataRow(Provider.SqlServer), DataRow(Provider.Sqlite)] + public void What_tables_are_made_of_is_not_rebuilt_around_a_collision(Provider provider) + { + var error = Assert.ThrowsExactly(() => Diff(provider, to => + ((TestDomainContributor)to.GetService().Find("Test.Domain")!).ClaimsOverride = + descriptor => [new NameClaim.Relation(descriptor.Name.Schema, descriptor.Name.Name)])); + + StringAssert.Contains(error.Message, "TestDomainContributor'.Plan answered InPlace for 'dom' becoming 'dom2'"); + } + + /// Without a claim that collides, the domain's rename stays the statement it was. + [TestMethod] + public void What_tables_are_made_of_is_renamed_where_nothing_collides() + => CollectionAssert.AreEqual(new[] { "CreateTableOperation", "ALTER DOMAIN" }, Diff(Provider.Npgsql)); +}