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.

Sign up for GitHub

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

DBAL v4, Middleware, Driver, Platform, and SchemaManager implementation #61

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

DBAL v4, Middleware, Driver, Platform, and SchemaManager implementation #61

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
c0a309b
Update DBAL to a minimum of 3.2
GwendolenLynch Jul 1, 2023
dba5510
DBAL middleware, driver, and extended platform
GwendolenLynch Jul 1, 2023
69f1e8f
Migrate SchemaManager to extend DBAL's
GwendolenLynch Jul 1, 2023
95bebb4
Remove DBAL event subscriber & change ORM to listener
GwendolenLynch Jul 1, 2023
cd6e746
Add @covers to generated Functions
GwendolenLynch Jul 1, 2023
69903ed
Update existing tests and add new tests cover added classes
GwendolenLynch Jul 1, 2023
73b7f7a
Update docs
GwendolenLynch Jul 1, 2023
e795067
Updates for DBAL v4 deprecations
GwendolenLynch Mar 7, 2024
4ec73b9
Switch functions from Lexer constants to TokenType enum
GwendolenLynch Mar 7, 2024
7fb9091
Update minimum required versions
GwendolenLynch Mar 7, 2024
07e9d7a
Update psalm baseline
GwendolenLynch Mar 7, 2024
1dea92f
Check platform option exists before use
GwendolenLynch Mar 8, 2024
c2b86ff
Update docs
GwendolenLynch Apr 23, 2024
9a88d1f
Raise Docker PHP version to 8.2
GwendolenLynch Apr 23, 2024
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
16 changes: 14 additions & 2 deletions src/Driver/Driver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@
namespace Jsor\Doctrine\PostGIS\Driver;

use Doctrine\DBAL;
use Doctrine\DBAL\Connection\StaticServerVersionProvider;
use Doctrine\DBAL\Driver\AbstractPostgreSQLDriver;
use Doctrine\DBAL\Driver\API\ExceptionConverter;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\DBAL\ServerVersionProvider;
use Doctrine\DBAL\Types\Type;
use Jsor\Doctrine\PostGIS\Types\GeographyType;
use Jsor\Doctrine\PostGIS\Types\GeometryType;
use Jsor\Doctrine\PostGIS\Types\PostGISType;
use Jsor\Doctrine\PostGIS\Utils\Doctrine;

final class Driver extends AbstractPostgreSQLDriver
{
Expand All @@ -36,14 +40,22 @@ public function connect(array $params): DBAL\Driver\Connection
return $connection;
}

public function getDatabasePlatform(): AbstractPlatform
public function getDatabasePlatform(?ServerVersionProvider $versionProvider = null): PostgreSQLPlatform
{
return new PostGISPlatform();
}

/**
* @param string $version
*/
public function createDatabasePlatformForVersion($version): AbstractPlatform
{
return $this->getDatabasePlatform();
// Remove when DBAL v3 support is dropped.
if (Doctrine::isV3()) {
return $this->getDatabasePlatform();
}

return $this->getDatabasePlatform(new StaticServerVersionProvider($version));
}

public function getExceptionConverter(): ExceptionConverter
Expand Down
57 changes: 22 additions & 35 deletions src/Driver/PostGISPlatform.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
use Doctrine\DBAL\Schema\ColumnDiff;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\PostgreSQLSchemaManager;
use Doctrine\DBAL\Schema\SchemaDiff;
use Doctrine\DBAL\Schema\Table;
Expand All @@ -28,31 +27,21 @@ public function createSchemaManager(Connection $connection): PostgreSQLSchemaMan

public function getAlterSchemaSQL(SchemaDiff $diff): array
{
$spatialIndexes = [];
$sql = parent::getAlterSchemaSQL(SpatialIndexes::filterSchemaDiff($diff));

$spatialIndexSqlGenerator = new SpatialIndexSqlGenerator($this);

foreach ($diff->getAlteredTables() as $tableDiff) {
$table = $tableDiff->getOldTable();
if (!$table) {
continue;
}

/** @var Index[] $indices */
$indices = [];
foreach (SpatialIndexes::ensureTableDiffFlag($tableDiff) as $index) {
$indices[] = $index;
}
$spatialIndexes[$table->getName()] = ['table' => $table, 'indexes' => $indices];

SpatialIndexes::filterTableDiff($tableDiff);
}
SpatialIndexes::ensureSpatialIndexFlags($tableDiff);

$sql = parent::getAlterSchemaSQL($diff);
foreach (SpatialIndexes::extractSpatialIndicies($tableDiff->getAddedIndexes()) as $index) {
$sql[] = $spatialIndexSqlGenerator->getSql($index, $table);
}

$spatialIndexSqlGenerator = new SpatialIndexSqlGenerator($this);
foreach ($spatialIndexes as $spatialIndex) {
/** @var Table $table */
$table = $spatialIndex['table'];
/** @var Index $index */
foreach ($spatialIndex['indexes'] as $index) {
foreach (SpatialIndexes::extractSpatialIndicies($tableDiff->getModifiedIndexes()) as $index) {
$sql[] = $this->getDropIndexSQL($index->getName(), $table->getName());
$sql[] = $spatialIndexSqlGenerator->getSql($index, $table);
}
}
Expand All @@ -62,8 +51,9 @@ public function getAlterSchemaSQL(SchemaDiff $diff): array

public function getCreateTableSQL(Table $table, $createFlags = self::CREATE_INDEXES): array
{
$spatialIndexes = SpatialIndexes::ensureTableFlag($table);
SpatialIndexes::ensureSpatialIndexFlags($table);

$spatialIndexes = SpatialIndexes::extractSpatialIndicies($table->getIndexes());
foreach ($spatialIndexes as $index) {
$table->dropIndex($index->getName());
}
Expand All @@ -85,8 +75,9 @@ public function getCreateTablesSQL(array $tables): array

/** @var Table $table */
foreach ($tables as $table) {
$spatialIndexes = SpatialIndexes::ensureTableFlag($table);
SpatialIndexes::ensureSpatialIndexFlags($table);

$spatialIndexes = SpatialIndexes::extractSpatialIndicies($table->getIndexes());
foreach ($spatialIndexes as $index) {
$table->dropIndex($index->getName());
}
Expand Down Expand Up @@ -116,28 +107,24 @@ public function getCreateTablesSQL(array $tables): array
public function getAlterTableSQL(TableDiff $diff): array
{
$table = $diff->getOldTable();
$spatialIndexes = [];
$spatialIndexSqlGenerator = new SpatialIndexSqlGenerator($this);

if ($table) {
$spatialIndexes = SpatialIndexes::ensureTableDiffFlag($diff);
}

SpatialIndexes::filterTableDiff($diff);
SpatialIndexes::ensureSpatialIndexFlags($diff);

$sql = parent::getAlterTableSQL($diff);
$sql = parent::getAlterTableSQL(SpatialIndexes::filterTableDiff($diff));

if (!$table) {
return $sql;
foreach (SpatialIndexes::extractSpatialIndicies($diff->getAddedIndexes()) as $spatialIndex) {
$sql[] = $spatialIndexSqlGenerator->getSql($spatialIndex, $table);
}

foreach ($spatialIndexes as $spatialIndex) {
$sql[] = $spatialIndexSqlGenerator->getSql($spatialIndex, $table);
foreach (SpatialIndexes::extractSpatialIndicies($diff->getModifiedIndexes()) as $index) {
$sql[] = $this->getDropIndexSQL($index->getName(), $table->getName());
$sql[] = $spatialIndexSqlGenerator->getSql($index, $table);
}

/** @var ColumnDiff $columnDiff */
foreach ($diff->getModifiedColumns() as $columnDiff) {
if ($columnDiff->hasChanged('srid')) {
if ($columnDiff->getOldColumn()->getPlatformOption('srid') !== $columnDiff->getNewColumn()->getPlatformOption('srid')) {
Copy link

Choose a reason for hiding this comment

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

This throws an exception when the 'srid' option is not defined (example: the column is not one from PosGis)

I would add something like this before the check:

            if( ! $columnDiff->getNewColumn()->hasPlatformOption('srid')) {
                continue;
            }

Copy link

Choose a reason for hiding this comment

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

The error is thrown when running bin/console doctrine:schema:validate -v

Copy link
Author

Choose a reason for hiding this comment

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

Thanks so much for the testing! Fix pushed.

Copy link

Choose a reason for hiding this comment

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

Thank you for the PR! 👍

Unfortunately DBAL v4 is not (yet) compatible with Symfony, thus my tests will be limited to v3 only from now on

$sql[] = sprintf(
"SELECT UpdateGeometrySRID('%s', '%s', %d)",
$table->getName(),
Expand Down
55 changes: 41 additions & 14 deletions src/Schema/SchemaManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@

namespace Jsor\Doctrine\PostGIS\Schema;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Schema\Column;
use Doctrine\DBAL\Schema\Index;
use Doctrine\DBAL\Schema\PostgreSQLSchemaManager;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Schema\TableDiff;
use Doctrine\DBAL\Types\Type;
use Jsor\Doctrine\PostGIS\Types\GeographyType;
use Jsor\Doctrine\PostGIS\Types\GeometryType;
use Jsor\Doctrine\PostGIS\Types\PostGISType;
Expand All @@ -18,9 +21,6 @@ final class SchemaManager extends PostgreSQLSchemaManager
public function alterTable(TableDiff $tableDiff): void
{
$oldTable = $tableDiff->getOldTable();
if (!$oldTable) {
return;
}

foreach ($tableDiff->getModifiedColumns() as $columnDiff) {
if (!$columnDiff->getNewColumn()->getType() instanceof PostGISType) {
Expand All @@ -31,11 +31,16 @@ public function alterTable(TableDiff $tableDiff): void
$oldColumn = $columnDiff->getOldColumn();

if ($columnDiff->hasTypeChanged()) {
throw new RuntimeException('The type of a spatial column cannot be changed (Requested changing type from "' . ($oldColumn?->getType()?->getName() ?? 'N/A') . '" to "' . $newColumn->getType()->getName() . '" for column "' . $newColumn->getName() . '" in table "' . $oldTable->getName() . '")');
$oldType = Type::lookupName($oldColumn->getType());
$newType = Type::lookupName($newColumn->getType());

throw new RuntimeException('The type of a spatial column cannot be changed (Requested changing type from "' . $oldType . '" to "' . $newType . '" for column "' . $newColumn->getName() . '" in table "' . $oldTable->getName() . '")');
}

if ($columnDiff->hasChanged('geometry_type')) {
throw new RuntimeException('The geometry_type of a spatial column cannot be changed (Requested changing type from "' . strtoupper((string) ($oldColumn?->getPlatformOption('geometry_type') ?? 'N/A')) . '" to "' . strtoupper((string) $newColumn->getPlatformOption('geometry_type')) . '" for column "' . $newColumn->getName() . '" in table "' . $oldTable->getName() . '")');
$oldGT = (string) ($oldColumn->hasPlatformOption('geometry_type') ? $oldColumn->getPlatformOption('geometry_type') : 'N/A');
$newGT = (string) ($newColumn->hasPlatformOption('geometry_type') ? $newColumn->getPlatformOption('geometry_type') : 'N/A');
if (strtolower($oldGT) !== strtolower($newGT)) {
throw new RuntimeException('The geometry_type of a spatial column cannot be changed (Requested changing type from "' . strtoupper($oldGT) . '" to "' . strtoupper($newGT) . '" for column "' . $newColumn->getName() . '" in table "' . $oldTable->getName() . '")');
}
}

Expand All @@ -46,11 +51,16 @@ public function introspectTable(string $name): Table
{
$table = parent::introspectTable($name);

SpatialIndexes::ensureTableFlag($table);
SpatialIndexes::ensureSpatialIndexFlags($table);

return $table;
}

/**
* @param string $table
*
* @return array<string, Index>
*/
public function listTableIndexes($table): array
{
$indexes = parent::listTableIndexes($table);
Expand Down Expand Up @@ -85,7 +95,7 @@ public function listSpatialIndexes(string $table): array
ORDER BY i.relname";

/** @var array<array{relname: string, indkey: string, inddef: string, oid: string}> $tableIndexes */
$tableIndexes = $this->_conn->fetchAllAssociative(
$tableIndexes = $this->getConnection()->fetchAllAssociative(
$sql,
[
$this->trimQuotes($table),
Expand All @@ -104,7 +114,7 @@ public function listSpatialIndexes(string $table): array
AND a.attnum IN (" . implode(',', explode(' ', $row['indkey'])) . ')
AND a.atttypid = t.oid';

$stmt = $this->_conn->executeQuery($sql);
$stmt = $this->getConnection()->executeQuery($sql);

/** @var array<array{attname: string, typname: string}> $indexColumns */
$indexColumns = $stmt->fetchAllAssociative();
Expand Down Expand Up @@ -138,7 +148,7 @@ public function getGeometrySpatialColumnInfo(string $table, string $column): ?ar
AND f_geometry_column = ?';

/** @var array{coord_dimension: string, srid: string|int|null, type: string}|null $row */
$row = $this->_conn->fetchAssociative(
$row = $this->getConnection()->fetchAssociative(
$sql,
[
$this->trimQuotes($table),
Expand All @@ -165,7 +175,7 @@ public function getGeographySpatialColumnInfo(string $table, string $column): ?a
AND f_geography_column = ?';

/** @var array{coord_dimension: string, srid: string|int|null, type: string}|null $row */
$row = $this->_conn->fetchAssociative(
$row = $this->getConnection()->fetchAssociative(
$sql,
[
$this->trimQuotes($table),
Expand All @@ -180,6 +190,13 @@ public function getGeographySpatialColumnInfo(string $table, string $column): ?a
return $this->buildSpatialColumnInfo($row);
}

/**
* @param string $table
* @param string $database
* @param array<int, array<string, mixed>> $tableColumns
*
* @return array<string, Column>
*/
protected function _getPortableTableColumnList($table, $database, $tableColumns): array
{
$columns = parent::_getPortableTableColumnList($table, $database, $tableColumns);
Expand All @@ -195,7 +212,7 @@ protected function _getPortableTableColumnDefinition($tableColumn): Column
{
$column = parent::_getPortableTableColumnDefinition($tableColumn);

if ($tableColumn['table_name'] ?? false) {
if (isset($tableColumn['table_name'])) {
$this->resolveSpatialColumnInfo($column, (string) $tableColumn['table_name']);
}

Expand All @@ -214,7 +231,7 @@ protected function resolveSpatialColumnInfo(Column $column, string $tableName):
default => null,
};

if (!$info) {
if ($info === null) {
return;
}

Expand All @@ -225,7 +242,7 @@ protected function resolveSpatialColumnInfo(Column $column, string $tableName):
}

$column
->setType(PostGISType::getType($column->getType()->getName()))
->setType(PostGISType::getType(Type::lookupName($column->getType())))
->setDefault($default)
->setPlatformOption('geometry_type', $info['type'])
->setPlatformOption('srid', $info['srid'])
Expand Down Expand Up @@ -263,4 +280,14 @@ private function trimQuotes(string $identifier): string
{
return str_replace(['`', '"', '[', ']'], '', $identifier);
}

private function getConnection(): Connection
{
// DBAL v3
if (property_exists($this, '_conn')) {
return $this->_conn;
}

return $this->connection;
}
}
Loading