diff --git a/.github/workflows/static.yml b/.github/workflows/static.yml index a1953fc6..00386dd0 100644 --- a/.github/workflows/static.yml +++ b/.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 diff --git a/.github/workflows/test-application.yaml b/.github/workflows/test-application.yaml index eb638944..cb81a1fe 100644 --- a/.github/workflows/test-application.yaml +++ b/.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 }} diff --git a/CHANGELOG.md b/CHANGELOG.md index cd96957e..edc8e9b2 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/composer.json b/composer.json index c6fb0fea..b9b2a8f6 100644 --- a/composer.json +++ b/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", diff --git a/src/Jackalope/RepositoryFactoryDoctrineDBAL.php b/src/Jackalope/RepositoryFactoryDoctrineDBAL.php index 91048946..99c9dc07 100644 --- a/src/Jackalope/RepositoryFactoryDoctrineDBAL.php +++ b/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'); diff --git a/src/Jackalope/Transport/DoctrineDBAL/CachedClient.php b/src/Jackalope/Transport/DoctrineDBAL/CachedClient.php index aa01b04a..f168554c 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/CachedClient.php +++ b/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) { diff --git a/src/Jackalope/Transport/DoctrineDBAL/Client.php b/src/Jackalope/Transport/DoctrineDBAL/Client.php index fb12946c..cd3c55a6 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/Client.php +++ b/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); diff --git a/src/Jackalope/Transport/DoctrineDBAL/LoggingClient.php b/src/Jackalope/Transport/DoctrineDBAL/LoggingClient.php index 7bf6983b..39bf99c7 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/LoggingClient.php +++ b/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); } diff --git a/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php b/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php index 28e0e8bd..3c71f400 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php +++ b/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; diff --git a/src/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParser.php b/src/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParser.php index 3e86458e..390ee527 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParser.php +++ b/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; diff --git a/tests/Jackalope/Test/TestCase.php b/tests/Jackalope/Test/TestCase.php index 7f05e7e0..7e8d7339 100644 --- a/tests/Jackalope/Test/TestCase.php +++ b/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; } diff --git a/tests/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParserTest.php b/tests/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParserTest.php index 4f656caa..720af65f 100644 --- a/tests/Jackalope/Transport/DoctrineDBAL/XmlParser/XmlToPropsParserTest.php +++ b/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, diff --git a/tests/Jackalope/Transport/DoctrineDBAL/XmlPropsRemover/XmlPropsRemoverTest.php b/tests/Jackalope/Transport/DoctrineDBAL/XmlPropsRemover/XmlPropsRemoverTest.php index 7340fbf0..daebcc18 100644 --- a/tests/Jackalope/Transport/DoctrineDBAL/XmlPropsRemover/XmlPropsRemoverTest.php +++ b/tests/Jackalope/Transport/DoctrineDBAL/XmlPropsRemover/XmlPropsRemoverTest.php @@ -75,7 +75,7 @@ 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, @@ -83,7 +83,7 @@ private function createXmlPropsRemover(string $xml, array $propNames = null): Xm ); } - private function createXmlToPropsParser(string $xml, array $propNames = null): XmlToPropsParser + private function createXmlToPropsParser(string $xml, ?array $propNames = null): XmlToPropsParser { $factory = new Factory(); diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 07babcc9..261b7685 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -32,7 +32,7 @@ * 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'], @@ -40,7 +40,16 @@ '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')) { diff --git a/tests/generate_phpunit_config.php b/tests/generate_phpunit_config.php index 047eacd6..35865bc7 100644 --- a/tests/generate_phpunit_config.php +++ b/tests/generate_phpunit_config.php @@ -1,7 +1,7 @@ @@ -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"); }