thephpleague / route

Fast PSR-7 based routing and dispatch component including PSR-15 middleware, built on top of FastRoute.
http://route.thephpleague.com
MIT License
651 stars 126 forks source link

Feature request: make $request available in getOptionsCallable #304

Closed vixducis closed 3 years ago

vixducis commented 3 years ago

Lately, the getOptionsCallable was implemented as part of OptionsHandlerInterface (which is a pretty nice way to handle OPTIONS requests). However, I feel like it could be even better when the $request is passed on top of $methods. Some headers that need to be returned, like Access-Control-Allow-Origin and Access-Control-Allow-Headers depend on the request headers. This scenario currently needs some workaround by injecting the request into the Strategy, which is sub-ideal.

This seems pretty doable, I think, as getOptionsCallable is called by buildOptionsRoutes, which on it's turn is called by prepareRoutes, which has the $request parameter available.

philipobenito commented 3 years ago

Want to have a go at a PR? Try not to introduce a breaking change if possible, if that turns out to be quite difficult then we can discuss it.

Sent from ProtonMail for iOS

On Wed, Jul 14, 2021 at 19:19, vixducis @.***> wrote:

Lately, the getOptionsCallable was implemented as part of OptionsHandlerInterface (which is a pretty nice way to handle OPTIONS requests). However, I feel like it could be even better when the $request is passed on top $methods. Some headers that need to be returned, like Access-Control-Allow-Origin and Access-Control-Allow-Headers depend on the request headers. This scenario currently needs some workaround by injecting the request into the Strategy, which is sub-ideal.

This seems pretty doable, I think, as getOptionsCallable is called by buildOptionsRoutes, which on it's turn is called by prepareRoutes, which has the $request parameter available.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

philipobenito commented 3 years ago

I've just been playing around with things for 6.x and realised the callable returned from getOptionsCallable is already passed the request object, this is because it's already a controller, I imagine this isn't clear as the example in JsonStrategy doesn't define a $request argument since it doesn't need it.

I think renaming getOptionsCallable to getOptionsController will make things a little clearer with some better docs too.

Similarly, all the other strategy methods also already have access to the request object since the middleware they return is passed a request object when invoked.

So if in your getOptionsCallable method you define something like this, it'll work:

public function getOptionsCallable(): callable
{
    return function (ServerRequestInterface $request) {
        // do something with $request and return a response
    };
}

I'll close this as it is by design, sorry I didn't realise earlier.