serializer done

soyuka · Aug 10, 2023 · aa33674 · aa33674
1 parent 1ddf2d3
commit aa33674
Show file tree

Hide file tree

Showing 54 changed files with 1,817 additions and 239 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -196,6 +196,7 @@ jobs:
           - HttpCache
           - RamseyUuid
           - GraphQl
+          - Serializer
       fail-fast: false
     steps:
       - name: Checkout

diff --git a/notes b/notes
@@ -0,0 +1,70 @@
+First, this patch allows `stateOptions: entityClass` to work with Doctrine ODM.
+
+Then, it provides a way to hook into our providers to change how links are provided to Doctrine. This really help with improving performances, it is quite complicated let me try to explain.
+
+## The Problem
+
+Say you have `/company/les-tilleuls/employees/soyuka`. API Platform tries to handle your links, and the algorithm tries to cover all the cases so it'd probably do something like this:
+
+```sql
+SELECT * FROM Employee e
+INNER JOIN Company c ON e.company_id = c.id
+WHERE c.id = 'les-tilleuls' and e.id = 'soyuka'
+```
+
+First it's not that simple as we work with doctrine, relations between multiple tables and different nature (toMany, toOne) are sometimes really tricky and API Platform does things like this:
+
+```sql
+SELECT * FROM Employee e
+WHERE e.id IN (
+    SELECT c.employee_id FROM Company c
+    WHERE c.id = 'les-tilleuls'
+)
+```
+
+Depending on the nature of the relation it can be over-complicated and probably also bad in term of query performances.
+
+A solution to this is to say that, depending on business rules, we decide to use:
+
+```
+SELECT * FROM Employee e
+WHERE e.company = 'les-tilleuls' and e.id = 'soyuka'
+```
+
+## DX
+
+Today you'd have to write a custom provider. Thing is, people love our filters and our pagination handling. Despite trying my best to work on that extensibility, for now, the best is to "copy paste API Platform code" (and keep our copyright thanks <3).
+
+[URI Variables](https://github.com/api-platform/core/blob/main/docs/adr/0003-uri-variables.md) came with this huge refactoring and re-visiting data retrieval on subresources. This lead to a quite natural extension point where all our logic resides:
+
+https://github.com/api-platform/core/blob/92a81f024541054b9322e7457b75c721261e14e0/src/Doctrine/Odm/State/ItemProvider.php#L62
+
+https://github.com/api-platform/core/blob/92a81f024541054b9322e7457b75c721261e14e0/src/Doctrine/Orm/State/ItemProvider.php#L67
+
+## Current implementation
+
+Maybe this needs re-visiting, maybe that we need a new interface, but it'd need an ORM-specific signature... We already happen to have `ApiPlatfirm\State\Option` for this?
+
+```
+use ApiPlatform\Doctrine\Orm\State;
+use Doctrine\ORM\QueryBuilder;
+use ApiPlatform\Doctrine\Orm\Util\QueryNameGenerator;
+use ApiPlatform\Metadata\Operation;
+
+#[ApiResource(uriTemplate: '/company/{company}/employees/{employee}' stateOptions: new Options(handleLinks: [Employee::class, 'handleLinks']))]
+#[ORM\Entity]
+final class Employee {
+    public string $id;
+    public string $employee;
+
+    static function handleLinks(QueryBuilder $queryBuilder, array $identifiers, QueryNameGenerator $queryNameGenerator, array $context, string $entityClass, Operation $operation) {
+        $alias = $queryBuilder->getRootAliases()[0];
+        $queryBuilder->andWhere("$alias.id = :id");
+        $queryBuilder->setParameter('id', $identifiers['employee']);
+    }
+}
+```
+
+You really have all you need in that signature, but we could also move some of them in the `$context` (entityClass and operation).
+
+Let me know your thoughts.
diff --git a/src/Api/IdentifiersExtractor.php b/src/Api/IdentifiersExtractor.php
@@ -13,152 +13,10 @@
 
 namespace ApiPlatform\Api;
 
-use ApiPlatform\Exception\RuntimeException;
-use ApiPlatform\Metadata\GraphQl\Operation as GraphQlOperation;
-use ApiPlatform\Metadata\HttpOperation;
-use ApiPlatform\Metadata\Operation;
-use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
-use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
-use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
-use ApiPlatform\Metadata\Util\ResourceClassInfoTrait;
-use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
-use Symfony\Component\PropertyAccess\PropertyAccess;
-use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
+class_exists(\ApiPlatform\Metadata\IdentifiersExtractor::class);
 
-/**
- * {@inheritdoc}
- *
- * @author Antoine Bluchet <[email protected]>
- */
-final class IdentifiersExtractor implements IdentifiersExtractorInterface
-{
-    use ResourceClassInfoTrait;
-    private readonly PropertyAccessorInterface $propertyAccessor;
-
-    public function __construct(ResourceMetadataCollectionFactoryInterface $resourceMetadataFactory, ResourceClassResolverInterface $resourceClassResolver, private readonly PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, private readonly PropertyMetadataFactoryInterface $propertyMetadataFactory, PropertyAccessorInterface $propertyAccessor = null)
+if (false) {
+    final class IdentifiersExtractor extends \ApiPlatform\Metadata\IdentifiersExtractor
     {
-        $this->resourceMetadataFactory = $resourceMetadataFactory;
-        $this->resourceClassResolver = $resourceClassResolver;
-        $this->propertyAccessor = $propertyAccessor ?? PropertyAccess::createPropertyAccessor();
-    }
-
-    /**
-     * {@inheritdoc}
-     *
-     * TODO: 3.0 identifiers should be stringable?
-     */
-    public function getIdentifiersFromItem(object $item, Operation $operation = null, array $context = []): array
-    {
-        if (!$this->isResourceClass($this->getObjectClass($item))) {
-            return ['id' => $this->propertyAccessor->getValue($item, 'id')];
-        }
-
-        if ($operation && $operation->getClass()) {
-            return $this->getIdentifiersFromOperation($item, $operation, $context);
-        }
-
-        $resourceClass = $this->getResourceClass($item, true);
-        $operation ??= $this->resourceMetadataFactory->create($resourceClass)->getOperation(null, false, true);
-
-        return $this->getIdentifiersFromOperation($item, $operation, $context);
-    }
-
-    private function getIdentifiersFromOperation(object $item, Operation $operation, array $context = []): array
-    {
-        if ($operation instanceof HttpOperation) {
-            $links = $operation->getUriVariables();
-        } elseif ($operation instanceof GraphQlOperation) {
-            $links = $operation->getLinks();
-        }
-
-        $identifiers = [];
-        foreach ($links ?? [] as $link) {
-            if (1 < (is_countable($link->getIdentifiers()) ? \count($link->getIdentifiers()) : 0)) {
-                $compositeIdentifiers = [];
-                foreach ($link->getIdentifiers() as $identifier) {
-                    $compositeIdentifiers[$identifier] = $this->getIdentifierValue($item, $link->getFromClass() ?? $operation->getClass(), $identifier, $link->getParameterName());
-                }
-
-                $identifiers[$link->getParameterName()] = CompositeIdentifierParser::stringify($compositeIdentifiers);
-                continue;
-            }
-
-            $parameterName = $link->getParameterName();
-            $identifiers[$parameterName] = $this->getIdentifierValue($item, $link->getFromClass() ?? $operation->getClass(), $link->getIdentifiers()[0], $parameterName, $link->getToProperty());
-        }
-
-        return $identifiers;
-    }
-
-    /**
-     * Gets the value of the given class property.
-     */
-    private function getIdentifierValue(object $item, string $class, string $property, string $parameterName, string $toProperty = null): float|bool|int|string
-    {
-        if ($item instanceof $class) {
-            try {
-                return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, $property), $parameterName);
-            } catch (NoSuchPropertyException $e) {
-                throw new RuntimeException('Not able to retrieve identifiers.', $e->getCode(), $e);
-            }
-        }
-
-        if ($toProperty) {
-            return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, "$toProperty.$property"), $parameterName);
-        }
-
-        $resourceClass = $this->getResourceClass($item, true);
-        foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) {
-            $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName);
-
-            $types = $propertyMetadata->getBuiltinTypes();
-            if (null === ($type = $types[0] ?? null)) {
-                continue;
-            }
-
-            try {
-                if ($type->isCollection()) {
-                    $collectionValueType = $type->getCollectionValueTypes()[0] ?? null;
-
-                    if (null !== $collectionValueType && $collectionValueType->getClassName() === $class) {
-                        return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, sprintf('%s[0].%s', $propertyName, $property)), $parameterName);
-                    }
-                }
-
-                if ($type->getClassName() === $class) {
-                    return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, "$propertyName.$property"), $parameterName);
-                }
-            } catch (NoSuchPropertyException $e) {
-                throw new RuntimeException('Not able to retrieve identifiers.', $e->getCode(), $e);
-            }
-        }
-
-        throw new RuntimeException('Not able to retrieve identifiers.');
-    }
-
-    /**
-     * TODO: in 3.0 this method just uses $identifierValue instanceof \Stringable and we remove the weird behavior.
-     *
-     * @param mixed|\Stringable $identifierValue
-     */
-    private function resolveIdentifierValue(mixed $identifierValue, string $parameterName): float|bool|int|string
-    {
-        if (null === $identifierValue) {
-            throw new RuntimeException('No identifier value found, did you forget to persist the entity?');
-        }
-
-        if (\is_scalar($identifierValue)) {
-            return $identifierValue;
-        }
-
-        if ($identifierValue instanceof \Stringable) {
-            return (string) $identifierValue;
-        }
-
-        if ($identifierValue instanceof \BackedEnum) {
-            return (string) $identifierValue->value;
-        }
-
-        throw new RuntimeException(sprintf('We were not able to resolve the identifier matching parameter "%s".', $parameterName));
     }
 }
diff --git a/src/Api/IdentifiersExtractorInterface.php b/src/Api/IdentifiersExtractorInterface.php
@@ -13,20 +13,10 @@
 
 namespace ApiPlatform\Api;
 
-use ApiPlatform\Exception\RuntimeException;
-use ApiPlatform\Metadata\Operation;
+class_alias(\ApiPlatform\Metadata\IdentifiersExtractorInterface::class, \ApiPlatform\Api\IdentifiersExtractorInterface::class);
 
-/**
- * Extracts identifiers for a given Resource according to the retrieved Metadata.
- *
- * @author Antoine Bluchet <[email protected]>
- */
-interface IdentifiersExtractorInterface
-{
-    /**
-     * Finds identifiers from an Item (object).
-     *
-     * @throws RuntimeException
-     */
-    public function getIdentifiersFromItem(object $item, Operation $operation = null, array $context = []): array;
+if (false) {
+    interface IdentifiersExtractorInterface extends \ApiPlatform\Metadata\IdentifiersExtractorInterface
+    {
+    }
 }
diff --git a/src/Api/IriConverterInterface.php b/src/Api/IriConverterInterface.php
@@ -13,6 +13,10 @@
 
 namespace ApiPlatform\Api;
 
-interface IriConverterInterface extends \ApiPlatform\Metadata\IriConverterInterface
-{
+class_alias(\ApiPlatform\Metadata\IriConverterInterface::class, \ApiPlatform\Api\IriConverterInterface::class);
+
+if (false) {
+    interface IriConverterInterface extends \ApiPlatform\Metadata\IriConverterInterface
+    {
+    }
 }
diff --git a/src/Api/ResourceClassResolverInterface.php b/src/Api/ResourceClassResolverInterface.php
@@ -13,6 +13,8 @@
 
 namespace ApiPlatform\Api;
 
-interface ResourceClassResolverInterface extends \ApiPlatform\Metadata\ResourceClassResolverInterface
-{
+class_alias(\ApiPlatform\Metadata\ResourceClassResolverInterface::class, \ApiPlatform\Api\ResourceClassResolverInterface::class);
+
+if (false) {
+    interface ResourceClassResolverInterface extends \ApiPlatform\Metadata\ResourceClassResolverInterface {}
 }
diff --git a/src/GraphQl/Serializer/ItemNormalizer.php b/src/GraphQl/Serializer/ItemNormalizer.php
@@ -13,7 +13,7 @@
 
 namespace ApiPlatform\GraphQl\Serializer;
 
-use ApiPlatform\Api\IdentifiersExtractorInterface;
+use ApiPlatform\Metadata\IdentifiersExtractorInterface;
 use ApiPlatform\Metadata\ApiProperty;
 use ApiPlatform\Metadata\IriConverterInterface;
 use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;

diff --git a/src/GraphQl/Tests/Fixtures/Type/Definition/DateTimeType.php b/src/GraphQl/Tests/Fixtures/Type/Definition/DateTimeType.php
@@ -78,7 +78,7 @@ public function parseValue($value): string
     /**
      * {@inheritdoc}
      */
-    public function parseLiteral(Node $valueNode, ?array $variables = null): string
+    public function parseLiteral(Node $valueNode, array $variables = null): string
     {
         if ($valueNode instanceof StringValueNode && false !== \DateTime::createFromFormat(\DateTime::ATOM, $valueNode->value)) {
             return $valueNode->value;

diff --git a/src/GraphQl/composer.json b/src/GraphQl/composer.json
@@ -25,18 +25,14 @@
         "api-platform/state": "*@dev || ^3.1",
         "symfony/property-info": "^6.1",
         "symfony/serializer": "^6.1",
-        "symfony/http-foundation": "^6.1",
-        "symfony/http-kernel": "^6.1"
+        "webonyx/graphql-php": "^14.0 || ^15.0"
     },
     "require-dev": {
-        "doctrine/common": "^3.2.2",
         "phpspec/prophecy-phpunit": "^2.0",
         "symfony/phpunit-bridge": "^6.1",
         "symfony/routing": "^6.1",
         "symfony/validator": "^6.1",
-        "symfony/twig-bundle": "^6.1",
-        "symfony/mercure-bundle": "*",
-        "webonyx/graphql-php": "^14.0 || ^15.0"
+        "symfony/mercure-bundle": "*"
     },
     "autoload": {
         "psr-4": {

diff --git a/src/Hydra/Serializer/CollectionFiltersNormalizer.php b/src/Hydra/Serializer/CollectionFiltersNormalizer.php
@@ -17,8 +17,8 @@
 use ApiPlatform\Doctrine\Orm\State\Options;
 use ApiPlatform\Metadata\FilterInterface;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
-use ApiPlatform\Serializer\CacheableSupportsMethodInterface;
 use ApiPlatform\Metadata\ResourceClassResolverInterface;
+use ApiPlatform\Serializer\CacheableSupportsMethodInterface;
 use Psr\Container\ContainerInterface;
 use Symfony\Component\Serializer\Exception\UnexpectedValueException;
 use Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer;

diff --git a/src/Metadata/ApiFilter.php b/src/Metadata/ApiFilter.php
@@ -24,18 +24,18 @@
 #[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_CLASS | \Attribute::IS_REPEATABLE)]
 final class ApiFilter
 {
-	/**
-	 * @param string|class-string<FilterInterface>|class-string<LegacyFilterInterface> $filterClass
-	 */
-	public function __construct(
-		public string $filterClass,
-		public ?string $id = null,
-		public ?string $strategy = null,
-		public array $properties = [],
-		public array $arguments = [],
-	) {
-		if (!is_a($this->filterClass, FilterInterface::class, true) && !is_a($this->filterClass, LegacyFilterInterface::class, true)) {
-			throw new InvalidArgumentException(sprintf('The filter class "%s" does not implement "%s". Did you forget a use statement?', $this->filterClass, FilterInterface::class));
-		}
-	}
+    /**
+     * @param string|class-string<FilterInterface>|class-string<LegacyFilterInterface> $filterClass
+     */
+    public function __construct(
+        public string $filterClass,
+        public ?string $id = null,
+        public ?string $strategy = null,
+        public array $properties = [],
+        public array $arguments = [],
+    ) {
+        if (!is_a($this->filterClass, FilterInterface::class, true) && !is_a($this->filterClass, LegacyFilterInterface::class, true)) {
+            throw new InvalidArgumentException(sprintf('The filter class "%s" does not implement "%s". Did you forget a use statement?', $this->filterClass, FilterInterface::class));
+        }
+    }
 }