verdie-g / StoredProcedureEFCore

Entity Framework Core extension to execute stored procedures
MIT License
192 stars 44 forks source link

Close connection only in transaction-less case #44

Closed SeriousM closed 4 years ago

SeriousM commented 4 years ago

An existing transaction may hold an open transaction without a transaction can't exist. If the connection is closed, the transaction is A) broken/rolled back and B) doesn't know that the transaction was closed. Any other use of the transaction would result in an exception.

Solution: only close the transaction if no transaction was available.

verdie-g commented 4 years ago

Duplicate of #40 which is just missing tests. The tests are a bit annoying to setup, you need to create a db and load the stored procedures to run them.

SeriousM commented 4 years ago

The test is indeed annoying to setup. I'm usually very strict about tests but in this case it's obvious that you shouldn't close a connection yourself if a transaction existed during StoredProcBuilder creation, isn't it? It's not a feature we're introducing which requires a test instead it's implementing a correct behaviour like disposing an object after usage. Please be kind and accept either #40 or #44 as the test setup is really hard and can't be achieved with memorydb but a real db.

verdie-g commented 4 years ago

Okay. Closed in favor of #40.

SeriousM commented 4 years ago

thank you very much!