Use query builder and fix encoding autodetection default

* bump to dbal 3.8 minimum to avoid problems with 'forUpdate()'
* while we phpstan with dbal 3, use the old lowercase form for sqlite
* forUpdate is not platform aware, check ourselves
* make encoding required

.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

@@ -9,7 +9,7 @@ on:
       - '[0-9]+.[0-9]+.x'
 jobs:
   test:
-    name: 'PHP ${{ matrix.php-version }}, Database ${{ matrix.db }} ${{ matrix.dependencies }}, Composer ${{ matrix.composer-stability }}'
+    name: 'PHP ${{ matrix.php-version }}, Database ${{ matrix.db }} ${{ matrix.dependencies }}'
     runs-on: ubuntu-20.04
 
     env:
@@ -30,9 +30,6 @@ jobs:
           - sqlite
         dependencies:
           - highest
-        composer-stability:
-          - dev
-          - stable
         include:
           - php-version: '8.0'
             dependencies: lowest
@@ -76,10 +73,6 @@ jobs:
           extensions: "pdo, pdo_sqlite, pdo_mysql, mysql, pdo_pgsql"
           tools: 'composer:v2'
 
-      - name: Set composer stability
-        if: ${{ matrix.composer-stability }}
-        run: composer config minimum-stability ${{ matrix.composer-stability }}
-
       - name: PHP 8.0 simple cache
         # Symfony 5 is not compatible with SimpleCache 3 but does not declare a conflict. Symfony 6 can not be installed on PHP 8.0.
         if: ${{ '8.0' == matrix.php-version }}

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.6 || ^4.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

@@ -11,7 +11,6 @@
 use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
 use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
 use Doctrine\DBAL\Platforms\SqlitePlatform;
-use Doctrine\DBAL\Platforms\SQLServerPlatform;
 use Doctrine\DBAL\Statement;
 use Jackalope\FactoryInterface;
 use Jackalope\Node;
@@ -258,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');
@@ -321,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;
 
@@ -365,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
@@ -376,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';
     }
@@ -556,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();
 
@@ -1362,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);
     }
@@ -1375,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;
@@ -1386,7 +1388,7 @@ private function getSystemIdForNode(string $identifier, string $workspaceName =
         } else {
             $platform = $this->getConnection()->getDatabasePlatform();
             if ($platform instanceof AbstractMySQLPlatform) {
-                $query = 'SELECT id FROM phpcr_nodes WHERE path COLLATE '.$this->getCaseSensitiveEncoding().' = ? AND workspace_name = ?';
+                $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 = ?';
             }
@@ -1459,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.');
@@ -1681,15 +1683,20 @@ private function moveNode(string $srcAbsPath, string $destAbsPath): void
             throw new PathNotFoundException("Parent of the destination path '".$destAbsPath."' has to exist.");
         }
 
-        $forUpdateSql = ' FOR UPDATE'; // https://github.com/doctrine/dbal/blob/79ea9d6eda8e8e8705f2db58439e9934d8c769da/src/Platforms/AbstractPlatform.php#L1776
-        if ($this->getConnection()->getDatabasePlatform() instanceof SqlitePlatform) {
-            $forUpdateSql = ''; // https://github.com/doctrine/dbal/blob/79ea9d6eda8e8e8705f2db58439e9934d8c769da/src/Platforms/SqlitePlatform.php#L753
-        } elseif ($this->getConnection()->getDatabasePlatform() instanceof SQLServerPlatform) {
-            $forUpdateSql = ''; // https://github.com/doctrine/dbal/blob/79ea9d6eda8e8e8705f2db58439e9934d8c769da/src/Platforms/SQLServerPlatform.php#L1625
+        $qb = $this->getConnection()->createQueryBuilder()
+            ->select('path, id')
+            ->from('phpcr_nodes')
+            ->where('(path LIKE :path_prefix OR path = :path) AND workspace_name = :workspace')
+        ;
+        if (!$this->getConnection()->getDatabasePlatform() instanceof SqlitePlatform) {
+            $qb->forUpdate();
         }
 
-        $query = 'SELECT path, id FROM phpcr_nodes WHERE path LIKE ? OR path = ? AND workspace_name = ? '.$forUpdateSql;
-        $stmt = $this->getConnection()->executeQuery($query, [$srcAbsPath.'/%', $srcAbsPath, $this->workspaceName]);
+        $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
@@ -2359,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);
     }
@@ -2376,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/RepositorySchema.php

@@ -23,7 +23,7 @@ class RepositorySchema extends Schema
     /**
      * @param array $options the options could be use to make the table names configurable
      */
-    public function __construct(array $options = [], Connection $connection = null)
+    public function __construct(array $options = [], ?Connection $connection = null)
     {
         $this->connection = $connection;
         $schemaConfig = null;

src/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParser.php

@@ -38,7 +38,7 @@ final class XmlToPropsParser
     public function __construct(
         string $xml,
         ValueConverter $valueConverter,
-        array $propertyNames = null
+        ?array $propertyNames = null
     ) {
         $this->xml = $xml;
         $this->propertyNames = $propertyNames;

tests/Jackalope/Test/TestCase.php

@@ -21,14 +21,25 @@ protected function getConnection(): Connection
             return $this->conn;
         }
 
-        $this->conn = DriverManager::getConnection([
+        $params = [
             'driver' => @$GLOBALS['phpcr.doctrine.dbal.driver'],
             'path' => @$GLOBALS['phpcr.doctrine.dbal.path'],
             'host' => @$GLOBALS['phpcr.doctrine.dbal.host'],
+            'port' => @$GLOBALS['phpcr.doctrine.dbal.port'],
             'user' => @$GLOBALS['phpcr.doctrine.dbal.username'],
             'password' => @$GLOBALS['phpcr.doctrine.dbal.password'],
             'dbname' => @$GLOBALS['phpcr.doctrine.dbal.dbname'],
-        ]);
+        ];
+        if (array_key_exists('phpcr.doctrine.dbal.collate', $GLOBALS)) {
+            $params['defaultTableOptions'] = [
+                'collate' => $GLOBALS['phpcr.doctrine.dbal.collate'],
+            ];
+        }
+        if (array_key_exists('phpcr.doctrine.dbal.charset', $GLOBALS)) {
+            $params['charset'] = $GLOBALS['phpcr.doctrine.dbal.charset'];
+        }
+
+        $this->conn = DriverManager::getConnection($params);
 
         return $this->conn;
     }

tests/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParserTest.php

@@ -147,7 +147,7 @@ public function testParseEncoding(): void
         $this->assertSame('foo & bar&baz', $data->{'ampersand'});
     }
 
-    private function createXmlToPropsParser(string $xml, array $propNames = null): XmlToPropsParser
+    private function createXmlToPropsParser(string $xml, ?array $propNames = null): XmlToPropsParser
     {
         return new XmlToPropsParser(
             $xml,

tests/Jackalope/Transport/DoctrineDBAL/XmlPropsRemover/XmlPropsRemoverTest.php

@@ -75,15 +75,15 @@ public function testRemoveProps(): void
         ], $references);
     }
 
-    private function createXmlPropsRemover(string $xml, array $propNames = null): XmlPropsRemover
+    private function createXmlPropsRemover(string $xml, ?array $propNames = null): XmlPropsRemover
     {
         return new XmlPropsRemover(
             $xml,
             $propNames
         );
     }
 
-    private function createXmlToPropsParser(string $xml, array $propNames = null): XmlToPropsParser
+    private function createXmlToPropsParser(string $xml, ?array $propNames = null): XmlToPropsParser
     {
         $factory = new Factory();
 

tests/bootstrap.php

@@ -32,15 +32,24 @@
  * For further details, please see Doctrine configuration page.
  * http://docs.doctrine-project.org/projects/doctrine-dbal/en/latest/reference/configuration.html#connection-details.
  */
-$dbConn = DriverManager::getConnection([
+$params = [
     'driver' => @$GLOBALS['phpcr.doctrine.dbal.driver'],
     'path' => @$GLOBALS['phpcr.doctrine.dbal.path'],
     'host' => @$GLOBALS['phpcr.doctrine.dbal.host'],
     'port' => @$GLOBALS['phpcr.doctrine.dbal.port'],
     'user' => @$GLOBALS['phpcr.doctrine.dbal.username'],
     'password' => @$GLOBALS['phpcr.doctrine.dbal.password'],
     'dbname' => @$GLOBALS['phpcr.doctrine.dbal.dbname'],
-]);
+];
+if (array_key_exists('phpcr.doctrine.dbal.collate', $GLOBALS)) {
+    $params['defaultTableOptions'] = [
+        'collate' => $GLOBALS['phpcr.doctrine.dbal.collate'],
+    ];
+}
+if (array_key_exists('phpcr.doctrine.dbal.charset', $GLOBALS)) {
+    $params['charset'] = $GLOBALS['phpcr.doctrine.dbal.charset'];
+}
+$dbConn = DriverManager::getConnection($params);
 
 /* Recreate database schema */
 if (!getenv('JACKALOPE_NO_TEST_DB_INIT')) {

tests/generate_phpunit_config.php

@@ -1,7 +1,7 @@
 <?php
 
 /**
- * create database specific phpunit.xml config file for travis.
+ * create database specific phpunit.xml config file for CI.
  * e.g. sqlite.phpunit.xml.
  *
  * @author  cryptocompress <[email protected]>
@@ -14,6 +14,8 @@
         'phpcr.doctrine.dbal.username' => 'root',
         'phpcr.doctrine.dbal.password' => 'root',
         'phpcr.doctrine.dbal.dbname' => 'phpcr_tests',
+        'phpcr.doctrine.dbal.collate' => 'utf8mb4_bin',
+        'phpcr.doctrine.dbal.charset' => 'utf8mb4',
     ],
     'pgsql' => [
         'phpcr.doctrine.dbal.driver' => 'pdo_pgsql',
@@ -28,7 +30,7 @@
     ],
 ];
 
-if (!in_array(@$argv[1], array_keys($config))) {
+if (!array_key_exists(@$argv[1], $config)) {
     exit('Error:'."\n\t".'Database "'.@$argv[1].'" not supported.'."\n".
         'Usage:'."\n\t".'php tests/'.basename(__FILE__).' ['.implode('|', array_keys($config)).']'."\n");
 }