Configure bit Besql dbContext so it can resolve scoped services (#11775) #11776
Conversation
WalkthroughThe PR refactors the Besql DbContext factory implementation from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/Besql/Bit.Besql/wwwroot/bit-besql.js (1)
20-23: Remove invalidstatusproperty from Blob options.The
status: 200property is not a valid option for theBlobconstructor. The Blob constructor only acceptstypeandendingsoptions. This property is silently ignored but represents dead code.const blob = new Blob([data], { - type: 'application/octet-stream', - status: 200 + type: 'application/octet-stream' });src/Besql/Bit.Besql/DbContextFactoryBase.cs (1)
17-29: Race condition in initialization check.The check
if (dbContextInitializerTcs is null)followed byawait StartRunningDbContextInitializer()is not thread-safe. Multiple concurrent calls toCreateDbContextAsynccould both seenulland attempt to start initialization.Consider using
Interlocked.CompareExchangeor a lock to ensure single initialization:+private readonly object _initLock = new(); private TaskCompletionSource? dbContextInitializerTcs; [RequiresUnreferencedCode("Calls StartRunningDbContextInitializer()")] public virtual async Task<TDbContext> CreateDbContextAsync(CancellationToken cancellationToken = default) { - if (dbContextInitializerTcs is null) - { - await StartRunningDbContextInitializer(); - } + if (dbContextInitializerTcs is null) + { + lock (_initLock) + { + if (dbContextInitializerTcs is null) + { + _ = StartRunningDbContextInitializerLocked(); + } + } + } await dbContextInitializerTcs!.Task.ConfigureAwait(false);Alternatively, use
Lazy<Task>for thread-safe lazy initialization.
♻️ Duplicate comments (1)
src/Besql/Bit.Besql/DbContextFactoryBase.cs (1)
36-53: Race condition inStartRunningDbContextInitializer.This method has the same race condition issue - multiple threads could pass the
if (dbContextInitializerTcs is not null)check before any of them assigns theTaskCompletionSourceat line 42. This would result in multiple initialization attempts.This is covered by the fix suggested above for
CreateDbContextAsync.
🧹 Nitpick comments (3)
src/Besql/Bit.Besql/wwwroot/bit-besql.js (2)
4-4: Consider aligning internal function name with property name.The named function expression
besqlPersistdiffers from its assigned property namepersist. While named function expressions help with debugging, consistent naming improves maintainability.-BitBesql.persist = async function besqlPersist(fileName) { +BitBesql.persist = async function persist(fileName) {
36-36: Consider aligning internal function name with property name.Same naming inconsistency as
persistabove.-BitBesql.load = async function besqlLoad(fileName) { +BitBesql.load = async function load(fileName) {src/Besql/Bit.Besql/BesqlDbContextFactory.cs (1)
43-48: Empty catch block silently swallows exceptions.The empty catch block hides potential issues with
LazyAssemblyLoader. Consider logging the exception or at least adding a comment explaining why all exceptions are intentionally ignored.try { await using var scope = _serviceProvider.CreateAsyncScope(); await scope.ServiceProvider.GetRequiredService<LazyAssemblyLoader>().LoadAssembliesAsync(["System.Private.Xml.wasm"]); } - catch { } + catch (Exception) + { + // LazyAssemblyLoader may not be available in all hosting scenarios (e.g., server-side). + // Failures are non-fatal as the assembly may already be loaded or not required. + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (13)
src/Besql/Bit.Besql/BesqlDbContextFactory.cs(1 hunks)src/Besql/Bit.Besql/BesqlDbContextInterceptor.cs(1 hunks)src/Besql/Bit.Besql/Bit.Besql.csproj(1 hunks)src/Besql/Bit.Besql/DbContextFactoryBase.cs(3 hunks)src/Besql/Bit.Besql/IServiceCollectionBesqlExtentions.cs(2 hunks)src/Besql/Bit.Besql/wwwroot/bit-besql.js(2 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Data/CompiledModel/OfflineDbContextModelBuilder.cs(2 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20240105010046_InitialMigration.Designer.cs(0 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.Designer.cs(1 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.cs(1 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/OfflineDbContextModelSnapshot.cs(1 hunks)src/Besql/Demo/Bit.Besql.Demo.Client/Extensions/ServiceCollectionExtensions.cs(1 hunks)src/Besql/Demo/Bit.Besql.Demo/Bit.Besql.Demo.csproj(0 hunks)
💤 Files with no reviewable changes (2)
- src/Besql/Demo/Bit.Besql.Demo/Bit.Besql.Demo.csproj
- src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20240105010046_InitialMigration.Designer.cs
🧰 Additional context used
🧬 Code graph analysis (6)
src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.Designer.cs (3)
src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/OfflineDbContextModelSnapshot.cs (1)
DbContext(11-66)src/Besql/Demo/Bit.Besql.Demo.Client/Data/OfflineDbContext.cs (1)
OfflineDbContext(7-25)src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.cs (1)
Initial(10-52)
src/Besql/Demo/Bit.Besql.Demo.Client/Extensions/ServiceCollectionExtensions.cs (1)
src/Besql/Demo/Bit.Besql.Demo.Client/Data/CompiledModel/OfflineDbContextModelBuilder.cs (2)
OfflineDbContextModel(11-26)OfflineDbContextModel(13-16)
src/Besql/Bit.Besql/BesqlDbContextFactory.cs (1)
src/Besql/Bit.Besql/DbContextFactoryBase.cs (2)
TDbContext(31-34)DbContextFactoryBase(8-64)
src/Besql/Bit.Besql/IServiceCollectionBesqlExtentions.cs (4)
src/Besql/Bit.Besql/BesqlDbContextFactory.cs (3)
TDbContext(58-61)BesqlDbContextFactory(11-62)BesqlDbContextFactory(19-38)src/Besql/Bit.Besql/DbContextFactoryBase.cs (2)
TDbContext(31-34)DbContextFactoryBase(8-64)src/Besql/Bit.Besql/BitBesqlNoopStoage.cs (4)
Task(8-11)Task(13-16)Task(22-25)BitBesqlNoopStoage(6-26)src/Besql/Bit.Besql/IBitBesqlStorage.cs (3)
Task(11-11)Task(18-18)Task(35-35)
src/Besql/Bit.Besql/DbContextFactoryBase.cs (1)
src/Besql/Bit.Besql/BesqlDbContextFactory.cs (2)
TDbContext(58-61)RequiresUnreferencedCode(40-56)
src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/OfflineDbContextModelSnapshot.cs (2)
src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.Designer.cs (1)
DbContext(12-69)src/Besql/Demo/Bit.Besql.Demo.Client/Data/OfflineDbContext.cs (1)
OfflineDbContext(7-25)
🪛 GitHub Check: build Bit.Besql
src/Besql/Bit.Besql/DbContextFactoryBase.cs
[warning] 17-17:
Member 'Bit.Besql.DbContextFactoryBase.CreateDbContextAsync(CancellationToken)' with 'RequiresUnreferencedCodeAttribute' implements interface member 'Microsoft.EntityFrameworkCore.IDbContextFactory.CreateDbContextAsync(CancellationToken)' without 'RequiresUnreferencedCodeAttribute'. 'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides.
[warning] 17-17:
Member 'Bit.Besql.DbContextFactoryBase.CreateDbContextAsync(CancellationToken)' with 'RequiresUnreferencedCodeAttribute' implements interface member 'Microsoft.EntityFrameworkCore.IDbContextFactory.CreateDbContextAsync(CancellationToken)' without 'RequiresUnreferencedCodeAttribute'. 'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides.
[warning] 17-17:
Member 'Bit.Besql.DbContextFactoryBase.CreateDbContextAsync(CancellationToken)' with 'RequiresUnreferencedCodeAttribute' implements interface member 'Microsoft.EntityFrameworkCore.IDbContextFactory.CreateDbContextAsync(CancellationToken)' without 'RequiresUnreferencedCodeAttribute'. 'RequiresUnreferencedCodeAttribute' annotations must match across all interface implementations or overrides.
🔇 Additional comments (14)
src/Besql/Bit.Besql/BesqlDbContextInterceptor.cs (1)
10-10: Improved SQL keyword detection with trailing spaces.Good change. Adding trailing spaces to the keywords prevents false positives (e.g., matching "UPDATING" as "UPDATE"). The patterns correctly match standard SQL statement prefixes.
src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/OfflineDbContextModelSnapshot.cs (1)
1-66: Auto-generated EF Core model snapshot.This is auto-generated code from EF Core migrations tooling. The ProductVersion update to 10.0.0 is consistent with the broader EF Core version alignment in this PR.
src/Besql/Demo/Bit.Besql.Demo.Client/Data/CompiledModel/OfflineDbContextModelBuilder.cs (1)
1-26: Auto-generated EF Core compiled model.This is auto-generated code from EF Core compiled model tooling. The updated
modelIdand ProductVersion 10.0.0 are expected artifacts of model regeneration.src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.cs (1)
10-10: Migration class renamed to follow EF Core conventions.The rename from
InitialMigrationtoInitialaligns with the migration identifier in the designer file ([Migration("20251130123559_Initial")]). The migration logic for creating theWeatherForecaststable and seeding data is correct.src/Besql/Bit.Besql/Bit.Besql.csproj (1)
8-8: Document why EF1001 suppression is necessary.Suppressing
EF1001indicates theDbContextFactoryBaseimplementation relies on internal Entity Framework Core APIs that may change or be removed in future versions without notice. Add comments to the implementation explaining which internal APIs are used and why public alternatives aren't viable, so maintainers understand the compatibility risk and can address it during EF Core upgrades.src/Besql/Demo/Bit.Besql.Demo.Client/Extensions/ServiceCollectionExtensions.cs (1)
10-15: LGTM!The fully-qualified path for
OfflineDbContextModel.Instanceis explicit and avoids potential namespace conflicts. The DbContext configuration correctly wires up the compiled model and migration initializer.src/Besql/Demo/Bit.Besql.Demo.Client/Data/Migrations/20251130123559_Initial.Designer.cs (1)
1-69: LGTM!This is a standard auto-generated EF Core migration designer file. The model configuration and seed data are consistent with the corresponding migration and snapshot files.
src/Besql/Bit.Besql/IServiceCollectionBesqlExtentions.cs (3)
17-20: LGTM: Lifetime parameter enables scoped service resolution.The new
ServiceLifetimeparameter with aSingletondefault maintains backward compatibility while enabling the PR's objective of supporting scoped services when needed.
53-59: Non-browser path correctly updated.The non-browser path now uses
DbContextFactoryBase<TDbContext>instead of a pooled factory, enabling scoped service resolution consistently across both browser and non-browser scenarios.
41-48: Verify the factory type is correctly resolved by DI.The change from
BesqlPooledDbContextFactory<TDbContext>toBesqlDbContextFactory<TDbContext>needs confirmation thatDbContextFactoryBase<TDbContext>properly declaresIDbContextFactorySource<TDbContext>as a constructor dependency and that EF Core'sAddDbContextFactoryprovides this service during resolution.src/Besql/Bit.Besql/BesqlDbContextFactory.cs (2)
19-24: Constructor signature change aligns with the new factory pattern.The reordered parameters and addition of
IDbContextFactorySource<TDbContext>enable the factory to create DbContext instances through the DI-aware factory source rather than a pool, supporting the PR's goal of resolving scoped services.
58-61: Intentional restriction on synchronous CreateDbContext.The
NotSupportedExceptionenforces async-first usage, which is appropriate given the async initialization requirements (storage loading, assembly loading). The error message clearly guides users to useCreateDbContextAsync.src/Besql/Bit.Besql/DbContextFactoryBase.cs (2)
55-63: LGTM!The initialization pattern correctly creates a temporary context, runs the initializer delegate, and disposes it. Using
dbContext.GetService<IServiceProvider>()appropriately provides access to the scoped service provider.
3-3: Using internal EF Core API without version constraint.
IDbContextFactorySource<TDbContext>fromMicrosoft.EntityFrameworkCore.Internalis not part of the public API surface and may change between EF Core versions. Verify this type is available in your target EF Core version and document the minimum required version in your project file or configuration comments.
1 participant
closes #11775
Summary by CodeRabbit
New Features
Bug Fixes
Chores
besqlPersist,besqlLoad).✏️ Tip: You can customize this high-level summary in your review settings.