sonata-project / EntityAuditBundle

Audit for Doctrine Entities
https://sonata-project.org
MIT License
628 stars 257 forks source link

spl_object_id() expects parameter 1 to be object, string given in PHP 7.4 and version AuditBundle 1.12 #623

Open DenyPt96 opened 2 months ago

DenyPt96 commented 2 months ago

Environment

Sonata packages

show

``` Direct dependencies required in composer.json: sonata-project/entity-audit-bundle 1.12.0 1.12.0 Audit for Doctrine Entities ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' # Put the result here. ```

PHP version

php -v
PHP 7.4.33 (cli) (built: Apr 11 2024 22:13:28) ( NTS )
Copyright (c) The PHP Group
Zend Engine v3.4.0, Copyright (c) Zend Technologies
    with Zend OPcache v7.4.33, Copyright (c), by Zend Technologies
    with Xdebug v3.1.6, Copyright (c) 2002-2022, by Derick Rethans

Subject

Auditing entity which uses foreign key as primary key throws exception when entity manager flushes.

Minimal repository with the bug

Steps to reproduce

When entity manager flushes and exception is thrown: spl_object_id() expects parameter 1 to be object, string given Screenshot from 2024-05-03 11-20-59

Table product: Screenshot from 2024-05-03 11-45-08 Screenshot from 2024-05-03 11-45-20

Table product_company Screenshot from 2024-05-03 11-42-35

Expected results

Actual results

VincentLanglet commented 2 months ago

It seems related to isInIdentityMap, and there is two occurences https://github.com/search?q=repo%3Asonata-project%2FEntityAuditBundle+isInIdentityMap&type=code

I think we're passing the old data and the new data so without any more information, I would think you are passing string instead of object to one of your oTo/oTm/mTo/mTm relations.

DenyPt96 commented 1 month ago

I suspect that array_merge in the LogRevisionsListener::postUpdate() function (https://github.com/sonata-project/EntityAuditBundle/blob/1.12.0/src/EventListener/LogRevisionsListener.php#L246) provides an unexpected result for those entities which primary keys include a foreign key/relationship to another entity (which is our case with ProductCompany entity)

i have dumped data of each of the three arrays that are being merged there:

$this->getOriginalEntityData($em, $entity) image

$uow->getEntityIdentifier($entity), image (1)

$this->getManyToManyRelations($em, $entity) image (2)

values from $uow->getEntityIdentifier($entity) override those from $this->getOriginalEntityData($em, $entity), so the resulting array is image (3)

in which the 'product' and 'company' entities have been replaced with only their ids

later this causes the 'spl_object_id() expects parameter 1 to be object, string given' TypeError in the LogRevisionsListener::saveRevisionEntityData() function (https://github.com/sonata-project/EntityAuditBundle/blob/1.12.0/src/EventListener/LogRevisionsListener.php#L522) where function \Doctrine\ORM\UnitOfWork::isInIdentityMap($entity) recieves 'PN0002624' in the $entity parameter, instead of the expected Product entity (the same would then happen for the 'PSP' string in place of the expected Company entity)

i think we are saving our entities correctly and this is an oversight in EntityAuditBundle related to primary keys composed of foreign keys (similar to https://github.com/sonata-project/EntityAuditBundle/issues/502)