Merge pull request #16 from ingenerator/update-doctrine-dbal

Update doctrine/dbal to v3

CHANGELOG.md

@@ -1,5 +1,23 @@
 ### Unreleased
 
+## v2.0.0 (2022-10-28)
+
+* [BREAKING] Update doctrine/dbal to v3
+  - This is a major release that introduces a number of breaking changes and deprecations - see the full upgrade notes
+    at https://github.com/doctrine/dbal/blob/3.5.x/UPGRADE.md
+
+  - Due to changes in the DBAL driver structure, `DoctrineFactory::getRawPDO` and the `doctrine.pdo_connection` service
+    now return an **actual** native PDO object, rather than a doctrine-specific wrapper/extension class. The major 
+    difference will be that any calls will now throw native PDOException rather than doctrine exceptions. This should
+    not affect actual calling code (which presumably expects a `PDOException` if it is calling methods on a `PDO`). 
+    However, things like loggers and global exception handlers will now potentially get two different classes of
+    database exceptions.
+
+  - The connection config returned when no database is configured (e.g. in an isolated unit-test environment) has 
+    changed. We still use our NullPDO fake database connection, but this is now wrapped in a new FakeMysqlDriver class
+    as doctrine have split the driver and underlying PDO classes. NullPDO is now hardcoded to report that it is 
+    connected to mysql 5.7.29.
+
 ## v1.4.0 (2022-10-14)
 
 * Support PHP 8.1 and PHP 8.2

README.md

@@ -47,9 +47,12 @@ return [
 
 Sometimes - e.g. for unit tests or running Doctrine build tooling - you might not have a database server actually 
 available. If you configure `'hostname' => NULL` we will use a `NullPDO` driver to allow Doctrine to bootstrap itself
-without failing on a database connection error. That connection is sufficent to run things like `orm:generate-proxies` 
-or `orm:validate-schema --skip-sync`. If your code does anything that attempts to actually make PDO calls we'll throw
-an exception.
+without failing on a database connection error. 
+
+That connection is sufficent to run things like `orm:generate-proxies` or `orm:validate-schema --skip-sync`. You should
+be aware that the NullPDO driver reports a mysql version of 5.7.29 - at time of writing doctrine/dbal does not vary any
+SQL or column definitions between mysql 5.7.x and 8.x, so it shouldn't matter if this version differs from your runtime
+mysql version. If your code does anything that attempts to actually make PDO calls we'll throw an exception.
 
 ## Configuring entities and options
 

composer.json

@@ -38,7 +38,7 @@
     "doctrine/annotations": "^1.13",
     "doctrine/cache": "^1",
     "doctrine/common": "^3.0",
-    "doctrine/dbal": "^2",
+    "doctrine/dbal": "^3.3",
     "doctrine/orm": "^2.5",
     "doctrine/persistence": "^2.3",
     "ingenerator/kohana-core": "^4.9",

src/DatabaseNotConfiguredException.php

@@ -11,4 +11,9 @@ public static function forMethod($method)
         return new static('The database connection is not configured - method `'.$method.'` failed');
     }
 
-}
+    public static function forGetAttribute(int $attribute)
+    {
+        return new static('The database connection is not configured - cannot getAttribute('.$attribute.')');
+    }
+
+}

src/Dependency/ConnectionConfigProvider.php

@@ -3,8 +3,6 @@
 namespace Ingenerator\KohanaDoctrine\Dependency;
 
 
-use Ingenerator\KohanaDoctrine\NullPDO;
-
 /**
  * Maps database config (in same structure as for legacy Kohana database components) to a doctrine config
  *
@@ -62,8 +60,8 @@ public function getConnection()
 
         if ($this->config['connection']['hostname'] === NULL) {
             return [
-                'driver'  => 'pdo_mysql',
-                'pdo'     => new NullPDO('pdo_mysql'),
+                'driverClass'  => \Ingenerator\KohanaDoctrine\FakeMysqlDriver::class,
+                //                'pdo'     => new NullPDO('pdo_mysql'),
                 'charset' => $this->config['charset'],
             ];
         }

src/Dependency/DoctrineFactory.php

@@ -6,14 +6,13 @@
 use Doctrine\Common\Annotations\AnnotationRegistry;
 use Doctrine\Common\Annotations\CachedReader;
 use Doctrine\Common\Annotations\SimpleAnnotationReader;
-use Doctrine\Common\Cache\ApcuCache;
 use Doctrine\Common\Cache\ArrayCache;
 use Doctrine\Common\Cache\Cache;
 use Doctrine\Common\EventManager;
-use Doctrine\Persistence\Mapping\Driver\MappingDriver;
 use Doctrine\DBAL\Types\Type;
 use Doctrine\ORM\Configuration;
 use Doctrine\ORM\EntityManager;
+use Doctrine\Persistence\Mapping\Driver\MappingDriver;
 use Ingenerator\KohanaDoctrine\ExplicitClasslistAnnotationDriver;
 
 class DoctrineFactory
@@ -271,7 +270,20 @@ public static function buildORMConfig(
      */
     public static function getRawPDO(EntityManager $entityManager)
     {
-        $driver = $entityManager->getConnection()->getWrappedConnection();
+        // NOTE: getNativeConnection() returns a raw PDO object, *not* a Doctrine extension of the PDO object
+        // as in DBAL < 3. That is because in DBAL >= 3, the doctrine connection object is a *proxy* to the
+        // native PDO, not an *extension* of it. Although the DBAL3 class is nominally API-compatible with
+        // the PDO interface (with a few tweaks) it obviously won't pass typehints where a dependency is expecting
+        // an instance of PDO.
+        //
+        // The implementation of the querying etc is much the same between the two DBAL versions, but the major 
+        // change is that the Doctrine proxy wraps PDO exceptions in a Doctrine\DBAL\Driver\Exception whereas
+        // using the native PDO will of course throw native PDOException.
+        //
+        // My hunch is it'll be easier to update our exception handling (if required) than to change typehints
+        // to couple things that currently know nothing about doctrine such as the MysqlSession handler from
+        // php-utils.
+        $driver = $entityManager->getConnection()->getNativeConnection();
         if ( ! $driver instanceof \PDO) {
             throw new \InvalidArgumentException(
                 'Expected Doctrine connection to be instance of '.\PDO::class.', got '.\get_class($driver)

src/FakeMysqlDriver.php

@@ -0,0 +1,22 @@
+<?php
+
+namespace Ingenerator\KohanaDoctrine;
+
+
+use Doctrine\DBAL\Driver;
+use Doctrine\DBAL\Driver\PDO\Connection;
+
+class FakeMysqlDriver extends Driver\AbstractMySQLDriver
+{
+
+    public function connect(array $params)
+    {
+        return new Connection(new NullPDO('pdo_mysql'));
+    }
+
+    public function getName()
+    {
+        return 'pdo_mysql';
+    }
+
+}

src/NullPDO.php

@@ -27,13 +27,16 @@ public function setAttribute($attribute, $value): bool
 	    return FALSE;
     }
 
-    public function getAttribute($attribute): mixed
+    public function getAttribute(int $attribute): mixed
     {
-	    if ($attribute == static::ATTR_DRIVER_NAME) {
-		    return $this->driver;
-	    }
+        return match ($attribute) {
+            static::ATTR_DRIVER_NAME => $this->driver,
 
-        throw DatabaseNotConfiguredException::forMethod(__METHOD__);
+            // The actual server version is not important, dbal just needs something (that it supports)
+            static::ATTR_SERVER_VERSION => '5.7.29',
+
+            default => throw DatabaseNotConfiguredException::forGetAttribute($attribute)
+        };
     }
 
     public function prepare($statement, $driver_options = []): \PDOStatement|false
@@ -91,4 +94,4 @@ public function quote($string, $parameter_type = PDO::PARAM_STR): string|false
         throw DatabaseNotConfiguredException::forMethod(__METHOD__);
     }
 
-}
+}

test/integration/Dependency/DoctrineFactoryTest.php

@@ -282,7 +282,7 @@ public function test_it_provides_the_raw_pdo_connection_from_doctrine()
         $this->assertInstanceOf(\PDO::class, $pdo);
         $em = $container->get('doctrine.entity_manager');
         /** @var EntityManager $em */
-        $this->assertSame($pdo, $em->getConnection()->getWrappedConnection());
+        $this->assertSame($pdo, $em->getConnection()->getNativeConnection());
     }
 
     public function test_it_attaches_event_manager_to_doctrine()

test/unit/Dependency/ConnectionConfigProviderTest.php

@@ -4,7 +4,7 @@
 
 
 use Ingenerator\KohanaDoctrine\Dependency\ConnectionConfigProvider;
-use Ingenerator\KohanaDoctrine\NullPDO;
+use Ingenerator\KohanaDoctrine\FakeMysqlDriver;
 use InvalidArgumentException;
 use PHPUnit\Framework\TestCase;
 use function array_intersect_key;
@@ -61,11 +61,10 @@ public function test_it_parses_config_structure_and_returns_null_pdo_if_no_host_
             ],
             'charset'    => 'cp1212',
         ];
-        $this->assertEquals(
+        $this->assertSame(
             [
-                'driver'  => 'pdo_mysql',
-                'pdo'     => new NullPDO('pdo_mysql'),
-                'charset' => 'cp1212',
+                'driverClass' => FakeMysqlDriver::class,
+                'charset'     => 'cp1212',
             ],
             $this->newSubject()->getConnection()
         );

test/unit/NullPDOTest.php

@@ -42,10 +42,17 @@ public function test_it_returns_driver_name_attribute()
         $this->assertSame('mysql', $this->newSubject()->getAttribute(\PDO::ATTR_DRIVER_NAME));
     }
 
+    public function test_it_returns_mysql_version_5_7()
+    {
+        // The actual server version is somewhat unimportant, doctrine/dbal doesn't differentiate between 5.x and 8.x
+        // in any meaningful way at this point. We just need to return a version.
+        $this->assertSame('5.7.29', $this->newSubject()->getAttribute(\PDO::ATTR_SERVER_VERSION));
+    }
+
     public function test_it_throws_on_access_to_unexpected_get_attribute_call()
     {
         $this->expectException(DatabaseNotConfiguredException::class);
-        $this->newSubject()->getAttribute(\PDO::ATTR_SERVER_VERSION);
+        $this->newSubject()->getAttribute(\PDO::ATTR_CONNECTION_STATUS);
     }
 
     public function provider_throwing_method_calls()