RFC: Adding arbitrary middlewares to command & query routes

[basz]

To add additional middleware to prooph routes one currently has to add those manually to each defined route.

eg.

[
  'name'            => 'query::support-doc::list-faqs',
  'path'            => '/support-doc/categories/{category_id}/faqs',
  'middleware'      => [
    JsonPayload::class,
    JsonApiProblem::class,
    ResourceServerMiddleware::class,
    JsonApiRequestMiddleware::class,
    ContentLanguageMiddleware::class,
    QueryMiddleware::class,
  ],
  'allowed_methods' => ['GET'],
  'options'         => ['values' => [QueryMiddleware::NAME_ATTRIBUTE => QueryName::listFaqs]],
],

We could introduce a way so QueryMiddleware and CommandMiddleware are capable of adding arbitrary middlewares so it can be easily configured.

I'm thinking of this configuration bit and some factory modifications;

    'prooph' => [
        'middleware' => [
            'query'   => [
                'response_strategy' => \HF\Api\ResponseStrategy\JsonApiResponseStrategy::class,
                'message_factory'   => \HF\Api\Messaging\QueryMessageFactory::class,
                'metadata_gatherer' => \HF\Api\Middleware\RequestMetadataGatherer::class,
                'middlewares'       =>
                    [
                        JsonPayload::class,
                        JsonApiProblem::class,
                        JsonApiRequestMiddleware::class,
                        ContentLanguageMiddleware::class,
                    ],
            ],
            'command' => [
                'response_strategy' => \HF\Api\ResponseStrategy\JsonApiResponseStrategy::class,
                'message_factory'   => \HF\Api\Messaging\CommandMessageFactory::class,
                'metadata_gatherer' => \HF\Api\Middleware\RequestMetadataGatherer::class,
                'middlewares'       =>
                    [
                        JsonPayload::class,
                        JsonApiProblem::class,
                        JsonApiRequestMiddleware::class,
                        ContentLanguageMiddleware::class,
                    ],
            ],
            ...
        ],
    ],

These 'middlewares' would need to be fetched from the container by Command and Query Factories and passed to the Command and QueryMiddlewares whom should process them in order and before the MetadataGatherer runs.

Shouldn't be too difficult, thoughts?

[prolic]

I think we would need pre- and post-middlewares. The metadata gatherer can then be removed as option and simply added to the pre-middleware list.

[basz]

The metadata gatherer can then be removed as option and simply added to the pre-middleware list.

As it stands the MetadataGatherer isn't technically a Middleware

[prolic]

Ok my bad

On Nov 9, 2017 8:13 PM, "Bas Kamer" notifications@github.com wrote:

The metadata gatherer can then be removed as option and simply added to the pre-middleware list.

As it stands the MetadataGatherer isn't technically a Middleware

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/prooph/psr7-middleware/issues/29#issuecomment-343136825, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYEvAxxrq-baEewerTq2Tt_eykbrC3hks5s0uxcgaJpZM4QXodv .

[basz]

I believe the biggest hurdle is to somehow insert the given middlewares list into the pipeline of the current route and have those execute from within the executing Command- & QueryMiddleware. Any idea on how to do that?

[basz]

https://zendframework.slack.com/archives/C4QBQUEG5/p1509550722000242 https://zendframework.slack.com/archives/C4QBQUEG5/p1509117340000326

@danizord could you share your experiences and perhaps have some advise

[prolic]

@basz I don't have a slack account for the zendframework workspace. Slack tells me I should ask the admin, so it seems not to be a public forum. Can you share the information for non-zf-slack users?

[weierophinney]

@prolic https://zendframework-slack.herokuapp.com for invites.

[prolic]

I am not sure, just a shot into the blue:

if (isset($options['middleware'])) {
            $middlewares = (array) $options['middleware'];
            $middlewares[] = $commandMiddleware;
            $pipe = new SplQueue();
            foreach ($middlewares as $middleware) {
                if (is_string($middleware)) {
                    $middleware = $container->get($middleware);
                }
                $pipe->enqueue($middleware);
            }
            return new Next($pipe);
        }

[danizord]

I believe the biggest hurdle is to somehow insert the given middlewares list into the pipeline of the current route and have those execute from within the executing Command- & QueryMiddleware. Any idea on how to do that?

@basz @prolic you can also do that with MiddlewarePipe and LazyLoadingMiddleware:

use Zend\Expressive\Middleware\LazyLoadingMiddleware;
use Zend\Stratigility\MiddlewarePipe;

$pipeline = new MiddlewarePipe();
$response = new Response();

foreach ($middlewares as $middleware) {
    $pipeline->pipe(new LazyLoadingMiddleware($container, $response, $middleware));
}

$pipeline->process($request, $queryOrCommandMiddleware);

---

Having said that, let me show you the approach (a bit unrelated) that I've been working with. Most middlewares would require different configuration for each route, e.g an JsonSchemaValidationMiddleware would use a different schema per route. Even QueryMiddleware and CommandMiddleware shipped by this package requires route-based configuration such as the query/command name.

I can see in your example that this package solves that using route options, but it has some problems:

1. Looking at your route config, you can't identify for which middleware each option is for;

2. The route options are not type-checked or analyzable statically, which makes it harder to find bugs before running the code;

3. Due to 1 and 2, people avoid writing too much configurable middlewares and end up writing either fat middlewares that solves multiple problems and violates SRP (hard to remove/replace specific behaviors) or middlewares with their own plugin systems (response strategy, message factory, metadata gatherer...)

The approach I'm using to solve that is passing these configuration as params to middleware constructors and creating different instances for each route:

// These with "Factory" suffix are custom factories that already contains dependencies
// injected by the container and expects the configuration to create actual middlewares.
$inputValidation = $container->get(InputValidationMiddleware::class);
$createQuery     = $container->get(CreateQueryMiddlewareFactory::class);
$createCommand   = $container->get(CreateCommandMiddlewareFactory::class);
$dispatchCommand = DispatchCommandMiddleware::class;
$dispatchQuery   = DispatchQueryMiddleware::class;
$commandResponse = new CommandResponseMiddleware();
$queryResponse   = $container->get(QueryResponseMiddlewareFactory::class);

// Ping
$app->get('/internal/ping', $commandResponse->withSuccessMessage('ok'));

// ScheduleReminder
$app->post('/api/reminders', [
    // I'm using with* methods because I found it more readable, but it can be pure
    // callables like $inputValidation('schedule-reminders.json')
    $inputValidation->withJsonSchema('schedule-reminders.json'),

    // Thanks to callable typehint, PHPStorm recognizes it as the class static method,
    // So you can refactor, find usages, click to navigate and such :)
    $createCommand->withCommandFactory([UpdateSettings::class, 'fromRequest']),

    // This middleware does not require any configuration, so we don't need to
    // instantiate it here, we can pass the name and Expressive will lazy instantiate it
    $dispatchCommand,

    // Some middlewares wouldn't need dependencies from container, so you don't need
    // to create a custom factory, you can implement immutbale with* methods like PSR-7
    $commandResponse->withRedirectTo('/api/reminders/{id}'),
]);

// GetReminders
// You can see how the middleware pipeline looks very readable :)
$app->get('/api/reminders', [
    $createQuery->withQueryFactory([GetReminders::class, 'fromRequest']),
    $dispatchQuery,
    $queryResponse->withResourceClass(ReminderResource::class),
]);

[basz]

Thank you @danizord lots to play with and contemplate. I'll start with the MiddlewarePipe thing, but I think @prolic and @codeliner should have a read over your second point. Definitely a BC but it might be worth investigating...

[sprolic-modus]

BC is not a problem, as we have only dev-releases so far and not even 1.0 final.

[sprolic-modus]

@danizord your approach loads all the middlewares for all requests, despite of being used, right? I kind of don't like that.

[danizord]

@sprolic-modus if this is hurting performance, you can do some workaround to pipe the middlewares lazily:

$app->get('/api/reminders', lazyPipeline(function () use ($container) {
    // Pull from container...

    return [
        $createQuery->withQueryFactory([GetReminders::class, 'fromRequest']),
        $dispatchQuery,
        $queryResponse->withResourceClass(ReminderResource::class),
    ];
}));

But the syntax starts to become confusing (would be nice if PHP had arrow functions) :D

Also since the middleware pipeline is stateless, you can use PHPFastCGI, ReactPHP or something like that to bootstrap once and keep running.

[codeliner]

@sprolic-modus are you the real @prolic ? :D

@danizord I really like the approach, but would definitely go with the lazyPipeline approach. Using reactPHP/amp for bootstrapping once can solve the problem, BUT you introduce complexity in your app that is not always needed. reactPHP/amp is single threaded non-blocking I/O just like Node.js is. While this is a performance boost when done right you have to be very careful. Not only the middleware pipeline needs to be stateless, but the entire app and all dependencies as well. That's not all. You also need to make sure that you don't block the event loop! One mistake and you have a lot of trouble in production. CQRS/ES is an architecture that plays well with PHP's shared nothing environment. I don't want to give up this simplicity so loading only the stuff needed to serve a single request/message is a must-have and support for reactPHP/amp is a nice-to-have IMHO.

[codeliner]

@weierophinney Why slack? :D

I mean we use it for work and it is working fine but for an OSS project hosted on github we really like gitter. People can join with their github or twitter account and directly start asking questions. And gitter is completely open source.

[prolic]

Confirming, sprolic-modus is me. Sorry was logged in wrong account. This one is for my current client only.

On Nov 15, 2017 4:40 AM, "Alexander Miertsch" notifications@github.com wrote:

@weierophinney https://github.com/weierophinney Why slack? :D

I mean we use it for work and it is working fine but for an OSS project hosted on github we really like gitter. People can join with their github or twitter account and directly start asking questions. And gitter is completely open source.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/prooph/psr7-middleware/issues/29#issuecomment-344391091, or mute the thread https://github.com/notifications/unsubscribe-auth/AAYEvGYJOLCwEQUjEcdz1FQbzMJFk1yQks5s2fqdgaJpZM4QXodv .

[codeliner]

ah ok. @sprolic-modus sounds like your hacker account (hopefully white hat) :P

[sprolic-modus]

Haha

On Nov 15, 2017 4:50 AM, "Alexander Miertsch" notifications@github.com wrote:

WARNING - This email was received from outside EDR but claims to be from an EDR email address. Unless you are specifically expecting this email, it is spam/phishing and should be deleted.

ah ok. @sprolic-modus https://github.com/sprolic-modus sounds like your hacker account (hopefully white hat) :P

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/prooph/psr7-middleware/issues/29#issuecomment-344393918, or mute the thread https://github.com/notifications/unsubscribe-auth/AcsirIhA-vtJv5ntiKbbsThP0XElQpEIks5s2f0KgaJpZM4QXodv .

[weierophinney]

@codeliner —

Why slack?

• Developer familiarity
• Moderation tooling
• Amount of integrations
• Large number of resources for integrating bots
• etc. etc. etc.

Not going to debate it again. It works for us, and, frankly, developer engagement has gone way up since we started using it.

[codeliner]

interesting. thx @weierophinney and definitely better than IRC. Never was a fan of IRC, though

[prolic]

@basz Please move this issue to https://github.com/prooph/http-middleware/

[basz]

This issue was moved to prooph/http-middleware#5