rebus-org / Rebus.ServiceProvider

:bus: Microsoft Extensions Dependency Injection container adapter for Rebus
https://mookid.dk/category/rebus
Other
68 stars 32 forks source link

Add support for scope #11

Closed rosieks closed 4 years ago

rosieks commented 7 years ago

It looks that Rebus.ServiceProvider does not support correctly AddScoped method. Right now there is no difference betweeen using AddScoped and AddSingleton. I would expect that AddScoped means that some class is resolved once per message.

trreeves commented 7 years ago

To achieve handler instances per message, use a Transient lifestyle, which I think makes sense.

What value does supporting Scoped add?

rosieks commented 7 years ago

Suppose that I have message handler that depends on class A and B. And both this classes depends on DbContext. I want that both this classes share the same DbContext. In order to do that I register DbContext as Scoped.

Also now it is confusing that in asp.net when I register something as scoped then it's resolved per request and here it is a singleton.

trreeves commented 7 years ago

Aah yes, that sounds useful....and the default ServiceProviderEngineScope will do the same disposing of the handlers.

So with Transient registered handlers, their lifestyle would still behave as expected right? And get disposed by the scope.

rosieks commented 7 years ago

Exactly, ServiceProviderEngineScope will dispose all IDiposable objects that where resolved, so in case of my example also DbContext will be disposed.

xhafan commented 5 years ago

The current implementation which creates the scope in NetCoreServiceProviderContainerAdapter does not work in my scenario. In my scenario, I use Rebus.UnitOfWork, where in the unitOfWorkFactory method I create a database transaction wrapped in an IUnitOfWork service, which should have scoped lifestyle, and which could be injected into other services which would share the database transaction. The problem is that the unitOfWorkFactory method is called first with no scope created yet, after that the scope is created by NetCoreServiceProviderContainerAdapter. I think it should be the other way around - first, the scope should be created, and then the scope should be passed into the unitOfWorkFactory method.

Example: currently I use Castle Windsor with Rebus which works fine for me. Here is the console app, and here is the unitOfWorkFactory method RebusUnitOfWork.Create. The console app using the ServiceProvider, which didn't work for me, is here. As mentioned above, the RebusUnitOfWork.Create is called first before the scope is created, but that method should create a scoped IUnitOfWork.

My feeling is that Rebus.UnitOfWork and Rebus.ServiceProvider are not friends yet.

Liero commented 5 years ago

I also vote for this. Actually I believe this should be labeled as a bug.

In asp.net core the behavior is:

I believe the only reasonable equivalent for Rebus is:

I can't imagine scenario, where scope would be expected to end after the handlers are created but before they are invoked.

My scenario is similar to @xhafan's

  1. create single DbContext per message
  2. invoke all message handlers using the DbContext instance
  3. call DbContext.SaveChanges() once per Message after all handlers are invoked.
mookid8000 commented 4 years ago

A change was introduced recently, which I believe fixes this.

Please check out Rebus.ServiceProvider 5.0.1, which will resolve handlers from a lifetime scope scoped to the incoming step context.

It also supports providing the scope manually, so that incoming pipeline steps earlier in the pipeline may create the scope and have it used throughout.