Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve EntityValueResolver #3

Merged
merged 1 commit into from
Jul 12, 2022
Merged

Improve EntityValueResolver #3

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 38 additions & 85 deletions src/Symfony/Bridge/Doctrine/ArgumentResolver/EntityValueResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,24 +31,11 @@
*/
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(),
jderusse marked this conversation as resolved.
Show resolved Hide resolved
) {
$this->defaultOptions = array_merge($this->defaultOptions, $defaultOptions);
}

/**
Expand All @@ -61,20 +48,16 @@ public function supports(Request $request, ArgumentMetadata $argument): bool
}

$options = $this->getOptions($argument);
if (null === $options['class']) {
return false;
}

if ($options['attribute_only'] && !$options['has_attribute']) {
if (!$options->class || $options->disabled) {
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);
}

/**
Expand All @@ -83,20 +66,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));
}
Expand Down Expand Up @@ -134,9 +115,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;
}

Expand All @@ -145,8 +126,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);
Expand All @@ -160,11 +141,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);
Expand All @@ -176,59 +157,55 @@ 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']) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for option auto_mapping: making mapping nullable does the job.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch

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)) {
jderusse marked this conversation as resolved.
Show resolved Hide resolved
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;
}

$criteria[$field] = $request->attributes->get($attribute);
}

if ($options['strip_null']) {
if ($options->stripNull) {
$criteria = array_filter($criteria, static fn ($value) => null !== $value);
}

Expand All @@ -243,51 +220,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());
}
}
32 changes: 24 additions & 8 deletions src/Symfony/Bridge/Doctrine/Attribute/MapEntity.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,30 @@
class MapEntity
{
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 $disabled = false,
) {
}

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;
jderusse marked this conversation as resolved.
Show resolved Hide resolved

return $clone;
}
}
5 changes: 5 additions & 0 deletions src/Symfony/Bridge/Doctrine/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
CHANGELOG
=========

6.2
---

* Add `#[MapEntity]` with its corresponding `EntityArgumentResolver`

6.0
---

Expand Down
4 changes: 2 additions & 2 deletions src/Symfony/Bridge/Doctrine/Tests/ArgumentResolver/EntityValueResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(disabled: true));

$registry->expects($this->once())
->method('getManagerNames')
Expand Down Expand Up @@ -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);
Expand Down