Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for doctrine/dbal 4 version #436

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .github/workflows/test-application.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -30,6 +30,9 @@ jobs:
- sqlite
dependencies:
- highest
composer-stability:
- dev
- stable
include:
- php-version: '8.0'
dependencies: lowest
Expand All @@ -43,7 +46,7 @@ jobs:

services:
mysql:
image: mysql:5.7
image: mysql:8.3
env:
MYSQL_ROOT_PASSWORD: root
MYSQL_DATABASE: phpcr_tests
Expand Down Expand Up @@ -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 }}
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
44 changes: 23 additions & 21 deletions src/Jackalope/Transport/DoctrineDBAL/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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]
);
}

Expand Down Expand Up @@ -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 = ?';
Expand Down Expand Up @@ -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]
);
}

Expand Down Expand Up @@ -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;
Comment on lines +1684 to +1691
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried leveraging the query builder for adding lock hints?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not familiar with that. @dbu how would you change this part?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i am also not familiar with the dbal query builder. did doctrine drop the getForUpdateSQL method? does the changelog not explain what to do instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$stmt = $this->getConnection()->executeQuery($query, [$srcAbsPath.'/%', $srcAbsPath, $this->workspaceName]);

/*
Expand All @@ -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;
Expand Down
17 changes: 9 additions & 8 deletions src/Jackalope/Transport/DoctrineDBAL/Query/QOMWalker.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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];
}
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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));
}

Expand All @@ -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));
}

Expand All @@ -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);
}

Expand All @@ -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);
Expand Down
26 changes: 13 additions & 13 deletions src/Jackalope/Transport/DoctrineDBAL/RepositorySchema.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down Expand Up @@ -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']);
}

Expand Down Expand Up @@ -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'));
}
Expand All @@ -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']);
}
}

Expand All @@ -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']);
}
Expand All @@ -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']);
}

Expand All @@ -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']);
}

Expand All @@ -226,15 +226,15 @@ 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;
}
}

if ($currentMaxLength && (
null === $this->maxIndexLength
255 === $this->maxIndexLength
|| $currentMaxLength < $this->maxIndexLength
)) {
return $currentMaxLength;
Expand Down
11 changes: 9 additions & 2 deletions tests/Jackalope/Tools/Console/InitDoctrineDbalCommandTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -35,6 +36,11 @@ class InitDoctrineDbalCommandTest extends TestCase
*/
protected $schemaManager;

/**
* @var SchemaConfig
*/
protected $schemaConfig;

/**
* @var Application
*/
Expand All @@ -48,6 +54,7 @@ public function setUp(): void
->willReturn([])
;
$this->schemaManager = $this->createMock(AbstractSchemaManager::class);
$this->schemaConfig = new SchemaConfig();

$this->platform = new MySQLPlatform();

Expand All @@ -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([
Expand Down
Loading