Share a single MongoClient and fix LiteDB TableCollection - #757
Merged
Conversation
The database was registered as AddScoped(_ => new MongoClient(clientSettings).GetDatabase(databaseName)), so a MongoClient was constructed for every DI scope, that is every request. The driver deduplicates the underlying cluster through ClusterRegistry, so this was not a connection pool per request, but the client is documented as a single, thread-safe instance that owns the pool and cluster monitoring. Building one per request allocates and freezes settings on a hot path for no benefit. IMongoClient is now a singleton and the scoped IMongoDatabase is taken from it, so consumers are unchanged. The parameterless constructors in MongoRepository and MongoStoreFilesContext still build their own client; DI only selects them before installation, when IMongoDatabase is not registered yet, so they are left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
nameof(T) on a generic type parameter is the literal string "T", so TableCollection<C>() queried a collection named "T" rather than the one named after the entity, and always came back empty. The Mongo implementation already uses typeof(T).Name. The method backs the generic API query handler, so under LiteDB every $-query returned nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Type: bugfix
Issue
No issues were open for these; both came out of an audit of the data layer. Two independent changes,
one commit each. They are not of equal weight, so to be straight about it:
1.
LiteDBRepository.TableCollection<C>()queried a collection named"T"— a real bug.nameof(T)on a generic type parameter evaluates to the literal string"T", not the entity name:MongoRepository.TableCollection<C>()already usedtypeof(T).Name, so the two providers disagreed.Reach is narrow and worth stating plainly:
TableCollection<C>()has exactly one productionconsumer,
GetGenericQueryHandlerinGrand.Module.Api. Triggering the bug needs the LiteDBprovider and the API module enabled, and both are opt-in. In that combination every generic API
query returned an empty result set.
2. A
MongoClientwas constructed per DI scope — hygiene, not a defect.src/Core/Grand.Infrastructure/Startup/StartupApplication.csregistered the database asAddScoped(_ => new MongoClient(clientSettings).GetDatabase(databaseName)), so a client was builtper scope, that is per request.
An earlier draft of this description called that the most impactful defect in the data layer. That
was wrong, and the corrected version is less dramatic: the driver deduplicates the underlying
cluster through
ClusterRegistry, so this was not a connection pool per request, andMongoClientSettings.FromConnectionStringalready ran once at startup. What actually happened onevery request was a settings clone and freeze, a
ClusterKeycomputation, a lock-protected registrylookup, and a client wrapper allocation.
That is small. It is still contrary to the driver's documented usage —
MongoClientis meant to be asingle, thread-safe instance owning the connection pool and cluster monitoring — and removing it
costs four lines and changes no behaviour.
Solution
typeof(T).NameinLiteDBRepository.TableCollection<C>(), matching the Mongoimplementation, with a regression test in
Grand.Data.Tests.IMongoClientas a singleton and resolve the scopedIMongoDatabasefrom it. Consumersare untouched — they still inject
IMongoDatabase, still scoped.On lifetime, since it is the one thing that could bite: previously the scoped clients were created
by a factory, so the container disposed one per scope. If
MongoClient.Disposetore down the sharedregistry cluster, the application would fail continuously — it does not. The singleton is disposed
at shutdown, which is correct. Nothing else in the solution resolves
IMongoClient.Note for reviewers: the parameterless constructors of
MongoRepositoryandMongoStoreFilesContextstill build their own
MongoClient. DI only selects those before installation, whenIMongoDatabaseis not registered yet — afterwards it picks theIMongoDatabaseoverloads. Theyare deliberately left alone rather than widened into this change.
Breaking changes
None.
IMongoClientwas not registered before, so the new registration cannot collide, and nopublic signature changed. The scoped
IMongoDatabaseregistration is preserved, so every existingconsumer resolves exactly as it did.
Testing
dotnet build ./GrandNode.sln— succeeds.dotnet test ./src/Tests/Grand.Data.Tests/Grand.Data.Tests.csproj— 46/46 pass.dotnet test ./src/Tests/Grand.Infrastructure.Tests/Grand.Infrastructure.Tests.csproj— 84/84 pass.nameof(T)back inLiteDBRepository.TableCollection<C>()and re-run step 2.TableCollection_ReadsTheEntityCollectionfails withexpected: 2, actual: 0. Revert.order, and open the admin panel. Everything resolving
IMongoDatabasemust behave as before —the point of the change is that only the client's lifetime moved.
Database:UseLiteDbtotrue, enable theGrand.Module.Apifeature, and calla generic API query endpoint. It returns rows; before this change it returned an empty set.
Steps 5 and 6 have not been run here — they need a running instance and, for 6, a LiteDB
installation with the API module switched on.
🤖 Generated with Claude Code