From dedf7f591fe3b3c3493201e3e938e837f9a26bb2 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 20 Jun 2024 00:02:38 +0800 Subject: [PATCH 1/2] Improve ease of extention In our usage we need to extend the ControllerInvoker to have responses normalised from a generic Payload into a standard Response. To make this easier it would be helpful if some private methods were instead protected and the Invoker fetching the default resolver chain were separated out from the creation of the ControllerInvoker. Fixes #105 --- src/Bridge.php | 15 ++++++++++++--- src/ControllerInvoker.php | 21 ++++++++++++++++++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Bridge.php b/src/Bridge.php index 127cdc9..6ae28df 100644 --- a/src/Bridge.php +++ b/src/Bridge.php @@ -39,7 +39,10 @@ public static function create(?ContainerInterface $container = null): App return $app; } - private static function createControllerInvoker(ContainerInterface $container): ControllerInvoker + /** + * Create an invoker with the default resolvers. + */ + protected static function createInvoker(ContainerInterface $container): Invoker { $resolvers = [ // Inject parameters by name first @@ -50,8 +53,14 @@ private static function createControllerInvoker(ContainerInterface $container): new DefaultValueResolver, ]; - $invoker = new Invoker(new ResolverChain($resolvers), $container); + return new Invoker(new ResolverChain($resolvers), $container); + } - return new ControllerInvoker($invoker); + /** + * Create a controller invoker with the default resolvers. + */ + protected static function createControllerInvoker(ContainerInterface $container): ControllerInvoker + { + return new ControllerInvoker(self::createInvoker($container)); } } diff --git a/src/ControllerInvoker.php b/src/ControllerInvoker.php index bc1a632..98e114c 100644 --- a/src/ControllerInvoker.php +++ b/src/ControllerInvoker.php @@ -24,7 +24,6 @@ public function __construct(InvokerInterface $invoker) * @param ServerRequestInterface $request The request object. * @param ResponseInterface $response The response object. * @param array $routeArguments The route's placeholder arguments - * @return ResponseInterface|string The response from the callable. */ public function __invoke( callable $callable, @@ -42,10 +41,26 @@ public function __invoke( // Inject the attributes defined on the request $parameters += $request->getAttributes(); - return $this->invoker->call($callable, $parameters); + return $this->processResponse($this->invoker->call($callable, $parameters)); } - private static function injectRouteArguments(ServerRequestInterface $request, array $routeArguments): ServerRequestInterface + /** + * Allow for child classes to process the response. + * + * @param ResponseInterface|string $response The response from the callable. + * @return ResponseInterface|string The processed response + */ + protected function processResponse($response) + { + return $response; + } + + /** + * Inject route arguments into the request. + * + * @param array $routeArguments + */ + protected static function injectRouteArguments(ServerRequestInterface $request, array $routeArguments): ServerRequestInterface { $requestWithArgs = $request; foreach ($routeArguments as $key => $value) { From 60c7141acff3a38884b1b26efbff1bbad2585007 Mon Sep 17 00:00:00 2001 From: Andrew Nicols Date: Thu, 20 Jun 2024 00:31:23 +0800 Subject: [PATCH 2/2] Allow configuration of placeholder overriding attributes Fixes #86 --- src/Bridge.php | 12 +++++++----- src/ControllerInvoker.php | 18 ++++++++++++++---- tests/RoutingTest.php | 20 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/Bridge.php b/src/Bridge.php index 6ae28df..61dba1d 100644 --- a/src/Bridge.php +++ b/src/Bridge.php @@ -22,8 +22,10 @@ */ class Bridge { - public static function create(?ContainerInterface $container = null): App - { + public static function create( + ?ContainerInterface $container = null, + bool $prioritiseAttributesOverParams = false + ): App { $container = $container ?: new Container; $callableResolver = new InvokerCallableResolver($container); @@ -33,7 +35,7 @@ public static function create(?ContainerInterface $container = null): App $container->set(App::class, $app); - $controllerInvoker = static::createControllerInvoker($container); + $controllerInvoker = static::createControllerInvoker($container, $prioritiseAttributesOverParams); $app->getRouteCollector()->setDefaultInvocationStrategy($controllerInvoker); return $app; @@ -59,8 +61,8 @@ protected static function createInvoker(ContainerInterface $container): Invoker /** * Create a controller invoker with the default resolvers. */ - protected static function createControllerInvoker(ContainerInterface $container): ControllerInvoker + protected static function createControllerInvoker(ContainerInterface $container, bool $prioritiseAttributesOverParams): ControllerInvoker { - return new ControllerInvoker(self::createInvoker($container)); + return new ControllerInvoker(self::createInvoker($container), $prioritiseAttributesOverParams); } } diff --git a/src/ControllerInvoker.php b/src/ControllerInvoker.php index 98e114c..c8952cf 100644 --- a/src/ControllerInvoker.php +++ b/src/ControllerInvoker.php @@ -12,9 +12,13 @@ class ControllerInvoker implements InvocationStrategyInterface /** @var InvokerInterface */ private $invoker; - public function __construct(InvokerInterface $invoker) + /** @var bool Whether attributes should override parameters */ + protected $prioritiseAttributesOverParams; + + public function __construct(InvokerInterface $invoker, bool $prioritiseAttributesOverParams) { $this->invoker = $invoker; + $this->prioritiseAttributesOverParams = $prioritiseAttributesOverParams; } /** @@ -33,7 +37,7 @@ public function __invoke( ): ResponseInterface { // Inject the request and response by parameter name $parameters = [ - 'request' => self::injectRouteArguments($request, $routeArguments), + 'request' => self::injectRouteArguments($request, $routeArguments, $this->prioritiseAttributesOverParams), 'response' => $response, ]; // Inject the route arguments by name @@ -60,10 +64,16 @@ protected function processResponse($response) * * @param array $routeArguments */ - protected static function injectRouteArguments(ServerRequestInterface $request, array $routeArguments): ServerRequestInterface - { + protected static function injectRouteArguments( + ServerRequestInterface $request, + array $routeArguments, + bool $prioritiseAttributesOverParams + ): ServerRequestInterface { $requestWithArgs = $request; foreach ($routeArguments as $key => $value) { + if ($prioritiseAttributesOverParams && $request->getAttribute($key) !== null) { + continue; + } $requestWithArgs = $requestWithArgs->withAttribute($key, $value); } return $requestWithArgs; diff --git a/tests/RoutingTest.php b/tests/RoutingTest.php index 3fbf947..c850e76 100644 --- a/tests/RoutingTest.php +++ b/tests/RoutingTest.php @@ -127,6 +127,26 @@ public function injects_route_placeholder_over_request_attribute() $this->assertEquals('Hello matt', (string) $response->getBody()); } + /** + * @test + */ + public function prefers_request_attribute_over_route_placeholder_if_configured() + { + $app = Bridge::create(null, true); + $app->add(function (ServerRequestInterface $request, RequestHandlerInterface $next) { + return $next->handle($request->withAttribute('name', 'Bob')); + }); + $app->get('/{name}', function ($name, $request, $response) { + $response->getBody()->write('Hello ' . $name . ', from ' . $request->getAttribute('name')); + return $response; + }); + + $response = $app->handle(RequestFactory::create('/matt')); + + // The route placeholder does not override the request attribute but the placeholder is still provided to the controller + $this->assertEquals('Hello matt, from Bob', (string) $response->getBody()); + } + /** * @test */