From a5cbc141ebc45827bdc44aabd75510172ad4b36a Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 4 Nov 2025 23:17:12 -0500 Subject: [PATCH 1/4] 5.x Use cakephp/database Column as a base class Align migrations with cakephp/database where possible. I've introduced some soft deprecations for the properties that have different names in migrations currently. Longer term, I'd like the internal data objects to be aligned and the compatibility shim is only used when constructing columns from arrays. --- src/Db/Table/Column.php | 192 ++++++------------ .../TestCase/Db/Adapter/SqliteAdapterTest.php | 1 - 2 files changed, 63 insertions(+), 130 deletions(-) diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 42f3fc7ae..0ece76f88 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -10,8 +10,8 @@ use Cake\Core\Configure; use Cake\Database\Expression\QueryExpression; +use Cake\Database\Schema\Column as DatabaseColumn; use Cake\Database\Schema\TableSchemaInterface; -use Migrations\Db\Adapter\AdapterInterface; use Migrations\Db\Adapter\PostgresAdapter; use Migrations\Db\Literal; use RuntimeException; @@ -19,7 +19,7 @@ /** * This object is based loosely on: https://api.rubyonrails.org/classes/ActiveRecord/ConnectionAdapters/Table.html. */ -class Column +class Column extends DatabaseColumn { public const BIGINTEGER = TableSchemaInterface::TYPE_BIGINTEGER; public const SMALLINTEGER = TableSchemaInterface::TYPE_SMALLINTEGER; @@ -40,90 +40,33 @@ class Column public const BINARYUUID = TableSchemaInterface::TYPE_BINARY_UUID; public const NATIVEUUID = TableSchemaInterface::TYPE_NATIVE_UUID; /** MySQL-only column type */ - public const YEAR = AdapterInterface::PHINX_TYPE_YEAR; + public const YEAR = TableSchemaInterface::TYPE_YEAR; /** MySQL/Postgres-only column type */ public const JSON = TableSchemaInterface::TYPE_JSON; /** Postgres-only column type */ - public const CIDR = AdapterInterface::PHINX_TYPE_CIDR; + public const CIDR = TableSchemaInterface::TYPE_CIDR; /** Postgres-only column type */ - public const INET = AdapterInterface::PHINX_TYPE_INET; + public const INET = TableSchemaInterface::TYPE_INET; /** Postgres-only column type */ - public const MACADDR = AdapterInterface::PHINX_TYPE_MACADDR; + public const MACADDR = TableSchemaInterface::TYPE_MACADDR; /** Postgres-only column type */ - public const INTERVAL = AdapterInterface::PHINX_TYPE_INTERVAL; - - /** - * @var string - */ - protected string $name = ''; - - /** - * @var string|\Migrations\Db\Literal - */ - protected string|Literal $type; - - /** - * @var int|null - */ - protected ?int $limit = null; - - /** - * @var bool - */ - protected bool $null = true; - - /** - * @var mixed - */ - protected mixed $default = null; - - /** - * @var bool - */ - protected bool $identity = false; - - /** - * Postgres-only column option for identity (always|default) - * - * @var ?string - */ - protected ?string $generated = PostgresAdapter::GENERATED_BY_DEFAULT; + public const INTERVAL = TableSchemaInterface::TYPE_INTERVAL; /** * @var int|null */ protected ?int $seed = null; - /** - * @var int|null - */ - protected ?int $increment = null; - /** * @var int|null */ protected ?int $scale = null; - /** - * @var string|null - */ - protected ?string $after = null; - /** * @var string|null */ protected ?string $update = null; - /** - * @var string|null - */ - protected ?string $comment = null; - - /** - * @var bool - */ - protected bool $signed = true; - /** * @var bool */ @@ -139,16 +82,6 @@ class Column */ protected ?string $collation = null; - /** - * @var string|null - */ - protected ?string $encoding = null; - - /** - * @var int|null - */ - protected ?int $srid = null; - /** * @var array|null */ @@ -156,8 +89,45 @@ class Column /** * Column constructor - */ - public function __construct() + * + * @param string $name The name of the column. + * @param string $type The type of the column. + * @param bool|null $null Whether the column allows nulls. + * @param mixed $default The default value for the column. + * @param int|null $length The length of the column. + * @param bool $identity Whether the column is an identity column. + * @param string|null $generated Postgres-only generated option for identity columns (always|default). + * @param int|null $precision The precision for decimal columns. + * @param int|null $increment The increment for identity columns. + * @param string|null $after The column to add this column after. + * @param string|null $onUpdate The ON UPDATE function for the column. + * @param string|null $comment The comment for the column. + * @param bool|null $unsigned Whether the column is unsigned. + * @param string|null $collate The collation for the column. + * @param int|null $srid The SRID for spatial columns. + * @param string|null $encoding The character set encoding for the column. + * @param string|null $baseType The base type for the column. + * @return void + */ + public function __construct( + protected string $name = '', + protected string $type = '', + protected ?bool $null = null, + protected mixed $default = null, + protected ?int $length = null, + protected bool $identity = false, + protected ?string $generated = PostgresAdapter::GENERATED_BY_DEFAULT, + protected ?int $precision = null, + protected ?int $increment = null, + protected ?string $after = null, + protected ?string $onUpdate = null, + protected ?string $comment = null, + protected ?bool $unsigned = null, + protected ?string $collate = null, + protected ?int $srid = null, + protected ?string $encoding = null, + protected ?string $baseType = null, + ) { $this->null = (bool)Configure::read('Migrations.column_null_default'); } @@ -185,38 +155,16 @@ public function getName(): ?string return $this->name; } - /** - * Sets the column type. - * - * @param string|\Migrations\Db\Literal $type Column type - * @return $this - */ - public function setType(string|Literal $type) - { - $this->type = $type; - - return $this; - } - - /** - * Gets the column type. - * - * @return string|\Migrations\Db\Literal - */ - public function getType(): string|Literal - { - return $this->type; - } - /** * Sets the column limit. * * @param int|null $limit Limit * @return $this + * @deprecated 5.0 Use setLength() instead. */ public function setLimit(?int $limit) { - $this->limit = $limit; + $this->length = $limit; return $this; } @@ -225,10 +173,11 @@ public function setLimit(?int $limit) * Gets the column limit. * * @return int|null + * @deprecated 5.0 Use getLength() instead. */ public function getLimit(): ?int { - return $this->limit; + return $this->length; } /** @@ -412,10 +361,11 @@ public function setPrecision(?int $precision) * and the column could store value from -999.99 to 999.99. * * @return int|null + * @deprecated 5.0 Use getLength() instead. */ public function getPrecision(): ?int { - return $this->limit; + return $this->length; } /** @@ -539,10 +489,11 @@ public function getComment(): ?string * * @param bool $signed Signed * @return $this + * @deprecated 5.0 Use setUnsigned() instead. */ public function setSigned(bool $signed) { - $this->signed = $signed; + $this->unsigned = !$signed; return $this; } @@ -551,16 +502,18 @@ public function setSigned(bool $signed) * Gets whether field should be signed. * * @return bool + * @deprecated 5.0 Use getUnsigned() instead. */ public function getSigned(): bool { - return $this->signed; + return $this->unsigned === null ? true : !$this->unsigned; } /** * Should the column be signed? * * @return bool + * @deprecated 5.0 Use isUnsigned() instead. */ public function isSigned(): bool { @@ -655,10 +608,11 @@ public function getValues(): ?array * * @param string $collation Collation * @return $this + * @deprecated 5.0 Use setCollate() instead. */ public function setCollation(string $collation) { - $this->collation = $collation; + $this->collate = $collation; return $this; } @@ -667,10 +621,11 @@ public function setCollation(string $collation) * Gets the column collation. * * @return string|null + * @deprecated 5.0 Use getCollate() instead. */ public function getCollation(): ?string { - return $this->collation; + return $this->collate; } /** @@ -696,29 +651,6 @@ public function getEncoding(): ?string return $this->encoding; } - /** - * Sets the column SRID. - * - * @param int $srid SRID - * @return $this - */ - public function setSrid(int $srid) - { - $this->srid = $srid; - - return $this; - } - - /** - * Gets the column SRID. - * - * @return int|null - */ - public function getSrid(): ?int - { - return $this->srid; - } - /** * Gets all allowed options. Each option must have a corresponding `setFoo` method. * @@ -740,6 +672,7 @@ protected function getValidOptions(): array 'properties', 'values', 'collation', + 'collate', 'encoding', 'srid', 'seed', @@ -759,6 +692,7 @@ protected function getAliasedOptions(): array 'length' => 'limit', 'precision' => 'limit', 'autoIncrement' => 'identity', + 'collation' => 'collate', ]; } diff --git a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php index e3d70afd6..50ac96f3d 100644 --- a/tests/TestCase/Db/Adapter/SqliteAdapterTest.php +++ b/tests/TestCase/Db/Adapter/SqliteAdapterTest.php @@ -2690,7 +2690,6 @@ public static function provideColumnTypesForValidation() [SqliteAdapter::PHINX_TYPE_MACADDR, false], [SqliteAdapter::PHINX_TYPE_POINT, false], [SqliteAdapter::PHINX_TYPE_POLYGON, false], - [Literal::from('someType'), true], ['someType', false], ]; } From a8aab51b333ae03898bf6ae1e31f182f106bb9ee Mon Sep 17 00:00:00 2001 From: Mark Story Date: Tue, 4 Nov 2025 23:36:37 -0500 Subject: [PATCH 2/4] Make null parameter work better. --- src/Db/Table/Column.php | 5 ++--- tests/TestCase/Db/Table/ColumnTest.php | 16 ++++++++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Db/Table/Column.php b/src/Db/Table/Column.php index 0ece76f88..7d8733fe7 100644 --- a/src/Db/Table/Column.php +++ b/src/Db/Table/Column.php @@ -127,9 +127,8 @@ public function __construct( protected ?int $srid = null, protected ?string $encoding = null, protected ?string $baseType = null, - ) - { - $this->null = (bool)Configure::read('Migrations.column_null_default'); + ) { + $this->null = $null ?? (bool)Configure::read('Migrations.column_null_default'); } /** diff --git a/tests/TestCase/Db/Table/ColumnTest.php b/tests/TestCase/Db/Table/ColumnTest.php index b333ee7f3..9db0c746d 100644 --- a/tests/TestCase/Db/Table/ColumnTest.php +++ b/tests/TestCase/Db/Table/ColumnTest.php @@ -14,6 +14,22 @@ class ColumnTest extends TestCase { + public function testNullConstructorParameter() + { + $column = new Column(name: 'title'); + $this->assertTrue($column->isNull()); + + $column = new Column(name: 'title', null: true); + $this->assertTrue($column->isNull()); + + $column = new Column(name: 'title', null: false); + $this->assertFalse($column->isNull()); + + Configure::write('Migrations.column_null_default', true); + $column = new Column(name: 'title'); + $this->assertTrue($column->isNull()); + } + public function testSetOptionThrowsExceptionIfOptionIsNotString() { $column = new Column(); From 111985b7b3def4e847e537ebc2118e1b5bc2cccf Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 5 Nov 2025 14:02:14 -0500 Subject: [PATCH 3/4] Remove remaining support for Literal column types --- src/Db/Action/AddColumn.php | 4 ++-- src/Db/Action/ChangeColumn.php | 4 ++-- src/Db/Adapter/AbstractAdapter.php | 2 +- src/Db/Adapter/MysqlAdapter.php | 5 +---- 4 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/Db/Action/AddColumn.php b/src/Db/Action/AddColumn.php index f739b47d9..84f77fc3e 100644 --- a/src/Db/Action/AddColumn.php +++ b/src/Db/Action/AddColumn.php @@ -38,11 +38,11 @@ public function __construct(TableMetadata $table, Column $column) * * @param \Migrations\Db\Table\TableMetadata $table The table to add the column to * @param string $columnName The column name - * @param string|\Migrations\Db\Literal $type The column type + * @param string $type The column type * @param array $options The column options * @return self */ - public static function build(TableMetadata $table, string $columnName, string|Literal $type, array $options = []): self + public static function build(TableMetadata $table, string $columnName, string $type, array $options = []): self { $column = new Column(); $column->setName($columnName); diff --git a/src/Db/Action/ChangeColumn.php b/src/Db/Action/ChangeColumn.php index 035a837c8..6df9181c1 100644 --- a/src/Db/Action/ChangeColumn.php +++ b/src/Db/Action/ChangeColumn.php @@ -53,11 +53,11 @@ public function __construct(TableMetadata $table, string $columnName, Column $co * * @param \Migrations\Db\Table\TableMetadata $table The table to alter * @param string $columnName The name of the column to change - * @param string|\Migrations\Db\Literal $type The type of the column + * @param string $type The type of the column * @param array $options Additional options for the column * @return self */ - public static function build(TableMetadata $table, string $columnName, string|Literal $type, array $options = []): self + public static function build(TableMetadata $table, string $columnName, string $type, array $options = []): self { $column = new Column(); $column->setName($columnName); diff --git a/src/Db/Adapter/AbstractAdapter.php b/src/Db/Adapter/AbstractAdapter.php index c6a32e7ab..55f521b5a 100644 --- a/src/Db/Adapter/AbstractAdapter.php +++ b/src/Db/Adapter/AbstractAdapter.php @@ -391,7 +391,7 @@ public function getAdapterType(): string */ public function isValidColumnType(Column $column): bool { - return $column->getType() instanceof Literal || in_array($column->getType(), $this->getColumnTypes(), true); + return in_array($column->getType(), $this->getColumnTypes(), true); } /** diff --git a/src/Db/Adapter/MysqlAdapter.php b/src/Db/Adapter/MysqlAdapter.php index e15fad16e..3d8694942 100644 --- a/src/Db/Adapter/MysqlAdapter.php +++ b/src/Db/Adapter/MysqlAdapter.php @@ -642,10 +642,7 @@ static function ($value) { $extra = ' ' . implode(' ', $extras); if (($row['Default'] !== null)) { - $columnType = $targetColumn->getType(); - // Column::getType() can return string|Literal, but getDefaultValueDefinition expects string|null - $columnTypeName = is_string($columnType) ? $columnType : null; - $extra .= $this->getDefaultValueDefinition($row['Default'], $columnTypeName); + $extra .= $this->getDefaultValueDefinition($row['Default'], $targetColumn->getType()); } $definition = $row['Type'] . ' ' . $null . $extra . $comment; From cc675cdd9ce673ab5f2c1e1ad1fbed7cac38afe8 Mon Sep 17 00:00:00 2001 From: Mark Story Date: Wed, 5 Nov 2025 14:04:53 -0500 Subject: [PATCH 4/4] More pruning related to Literal types --- src/Db/Action/AddColumn.php | 1 - src/Db/Action/ChangeColumn.php | 1 - src/Db/Table.php | 10 ++++------ 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Db/Action/AddColumn.php b/src/Db/Action/AddColumn.php index 84f77fc3e..c4948740a 100644 --- a/src/Db/Action/AddColumn.php +++ b/src/Db/Action/AddColumn.php @@ -8,7 +8,6 @@ namespace Migrations\Db\Action; -use Migrations\Db\Literal; use Migrations\Db\Table\Column; use Migrations\Db\Table\TableMetadata; diff --git a/src/Db/Action/ChangeColumn.php b/src/Db/Action/ChangeColumn.php index 6df9181c1..267e30aa9 100644 --- a/src/Db/Action/ChangeColumn.php +++ b/src/Db/Action/ChangeColumn.php @@ -8,7 +8,6 @@ namespace Migrations\Db\Action; -use Migrations\Db\Literal; use Migrations\Db\Table\Column; use Migrations\Db\Table\TableMetadata; diff --git a/src/Db/Table.php b/src/Db/Table.php index 858163ac5..cc2d0d2b6 100644 --- a/src/Db/Table.php +++ b/src/Db/Table.php @@ -325,18 +325,16 @@ public function addPrimaryKey(string|array $columns) * Valid options can be: limit, default, null, precision or scale. * * @param string|\Migrations\Db\Table\Column $columnName Column Name - * @param string|\Migrations\Db\Literal|null $type Column Type + * @param string|null $type Column Type * @param array $options Column Options * @throws \InvalidArgumentException * @return $this */ - public function addColumn(string|Column $columnName, string|Literal|null $type = null, array $options = []) + public function addColumn(string|Column $columnName, ?string $type = null, array $options = []) { assert($columnName instanceof Column || $type !== null); if ($columnName instanceof Column) { $action = new AddColumn($this->table, $columnName); - } elseif ($type instanceof Literal) { - $action = AddColumn::build($this->table, $columnName, $type, $options); } else { $action = new AddColumn($this->table, $this->getAdapter()->getColumnForType($columnName, $type, $options)); } @@ -388,11 +386,11 @@ public function renameColumn(string $oldName, string $newName) * Change a table column type. * * @param string $columnName Column Name - * @param string|\Migrations\Db\Table\Column|\Migrations\Db\Literal $newColumnType New Column Type + * @param string|\Migrations\Db\Table\Column $newColumnType New Column Type * @param array $options Options * @return $this */ - public function changeColumn(string $columnName, string|Column|Literal $newColumnType, array $options = []) + public function changeColumn(string $columnName, string|Column $newColumnType, array $options = []) { if ($newColumnType instanceof Column) { $action = new ChangeColumn($this->table, $columnName, $newColumnType);