[PM-35184] Use EF inserts instead of linq2db bulk copy on SQLite - #8264
[PM-35184] Use EF inserts instead of linq2db bulk copy on SQLite#8264AlexRubik wants to merge 2 commits into
Conversation
EventRepository.CreateManyAsync wrote event batches through linq2db's BulkCopyAsync, which resolves a SQLite provider adapter before emitting any SQL. On self-hosted SQLite deployments that resolution can select the Classic System.Data.SQLite adapter, an assembly this repository does not reference at any version, and the resulting InvalidOperationException propagates out of BaseRequestValidator.BuildSuccessResultAsync and fails the login. Batches on this path are one event for the user plus one per events-enabled organization or provider, so bulk copy buys nothing here. Branch on Database.IsSqlite() and use AddRangeAsync with a single SaveChangesAsync, matching the provider-branching precedent in CipherRepository and CollectionRepository. MySQL and Postgres keep the bulk copy path. [PM-35184]
CreateManyAsync had no test asserting it persists what it was given. The only coverage was incidental, through EventRepositoryDeleteManyTests, where the writes are setup for a delete assertion. Add multi-provider coverage for the batch path, the single-event short circuit, and the empty guard, so the SQLite branch and the bulk copy branch are both exercised on every configured provider. [PM-35184]
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Reviewed the SQLite provider branch added to Code Review DetailsNo findings met the bar for posting. Notes on what was checked and cleared:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8264 +/- ##
==========================================
+ Coverage 17.61% 17.65% +0.03%
==========================================
Files 1476 1476
Lines 63928 63933 +5
Branches 5116 5118 +2
==========================================
+ Hits 11262 11285 +23
+ Misses 52468 52450 -18
Partials 198 198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
🎟️ Tracking
PM-35184
📔 Objective
EventRepository.CreateManyAsyncwrote event batches through linq2db'sBulkCopyAsync, which resolves a SQLite provider adapter before emitting any SQL. A self-hosted SQLite reporter saw that resolution select the ClassicSystem.Data.SQLiteadapter, an assembly this repository does not reference at any version, and the resultingInvalidOperationExceptionpropagated out ofBaseRequestValidator.BuildSuccessResultAsyncand failed their login.This branches on
Database.IsSqlite()and usesAddRangeAsyncwith a singleSaveChangesAsyncinstead, matching the provider-branching precedent inCipherRepositoryandCollectionRepository. Batches on this path are one event for the user plus one per events-enabled organization or provider, so bulk copy was buying nothing here. MySQL and Postgres keep the bulk copy path, and SQL Server self-host uses the Dapper implementation and is unaffected.Also adds
EventRepositoryCreateManyTests, sinceCreateManyAsyncpreviously had no test asserting it persists what it was given. Its only coverage was incidental, throughEventRepositoryDeleteManyTests, where the writes are setup for a delete assertion.Worth flagging for the reviewer
The reporter's exception does not reproduce on
main. The existing SQLite integration tests exercise this path and pass, so the adapter resolves correctly from source, andSystem.Data.SQLiteappears in no.csprojorpackages.lock.jsonin this repo. The failure appears specific to how thelite:2026.3.2image is published rather than to this code.So this is hardening with a test, not a confirmed-reproduction fix: it removes the dependency the reporter's stack trace blames from the authentication path. The image packaging question is being followed up on the ticket with the reporter.
Two smaller notes:
CipherRepositoryhas six unguardedBulkCopyAsynccalls and was suspected of breaking self-hosted import the same way. It does not: a throwaway probe exercisingCreateAsync(userId, ciphers, folders)passed on SQLite, despite it passingKeepIdentity = true, which asks more of the provider adapter than the call changed here. Deliberately left alone.CreateManyAsync_NoEvents_DoesNotThrowis named for what it actually pins. ItsAssert.Emptyis near-vacuous, since the organization id is freshly generated; the real assertion is that an empty batch does not throw onentities.First().Verification
dotnet build bitwarden-server.slnx: 0 errors.Infrastructure.IntegrationTestfiltered toEventRepositoryagainst a migrated SQLite database: 6 passed, 0 failed.BulkCopyAsyncline temporarily replaced by athrow, the SQLite tests still passed.📸 Screenshots
Not applicable, no UI changes.