diff --git a/src/Template/Bake/config/snapshot.ctp b/src/Template/Bake/config/snapshot.ctp index 6bcfbdc3..df62a5e1 100644 --- a/src/Template/Bake/config/snapshot.ctp +++ b/src/Template/Bake/config/snapshot.ctp @@ -15,11 +15,16 @@ use Cake\Database\Schema\Table; -$wantedOptions = array_flip(['length', 'limit', 'default', 'unsigned', 'null', 'comment', 'autoIncrement', 'precision']); +$wantedOptions = array_flip(['length', 'limit', 'default', 'signed', 'null', 'comment', 'autoIncrement', 'precision']); $tableMethod = $this->Migration->tableMethod($action); $columnMethod = $this->Migration->columnMethod($action); $indexMethod = $this->Migration->indexMethod($action); $constraints = $foreignKeys = $dropForeignKeys = []; +$hasUnsignedPk = $this->Migration->hasUnsignedPrimaryKey($tables); + +if ($autoId && $hasUnsignedPk) { + $autoId = false; +} %> extends AbstractMigration if (empty($columnOptions['precision'])) { unset($columnOptions['precision']); } + if (isset($columnOptions['signed']) && $columnOptions['signed'] === true) { + unset($columnOptions['signed']); + } echo $this->Migration->stringifyList($columnOptions, ['indent' => 4]); %>]) <%- endforeach; @@ -76,6 +84,9 @@ class <%= $name %> extends AbstractMigration if (empty($columnOptions['autoIncrement'])) { unset($columnOptions['autoIncrement']); } + if (isset($columnOptions['signed']) && $columnOptions['signed'] === true) { + unset($columnOptions['signed']); + } if (empty($columnOptions['precision'])) { unset($columnOptions['precision']); } else { diff --git a/src/View/Helper/MigrationHelper.php b/src/View/Helper/MigrationHelper.php index 5020d972..2ce07fb2 100644 --- a/src/View/Helper/MigrationHelper.php +++ b/src/View/Helper/MigrationHelper.php @@ -217,6 +217,31 @@ public function primaryKeys($table) return $primaryKeys; } + /** + * Returns whether the $tables list given as arguments contains primary keys + * unsigned. + * + * @param array $tables List of tables to check + * @return bool + */ + public function hasUnsignedPrimaryKey($tables) + { + foreach ($tables as $table) { + $collection = $this->config('collection'); + $tableSchema = $collection->describe($table); + $tablePrimaryKeys = $tableSchema->primaryKey(); + + foreach ($tablePrimaryKeys as $primaryKey) { + $column = $tableSchema->column($primaryKey); + if (isset($column['unsigned']) && $column['unsigned'] === true) { + return true; + } + } + } + + return false; + } + /** * Returns the primary key columns name for a given table * diff --git a/tests/Fixture/ArticlesFixture.php b/tests/Fixture/ArticlesFixture.php index 4ceb37d6..374f6716 100644 --- a/tests/Fixture/ArticlesFixture.php +++ b/tests/Fixture/ArticlesFixture.php @@ -32,6 +32,7 @@ class ArticlesFixture extends TestFixture 'title' => ['type' => 'string', 'null' => true, 'length' => 255, 'comment' => 'Article title'], 'category_id' => ['type' => 'integer', 'length' => 11], 'product_id' => ['type' => 'integer', 'length' => 11], + 'counter' => ['type' => 'integer', 'length' => 11, 'unsigned' => true], 'created' => ['type' => 'timestamp', 'null' => true, 'default' => null], 'modified' => ['type' => 'timestamp', 'null' => true, 'default' => null], '_indexes' => [ diff --git a/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php b/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php index 0a7f7b30..0c456db5 100644 --- a/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php +++ b/tests/TestCase/Shell/Task/MigrationSnapshotTaskTest.php @@ -14,6 +14,7 @@ use Bake\Shell\Task\BakeTemplateTask; use Cake\Core\Configure; use Cake\Core\Plugin; +use Cake\Database\Schema\Table; use Cake\Datasource\ConnectionManager; use Cake\TestSuite\StringCompareTrait; use Cake\TestSuite\TestCase; @@ -147,6 +148,19 @@ public function testAutoIdDisabledSnapshot() */ public function testPluginBlog() { + $db = ConnectionManager::get('test'); + $collection = $db->schemaCollection(); + $table = new Table('parts', [ + 'id' => ['type' => 'integer', 'unsigned' => true], + 'name' => ['type' => 'string', 'length' => 255], + 'number' => ['type' => 'integer', 'null' => true, 'length' => 10, 'unsigned' => true] + ]); + $table->addConstraint('primary', ['type' => 'primary', 'columns' => ['id']]); + $sql = $table->createSql($db); + foreach ($sql as $stmt) { + $db->execute($stmt); + } + $task = $this->getTaskMock(['in', 'err', 'dispatchShell', '_stop']); $task->params['require-table'] = false; $task->params['connection'] = 'test'; @@ -157,6 +171,11 @@ public function testPluginBlog() $result = $task->bake($bakeName); $this->assertCorrectSnapshot($bakeName, $result); + + $sql = $table->dropSql($db); + foreach ($sql as $stmt) { + $db->execute($stmt); + } } /** diff --git a/tests/comparisons/Migration/pgsql/test_auto_id_disabled_snapshot_pgsql.php b/tests/comparisons/Migration/pgsql/test_auto_id_disabled_snapshot_pgsql.php index 790021e3..43156f73 100644 --- a/tests/comparisons/Migration/pgsql/test_auto_id_disabled_snapshot_pgsql.php +++ b/tests/comparisons/Migration/pgsql/test_auto_id_disabled_snapshot_pgsql.php @@ -33,6 +33,11 @@ public function up() 'limit' => 10, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 10, + 'null' => true, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, diff --git a/tests/comparisons/Migration/pgsql/test_not_empty_snapshot_pgsql.php b/tests/comparisons/Migration/pgsql/test_not_empty_snapshot_pgsql.php index 949f78f6..ad28767c 100644 --- a/tests/comparisons/Migration/pgsql/test_not_empty_snapshot_pgsql.php +++ b/tests/comparisons/Migration/pgsql/test_not_empty_snapshot_pgsql.php @@ -23,6 +23,11 @@ public function up() 'limit' => 10, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 10, + 'null' => true, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, diff --git a/tests/comparisons/Migration/pgsql/test_plugin_blog_pgsql.php b/tests/comparisons/Migration/pgsql/test_plugin_blog_pgsql.php index 6eaae086..110bcc5a 100644 --- a/tests/comparisons/Migration/pgsql/test_plugin_blog_pgsql.php +++ b/tests/comparisons/Migration/pgsql/test_plugin_blog_pgsql.php @@ -23,6 +23,11 @@ public function up() 'limit' => 10, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 10, + 'null' => true, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, @@ -85,6 +90,20 @@ public function up() ) ->create(); + $table = $this->table('parts'); + $table + ->addColumn('name', 'string', [ + 'default' => null, + 'limit' => 255, + 'null' => true, + ]) + ->addColumn('number', 'integer', [ + 'default' => null, + 'limit' => 10, + 'null' => true, + ]) + ->create(); + $this->table('articles') ->addForeignKey( 'category_id', @@ -120,5 +139,6 @@ public function down() $this->dropTable('articles'); $this->dropTable('categories'); + $this->dropTable('parts'); } } diff --git a/tests/comparisons/Migration/sqlite/test_auto_id_disabled_snapshot_sqlite.php b/tests/comparisons/Migration/sqlite/test_auto_id_disabled_snapshot_sqlite.php index 91a777e9..b5db2b86 100644 --- a/tests/comparisons/Migration/sqlite/test_auto_id_disabled_snapshot_sqlite.php +++ b/tests/comparisons/Migration/sqlite/test_auto_id_disabled_snapshot_sqlite.php @@ -32,6 +32,12 @@ public function up() 'limit' => 11, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 11, + 'null' => true, + 'signed' => false, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, diff --git a/tests/comparisons/Migration/sqlite/test_not_empty_snapshot_sqlite.php b/tests/comparisons/Migration/sqlite/test_not_empty_snapshot_sqlite.php index 7829d2bb..ec23e455 100644 --- a/tests/comparisons/Migration/sqlite/test_not_empty_snapshot_sqlite.php +++ b/tests/comparisons/Migration/sqlite/test_not_empty_snapshot_sqlite.php @@ -22,6 +22,12 @@ public function up() 'limit' => 11, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 11, + 'null' => true, + 'signed' => false, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, diff --git a/tests/comparisons/Migration/sqlite/test_plugin_blog_sqlite.php b/tests/comparisons/Migration/sqlite/test_plugin_blog_sqlite.php index f8cadf53..cad04594 100644 --- a/tests/comparisons/Migration/sqlite/test_plugin_blog_sqlite.php +++ b/tests/comparisons/Migration/sqlite/test_plugin_blog_sqlite.php @@ -22,6 +22,12 @@ public function up() 'limit' => 11, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 11, + 'null' => true, + 'signed' => false, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, @@ -84,6 +90,21 @@ public function up() ) ->create(); + $table = $this->table('parts'); + $table + ->addColumn('name', 'string', [ + 'default' => null, + 'limit' => 255, + 'null' => true, + ]) + ->addColumn('number', 'integer', [ + 'default' => null, + 'limit' => 10, + 'null' => true, + 'signed' => false, + ]) + ->create(); + $this->table('articles') ->addForeignKey( 'category_id', @@ -119,5 +140,6 @@ public function down() $this->dropTable('articles'); $this->dropTable('categories'); + $this->dropTable('parts'); } } diff --git a/tests/comparisons/Migration/test_auto_id_disabled_snapshot.php b/tests/comparisons/Migration/test_auto_id_disabled_snapshot.php index 4d696224..b822fc33 100644 --- a/tests/comparisons/Migration/test_auto_id_disabled_snapshot.php +++ b/tests/comparisons/Migration/test_auto_id_disabled_snapshot.php @@ -33,6 +33,12 @@ public function up() 'limit' => 11, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 11, + 'null' => true, + 'signed' => false, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, diff --git a/tests/comparisons/Migration/test_not_empty_snapshot.php b/tests/comparisons/Migration/test_not_empty_snapshot.php index 831aa176..a7eef355 100644 --- a/tests/comparisons/Migration/test_not_empty_snapshot.php +++ b/tests/comparisons/Migration/test_not_empty_snapshot.php @@ -23,6 +23,12 @@ public function up() 'limit' => 11, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 11, + 'null' => true, + 'signed' => false, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, diff --git a/tests/comparisons/Migration/test_plugin_blog.php b/tests/comparisons/Migration/test_plugin_blog.php index 4d165f16..1027e3a5 100644 --- a/tests/comparisons/Migration/test_plugin_blog.php +++ b/tests/comparisons/Migration/test_plugin_blog.php @@ -3,10 +3,20 @@ class TestPluginBlog extends AbstractMigration { + + public $autoId = false; + public function up() { $table = $this->table('articles'); $table + ->addColumn('id', 'integer', [ + 'autoIncrement' => true, + 'default' => null, + 'limit' => 11, + 'null' => false, + ]) + ->addPrimaryKey(['id']) ->addColumn('title', 'string', [ 'comment' => 'Article title', 'default' => null, @@ -23,6 +33,12 @@ public function up() 'limit' => 11, 'null' => true, ]) + ->addColumn('counter', 'integer', [ + 'default' => null, + 'limit' => 11, + 'null' => true, + 'signed' => false, + ]) ->addColumn('created', 'timestamp', [ 'default' => null, 'limit' => null, @@ -52,6 +68,13 @@ public function up() $table = $this->table('categories'); $table + ->addColumn('id', 'integer', [ + 'autoIncrement' => true, + 'default' => null, + 'limit' => 11, + 'null' => false, + ]) + ->addPrimaryKey(['id']) ->addColumn('parent_id', 'integer', [ 'default' => null, 'limit' => 11, @@ -85,6 +108,29 @@ public function up() ) ->create(); + $table = $this->table('parts'); + $table + ->addColumn('id', 'integer', [ + 'autoIncrement' => true, + 'default' => null, + 'limit' => 10, + 'null' => false, + 'signed' => false, + ]) + ->addPrimaryKey(['id']) + ->addColumn('name', 'string', [ + 'default' => null, + 'limit' => 255, + 'null' => true, + ]) + ->addColumn('number', 'integer', [ + 'default' => null, + 'limit' => 10, + 'null' => true, + 'signed' => false, + ]) + ->create(); + $this->table('articles') ->addForeignKey( 'category_id', @@ -120,5 +166,6 @@ public function down() $this->dropTable('articles'); $this->dropTable('categories'); + $this->dropTable('parts'); } } diff --git a/tests/test_app/Plugin/TestBlog/src/Model/Table/PartsTable.php b/tests/test_app/Plugin/TestBlog/src/Model/Table/PartsTable.php new file mode 100644 index 00000000..ee25ffd7 --- /dev/null +++ b/tests/test_app/Plugin/TestBlog/src/Model/Table/PartsTable.php @@ -0,0 +1,12 @@ +