diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 7b8caea0..37a96bd5 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -930,42 +930,6 @@ parameters: count: 1 path: src/Database/Schema/Grammars/PostgresGrammar.php - - - message: '#^Access to an undefined property Illuminate\\Support\\Fluent\:\:\$autoIncrement\.$#' - identifier: property.notFound - count: 1 - path: src/Database/Schema/Grammars/SQLiteGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Support\\Fluent\:\:\$name\.$#' - identifier: property.notFound - count: 2 - path: src/Database/Schema/Grammars/SQLiteGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Support\\Fluent\:\:\$storedAs\.$#' - identifier: property.notFound - count: 1 - path: src/Database/Schema/Grammars/SQLiteGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Support\\Fluent\:\:\$storedAsJson\.$#' - identifier: property.notFound - count: 1 - path: src/Database/Schema/Grammars/SQLiteGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Support\\Fluent\:\:\$virtualAs\.$#' - identifier: property.notFound - count: 1 - path: src/Database/Schema/Grammars/SQLiteGrammar.php - - - - message: '#^Access to an undefined property Illuminate\\Support\\Fluent\:\:\$virtualAsJson\.$#' - identifier: property.notFound - count: 1 - path: src/Database/Schema/Grammars/SQLiteGrammar.php - - message: '#^Access to an undefined property Illuminate\\Database\\Schema\\ColumnDefinition\:\:\$name\.$#' identifier: property.notFound diff --git a/src/Database/Schema/Blueprint.php b/src/Database/Schema/Blueprint.php index 53bd12ee..000b01f1 100644 --- a/src/Database/Schema/Blueprint.php +++ b/src/Database/Schema/Blueprint.php @@ -24,4 +24,23 @@ public function dropColumnIfExists($columns) return !empty($columns) ? $this->dropColumn($columns) : $this; } + + /** + * Add the commands that are implied by the blueprint's state. + * + * Swaps in Winter's BlueprintState so that a `->change()` preserves the existing column's + * attributes (see {@see \Winter\Storm\Database\Schema\BlueprintState}). Laravel hard-codes its + * own state class when an alter command is present, so we re-seed with ours once the base + * implied commands - and therefore the base state - have been created. + * + * @return void + */ + protected function addImpliedCommands() + { + parent::addImpliedCommands(); + + if (!is_null($this->state)) { + $this->state = new BlueprintState($this, $this->connection); + } + } } diff --git a/src/Database/Schema/BlueprintState.php b/src/Database/Schema/BlueprintState.php new file mode 100644 index 00000000..09b999c9 --- /dev/null +++ b/src/Database/Schema/BlueprintState.php @@ -0,0 +1,77 @@ +change()` unless the new column definition explicitly overrides them. + * + * Laravel 11 replaces the column definition wholesale when a column is changed, dropping any + * attribute (nullable, default, collation, generated expression, ...) the migration did not + * re-specify. On engines that rebuild the table to apply a change (SQLite) this state drives the + * rebuild, so merging the previous attributes here restores the expected behaviour through Laravel's + * own single rebuild - without Storm maintaining a second, hand-rolled rebuild in the grammar. + */ +class BlueprintState extends BaseBlueprintState +{ + /** + * Attributes carried over from the existing column definition when the changed definition does + * not set them explicitly. + * + * This mirrors the standard column modifiers Laravel's grammars emit. Should a future Laravel + * version introduce a new preservable modifier, add it here - omitting one is never a + * regression (Laravel already drops it on change), only a missed preservation opportunity. + * + * @var string[] + */ + protected array $preservedAttributes = [ + 'nullable', + 'default', + 'collation', + 'comment', + 'virtualAs', + 'virtualAsJson', + 'storedAs', + 'storedAsJson', + ]; + + /** + * Update the blueprint's state, preserving existing column attributes when a column is changed. + * + * @param \Illuminate\Support\Fluent $command + * @return void + */ + public function update(Fluent $command) + { + if ($command['name'] === 'change' && $command['column'] instanceof Fluent) { + $this->preserveExistingColumnAttributes($command['column']); + } + + parent::update($command); + } + + /** + * Copy any preserved attribute from the current (pre-change) column definition onto the changed + * column definition when the migration did not set it explicitly. + * + * @param \Illuminate\Support\Fluent $column + * @return void + */ + protected function preserveExistingColumnAttributes(Fluent $column): void + { + foreach ($this->getColumns() as $existing) { + if ($existing['name'] !== $column['name']) { + continue; + } + + foreach ($this->preservedAttributes as $attribute) { + if (!isset($column[$attribute]) && isset($existing[$attribute])) { + $column[$attribute] = $existing[$attribute]; + } + } + + return; + } + } +} diff --git a/src/Database/Schema/Grammars/SQLiteGrammar.php b/src/Database/Schema/Grammars/SQLiteGrammar.php index 65d4ceb3..bee7067d 100755 --- a/src/Database/Schema/Grammars/SQLiteGrammar.php +++ b/src/Database/Schema/Grammars/SQLiteGrammar.php @@ -2,152 +2,16 @@ namespace Winter\Storm\Database\Schema\Grammars; -use Illuminate\Database\Query\Expression; -use Illuminate\Database\Schema\Blueprint; -use Illuminate\Database\Schema\ColumnDefinition; -use Illuminate\Database\Schema\ForeignKeyDefinition; -use Illuminate\Database\Schema\IndexDefinition; use Illuminate\Database\Schema\Grammars\SQLiteGrammar as BaseSQLiteGrammar; -use Illuminate\Support\Fluent; class SQLiteGrammar extends BaseSQLiteGrammar { /** - * Compile a change column command into a series of SQL statements. + * Format a value so that it can be used in "default" clauses. * - * Starting with Laravel 11, previous column attributes do not persist when changing a column. - * This restores Laravel previous behavior where existing column attributes are kept - * unless they get changed by the new Blueprint. - * - * @param \Illuminate\Database\Schema\Blueprint $blueprint - * @param \Illuminate\Support\Fluent $command - * @return array|string - * - * @throws \RuntimeException + * @param mixed $value + * @return string */ - public function compileChange(Blueprint $blueprint, Fluent $command) - { - $autoIncrementColumn = null; - $columnNames = []; - $schema = $this->connection->getSchemaBuilder(); - $table = $blueprint->getTable(); - - $changedColumns = collect($blueprint->getChangedColumns()); - $oldColumns = collect($schema->getColumns($table)); - - $columns = $oldColumns - ->map(function ($column) use ($blueprint, $changedColumns, &$columnNames, &$autoIncrementColumn, $oldColumns) { - $column = $changedColumns->first(fn ($col) => $col->name === $column['name'], $column); - - if (! $column instanceof Fluent) { - $isGenerated = ! is_null($column['generation']); - $column = new ColumnDefinition([ - 'change' => true, - 'name' => $column['name'], - 'type' => $column['type_name'], - 'nullable' => $column['nullable'], - 'default' => $column['default'] ? new Expression($column['default']) : null, - 'autoIncrement' => $column['auto_increment'], - 'collation' => $column['collation'], - 'comment' => $column['comment'], - 'virtualAs' => $isGenerated && $column['generation']['type'] === 'virtual' - ? $column['generation']['expression'] : null, - 'storedAs' => $isGenerated && $column['generation']['type'] === 'stored' - ? $column['generation']['expression'] : null, - ]); - } - - $name = $this->wrap($column); - $autoIncrementColumn = $column->autoIncrement ? $column->name : $autoIncrementColumn; - - if (is_null($column->virtualAs) && is_null($column->virtualAsJson) && - is_null($column->storedAs) && is_null($column->storedAsJson) - ) { - $columnNames[] = $name; - } - - $oldColumn = $oldColumns->where('name', $column->name)->first(); - if (!$oldColumn instanceof ColumnDefinition) { - $oldColumn = new ColumnDefinition($oldColumn); - } - $sql = $name.' '.$this->getType($column); - - foreach ($this->modifiers as $modifier) { - if (method_exists($this, $method = "modify{$modifier}")) { - $mod = strtolower($modifier); - $col = isset($oldColumn->{$mod}) && !isset($column->{$mod}) ? $oldColumn : $column; - $sql .= $this->{$method}($blueprint, $col); - } - } - return $sql; - })->all(); - - $foreignKeys = collect($schema->getForeignKeys($table))->map(fn ($foreignKey) => new ForeignKeyDefinition([ - 'columns' => $foreignKey['columns'], - 'on' => $foreignKey['foreign_table'], - 'references' => $foreignKey['foreign_columns'], - 'onUpdate' => $foreignKey['on_update'], - 'onDelete' => $foreignKey['on_delete'], - ]))->all(); - - [$primary, $indexes] = collect($schema->getIndexes($table))->map(fn ($index) => new IndexDefinition([ - 'name' => match (true) { - $index['primary'] => 'primary', - $index['unique'] => 'unique', - default => 'index', - }, - 'index' => $index['name'], - 'columns' => $index['columns'], - ]))->partition(fn ($index) => $index->name === 'primary'); - - $indexes = collect($indexes)->reject(fn ($index) => str_starts_with('sqlite_', $index->index))->map( - fn ($index) => $this->{'compile'.ucfirst($index->name)}($blueprint, $index) - )->all(); - - $tempTable = $this->wrapTable($blueprint, '__temp__'.$this->connection->getTablePrefix()); - $table = $this->wrapTable($blueprint); - $columnNames = implode(', ', $columnNames); - - $foreignKeyConstraintsEnabled = $this->connection->scalar($this->pragma('foreign_keys')); - - $sqlQuery = array_filter( - array_merge( - [ - $foreignKeyConstraintsEnabled ? $this->compileDisableForeignKeyConstraints() : null, - sprintf( - 'create table %s (%s%s%s)', - $tempTable, - implode(', ', $columns), - $this->addForeignKeys($foreignKeys), - $autoIncrementColumn ? '' : $this->addPrimaryKeys($primary->first()) - ), - sprintf( - 'insert into %s (%s) select %s from %s', - $tempTable, - $columnNames, - $columnNames, - $table - ), - sprintf( - 'drop table %s', - $table - ), - sprintf( - 'alter table %s rename to %s', - $tempTable, - $table - ), - ], - $indexes, - [ - $foreignKeyConstraintsEnabled ? $this->compileEnableForeignKeyConstraints() : null - ] - ) - ); - - return $sqlQuery; - } - public function getDefaultValue($value) { if (is_string($value)) { @@ -156,30 +20,4 @@ public function getDefaultValue($value) return parent::getDefaultValue($value); } - - /** - * Create the column definition for a tinyint type. - * - * SQLite's column introspection returns 'tinyint' as the type_name for boolean and tinyInteger - * columns. The base grammar only has typeTinyInteger, so we need this alias to avoid - * BadMethodCallException when compileChange rebuilds a table that contains boolean columns. - * - * @param \Illuminate\Support\Fluent $column - * @return string - */ - protected function typeTinyint(Fluent $column) - { - return 'integer'; - } - - /** - * Create the column definition for a varchar type. - * - * @param \Illuminate\Support\Fluent $column - * @return string - */ - protected function typeVarChar(Fluent $column) - { - return 'varchar'; - } } diff --git a/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php b/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php index 2426caa6..71a7b48f 100644 --- a/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php +++ b/tests/Database/Schema/Grammars/SQLiteSchemaGrammarTest.php @@ -2,9 +2,9 @@ namespace Winter\Storm\Tests\Database\Schema\Grammars; -use Illuminate\Database\Schema\Blueprint; use Illuminate\Database\Schema\SQLiteBuilder; use Illuminate\Database\SQLiteConnection; +use Winter\Storm\Database\Schema\Blueprint; use Winter\Storm\Database\Schema\Grammars\SQLiteGrammar; use Winter\Storm\Tests\GrammarTestCase; @@ -18,69 +18,127 @@ public function setUp(): void parent::setUp(); } - public function testNoInitialModifiersAddNullable() + /** + * Boot a real in-memory SQLite connection wired with Winter's schema grammar and Blueprint, then + * seed it with the provided table definition. Column changes on SQLite force a full table + * rebuild, so a real connection (not a mocked one) is required to exercise the introspection that + * drives it. + * + * @return array{0: \PDO, 1: \Illuminate\Database\SQLiteConnection, 2: \Illuminate\Database\Schema\SQLiteBuilder} + */ + protected function bootSqlite(string $createTableSql): array { - $initialBlueprint = $this->getBlueprint('users'); - $initialBlueprint->string('name'); + $pdo = new \PDO('sqlite::memory:'); + $pdo->exec($createTableSql); - $statements = $this->runBlueprint($initialBlueprint); - $this->assertSame('alter table "users" add column "name" varchar not null', $statements[0]); + $connection = new SQLiteConnection($pdo, ':memory:', ''); + $connection->setSchemaGrammar(new SQLiteGrammar($connection)); - $changedBlueprint = $this->getBlueprint('users'); - $changedBlueprint->string('name')->nullable()->change(); + $builder = $connection->getSchemaBuilder(); + $builder->blueprintResolver(fn ($conn, $table, $callback) => new Blueprint($conn, $table, $callback)); - $statements = $this->runBlueprint($changedBlueprint); - $this->assertStringContainsString('"name" varchar', $statements[0]); + return [$pdo, $connection, $builder]; } - public function testNullableInitialModifierAddDefault() + /** + * Return the PRAGMA table_info row for a single column. + * + * @return array{name: string, type: string, notnull: string, dflt_value: ?string, pk: string} + */ + protected function columnInfo(\PDO $pdo, string $table, string $column): array { - $initialBlueprint = $this->getBlueprint('users'); - $initialBlueprint->string('name')->nullable(); + foreach ($pdo->query('PRAGMA table_info("' . $table . '")') as $info) { + if ($info['name'] === $column) { + return $info; + } + } - $statements = $this->runBlueprint($initialBlueprint); - $this->assertSame('alter table "users" add column "name" varchar', $statements[0]); + $this->fail("Column [{$column}] was not found on table [{$table}]."); + } - $changedBlueprint = $this->getBlueprint('users'); - $changedBlueprint->string('name')->default('admin')->change(); + public function testChangeMakesColumnNullable(): void + { + [$pdo, , $builder] = $this->bootSqlite('CREATE TABLE users (id integer primary key, name varchar not null)'); - $statements = $this->runBlueprint($changedBlueprint); - $this->assertStringContainsString("varchar default 'admin'", $statements[0]); + $builder->table('users', fn (Blueprint $table) => $table->string('name')->nullable()->change()); + + $this->assertSame(0, (int) $this->columnInfo($pdo, 'users', 'name')['notnull']); } - public function testNullableInitialModifierAddDefaultNotNullable() + public function testChangeAddingDefaultPreservesExistingNullable(): void { - $initialBlueprint = $this->getBlueprint('users'); - $initialBlueprint->string('name')->nullable(); + [$pdo, , $builder] = $this->bootSqlite('CREATE TABLE users (id integer primary key, name varchar)'); - $statements = $this->runBlueprint($initialBlueprint); - $this->assertSame('alter table "users" add column "name" varchar', $statements[0]); + // Only a default is specified; the column's existing nullable state must be preserved. + $builder->table('users', fn (Blueprint $table) => $table->string('name')->default('admin')->change()); - $changedBlueprint = $this->getBlueprint('users'); - $changedBlueprint->string('name')->default('admin')->nullable(false)->change(); + $info = $this->columnInfo($pdo, 'users', 'name'); + $this->assertSame(0, (int) $info['notnull'], 'The column should remain nullable.'); + $this->assertSame("'admin'", $info['dflt_value']); + } - $statements = $this->runBlueprint($changedBlueprint); - $this->assertStringContainsString("\"name\" varchar not null default 'admin'", $statements[0]); + public function testChangeCanAddDefaultAndDropNullable(): void + { + [$pdo, , $builder] = $this->bootSqlite('CREATE TABLE users (id integer primary key, name varchar)'); + + $builder->table('users', fn (Blueprint $table) => $table->string('name')->default('admin')->nullable(false)->change()); + + $info = $this->columnInfo($pdo, 'users', 'name'); + $this->assertSame(1, (int) $info['notnull']); + $this->assertSame("'admin'", $info['dflt_value']); } - public function testTypeTinyintTypeIsValid(): void + public function testChangePreservesUnspecifiedAttributes(): void { - $pdo = new \PDO('sqlite::memory:'); - $pdo->exec('CREATE TABLE users (is_active tinyint not null, name varchar not null)'); + [$pdo, , $builder] = $this->bootSqlite("CREATE TABLE users (id integer primary key, name varchar default 'bob')"); - $connection = new SQLiteConnection($pdo, ':memory:', ''); - $grammar = new SQLiteGrammar($connection); - $connection->setSchemaGrammar($grammar); + // Change only the type; the existing default must survive (pre-Laravel 11 behaviour). + $builder->table('users', fn (Blueprint $table) => $table->text('name')->change()); + + $info = $this->columnInfo($pdo, 'users', 'name'); + $this->assertSame('text', strtolower($info['type'])); + $this->assertSame("'bob'", $info['dflt_value']); + } + + public function testChangePreservesTinyintTypeOfOtherColumns(): void + { + [$pdo, , $builder] = $this->bootSqlite('CREATE TABLE users (is_active tinyint not null, name varchar not null)'); + + // Changing an unrelated column rebuilds the whole table; is_active must keep its declared + // type verbatim rather than being re-derived to "integer". + $builder->table('users', fn (Blueprint $table) => $table->string('name')->nullable()->change()); + + $info = $this->columnInfo($pdo, 'users', 'is_active'); + $this->assertSame('tinyint', strtolower($info['type'])); + $this->assertSame(1, (int) $info['notnull']); + } + + public function testChangeOnTableWithDecimalAndBinaryColumnsIsFaithful(): void + { + [$pdo, , $builder] = $this->bootSqlite( + 'CREATE TABLE t (id integer primary key, amount numeric(9,2), payload blob, name varchar)' + ); + + // Prior to this fix the rebuild re-derived every column through the grammar, throwing + // "Method SQLiteGrammar::typeNumeric does not exist" for the decimal column. + $builder->table('t', fn (Blueprint $table) => $table->text('name')->change()); + + $this->assertSame('numeric(9,2)', strtolower($this->columnInfo($pdo, 't', 'amount')['type'])); + $this->assertSame('blob', strtolower($this->columnInfo($pdo, 't', 'payload')['type'])); + } + + public function testChangeRebuildsTableExactlyOnce(): void + { + [, $connection] = $this->bootSqlite('CREATE TABLE t (id integer primary key, name varchar)'); - $blueprint = new Blueprint($connection, 'users'); - $blueprint->string('name')->nullable()->change(); + $blueprint = new Blueprint($connection, 't'); + $blueprint->text('name')->change(); - // Prior to the typeTinyint fix, this would throw: - // BadMethodCallException: Method SQLiteGrammar::typeTinyint does not exist - $statements = $blueprint->toSql(); + $rebuilds = count(array_filter( + $blueprint->toSql(), + fn ($statement) => str_contains($statement, 'create table "__temp__') + )); - // compileChange maps 'tinyint' (SQLite's introspected type_name) to 'integer' - $this->assertNotEmpty($statements); - $this->assertStringContainsString('"is_active" integer', $statements[0]); + $this->assertSame(1, $rebuilds, 'A single ->change() must rebuild the table exactly once.'); } }