diff --git a/CHANGELOG.md b/CHANGELOG.md index e5b44b7..3e21e8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,22 @@ covering only what changed for that package: [PostgreSQL](src/EFCore.ComplexIndexes.PostgreSQL/CHANGELOG.md), [SQL Server](src/EFCore.ComplexIndexes.SqlServer/CHANGELOG.md). +## 5.1.0 + +Small enhancements around the two seams, plus the silent failures found while planning the next +major: a pending-changes check that never saw this package, and two ways a declaration could vanish +from a migration without a word. + +- **Fixed:** `HasDifferences` now reports changes to complex indexes, exclusion constraints and temporal constraints. EF Core's base implementation runs its own `Diff` rather than the `GetDifferences` this package overrides, so every check built on it reported "no changes" when only a declaration from this package had changed: `dotnet ef migrations has-pending-model-changes`, the pending-model-changes warning `Migrate()` raises since EF Core 9, and the snapshot check in `migrations remove`. A CI gate built on `has-pending-model-changes` may now fail where it previously passed — that is the gate working. +- **Fixed:** a complex index whose name matches a native `HasIndex` on the same table is now rejected at `dotnet ef migrations add`. The base differ emitted one and this package the other, neither seeing the other, so the migration scaffolded two `CREATE INDEX` statements under one name and failed when applied (42P07). Only the target model's native indexes are consulted, so an index *moving* between a native declaration and a complex one under the same name still diffs as before. +- **Fixed:** a provider option from the other satellite on a *property-level* complex index is now rejected at `migrations add`, like an entity-level one, instead of being dropped by the forwarding whitelist without a word — `.UseGin()` on a property-level index diffed by the SQL Server satellite scaffolded a plain B-tree. The PostgreSQL differ likewise rejects SQL Server options, which it previously passed through to a generator that ignored them. +- **New:** runtime registration of the differ. `Database.EnsureCreated()`, `GenerateCreateScript()` and the pending-model-changes check `Migrate()` performs run the context's *runtime* differ, which the design-time wiring never reaches — so `EnsureCreated()` created the tables and silently none of the complex indexes, and `Migrate()` never warned about one that was not scaffolded. `UseNpgsqlComplexIndexes()` now registers the PostgreSQL differ alongside the generator; SQL Server gets `UseSqlServerComplexIndexes()`; providers without a satellite get `UseComplexIndexes()` from the core package. Each has an `Add…ComplexIndexes` counterpart for a custom internal service provider. With a satellite installed, call the satellite's method only. +- **New:** whole-document JSON indexes on PostgreSQL. Pointing `HasComplexIndex` at a `ToJson()` complex property — or at a complex collection, which is always JSON — now indexes its `jsonb` container column, so `HasComplexIndex(x => x.Payload, ix => ix.UseGin().HasOperators("jsonb_path_ops"))` produces the idiomatic GIN index. Previously the path failed with "could not resolve property path", and complex collections could not be indexed at all. The container is a real column, so no runtime wiring is involved; a complex property nested inside the document resolves to a `->` extraction and renders like any expression index. +- **New:** `HasStorageParameter(name, value)` on PostgreSQL complex and expression indexes — `WITH (fillfactor=70)`, `WITH (fastupdate=false)`, and so on. Column indexes render through Npgsql's own generator; expression indexes through this package's, in the same clause position. +- **New:** `UseCollation(params string[])` on PostgreSQL complex and expression indexes — per-column index collations (`"Name" COLLATE "C"`), positional, with an empty entry leaving that column on its default. Independent of the column's own collation, which is never copied onto the index. +- **New:** the property-level `HasComplexIndex` overloads also exist on the non-generic `ComplexTypePropertyBuilder`, so a property configured by name (`c.Property("Value")`) or by type can carry a complex index. +- **Changed:** a complex index, exclusion constraint or temporal constraint declared on an entity type that is mapped to no table — typically the abstract base of a TPC hierarchy — now fails at `migrations add` instead of producing nothing without a word. Declarations on view-mapped and query-mapped types are still ignored. + ## 5.0.3 A packaging and documentation release. No behaviour changes to the differ or the generated SQL. diff --git a/CLAUDE.md b/CLAUDE.md index a51119b..efca8f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,7 +224,11 @@ that stands alone, and `DocumentationLinkTests` guards only the part that fails Property-level annotations reach the `CreateIndexOperation` only through `IsForwardedIndexAnnotation` (virtual on the core differ, default **nothing**; the Npgsql differ -whitelists exactly its five `Npgsql:*` index-option keys). Never revert to sweeping "everything +whitelists exactly its six `Npgsql:*` index-option keys, plus every key under the per-parameter +`Npgsql:StorageParameter:` prefix). The key an option is *stored* under can differ from the key the +provider generator *reads*: `ToOperationAnnotationName` maps `Npgsql:IndexCollation` to +`Relational:Collation` at stamping time, because the property-level API writes options onto the +property, where the relational key would be read as the column's collation. Never revert to sweeping "everything except known keys": column facets (`Relational:ColumnName`, `Relational:ColumnType`, …) leaked into scaffolded migrations that way, and snapshot/code-model asymmetries caused phantom drop/create churn (see `PhantomIndexChurnTests`). @@ -281,6 +285,19 @@ There are two distinct hook points, and it matters which one a feature uses: - **Design-time** (`IDesignTimeServices` via the `.targets`-injected attribute) replaces `IMigrationsModelDiffer`. This runs during `dotnet ef migrations add` and is auto-wired — consumers do nothing. - **Runtime** (`IMigrationsSqlGenerator`) converts operations to SQL when migrations are *applied*. This is **not** auto-wired; consumers opt in with `optionsBuilder.UseNpgsqlComplexIndexes()` (a `ReplaceService` helper). +Since 5.1.0 the runtime seam also carries the **differ**: `UseComplexIndexes()` (core), +`UseNpgsqlComplexIndexes()` and `UseSqlServerComplexIndexes()` replace `IMigrationsModelDiffer` in the +context's own service provider, because `EnsureCreated()`, `GenerateCreateScript()` and the +pending-model-changes check in `Migrate()` run *that* differ and never see the design-time attribute — +without it, `EnsureCreated()` creates the tables and silently none of the indexes. Design-time +selection is unaffected: EF's `AddDbContextDesignTimeServices` seeds the design-time collection with +the context's differ as a factory registration, and the `.targets` registration is appended after it +(`DesignTimeServiceRegistrationTests`). Related and easy to miss: `MigrationsModelDiffer.HasDifferences` +runs EF's protected `Diff`, not `GetDifferences`, so the core overrides it to route through +`GetDifferences` — otherwise `dotnet ef migrations has-pending-model-changes`, `Migrate()`'s +pending-changes warning and `migrations remove` all reported "no changes" for a complex-index-only +change (`PendingModelChangesTests`). + Anything that depends on the runtime seam silently degrades when a consumer forgets the wiring, so **prefer rendering DDL at design time** (a `SqlOperation` baked into the migration) whenever the statement can be built from resolved column names — that is why exclusion *and* temporal @@ -317,7 +334,11 @@ differ let satellites resolve what the core cannot: - `ResolveUnmappedPart` — a path with no table column; the Npgsql differ builds a JSON extraction (`"col" -> 'A' ->> 'B'`) when the path traverses a `ToJson()` complex property, honoring `HasJsonPropertyName`. Members extract as text — no automatic casts (text→timestamptz casts are - not IMMUTABLE and would blow up `CREATE INDEX`). + not IMMUTABLE and would blow up `CREATE INDEX`). A path that *ends* at the JSON-mapped complex + property — or at a complex collection, which is always JSON — resolves to the container column + as a plain **column** part (so a whole-document GIN needs no runtime wiring); a complex property + nested inside the document resolves to a `->` extraction yielding `jsonb`. A table-split complex + property stays unresolved: there is no single column to stand for it. - `ResolveTemplatePart` — substitutes template placeholders with quoted columns or parenthesized JSON extractions; core throws (identifier quoting is provider-specific). @@ -365,7 +386,21 @@ policing those turns any provider index option the satellite doesn't happen to k hard failure of the consumer's whole `migrations add` — for indexes that never touched this package. The check has to exist because entity-level provider annotations reach the operation *unfiltered* (only the property-level path goes through `IsForwardedIndexAnnotation`), so `.UseGin()` on a SQL -Server model is caught, while a native `HasIndex(...).HasMethod("gin")` is left alone. +Server model is caught, while a native `HasIndex(...).HasMethod("gin")` is left alone. The +property-level path used to be the loophole: the whitelist dropped the other satellite's options +without a word, so a property-level `.UseGin()` diffed by SQL Server applied as a plain B-tree. +Since 5.1.0 each satellite's `IsForwardedIndexAnnotation` also returns true for the *other* +provider's prefix (`Npgsql:` / `SqlServer:`) — forwarded solely so the same +`ValidateCreateIndexOperation` rejects it, which keeps one message for both declaration styles. + +### Declarations on types with no table + +An entity type mapped to no table — the abstract base of a TPC hierarchy is the usual one — used to +be skipped by every descriptor scan (`if (tableName is null) continue;`), so an index or constraint +declared there produced nothing: no DDL, no error. Since 5.1.0 the scans call +`ThrowIfDeclaredOnUnmappedType` when such a type carries declarations; the satellites use the same +helper for exclusion and temporal descriptors. Types mapped to a view, SQL query or function are +still skipped silently — an index on those is nothing this package could create. ### Key extension points diff --git a/Directory.Build.props b/Directory.Build.props index 3d503e2..74808d4 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,6 +1,6 @@ - 5.0.3 + 5.1.0 CaffeinatedCoder MIT true diff --git a/README.md b/README.md index d2917ec..8d442fd 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ EF Core 8.0 introduced complex properties, but migration tooling doesn't automat | Package | NuGet | Description | |---|---|---| | **EFCore.ComplexIndexes** | [![nuget](https://img.shields.io/nuget/v/EFCore.ComplexIndexes.svg)](https://www.nuget.org/packages/EFCore.ComplexIndexes/) | Core library — single-column, composite, unique, and filtered indexes on complex type properties. Works with any EF Core relational provider. | -| **EFCore.ComplexIndexes.PostgreSQL** | [![nuget](https://img.shields.io/nuget/v/EFCore.ComplexIndexes.PostgreSQL.svg)](https://www.nuget.org/packages/EFCore.ComplexIndexes.PostgreSQL/) | PostgreSQL extensions via [Npgsql](https://www.npgsql.org/efcore/) — adds GIN, GiST, BRIN, SP-GiST, and Hash index methods, operator classes, covering indexes (`INCLUDE`), concurrent creation, nulls-distinct control, `NULLS FIRST/LAST`, **expression (functional) indexes** (raw SQL and **typed LINQ**), **JSON member indexes**, **temporal `UNIQUE` constraints (`WITHOUT OVERLAPS`)**, and **exclusion constraints (`EXCLUDE`)**. | +| **EFCore.ComplexIndexes.PostgreSQL** | [![nuget](https://img.shields.io/nuget/v/EFCore.ComplexIndexes.PostgreSQL.svg)](https://www.nuget.org/packages/EFCore.ComplexIndexes.PostgreSQL/) | PostgreSQL extensions via [Npgsql](https://www.npgsql.org/efcore/) — adds GIN, GiST, BRIN, SP-GiST, and Hash index methods, operator classes, covering indexes (`INCLUDE`), concurrent creation, nulls-distinct control, per-column collation, storage parameters, `NULLS FIRST/LAST`, **expression (functional) indexes** (raw SQL and **typed LINQ**), **JSON member indexes**, **temporal `UNIQUE` constraints (`WITHOUT OVERLAPS`)**, and **exclusion constraints (`EXCLUDE`)**. | | **EFCore.ComplexIndexes.SqlServer** | [![nuget](https://img.shields.io/nuget/v/EFCore.ComplexIndexes.SqlServer.svg)](https://www.nuget.org/packages/EFCore.ComplexIndexes.SqlServer/) | SQL Server extensions — clustered/nonclustered control, covering indexes (`INCLUDE`), online index builds, fill factor, sort-in-tempdb, and data compression on complex-property indexes. Rendered by the stock SQL Server generator; no runtime wiring. | > **Which package do I need?** @@ -81,6 +81,26 @@ var provider = new ServiceCollection() .BuildServiceProvider(); ``` +### `EnsureCreated`, `GenerateCreateScript`, and the pending-changes check + +Migrations go through the design-time differ, which the packages wire up automatically. Three things +use the **runtime** differ instead and never see that wiring: `Database.EnsureCreated()`, +`Database.GenerateCreateScript()`, and the pending-model-changes check `Migrate()` performs. Without a +runtime registration they run EF's stock differ, which cannot see this package's declarations — +`EnsureCreated()` creates the tables and silently none of the indexes, and `Migrate()` does not warn +about a complex index that was never scaffolded. Register the differ once, next to the provider: + +| Provider | Call | +|---|---| +| PostgreSQL | `UseNpgsqlComplexIndexes()` — the same call as above; since 5.1.0 it registers the differ too | +| SQL Server | `UseSqlServerComplexIndexes()` | +| Any other provider (SQLite, …) | `UseComplexIndexes()` from the core package | + +With a satellite installed, call only the satellite's method: the core differ would give +`EnsureCreated()` a schema without the satellite's features, such as exclusion constraints. Each call +has a counterpart for a custom internal service provider: `AddComplexIndexes()`, +`AddNpgsqlComplexIndexes()` and `AddSqlServerComplexIndexes()`. + --- ## Core usage — any relational provider @@ -94,6 +114,9 @@ builder.ComplexProperty(x => x.EmailAddress, c => ); ``` +The same overloads exist on the non-generic builder, so a property configured by name works too: +`c.Property("Value").HasComplexIndex()`. + A property-level declaration holds **one** index per property. To give the same column several differently-filtered indexes (the classic soft-delete pattern), declare them at the **entity level** — the selector reaches into complex properties, and each index needs its own explicit name: diff --git a/SECURITY.md b/SECURITY.md index be25dea..b28d217 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -50,8 +50,8 @@ remedy for those is to upgrade. | Version | Supported | |---|---| -| 5.0.x | ✅ | -| < 5.0 | ❌ | +| 5.1.x | ✅ | +| < 5.1 | ❌ | ### For how long @@ -82,7 +82,10 @@ The package has **no runtime presence in your application's request path**. It r 1. **Design time** — a replacement `IMigrationsModelDiffer` invoked by `dotnet ef migrations add`. It reads your model and emits migration operations. 2. **Migration apply time** — only for PostgreSQL expression indexes and `NULLS FIRST/LAST` - ordering, and only when a consumer opts in with `UseNpgsqlComplexIndexes()`. + ordering, and only when a consumer opts in with `UseNpgsqlComplexIndexes()`. Since 5.1.0 that + call (and `UseComplexIndexes()` / `UseSqlServerComplexIndexes()`) also registers the differ at + runtime, where `EnsureCreated()`, `GenerateCreateScript()` and `Migrate()`'s pending-model-changes + check run it — still against your own model, still opt-in. Its inputs come from your own `OnModelCreating` code, not from user input. Anyone who can change that code can already run arbitrary code in your build. diff --git a/docs/postgresql-indexes.md b/docs/postgresql-indexes.md index b2f24c6..7b77276 100644 --- a/docs/postgresql-indexes.md +++ b/docs/postgresql-indexes.md @@ -31,6 +31,21 @@ builder.ComplexProperty(x => x.Payload, c => ); ``` +Storage parameters render as `WITH (…)`; call `HasStorageParameter` once per parameter. Strings are +quoted, booleans render bare. Per-column collations are positional — an empty entry leaves that +column on its default: + +```csharp +builder.HasComplexCompositeIndex(x => new { x.Name, x.Email.Value }, idx => idx + .UseCollation("C", "") + .HasStorageParameter("fillfactor", 70) + .HasStorageParameter("deduplicate_items", false)); +// CREATE INDEX ... ON people ("Name" COLLATE "C", email) WITH (fillfactor=70, deduplicate_items=false); +``` + +The index collation is independent of the column's own `UseCollation` on the property, which is never +copied onto the index. + ## Expression (functional) indexes > Requires [`UseNpgsqlComplexIndexes()`](../README.md#runtime-wiring--the-two-features-that-need-it). @@ -146,3 +161,26 @@ builder.HasComplexIndex(x => x.Name.ShortName, isUnique: true, indexName: "ux_em Nested complex types become `->` segments (`("profile" -> 'Address' ->> 'City')`), and `HasJsonPropertyName` is honored. Members are extracted as **text**; for typed comparisons or ordering semantics use `HasExpressionIndex` with an explicit cast. + +### Indexing the whole document + +> No runtime wiring: the container is a real column, so the index renders through the stock generator. + +Point the selector at the JSON-mapped complex property itself — or at a complex collection, which is +always JSON — and the index lands on the `jsonb` container column. The PostgreSQL idiom is a GIN +index, usually with `jsonb_path_ops`: + +```csharp +builder.ComplexProperty(x => x.Payload, c => c.ToJson("payload")); +builder.ComplexCollection(x => x.Tags, c => c.ToJson("tags")); + +builder.HasComplexIndex(x => x.Payload, ix => ix.UseGin().HasOperators("jsonb_path_ops")); +// CREATE INDEX "IX_orders_payload" ON orders USING gin (payload jsonb_path_ops); + +builder.HasComplexIndex(x => x.Tags, ix => ix.UseGin()); +// CREATE INDEX "IX_orders_tags" ON orders USING gin (tags); +``` + +A complex property *nested inside* the document has no column of its own and resolves to a `->` +extraction instead (`("payload" -> 'Address')`, yielding `jsonb`), so it is an expression index and +needs the runtime wiring like the member indexes above. diff --git a/docs/sqlserver.md b/docs/sqlserver.md index ca0dcc3..2816a6a 100644 --- a/docs/sqlserver.md +++ b/docs/sqlserver.md @@ -1,9 +1,15 @@ # SQL Server Provided by the **EFCore.ComplexIndexes.SqlServer** package. The core package is included -automatically, and there is **no runtime wiring at all** — every option flows as a native SQL Server +automatically, and migrations need **no runtime wiring** — every option flows as a native SQL Server annotation that the provider's own migrations SQL generator renders. +One optional call exists: `UseSqlServerComplexIndexes()` registers the differ at runtime, so +`Database.EnsureCreated()` and `GenerateCreateScript()` include the complex indexes and the +pending-model-changes check in `Migrate()` sees one that was never scaffolded. See +[the runtime wiring section](../README.md#ensurecreated-generatecreatescript-and-the-pending-changes-check) +in the root README. + ## Index options The **EFCore.ComplexIndexes.SqlServer** package brings the SQL Server option set to complex-property diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/CHANGELOG.md b/src/EFCore.ComplexIndexes.PostgreSQL/CHANGELOG.md index 40dfb4c..1985d24 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/CHANGELOG.md +++ b/src/EFCore.ComplexIndexes.PostgreSQL/CHANGELOG.md @@ -4,6 +4,32 @@ Changes to the PostgreSQL satellite, newest first. The [root changelog](https://github.com/CaffeinatedCoder/EFCore.ComplexIndexes/blob/main/CHANGELOG.md) covers all three packages. +## 5.1.0 + +- **Changed:** `UseNpgsqlComplexIndexes()` / `AddNpgsqlComplexIndexes()` also register the PostgreSQL + differ at runtime, so `EnsureCreated()` and `GenerateCreateScript()` include complex indexes, + expression indexes, and exclusion and temporal constraints, and `Migrate()`'s pending-model-changes + check sees a declaration that was never scaffolded. Previously `EnsureCreated()` created the tables + and silently none of them. +- **New:** whole-document JSON indexes. A `HasComplexIndex` selector ending at a `ToJson()` complex + property or a complex collection indexes the `jsonb` container column — the idiomatic + `USING gin (payload jsonb_path_ops)` — through the stock generator, no runtime wiring. Previously + the path failed to resolve, and complex collections could not be indexed at all. A complex property + nested inside the document resolves to a `->` extraction (an expression index). +- **New:** `HasStorageParameter(name, value)` — PostgreSQL storage parameters (`WITH (fillfactor=70)`) + on complex and expression indexes, one call per parameter. Forwarded under the per-parameter + `Npgsql:StorageParameter:` prefix, which the whitelist and the unknown-key rejection now both accept. +- **New:** `UseCollation(params string[])` — per-column index collations, positional (`UseCollation("C", "")` + collates only the first column). Stored under Npgsql's model key and mapped to `Relational:Collation` + on the operation, where Npgsql's generator reads it; a column's own collation is never copied onto + the index. +- **Fixed:** SQL Server index options (`IsClustered`, `HasFillFactor`, …) on a complex index diffed by + this satellite are rejected at `migrations add` — property-level and entity-level alike — instead of + reaching Npgsql's generator, which ignored them. +- **Changed:** an exclusion constraint, temporal constraint or temporal foreign key declared on an + entity type mapped to no table — typically the abstract base of a TPC hierarchy — fails at + `migrations add` instead of producing nothing. + ## 5.0.3 - **Changed:** the `Npgsql.EntityFrameworkCore.PostgreSQL` dependency is now `[10.0.0, 11.0.0)`. This diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlAnnotations.cs b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlAnnotations.cs index fda5199..464f5a9 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlAnnotations.cs +++ b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlAnnotations.cs @@ -13,4 +13,20 @@ internal static class NpgsqlAnnotations public const string IndexNullSortOrder = "Npgsql:IndexNullSortOrder"; public const string CreatedConcurrently = "Npgsql:CreatedConcurrently"; public const string NullsDistinct = "Npgsql:NullsDistinct"; + + /// + /// Per-column index collations. Npgsql's model key; on the operation Npgsql's generator reads + /// Relational:Collation instead, which the differ maps to (see + /// NpgsqlComplexIndexMigrationsModelDiffer.ToOperationAnnotationName). + /// + public const string IndexCollation = "Npgsql:IndexCollation"; + + /// + /// Prefix of the per-parameter storage-parameter keys (Npgsql:StorageParameter:fillfactor, …). + /// Npgsql's generator renders every operation annotation under this prefix as WITH (name=value). + /// + public const string StorageParameterPrefix = "Npgsql:StorageParameter:"; + + public static bool IsStorageParameter(string annotationName) + => annotationName.StartsWith(StorageParameterPrefix, StringComparison.Ordinal); } \ No newline at end of file diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexBuilderExtensions.cs b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexBuilderExtensions.cs index 11c3c77..78bfe86 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexBuilderExtensions.cs +++ b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexBuilderExtensions.cs @@ -46,6 +46,32 @@ public static TBuilder IsCreatedConcurrently(this TBuilder builder, bo public static TBuilder AreNullsDistinct(this TBuilder builder, bool nullsDistinct = true) where TBuilder : IIndexAnnotationBuilder => builder.Set(NpgsqlAnnotations.NullsDistinct, nullsDistinct); + /// + /// Specifies per-column collations for the index, positionally — an empty entry leaves that + /// column on its default collation: UseCollation("C", "") collates only the first column. + /// Rendered as column COLLATE "C". This is the index's collation, independent of the + /// column's own. + /// + public static TBuilder UseCollation(this TBuilder builder, params string[] collations) where TBuilder : IIndexAnnotationBuilder + { + ArgumentNullException.ThrowIfNull(collations); + if (collations.Length == 0) + throw new ArgumentException("Specify at least one collation.", nameof(collations)); + return builder.Set(NpgsqlAnnotations.IndexCollation, collations); + } + + /// + /// Sets a PostgreSQL storage parameter on the index, rendered as WITH (name=value) — e.g. + /// HasStorageParameter("fillfactor", 70) or HasStorageParameter("fastupdate", false). + /// Strings are quoted, booleans render as true/false. Call once per parameter. + /// + public static TBuilder HasStorageParameter(this TBuilder builder, string name, object value) where TBuilder : IIndexAnnotationBuilder + { + ArgumentException.ThrowIfNullOrWhiteSpace(name); + ArgumentNullException.ThrowIfNull(value); + return builder.Set(NpgsqlAnnotations.StorageParameterPrefix + name, value); + } + private static TBuilder Set(this TBuilder builder, string key, object? value) where TBuilder : IIndexAnnotationBuilder { builder.Annotations[key] = value; diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexDbContextOptionsExtensions.cs b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexDbContextOptionsExtensions.cs index f1d4b3a..ad9a573 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexDbContextOptionsExtensions.cs +++ b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexDbContextOptionsExtensions.cs @@ -5,7 +5,10 @@ namespace EFCore.ComplexIndexes.PostgreSQL; /// -/// Runtime wiring for expression indexes on PostgreSQL. +/// Runtime wiring for PostgreSQL: the SQL generator that renders expression indexes and +/// NULLS FIRST/LAST when migrations are applied, and the differ that lets +/// EnsureCreated(), GenerateCreateScript() and Migrate()'s +/// pending-model-changes check see this package's declarations. /// public static class NpgsqlComplexIndexDbContextOptionsExtensions { @@ -13,20 +16,34 @@ public static class NpgsqlComplexIndexDbContextOptionsExtensions { /// /// Replaces the migrations SQL generator with one that can render expression indexes - /// defined via HasExpressionIndex. Call this after UseNpgsql(...): + /// defined via HasExpressionIndex and per-column null ordering, and registers the + /// PostgreSQL complex-index differ at runtime so EnsureCreated() builds the declared + /// indexes and constraints. Call this after UseNpgsql(...): /// options.UseNpgsql(connectionString).UseNpgsqlComplexIndexes(); /// public DbContextOptionsBuilder UseNpgsqlComplexIndexes() - => optionsBuilder.ReplaceService(); + => optionsBuilder + .ReplaceService() + .ReplaceService(); } - + + extension(DbContextOptionsBuilder optionsBuilder) where TContext : DbContext + { + /// + public DbContextOptionsBuilder UseNpgsqlComplexIndexes() + => (DbContextOptionsBuilder)((DbContextOptionsBuilder)optionsBuilder).UseNpgsqlComplexIndexes(); + } + extension(IServiceCollection services) { /// - /// Registers the migrations SQL generator required for expression indexes - /// when using a custom internal service provider. + /// Registers the migrations SQL generator and the PostgreSQL complex-index differ on a + /// custom internal service provider — the equivalent of UseNpgsqlComplexIndexes() + /// for applications that build their own IServiceProvider. /// - public IServiceCollection AddNpgsqlComplexIndexes() - => services.AddScoped(); + public IServiceCollection AddNpgsqlComplexIndexes() + => services + .AddScoped() + .AddScoped(); } } diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexMigrationsModelDiffer.cs b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexMigrationsModelDiffer.cs index 2d93fae..3f393cc 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexMigrationsModelDiffer.cs +++ b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexMigrationsModelDiffer.cs @@ -44,7 +44,8 @@ CommandBatchPreparerDependencies commandBatchPreparerDependencies NpgsqlAnnotations.IndexOperators, NpgsqlAnnotations.IndexInclude, NpgsqlAnnotations.CreatedConcurrently, - NpgsqlAnnotations.NullsDistinct + NpgsqlAnnotations.NullsDistinct, + NpgsqlAnnotations.IndexCollation ]; /// @@ -60,25 +61,52 @@ CommandBatchPreparerDependencies commandBatchPreparerDependencies NpgsqlAnnotations.IndexNullSortOrder ]; - /// Forwards exactly the Npgsql index-option annotations Npgsql's SQL generator renders. + /// + /// Forwards exactly the Npgsql index-option annotations Npgsql's SQL generator renders: the + /// whitelisted keys plus every Npgsql:StorageParameter:* key, which is per-parameter. Every + /// SqlServer:* key is forwarded too, only so that + /// rejects it — a property-level .IsClustered() on a model diffed by this satellite would + /// otherwise be dropped by the whitelist without a word. + /// protected override bool IsForwardedIndexAnnotation(string annotationName) - => SupportedNpgsqlAnnotations.Contains(annotationName); + => SupportedNpgsqlAnnotations.Contains(annotationName) + || NpgsqlAnnotations.IsStorageParameter(annotationName) + || annotationName.StartsWith("SqlServer:", StringComparison.Ordinal); /// PostgreSQL renames indexes standalone (ALTER INDEX … RENAME TO). protected override bool CanRenameIndexes => true; + /// + /// Npgsql's generator reads index collations from Relational:Collation on the operation; + /// the option is stored under Npgsql's model key so that a property-level declaration is never + /// mistaken for the column's collation. + /// + protected override string ToOperationAnnotationName(string annotationName) + => annotationName == NpgsqlAnnotations.IndexCollation + ? RelationalAnnotationNames.Collation + : base.ToOperationAnnotationName(annotationName); + /// /// Rejects Npgsql:* index options this package does not render — typically an entity-level /// declaration carrying an option the satellite has no support for, since entity-level provider /// annotations reach the operation unfiltered (the property-level path is already whitelisted by - /// ). + /// ) — and every SqlServer:* option, which belongs + /// to the other satellite. /// protected override void ValidateCreateIndexOperation(CreateIndexOperation operation) { foreach (var annotation in operation.GetAnnotations()) { + // The other satellite's options: Npgsql's generator would ignore them, so a clustered or + // fill-factor declaration would apply as a plain index without a word. + if (annotation.Name.StartsWith("SqlServer:", StringComparison.Ordinal)) + throw new InvalidOperationException( + $"Complex index '{operation.Name}' carries the SQL Server annotation '{annotation.Name}', but the " + + "model is diffed with the PostgreSQL satellite. Use the EFCore.ComplexIndexes.PostgreSQL options instead."); + if (!annotation.Name.StartsWith("Npgsql:", StringComparison.Ordinal) - || SupportedNpgsqlAnnotations.Contains(annotation.Name)) + || SupportedNpgsqlAnnotations.Contains(annotation.Name) + || NpgsqlAnnotations.IsStorageParameter(annotation.Name)) continue; // Superseded keys get their own message: they are not unknown, they are the wrong way @@ -106,12 +134,15 @@ StoreObjectIdentifier storeObject : base.TransformIndexAnnotation(entityType, annotationName, value, storeObject); /// - /// Resolves an index part whose path traverses a complex property mapped to JSON - /// (ToJson()) into a PostgreSQL extraction expression, e.g. - /// "name" -> 'Inner' ->> 'Leaf'. Members are extracted as text - /// (->>) and honor HasJsonPropertyName; for typed semantics use - /// HasExpressionIndex with an explicit cast. Like all expression parts, rendering - /// requires the UseNpgsqlComplexIndexes() runtime wiring. + /// Resolves an index part whose path has no table column: a member of a complex property mapped + /// to JSON via ToJson(), or the JSON-mapped complex property (or complex collection) + /// itself. A member becomes a PostgreSQL text extraction, e.g. + /// "name" -> 'Inner' ->> 'Leaf', honoring HasJsonPropertyName; for typed + /// semantics use HasExpressionIndex with an explicit cast. A path ending at the JSON-mapped + /// complex property resolves to its container column — a plain column index, typically + /// USING gin, that the stock generator renders with no runtime wiring. A complex property + /// nested inside the document resolves to a -> extraction yielding jsonb, which + /// GIN indexes too. Expression parts require the UseNpgsqlComplexIndexes() runtime wiring. /// protected override ResolvedIndexPart? ResolveUnmappedPart( IEntityType entityType, @@ -143,24 +174,61 @@ StoreObjectIdentifier storeObject current = complexProperty.ComplexType; } - if (containerColumn is null) - return null; - var leaf = current.FindProperty(segments[^1]); if (leaf is null) + return ResolveComplexLeaf(current, segments[^1], containerColumn, jsonPath, part); + + if (containerColumn is null) return null; jsonPath.Add(leaf.GetJsonPropertyName() ?? leaf.Name); + return new ResolvedIndexPart(true, BuildJsonExtraction(containerColumn, jsonPath, asText: true), part.Descending, part.NullSort); + } + + // The path ends at a complex property rather than a scalar: the whole document, or a + // sub-document. At the top of a ToJson() mapping — and a complex collection is always JSON — + // that is the container column itself, so the index is a plain column index the stock generator + // renders. Nested inside a document it is a `->` extraction, which yields jsonb rather than text. + // A table-split complex property has no single column to stand for it, so that stays unresolved. + private static ResolvedIndexPart? ResolveComplexLeaf( + ITypeBase current, + string name, + string? containerColumn, + List jsonPath, + IndexPartDefinition part + ) + { + var complexProperty = current.FindComplexProperty(name); + if (complexProperty is null) + return null; + + if (containerColumn is null) + { + var column = complexProperty.ComplexType.GetContainerColumnName(); + return column is null + ? null + : new ResolvedIndexPart(false, column, part.Descending, part.NullSort); + } + + jsonPath.Add(complexProperty.GetJsonPropertyName() ?? complexProperty.Name); + + return new ResolvedIndexPart(true, BuildJsonExtraction(containerColumn, jsonPath, asText: false), part.Descending, part.NullSort); + } + + // "col" -> 'A' -> 'B' (jsonb) or, with asText, "col" -> 'A' ->> 'B' (text) for the last step. + private static string BuildJsonExtraction(string containerColumn, List jsonPath, bool asText) + { var sql = new System.Text.StringBuilder(Quote(containerColumn)); for (var i = 0; i < jsonPath.Count; i++) { - sql.Append(i == jsonPath.Count - 1 ? " ->> '" : " -> '") + var last = i == jsonPath.Count - 1; + sql.Append(last && asText ? " ->> '" : " -> '") .Append(jsonPath[i].Replace("'", "''")) .Append('\''); } - return new ResolvedIndexPart(true, sql.ToString(), part.Descending, part.NullSort); + return sql.ToString(); } /// @@ -225,7 +293,7 @@ private string ResolvePlaceholder(IEntityType entityType, string path, StoreObje var jsonPart = ResolveUnmappedPart(entityType, new IndexPartDefinition { PropertyPath = path }, storeObject); if (jsonPart is not null) - return $"({jsonPart.Value})"; + return jsonPart.IsExpression ? $"({jsonPart.Value})" : Quote(jsonPart.Value); throw new InvalidOperationException( $"Could not resolve property path '{path}' referenced by an index expression on entity '{entityType.Name}'."); @@ -592,7 +660,11 @@ private static HashSet BuildExclusionDescriptors(IRelationa continue; var table = entityType.GetTableName(); - if (table is null) continue; + if (table is null) + { + ThrowIfDeclaredOnUnmappedType(entityType, "exclusion constraints"); + continue; + } var schema = entityType.GetSchema(); var storeObject = StoreObjectIdentifier.Table(table, schema); @@ -692,7 +764,11 @@ IRelationalTypeMappingSource typeMappingSource continue; var table = entityType.GetTableName(); - if (table is null) continue; + if (table is null) + { + ThrowIfDeclaredOnUnmappedType(entityType, "temporal constraints"); + continue; + } var schema = entityType.GetSchema(); var storeObject = StoreObjectIdentifier.Table(table, schema); @@ -748,7 +824,11 @@ HashSet temporalConstraints continue; var dependentTable = dependentEntityType.GetTableName(); - if (dependentTable is null) continue; + if (dependentTable is null) + { + ThrowIfDeclaredOnUnmappedType(dependentEntityType, "temporal foreign keys"); + continue; + } var dependentSchema = dependentEntityType.GetSchema(); var dependentStoreObject = StoreObjectIdentifier.Table(dependentTable, dependentSchema); diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexSqlGenerator.cs b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexSqlGenerator.cs index ad4235d..fe961a5 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexSqlGenerator.cs +++ b/src/EFCore.ComplexIndexes.PostgreSQL/NpgsqlComplexIndexSqlGenerator.cs @@ -48,6 +48,7 @@ protected override void Generate( var concurrently = operation[NpgsqlAnnotations.CreatedConcurrently] is true; var method = operation[NpgsqlAnnotations.IndexMethod] as string; var operators = ToStringList(operation[NpgsqlAnnotations.IndexOperators]); + var collations = ToStringList(operation[RelationalAnnotationNames.Collation]); var include = ToStringList(operation[NpgsqlAnnotations.IndexInclude]); var nullsDistinct = operation[NpgsqlAnnotations.NullsDistinct]; @@ -76,10 +77,13 @@ protected override void Generate( ? $"({parts[i].Value})" : sqlHelper.DelimitIdentifier(parts[i].Value)); + // PostgreSQL clause order: collation, operator class, direction, null ordering. + if (collations is not null && i < collations.Count && !string.IsNullOrEmpty(collations[i])) + builder.Append(" COLLATE ").Append(sqlHelper.DelimitIdentifier(collations[i])); + if (operators is not null && i < operators.Count && !string.IsNullOrEmpty(operators[i])) builder.Append(" ").Append(operators[i]); - // PostgreSQL clause order: operator class, then direction, then null ordering. if (parts[i].Descending) builder.Append(" DESC"); @@ -102,6 +106,14 @@ protected override void Generate( if (nullsDistinct is false) builder.Append(" NULLS NOT DISTINCT"); + var storageParameters = operation.GetAnnotations() + .Where(a => NpgsqlAnnotations.IsStorageParameter(a.Name)) + .Select(a => $"{a.Name[NpgsqlAnnotations.StorageParameterPrefix.Length..]}={FormatStorageParameter(a.Value)}") + .ToList(); + + if (storageParameters.Count > 0) + builder.Append(" WITH (").Append(string.Join(", ", storageParameters)).Append(")"); + if (!string.IsNullOrEmpty(operation.Filter)) builder.Append(" WHERE ").Append(operation.Filter); @@ -245,6 +257,15 @@ bool terminate } } + // Mirrors Npgsql's own formatting: booleans bare, strings quoted, numbers invariant. + private static string FormatStorageParameter(object? value) => value switch + { + bool b => b ? "true" : "false", + string s => $"'{s.Replace("'", "''")}'", + IFormattable f => f.ToString(null, System.Globalization.CultureInfo.InvariantCulture), + _ => value?.ToString() ?? string.Empty + }; + private static IReadOnlyList? ToStringList(object? value) => value switch { diff --git a/src/EFCore.ComplexIndexes.PostgreSQL/README.md b/src/EFCore.ComplexIndexes.PostgreSQL/README.md index 1da1af5..d54f1e3 100644 --- a/src/EFCore.ComplexIndexes.PostgreSQL/README.md +++ b/src/EFCore.ComplexIndexes.PostgreSQL/README.md @@ -7,10 +7,12 @@ PostgreSQL index and constraint features for Adds, on top of the core's complex-property, composite, unique, and filtered indexes: - **Index methods** — GIN, GiST, BRIN, SP-GiST, Hash — plus operator classes, covering (`INCLUDE`) - indexes, concurrent creation, and nulls-distinct control + indexes, concurrent creation, nulls-distinct control, per-column collation, and storage parameters + (`WITH (fillfactor=70)`) - **`NULLS FIRST` / `NULLS LAST`** per-column null ordering - **Expression (functional) indexes** — raw SQL *or* typed LINQ, on any entity, complex or not -- **JSON member indexes** — index members of `ToJson()` complex properties as `->>` extractions +- **JSON indexes** — index members of `ToJson()` complex properties as `->>` extractions, or the + whole document (or a complex collection) with a GIN over the `jsonb` column - **Temporal `UNIQUE … WITHOUT OVERLAPS` constraints and temporal foreign keys** (PostgreSQL 18) - **Exclusion (`EXCLUDE`) constraints** — filtered overlap protection, on every supported version @@ -29,6 +31,7 @@ and those need a one-time opt-in: | Exclusion constraints | no | | **Expression indexes** (raw SQL, typed LINQ, JSON member) | **yes** | | **`DbOrder.NullsFirst` / `NullsLast`** | **yes** | +| **`EnsureCreated()` / `GenerateCreateScript()` including the declarations** | **yes** *(since 5.1.0)* | ```csharp services.AddDbContext(options => @@ -41,7 +44,13 @@ services.AddDbContext(options => > named `__requires_UseNpgsqlComplexIndexes__`, so the stock generator fails loudly with that name > in the error message. -Building your own internal service provider? Register the generator directly instead: +Since 5.1.0 the same call also registers the PostgreSQL differ at runtime, so +`Database.EnsureCreated()` and `GenerateCreateScript()` build the declared indexes and constraints, +and the pending-model-changes check in `Migrate()` sees a complex index that was never scaffolded. +Both run the *runtime* differ, which the design-time wiring never reaches; without the call, +`EnsureCreated()` creates the tables and silently none of the indexes. + +Building your own internal service provider? Register the generator and differ directly instead: ```csharp var provider = new ServiceCollection() diff --git a/src/EFCore.ComplexIndexes.SqlServer/CHANGELOG.md b/src/EFCore.ComplexIndexes.SqlServer/CHANGELOG.md index 4297869..5ce9849 100644 --- a/src/EFCore.ComplexIndexes.SqlServer/CHANGELOG.md +++ b/src/EFCore.ComplexIndexes.SqlServer/CHANGELOG.md @@ -4,6 +4,16 @@ Changes to the SQL Server satellite, newest first. The [root changelog](https://github.com/CaffeinatedCoder/EFCore.ComplexIndexes/blob/main/CHANGELOG.md) covers all three packages. +## 5.1.0 + +- **New:** `UseSqlServerComplexIndexes()` / `AddSqlServerComplexIndexes()` — optional runtime + registration of the differ, so `EnsureCreated()` and `GenerateCreateScript()` include the complex + indexes and `Migrate()`'s pending-model-changes check sees one that was never scaffolded. Migrations + still need no wiring. +- **Fixed:** a PostgreSQL option (`UseGin`, `HasOperators`, …) on a *property-level* complex index is + rejected at `migrations add` like an entity-level one, instead of being dropped by the forwarding + whitelist — the index scaffolded as a plain B-tree without a word. + ## 5.0.3 - **Changed:** the `Microsoft.EntityFrameworkCore.SqlServer` dependency is now `[10.0.0, 11.0.0)`. diff --git a/src/EFCore.ComplexIndexes.SqlServer/README.md b/src/EFCore.ComplexIndexes.SqlServer/README.md index df7c636..e2cede1 100644 --- a/src/EFCore.ComplexIndexes.SqlServer/README.md +++ b/src/EFCore.ComplexIndexes.SqlServer/README.md @@ -17,9 +17,20 @@ Brings the SQL Server option set to complex-property indexes: ## Setup -None. Every option flows as a native SQL Server annotation that the provider's own migrations SQL -generator renders, so there is **no runtime wiring at all** — install the package, declare your -indexes, and run `dotnet ef migrations add`. +None for migrations. Every option flows as a native SQL Server annotation that the provider's own +migrations SQL generator renders — install the package, declare your indexes, and run +`dotnet ef migrations add`. + +One optional call exists. `Database.EnsureCreated()`, `GenerateCreateScript()` and the +pending-model-changes check in `Migrate()` run the *runtime* differ, which the design-time wiring never +reaches; without a registration, `EnsureCreated()` creates the tables and silently none of the +indexes. Register the differ once so those see the complex indexes too: + +```csharp +options.UseSqlServer(connectionString).UseSqlServerComplexIndexes(); +``` + +`AddSqlServerComplexIndexes()` is the equivalent for a custom internal service provider. ## Usage diff --git a/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexDbContextOptionsExtensions.cs b/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexDbContextOptionsExtensions.cs new file mode 100644 index 0000000..5323c5e --- /dev/null +++ b/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexDbContextOptionsExtensions.cs @@ -0,0 +1,42 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.Extensions.DependencyInjection; + +namespace EFCore.ComplexIndexes.SqlServer; + +/// +/// Optional runtime wiring for SQL Server. Migrations need none: every option renders through the +/// provider's own SQL generator. This registers the SQL Server differ at runtime so that +/// EnsureCreated() and GenerateCreateScript() include the complex indexes, and so that +/// Migrate()'s pending-model-changes check sees a complex index that was never scaffolded. +/// +public static class SqlServerComplexIndexDbContextOptionsExtensions +{ + extension(DbContextOptionsBuilder optionsBuilder) + { + /// + /// Registers the SQL Server complex-index differ at runtime. Call it after the provider: + /// options.UseSqlServer(connectionString).UseSqlServerComplexIndexes(); + /// + public DbContextOptionsBuilder UseSqlServerComplexIndexes() + => optionsBuilder.ReplaceService(); + } + + extension(DbContextOptionsBuilder optionsBuilder) where TContext : DbContext + { + /// + public DbContextOptionsBuilder UseSqlServerComplexIndexes() + => (DbContextOptionsBuilder)((DbContextOptionsBuilder)optionsBuilder).UseSqlServerComplexIndexes(); + } + + extension(IServiceCollection services) + { + /// + /// Registers the SQL Server complex-index differ on a custom internal service provider — the + /// equivalent of UseSqlServerComplexIndexes() for applications that build their own + /// IServiceProvider and pass it to UseInternalServiceProvider. + /// + public IServiceCollection AddSqlServerComplexIndexes() + => services.AddScoped(); + } +} diff --git a/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexMigrationsModelDiffer.cs b/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexMigrationsModelDiffer.cs index bead986..84e3621 100644 --- a/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexMigrationsModelDiffer.cs +++ b/src/EFCore.ComplexIndexes.SqlServer/SqlServerComplexIndexMigrationsModelDiffer.cs @@ -39,9 +39,16 @@ CommandBatchPreparerDependencies commandBatchPreparerDependencies SqlServerAnnotations.DataCompression ]; - /// Forwards exactly the SQL Server index-option annotations the provider's SQL generator renders. + /// + /// Forwards exactly the SQL Server index-option annotations the provider's SQL generator renders — + /// and every Npgsql:* key, which is forwarded only so that + /// rejects it. A property-level .UseGin() on a + /// model diffed by this satellite would otherwise be dropped by the whitelist without a word, + /// leaving a plain B-tree where the entity-level declaration of the same option fails loudly. + /// protected override bool IsForwardedIndexAnnotation(string annotationName) - => SupportedSqlServerAnnotations.Contains(annotationName); + => SupportedSqlServerAnnotations.Contains(annotationName) + || annotationName.StartsWith("Npgsql:", StringComparison.Ordinal); /// SQL Server renames indexes standalone (sp_rename). protected override bool CanRenameIndexes => true; diff --git a/src/EFCore.ComplexIndexes/CHANGELOG.md b/src/EFCore.ComplexIndexes/CHANGELOG.md index 17bbb7a..c6fee4a 100644 --- a/src/EFCore.ComplexIndexes/CHANGELOG.md +++ b/src/EFCore.ComplexIndexes/CHANGELOG.md @@ -4,6 +4,26 @@ Changes to the core package, newest first. The [root changelog](https://github.com/CaffeinatedCoder/EFCore.ComplexIndexes/blob/main/CHANGELOG.md) covers all three packages. +## 5.1.0 + +- **Fixed:** `HasDifferences` now reports changes to complex indexes (and, through the satellites' + overrides, exclusion and temporal constraints). EF Core's base implementation bypasses + `GetDifferences`, so `dotnet ef migrations has-pending-model-changes`, the pending-model-changes + warning `Migrate()` raises, and the snapshot check in `migrations remove` all reported "no changes" + when only a declaration from this package had changed. +- **New:** `UseComplexIndexes()` / `AddComplexIndexes()` register the differ at runtime, for providers + without a satellite package. `EnsureCreated()`, `GenerateCreateScript()` and `Migrate()`'s + pending-model-changes check run the runtime differ, which the design-time wiring never reaches — + without this, `EnsureCreated()` created the tables and silently none of the complex indexes. +- **Fixed:** a complex index named like a native `HasIndex` on the same table is rejected at + `migrations add` instead of scaffolding two `CREATE INDEX` statements under one name that fail when + applied. An index moving between a native and a complex declaration under one name still diffs. +- **New:** the property-level `HasComplexIndex` overloads also exist on the non-generic + `ComplexTypePropertyBuilder` (`c.Property("Value").HasComplexIndex()`). +- **Changed:** a complex index declared on an entity type mapped to no table — typically the abstract + base of a TPC hierarchy — fails at `migrations add` instead of producing nothing. View-mapped and + query-mapped types are still skipped. + ## 5.0.3 - **Changed:** the `Microsoft.EntityFrameworkCore.Abstractions` dependency is now `[10.0.0, 11.0.0)`. diff --git a/src/EFCore.ComplexIndexes/ComplexIndexDbContextOptionsExtensions.cs b/src/EFCore.ComplexIndexes/ComplexIndexDbContextOptionsExtensions.cs new file mode 100644 index 0000000..973d86d --- /dev/null +++ b/src/EFCore.ComplexIndexes/ComplexIndexDbContextOptionsExtensions.cs @@ -0,0 +1,56 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations; +using Microsoft.Extensions.DependencyInjection; + +namespace EFCore.ComplexIndexes; + +/// +/// Runtime wiring of the complex-index differ, for providers without a satellite package (SQLite, …). +/// +/// +/// Migrations are scaffolded by the design-time differ, which the packaged .targets +/// wire up automatically. Two things run the runtime differ instead and never see that +/// wiring: EnsureCreated() / GenerateCreateScript(), which build the schema straight +/// from the model, and the pending-model-changes check Migrate() performs. Without this +/// registration both use EF's stock differ, which cannot see this package's declarations — +/// EnsureCreated() creates the tables without their complex indexes, silently, and +/// Migrate() does not warn about a complex index that was never scaffolded. +/// +/// With a provider satellite installed, call that package's method instead +/// (UseNpgsqlComplexIndexes(), UseSqlServerComplexIndexes()): the core differ registered +/// here would give EnsureCreated() a schema without the satellite's features, such as +/// exclusion constraints. +/// +/// +public static class ComplexIndexDbContextOptionsExtensions +{ + extension(DbContextOptionsBuilder optionsBuilder) + { + /// + /// Registers the complex-index differ at runtime, so EnsureCreated(), + /// GenerateCreateScript() and Migrate()'s pending-model-changes check see the + /// indexes declared with this package. Call it after the provider: + /// options.UseSqlite(connection).UseComplexIndexes(); + /// + public DbContextOptionsBuilder UseComplexIndexes() + => optionsBuilder.ReplaceService(); + } + + extension(DbContextOptionsBuilder optionsBuilder) where TContext : DbContext + { + /// + public DbContextOptionsBuilder UseComplexIndexes() + => (DbContextOptionsBuilder)((DbContextOptionsBuilder)optionsBuilder).UseComplexIndexes(); + } + + extension(IServiceCollection services) + { + /// + /// Registers the complex-index differ on a custom internal service provider — the equivalent + /// of UseComplexIndexes() for applications that build their own + /// IServiceProvider and pass it to UseInternalServiceProvider. + /// + public IServiceCollection AddComplexIndexes() + => services.AddScoped(); + } +} diff --git a/src/EFCore.ComplexIndexes/ComplexIndexExtensions.cs b/src/EFCore.ComplexIndexes/ComplexIndexExtensions.cs index 40ee620..516a71c 100644 --- a/src/EFCore.ComplexIndexes/ComplexIndexExtensions.cs +++ b/src/EFCore.ComplexIndexes/ComplexIndexExtensions.cs @@ -12,7 +12,10 @@ public static class ComplexIndexExtensions { // ── Single-column index on a complex type property ── - extension(ComplexTypePropertyBuilder builder) + // The non-generic builder is what EF hands back for properties configured by name + // (`c.Property("Value")`) or type (`c.Property(typeof(string), "Value")`); the generic one derives + // from it, so the typed overloads below delegate here and keep their typed return. + extension(ComplexTypePropertyBuilder builder) { /// /// Configures a single-column index on a complex type property. @@ -21,7 +24,7 @@ public static class ComplexIndexExtensions /// A SQL filter for the index. /// The custom name of the index. /// The same builder instance so that multiple configuration calls can be chained. - public ComplexTypePropertyBuilder HasComplexIndex( + public ComplexTypePropertyBuilder HasComplexIndex( bool isUnique = false, string? filter = null, string? indexName = null @@ -44,8 +47,10 @@ public ComplexTypePropertyBuilder HasComplexIndex( /// Provider-specific options (e.g., GIN, clustered) are available as extension methods /// on from the corresponding satellite package. /// - public ComplexTypePropertyBuilder HasComplexIndex(Action configure) + public ComplexTypePropertyBuilder HasComplexIndex(Action configure) { + ArgumentNullException.ThrowIfNull(configure); + var indexBuilder = new ComplexIndexBuilder(); configure(indexBuilder); @@ -58,6 +63,27 @@ public ComplexTypePropertyBuilder HasComplexIndex(Action(ComplexTypePropertyBuilder builder) + { + /// + public ComplexTypePropertyBuilder HasComplexIndex( + bool isUnique = false, + string? filter = null, + string? indexName = null + ) + { + ((ComplexTypePropertyBuilder)builder).HasComplexIndex(isUnique, filter, indexName); + return builder; + } + + /// + public ComplexTypePropertyBuilder HasComplexIndex(Action configure) + { + ((ComplexTypePropertyBuilder)builder).HasComplexIndex(configure); + return builder; + } + } + // ── Multi-column composite index ── extension(EntityTypeBuilder builder) diff --git a/src/EFCore.ComplexIndexes/CustomMigrationsModelDiffer.cs b/src/EFCore.ComplexIndexes/CustomMigrationsModelDiffer.cs index 00ba00f..d3c9de0 100644 --- a/src/EFCore.ComplexIndexes/CustomMigrationsModelDiffer.cs +++ b/src/EFCore.ComplexIndexes/CustomMigrationsModelDiffer.cs @@ -75,6 +75,7 @@ public override IReadOnlyList GetDifferences( // Target only: the source is history. A snapshot that already contains a collision must // still be diffable, or the model could never be fixed. ValidateUniqueIndexNames(targetIndexes); + ValidateNoNativeIndexNameCollision(target, targetIndexes); if (sourceIndexes.Count == 0 && targetIndexes.Count == 0) return operations; @@ -176,7 +177,7 @@ public override IReadOnlyList GetDifferences( // Forward the whitelisted provider annotations — provider SQL generators handle their own foreach (var (key, value) in tgt.ProviderAnnotations) - op.AddAnnotation(key, value); + op.AddAnnotation(ToOperationAnnotationName(key), value); // Ordered parts are needed when the stock generator can't render the index: expression // parts have no slot in Columns, and NULLS FIRST/LAST has no slot on the native @@ -197,6 +198,24 @@ public override IReadOnlyList GetDifferences( return [.. drops, .. operations, .. renames, .. creates]; } + /// + /// Reports whether the two models differ, including in the declarations this package owns. + /// + /// + /// EF Core's implementation runs its protected Diff directly rather than the public + /// this class overrides, so it never saw a complex index, exclusion + /// constraint or temporal constraint change. Everything built on it then reported "no changes" + /// for exactly those changes: dotnet ef migrations has-pending-model-changes, the + /// pending-model-changes warning Migrate() raises, and the snapshot check in + /// migrations remove. Routing through also picks up whatever + /// a provider satellite adds in its own override. + /// + /// The model migrated from — typically the snapshot. + /// The model migrated to — the current OnModelCreating result. + /// true if migrating from to needs any operation. + public override bool HasDifferences(IRelationalModel? source, IRelationalModel? target) + => GetDifferences(source, target).Count > 0; + /// /// Called for each this differ emits, before it joins the /// operation list. Provider satellites override this to reject declarations their provider @@ -242,6 +261,20 @@ protected virtual void ValidateCreatedIndexes(IReadOnlyList protected virtual bool IsForwardedIndexAnnotation(string annotationName) => false; + /// + /// Maps the key an index option is stored under to the key the provider's SQL generator + /// reads from the operation, when the two differ. The default keeps the key. + /// + /// + /// Options are stored under provider model keys (Npgsql:IndexCollation) because the + /// property-level API writes them onto the property, where an EF relational key such as + /// Relational:Collation would be read as a column facet. Npgsql's generator, + /// however, reads index collations from Relational:Collation on the operation — the model + /// annotation provider does that mapping for native indexes, and this hook does it for ours. + /// Comparison happens on the stored key, so the mapping never affects diffing. + /// + protected virtual string ToOperationAnnotationName(string annotationName) => annotationName; + /// /// Transforms a forwarded provider-annotation value before it is compared and stamped onto the /// index operation. Satellites use this to resolve property paths inside their option values — @@ -328,6 +361,55 @@ static string Describe(IndexDescriptor descriptor) } } + /// + /// Fails when a complex index resolves to the name of a native HasIndex on the same table. + /// + /// + /// The base differ emits the native index and this differ emits the complex one, neither seeing + /// the other, so the migration scaffolded two CREATE INDEX statements under one name and + /// failed at apply time (PostgreSQL 42P07). Only the target model's native indexes are consulted: + /// an index moving between a native declaration and a complex one under the same name + /// is a legitimate drop-and-create, not a collision, and must keep diffing. + /// + private static void ValidateNoNativeIndexNameCollision(IRelationalModel? target, HashSet descriptors) + { + if (target is null || descriptors.Count == 0) + return; + + var native = new Dictionary<(string Table, string? Schema, string Name), string>(); + + foreach (var entityType in target.Model.GetEntityTypes()) + { + var tableName = entityType.GetTableName(); + if (tableName is null) continue; + + var schema = entityType.GetSchema(); + var storeObject = StoreObjectIdentifier.Table(tableName, schema); + + // GetIndexes includes inherited ones; under TPH the base and derived types share the table + // and report the same index, which TryAdd collapses. + foreach (var index in entityType.GetIndexes()) + { + var name = index.GetDatabaseName(storeObject); + if (name is null) continue; + + native.TryAdd((tableName, schema, name), string.Join(", ", index.Properties.Select(p => p.Name))); + } + } + + foreach (var descriptor in descriptors) + { + if (!native.TryGetValue((descriptor.TableName, descriptor.Schema, descriptor.IndexName), out var properties)) + continue; + + throw new InvalidOperationException( + $"The complex index '{descriptor.IndexName}' on table '{descriptor.TableName}' has the same name as " + + $"the native index on ({properties}) declared with HasIndex. Index names must be unique per table — " + + "the migration would scaffold two CREATE INDEX statements under one name and fail when applied. " + + "Give one of them a different name."); + } + } + private HashSet ExtractAllIndexDescriptors(IRelationalModel? relationalModel) { var result = new HashSet(); @@ -337,7 +419,12 @@ private HashSet ExtractAllIndexDescriptors(IRelationalModel? re { var tableName = entityType.GetTableName(); var schema = entityType.GetSchema(); - if (tableName is null) continue; + if (tableName is null) + { + if (DeclaresComplexIndexes(entityType)) + ThrowIfDeclaredOnUnmappedType(entityType, "complex indexes"); + continue; + } var storeObject = StoreObjectIdentifier.Table(tableName, schema); @@ -348,6 +435,37 @@ private HashSet ExtractAllIndexDescriptors(IRelationalModel? re return result; } + /// + /// Fails when is mapped to no table yet carries declarations only a + /// table can satisfy. Call it after establishing both; satellites use it for their own descriptors. + /// + /// + /// The usual shape is the abstract base of a TPC hierarchy: it has no table of its own, so its + /// declarations produced nothing — no DDL, no error. Types mapped to a view, a SQL query or a + /// function are left alone: an index on those is nothing this package could create, and models + /// have carried the annotation there harmlessly. + /// + protected static void ThrowIfDeclaredOnUnmappedType(IEntityType entityType, string declarations) + { + if (entityType.GetViewName() is not null + || entityType.GetSqlQuery() is not null + || entityType.GetFunctionName() is not null) + return; + + throw new InvalidOperationException( + $"'{entityType.DisplayName()}' declares {declarations} but is not mapped to a table, so they cannot be " + + "created. This is typically the abstract base of a TPC hierarchy, whose columns live on each concrete " + + "table: declare them on the concrete entity types instead."); + } + + private static bool DeclaresComplexIndexes(IEntityType entityType) + => entityType.FindAnnotation(ComplexIndexAnnotations.CompositeIndexes)?.Value is string { Length: > 2 } + || DeclaresPropertyIndexes(entityType); + + private static bool DeclaresPropertyIndexes(ITypeBase typeBase) + => typeBase.GetDeclaredProperties().Any(p => p.FindAnnotation(ComplexIndexAnnotations.IsIndexed)?.Value is true) + || typeBase.GetDeclaredComplexProperties().Any(cp => DeclaresPropertyIndexes(cp.ComplexType)); + private void ScanForSingleColumnIndexes( IEntityType rootEntityType, ITypeBase typeBase, diff --git a/test/EFCore.ComplexIndexes.Tests/BuilderApiParityTests.cs b/test/EFCore.ComplexIndexes.Tests/BuilderApiParityTests.cs index 3555cf1..945e635 100644 --- a/test/EFCore.ComplexIndexes.Tests/BuilderApiParityTests.cs +++ b/test/EFCore.ComplexIndexes.Tests/BuilderApiParityTests.cs @@ -74,6 +74,7 @@ private static object SampleArgument(ParameterInfo parameter, string methodName) if (type == typeof(int)) return 1; // fill factor is range-checked to 1..100 if (type == typeof(string)) return "sample"; if (type == typeof(string[])) return new[] { "sample" }; + if (type == typeof(object)) return 1; // storage parameter value if (type.IsEnum) return Enum.GetValues(type).GetValue(0)!; throw new NotSupportedException( diff --git a/test/EFCore.ComplexIndexes.Tests/CrossProviderOptionRejectionTests.cs b/test/EFCore.ComplexIndexes.Tests/CrossProviderOptionRejectionTests.cs new file mode 100644 index 0000000..afc6259 --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/CrossProviderOptionRejectionTests.cs @@ -0,0 +1,101 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using EFCore.ComplexIndexes.SqlServer; +using Microsoft.EntityFrameworkCore; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// An index option from the other satellite must fail at migrations add whichever way it was +/// declared. Entity-level options reach the operation unfiltered and were always rejected; the +/// property-level path goes through the forwarding whitelist, which dropped them without a word — a +/// property-level .UseGin() diffed by the SQL Server satellite scaffolded a plain B-tree. +/// The PostgreSQL differ had the mirror gap for both paths: SqlServer:* options passed +/// through to Npgsql's generator, which ignored them. +/// +[TestClass] +public class CrossProviderOptionRejectionTests +{ + private class Payload + { + public string Json { get; set; } = ""; + } + + private class Document + { + public Guid Id { get; set; } + public string Title { get; set; } = ""; + public Payload Payload { get; set; } = new(); + } + + private class PropertyLevelGinContext(DbContextOptions options) : DbContext(options) + { + public DbSet Documents => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("docs"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Payload, c => c.Property(x => x.Json).HasComplexIndex(ix => ix.UseGin())); + }); + } + + private class PropertyLevelClusteredContext(DbContextOptions options) : DbContext(options) + { + public DbSet Documents => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("docs"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Payload, c => c.Property(x => x.Json).HasComplexIndex(ix => ix.HasFillFactor(80))); + }); + } + + private class EntityLevelFillFactorContext(DbContextOptions options) : DbContext(options) + { + public DbSet Documents => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("docs"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Payload); + b.HasComplexIndex(x => x.Payload.Json, ix => ix.HasFillFactor(80)); + }); + } + + [TestMethod(DisplayName = "SQL Server rejects a property-level PostgreSQL option instead of dropping it")] + public void SqlServer_rejects_property_level_npgsql_option() + { + var target = MigrationHarness.SqlServerModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.SqlServerDiff(null, target)); + + StringAssert.Contains(exception.Message, "Npgsql:IndexMethod"); + StringAssert.Contains(exception.Message, "SQL Server satellite"); + } + + [TestMethod(DisplayName = "PostgreSQL rejects a property-level SQL Server option instead of dropping it")] + public void Npgsql_rejects_property_level_sqlserver_option() + { + var target = MigrationHarness.NpgsqlModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.NpgsqlDiff(null, target)); + + StringAssert.Contains(exception.Message, "SqlServer:FillFactor"); + StringAssert.Contains(exception.Message, "PostgreSQL satellite"); + } + + [TestMethod(DisplayName = "PostgreSQL rejects an entity-level SQL Server option instead of passing it to a generator that ignores it")] + public void Npgsql_rejects_entity_level_sqlserver_option() + { + var target = MigrationHarness.NpgsqlModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.NpgsqlDiff(null, target)); + + StringAssert.Contains(exception.Message, "SqlServer:FillFactor"); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/DesignTimeServiceRegistrationTests.cs b/test/EFCore.ComplexIndexes.Tests/DesignTimeServiceRegistrationTests.cs index 611eb84..c5e001b 100644 --- a/test/EFCore.ComplexIndexes.Tests/DesignTimeServiceRegistrationTests.cs +++ b/test/EFCore.ComplexIndexes.Tests/DesignTimeServiceRegistrationTests.cs @@ -4,6 +4,7 @@ using Microsoft.EntityFrameworkCore.Design; using Microsoft.EntityFrameworkCore.Migrations; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace EFCore.ComplexIndexes.Tests; @@ -65,6 +66,35 @@ public void Satellite_replaces_rather_than_stacks() Assert.ContainsSingle(services.Where(d => d.ServiceType == typeof(IMigrationsModelDiffer))); } + /// + /// EF's AddDbContextDesignTimeServices seeds the design-time collection with the context's + /// own differ as a factory registration — which, once a consumer opts into the runtime wiring, is + /// already one of ours. The design-time registration has to win over that seed in every + /// combination, or a runtime UseComplexIndexes() could change which differ scaffolds. + /// + [TestMethod(DisplayName = "A differ seeded from the context does not displace the design-time registration")] + public void Context_seeded_differ_does_not_win() + { + foreach (var configurators in new IDesignTimeServices[][] + { + [new CustomDesignTimeServices()], + [new CustomDesignTimeServices(), new NpgsqlComplexIndexDesignTimeServices()], + [new SqlServerComplexIndexDesignTimeServices(), new CustomDesignTimeServices()] + }) + { + var services = new ServiceCollection(); + services.TryAdd(ServiceDescriptor.Scoped(_ => throw new InvalidOperationException("seed"))); + + foreach (var configurator in configurators) + configurator.ConfigureDesignTimeServices(services); + + var winner = services.Last(d => d.ServiceType == typeof(IMigrationsModelDiffer)); + + Assert.IsNotNull(winner.ImplementationType, "The context-seeded factory registration won."); + Assert.IsTrue(typeof(CustomMigrationsModelDiffer).IsAssignableFrom(winner.ImplementationType)); + } + } + [TestMethod(DisplayName = "Core alone still registers the core differ")] public void Core_alone_registers_core_differ() => Assert.AreEqual(typeof(CustomMigrationsModelDiffer), ResolveDiffer(new CustomDesignTimeServices())); diff --git a/test/EFCore.ComplexIndexes.Tests/DocumentationApiTests.cs b/test/EFCore.ComplexIndexes.Tests/DocumentationApiTests.cs index 8dd3c21..7fa5ba4 100644 --- a/test/EFCore.ComplexIndexes.Tests/DocumentationApiTests.cs +++ b/test/EFCore.ComplexIndexes.Tests/DocumentationApiTests.cs @@ -50,10 +50,13 @@ public class DocumentationApiTests private static readonly HashSet ExternalApi = new(StringComparer.Ordinal) { // EF Core - "ComplexProperty", "Property", "HasColumnName", "HasKey", "ToJson", "Entity", + "ComplexProperty", "ComplexCollection", "Property", "HasColumnName", "HasKey", "ToJson", "Entity", "MigrationsAssembly", "UseInternalServiceProvider", + "EnsureCreated", "GenerateCreateScript", "Migrate", // Npgsql "UseNpgsql", "AddEntityFrameworkNpgsql", + // SQL Server + "UseSqlServer", // Dependency injection "ServiceCollection", "BuildServiceProvider", "AddDbContext", // BCL diff --git a/test/EFCore.ComplexIndexes.Tests/IndexNameCollisionTests.cs b/test/EFCore.ComplexIndexes.Tests/IndexNameCollisionTests.cs new file mode 100644 index 0000000..0566adb --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/IndexNameCollisionTests.cs @@ -0,0 +1,146 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// A complex index and a native HasIndex on the same table can resolve to the same name. The +/// base differ emits one and this differ the other, neither seeing the other, so the migration +/// scaffolded two CREATE INDEX statements under one name and failed at apply time (42P07). +/// The differ's own name check only compared complex indexes with each other. +/// +[TestClass] +public class IndexNameCollisionTests +{ + private class EmailAddress + { + public string Value { get; set; } = ""; + } + + private class Person + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + public EmailAddress Email { get; set; } = new(); + } + + private class Company + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + } + + private class CollisionContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.HasIndex(x => x.Name).HasDatabaseName("IX_dup"); + b.ComplexProperty(x => x.Email); + b.HasComplexIndex(x => x.Email.Value, indexName: "IX_dup"); + }); + } + + // Same name on a different table is not a collision. + private class SeparateTablesContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + public DbSet Companies => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(b => + { + b.ToTable("companies"); + b.HasKey(x => x.Id); + b.HasIndex(x => x.Name).HasDatabaseName("IX_name"); + }); + + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email); + b.HasComplexIndex(x => x.Email.Value, indexName: "IX_name"); + }); + } + } + + // The handover shape: the index used to be native and is now complex, under one name. + private class NativeBeforeContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.HasIndex(x => x.Name).HasDatabaseName("IX_people_name"); + b.ComplexProperty(x => x.Email); + }); + } + + private class ComplexAfterContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email); + b.HasComplexIndex(x => x.Email.Value, indexName: "IX_people_name"); + }); + } + + [TestMethod(DisplayName = "A complex index named like a native HasIndex on the same table is rejected at migrations add")] + public void Collision_with_native_index_throws() + { + var target = MigrationHarness.SqliteModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.CoreDiff(null, target)); + + StringAssert.Contains(exception.Message, "IX_dup"); + StringAssert.Contains(exception.Message, "HasIndex"); + StringAssert.Contains(exception.Message, "Name"); + } + + [TestMethod(DisplayName = "The satellite differs inherit the check")] + public void Collision_is_rejected_by_the_npgsql_differ() + { + var target = MigrationHarness.NpgsqlModel(); + + Assert.ThrowsExactly(() => MigrationHarness.NpgsqlDiff(null, target)); + } + + [TestMethod(DisplayName = "The same name on a different table is not a collision")] + public void Same_name_on_another_table_is_fine() + { + var target = MigrationHarness.SqliteModel(); + + var creates = MigrationHarness.CoreDiff(null, target).OfType().Where(o => o.Name == "IX_name").ToList(); + + Assert.HasCount(2, creates); + CollectionAssert.AreEquivalent(new[] { "companies", "people" }, creates.Select(o => o.Table).ToList()); + } + + [TestMethod(DisplayName = "An index moving from native to complex under one name still diffs")] + public void Handover_from_native_to_complex_still_diffs() + { + var source = MigrationHarness.SqliteModel(); + var target = MigrationHarness.SqliteModel(); + + var operations = MigrationHarness.CoreDiff(source, target); + + // The native index is dropped by the base differ, the complex one created here; only the + // target's native indexes are consulted, so this is a legitimate move, not a collision. + Assert.IsTrue(operations.OfType().Any(o => o.Name == "IX_people_name")); + Assert.IsTrue(operations.OfType().Any(o => o.Name == "IX_people_name")); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/NpgsqlCollationTests.cs b/test/EFCore.ComplexIndexes.Tests/NpgsqlCollationTests.cs new file mode 100644 index 0000000..c88eee4 --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/NpgsqlCollationTests.cs @@ -0,0 +1,169 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// Per-column index collations. The option is stored under Npgsql's model key +/// (Npgsql:IndexCollation) — on a property, EF's Relational:Collation would mean the +/// column's collation — and mapped to Relational:Collation on the operation, which is +/// where Npgsql's generator reads it. The reverse must never happen: a column's own collation is a +/// column facet and stays off the index. +/// +[TestClass] +public class NpgsqlCollationTests +{ + private const string OperationCollation = "Relational:Collation"; + + private class EmailAddress + { + public string Value { get; set; } = ""; + } + + private class Person + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + public EmailAddress Email { get; set; } = new(); + } + + private class CollationContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email, c => + { + c.Property(x => x.Value).HasColumnName("email"); + + // Property-level: the option travels through the whitelist. + c.Property(x => x.Value).HasComplexIndex(ix => ix.UseCollation("C").HasName("ix_people_email_c")); + }); + + // Entity-level, positional: only the first column is collated. + b.HasComplexCompositeIndex(x => new { x.Name, x.Email.Value }, ix => ix + .UseCollation("C", "") + .HasName("ix_people_name_email")); + + // Expression index with a collation and an operator class: COLLATE goes first. + b.HasExpressionIndex(ix => ix + .Expression("lower(email)") + .UseCollation("C") + .HasOperators("text_pattern_ops") + .HasName("ix_people_email_ci")); + }); + } + + // The leak case: the column has a collation of its own, the index does not. + private class ColumnCollationContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email, c => + { + c.Property(x => x.Value).HasColumnName("email").UseCollation("de-DE-x-icu"); + c.Property(x => x.Value).HasComplexIndex(indexName: "ix_people_email"); + }); + }); + } + + // Only the collation option, entity-level so it reaches the operation unfiltered, so the SQL + // Server rejection below is unambiguously about it. + private class IndexCollationOnlyContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email); + b.HasComplexIndex(x => x.Email.Value, ix => ix.UseCollation("C")); + }); + } + + private static Dictionary Creates() + => MigrationHarness.NpgsqlDiff(null, MigrationHarness.NpgsqlModel()) + .OfType() + .ToDictionary(o => o.Name); + + [TestMethod(DisplayName = "A property-level index collation reaches the operation under Relational:Collation")] + public void Property_level_collation_is_mapped_onto_the_operation() + { + var op = Creates()["ix_people_email_c"]; + + CollectionAssert.AreEqual(new[] { "C" }, (string[])op[OperationCollation]!); + Assert.IsNull(op["Npgsql:IndexCollation"], "The stored key must not leak onto the operation alongside the mapped one."); + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op], complexIndexWiring: false), + "CREATE INDEX ix_people_email_c ON people (email COLLATE \"C\")"); + } + + [TestMethod(DisplayName = "Entity-level collations are positional and render through the stock generator")] + public void Entity_level_collation_is_positional() + { + var op = Creates()["ix_people_name_email"]; + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op], complexIndexWiring: false), + "ON people (\"Name\" COLLATE \"C\", email)"); + } + + [TestMethod(DisplayName = "The custom generator renders COLLATE before the operator class on expression indexes")] + public void Expression_index_collation_renders_before_operator_class() + { + var op = Creates()["ix_people_email_ci"]; + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op]), + "CREATE INDEX ix_people_email_ci ON people ((lower(email)) COLLATE \"C\" text_pattern_ops)"); + } + + [TestMethod(DisplayName = "A column's own collation is never copied onto the index")] + public void Column_collation_does_not_leak_onto_the_index() + { + var op = MigrationHarness.NpgsqlDiff(null, MigrationHarness.NpgsqlModel()) + .OfType() + .Single(o => o.Name == "ix_people_email"); + + Assert.IsNull(op[OperationCollation]); + Assert.IsNull(op["Npgsql:IndexCollation"]); + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op], complexIndexWiring: false), + "CREATE INDEX ix_people_email ON people (email);"); + } + + [TestMethod(DisplayName = "Collations do not churn between two builds of the same model")] + public void Collations_do_not_churn() + { + var operations = MigrationHarness.NpgsqlDiff( + MigrationHarness.NpgsqlModel(), + MigrationHarness.NpgsqlModel()); + + Assert.IsFalse(operations.OfType().Any()); + Assert.IsFalse(operations.OfType().Any()); + } + + [TestMethod(DisplayName = "The SQL Server differ rejects a PostgreSQL collation option with a targeted error")] + public void SqlServer_rejects_npgsql_collation() + { + var target = MigrationHarness.SqlServerModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.SqlServerDiff(null, target)); + + StringAssert.Contains(exception.Message, "Npgsql:"); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/NpgsqlJsonContainerIndexTests.cs b/test/EFCore.ComplexIndexes.Tests/NpgsqlJsonContainerIndexTests.cs new file mode 100644 index 0000000..12efecd --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/NpgsqlJsonContainerIndexTests.cs @@ -0,0 +1,160 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// Indexing a ToJson() complex property — or a complex collection, which is always JSON — as a +/// whole: the PostgreSQL idiom is a GIN index over the jsonb container column. Before 5.1.0 +/// the path resolved to nothing and failed with "could not resolve property path", and complex +/// collections were unreachable altogether (their members have no column and their element builder +/// is not the one the property-level API extends). The container is a real column, so these indexes +/// render through the stock generator with no runtime wiring; a sub-document nested inside the +/// document is a -> extraction and goes through the custom generator like any expression. +/// +[TestClass] +public class NpgsqlJsonContainerIndexTests +{ + private class Address + { + public string City { get; set; } = ""; + } + + private class Payload + { + public string Note { get; set; } = ""; + public Address Address { get; set; } = new(); + } + + private class Tag + { + public string Name { get; set; } = ""; + } + + private class Order + { + public int Id { get; set; } + public string Name { get; set; } = ""; + public Payload Payload { get; set; } = new(); + public List Tags { get; set; } = []; + } + + private class ContainerIndexContext(DbContextOptions options) : DbContext(options) + { + public DbSet Orders => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("orders"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Payload, c => + { + c.ToJson("payload"); + c.ComplexProperty(p => p.Address, a => a.HasJsonPropertyName("addr")); + }); + b.ComplexCollection(x => x.Tags, c => c.ToJson("tags")); + + // Whole document, with an operator class: the common jsonb_path_ops GIN. + b.HasComplexIndex(x => x.Payload, ix => ix.UseGin().HasOperators("jsonb_path_ops").HasName("ix_orders_payload")); + + // Whole collection. + b.HasComplexIndex(x => x.Tags, ix => ix.UseGin().HasName("ix_orders_tags")); + + // A sub-document inside the JSON document. + b.HasComplexIndex(x => x.Payload.Address, ix => ix.UseGin().HasName("ix_orders_payload_addr")); + + // The container column as one part of a composite index. + b.HasComplexCompositeIndex(x => new { x.Name, x.Payload }, indexName: "ix_orders_name_payload"); + }); + } + + private static Dictionary Creates() + => MigrationHarness.NpgsqlDiff(null, MigrationHarness.NpgsqlModel()) + .OfType() + .ToDictionary(o => o.Name); + + [TestMethod(DisplayName = "A ToJson complex property resolves to its container column — a plain column index")] + public void Json_property_resolves_to_container_column() + { + var op = Creates()["ix_orders_payload"]; + + CollectionAssert.AreEqual(new[] { "payload" }, op.Columns); + Assert.AreEqual("gin", op["Npgsql:IndexMethod"]); + CollectionAssert.AreEqual(new[] { "jsonb_path_ops" }, (string[])op["Npgsql:IndexOperators"]!); + Assert.IsNull(op[ComplexIndexAnnotations.IndexParts], "A container column needs no parts annotation."); + Assert.DoesNotContain(CustomMigrationsModelDiffer.RuntimeWiringSentinel, op.Columns); + } + + [TestMethod(DisplayName = "The stock Npgsql generator renders the container GIN index — no runtime wiring needed")] + public void Container_index_renders_through_the_stock_generator() + { + var sql = MigrationHarness.NpgsqlSql([Creates()["ix_orders_payload"]], complexIndexWiring: false); + + StringAssert.Contains(sql, "CREATE INDEX ix_orders_payload ON orders USING gin (payload jsonb_path_ops)"); + } + + [TestMethod(DisplayName = "A complex collection resolves to its container column")] + public void Complex_collection_resolves_to_container_column() + { + var op = Creates()["ix_orders_tags"]; + + CollectionAssert.AreEqual(new[] { "tags" }, op.Columns); + Assert.AreEqual("gin", op["Npgsql:IndexMethod"]); + Assert.IsNull(op[ComplexIndexAnnotations.IndexParts]); + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op], complexIndexWiring: false), + "CREATE INDEX ix_orders_tags ON orders USING gin (tags)"); + } + + [TestMethod(DisplayName = "A sub-document inside the JSON document resolves to a -> extraction honoring HasJsonPropertyName")] + public void Nested_complex_property_resolves_to_jsonb_extraction() + { + var op = Creates()["ix_orders_payload_addr"]; + + Assert.IsNotNull(op[ComplexIndexAnnotations.IndexParts], "An extraction is an expression part."); + Assert.AreEqual("\"payload\" -> 'addr'", op.Columns[0]); + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op]), + "CREATE INDEX ix_orders_payload_addr ON orders USING gin ((\"payload\" -> 'addr'))"); + } + + [TestMethod(DisplayName = "The container column can be one part of a composite index")] + public void Container_column_in_a_composite_index() + { + var op = Creates()["ix_orders_name_payload"]; + + CollectionAssert.AreEqual(new[] { "Name", "payload" }, op.Columns); + Assert.IsNull(op[ComplexIndexAnnotations.IndexParts]); + } + + // ── The core differ has no JSON knowledge: the same declaration must still fail loudly there ── + + private class CoreContainerContext(DbContextOptions options) : DbContext(options) + { + public DbSet Orders => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("orders"); + b.HasKey(x => x.Id); + b.Ignore(x => x.Tags); + b.ComplexProperty(x => x.Payload, c => c.ToJson("payload")); + b.HasComplexIndex(x => x.Payload, indexName: "ix_orders_payload"); + }); + } + + [TestMethod(DisplayName = "The core differ still rejects a container-column index rather than guessing")] + public void Core_differ_rejects_container_index() + { + var target = MigrationHarness.SqliteModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.CoreDiff(null, target)); + + StringAssert.Contains(exception.Message, "Payload"); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/NpgsqlStorageParameterTests.cs b/test/EFCore.ComplexIndexes.Tests/NpgsqlStorageParameterTests.cs new file mode 100644 index 0000000..1aaaa36 --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/NpgsqlStorageParameterTests.cs @@ -0,0 +1,130 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// PostgreSQL storage parameters (WITH (fillfactor=70)) are per-parameter annotations under +/// the Npgsql:StorageParameter: prefix. Npgsql's generator renders them from the operation, so +/// column indexes need no runtime wiring; the custom generator has to render them too for +/// expression indexes, in the same clause position — after INCLUDE/NULLS NOT DISTINCT, +/// before WHERE. The whitelist and the unknown-key rejection match exact keys, so the prefix +/// needs its own rule in both. +/// +[TestClass] +public class NpgsqlStorageParameterTests +{ + private class Payload + { + public string Json { get; set; } = ""; + } + + private class Document + { + public Guid Id { get; set; } + public string Title { get; set; } = ""; + public Payload Payload { get; set; } = new(); + } + + private class StorageParameterContext(DbContextOptions options) : DbContext(options) + { + public DbSet Documents => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("docs"); + b.HasKey(x => x.Id); + b.Property(x => x.Title).HasColumnName("title"); + b.ComplexProperty(x => x.Payload, c => + { + c.Property(x => x.Json).HasColumnName("json").HasColumnType("jsonb"); + + // Property-level: forwarded through the whitelist. + c.Property(x => x.Json).HasComplexIndex(ix => ix + .UseGin() + .HasStorageParameter("fastupdate", false) + .HasName("ix_docs_json")); + }); + + // Entity-level: stored as JSON in the definition, so the value round-trips through + // the serializer before it reaches the operation. + b.HasComplexCompositeIndex(x => new { x.Title, x.Payload.Json }, ix => ix + .HasStorageParameter("fillfactor", 70) + .HasStorageParameter("deduplicate_items", false) + .HasName("ix_docs_title_json")); + + // Expression index: rendered by the custom generator. + b.HasExpressionIndex(ix => ix + .Expression("lower(title)") + .HasStorageParameter("fillfactor", 70) + .HasFilter("title IS NOT NULL") + .HasName("ix_docs_title_ci")); + }); + } + + private static Dictionary Creates() + => MigrationHarness.NpgsqlDiff(null, MigrationHarness.NpgsqlModel()) + .OfType() + .ToDictionary(o => o.Name); + + [TestMethod(DisplayName = "A property-level storage parameter is forwarded and rendered by the stock generator")] + public void Property_level_storage_parameter_renders() + { + var op = Creates()["ix_docs_json"]; + + Assert.AreEqual(false, op["Npgsql:StorageParameter:fastupdate"]); + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op], complexIndexWiring: false), + "CREATE INDEX ix_docs_json ON docs USING gin (json) WITH (fastupdate=false)"); + } + + [TestMethod(DisplayName = "Entity-level storage parameters survive the JSON round trip as their original types")] + public void Entity_level_storage_parameters_render() + { + var op = Creates()["ix_docs_title_json"]; + + // An int, not a double — a boxed 70.0 would render as "70" too, but a generator reading the + // option `as int?` would drop it, which is how FILLFACTOR went missing once before. + Assert.AreEqual(70, op["Npgsql:StorageParameter:fillfactor"]); + Assert.AreEqual(false, op["Npgsql:StorageParameter:deduplicate_items"]); + + var sql = MigrationHarness.NpgsqlSql([op], complexIndexWiring: false); + StringAssert.Contains(sql, "ON docs (title, json) WITH ("); + StringAssert.Contains(sql, "fillfactor=70"); + StringAssert.Contains(sql, "deduplicate_items=false"); + } + + [TestMethod(DisplayName = "The custom generator renders storage parameters on expression indexes, before WHERE")] + public void Expression_index_storage_parameter_renders_in_clause_order() + { + var op = Creates()["ix_docs_title_ci"]; + + StringAssert.Contains( + MigrationHarness.NpgsqlSql([op]), + "CREATE INDEX ix_docs_title_ci ON docs ((lower(title))) WITH (fillfactor=70) WHERE title IS NOT NULL"); + } + + [TestMethod(DisplayName = "Storage parameters do not churn between two builds of the same model")] + public void Storage_parameters_do_not_churn() + { + var operations = MigrationHarness.NpgsqlDiff( + MigrationHarness.NpgsqlModel(), + MigrationHarness.NpgsqlModel()); + + Assert.IsFalse(operations.OfType().Any()); + Assert.IsFalse(operations.OfType().Any()); + } + + [TestMethod(DisplayName = "A string-valued storage parameter is quoted")] + public void String_storage_parameter_is_quoted() + { + var op = new CreateIndexOperation { Name = "ix_s", Table = "docs", Columns = ["lower(title)"] }; + op.AddAnnotation(ComplexIndexAnnotations.IndexParts, IndexPartsSerializer.Serialize([new ResolvedIndexPart(true, "lower(title)")])); + op.AddAnnotation("Npgsql:StorageParameter:buffering", "on"); + + StringAssert.Contains(MigrationHarness.NpgsqlSql([op]), "WITH (buffering='on')"); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/PendingModelChangesTests.cs b/test/EFCore.ComplexIndexes.Tests/PendingModelChangesTests.cs new file mode 100644 index 0000000..07c859b --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/PendingModelChangesTests.cs @@ -0,0 +1,170 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Infrastructure; +using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Migrations; +using NpgsqlTypes; + +namespace EFCore.ComplexIndexes.Tests; + +#pragma warning disable EF1001 + +/// +/// EF Core's MigrationsModelDiffer.HasDifferences runs the protected Diff, not the +/// public GetDifferences this package overrides. Without an override of its own, every check +/// built on it reported "no changes" when only a declaration from this package had changed: +/// dotnet ef migrations has-pending-model-changes, the pending-model-changes warning +/// Migrate() raises, and the snapshot check in migrations remove. A CI gate built on the +/// first of those passed while a complex index was missing from the migrations. +/// +[TestClass] +public class PendingModelChangesTests +{ + private class EmailAddress + { + public string Value { get; set; } = ""; + } + + private class Person + { + public Guid Id { get; set; } + public EmailAddress Email { get; set; } = new(); + } + + private class WithoutIndexContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email); + }); + } + + private class WithIndexContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email); + b.HasComplexIndex(x => x.Email.Value, isUnique: true); + }); + } + + private class Grant + { + public int Id { get; set; } + public int GranteeId { get; set; } + public NpgsqlRange Period { get; set; } + } + + private class WithoutConstraintContext(DbContextOptions options) : DbContext(options) + { + public DbSet Grants => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("grants"); + b.HasKey(x => x.Id); + }); + } + + private class WithConstraintContext(DbContextOptions options) : DbContext(options) + { + public DbSet Grants => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("grants"); + b.HasKey(x => x.Id); + b.HasExclusionConstraint(x => x.GranteeId, x => x.Period); + }); + } + + private static CustomMigrationsModelDiffer CoreDiffer() + { + using var context = new MigrationHarness.EmptyContext( + new DbContextOptionsBuilder().UseSqlite(MigrationHarness.SqliteConnection).Options); + return MigrationHarness.CreateDiffer(context); + } + + private static NpgsqlComplexIndexMigrationsModelDiffer NpgsqlDiffer() + { + using var context = new MigrationHarness.EmptyContext(MigrationHarness.NpgsqlOptions()); + return MigrationHarness.CreateDiffer(context); + } + + [TestMethod(DisplayName = "A complex-index-only change counts as a pending model change")] + public void Complex_index_only_change_is_a_difference() + { + var source = MigrationHarness.SqliteModel(); + var target = MigrationHarness.SqliteModel(); + + Assert.IsTrue( + CoreDiffer().HasDifferences(source, target), + "Adding a complex index is a model change. has-pending-model-changes and Migrate()'s " + + "pending-changes check both rely on HasDifferences to see it."); + } + + [TestMethod(DisplayName = "Removing a complex index counts as a pending model change")] + public void Complex_index_removal_is_a_difference() + { + var source = MigrationHarness.SqliteModel(); + var target = MigrationHarness.SqliteModel(); + + Assert.IsTrue(CoreDiffer().HasDifferences(source, target)); + } + + [TestMethod(DisplayName = "Identical models report no pending change")] + public void Identical_models_are_not_a_difference() + { + var model = MigrationHarness.SqliteModel(); + + // Migrate() also calls HasDifferences with two builds of the same model to detect a + // non-deterministic OnModelCreating; a false positive here would misreport every model. + Assert.IsFalse(CoreDiffer().HasDifferences(model, MigrationHarness.SqliteModel())); + Assert.IsFalse(CoreDiffer().HasDifferences(model, model)); + } + + [TestMethod(DisplayName = "An exclusion-constraint-only change counts as a pending model change (Npgsql)")] + public void Exclusion_constraint_only_change_is_a_difference() + { + var source = MigrationHarness.NpgsqlModel(); + var target = MigrationHarness.NpgsqlModel(); + + // The satellite adds its constraint diffing in its own GetDifferences override; the core's + // HasDifferences has to dispatch through that, not through the base Diff. + Assert.IsTrue(NpgsqlDiffer().HasDifferences(source, target)); + Assert.IsFalse(NpgsqlDiffer().HasDifferences(target, MigrationHarness.NpgsqlModel())); + } + + /// + /// Documents the premise. EF's stock differ cannot see this package's declarations, which is why + /// the runtime registration exists for Migrate()'s check and why the override above exists + /// for the design-time commands. Should a future EF release start seeing them, this test says so. + /// + [TestMethod(DisplayName = "The stock EF differ does not see a complex-index-only change")] + public void Stock_differ_does_not_see_the_change() + { + var source = MigrationHarness.SqliteModel(); + var target = MigrationHarness.SqliteModel(); + + using var context = new MigrationHarness.EmptyContext( + new DbContextOptionsBuilder().UseSqlite(MigrationHarness.SqliteConnection).Options); + var stock = context.GetService(); + + Assert.IsFalse( + stock.HasDifferences(source, target), + "EF's stock differ now sees complex-index declarations — revisit whether this package's " + + "HasDifferences override and runtime registration are still needed."); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/PostgresIntegrationTests.cs b/test/EFCore.ComplexIndexes.Tests/PostgresIntegrationTests.cs index d8e94b6..f7f376c 100644 --- a/test/EFCore.ComplexIndexes.Tests/PostgresIntegrationTests.cs +++ b/test/EFCore.ComplexIndexes.Tests/PostgresIntegrationTests.cs @@ -450,4 +450,54 @@ public void Native_and_complex_index_roundtrip_applies_cleanly() "SELECT count(*) FROM pg_indexes WHERE indexname = 'ix_ig_customers_email'", connection); Assert.AreEqual(1L, cmd.ExecuteScalar()); } + + // ── EnsureCreated: the runtime differ registration, end to end ── + + private class EnsureCreatedContext(DbContextOptions options) : DbContext(options) + { + public DbSet Grants => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("ig_ensure_grants"); + b.HasKey(x => x.Id); + b.Property(x => x.GranteeId).HasColumnName("grantee_id"); + b.Property(x => x.RoleId).HasColumnName("role_id"); + b.Property(x => x.Period).HasColumnName("period"); + b.Property(x => x.RevokedAt).HasColumnName("revoked_at"); + b.HasComplexCompositeIndex(x => new { x.GranteeId, x.RoleId }, indexName: "ix_ig_ensure_grantee_role"); + b.HasExclusionConstraint(x => x.GranteeId, x => x.Period, name: "ex_ig_ensure_grantee_period"); + }); + } + + /// + /// EnsureCreated() runs the context's runtime differ, so it never saw the + /// design-time registration; without UseNpgsqlComplexIndexes() it creates the table and + /// silently nothing else. A fresh database is used because EnsureCreated() is a no-op on a + /// database that already has tables — which the shared one does by the time this runs. + /// + [TestMethod(DisplayName = "EnsureCreated builds the complex index and exclusion constraint once the runtime differ is registered")] + public void EnsureCreated_includes_declarations_with_runtime_registration() + { + Sql("CREATE DATABASE ig_ensure_created"); + + var fresh = new NpgsqlConnectionStringBuilder(ConnectionString) { Database = "ig_ensure_created" }.ConnectionString; + var options = new DbContextOptionsBuilder() + .UseNpgsql(fresh) + .UseNpgsqlComplexIndexes() + .Options; + + using (var context = new EnsureCreatedContext(options)) + Assert.IsTrue(context.Database.EnsureCreated(), "EnsureCreated should have created the schema in the fresh database."); + + using var connection = new NpgsqlConnection(fresh); + connection.Open(); + + using (var indexes = new NpgsqlCommand("SELECT count(*) FROM pg_indexes WHERE tablename = 'ig_ensure_grants' AND indexname = 'ix_ig_ensure_grantee_role'", connection)) + Assert.AreEqual(1L, Convert.ToInt64(indexes.ExecuteScalar()), "The complex index was not created by EnsureCreated."); + + using (var constraints = new NpgsqlCommand("SELECT count(*) FROM pg_constraint WHERE conname = 'ex_ig_ensure_grantee_period' AND contype = 'x'", connection)) + Assert.AreEqual(1L, Convert.ToInt64(constraints.ExecuteScalar()), "The exclusion constraint was not created by EnsureCreated."); + } } diff --git a/test/EFCore.ComplexIndexes.Tests/PropertyBuilderOverloadTests.cs b/test/EFCore.ComplexIndexes.Tests/PropertyBuilderOverloadTests.cs new file mode 100644 index 0000000..9ffbd3b --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/PropertyBuilderOverloadTests.cs @@ -0,0 +1,65 @@ +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Metadata.Builders; +using Microsoft.EntityFrameworkCore.Migrations.Operations; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// EF hands back the non-generic ComplexTypePropertyBuilder for properties configured by name +/// or by type. The property-level API only existed on the generic builder, so those properties could +/// not carry a complex index at all — the call simply did not compile. +/// +[TestClass] +public class PropertyBuilderOverloadTests +{ + private class Name + { + public string First { get; set; } = ""; + public string Last { get; set; } = ""; + public string Nick { get; set; } = ""; + } + + private class Person + { + public Guid Id { get; set; } + public Name Name { get; set; } = new(); + } + + private class ByNameContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Name, c => + { + // Non-generic builders: by name, and by type and name. + c.Property("First").HasComplexIndex(isUnique: true, indexName: "ux_people_first"); + c.Property(typeof(string), "Last").HasComplexIndex(ix => ix.HasName("ix_people_last").HasFilter("\"Name_Last\" <> ''")); + + // The generic overload still returns the generic builder, so typed chaining keeps working. + ComplexTypePropertyBuilder typed = c.Property(x => x.Nick).HasComplexIndex(indexName: "ix_people_nick"); + typed.HasMaxLength(50); + }); + }); + } + + [TestMethod(DisplayName = "Properties configured by name or type carry complex indexes like typed ones")] + public void Non_generic_builders_produce_indexes() + { + var creates = MigrationHarness.CoreDiff(null, MigrationHarness.SqliteModel()) + .OfType() + .ToDictionary(o => o.Name); + + Assert.IsTrue(creates["ux_people_first"].IsUnique); + CollectionAssert.AreEqual(new[] { "Name_First" }, creates["ux_people_first"].Columns); + + Assert.AreEqual("\"Name_Last\" <> ''", creates["ix_people_last"].Filter); + CollectionAssert.AreEqual(new[] { "Name_Last" }, creates["ix_people_last"].Columns); + + CollectionAssert.AreEqual(new[] { "Name_Nick" }, creates["ix_people_nick"].Columns); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/RuntimeRegistrationTests.cs b/test/EFCore.ComplexIndexes.Tests/RuntimeRegistrationTests.cs new file mode 100644 index 0000000..9354f0b --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/RuntimeRegistrationTests.cs @@ -0,0 +1,185 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using EFCore.ComplexIndexes.SqlServer; +using Microsoft.Data.Sqlite; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.DependencyInjection; +using NpgsqlTypes; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// EnsureCreated() and GenerateCreateScript() build the schema through the +/// runtime IMigrationsModelDiffer, not the design-time one the .targets wire +/// up. Without a runtime registration they use EF's stock differ, which cannot see this package's +/// declarations: the tables are created and the indexes are simply absent — no error, nothing in a +/// log. These tests pin both halves: the registration makes the declarations appear, and its absence +/// is exactly the silent omission the feature exists to close. +/// +[TestClass] +public class RuntimeRegistrationTests +{ + private class EmailAddress + { + public string Value { get; set; } = ""; + } + + private class Person + { + public Guid Id { get; set; } + public string Name { get; set; } = ""; + public EmailAddress Email { get; set; } = new(); + public NpgsqlRange Period { get; set; } + } + + private class PersonContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.Ignore(x => x.Period); + b.ComplexProperty(x => x.Email, c => c.Property(x => x.Value).HasColumnName("email")); + b.HasComplexIndex(x => x.Email.Value, isUnique: true, indexName: "ux_people_email"); + }); + } + + // The PostgreSQL shape adds the two things only the Npgsql differ produces: an exclusion + // constraint (design-time DDL) and an expression index (rendered by the custom generator). + private class NpgsqlPersonContext(DbContextOptions options) : DbContext(options) + { + public DbSet People => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.ToTable("people"); + b.HasKey(x => x.Id); + b.ComplexProperty(x => x.Email, c => c.Property(x => x.Value).HasColumnName("email")); + b.HasComplexIndex(x => x.Email.Value, isUnique: true, indexName: "ux_people_email"); + b.HasExclusionConstraint(x => x.Name, x => x.Period, name: "ex_people_name_period"); + b.HasExpressionIndex("lower(email)", indexName: "ix_people_email_ci"); + }); + } + + // ── SQLite: EnsureCreated against a real (in-memory) database ── + + private static List SqliteIndexesAfterEnsureCreated(Func> configure) + { + using var connection = new SqliteConnection("DataSource=:memory:"); + connection.Open(); + + using (var context = new PersonContext(configure(connection))) + Assert.IsTrue(context.Database.EnsureCreated(), "EnsureCreated should have created the schema."); + + using var command = connection.CreateCommand(); + command.CommandText = "SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = 'people' AND sql IS NOT NULL"; + using var reader = command.ExecuteReader(); + + var names = new List(); + while (reader.Read()) + names.Add(reader.GetString(0)); + return names; + } + + [TestMethod(DisplayName = "EnsureCreated builds the complex index once the differ is registered at runtime")] + public void EnsureCreated_includes_complex_index_with_UseComplexIndexes() + { + var indexes = SqliteIndexesAfterEnsureCreated(connection => + new DbContextOptionsBuilder().UseSqlite(connection).UseComplexIndexes().Options); + + Assert.Contains("ux_people_email", indexes); + } + + [TestMethod(DisplayName = "AddComplexIndexes registers the differ on a custom internal service provider")] + public void EnsureCreated_includes_complex_index_with_AddComplexIndexes() + { + var provider = new ServiceCollection() + .AddEntityFrameworkSqlite() + .AddComplexIndexes() + .BuildServiceProvider(); + + var indexes = SqliteIndexesAfterEnsureCreated(connection => + new DbContextOptionsBuilder() + .UseSqlite(connection) + .UseInternalServiceProvider(provider) + .Options); + + Assert.Contains("ux_people_email", indexes); + } + + /// + /// The premise, kept as a test so that the day EF's own differ starts seeing these declarations + /// the registration can be retired knowingly rather than left as ceremony. + /// + [TestMethod(DisplayName = "Without the runtime registration, EnsureCreated silently omits the complex index")] + public void EnsureCreated_omits_complex_index_without_registration() + { + var indexes = SqliteIndexesAfterEnsureCreated(connection => + new DbContextOptionsBuilder().UseSqlite(connection).Options); + + Assert.DoesNotContain("ux_people_email", indexes); + } + + // ── PostgreSQL and SQL Server: GenerateCreateScript, the same code path without a server ── + + private static string NpgsqlCreateScript(bool wired) + { + var builder = new DbContextOptionsBuilder().UseNpgsql(MigrationHarness.NpgsqlConnection); + if (wired) + builder.UseNpgsqlComplexIndexes(); + + using var context = new NpgsqlPersonContext(builder.Options); + return context.Database.GenerateCreateScript(); + } + + [TestMethod(DisplayName = "UseNpgsqlComplexIndexes makes GenerateCreateScript include indexes, exclusion constraints and expression indexes")] + public void Npgsql_create_script_includes_declarations_when_wired() + { + var script = NpgsqlCreateScript(wired: true); + + // Npgsql leaves identifiers that need no quoting bare. + StringAssert.Contains(script, "CREATE UNIQUE INDEX ux_people_email ON people (email)"); + StringAssert.Contains(script, "EXCLUDE USING gist"); + StringAssert.Contains(script, "ex_people_name_period"); + // Rendered by the custom generator, which the same call registers — no sentinel column leaks. + StringAssert.Contains(script, "CREATE INDEX ix_people_email_ci ON people ((lower(email)))"); + Assert.DoesNotContain(CustomMigrationsModelDiffer.RuntimeWiringSentinel, script); + } + + [TestMethod(DisplayName = "Without UseNpgsqlComplexIndexes, GenerateCreateScript has none of the declarations")] + public void Npgsql_create_script_omits_declarations_when_not_wired() + { + var script = NpgsqlCreateScript(wired: false); + + StringAssert.Contains(script, "CREATE TABLE people"); + Assert.DoesNotContain("ux_people_email", script); + Assert.DoesNotContain("EXCLUDE", script); + Assert.DoesNotContain("ix_people_email_ci", script); + } + + private static string SqlServerCreateScript(bool wired) + { + var builder = new DbContextOptionsBuilder().UseSqlServer(MigrationHarness.SqlServerConnection); + if (wired) + builder.UseSqlServerComplexIndexes(); + + using var context = new PersonContext(builder.Options); + return context.Database.GenerateCreateScript(); + } + + [TestMethod(DisplayName = "UseSqlServerComplexIndexes makes GenerateCreateScript include the complex index")] + public void SqlServer_create_script_includes_index_when_wired() + => StringAssert.Contains(SqlServerCreateScript(wired: true), "CREATE UNIQUE INDEX [ux_people_email] ON [people] ([email])"); + + [TestMethod(DisplayName = "Without UseSqlServerComplexIndexes, GenerateCreateScript omits the complex index")] + public void SqlServer_create_script_omits_index_when_not_wired() + { + var script = SqlServerCreateScript(wired: false); + + StringAssert.Contains(script, "CREATE TABLE [people]"); + Assert.DoesNotContain("ux_people_email", script); + } +} diff --git a/test/EFCore.ComplexIndexes.Tests/UnmappedDeclarationTests.cs b/test/EFCore.ComplexIndexes.Tests/UnmappedDeclarationTests.cs new file mode 100644 index 0000000..31ada2b --- /dev/null +++ b/test/EFCore.ComplexIndexes.Tests/UnmappedDeclarationTests.cs @@ -0,0 +1,170 @@ +using EFCore.ComplexIndexes.PostgreSQL; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Migrations.Operations; +using NpgsqlTypes; + +namespace EFCore.ComplexIndexes.Tests; + +/// +/// Every descriptor scan skipped entity types with no table. The abstract base of a TPC hierarchy is +/// one, so an index or constraint declared on it produced nothing — no DDL, no error. Under EF Core +/// 10 the base's complex columns are not mapped onto the concrete tables either, so the declaration +/// cannot be satisfied anywhere; failing at migrations add says so. View- and query-mapped +/// types keep being skipped: an index on a view is nothing this package could create. +/// +[TestClass] +public class UnmappedDeclarationTests +{ + private class Email + { + public string Value { get; set; } = ""; + } + + private abstract class Animal + { + public int Id { get; set; } + public Email Contact { get; set; } = new(); + public NpgsqlRange Period { get; set; } + } + + private class Cat : Animal + { + public int Lives { get; set; } + } + + private class Dog : Animal + { + public bool Barks { get; set; } + } + + private class Person + { + public string Name { get; set; } = ""; + public Email Email { get; set; } = new(); + } + + private class TpcPropertyLevelContext(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(b => + { + b.UseTpcMappingStrategy(); + b.Ignore(x => x.Period); + b.ComplexProperty(x => x.Contact, c => c.Property(x => x.Value).HasComplexIndex(isUnique: true)); + }); + modelBuilder.Entity().ToTable("cats"); + modelBuilder.Entity().ToTable("dogs"); + } + } + + private class TpcEntityLevelContext(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(b => + { + b.UseTpcMappingStrategy(); + b.Ignore(x => x.Period); + b.ComplexProperty(x => x.Contact); + b.HasComplexIndex(x => x.Contact.Value); + }); + modelBuilder.Entity().ToTable("cats"); + modelBuilder.Entity().ToTable("dogs"); + } + } + + private class TpcExclusionContext(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(b => + { + b.UseTpcMappingStrategy(); + b.ComplexProperty(x => x.Contact); + b.HasExclusionConstraint(x => x.Id, x => x.Period); + }); + modelBuilder.Entity().ToTable("cats"); + modelBuilder.Entity().ToTable("dogs"); + } + } + + // TPT: the base has its own table, and that is where the columns live. + private class TptContext(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(b => + { + b.UseTptMappingStrategy(); + b.ToTable("animals"); + b.Ignore(x => x.Period); + b.ComplexProperty(x => x.Contact, c => c.Property(x => x.Value).HasComplexIndex(isUnique: true)); + }); + modelBuilder.Entity().ToTable("cats"); + modelBuilder.Entity().ToTable("dogs"); + } + } + + private class ViewContext(DbContextOptions options) : DbContext(options) + { + protected override void OnModelCreating(ModelBuilder modelBuilder) => + modelBuilder.Entity(b => + { + b.HasNoKey(); + b.ToView("v_people"); + b.ComplexProperty(x => x.Email, c => c.Property(x => x.Value).HasComplexIndex()); + }); + } + + [TestMethod(DisplayName = "A property-level index on a TPC base fails instead of producing nothing")] + public void Tpc_property_level_index_throws() + { + var target = MigrationHarness.SqliteModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.CoreDiff(null, target)); + + StringAssert.Contains(exception.Message, "Animal"); + StringAssert.Contains(exception.Message, "not mapped to a table"); + StringAssert.Contains(exception.Message, "TPC"); + } + + [TestMethod(DisplayName = "An entity-level index on a TPC base fails instead of producing nothing")] + public void Tpc_entity_level_index_throws() + { + var target = MigrationHarness.SqliteModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.CoreDiff(null, target)); + + StringAssert.Contains(exception.Message, "complex indexes"); + } + + [TestMethod(DisplayName = "An exclusion constraint on a TPC base fails instead of producing nothing")] + public void Tpc_exclusion_constraint_throws() + { + var target = MigrationHarness.NpgsqlModel(); + + var exception = Assert.ThrowsExactly(() => MigrationHarness.NpgsqlDiff(null, target)); + + StringAssert.Contains(exception.Message, "exclusion constraints"); + } + + [TestMethod(DisplayName = "Under TPT the base table carries the index")] + public void Tpt_base_table_gets_the_index() + { + var creates = MigrationHarness.CoreDiff(null, MigrationHarness.SqliteModel()) + .OfType() + .ToList(); + + Assert.HasCount(1, creates); + Assert.AreEqual("animals", creates[0].Table); + } + + [TestMethod(DisplayName = "A declaration on a view-mapped type is still skipped without error")] + public void View_mapped_type_is_skipped() + { + var operations = MigrationHarness.CoreDiff(null, MigrationHarness.SqliteModel()); + + Assert.IsFalse(operations.OfType().Any()); + } +} diff --git a/test/consumer-smoke-test.sh b/test/consumer-smoke-test.sh index 71fe17c..5889bb4 100755 --- a/test/consumer-smoke-test.sh +++ b/test/consumer-smoke-test.sh @@ -178,8 +178,11 @@ public sealed class ShopContext : DbContext { public DbSet Orders => Set(); + // The runtime wiring also registers the differ in the context's own service provider, which EF's + // design-time host copies before applying the .targets-injected registration. Present here so the + // scaffold below proves that the design-time registration still wins with it in place. protected override void OnConfiguring(DbContextOptionsBuilder options) - => options.UseNpgsql("Host=localhost;Database=smoke"); + => options.UseNpgsql("Host=localhost;Database=smoke").UseNpgsqlComplexIndexes(); protected override void OnModelCreating(ModelBuilder model) { @@ -247,8 +250,10 @@ public sealed class ShopContext : DbContext { public DbSet Orders => Set(); + // Runtime wiring present for the same reason as in the PostgreSQL consumer. protected override void OnConfiguring(DbContextOptionsBuilder options) - => options.UseSqlServer("Server=localhost;Database=smoke;Trusted_Connection=True;TrustServerCertificate=True"); + => options.UseSqlServer("Server=localhost;Database=smoke;Trusted_Connection=True;TrustServerCertificate=True") + .UseSqlServerComplexIndexes(); protected override void OnModelCreating(ModelBuilder model) {