From d07cd43027e399a09c649303d6e5560a4a827afd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Deruss=C3=A9?= Date: Tue, 12 Jul 2022 11:16:52 +0200 Subject: [PATCH] Improve EntityValueResolver --- .../ArgumentResolver/EntityValueResolver.php | 122 ++++++------------ .../Bridge/Doctrine/Attribute/MapEntity.php | 34 +++-- src/Symfony/Bridge/Doctrine/CHANGELOG.md | 5 + .../EntityValueResolverTest.php | 4 +- 4 files changed, 73 insertions(+), 92 deletions(-) diff --git a/src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php b/src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php index 2c46924b55aab..0f651f8813a48 100644 --- a/src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php +++ b/src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php @@ -31,24 +31,12 @@ */ final class EntityValueResolver implements ArgumentValueResolverInterface { - private array $defaultOptions = [ - 'object_manager' => null, - 'expr' => null, - 'mapping' => [], - 'exclude' => [], - 'strip_null' => false, - 'id' => null, - 'evict_cache' => false, - 'auto_mapping' => true, - 'attribute_only' => false, - ]; - public function __construct( private ManagerRegistry $registry, - private ?ExpressionLanguage $language = null, - array $defaultOptions = [], + private ?ExpressionLanguage $expressionLanguage = null, + private MapEntity $defaults = new MapEntity(), ) { - $this->defaultOptions = array_merge($this->defaultOptions, $defaultOptions); + $defaults->hasAttribute = false; } /** @@ -61,20 +49,20 @@ public function supports(Request $request, ArgumentMetadata $argument): bool } $options = $this->getOptions($argument); - if (null === $options['class']) { + if (!$options->class) { return false; } - if ($options['attribute_only'] && !$options['has_attribute']) { + if (!$this->defaults->autoConvert && !$options->hasAttribute) { return false; } // Doctrine Entity? - if (null === $objectManager = $this->getManager($options['object_manager'], $options['class'])) { + if (!$objectManager = $this->getManager($options->objectManager, $options->class)) { return false; } - return !$objectManager->getMetadataFactory()->isTransient($options['class']); + return !$objectManager->getMetadataFactory()->isTransient($options->class); } /** @@ -83,20 +71,18 @@ public function supports(Request $request, ArgumentMetadata $argument): bool public function resolve(Request $request, ArgumentMetadata $argument): iterable { $options = $this->getOptions($argument); - $name = $argument->getName(); - $class = $options['class']; + $class = $options->class; $errorMessage = null; - if (null !== $options['expr']) { - if (null === $object = $this->findViaExpression($class, $request, $options['expr'], $options)) { - $errorMessage = sprintf('The expression "%s" returned null', $options['expr']); + if (null !== $options->expr) { + if (null === $object = $this->findViaExpression($class, $request, $options->expr, $options)) { + $errorMessage = sprintf('The expression "%s" returned null', $options->expr); } // find by identifier? } elseif (false === $object = $this->find($class, $request, $options, $name)) { // find by criteria - $object = $this->findOneBy($class, $request, $options); - if (false === $object) { + if (false === $object = $this->findOneBy($class, $request, $options)) { if (!$argument->isNullable()) { throw new \LogicException(sprintf('Unable to guess how to get a Doctrine instance from the request information for parameter "%s".', $name)); } @@ -134,9 +120,9 @@ private function getManager(?string $name, string $class): ?ObjectManager } } - private function find(string $class, Request $request, array $options, string $name): false|object|null + private function find(string $class, Request $request, MapEntity $options, string $name): false|object|null { - if ($options['mapping'] || $options['exclude']) { + if ($options->mapping || $options->exclude) { return false; } @@ -145,8 +131,8 @@ private function find(string $class, Request $request, array $options, string $n return false; } - $objectManager = $this->getManager($options['object_manager'], $class); - if ($options['evict_cache'] && $objectManager instanceof EntityManagerInterface) { + $objectManager = $this->getManager($options->objectManager, $class); + if ($options->evictCache && $objectManager instanceof EntityManagerInterface) { $cacheProvider = $objectManager->getCache(); if ($cacheProvider && $cacheProvider->containsEntity($class, $id)) { $cacheProvider->evictEntity($class, $id); @@ -160,11 +146,11 @@ private function find(string $class, Request $request, array $options, string $n } } - private function getIdentifier(Request $request, array $options, string $name): mixed + private function getIdentifier(Request $request, MapEntity $options, string $name): mixed { - if (\is_array($options['id'])) { + if (\is_array($options->id)) { $id = []; - foreach ($options['id'] as $field) { + foreach ($options->id as $field) { // Convert "%s_uuid" to "foobar_uuid" if (str_contains($field, '%s')) { $field = sprintf($field, $name); @@ -176,51 +162,47 @@ private function getIdentifier(Request $request, array $options, string $name): return $id; } - if (null !== $options['id']) { - $name = $options['id']; + if (null !== $options->id) { + $name = $options->id; } if ($request->attributes->has($name)) { return $request->attributes->get($name); } - if (!$options['id'] && $request->attributes->has('id')) { + if (!$options->id && $request->attributes->has('id')) { return $request->attributes->get('id'); } return false; } - private function findOneBy(string $class, Request $request, array $options): false|object|null + private function findOneBy(string $class, Request $request, MapEntity $options): false|object|null { - if (!$options['mapping']) { - if (!$options['auto_mapping']) { - return false; - } - + if (null === $mapping = $options->mapping) { $keys = $request->attributes->keys(); - $options['mapping'] = $keys ? array_combine($keys, $keys) : []; + $mapping = $keys ? array_combine($keys, $keys) : []; } - foreach ($options['exclude'] as $exclude) { - unset($options['mapping'][$exclude]); + foreach ($options->exclude as $exclude) { + unset($mapping[$exclude]); } - if (!$options['mapping']) { + if (!$mapping) { return false; } // if a specific id has been defined in the options and there is no corresponding attribute // return false in order to avoid a fallback to the id which might be of another object - if ($options['id'] && null === $request->attributes->get($options['id'])) { + if (\is_string($options->id) && null === $request->attributes->get($options->id)) { return false; } $criteria = []; - $objectManager = $this->getManager($options['object_manager'], $class); + $objectManager = $this->getManager($options->objectManager, $class); $metadata = $objectManager->getClassMetadata($class); - foreach ($options['mapping'] as $attribute => $field) { + foreach ($mapping as $attribute => $field) { if (!$metadata->hasField($field) && (!$metadata->hasAssociation($field) || !$metadata->isSingleValuedAssociation($field))) { continue; } @@ -228,7 +210,7 @@ private function findOneBy(string $class, Request $request, array $options): fal $criteria[$field] = $request->attributes->get($attribute); } - if ($options['strip_null']) { + if ($options->stripNull) { $criteria = array_filter($criteria, static fn ($value) => null !== $value); } @@ -243,51 +225,27 @@ private function findOneBy(string $class, Request $request, array $options): fal } } - private function findViaExpression(string $class, Request $request, string $expression, array $options): ?object + private function findViaExpression(string $class, Request $request, string $expression, MapEntity $options): ?object { - if (null === $this->language) { + if (!$this->expressionLanguage) { throw new \LogicException(sprintf('You cannot use the "%s" if the ExpressionLanguage component is not available. Try running "composer require symfony/expression-language".', __CLASS__)); } - $repository = $this->getManager($options['object_manager'], $class)->getRepository($class); + $repository = $this->getManager($options->objectManager, $class)->getRepository($class); $variables = array_merge($request->attributes->all(), ['repository' => $repository]); try { - return $this->language->evaluate($expression, $variables); + return $this->expressionLanguage->evaluate($expression, $variables); } catch (NoResultException|ConversionException) { return null; } } - private function getOptions(ArgumentMetadata $argument): array + private function getOptions(ArgumentMetadata $argument): MapEntity { - /** @var ?MapEntity $configuration */ - $configuration = $argument->getAttributes(MapEntity::class, ArgumentMetadata::IS_INSTANCEOF)[0] ?? null; - - $argumentClass = $argument->getType(); - if ($argumentClass && !class_exists($argumentClass)) { - $argumentClass = null; - } - - if (null === $configuration) { - return array_merge($this->defaultOptions, [ - 'class' => $argumentClass, - 'has_attribute' => false, - ]); - } + /** @var MapEntity $options */ + $options = $argument->getAttributes(MapEntity::class, ArgumentMetadata::IS_INSTANCEOF)[0] ?? $this->defaults; - return [ - 'class' => $configuration->class ?? $argumentClass, - 'object_manager' => $configuration->objectManager ?? $this->defaultOptions['object_manager'], - 'expr' => $configuration->expr ?? $this->defaultOptions['expr'], - 'mapping' => $configuration->mapping ?? $this->defaultOptions['mapping'], - 'exclude' => $configuration->exclude ?? $this->defaultOptions['exclude'], - 'strip_null' => $configuration->stripNull ?? $this->defaultOptions['strip_null'], - 'id' => $configuration->id ?? $this->defaultOptions['id'], - 'evict_cache' => $configuration->evictCache ?? $this->defaultOptions['evict_cache'], - 'has_attribute' => true, - 'auto_mapping' => $this->defaultOptions['auto_mapping'], - 'attribute_only' => $this->defaultOptions['attribute_only'], - ]; + return $options->withDefaults($this->defaults, $argument->getType()); } } diff --git a/src/Symfony/Bridge/Doctrine/Attribute/MapEntity.php b/src/Symfony/Bridge/Doctrine/Attribute/MapEntity.php index b8b84848e4f15..ab8bb711213c3 100644 --- a/src/Symfony/Bridge/Doctrine/Attribute/MapEntity.php +++ b/src/Symfony/Bridge/Doctrine/Attribute/MapEntity.php @@ -17,15 +17,33 @@ #[\Attribute(\Attribute::TARGET_PARAMETER)] class MapEntity { + public $hasAttribute = true; + public function __construct( - public readonly ?string $class = null, - public readonly ?string $objectManager = null, - public readonly ?string $expr = null, - public readonly array $mapping = [], - public readonly array $exclude = [], - public readonly bool $stripNull = false, - public readonly array|string|null $id = null, - public readonly bool $evictCache = false, + public ?string $class = null, + public ?string $objectManager = null, + public ?string $expr = null, + public ?array $mapping = null, + public ?array $exclude = null, + public ?bool $stripNull = null, + public array|string|null $id = null, + public ?bool $evictCache = null, + public bool $autoConvert = true, ) { } + + public function withDefaults(self $defaults, ?string $class): static + { + $clone = clone $this; + $clone->class ??= class_exists($class ?? '') ? $class : null; + $clone->objectManager ??= $defaults->objectManager; + $clone->expr ??= $defaults->expr; + $clone->mapping ??= $defaults->mapping; + $clone->exclude ??= $defaults->exclude ?? []; + $clone->stripNull ??= $defaults->stripNull ?? false; + $clone->id ??= $defaults->id; + $clone->evictCache ??= $defaults->evictCache ?? false; + + return $clone; + } } diff --git a/src/Symfony/Bridge/Doctrine/CHANGELOG.md b/src/Symfony/Bridge/Doctrine/CHANGELOG.md index 7c3e2c010b2a5..f33a77ca351db 100644 --- a/src/Symfony/Bridge/Doctrine/CHANGELOG.md +++ b/src/Symfony/Bridge/Doctrine/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +6.2 +--- + + * Add `#[MapEntity]` with its corresponding `EntityArgumentResolver` + 6.0 --- diff --git a/src/Symfony/Bridge/Doctrine/Tests/ArgumentResolver/EntityValueResolverTest.php b/src/Symfony/Bridge/Doctrine/Tests/ArgumentResolver/EntityValueResolverTest.php index 85df29d34c5a4..c8fd399e16a9b 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/ArgumentResolver/EntityValueResolverTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/ArgumentResolver/EntityValueResolverTest.php @@ -85,7 +85,7 @@ public function testSupportWithoutClass() public function testSupportWithoutAttribute() { $registry = $this->getMockBuilder(ManagerRegistry::class)->getMock(); - $converter = new EntityValueResolver($registry, null, ['attribute_only' => true]); + $converter = new EntityValueResolver($registry, null, new MapEntity(autoConvert: false)); $registry->expects($this->once()) ->method('getManagerNames') @@ -353,7 +353,7 @@ public function testApplyWithMappingAndExclude() public function testIgnoreMappingWhenAutoMappingDisabled() { $registry = $this->getMockBuilder(ManagerRegistry::class)->getMock(); - $converter = new EntityValueResolver($registry, null, ['auto_mapping' => false]); + $converter = new EntityValueResolver($registry, null, new MapEntity(mapping: [])); $request = new Request(); $request->attributes->set('foo', 1);