Auditor fails to compute diff for new entities created in a transaction that depend on typed properties being used in the __toString() about auditor

[SanderVerkuil]

What I think happened, was that I was using the ReferenceRepository, which internally fetches only the reference.

After a fixture is loaded, the manager is cleared and the other fixtures are being loaded. I tried to recreate it by adding the fixture bundle as a dependency, and using the ORMExecutor with the references, but to no avail.

I think it happens when the DB has an entity DummyEntity with an ID of 1, and instead of fetching that entity fully, the underlying system calls the $em->getReference(DummyEntity, 1) which loads the partial entity.

However, I do see that the initializeObject method is called, so it could be that the initializer of the object is just wrong :/

Thanks for looking into it though, and wrapping it in a Throwable. It might be worth it to throw a user warning, so we don't completely hide the error that happens?

[DamienHarper]

Hi @SanderVerkuil, I've not been able to reproduce your issue so far. Could you provide a code sample that helps reproducing the issue? I tested something very similar to the code you initially provided but everything worked as intended.

// Auditable entity
/**
 * @ORM\Entity
 * @ORM\Table(name="issue95")
 * @Auditable
 */
class Issue95
{
    /**
     * @ORM\Id
     * @ORM\Column(type="integer")
     * @ORM\GeneratedValue(strategy="IDENTITY")
     */
    private $id;

    /**
     * @ORM\Column(type="string", name="type", length=50)
     */
    private string $name;

    public function __construct(string $name)
    {
        $this->name = $name;
    }

    public function __toString(): string
    {
        return $this->name;
    }

    public function getName(): string
    {
        return $this->name;
    }
}


// sample code that persist an instance of this entity
$em = $this->provider->getStorageServiceForEntity(Issue95::class)->getEntityManager();
$entity = new Issue95('issue95');
$em->persist($entity);
$em->flush();

// As expected, 1 record is added to `issue95` table as well as 1 record added to `issue95_audit` table

[DamienHarper]

@SanderVerkuil Looking at the provided traces, it seems that your Entity instance do not have the type property set at save time.

    ./src/Entity.php:35 {
      Entity->__toString()
      › {
      ›     return $this->type;
      › }
    }

That way, a PHP Fatal error is triggered at save time (because Entity::__toString() is called, if this method exists, to get a "representation" of the instance) but I have no way to handle this properly.
I cannot wrap the __toString call in a try/catch as it triggers a fatal error (which is different from an exception) and even if I could handle this at auditor level, this method would fail anyway if called from anywhere else.

[SanderVerkuil]

@DamienHarper Thanks for taking a look at it. The weird thing though, is that it was initialized, as it must be initialized through the constructor, so it could not be uninitialized. For some reason though, it was actually initialized.

Maybe this is a weird mixture of other dependencies that don't play together very well, as I couldn't reproduce it here either.

When I tried to reproduce it locally in my own project it also worked just fine going through the regular business rules, but it would keep on breaking when creating the entities with DataFixtures. I did manage to fix it by setting the __toString() method as follows:

/** @Auditable */
class Test 
{
    private string $type;

    public function __construct(string $type) 
    {
        $this->$type = $type;
    }

    public function __toString()
    {
        # Unfortunately this change is necessary, otherwise the Auditor fails to generate a nice diff. Might be solved with a new auditor update.
        $prop = (new \ReflectionClass(self::class))->getProperty('type');
        $prop->setAccessible(true);
        if (!$prop->isInitialized($this)) {
            return spl_object_hash($this);
        }
        return $this->type;
    }
}

And the even weirder behavior was that when setting the property to '' within the property definition, the property had been initialized to null, so I'd get an error somewhere else stating that null can't be used as a string value.

I'll try and see if I can dig into it more, and see what dark magic the DataFixture performed, resulting in this weird behavior.

I think it would be solved by wrapping the (string) in a try/catch, like:

try {
  $representation = (string)$entity;
} catch (\Throwable $t)
{
  // handle the error, maybe like so:
  $representation = spl_object_hash($entity);
}

though I'm not so sure about that, and whether this is something that this project should actively do. And in order to correctly verify this, we ought to determine where the error actually comes from.

[DamienHarper]

@SanderVerkuil you're right, I can workaround the issue using a try/catch regarding a Throwable instead of an Exception (my bad), will do a PR shortly.

[DamienHarper]

@SanderVerkuil feel free to share your findings about DataFixture's dark magic regarding typed properties ;)

[DamienHarper]

It might be worth it to throw a user warning, so we don't completely hide the error that happens?

Well, I think it might be ok to add that warning on one hand. But on the other hand I think most users won't find the root cause of the warning or won't be able to fix it if it comes from Doctrine. So they'll get warnings they can't get rid of which can be frustrating.