ObjectMapper lead to failing tests for PATCH operations on existing Entity

[rvanlaak]

API Platform version(s) affected: 4.2.2 is the latest working patch version without the issue, this issue relates to updating to 4.2.4

Description

When updated to 4.2.4 (4.2.3 had compilation issue) we started seeing failing tests. They seem to relate to patch / update operations on existing Doctrine entities.

Related stack trace:

{
  "@context": "\/contexts\/Error",
  "@id": "\/errors\/500",
  "@type": "Error",
  "title": "An error occurred",
  "detail": "App\\Domain\\Foo\\Bar::__construct(): Argument #1 ($baz) must be of type App\\Domain\\Bar\\Baz, null given, called in \/app\/vendor\/symfony\/object-mapper\/ObjectMapper.php on line 170",
  "status": 500,
  "type": "\/errors\/500",
  "description": "App\\Domain\\Foo\\Bar::__construct(): Argument #1 ($baz) must be of type App\\Domain\\Bar\\Baz, null given, called in \/app\/vendor\/symfony\/object-mapper\/ObjectMapper.php on line 170",
  "trace": [
    {
      "file": "\/app\/vendor\/symfony\/object-mapper\/ObjectMapper.php",
      "line": 170,
      "function": "__construct",
      "class": "App\\Domain\\Foo\\Bar",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/symfony\/object-mapper\/ObjectMapper.php",
      "line": 54,
      "function": "doMap",
      "class": "Symfony\\Component\\ObjectMapper\\ObjectMapper",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/state\/ObjectMapper\/ObjectMapper.php",
      "line": 44,
      "function": "map",
      "class": "Symfony\\Component\\ObjectMapper\\ObjectMapper",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/state\/Processor\/ObjectMapperProcessor.php",
      "line": 67,
      "function": "map",
      "class": "ApiPlatform\\State\\ObjectMapper\\ObjectMapper",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/state\/Processor\/WriteProcessor.php",
      "line": 58,
      "function": "process",
      "class": "ApiPlatform\\State\\Processor\\ObjectMapperProcessor",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/symfony\/Controller\/MainController.php",
      "line": 124,
      "function": "process",
      "class": "ApiPlatform\\State\\Processor\\WriteProcessor",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/symfony\/http-kernel\/HttpKernel.php",
      "line": 183,
      "function": "__invoke",
      "class": "ApiPlatform\\Symfony\\Controller\\MainController",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/symfony\/http-kernel\/HttpKernel.php",
      "line": 76,
      "function": "handleRaw",
      "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/symfony\/http-kernel\/Kernel.php",
      "line": 193,
      "function": "handle",
      "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
      "type": "-\u003E"
    },
    # ...
  ]
}

How to reproduce

Possible Solution

Verify whether the object mapper has the right object to patch?

Could this have something to do with the entity from the ReadProvider not being passed along to the ObjectMapperProcessor? And hence leading to the ObjectMapper calling the constructor when creating a new object, while an existing object should be written?

Additional Context

symfony/object-mapper version is 7.4

[rvanlaak]

Follow-up; after having updated one patch version at the time, when updating to APIP 4.2.6 (without any other packages being updated) the error messages as described above changes and leads to an EntityIdentityCollisionException. This exception message still is the same on the latest 4.2.10 version:

{
  "@context": "\/contexts\/Error",
  "@id": "\/errors\/500",
  "@type": "Error",
  "title": "An error occurred",
  "detail": "While adding an entity of class App\\Domain\\Foo\\Bar with an ID hash of \u0022019b0ed2-e1dd-79fe-a425-abac9ba915c4\u0022 to the identity map,\nanother object of class App\\Domain\\Foo\\Bar was already present for the same ID. This exception\nis a safeguard against an internal inconsistency - IDs should uniquely map to\nentity object instances. This problem may occur if:\n\n- you use application-provided IDs and reuse ID values;\n- database-provided IDs are reassigned after truncating the database without\nclearing the EntityManager;\n- you might have been using EntityManager#getReference() to create a reference\nfor a nonexistent ID that was subsequently (by the RDBMS) assigned to another\nentity.\n\nOtherwise, it might be an ORM-internal inconsistency, please report it.",
  "status": 500,
  "type": "\/errors\/500",
  "description": "While adding an entity of class App\\Domain\\Foo\\Bar with an ID hash of \u0022019b0ed2-e1dd-79fe-a425-abac9ba915c4\u0022 to the identity map,\nanother object of class App\\Domain\\Foo\\Bar was already present for the same ID. This exception\nis a safeguard against an internal inconsistency - IDs should uniquely map to\nentity object instances. This problem may occur if:\n\n- you use application-provided IDs and reuse ID values;\n- database-provided IDs are reassigned after truncating the database without\nclearing the EntityManager;\n- you might have been using EntityManager#getReference() to create a reference\nfor a nonexistent ID that was subsequently (by the RDBMS) assigned to another\nentity.\n\nOtherwise, it might be an ORM-internal inconsistency, please report it.",
  "trace": [
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 1554,
      "function": "create",
      "class": "Doctrine\\ORM\\Exception\\EntityIdentityCollisionException",
      "type": "::"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 1402,
      "function": "addToIdentityMap",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 939,
      "function": "scheduleForInsert",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 869,
      "function": "persistNew",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 755,
      "function": "computeAssociationChanges",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 805,
      "function": "computeChangeSet",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/UnitOfWork.php",
      "line": 353,
      "function": "computeChangeSets",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/doctrine\/orm\/src\/EntityManager.php",
      "line": 268,
      "function": "commit",
      "class": "Doctrine\\ORM\\UnitOfWork",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/doctrine-common\/State\/PersistProcessor.php",
      "line": 144,
      "function": "flush",
      "class": "Doctrine\\ORM\\EntityManager",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/state\/CallableProcessor.php",
      "line": 52,
      "function": "process",
      "class": "ApiPlatform\\Doctrine\\Common\\State\\PersistProcessor",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/state\/Processor\/ObjectMapperProcessor.php",
      "line": 49,
      "function": "process",
      "class": "ApiPlatform\\State\\CallableProcessor",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/state\/Processor\/WriteProcessor.php",
      "line": 58,
      "function": "process",
      "class": "ApiPlatform\\State\\Processor\\ObjectMapperProcessor",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/api-platform\/symfony\/Controller\/MainController.php",
      "line": 125,
      "function": "process",
      "class": "ApiPlatform\\State\\Processor\\WriteProcessor",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/symfony\/http-kernel\/HttpKernel.php",
      "line": 183,
      "function": "__invoke",
      "class": "ApiPlatform\\Symfony\\Controller\\MainController",
      "type": "-\u003E"
    },
    {
      "file": "\/app\/vendor\/symfony\/http-kernel\/HttpKernel.php",
      "line": 76,
      "function": "handleRaw",
      "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
      "type": "-\u003E"
    },
    # ...
  ]
}

[soyuka]

Interesting indeed as the object exists we do reuse the entity id, and as the object mapper creates a new object we re-assign that id though the object may be different inside the unit of work, do you have a full reproducer for this?

[rvanlaak]

Started working on a reproducer, but am struggling with getting it reproduced. The below version does work for PATCH with having uuid as id on the entity, and while using ObjectMapper: https://github.com/rvanlaak/api-platform/tree/reproducer-7605

Out of time for Today, but will try once more Tomorrow.

[soyuka]

Maybe that we need to do the same thing we do for 'PUT' at https://github.com/api-platform/core/blob/main/src/Doctrine/Common/State/PersistProcessor.php ?

[rvanlaak]

Oh that's an interesting pointer. Will make time before Wednesday to verify that.

Also have another assumption I'd have to test; Doctrine ORM 3.4's native & lazy ghost objects, which I've got enabled for my project.

[soyuka]

I see also related doctrine/orm#12297

[rvanlaak]

@soyuka now ObjectManager is among us, it feels like it'll be able to fully replace that custom logic using reflection in the PersistProcessor for the PUT method. In other words; maybe using PUT and PATCH internally always implies using the object mapper?

Even when commenting out the entire section of custom logic in the PersistProcessor, I still get the same "was already present for the same ID" exception as above. So it seems like the issue isn't with that processor, but the reference to the Doctrine object that should get written gets broken earlier with another provider / processor already.

For now still can't pin down an easy reproducer yet.

[rvanlaak]

Disabling ghosts / lazy objects does not resolve this either. Tests still fail with the same exception code.

[soyuka]

Would you be able to create a minimal reproducer?

[rvanlaak]

Trying to, but have been trying to get a reproducer for hours and still no luck. Am trying to by using the same shape of config / entity / resource / operation / input / etc, but still can't reproduce it outside my own codebase. With XDebug I'm also having a hard time to find out where the entity gets detached in such a way that the PersistProcessor is handling a detached entity.