Dispose MongoDb, SLDR in singletons#3924
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3924 +/- ##
==========================================
+ Coverage 74.13% 74.15% +0.01%
==========================================
Files 288 288
Lines 10614 10621 +7
Branches 1326 1327 +1
==========================================
+ Hits 7869 7876 +7
Misses 2359 2359
Partials 386 386
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // Register concrete types for dependency injection | ||
|
|
||
| // Mongo context for use in repo contexts | ||
| services.AddSingleton<IMongoDbContext, MongoDbContext>(); |
There was a problem hiding this comment.
I would make this Scoped. Otherwise you will be sharing the same client across all requests and I would be quite surprised if the client was threadsafe.
There was a problem hiding this comment.
It turns out it is and this usage is actually recommended:
https://mongodb.github.io/mongo-csharp-driver/2.0/reference/driver/connecting/
There was a problem hiding this comment.
Whew! It was gonna be a pain to make it (mostly) Scoped since it's used by the SemanticDomainContext which we've made a Singleton.
|
I would probably set Db to null here, I'm not sure if there are any consequences to using it after the client is disposed, but it would certainly be unintended. |
| // Register concrete types for dependency injection | ||
|
|
||
| // Mongo context for use in repo contexts | ||
| services.AddSingleton<IMongoDbContext, MongoDbContext>(); |
There was a problem hiding this comment.
It turns out it is and this usage is actually recommended:
https://mongodb.github.io/mongo-csharp-driver/2.0/reference/driver/connecting/
jasonleenaylor
left a comment
jasonleenaylor
left a comment
There was a problem hiding this comment.
Reviewed 19 of 19 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @hahn-kev and @imnasnainaec)
imnasnainaec
left a comment
imnasnainaec
left a comment
There was a problem hiding this comment.
Reviewable status: 18 of 19 files reviewed, 1 unresolved discussion (waiting on @hahn-kev and @jasonleenaylor)
| // Register concrete types for dependency injection | ||
|
|
||
| // Mongo context for use in repo contexts | ||
| services.AddSingleton<IMongoDbContext, MongoDbContext>(); |
There was a problem hiding this comment.
Whew! It was gonna be a pain to make it (mostly) Scoped since it's used by the SemanticDomainContext which we've made a Singleton.
3 participants
Resolves #3921
This change is