From 7e3b176bdbf612212418ee9c3dec20ea7f221c0e Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 17 Feb 2017 14:39:33 +0300 Subject: [PATCH 01/22] Security * RBAC * Route options --- Exception/AccessDeniedException.php | 16 ++++++++++ Listener/SecurityListener.php | 40 +++++++++++++++++++++++++ Resources/config/security.yml | 5 ++++ Routing/Route.php | 46 +++++++++++++++++++++++++---- composer.json | 1 + 5 files changed, 103 insertions(+), 5 deletions(-) create mode 100644 Exception/AccessDeniedException.php create mode 100644 Listener/SecurityListener.php create mode 100644 Resources/config/security.yml diff --git a/Exception/AccessDeniedException.php b/Exception/AccessDeniedException.php new file mode 100644 index 0000000..3f34d8a --- /dev/null +++ b/Exception/AccessDeniedException.php @@ -0,0 +1,16 @@ +getRequest()->getAttributes()->get('_route'); + + if (null === $route) { + return; + } + + $roles = $route->getOption('_roles', null); + if (is_array($roles) && !$this->filterByRoles($roles)) { + throw AccessDeniedException::rolesNeeded($roles); + } + } + + private function filterByRoles(array $roles) + { + foreach ($roles as $role) { + if (!$this->authenticator->isGranted($role)) { + return false; + } + } + + return true; + } +} diff --git a/Resources/config/security.yml b/Resources/config/security.yml new file mode 100644 index 0000000..942b8a3 --- /dev/null +++ b/Resources/config/security.yml @@ -0,0 +1,5 @@ +services: + rpc.listeners.security_filter: + class: Bankiru\Api\Rpc\Listener\SecurityListener + arguments: + - "@security.authorization_checker" diff --git a/Routing/Route.php b/Routing/Route.php index 626e9e1..bcbc08c 100644 --- a/Routing/Route.php +++ b/Routing/Route.php @@ -2,6 +2,8 @@ namespace Bankiru\Api\Rpc\Routing; +use Symfony\Component\HttpFoundation\ParameterBag; + class Route { /** @var string */ @@ -14,8 +16,8 @@ class Route private $defaultContext; /** @var bool */ private $inheritContext; - /** @var array */ - private $options = []; + /** @var ParameterBag */ + private $options; /** * Route constructor. @@ -25,20 +27,22 @@ class Route * @param array $context * @param bool $defaultContext * @param bool $inheritContext + * @param array $options */ public function __construct( $method, $controller, array $context, $defaultContext = true, - $inheritContext = true - ) - { + $inheritContext = true, + array $options = [] + ) { $this->method = $method; $this->controller = $controller; $this->context = $context; $this->defaultContext = (bool)$defaultContext; $this->inheritContext = (bool)$inheritContext; + $this->options = new ParameterBag($options); } /** @@ -115,4 +119,36 @@ public function inheritContext() { return $this->inheritContext; } + + /** + * @param string $key + * @param mixed $default + * + * @return mixed + */ + public function getOption($key, $default = null) + { + return $this->options->get($key, $default); + } + + /** + * @param string $key + * @param mixed $value + */ + public function setOption($key, $value) + { + $this->options->set($key, $value); + } + + public function getOptions() + { + return $this->options; + } + + public function addOptions(array $options) + { + foreach ($options as $key => $value) { + $this->options->set($key, $value); + } + } } diff --git a/composer.json b/composer.json index f0be09d..ba3d5c6 100644 --- a/composer.json +++ b/composer.json @@ -28,6 +28,7 @@ }, "suggest": { "symfony/console": "For using debug console command", + "symfony/security": "For enabling role and expressions based security filters", "nelmio/api-doc-bundle": "For RpcApiDoc extraction" }, "autoload": { From 6e6bc9239d10641e378afec7dcfd35e81945e01c Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 17 Feb 2017 14:49:55 +0300 Subject: [PATCH 02/22] Install security-bundle for tests --- DependencyInjection/RpcExtension.php | 4 ++++ Tests/Fixtures/Kernel.php | 2 ++ Tests/Fixtures/config/test.yml | 2 ++ composer.json | 1 + 4 files changed, 9 insertions(+) diff --git a/DependencyInjection/RpcExtension.php b/DependencyInjection/RpcExtension.php index 9eef06d..9c42534 100644 --- a/DependencyInjection/RpcExtension.php +++ b/DependencyInjection/RpcExtension.php @@ -23,6 +23,10 @@ public function load(array $configs, ContainerBuilder $container) $loader->load('nelmio.yml'); } + if ($container->has('security.authorization_checker')) { + $loader->load('security.yml'); + } + $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); diff --git a/Tests/Fixtures/Kernel.php b/Tests/Fixtures/Kernel.php index 259da0b..fc2539b 100644 --- a/Tests/Fixtures/Kernel.php +++ b/Tests/Fixtures/Kernel.php @@ -4,6 +4,7 @@ use Bankiru\Api\Rpc\RpcBundle; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; +use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\HttpKernel\Kernel as BaseKernel; @@ -32,6 +33,7 @@ public function registerBundles() return [ new FrameworkBundle(), new RpcBundle(), + new SecurityBundle(), ]; } diff --git a/Tests/Fixtures/config/test.yml b/Tests/Fixtures/config/test.yml index ee63ff9..ddfd877 100644 --- a/Tests/Fixtures/config/test.yml +++ b/Tests/Fixtures/config/test.yml @@ -8,6 +8,8 @@ services: logger: class: Psr\Log\NullLogger +security: ~ + rpc: router: endpoints: diff --git a/composer.json b/composer.json index ba3d5c6..6c8fd68 100644 --- a/composer.json +++ b/composer.json @@ -23,6 +23,7 @@ "symfony/console": "~2.7 || ~3.0", "symfony/routing": "~2.7 || ~3.0", "symfony/framework-bundle": "~2.7 || ~3.0", + "symfony/security-bundle": "~2.7 || ~3.0", "phpunit/phpunit": "~4.8 || ~5.1", "symfony/browser-kit": "~2.7 || ~3.0" }, From 6b6a79d050deb229e031ad5bbf529608f33fcdad Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Thu, 22 Jun 2017 14:48:41 +0300 Subject: [PATCH 03/22] Router refactoring * Implement route caching mechanism --- .travis.yml | 6 +- ApiDoc/Annotation/RpcApiDoc.php | 101 --------- .../Provider/AbstractRpcMethodProvider.php | 85 -------- .../Provider/MethodAnnotationsProvider.php | 49 ----- BankiruRpcServerBundle.php | 22 ++ Cache/RouterCacheWarmer.php | 50 +++++ Command/RouterDebugCommand.php | 14 +- Controller/RpcController.php | 40 ++-- Controller/RpcControllerNameParser.php | 101 ++++++--- .../BankiruRpcServerExtension.php | 95 +++++++++ DependencyInjection/RpcExtension.php | 91 -------- Event/RpcResponseEvent.php | 2 + Exception/AccessDeniedException.php | 7 +- Listener/RouterListener.php | 10 +- Listener/SecurityListener.php | 10 + Resources/config/nelmio.yml | 12 -- Routing/Annotation/Method.php | 17 ++ Routing/AttributesHelper.php | 24 +++ Routing/CollectionMatcher.php | 33 +++ Routing/Loader/AnnotationClassLoader.php | 50 +++-- Routing/LoaderResolver.php | 2 +- Routing/MatcherDumper.php | 53 +++++ Routing/MethodCollection.php | 33 +-- Routing/MethodCollectionLoader.php | 13 ++ Routing/MethodMatcher.php | 17 ++ Routing/ResourceMethodCollectionLoader.php | 55 +++++ Routing/Router.php | 198 +++++++++++++++--- Routing/RouterCollection.php | 2 +- RpcBundle.php | 12 +- {Impl => Tests/Fixtures/Impl}/Request.php | 2 +- {Impl => Tests/Fixtures/Impl}/Response.php | 2 +- Tests/Fixtures/Kernel.php | 4 +- Tests/Fixtures/Rpc/RpcImplController.php | 4 +- Tests/Fixtures/Rpc/SecurityController.php | 16 ++ Tests/Fixtures/Rpc/TestController.php | 4 +- Tests/RoutingTest.php | 2 +- composer.json | 3 +- 37 files changed, 730 insertions(+), 511 deletions(-) delete mode 100644 ApiDoc/Annotation/RpcApiDoc.php delete mode 100644 ApiDoc/Extractor/Provider/AbstractRpcMethodProvider.php delete mode 100644 ApiDoc/Extractor/Provider/MethodAnnotationsProvider.php create mode 100644 BankiruRpcServerBundle.php create mode 100644 Cache/RouterCacheWarmer.php create mode 100644 DependencyInjection/BankiruRpcServerExtension.php delete mode 100644 DependencyInjection/RpcExtension.php delete mode 100644 Resources/config/nelmio.yml create mode 100644 Routing/AttributesHelper.php create mode 100644 Routing/CollectionMatcher.php create mode 100644 Routing/MatcherDumper.php create mode 100644 Routing/MethodCollectionLoader.php create mode 100644 Routing/MethodMatcher.php create mode 100644 Routing/ResourceMethodCollectionLoader.php rename {Impl => Tests/Fixtures/Impl}/Request.php (95%) rename {Impl => Tests/Fixtures/Impl}/Response.php (93%) create mode 100644 Tests/Fixtures/Rpc/SecurityController.php diff --git a/.travis.yml b/.travis.yml index 154236b..e5373ea 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,9 +10,9 @@ php: env: - SYMFONY_VERSION='2.7.*' deps='no' - SYMFONY_VERSION='2.8.*' deps='no' - - SYMFONY_VERSION='3.0.*' deps='no' - - SYMFONY_VERSION='3.1.*' deps='no' - - SYMFONY_VERSION='~3.2@dev' deps='no' + - SYMFONY_VERSION='3.3.*' deps='no' + - SYMFONY_VERSION='~3.4@dev' deps='no' + - SYMFONY_VERSION='~4.0@dev' deps='no' matrix: include: diff --git a/ApiDoc/Annotation/RpcApiDoc.php b/ApiDoc/Annotation/RpcApiDoc.php deleted file mode 100644 index 5ede2c7..0000000 --- a/ApiDoc/Annotation/RpcApiDoc.php +++ /dev/null @@ -1,101 +0,0 @@ -setRpcMethod($data['rpc-method']); - } - if (isset($data['endpoint'])) { - $this->setEndpoint($data['endpoint']); - } - if (isset($data['classname'])) { - $this->setClassname($data['classname']); - } - } - - /** - * @return mixed - */ - public function getEndpoint() - { - return $this->endpoint; - } - - /** - * @param mixed $endpoint - */ - public function setEndpoint($endpoint) - { - $this->endpoint = $endpoint; - } - - public function getResource() - { - return $this->getClassname(); - } - - /** - * @return mixed - */ - public function getClassname() - { - return $this->classname; - } - - /** - * @param mixed $classname - */ - public function setClassname($classname) - { - $this->classname = $classname; - } - - /** - * @return array - */ - public function getStatusCodes() - { - $data = $this->toArray(); - - return array_key_exists('statusCodes', $data) ? $data['statusCodes'] : []; - } - - public function toArray() - { - $array = parent::toArray(); - $array['uri'] = $this->getRpcMethod() ? $this->getRpcMethod()->getMethod() : ''; -// $array['method'] = 'RPC'; - - return $array; - } - - public function getRpcMethod() - { - return $this->rpcMethod; - } - - public function setRpcMethod(Route $method) - { - $this->rpcMethod = $method; - } -} diff --git a/ApiDoc/Extractor/Provider/AbstractRpcMethodProvider.php b/ApiDoc/Extractor/Provider/AbstractRpcMethodProvider.php deleted file mode 100644 index 1faa192..0000000 --- a/ApiDoc/Extractor/Provider/AbstractRpcMethodProvider.php +++ /dev/null @@ -1,85 +0,0 @@ -resolver = $resolver; - $this->reader = $reader; - $this->httpCollection = $httpCollection; - } - - - /** - * @param Method $method - * @param string $endpoint - * - * @return RpcApiDoc - */ - protected function processMethod(Method $method, $endpoint) - { - /** @var string[] $views */ - $views = $method->getContext(); - if ($method->includeDefaultContext()) { - $views[] = 'Default'; - } - - $views[] = 'default'; - - $request = new Request($method, [], new ParameterBag(['_controller' => $method->getController()])); - - /** @var array $controller */ - $controller = $this->resolver->getController($request); - $refl = new \ReflectionMethod($controller[0], $controller[1]); - - /** @var RpcApiDoc $methodDoc */ - $methodDoc = $this->reader->getMethodAnnotation($refl, RpcApiDoc::class); - - if (null === $methodDoc) { - $methodDoc = new RpcApiDoc(['resource' => $endpoint]); - } - $methodDoc = clone $methodDoc; - - $methodDoc->setEndpoint($endpoint); - $methodDoc->setRpcMethod($method); - if (null === $methodDoc->getSection()) { - $methodDoc->setSection($endpoint); - } - foreach ($views as $view) { - $methodDoc->addView($view); - } - - $route = new Route($endpoint); - $route->setMethods([$endpoint]); - $route->setDefault('_controller', (get_class($controller[0]) . '::' . $controller[1])); - $methodDoc->setRoute($route); - - return $methodDoc; - } -} diff --git a/ApiDoc/Extractor/Provider/MethodAnnotationsProvider.php b/ApiDoc/Extractor/Provider/MethodAnnotationsProvider.php deleted file mode 100644 index bc8fd68..0000000 --- a/ApiDoc/Extractor/Provider/MethodAnnotationsProvider.php +++ /dev/null @@ -1,49 +0,0 @@ -routerCollection = $routerCollection; - parent::__construct($resolver, $reader, $router->getRouteCollection()); - } - - /** - * Returns an array ApiDoc annotations. - * - * @return \Nelmio\ApiDocBundle\Annotation\ApiDoc[] - */ - public function getAnnotations() - { - $docs = []; - foreach ($this->routerCollection->all() as $endpoint => $router) { - foreach ($router->getCollection()->all() as $method) { - $docs[] = $this->processMethod($method, $endpoint); - } - } - - return $docs; - } -} diff --git a/BankiruRpcServerBundle.php b/BankiruRpcServerBundle.php new file mode 100644 index 0000000..53c0b9a --- /dev/null +++ b/BankiruRpcServerBundle.php @@ -0,0 +1,22 @@ +addCompilerPass(new RouterLoaderPass()); + } + + public function getContainerExtension() + { + return new BankiruRpcServerExtension(); + } +} diff --git a/Cache/RouterCacheWarmer.php b/Cache/RouterCacheWarmer.php new file mode 100644 index 0000000..59aad56 --- /dev/null +++ b/Cache/RouterCacheWarmer.php @@ -0,0 +1,50 @@ +router = $router; + } + + /** + * Warms up the cache. + * + * @param string $cacheDir The cache directory + */ + public function warmUp($cacheDir) + { + if ($this->router instanceof WarmableInterface) { + $this->router->warmUp($cacheDir); + } + } + + /** + * Checks whether this warmer is optional or not. + * + * @return bool always true + */ + public function isOptional() + { + return true; + } +} diff --git a/Command/RouterDebugCommand.php b/Command/RouterDebugCommand.php index 7dde510..e39dc7f 100644 --- a/Command/RouterDebugCommand.php +++ b/Command/RouterDebugCommand.php @@ -16,11 +16,13 @@ protected function configure() parent::configure(); $this->setName('debug:rpc_router'); $this->setDescription('Display essential info about RPC routing'); - $this->addOption('endpoint', - 'p', - InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, - 'Filter endpoint', - null); + $this->addOption( + 'endpoint', + 'p', + InputOption::VALUE_OPTIONAL | InputOption::VALUE_IS_ARRAY, + 'Filter endpoint', + null + ); } protected function execute(InputInterface $input, OutputInterface $output) @@ -71,7 +73,7 @@ protected function getRoutes() $collection = $this->getContainer()->get('rpc.router.collection'); foreach ($collection->all() as $endpoint => $router) { - $routes[$endpoint] = $router->getCollection()->all(); + $routes[$endpoint] = $router->getMethodCollection()->all(); } return $routes; diff --git a/Controller/RpcController.php b/Controller/RpcController.php index c5e2772..11dedb4 100644 --- a/Controller/RpcController.php +++ b/Controller/RpcController.php @@ -39,7 +39,7 @@ abstract class RpcController implements ContainerAwareInterface public function setContainer(ContainerInterface $container = null) { $this->container = $container; - $this->dispatcher = $container->get('event_dispatcher'); + $this->dispatcher = $this->get('event_dispatcher'); } /** @@ -115,25 +115,6 @@ protected function handleSingleRequest(RequestInterface $request) return $this->filterResponse($response, $request); } - /** - * @return KernelInterface - * @throws \RuntimeException - */ - private function getKernel() - { - try { - /** @var KernelInterface|null $kernel */ - $kernel = $this->get('kernel'); - } catch (ExceptionInterface $e) { - throw new \RuntimeException('Cannot obtain Kernel from container: ' . $e->getMessage(), $e->getCode(), $e); - } - if (null === $kernel) { - throw new \RuntimeException('Cannot obtain Kernel from container'); - } - - return $kernel; - } - /** * @param $name * @@ -250,4 +231,23 @@ protected function handleException(\Exception $e, RequestInterface $request) return $response; } } + + /** + * @return KernelInterface + * @throws \RuntimeException + */ + private function getKernel() + { + try { + /** @var KernelInterface|null $kernel */ + $kernel = $this->get('kernel'); + } catch (ExceptionInterface $e) { + throw new \RuntimeException('Cannot obtain Kernel from container: ' . $e->getMessage(), $e->getCode(), $e); + } + if (null === $kernel) { + throw new \RuntimeException('Cannot obtain Kernel from container'); + } + + return $kernel; + } } diff --git a/Controller/RpcControllerNameParser.php b/Controller/RpcControllerNameParser.php index 99b69a7..7b76a93 100644 --- a/Controller/RpcControllerNameParser.php +++ b/Controller/RpcControllerNameParser.php @@ -8,7 +8,7 @@ /** * ControllerNameParser converts controller from the short notation a:b:c * (BlogBundle:Post:index) to a fully-qualified class::method string - * (Bundle\BlogBundle\Controller\PostController::indexAction). + * (Bundle\BlogBundle\Rpc\PostController::indexAction). * * @author Fabien Potencier * @author Pavel Batanov @@ -32,12 +32,14 @@ public function parse($controller) { $originalController = $controller; if (3 !== count($parts = explode(':', $controller))) { - throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller)); + throw new \InvalidArgumentException( + sprintf('The "%s" controller is not a valid "a:b:c" controller string.', $controller) + ); } list($bundle, $controller, $action) = $parts; $controller = str_replace('/', '\\', $controller); - $bundles = array(); + $bundles = []; try { // this throws an exception if there is no such bundle @@ -63,11 +65,25 @@ public function parse($controller) } $bundles[] = $b->getName(); - $msg = sprintf('The _controller value "%s:%s:%s" maps to a "%s" class, but this class was not found. Create this class or check the spelling of the class and its namespace.', $bundle, $controller, $action, $try); + $msg = + sprintf( + 'The _controller value "%s:%s:%s" maps to a "%s" class, but this class was not found. ' . + 'Create this class or check the spelling of the class and its namespace.', + $bundle, + $controller, + $action, + $try + ); } if (count($bundles) > 1) { - $msg = sprintf('Unable to find controller "%s:%s" in bundles %s.', $bundle, $controller, implode(', ', $bundles)); + $msg = + sprintf( + 'Unable to find controller "%s:%s" in bundles %s.', + $bundle, + $controller, + implode(', ', $bundles) + ); } throw new \InvalidArgumentException($msg); @@ -76,13 +92,15 @@ public function parse($controller) /** {@inheritdoc} */ public function build($controller) { - if (0 === preg_match('#^(.*?\\\\Controller\\\\(.+)Controller)::(.+)Action$#', $controller, $match)) { - throw new \InvalidArgumentException(sprintf('The "%s" controller is not a valid "class::method" string.', $controller)); + if (0 === preg_match($this->getDefaultControllerPattern(), $controller, $match)) { + throw new \InvalidArgumentException( + sprintf('The "%s" controller is not a valid "class::method" string.', $controller) + ); } - $className = $match[1]; + $className = $match[1]; $controllerName = $match[2]; - $actionName = $match[3]; + $actionName = $match[3]; foreach ($this->kernel->getBundles() as $name => $bundle) { if (0 !== strpos($className, $bundle->getNamespace())) { continue; @@ -91,7 +109,39 @@ public function build($controller) return sprintf('%s:%s:%s', $name, $controllerName, $actionName); } - throw new \InvalidArgumentException(sprintf('Unable to find a bundle that defines controller "%s".', $controller)); + throw new \InvalidArgumentException( + sprintf('Unable to find a bundle that defines controller "%s".', $controller) + ); + } + + /** + * @param BundleInterface $bundle + * @param string $controller + * + * @return string Guessed controller FQCN + */ + protected function guessControllerClassName(BundleInterface $bundle, $controller) + { + return $bundle->getNamespace() . '\\Controller\\' . $controller . 'Controller'; + } + + /** + * @param string $controller + * @param string $action + * + * @return string guessed FQCN::function string + */ + protected function guessActionString($controller, $action) + { + return $controller . '::' . $action . 'Action'; + } + + /** + * @return string + */ + protected function getDefaultControllerPattern() + { + return '#^(.*?\\\\Rpc\\\\(.+)Controller)::(.+)Action$#'; } /** @@ -103,12 +153,15 @@ public function build($controller) */ private function findAlternative($nonExistentBundleName) { - $bundleNames = array_map(function (BundleInterface $b) { - return $b->getName(); - }, $this->kernel->getBundles()); + $bundleNames = array_map( + function (BundleInterface $b) { + return $b->getName(); + }, + $this->kernel->getBundles() + ); $alternative = null; - $shortest = null; + $shortest = null; foreach ($bundleNames as $bundleName) { // if there's a partial match, return it immediately if (false !== strpos($bundleName, $nonExistentBundleName)) { @@ -123,24 +176,4 @@ private function findAlternative($nonExistentBundleName) return $alternative; } - - /** - * @param BundleInterface $bundle - * @param string $controller - * @return string Guessed controller FQCN - */ - protected function guessControllerClassName(BundleInterface $bundle, $controller) - { - return $bundle->getNamespace().'\\Controller\\'.$controller.'Controller'; - } - - /** - * @param string $controller - * @param string $action - * @return string guessed FQCN::function string - */ - protected function guessActionString($controller, $action) - { - return $controller.'::'.$action.'Action'; - } } diff --git a/DependencyInjection/BankiruRpcServerExtension.php b/DependencyInjection/BankiruRpcServerExtension.php new file mode 100644 index 0000000..b6d6071 --- /dev/null +++ b/DependencyInjection/BankiruRpcServerExtension.php @@ -0,0 +1,95 @@ +getParameter('kernel.bundles'))) { + $loader->load('nelmio.yml'); + } + + if ($container->has('security.authorization_checker')) { + $loader->load('security.yml'); + } + + $configuration = new Configuration(); + $config = $this->processConfiguration($configuration, $configs); + + $loader->load('rpc.yml'); + + $this->configureRouter($config['router'], $container); + } + + public function getAlias() + { + return 'rpc'; + } + + /** + * @param array $router + * @param ContainerBuilder $container + */ + private function configureRouter(array $router, ContainerBuilder $container) + { + $endpoints = $router['endpoints']; + $endpointLoader = $container->getDefinition('rpc.router.loader'); + $routerCollection = $container->getDefinition('rpc.router.collection'); + + foreach ($endpoints as $name => $config) { + $routerId = sprintf('rpc.endpoint_router.%s', $name); + + $container->register($routerId, Router::class) + ->setArguments( + [ + new Definition( + ResourceMethodCollectionLoader::class, + [ + new Reference('rpc.router.resolver'), + $config['resources'], + $config['context'], + ] + ), + $name, + [ + 'cache_dir' => '%kernel.cache_dir%', + ], + ] + ) + ->setPublic(false) + ->addTag('rpc_router'); + + $container->register(sprintf('rpc.router_warmer.%s', $name), RouterCacheWarmer::class) + ->setPublic(false) + ->setArguments( + [ + new Reference($routerId), + ] + ) + ->addTag('kernel.cache_warmer'); + + $container->register('rpc.router_listener.' . $name, RouterListener::class) + ->setArguments([$name, new Reference($routerId)]) + ->addTag('kernel.event_listener', ['event' => 'rpc.request', 'method' => 'onRequest']); + + $endpointLoader->addMethodCall('addEndpoint', [$name, $config]); + $routerCollection->addMethodCall('addRouter', [$name, new Reference($routerId)]); + } + } +} diff --git a/DependencyInjection/RpcExtension.php b/DependencyInjection/RpcExtension.php deleted file mode 100644 index 9c42534..0000000 --- a/DependencyInjection/RpcExtension.php +++ /dev/null @@ -1,91 +0,0 @@ -getParameter('kernel.bundles'))) { - $loader->load('nelmio.yml'); - } - - if ($container->has('security.authorization_checker')) { - $loader->load('security.yml'); - } - - $configuration = new Configuration(); - $config = $this->processConfiguration($configuration, $configs); - - - $loader->load('rpc.yml'); - - $this->configureRouter($config['router'], $container); - } - - /** - * @param array $router - * @param ContainerBuilder $container - */ - private function configureRouter(array $router, ContainerBuilder $container) - { - $endpoints = $router['endpoints']; - - $endpointLoader = $container->getDefinition('rpc.router.loader'); - - $routerCollection = $container->getDefinition('rpc.router.collection'); - - foreach ($endpoints as $name => $config) { - - $collection = new Definition(MethodCollection::class); - $endpointRouter = new Definition( - Router::class, - [ - $container->getDefinition('rpc.router.resolver'), - $config['resources'], - $collection, - $config['context'] - ] - ); - - $endpointRouter->addTag('rpc_router'); - $endpointLoader->addMethodCall('addEndpoint', [$name, $config]); - $this->configureRouteListener($container, $name, $endpointRouter); - - //@todo review (@scaytrase) - $container->setDefinition(sprintf('rpc.endpoint_router.%s', $name), $endpointRouter); - - $routerCollection->addMethodCall('addRouter', [$name, $endpointRouter]); - } - } - - /** - * @param ContainerBuilder $container - * @param string $name - * @param Definition $endpointRouter - */ - private function configureRouteListener(ContainerBuilder $container, $name, Definition $endpointRouter) - { - $listener = new Definition(RouterListener::class, [$name, $endpointRouter]); - $listener->addTag('kernel.event_listener', ['event' => 'rpc.request', 'method' => 'onRequest']); - $container->setDefinition('rpc.router_listener.' . $name, $listener); - } - - public function getAlias() - { - return 'rpc'; - } -} diff --git a/Event/RpcResponseEvent.php b/Event/RpcResponseEvent.php index b217ff7..7abcb1f 100644 --- a/Event/RpcResponseEvent.php +++ b/Event/RpcResponseEvent.php @@ -28,5 +28,7 @@ public function getResponse() public function setResponse(RpcResponseInterface $response) { $this->response = $response; + + $this->stopPropagation(); } } diff --git a/Exception/AccessDeniedException.php b/Exception/AccessDeniedException.php index 3f34d8a..afb2cb3 100644 --- a/Exception/AccessDeniedException.php +++ b/Exception/AccessDeniedException.php @@ -11,6 +11,11 @@ class AccessDeniedException extends \RuntimeException implements RpcException */ public static function rolesNeeded(array $roles) { - return new static('Insufficient privileges: required one of ' . implode(', ', $roles)); + return new static( + sprintf( + 'Insufficient privileges: required one of %s role to access this method', + implode(', ', $roles) + ) + ); } } diff --git a/Listener/RouterListener.php b/Listener/RouterListener.php index a76d52c..b2b44b7 100644 --- a/Listener/RouterListener.php +++ b/Listener/RouterListener.php @@ -32,14 +32,6 @@ public function onRequest(GetResponseEvent $event) } $method = $event->getRequest()->getMethod(); - $route = $this->router->getCollection()->match($method); - if (null === $route) { - return; - } - - $event->getRequest()->getAttributes()->set('_route', $route); - $event->getRequest()->getAttributes()->set('_controller', $route->getController()); - $event->getRequest()->getAttributes()->set('_with_default_context', $route->includeDefaultContext()); - $event->getRequest()->getAttributes()->set('_context', $route->getContext()); + $event->getRequest()->getAttributes()->replace($this->router->match($method)); } } diff --git a/Listener/SecurityListener.php b/Listener/SecurityListener.php index 8e7bded..418d4e8 100644 --- a/Listener/SecurityListener.php +++ b/Listener/SecurityListener.php @@ -12,6 +12,16 @@ final class SecurityListener /** @var AuthorizationChecker */ private $authenticator; + /** + * SecurityListener constructor. + * + * @param AuthorizationChecker $authenticator + */ + public function __construct(AuthorizationChecker $authenticator) + { + $this->authenticator = $authenticator; + } + public function onFilterController(FilterControllerEvent $event) { /** @var Route $route */ diff --git a/Resources/config/nelmio.yml b/Resources/config/nelmio.yml deleted file mode 100644 index a744c2b..0000000 --- a/Resources/config/nelmio.yml +++ /dev/null @@ -1,12 +0,0 @@ -services: - api_doc.rpc.router_provider: - class: Bankiru\Api\Rpc\ApiDoc\Extractor\Provider\MethodAnnotationsProvider - public: false - arguments: - - "@annotation_reader" - - "@rpc.controller_resolver" - - "@rpc.router.collection" - - "@router" - tags: - - {name: nelmio_api_doc.extractor.annotations_provider} - diff --git a/Routing/Annotation/Method.php b/Routing/Annotation/Method.php index 9ce652f..4cec2b7 100644 --- a/Routing/Annotation/Method.php +++ b/Routing/Annotation/Method.php @@ -17,6 +17,7 @@ class Method public $context = []; public $defaultContext = true; public $inherit = true; + public $options = []; public function __construct(array $values) { @@ -121,4 +122,20 @@ public function setInherit($inherit) { $this->inherit = (bool)$inherit; } + + /** + * @return array + */ + public function getOptions() + { + return $this->options; + } + + /** + * @param array $options + */ + public function setOptions(array $options = null) + { + $this->options = $options; + } } diff --git a/Routing/AttributesHelper.php b/Routing/AttributesHelper.php new file mode 100644 index 0000000..660518c --- /dev/null +++ b/Routing/AttributesHelper.php @@ -0,0 +1,24 @@ + $name, + '_controller' => $method->getController(), + '_with_default_context' => $method->includeDefaultContext(), + '_context' => $method->getContext(), + ]; + } +} diff --git a/Routing/CollectionMatcher.php b/Routing/CollectionMatcher.php new file mode 100644 index 0000000..07a304f --- /dev/null +++ b/Routing/CollectionMatcher.php @@ -0,0 +1,33 @@ +collection = $collection; + } + + /** {@inheritdoc} */ + public function match($method) + { + if (!$this->collection->has($method)) { + throw new MethodNotFoundException($method); + } + + return AttributesHelper::getAttributes($this->collection->get($method), $method); + } +} diff --git a/Routing/Loader/AnnotationClassLoader.php b/Routing/Loader/AnnotationClassLoader.php index aa7ad22..b214215 100644 --- a/Routing/Loader/AnnotationClassLoader.php +++ b/Routing/Loader/AnnotationClassLoader.php @@ -24,7 +24,6 @@ public function __construct(Reader $reader) $this->reader = $reader; } - /** {@inheritdoc} */ public function load($class, $type = null) { @@ -48,7 +47,7 @@ public function load($class, $type = null) if (!$method->isPublic()) { continue; } - + foreach ($this->reader->getMethodAnnotations($method) as $annot) { if ($annot instanceof Method) { $this->addRoute($collection, $annot, $parents, $class, $method); @@ -61,12 +60,31 @@ public function load($class, $type = null) return $collection; } + /** {@inheritdoc} */ + public function supports($resource, $type = null) + { + return is_string($resource) && + preg_match('/^(?:\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)+$/', $resource) && + (!$type || 'annotation' === $type); + } + + /** {@inheritdoc} */ + public function getResolver() + { + } + + /** {@inheritdoc} */ + public function setResolver($resolver) + { + } + protected function getParentAnnotations(\ReflectionClass $class) { $parents = [ 'method' => '', 'context' => [], 'default_context' => true, + 'options' => [], ]; /** @var Method $annot */ @@ -78,6 +96,10 @@ protected function getParentAnnotations(\ReflectionClass $class) if (null !== $annot->getContext()) { $parents['context'] = $annot->getContext(); } + + if (null !== $annot->getOptions()) { + $parents['options'] = $annot->getOptions(); + } } return $parents; @@ -89,8 +111,7 @@ protected function addRoute( array $parents, \ReflectionClass $class, \ReflectionMethod $method - ) - { + ) { $collection->add( $annot->getMethod(), new Route( @@ -98,26 +119,9 @@ protected function addRoute( $class->getName() . '::' . $method->getName(), array_merge($parents['context'], $annot->getContext()), $parents['default_context'] && $annot->isDefaultContext(), - $annot->isInherit() + $annot->isInherit(), + array_merge_recursive($parents['options'], $annot->getOptions()) ) ); } - - /** {@inheritdoc} */ - public function supports($resource, $type = null) - { - return is_string($resource) && - preg_match('/^(?:\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)+$/', $resource) && - (!$type || 'annotation' === $type); - } - - /** {@inheritdoc} */ - public function getResolver() - { - } - - /** {@inheritdoc} */ - public function setResolver($resolver) - { - } } diff --git a/Routing/LoaderResolver.php b/Routing/LoaderResolver.php index 8ce7e3b..286ccd9 100644 --- a/Routing/LoaderResolver.php +++ b/Routing/LoaderResolver.php @@ -2,7 +2,7 @@ namespace Bankiru\Api\Rpc\Routing; -class LoaderResolver implements LoaderResolverInterface +final class LoaderResolver implements LoaderResolverInterface { /** * @var LoaderInterface[] An array of LoaderInterface objects diff --git a/Routing/MatcherDumper.php b/Routing/MatcherDumper.php new file mode 100644 index 0000000..7298975 --- /dev/null +++ b/Routing/MatcherDumper.php @@ -0,0 +1,53 @@ +all() as $name => $method) { + $ret = var_export(AttributesHelper::getAttributes($method, $name), true); + $content .= PHP_EOL . + <<getMethod()}' => {$ret}, +CONTENT; + } + + $content .= + <<resources); } - /** - * Gets the current RouteCollection as an Iterator that includes all routes. - * - * It implements \IteratorAggregate. - * - * @see all() - * - * @return \ArrayIterator An \ArrayIterator object for iterating over routes - */ + /** {@inheritdoc} */ public function getIterator() { return new \ArrayIterator($this->routes); } - /** - * Gets the number of Routes in this collection. - * - * @return int The number of routes - */ + /** {@inheritdoc} */ public function count() { return count($this->routes); @@ -126,21 +114,8 @@ public function count() /** * @param ResourceInterface $resource */ - public function addResource(ResourceInterface $resource) { $this->resources[] = $resource; } - - /** - * @param string $method - * - * @return Route|null - */ - public function match($method) + public function addResource(ResourceInterface $resource) { - foreach ($this->routes as $route) { - if ($route->getMethod() === $method) { - return $route; - } - } - - return null; + $this->resources[] = $resource; } } diff --git a/Routing/MethodCollectionLoader.php b/Routing/MethodCollectionLoader.php new file mode 100644 index 0000000..c0c90ae --- /dev/null +++ b/Routing/MethodCollectionLoader.php @@ -0,0 +1,13 @@ +resources = $resources; + $this->resolver = $resolver; + $this->context = $context; + } + + /** {@inheritdoc} */ + public function loadCollection() + { + $collection = new MethodCollection(); + + foreach ($this->resources as $resource) { + $loader = $this->resolver->resolve($resource); + + if (false === $loader) { + throw new \RuntimeException(sprintf('Could not resolve loader for resource "%s"', $resource)); + } + + $collection->addCollection($loader->load($resource)); + } + + foreach ($this->context as $item) { + $collection->addContext($item); + } + + return $collection; + } +} diff --git a/Routing/Router.php b/Routing/Router.php index 5494672..122af2c 100644 --- a/Routing/Router.php +++ b/Routing/Router.php @@ -2,51 +2,199 @@ namespace Bankiru\Api\Rpc\Routing; -class Router +use Symfony\Component\Config\ConfigCacheFactory; +use Symfony\Component\Config\ConfigCacheFactoryInterface; +use Symfony\Component\Config\ConfigCacheInterface; +use Symfony\Component\Config\Resource\FileResource; +use Symfony\Component\Config\Resource\ReflectionClassResource; +use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface; + +final class Router implements MethodMatcher, WarmableInterface { - /** @var MethodCollection */ + /** @var MethodCollectionLoader */ + private $loader; + /** @var MethodCollection */ private $collection; + /** @var MethodMatcher */ + private $matcher; + /** + * @var string + */ + private $name; + /** @var ConfigCacheFactoryInterface */ + private $configCacheFactory; + private $options = []; /** * Router constructor. * - * @param LoaderResolverInterface $resolver - * @param array $resources - * @param MethodCollection|null $collection - * - * @param array $context + * @param MethodCollectionLoader $loader + * @param string $name */ - public function __construct( - LoaderResolverInterface $resolver, - array $resources = [], - MethodCollection $collection = null, - array $context = [] - ) + public function __construct(MethodCollectionLoader $loader, $name, array $options = []) { - $this->collection = $collection; + $this->loader = $loader; + $this->name = $name; + $this->setOptions($options); + } + /** + * @return MethodCollection + */ + public function getMethodCollection() + { if (null === $this->collection) { - $this->collection = new MethodCollection(); + $this->collection = $this->loader->loadCollection(); + } + + return $this->collection; + } + + /** {@inheritdoc} */ + public function match($method) + { + return $this->getMatcher()->match($method); + } + + /** + * Warms up the cache. + * + * @param string $cacheDir The cache directory + */ + public function warmUp($cacheDir) + { + $currentDir = $this->getOption('cache_dir'); + + // force cache generation + $this->setOption('cache_dir', $cacheDir); + + $this->getMatcher(); + $this->setOption('cache_dir', $currentDir); + } + + /** + * Sets an option. + * + * @param string $key The key + * @param mixed $value The value + * + * @throws \InvalidArgumentException + */ + public function setOption($key, $value) + { + if (!array_key_exists($key, $this->options)) { + throw new \InvalidArgumentException(sprintf('The Router does not support the "%s" option.', $key)); + } + + $this->options[$key] = $value; + } + + /** + * Gets an option value. + * + * @param string $key The key + * + * @return mixed The value + * + * @throws \InvalidArgumentException + */ + public function getOption($key) + { + if (!array_key_exists($key, $this->options)) { + throw new \InvalidArgumentException(sprintf('The Router does not support the "%s" option.', $key)); } - foreach ($resources as $resource) { - $loader = $resolver->resolve($resource); - if (false === $loader) { - throw new \RuntimeException(sprintf('Could not resolve loader for resource "%s"', $resource)); + return $this->options[$key]; + } + + /** + * Sets options. + * + * Available options: + * + * * cache_dir: The cache directory (or null to disable caching) + * * debug: Whether to enable debugging or not (false by default) + * + * @param array $options An array of options + * + * @throws \InvalidArgumentException When unsupported option is provided + */ + public function setOptions(array $options) + { + $this->options = [ + 'cache_dir' => null, + 'debug' => false, + 'matcher_cache_class' => ucfirst($this->name) . 'MethodMatcher', + ]; + + // check option names and live merge, if errors are encountered Exception will be thrown + $invalid = []; + foreach ($options as $key => $value) { + if (array_key_exists($key, $this->options)) { + $this->options[$key] = $value; + } else { + $invalid[] = $key; } - $this->collection->addCollection($loader->load($resource)); } - foreach ($context as $item) { - $this->collection->addContext($item); + if ($invalid) { + throw new \InvalidArgumentException( + sprintf('The Router does not support the following options: "%s".', implode('", "', $invalid)) + ); } } /** - * @return MethodCollection + * @return MethodMatcher */ - public function getCollection() + private function getMatcher() { - return $this->collection; + if (null !== $this->matcher) { + return $this->matcher; + } + + if (null === $this->options['cache_dir'] || null === $this->options['matcher_cache_class']) { + $this->matcher = new CollectionMatcher($this->getMethodCollection()); + + return $this->matcher; + } + + $cache = $this->getConfigCacheFactory()->cache( + $this->options['cache_dir'] . '/rpc/' . $this->options['matcher_cache_class'] . '.php', + function (ConfigCacheInterface $cache) { + $dumper = new MatcherDumper(); + + $options = [ + 'class' => $this->options['matcher_cache_class'], + ]; + + $resources = $this->getMethodCollection()->getResources(); + $refl = new \ReflectionClass(MatcherDumper::class); + $resources[] = new FileResource($refl->getFileName()); + + $cache->write($dumper->dump($this->getMethodCollection(), $options), $resources); + } + ); + + require_once $cache->getPath(); + + $this->matcher = new $this->options['matcher_cache_class']($cache); + + return $this->matcher; + } + + /** + * Provides the ConfigCache factory implementation, falling back to a + * default implementation if necessary. + * + * @return ConfigCacheFactoryInterface $configCacheFactory + */ + private function getConfigCacheFactory() + { + if (null === $this->configCacheFactory) { + $this->configCacheFactory = new ConfigCacheFactory(true); + } + + return $this->configCacheFactory; } } diff --git a/Routing/RouterCollection.php b/Routing/RouterCollection.php index 19f259e..a3578e3 100644 --- a/Routing/RouterCollection.php +++ b/Routing/RouterCollection.php @@ -2,7 +2,7 @@ namespace Bankiru\Api\Rpc\Routing; -class RouterCollection +final class RouterCollection { /** @var Router[] */ private $routers = []; diff --git a/RpcBundle.php b/RpcBundle.php index 226efad..36fe3cc 100644 --- a/RpcBundle.php +++ b/RpcBundle.php @@ -2,15 +2,7 @@ namespace Bankiru\Api\Rpc; -use Bankiru\Api\Rpc\DependencyInjection\Compiler\RouterLoaderPass; -use Symfony\Component\DependencyInjection\ContainerBuilder; -use Symfony\Component\HttpKernel\Bundle\Bundle; - -class RpcBundle extends Bundle +/** @deprecated */ +final class RpcBundle extends BankiruRpcServerBundle { - public function build(ContainerBuilder $container) - { - parent::build($container); - $container->addCompilerPass(new RouterLoaderPass()); - } } diff --git a/Impl/Request.php b/Tests/Fixtures/Impl/Request.php similarity index 95% rename from Impl/Request.php rename to Tests/Fixtures/Impl/Request.php index b4fa90c..6b3e462 100644 --- a/Impl/Request.php +++ b/Tests/Fixtures/Impl/Request.php @@ -1,6 +1,6 @@ getContainer()->get('rpc.endpoint_router.test'); self::assertNotNull($router); /** @var MethodCollection $collection */ - $collection = $router->getCollection(); + $collection = $router->getMethodCollection(); self::assertNotNull($router); self::assertInstanceOf(MethodCollection::class, $collection); diff --git a/composer.json b/composer.json index 6c8fd68..cc5b797 100644 --- a/composer.json +++ b/composer.json @@ -29,8 +29,7 @@ }, "suggest": { "symfony/console": "For using debug console command", - "symfony/security": "For enabling role and expressions based security filters", - "nelmio/api-doc-bundle": "For RpcApiDoc extraction" + "symfony/security": "For enabling role and expressions based security filters" }, "autoload": { "psr-4": { From 927a796c01cc92e2783bc2963f875857c7b8c8dd Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Thu, 22 Jun 2017 16:35:57 +0300 Subject: [PATCH 04/22] Deps --- .travis.yml | 7 ++++++- README.md | 2 +- composer.json | 20 ++++++++++---------- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/.travis.yml b/.travis.yml index e5373ea..fcf4a32 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,20 +4,25 @@ php: - 5.5 - 5.6 - 7 + - 7.1 - nightly - hhvm env: - SYMFONY_VERSION='2.7.*' deps='no' - SYMFONY_VERSION='2.8.*' deps='no' + - SYMFONY_VERSION='3.2.*' deps='no' - SYMFONY_VERSION='3.3.*' deps='no' - SYMFONY_VERSION='~3.4@dev' deps='no' - - SYMFONY_VERSION='~4.0@dev' deps='no' matrix: include: - php: 5.5 env: SYMFONY_VERSION='2.7.*' deps='low' + - php: 7.1 + env: SYMFONY_VERSION='~4.0@dev' deps='no' + - php: nightly + env: SYMFONY_VERSION='~4.0@dev' deps='no' allow_failures: - php: hhvm diff --git a/README.md b/README.md index 2c6072b..7d026a4 100644 --- a/README.md +++ b/README.md @@ -52,7 +52,7 @@ rpc: my-public-endpoint: path: / defaults: - _controller: JsonRpcBundle:JsonRpc:jsonRpc + _controller: BankiruJsonRpcServerBundle:JsonRpc:jsonRpc _format: json context: Default resources: diff --git a/composer.json b/composer.json index cc5b797..148677c 100644 --- a/composer.json +++ b/composer.json @@ -12,20 +12,20 @@ ], "require": { "php": "~5.5 || ~7.0", - "symfony/config": "~2.7 || ~3.0", - "symfony/http-kernel": "~2.7 || ~3.0", - "symfony/yaml": "~2.7 || ~3.0", - "symfony/dependency-injection": "~2.7 || ~3.0", + "symfony/config": "~2.7 || ~3.0 || ~4.0", + "symfony/http-kernel": "~2.7 || ~3.0 || ~4.0", + "symfony/yaml": "~2.7 || ~3.0 || ~4.0", + "symfony/dependency-injection": "~2.7 || ~3.0 || ~4.0", "doctrine/annotations": "~1.2", "scaytrase/rpc-common": "~1.0" }, "require-dev": { - "symfony/console": "~2.7 || ~3.0", - "symfony/routing": "~2.7 || ~3.0", - "symfony/framework-bundle": "~2.7 || ~3.0", - "symfony/security-bundle": "~2.7 || ~3.0", - "phpunit/phpunit": "~4.8 || ~5.1", - "symfony/browser-kit": "~2.7 || ~3.0" + "symfony/console": "~2.7 || ~3.0 || ~4.0", + "symfony/routing": "~2.7 || ~3.0 || ~4.0", + "symfony/framework-bundle": "~2.7 || ~3.0 || ~4.0", + "symfony/security-bundle": "~2.7 || ~3.0 || ~4.0", + "symfony/browser-kit": "~2.7 || ~3.0 || ~4.0", + "phpunit/phpunit": "~4.8 || ~5.1" }, "suggest": { "symfony/console": "For using debug console command", From 44d30748a0c674dd20f8c830df0583f432041a15 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Thu, 22 Jun 2017 17:44:54 +0300 Subject: [PATCH 05/22] Allow phpunit6 --- Tests/EndpointRouteLoaderTest.php | 3 +- Tests/RpcTest.php | 71 +++++++++++++++++++++---------- composer.json | 2 +- 3 files changed, 51 insertions(+), 25 deletions(-) diff --git a/Tests/EndpointRouteLoaderTest.php b/Tests/EndpointRouteLoaderTest.php index 0415258..c743d7d 100644 --- a/Tests/EndpointRouteLoaderTest.php +++ b/Tests/EndpointRouteLoaderTest.php @@ -3,10 +3,11 @@ namespace Bankiru\Api\Rpc\Tests; use Bankiru\Api\Rpc\Http\Routing\EndpointRouteLoader; +use PHPUnit\Framework\TestCase; use Symfony\Component\Config\Loader\LoaderResolver; use Symfony\Component\Routing\RouteCollection; -class EndpointRouteLoaderTest extends \PHPUnit_Framework_TestCase +class EndpointRouteLoaderTest extends TestCase { public function testLoader() { diff --git a/Tests/RpcTest.php b/Tests/RpcTest.php index f973730..20f55c8 100644 --- a/Tests/RpcTest.php +++ b/Tests/RpcTest.php @@ -10,44 +10,62 @@ class RpcTest extends WebTestCase { - public function getArgumentVariants() + public function getValidArgumentVariants() { return [ - 'missing all' => [[], false], - 'missing array' => [['noDefault' => 1], false], - 'not an array' => [['noDefault' => 1, 'array' => 2], false], - 'correct' => [['noDefault' => 1, 'array' => ['test1' => 2, 'abc']], true], - 'correct 2' => [['noDefault' => 1, 'array' => ['test', 'test2']], true], + 'correct' => [['noDefault' => 1, 'array' => ['test1' => 2, 'abc']]], + 'correct 2' => [['noDefault' => 1, 'array' => ['test', 'test2']]], 'correct w explicit default override' => [ ['default' => 'new_value', 'noDefault' => 1, 'array' => []], - true, ], ]; } + public function getInvalidArgumentVariants() + { + return [ + 'missing all' => [[]], + 'missing array' => [['noDefault' => 1]], + 'not an array' => [['noDefault' => 1, 'array' => 2]], + ]; + } + /** - * @dataProvider getArgumentVariants + * @dataProvider getValidArgumentVariants * * @param array $args - * @param bool $valid * * @throws InvalidMethodParametersException - * @throws \Error */ - public function testController(array $args, $valid) + public function testValidController(array $args) { $client = self::createClient(); - try { - $client->request( - 'POST', - '/test/', - array_replace(['method' => 'test/method',], $args) - ); - } catch (InvalidMethodParametersException $e) { - if ($valid) { - throw $e; - } - } + + $client->request( + 'POST', + '/test/', + array_replace(['method' => 'test/method',], $args) + ); + + self::assertTrue($client->getResponse()->isSuccessful()); + } + + /** + * @dataProvider getInvalidArgumentVariants + * + * @param array $args + * + * @expectedException \Bankiru\Api\Rpc\Exception\InvalidMethodParametersException + */ + public function testInvalidController(array $args) + { + $client = self::createClient(); + + $client->request( + 'POST', + '/test/', + array_replace(['method' => 'test/method',], $args) + ); } public function getExceptionTestData() @@ -75,7 +93,14 @@ public function getExceptionTestData() */ public function testException($endpoint, $args, $exception) { - $this->setExpectedExceptionRegExp($exception); + if (method_exists($this, 'expectException')) { + $this->expectException($exception); + } elseif (method_exists($this, 'setExpectedException')) { + $this->setExpectedException($exception); + } else { + throw new \BadMethodCallException('Unsupported PHPUnit version'); + } + $client = self::createClient(); $client->request( diff --git a/composer.json b/composer.json index 148677c..8725d3d 100644 --- a/composer.json +++ b/composer.json @@ -25,7 +25,7 @@ "symfony/framework-bundle": "~2.7 || ~3.0 || ~4.0", "symfony/security-bundle": "~2.7 || ~3.0 || ~4.0", "symfony/browser-kit": "~2.7 || ~3.0 || ~4.0", - "phpunit/phpunit": "~4.8 || ~5.1" + "phpunit/phpunit": "^4.8.35 || ^5.4 || ^6.0" }, "suggest": { "symfony/console": "For using debug console command", From ab06ee3327901e172520a2d1a6dd95a2fc6be970 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 23 Jun 2017 12:04:22 +0300 Subject: [PATCH 06/22] Add security annotations handling --- .../BankiruRpcServerExtension.php | 13 +- Listener/ControllerListener.php | 142 ++++++++++++++++++ Listener/SecurityListener.php | 136 +++++++++++++---- Resources/config/security.yml | 5 - Resources/config/sensio.yml | 18 +++ Routing/ControllerResolver/BaseResolver.php | 23 +++ .../ControllerResolver/ControllerResolver.php | 23 +++ Routing/Exception/FileLoaderLoadException.php | 23 +++ Routing/Loader/AnnotationDirectoryLoader.php | 9 ++ Routing/Loader/AnnotationFileLoader.php | 24 +++ Routing/Loader/DirectoryLoader.php | 23 +++ Routing/Loader/FileLoader.php | 23 +++ Routing/Loader/Loader.php | 23 +++ Routing/Route.php | 39 +---- Tests/Fixtures/Kernel.php | 2 + Tests/Fixtures/Rpc/SecurityController.php | 18 ++- Tests/Fixtures/config/rpc_routing.yml | 5 + Tests/Fixtures/config/test.yml | 12 +- Tests/SecurityControllerTest.php | 44 ++++++ composer.json | 5 + 20 files changed, 529 insertions(+), 81 deletions(-) create mode 100644 Listener/ControllerListener.php delete mode 100644 Resources/config/security.yml create mode 100644 Resources/config/sensio.yml create mode 100644 Tests/SecurityControllerTest.php diff --git a/DependencyInjection/BankiruRpcServerExtension.php b/DependencyInjection/BankiruRpcServerExtension.php index b6d6071..da31fe0 100644 --- a/DependencyInjection/BankiruRpcServerExtension.php +++ b/DependencyInjection/BankiruRpcServerExtension.php @@ -6,6 +6,7 @@ use Bankiru\Api\Rpc\Listener\RouterListener; use Bankiru\Api\Rpc\Routing\ResourceMethodCollectionLoader; use Bankiru\Api\Rpc\Routing\Router; +use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -21,19 +22,17 @@ public function load(array $configs, ContainerBuilder $container) { $loader = new YamlFileLoader($container, new FileLocator(__DIR__ . '/../Resources/config')); - if (array_key_exists('NelmioApiDocBundle', $container->getParameter('kernel.bundles'))) { - $loader->load('nelmio.yml'); - } - - if ($container->has('security.authorization_checker')) { - $loader->load('security.yml'); - } + $bundles = $container->getParameter('kernel.bundles'); $configuration = new Configuration(); $config = $this->processConfiguration($configuration, $configs); $loader->load('rpc.yml'); + if (in_array(SensioFrameworkExtraBundle::class, $bundles, true)) { + $loader->load('sensio.yml'); + } + $this->configureRouter($config['router'], $container); } diff --git a/Listener/ControllerListener.php b/Listener/ControllerListener.php new file mode 100644 index 0000000..b77a1c1 --- /dev/null +++ b/Listener/ControllerListener.php @@ -0,0 +1,142 @@ + + */ +final class ControllerListener implements EventSubscriberInterface +{ + /** + * @var Reader + */ + protected $reader; + + /** + * Constructor. + * + * @param Reader $reader An Reader instance + */ + public function __construct(Reader $reader) + { + $this->reader = $reader; + } + + /** + * Modifies the Request object to apply configuration information found in + * controllers annotations like the template to render or HTTP caching + * configuration. + * + * @param FilterControllerEvent $event A FilterControllerEvent instance + */ + public function onKernelController(FilterControllerEvent $event) + { + $controller = $event->getController(); + + if (!is_array($controller) && method_exists($controller, '__invoke')) { + $controller = [$controller, '__invoke']; + } + + if (!is_array($controller)) { + return; + } + + $className = + class_exists('Doctrine\Common\Util\ClassUtils') ? ClassUtils::getClass($controller[0]) : + get_class($controller[0]); + $object = new \ReflectionClass($className); + $method = $object->getMethod($controller[1]); + + $classConfigurations = $this->getConfigurations($this->reader->getClassAnnotations($object)); + $methodConfigurations = $this->getConfigurations($this->reader->getMethodAnnotations($method)); + + $configurations = []; + foreach (array_merge(array_keys($classConfigurations), array_keys($methodConfigurations)) as $key) { + if (!array_key_exists($key, $classConfigurations)) { + $configurations[$key] = $methodConfigurations[$key]; + } elseif (!array_key_exists($key, $methodConfigurations)) { + $configurations[$key] = $classConfigurations[$key]; + } else { + if (is_array($classConfigurations[$key])) { + if (!is_array($methodConfigurations[$key])) { + throw new \UnexpectedValueException( + 'Configurations should both be an array or both not be an array' + ); + } + $configurations[$key] = array_merge($classConfigurations[$key], $methodConfigurations[$key]); + } else { + // method configuration overrides class configuration + $configurations[$key] = $methodConfigurations[$key]; + } + } + } + + $request = $event->getRequest(); + foreach ($configurations as $key => $attributes) { + $request->getAttributes()->set($key, $attributes); + } + } + + public static function getSubscribedEvents() + { + return [ + RpcEvents::CONTROLLER => ['onKernelController', 255], + ]; + } + + protected function getConfigurations(array $annotations) + { + $configurations = []; + foreach ($annotations as $configuration) { + if ($configuration instanceof ConfigurationInterface) { + $index = '_' . $configuration->getAliasName(); + if ($configuration->allowArray()) { + $configurations[$index][] = $configuration; + } elseif (!isset($configurations[$index])) { + $configurations[$index] = $configuration; + } else { + throw new \LogicException( + sprintf('Multiple "%s" annotations are not allowed.', $configuration->getAliasName()) + ); + } + } + } + + return $configurations; + } +} diff --git a/Listener/SecurityListener.php b/Listener/SecurityListener.php index 418d4e8..a228193 100644 --- a/Listener/SecurityListener.php +++ b/Listener/SecurityListener.php @@ -1,50 +1,128 @@ + */ +final class SecurityListener implements EventSubscriberInterface { - /** @var AuthorizationChecker */ - private $authenticator; - - /** - * SecurityListener constructor. - * - * @param AuthorizationChecker $authenticator - */ - public function __construct(AuthorizationChecker $authenticator) - { - $this->authenticator = $authenticator; + private $tokenStorage; + private $authChecker; + private $language; + private $trustResolver; + private $roleHierarchy; + + public function __construct( + TokenStorageInterface $tokenStorage, + AuthorizationCheckerInterface $authChecker, + ExpressionLanguage $language = null, + AuthenticationTrustResolverInterface $trustResolver = null, + RoleHierarchyInterface $roleHierarchy = null + ) { + $this->tokenStorage = $tokenStorage; + $this->authChecker = $authChecker; + $this->language = $language; + $this->trustResolver = $trustResolver; + $this->roleHierarchy = $roleHierarchy; } - public function onFilterController(FilterControllerEvent $event) + public function onKernelController(FilterControllerEvent $event) { - /** @var Route $route */ - $route = $event->getRequest()->getAttributes()->get('_route'); - - if (null === $route) { + $request = $event->getRequest(); + if (!$configuration = $request->getAttributes()->get('_security')) { return; } - $roles = $route->getOption('_roles', null); - if (is_array($roles) && !$this->filterByRoles($roles)) { - throw AccessDeniedException::rolesNeeded($roles); + if (null === $this->tokenStorage || null === $this->trustResolver) { + throw new \LogicException('To use the @Security tag, you need to install the Symfony Security bundle.'); + } + + if (null === $this->tokenStorage->getToken()) { + throw new \LogicException('To use the @Security tag, your controller needs to be behind a firewall.'); + } + + if (null === $this->language) { + throw new \LogicException( + 'To use the @Security tag, you need to use the Security component 2.4 or newer and install the ExpressionLanguage component.' + ); } + + if (!$this->language->evaluate($configuration->getExpression(), $this->getVariables($request))) { + throw new AccessDeniedException(sprintf('Expression "%s" denied access.', $configuration->getExpression())); + } + } + + // code should be sync with Symfony\Component\Security\Core\Authorization\Voter\ExpressionVoter + + public static function getSubscribedEvents() + { + return [RpcEvents::CONTROLLER => ['onKernelController', -255]]; } - private function filterByRoles(array $roles) + private function getVariables(RequestInterface $request) { - foreach ($roles as $role) { - if (!$this->authenticator->isGranted($role)) { - return false; - } + $token = $this->tokenStorage->getToken(); + + if (null !== $this->roleHierarchy) { + $roles = $this->roleHierarchy->getReachableRoles($token->getRoles()); + } else { + $roles = $token->getRoles(); } - return true; + $variables = [ + 'token' => $token, + 'user' => $token->getUser(), + 'object' => $request, + 'request' => $request, + 'roles' => array_map( + function ($role) { + return $role->getRole(); + }, + $roles + ), + 'trust_resolver' => $this->trustResolver, + // needed for the is_granted expression function + 'auth_checker' => $this->authChecker, + ]; + + // controller variables should also be accessible + return array_merge($request->getAttributes()->all(), $variables); } } diff --git a/Resources/config/security.yml b/Resources/config/security.yml deleted file mode 100644 index 942b8a3..0000000 --- a/Resources/config/security.yml +++ /dev/null @@ -1,5 +0,0 @@ -services: - rpc.listeners.security_filter: - class: Bankiru\Api\Rpc\Listener\SecurityListener - arguments: - - "@security.authorization_checker" diff --git a/Resources/config/sensio.yml b/Resources/config/sensio.yml new file mode 100644 index 0000000..f9da4f2 --- /dev/null +++ b/Resources/config/sensio.yml @@ -0,0 +1,18 @@ +services: + rpc.listeners.security_listener: + class: Bankiru\Api\Rpc\Listener\SecurityListener + arguments: + - "@security.token_storage" + - "@security.authorization_checker" + - "@?sensio_framework_extra.security.expression_language" + - "@?security.authentication.trust_resolver" + - "@?security.role_hierarchy" + tags: + - { name: kernel.event_subscriber } + + rpc.listeners.controller_listener: + class: Bankiru\Api\Rpc\Listener\ControllerListener + arguments: + - "@annotation_reader" + tags: + - { name: kernel.event_subscriber } diff --git a/Routing/ControllerResolver/BaseResolver.php b/Routing/ControllerResolver/BaseResolver.php index 1463e97..95b0646 100644 --- a/Routing/ControllerResolver/BaseResolver.php +++ b/Routing/ControllerResolver/BaseResolver.php @@ -1,5 +1,28 @@ + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Bankiru\Api\Rpc\Routing\Loader; use Bankiru\Api\Rpc\Routing\MethodCollection; diff --git a/Routing/Loader/AnnotationFileLoader.php b/Routing/Loader/AnnotationFileLoader.php index 019f1fb..c822b80 100644 --- a/Routing/Loader/AnnotationFileLoader.php +++ b/Routing/Loader/AnnotationFileLoader.php @@ -1,5 +1,29 @@ method = $method; $this->controller = $controller; $this->context = $context; $this->defaultContext = (bool)$defaultContext; $this->inheritContext = (bool)$inheritContext; - $this->options = new ParameterBag($options); } /** @@ -119,36 +114,4 @@ public function inheritContext() { return $this->inheritContext; } - - /** - * @param string $key - * @param mixed $default - * - * @return mixed - */ - public function getOption($key, $default = null) - { - return $this->options->get($key, $default); - } - - /** - * @param string $key - * @param mixed $value - */ - public function setOption($key, $value) - { - $this->options->set($key, $value); - } - - public function getOptions() - { - return $this->options; - } - - public function addOptions(array $options) - { - foreach ($options as $key => $value) { - $this->options->set($key, $value); - } - } } diff --git a/Tests/Fixtures/Kernel.php b/Tests/Fixtures/Kernel.php index 4d420da..fda0cbf 100644 --- a/Tests/Fixtures/Kernel.php +++ b/Tests/Fixtures/Kernel.php @@ -3,6 +3,7 @@ namespace Bankiru\Api\Rpc\Tests\Fixtures; use Bankiru\Api\Rpc\BankiruRpcServerBundle; +use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; use Symfony\Bundle\FrameworkBundle\FrameworkBundle; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\Loader\LoaderInterface; @@ -31,6 +32,7 @@ public function getLogDir() public function registerBundles() { return [ + new SensioFrameworkExtraBundle(), new FrameworkBundle(), new BankiruRpcServerBundle(), new SecurityBundle(), diff --git a/Tests/Fixtures/Rpc/SecurityController.php b/Tests/Fixtures/Rpc/SecurityController.php index b220b7f..6e27270 100644 --- a/Tests/Fixtures/Rpc/SecurityController.php +++ b/Tests/Fixtures/Rpc/SecurityController.php @@ -3,14 +3,30 @@ namespace Bankiru\Api\Rpc\Tests\Fixtures\Rpc; use Bankiru\Api\Rpc\Routing\Annotation\Method; +use Bankiru\Api\Rpc\Tests\Fixtures\Impl\Response; +use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security; use Symfony\Bundle\FrameworkBundle\Controller\Controller; /** - * @Method(options={"_roles":{"IS_AUTHENTICATED_ANONYMOUSLY"}}) + * @Method("security/") */ final class SecurityController extends Controller { + /** + * @Method("public") + * @Security("is_granted('IS_AUTHENTICATED_ANONYMOUSLY')") + */ public function publicAction() { + return new Response(['success' => true]); + } + + /** + * @Method("private") + * @Security("is_granted('IS_AUTHENTICATED_FULLY')") + */ + public function privateAction() + { + return new Response(['success' => true]); } } diff --git a/Tests/Fixtures/config/rpc_routing.yml b/Tests/Fixtures/config/rpc_routing.yml index cba0c94..b2eaee6 100644 --- a/Tests/Fixtures/config/rpc_routing.yml +++ b/Tests/Fixtures/config/rpc_routing.yml @@ -16,3 +16,8 @@ annotation: resource: Bankiru\Api\Rpc\Tests\Fixtures\Rpc\RpcImplController type: annotation context: annotation + +security: + resource: Bankiru\Api\Rpc\Tests\Fixtures\Rpc\SecurityController + type: annotation + context: security diff --git a/Tests/Fixtures/config/test.yml b/Tests/Fixtures/config/test.yml index ddfd877..3efb613 100644 --- a/Tests/Fixtures/config/test.yml +++ b/Tests/Fixtures/config/test.yml @@ -8,7 +8,17 @@ services: logger: class: Psr\Log\NullLogger -security: ~ +security: + providers: + in_memory: + memory: ~ + + firewalls: + main: + anonymous: ~ + + access_control: + - { path: ^/, roles: IS_AUTHENTICATED_ANONYMOUSLY } rpc: router: diff --git a/Tests/SecurityControllerTest.php b/Tests/SecurityControllerTest.php new file mode 100644 index 0000000..9d5c764 --- /dev/null +++ b/Tests/SecurityControllerTest.php @@ -0,0 +1,44 @@ +request( + 'POST', + '/test/', + ['method' => 'security/public'] + ); + + self::assertTrue($client->getResponse()->isSuccessful()); + } + + /** + * @expectedException \Symfony\Component\Security\Core\Exception\InsufficientAuthenticationException + * @expectedExceptionMessage Full authentication is required to access this resource. + */ + public function testPrivateAction() + { + $client = self::createClient(); + + $client->request( + 'POST', + '/test/', + ['method' => 'security/private'] + ); + + self::assertTrue($client->getResponse()->isSuccessful()); + } + + protected static function getKernelClass() + { + return Kernel::class; + } +} diff --git a/composer.json b/composer.json index 8725d3d..5d3d1eb 100644 --- a/composer.json +++ b/composer.json @@ -20,6 +20,9 @@ "scaytrase/rpc-common": "~1.0" }, "require-dev": { + "doctrine/common": "~2.2", + "sensio/framework-extra-bundle": "~3.0 || ~4.0@dev", + "symfony/expression-language": "~2.7 || ~3.0 || ~4.0", "symfony/console": "~2.7 || ~3.0 || ~4.0", "symfony/routing": "~2.7 || ~3.0 || ~4.0", "symfony/framework-bundle": "~2.7 || ~3.0 || ~4.0", @@ -28,6 +31,8 @@ "phpunit/phpunit": "^4.8.35 || ^5.4 || ^6.0" }, "suggest": { + "sensio/framework-extra-bundle": "For controller annotations", + "symfony/expression-language": "For evaluating security expressions", "symfony/console": "For using debug console command", "symfony/security": "For enabling role and expressions based security filters" }, From 2005327e6bc5471d4f0bf252d3934e3808a715ee Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 23 Jun 2017 13:19:21 +0300 Subject: [PATCH 07/22] Allow exception override --- Event/GetExceptionResponseEvent.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/Event/GetExceptionResponseEvent.php b/Event/GetExceptionResponseEvent.php index d6948ea..1661610 100644 --- a/Event/GetExceptionResponseEvent.php +++ b/Event/GetExceptionResponseEvent.php @@ -3,8 +3,6 @@ namespace Bankiru\Api\Rpc\Event; use Bankiru\Api\Rpc\Http\RequestInterface; -use Bankiru\Api\Rpc\RpcEvent; -use ScayTrase\Api\Rpc\RpcResponseInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; class GetExceptionResponseEvent extends RpcResponseEvent @@ -23,4 +21,12 @@ public function getException() { return $this->exception; } + + /** + * @param \Exception $exception + */ + public function setException($exception) + { + $this->exception = $exception; + } } From 319f19a0f42164e457fabd27556fa499c2d8120d Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 23 Jun 2017 13:31:31 +0300 Subject: [PATCH 08/22] Update deps --- .travis.yml | 18 +++++++++--------- .../BankiruRpcServerExtension.php | 5 +++++ Resources/config/security.yml | 11 +++++++++++ Resources/config/sensio.yml | 11 ----------- composer.json | 2 +- 5 files changed, 26 insertions(+), 21 deletions(-) create mode 100644 Resources/config/security.yml diff --git a/.travis.yml b/.travis.yml index fcf4a32..2627c96 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,20 +9,20 @@ php: - hhvm env: - - SYMFONY_VERSION='2.7.*' deps='no' - - SYMFONY_VERSION='2.8.*' deps='no' - - SYMFONY_VERSION='3.2.*' deps='no' - - SYMFONY_VERSION='3.3.*' deps='no' - - SYMFONY_VERSION='~3.4@dev' deps='no' + - PACKAGES='symfony/symfony=2.7.*' deps='no' + - PACKAGES='symfony/symfony=2.8.*' deps='no' + - PACKAGES='symfony/symfony=3.2.*' deps='no' + - PACKAGES='symfony/symfony=3.3.*' deps='no' + - PACKAGES='symfony/symfony=~3.4@dev' deps='no' matrix: include: - php: 5.5 - env: SYMFONY_VERSION='2.7.*' deps='low' + env: PACKAGES='symfony/symfony=2.7.*' deps='low' - php: 7.1 - env: SYMFONY_VERSION='~4.0@dev' deps='no' + env: PACKAGES='symfony/symfony=~4.0@dev sensio/framework-extra-bundle=~4.0@dev' deps='no' - php: nightly - env: SYMFONY_VERSION='~4.0@dev' deps='no' + env: PACKAGES='symfony/symfony=~4.0@dev sensio/framework-extra-bundle=~4.0@dev' deps='no' allow_failures: - php: hhvm @@ -32,7 +32,7 @@ before_install: - travis_retry composer self-update install: - - composer require --no-update symfony/symfony=${SYMFONY_VERSION} + - composer require --no-update ${PACKAGES} - if [ "$deps" = "no" ]; then composer --prefer-source install; fi; - if [ "$deps" = "low" ]; then composer --prefer-source --prefer-lowest --prefer-stable update; fi; diff --git a/DependencyInjection/BankiruRpcServerExtension.php b/DependencyInjection/BankiruRpcServerExtension.php index da31fe0..cf331b6 100644 --- a/DependencyInjection/BankiruRpcServerExtension.php +++ b/DependencyInjection/BankiruRpcServerExtension.php @@ -7,6 +7,7 @@ use Bankiru\Api\Rpc\Routing\ResourceMethodCollectionLoader; use Bankiru\Api\Rpc\Routing\Router; use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; +use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -31,6 +32,10 @@ public function load(array $configs, ContainerBuilder $container) if (in_array(SensioFrameworkExtraBundle::class, $bundles, true)) { $loader->load('sensio.yml'); + + if (in_array(SecurityBundle::class, $bundles, true)) { + $loader->load('security.yml'); + } } $this->configureRouter($config['router'], $container); diff --git a/Resources/config/security.yml b/Resources/config/security.yml new file mode 100644 index 0000000..c94f168 --- /dev/null +++ b/Resources/config/security.yml @@ -0,0 +1,11 @@ +services: + rpc.listeners.security_listener: + class: Bankiru\Api\Rpc\Listener\SecurityListener + arguments: + - "@security.token_storage" + - "@security.authorization_checker" + - "@?sensio_framework_extra.security.expression_language" + - "@?security.authentication.trust_resolver" + - "@?security.role_hierarchy" + tags: + - { name: kernel.event_subscriber } diff --git a/Resources/config/sensio.yml b/Resources/config/sensio.yml index f9da4f2..2a0b669 100644 --- a/Resources/config/sensio.yml +++ b/Resources/config/sensio.yml @@ -1,15 +1,4 @@ services: - rpc.listeners.security_listener: - class: Bankiru\Api\Rpc\Listener\SecurityListener - arguments: - - "@security.token_storage" - - "@security.authorization_checker" - - "@?sensio_framework_extra.security.expression_language" - - "@?security.authentication.trust_resolver" - - "@?security.role_hierarchy" - tags: - - { name: kernel.event_subscriber } - rpc.listeners.controller_listener: class: Bankiru\Api\Rpc\Listener\ControllerListener arguments: diff --git a/composer.json b/composer.json index 5d3d1eb..fa96e99 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ }, "require-dev": { "doctrine/common": "~2.2", - "sensio/framework-extra-bundle": "~3.0 || ~4.0@dev", + "sensio/framework-extra-bundle": "~3.0", "symfony/expression-language": "~2.7 || ~3.0 || ~4.0", "symfony/console": "~2.7 || ~3.0 || ~4.0", "symfony/routing": "~2.7 || ~3.0 || ~4.0", From 26cfd2aac475c9e69ae42402b7c80d90ba0cb1eb Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 23 Jun 2017 14:37:52 +0300 Subject: [PATCH 09/22] Remove cache dir before test --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 2627c96..ffea29c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,5 +37,6 @@ install: - if [ "$deps" = "low" ]; then composer --prefer-source --prefer-lowest --prefer-stable update; fi; script: + - rm -rf build - mkdir -p build - vendor/bin/phpunit --colors -c phpunit.xml From deccadbc95a559d1dece5118b4d53728755990b6 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Wed, 28 Jun 2017 12:53:31 +0300 Subject: [PATCH 10/22] Proxy controller. Interfaces update --- ...stInterface.php => AttributedInterface.php | 5 +- BankiruRpcServerBundle.php | 2 +- Command/RouterDebugCommand.php | 17 ++- Controller/RpcController.php | 26 ++--- Event/FilterControllerEvent.php | 9 +- Event/FilterResponseEvent.php | 3 +- Event/GetExceptionResponseEvent.php | 4 +- Event/ViewEvent.php | 21 +++- Listener/SecurityListener.php | 3 +- Resources/config/rpc.yml | 5 + Routing/ControllerResolver/BaseResolver.php | 104 +++++++++--------- .../ControllerResolverInterface.php | 12 +- Rpc/ProxyController.php | 27 +++++ RpcBundle.php | 8 -- RpcEvent.php | 11 +- RpcRequestInterface.php | 9 ++ Tests/Fixtures/Impl/Request.php | 4 +- 17 files changed, 161 insertions(+), 109 deletions(-) rename Http/RequestInterface.php => AttributedInterface.php (52%) create mode 100644 Rpc/ProxyController.php delete mode 100644 RpcBundle.php create mode 100644 RpcRequestInterface.php diff --git a/Http/RequestInterface.php b/AttributedInterface.php similarity index 52% rename from Http/RequestInterface.php rename to AttributedInterface.php index 80d9202..ac0beaa 100644 --- a/Http/RequestInterface.php +++ b/AttributedInterface.php @@ -1,11 +1,10 @@ routers = $routers; + } + protected function configure() { parent::configure(); @@ -71,8 +81,7 @@ protected function getRoutes() { $routes = []; - $collection = $this->getContainer()->get('rpc.router.collection'); - foreach ($collection->all() as $endpoint => $router) { + foreach ($this->routers->all() as $endpoint => $router) { $routes[$endpoint] = $router->getMethodCollection()->all(); } diff --git a/Controller/RpcController.php b/Controller/RpcController.php index 11dedb4..2ea9c60 100644 --- a/Controller/RpcController.php +++ b/Controller/RpcController.php @@ -8,10 +8,10 @@ use Bankiru\Api\Rpc\Event\GetExceptionResponseEvent; use Bankiru\Api\Rpc\Event\GetResponseEvent; use Bankiru\Api\Rpc\Event\ViewEvent; -use Bankiru\Api\Rpc\Http\RequestInterface; use Bankiru\Api\Rpc\Routing\ControllerResolver\ControllerResolverInterface; use Bankiru\Api\Rpc\Routing\Exception\MethodNotFoundException; use Bankiru\Api\Rpc\RpcEvents; +use Bankiru\Api\Rpc\RpcRequestInterface; use ScayTrase\Api\Rpc\RpcResponseInterface; use Symfony\Component\DependencyInjection\ContainerAwareInterface; use Symfony\Component\DependencyInjection\ContainerInterface; @@ -43,13 +43,13 @@ public function setContainer(ContainerInterface $container = null) } /** - * @param RequestInterface $request - * @param string $endpoint + * @param RpcRequestInterface $request + * @param string $endpoint * * @return RpcResponseInterface * @throws \Exception */ - protected function getResponse(RequestInterface $request, $endpoint) + protected function getResponse(RpcRequestInterface $request, $endpoint) { $request->getAttributes()->set('_endpoint', $endpoint); @@ -63,7 +63,7 @@ protected function getResponse(RequestInterface $request, $endpoint) } /** - * @param RequestInterface $request + * @param RpcRequestInterface $request * * @return RpcResponseInterface * @@ -72,7 +72,7 @@ protected function getResponse(RequestInterface $request, $endpoint) * @throws \LogicException * @throws MethodNotFoundException */ - protected function handleSingleRequest(RequestInterface $request) + protected function handleSingleRequest(RpcRequestInterface $request) { // request $event = new GetResponseEvent($this->getKernel(), $request); @@ -131,12 +131,12 @@ protected function get($name) * Filters a response object. * * @param RpcResponseInterface $response A Response instance - * @param RequestInterface $request An error message in case the response is not a Response object + * @param RpcRequestInterface $request An error message in case the response is not a Response object * * @return RpcResponseInterface The filtered Response instance * @throws \RuntimeException */ - protected function filterResponse(RpcResponseInterface $response, RequestInterface $request) + protected function filterResponse(RpcResponseInterface $response, RpcRequestInterface $request) { $event = new FilterResponseEvent($this->getKernel(), $request, $response); $this->dispatcher->dispatch(RpcEvents::RESPONSE, $event); @@ -152,11 +152,11 @@ protected function filterResponse(RpcResponseInterface $response, RequestInterfa * operations such as {@link RequestStack::getParentRequest()} can lead to * weird results. * - * @param RequestInterface $request + * @param RpcRequestInterface $request * * @throws \RuntimeException */ - protected function finishRequest(RequestInterface $request) + protected function finishRequest(RpcRequestInterface $request) { $this->dispatcher->dispatch( RpcEvents::FINISH_REQUEST, @@ -206,14 +206,14 @@ protected function varToString($var) /** * Handles an exception by trying to convert it to a Response. * - * @param \Exception $e An \Exception instance - * @param RequestInterface $request A Request instance + * @param \Exception $e An \Exception instance + * @param RpcRequestInterface $request A Request instance * * @return RpcResponseInterface A Response instance * * @throws \Exception */ - protected function handleException(\Exception $e, RequestInterface $request) + protected function handleException(\Exception $e, RpcRequestInterface $request) { $event = new GetExceptionResponseEvent($this->getKernel(), $request, $e); $this->dispatcher->dispatch(RpcEvents::EXCEPTION, $event); diff --git a/Event/FilterControllerEvent.php b/Event/FilterControllerEvent.php index 906314b..9354097 100644 --- a/Event/FilterControllerEvent.php +++ b/Event/FilterControllerEvent.php @@ -2,8 +2,8 @@ namespace Bankiru\Api\Rpc\Event; -use Bankiru\Api\Rpc\Http\RequestInterface; use Bankiru\Api\Rpc\RpcEvent; +use Bankiru\Api\Rpc\RpcRequestInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; class FilterControllerEvent extends RpcEvent @@ -13,11 +13,12 @@ class FilterControllerEvent extends RpcEvent /** * FilterControllerEvent constructor. + * * @param HttpKernelInterface $kernel - * @param RequestInterface $request - * @param string|callable $controller + * @param RpcRequestInterface $request + * @param string|callable $controller */ - public function __construct(HttpKernelInterface $kernel, RequestInterface $request, $controller) + public function __construct(HttpKernelInterface $kernel, RpcRequestInterface $request, $controller) { parent::__construct($kernel, $request); $this->controller = $controller; diff --git a/Event/FilterResponseEvent.php b/Event/FilterResponseEvent.php index b245479..db8b559 100644 --- a/Event/FilterResponseEvent.php +++ b/Event/FilterResponseEvent.php @@ -4,6 +4,7 @@ use Bankiru\Api\Rpc\Http\RequestInterface; use Bankiru\Api\Rpc\RpcEvent; +use Bankiru\Api\Rpc\RpcRequestInterface; use ScayTrase\Api\Rpc\RpcResponseInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; @@ -14,7 +15,7 @@ class FilterResponseEvent extends RpcEvent public function __construct( HttpKernelInterface $kernel, - RequestInterface $request, + RpcRequestInterface $request, RpcResponseInterface $response ) { diff --git a/Event/GetExceptionResponseEvent.php b/Event/GetExceptionResponseEvent.php index 1661610..11de365 100644 --- a/Event/GetExceptionResponseEvent.php +++ b/Event/GetExceptionResponseEvent.php @@ -2,7 +2,7 @@ namespace Bankiru\Api\Rpc\Event; -use Bankiru\Api\Rpc\Http\RequestInterface; +use Bankiru\Api\Rpc\RpcRequestInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; class GetExceptionResponseEvent extends RpcResponseEvent @@ -10,7 +10,7 @@ class GetExceptionResponseEvent extends RpcResponseEvent /** @var \Exception */ private $exception; - public function __construct(HttpKernelInterface $kernel, RequestInterface $request, \Exception $exception) + public function __construct(HttpKernelInterface $kernel, RpcRequestInterface $request, \Exception $exception) { parent::__construct($kernel, $request); $this->exception = $exception; diff --git a/Event/ViewEvent.php b/Event/ViewEvent.php index 415d423..e5cc06f 100644 --- a/Event/ViewEvent.php +++ b/Event/ViewEvent.php @@ -2,8 +2,8 @@ namespace Bankiru\Api\Rpc\Event; -use Bankiru\Api\Rpc\Http\RequestInterface; use Bankiru\Api\Rpc\RpcEvent; +use Bankiru\Api\Rpc\RpcRequestInterface; use Symfony\Component\HttpKernel\HttpKernelInterface; class ViewEvent extends RpcEvent @@ -15,21 +15,30 @@ class ViewEvent extends RpcEvent * ViewEvent constructor. * * @param HttpKernelInterface $kernel - * @param RequestInterface $request + * @param RpcRequestInterface $request * @param mixed $response */ - public function __construct(HttpKernelInterface $kernel, RequestInterface $request, $response) + public function __construct(HttpKernelInterface $kernel, RpcRequestInterface $request, $response) { parent::__construct($kernel, $request); $this->response = $response; } /** @return mixed */ - public function getResponse() { return $this->response; } + public function getResponse() + { + return $this->response; + } /** @param mixed $response */ - public function setResponse($response) { $this->response = $response; } + public function setResponse($response) + { + $this->response = $response; + } /** @return bool */ - public function hasResponse() { return null !== $this->response; } + public function hasResponse() + { + return null !== $this->response; + } } diff --git a/Listener/SecurityListener.php b/Listener/SecurityListener.php index a228193..1a9ffa3 100644 --- a/Listener/SecurityListener.php +++ b/Listener/SecurityListener.php @@ -28,6 +28,7 @@ use Bankiru\Api\Rpc\Event\FilterControllerEvent; use Bankiru\Api\Rpc\Http\RequestInterface; use Bankiru\Api\Rpc\RpcEvents; +use Bankiru\Api\Rpc\RpcRequestInterface; use Sensio\Bundle\FrameworkExtraBundle\Security\ExpressionLanguage; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\Security\Core\Authentication\AuthenticationTrustResolverInterface; @@ -96,7 +97,7 @@ public static function getSubscribedEvents() return [RpcEvents::CONTROLLER => ['onKernelController', -255]]; } - private function getVariables(RequestInterface $request) + private function getVariables(RpcRequestInterface $request) { $token = $this->tokenStorage->getToken(); diff --git a/Resources/config/rpc.yml b/Resources/config/rpc.yml index e462050..18eda48 100644 --- a/Resources/config/rpc.yml +++ b/Resources/config/rpc.yml @@ -1,4 +1,9 @@ services: + bankiru_rpc.command.router_debug_command: + class: Bankiru\Api\Rpc\Command\RouterDebugCommand + arguments: + - "@rpc.router.collection" + rpc.controller_resolver: class: Bankiru\Api\Rpc\Routing\ControllerResolver\ControllerResolver arguments: diff --git a/Routing/ControllerResolver/BaseResolver.php b/Routing/ControllerResolver/BaseResolver.php index 95b0646..efc373b 100644 --- a/Routing/ControllerResolver/BaseResolver.php +++ b/Routing/ControllerResolver/BaseResolver.php @@ -26,7 +26,7 @@ namespace Bankiru\Api\Rpc\Routing\ControllerResolver; use Bankiru\Api\Rpc\Exception\InvalidMethodParametersException; -use Bankiru\Api\Rpc\Http\RequestInterface; +use Bankiru\Api\Rpc\RpcRequestInterface; use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; @@ -46,7 +46,7 @@ public function __construct(LoggerInterface $logger = null) } /** {@inheritdoc} */ - public function getController(RequestInterface $request) + public function getController(RpcRequestInterface $request) { if (!$controller = $request->getAttributes()->get('_controller')) { $this->logger->warning('Unable to look for the controller as the "_controller" parameter is missing.'); @@ -95,6 +95,21 @@ public function getController(RequestInterface $request) return $callable; } + /** {@inheritdoc} */ + public function getArguments(RpcRequestInterface $request, $controller) + { + if (is_array($controller)) { + $r = new \ReflectionMethod($controller[0], $controller[1]); + } elseif (is_object($controller) && !$controller instanceof \Closure) { + $r = new \ReflectionObject($controller); + $r = $r->getMethod('__invoke'); + } else { + $r = new \ReflectionFunction($controller); + } + + return $this->doGetArguments($request, $r->getParameters()); + } + /** * Returns an instantiated controller. * @@ -131,6 +146,39 @@ protected function createController($controller) return [$this->instantiateController($class), $method]; } + /** + * @param RpcRequestInterface $request + * @param \ReflectionParameter[] $parameters + * + * @return array + * @throws \RuntimeException + */ + protected function doGetArguments(RpcRequestInterface $request, array $parameters) + { + $attributes = $request->getAttributes()->all(); + $arguments = []; + $missing = []; + foreach ($parameters as $param) { + if (is_array($request->getParameters()) && array_key_exists($param->name, $request->getParameters())) { + $arguments[] = $this->checkType($request->getParameters()[$param->name], $param, $request); + } elseif (array_key_exists($param->name, $attributes)) { + $arguments[] = $this->checkType($attributes[$param->name], $param->name, $request); + } elseif ($param->getClass() && $param->getClass()->isInstance($request)) { + $arguments[] = $request; + } elseif ($param->isDefaultValueAvailable()) { + $arguments[] = $param->getDefaultValue(); + } else { + $missing[] = $param->name; + } + } + + if (count($missing) > 0) { + throw InvalidMethodParametersException::missing($request->getMethod(), $missing); + } + + return $arguments; + } + private function getControllerError($callable) { if (is_string($callable)) { @@ -195,64 +243,16 @@ private function getControllerError($callable) return $message; } - /** {@inheritdoc} */ - public function getArguments(RequestInterface $request, $controller) - { - if (is_array($controller)) { - $r = new \ReflectionMethod($controller[0], $controller[1]); - } elseif (is_object($controller) && !$controller instanceof \Closure) { - $r = new \ReflectionObject($controller); - $r = $r->getMethod('__invoke'); - } else { - $r = new \ReflectionFunction($controller); - } - - return $this->doGetArguments($request, $r->getParameters()); - } - - /** - * @param RequestInterface $request - * @param \ReflectionParameter[] $parameters - * - * @return array - * @throws \RuntimeException - */ - protected function doGetArguments(RequestInterface $request, array $parameters) - { - $attributes = $request->getAttributes()->all(); - $arguments = []; - $missing = []; - foreach ($parameters as $param) { - if (is_array($request->getParameters()) && array_key_exists($param->name, $request->getParameters())) { - $arguments[] = $this->checkType($request->getParameters()[$param->name], $param, $request); - } elseif (array_key_exists($param->name, $attributes)) { - $arguments[] = $this->checkType($attributes[$param->name], $param->name, $request); - } elseif ($param->getClass() && $param->getClass()->isInstance($request)) { - $arguments[] = $request; - } elseif ($param->isDefaultValueAvailable()) { - $arguments[] = $param->getDefaultValue(); - } else { - $missing[] = $param->name; - } - } - - if (count($missing) > 0) { - throw InvalidMethodParametersException::missing($request->getMethod(), $missing); - } - - return $arguments; - } - /** * Checks that argument matches parameter type * * @param mixed $argument * @param \ReflectionParameter $param - * @param RequestInterface $request + * @param RpcRequestInterface $request * * @return mixed */ - private function checkType($argument, \ReflectionParameter $param, RequestInterface $request) + private function checkType($argument, \ReflectionParameter $param, RpcRequestInterface $request) { $actual = is_object($argument) ? get_class($argument) : gettype($argument); if (null !== $param->getClass()) { diff --git a/Routing/ControllerResolver/ControllerResolverInterface.php b/Routing/ControllerResolver/ControllerResolverInterface.php index aad2e10..7734f1e 100644 --- a/Routing/ControllerResolver/ControllerResolverInterface.php +++ b/Routing/ControllerResolver/ControllerResolverInterface.php @@ -2,24 +2,24 @@ namespace Bankiru\Api\Rpc\Routing\ControllerResolver; -use Bankiru\Api\Rpc\Http\RequestInterface; +use Bankiru\Api\Rpc\RpcRequestInterface; interface ControllerResolverInterface { /** - * @param RequestInterface $request + * @param RpcRequestInterface $request * * @return callable|false * @throws \InvalidArgumentException */ - public function getController(RequestInterface $request); + public function getController(RpcRequestInterface $request); /** - * @param RequestInterface $request - * @param $controller + * @param RpcRequestInterface $request + * @param $controller * * @return array * @throws \RuntimeException */ - public function getArguments(RequestInterface $request, $controller); + public function getArguments(RpcRequestInterface $request, $controller); } diff --git a/Rpc/ProxyController.php b/Rpc/ProxyController.php new file mode 100644 index 0000000..90d7bb2 --- /dev/null +++ b/Rpc/ProxyController.php @@ -0,0 +1,27 @@ +client = $client; + } + + public function handle(RpcRequestInterface $request) + { + return $this->client->invoke($request); + } +} diff --git a/RpcBundle.php b/RpcBundle.php deleted file mode 100644 index 36fe3cc..0000000 --- a/RpcBundle.php +++ /dev/null @@ -1,8 +0,0 @@ -kernel = $kernel; $this->request = $request; @@ -34,7 +33,7 @@ public function getKernel() } /** - * @return RequestInterface + * @return RpcRequestInterface */ public function getRequest() { diff --git a/RpcRequestInterface.php b/RpcRequestInterface.php new file mode 100644 index 0000000..10e914f --- /dev/null +++ b/RpcRequestInterface.php @@ -0,0 +1,9 @@ + Date: Wed, 19 Jul 2017 11:59:03 +0300 Subject: [PATCH 11/22] Update DI identifiers --- .travis.yml | 40 +++++++++++-------- .../BankiruRpcServerExtension.php | 14 +++---- .../Compiler/RouterLoaderPass.php | 4 +- DependencyInjection/Configuration.php | 2 +- README.md | 6 +-- Resources/config/rpc.yml | 36 ++++++++--------- Resources/config/security.yml | 2 +- Resources/config/sensio.yml | 2 +- Tests/Fixtures/Kernel.php | 11 +++-- Tests/Fixtures/Rpc/TestController.php | 2 +- Tests/Fixtures/config/test.yml | 2 +- Tests/RoutingTest.php | 2 +- dump_reference.php | 12 ++++++ 13 files changed, 80 insertions(+), 55 deletions(-) create mode 100644 dump_reference.php diff --git a/.travis.yml b/.travis.yml index ffea29c..851be4c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -6,35 +6,43 @@ php: - 7 - 7.1 - nightly - - hhvm env: - PACKAGES='symfony/symfony=2.7.*' deps='no' - PACKAGES='symfony/symfony=2.8.*' deps='no' - PACKAGES='symfony/symfony=3.2.*' deps='no' - PACKAGES='symfony/symfony=3.3.*' deps='no' - - PACKAGES='symfony/symfony=~3.4@dev' deps='no' + - PACKAGES='symfony/symfony=~3.4' STABILITY=dev deps='no' + +## Run on container environment +sudo: false + +## Cache composer bits +cache: + directories: + - $HOME/.composer/cache matrix: - include: - - php: 5.5 - env: PACKAGES='symfony/symfony=2.7.*' deps='low' - - php: 7.1 - env: PACKAGES='symfony/symfony=~4.0@dev sensio/framework-extra-bundle=~4.0@dev' deps='no' - - php: nightly - env: PACKAGES='symfony/symfony=~4.0@dev sensio/framework-extra-bundle=~4.0@dev' deps='no' - - allow_failures: - - php: hhvm - - php: nightly + include: + - php: 5.5 + env: PACKAGES='symfony/symfony=2.7.*' deps='low' + - php: 7.1 + env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev deps='no' + - php: nightly + env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev deps='no' + + allow_failures: + - php: nightly + - env: PACKAGES='symfony/symfony=~4.0@dev' STABILITY=dev before_install: - travis_retry composer self-update install: - - composer require --no-update ${PACKAGES} - - if [ "$deps" = "no" ]; then composer --prefer-source install; fi; - - if [ "$deps" = "low" ]; then composer --prefer-source --prefer-lowest --prefer-stable update; fi; + - composer require --no-update ${PACKAGES} + - if [ "$STABILITY" = "dev" ]; then composer config minimum-stability dev; fi; + - if [ "$deps" = "no" ]; then composer --prefer-source install; fi; + - if [ "$deps" = "low" ]; then composer --prefer-source --prefer-lowest --prefer-stable update; fi; script: - rm -rf build diff --git a/DependencyInjection/BankiruRpcServerExtension.php b/DependencyInjection/BankiruRpcServerExtension.php index cf331b6..fd9577b 100644 --- a/DependencyInjection/BankiruRpcServerExtension.php +++ b/DependencyInjection/BankiruRpcServerExtension.php @@ -43,7 +43,7 @@ public function load(array $configs, ContainerBuilder $container) public function getAlias() { - return 'rpc'; + return 'rpc_server'; } /** @@ -53,11 +53,11 @@ public function getAlias() private function configureRouter(array $router, ContainerBuilder $container) { $endpoints = $router['endpoints']; - $endpointLoader = $container->getDefinition('rpc.router.loader'); - $routerCollection = $container->getDefinition('rpc.router.collection'); + $endpointLoader = $container->getDefinition('rpc_server.router.loader'); + $routerCollection = $container->getDefinition('rpc_server.router.collection'); foreach ($endpoints as $name => $config) { - $routerId = sprintf('rpc.endpoint_router.%s', $name); + $routerId = sprintf('rpc_server.endpoint_router.%s', $name); $container->register($routerId, Router::class) ->setArguments( @@ -65,7 +65,7 @@ private function configureRouter(array $router, ContainerBuilder $container) new Definition( ResourceMethodCollectionLoader::class, [ - new Reference('rpc.router.resolver'), + new Reference('rpc_server.router.resolver'), $config['resources'], $config['context'], ] @@ -79,7 +79,7 @@ private function configureRouter(array $router, ContainerBuilder $container) ->setPublic(false) ->addTag('rpc_router'); - $container->register(sprintf('rpc.router_warmer.%s', $name), RouterCacheWarmer::class) + $container->register(sprintf('rpc_server.router_warmer.%s', $name), RouterCacheWarmer::class) ->setPublic(false) ->setArguments( [ @@ -88,7 +88,7 @@ private function configureRouter(array $router, ContainerBuilder $container) ) ->addTag('kernel.cache_warmer'); - $container->register('rpc.router_listener.' . $name, RouterListener::class) + $container->register('rpc_server.router_listener.' . $name, RouterListener::class) ->setArguments([$name, new Reference($routerId)]) ->addTag('kernel.event_listener', ['event' => 'rpc.request', 'method' => 'onRequest']); diff --git a/DependencyInjection/Compiler/RouterLoaderPass.php b/DependencyInjection/Compiler/RouterLoaderPass.php index ebdb01e..dbc1d6b 100644 --- a/DependencyInjection/Compiler/RouterLoaderPass.php +++ b/DependencyInjection/Compiler/RouterLoaderPass.php @@ -15,7 +15,7 @@ public function process(ContainerBuilder $container) { if ($container->has('logger')) { $container - ->register('rpc.exception_listener', ExceptionListener::class) + ->register('rpc_server.exception_listener', ExceptionListener::class) ->setArguments([new Reference('logger')]) ->addTag( 'kernel.event_listener', @@ -26,7 +26,7 @@ public function process(ContainerBuilder $container) ); } - $loader = $container->getDefinition('rpc.router.resolver'); + $loader = $container->getDefinition('rpc_server.router.resolver'); $taggedServices = $container->findTaggedServiceIds('rpc.route_loader'); diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index 1acf0e4..a8f478b 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -19,7 +19,7 @@ final class Configuration implements ConfigurationInterface public function getConfigTreeBuilder() { $builder = new TreeBuilder(); - $root = $builder->root('rpc'); + $root = $builder->root('rpc_server'); $this->configureRouter($root->children()->arrayNode('router')); return $builder; diff --git a/README.md b/README.md index 7d026a4..cb58908 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ # HTTP RPC Server bundle -This bundle provides default controller realisation to handle RPC +This bundle provides default controller realization to handle RPC requests which come to the application via HTTP requests ## Implementations @@ -46,7 +46,7 @@ configured vua config Basic endpoint configuration looks like ```yaml -rpc: +rpc_server: router: endpoints: my-public-endpoint: @@ -79,7 +79,7 @@ my_bundle: type: annotation ``` -Different resource types are supported. Built-in are +Different resource types are supported #### Annotation diff --git a/Resources/config/rpc.yml b/Resources/config/rpc.yml index 18eda48..15c6ace 100644 --- a/Resources/config/rpc.yml +++ b/Resources/config/rpc.yml @@ -1,64 +1,64 @@ services: - bankiru_rpc.command.router_debug_command: + rpc_server.command.router_debug_command: class: Bankiru\Api\Rpc\Command\RouterDebugCommand arguments: - - "@rpc.router.collection" + - "@rpc_server.router.collection" - rpc.controller_resolver: + rpc_server.controller_resolver: class: Bankiru\Api\Rpc\Routing\ControllerResolver\ControllerResolver arguments: - "@service_container" - - "@rpc.controller_name_parser" + - "@rpc_server.controller_name_parser" - "@?logger" - rpc.router.loader: + rpc_server.router.loader: class: Bankiru\Api\Rpc\Http\Routing\EndpointRouteLoader tags: - { name: routing.loader } - rpc.controller_name_parser: + rpc_server.controller_name_parser: class: Bankiru\Api\Rpc\Controller\RpcControllerNameParser arguments: - "@kernel" - rpc.router.resolver: + rpc_server.router.resolver: class: Bankiru\Api\Rpc\Routing\LoaderResolver - rpc.router.collection: + rpc_server.router.collection: class: Bankiru\Api\Rpc\Routing\RouterCollection - rpc.router.loader.file: + rpc_server.router.loader.file: abstract: true class: Bankiru\Api\Rpc\Routing\Loader\FileLoader calls: - - [ "setResolver", ["@rpc.router.resolver"]] + - [ "setResolver", ["@rpc_server.router.resolver"]] - rpc.router.loader.yaml: + rpc_server.router.loader.yaml: class: Bankiru\Api\Rpc\Routing\Loader\YamlLoader - parent: rpc.router.loader.file + parent: rpc_server.router.loader.file arguments: - "@file_locator" tags: - { name: rpc.route_loader } - rpc.router.loader.annotation_class: + rpc_server.router.loader.annotation_class: class: Bankiru\Api\Rpc\Routing\Loader\AnnotationClassLoader arguments: - "@annotation_reader" tags: - { name: rpc.route_loader } - rpc.router.loader.annotation_file: + rpc_server.router.loader.annotation_file: class: Bankiru\Api\Rpc\Routing\Loader\AnnotationFileLoader - parent: rpc.router.loader.file + parent: rpc_server.router.loader.file arguments: - "@file_locator" - - "@rpc.router.loader.annotation_class" + - "@rpc_server.router.loader.annotation_class" tags: - { name: rpc.route_loader } - rpc.router.loader.annotation_directory: + rpc_server.router.loader.annotation_directory: class: Bankiru\Api\Rpc\Routing\Loader\AnnotationDirectoryLoader - parent: rpc.router.loader.annotation_file + parent: rpc_server.router.loader.annotation_file tags: - { name: rpc.route_loader } diff --git a/Resources/config/security.yml b/Resources/config/security.yml index c94f168..386d94e 100644 --- a/Resources/config/security.yml +++ b/Resources/config/security.yml @@ -1,5 +1,5 @@ services: - rpc.listeners.security_listener: + rpc_server.listeners.security_listener: class: Bankiru\Api\Rpc\Listener\SecurityListener arguments: - "@security.token_storage" diff --git a/Resources/config/sensio.yml b/Resources/config/sensio.yml index 2a0b669..10d5bda 100644 --- a/Resources/config/sensio.yml +++ b/Resources/config/sensio.yml @@ -1,5 +1,5 @@ services: - rpc.listeners.controller_listener: + rpc_server.listeners.controller_listener: class: Bankiru\Api\Rpc\Listener\ControllerListener arguments: - "@annotation_reader" diff --git a/Tests/Fixtures/Kernel.php b/Tests/Fixtures/Kernel.php index fda0cbf..c7dd4f6 100644 --- a/Tests/Fixtures/Kernel.php +++ b/Tests/Fixtures/Kernel.php @@ -9,7 +9,7 @@ use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\HttpKernel\Kernel as BaseKernel; -class Kernel extends BaseKernel +final class Kernel extends BaseKernel { public function __construct($environment, $debug) { @@ -20,12 +20,12 @@ public function __construct($environment, $debug) public function getCacheDir() { - return __DIR__ . '/../../build/cache/'; + return __DIR__ . '/../../build/' . $this->getName() . '/cache'; } public function getLogDir() { - return __DIR__ . '/../../build/log/'; + return __DIR__ . '/../../build/' . $this->getName() . '/log'; } /** {@inheritdoc} */ @@ -39,6 +39,11 @@ public function registerBundles() ]; } + public function getName() + { + return 'rpc_test'; + } + /** {@inheritdoc} */ public function registerContainerConfiguration(LoaderInterface $loader) { diff --git a/Tests/Fixtures/Rpc/TestController.php b/Tests/Fixtures/Rpc/TestController.php index 3b08c94..6d6ca80 100644 --- a/Tests/Fixtures/Rpc/TestController.php +++ b/Tests/Fixtures/Rpc/TestController.php @@ -27,6 +27,6 @@ public function rpcAction(Request $request) */ protected function getResolver() { - return $this->get('rpc.controller_resolver'); + return $this->get('rpc_server.controller_resolver'); } } diff --git a/Tests/Fixtures/config/test.yml b/Tests/Fixtures/config/test.yml index 3efb613..382f90c 100644 --- a/Tests/Fixtures/config/test.yml +++ b/Tests/Fixtures/config/test.yml @@ -20,7 +20,7 @@ security: access_control: - { path: ^/, roles: IS_AUTHENTICATED_ANONYMOUSLY } -rpc: +rpc_server: router: endpoints: test: diff --git a/Tests/RoutingTest.php b/Tests/RoutingTest.php index 887d4aa..341bd28 100644 --- a/Tests/RoutingTest.php +++ b/Tests/RoutingTest.php @@ -28,7 +28,7 @@ public function testRpcRouterCollection() { $client = self::createClient(); /** @var Router $router */ - $router = $client->getContainer()->get('rpc.endpoint_router.test'); + $router = $client->getContainer()->get('rpc_server.endpoint_router.test'); self::assertNotNull($router); /** @var MethodCollection $collection */ $collection = $router->getMethodCollection(); diff --git a/dump_reference.php b/dump_reference.php new file mode 100644 index 0000000..7ebaa42 --- /dev/null +++ b/dump_reference.php @@ -0,0 +1,12 @@ +dump(new Configuration()); + + From d7aa2b497f520f2238e5fd20ec2fdb6f7e12bfb1 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Wed, 19 Jul 2017 12:01:57 +0300 Subject: [PATCH 12/22] Update build order --- .travis.yml | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/.travis.yml b/.travis.yml index 851be4c..bac039c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,18 +1,18 @@ language: php php: - - 5.5 - - 5.6 - - 7 - - 7.1 - nightly + - 7.1 + - 7 + - 5.6 + - 5.5 env: - - PACKAGES='symfony/symfony=2.7.*' deps='no' - - PACKAGES='symfony/symfony=2.8.*' deps='no' - - PACKAGES='symfony/symfony=3.2.*' deps='no' - - PACKAGES='symfony/symfony=3.3.*' deps='no' - - PACKAGES='symfony/symfony=~3.4' STABILITY=dev deps='no' + - PACKAGES='symfony/symfony=~3.4' STABILITY=dev + - PACKAGES='symfony/symfony=3.3.*' + - PACKAGES='symfony/symfony=3.2.*' + - PACKAGES='symfony/symfony=2.8.*' + - PACKAGES='symfony/symfony=2.7.*' ## Run on container environment sudo: false @@ -24,16 +24,14 @@ cache: matrix: include: - - php: 5.5 - env: PACKAGES='symfony/symfony=2.7.*' deps='low' - php: 7.1 - env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev deps='no' + env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev - php: nightly - env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev deps='no' + env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev allow_failures: - php: nightly - - env: PACKAGES='symfony/symfony=~4.0@dev' STABILITY=dev + - env: PACKAGES='symfony/symfony=~4.0' STABILITY=dev before_install: - travis_retry composer self-update @@ -41,7 +39,7 @@ before_install: install: - composer require --no-update ${PACKAGES} - if [ "$STABILITY" = "dev" ]; then composer config minimum-stability dev; fi; - - if [ "$deps" = "no" ]; then composer --prefer-source install; fi; + - if [ "$deps" = "no" ] || [ -z "$deps" ]; then composer --prefer-source install; fi; - if [ "$deps" = "low" ]; then composer --prefer-source --prefer-lowest --prefer-stable update; fi; script: From 82bcc526252cea6aab8f072179cc1d70e6712fe6 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Wed, 19 Jul 2017 17:00:39 +0300 Subject: [PATCH 13/22] Update test layout --- Command/RouterDebugCommand.php | 2 +- Controller/RpcController.php | 23 ++++++++------ Event/FilterResponseEvent.php | 3 +- Event/GetResponseEvent.php | 3 -- .../InvalidMethodParametersException.php | 10 +++++-- Listener/ControllerListener.php | 2 +- README.md | 2 +- .../ControllerResolver/ControllerResolver.php | 3 +- Routing/Exception/FileLoaderLoadException.php | 27 ++++++++++------- Routing/Loader/AnnotationDirectoryLoader.php | 6 ++-- Routing/Loader/AnnotationFileLoader.php | 21 +++++++------ Routing/Loader/DirectoryLoader.php | 2 +- Routing/Loader/YamlLoader.php | 30 +++++++++---------- Routing/LoaderResolver.php | 4 +-- Routing/MatcherDumper.php | 2 +- Routing/Route.php | 2 -- Routing/Router.php | 2 +- Rpc/ProxyController.php | 2 +- {Tests/Fixtures => Test}/Impl/Request.php | 2 +- {Tests/Fixtures => Test}/Impl/Response.php | 8 +++-- {Tests/Fixtures => Test}/Kernel.php | 2 +- .../Rpc/RpcImplController.php | 4 +-- .../Rpc/SecurityController.php | 4 +-- .../Fixtures => Test}/Rpc/TestController.php | 4 +-- {Tests => Test/Tests}/RoutingTest.php | 6 ++-- {Tests => Test/Tests}/RpcTest.php | 4 +-- .../Tests}/SecurityControllerTest.php | 4 +-- {Tests/Fixtures => Test}/config/routing.yml | 0 Test/config/rpc_routing.yml | 23 ++++++++++++++ {Tests/Fixtures => Test}/config/test.yml | 4 +-- Tests/Fixtures/config/rpc_routing.yml | 23 -------------- bootstrap.php | 4 +-- phpunit.xml | 11 +++++-- 33 files changed, 134 insertions(+), 115 deletions(-) rename {Tests/Fixtures => Test}/Impl/Request.php (95%) rename {Tests/Fixtures => Test}/Impl/Response.php (82%) rename {Tests/Fixtures => Test}/Kernel.php (96%) rename {Tests/Fixtures => Test}/Rpc/RpcImplController.php (87%) rename {Tests/Fixtures => Test}/Rpc/SecurityController.php (87%) rename {Tests/Fixtures => Test}/Rpc/TestController.php (88%) rename {Tests => Test/Tests}/RoutingTest.php (94%) rename {Tests => Test/Tests}/RpcTest.php (97%) rename {Tests => Test/Tests}/SecurityControllerTest.php (92%) rename {Tests/Fixtures => Test}/config/routing.yml (100%) create mode 100644 Test/config/rpc_routing.yml rename {Tests/Fixtures => Test}/config/test.yml (80%) delete mode 100644 Tests/Fixtures/config/rpc_routing.yml diff --git a/Command/RouterDebugCommand.php b/Command/RouterDebugCommand.php index 332a37f..65aaaaf 100644 --- a/Command/RouterDebugCommand.php +++ b/Command/RouterDebugCommand.php @@ -14,7 +14,7 @@ class RouterDebugCommand extends Command { /** @var RouterCollection */ private $routers; - + public function __construct(RouterCollection $routers) { parent::__construct(); diff --git a/Controller/RpcController.php b/Controller/RpcController.php index 2ea9c60..5b551e9 100644 --- a/Controller/RpcController.php +++ b/Controller/RpcController.php @@ -25,8 +25,6 @@ abstract class RpcController implements ContainerAwareInterface { /** @var ContainerInterface */ private $container; - /** @var EventDispatcherInterface */ - private $dispatcher; /** * Sets the container. @@ -39,7 +37,6 @@ abstract class RpcController implements ContainerAwareInterface public function setContainer(ContainerInterface $container = null) { $this->container = $container; - $this->dispatcher = $this->get('event_dispatcher'); } /** @@ -76,7 +73,7 @@ protected function handleSingleRequest(RpcRequestInterface $request) { // request $event = new GetResponseEvent($this->getKernel(), $request); - $this->dispatcher->dispatch(RpcEvents::REQUEST, $event); + $this->getDispatcher()->dispatch(RpcEvents::REQUEST, $event); if ($event->hasResponse()) { return $this->filterResponse($event->getResponse(), $request); } @@ -85,7 +82,7 @@ protected function handleSingleRequest(RpcRequestInterface $request) throw new MethodNotFoundException($request->getMethod()); } $event = new FilterControllerEvent($this->getKernel(), $request, $controller); - $this->dispatcher->dispatch(RpcEvents::CONTROLLER, $event); + $this->getDispatcher()->dispatch(RpcEvents::CONTROLLER, $event); $controller = $event->getController(); // controller arguments $arguments = $this->getResolver()->getArguments($request, $controller); @@ -94,7 +91,7 @@ protected function handleSingleRequest(RpcRequestInterface $request) // view if (!$response instanceof RpcResponseInterface) { $event = new ViewEvent($this->getKernel(), $request, $response); - $this->dispatcher->dispatch(RpcEvents::VIEW, $event); + $this->getDispatcher()->dispatch(RpcEvents::VIEW, $event); if ($event->hasResponse()) { $response = $event->getResponse(); } @@ -139,7 +136,7 @@ protected function get($name) protected function filterResponse(RpcResponseInterface $response, RpcRequestInterface $request) { $event = new FilterResponseEvent($this->getKernel(), $request, $response); - $this->dispatcher->dispatch(RpcEvents::RESPONSE, $event); + $this->getDispatcher()->dispatch(RpcEvents::RESPONSE, $event); $this->finishRequest($request); return $event->getResponse(); @@ -158,7 +155,7 @@ protected function filterResponse(RpcResponseInterface $response, RpcRequestInte */ protected function finishRequest(RpcRequestInterface $request) { - $this->dispatcher->dispatch( + $this->getDispatcher()->dispatch( RpcEvents::FINISH_REQUEST, new FinishRequestEvent($this->getKernel(), $request) ); @@ -216,7 +213,7 @@ protected function varToString($var) protected function handleException(\Exception $e, RpcRequestInterface $request) { $event = new GetExceptionResponseEvent($this->getKernel(), $request, $e); - $this->dispatcher->dispatch(RpcEvents::EXCEPTION, $event); + $this->getDispatcher()->dispatch(RpcEvents::EXCEPTION, $event); // a listener might have replaced the exception $e = $event->getException(); if (!$event->hasResponse()) { @@ -232,6 +229,14 @@ protected function handleException(\Exception $e, RpcRequestInterface $request) } } + /** + * @return EventDispatcherInterface + */ + protected function getDispatcher() + { + return $this->get('event_dispatcher'); + } + /** * @return KernelInterface * @throws \RuntimeException diff --git a/Event/FilterResponseEvent.php b/Event/FilterResponseEvent.php index db8b559..2734221 100644 --- a/Event/FilterResponseEvent.php +++ b/Event/FilterResponseEvent.php @@ -17,8 +17,7 @@ public function __construct( HttpKernelInterface $kernel, RpcRequestInterface $request, RpcResponseInterface $response - ) - { + ) { parent::__construct($kernel, $request); $this->response = $response; } diff --git a/Event/GetResponseEvent.php b/Event/GetResponseEvent.php index 8d0b720..3b6e3a9 100644 --- a/Event/GetResponseEvent.php +++ b/Event/GetResponseEvent.php @@ -2,9 +2,6 @@ namespace Bankiru\Api\Rpc\Event; -use Bankiru\Api\Rpc\RpcEvent; -use ScayTrase\Api\Rpc\RpcResponseInterface; - class GetResponseEvent extends RpcResponseEvent { public function getEndpoint() diff --git a/Exception/InvalidMethodParametersException.php b/Exception/InvalidMethodParametersException.php index 5cee084..ccb25c2 100644 --- a/Exception/InvalidMethodParametersException.php +++ b/Exception/InvalidMethodParametersException.php @@ -5,7 +5,7 @@ class InvalidMethodParametersException extends \InvalidArgumentException implements RpcException { /** - * @param string $method + * @param string $method * @param string[] $missing * * @return static @@ -32,7 +32,13 @@ public static function missing($method, array $missing) public static function typeMismatch($method, $name, $expected, $actual) { return new static( - sprintf('Parameter %s for method "%s" has invalid type: %s given, %s expected', $name, $method, $actual, $expected) + sprintf( + 'Parameter %s for method "%s" has invalid type: %s given, %s expected', + $name, + $method, + $actual, + $expected + ) ); } } diff --git a/Listener/ControllerListener.php b/Listener/ControllerListener.php index b77a1c1..15ee187 100644 --- a/Listener/ControllerListener.php +++ b/Listener/ControllerListener.php @@ -45,7 +45,7 @@ final class ControllerListener implements EventSubscriberInterface /** * @var Reader */ - protected $reader; + private $reader; /** * Constructor. diff --git a/README.md b/README.md index cb58908..9f27575 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,7 @@ To enable HTTP endpoint you should enable custom endpoint router loader via the ```yaml # app/config/routing.yml -rpc: +rpc_server: resource: . type: endpoint diff --git a/Routing/ControllerResolver/ControllerResolver.php b/Routing/ControllerResolver/ControllerResolver.php index 8db50b6..dfa5d0e 100644 --- a/Routing/ControllerResolver/ControllerResolver.php +++ b/Routing/ControllerResolver/ControllerResolver.php @@ -48,8 +48,7 @@ public function __construct( ContainerInterface $container, ControllerNameParser $parser, LoggerInterface $logger = null - ) - { + ) { $this->container = $container; $this->parser = $parser; diff --git a/Routing/Exception/FileLoaderLoadException.php b/Routing/Exception/FileLoaderLoadException.php index 37ae8c1..fcd5e5a 100644 --- a/Routing/Exception/FileLoaderLoadException.php +++ b/Routing/Exception/FileLoaderLoadException.php @@ -42,7 +42,7 @@ public function __construct($resource, $sourceResource = null, $code = null, $pr // Trim the trailing period of the previous message. We only want 1 period remove so no rtrim... if ('.' === substr($previous->getMessage(), -1)) { $trimmedMessage = substr($previous->getMessage(), 0, -1); - $message .= sprintf('%s', $trimmedMessage) . ' in '; + $message .= sprintf('%s', $trimmedMessage) . ' in '; } else { $message .= sprintf('%s', $previous->getMessage()) . ' in '; } @@ -55,24 +55,29 @@ public function __construct($resource, $sourceResource = null, $code = null, $pr $message .= sprintf('(which is being imported from "%s")', $this->varToString($sourceResource)); } $message .= '.'; - // if there's no previous message, present it the default way } elseif (null === $sourceResource) { $message .= sprintf('Cannot load resource "%s".', $this->varToString($resource)); } else { - $message .= sprintf('Cannot import resource "%s" from "%s".', - $this->varToString($resource), - $this->varToString($sourceResource)); + $message .= sprintf( + 'Cannot import resource "%s" from "%s".', + $this->varToString($resource), + $this->varToString($sourceResource) + ); } // Is the resource located inside a bundle? if ('@' === $resource[0]) { - $parts = explode(DIRECTORY_SEPARATOR, $resource); - $bundle = substr($parts[0], 1); - $message .= sprintf(' Make sure the "%s" bundle is correctly registered and loaded in the application kernel class.', - $bundle); - $message .= sprintf(' If the bundle is registered, make sure the bundle path "%s" is not empty.', - $resource); + $parts = explode(DIRECTORY_SEPARATOR, $resource); + $bundle = substr($parts[0], 1); + $message .= sprintf( + ' Make sure the "%s" bundle is correctly registered and loaded in the application kernel class.', + $bundle + ); + $message .= sprintf( + ' If the bundle is registered, make sure the bundle path "%s" is not empty.', + $resource + ); } parent::__construct($message, $code, $previous); diff --git a/Routing/Loader/AnnotationDirectoryLoader.php b/Routing/Loader/AnnotationDirectoryLoader.php index 5121dd8..284ffd9 100644 --- a/Routing/Loader/AnnotationDirectoryLoader.php +++ b/Routing/Loader/AnnotationDirectoryLoader.php @@ -39,10 +39,12 @@ public function load($path, $type = null) \RecursiveIteratorIterator::LEAVES_ONLY ) ); - usort($files, + usort( + $files, function (\SplFileInfo $a, \SplFileInfo $b) { return (string)$a > (string)$b ? 1 : -1; - }); + } + ); foreach ($files as $file) { if (!$file->isFile() || '.php' !== substr($file->getFilename(), -4)) { diff --git a/Routing/Loader/AnnotationFileLoader.php b/Routing/Loader/AnnotationFileLoader.php index c822b80..fbe024c 100644 --- a/Routing/Loader/AnnotationFileLoader.php +++ b/Routing/Loader/AnnotationFileLoader.php @@ -23,7 +23,6 @@ * */ - namespace Bankiru\Api\Rpc\Routing\Loader; use Bankiru\Api\Rpc\Routing\MethodCollection; @@ -89,6 +88,16 @@ public function load($file, $type = null) return $collection; } + /** + * {@inheritdoc} + */ + public function supports($resource, $type = null) + { + return is_string($resource) && + 'php' === pathinfo($resource, PATHINFO_EXTENSION) && + (!$type || 'annotation' === $type); + } + /** * Returns the full class name for the first class in the file. * @@ -131,14 +140,4 @@ protected function findClass($file) return false; } - - /** - * {@inheritdoc} - */ - public function supports($resource, $type = null) - { - return is_string($resource) && - 'php' === pathinfo($resource, PATHINFO_EXTENSION) && - (!$type || 'annotation' === $type); - } } diff --git a/Routing/Loader/DirectoryLoader.php b/Routing/Loader/DirectoryLoader.php index 3648eac..af623b6 100644 --- a/Routing/Loader/DirectoryLoader.php +++ b/Routing/Loader/DirectoryLoader.php @@ -43,7 +43,7 @@ public function load($file, $type = null) foreach (scandir($path) as $dir) { if ('.' !== $dir[0]) { $this->setCurrentDir($path); - $subPath = $path.'/'.$dir; + $subPath = $path . '/' . $dir; $subType = null; if (is_dir($subPath)) { diff --git a/Routing/Loader/YamlLoader.php b/Routing/Loader/YamlLoader.php index 243614e..ca09abb 100644 --- a/Routing/Loader/YamlLoader.php +++ b/Routing/Loader/YamlLoader.php @@ -81,6 +81,21 @@ public function load($resource, $type = null) return $collection; } + /** + * Returns whether this class supports the given resource. + * + * @param mixed $resource A resource + * @param string|null $type The resource type or null if unknown + * + * @return bool True if this class supports the given resource, false otherwise + */ + public function supports($resource, $type = null) + { + return is_string($resource) && + in_array(pathinfo($resource, PATHINFO_EXTENSION), ['yml', 'yaml'], true) && + (!$type || 'yaml' === $type); + } + /** * Validates the route configuration. * @@ -182,19 +197,4 @@ protected function parseRoute(MethodCollection $collection, $name, array $config $collection->add($name, $route); } - - /** - * Returns whether this class supports the given resource. - * - * @param mixed $resource A resource - * @param string|null $type The resource type or null if unknown - * - * @return bool True if this class supports the given resource, false otherwise - */ - public function supports($resource, $type = null) - { - return is_string($resource) && - in_array(pathinfo($resource, PATHINFO_EXTENSION), ['yml', 'yaml'], true) && - (!$type || 'yaml' === $type); - } } diff --git a/Routing/LoaderResolver.php b/Routing/LoaderResolver.php index 286ccd9..3c097fe 100644 --- a/Routing/LoaderResolver.php +++ b/Routing/LoaderResolver.php @@ -7,14 +7,14 @@ final class LoaderResolver implements LoaderResolverInterface /** * @var LoaderInterface[] An array of LoaderInterface objects */ - private $loaders = array(); + private $loaders = []; /** * Constructor. * * @param LoaderInterface[] $loaders An array of loaders */ - public function __construct(array $loaders = array()) + public function __construct(array $loaders = []) { foreach ($loaders as $loader) { $this->addLoader($loader); diff --git a/Routing/MatcherDumper.php b/Routing/MatcherDumper.php index 7298975..c587e01 100644 --- a/Routing/MatcherDumper.php +++ b/Routing/MatcherDumper.php @@ -28,7 +28,7 @@ public function match(\$method) CONTENT; foreach ($collection->all() as $name => $method) { - $ret = var_export(AttributesHelper::getAttributes($method, $name), true); + $ret = var_export(AttributesHelper::getAttributes($method, $name), true); $content .= PHP_EOL . <<getMethod()}' => {$ret}, diff --git a/Routing/Route.php b/Routing/Route.php index 74c5f2d..48a06b7 100644 --- a/Routing/Route.php +++ b/Routing/Route.php @@ -2,8 +2,6 @@ namespace Bankiru\Api\Rpc\Routing; -use Symfony\Component\HttpFoundation\ParameterBag; - class Route { /** @var string */ diff --git a/Routing/Router.php b/Routing/Router.php index 122af2c..f2bf4d8 100644 --- a/Routing/Router.php +++ b/Routing/Router.php @@ -6,7 +6,6 @@ use Symfony\Component\Config\ConfigCacheFactoryInterface; use Symfony\Component\Config\ConfigCacheInterface; use Symfony\Component\Config\Resource\FileResource; -use Symfony\Component\Config\Resource\ReflectionClassResource; use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface; final class Router implements MethodMatcher, WarmableInterface @@ -30,6 +29,7 @@ final class Router implements MethodMatcher, WarmableInterface * * @param MethodCollectionLoader $loader * @param string $name + * @param array $options */ public function __construct(MethodCollectionLoader $loader, $name, array $options = []) { diff --git a/Rpc/ProxyController.php b/Rpc/ProxyController.php index 90d7bb2..897e6e6 100644 --- a/Rpc/ProxyController.php +++ b/Rpc/ProxyController.php @@ -22,6 +22,6 @@ public function __construct(RpcClientInterface $client) public function handle(RpcRequestInterface $request) { - return $this->client->invoke($request); + return $this->client->invoke($request)->getResponse($request); } } diff --git a/Tests/Fixtures/Impl/Request.php b/Test/Impl/Request.php similarity index 95% rename from Tests/Fixtures/Impl/Request.php rename to Test/Impl/Request.php index f5f5349..d917137 100644 --- a/Tests/Fixtures/Impl/Request.php +++ b/Test/Impl/Request.php @@ -1,6 +1,6 @@ body = $body; } - + public function __construct($body) + { + $this->body = $body; + } /** @return bool */ public function isSuccessful() diff --git a/Tests/Fixtures/Kernel.php b/Test/Kernel.php similarity index 96% rename from Tests/Fixtures/Kernel.php rename to Test/Kernel.php index c7dd4f6..6b913e2 100644 --- a/Tests/Fixtures/Kernel.php +++ b/Test/Kernel.php @@ -1,6 +1,6 @@ - - + + ./Test/Tests/ + + ./Tests/ @@ -22,6 +24,11 @@ vendor/ build/ + Test/ + Tests/ + + ./ + From a9ce182ab02d744211e436bde809e59e0b81a3a3 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Thu, 20 Jul 2017 09:52:49 +0300 Subject: [PATCH 14/22] Code fixes (scrutinizer) --- .gitignore | 51 ------------------- Cache/RouterCacheWarmer.php | 3 +- Controller/RpcController.php | 2 +- .../InvalidMethodParametersException.php | 2 +- Listener/ControllerListener.php | 10 ++-- Routing/Annotation/Method.php | 17 ------- Routing/Loader/AnnotationClassLoader.php | 8 +-- Routing/Router.php | 16 ++++-- Test/Tests/RpcTest.php | 14 +---- Test/Tests/SecurityControllerTest.php | 17 +------ 10 files changed, 26 insertions(+), 114 deletions(-) diff --git a/.gitignore b/.gitignore index 1d5c172..178ff43 100644 --- a/.gitignore +++ b/.gitignore @@ -1,55 +1,4 @@ -/*.pnproj -/.idea -.htaccess -atlassian-ide-plugin.xml - -### Symfony template -# Cache and logs (Symfony2) -/app/cache/* -/app/logs/* -!app/cache/.gitkeep -!app/logs/.gitkeep - -# Cache and logs (Symfony3) -/var/* -!/var/cache -/var/cache/* -!var/cache/.gitkeep -!/var/logs -/var/logs/* -!var/logs/.gitkeep -!/var/sessions -/var/sessions/* -!var/sessions/.gitkeep -var/SymfonyRequirements.php - -# Parameters -/app/config/parameters.yml -/app/config/parameters.local.yml -/app/config/parameters.ini - -# Managed by Composer -/app/bootstrap.php.cache -/var/bootstrap.php.cache -/bin/* -!bin/console -bin/symfony_requirements /vendor/ - -# Assets and user uploads -/web/bundles/ -/web/uploads/ - -# PHPUnit -/app/phpunit.xml - -# Build data /build/ /target/ - -# Any PHARs -*.phar - -Thumbs.db - composer.lock diff --git a/Cache/RouterCacheWarmer.php b/Cache/RouterCacheWarmer.php index 59aad56..1f7c91c 100644 --- a/Cache/RouterCacheWarmer.php +++ b/Cache/RouterCacheWarmer.php @@ -3,7 +3,6 @@ namespace Bankiru\Api\Rpc\Cache; use Bankiru\Api\Rpc\Routing\MethodMatcher; -use Bankiru\Api\Rpc\Routing\Router; use Symfony\Component\HttpKernel\CacheWarmer\CacheWarmerInterface; use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface; @@ -12,7 +11,7 @@ final class RouterCacheWarmer implements CacheWarmerInterface const CACHE_DIR = 'rpc'; /** - * @var Router + * @var MethodMatcher */ private $router; diff --git a/Controller/RpcController.php b/Controller/RpcController.php index 5b551e9..faa217e 100644 --- a/Controller/RpcController.php +++ b/Controller/RpcController.php @@ -36,7 +36,7 @@ abstract class RpcController implements ContainerAwareInterface */ public function setContainer(ContainerInterface $container = null) { - $this->container = $container; + $this->container = $container; } /** diff --git a/Exception/InvalidMethodParametersException.php b/Exception/InvalidMethodParametersException.php index ccb25c2..76b04a8 100644 --- a/Exception/InvalidMethodParametersException.php +++ b/Exception/InvalidMethodParametersException.php @@ -5,7 +5,7 @@ class InvalidMethodParametersException extends \InvalidArgumentException implements RpcException { /** - * @param string $method + * @param string $method * @param string[] $missing * * @return static diff --git a/Listener/ControllerListener.php b/Listener/ControllerListener.php index 15ee187..561654f 100644 --- a/Listener/ControllerListener.php +++ b/Listener/ControllerListener.php @@ -77,10 +77,12 @@ public function onKernelController(FilterControllerEvent $event) } $className = - class_exists('Doctrine\Common\Util\ClassUtils') ? ClassUtils::getClass($controller[0]) : - get_class($controller[0]); - $object = new \ReflectionClass($className); - $method = $object->getMethod($controller[1]); + class_exists('Doctrine\Common\Util\ClassUtils') + ? ClassUtils::getClass($controller[0]) + : get_class($controller[0]); + + $object = new \ReflectionClass($className); + $method = $object->getMethod($controller[1]); $classConfigurations = $this->getConfigurations($this->reader->getClassAnnotations($object)); $methodConfigurations = $this->getConfigurations($this->reader->getMethodAnnotations($method)); diff --git a/Routing/Annotation/Method.php b/Routing/Annotation/Method.php index 4cec2b7..9ce652f 100644 --- a/Routing/Annotation/Method.php +++ b/Routing/Annotation/Method.php @@ -17,7 +17,6 @@ class Method public $context = []; public $defaultContext = true; public $inherit = true; - public $options = []; public function __construct(array $values) { @@ -122,20 +121,4 @@ public function setInherit($inherit) { $this->inherit = (bool)$inherit; } - - /** - * @return array - */ - public function getOptions() - { - return $this->options; - } - - /** - * @param array $options - */ - public function setOptions(array $options = null) - { - $this->options = $options; - } } diff --git a/Routing/Loader/AnnotationClassLoader.php b/Routing/Loader/AnnotationClassLoader.php index b214215..a1653b9 100644 --- a/Routing/Loader/AnnotationClassLoader.php +++ b/Routing/Loader/AnnotationClassLoader.php @@ -84,7 +84,6 @@ protected function getParentAnnotations(\ReflectionClass $class) 'method' => '', 'context' => [], 'default_context' => true, - 'options' => [], ]; /** @var Method $annot */ @@ -96,10 +95,6 @@ protected function getParentAnnotations(\ReflectionClass $class) if (null !== $annot->getContext()) { $parents['context'] = $annot->getContext(); } - - if (null !== $annot->getOptions()) { - $parents['options'] = $annot->getOptions(); - } } return $parents; @@ -119,8 +114,7 @@ protected function addRoute( $class->getName() . '::' . $method->getName(), array_merge($parents['context'], $annot->getContext()), $parents['default_context'] && $annot->isDefaultContext(), - $annot->isInherit(), - array_merge_recursive($parents['options'], $annot->getOptions()) + $annot->isInherit() ) ); } diff --git a/Routing/Router.php b/Routing/Router.php index f2bf4d8..412b5fb 100644 --- a/Routing/Router.php +++ b/Routing/Router.php @@ -82,9 +82,7 @@ public function warmUp($cacheDir) */ public function setOption($key, $value) { - if (!array_key_exists($key, $this->options)) { - throw new \InvalidArgumentException(sprintf('The Router does not support the "%s" option.', $key)); - } + $this->validateOption($key); $this->options[$key] = $value; } @@ -137,7 +135,7 @@ public function setOptions(array $options) } } - if ($invalid) { + if (!empty($invalid)) { throw new \InvalidArgumentException( sprintf('The Router does not support the following options: "%s".', implode('", "', $invalid)) ); @@ -197,4 +195,14 @@ private function getConfigCacheFactory() return $this->configCacheFactory; } + + /** + * @param $key + */ + private function validateOption($key) + { + if (!array_key_exists($key, $this->options)) { + throw new \InvalidArgumentException(sprintf('The Router does not support the "%s" option.', $key)); + } + } } diff --git a/Test/Tests/RpcTest.php b/Test/Tests/RpcTest.php index feeb076..4d2c5cf 100644 --- a/Test/Tests/RpcTest.php +++ b/Test/Tests/RpcTest.php @@ -40,12 +40,7 @@ public function getInvalidArgumentVariants() public function testValidController(array $args) { $client = self::createClient(); - - $client->request( - 'POST', - '/test/', - array_replace(['method' => 'test/method',], $args) - ); + $client->request('POST', '/test/', array_replace(['method' => 'test/method'], $args)); self::assertTrue($client->getResponse()->isSuccessful()); } @@ -60,12 +55,7 @@ public function testValidController(array $args) public function testInvalidController(array $args) { $client = self::createClient(); - - $client->request( - 'POST', - '/test/', - array_replace(['method' => 'test/method',], $args) - ); + $client->request('POST', '/test/', array_replace(['method' => 'test/method'], $args)); } public function getExceptionTestData() diff --git a/Test/Tests/SecurityControllerTest.php b/Test/Tests/SecurityControllerTest.php index 580cd32..7c43363 100644 --- a/Test/Tests/SecurityControllerTest.php +++ b/Test/Tests/SecurityControllerTest.php @@ -10,12 +10,7 @@ final class SecurityControllerTest extends WebTestCase public function testPublicAction() { $client = self::createClient(); - - $client->request( - 'POST', - '/test/', - ['method' => 'security/public'] - ); + $client->request('POST', '/test/', ['method' => 'security/public']); self::assertTrue($client->getResponse()->isSuccessful()); } @@ -26,15 +21,7 @@ public function testPublicAction() */ public function testPrivateAction() { - $client = self::createClient(); - - $client->request( - 'POST', - '/test/', - ['method' => 'security/private'] - ); - - self::assertTrue($client->getResponse()->isSuccessful()); + self::createClient()->request('POST', '/test/', ['method' => 'security/private']); } protected static function getKernelClass() From 64bcdd97e8f47ab56f625eb9a71d4cfcde05220f Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Thu, 20 Jul 2017 10:22:41 +0300 Subject: [PATCH 15/22] Code fixes (insight) --- Listener/SecurityListener.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Listener/SecurityListener.php b/Listener/SecurityListener.php index 1a9ffa3..89cc2d8 100644 --- a/Listener/SecurityListener.php +++ b/Listener/SecurityListener.php @@ -36,6 +36,7 @@ use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; use Symfony\Component\Security\Core\Exception\AccessDeniedException; use Symfony\Component\Security\Core\Role\RoleHierarchyInterface; +use Symfony\Component\Security\Core\Role\RoleInterface; /** * SecurityListener handles security restrictions on controllers. @@ -113,7 +114,7 @@ private function getVariables(RpcRequestInterface $request) 'object' => $request, 'request' => $request, 'roles' => array_map( - function ($role) { + function (RoleInterface $role) { return $role->getRole(); }, $roles From 12ab208cd044dc3e6692d2923c667304102c391c Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Thu, 27 Jul 2017 17:46:18 +0300 Subject: [PATCH 16/22] Fix command --- Resources/config/rpc.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Resources/config/rpc.yml b/Resources/config/rpc.yml index 15c6ace..04fac13 100644 --- a/Resources/config/rpc.yml +++ b/Resources/config/rpc.yml @@ -3,6 +3,8 @@ services: class: Bankiru\Api\Rpc\Command\RouterDebugCommand arguments: - "@rpc_server.router.collection" + tags: + - "console.command" rpc_server.controller_resolver: class: Bankiru\Api\Rpc\Routing\ControllerResolver\ControllerResolver From 199aa8eae7949fd041778abf0c2420152fb2bcd9 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 28 Jul 2017 08:48:47 +0300 Subject: [PATCH 17/22] Make annotations dev dependency --- composer.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/composer.json b/composer.json index fa96e99..fb14357 100644 --- a/composer.json +++ b/composer.json @@ -16,10 +16,10 @@ "symfony/http-kernel": "~2.7 || ~3.0 || ~4.0", "symfony/yaml": "~2.7 || ~3.0 || ~4.0", "symfony/dependency-injection": "~2.7 || ~3.0 || ~4.0", - "doctrine/annotations": "~1.2", "scaytrase/rpc-common": "~1.0" }, "require-dev": { + "doctrine/annotations": "~1.2", "doctrine/common": "~2.2", "sensio/framework-extra-bundle": "~3.0", "symfony/expression-language": "~2.7 || ~3.0 || ~4.0", @@ -31,6 +31,7 @@ "phpunit/phpunit": "^4.8.35 || ^5.4 || ^6.0" }, "suggest": { + "doctrine/annotations": "For RPC method annotations", "sensio/framework-extra-bundle": "For controller annotations", "symfony/expression-language": "For evaluating security expressions", "symfony/console": "For using debug console command", From 5c055b3fb6a4fb698fbc5d82840cfa85a2d7c4e2 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Fri, 28 Jul 2017 10:04:36 +0300 Subject: [PATCH 18/22] Fix tag notation --- Resources/config/rpc.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Resources/config/rpc.yml b/Resources/config/rpc.yml index 04fac13..bae009f 100644 --- a/Resources/config/rpc.yml +++ b/Resources/config/rpc.yml @@ -4,7 +4,7 @@ services: arguments: - "@rpc_server.router.collection" tags: - - "console.command" + - { name: "console.command" } rpc_server.controller_resolver: class: Bankiru\Api\Rpc\Routing\ControllerResolver\ControllerResolver From 2f5fc69f2ec91ff7b458c0d00a2da3119f6d59fb Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Mon, 31 Jul 2017 10:43:53 +0300 Subject: [PATCH 19/22] Do not include default context flag, handled internally --- Routing/AttributesHelper.php | 1 - 1 file changed, 1 deletion(-) diff --git a/Routing/AttributesHelper.php b/Routing/AttributesHelper.php index 660518c..76cdaf7 100644 --- a/Routing/AttributesHelper.php +++ b/Routing/AttributesHelper.php @@ -17,7 +17,6 @@ public static function getAttributes(Route $method, $name) return [ '_route' => $name, '_controller' => $method->getController(), - '_with_default_context' => $method->includeDefaultContext(), '_context' => $method->getContext(), ]; } From d245a6e2f7c28790d824d7c2ce6320765033dc32 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Mon, 31 Jul 2017 11:26:23 +0300 Subject: [PATCH 20/22] Rename default context accessor --- Routing/Annotation/Method.php | 2 +- Routing/Loader/AnnotationClassLoader.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Routing/Annotation/Method.php b/Routing/Annotation/Method.php index 9ce652f..6ff282f 100644 --- a/Routing/Annotation/Method.php +++ b/Routing/Annotation/Method.php @@ -93,7 +93,7 @@ public function setContext($context) /** * @return boolean */ - public function isDefaultContext() + public function withDefaultContext() { return $this->defaultContext; } diff --git a/Routing/Loader/AnnotationClassLoader.php b/Routing/Loader/AnnotationClassLoader.php index a1653b9..83b2e32 100644 --- a/Routing/Loader/AnnotationClassLoader.php +++ b/Routing/Loader/AnnotationClassLoader.php @@ -113,7 +113,7 @@ protected function addRoute( $annot->getMethod(), $class->getName() . '::' . $method->getName(), array_merge($parents['context'], $annot->getContext()), - $parents['default_context'] && $annot->isDefaultContext(), + $parents['default_context'] && $annot->withDefaultContext(), $annot->isInherit() ) ); From d3be0703a7670bde8b3f620cdf5aecf8c8731b17 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Mon, 31 Jul 2017 14:14:46 +0300 Subject: [PATCH 21/22] Add debug option for exception listener --- .../BankiruRpcServerExtension.php | 13 +++++++++ .../Compiler/RouterLoaderPass.php | 17 +---------- DependencyInjection/Configuration.php | 4 ++- Listener/ExceptionListener.php | 29 ++++++++++++------- 4 files changed, 36 insertions(+), 27 deletions(-) diff --git a/DependencyInjection/BankiruRpcServerExtension.php b/DependencyInjection/BankiruRpcServerExtension.php index fd9577b..8c16d72 100644 --- a/DependencyInjection/BankiruRpcServerExtension.php +++ b/DependencyInjection/BankiruRpcServerExtension.php @@ -3,9 +3,11 @@ namespace Bankiru\Api\Rpc\DependencyInjection; use Bankiru\Api\Rpc\Cache\RouterCacheWarmer; +use Bankiru\Api\Rpc\Listener\ExceptionListener; use Bankiru\Api\Rpc\Listener\RouterListener; use Bankiru\Api\Rpc\Routing\ResourceMethodCollectionLoader; use Bankiru\Api\Rpc\Routing\Router; +use Bankiru\Api\Rpc\RpcEvents; use Sensio\Bundle\FrameworkExtraBundle\SensioFrameworkExtraBundle; use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\FileLocator; @@ -38,6 +40,17 @@ public function load(array $configs, ContainerBuilder $container) } } + $container + ->register('rpc_server.exception_listener', ExceptionListener::class) + ->setArguments([$config['debug'], new Reference('logger')]) + ->addTag( + 'kernel.event_listener', + [ + 'event' => RpcEvents::EXCEPTION, + 'method' => 'onException', + ] + ); + $this->configureRouter($config['router'], $container); } diff --git a/DependencyInjection/Compiler/RouterLoaderPass.php b/DependencyInjection/Compiler/RouterLoaderPass.php index dbc1d6b..7f80af9 100644 --- a/DependencyInjection/Compiler/RouterLoaderPass.php +++ b/DependencyInjection/Compiler/RouterLoaderPass.php @@ -2,30 +2,15 @@ namespace Bankiru\Api\Rpc\DependencyInjection\Compiler; -use Bankiru\Api\Rpc\Listener\ExceptionListener; -use Bankiru\Api\Rpc\RpcEvents; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; -class RouterLoaderPass implements CompilerPassInterface +final class RouterLoaderPass implements CompilerPassInterface { /** {@inheritdoc} */ public function process(ContainerBuilder $container) { - if ($container->has('logger')) { - $container - ->register('rpc_server.exception_listener', ExceptionListener::class) - ->setArguments([new Reference('logger')]) - ->addTag( - 'kernel.event_listener', - [ - 'event' => RpcEvents::EXCEPTION, - 'method' => 'onException', - ] - ); - } - $loader = $container->getDefinition('rpc_server.router.resolver'); $taggedServices = $container->findTaggedServiceIds('rpc.route_loader'); diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index a8f478b..fe32644 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -10,7 +10,6 @@ final class Configuration implements ConfigurationInterface { - /** * Generates the configuration tree builder. * @@ -20,6 +19,9 @@ public function getConfigTreeBuilder() { $builder = new TreeBuilder(); $root = $builder->root('rpc_server'); + + $root->children()->booleanNode('debug')->defaultValue('%kernel.debug%'); + $this->configureRouter($root->children()->arrayNode('router')); return $builder; diff --git a/Listener/ExceptionListener.php b/Listener/ExceptionListener.php index bb92a78..0133554 100644 --- a/Listener/ExceptionListener.php +++ b/Listener/ExceptionListener.php @@ -4,20 +4,27 @@ use Bankiru\Api\Rpc\Event\GetExceptionResponseEvent; use Psr\Log\LoggerInterface; +use Psr\Log\NullLogger; final class ExceptionListener { /** @var LoggerInterface */ private $logger; + /** + * @var bool + */ + private $debug; /** * ExceptionHandlerListener constructor. * + * @param bool $debug * @param LoggerInterface $logger */ - public function __construct(LoggerInterface $logger) + public function __construct($debug = false, LoggerInterface $logger = null) { - $this->logger = $logger; + $this->logger = $logger ?: new NullLogger(); + $this->debug = $debug; } public function onException(GetExceptionResponseEvent $event) @@ -25,13 +32,15 @@ public function onException(GetExceptionResponseEvent $event) $request = $event->getRequest(); $exception = $event->getException(); - $this->logger->critical( - $exception->getMessage(), - [ - 'endpoint' => $request->getAttributes()->get('_endpoint'), - 'method' => $request->getMethod(), - 'parameters' => json_decode(json_encode($request->getParameters()), true), - ] - ); + $context = [ + 'endpoint' => $request->getAttributes()->get('_endpoint'), + 'method' => $request->getMethod(), + ]; + + if ($this->debug) { + $context['parameters'] = json_decode(json_encode($request->getParameters()), true); + } + + $this->logger->critical($exception->getMessage(), $context); } } From 8da1a5b330987d84d1fc1252a26895490fe96f15 Mon Sep 17 00:00:00 2001 From: Pavel Batanov Date: Tue, 1 Aug 2017 08:18:50 +0300 Subject: [PATCH 22/22] Ignore missing logger --- DependencyInjection/BankiruRpcServerExtension.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DependencyInjection/BankiruRpcServerExtension.php b/DependencyInjection/BankiruRpcServerExtension.php index 8c16d72..496c585 100644 --- a/DependencyInjection/BankiruRpcServerExtension.php +++ b/DependencyInjection/BankiruRpcServerExtension.php @@ -12,6 +12,7 @@ use Symfony\Bundle\SecurityBundle\SecurityBundle; use Symfony\Component\Config\FileLocator; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Extension\Extension; use Symfony\Component\DependencyInjection\Loader\YamlFileLoader; @@ -42,7 +43,7 @@ public function load(array $configs, ContainerBuilder $container) $container ->register('rpc_server.exception_listener', ExceptionListener::class) - ->setArguments([$config['debug'], new Reference('logger')]) + ->setArguments([$config['debug'], new Reference('logger', ContainerInterface::NULL_ON_INVALID_REFERENCE)]) ->addTag( 'kernel.event_listener', [