Skip to content

DB layer review: outbox, EF Core concurrency/tenancy, and error-mapping fixes - #179

Merged
chullybun merged 7 commits into
mainfrom
db-fixes
Aug 5, 2026
Merged

DB layer review: outbox, EF Core concurrency/tenancy, and error-mapping fixes#179
chullybun merged 7 commits into
mainfrom
db-fixes

Conversation

@chullybun

@chullybun chullybun commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

End-to-end review and remediation across CoreEx's data-access layer: CoreEx.Database (base), CoreEx.Database.SqlServer, CoreEx.Database.Postgres, and CoreEx.EntityFrameworkCore.

  • Outbox relay: fixed lease/backoff duration clamping (was Math.Min, should be Math.Max), centralized duration-to-seconds logic in OutboxDuration, fixed PartitionSize being ignored, fixed a CancellationTokenSource leak in the relay loop, removed silent exception swallowing, and corrected outbox metrics being captured after publish.
  • CoreEx.Database base: fixed an inverted MultiSetCollArgs guard, a connection-disposal bug in Database.cs, a redundant row mapping in SelectFirst, and added a guard for indeterminate SQL statements.
  • CoreEx.EntityFrameworkCore:
    • Fixed a real optimistic-concurrency gap: an already-tracked entity being updated (e.g. re-fetched and mutated within the same DbContext) without a configured EF concurrency token could silently overwrite a concurrent write from another process, since the in-memory ETag never reflected the current DB state. Now falls back to a non-tracking-disturbing GetDatabaseValuesAsync() check.
    • Fixed WithTenantFilter reading the ambient ExecutionContext.Current instead of the ExecutionContext explicitly injected into EfDb<TDbContext> — multi-tenant background/fan-out scenarios using a non-ambient context were filtering by the wrong tenant on the query path.
    • Fixed tenant-mismatch and null-TenantId checks to use the injected context and to throw InvalidOperationException (internal data-integrity issue) rather than a validation error (which implies caller fault).
    • Removed a dead catch in delete logic, added null-guards in mapping extensions, corrected WithFilter's allowFilterBypass default to match its documented behaviour, and clarified several XML docs (ClearChangeTrackerAfterGet, WithFilter dual-context evaluation, IEfDbContext.BaseDatabase).
    • Rewrote CoreEx.EntityFrameworkCore/README.md to remove several inaccurate API descriptions.
  • SQL Server / Postgres providers: fixed PostgresDatabase to throw on a null dataSource, fixed SqlServerExtensions.ParamWith generic casting, added .ConfigureAwait(false) throughout outbox relay code, and added a JSON ValueComparer for correct EF Core change tracking on JSON-backed columns.
  • Test coverage: added regression tests for every fix above (SQL Server and Postgres, mirrored), plus new Contoso.Shopping.Test.Relay end-to-end outbox relay tests, error-mapping tests, and session-context tests. High-risk fixes (the EF Core concurrency gap and tenant-filter context bug) were verified with a stash/revert cycle — confirming the new regression test fails without the fix and passes with it restored.
  • Docs: corrected a .sql/.pgsql migration-naming ambiguity in coreex-tooling.instructions.md for Postgres-only readers.

Test plan

  • CoreEx.Database.SqlServer.Test.Unit — 82/82 passing against a live SQL Server instance
  • CoreEx.Database.Postgres.Test.Unit — 71/71 passing against a live Postgres instance
  • Full solution build (CoreEx.sln) — 0 warnings, 0 errors
  • High-risk fixes (EF Core concurrency gap, tenant-filter ambient-context bug) independently verified via stash-revert: confirmed each regression test fails without its fix and passes with it restored

🤖 Generated with Claude Code

@chullybun
Add JSON ValueComparer, improve schema naming & docs
b7e8ab2 
Introduced TypeToJsonStringEfComparer<T> for robust EF Core JSON column change tracking, paired with the existing converter in code generation and DbContext files. Enhanced template and migration logic to support PascalCase-to-snake_case schema naming for Postgres, ensuring correct identifiers for multi-word domains. Updated documentation, global usings, and test automation to reflect these changes. Refined Postgres outbox publisher/relay and improved comments for clarity.
@chullybun
Fix outbox duration logic, DB command guards, and tests
f67c2ce 
Centralize outbox duration-to-seconds logic in OutboxDuration,
ensuring correct lease/backoff calculation and minimums. Update all
relay usages to use new helpers, fixing previous clamping bug.
Improve exception handling and logging in DatabaseOutboxRelayBase.
Use configured PartitionSize in PartitionPicker. Add guard in
DatabaseCommand for indeterminate SqlStatement. Optimize
SelectSingleFirstInternalAsync and SelectMultiSetAsync for result
handling. Fix MultiSetCollArgs validation logic. Simplify
DatabaseWildcard replacement. Add missing global usings. Add and
update unit tests for duration, argument validation, command guards,
and wildcards. Minor cleanup and comments.
@chullybun
Add Shopping outbox relay E2E tests and DB regression fixes
17703eb 
Added Contoso.Shopping.Test.Relay for end-to-end relay host integration tests, mirroring Products relay coverage. Updated solution and filter files to include the new test project. Implemented tests for outbox event publishing, relay forwarding, Service Bus delivery, health, and hosted-service endpoints. Added test resources, configuration, and seed data.

Added regression and coverage tests for SQL Server and PostgreSQL error mapping, parameter extensions, and session context logic. Fixed PostgresDatabase to throw ArgumentNullException for null dataSource and refactored connection creation. Updated SqlServerExtensions.ParamWith to use independent generic types, preventing invalid casts. Ensured .ConfigureAwait(false) on all outbox relay awaits.

Updated documentation and migration scripts, including spSetSessionContext. Clarified relay test coverage split and parity in copilot-instructions.md and README.md.
@chullybun
Clarify migration script naming for DB providers
cf1d12b 
Detailed migration script naming conventions now specify the correct file extension for each database provider: `.sql` for SQL Server and `.pgsql` for PostgreSQL. Examples for both are included, and guidance notes that the extension must match the provider. This ensures clear instructions for naming migration files across supported databases.
@chullybun
Improve EF Core behavior, docs, and validation
2f20cba 
- Clarified XML docs for `EfDbArgs.ClearChangeTrackerAfterGet` (detaches all entities, warns on shared context).
- Added null checks for `mapper` in mapping extensions; throw `ArgumentNullException` if null; updated tests.
- Refactored delete logic in `EfDbModel.Delete.cs` to use `switch`, removed `NotFoundException` swallowing.
- Improved tenant validation in `EfDbModel.cs`: throw `InvalidOperationException` for null/empty `TenantId`, use injected `ExecutionContext`.
- Changed `allowFilterBypass` default to `false` in `EfDbModelOptions<TModel>.WithFilter`; updated tests.
- Added remark to `IEfDbContext.BaseDatabase` about returning the same instance.
- Updated `README.md` for accuracy on logging, filtering, and API descriptions.
- Added new `EntityFrameworkBehaviorTests` for SQL Server and PostgreSQL covering tenant checks, null validation, filter bypass, upsert, tracking, and extension hooks.
@chullybun
Tenant filtering and concurrency improvements in EfDb
2665722 
- Tenant filters now use injected ExecutionContext from EfDb, not ambient context, ensuring correct multi-tenancy.
- Refactored WithTenantFilter to store config flags and apply filters using the provided context.
- EfDbModel<TModel>.Query passes EfDb's ExecutionContext to ApplyFilters.
- Added SQL Server and Postgres tests verifying tenant filtering uses the injected context.
- Improved concurrency for attached entities lacking EF concurrency token: if ETag is present but not configured, now queries DB for current ETag and returns concurrency error on mismatch.
- Added tests for new concurrency check with custom DbContext types lacking ETag concurrency token.
Copilot AI review requested due to automatic review settings August 5, 2026 17:33
Copilot AI reviewed Aug 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR delivers a broad set of fixes and regression coverage across CoreEx’s database and EF Core integration layers, focused on outbox relay correctness, EF Core concurrency/tenancy behavior, and provider-specific error/parameter handling.

Changes:

  • Fixed multiple outbox relay issues (duration clamping, partition sizing, CTS disposal, transient logging, and metrics timing) and added duration utilities.
  • Hardened CoreEx.Database and CoreEx.EntityFrameworkCore behavior (multi-set bounds, indeterminate statements guard, EF tracked-entity concurrency fallback, tenant filter context correctness, mapper null-guards).
  • Expanded regression tests across CoreEx.Database (base + SqlServer + Postgres) and added a new Shopping relay E2E test project; updated templates/docs accordingly.

Reviewed changes

Copilot reviewed 70 out of 71 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/validate-template-pack.ps1 Adds multiword-name template validation scenarios and glob-content assertions.
tests/CoreEx.Database.Test.Unit/OutboxDurationTests.cs Unit tests for outbox duration-to-seconds logic and buffer behavior.
tests/CoreEx.Database.Test.Unit/MultiSetCollArgsTests.cs Regression tests for MultiSetCollArgs bounds validation.
tests/CoreEx.Database.Test.Unit/DatabaseWildcardTests.cs Tests for database wildcard translation/escaping behavior.
tests/CoreEx.Database.Test.Unit/DatabaseCommandTests.cs Regression test for indeterminate statement guard.
tests/CoreEx.Database.SqlServer.Test.Unit/SqlServerSessionContextTests.cs Verifies SQL Server session-context stored proc parameter behavior.
tests/CoreEx.Database.SqlServer.Test.Unit/SqlServerExtensionsParametersTests.cs Covers SqlServer parameter extension behavior, including ParamWith cast fix.
tests/CoreEx.Database.SqlServer.Test.Unit/EntityFrameworkBehaviorTests.cs SQL Server EF behavior regression coverage (tenancy, filters, mapping, concurrency gap).
tests/CoreEx.Database.SqlServer.Test.Unit/DatabaseRecordTests.cs SQL Server DatabaseRecord behavior tests (ordinal/value/json/rowversion).
tests/CoreEx.Database.SqlServer.Test.Unit/DatabaseErrorMappingTests.cs SQL Server exception/error-number mapping regression tests.
tests/CoreEx.Database.SqlServer.Test.Console/Program.cs Console migration harness minor formatting adjustment.
tests/CoreEx.Database.SqlServer.Test.Console/Migrations/004-create-sp-set-session-context.sql Adds stored proc for session-context testing.
tests/CoreEx.Database.Postgres.Test.Unit/PostgresDatabaseTests.cs Postgres constructor null-guard and SQLSTATE mapping tests.
tests/CoreEx.Database.Postgres.Test.Unit/EntityFrameworkBehaviorTests.cs Postgres EF behavior regression coverage matching SQL Server suite.
src/CoreEx.Template/content/CoreEx.Core/tools/app-name.Database/Program.cs Template: uses pg-schema token for Postgres data reset filter.
src/CoreEx.Template/content/CoreEx.Core/tools/app-name.Database/Migrations/date-now-000002-create-db-name-outbox-tables.pgsql Template: Postgres outbox DDL updated to use pg-schema token.
src/CoreEx.Template/content/CoreEx.Core/tools/app-name.Database/Migrations/date-now-000001-create-db-name-schema.pgsql Template: schema creation uses pg-schema token.
src/CoreEx.Template/content/CoreEx.Core/tools/app-name.Database/dbex.yaml Template: Postgres schema token switched to pg-schema.
src/CoreEx.Template/content/CoreEx.Core/tools/app-name.Database/Data/ref-data.seed.yaml Template: Postgres seed schema key switched to pg-schema.
src/CoreEx.Template/content/CoreEx.Core/.template.config/template.json Template: new transforms for kebab/snake casing; adds pg-schema parameter.
src/CoreEx.EntityFrameworkCore/README.md Corrects/clarifies EF integration behavior and responsibilities.
src/CoreEx.EntityFrameworkCore/IEfDbContext.cs Documents BaseDatabase instance stability requirement (event subscription safety).
src/CoreEx.EntityFrameworkCore/GlobalUsing.cs Adds CoreEx.Json global using (needs re-sort per conventions).
src/CoreEx.EntityFrameworkCore/EfDbModelOptions.cs Fixes tenant-filter execution-context source; adjusts filter-bypass default and docs.
src/CoreEx.EntityFrameworkCore/EfDbModel.Update.cs Adds tracked-entity concurrency fallback via GetDatabaseValuesAsync.
src/CoreEx.EntityFrameworkCore/EfDbModel.Query.cs Ensures filters are applied using injected execution context.
src/CoreEx.EntityFrameworkCore/EfDbModel.Delete.cs Removes dead catch and simplifies delete flow.
src/CoreEx.EntityFrameworkCore/EfDbModel.cs Uses injected execution context for tenant checks; changes null/tenant mismatch handling.
src/CoreEx.EntityFrameworkCore/EfDbExtensions.cs Adds mapper null-guards to mapping extension overloads.
src/CoreEx.EntityFrameworkCore/EfDbArgs.cs Clarifies ClearChangeTrackerAfterGet behavior and risks.
src/CoreEx.EntityFrameworkCore/Converters/TypeToJsonStringEfComparer.cs Adds JSON-based ValueComparer for JSON-string converted properties.
src/CoreEx.EntityFrameworkCore/Converters/README.md Documents arbitrary-type JSON conversion + comparer pairing guidance.
src/CoreEx.Database/Templates/EfModelBuilder_cs.hbs Codegen: uses JSON comparer alongside JSON converter for non-string JSON columns.
src/CoreEx.Database/Outbox/OutboxDuration.cs Centralizes duration-to-seconds and buffered-lease conversion logic.
src/CoreEx.Database/Outbox/DatabaseOutboxRelayHostedServiceBase.cs Honors configured PartitionSize when constructing PartitionPicker.
src/CoreEx.Database/Outbox/DatabaseOutboxRelayBase.cs Fixes CTS disposal, duration conversion, transient logging, cancel error handling.
src/CoreEx.Database/GlobalUsing.cs Adds converter namespace as global using for database layer.
src/CoreEx.Database/Extended/MultiSetCollArgs.cs Fixes inverted min/max guard and error message.
src/CoreEx.Database/Extended/DatabaseWildcard.cs Simplifies wildcard parsing to use configured Wildcard instance.
src/CoreEx.Database/DatabaseRecord.cs Removes per-file using; simplifies rowversion conversion return.
src/CoreEx.Database/DatabaseParameterCollection.cs Removes per-file using (moved to global).
src/CoreEx.Database/DatabaseCommand.SelectMultiSet.cs Uses NextResultAsync and tightens “missing result sets” guard.
src/CoreEx.Database/DatabaseCommand.SelectFirstSingle.cs Avoids redundant mapping when only “first” semantics needed.
src/CoreEx.Database/DatabaseCommand.cs Adds guard to prevent executing indeterminate SqlStatement.
src/CoreEx.Database/Database.cs Clarifies disposal semantics (connection owned externally).
src/CoreEx.Database/Abstractions/DatabaseInvoker.cs Captures outbox metrics count before publish.
src/CoreEx.Database.SqlServer/SqlServerExtensions.Parameters.cs Fixes ParamWith generic casting by splitting check/value generics.
src/CoreEx.Database.SqlServer/SqlServerDatabase.SessionContext.cs Fixes unused variable and ensures awaited execution with ConfigureAwait(false).
src/CoreEx.Database.SqlServer/Outbox/SqlServerOutboxRelay.cs Adds ConfigureAwait(false) and clarifies schema defaulting docs.
src/CoreEx.Database.SqlServer/Outbox/SqlServerOutboxPublisher.cs Adds SetStatementByConvention API and invokes it from constructor.
src/CoreEx.Database.Postgres/README.md Updates telemetry/DI API naming in documentation.
src/CoreEx.Database.Postgres/PostgresDatabase.cs Adds null-guard by deferring CreateConnection call.
src/CoreEx.Database.Postgres/Outbox/PostgresOutboxRelay.cs Corrects schema defaulting + ConfigureAwait(false).
src/CoreEx.Database.Postgres/Outbox/PostgresOutboxPublisher.cs Adds SetStatementByConvention API and schema handling cleanup.
samples/tests/Contoso.Shopping.Test.Relay/Resources/BasketUpdatedCloudEvent.json Adds relay test CloudEvent resource.
samples/tests/Contoso.Shopping.Test.Relay/Resources/BasketCreatedCloudEvent.json Adds relay test CloudEvent resource.
samples/tests/Contoso.Shopping.Test.Relay/RelayTests.cs New Shopping relay E2E outbox-to-ServiceBus regression test.
samples/tests/Contoso.Shopping.Test.Relay/OtherTests.HostedServices.cs Adds hosted-service pause/resume endpoint tests for relay host.
samples/tests/Contoso.Shopping.Test.Relay/OtherTests.Health.cs Adds relay health endpoint tests (minor local naming fix needed).
samples/tests/Contoso.Shopping.Test.Relay/OtherTests.cs Shared relay test setup.
samples/tests/Contoso.Shopping.Test.Relay/GlobalUsing.cs Adds global usings for new relay test project.
samples/tests/Contoso.Shopping.Test.Relay/Contoso.Shopping.Test.Relay.csproj Adds new Shopping relay test project configuration and references.
samples/tests/Contoso.Shopping.Test.Relay/appsettings.unittest.json Relay test host configuration for faster polling and SB retries.
samples/src/Contoso.Shopping.Infrastructure/Repositories/ShoppingDbContext.g.cs Regenerated: JSON properties now use converter + comparer.
samples/src/Contoso.Products.Infrastructure/Repositories/ProductsDbContext.g.cs Regenerated: JSON properties now use converter + comparer.
CoreEx.slnx Adds Shopping relay test project to solution filter tree.
CoreEx.sln Adds Shopping relay test project (currently has an invalid leading blank line).
CoreEx.Samples.Test.slnf Includes Shopping relay test project in samples test filter.
CoreEx.Samples.Build.slnf Includes Shopping relay test project in samples build filter.
.github/instructions/coreex-tooling.instructions.md Clarifies migration naming guidance for .sql vs .pgsql.
.github/copilot-instructions.md Updates repo guidance re: relay test coverage split and parity expectations.

Comment thread src/CoreEx.EntityFrameworkCore/GlobalUsing.cs
Comment thread CoreEx.sln Outdated
Comment thread samples/tests/Contoso.Shopping.Test.Relay/OtherTests.Health.cs Outdated
@chullybun @claude
Address Copilot review nits on PR #179
61a5096 
Re-sort CoreEx.Json into ordinal position in EfCore GlobalUsing.cs,
remove a stray leading blank line before the .sln header, and rename
a local variable that used the private-field underscore prefix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 5, 2026 17:52
Copilot AI reviewed Aug 5, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 70 out of 71 changed files in this pull request and generated no new comments.

@chullybun chullybun added this to the v4.0.0-preview-4 milestone Aug 5, 2026
@chullybun
chullybun merged commit 985e91d into main Aug 5, 2026
4 checks passed
@chullybun
chullybun deleted the db-fixes branch August 5, 2026 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v4.0.0-preview-4

Development

Successfully merging this pull request may close these issues.

2 participants

@chullybun