Merge pull request #439 from jackalope/enhancement/test-against-dev-s…

…tability

Add support for doctrine/dbal 4 version

.github/workflows/static.yml

@@ -18,8 +18,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Pull in optional dependencies
+        # also lock dbal to version 3 for now, as we need to stay compatible with it
         run: |
-          composer require --no-update psr/simple-cache
+          composer require --no-update psr/simple-cache doctrine/dbal:^3.8
           composer update --no-dev --no-progress
 
       - name: PHPStan
@@ -38,7 +39,9 @@ jobs:
         uses: actions/checkout@v3
 
       - name: Install dependencies
+        # also lock dbal to version 3 for now, as we need to stay compatible with it
         run: |
+          composer require --no-update doctrine/dbal:^3.8
           composer update --no-progress
 
       - name: PHPStan

.github/workflows/test-application.yaml

@@ -43,7 +43,7 @@ jobs:
 
     services:
       mysql:
-        image: mysql:5.7
+        image: mysql:8.3
         env:
           MYSQL_ROOT_PASSWORD: root
           MYSQL_DATABASE: phpcr_tests

CHANGELOG.md

@@ -10,6 +10,9 @@ Changelog
 * [BC Break]: CachedClient now expects a PSR-16 cache rather than the abandoned doctrine/cache.
   When instantiating the client, you need to provide at least the cache instance for metadata, as CachedClient does not know which implementation to pick.
 * Support for new Symfony versions.
+* Support for doctrine/dbal 4.
+* For MySQL/MariaDB, it is now required to configure `collate` or `charset` in the Doctrine connection, or alternatively
+  set the encoding explicitly with `Client::setCaseSensitiveEncoding('')` (e.g. `utf8mb4_bin`).
 * If you are on PHP 8.0 and install Jackalope with `symfony/cache`, you need to restrict `psr/simple-cache` to `^1.0 || ^2.0` in your application because Symfony 5 does not declare a conflict with it, but fails at runtime.
 * Drop support for PHP 7.
 * Fixed: While it is allowed to call `Repository::login` with `null` credentials, there used to be an error. It now correctly works.

composer.json

@@ -19,7 +19,7 @@
         "ext-dom": "*",
         "ext-pdo": "*",
         "ext-xml": "*",
-        "doctrine/dbal": "^3.0",
+        "doctrine/dbal": "^3.8.1 || ^4.0",
         "phpcr/phpcr": "~2.1.5",
         "phpcr/phpcr-utils": "^1.8 || ^2.0",
         "jackalope/jackalope": "^2.0.0-RC1",

src/Jackalope/RepositoryFactoryDoctrineDBAL.php

@@ -77,7 +77,7 @@ class RepositoryFactoryDoctrineDBAL implements RepositoryFactoryInterface
      *
      * @api
      */
-    public function getRepository(array $parameters = null): RepositoryInterface
+    public function getRepository(?array $parameters = null): RepositoryInterface
     {
         if (null === $parameters) {
             throw new ConfigurationException('Jackalope-doctrine-dbal needs parameters');

src/Jackalope/Transport/DoctrineDBAL/CachedClient.php

@@ -63,7 +63,7 @@ public function setKeySanitizer(\Closure $sanitizer): void
     /**
      * @param array|null $caches which caches to invalidate, null means all except meta
      */
-    private function clearCaches(array $caches = null): void
+    private function clearCaches(?array $caches = null): void
     {
         $caches = $caches ?: ['nodes', 'query'];
         foreach ($caches as $cache) {

src/Jackalope/Transport/DoctrineDBAL/Client.php

@@ -2,11 +2,12 @@
 
 namespace Jackalope\Transport\DoctrineDBAL;
 
+use Doctrine\DBAL\ArrayParameterType;
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Driver\PDO\Connection as PDOConnection;
 use Doctrine\DBAL\Exception as DBALException;
 use Doctrine\DBAL\ParameterType;
-use Doctrine\DBAL\Platforms\MySQLPlatform;
+use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
@@ -256,7 +257,7 @@ private function generateUuid(): string
     /**
      * @throws NotImplementedException
      */
-    public function createWorkspace(string $name, string $srcWorkspace = null): void
+    public function createWorkspace(string $name, ?string $srcWorkspace = null): void
     {
         if (null !== $srcWorkspace) {
             throw new NotImplementedException('Creating workspace as clone of existing workspace not supported');
@@ -319,7 +320,7 @@ public function deleteWorkspace(string $name): void
         }
     }
 
-    public function login(CredentialsInterface $credentials = null, string $workspaceName = null): string
+    public function login(?CredentialsInterface $credentials = null, ?string $workspaceName = null): string
     {
         $this->credentials = $credentials;
 
@@ -363,7 +364,7 @@ public function setCheckLoginOnServer(bool $bool): void
     }
 
     /**
-     * This will control the collate which is being used on MySQL when querying nodes. It will be autodetected by just
+     * This will control the collation which is being used on MySQL when querying nodes. It will be autodetected by just
      * appending _bin to the current charset, which is good enough in most cases.
      */
     public function setCaseSensitiveEncoding(string $encoding): void
@@ -374,17 +375,20 @@ public function setCaseSensitiveEncoding(string $encoding): void
     /**
      * Returns the collate which is being used on MySQL when querying nodes.
      */
-    private function getCaseSensitiveEncoding(): string
+    private function getMySQLCaseSensitiveEncoding(): string
     {
         if (null !== $this->caseSensitiveEncoding) {
             return $this->caseSensitiveEncoding;
         }
 
         $params = $this->conn->getParams();
-        $charset = $params['charset'] ?? 'utf8';
         if (isset($params['defaultTableOptions']['collate'])) {
             return $this->caseSensitiveEncoding = $params['defaultTableOptions']['collate'];
         }
+        if (!array_key_exists('charset', $params)) {
+            throw new \InvalidArgumentException('For MySQL, the Doctrine dbal connection must have either "collate" or "charset" configured. Alternatively, you can set the encoding with '.__CLASS__.'::setCaseSensitiveEncoding().');
+        }
+        $charset = $params['charset'] ?? 'utf8';
 
         return $this->caseSensitiveEncoding = 'binary' === $charset ? $charset : $charset.'_bin';
     }
@@ -543,7 +547,7 @@ protected function setNamespaces($namespaces): void
      */
     private function executeChunkedUpdate(string $query, array $params): void
     {
-        $types = [Connection::PARAM_INT_ARRAY];
+        $types = [ArrayParameterType::INTEGER];
 
         if ($this->getConnection()->getDatabasePlatform() instanceof SqlitePlatform) {
             foreach (array_chunk($params, self::SQLITE_MAXIMUM_IN_PARAM_COUNT) as $chunk) {
@@ -554,7 +558,7 @@ private function executeChunkedUpdate(string $query, array $params): void
         }
     }
 
-    public function copyNode(string $srcAbsPath, string $destAbsPath, string $srcWorkspace = null): void
+    public function copyNode(string $srcAbsPath, string $destAbsPath, ?string $srcWorkspace = null): void
     {
         $this->assertLoggedIn();
 
@@ -921,11 +925,11 @@ private function syncReferences(array $referencesToUpdate): void
             if ($this->getConnection()->getDatabasePlatform() instanceof SqlitePlatform) {
                 $missingTargets = [];
                 foreach (array_chunk($params, self::SQLITE_MAXIMUM_IN_PARAM_COUNT) as $chunk) {
-                    $stmt = $this->getConnection()->executeQuery($query, [$chunk], [Connection::PARAM_INT_ARRAY]);
+                    $stmt = $this->getConnection()->executeQuery($query, [$chunk], [ArrayParameterType::INTEGER]);
                     $missingTargets = array_merge($missingTargets, array_column($stmt->fetchAllNumeric(), 0));
                 }
             } else {
-                $stmt = $this->getConnection()->executeQuery($query, [$params], [Connection::PARAM_INT_ARRAY]);
+                $stmt = $this->getConnection()->executeQuery($query, [$params], [ArrayParameterType::INTEGER]);
                 $missingTargets = array_column($stmt->fetchAllNumeric(), 0);
             }
             if ($missingTargets) {
@@ -1257,14 +1261,14 @@ private function getNodesData(array $rows): array
                 $childrenRows += $this->getConnection()->fetchAllAssociative(
                     $query,
                     [$chunk, $this->workspaceName],
-                    [Connection::PARAM_STR_ARRAY, null]
+                    [ArrayParameterType::STRING, null]
                 );
             }
         } else {
             $childrenRows = $this->getConnection()->fetchAllAssociative(
                 $query,
                 [$paths, $this->workspaceName],
-                [Connection::PARAM_STR_ARRAY, null]
+                [ArrayParameterType::STRING, null]
             );
         }
 
@@ -1360,7 +1364,7 @@ public function getNodes(array $paths): array
      * @param string      $path          Path of the node
      * @param string|null $workspaceName To overwrite the current workspace
      */
-    private function pathExists(string $path, string $workspaceName = null): bool
+    private function pathExists(string $path, ?string $workspaceName = null): bool
     {
         return (bool) $this->getSystemIdForNode($path, $workspaceName);
     }
@@ -1373,7 +1377,7 @@ private function pathExists(string $path, string $workspaceName = null): bool
      *
      * @return bool|string The database id
      */
-    private function getSystemIdForNode(string $identifier, string $workspaceName = null)
+    private function getSystemIdForNode(string $identifier, ?string $workspaceName = null)
     {
         if (null === $workspaceName) {
             $workspaceName = $this->workspaceName;
@@ -1383,8 +1387,8 @@ private function getSystemIdForNode(string $identifier, string $workspaceName =
             $query = 'SELECT id FROM phpcr_nodes WHERE identifier = ? AND workspace_name = ?';
         } else {
             $platform = $this->getConnection()->getDatabasePlatform();
-            if ($platform instanceof MySQLPlatform) {
-                $query = 'SELECT id FROM phpcr_nodes WHERE path COLLATE '.$this->getCaseSensitiveEncoding().' = ? AND workspace_name = ?';
+            if ($platform instanceof AbstractMySQLPlatform) {
+                $query = 'SELECT id FROM phpcr_nodes WHERE path COLLATE '.$this->getMySQLCaseSensitiveEncoding().' = ? AND workspace_name = ?';
             } else {
                 $query = 'SELECT id FROM phpcr_nodes WHERE path = ? AND workspace_name = ?';
             }
@@ -1428,14 +1432,14 @@ public function getNodesByIdentifier(array $identifiers): array
                 $all += $this->getConnection()->fetchAllAssociative(
                     $query,
                     [$this->workspaceName, $chunk],
-                    [ParameterType::STRING, Connection::PARAM_STR_ARRAY]
+                    [ParameterType::STRING, ArrayParameterType::STRING]
                 );
             }
         } else {
             $all = $this->getConnection()->fetchAllAssociative(
                 $query,
                 [$this->workspaceName, $identifiers],
-                [ParameterType::STRING, Connection::PARAM_STR_ARRAY]
+                [ParameterType::STRING, ArrayParameterType::STRING]
             );
         }
 
@@ -1457,7 +1461,7 @@ public function getNodesByIdentifier(array $identifiers): array
         return $nodes;
     }
 
-    public function getNodePathForIdentifier(string $uuid, string $workspace = null): string
+    public function getNodePathForIdentifier(string $uuid, ?string $workspace = null): string
     {
         if (null !== $workspace) {
             throw new NotImplementedException('Specifying the workspace is not yet supported.');
@@ -1679,10 +1683,20 @@ private function moveNode(string $srcAbsPath, string $destAbsPath): void
             throw new PathNotFoundException("Parent of the destination path '".$destAbsPath."' has to exist.");
         }
 
-        $query = 'SELECT path, id FROM phpcr_nodes WHERE path LIKE ? OR path = ? AND workspace_name = ? '.$this->getConnection()
-                ->getDatabasePlatform()->getForUpdateSQL()
+        $qb = $this->getConnection()->createQueryBuilder()
+            ->select('path, id')
+            ->from('phpcr_nodes')
+            ->where('(path LIKE :path_prefix OR path = :path) AND workspace_name = :workspace')
         ;
-        $stmt = $this->getConnection()->executeQuery($query, [$srcAbsPath.'/%', $srcAbsPath, $this->workspaceName]);
+        if (!$this->getConnection()->getDatabasePlatform() instanceof SqlitePlatform) {
+            $qb->forUpdate();
+        }
+
+        $stmt = $this->getConnection()->executeQuery($qb->getSQL(), [
+            'path_prefix' => $srcAbsPath.'/%',
+            'path' => $srcAbsPath,
+            'workspace' => $this->workspaceName,
+            ]);
 
         /*
          * TODO: https://github.com/jackalope/jackalope-doctrine-dbal/pull/26/files#L0R1057
@@ -1704,15 +1718,10 @@ private function moveNode(string $srcAbsPath, string $destAbsPath): void
 
         // TODO: Find a better way to do this
         // Calculate CAST type for CASE statement
-        switch ($this->getConnection()->getDatabasePlatform()->getName()) {
-            case 'pgsql':
-                $intType = 'integer';
-                break;
-            case 'mysql':
-                $intType = 'unsigned';
-                break;
-            default:
-                $intType = 'integer';
+
+        $intType = 'integer';
+        if ($this->getConnection()->getDatabasePlatform() instanceof AbstractMySQLPlatform) {
+            $intType = 'unsigned';
         }
 
         $i = 0;
@@ -2357,7 +2366,7 @@ public function unregisterNamespace(string $prefix): void
         }
     }
 
-    public function getReferences(string $path, string $name = null): array
+    public function getReferences(string $path, ?string $name = null): array
     {
         return $this->getNodeReferences($path, $name, false);
     }
@@ -2374,7 +2383,7 @@ public function getWeakReferences($path, $name = null): array
      *
      * @return string[] list of paths to nodes that reference $path
      */
-    private function getNodeReferences(string $path, string $name = null, bool $weakReference = false): array
+    private function getNodeReferences(string $path, ?string $name = null, bool $weakReference = false): array
     {
         $targetId = $this->getSystemIdForNode($path);
 

src/Jackalope/Transport/DoctrineDBAL/LoggingClient.php

@@ -80,7 +80,7 @@ public function registerNodeTypes($types, $allowUpdate): void
         $this->transport->registerNodeTypes($types, $allowUpdate);
     }
 
-    public function createWorkspace(string $name, string $srcWorkspace = null): void
+    public function createWorkspace(string $name, ?string $srcWorkspace = null): void
     {
         $this->transport->createWorkspace($name, $srcWorkspace);
     }

src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php

@@ -3,8 +3,8 @@
 namespace Jackalope\Transport\DoctrineDBAL\Query;
 
 use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
 use Doctrine\DBAL\Platforms\AbstractPlatform;
-use Doctrine\DBAL\Platforms\MySQLPlatform;
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
@@ -119,11 +119,12 @@ public function walkQOMQuery(QueryObjectModel $qom): array
         $offset = $qom->getOffset();
 
         if (null !== $offset && null === $limit
-            && ($this->platform instanceof MySQLPlatform || $this->platform instanceof SqlitePlatform)
+            && ($this->platform instanceof AbstractMySQLPlatform || $this->platform instanceof SqlitePlatform)
         ) {
             $limit = PHP_INT_MAX;
         }
-        $sql = $this->platform->modifyLimitQuery($sql, $limit, $offset);
+
+        $sql = $this->platform->modifyLimitQuery($sql, $limit, $offset ?: 0);
 
         return [$selectors, $this->alias, $sql];
     }
@@ -627,7 +628,7 @@ public function walkNumComparisonConstraint(QOM\PropertyValueInterface $property
         $alias = $this->getTableAlias($propertyOperand->getSelectorName().'.'.$propertyOperand->getPropertyName());
         $property = $propertyOperand->getPropertyName();
 
-        if ($this->platform instanceof MySQLPlatform && '=' === $operator) {
+        if ($this->platform instanceof AbstractMySQLPlatform && '=' === $operator) {
             return sprintf(
                 "0 != FIND_IN_SET('%s', REPLACE(EXTRACTVALUE(%s.props, '//sv:property[@sv:name=%s]/sv:value'), ' ', ','))",
                 $literalOperand->getLiteralValue(),
@@ -805,7 +806,7 @@ private function getLiteralValue(QOM\LiteralInterface $operand): string
      */
     private function sqlXpathValueExists(string $alias, string $property): string
     {
-        if ($this->platform instanceof MySQLPlatform) {
+        if ($this->platform instanceof AbstractMySQLPlatform) {
             return sprintf("EXTRACTVALUE(%s.props, 'count(//sv:property[@sv:name=%s]/sv:value[1])') = 1", $alias, Xpath::escape($property));
         }
 
@@ -825,7 +826,7 @@ private function sqlXpathValueExists(string $alias, string $property): string
      */
     private function sqlXpathExtractValue(string $alias, string $property, string $column = 'props'): string
     {
-        if ($this->platform instanceof MySQLPlatform) {
+        if ($this->platform instanceof AbstractMySQLPlatform) {
             return sprintf("EXTRACTVALUE(%s.%s, '//sv:property[@sv:name=%s]/sv:value[1]')", $alias, $column, Xpath::escape($property));
         }
 
@@ -851,7 +852,7 @@ private function sqlXpathExtractNumValue(string $alias, string $property): strin
 
     private function sqlXpathExtractValueAttribute(string $alias, string $property, string $attribute, int $valueIndex = 1): string
     {
-        if ($this->platform instanceof MySQLPlatform) {
+        if ($this->platform instanceof AbstractMySQLPlatform) {
             return sprintf("EXTRACTVALUE(%s.props, '//sv:property[@sv:name=%s]/sv:value[%d]/@%s')", $alias, Xpath::escape($property), $valueIndex, $attribute);
         }
 
@@ -874,7 +875,7 @@ private function sqlXpathComparePropertyValue(string $alias, string $property, s
     {
         $expression = null;
 
-        if ($this->platform instanceof MySQLPlatform) {
+        if ($this->platform instanceof AbstractMySQLPlatform) {
             $expression = sprintf("EXTRACTVALUE(%s.props, 'count(//sv:property[@sv:name=%s]/sv:value[text()%%s%%s]) > 0')", $alias, Xpath::escape($property));
             // mysql does not escape the backslashes for us, while postgres and sqlite do
             $value = Xpath::escapeBackslashes($value);