Skip to content

Commit 8dd7d44

Browse files
nicolas-grekasnicolas-grekas
committed
[DoctrineBridge] Deprecate auto-mapping of entities in favor of mapped route parameters
1 parent fb23eef commit 8dd7d44

File tree

4 files changed

+107
-43
lines changed

4 files changed

+107
-43
lines changed

‎ArgumentResolver/EntityValueResolver.php‎

Lines changed: 41 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,9 @@ public function resolve(Request $request, ArgumentMetadata $argument): array
6060
$message = sprintf(' The expression "%s" returned null.', $options->expr);
6161
}
6262
// find by identifier?
63-
} elseif (false === $object = $this->find($manager, $request, $options, $argument->getName())) {
63+
} elseif (false === $object = $this->find($manager, $request, $options, $argument)) {
6464
// find by criteria
65-
if (!$criteria = $this->getCriteria($request, $options, $manager)) {
65+
if (!$criteria = $this->getCriteria($request, $options, $manager, $argument)) {
6666
return [];
6767
}
6868
try {
@@ -94,13 +94,13 @@ private function getManager(?string $name, string $class): ?ObjectManager
9494
return $manager->getMetadataFactory()->isTransient($class) ? null : $manager;
9595
}
9696

97-
private function find(ObjectManager $manager, Request $request, MapEntity $options, string $name): false|object|null
97+
private function find(ObjectManager $manager, Request $request, MapEntity $options, ArgumentMetadata $argument): false|object|null
9898
{
9999
if ($options->mapping || $options->exclude) {
100100
return false;
101101
}
102102

103-
$id = $this->getIdentifier($request, $options, $name);
103+
$id = $this->getIdentifier($request, $options, $argument);
104104
if (false === $id || null === $id) {
105105
return $id;
106106
}
@@ -119,14 +119,14 @@ private function find(ObjectManager $manager, Request $request, MapEntity $optio
119119
}
120120
}
121121

122-
private function getIdentifier(Request $request, MapEntity $options, string $name): mixed
122+
private function getIdentifier(Request $request, MapEntity $options, ArgumentMetadata $argument): mixed
123123
{
124124
if (\is_array($options->id)) {
125125
$id = [];
126126
foreach ($options->id as $field) {
127127
// Convert "%s_uuid" to "foobar_uuid"
128128
if (str_contains($field, '%s')) {
129-
$field = sprintf($field, $name);
129+
$field = sprintf($field, $argument->getName());
130130
}
131131

132132
$id[$field] = $request->attributes->get($field);
@@ -135,28 +135,54 @@ private function getIdentifier(Request $request, MapEntity $options, string $nam
135135
return $id;
136136
}
137137

138-
if (null !== $options->id) {
139-
$name = $options->id;
138+
if ($options->id) {
139+
return $request->attributes->get($options->id) ?? ($options->stripNull ? false : null);
140140
}
141141

142+
$name = $argument->getName();
143+
142144
if ($request->attributes->has($name)) {
143-
return $request->attributes->get($name) ?? ($options->stripNull ? false : null);
145+
if (\is_array($id = $request->attributes->get($name))) {
146+
return false;
147+
}
148+
149+
foreach ($request->attributes->get('_route_mapping') ?? [] as $parameter => $attribute) {
150+
if ($name === $attribute) {
151+
$options->mapping = [$name => $parameter];
152+
153+
return false;
154+
}
155+
}
156+
157+
return $id ?? ($options->stripNull ? false : null);
144158
}
159+
if ($request->attributes->has('id')) {
160+
trigger_deprecation('symfony/doctrine-bridge', '7.2', 'Relying on auto-mapping for Doctrine entities is deprecated for argument $%s of "%s": declare the mapping using either the #[MapEntity] attribute or mapped route parameters.', $argument->getName(), $argument->getControllerName());
145161

146-
if (!$options->id && $request->attributes->has('id')) {
147162
return $request->attributes->get('id') ?? ($options->stripNull ? false : null);
148163
}
149164

150165
return false;
151166
}
152167

153-
private function getCriteria(Request $request, MapEntity $options, ObjectManager $manager): array
168+
private function getCriteria(Request $request, MapEntity $options, ObjectManager $manager, ArgumentMetadata $argument): array
154169
{
155-
if (null === $mapping = $options->mapping) {
170+
if (!($mapping = $options->mapping) && \is_array($criteria = $request->attributes->get($argument->getName()))) {
171+
foreach ($options->exclude as $exclude) {
172+
unset($criteria[$exclude]);
173+
}
174+
175+
if ($options->stripNull) {
176+
$criteria = array_filter($criteria, static fn ($value) => null !== $value);
177+
}
178+
179+
return $criteria;
180+
} elseif (null === $mapping) {
181+
trigger_deprecation('symfony/doctrine-bridge', '7.2', 'Relying on auto-mapping for Doctrine entities is deprecated for argument $%s of "%s": declare the identifier using either the #[MapEntity] attribute or mapped route parameters.', $argument->getName(), $argument->getControllerName());
156182
$mapping = $request->attributes->keys();
157183
}
158184

159-
if ($mapping && \is_array($mapping) && array_is_list($mapping)) {
185+
if ($mapping && array_is_list($mapping)) {
160186
$mapping = array_combine($mapping, $mapping);
161187
}
162188

@@ -168,17 +194,11 @@ private function getCriteria(Request $request, MapEntity $options, ObjectManager
168194
return [];
169195
}
170196

171-
// if a specific id has been defined in the options and there is no corresponding attribute
172-
// return false in order to avoid a fallback to the id which might be of another object
173-
if (\is_string($options->id) && null === $request->attributes->get($options->id)) {
174-
return [];
175-
}
176-
177197
$criteria = [];
178-
$metadata = $manager->getClassMetadata($options->class);
198+
$metadata = null === $options->mapping ? $manager->getClassMetadata($options->class) : false;
179199

180200
foreach ($mapping as $attribute => $field) {
181-
if (!$metadata->hasField($field) && (!$metadata->hasAssociation($field) || !$metadata->isSingleValuedAssociation($field))) {
201+
if ($metadata && !$metadata->hasField($field) && (!$metadata->hasAssociation($field) || !$metadata->isSingleValuedAssociation($field))) {
182202
continue;
183203
}
184204

‎Attribute/MapEntity.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ public function __construct(
4747
public ?string $message = null,
4848
) {
4949
parent::__construct($resolver, $disabled);
50+
$this->selfValidate();
5051
}
5152

5253
public function withDefaults(self $defaults, ?string $class): static
@@ -62,6 +63,22 @@ public function withDefaults(self $defaults, ?string $class): static
6263
$clone->evictCache ??= $defaults->evictCache ?? false;
6364
$clone->message ??= $defaults->message;
6465

66+
$clone->selfValidate();
67+
6568
return $clone;
6669
}
70+
71+
private function selfValidate(): void
72+
{
73+
if (!$this->id) {
74+
return;
75+
}
76+
if ($this->mapping) {
77+
throw new \LogicException('The "id" and "mapping" options cannot be used together on #[MapEntity] attributes.');
78+
}
79+
if ($this->exclude) {
80+
throw new \LogicException('The "id" and "exclude" options cannot be used together on #[MapEntity] attributes.');
81+
}
82+
$this->mapping = [];
83+
}
6784
}

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ CHANGELOG
88
* Allow `EntityValueResolver` to return a list of entities
99
* Add support for auto-closing idle connections
1010
* Allow validating every class against `UniqueEntity` constraint
11+
* Deprecate auto-mapping of entities in favor of mapped route parameters
1112

1213
7.0
1314
---

‎Tests/ArgumentResolver/EntityValueResolverTest.php‎

Lines changed: 48 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@ public function testResolveWithoutManager()
6363
$this->assertSame([], $resolver->resolve($request, $argument));
6464
}
6565

66+
/**
67+
* @group legacy
68+
*/
6669
public function testResolveWithNoIdAndDataOptional()
6770
{
6871
$manager = $this->getMockBuilder(ObjectManager::class)->getMock();
@@ -83,18 +86,10 @@ public function testResolveWithStripNulls()
8386

8487
$request = new Request();
8588
$request->attributes->set('arg', null);
86-
$argument = $this->createArgument('stdClass', new MapEntity(stripNull: true), 'arg', true);
89+
$argument = $this->createArgument('stdClass', new MapEntity(mapping: ['arg'], stripNull: true), 'arg', true);
8790

88-
$metadata = $this->getMockBuilder(ClassMetadata::class)->getMock();
89-
$metadata->expects($this->once())
90-
->method('hasField')
91-
->with('arg')
92-
->willReturn(true);
93-
94-
$manager->expects($this->once())
95-
->method('getClassMetadata')
96-
->with('stdClass')
97-
->willReturn($metadata);
91+
$manager->expects($this->never())
92+
->method('getClassMetadata');
9893

9994
$manager->expects($this->never())
10095
->method('getRepository');
@@ -139,7 +134,7 @@ public function testResolveWithNullId()
139134
$request = new Request();
140135
$request->attributes->set('id', null);
141136

142-
$argument = $this->createArgument(isNullable: true);
137+
$argument = $this->createArgument(isNullable: true, entity: new MapEntity(id: 'id'));
143138

144139
$this->assertSame([null], $resolver->resolve($request, $argument));
145140
}
@@ -195,6 +190,9 @@ public static function idsProvider(): iterable
195190
yield ['foo'];
196191
}
197192

193+
/**
194+
* @group legacy
195+
*/
198196
public function testResolveGuessOptional()
199197
{
200198
$manager = $this->getMockBuilder(ObjectManager::class)->getMock();
@@ -232,16 +230,8 @@ public function testResolveWithMappingAndExclude()
232230
new MapEntity(mapping: ['foo' => 'Foo'], exclude: ['bar'])
233231
);
234232

235-
$metadata = $this->getMockBuilder(ClassMetadata::class)->getMock();
236-
$metadata->expects($this->once())
237-
->method('hasField')
238-
->with('Foo')
239-
->willReturn(true);
240-
241-
$manager->expects($this->once())
242-
->method('getClassMetadata')
243-
->with('stdClass')
244-
->willReturn($metadata);
233+
$manager->expects($this->never())
234+
->method('getClassMetadata');
245235

246236
$repository = $this->getMockBuilder(ObjectRepository::class)->getMock();
247237
$repository->expects($this->once())
@@ -257,6 +247,42 @@ public function testResolveWithMappingAndExclude()
257247
$this->assertSame([$object], $resolver->resolve($request, $argument));
258248
}
259249

250+
public function testResolveWithRouteMapping()
251+
{
252+
$manager = $this->getMockBuilder(ObjectManager::class)->getMock();
253+
$registry = $this->createRegistry($manager);
254+
$resolver = new EntityValueResolver($registry);
255+
256+
$request = new Request();
257+
$request->attributes->set('conference', 'vienna-2024');
258+
$request->attributes->set('article', ['title' => 'foo']);
259+
$request->attributes->set('_route_mapping', ['slug' => 'conference']);
260+
261+
$argument1 = $this->createArgument('Conference', new MapEntity('Conference'), 'conference');
262+
$argument2 = $this->createArgument('Article', new MapEntity('Article'), 'article');
263+
264+
$manager->expects($this->never())
265+
->method('getClassMetadata');
266+
267+
$conference = new \stdClass();
268+
$article = new \stdClass();
269+
270+
$repository = $this->getMockBuilder(ObjectRepository::class)->getMock();
271+
$repository->expects($this->any())
272+
->method('findOneBy')
273+
->willReturnCallback(static fn ($v) => match ($v) {
274+
['slug' => 'vienna-2024'] => $conference,
275+
['title' => 'foo'] => $article,
276+
});
277+
278+
$manager->expects($this->any())
279+
->method('getRepository')
280+
->willReturn($repository);
281+
282+
$this->assertSame([$conference], $resolver->resolve($request, $argument1));
283+
$this->assertSame([$article], $resolver->resolve($request, $argument2));
284+
}
285+
260286
public function testExceptionWithExpressionIfNoLanguageAvailable()
261287
{
262288
$manager = $this->getMockBuilder(ObjectManager::class)->getMock();

0 commit comments

Comments
 (0)