Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http-middlewares CommandHandler can not retrieve configure RouteOptions via ServerRequestInterface::getAttribute() #162

Closed
BreiteSeite opened this issue Mar 27, 2018 · 13 comments

Comments

@BreiteSeite
Copy link

BreiteSeite commented Mar 27, 2018

Here's how i configured the route (slightly anonymized) in zend expressive (inspired by the proophessor-do routing) which does not work:

$app
        ->post(
            '/foo',
            [
                \Middlewares\JsonPayload::class,
                \Zend\ProblemDetails\ProblemDetailsMiddleware::class,
                \Prooph\HttpMiddleware\CommandMiddleware::class,
            ],
            'command:foo'
        )
        ->setOptions([
            'values' => [
                \Prooph\HttpMiddleware\CommandMiddleware::NAME_ATTRIBUTE => \Acme\Foo:class,
            ],
        ]);

I'm new to expressive and not 100% sure how route options and request-attributes are meant to be used correctly, but according to the psr interface itself it's the following-definition:

Additionally, this interface recognizes the utility of introspecting a
request to derive and match additional parameters (e.g., via URI path
matching, decrypting cookie values, deserializing non-form-encoded body
content, matching authorization headers to users, etc). These parameters
are stored in an "attributes" property.

I made it temporary working by applying this hack:

--- vendor/prooph/http-middleware/src/CommandMiddleware_original.php	2018-03-27 11:06:09.000000000 +0200
+++ vendor/prooph/http-middleware/src/CommandMiddleware.php	2018-03-27 11:06:09.000000000 +0200
@@ -80,7 +80,7 @@
 
     public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface
     {
-        $commandName = $request->getAttribute(self::NAME_ATTRIBUTE);
+        $commandName = $request->getAttribute(\Zend\Expressive\Router\RouteResult::class)->getMatchedRoute()->getOptions()['values']['prooph_command_name'];
 
         if (null === $commandName) {
             throw new RuntimeException(

So i think there is no way to pass information 1:1 from the route definition to the Middleware.
It seems we have to use a middleware to pass the information along which command handler should be used. At least if we want to stay framework agnostic.

Maybe prooph can provide a generic one to be used in the future or we have a different way to pass this information along?

Thats how i configured it in the to get it working without the hack:

$app
        ->post(
            '/foo',
            [
                \Middlewares\JsonPayload::class,
                \Zend\ProblemDetails\ProblemDetailsMiddleware::class,
                function (\Psr\Http\Message\ServerRequestInterface $request, \Psr\Http\Server\RequestHandlerInterface $handler) : \Psr\Http\Message\ResponseInterface {
                    $request = $request->withAttribute(
                        \Prooph\HttpMiddleware\CommandMiddleware::NAME_ATTRIBUTE,
                        \Acme\Foo:class
                    );

                    return $handler->handle($request);
                },
                \Prooph\HttpMiddleware\CommandMiddleware::class,
            ],
            'command:foo'
        );

This issue is mainly to validate if my approach is correct and also can be used to update the proophessor-do example if that's the case.

Also we might create other issues from here for different prooph components (for example to provide a helper callback to pass this information along).

Output of composer info

beberlei/assert                         v2.9.3             Thin assertion library for input validation in business models.
container-interop/container-interop     1.2.0              Promoting the interoperability of container objects (DIC, SL, etc.)
doctrine/annotations                    v1.6.0             Docblock Annotations Parser
doctrine/cache                          v1.7.1             Caching library offering an object-oriented API for many cache backends
doctrine/collections                    v1.5.0             Collections Abstraction library
doctrine/common                         v2.8.1             Common Library for Doctrine projects
doctrine/dbal                           v2.6.3             Database Abstraction Layer
doctrine/inflector                      v1.3.0             Common String Manipulations with regard to casing and singular/plural...
doctrine/instantiator                   1.1.0              A small, lightweight utility to instantiate objects in PHP without in...
doctrine/lexer                          v1.0.1             Base library for a lexer that can be used in Top-Down, Recursive Desc...
doctrine/migrations                     v1.6.2             Database Schema migrations using Doctrine DBAL
fig/http-message-util                   1.1.2              Utility classes and constants for use with PSR-7 (psr/http-message)
filp/whoops                             2.1.14             php error handling for cool kids
http-interop/http-factory               0.3.0              Common interface for HTTP message factories
marc-mabe/php-enum                      v3.0.0             Simple and fast implementation of enumerations with native PHP>=5.6
middlewares/payload                     v1.0.0             Middleware to parse the body of the request with support for json, cs...
middlewares/utils                       v1.0.0             Common utils to create PSR-15 middleware packages
myclabs/deep-copy                       1.7.0              Create deep copies (clones) of your objects
nikic/fast-route                        v1.3.0             Fast request router for PHP
ocramius/package-versions               1.3.0              Composer plugin that provides efficient querying for installed packag...
ocramius/proxy-manager                  2.2.0              A library providing utilities to generate, instantiate and generally ...
paragonie/random_compat                 v2.0.11            PHP 5.x polyfill for random_bytes() and random_int() from PHP 7
phar-io/manifest                        1.0.1              Component for reading phar.io manifest information from a PHP Archive...
phar-io/version                         1.0.1              Library for handling version information and constraints
phpdocumentor/reflection-common         1.0.1              Common reflection classes used by phpdocumentor to reflect the code s...
phpdocumentor/reflection-docblock       4.3.0              With this component, a library can provide support for annotations vi...
phpdocumentor/type-resolver             0.4.0             
phpspec/prophecy                        1.7.5              Highly opinionated mocking framework for PHP 5.3+
phpunit/php-code-coverage               6.0.1              Library that provides collection, processing, and rendering functiona...
phpunit/php-file-iterator               1.4.5              FilterIterator implementation that filters files based on a list of s...
phpunit/php-text-template               1.2.1              Simple template engine.
phpunit/php-timer                       2.0.0              Utility class for timing
phpunit/php-token-stream                3.0.0              Wrapper around PHP's tokenizer extension.
phpunit/phpunit                         7.0.3              The PHP Unit Testing framework.
phpunit/phpunit-mock-objects            6.0.1              Mock Object library for PHPUnit
prooph/common                           v4.2.2             Common classes used across prooph packages
prooph/event-sourcing                   v5.3.0             PHP EventSourcing library
prooph/event-store                      v7.3.3             PHP EventStore Implementation
prooph/event-store-bus-bridge           v3.1.0             Marry CQRS with Event Sourcing
prooph/http-middleware                  v0.1.0             http middleware for prooph components
prooph/pdo-event-store                  v1.7.3             Prooph PDO EventStore
prooph/service-bus                      v6.2.2             PHP Enterprise Service Bus Implementation supporting CQRS and DDD
psr/container                           1.0.0              Common Container Interface (PHP FIG PSR-11)
psr/http-message                        1.0.1              Common interface for HTTP messages
psr/http-server-handler                 1.0.0              Common interface for HTTP server-side request handler
psr/http-server-middleware              1.0.0              Common interface for HTTP server-side middleware
psr/log                                 1.0.2              Common interface for logging libraries
ramsey/uuid                             3.7.3              Formerly rhumsaa/uuid. A PHP 5.4+ library for generating RFC 4122 ver...
react/promise                           v2.5.1             A lightweight implementation of CommonJS Promises/A for PHP
roave/security-advisories               dev-master 4a272b6 Prevents installation of composer packages with known security vulner...
sandrokeil/interop-config               2.1.0              Provides interfaces and a concrete implementation to create instances...
sebastian/code-unit-reverse-lookup      1.0.1              Looks up which function or method a line of code belongs to
sebastian/comparator                    2.1.3              Provides the functionality to compare PHP values for equality
sebastian/diff                          3.0.0              Diff implementation
sebastian/environment                   3.1.0              Provides functionality to handle HHVM/PHP environments
sebastian/exporter                      3.1.0              Provides the functionality to export PHP variables for visualization
sebastian/global-state                  2.0.0              Snapshotting of global state
sebastian/object-enumerator             3.0.3              Traverses array structures and object graphs to enumerate all referen...
sebastian/object-reflector              1.1.1              Allows reflection of object attributes, including inherited and non-p...
sebastian/recursion-context             3.0.0              Provides functionality to recursively process PHP variables
sebastian/resource-operations           1.0.0              Provides a list of PHP built-in functions that operate on resources
sebastian/version                       2.0.1              Library that helps with managing the version number of Git-hosted PHP...
spatie/array-to-xml                     2.7.1              Convert an array to xml
squizlabs/php_codesniffer               2.9.1              PHP_CodeSniffer tokenizes PHP, JavaScript and CSS files and detects v...
symfony/console                         v4.0.6             Symfony Console Component
symfony/polyfill-mbstring               v1.7.0             Symfony polyfill for the Mbstring extension
symfony/yaml                            v4.0.6             Symfony Yaml Component
theseer/tokenizer                       1.1.0              A small library for converting tokenized PHP source code into XML and...
webmozart/assert                        1.3.0              Assertions to validate method input/output with nice error messages.
willdurand/negotiation                  v2.3.1             Content Negotiation tools for PHP provided as a standalone library.
zendframework/zend-code                 3.3.0              provides facilities to generate arbitrary code using an object orient...
zendframework/zend-component-installer  2.1.1              Composer plugin for automating component registration in zend-mvc and...
zendframework/zend-config-aggregator    1.1.0              Lightweight library for collecting and merging configuration from dif...
zendframework/zend-diactoros            1.7.1              PSR HTTP Message implementations
zendframework/zend-escaper              2.5.2             
zendframework/zend-eventmanager         3.2.0              Trigger and listen to events within a PHP application
zendframework/zend-expressive           3.0.1              PSR-15 Middleware Microframework
zendframework/zend-expressive-fastroute 3.0.1              FastRoute integration for Expressive
zendframework/zend-expressive-helpers   5.0.0              Helper/Utility classes for Expressive
zendframework/zend-expressive-router    3.0.2              Router subcomponent for Expressive
zendframework/zend-expressive-template  2.0.0              Template subcomponent for Expressive
zendframework/zend-expressive-tooling   1.0.0              Migration and development tooling for Expressive
zendframework/zend-httphandlerrunner    1.0.1              Execute PSR-15 RequestHandlerInterface instances and emit responses t...
zendframework/zend-problem-details      1.0.0              Problem Details for PSR-7 HTTP APIs
zendframework/zend-servicemanager       3.3.2              Factory-Driven Dependency Injection Container
zendframework/zend-stdlib               3.1.0             
zendframework/zend-stratigility         3.0.0              PSR-7 middleware foundation for building and dispatching middleware p...
zfcampus/zf-composer-autoloading        2.0.0              Sets up Composer-based autoloading for your Zend Framework modules
zfcampus/zf-development-mode            3.1.0              Zend Framework development mode script
@BreiteSeite BreiteSeite changed the title CommandHandler can not retrieve configure RouteOptions via ServerRequestInterface::getAttribute() http-middlewares CommandHandler can not retrieve configure RouteOptions via ServerRequestInterface::getAttribute() Mar 27, 2018
@prolic
Copy link
Member

prolic commented Mar 27, 2018

@basz wanna have a look into this?

@basz
Copy link
Member

basz commented Mar 27, 2018

Should work.

Might be a bug in zendframework/zend-expressive-fastroute. You could start by checking the getOptions method in that repo. It should indeed use 'values'. Not using fast router myself, nor does the proophessor-do app.

@basz
Copy link
Member

basz commented Mar 27, 2018

This is a bug in the fast router. I've created an issue there. Switch to aura router if this is blocking you

@basz
Copy link
Member

basz commented Mar 27, 2018

Seems to be not a bug but a limitation of fastroute (see zendframework/zend-expressive-fastroute#58 (comment)) and the reason prooph choose aura router...

@prolic
Copy link
Member

prolic commented Mar 27, 2018

@codeliner - thoughts on this?

@codeliner
Copy link
Member

yes, it is a limitation on fastroute. That's the reason why we use aura router in the example app.

If the router does not provide this functionality you have to pipe a middleware in front of the request handler to map the request to a command. We cannot do much about it. @BreiteSeite your approach is valid. Not sure if we should provide additional middleware. We're not designing a middleware dispatcher and proophessor-do is an example app using expressive together with aura router.

We just provide the last handler in the middleware pipe that takes the request body and the command name from a request attribute, creates the command and dispatches it on the bus.

The http-middleware package also provides a more generic MessageMiddleware that expects the request body to contain a serialized message. You can use that one if you want to include the command name in the request body:

{
  "message_name": "My.Command",
  "payload": {
    "key": "value",
    ...
  }
}

@codeliner
Copy link
Member

ah ok should have read the comment in the linked issue first ...

thx to @xtreamwayz
we may change our middleware to do this instead:

$routeResult = $request->getAttribute(\Zend\Expressive\Router\RouteResult::class, false);
$matchedRoute = $routeResult->getMatchedRoute();
$matchedRoute->getOptions()['values']['prooph_command_name'];

@codeliner
Copy link
Member

@BreiteSeite want to provide a PR here: https://github.com/prooph/http-middleware ?

@codeliner
Copy link
Member

arrgh no, next problem ...

The snippet above would couple our middleware to expressive router. We cannot do that, sorry. So my first statement is valid again!

@BreiteSeite
Copy link
Author

BreiteSeite commented Mar 27, 2018

@codeliner yes thats why i described my patch as "hack", as i also think this coupling is unnecessary. However it seems evident that there is a lack of some interop-standard regarding the communication between the middleware and the router config.

This is why i used another middleware, because communicating from middleware to middleware via request attributes is much more robust. And thats why i think switching the proophessor-do to the middleware approach would be good and also to provide a little helper (not a must-have though) would be the best way to approach this.

If you disagree with that, we should at least document this limitation from the example project somewhere (if it isn't already).

Maybe i can provide you with the PRs if you tell me what exactly you think should happen here.

Another option would be to check for the Zend-Expressive RouteResult as a fallback, however, i'm not a fan. But i think any implementation - including the current one - relying on the router implementation to silently do something unspecified is not the way to go, hence my another-middleware-approach.

Let me know what you think we should do here.

@codeliner
Copy link
Member

@BreiteSeite you could provide a PR for proophessor-do that uses a middleware instead of the aura router to set the command name as an attribute on the request. Maybe that's a good starting point for further discussions.

@grizzm0
Copy link

grizzm0 commented Jun 10, 2018

I had the same issues with FastRoute not picking up the values array a while back.

Can't remember how I found the solution, but I've simply replaced "values" with "defaults" and the route works just fine with FastRoute. Haven't tried it with Aura.Router tho.

@prolic
Copy link
Member

prolic commented Nov 26, 2018

No activity, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants