search

JasperFx / wolverine

Supercharged .NET server side development!
https://wolverinefx.net
MIT License
1.27k stars 139 forks source link

EFCore transactions code generation and Compound Handlers issue #1118

Closed fredrikeriksson closed 3 weeks ago

fredrikeriksson commented 3 weeks ago

Describe the bug The code that handles transactions isn't generated when combined with Compound Handlers. For example with a 'Handle method' without DbContext and a 'Load method' with DbContext. This applies to both to AutoApplyTransactions and marking the 'Handler method' with [Transactional].

The docs explicitly says that the transaction is enforced if the 'Handler method' have any type of DbContext as a dependency so I guess it does what it says. However, one could argue that the 'Load method' also could be a factor for this functionality as you probably could ommit the DbContext from the handler alltogether in a majority of cases and make unit tests slightly cleaner.

Also when Wolverine is used with Marten the behaviour seems to similar, as you could have no dependencies on IDocumentSession and it would still work, although I guess this is due to the 'Aggregate Handlers' feature.

It is by no means a blocker and can easily be worked around by adding the dependency, though the dependency would then be unused in this scenario.

To Reproduce Steps to reproduce the behavior:

  1. Setup a project with WolverineFx.EntityFrameworkCore 3.1.0 with AddDbContextWithWolverineIntegration and UseEntityFrameworkCoreTransactions
  2. Create a Handler
  3. Create the Handle method without DbContext and marked with [Transactional]
  4. Create the Load method with DbContext
  5. dotnet run -- codegen write
  6. Transactional code is not generated

Expected behavior Transactional code should be generated.

jeremydmiller commented 3 weeks ago

It's not technically a "bug" per se so much as a usage we just never anticipated. Won't be a huge effort to address I think

jeremydmiller commented 3 weeks ago

Timing issue in the code. Got a fix, now just seeing if there's any regression damage:(

fredrikeriksson commented 3 weeks ago

@jeremydmiller I just updated and can confirm that the change made my scenario work for both the attribute and auto apply! Awesomely quick, thank you! 😄 Now I can make my wolverine comparison pocs even more tidy! 🥳