Skip to content

Commit 41c9f8d

Browse files
Rodrigue Villetardteohhanhui
authored andcommitted
Allow DataPersister to return a new instance instead of mutating
see : #1799
1 parent 677d404 commit 41c9f8d

File tree

8 files changed

+63
-9
lines changed

8 files changed

+63
-9
lines changed

src/Bridge/Doctrine/Common/DataPersister.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public function supports($data): bool
4848
public function persist($data)
4949
{
5050
if (!$manager = $this->getManager($data)) {
51-
return;
51+
return $data;
5252
}
5353

5454
if (!$manager->contains($data)) {
@@ -57,6 +57,8 @@ public function persist($data)
5757

5858
$manager->flush();
5959
$manager->refresh($data);
60+
61+
return $data;
6062
}
6163

6264
/**

src/Bridge/Doctrine/EventListener/WriteListener.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,9 @@ public function onKernelView(GetResponseForControllerResultEvent $event)
5858

5959
switch ($request->getMethod()) {
6060
case 'POST':
61-
$objectManager->persist($controllerResult);
61+
$event->setControllerResult(
62+
$objectManager->persist($controllerResult) ?? $controllerResult
63+
);
6264
break;
6365
case 'DELETE':
6466
$objectManager->remove($controllerResult);

src/DataPersister/ChainDataPersister.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public function persist($data)
5151
{
5252
foreach ($this->persisters as $persister) {
5353
if ($persister->supports($data)) {
54-
$persister->persist($data);
54+
return $persister->persist($data) ?? $data;
5555
}
5656
}
5757
}

src/DataPersister/DataPersisterInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ public function supports($data): bool;
3333
* Persists the data.
3434
*
3535
* @param mixed $data
36+
*
37+
* @return object|void Void will not be supported in API Platform 3, an object should always be returned
3638
*/
3739
public function persist($data);
3840

src/EventListener/WriteListener.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,10 @@ public function onKernelView(GetResponseForControllerResultEvent $event)
5050
case 'PUT':
5151
case 'PATCH':
5252
case 'POST':
53-
$this->dataPersister->persist($controllerResult);
53+
$event->setControllerResult(
54+
$this->dataPersister->persist($controllerResult)
55+
?? $controllerResult
56+
);
5457
break;
5558
case 'DELETE':
5659
$this->dataPersister->remove($controllerResult);

src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ public function __invoke(string $resourceClass = null, string $rootClass = null,
9393
$context += $resourceMetadata->getGraphqlAttribute($operationName, 'denormalization_context', [], true);
9494
$item = $this->normalizer->denormalize($args['input'], $resourceClass, ItemNormalizer::FORMAT, $context);
9595
$this->validate($item, $info, $resourceMetadata, $operationName);
96-
$this->dataPersister->persist($item);
96+
$persistResult = $this->dataPersister->persist($item) ?? $item;
9797

98-
return $this->normalizer->normalize($item, ItemNormalizer::FORMAT, $normalizationContext) + $data;
98+
return $this->normalizer->normalize($persistResult, ItemNormalizer::FORMAT, $normalizationContext) + $data;
9999
case 'delete':
100100
if ($item) {
101101
$this->dataPersister->remove($item);

tests/Bridge/Doctrine/Common/DataPersisterTest.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ public function testPersist()
5656
$managerRegistryProphecy = $this->prophesize(ManagerRegistry::class);
5757
$managerRegistryProphecy->getManagerForClass(Dummy::class)->willReturn($objectManagerProphecy->reveal())->shouldBeCalled();
5858

59-
(new DataPersister($managerRegistryProphecy->reveal()))->persist($dummy);
59+
$result = (new DataPersister($managerRegistryProphecy->reveal()))->persist($dummy);
60+
$this->assertSame($dummy, $result);
6061
}
6162

6263
public function testPersistIfEntityAlreadyManaged()
@@ -72,15 +73,19 @@ public function testPersistIfEntityAlreadyManaged()
7273
$managerRegistryProphecy = $this->prophesize(ManagerRegistry::class);
7374
$managerRegistryProphecy->getManagerForClass(Dummy::class)->willReturn($objectManagerProphecy->reveal())->shouldBeCalled();
7475

75-
(new DataPersister($managerRegistryProphecy->reveal()))->persist($dummy);
76+
$result = (new DataPersister($managerRegistryProphecy->reveal()))->persist($dummy);
77+
$this->assertSame($dummy, $result);
7678
}
7779

7880
public function testPersistWithNullManager()
7981
{
82+
$dummy = new Dummy();
83+
8084
$managerRegistryProphecy = $this->prophesize(ManagerRegistry::class);
8185
$managerRegistryProphecy->getManagerForClass(Dummy::class)->willReturn(null)->shouldBeCalled();
8286

83-
(new DataPersister($managerRegistryProphecy->reveal()))->persist(new Dummy());
87+
$result = (new DataPersister($managerRegistryProphecy->reveal()))->persist($dummy);
88+
$this->assertSame($dummy, $result);
8489
}
8590

8691
public function testRemove()

tests/EventListener/WriteListenerTest.php

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,46 @@ public function testOnKernelViewWithControllerResultAndPersist()
4949
$request->setMethod($httpMethod);
5050

5151
(new WriteListener($dataPersisterProphecy->reveal()))->onKernelView($event);
52+
$this->assertSame($dummy, $event->getControllerResult());
53+
}
54+
}
55+
56+
/**
57+
* @see https://github.com/api-platform/core/issues/1799
58+
*/
59+
public function testOnKernelViewWithControllerResultAndPersistWithImmutableResource()
60+
{
61+
$dummy = new Dummy();
62+
$dummy->setName('Dummyrino');
63+
64+
$dummy2 = new Dummy();
65+
$dummy2->setName('Dummyferoce');
66+
67+
$dataPersisterProphecy = $this->prophesize(DataPersisterInterface::class);
68+
$dataPersisterProphecy->supports($dummy)->willReturn(true)->shouldBeCalled();
69+
70+
$dataPersisterProphecy
71+
->persist($dummy)
72+
->willReturn($dummy2) // Persist is not mutating $dummy, but return a brand new technically unrelated object instead
73+
->shouldBeCalled()
74+
;
75+
76+
$request = new Request();
77+
$request->attributes->set('_api_resource_class', Dummy::class);
78+
79+
foreach (['PATCH', 'PUT', 'POST'] as $httpMethod) {
80+
$event = new GetResponseForControllerResultEvent(
81+
$this->prophesize(HttpKernelInterface::class)->reveal(),
82+
$request,
83+
HttpKernelInterface::MASTER_REQUEST,
84+
$dummy
85+
);
86+
87+
$request->setMethod($httpMethod);
88+
89+
(new WriteListener($dataPersisterProphecy->reveal()))->onKernelView($event);
90+
91+
$this->assertSame($dummy2, $event->getControllerResult());
5292
}
5393
}
5494

0 commit comments

Comments
 (0)