Repository: DamienHarper/auditor (php)

Issue:
exception while creating audit table schema when using single table inheritance
| Q                    | A
| ---------------------| -----------------------
| `auditor` version    | 2.3.0
| PHP version          | 8.1.8
| Database             | MySQL / SQLite

#### Summary

I have an abstract parent entity which is not audited, but uses doctrine/orm inheritance type "SINGLE_TABLE" and defines the database table for its child entities. Some child entities are configured to be audited.

With auditor 2.3.0, creating the audit database schema throws an exception.

#### Current behavior

When the database schema is created for this abstract parent entity, the CreateSchemaListener wants to create the corresponding audit table. This throws an exception while trying to determine the table name from the configuration with the new logic introduced in auditor 2.3.0, as the parent entity does not have any configuration: `Undefined array key "App\AbstractParentEntity"`

https://github.com/DamienHarper/auditor/blob/19ff16a658e7bf268dbea7b88c08118da5caf1ae/src/Provider/Doctrine/Persistence/Schema/SchemaManager.php#L173

In versions before 2.3.0, the table name from the event was being used, which is independent of the configuration.

#### How to reproduce

Define an abstract parent entity and an audited child entity:

```
namespace App;

/**
 * @ORM\Table(name="entities")
 * @ORM\Entity()
 * @ORM\InheritanceType("SINGLE_TABLE")
 */
abstract class AbstractParentEntity {}

/**
 * @ORM\Entity
 */
class Entity {}
```

Configure auditing for the child entity:

```
dh_auditor:
    enabled: true
    providers:
        doctrine:
            table_prefix: ~
            table_suffix: '_audit'
            entities:
                App\Entity: ~
```

Create the database schema using `Doctrine\ORM\Tools\SchemaTool::createSchema` for the parent entity to create an `Doctrine\ORM\Tools\ToolEvents::postGenerateSchemaTable` event.

#### Expected behavior

Creating the database schema for the abstract parent entity and its child entities does not throw an exception and creates the corresponding audit table schema.


Pull request:
Fix bug: exception while creating audit table schema when using single table inheritance
Closes #132 

Patch:
diff --git a/phpstan.neon b/phpstan.neon
index 2c6fadc..fd8d16e 100644
--- a/phpstan.neon
+++ b/phpstan.neon
@@ -16,4 +16,3 @@ parameters:
         - '~Dead catch - Throwable is never thrown in the try block~'
         - '~Parameter \#1 \$className of method Doctrine\\ORM\\EntityManagerInterface\:\:getClassMetadata\(\) expects class-string<object>, string given\.~'
         - '~Unable to resolve the template type T in call to method Doctrine\\ORM\\EntityManagerInterface\:\:getClassMetadata\(\)~'
-        - '~Property DH\\Auditor\\Provider\\Doctrine\\Configuration\:\:\$(storage|auditing)Services is never read, only written\.~'
diff --git a/src/Provider/Doctrine/Configuration.php b/src/Provider/Doctrine/Configuration.php
index 886b9cf..332fc5f 100644
--- a/src/Provider/Doctrine/Configuration.php
+++ b/src/Provider/Doctrine/Configuration.php
@@ -25,12 +25,10 @@ class Configuration implements ConfigurationInterface
 
     private ?array $entities = null;
 
-    private array $storageServices = [];
-
-    private array $auditingServices = [];
-
     private bool $isViewerEnabled;
 
+    private bool $initialized = false;
+
     /**
      * @var callable
      */
@@ -55,8 +53,6 @@ class Configuration implements ConfigurationInterface
             }
         }
 
-        $this->storageServices = $config['storage_services'];
-        $this->auditingServices = $config['auditing_services'];
         $this->isViewerEnabled = $config['viewer'];
         $this->storageMapper = $config['storage_mapper'];
     }
@@ -70,8 +66,6 @@ class Configuration implements ConfigurationInterface
                 'table_suffix' => '_audit',
                 'ignored_columns' => [],
                 'entities' => [],
-                'storage_services' => [],
-                'auditing_services' => [],
                 'viewer' => true,
                 'storage_mapper' => null,
             ])
@@ -79,8 +73,6 @@ class Configuration implements ConfigurationInterface
             ->setAllowedTypes('table_suffix', 'string')
             ->setAllowedTypes('ignored_columns', 'array')
             ->setAllowedTypes('entities', 'array')
-            ->setAllowedTypes('storage_services', 'array')
-            ->setAllowedTypes('auditing_services', 'array')
             ->setAllowedTypes('viewer', 'bool')
             ->setAllowedTypes('storage_mapper', ['null', 'string', 'callable'])
         ;
@@ -97,6 +89,7 @@ class Configuration implements ConfigurationInterface
     public function setEntities(array $entities): self
     {
         $this->entities = $entities;
+        $this->initialized = false;
 
         return $this;
     }
@@ -164,6 +157,9 @@ class Configuration implements ConfigurationInterface
      */
     public function getEntities(): array
     {
+        if ($this->initialized && null !== $this->entities) {
+            return $this->entities;
+        }
         if (null !== $this->provider) {
             $schemaManager = new SchemaManager($this->provider);
 
@@ -182,19 +178,24 @@ class Configuration implements ConfigurationInterface
                 \assert(null !== $this->entities);
                 foreach ($this->entities as $entity => $config) {
                     $meta = $entityManager->getClassMetadata(DoctrineHelper::getRealClassName($entity));
-                    $tableName = $meta->getTableName();
+                    $entityTableName = $meta->getTableName();
                     $namespaceName = $meta->getSchemaName() ?? '';
 
+                    $computedTableName = $schemaManager->resolveTableName($entityTableName, $namespaceName, $platform);
                     $this->entities[$entity]['table_schema'] = $namespaceName;
-                    $this->entities[$entity]['table_name'] = $tableName;
-//                    $this->entities[$entity]['computed_table_name'] = $tableName;
-                    $this->entities[$entity]['computed_table_name'] = $schemaManager->resolveTableName($tableName, $namespaceName, $platform);
+                    $this->entities[$entity]['table_name'] = $entityTableName;
+//                    $this->entities[$entity]['computed_table_name'] = $entityTableName;
+                    $this->entities[$entity]['computed_table_name'] = $computedTableName;
                     $this->entities[$entity]['audit_table_schema'] = $namespaceName;
-                    $this->entities[$entity]['audit_table_name'] = $schemaManager->computeAuditTablename($this->entities[$entity], $this, $platform);
+                    $this->entities[$entity]['audit_table_name'] = $schemaManager->computeAuditTablename($entityTableName, $this);
 //                    $this->entities[$entity]['computed_audit_table_name'] = $schemaManager->computeAuditTablename($this->entities[$entity], $this, $platform);
-                    $this->entities[$entity]['computed_audit_table_name'] = $schemaManager->resolveAuditTableName($this->entities[$entity], $this, $platform);
+                    $this->entities[$entity]['computed_audit_table_name'] = $schemaManager->computeAuditTablename(
+                        $computedTableName,
+                        $this
+                    );
                 }
             }
+            $this->initialized = true;
         }
 
         return $this->entities ?? [];
@@ -255,5 +256,6 @@ class Configuration implements ConfigurationInterface
     public function setProvider(DoctrineProvider $provider): void
     {
         $this->provider = $provider;
+        $this->initialized = false;
     }
 }
diff --git a/src/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommand.php b/src/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommand.php
index 01768a0..dee17e4 100644
--- a/src/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommand.php
+++ b/src/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommand.php
@@ -105,7 +105,6 @@ class CleanAuditLogsCommand extends Command
         if ($confirm) {
             /** @var Configuration $configuration */
             $configuration = $provider->getConfiguration();
-            $entities = $configuration->getEntities();
 
             $progressBar = new ProgressBar($output, $count);
             $progressBar->setBarWidth(70);
@@ -118,7 +117,7 @@ class CleanAuditLogsCommand extends Command
             foreach ($repository as $name => $classes) {
                 foreach ($classes as $entity => $tablename) {
                     $connection = $storageServices[$name]->getEntityManager()->getConnection();
-                    $auditTable = $schemaManager->resolveAuditTableName($entities[$entity], $configuration, $connection->getDatabasePlatform());
+                    $auditTable = $schemaManager->resolveAuditTableName($entity, $configuration, $connection->getDatabasePlatform());
 
                     /**
                      * @var QueryBuilder
diff --git a/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php b/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php
index c9417e5..1345133 100644
--- a/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php
+++ b/src/Provider/Doctrine/Persistence/Event/CreateSchemaListener.php
@@ -49,6 +49,8 @@ class CreateSchemaListener implements EventSubscriber
                 foreach ($metadata->subClasses as $subClass) {
                     if ($this->provider->isAuditable($subClass)) {
                         $audited = true;
+
+                        break;
                     }
                 }
             }
diff --git a/src/Provider/Doctrine/Persistence/Reader/Reader.php b/src/Provider/Doctrine/Persistence/Reader/Reader.php
index 72d34a8..c2b9ef4 100644
--- a/src/Provider/Doctrine/Persistence/Reader/Reader.php
+++ b/src/Provider/Doctrine/Persistence/Reader/Reader.php
@@ -120,7 +120,7 @@ class Reader
         $results = [];
 
         $entities = $configuration->getEntities();
-        foreach ($entities as $entity => $tablename) {
+        foreach ($entities as $entity => $entityOptions) {
             try {
                 $audits = $this->createQuery($entity, ['transaction_hash' => $transactionHash])->execute();
                 if (\count($audits) > 0) {
diff --git a/src/Provider/Doctrine/Persistence/Schema/SchemaManager.php b/src/Provider/Doctrine/Persistence/Schema/SchemaManager.php
index b7f7073..b98ee5f 100644
--- a/src/Provider/Doctrine/Persistence/Schema/SchemaManager.php
+++ b/src/Provider/Doctrine/Persistence/Schema/SchemaManager.php
@@ -111,8 +111,6 @@ class SchemaManager
         // Collect auditable entities from auditing entity managers
         $repository = $this->collectAuditableEntities();
 
-        $entities = $configuration->getEntities();
-
         // Compute and collect SQL queries
         $sqls = [];
         foreach ($repository as $name => $classes) {
@@ -126,7 +124,7 @@ class SchemaManager
             foreach ($classes as $entityFQCN => $tableName) {
                 if (!\in_array($entityFQCN, $processed, true)) {
                     /** @var string $auditTablename */
-                    $auditTablename = $this->resolveAuditTableName($entities[$entityFQCN], $configuration, $storageConnection->getDatabasePlatform());
+                    $auditTablename = $this->resolveAuditTableName($entityFQCN, $configuration, $storageConnection->getDatabasePlatform());
 
                     if ($storageSchema->hasTable($auditTablename)) {
                         // Audit table exists, let's update it if needed
@@ -163,9 +161,7 @@ class SchemaManager
 
         /** @var Configuration $configuration */
         $configuration = $this->provider->getConfiguration();
-        $entities = $configuration->getEntities();
-
-        $auditTablename = $this->resolveAuditTableName($entities[$entity], $configuration, $connection->getDatabasePlatform());
+        $auditTablename = $this->resolveAuditTableName($entity, $configuration, $connection->getDatabasePlatform());
 
         if (null !== $auditTablename && !$schema->hasTable($auditTablename)) {
             $auditTable = $schema->createTable($auditTablename);
@@ -219,9 +215,8 @@ class SchemaManager
 
         /** @var Configuration $configuration */
         $configuration = $this->provider->getConfiguration();
-        $entities = $configuration->getEntities();
 
-        $auditTablename = $this->resolveAuditTableName($entities[$entity], $configuration, $connection->getDatabasePlatform());
+        $auditTablename = $this->resolveAuditTableName($entity, $configuration, $connection->getDatabasePlatform());
         \assert(\is_string($auditTablename));
         $table = $schema->getTable($auditTablename);
 
@@ -237,7 +232,7 @@ class SchemaManager
     /**
      * Resolves table name, including namespace/schema.
      */
-    public function resolveTableName(string $tableName, string $namespaceName, AbstractPlatform $platform): ?string
+    public function resolveTableName(string $tableName, string $namespaceName, AbstractPlatform $platform): string
     {
         if (empty($namespaceName)) {
             $prefix = '';
@@ -253,35 +248,28 @@ class SchemaManager
     /**
      * Resolves audit table name, including namespace/schema.
      */
-    public function resolveAuditTableName(array $entityOptions, Configuration $configuration, AbstractPlatform $platform): ?string
+    public function resolveAuditTableName(string $entity, Configuration $configuration, AbstractPlatform $platform): ?string
     {
+        $entities = $configuration->getEntities();
+        $entityOptions = $entities[$entity];
         $tablename = $this->resolveTableName($entityOptions['table_name'], $entityOptions['audit_table_schema'], $platform);
-        \assert(\is_string($tablename));
 
-        return preg_replace(
-            sprintf('#^([^\.]+\.)?(%s)$#', preg_quote($tablename, '#')),
-            sprintf(
-                '$1%s$2%s',
-                preg_quote($configuration->getTablePrefix(), '#'),
-                preg_quote($configuration->getTableSuffix(), '#')
-            ),
-            $tablename
-        );
+        return $this->computeAuditTablename($tablename, $configuration);
     }
 
     /**
      * Computes audit table name **without** namespace/schema.
      */
-    public function computeAuditTablename(array $entityOptions, Configuration $configuration, AbstractPlatform $platform): ?string
+    public function computeAuditTablename(string $entityTableName, Configuration $configuration): ?string
     {
         return preg_replace(
-            sprintf('#^([^\.]+\.)?(%s)$#', preg_quote($entityOptions['table_name'], '#')),
+            sprintf('#^([^\.]+\.)?(%s)$#', preg_quote($entityTableName, '#')),
             sprintf(
                 '$1%s$2%s',
                 preg_quote($configuration->getTablePrefix(), '#'),
                 preg_quote($configuration->getTableSuffix(), '#')
             ),
-            $entityOptions['table_name']
+            $entityTableName
         );
     }
 


Test patch:
diff --git a/tests/Provider/Doctrine/Fixtures/Issue132/AbstractParentEntity.php b/tests/Provider/Doctrine/Fixtures/Issue132/AbstractParentEntity.php
new file mode 100644
index 0000000..3898f08
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue132/AbstractParentEntity.php
@@ -0,0 +1,32 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue132;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ * @ORM\Table('parent_entities')
+ * @ORM\InheritanceType('SINGLE_TABLE')
+ * @ORM\DiscriminatorColumn('type', 'string)
+ * @ORM\DiscriminatorMap(['Dummy' => DummyEntity::class])
+ */
+#[ORM\Entity]
+#[ORM\Table('parent_entities')]
+#[ORM\InheritanceType('SINGLE_TABLE')]
+#[ORM\DiscriminatorColumn('type', 'string')]
+#[ORM\DiscriminatorMap(['Dummy' => DummyEntity::class])]
+abstract class AbstractParentEntity
+{
+    /**
+     * @ORM\Id
+     * @ORM\Column
+     * @ORM\GeneratedValue
+     */
+    #[ORM\Id]
+    #[ORM\Column]
+    #[ORM\GeneratedValue]
+    private ?int $id = null;
+}
diff --git a/tests/Provider/Doctrine/Fixtures/Issue132/DummyEntity.php b/tests/Provider/Doctrine/Fixtures/Issue132/DummyEntity.php
new file mode 100644
index 0000000..3961a4f
--- /dev/null
+++ b/tests/Provider/Doctrine/Fixtures/Issue132/DummyEntity.php
@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue132;
+
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * @ORM\Entity
+ */
+#[ORM\Entity]
+class DummyEntity extends AbstractParentEntity
+{
+}
diff --git a/tests/Provider/Doctrine/Issues/Issue132Test.php b/tests/Provider/Doctrine/Issues/Issue132Test.php
new file mode 100644
index 0000000..8dfd2cc
--- /dev/null
+++ b/tests/Provider/Doctrine/Issues/Issue132Test.php
@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Issues;
+
+use DH\Auditor\Provider\Doctrine\Persistence\Schema\SchemaManager;
+use DH\Auditor\Tests\Provider\Doctrine\Fixtures\Issue132\DummyEntity;
+use DH\Auditor\Tests\Provider\Doctrine\Traits\Schema\DefaultSchemaSetupTrait;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @internal
+ *
+ * @small
+ */
+final class Issue132Test extends TestCase
+{
+    use DefaultSchemaSetupTrait;
+
+    public function testIssue132(): void
+    {
+        $manager = new SchemaManager($this->provider);
+        $schema = $manager->createAuditTable(DummyEntity::class);
+        self::assertThat('parent_entities_audit', self::callback(static function (string $tableName) use ($schema): bool {
+            foreach ($schema->getTables() as $table) {
+                if ($table->getName() === $tableName) {
+                    return true;
+                }
+            }
+
+            return false;
+        }));
+    }
+
+    private function configureEntities(): void
+    {
+        $this->provider->getConfiguration()->setEntities([
+            DummyEntity::class => ['enabled' => true],
+        ]);
+    }
+}
diff --git a/tests/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommandTest.php b/tests/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommandTest.php
index b9d6b66..9210812 100644
--- a/tests/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommandTest.php
+++ b/tests/Provider/Doctrine/Persistence/Command/CleanAuditLogsCommandTest.php
@@ -68,7 +68,7 @@ final class CleanAuditLogsCommandTest extends TestCase
         foreach ($entities as $entity => $entityOptions) {
             $storageService = $this->provider->getStorageServiceForEntity($entity);
             $platform = $storageService->getEntityManager()->getConnection()->getDatabasePlatform();
-            $expected = 'DELETE FROM '.$schemaManager->resolveAuditTableName($entityOptions, $configuration, $platform);
+            $expected = 'DELETE FROM '.$schemaManager->resolveAuditTableName($entity, $configuration, $platform);
             self::assertStringContainsString($expected, $output);
         }