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

Question: How does this work for DI and also IDisposable? #5

Open
lukos opened this issue Jun 4, 2019 · 5 comments
Open

Question: How does this work for DI and also IDisposable? #5

lukos opened this issue Jun 4, 2019 · 5 comments
Labels
question Further information is requested

Comments

@lukos
Copy link

lukos commented Jun 4, 2019

We currently use this library to be able to unit test our repository classes but now we are in a situation where we are not utilising connection pooling properly because the connection is already created (and injected) when the repository is invoked, you cannot use the normal using syntax when creating a connection inside an individual method.

I can't think of the correct pattern:

  1. Use the normal using ( var connection = new DataAccessor(...) ) which disposes and pools correctly but prevents constructor injection being possible so unit tests won't work.

  2. Inject IDataAccessor into the constructor as we currently do. This allows unit tests but does not permit the use of using, which means we should probably open the connection in the constructor and ensure our DI services are created in a liftetime scope so they don't tie up the connection for too long.

  3. Inject the IDataAccessor into the constructor but don't open it and then in every method, call Open() and Close(). This would generate more code but would ensure we are not holding onto connections and not opening the connection if we don't need it (since we are using EF for some parts of the repositories)

  4. Have constructor overloads one that takes IDataAccessor for unit tests and one that doesn't but then how would a method use the correct connection e.g. if the shared conn from the unit tests is available, use that, otherwise use the using syntax. This sounds messy.

Do you have any ideas? Thanks.

@codeapologist
Copy link
Owner

In my designs, I typically create a factory class that creates instances of IDataAccessor. You can then inject the factory into your constructor and create an IDataAccessor as needed in your individual methods with using statements. In your tests, you can then mock out the factory and IDataAccessor as well. One alternative is to use a factory method with your DI (if your DI container supports it), instead of creating your own factory class.

Another option... If its a web application, you can use constructor injection and register the IDataAccessor under a request scope lifecycle with your DI. Then use your DI lifecycle hooks/interceptors to create, open, close, and dispose the IDataAccessor automatically before and after the request. That way you don't have to manage the connection in your code.

@codeapologist codeapologist added the question Further information is requested label Jun 8, 2019
@f1ana
Copy link

f1ana commented Dec 1, 2019

My understanding of Dapper was that a supplied connection, if originally closed, will be opened and subsequently closed when an operation is complete. If you are injecting an IDataAccessor, the connection should not be opened by default, correct?
Here's a snippet from the Dapper library: https://github.com/StackExchange/Dapper/blob/master/Dapper/SqlMapper.cs#L528

@jvanzella
Copy link

If you didn't need to use a factory, and could just inject this service by having the service deal with the disposal of the connection, this would be a pretty elegant way to inject SQL commands into a service and still allowing the service to be thoroughly unit tested, I'm not sure why you are abstracting it out by adding a really unneeded factory in the middle.

@codeapologist
Copy link
Owner

codeapologist commented Sep 13, 2020

There are two main reasons why I choose to implement a factory pattern in my designs:

  1. To support multi-threaded scenarios. There are cases when I want to execute multiple queries concurrently, and you are not able to do that safely with a single database connection.
  2. The ability resume execution when the connection object is in a "bad state". This is an rare scenario, but if the db connection becomes corrupted and is irrecoverable, having a factory allows me to recover and continue execution or even attempt retry logic if desired.

Also, it is generally "best practice" to dispose of connection objects per query as a safety measure. Especially since there is no performance impacts with creating connection objects because of database connection pooling.

@cphorton
Copy link

There are two main reasons why I choose to implement a factory pattern in my designs:

  1. To support multi-threaded scenarios. There are cases when I want to execute multiple queries concurrently, and you are not able to do that safely with a single database connection.
  2. The ability resume execution when the connection object is in a "bad state". This is an rare scenario, but if the db connection becomes corrupted and is irrecoverable, having a factory allows me to recover and continue execution or even attempt retry logic if desired.

Also, it is generally "best practice" to dispose of connection objects per query as a safety measure. Especially since there is no performance impacts with creating connection objects because of database connection pooling.

We used a factory for these exact reasons, plus it also give us the opportunity to handle token management for Azure SQL managed identity connections.

p.s. Love the library, thanks so much @codeapologist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants