3.x (#203)

* Fix bug: exception while creating audit table schema when using single table inheritance (#133) * fix #132 * entity Co-authored-by: a.dmitryuk <[email protected]> * Updated README.md * Updated CI * Fixed annotations in tests * Regression fix. Fixes DamienHarper/auditor-bundle#334 * Updated CI * Performance issues with metadatas (#137) * Cache DH Annotations * speed * remove getOwner call Co-authored-by: a.dmitryuk <[email protected]> * CreateSchemaListener not updating inheritance tables (#139) * Fix: Attempted to call an undefined method named "getMetadataCache" of class "Doctrine\ORM\Configuration". (#144) * Fix: Attempted to call an undefined method named "introspectSchema" of class "Doctrine\DBAL\Schema\PostgreSQLSchemaManager" (#143) * Fixed invokable storage mapper (see #146) (#148) * Fixed broken annotation registration since doctrine/orm 2.14 * improved the TransactionProcessor to convert encoding of diff properly (#152) * improved the TransactionProcessor to convert encoding of diff properly * adapted quotes in TransactionProcessor to single quotes * Fixed compatibility issue with doctrine/event-manager ^2.0 (#157) * PHP-CS-Fixer * Validate that diff is a string before passing to mb_convert_encoding (#156) * PHP-CS-Fixer * Support doctrine annotations 2.0 (#158) * topic 344 doctrine annotations 2.0 --------- Co-authored-by: a.dmitryuk <[email protected]> * PHP-CS-Fixer * Fix failing CI (lock related) * PHP-CS-Fixer * Fixes `diffs` column type when JSON type is supported * Fixed CI (2.x) * Fixed CI (2.x) * Fixed CI (2.x) * Fixed CI (2.x) * Fixed CI (2.x) * Fix 185 (#186) * fixes #185 * PHP-CS-Fixer --------- Co-authored-by: Damien Harper <[email protected]> * skip embedded class from schema listener (#189) Co-authored-by: Guillaume Sainthillier <[email protected]> * Fixed CI (2.x) * Typo * PHP-CS-Fixer * Update deps + PHP-CS-Fixer * PHP-CS-Fixer * PHP-CS-Fixer (3.48.0) * Detected wrapped driver (#195) * - updated DHDriver and use parent::connect instead of dedicated field - added getWrappedDriver to extract wrapped driver. - tests Signed-off-by: Oleg Andreyev <[email protected]> * Fixed PHPStan error --------- Signed-off-by: Oleg Andreyev <[email protected]> Co-authored-by: Damien Harper <[email protected]> * fix Signed-off-by: Oleg Andreyev <[email protected]> * php-cs-fixer Signed-off-by: Oleg Andreyev <[email protected]> * removed getSubscribedEvents from CreateSchemaListener Signed-off-by: Oleg Andreyev <[email protected]> * phpstan Signed-off-by: Oleg Andreyev <[email protected]> * fix DoctrineSubscriber + tests Signed-off-by: Oleg Andreyev <[email protected]> * fix Issue174Test Signed-off-by: Oleg Andreyev <[email protected]> * removed duplicate code, fixed test Signed-off-by: Oleg Andreyev <[email protected]> --------- Signed-off-by: Oleg Andreyev <[email protected]> Co-authored-by: Alexander Dmitryuk <[email protected]> Co-authored-by: a.dmitryuk <[email protected]> Co-authored-by: damienharper <[email protected]> Co-authored-by: Jörn Dyherrn <[email protected]> Co-authored-by: Martijn Boers <[email protected]> Co-authored-by: Guillaume Sainthillier <[email protected]>
DamienHarper · Jul 14, 2024 · aa0f9ee · aa0f9ee
1 parent 0a35944
commit aa0f9ee
Show file tree

Hide file tree

Showing 6 changed files with 240 additions and 24 deletions.
diff --git a/.github/workflows/ci-2.x.yml b/.github/workflows/ci-2.x.yml
@@ -116,7 +116,7 @@ jobs:
         run: echo "dir=$(composer config cache-files-dir)" >> $GITHUB_OUTPUT
 
       - name: Cache dependencies
-        uses: actions/cache@v3
+        uses: actions/cache@v4
         with:
           path: ${{ steps.composer-cache.outputs.dir }}
           key: ${{ matrix.php }}-composer-${{ hashFiles('**/composer.json') }}

diff --git a/phpstan.neon b/phpstan.neon
@@ -7,6 +7,9 @@ parameters:
         param_type: 100
         property_type: 100
 
+    paths:
+        - src
+
     ignoreErrors:
         - identifier: missingType.iterableValue
         - identifier: missingType.generics

diff --git a/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php b/src/Provider/Doctrine/Auditing/Event/DoctrineSubscriber.php
@@ -4,21 +4,23 @@
 
 namespace DH\Auditor\Provider\Doctrine\Auditing\Event;
 
+use Closure;
 use DH\Auditor\Provider\Doctrine\Auditing\Logger\Middleware\DHDriver;
-use DH\Auditor\Provider\Doctrine\Auditing\Transaction\TransactionManager;
 use DH\Auditor\Provider\Doctrine\Model\Transaction;
+use DH\Auditor\Transaction\TransactionManagerInterface;
 use Doctrine\Common\EventSubscriber;
+use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
 use Doctrine\ORM\Event\OnFlushEventArgs;
 use Doctrine\ORM\Events;
+use ReflectionClass;
 
 final class DoctrineSubscriber implements EventSubscriber
 {
-    private TransactionManager $transactionManager;
+    /** @var Transaction[] */
+    private array $transactions = [];
 
-    public function __construct(TransactionManager $transactionManager)
-    {
-        $this->transactionManager = $transactionManager;
-    }
+    public function __construct(private readonly TransactionManagerInterface $transactionManager) {}
 
     /**
      * It is called inside EntityManager#flush() after the changes to all the managed entities
@@ -29,12 +31,20 @@ public function __construct(TransactionManager $transactionManager)
     public function onFlush(OnFlushEventArgs $args): void
     {
         $entityManager = $args->getObjectManager();
-        $transaction = new Transaction($entityManager);
+        $entityManagerId = spl_object_id($entityManager);
+
+        // cached transaction model, if it holds same EM no need to create a new one
+        $transaction = ($this->transactions[$entityManagerId] ??= new Transaction($entityManager));
 
         // Populate transaction
         $this->transactionManager->populate($transaction);
 
         $driver = $entityManager->getConnection()->getDriver();
+
+        if (!$driver instanceof DHDriver) {
+            $driver = $this->getWrappedDriver($driver);
+        }
+
         if ($driver instanceof DHDriver) {
             $driver->addDHFlusher(function () use ($transaction): void {
                 $this->transactionManager->process($transaction);
@@ -47,4 +57,39 @@ public function getSubscribedEvents(): array
     {
         return [Events::onFlush];
     }
+
+    /**
+     * @internal this method is used to retrieve the wrapped driver from the given driver
+     */
+    public function getWrappedDriver(Driver $driver): Closure|Driver
+    {
+        $that = $this;
+
+        // if the driver is already a DHDriver, return it
+        if ($driver instanceof DHDriver) {
+            return $driver;
+        }
+
+        // if the driver is an instance of AbstractDriverMiddleware, return the wrapped driver
+        if ($driver instanceof AbstractDriverMiddleware) {
+            return Closure::bind(function () use ($that) {
+                // @var AbstractDriverMiddleware $this
+                return $that->getWrappedDriver($this->wrappedDriver);
+            }, $driver, AbstractDriverMiddleware::class)();
+        }
+
+        return Closure::bind(function () use ($that): Closure|Driver|null {
+            /** @var Driver $this */
+            $properties = (new ReflectionClass($this))->getProperties();
+            foreach ($properties as $property) {
+                $property->setAccessible(true);
+                $value = $property->getValue($this);
+                if ($value instanceof Driver) {
+                    return $that->getWrappedDriver($value);
+                }
+            }
+
+            return null;
+        }, $driver, Driver::class)() ?: $driver;
+    }
 }
diff --git a/src/Provider/Doctrine/Auditing/Logger/Middleware/DHDriver.php b/src/Provider/Doctrine/Auditing/Logger/Middleware/DHDriver.php
@@ -4,31 +4,19 @@
 
 namespace DH\Auditor\Provider\Doctrine\Auditing\Logger\Middleware;
 
-use Doctrine\DBAL\Driver as DriverInterface;
 use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
 
 /**
  * @interal
  */
 final class DHDriver extends AbstractDriverMiddleware
 {
-    private DriverInterface $driver;
-
     /** @var array<callable> */
     private array $flusherList = [];
 
-    public function __construct(DriverInterface $driver)
-    {
-        parent::__construct($driver);
-        $this->driver = $driver;
-    }
-
     public function connect(array $params): DHConnection
     {
-        return new DHConnection(
-            $this->driver->connect($params),
-            $this
-        );
+        return new DHConnection(parent::connect($params), $this);
     }
 
     public function addDHFlusher(callable $flusher): void

diff --git a/tests/Provider/Doctrine/Event/DoctrineSubscriberTest.php b/tests/Provider/Doctrine/Event/DoctrineSubscriberTest.php
@@ -0,0 +1,183 @@
+<?php
+
+declare(strict_types=1);
+
+namespace DH\Auditor\Tests\Provider\Doctrine\Event;
+
+use DH\Auditor\Provider\Doctrine\Auditing\Event\DoctrineSubscriber;
+use DH\Auditor\Provider\Doctrine\Auditing\Logger\Middleware\DHDriver;
+use DH\Auditor\Transaction\TransactionManagerInterface;
+use Doctrine\DBAL\Configuration;
+use Doctrine\DBAL\Connection as ConnectionDbal;
+use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\API\MySQL\ExceptionConverter;
+use Doctrine\DBAL\Driver\Middleware\AbstractDriverMiddleware;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\VersionAwarePlatformDriver;
+use Doctrine\ORM\EntityManagerInterface;
+use Doctrine\ORM\Event\OnFlushEventArgs;
+use PHPUnit\Framework\TestCase;
+use RuntimeException;
+
+/**
+ * @internal
+ *
+ * @small
+ */
+final class DoctrineSubscriberTest extends TestCase
+{
+    public function testIssue184IfAbstractDriverMiddleware(): void
+    {
+        $transactionManager = new class() implements TransactionManagerInterface {
+            public function populate($transaction): void {}
+
+            public function process($transaction): void
+            {
+                static $i = 0;
+                ++$i;
+                if ($i > 1) {
+                    throw new RuntimeException('Expected only once');
+                }
+            }
+        };
+        $objectManager = $this->createMock(EntityManagerInterface::class);
+
+        $args = new OnFlushEventArgs($objectManager);
+
+        $nativeDriver = $this->createMock(Driver::class);
+        $dhDriver = new DHDriver($nativeDriver);
+        $driver = new class($dhDriver) extends AbstractDriverMiddleware {};
+
+        $objectManager
+            ->method('getConnection')
+            ->willReturn($connection = $this->createMock(ConnectionDbal::class))
+        ;
+
+        $connection
+            ->method('getDriver')
+            ->willReturn($driver)
+        ;
+
+        $target = new DoctrineSubscriber($transactionManager);
+        $target->onFlush($args);
+
+        foreach ($dhDriver->getFlusherList() as $item) {
+            ($item)();
+        }
+
+        self::assertTrue(true);
+    }
+
+    public function testIssue184IfNotAbstractDriverMiddleware(): void
+    {
+        $transactionManager = new class() implements TransactionManagerInterface {
+            public function populate($transaction): void {}
+
+            public function process($transaction): void
+            {
+                static $i = 0;
+                ++$i;
+                if ($i > 1) {
+                    throw new RuntimeException('Expected only once');
+                }
+            }
+        };
+        $objectManager = $this->createMock(EntityManagerInterface::class);
+
+        $args = new OnFlushEventArgs($objectManager);
+
+        $nativeDriver = $this->createMock(Driver::class);
+        $dhDriver = new DHDriver($nativeDriver);
+        $driver = new class($dhDriver) implements VersionAwarePlatformDriver {
+            /** @noinspection PhpPropertyOnlyWrittenInspection */
+            private Driver $wrappedDriver;
+
+            public function __construct(Driver $wrappedDriver)
+            {
+                $this->wrappedDriver = $wrappedDriver;
+            }
+
+            public function connect(array $params): void {}
+
+            public function getDatabasePlatform(): void {}
+
+            public function getSchemaManager(ConnectionDbal $conn, AbstractPlatform $platform): void {}
+
+            public function getExceptionConverter(): Driver\API\ExceptionConverter
+            {
+                return new ExceptionConverter();
+            }
+
+            public function createDatabasePlatformForVersion($version): void {}
+        };
+
+        $objectManager
+            ->method('getConnection')
+            ->willReturn($connection = $this->createMock(ConnectionDbal::class))
+        ;
+
+        $connection
+            ->method('getDriver')
+            ->willReturn($driver)
+        ;
+
+        $target = new DoctrineSubscriber($transactionManager);
+        $target->onFlush($args);
+
+        foreach ($dhDriver->getFlusherList() as $item) {
+            ($item)();
+        }
+
+        self::assertTrue(true);
+    }
+
+    public function testIssue184Unexpected(): void
+    {
+        $transactionManager = new class() implements TransactionManagerInterface {
+            public function populate($transaction): void {}
+
+            public function process($transaction): void
+            {
+                throw new RuntimeException('Unexpected call');
+            }
+        };
+        $objectManager = $this->createMock(EntityManagerInterface::class);
+
+        $args = new OnFlushEventArgs($objectManager);
+
+        $driver = new class() implements VersionAwarePlatformDriver {
+            public function connect(array $params): void {}
+
+            public function getDatabasePlatform(): void {}
+
+            public function getSchemaManager(ConnectionDbal $conn, AbstractPlatform $platform): void {}
+
+            public function getExceptionConverter(): Driver\API\ExceptionConverter
+            {
+                return new ExceptionConverter();
+            }
+
+            public function createDatabasePlatformForVersion($version): void {}
+        };
+
+        $objectManager
+            ->method('getConnection')
+            ->willReturn($connection = $this->createMock(ConnectionDbal::class))
+        ;
+
+        $connection
+            ->method('getDriver')
+            ->willReturn($driver)
+        ;
+
+        $connection
+            ->method('getConfiguration')
+            ->willReturn($configuration = $this->createMock(Configuration::class))
+        ;
+
+        $target = new DoctrineSubscriber($transactionManager);
+        $target->onFlush($args);
+
+        self::assertTrue(true);
+    }
+}
diff --git a/tests/Provider/Doctrine/Issues/Issue174Test.php b/tests/Provider/Doctrine/Issues/Issue174Test.php
@@ -14,9 +14,6 @@
  * @internal
  */
 #[Small]
-/**
- * @internal
- */
 final class Issue174Test extends TestCase
 {
     use DefaultSchemaSetupTrait;