Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow DI for pooled DbContexts #30739

Merged
merged 1 commit into from
Apr 21, 2023

Conversation

stevendarby
Copy link
Contributor

@stevendarby stevendarby commented Apr 20, 2023

Fixes #27752

This allows resolving additional constructor parameters from the root service provider when creating a new DbContext in DbContextPool. The main intention here is to allow singleton services, which should generally be safe, but there are some risks with this approach:

  • You can create scoped services from the root provider if scope validation is disabled. I think it's generally good practice to have scope validation enabled at least during development (this is default in hosted environments) so these issues can be caught, so I think this might be alright.
  • You can create transient services from the root provider, even with scope validation. Although it's already the case today that any singleton today can inject a transient service, let's think about it in the context of developer who is used to the typical setup-- without pooling and with a scoped DbContext. A transient service injected into a scoped service is safe to do things like store information specific to a request, and maybe that is how they've designed some transient services. The developer moves to pooling and creates a scoped factory so he can go on treating the DbContext as scoped. But now the transient service it has injected (facilitated by this PR) actually has the lifetime of the container and its behaviour is dangerous.

I'm not sure if these risks are unique to this situation, or are always there in some form when using DI. The documentation for pooling is already quite good at highlighting some of this. On the other hand, the current pattern - where you're forced to manually set scoped dependencies in a scoped factory after you get the DbContext from the pool - isn't really that arduous and perhaps makes the developer more conscious of all this. Therefore I really wouldn't mind if this PR is rejected, but it was fun & a learning experience!

  • I've read the guidelines for contributing and seen the walkthrough
  • I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
  • The code builds and tests pass locally (also verified by our automated build checks)
  • Commit messages follow this format:
        Summary of the changes
        - Detail 1
        - Detail 2

        Fixes #bugnumber
  • Tests for the changes have been added (for bug fixes / features)
  • Code follows the same patterns and style as existing code in this repo

- Allow root provider DI for pooled DbContexts
@ajcvickers ajcvickers merged commit b35364d into dotnet:main Apr 21, 2023
@ajcvickers
Copy link
Member

Thanks @stevendarby. Much appreciated!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow pooling DbContext with singleton services
2 participants