Split Bus App Lifetime Dependencies from CommandProcessor Instance Lifetime Dependencies

[iancooper]

Move that which has app lifetime into a service. This is a WIP.

First pass, approach:

• Create CommandProcessorService

• Move all private methods there

• Copy all private state there

• Create singleton for service

• Remove all private state from service that is scoped/transient

• Fix any methods that need scoped/transient fields

• Pass tests

• Review CommandProcessor has any scoped/transient state

• Review thread safety

• Review split of methods - anything obviously in the home that breaks responsibility principles?

[iancooper]

@preardon WIP I need to stop for a while, should have a little time later. ETA for something that shows if, and how this might work would be lunchtime 7 July

[iancooper]

PS Looking at the principal that as an implementation detail, we don't want to expose this as public just to allow us to get the singleton via DI, so using an old-skool double lock here instead.

[preardon]

Just noticed that, I'll have a look as soon as I get s second, I will have to get my head around a few things (i.e. call back on a disposed object) but I'll see if I can have a look later tonight

[iancooper]

Just noticed that, I'll have a look as soon as I get s second, I will have to get my head around a few things (i.e. call back on a disposed object) but I'll see if I can have a look later tonight

Let me finish up first, as I am not sure the Disposable CallContext will make the cut as it may not be thread proof. It will be in better shape tomorrow

[iancooper]

@preardon I am much happier with this starting point. We move the dependencies for the external bus into a singleton, as their lifetime is not per request.

• Likely breaks tests as we don't close the service between requests. Probably needs a static method for tests to allow you to clear the static _bus field on a CommandProcessor. Most of our CommandProcessor tests will need this, we'll look for the empty Dispose.
• It surfaces the issue of 'this has app lifetime' it doesn't fix it yet. So it surfaces that we have an outbox with app lifetime, that may want a provider to grab its connection from on an add; it surfaces that you only have one producer in the app right now, what if you want two? I think this is good, as it indicates what we need to address.

I'll get the tests passing, but this is a branch, so we can both work on it.

[iancooper]

@preardon To understand it, just look at the CommandProcessor and ExternalBusServices files. They latter is what CommandProcessorServices turned into.

[preardon]

@iancooper I'll have a look as soon as I get a minute

[preardon]

Hey @iancooper ,

Sorry its taken so long but I've had a quick look, Just to make sure I'm on the same page, the Idea here is that we could set the Command Processor to have any service lifetime as the bits that need to be a singleton (External Bus Service are set once)?

From what I can see this will work for Dependencies inside the Handler Factory, I am still having a think how this is going to work for what I had in my head around the EF Connection Provider for outbox (and some of the changes in the new Microsoft ASB Library, however I think I can keep them container if I add a Connection provider / Token Helper)

Let me know what I can do to lend a hand here or if you would like to pair

[iancooper]

Just to make sure I'm on the same page, the Idea here is that we could set the Command Processor to have any service lifetime as the bits that need to be a singleton (External Bus Service are set once)?
That was the driver, but I noted that the dependencies that need to have app lifetime were all related to using Brighter with an external service bus - to send messages. So I decided that the modifiability of the software could be improved by creating a class that had responsibility for access to the external service bus, and thus handled producers and outbox. (That does mean that the pipeline cache which is also a singleton continues to live in the pipeline, but I like the allocation of responsibilities here)

It's an idea that emerged for me during refactoring

I am still having a think how this is going to work for what I had in my head around the EF Connection Provider for outbox (and some of the changes in the new Microsoft ASB Library, however I think I can keep them container if I add a Connection provider / Token Helper)

Yes that is still outstanding

Let me know what I can do to lend a hand here or if you would like to pair
I was really inspired by your PR, but just felt there was a better split. Now we have to follow through to a fix

• I am plugging through the tests to get this working
• I'll look at the DSL to make sure we set the scope controls for cp and handlers sensibly, looking at your PR
• If you can figure out how to provide a connection provider that would be great. I think it becomes a CommandProcessor parameter where there is an Outbox, that we pass to ExternalServiceBus that uses it during an Add, but you are closer on this than me.

[preardon]

I've been having a thing about this and I have an Idea, going to drive it out to see what the least about of invasive I can make it is. The Crux of it is that if we want the Outbox as a singleton (outside of scope) then it needs a Connection Provider that is out of Scope as well however if we want to provide the same connection as the Application, then it needs to be in scope as well, I think I've also got Token based Authentication mixed up in this in my head (as you need to get a token when the one you have expires) but I think I can break these into 2 separate problems, and Token Expiry is definitely something I can solve easily (will have to do this for ASB soon as well)

I have the current 2 streams of though in my head

1. Give the outbox a Singleton Connection provider and then the ability to provide an override connection provider for when you call Deposit or DepositAsync (No point on Post in my opinion) and I also don't feel we should worry on clear

2. Attempt to inject a Scoped Connection Provider and if there allow it to be used on Depositing

The thing that I'm not going to Love about this is that I will probably have to make a OutboxConnectionProvider type and then cast and check in each type of outbox

Either way Will drive them out and see where I end up

[iancooper]

1. Give the outbox a Singleton Connection provider and then the ability to provide an override connection provider for when you call Deposit or DepositAsync (No point on Post in my opinion) and I also don't feel we should worry on clear

I think this is my thinking, but more really:

• A CommandProcessor can take a Connection Provider as a dependency, that has the same lifetime as the CommandProcessor.
• The call to the External Service Bus methods that use the Outbox (for Deposit and Post as Post calls Deposit) then pass the OutboxProvider in the call to the ExternalServiceBus, which checks if they are not null and uses them to set a temp connection for the outbox that it uses for that call

[preardon]

That is effectively what my PR ( #1635 ) does

[iancooper]

That is effectively what my PR ( #1635 ) does

OK, we'll try to figure it out. I wanted to make sure I had the lifetimes right here, so that they could be set, and then look at how to merge the connection provider onto this. Trying to avoid too many moving parts now

[preardon]

Yeah, it's getting to be a lot of change, I've added an extension method that registers the overriding connection provider so that you can set it to a separate lifetime to the rest of the outbox. The overriding connection provider should have the same lifetime as the command processor where as the default connection provider should be a singleton

[iancooper]

@preardon Going to merge and tag you in. I want a WebAPI + Ef Core sample, but I will create a separate branch and PR for that, so you can pursue the scope issue for outbox independent of that