From d56773e53be35f5af6bc095daaf3723de58c0e09 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Fri, 12 Jan 2018 17:31:35 +0100 Subject: [PATCH 1/3] [WIP] Draft implementation to outline general ideas for a general batch API endpoint --- src/Action/BatchEndpointAction.php | 149 ++++++++++++++++++ .../ApiPlatformExtension.php | 1 + .../DependencyInjection/Configuration.php | 1 + .../Symfony/Bundle/Resources/config/api.xml | 5 + src/Bridge/Symfony/Routing/ApiLoader.php | 21 ++- src/EventListener/DeserializeListener.php | 30 +++- src/Util/RequestAttributesExtractor.php | 4 + 7 files changed, 204 insertions(+), 7 deletions(-) create mode 100644 src/Action/BatchEndpointAction.php diff --git a/src/Action/BatchEndpointAction.php b/src/Action/BatchEndpointAction.php new file mode 100644 index 00000000000..cbde23881bf --- /dev/null +++ b/src/Action/BatchEndpointAction.php @@ -0,0 +1,149 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Action; +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\HttpKernel\HttpKernelInterface; + +/** + * Provides an endpoint for batch processing by splitting up + * + * @author Yanick Witschi + */ +class BatchEndpointAction +{ + /** + * @var HttpKernelInterface + */ + private $kernel; + + /** + * @var array + */ + private $validKeys = [ + 'path', + 'method', + 'headers', + 'body', + ]; + + /** + * @param HttpKernelInterface $kernel + */ + public function __construct(HttpKernelInterface $kernel) + { + $this->kernel = $kernel; + } + + /** + * Execute multiple subrequests from batch request. + * + * @param Request $request + * @param array $data + * + * @return array + */ + public function __invoke(Request $request, array $data = null): array + { + if (!$this->validateBatchData((array) $data)) { + throw new BadRequestHttpException('Batch request data not accepted.'); + } + + $result = []; + + foreach ($data as $k => $item) { + + // Copy current headers if no specific provided for simplicity + // otherwise one would have to provide Content-Type with every + // single entry. + if (!isset($item['headers'])) { + $item['headers'] = $request->headers->all(); + } + + $result[] = $this->convertResponse( + $this->executeSubRequest( + $k, + $item['path'], + $item['method'], + $item['headers'], + $item['body'] + ) + ); + } + + return $result; + } + + /** + * Converts a response into an array. + * + * @param Response $response + * + * @return array + */ + private function convertResponse(Response $response): array + { + return [ + 'status' => $response->getStatusCode(), + 'body' => $response->getContent(), + 'headers' => $response->headers->all(), + ]; + } + + /** + * Validates that the keys are all correctly present. + * + * @param array $data + * + * @return bool + */ + private function validateBatchData(array $data) + { + if (0 === count($data)) { + return false; + } + + foreach ($data as $item) { + if (0 !== count(array_diff(array_keys($item), $this->validKeys))) { + return false; + } + } + + return true; + } + + /** + * Executes a subrequest. + * + * @param int $index + * @param string $path + * @param string $method + * @param array $headers + * @param string $body + * + * @return Response + */ + private function executeSubRequest(int $index, string $path, string $method, array $headers, string $body): Response + { + $subRequest = Request::create($path, $method, [], [], [], [], $body); + $subRequest->headers->replace($headers); + + try { + return $this->kernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST); + } catch (\Exception $e) { + return Response::create(sprintf('Batch element #%d failed, check the log files.', $index), 400); + } + } +} diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index 0affa1ffedb..813f659e268 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -145,6 +145,7 @@ private function handleConfig(ContainerBuilder $container, array $config, array { $container->setParameter('api_platform.enable_entrypoint', $config['enable_entrypoint']); $container->setParameter('api_platform.enable_docs', $config['enable_docs']); + $container->setParameter('api_platform.enable_batch_endpoint', $config['enable_batch_endpoint']); $container->setParameter('api_platform.title', $config['title']); $container->setParameter('api_platform.description', $config['description']); $container->setParameter('api_platform.version', $config['version']); diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index ae5bad9f34a..fb506a1041c 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -101,6 +101,7 @@ public function getConfigTreeBuilder() ->booleanNode('enable_swagger_ui')->defaultValue(class_exists(TwigBundle::class))->info('Enable Swagger ui.')->end() ->booleanNode('enable_entrypoint')->defaultTrue()->info('Enable the entrypoint')->end() ->booleanNode('enable_docs')->defaultTrue()->info('Enable the docs')->end() + ->booleanNode('enable_batch_endpoint')->defaultFalse()->info('Enable the batch endpoint')->end() ->arrayNode('oauth') ->canBeEnabled() diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml index ee34b703664..d50e55a6de5 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -40,6 +40,7 @@ %api_platform.graphql.enabled% %api_platform.enable_entrypoint% %api_platform.enable_docs% + %api_platform.enable_batch_endpoint% @@ -195,6 +196,10 @@ + + + + %api_platform.title% diff --git a/src/Bridge/Symfony/Routing/ApiLoader.php b/src/Bridge/Symfony/Routing/ApiLoader.php index 08f57cdafb9..ae351eebc65 100644 --- a/src/Bridge/Symfony/Routing/ApiLoader.php +++ b/src/Bridge/Symfony/Routing/ApiLoader.php @@ -53,8 +53,9 @@ final class ApiLoader extends Loader private $graphqlEnabled; private $entrypointEnabled; private $docsEnabled; + private $batchEndpointEnabled; - public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true) + public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, bool $batchEndpointEnabled = false) { $this->fileLoader = new XmlFileLoader(new FileLocator($kernel->locateResource('@ApiPlatformBundle/Resources/config/routing'))); $this->resourceNameCollectionFactory = $resourceNameCollectionFactory; @@ -67,6 +68,7 @@ public function __construct(KernelInterface $kernel, ResourceNameCollectionFacto $this->graphqlEnabled = $graphqlEnabled; $this->entrypointEnabled = $entrypointEnabled; $this->docsEnabled = $docsEnabled; + $this->batchEndpointEnabled = $batchEndpointEnabled; } /** @@ -130,6 +132,23 @@ public function load($data, $type = null): RouteCollection } } + if ($this->batchEndpointEnabled) { + $routeCollection->add(RouteNameGenerator::ROUTE_NAME_PREFIX . 'batch_endpoint', new Route( + '/batch', // TODO: make configurable + [ + '_controller' => self::DEFAULT_ACTION_PATTERN.'batch_endpoint', + '_format' => null, + '_api_batch_request' => true, + '_api_respond' => true, + ], + [], + [], + '', + [], + ['POST'] + )); + } + return $routeCollection; } diff --git a/src/EventListener/DeserializeListener.php b/src/EventListener/DeserializeListener.php index c9e92bb6ee1..537378bd18f 100644 --- a/src/EventListener/DeserializeListener.php +++ b/src/EventListener/DeserializeListener.php @@ -18,6 +18,7 @@ use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException; +use Symfony\Component\Serializer\Encoder\DecoderInterface; use Symfony\Component\Serializer\Normalizer\AbstractNormalizer; use Symfony\Component\Serializer\SerializerInterface; @@ -47,17 +48,34 @@ public function __construct(SerializerInterface $serializer, SerializerContextBu public function onKernelRequest(GetResponseEvent $event) { $request = $event->getRequest(); - if ( - $request->isMethodSafe(false) + + if ($request->isMethodSafe(false) || $request->isMethod(Request::METHOD_DELETE) - || !($attributes = RequestAttributesExtractor::extractAttributes($request)) - || !$attributes['receive'] - || ('' === ($requestContent = $request->getContent()) && $request->isMethod(Request::METHOD_PUT)) ) { return; } $format = $this->getFormat($request); + + if ($request->attributes->get('_api_batch_request') && '' !== $request->getContent()) { + if (!$this->serializer instanceof DecoderInterface) { + return; + } + + $request->attributes->set( + 'data', + $this->serializer->decode($request->getContent(), $format) + ); + return; + } + + if (!($attributes = RequestAttributesExtractor::extractAttributes($request)) + || !$attributes['receive'] + || ('' === $request->getContent() && $request->isMethod(Request::METHOD_PUT)) + ) { + return; + } + $context = $this->serializerContextBuilder->createFromRequest($request, false, $attributes); $data = $request->attributes->get('data'); @@ -68,7 +86,7 @@ public function onKernelRequest(GetResponseEvent $event) $request->attributes->set( 'data', $this->serializer->deserialize( - $requestContent, $attributes['resource_class'], $format, $context + $request->getContent(), $attributes['resource_class'], $this->getFormat($request), $context ) ); } diff --git a/src/Util/RequestAttributesExtractor.php b/src/Util/RequestAttributesExtractor.php index 77d60e91119..dcc3b291848 100644 --- a/src/Util/RequestAttributesExtractor.php +++ b/src/Util/RequestAttributesExtractor.php @@ -45,6 +45,10 @@ public static function extractAttributes(Request $request) $result['subresource_context'] = $subresourceContext; } + if ($isBatchRequest = $request->attributes->get('_api_batch_request')) { + $result['batch_request'] = true; + } + if (null === $result['resource_class']) { return []; } From 90806c545db03836d6a48c9e094827a9f905e5d4 Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Mon, 15 Jan 2018 12:16:05 +0100 Subject: [PATCH 2/3] Batch endpoint path is now configurable --- .../Bundle/DependencyInjection/ApiPlatformExtension.php | 2 +- .../Symfony/Bundle/DependencyInjection/Configuration.php | 2 +- src/Bridge/Symfony/Bundle/Resources/config/api.xml | 2 +- src/Bridge/Symfony/Routing/ApiLoader.php | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index 813f659e268..d4efa0ad55b 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -145,7 +145,7 @@ private function handleConfig(ContainerBuilder $container, array $config, array { $container->setParameter('api_platform.enable_entrypoint', $config['enable_entrypoint']); $container->setParameter('api_platform.enable_docs', $config['enable_docs']); - $container->setParameter('api_platform.enable_batch_endpoint', $config['enable_batch_endpoint']); + $container->setParameter('api_platform.batch_endpoint', $config['batch_endpoint']); $container->setParameter('api_platform.title', $config['title']); $container->setParameter('api_platform.description', $config['description']); $container->setParameter('api_platform.version', $config['version']); diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index fb506a1041c..97be6909863 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -101,7 +101,7 @@ public function getConfigTreeBuilder() ->booleanNode('enable_swagger_ui')->defaultValue(class_exists(TwigBundle::class))->info('Enable Swagger ui.')->end() ->booleanNode('enable_entrypoint')->defaultTrue()->info('Enable the entrypoint')->end() ->booleanNode('enable_docs')->defaultTrue()->info('Enable the docs')->end() - ->booleanNode('enable_batch_endpoint')->defaultFalse()->info('Enable the batch endpoint')->end() + ->scalarNode('batch_endpoint')->defaultValue('')->info('Set the batch endpoint path. If an empty string is provided, the endpoint is disabled.')->end() ->arrayNode('oauth') ->canBeEnabled() diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml index d50e55a6de5..0cd145effa4 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -40,7 +40,7 @@ %api_platform.graphql.enabled% %api_platform.enable_entrypoint% %api_platform.enable_docs% - %api_platform.enable_batch_endpoint% + %api_platform.batch_endpoint% diff --git a/src/Bridge/Symfony/Routing/ApiLoader.php b/src/Bridge/Symfony/Routing/ApiLoader.php index ae351eebc65..e5751dd6034 100644 --- a/src/Bridge/Symfony/Routing/ApiLoader.php +++ b/src/Bridge/Symfony/Routing/ApiLoader.php @@ -53,9 +53,9 @@ final class ApiLoader extends Loader private $graphqlEnabled; private $entrypointEnabled; private $docsEnabled; - private $batchEndpointEnabled; + private $batchEndpoint; - public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, bool $batchEndpointEnabled = false) + public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, string $batchEndpoint = '') { $this->fileLoader = new XmlFileLoader(new FileLocator($kernel->locateResource('@ApiPlatformBundle/Resources/config/routing'))); $this->resourceNameCollectionFactory = $resourceNameCollectionFactory; @@ -132,9 +132,9 @@ public function load($data, $type = null): RouteCollection } } - if ($this->batchEndpointEnabled) { + if ('' !== $this->batchEndpoint) { $routeCollection->add(RouteNameGenerator::ROUTE_NAME_PREFIX . 'batch_endpoint', new Route( - '/batch', // TODO: make configurable + $this->batchEndpoint, [ '_controller' => self::DEFAULT_ACTION_PATTERN.'batch_endpoint', '_format' => null, From 707959eadb7a663b9adbee2615c65b7bbdec169b Mon Sep 17 00:00:00 2001 From: Yanick Witschi Date: Mon, 15 Jan 2018 12:17:07 +0100 Subject: [PATCH 3/3] Ensure content-length header --- src/Action/BatchEndpointAction.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Action/BatchEndpointAction.php b/src/Action/BatchEndpointAction.php index cbde23881bf..29cb17d195f 100644 --- a/src/Action/BatchEndpointAction.php +++ b/src/Action/BatchEndpointAction.php @@ -72,6 +72,9 @@ public function __invoke(Request $request, array $data = null): array $item['headers'] = $request->headers->all(); } + // Make sure Content-Length is always correct + $item['headers']['content-length'] = strlen($item['body']); + $result[] = $this->convertResponse( $this->executeSubRequest( $k,