[FRAM-150] Migrate deprecated doctrine APIs (#88)

* chore: Migrate deprecate doctrine APIs (#FRAM-150)

* test: improve test coverage (#FRAM-150)

CHANGELOG

@@ -12,10 +12,16 @@ v2.2.0
     * `Whereable::orWhere()` on first parameter (column)
     * `Whereable::whereNull()`
     * `Whereable::whereNotNull()`
-
+* Feat: Add `Bdf\Prime\Platform\PlatformSpecificOperationInterface` and `PlatformInterface::apply()` to allow changing the behavior on the platform
+* Change: Rework visitors to remove the use of deprecated doctrine visitor API
+
 BC Breaks
 * Change: `Bdf\Prime\Configuration` instances are not shared anymore between connections. So change of the default configuration during runtime will not affect already created connections.
 * Change: `Bdf\Prime\Query/Expression/Raw::__toString()` is now deprecated, and its constructor value will be cast to string.
+* Minimum version of doctrine/dbal is now 3.7
+* Deprecate: `Bdf\Prime\Platform\PlatformInterface::name()`. Use `PlatformInterface::apply()` instead.
+* Deprecate: `Bdf\Prime\Connection\ConnectionInterface::getEventManager()` and `Bdf\Prime\Connection\Event\ConnectionClosedListenerInterface`. Use `ConnectionInterface::addConnectionClosedListener()` and `ConnectionInterface::removeConnectionClosedListener()` instead.
+* Change: `Doctrine\DBAL\Schema\Column::getPlatformOptions()`/`setPlatformOptions()` is used instead of `getCustomSchemaOptions()`/`setCustomSchemaOptions()`, which can cause change on migration queries.
 
 v2.1.0
 ------

composer.json

@@ -33,7 +33,7 @@
     "b2pweb/bdf-event-notifier": "~1.0",
     "b2pweb/bdf-serializer": "~1.0",
     "b2pweb/bdf-util": "~1.0",
-    "doctrine/dbal": "~3.0",
+    "doctrine/dbal": "~3.7",
     "doctrine/inflector": "~1.0|~2.0",
     "doctrine/instantiator": "^1.0.3|~2.0",
     "psr/container": "~1.0|~2.0",

src/Configuration.php

@@ -34,6 +34,7 @@ class Configuration extends BaseConfiguration
      * Set configuration
      *
      * @param array $options
+     * @psalm-suppress DeprecatedMethod
      */
     public function __construct(array $options = [])
     {

src/Connection/ConnectionInterface.php

@@ -16,12 +16,16 @@
 use Bdf\Prime\Schema\Manager\DatabaseManagerInterface;
 use Bdf\Prime\Schema\SchemaManagerInterface;
 use Bdf\Prime\Types\TypeInterface;
+use Closure;
 use Doctrine\Common\EventManager;
 
 /**
  * Base connection type
  *
  * Allows creating and executing queries, and handle a platform
+ *
+ * @method void addConnectionClosedListener(Closure $listener)
+ * @method void removeConnectionClosedListener(Closure $listener)
  */
 interface ConnectionInterface
 {
@@ -163,9 +167,29 @@ public function platform(): PlatformInterface;
      *       C'est actuellement le plus simple et léger, mais ajoute une dépendence forte à Doctrine
      *
      * @internal
+     * @deprecated Since 2.2. Will be removed in 3.0 without replacement
      */
     public function getEventManager();
 
+    /**
+     * Add a new listener to be notified when the connection is closed or reset
+     *
+     * @param Closure(ConnectionInterface):void $listener The listener to add. The connection instance is passed as argument.
+     *
+     * @return void
+     */
+    //public function addConnectionClosedListener(Closure $listener): void;
+
+    /**
+     * Remove the connection closed listener
+     * If the listener is not registered, do nothing
+     *
+     * @param Closure(ConnectionInterface):void $listener The listener to remove. Should be the same instance as the added listener.
+     *
+     * @return void
+     */
+    //public function removeConnectionClosedListener(Closure $listener): void;
+
     /**
      * Closes the connection and trigger "onConnectionClosed" event
      *

src/Connection/Event/ConnectionClosedListenerInterface.php

@@ -2,8 +2,12 @@
 
 namespace Bdf\Prime\Connection\Event;
 
+use Bdf\Prime\Connection\ConnectionInterface;
+
 /**
  * Listener for closed connection
+ *
+ * @deprecated Since 2.2. Use {@see ConnectionInterface::addConnectionClosedListener()} instead.
  */
 interface ConnectionClosedListenerInterface
 {
@@ -13,6 +17,7 @@ interface ConnectionClosedListenerInterface
      * The connection is closed
      *
      * @return void
+     * @deprecated Since 2.2. Use {@see ConnectionInterface::addConnectionClosedListener()} instead.
      */
     public function onConnectionClosed();
 }

src/Connection/Result/ArrayResultSet.php

@@ -40,6 +40,8 @@ public function __construct($array = [], $flags = 0)
 
     /**
      * {@inheritdoc}
+     *
+     * @psalm-suppress DeprecatedConstant
      */
     public function fetchMode($mode, $options = null)
     {

src/Connection/Result/DoctrineResultSet.php

@@ -93,6 +93,8 @@ public function valid(): bool
 
     /**
      * {@inheritdoc}
+     *
+     * @psalm-suppress DeprecatedConstant
      */
     public function fetchMode($mode, $options = null)
     {

src/Connection/SimpleConnection.php

@@ -38,6 +38,8 @@
 use Doctrine\DBAL\Result;
 use Doctrine\DBAL\Statement;
 
+use function spl_object_id;
+
 /**
  * Connection
  *
@@ -72,6 +74,14 @@ class SimpleConnection extends BaseConnection implements ConnectionInterface, Tr
      */
     private $factory;
 
+    /**
+     * List of listeners to call when the connection is closed,
+     * indexed by the listener object id
+     *
+     * @var array<int, Closure(ConnectionInterface):void>
+     */
+    private array $onConnectionClosedListeners = [];
+
     /**
      * SimpleConnection constructor.
      *
@@ -170,6 +180,28 @@ public function platform(): PlatformInterface
         return $this->platform;
     }
 
+    /**
+     * {@inheritdoc}
+     *
+     * @param Closure(ConnectionInterface):void $listener
+     */
+    public function addConnectionClosedListener(Closure $listener): void
+    {
+        $id = spl_object_id($listener);
+
+        $this->onConnectionClosedListeners[$id] = $listener;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function removeConnectionClosedListener(Closure $listener): void
+    {
+        $id = spl_object_id($listener);
+
+        unset($this->onConnectionClosedListeners[$id]);
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -399,18 +431,29 @@ public function rollBack(): bool
 
     /**
      * {@inheritdoc}
+     *
+     * @psalm-suppress DeprecatedProperty
+     * @psalm-suppress DeprecatedClass
      */
     public function close(): void
     {
         parent::close();
 
+        // To remove in 3.0
         $this->_eventManager->dispatchEvent(ConnectionClosedListenerInterface::EVENT_NAME);
+
+        foreach ($this->onConnectionClosedListeners as $listener) {
+            $listener($this);
+        }
     }
 
     /**
      * Setup the logger by setting the connection
      *
      * @return void
+     * @psalm-suppress DeprecatedMethod
+     * @psalm-suppress DeprecatedClass
+     * @todo remove on prime 3.0
      */
     protected function prepareLogger(): void
     {

src/Console/GraphCommand.php

@@ -68,7 +68,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
             : $this->getSchemaFromModel($io, $io->argument('path'));
 
         $graph = new Graphviz();
-        $schema->visit($graph);
+        $graph->onSchema($schema);
 
         if (!$io->option('output')) {
             $io->line($graph->getOutput());

src/Console/MapperCommand.php

@@ -74,7 +74,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
         }
 
         $visitor = new MapperVisitor($connection->getName(), $this->locator->mappers()->getNameResolver());
-        $schema->visit($visitor);
+        $visitor->onSchema($schema);
 
         if (!$io->option('output')) {
             $io->line($visitor->getOutput());

src/IdGenerators/TableGenerator.php

@@ -5,7 +5,18 @@
 use Bdf\Prime\Connection\ConnectionInterface;
 use Bdf\Prime\Exception\PrimeException;
 use Bdf\Prime\Mapper\Metadata;
+use Bdf\Prime\Platform\Sql\SqlPlatform;
+use Bdf\Prime\Platform\Sql\SqlPlatformOperationInterface;
+use Bdf\Prime\Platform\Sql\SqlPlatformOperationTrait;
 use Bdf\Prime\ServiceLocator;
+use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
+use Doctrine\DBAL\Platforms\AbstractPlatform;
+use Doctrine\DBAL\Platforms\SqlitePlatform;
+
+use LogicException;
+
+use function get_class;
+use function method_exists;
 
 /**
  * Sequence table
@@ -45,24 +56,49 @@ protected function doGenerate($property, array &$data, ServiceLocator $serviceLo
      */
     protected function incrementSequence(ConnectionInterface $connection, Metadata $metadata)
     {
+        $table = $metadata->sequence['table'];
+        $column = $metadata->sequence['column'];
+
         $platform = $connection->platform();
 
-        switch ($platform->name()) {
-            case 'mysql':
-                $connection->executeUpdate('UPDATE '.$metadata->sequence['table']
-                    .' SET '. $metadata->sequence['column'].' = LAST_INSERT_ID('.$metadata->sequence['column'].'+1)');
-                return (string) $connection->lastInsertId();
-
-            case 'sqlite':
-                $connection->executeUpdate('UPDATE '.$metadata->sequence['table']
-                    .' SET '.$metadata->sequence['column'].' = '.$metadata->sequence['column'].'+1');
-                return (string) $connection->executeQuery('SELECT '.$metadata->sequence['column']
-                    .' FROM '.$metadata->sequence['table'])->fetchOne();
-
-            default:
-                return (string) $connection->executeQuery(
-                    $platform->grammar()->getSequenceNextValSQL($metadata->sequence['table'])
-                )->fetchOne();
+        if (!method_exists($platform, 'apply')) {
+            throw new LogicException('The platform ' . get_class($platform) . ' does not support the method apply().');
         }
+
+        return $platform->apply(new class ($connection, $table, $column) implements SqlPlatformOperationInterface {
+            use SqlPlatformOperationTrait;
+
+            /** @var \Bdf\Prime\Connection\ConnectionInterface&\Doctrine\DBAL\Connection */
+            private ConnectionInterface $connection;
+            private string $table;
+            private string $column;
+
+            /**
+             * @param \Bdf\Prime\Connection\ConnectionInterface&\Doctrine\DBAL\Connection $connection
+             */
+            public function __construct(ConnectionInterface $connection, string $table, string $column)
+            {
+                $this->connection = $connection;
+                $this->table = $table;
+                $this->column = $column;
+            }
+
+            public function onMysqlPlatform(SqlPlatform $platform, AbstractMySQLPlatform $grammar): string
+            {
+                $this->connection->executeStatement('UPDATE '.$this->table.' SET '. $this->column.' = LAST_INSERT_ID('.$this->column.'+1)');
+                return (string) $this->connection->lastInsertId();
+            }
+
+            public function onSqlitePlatform(SqlPlatform $platform, SqlitePlatform $grammar): string
+            {
+                $this->connection->executeStatement('UPDATE '.$this->table.' SET '.$this->column.' = '.$this->column.'+1');
+                return (string) $this->connection->executeQuery('SELECT '.$this->column.' FROM '.$this->table)->fetchOne();
+            }
+
+            public function onGenericSqlPlatform(SqlPlatform $platform, AbstractPlatform $grammar): string
+            {
+                return (string) $this->connection->executeQuery($grammar->getSequenceNextValSQL($this->table))->fetchOne();
+            }
+        });
     }
 }

src/Logger/PsrDecorator.php

@@ -13,6 +13,7 @@
  *
  * @deprecated Since Prime 2.2. Use middleware instead.
  * @see \Bdf\Prime\Connection\Middleware\LoggerMiddleware for the replacement
+ * @psalm-suppress DeprecatedInterface
  */
 class PsrDecorator implements SQLLogger, ConnectionAwareInterface
 {

src/Migration/Migration.php

@@ -211,7 +211,7 @@ public function query($sql, array $params = [], $connectionName = null): Result
      */
     public function update($sql, array $params = [], $connectionName = null)
     {
-        return $this->connection($connectionName)->executeUpdate($sql, $params);
+        return (int) $this->connection($connectionName)->executeStatement($sql, $params);
     }
 
     /**

src/Platform/PlatformInterface.php

@@ -6,13 +6,16 @@
 
 /**
  * Interface for Prime connection platforms
+ *
+ * @method mixed apply(PlatformSpecificOperationInterface $operation)
  */
 interface PlatformInterface
 {
     /**
      * Get the platform name
      *
      * @return string
+     * @deprecated Since 2.2. Use {@see PlatformInterface::apply()} to discriminate platform.
      */
     public function name(): string;
 
@@ -30,6 +33,20 @@ public function types(): PlatformTypesInterface;
      * Get the platform grammar instance
      *
      * @return AbstractPlatform
+     * @internal This method should not be used by end user. Use {@see PlatformInterface::apply()} instead.
      */
     public function grammar();
+
+    /**
+     * Apply the operation on the platform
+     *
+     * The platform will try to find the correct operation to apply.
+     * If the operation do not support the platform, {@see PlatformSpecificOperationInterface::onUnknownPlatform()} will be called.
+     *
+     * @param PlatformSpecificOperationInterface<R> $operation
+     * @return R The value returned by the operation
+     *
+     * @template R
+     */
+    //public function apply(PlatformSpecificOperationInterface $operation);
 }

src/Platform/PlatformSpecificOperationInterface.php

@@ -0,0 +1,27 @@
+<?php
+
+namespace Bdf\Prime\Platform;
+
+/**
+ * Type for an operation which depends on a specific platform
+ * Use this type when the operation is implemented differently on each platform (e.g. different SQL syntax between MySQL and SQLite)
+ *
+ * All methods of this type takes as first parameter the platform instance, and as second parameter the grammar instance,
+ * and return the result of the operation, of type "R" as template
+ *
+ * Note: do not use directly this interface, but use specific one for the target platform
+ *
+ * @template R
+ */
+interface PlatformSpecificOperationInterface
+{
+    /**
+     * Fallback method when the actual platform cannot be found, or is not supported by the operation
+     *
+     * @param PlatformInterface $platform
+     * @param object $grammar
+     *
+     * @return R
+     */
+    public function onUnknownPlatform(PlatformInterface $platform, object $grammar);
+}