fix(state): object-mapper reuse related entity

[soyuka]

Q	A
Branch?	main
License	MIT

Fixes an issue where we keep fetched relations so that we don't create new entities instead of persisting the existing one. Check the test and the issue at symfony/symfony#61119.

This needs symfony/symfony#61145 to work.

[alanpoulain]

Don't we want to check if a target has not been given explicitly by the user before overriding it?

[soyuka]

In my tests the target is a string and I do need to map but I'll test this further.

[mrossard]

I agree, if the target is an object it shouldn't be overridden.

[mrossard]

I was taking a look at the ObjectMapper, and I have doubts about simple patches in the current implementation - if you add $this->assertEquals($uri, $r->toArray()['@id']); here : https://github.com/api-platform/core/blob/main/tests/Functional/MappingTest.php#L69 you get this :

1) ApiPlatform\Tests\Functional\MappingTest::testShouldMapBetweenResourceAndEntity
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'/mapped_resources/11'
+'/mapped_resources/12'

Would this fix work on this too?

[edit]

Yes, it should.

[mrossard]

Shouldn't you also add $this->objectMap[$source] = $target; ? In persist operations you have multiple back and forth mappings between resource and entity.

This probably should be indexed by the object class too, otherwise multiple mappings will not work - you'll get errors similar to what was fixed by #7311

[soyuka]

Do you have a test case for this?

[mrossard]

I'll try to build one, but you can imagine something like a resource that includes 2 different subresources that are actually representations of the same entity. You'd be storing that $entity maps to $firstResource, then on the call for SecondResource::class you'd get back $firstResource.

[mrossard]

...and if it was about the first part of my comment, ObjectMapperProcessor does the transformation both ways :

• first the resource is mapped to the entity
• the entity is persisted
• the entity is mapped back to the resource

But as i'm writing this i realize it shouldn't reuse the previous object since its contents can change on persist, so you can just ignore my ramblings :D

[mrossard]

I agree, if the target is an object it shouldn't be overridden.