RFC: Expressive 3 Design Changes

psr-15
expressive

#1

zend-stratigility has had a number of refactors put into place for its upcoming version 3. Among these:

  • It will only accept PSR-15 MiddlewareInterface instances to MiddlewarePipe::pipe.
  • MiddlewarePipe::pipe no longer accepts a path argument.
  • The MiddlewarePipe itself has been marked final.
  • It now provides a number of decorators:
    • PathMiddlewareDecorator allows providing path-segregated middleware, and accepts a string path and a MiddlewareInterface middleware to its constructor. A path() utility function acts as a convenience factory for these instances.
    • CallableMiddlewareDecorator allows decorating callable middleware that follows the PSR-15 signature, and accepts the middleware to its constructor. The utility function middleware() acts as a factory for the decorator.
    • DoublePassMiddlewareDecorator allows decorating callable middleware that follows the double-pass signature, and accepts the middleware to its constructor, as well as a ResponseInterface. The utility function doublePassMiddleware() acts as a factory for the decorator.

With these changes, we began to rethink some of the architecture for the zend-expressive package, and have come up with the following.

Proposal for version 3

  • Pin to the Stratigility 3.0.0 release, which will have the PSR-15 features and new architecture.
  • The Application class will now implement the PSR-15 MiddlewareInterface and RequestHandlerInterface, and compose a MiddlewarePipe instance internally.
  • Application::pipe() will accept only a $middleware argument, and no longer do path segregation directly.
  • The $middleware argument to both pipe() and route() (and the various routing-related methods) will allow the same arguments as v2, except they will no longer allow callable double-pass middleware.
  • Routing and dispatch middleware will be services; no more pipeRoutingMiddleware() or pipeDispatchMiddleware(). Just pipe the services.
  • We will remove the MarshalMiddlewareTrait, and Application and other classes will no longer use it. More on this below.
  • We will add two new classes:
    • MiddlewareContainer will implement the PSR-11 ContainerInterface, and decorate a PSR-11 container. Internally, it will:
      • resolve service names that are not registered in the container but resolve to classes
      • raise an exception if the service pulled from the composed container is not a MiddlewareInterface instance
    • MiddlewareFactory will compose a MiddlewareContainer and provide methods for decorating middleware:
      • callable(callable $middleware) : CallableMiddlewareDecorator
      • `lazy(string $middleware) : LazyLoadingMiddleware
      • prepare($middleware) : MiddlewareInterface will accept a string, callable, array, or MiddlewareInterface and return a MiddlewareInterface based on the argument type.
      • pipeline(...$middleware) : MiddlewarePipe will create a MiddlewarePipe, piping each argument after first passing it to prepare().
      • path(string $path, $middleware) : PathMiddlewareDecorator will create a PathMiddlewareDecorator after first passing $middleware to prepare().
  • Application will create a MiddlewareFactory from the container it composes, and use that container to resolve all middleware passed to pipe() or one of the routing methods.
  • Application will expose the composed MiddlewareFactory via a new method, getMiddlewareFactory().

Double pass middleware MUST be passed to pipe() or one of the routing methods by first decorating it in a DoublePassMiddlewareDecorator instance (either via direct instantiation, or using the doublePassMiddleware() utility function).

By exposing the factory, users can create “utility functions” by fetching the factory and creating callables.

Examples:

// In pipeline.php:
$factory = $app->getMiddlewareFactory();
$path = [$factory, 'path'];
$pipeline = [$factory, 'pipeline'];

// Pondering renaming `ErrorHandler` to `ErrorMiddleware` to disambiguate
// the name.
$app->pipe(ErrorMiddleware::class);
$app->pipe(ServerUrlMiddleware::class);

// Here's path-segregated middleware:
$app->pipe($path('/auth', OAuth2CallbackMiddleware::class));

// This one combines path-segregated with pipeline middleware, while still using
// v2 semantics for the pipeline:
$app->pipe($path('/api', [
    ProblemDetailsMiddleware::class,
    CorsMiddleware::class,
    SessionMiddleware::class,
    AuthenticationMiddleware::class,
    AuthorizationMiddleware::class,
]);

// The above could be done more explicitly as:
$app->pipe($path('/api', $pipeline(
    ProblemDetailsMiddleware::class,
    CorsMiddleware::class,
    SessionMiddleware::class,
    AuthenticationMiddleware::class,
    AuthorizationMiddleware::class // Note: no trailing slash; it's an argument!
));


// The routing middleware now becomes a first-class service instead:
$app->pipe(RouteMiddleware::class);

$app->pipe(ImplicitHeadMiddleware::class);
$app->pipe(ImplicitOptionsMiddleware::class);
$app->pipe(UrlHelperMiddleware::class);

// Dispatch middleware also is now a first-class service:
$app->pipe(DispatchMiddleware::class);

// Pondering renaming `NotFoundHandler` to `NotFoundMiddleware` to disambiguate
// the name.
$app->pipe(NotFoundMiddleware::class);

// In routes.php:
$uuidRegex = '. . .';
$factory = $app->getMiddlewareFactory();
$path = [$factory, 'path'];
$pipeline = [$factory, 'pipeline'];

$app->get('/', HomePageMiddleware::class, 'home');

// V2 standards continue to work:
$app->get('/auth/status', [
    ProblemDetailsMiddleware::class,
    CorsMiddleware::class,
    SessionMiddleware::class,
    AuthenticationMiddleware::class,
    LoginStatusHandler::class,
], 'auth.status');

// Or, more explicitly, using the pipeline factory:
$app->get('/auth/status', $pipeline(
    ProblemDetailsMiddleware::class,
    CorsMiddleware::class,
    SessionMiddleware::class,
    AuthenticationMiddleware::class,
    LoginStatusHandler::class
), 'auth.status');

$app->get('/api/books', SearchMiddleware::class, 'books');
$app->post('/api/books', [
    BodyParamsMiddleware::class,
    AddBookValidationMiddleware::class,
    AddBookMiddleware::class,
]);

$app->get('/api/books/{id:' . $uuidRegex . '}', BookMiddleware::class, 'book');
$app->patch('/api/books/{id:' . $uuidRegex . '}', [
    BodyParamsMiddleware::class,
    UpdateBookValidationMiddleware::class,
    UpdateBookMiddleware::class
]);

Proposal for version 2.2.0

  • Pin to the Stratigility 2.2.0 release, which will have the forwards compatibilty features.
  • Backport the MiddlewareFactory and MiddlewareContainer, but update the MiddlewareFactory to differentiate between interop and double-pass middleware. When decorating double-pass middleware, emit an E_USER_DEPRECATED, urging developers to decorate using doublePassMiddleware() or DoublePassMiddlewareDecorator instances. The factory will also require a ResponseInterface instance in order to generate DoublePassMiddlewareDecorator instances.
  • Update Application to use the MiddlewareFactory for marshaling middleware.
  • Continue accepting the path argument to Application::pipe, but:
    • trigger an E_USER_DEPRECATED urging developers to use path() or the getMiddlewareFactory()->path() functionality.
    • Use the MiddlewareFactory to decorate the path and middleware before piping it.
  • Continue accepting the pipeRoutingMiddleware() method, but:
    • Provide a factory for the routing middleware that consumes the same RouterInterface service, and a ResponseInterface service.
    • trigger an E_USER_DEPRECATED urging developers to simply pipe the RouteMiddleware service.
    • pull the RouteMiddleware service from the container and pipe it.
  • Continue accepting the pipeDispatchMiddleware() method, but:
    • Provide a factory for the routing middleware that consumes the same RouterInterface service, a ResponseInterface service, and the container itself. (In v3, it would only accept the container.)
    • trigger an E_USER_DEPRECATED urging developers to simply pipe the DispatchMiddleware service.
    • pull the DispatchMiddleware service from the container and pipe it.
  • Update LazyLoadingMiddleware:
    • Create and use a MiddlewareFactory internally instead of using the MarshalMiddlewareTrait.
    • Mark the ResponseInterface constructor argument as deprecated.
  • Mark the MarshalMiddlewareTrait as deprecated, and modify it to use the MiddlewareFactory instead.
  • Mark the IsCallableInteropMiddlewareTrait as deprecated.

History

  • 2018-01-18: Initial creation
  • 2018-01-24: Updated to remove path/pipeline decorator creation methods and instead propose the MiddlewareFactory/MiddlewareContainer combination, and indicate all previous types except double-pass middleware will continue to be directly supported.

#2

What was the reason for the PathDecorator and PipelineDecorator? 80% less code if I remember correctly?
With those 2 in place, does the config injection of routes and the pipeline still work?

I’m all in favor if this reduces code.

That’s it for now. I’ll read it more thoroughly tomorrow.


#3

The reason for them is to allow specifying middleware using either a service name or a MiddlewareInterface instance. In the former case, for this to work, we need to either pull the middleware from the container, or create a LazyLoadingMiddleware instance, which also needs a container. These methods create and return closures, which allow us to do that, without the user needing to know that specific detail, or do the work themselves. It’s essentially a convenience mechanism.

In terms of configuration-driven routes/pipeline, the ApplicationConfigInjectionTrait will also be updated. The main changes it needs are:

  • To pipe the routing and dispatch middleware via their service names.
  • If a path is specified in a middleware pipeline entry, it needs to use the path decorator.
  • If a middleware pipeline entry specifies an array of middleware, it needs to use the pipeline decorator.

(I’m actually working on exactly that in a prototype right now.)

So far, I’ve been able to eliminate the MarshalMiddlewareTrait entirely, and the MiddlewareContainer is a matter of only around 10 actual lines of code, so it’s definitely a net gain, and we get type safety as a result.


#4

I have concerns about it, because this class implements two interfaces, and probably we are going to typehint with this class. If so it could make testing difficult, because it is not possible to mock final classes:

  • prophecy:

    Could not reflect class Zend\Stratigility\MiddlewarePipe as it is marked final.

  • PHPUnit:

    Class “Zend\Stratigility\MiddlewarePipe” is declared “final” and cannot be mocked.

Maybe then we should create interface consistent with public interface of MiddlewarePipe and use this typehint instead?

Inspired by PR https://github.com/zendframework/zend-expressive/pull/370 I’ve created one more decorator HostMiddlewareDecorator https://github.com/zendframework/zend-stratigility/pull/142 to segregate middlewares by host. Because of this decorator, maybe we would need to add also Application::getHostDecorator():

$host = $app->getHostDecorator();

$app->pipe($host('example.com', ExampleMiddleware::class));

// It should be also possible to use more than one decorator
// when we want to have middleware only for `example.com/path`:
$app->pipe($path('/path', $host('example.com', MyMiddleware::class)));

Do we need it? Maybe we should allow MiddlewareInterface, string or array of these types in methods pipe/get/post/etc and do it internally? What is the advantage to have it out of the application?

@matthew I have another question: what convinced you tu use function (path, middleware, doublePassMiddleware) as you seemed to be against it at the beginning (one of the reason I remember: “no function autoload in PHP”).
Probably I’ve missed some discussion about it when I was on my holidays, so if you could point me out place where you’ve discussed it.

TBH I can’t see any big advantages of having them, as these are just proxies to the decorator classes, and we need to load more files. We can do ‘almost’ the same and also quite short notation without these functions:

$path = PathMiddlewareDecorator::class;

$pipe->pipe(new $path('/path', $middleware));
$pipe->pipe(new $path('/foo', $fooMiddleware));

Of course I don’t need to use them, but these are going to be always loaded.

For now that it. In general Stratigility v3 looks very nice :+1: :heart:


#5

This is not easily done, unfortunately. If we require that the signature of MiddlewarePipe::pipe() only accept a MiddlewareInterface, we cannot overload it in Application to accept either a MiddlewareInterface or string service name.

Within Expressive, why would marking MiddlewarePipe be an issue, exactly? It’s an internal implementation detail; we’re not injecting the instance, we’re creating one internally to which to pipe middleware, and to which we proxy when processing/handling the application. From the work I’ve been doing so far, I’ve not run into any issues.

Knowing this, and seeing your ideas here, I’m thinking it may make sense to move the middleware marshaling aspect into a separate collaborator, which we can then fetch in order to create these factory functions and/or the specific middleware they each create. I’ll work on a design for that today.

The advantage is that it reduces internal complexity. In the branch I’m working on currently, I was essentially able to remove the MarshalMiddlewareTrait entirely, and replace it with a combination of:

  • the MiddlewareContainer, which essentially only worries about whether a service resolves to an invokable class if it is not currently registered in the container; this is something like 10 lines of code.
  • a method in Application that verifies a middleware is either a string name or a MiddlewareInterface.
  • the path and pipeline factory methods, which are around 10 lines of code between the two of them.

pipe() becomes something like 3 or 4 lines of code. Code is also reduced in route(). And the number of paths in both is radically reduced. Additionally, the signatures are far simpler; instead of having to remember that you can pass a string, a callable interop middleware, a callable double-pass middleware, a MiddlewareInterface implementation, or an array of any of these, it’s just a string or a MiddlewareInterface implementation. If you want to do anything else, you use one of the factory functions or directly create the decorator instances.

While there is no true function autoloading in PHP, Composer gives it to us via files autoload directives. Further, in production, these are essentially inlined into the generated autoloader, meaning there is no additional I/O for them.

I’ve started investigating them for a few reasons. First, they provide convenience around class instantiation. yes, we can import the decorator classes and directly instantiate, but it’s harder to remember PathMiddlewareDecorator (and the namespace in which it lives!) than it is to remember path().

Second, middleware is a very functional paradigm, in the meaning of functional programming. These will perform the same way each time they are run, and have no side effects. For many people, they don’t need to know that the class being used is a decorator, they only need to know that it will transform something that isn’t currently middleware into something that is. This is conveyed elegantly with a function signature.

Finally, in the case of the returned factory closures from Application, it allows us to bind the factories to internal functionality. In this particular case, it’s ensuring we use the composed MiddlewareContainer within these factories in order to ensure the “middleware” passed to the function is valid for the decorator class. Otherwise, we end up with weird syntax like:

$pipeline->pipe(new PathMiddlewareDecorator(
    '/path',
    $pipeline->getMiddlewareDecorator()->marshalMiddleware($middleware)
));

Being able to hide the internals makes the functionality more usable for developers. Considering that usability is a selling point for middleware, anything we can do to make composing middleware of different types easier is just another step in the same direction.

For those that need more control or who want to handle things more explicitly, they’ll still have that option; these functions are for convenience, particularly when you have no reason to handle them differently than we would.


#6

I think you didn’t understand me correctly. I’ve meant that we deliver final class in Stratigility without abstraction.
I don’t want Application to extend MiddlewarePipe, we discuss it before, and I know we can’t do it anymore, because we’d like to have middleware lazy loading.

As mentioned above, my point was that we are marking class as final but we are not delivering abstraction. I understand now how you’d like to use it, that it will be not passed into constructor but composed internally, but in another case we can have public interface with MiddlewarePipe typehint and it could make testing difficult.
Let me quote @ocramius:

Final classes only work effectively under following assumptions:

  • There is an abstraction (interface) that the final class implements
  • All of the public API of the final class is part of that interface

If one of these two pre-conditions is missing, then you will likely reach a point in time when you will make the class extensible, as your code is not truly relying on abstractions.

https://ocramius.github.io/blog/when-to-declare-classes-final/

I need to see then your code proposal, but still I think it will be better to have MiddlewareInterface, string or array of these and keep getPipelineDecorator only internally, to simplify it for developers.

I didn’t know about it. Thanks for all explanations around functions :smiley:


#7

@matthew I really like the idea to remove the path from MiddlewarePipe::pipe and change the routing and dispatch as services, without the pipeRoutingMiddleware() and pipeDispatchMiddleware() calls. I think these changes will simplify the Expressive usage.

That said, I would like to simplify the route part that you proposed, without the usage of the $pipeline decorator. I think we should try to don’t change the route.php file and continue to support the old syntax.
I see the usage of decorators App::getPipelineDecorator and App::getPathDecorator useful to migrate from Expressive 2 to 3 but not in the Expressive 3 skeleton project. I would like to simplify and remove code as much as possible in version 3.


#8

I was thinking the same. Instead of having various $app->get*Decorator(), we could have:

// This factory encapsulates the marshaling logic
$lazy = $app->getLazyMiddlewareFactory();

// Then Expressive can typehint against MiddlewareInterface only
$app->pipe($lazy(ErrorMiddleware::class));

// And we can can use `path()` and `host()` directly from Stratigility
$app->pipe(host('api.example.com', $lazy(AuthenticationMiddleware::class)));
$app->pipe(path('admin', $lazy(AuthorizationMiddleware::class)));

// As well as any other further decorator or custom userland decorators:
$app->pipe(isAuthenticated($lazy(AccessLogMiddleware::class)));

While this increases flexibility in 9000%, it also becomes a bit more verbose. So I’m not sure if it is a good tradeoff :thinking:

Thoughts?


#9

Honestly, I really like this approach. However, we need to consider usability, as well as migration from existing applications. That said, since we’re not extending the MiddlewarePipe in this proposal, we could potentially just pass any arguments to pipe() to a factory class like this and have it manage the complexity. Users could then do what you suggest above, or use existing syntax, but our examples and recommendations would be for the above.


#10

I’ve just worked on an experiment, and think we can accomplish both this, as well as address the feedback from @enrico and @webimpress pretty easily.

Essentially, I have a new class, MiddlewareFactory. It accepts a PSR-11 container, which it then decorates internally using MiddlewareContainer. This class contains a method, prepare(), which accepts all arguments the current v2 release allows minus double-pass middleware, and then delegates creation of a MiddlewareInterface instance to various other methods:

  • callable(callable $middleware) returns a CallableMiddlewareDecorator instance
  • lazy(string $middleware) returns a LazyLoadingMiddleware instance
  • pipeline(...$middleware) returns a MiddlewarePipe instance after first passing all arguments (or, if a single array is passed, all items in that array) to prepare()
  • path(string $path, $middleware) returns a PathMiddlewareDecorator instance after first passing $middleware to prepare().

Application::pipe() and Application::route() then each pass their $middleware arguments to $this->middlewareFactory->prepare().

I find several things interesting about this approach:

  • We can support essentially the same argument set that we do currently.
  • You can easily create utility functions:
    $factory = $app->getMiddlewareFactory();
    $pipeline = [$factory, 'pipeline'];
    $path = [$factory, 'path'];
    $lazy = [$factory, 'lazy'];
    
    $app->pipe($path('/api', $pipeline($cors, $authentication, $authorization))));
    $app->post('/api/books', $pipeline($validator, CreateBookHandler::class));
    
  • You could easily extend the factory to add other types; just pass the container to your custom factory:
    $factory = new CustomMiddlewareFactory($app->getContainer());
    
  • As you note, a user could use some of the existing utility functions or any of their own by using the factory to handle middleware marshaling:
    $app->pipe(isAuthenticated($pipeline(CorsMiddleware::class, AccessLogMiddleware::class)));
    

I think this solution hits the sweet spot of providing ease of upgrading, along with future flexibility.

Thoughts, everyone?


#11

@matthew I like this new approach but I would like to see a complete skeleton for v3 in place. Again, I think the goal should be to simplify the API and continue to support the previous v2 syntax as much we can.


#12

The skeleton essentially remains the same as it currently is, with four changes that impact the pipeline.php and routes.php files:

  • pipeRoutingMiddleware() goes away, to be replaced by piping the routing middleware service directly. This is trivial.
  • pipeDispatchMiddleware() goes away, to be replaced by piping the dispatch middleware service directly. This is also trivial.
  • double-pass middleware is no longer directly supported, and requires that you decorate it in a DoublePassMiddlewareDecorator (which can be done by instantiating one directly, or using the doublePassMiddleware() utility function, both of which also require a response instance).
  • path segregation is no longer directly supported, and will require you decorate it in a PathMiddlewareDecorator instance. This can be handled by instantiating one directly, with your middleware instance, or by using the path() utility function (which simply proxies to the class constructor). As a measure to allow using service names and/or arrays of middleware (including service names), we would need to provide some sort of factory tied to the container. This could either be via backporting the MiddlewareFactory or having a dedicated getPathDecorator() method. I’m leaning towards the former, however, as it will help us reduce complexity in the v2 tree, and provide forwards-compatibility to v3 usage.

That means that two lines change specifically in the config/pipeline.php file:

$app->pipeRoutingMiddleware();
// becomes:
$app->pipe(RouteMiddleware::class);

// and:
$app->pipeDispatchMiddleware();
// becomes:
$app->pipe(DispatchMiddleware::class);

There would be other changes as well, but these would be in the docblocks, and would not be actual code changes. These include documenting what types are allowed for middleware, and how to do things like path segregation.

The config/routes.php file would have no changers at all currently.


#13

@matthew should we prepare then also migration tool for pipeline.php file as described above? What do you think?


#14

Yes, but it will be more limited in scope; see the changes to the spec that I just published. (Limited to double pass middleware and path segregated middleware.)


#15

Sure, first I’d like to see PR to expressive with all of these changes… Sometimes it’s hard to see it without code for me :smiley:


#16

I’ll be finalizing that this afternoon, hopefully!


#17

+1 to simplify for developers.

And about this:

Serious!?
Can I have a more simple use? Just like:

$app->pipePath('/auth', Middleware1::class, Middleware2::class, Middleware3::class);
// from:
public function pipePath(string $path, ...$middlewares) { } // Variable-length arguments

// or
$app->pipePath('/auth', [Middleware1::class, Middleware2::class]); // array style
$app->pipePath('/auth', Middleware1::class); // string
// from: 
public function pipePath(string $path, $middlewares) { }

#18

I really like the idea of @CarlosEduardo for a pipePath(), we can offer a simplified API that does the same of the previous pipe() of v2. The proposal of @matthew will also be possible, but most likely for advanced users.

I think we should use with the syntax:
pipe(string $path, string|array|callable $middleware)


#19

@CarlosEduardo and @enrico

Overall, I’d rather reduce the number of ways you interact with the Application instance, rather than add methods. This was one of the reasons I liked the idea of extracting the MiddlewareFactory, as it means a single “getter” method (getMiddlewareFactory()) vs multiple methods for various decorator factories. In point of fact, we could even remove it entirely, as the following is equivalent:

$factory = new MiddlewareFactory($app->getContainer());

As such, adding another method to Application to accomplish the same thing (piping middleware) is contrary to the idea of simplifying and reducing the public interface of Application.

(Honestly, at this point, I’d like to go even further and move the various routing functionalities to the RouteMiddleware — as that’s where they really belong — thus allowing us to reduce the footprint of Application further. But that would be an even more radical departure and introduce a larger BC break, which I’m largely trying to avoid.)

In terms of allowing the MiddlewareFactory::path() method to allow multiple middleware arguments, that’s likely doable, and I’ll see if I can make that happen without too much complexity today. But adding another method for piping to the Application class is a non-starter as far as I’m concerned.


#20

I completely agree with this. Adding more ways to accomplish the same isn’t making is simpler.

I would say go ahead with this. A major version is about to be released and it would be a good time to move things around and take the BC break.