From 19cee13563a8196858b8491fa7a0c6811689e0ae Mon Sep 17 00:00:00 2001 From: Alexander Schranz Date: Fri, 26 Jan 2024 13:24:19 +0100 Subject: [PATCH 1/2] Add support for doctrine/dbal 4 version * Update array string type and int type * Fix removed getForUpdateSQL * Fix modifyLimitQuery offset null * Remove unnecessary mocks * Fix defining the max length everywhere * Give existing schemaconfig in it * Upgrade Mysql from 5 to 8 * Use createSchemaManager always * Replace MysqlPlatform check with AbstractMySQLPlatform for MariaDB compatibility --- .github/workflows/test-application.yaml | 11 ++++- composer.json | 2 +- .../Transport/DoctrineDBAL/Client.php | 44 ++++++++++--------- .../DoctrineDBAL/Query/QOMWalker.php | 17 +++---- .../DoctrineDBAL/RepositorySchema.php | 26 +++++------ .../Console/InitDoctrineDbalCommandTest.php | 11 ++++- 6 files changed, 64 insertions(+), 47 deletions(-) diff --git a/.github/workflows/test-application.yaml b/.github/workflows/test-application.yaml index b326813f..eb638944 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 }}' + name: 'PHP ${{ matrix.php-version }}, Database ${{ matrix.db }} ${{ matrix.dependencies }}, Composer ${{ matrix.composer-stability }}' runs-on: ubuntu-20.04 env: @@ -30,6 +30,9 @@ jobs: - sqlite dependencies: - highest + composer-stability: + - dev + - stable include: - php-version: '8.0' dependencies: lowest @@ -43,7 +46,7 @@ jobs: services: mysql: - image: mysql:5.7 + image: mysql:8.3 env: MYSQL_ROOT_PASSWORD: root MYSQL_DATABASE: phpcr_tests @@ -73,6 +76,10 @@ 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/composer.json b/composer.json index 2af7b5ed..c6fb0fea 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "ext-dom": "*", "ext-pdo": "*", "ext-xml": "*", - "doctrine/dbal": "^3.0", + "doctrine/dbal": "^3.6 || ^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/Transport/DoctrineDBAL/Client.php b/src/Jackalope/Transport/DoctrineDBAL/Client.php index cc7299c1..fb12946c 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/Client.php +++ b/src/Jackalope/Transport/DoctrineDBAL/Client.php @@ -2,14 +2,16 @@ 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; +use Doctrine\DBAL\Platforms\SQLServerPlatform; use Doctrine\DBAL\Statement; use Jackalope\FactoryInterface; use Jackalope\Node; @@ -543,7 +545,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) { @@ -921,11 +923,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 +1259,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] ); } @@ -1383,7 +1385,7 @@ 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) { + if ($platform instanceof AbstractMySQLPlatform) { $query = 'SELECT id FROM phpcr_nodes WHERE path COLLATE '.$this->getCaseSensitiveEncoding().' = ? AND workspace_name = ?'; } else { $query = 'SELECT id FROM phpcr_nodes WHERE path = ? AND workspace_name = ?'; @@ -1428,14 +1430,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] ); } @@ -1679,9 +1681,14 @@ 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() - ; + $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 + } + + $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]); /* @@ -1704,15 +1711,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; diff --git a/src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php b/src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php index a8d3b334..4f29fe28 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php +++ b/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); diff --git a/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php b/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php index 009ee555..28e0e8bd 100644 --- a/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php +++ b/src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php @@ -28,7 +28,7 @@ public function __construct(array $options = [], Connection $connection = null) $this->connection = $connection; $schemaConfig = null; if ($connection) { - $schemaManager = method_exists($connection, 'createSchemaManager') ? $connection->createSchemaManager() : $connection->getSchemaManager(); + $schemaManager = $connection->createSchemaManager(); $schemaConfig = $schemaManager->createSchemaConfig(); } @@ -66,7 +66,7 @@ protected function addNamespacesTable(): void { $namespace = $this->createTable('phpcr_namespaces'); $namespace->addColumn('prefix', 'string', ['length' => $this->getMaxIndexLength()]); - $namespace->addColumn('uri', 'string'); + $namespace->addColumn('uri', 'string', ['length' => 255]); $namespace->setPrimaryKey(['prefix']); } @@ -136,7 +136,7 @@ protected function addNodesReferencesTable(Table $nodes): void $references->setPrimaryKey(['source_id', 'source_property_name', 'target_id']); $references->addIndex(['target_id']); if (!array_key_exists('disable_fk', $this->options) || !$this->options['disable_fk']) { - $references->addForeignKeyConstraint($nodes, ['source_id'], ['id'], ['onDelete' => 'CASCADE']); + $references->addForeignKeyConstraint($nodes->getName(), ['source_id'], ['id'], ['onDelete' => 'CASCADE']); // TODO: this should be reenabled on RDBMS with deferred FK support // $references->addForeignKeyConstraint($nodes, array('target_id'), array('id')); } @@ -151,8 +151,8 @@ protected function addNodesWeakreferencesTable(Table $nodes): void $weakreferences->setPrimaryKey(['source_id', 'source_property_name', 'target_id']); $weakreferences->addIndex(['target_id']); if (!array_key_exists('disable_fk', $this->options) || !$this->options['disable_fk']) { - $weakreferences->addForeignKeyConstraint($nodes, ['source_id'], ['id'], ['onDelete' => 'CASCADE']); - $weakreferences->addForeignKeyConstraint($nodes, ['target_id'], ['id'], ['onDelete' => 'CASCADE']); + $weakreferences->addForeignKeyConstraint($nodes->getName(), ['source_id'], ['id'], ['onDelete' => 'CASCADE']); + $weakreferences->addForeignKeyConstraint($nodes->getName(), ['target_id'], ['id'], ['onDelete' => 'CASCADE']); } } @@ -161,12 +161,12 @@ protected function addTypeNodesTable(): void $types = $this->createTable('phpcr_type_nodes'); $types->addColumn('node_type_id', 'integer', ['autoincrement' => true]); $types->addColumn('name', 'string', ['length' => $this->getMaxIndexLength()]); - $types->addColumn('supertypes', 'string'); + $types->addColumn('supertypes', 'string', ['length' => 255]); $types->addColumn('is_abstract', 'boolean'); $types->addColumn('is_mixin', 'boolean'); $types->addColumn('queryable', 'boolean'); $types->addColumn('orderable_child_nodes', 'boolean'); - $types->addColumn('primary_item', 'string', ['notnull' => false]); + $types->addColumn('primary_item', 'string', ['length' => 255, 'notnull' => false]); $types->setPrimaryKey(['node_type_id']); $types->addUniqueIndex(['name']); } @@ -185,7 +185,7 @@ protected function addTypePropsTable(): void $propTypes->addcolumn('query_orderable', 'boolean'); $propTypes->addColumn('required_type', 'integer'); $propTypes->addColumn('query_operators', 'integer'); // BITMASK - $propTypes->addColumn('default_value', 'string', ['notnull' => false]); + $propTypes->addColumn('default_value', 'string', ['length' => 255, 'notnull' => false]); $propTypes->setPrimaryKey(['node_type_id', 'name']); } @@ -194,13 +194,13 @@ protected function addTypeChildsTable(): void $childTypes = $this->createTable('phpcr_type_childs'); $childTypes->addColumn('id', 'integer', ['autoincrement' => true]); $childTypes->addColumn('node_type_id', 'integer'); - $childTypes->addColumn('name', 'string'); + $childTypes->addColumn('name', 'string', ['length' => 255]); $childTypes->addColumn('protected', 'boolean'); $childTypes->addColumn('auto_created', 'boolean'); $childTypes->addColumn('mandatory', 'boolean'); $childTypes->addColumn('on_parent_version', 'integer'); - $childTypes->addColumn('primary_types', 'string'); - $childTypes->addColumn('default_type', 'string', ['notnull' => false]); + $childTypes->addColumn('primary_types', 'string', ['length' => 255]); + $childTypes->addColumn('default_type', 'string', ['length' => 255, 'notnull' => false]); $childTypes->setPrimaryKey(['id']); } @@ -226,7 +226,7 @@ public function reset(): void private function getMaxIndexLength($currentMaxLength = null) { if (-1 === $this->maxIndexLength) { - $this->maxIndexLength = null; + $this->maxIndexLength = 255; if ($this->isConnectionCharsetUtf8mb4()) { $this->maxIndexLength = 191; @@ -234,7 +234,7 @@ private function getMaxIndexLength($currentMaxLength = null) } if ($currentMaxLength && ( - null === $this->maxIndexLength + 255 === $this->maxIndexLength || $currentMaxLength < $this->maxIndexLength )) { return $currentMaxLength; diff --git a/tests/Jackalope/Tools/Console/InitDoctrineDbalCommandTest.php b/tests/Jackalope/Tools/Console/InitDoctrineDbalCommandTest.php index 5f2c89ad..93faf678 100644 --- a/tests/Jackalope/Tools/Console/InitDoctrineDbalCommandTest.php +++ b/tests/Jackalope/Tools/Console/InitDoctrineDbalCommandTest.php @@ -6,6 +6,7 @@ use Doctrine\DBAL\Platforms\AbstractPlatform; use Doctrine\DBAL\Platforms\MySQLPlatform; use Doctrine\DBAL\Schema\AbstractSchemaManager; +use Doctrine\DBAL\Schema\SchemaConfig; use Jackalope\Tools\Console\Command\InitDoctrineDbalCommand; use Jackalope\Tools\Console\Helper\DoctrineDbalHelper; use PHPUnit\Framework\TestCase; @@ -35,6 +36,11 @@ class InitDoctrineDbalCommandTest extends TestCase */ protected $schemaManager; + /** + * @var SchemaConfig + */ + protected $schemaConfig; + /** * @var Application */ @@ -48,6 +54,7 @@ public function setUp(): void ->willReturn([]) ; $this->schemaManager = $this->createMock(AbstractSchemaManager::class); + $this->schemaConfig = new SchemaConfig(); $this->platform = new MySQLPlatform(); @@ -57,10 +64,10 @@ public function setUp(): void $this->schemaManager ->method('createSchemaConfig') - ->willReturn(null); + ->willReturn($this->schemaConfig); $this->connection - ->method('getSchemaManager') + ->method('createSchemaManager') ->willReturn($this->schemaManager); $this->helperSet = new HelperSet([ From 2319292746a5778223c9053b1619a148796ecc86 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Fri, 2 Feb 2024 16:08:11 +0100 Subject: [PATCH 2/2] 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 | 5 +- .github/workflows/test-application.yaml | 9 +--- CHANGELOG.md | 3 ++ composer.json | 2 +- .../RepositoryFactoryDoctrineDBAL.php | 2 +- .../Transport/DoctrineDBAL/CachedClient.php | 2 +- .../Transport/DoctrineDBAL/Client.php | 47 +++++++++++-------- .../Transport/DoctrineDBAL/LoggingClient.php | 2 +- .../DoctrineDBAL/RepositorySchema.php | 2 +- .../XmlParser/XmlToPropsParser.php | 2 +- tests/Jackalope/Test/TestCase.php | 15 +++++- .../XmlParser/XmlToPropsParserTest.php | 2 +- .../XmlPropsRemover/XmlPropsRemoverTest.php | 4 +- tests/bootstrap.php | 13 ++++- tests/generate_phpunit_config.php | 6 ++- 15 files changed, 72 insertions(+), 44 deletions(-) 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"); }