Refactor path resolver, allow subresources in swagger

1. Implement SubresourceOperationFactory
2. Deprecates UnderscoreOperationPathResolver/DashOperationPathResolver in
favor of PathSegmentNameGenerator

Author: soyuka

src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php

@@ -43,8 +43,15 @@ public function getConfigTreeBuilder()
                 ->scalarNode('title')->defaultValue('')->info('The title of the API.')->end()
                 ->scalarNode('description')->defaultValue('')->info('The description of the API.')->end()
                 ->scalarNode('version')->defaultValue('0.0.0')->info('The version of the API.')->end()
-                ->scalarNode('default_operation_path_resolver')->defaultValue('api_platform.operation_path_resolver.underscore')->info('Specify the default operation path resolver to use for generating resources operations path.')->end()
+                ->scalarNode('default_operation_path_resolver')
+                    ->beforeNormalization()->always(function ($v) {
+                        if (isset($v['default_operation_path_resolver'])) {
+                            @trigger_error('The use of the `default_operation_path_resolver` has been deprecated in 2.1 and will be removed in 3.0. Use `path_segment_name_generator` instead.', E_USER_DEPRECATED);
+                        }
+                    })->end()
+                ->defaultValue('api_platform.operation_path_resolver.underscore')->info('Specify the default operation path resolver to use for generating resources operations path.')->end()
                 ->scalarNode('name_converter')->defaultNull()->info('Specify a name converter to use.')->end()
+                ->scalarNode('path_segment_name_generator')->defaultValue('api_platform.path_segment_name_generator.underscore')->info('Specify a path name generator to use.')->end()
                 ->scalarNode('api_resources_directory')->defaultValue('Entity')->info('The name of the directory within the bundles that contains the api resources.')->end()
                 ->arrayNode('eager_loading')
                     ->canBeDisabled()

src/Bridge/Symfony/Bundle/Resources/config/api.xml

@@ -36,8 +36,7 @@
             <argument type="service" id="service_container" />
             <argument>%api_platform.formats%</argument>
             <argument>%api_platform.resource_class_directories%</argument>
-            <argument type="service" id="api_platform.metadata.property.name_collection_factory" />
-            <argument type="service" id="api_platform.metadata.property.metadata_factory" />
+            <argument type="service" id="api_platform.subresource_operation_factory" />
 
             <tag name="routing.loader" />
         </service>
@@ -91,14 +90,31 @@
         <service id="api_platform.operation_path_resolver.router" class="ApiPlatform\Core\Bridge\Symfony\Routing\RouterOperationPathResolver" public="false">
             <argument type="service" id="api_platform.router" />
             <argument type="service" id="api_platform.operation_path_resolver.custom" />
+            <argument type="service" id="api_platform.subresource_operation_factory" />
         </service>
 
         <service id="api_platform.operation_path_resolver.custom" class="ApiPlatform\Core\PathResolver\CustomOperationPathResolver" public="false">
+            <!-- should be "api_platform.operation_path_resolver.generator" when the default is removed -->
             <argument type="service" id="api_platform.operation_path_resolver.default" />
         </service>
 
-        <service id="api_platform.operation_path_resolver.underscore" class="ApiPlatform\Core\PathResolver\UnderscoreOperationPathResolver" public="false" />
-        <service id="api_platform.operation_path_resolver.dash" class="ApiPlatform\Core\PathResolver\DashOperationPathResolver" public="false" />
+        <service id="api_platform.operation_path_resolver.generator" class="ApiPlatform\Core\PathResolver\OperationPathResolver" public="false">
+            <argument type="service" id="api_platform.path_segment_name_generator" />
+        </service>
+
+        <service id="api_platform.operation_path_resolver.underscore" class="ApiPlatform\Core\PathResolver\UnderscoreOperationPathResolver" public="false">
+                <deprecated>The "%service_id%" service is deprecated since ApiPlatform 2.1 and will be removed in 3.0. Use PathSegmentNameGenerator instead.</deprecated>
+        </service>
+
+        <service id="api_platform.operation_path_resolver.dash" class="ApiPlatform\Core\PathResolver\DashOperationPathResolver" public="false">
+                <deprecated>The "%service_id%" service is deprecated since ApiPlatform 2.1 and will be removed in 3.0. Use PathSegmentNameGenerator instead.</deprecated>
+        </service>
+
+        <!-- Path name generator -->
+
+        <service id="api_platform.path_segment_name_generator" alias="api_platform.path_segment_name_generator.underscore" public="false" />
+        <service id="api_platform.path_segment_name_generator.underscore" class="ApiPlatform\Core\Operation\UnderscorePathSegmentNameGenerator" public="false" />
+        <service id="api_platform.path_segment_name_generator.dash" class="ApiPlatform\Core\Operation\DashPathSegmentNameGenerator" public="false" />
 
         <!-- Event listeners -->
 
@@ -194,6 +210,20 @@
             <argument type="service" id="api_platform.property_accessor" />
         </service>
 
+        <!-- Subresources -->
+
+        <service id="api_platform.subresource_operation_factory" class="ApiPlatform\Core\Operation\Factory\SubresourceOperationFactory" public="false">
+            <argument type="service" id="api_platform.metadata.resource.metadata_factory" />
+            <argument type="service" id="api_platform.metadata.property.name_collection_factory" />
+            <argument type="service" id="api_platform.metadata.property.metadata_factory" />
+            <argument type="service" id="api_platform.path_segment_name_generator" />
+        </service>
+
+        <service id="api_platform.subresource_operation_factory.cached" class="ApiPlatform\Core\Operation\Factory\CachedSubresourceOperationFactory" decorates="api_platform.subresource_operation_factory" decoration-priority="-10" public="false">
+            <argument type="service" id="api_platform.cache.subresource_operation_factory" />
+            <argument type="service" id="api_platform.subresource_operation_factory.cached.inner" />
+        </service>
+
         <!-- Cache -->
 
         <service id="api_platform.cache.route_name_resolver" parent="cache.system" public="false">
@@ -203,6 +233,10 @@
         <service id="api_platform.cache.identifiers_extractor" parent="cache.system" public="false">
             <tag name="cache.pool" />
         </service>
+
+        <service id="api_platform.cache.subresource_operation_factory" parent="cache.system" public="false">
+            <tag name="cache.pool" />
+        </service>
     </services>
 
 </container>

src/Bridge/Symfony/Bundle/Resources/config/swagger.xml

@@ -22,6 +22,7 @@
             <argument>%api_platform.oauth.tokenUrl%</argument>
             <argument>%api_platform.oauth.authorizationUrl%</argument>
             <argument>%api_platform.oauth.scopes%</argument>
+            <argument type="service" id="api_platform.subresource_operation_factory"></argument>
             <tag name="serializer.normalizer" priority="16" />
         </service>
 

src/Bridge/Symfony/Routing/ApiLoader.php

@@ -16,10 +16,9 @@
 use ApiPlatform\Core\Api\OperationType;
 use ApiPlatform\Core\Exception\InvalidResourceException;
 use ApiPlatform\Core\Exception\RuntimeException;
-use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
-use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceNameCollectionFactoryInterface;
+use ApiPlatform\Core\Operation\Factory\SubresourceOperationFactoryInterface;
 use ApiPlatform\Core\PathResolver\OperationPathResolverInterface;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\Config\Loader\Loader;
@@ -42,7 +41,6 @@ final class ApiLoader extends Loader
      */
     const ROUTE_NAME_PREFIX = 'api_';
     const DEFAULT_ACTION_PATTERN = 'api_platform.action.';
-    const SUBRESOURCE_SUFFIX = '_get_subresource';
 
     private $fileLoader;
     private $propertyNameCollectionFactory;
@@ -53,8 +51,9 @@ final class ApiLoader extends Loader
     private $container;
     private $formats;
     private $resourceClassDirectories;
+    private $subresourceOperationFactory;
 
-    public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory = null, PropertyMetadataFactoryInterface $propertyMetadataFactory = null)
+    public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null)
     {
         $this->fileLoader = new XmlFileLoader(new FileLocator($kernel->locateResource('@ApiPlatformBundle/Resources/config/routing')));
         $this->resourceNameCollectionFactory = $resourceNameCollectionFactory;
@@ -63,8 +62,7 @@ public function __construct(KernelInterface $kernel, ResourceNameCollectionFacto
         $this->container = $container;
         $this->formats = $formats;
         $this->resourceClassDirectories = $resourceClassDirectories;
-        $this->propertyNameCollectionFactory = $propertyNameCollectionFactory;
-        $this->propertyMetadataFactory = $propertyMetadataFactory;
+        $this->subresourceOperationFactory = $subresourceOperationFactory;
     }
 
     /**
@@ -99,90 +97,34 @@ public function load($data, $type = null): RouteCollection
                 }
             }
 
-            $this->computeSubresourceOperations($routeCollection, $resourceClass);
-        }
-
-        return $routeCollection;
-    }
-
-    /**
-     * Handles subresource operations recursively and declare their corresponding routes.
-     *
-     * @param RouteCollection $routeCollection
-     * @param string          $resourceClass
-     * @param string          $rootResourceClass null on the first iteration, it then keeps track of the origin resource class
-     * @param array           $parentOperation   the previous call operation
-     */
-    private function computeSubresourceOperations(RouteCollection $routeCollection, string $resourceClass, string $rootResourceClass = null, array $parentOperation = null, array $visited = [])
-    {
-        if (null === $this->propertyNameCollectionFactory || null === $this->propertyMetadataFactory) {
-            return;
-        }
-
-        if (null === $rootResourceClass) {
-            $rootResourceClass = $resourceClass;
-        }
-
-        foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $property) {
-            $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $property);
-
-            if (!$propertyMetadata->hasSubresource()) {
-                continue;
-            }
-
-            $subresource = $propertyMetadata->getSubresource();
-
-            $operation = [
-                'property' => $property,
-                'collection' => $subresource->isCollection(),
-            ];
-
-            $visiting = "$rootResourceClass $resourceClass $property {$subresource->isCollection()} {$subresource->getResourceClass()}";
-
-            if (in_array($visiting, $visited, true)) {
+            if (null === $this->subresourceOperationFactory) {
                 continue;
             }
 
-            $visited[] = $visiting;
-
-            if (null === $parentOperation) {
-                $rootResourceMetadata = $this->resourceMetadataFactory->create($rootResourceClass);
-                $rootShortname = $rootResourceMetadata->getShortName();
-
-                $operation['identifiers'] = [['id', $rootResourceClass]];
-                $operation['route_name'] = RouteNameGenerator::generate('get', $rootShortname, OperationType::SUBRESOURCE, $operation);
-                $operation['path'] = $this->operationPathResolver->resolveOperationPath($rootShortname, $operation, OperationType::SUBRESOURCE, $operation['route_name']);
-            } else {
-                $operation['identifiers'] = $parentOperation['identifiers'];
-                $operation['identifiers'][] = [$parentOperation['property'], $resourceClass];
-                $operation['route_name'] = str_replace('get'.RouteNameGenerator::SUBRESOURCE_SUFFIX, RouteNameGenerator::routeNameResolver($property, $operation['collection']).'_get'.RouteNameGenerator::SUBRESOURCE_SUFFIX, $parentOperation['route_name']);
-                $operation['path'] = $this->operationPathResolver->resolveOperationPath($parentOperation['path'], $operation, OperationType::SUBRESOURCE, $operation['route_name']);
-            }
-
-            $route = new Route(
-                $operation['path'],
-                [
-                    '_controller' => self::DEFAULT_ACTION_PATTERN.'get_subresource',
-                    '_format' => null,
-                    '_api_resource_class' => $subresource->getResourceClass(),
-                    '_api_subresource_operation_name' => $operation['route_name'],
-                    '_api_subresource_context' => [
-                        'property' => $operation['property'],
-                        'identifiers' => $operation['identifiers'],
-                        'collection' => $subresource->isCollection(),
+            foreach ($this->subresourceOperationFactory->create($resourceClass) as $operation) {
+                $routeCollection->add($operation['route_name'], new Route(
+                    $operation['path'],
+                    [
+                        '_controller' => self::DEFAULT_ACTION_PATTERN.'get_subresource',
+                        '_format' => null,
+                        '_api_resource_class' => $operation['resource_class'],
+                        '_api_subresource_operation_name' => $operation['route_name'],
+                        '_api_subresource_context' => [
+                            'property' => $operation['property'],
+                            'identifiers' => $operation['identifiers'],
+                            'collection' => $operation['collection'],
+                        ],
                     ],
-                ],
-                [],
-                [],
-                '',
-                [],
-                ['GET']
-            );
-
-            $routeCollection->add($operation['route_name'], $route);
-
-            $this->computeSubresourceOperations($routeCollection, $subresource->getResourceClass(), $rootResourceClass, $operation, $visited);
+                    [],
+                    [],
+                    '',
+                    [],
+                    ['GET']
+                ));
+            }
         }
+
+        return $routeCollection;
     }
 
     /**

src/Bridge/Symfony/Routing/RouteNameGenerator.php

@@ -25,10 +25,9 @@
  *
  * @author Baptiste Meyer <[email protected]>
  */
-class RouteNameGenerator
+final class RouteNameGenerator
 {
     const ROUTE_NAME_PREFIX = 'api_';
-    const SUBRESOURCE_SUFFIX = '_subresource';
 
     private function __construct()
     {
@@ -40,33 +39,21 @@ private function __construct()
      * @param string      $operationName
      * @param string      $resourceShortName
      * @param string|bool $operationType
-     * @param array       $subresourceContext
      *
      * @throws InvalidArgumentException
      *
      * @return string
      */
-    public static function generate(string $operationName, string $resourceShortName, $operationType, array $subresourceContext = []): string
+    public static function generate(string $operationName, string $resourceShortName, $operationType): string
     {
         if (OperationType::SUBRESOURCE === $operationType = OperationTypeDeprecationHelper::getOperationType($operationType)) {
-            if (!isset($subresourceContext['property'])) {
-                throw new InvalidArgumentException('Missing "property" to generate a route name from a subresource');
-            }
-
-            return sprintf(
-              '%s%s_%s_%s%s',
-              static::ROUTE_NAME_PREFIX,
-              self::routeNameResolver($resourceShortName),
-              self::routeNameResolver($subresourceContext['property'], $subresourceContext['collection'] ?? false),
-              $operationName,
-              self::SUBRESOURCE_SUFFIX
-            );
+            throw new InvalidArgumentException('Subresource operations are not supported by the RouteNameGenerator.');
         }
 
         return sprintf(
             '%s%s_%s_%s',
             static::ROUTE_NAME_PREFIX,
-            self::routeNameResolver($resourceShortName),
+            self::inflector($resourceShortName),
             $operationName,
             $operationType
         );
@@ -79,7 +66,7 @@ public static function generate(string $operationName, string $resourceShortName
      *
      * @return string A string that is a part of the route name
      */
-    public static function routeNameResolver(string $name, bool $pluralize = true): string
+    public static function inflector(string $name, bool $pluralize = true): string
     {
         $name = Inflector::tableize($name);
 

src/Bridge/Symfony/Routing/RouterOperationPathResolver.php

@@ -13,6 +13,7 @@
 
 namespace ApiPlatform\Core\Bridge\Symfony\Routing;
 
+use ApiPlatform\Core\Api\OperationType;
 use ApiPlatform\Core\Api\OperationTypeDeprecationHelper;
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\PathResolver\OperationPathResolverInterface;
@@ -51,10 +52,14 @@ public function resolveOperationPath(string $resourceShortName, array $operation
 
         if (isset($operation['route_name'])) {
             $routeName = $operation['route_name'];
-        } elseif (null !== $operationName) {
-            $routeName = RouteNameGenerator::generate($operationName, $resourceShortName, $operationType, $operation);
+        } elseif (OperationType::SUBRESOURCE === $operationType) {
+            throw new InvalidArgumentException('Subresource operations are not supported by the RouterOperationPathResolver.');
         } else {
-            return $this->deferred->resolveOperationPath($resourceShortName, $operation, OperationTypeDeprecationHelper::getOperationType($operationType), $operationName);
+            if (null !== $operationName) {
+                $routeName = RouteNameGenerator::generate($operationName, $resourceShortName, $operationType);
+            } else {
+                return $this->deferred->resolveOperationPath($resourceShortName, $operation, OperationTypeDeprecationHelper::getOperationType($operationType), $operationName);
+            }
         }
 
         if (!$route = $this->router->getRouteCollection()->get($routeName)) {

src/Operation/DashPathSegmentNameGenerator.php

@@ -0,0 +1,39 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <[email protected]>
+ *
+ * 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\Operation;
+
+use Doctrine\Common\Util\Inflector;
+
+/**
+ * Generate a path name with an underscore separator according to a string and whether it needs pluralization.
+ *
+ * @author Antoine Bluchet <[email protected]>
+ */
+final class DashPathSegmentNameGenerator implements PathSegmentNameGeneratorInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function getSegmentName(string $name, bool $pluralize = true): string
+    {
+        $name = $this->dashize($name);
+
+        return $pluralize ? Inflector::pluralize($name) : $name;
+    }
+
+    private function dashize(string $string): string
+    {
+        return strtolower(preg_replace('~(?<=\\w)([A-Z])~', '-$1', $string));
+    }
+}