From b732a318a75f1050be1ff1e14b6a474567e48a3d Mon Sep 17 00:00:00 2001 From: Frits Date: Sun, 4 Aug 2019 21:37:57 +0200 Subject: [PATCH 1/8] v4 support --- src/App.php | 48 +++++++++++++----------- src/CallableResolver.php | 3 +- src/ControllerInvoker.php | 5 +-- src/config.php | 78 --------------------------------------- 4 files changed, 29 insertions(+), 105 deletions(-) delete mode 100644 src/config.php diff --git a/src/App.php b/src/App.php index e1e0a56..4a1cb7a 100644 --- a/src/App.php +++ b/src/App.php @@ -2,7 +2,14 @@ namespace DI\Bridge\Slim; -use DI\ContainerBuilder; +use Invoker\Invoker; +use Invoker\ParameterResolver\AssociativeArrayResolver; +use Invoker\ParameterResolver\Container\TypeHintContainerResolver; +use Invoker\ParameterResolver\DefaultValueResolver; +use Invoker\ParameterResolver\ResolverChain; +use Psr\Container\ContainerInterface; +use Slim\Factory\AppFactory; +use \Invoker\CallableResolver as InvokerCallableResolver; /** * Slim application configured with PHP-DI. @@ -10,29 +17,28 @@ * As you can see, this class is very basic and is only useful to get started quickly. * You can also very well *not* use it and build the container manually. */ -class App extends \Slim\App +class App { - public function __construct() + public static function boot(ContainerInterface $container) { - $containerBuilder = new ContainerBuilder; - $containerBuilder->addDefinitions(__DIR__ . '/config.php'); - $this->configureContainer($containerBuilder); - $container = $containerBuilder->build(); + AppFactory::setContainer($container); + $callableResolver = new InvokerCallableResolver($container); + AppFactory::setCallableResolver(new CallableResolver($callableResolver)); - parent::__construct($container); - } + $app = AppFactory::create(); - /** - * Override this method to configure the container builder. - * - * For example, to load additional configuration files: - * - * protected function configureContainer(ContainerBuilder $builder) - * { - * $builder->addDefinitions(__DIR__ . 'my-config-file.php'); - * } - */ - protected function configureContainer(ContainerBuilder $builder) - { + $resolvers = [ + // Inject parameters by name first + new AssociativeArrayResolver(), + // Then inject services by type-hints for those that weren't resolved + new TypeHintContainerResolver($container), + // Then fall back on parameters default values for optional route parameters + new DefaultValueResolver(), + ]; + + $invoker = new Invoker(new ResolverChain($resolvers), $container); + $app->getRouteCollector()->setDefaultInvocationStrategy(new ControllerInvoker($invoker)); + + return $app; } } diff --git a/src/CallableResolver.php b/src/CallableResolver.php index 40cc861..1321124 100644 --- a/src/CallableResolver.php +++ b/src/CallableResolver.php @@ -18,11 +18,10 @@ public function __construct(\Invoker\CallableResolver $callableResolver) { $this->callableResolver = $callableResolver; } - /** * {@inheritdoc} */ - public function resolve($toResolve) + public function resolve($toResolve): callable { return $this->callableResolver->resolve($toResolve); } diff --git a/src/ControllerInvoker.php b/src/ControllerInvoker.php index e16b8a4..8817b81 100644 --- a/src/ControllerInvoker.php +++ b/src/ControllerInvoker.php @@ -18,7 +18,6 @@ public function __construct(InvokerInterface $invoker) { $this->invoker = $invoker; } - /** * Invoke a route callable. * @@ -34,16 +33,14 @@ public function __invoke( ServerRequestInterface $request, ResponseInterface $response, array $routeArguments - ) { + ): ResponseInterface { // Inject the request and response by parameter name $parameters = [ 'request' => $request, 'response' => $response, ]; - // Inject the route arguments by name $parameters += $routeArguments; - // Inject the attributes defined on the request $parameters += $request->getAttributes(); diff --git a/src/config.php b/src/config.php deleted file mode 100644 index ea2bd61..0000000 --- a/src/config.php +++ /dev/null @@ -1,78 +0,0 @@ - '1.1', - 'settings.responseChunkSize' => 4096, - 'settings.outputBuffering' => 'append', - 'settings.determineRouteBeforeAppMiddleware' => false, - 'settings.displayErrorDetails' => false, - 'settings.addContentLengthHeader' => true, - 'settings.routerCacheFile' => false, - - 'settings' => [ - 'httpVersion' => get('settings.httpVersion'), - 'responseChunkSize' => get('settings.responseChunkSize'), - 'outputBuffering' => get('settings.outputBuffering'), - 'determineRouteBeforeAppMiddleware' => get('settings.determineRouteBeforeAppMiddleware'), - 'displayErrorDetails' => get('settings.displayErrorDetails'), - 'addContentLengthHeader' => get('settings.addContentLengthHeader'), - 'routerCacheFile' => get('settings.routerCacheFile'), - ], - - // Default Slim services - 'router' => create(Slim\Router::class) - ->method('setContainer', get(Container::class)) - ->method('setCacheFile', get('settings.routerCacheFile')), - Slim\Router::class => get('router'), - 'errorHandler' => create(Slim\Handlers\Error::class) - ->constructor(get('settings.displayErrorDetails')), - 'phpErrorHandler' => create(Slim\Handlers\PhpError::class) - ->constructor(get('settings.displayErrorDetails')), - 'notFoundHandler' => create(Slim\Handlers\NotFound::class), - 'notAllowedHandler' => create(Slim\Handlers\NotAllowed::class), - 'environment' => function () { - return new Slim\Http\Environment($_SERVER); - }, - 'request' => function (ContainerInterface $c) { - return Request::createFromEnvironment($c->get('environment')); - }, - 'response' => function (ContainerInterface $c) { - $headers = new Headers(['Content-Type' => 'text/html; charset=UTF-8']); - $response = new Response(200, $headers); - return $response->withProtocolVersion($c->get('settings')['httpVersion']); - }, - 'foundHandler' => create(ControllerInvoker::class) - ->constructor(get('foundHandler.invoker')), - 'foundHandler.invoker' => function (ContainerInterface $c) { - $resolvers = [ - // Inject parameters by name first - new AssociativeArrayResolver, - // Then inject services by type-hints for those that weren't resolved - new TypeHintContainerResolver($c), - // Then fall back on parameters default values for optional route parameters - new DefaultValueResolver(), - ]; - return new Invoker(new ResolverChain($resolvers), $c); - }, - - 'callableResolver' => autowire(CallableResolver::class), - -]; From 57c8beb37750e2a7fca003a672ac749ab75a6d1e Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 5 Aug 2019 23:13:34 +0200 Subject: [PATCH 2/8] Rename and fix the main class --- composer.json | 5 +++-- src/{App.php => Bridge.php} | 10 +++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) rename src/{App.php => Bridge.php} (87%) diff --git a/composer.json b/composer.json index e3d1df3..672edc8 100644 --- a/composer.json +++ b/composer.json @@ -17,9 +17,10 @@ "php": "~7.0", "php-di/php-di": "^6.0.0", "php-di/invoker": "^2.0.0", - "slim/slim": "^3.9.0" + "slim/slim": "^4.0.0" }, "require-dev": { - "phpunit/phpunit": "~6.0" + "phpunit/phpunit": "~6.0", + "zendframework/zend-diactoros": "^2.1" } } diff --git a/src/App.php b/src/Bridge.php similarity index 87% rename from src/App.php rename to src/Bridge.php index 4a1cb7a..e243f5a 100644 --- a/src/App.php +++ b/src/Bridge.php @@ -1,13 +1,15 @@ - Date: Mon, 5 Aug 2019 23:13:43 +0200 Subject: [PATCH 3/8] Update the tests for Slim 4 --- tests/ApplicationTest.php | 10 ++-- tests/ContainerTest.php | 59 ------------------- tests/ErrorTest.php | 98 -------------------------------- tests/Fixture/Middleware.php | 15 ----- tests/Fixture/UserController.php | 2 +- tests/MiddlewareTest.php | 17 +++--- tests/Mock/RequestFactory.php | 24 +++++--- tests/RoutingTest.php | 51 +++++++++-------- 8 files changed, 55 insertions(+), 221 deletions(-) delete mode 100644 tests/ContainerTest.php delete mode 100644 tests/ErrorTest.php delete mode 100644 tests/Fixture/Middleware.php diff --git a/tests/ApplicationTest.php b/tests/ApplicationTest.php index 7b228d2..697f8b6 100644 --- a/tests/ApplicationTest.php +++ b/tests/ApplicationTest.php @@ -2,10 +2,9 @@ namespace DI\Bridge\Slim\Test; -use DI\Bridge\Slim\App; +use DI\Bridge\Slim\Bridge; use DI\Bridge\Slim\Test\Mock\RequestFactory; use PHPUnit\Framework\TestCase; -use Slim\Http\Response; class ApplicationTest extends TestCase { @@ -14,14 +13,15 @@ class ApplicationTest extends TestCase */ public function runs() { - $app = new App; + $app = Bridge::create(); $called = false; - $app->get('/', function () use (&$called) { + $app->get('/', function ($request, $response) use (&$called) { $called = true; + return $response; }); + $app->handle(RequestFactory::create()); - $app->callMiddlewareStack(RequestFactory::create(), new Response); $this->assertTrue($called); } } diff --git a/tests/ContainerTest.php b/tests/ContainerTest.php deleted file mode 100644 index 2b8ea72..0000000 --- a/tests/ContainerTest.php +++ /dev/null @@ -1,59 +0,0 @@ -getContainer(); - /** @var \DI\Container $phpdiContainer */ - $phpdiContainer = $slimPhpDi->getContainer(); - - $expectedEntries = $defaultContainer->keys(); - - foreach ($expectedEntries as $expectedEntry) { - $this->assertTrue($phpdiContainer->has($expectedEntry), "Container entry $expectedEntry is missing"); - // Check that the service is created without exception - $phpdiContainer->get($expectedEntry); - } - } - - /** - * Slim expects some config options to exist. - * - * @test - */ - public function provides_default_config_options() - { - $slimDefault = new \Slim\App; - $slimPhpDi = new App; - - /** @var \Slim\Container $defaultContainer */ - $defaultContainer = $slimDefault->getContainer(); - /** @var \DI\Container $phpdiContainer */ - $phpdiContainer = $slimPhpDi->getContainer(); - - $expectedOptions = $defaultContainer->get('settings'); - $actualOptions = $phpdiContainer->get('settings'); - - foreach ($expectedOptions as $name => $value) { - $this->assertArrayHasKey($name, $actualOptions); - // Has the same default value - $this->assertEquals($value, $actualOptions[$name]); - } - } -} diff --git a/tests/ErrorTest.php b/tests/ErrorTest.php deleted file mode 100644 index d008157..0000000 --- a/tests/ErrorTest.php +++ /dev/null @@ -1,98 +0,0 @@ -getContainer(); - - // Sanity check - default for displayErrorDetails should be false - $displayErrorDetails = $c->get('settings.displayErrorDetails'); - $this->assertFalse($displayErrorDetails); - - /** @var Error $error */ - $error = $c->get('errorHandler'); - $response = $error(RequestFactory::create('/'), new Response(), new \Exception()); - $reasonPhrase = $response->getReasonPhrase(); - $this->assertEquals('Internal Server Error', $reasonPhrase); - - $log = file_get_contents($logFile); - $this->assertNotEmpty($log); - } - - /** - * Test custom errorHandler - * - * @test - */ - public function custom_exception_handling() - { - // Send error_log output to a temp file. - $logFile = tempnam(sys_get_temp_dir(), 'slim-bridge'); - ini_set('error_log', $logFile); - - $app = new BridgeApp( - [ - 'settings.displayErrorDetails' => true, - 'settings.outputBuffering' => 'append' - ]); - $c = $app->getContainer(); - - // Sanity checks - $displayErrorDetails = $c->get('settings.displayErrorDetails'); - $this->assertTrue($displayErrorDetails); - $outputBuffering = $c->get('settings.outputBuffering'); - $this->assertEquals('append', $outputBuffering); - - /** @var Error $error */ - $error = $c->get('errorHandler'); - - $response = $error(RequestFactory::create('/'), new Response(), new \Exception()); - $reasonPhrase = $response->getReasonPhrase(); - $this->assertEquals('Internal Server Error', $reasonPhrase); - - $log = file_get_contents($logFile); - $this->assertEmpty($log); - } -} - -/** - * Class BridgeApp - * - * Override the configuration via the configureContainer() hook. - */ -class BridgeApp extends App -{ - protected $config = []; - - public function __construct(array $config) - { - $this->config = $config; - - parent::__construct(); - } - - public function configureContainer(ContainerBuilder $builder) - { - $builder->addDefinitions($this->config); - } -} diff --git a/tests/Fixture/Middleware.php b/tests/Fixture/Middleware.php deleted file mode 100644 index 132fa84..0000000 --- a/tests/Fixture/Middleware.php +++ /dev/null @@ -1,15 +0,0 @@ -getBody()->write('Hello world!'); - return $response; - } -} diff --git a/tests/Fixture/UserController.php b/tests/Fixture/UserController.php index b0e85d3..85f8322 100644 --- a/tests/Fixture/UserController.php +++ b/tests/Fixture/UserController.php @@ -1,4 +1,4 @@ -add(function (ServerRequestInterface $request, ResponseInterface $response, callable $next) { - $response->getBody()->write('Hello ' . $request->getQueryParams()['foo']); - return $response; + $app = Bridge::create(); + $app->add(function (ServerRequestInterface $request, RequestHandlerInterface $next) { + return new TextResponse('Hello ' . $request->getQueryParams()['foo']); }); $app->get('/', function () {}); - $response = $app->callMiddlewareStack(RequestFactory::create('/', 'foo=matt'), new Response); + $response = $app->handle(RequestFactory::create('/', 'foo=matt')); $this->assertEquals('Hello matt', $response->getBody()->__toString()); } diff --git a/tests/Mock/RequestFactory.php b/tests/Mock/RequestFactory.php index ccacb93..5b13ab8 100644 --- a/tests/Mock/RequestFactory.php +++ b/tests/Mock/RequestFactory.php @@ -1,18 +1,24 @@ - 'index.php', - 'REQUEST_URI' => $uri, - 'QUERY_STRING' => $queryString, - ])); + parse_str($queryString, $queryParams); + return new ServerRequest( + [], + [], + $uri, + 'GET', + 'php://temp', + [], + [], + $queryParams + ); } } diff --git a/tests/RoutingTest.php b/tests/RoutingTest.php index 94bc656..dc55454 100644 --- a/tests/RoutingTest.php +++ b/tests/RoutingTest.php @@ -1,14 +1,14 @@ -get('/', function (ResponseInterface $response, ServerRequestInterface $request) { $response->getBody()->write('Hello ' . $request->getQueryParams()['foo']); return $response; }); - $response = $app->callMiddlewareStack(RequestFactory::create('/', 'foo=matt'), new Response); - $this->assertEquals('Hello matt', $response->getBody()->__toString()); + $response = $app->handle(RequestFactory::create('/', 'foo=matt')); + $this->assertEquals('Hello matt', (string) $response->getBody()); } /** @@ -33,14 +34,14 @@ public function injects_request_and_response() */ public function injects_route_placeholder() { - $app = new App; + $app = Bridge::create(); $app->get('/{name}', function ($name, $response) { $response->getBody()->write('Hello ' . $name); return $response; }); - $response = $app->callMiddlewareStack(RequestFactory::create('/matt'), new Response); - $this->assertEquals('Hello matt', $response->getBody()->__toString()); + $response = $app->handle(RequestFactory::create('/matt')); + $this->assertEquals('Hello matt', (string) $response->getBody()); } /** @@ -48,13 +49,13 @@ public function injects_route_placeholder() */ public function injects_optional_route_placeholder() { - $app = new App; + $app = Bridge::create(); $app->get('/[{name}]', function ($response, $name = null) { $response->getBody()->write('Hello ' . $name); return $response; }); - $response = $app->callMiddlewareStack(RequestFactory::create('/matt'), new Response); + $response = $app->handle(RequestFactory::create('/matt')); $this->assertEquals('Hello matt', (string) $response->getBody()); } @@ -63,13 +64,13 @@ public function injects_optional_route_placeholder() */ public function injects_default_value_in_optional_route_placeholder() { - $app = new App; + $app = Bridge::create(); $app->get('/[{name}]', function ($response, $name = 'john doe') { $response->getBody()->write('Hello ' . $name); return $response; }); - $response = $app->callMiddlewareStack(RequestFactory::create('/'), new Response); + $response = $app->handle(RequestFactory::create()); $this->assertEquals('Hello john doe', (string) $response->getBody()); } @@ -78,18 +79,18 @@ public function injects_default_value_in_optional_route_placeholder() */ public function injects_request_attribute() { - $app = new App; + $app = Bridge::create(); // Let's add a middleware that adds a request attribute - $app->add(function (ServerRequestInterface $request, $response, $next) { - return $next($request->withAttribute('name', 'Bob'), $response); + $app->add(function (ServerRequestInterface $request, RequestHandlerInterface $next) { + return $next->handle($request->withAttribute('name', 'Bob')); }); $app->get('/', function ($name, $response) { $response->getBody()->write('Hello ' . $name); return $response; }); - $response = $app->callMiddlewareStack(RequestFactory::create('/'), new Response); - $this->assertEquals('Hello Bob', $response->getBody()->__toString()); + $response = $app->handle(RequestFactory::create()); + $this->assertEquals('Hello Bob', (string) $response->getBody()); } /** @@ -97,16 +98,16 @@ public function injects_request_attribute() */ public function injects_route_placeholder_over_request_attribute() { - $app = new App; - $app->add(function (ServerRequestInterface $request, $response, $next) { - return $next($request->withAttribute('name', 'Bob'), $response); + $app = Bridge::create(); + $app->add(function (ServerRequestInterface $request, RequestHandlerInterface $next) { + return $next->handle($request->withAttribute('name', 'Bob')); }); $app->get('/{name}', function ($name, $response) { $response->getBody()->write('Hello ' . $name); return $response; }); - $response = $app->callMiddlewareStack(RequestFactory::create('/matt'), new Response); + $response = $app->handle(RequestFactory::create('/matt')); // The route placeholder has priority over the request attribute $this->assertEquals('Hello matt', (string) $response->getBody()); } @@ -116,10 +117,10 @@ public function injects_route_placeholder_over_request_attribute() */ public function resolve_controller_from_container() { - $app = new App; + $app = Bridge::create(); $app->get('/', [UserController::class, 'dashboard']); - $response = $app->callMiddlewareStack(RequestFactory::create(), new Response); - $this->assertEquals('Hello world!', $response->getBody()->__toString()); + $response = $app->handle(RequestFactory::create()); + $this->assertEquals('Hello world!', (string) $response->getBody()); } } From af69b1ecdc28dcebe1c6443f305f807eb1f2e031 Mon Sep 17 00:00:00 2001 From: Frits Date: Mon, 5 Aug 2019 15:52:29 +0200 Subject: [PATCH 4/8] travis php 7.1 --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index feb3d49..440a191 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,13 +1,12 @@ language: php php: - - 7.0 - 7.1 - 7.2 matrix: include: - - php: 7.0 + - php: 7.1 env: dependencies=lowest sudo: false From 7391024a14161e016a89d4b5878ffbbe5f5e2417 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 5 Aug 2019 23:18:42 +0200 Subject: [PATCH 5/8] Update PHP requirements --- .travis.yml | 1 + composer.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 440a191..4a03d48 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,6 +3,7 @@ language: php php: - 7.1 - 7.2 + - 7.3 matrix: include: diff --git a/composer.json b/composer.json index 672edc8..849b88e 100644 --- a/composer.json +++ b/composer.json @@ -14,7 +14,7 @@ } }, "require": { - "php": "~7.0", + "php": "~7.1", "php-di/php-di": "^6.0.0", "php-di/invoker": "^2.0.0", "slim/slim": "^4.0.0" From eca7ccf7039bd3da4f70a6dabe6d1f5fd554c33d Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 5 Aug 2019 23:21:33 +0200 Subject: [PATCH 6/8] Update comment --- src/Bridge.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Bridge.php b/src/Bridge.php index e243f5a..24cf69e 100644 --- a/src/Bridge.php +++ b/src/Bridge.php @@ -14,10 +14,10 @@ use \Invoker\CallableResolver as InvokerCallableResolver; /** - * Slim application configured with PHP-DI. + * This factory creates a Slim application correctly configured with PHP-DI. * - * As you can see, this class is very basic and is only useful to get started quickly. - * You can also very well *not* use it and build the container manually. + * To use this, replace `Slim\Factory\AppFactory::create()` + * with `DI\Bridge\Slim\Bridge::create()`. */ class Bridge { From 00c64bc878d9bd5e92d6eb76af8ac169e8a92a33 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 5 Aug 2019 23:23:24 +0200 Subject: [PATCH 7/8] Simplify code --- src/Bridge.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/Bridge.php b/src/Bridge.php index 24cf69e..c36a704 100644 --- a/src/Bridge.php +++ b/src/Bridge.php @@ -31,6 +31,14 @@ public static function create(ContainerInterface $container = null): App $app = AppFactory::create(); + $controllerInvoker = self::createControllerInvoker($container); + $app->getRouteCollector()->setDefaultInvocationStrategy($controllerInvoker); + + return $app; + } + + private static function createControllerInvoker(ContainerInterface $container): ControllerInvoker + { $resolvers = [ // Inject parameters by name first new AssociativeArrayResolver(), @@ -41,8 +49,7 @@ public static function create(ContainerInterface $container = null): App ]; $invoker = new Invoker(new ResolverChain($resolvers), $container); - $app->getRouteCollector()->setDefaultInvocationStrategy(new ControllerInvoker($invoker)); - return $app; + return new ControllerInvoker($invoker); } } From bd11c84945d20d7f7dad77d3767ef2e051fbf3c6 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Mon, 5 Aug 2019 23:24:44 +0200 Subject: [PATCH 8/8] Disable email notifications --- .travis.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.travis.yml b/.travis.yml index 4a03d48..2b0aa38 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,9 @@ language: php +notifications: + email: + on_success: never + php: - 7.1 - 7.2 @@ -10,6 +14,10 @@ matrix: - php: 7.1 env: dependencies=lowest +cache: + directories: + - $HOME/.composer/cache + sudo: false before_script: