Auto-wire Command Handler Middleware

[rdohms]

The Command Handler Middleware needs to be present and always needs to be executed. This should not be left up to the user and listed in docs as a reminder. We should do this as a post-fix to the configuration and let it be automatic.

[rosstuck]

That's not always true, if folks were queueing rather than directly executing or logging or plugged in an entirely different execution strategy, those might all require removing this middleware.

I agree it should be there 90% of the time but I don't think we should always force it.

[rdohms]

Ah, my bad was mislead by lack of knowledge.

Ok, do you think there is a chance to do any if this automatically? Maybe a test that can decide "you have no exit strategy" and add it or something?

Or a default value that can be turned off?

[rtuin]

At the moment I think it's best not to preconfigure the command handler middleware, but to emphasise it in the TacticianBundle documentation that this middleware is very useful to have in most cases. I think it already is, though. Documenting but not defaulting to encourages the user to better consider the implementation.

[rosstuck]

Hmm... What about a middle road where we don't force or autoappend via a switch, but we do add a default set of middleware, just to get people started? I think a low barrier to entry is really nice for people just experimenting.

However, when they start overriding the middleware in the config, they need to override them all and we create a good section about that in our Symfony2 docs.

If that sounds legit, I think we can have a discussion about the default setup to use for Symfony. I'd probably propose something like:

• validator
• locking?
• doctrine transaction (separate repo, https://github.com/thephpleague/tactician-doctrine)
• command handler

We could also go minimal so that an incomplete override wouldn't eliminate features people had grown accustomed to and it might even encourage them to get involved with enabling things. Regardless of the default middleware, we should document this verrrrry clearly on the Symfony2 bundle page (and ZF2 and Laravel and...)

[rosstuck]

Also, nice to have you on board, @rtuin :D

[rdohms]

I think i agree with your last paragraph.

We should just add a default setup to the docs in the "install" instructions and reference further documentation for detailed stuff. So they know to add this to get started but we don't force anything.

Further, correct me if this is wrong, "in case the command_handler middleware is present it should be the last", if this is true, we can add configuration validation that basically checks if one of the items it the handler and alerts if its not the last one.

Is there a use case where it is present and NOT the last?

[rtuin]

+1 on that idea!

Is there a use case where it is present and NOT the last?

I can't think of one right now

[rosstuck]

There isn't a use case for anything afterwards right now. The CommandHandlerMiddleware is terminal: in other words, it never invokes the $next callable and continues onwards. This kinda makes sense because it always returns the return value from the handler and even throws an exception if it doesn't find a matching handler.

I considered for the 0.5 release about making the CommandHandlerMiddleware always return the return value from the handler but still invoking the next callable so the chain can continue onwards but early feedback was that there's no reasonable use case and it's kinda confusing.

I'd be open for some kinda warning but not sure how we'd display that. Perhaps in some sorta data collector toolbar preview? A validate or recommendation CLI command? Remember though, if you're always queueing (or using a different execution strategy), that warning is always going to be annoying you.

[rdohms]

@rosstuck what i'm thinking is:

Use the Configuration embedded validation to analyse the layout of middleware, then:

• if tactician.middleware.command_handler is not present, return true.
• if tactician.middleware.command_handler is present and not last throw validation error

This way if you are not using the command_handler it will say nothing, but if you are using it it tells you 'hey this should be last'. This will happen at execution time, when symfony reads the config. Kinda like when you don't add default config or set config for a bundle that is not loaded.

It would just be a warning, for the specific use case of having the handler, if you are queing i figure the command handler is not in there?

[rtuin]

if you are queing i figure the command handler is not in there?

How I'm using it, it is. The queue handler league/tactician-bernard only handles QueuableCommand but passes everything else on to the next handler in the chain, which would be the CommandHandlerMiddleware.

[rtuin]

I've changed my mind about preconfiguring the default handler. Especially after the ideas in https://github.com/xtrasmal/TacticianBundle/issues/6 about the ability to explicitly set a default command bus. So, I think it's a great idea to have a default one.

[rdohms]

But then your queueing case does not invalidate my suggestion right?

[rtuin]

No, it does not. I think it's a great suggestion; config validation is good.

[rosstuck]

:+1: the default thing ties it all together nicely.

[rtuin]

@rdohms i guess this is solved with the multiple bus PR that got merged today. Can you confirm?

[rdohms]

Is not, opening PR right now, hang on.

[rdohms]

Ok that should cover all scenarios.

[rtuin]

You're fast man! :+1: