Should Slim 4's middleware be LIFO or FIFO?

[akrabat]

Slim 3 currently uses a Last In, First Out (LIFO) model for middleware.

That is:

$app->add('middleware3');
$app->add('middleware2');
$app->add('middleware1');

will execute the middleware in the order:

1. middleware1
2. middleware2
3. middleware3

This works if you imagine middleware as layers of an onion and adding a new middleware adds a new layer to the outside of the onion.

An alternative, would be to use First In, First Out (FIFO) model.

This is, the same PHP code would execute the middleware in this order:

1. middleware3
2. middleware2
3. middleware1

This works if you imagine middleware as a pipleline going from top to bottom where the first one you add is the first one that's executed, then the second one added is executed, etc.

Should Slim 4 change to the FIFO model?

[odan]

Definitely FIFO ("first in, first out") :-)

The code goes from top to bottom, so the middleware should also run in the same direction.

Reading the middleware settings would make it easier for the developer to read and maintain, as it corresponds to the mental and natural order of code and logic.

Showing code of a middleware stack in a training or presentation (slide) is better understood by everyone. Because everyone reads the code from above and can therefore follow the sequence better.

FIFO simplifies the writing of tests which have to consider the sequence of the stack. etc...

FIFO has become standard in the PHP community and in all other middleware based frameworks (see Zend Expressive).

[tflight]

With FIFO, which order would the middlewares run after $app? (It really doesn't matter much to me, I'm just curious.)

[bnf]

For technical reasons I would prefer LIFO. With FIFO Slim could not [1] implicitly build the recursive middleware stack when calling add (see MiddlewareAwareTrait) [2], but would need to cache the list of middlewares first, to create the stack (by reversing the cached middleware list) when the stack is executed.

While FIFO might be more convenient, I think LIFO fits better for Slim – well, because it should be slim.

[1] Switching the middleware implementation to a queue based one (where each middleware's $next points back to the Middleware Dispatcher which then calls the next middleware) would allow to keep the implicit building functionality, but requires managing a middleware index (I personally prefer the current implementation) [2] A middleware needs have a reference to the next middleware. In FIFO the next middleware is not yet known when calling App:add.

[SadeghPM]

FIFO mode is more natural and Zend expressive also using FIFO mode.

[aurmil]

FIFO is more natural when coding but I vote for the lightest solution (see @bnf comment above), Slim should stay as simplest / lightest / fastest as possible

[1ma]

The current LIFO model certainly adds some cognitive overhead for me.

[jupitern]

FIFO. More intuitive for me.

[rbairwell]

FIFO for the intuitive point, but ideally (and I know this may be outside scope), an optional priority/"before" option (ie 3rd party code may register authentication middle ware first, but API design needs CORs headers checked before authentication).

[hedii]

FIFO is definitely the more intuitive, less confusing.

[svpernova09]

Would prefer FIFO as more intuitive (like others have said) also easier to parse and understand when looking at it because you can change the order of $app->add(...) to change the order they're applied.

[trendschau]

FIFO for me, too, because it feels more natural, if you don't eat onions every day. I remember that I struggled with LIFO because i didn't read the Slim3-documentation carefully enough.

[richardjh]

I know it is LIFO so I work with it as it is and that is fine for me. Perhaps what is most important is that it is really clear in the docs what is used. Personally when I started I expected FIFO and only realized I misunderstood when my tests failed.

[theodorejb]

Either LIFO or FIFO is fine with me as long as it is clearly documented. I would be in favor of whichever one has the simplest implementation, to keep Slim as slim as possible. 😃

[mathmarques]

I prefer the LIFO approach, but if we change to FIFO's way, we could find an easy way to add an setting for that.

[richardjh]

+1 for @theodorejb "keep Slim as slim as possible".

[adigaaa]

I prefer FIFO.

[mrailton]

Throwing in my €0.02, I personally think FIFO makes more sense for reading as you can take a 'top down' approach

[geggleto]

We could solve the issue by allowing the developer to set with FIFO/LIFO

It would make the migration path easier if we decide to default it to FIFO.

[teoria]

FIFO by default !

[brayniverse]

I prefer FIFO myself.

[sunchuo]

FIFO set $this->container->get('settings')['middlewareFifo'] = true;

Slim/MiddlewareAwareTrait.php

<?php
/**
 * Slim Framework (https://slimframework.com)
 *
 * @see      https://github.com/slimphp/Slim
 *
 * @copyright Copyright (c) 2011-2017 Josh Lockhart
 * @license   https://github.com/slimphp/Slim/blob/3.x/LICENSE.md (MIT License)
 */

namespace Slim;

use RuntimeException;
use Psr\Http\Message\ServerRequestInterface;
use Psr\Http\Message\ResponseInterface;
use UnexpectedValueException;

/**
 * Middleware
 *
 * This is an internal class that enables concentric middleware layers. This
 * class is an implementation detail and is used only inside of the Slim
 * application; it is not visible to—and should not be used by—end users
 */
trait MiddlewareAwareTrait
{
    /**
     * middleware call stack
     *
     * @var callable
     */
    protected $stack = [];

    /**
     * Middleware stack lock
     *
     * @var bool
     */
    protected $middlewareLock = false;

    /**
     * Add middleware
     *
     * This method prepends new middleware to the application middleware stack
     *
     * @param callable $callable Any callable that accepts three arguments:
     *                           1. A Request object
     *                           2. A Response object
     *                           3. A "next" middleware callable
     *
     * @return static
     *
     * @throws RuntimeException         If middleware is added while the stack is dequeuing
     * @throws UnexpectedValueException If the middleware doesn't return a Psr\Http\Message\ResponseInterface
     */
    protected function addMiddleware(callable $callable)
    {
        if ($this->middlewareLock) {
            throw new RuntimeException('Middleware can’t be added once the stack is dequeuing');
        }

        if (empty($this->stack)) {
            $this->seedMiddlewareStack();
        }

        $this->stack[] = $callable;

        return $this;
    }

    /**
     * Seed middleware stack with first callable
     *
     * @param callable $kernel The last item to run as middleware
     *
     * @throws RuntimeException if the stack is seeded more than once
     */
    protected function seedMiddlewareStack(callable $kernel = null)
    {
        if (!empty($this->stack)) {
            throw new RuntimeException('MiddlewareStack can only be seeded once.');
        }
        if (null === $kernel) {
            $kernel = $this;
        }
        $this->stack[] = $kernel;
    }

    protected function prepareStack()
    {
        $handler = array_shift($this->stack);
        if (!empty($this->stack)) {
            if (isset($this->container) && isset($this->container->get('settings')['middlewareFifo'])
                && $this->container->get('settings')['middlewareFifo']) {
                $this->stack = array_reverse($this->stack);
            }

            foreach ($this->stack as $callable) {
                $next = $handler;
                $handler = function (
                    ServerRequestInterface $request,
                    ResponseInterface $response
                ) use (
                    $callable,
                    $next
                ) {
                    $result = call_user_func($callable, $request, $response, $next);
                    if (false === $result instanceof ResponseInterface) {
                        throw new UnexpectedValueException(
                            'Middleware must return instance of \Psr\Http\Message\ResponseInterface'
                        );
                    }

                    return $result;
                };
            }
        }

        return $handler;
    }

    /**
     * Call middleware stack
     *
     * @param ServerRequestInterface $request  A request object
     * @param ResponseInterface      $response A response object
     *
     * @return ResponseInterface
     */
    public function callMiddlewareStack(ServerRequestInterface $request, ResponseInterface $response)
    {
        if (empty($this->stack)) {
            $this->seedMiddlewareStack();
        }
        /** @var callable $stack */
        $stack = $this->prepareStack();
        $this->middlewareLock = true;
        $response = $stack($request, $response);
        $this->middlewareLock = false;

        return $response;
    }
}

4.6.2018 update.

tested!!!!

[root@localhost Slim]# php vendor/bin/phpunit UnitTest ./tests/MiddlewareAwareTest.php
PHPUnit 4.8.36 by Sebastian Bergmann and contributors.

.......

Time: 603 ms, Memory: 4.00MB

OK (7 tests, 7 assertions)

[hkodavid]

I prefer LIFO (3.x style) at default, the onion model is very spectacular and logic.

[RyanNerd]

I like @sunchuo 's approach. Have a default but let the user configure this via settings. When I first started trying to use Slim 3.x I really had a time with getting my head wrapped around LIFE (Last In First Executed). Having a choice would be a nice addition to the framework.

[theodorejb]

Making it a configurable option may sound nice, but it would make the code even more complex and also make it harder to contribute to other projects that use Slim, because it wouldn't be clear whether their middleware is LIFO or FIFO without looking at their configuration settings.

[RyanNerd]

@theodorejb This is why I said to have a default stack method -- but allow the user to define the stack methodology. @sunchuo 's code is untested but if it works and if this is the only needed change to the code then this is a simple change. If allowing user configuration for the stack is a complicated change then I see your point and completely agree with you. My experience with the stack code is that @sunchuo 's code is the only place that would need to change.

[ellotheth]

I like onion-style LIFO because it seems more consistent with the $app->group()->add() construction, where ->group() becomes the innermost middleware. Also I like onions.

[matthewtrask]

I like the onion style as well.

But I also think @geggleto has a point, although that could be entirely dependent on amount of work required.

[juliangut]

FWIW I'm used to and like LIFO

[xymz]

I thought LIFO is better. cuz It covers some contextual problems. For instance, once some middlewares were enabled, they could affect each same global context, like a monkey-patch(yes, not a good design). And FIFO doesn't guarantee same contextual environment. (e.g If B middleware performs only A was enabled, it gonna be crushed on FIFO)

[sunchuo]

@RyanNerd @theodorejb @ellotheth @matthewtrask

sorry for post untested code. there are some stupid errors. all were fixed. now it works well and passed the ./tests/MiddlewareAwareTest.php sorry for have no time to do better.

2423

[Machigatta]

I like it FIFO style. But i feel like the developer should be able to decide which way he want's to organize his code and middleware. The default should stand on LIFO, just to have lesser migration works for older-projects. (I'm just lazy and don't want to check back on some parts of my code 😆 )

[mariusklocke]

I think it should not be configurable. It would add unnecessary complexity, where clear documentation would solve the problem. If we speak about a "middleware stack", the word "stack" already tells me it is LIFO. So I suggest keeping LIFO.

[citadelnetworks]

I would say go with FIFO. I've just begun using Slim3 as a framework and at first I didn't realize the LIFO model and then when writing my own middleware and wondering why the second middleware wasn't getting the correct attributes, I realized the LIFO model. But by reading the code, I assume you read come from top to bottom, FIFO would make more sense.

Don't know if this is possible, but if we can set LIFO or FIFO as the default, and have an option when start the $app to change it to the other.

So if it's LIFO by default, you can change toe FIFO; vice versa.

[ghost]

Whichever way is chosen we must be mindful to name things properly:

• LIFO = Stack
• FIFO = Queue

[JustSteveKing]

FIFO would be beneficial for readability, also if the adding was extended to accept arrays - readability would be key.

LIFO would make sense on the after hook, FIFO makes sense on the before hook. But hey - smarter minds and the collective will prevail

[designermonkey]

If we decided to go the Queue route (which I and my alter ego @johnrobertporter think so) I think it would open up a great way of handling middleware that is not available to us yet.

A Queue could be altered during runtime to add or remove middleware further along the Queue.

I think this would be a powerful approach for route handling and route middleware, by simply appending them to the current middleware queue. This would mean a change in the signature for a route callback however.

I never really understood the need for passing args in a callback, and it means we could make all aspects of the application into middleware, instead of having separate signatures for separate aspects.

I am happy to discuss this idea further with anyone who's interested :)

[luispabon]

As a user I always found middlewares being LIFO something I tend to forget about from one project to the next. It's counter intuitive IMO.

[auroraeosrose]

I know we want slim to be - well - slim But a queue is different from a stack And you have to factor migration pain into this

I'd argue the middleware iterator should be injectable and match an interface Provide a default implementation of queue and stack (FIFO and LIFO) pick one as a default ( I don't really care, just name it right and document the heck out of it - I'd mildly recommend a script to help with migration that recommends the BC version perhaps? )

With that you open up

1. The default case works as expected out of the box with no configuration
2. the second most common use case works out of the box with simple configuration
3. Really complex cases become available by plugging in a custom implementation

My 2 cents :)

[sinitsa]

And on top of that all, async queue implementation option could be very useful, i think) ...at least in 5.x)

[unguul]

Should Slim 4 change to the FIFO model?

No.

Should Slim 4's middleware be LIFO or FIFO?

Neither.

I'd make it configurable defaulting to LIFO for an easier migration path.

I find @auroraeosrose 's suggestion to be the best approach.

[johnrhunt]

100% FIFO. We implemented something to reverse the reversed order ourselves as backwards makes no sense to laypeople like myself. But yeah, something that's changeable would be good too.

[RyanNerd]

I rescind my original vote for this to be configurable and instead vote to follow the standard set in Slim 3 (stack based LIFO/LIFE middleware)

I've changed my opinion on this and agree with @theodorejb that Slim 4 should be stack based (Last In First Executed). I've looked into other frameworks supporting middleware such as React's Mobx and Redux and these all appear to be stack based (at least by default). By making this configurable it will add unneeded complexity and litter Slim 4 with if (configuration==='stack') then [stack based code] else [queue based logic] code blocks.

[l0gicgate]

Order remains LIFO like it was in Slim 3.x. Closing as we just merged #2555