From 8602b7a21bd4b2ad5db25f7b8cab56d19c22fbfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 8 Mar 2019 15:03:35 +0100 Subject: [PATCH 1/3] Switching to RouteLoader --- .gitignore | 1 + .../GraphqliteController.php | 79 ++----------------- Resources/config/container/graphqlite.xml | 10 ++- Resources/config/routes.xml | 8 ++ Routing/GraphqliteRouteLoader.php | 43 ++++++++++ Tests/GraphqliteTestingKernel.php | 20 ++++- 6 files changed, 85 insertions(+), 76 deletions(-) rename EventListeners/RequestHandler.php => Controller/GraphqliteController.php (51%) create mode 100644 Resources/config/routes.xml create mode 100644 Routing/GraphqliteRouteLoader.php diff --git a/.gitignore b/.gitignore index e17ee95..216c574 100644 --- a/.gitignore +++ b/.gitignore @@ -2,3 +2,4 @@ /composer.lock /build /var +/cache \ No newline at end of file diff --git a/EventListeners/RequestHandler.php b/Controller/GraphqliteController.php similarity index 51% rename from EventListeners/RequestHandler.php rename to Controller/GraphqliteController.php index 743629b..0e286fb 100644 --- a/EventListeners/RequestHandler.php +++ b/Controller/GraphqliteController.php @@ -1,7 +1,7 @@ standardServer = $standardServer; $this->httpMessageFactory = $httpMessageFactory ?: new DiactorosFactory(); - $this->graphqlUri = $graphqlUri; - $this->debug = $debug === null ? false : $debug; + $this->debug = $debug ?? false; } /** @@ -61,22 +52,11 @@ public static function getSubscribedEvents() return $events; } - public function handleRequest(GetResponseEvent $event) + public function handleRequest(Request $request): Response { - // Let's only handle the main request (the event might be triggered for sub-requests for error displaying for instance) - if ($event->getRequestType() !== HttpKernelInterface::MASTER_REQUEST) { - return; - } - - $request = $event->getRequest(); - - if (!$this->isGraphqlRequest($request)) { - return; - } - $psr7Request = $this->httpMessageFactory->createRequest($request); - if (strtoupper($request->getMethod()) == "POST" && empty($psr7Request->getParsedBody())) { + if (strtoupper($request->getMethod()) === "POST" && empty($psr7Request->getParsedBody())) { $content = $psr7Request->getBody()->getContents(); $parsedBody = json_decode($content, true); if (json_last_error() !== JSON_ERROR_NONE) { @@ -89,22 +69,9 @@ public function handleRequest(GetResponseEvent $event) $uploadMiddleware = new UploadMiddleware(); $psr7Request = $uploadMiddleware->processRequest($psr7Request); - - // Hack for Graph - /*if (strtoupper($request->getMethod()) == "GET") { - $params = $request->getQueryParams(); - $params["variables"] = $params["variables"] === 'undefined' ? null : $params["variables"]; - $request = $request->withQueryParams($params); - } else { - $params = $request->getParsedBody(); - $params["variables"] = $params["variables"] === 'undefined' ? null : $params["variables"]; - $request = $request->withParsedBody($params); - }*/ - $result = $this->handlePsr7Request($psr7Request); - $response = new JsonResponse($result); - $event->setResponse($response); + return new JsonResponse($result); } private function handlePsr7Request(ServerRequestInterface $request): array @@ -124,34 +91,4 @@ private function handlePsr7Request(ServerRequestInterface $request): array } throw new RuntimeException('Unexpected response from StandardServer::executePsrRequest'); // @codeCoverageIgnore } - - private function isGraphqlRequest(Request $request) : bool - { - return $this->isMethodAllowed($request) && ($this->hasUri($request) || $this->hasGraphqlHeader($request)); - } - private function isMethodAllowed(Request $request) : bool - { - return in_array($request->getMethod(), $this->allowedMethods, true); - } - private function hasUri(Request $request) : bool - { - return $this->graphqlUri === $request->getPathInfo(); - } - private function hasGraphqlHeader(Request $request) : bool - { - if (! $request->headers->has('content-type')) { - return false; - } - - $requestHeaderList = $request->headers->get('content-type', null, false); - if ($requestHeaderList === null) { - return false; - } - foreach ($this->graphqlHeaderList as $allowedHeader) { - if (in_array($allowedHeader, $requestHeaderList, true)) { - return true; - } - } - return false; - } } diff --git a/Resources/config/container/graphqlite.xml b/Resources/config/container/graphqlite.xml index 52121e3..2d9d087 100644 --- a/Resources/config/container/graphqlite.xml +++ b/Resources/config/container/graphqlite.xml @@ -61,10 +61,6 @@ - - - - @@ -101,6 +97,12 @@ + + + + + + \ No newline at end of file diff --git a/Resources/config/routes.xml b/Resources/config/routes.xml new file mode 100644 index 0000000..30a44de --- /dev/null +++ b/Resources/config/routes.xml @@ -0,0 +1,8 @@ + + + + + diff --git a/Routing/GraphqliteRouteLoader.php b/Routing/GraphqliteRouteLoader.php new file mode 100644 index 0000000..72521bd --- /dev/null +++ b/Routing/GraphqliteRouteLoader.php @@ -0,0 +1,43 @@ +isLoaded) { + throw new \RuntimeException('Do not add the "graphqlite" loader twice'); + } + + $routes = new RouteCollection(); + + // prepare a new route + $path = '/graphql'; + $defaults = [ + '_controller' => 'TheCodingMachine\Graphqlite\Bundle\Controller\GraphqliteController::handleRequest', + ]; + $route = new Route($path, $defaults); + + // add the new route to the route collection + $routeName = 'graphqliteRoute'; + $routes->add($routeName, $route); + + $this->isLoaded = true; + + return $routes; + } + + public function supports($resource, $type = null) + { + return 'graphqlite' === $type; + } +} diff --git a/Tests/GraphqliteTestingKernel.php b/Tests/GraphqliteTestingKernel.php index 80c6df9..1a73640 100644 --- a/Tests/GraphqliteTestingKernel.php +++ b/Tests/GraphqliteTestingKernel.php @@ -4,16 +4,24 @@ namespace TheCodingMachine\Graphqlite\Bundle\Tests; +use Symfony\Bundle\FrameworkBundle\Kernel\MicroKernelTrait; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Kernel; +use Symfony\Component\Routing\RouteCollectionBuilder; use TheCodingMachine\Graphqlite\Bundle\GraphqliteBundle; class GraphqliteTestingKernel extends Kernel { + use MicroKernelTrait; const CONFIG_EXTS = '.{php,xml,yaml,yml}'; + public function __construct() + { + parent::__construct('test', true); + } + public function registerBundles() { return [ @@ -22,7 +30,7 @@ public function registerBundles() ]; } - public function registerContainerConfiguration(LoaderInterface $loader) + public function configureContainer(ContainerBuilder $c, LoaderInterface $loader) { $loader->load(function(ContainerBuilder $container) { $container->loadFromExtension('framework', array( @@ -42,4 +50,14 @@ public function registerContainerConfiguration(LoaderInterface $loader) $loader->load($confDir.'/{services}'.self::CONFIG_EXTS, 'glob'); $loader->load($confDir.'/{services}_'.$this->environment.self::CONFIG_EXTS, 'glob'); } + + protected function configureRoutes(RouteCollectionBuilder $routes) + { + $routes->import(__DIR__.'/../Resources/config/routes.xml'); + } + + public function getCacheDir() + { + return __DIR__.'/../cache/'.spl_object_hash($this); + } } From 8f23bf923898e1a3a01bcfc88ba7517906f90bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 8 Mar 2019 15:07:43 +0100 Subject: [PATCH 2/3] Switching to routes as a service --- Controller/GraphqliteController.php | 23 ++++++++---- Resources/config/container/graphqlite.xml | 4 --- Resources/config/routes.xml | 2 +- Routing/GraphqliteRouteLoader.php | 43 ----------------------- 4 files changed, 18 insertions(+), 54 deletions(-) delete mode 100644 Routing/GraphqliteRouteLoader.php diff --git a/Controller/GraphqliteController.php b/Controller/GraphqliteController.php index 0e286fb..39f9e09 100644 --- a/Controller/GraphqliteController.php +++ b/Controller/GraphqliteController.php @@ -21,6 +21,8 @@ use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\HttpKernelInterface; use Symfony\Component\HttpKernel\KernelEvents; +use Symfony\Component\Routing\Route; +use Symfony\Component\Routing\RouteCollection; /** * Listens to every single request and forward Graphql requests to Graphql Webonix standardServer. @@ -43,13 +45,22 @@ public function __construct(StandardServer $standardServer, HttpMessageFactoryIn $this->debug = $debug ?? false; } - /** - * {@inheritdoc} - */ - public static function getSubscribedEvents() + public function loadRoutes(): RouteCollection { - $events[KernelEvents::REQUEST][] = array('handleRequest', 33); - return $events; + $routes = new RouteCollection(); + + // prepare a new route + $path = '/graphql'; + $defaults = [ + '_controller' => self::class.'::handleRequest', + ]; + $route = new Route($path, $defaults); + + // add the new route to the route collection + $routeName = 'graphqliteRoute'; + $routes->add($routeName, $route); + + return $routes; } public function handleRequest(Request $request): Response diff --git a/Resources/config/container/graphqlite.xml b/Resources/config/container/graphqlite.xml index 2d9d087..83b2287 100644 --- a/Resources/config/container/graphqlite.xml +++ b/Resources/config/container/graphqlite.xml @@ -98,10 +98,6 @@ - - - - diff --git a/Resources/config/routes.xml b/Resources/config/routes.xml index 30a44de..171d2b7 100644 --- a/Resources/config/routes.xml +++ b/Resources/config/routes.xml @@ -4,5 +4,5 @@ xsi:schemaLocation="http://symfony.com/schema/routing http://symfony.com/schema/routing/routing-1.0.xsd"> - + diff --git a/Routing/GraphqliteRouteLoader.php b/Routing/GraphqliteRouteLoader.php deleted file mode 100644 index 72521bd..0000000 --- a/Routing/GraphqliteRouteLoader.php +++ /dev/null @@ -1,43 +0,0 @@ -isLoaded) { - throw new \RuntimeException('Do not add the "graphqlite" loader twice'); - } - - $routes = new RouteCollection(); - - // prepare a new route - $path = '/graphql'; - $defaults = [ - '_controller' => 'TheCodingMachine\Graphqlite\Bundle\Controller\GraphqliteController::handleRequest', - ]; - $route = new Route($path, $defaults); - - // add the new route to the route collection - $routeName = 'graphqliteRoute'; - $routes->add($routeName, $route); - - $this->isLoaded = true; - - return $routes; - } - - public function supports($resource, $type = null) - { - return 'graphqlite' === $type; - } -} From cce555f40cc5aa173f4f00dee5bf8899768b179e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Fri, 8 Mar 2019 16:03:05 +0100 Subject: [PATCH 3/3] Fixing PHPStan --- composer.json | 2 +- phpstan.neon | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 1445b21..9beff90 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "beberlei/porpaginas": "^1.2" }, "scripts": { - "phpstan": "phpstan analyse GraphqliteBundle.php DependencyInjection/ EventListeners/ Mappers/ Resources/ Security/ -c phpstan.neon --level=7 --no-progress" + "phpstan": "phpstan analyse GraphqliteBundle.php DependencyInjection/ Controller/ Mappers/ Resources/ Security/ -c phpstan.neon --level=7 --no-progress" }, "suggest": { "symfony/security-bundle": "To use @Logged or @Right annotations" diff --git a/phpstan.neon b/phpstan.neon index 1cd87eb..9481751 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -1,7 +1,6 @@ parameters: ignoreErrors: - "#Call to an undefined method Symfony\\\\Component\\\\Config\\\\Definition\\\\Builder\\\\NodeDefinition::children\\(\\)#" - - "/Parameter #2 $haystack of function in_array expects array, array|string given./" - "#Access to an undefined property GraphQL\\\\Type\\\\Definition\\\\NamedType&GraphQL\\\\Type\\\\Definition\\\\OutputType::\\$name.#" #includes: # - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon \ No newline at end of file