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)); +}