Better DI and preserve request object

[rickycheers]

Slim injects the dependency container every time a middleware is created from a class. This commit uses that instead of assuming that the $next callable is going to be an instance of \Slim\App.

Also, looks like there's no need to get the request object from the dependency container since the middleware is already receiving it as a parameter, otherwise any changes made to the PSR-7 request object are lost.

The above causes an issue for example when using Slim-Csrfbecause the request would no longer have "attributes" set by this middleware.

Hope it helps.

Regards!

[zeuxisoo]

Sorry, i cannot merge this PR, because i was created new commit to solve this issue by remove the re-declared $request variable.

Also, the $next variable is instance of Sim\App when middleware added. Ref:

1. Line 65
2. Line 93

About the container, it is based on Pimple by default. Reference to the documentation, it was said By default, each time you get a service, Pimple returns the same instance of it.

[rickycheers]

Yes, I understand that $next is going to be an instance of Slim\App, although, that is not always the case. If I'm not mistaken, that instance is only going to be passed to the first middleware in the stack; as per Line 93, the following values of $next are going to be either instances of a Closure or an instance of a class (\Zeuxisoo\Whoops\Provider\Slim\WhoopsMiddleware in this case)

Try the following example:

$app->add(function($req, $res, $next){
    return $next($req, $res);
});
$app->add(new \Zeuxisoo\Whoops\Provider\Slim\WhoopsMiddleware);
$app->get('/foo', function($req, $res, $args){
    $res->getBody()->write('Hello World!');
});

Error:

Fatal error: Call to undefined method Closure::getContainer() in \vendor\zeuxisoo\slim-whoops\src\Zeuxisoo\Whoops\Provider\Slim\WhoopsMiddleware.php on line 17

Slim binds the "container" instance to every closure so that you can access the container via $this inside the middleware. But again, that doesn't happen in this case, since the middleware is not a closure and rather is an invokable class.

One way to fix this issue would be to check if $next is an instance of a Slim\App or Slim\Container and get the container accordingly, but I don't think that is the best solution since, in those cases, Slim injects the dependency container when instantiating the class \Slim\App:109 and when the class is finally invoked DeferredCallable line 30 to 32

That's why I'm suggesting this pull request. The only difference would be that, we will have to pass the instance of the container when adding the middleware, or let Slim handle it for us if we send a string class that it can resolve.

Example:

$app->add(new \Zeuxisoo\Whoops\Provider\Slim\WhoopsMiddleware( $app->getContainer() ));
// or
$app->add('\Zeuxisoo\Whoops\Provider\Slim\WhoopsMiddleware');
// or
$app->add(\Zeuxisoo\Whoops\Provider\Slim\WhoopsMiddleware::class);

I hope it makes sense.

Regards!

[zeuxisoo]

Um, In the slim framework, the middleware is DeferredCallable object, so it will not trigger in the add method immediately.

Base on your first example, please reference to my closed issue, this whoops middleware must first added to the middleware stack. So, it will not got the exception of undefined method getContainer(). This step maybe a constraint. These are all problems left over by slim 2 -> 3 history, i don't want to make some changes currently. and I think the debugging tools should be in the first place.

[demaggus83]

@rickycheers is right - we need his patch.

The first middleware added is the last to be called - so if an exception happens in a middleware - there is no whoops support.

$app = new App(); $app->add(new WhoopsMiddleware()); $app->add(function ($request, $response, $next) { throw new Exception(); return $next($request, $response); });

[demaggus83]

omg

[zeuxisoo]

@stahlstift why you say omg ? Do you have read the commit log before post your comment ?