Repository: DamienHarper/auditor (php)

Issue:
Auditor fails to compute diff for new entities created in a transaction that depend on typed properties being used in the __toString()
<!--
- Please do not report an issue for a version of `auditor` that is no longer supported. Currently supported versions are listed here: https://github.com/DamienHarper/auditor#version-information
- Please fill in this template according to your issue.
- Please keep the table shown below at the top of your issue.
- You can retrieve `auditor` version by running `composer info | grep "auditor"`.
- You can retrieve PHP version by running `php -v`.
- Please post code as text (using proper markup). Do not post screenshots of code.
- Please remove this comment before submitting your issue.
-->

| Q                    | A
| ---------------------| -----------------------
| `auditor` version    | 2.0.1
| PHP version          | 7.4.28
| Database             | MySQL

#### Summary

<!-- Provide a summary describing the problem you are experiencing. -->

The auditor fails to create the audit trails for entities that are created within a transaction and have a custom `__toString()` method that depends on type hinted properties.

#### Current behavior

<!-- What is the current (buggy) behavior? -->

It currently throws an exception that a property must be initialized prior to accessing, or that `null` can't be used if the property has a default value.

<details><summary>Stack trace</summary>

```
Error {#220741
  #message: "Typed property Entity::$type must not be accessed before initialization"
  #code: 0
  #file: "./src/Entity.php"
  #line: 35
  trace: {
    ./src/Entity.php:35 {
      Entity->__toString()
      › {
      ›     return $this->type;
      › }
    }
    ./var/cache/selenium/doctrine/orm/Proxies/__CG__Entity.php:203 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php:198 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php:164 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php:60 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php:139 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Transaction/TransactionProcessor.php:38 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Transaction/TransactionManager.php:32 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php:43 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Logger/Logger.php:28 { …}
    ./vendor/damienharper/auditor/src/Provider/Doctrine/Auditing/Logger/LoggerChain.php:30 { …}
    ./vendor/doctrine/dbal/lib/Doctrine/DBAL/Connection.php:1783 { …}
    ./vendor/doctrine/orm/lib/Doctrine/ORM/EntityManager.php:242 { …}
    ./vendor/doctrine/data-fixtures/lib/Doctrine/Common/DataFixtures/Executor/ORMExecutor.php:76 { …}
    ./vendor/doctrine/doctrine-fixtures-bundle/Command/LoadDataFixturesDoctrineCommand.php:158 { …}
    ./vendor/symfony/console/Command/Command.php:298 { …}
    ./vendor/symfony/console/Application.php:1033 { …}
    ./vendor/symfony/framework-bundle/Console/Application.php:96 { …}
    ./vendor/symfony/console/Application.php:299 { …}
    ./vendor/symfony/framework-bundle/Console/Application.php:82 { …}
    ./vendor/symfony/console/Application.php:171 { …}
    ./vendor/symfony/runtime/Runner/Symfony/ConsoleApplicationRunner.php:54 { …}
    ./vendor/autoload_runtime.php:35 { …}
    ./bin/console:11 { …}
  }
}
```

</details>

#### How to reproduce

<!-- Provide steps to reproduce the bug. -->

I think the snippet below is the general case what happens

```php

/**
 * @Auditable()
 */
class TestEntity {
  private string $name;

  public function __construct(string $name) {
    $this->name = $name;
  }

  public function getName(): string
  {
    return $this->name;
  }

  public function __toString(): string
  {
    return $this->name;
  }
}

$em = $container->get(EntityManagerInterface::class);

$em->transactional(function($manager) {
  $test = new TestEntity('Hello');
  $manager->persist($test);
  $manager->flush();
};
```

I'll see whether I can create a PR that creates a test for this bug.

#### Expected behavior

<!-- What was the expected (correct) behavior? -->

I expected that it would not crash, and create an insert log item.


Pull request description:
Handle errors (Throwable) in entities `__toString` method
Fixes #95

Patch:
diff --git a/phpstan.neon b/phpstan.neon
index f9af3a6..5c809a5 100644
--- a/phpstan.neon
+++ b/phpstan.neon
@@ -21,3 +21,4 @@ parameters:
         - '~Method DH\\Auditor\\Provider\\Doctrine\\Persistence\\Reader\\Filter\\SimpleFilter\:\:__construct\(\) has parameter \$value with no type specified.~'
         - '~Property DH\\Auditor\\Provider\\Doctrine\\Configuration\:\:\$(auditing|storage)Services is never read, only written\.~'
         - '~Method DH\\Auditor\\Provider\\Doctrine\\Persistence\\Helper\\DoctrineHelper\:\:getDoctrineType\(\) should return string but returns mixed\.~'
+        - '~Dead catch - Throwable is never thrown in the try block~'
diff --git a/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php b/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php
index a2d4991..75ae754 100644
--- a/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php
+++ b/src/Provider/Doctrine/Auditing/Transaction/AuditTrait.php
@@ -10,6 +10,7 @@ use DH\Auditor\User\UserInterface;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\MappingException as ORMMappingException;
+use Throwable;
 
 trait AuditTrait
 {
@@ -195,7 +196,11 @@ trait AuditTrait
         $pkName = $meta->getSingleIdentifierFieldName();
 
         if (method_exists($entity, '__toString')) {
-            $label = (string) $entity;
+            try {
+                $label = (string) $entity;
+            } catch (Throwable $throwable) {
+                $label = DoctrineHelper::getRealClassName($entity).(null === $pkValue ? '' : '#'.$pkValue);
+            }
         } else {
             $label = DoctrineHelper::getRealClassName($entity).(null === $pkValue ? '' : '#'.$pkValue);
         }


Test patch:
diff --git a/tests/Provider/Doctrine/Fixtures/Issue95/Issue95.php b/tests/Provider/Doctrine/Fixtures/Issue95/Issue95.php
new file mode 100644
index 0000000..cc52a49
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue95/Issue95.php
@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue95;
+
+use DH\Auditor\Provider\Doctrine\Auditing\Annotation\Auditable;
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(name="issue95")
+ * @Auditable
+ */
+class Issue95
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column(type="integer")
+     * @ORM\GeneratedValue(strategy="IDENTITY")
+     */
+    private int $id;
+
+    /**
+     * @ORM\Column(type="string", name="type", length=50)
+     */
+    private string $name;
+
+    private string $type;   // uninitialized property
+
+    public function __construct(string $name)
+    {
+        $this->name = $name;
+    }
+
+    public function __toString(): string
+    {
+        return $this->name.$this->type;   // this triggers an uninitialized property error
+    }
+
+    public function getName(): string
+    {
+        return $this->name;
+    }
+}
diff --git a/tests/Provider/Doctrine/Issues/Issue95Test.php b/tests/Provider/Doctrine/Issues/Issue95Test.php
new file mode 100644
index 0000000..680db66
--- /dev/null
+++ b/tests/Provider/Doctrine/Issues/Issue95Test.php
@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Issues;
+
+use DH\Auditor\Model\Transaction;
+use DH\Auditor\Provider\Doctrine\DoctrineProvider;
+use DH\Auditor\Provider\Doctrine\Service\AuditingService;
+use DH\Auditor\Provider\Doctrine\Service\StorageService;
+use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue95\Issue95;
+use DH\Auditor\Tests\Provider\Doctrine\Traits\ReaderTrait;
+use DH\Auditor\Tests\Provider\Doctrine\Traits\Schema\SchemaSetupTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @internal
+ *
+ * @small
+ */
+final class Issue95Test extends TestCase
+{
+    use ReaderTrait;
+    use SchemaSetupTrait;
+
+    public function testIssue95(): void
+    {
+        $reader = $this->createReader();
+
+        $em = $this->provider->getStorageServiceForEntity(Issue95::class)->getEntityManager();
+        $entity = new Issue95('issue95');
+        $em->persist($entity);
+        $em->flush();
+
+        $audits = $reader->createQuery(Issue95::class)->execute();
+        self::assertCount(1, $audits, 'results count ok.');
+        self::assertSame(Transaction::INSERT, $audits[0]->getType(), 'Reader::INSERT operation.');
+    }
+
+    private function createAndInitDoctrineProvider(): void
+    {
+        $auditor = $this->createAuditor();
+        $this->provider = new DoctrineProvider($this->createProviderConfiguration());
+
+        $entityManager = $this->createEntityManager([
+            __DIR__.'/../../../../src/Provider/Doctrine/Auditing/Annotation',
+            __DIR__.'/../Fixtures/Issue95',
+        ]);
+        $this->provider->registerStorageService(new StorageService('default', $entityManager));
+        $this->provider->registerAuditingService(new AuditingService('default', $entityManager));
+
+        $auditor->registerProvider($this->provider);
+    }
+
+    private function configureEntities(): void
+    {
+        $this->provider->getConfiguration()->setEntities([
+            Issue95::class => ['enabled' => true],
+        ]);
+    }
+}