I've done my best to research the documentation or another issue that clarifies this, and I apologize if there is something that provides clarity to this.
Using the LIFO ( last in first out ) approach, my thought would be when executing the following:
class Middleware_Test
{
protected $_name;
public function __construct($name)
{
$this->_name = $name;
}
public function __invoke(\Slim\Http\Request $request, \Slim\Http\Response $response, $next)
{
print "\nHELLO FROM {$this->_name} Middleware";
$response = $next($request, $response);
return $response;
}
}
require "../vendor/autoload.php";
$app = new \Slim\App();
$app->add(new Middleware_Test("Middleware1"));
$app->add(new Middleware_Test("Middleware2"));
$app->add(new Middleware_Test("Middleware3"));
$app->group("/some/path", function () {
$this->get("/{testing}", function ($request, $response, $args) {
return $response->write("Middleware Test Response");
})->add(new Middleware_Test("Middleware6"))->add(new Middleware_Test("Middleware7"));
})->add(new Middleware_Test("Middleware4"))->add(new Middleware_Test("Middleware5"));
$app->run();
That the order of execution would be:
HELLO FROM Middleware7 Middleware
HELLO FROM Middleware6 Middleware
HELLO FROM Middleware5 Middleware
HELLO FROM Middleware4 Middleware
HELLO FROM Middleware3 Middleware
HELLO FROM Middleware2 Middleware
HELLO FROM Middleware1 Middleware
The logic being that the sequence is following what I believe to be the declaration and ultimately the application's logic.
Middleware1/Middleware2 are added before any routes are set, before any routing logic was processed.
Then, as FastRoute matches the group the groups Middleware is added, and finally the matching route's middleware inside the group gets added.
Instead the order is:
HELLO FROM Middleware3 Middleware
HELLO FROM Middleware2 Middleware
HELLO FROM Middleware1 Middleware
HELLO FROM Middleware5 Middleware
HELLO FROM Middleware4 Middleware
HELLO FROM Middleware7 Middleware
HELLO FROM Middleware6 Middleware
I'm having some difficulty following the logic of the order. It's the LIFO of the application, then the group specific LIFO ( if applicable ), then the route specific LIFO.
I could /possibly/ see if the order was 5,4,7,6,3,2,1 that the order of the group and specific route were just swapped, but still ahead of the application. However with the application, group and route all seemingly out of order from their declaration order, I'm a bit confused.
Provided this is expected, can someone provide the explanation on this order of execution?
This is the correct behavior, the reason being is that the group's path will be matched first before the final route's path. And then middleware ordering works the same while being nested (last in, first out).
This is the way it'll work:
/*
* These will all be executed prior to routing being done
* Unless you set `determineRouteBeforeAppMiddleware` to `true` in your App settings
* Then these would execute after routing in the same LIFO order (3, 2, 1) which means
* Middleware 5, 4, 7 and 6 would execute first and then Middleware 3, 2 and 1
*/
$app->add(new Middleware_Test("Middleware1"));
$app->add(new Middleware_Test("Middleware2"));
$app->add(new Middleware_Test("Middleware3"));
/*
* As for the middleware added here, as mentioned above the $app->group path will match first
* which will cause Middleware 5 and 4 to be executed
* Then the $this->get("/{testing}") path will be executed after which will cause
* Middleware 7 and 6 to be executed.
*/
$app->group("/some/path", function () {
$this->get("/{testing}", function ($request, $response, $args) {
return $response->write("Middleware Test Response");
})->add(new Middleware_Test("Middleware6"))->add(new Middleware_Test("Middleware7"));
})->add(new Middleware_Test("Middleware4"))->add(new Middleware_Test("Middleware5"));
Hope this answers your questions.
@l0gicgate , thanks for the clarification. Regarding:
These will all be executed prior to routing being done
- Unless you set
determineRouteBeforeAppMiddlewaretotruein your App settings
If you change the app declaration to :
$app = new \Slim\App(
array(
"settings" => array(
"determineRouteBeforeAppMiddleware" => true,
)
)
);
in the code I provided, it yields the same order ( 3,2,1,5,4,7,6 ). I'm testing with 3.9.2. Meaning, determineRouteBeforeAppMiddleware doesn't seem to have an impact.
determineRouteBeforeAppMiddleware = true is what I had set locally when initially running into this problem and I noticed that regardless of the value it yielded the same results, which is why I didn't included it in the original code. Sorry for not clarifying that.
Provided determineRouteBeforeAppMiddleware = true yielded the results you were describing, that would at least be closer to the experience I was expecting with the LIFO approach.
@conrad10781 you are correct actually. That setting does not affect how the middleware stack is being run.
I just wrote two test cases. My assumption was in fact correct though in the way the middleware is being executed, however I was wrong about the setting. All the setting does is provide the $request object with the attribute routeInfo and doesn't change the way the middleware stack is being run.
Here are the test cases I wrote and for the record the second test fails:
class Middleware
{
/**
* @var string
*/
protected $name;
/**
* Middleware constructor.
* @param $name
*/
public function __construct($name)
{
$this->name = $name;
}
public function __invoke(Request $request, Response $response, callable $next)
{
$text = $request->getAttribute('text', '');
$text .= "MW {$this->name}, ";
$request = $request->withAttribute('text', $text);
return $next($request, $response);
}
}
class MiddlewareOrderTest extends TestCase
{
public function testMiddlewareOrderWithDetermineRouteBeforeAppMiddlewareOn()
{
$app = new App([
'settings' => [
'determineRouteBeforeAppMiddleware' => true,
],
]);
$mw1 = new Middleware('1');
$mw2 = new Middleware('2');
$mw3 = new Middleware('3');
$app->add($mw1);
$app->add($mw2);
$app->add($mw3);
$mw4 = new Middleware('4');
$mw5 = new Middleware('5');
$mw6 = new Middleware('6');
$mw7 = new Middleware('7');
$app->group("/foo", function () use ($mw6, $mw7) {
$this->get("/bar", function (Request $request, Response $response, $args) {
$text = $request->getAttribute('text');
return $response->write($text);
})
->add($mw6)
->add($mw7);
})
->add($mw4)
->add($mw5);
$request = $this->requestFactory('/foo/bar');
$response = new Response();
$response = $app->process($request, $response);
$app->respond($response);
$this->expectOutputString("MW 3, MW 2, MW 1, MW 5, MW 4, MW 7, MW 6, ");
}
public function testMiddlewareOrderWithDetermineRouteBeforeAppMiddlewareOff()
{
$app = new App([
'settings' => [
'determineRouteBeforeAppMiddleware' => false,
],
]);
$mw1 = new Middleware('1');
$mw2 = new Middleware('2');
$mw3 = new Middleware('3');
$app->add($mw1);
$app->add($mw2);
$app->add($mw3);
$mw4 = new Middleware('4');
$mw5 = new Middleware('5');
$mw6 = new Middleware('6');
$mw7 = new Middleware('7');
$app->group("/foo", function () use ($mw6, $mw7) {
$this->get("/bar", function (Request $request, Response $response, $args) {
$text = $request->getAttribute('text');
return $response->write($text);
})
->add($mw6)
->add($mw7);
})
->add($mw4)
->add($mw5);
$request = $this->requestFactory('/foo/bar');
$response = new Response();
$response = $app->process($request, $response);
$app->respond($response);
$this->expectOutputString("MW 5, MW 4, MW 7, MW 6, MW 3, MW 2, MW 1, ");
}
/**
* @param $requestUri
* @param string $method
* @return Request
*/
public function requestFactory($requestUri, $method = 'GET')
{
$env = Environment::mock([
'SCRIPT_NAME' => '/index.php',
'REQUEST_URI' => $requestUri,
'REQUEST_METHOD' => $method,
]);
$uri = Uri::createFromEnvironment($env);
$headers = Headers::createFromEnvironment($env);
$cookies = [];
$serverParams = $env->all();
$body = new RequestBody();
$request = new Request($method, $uri, $headers, $cookies, $serverParams, $body);
return $request;
}
}
md5-ce8677744b37da9bd4be651bc7d270db
There was 1 failure:
1) Tests\MockRepo\AppTest::testMiddlewareOrderWithDetermineRouteBeforeAppMiddlewareOff
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'MW 5, MW 4, MW 7, MW 6, MW 3, MW 2, MW 1, '
+'MW 3, MW 2, MW 1, MW 5, MW 4, MW 7, MW 6, '
FAILURES!
Tests: 2, Assertions: 2, Failures: 1.
I wonder if we should change to FIFO for Slim 4?
If there is a vote, mine would be for FIFO by default.
I don't think it really creates a lot of issues after seeing the order in which Middleware is executed now in Slim 3.
For anyone migrating to Slim 4, it would still execute app, route group, route as it does now, just the order in which you ->add() Middleware only on those levels would change.
If it was changed to be more LIFO where it was route, route group, app, it seems to get a bit more hairy when considering users who are migrating because it's the same terminology but actually behaves different. Especially if that is relying on, or affected by the "determineRouteBeforeAppMiddleware" setting, or it's equivalent in Slim 4. Meaning, the idea that this setting could affect the order of Middleware execution seems like it would potentially create it's own issue.
So even if someone just read FIFO in the Slim 4 docs and thought "this changes everything". If they went back to Slim 3 and saw what was reported here, they would see it wasn't really that big of a change. As currently it's LIFO of FIFO declaration, and a change to FIFO would just be FIFO of FIFO declaration.
The last middleware layer added is the first to be executed.
I would be happy if the the order of the middleware stack changes in v4. I think the first middleware must be the first to be executed. That would just be more logical.
If we change the ordering now, migrating projects is going to be a bit more tricky. We will have to make sure we document the changes extensively.
@geggleto
If we change the ordering now, migrating projects is going to be a bit more tricky. We will have to make sure we document the changes extensively.
Please see https://github.com/slimphp/Slim/issues/2403#issuecomment-366967671 if you haven't already. I don't personally think this is going to be a huge change. Provided again it remains in the same order it does now of app, route group, route. The only change anyone would need is the ordering on those individual "levels", because currently it's LIFO of FIFO declaration, and a change to FIFO would just be FIFO of FIFO declaration. For me at least, this is more confusing.
I think that FIFO makes more sense, it's easier to comprehend for the end user. I also believe that Slim 4 is a new direction where we introduce new concepts and do things differently. There are already breaking changes, we should steer in the direction that makes the most sense. For the handful of users that will migrate entire projects from 3 to 4, we can provide support.
@conrad10781 we can't change how it works in 3.x (it would be a backwards compatibility break) however we can in 4.
Going to close this one as resolved. We can continue the discussions in the other issue about LIFO vs FIFO.
@geggleto , sorry for the confusion. I wasn't implying to change it for 3, just that if it changed in 4, despite it sounding like a huge change, it really wouldn't be if the declaration hierarchy order ( App, Route Group, Route ) stayed the same. Because as it is, it's LIFO of FIFO declaration, which honestly didn't match the expectation I had at least when reading the documentation and implementing.
Do you confirm that there is no way to have a valid LIFO (or FIFO) following the declaration order? As far as I know the current fixed order is:
Application MW LIFO, then Group LIFO, then Route LIFO.
Is there any way to follow the declaration order instead of general MW always before the router or group MW? thanks
@tobiascapin the order is fixed LIFO for everything in both Slim 3 and Slim 4.
Unless you rewrite the MiddlewareAwareTrait in Slim 3 and most of the App鈥檚 core you won鈥檛 be able to change that.
As for Slim 4 you鈥檇 need to rewrite the MiddlewareDispatcher component and the App鈥檚 constructor to use your rewritten dispatcher to make it possible.