diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 83e0e70ef..4c65b8e78 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -58,7 +58,7 @@ parameters: path: '*' identifier: class.nameCase message: '~^Class Doctrine\\DBAL\\(Platforms\\SqlitePlatform|Schema\\SqliteSchemaManager) referenced with incorrect case: Doctrine\\DBAL\\(Platforms\\SQLitePlatform|Schema\\SQLiteSchemaManager)\.$~' - count: 21 + count: 23 # TODO these rules are generated, this ignores should be fixed in the code # for src/Schema/TestCase.php diff --git a/src/Model.php b/src/Model.php index 28626ea2a..2e47e6eaa 100644 --- a/src/Model.php +++ b/src/Model.php @@ -384,7 +384,11 @@ protected function init(): void $this->_init(); if ($this->idField) { - $this->addField($this->idField, ['type' => 'integer', 'required' => true, 'system' => true]); + if (!$this->hasField($this->idField)) { + $this->addField($this->idField, ['type' => 'integer']); + } + $this->getIdField()->required = true; + $this->getIdField()->system = true; $this->initEntityIdHooks(); diff --git a/src/Persistence/Sql/Join.php b/src/Persistence/Sql/Join.php index de65d44b7..35dad6b38 100644 --- a/src/Persistence/Sql/Join.php +++ b/src/Persistence/Sql/Join.php @@ -21,7 +21,7 @@ protected function init(): void { parent::init(); - // TODO thus mutates the owner model! + // TODO this mutates the owner model! $this->getOwner()->persistenceData['use_table_prefixes'] = true; // our short name will be unique @@ -30,15 +30,15 @@ protected function init(): void $this->foreignAlias = ($this->getOwner()->tableAlias ?? '') . '_' . (str_starts_with($this->shortName, '#join-') ? substr($this->shortName, 6) : $this->shortName); } - // TODO thus mutates the owner model/joins! + // TODO this mutates the owner model/joins! if (!$this->reverse && !$this->getOwner()->hasField($this->masterField)) { $owner = $this->hasJoin() ? $this->getJoin() : $this->getOwner(); $field = $owner->addField($this->masterField, ['type' => 'integer', 'system' => true, 'readOnly' => true]); - $this->masterField = $field->shortName; // TODO thus mutates the join! + $this->masterField = $field->shortName; // TODO this mutates the join! } elseif ($this->reverse && !$this->getOwner()->hasField($this->foreignField) && $this->hasJoin()) { $owner = $this->getJoin(); $field = $owner->addField($this->foreignField, ['type' => 'integer', 'system' => true, 'readOnly' => true, 'actual' => $this->masterField]); - $this->foreignField = $field->shortName; // TODO thus mutates the join! + $this->foreignField = $field->shortName; // TODO this mutates the join! } } diff --git a/src/Schema/Migrator.php b/src/Schema/Migrator.php index b0882265f..d02adff4c 100644 --- a/src/Schema/Migrator.php +++ b/src/Schema/Migrator.php @@ -28,6 +28,7 @@ use Doctrine\DBAL\Schema\Identifier; use Doctrine\DBAL\Schema\Index; use Doctrine\DBAL\Schema\Table; +use Doctrine\DBAL\Types\Type; class Migrator { @@ -404,6 +405,29 @@ protected function fixTableNameForListMethod(string $tableName): string return $platform->quoteSingleIdentifier($tableName); } + public function introspectTableToModel(string $tableName): Model + { + $columns = $this->createSchemaManager()->listTableColumns($this->fixTableNameForListMethod($tableName)); + + $indexes = $this->createSchemaManager()->listTableIndexes($this->fixTableNameForListMethod($tableName)); + $primaryIndexes = array_filter($indexes, static fn ($v) => $v->isPrimary() && count($v->getColumns()) === 1); + if (count($primaryIndexes) !== 1) { + throw (new Exception('Table must contain exactly one primary key')) + ->addMoreInfo('table', $tableName); + } + $idFieldName = reset($primaryIndexes)->getUnquotedColumns()[0]; + + $model = new Model(null, ['table' => $tableName, 'idField' => $idFieldName]); + foreach ($columns as $column) { + $model->addField($column->getName(), [ + 'type' => Type::getTypeRegistry()->lookupName($column->getType()), // TODO simplify once https://github.com/doctrine/dbal/pull/6130 is merged + 'nullable' => !$column->getNotnull(), + ]); + } + + return $model; + } + /** * @param list $fields */ diff --git a/src/Schema/TestCase.php b/src/Schema/TestCase.php index 0a6bd220d..a3db74be5 100644 --- a/src/Schema/TestCase.php +++ b/src/Schema/TestCase.php @@ -308,14 +308,16 @@ public function createMigrator(?Model $model = null): Migrator } /** - * @param array>> $dbData + * @param array>> $dbData */ public function setDb(array $dbData, bool $importData = true): void { foreach ($dbData as $tableName => $data) { - $migrator = $this->createMigrator()->table($tableName); + $idField = $data['_types']['_idField'] ?? 'id'; + unset($data['_types']['_idField']); - $fieldTypes = []; + $fieldTypes = $data['_types'] ?? []; + unset($data['_types']); foreach ($data as $row) { foreach ($row as $k => $v) { if (isset($fieldTypes[$k])) { @@ -323,61 +325,52 @@ public function setDb(array $dbData, bool $importData = true): void } if (is_bool($v)) { - $fieldType = 'boolean'; + $type = 'boolean'; } elseif (is_int($v)) { - $fieldType = 'integer'; + $type = 'integer'; } elseif (is_float($v)) { - $fieldType = 'float'; + $type = 'float'; } elseif ($v instanceof \DateTimeInterface) { - $fieldType = 'datetime'; + $type = 'datetime'; } elseif ($v !== null) { - $fieldType = 'string'; + $type = 'string'; } else { - $fieldType = null; + $type = null; } - $fieldTypes[$k] = $fieldType; + $fieldTypes[$k] = $type; } } - foreach ($fieldTypes as $k => $fieldType) { - if ($fieldType === null) { + foreach ($fieldTypes as $k => $type) { + if ($type === null) { $fieldTypes[$k] = 'string'; } } - $idColumnName = isset($fieldTypes['_id']) ? '_id' : 'id'; - - // create table - $migrator->id($idColumnName, isset($fieldTypes[$idColumnName]) ? ['type' => $fieldTypes[$idColumnName]] : []); - foreach ($fieldTypes as $k => $fieldType) { - if ($k === $idColumnName) { - continue; - } + if (!isset($fieldTypes[$idField])) { + $fieldTypes = array_merge([$idField => 'integer'], $fieldTypes); + } - $migrator->field($k, ['type' => $fieldType]); + $model = new Model(null, ['table' => $tableName, 'idField' => $idField]); + foreach ($fieldTypes as $k => $type) { + $model->addField($k, ['type' => $type]); } + $model->setPersistence($this->db); + + // create table + $migrator = $this->createMigrator($model); $migrator->create(); // import data if ($importData) { - $this->db->atomic(function () use ($tableName, $data, $idColumnName) { - $hasId = array_key_first($data) !== 0; - + if (array_key_first($data) !== 0) { foreach ($data as $id => $row) { - $query = $this->db->dsql(); - if ($id === '_') { - continue; - } - - $query->table($tableName); - $query->setMulti($row); - - if (!isset($row[$idColumnName]) && $hasId) { - $query->set($idColumnName, $id); + if (!isset($row[$idField])) { + $data[$id][$idField] = $id; } - - $query->mode('insert')->executeStatement(); } - }); + } + + $model->import($data); } } } @@ -401,35 +394,18 @@ public function getDb(?array $tableNames = null, bool $noId = false): array $resAll = []; foreach ($tableNames as $table) { - $query = $this->db->dsql(); - $rows = $query->table($table)->getRows(); - - $res = []; - $idColumnName = null; - foreach ($rows as $row) { - if ($idColumnName === null) { - $idColumnName = isset($row['_id']) ? '_id' : 'id'; - } - - foreach ($row as $k => $v) { - if (preg_match('~(?:^|_)id$~', $k) && $v === (string) (int) $v) { - $row[$k] = (int) $v; - } - } - - if ($noId) { - unset($row[$idColumnName]); - $res[] = $row; - } else { - $res[$row[$idColumnName]] = $row; - } - } + $model = $this->createMigrator()->introspectTableToModel($table); + $model->setPersistence($this->db); if (!$noId) { - ksort($res); + $model->setOrder($model->idField); } - $resAll[$table] = $res; + $data = $noId + ? $model->export(array_diff(array_keys($model->getFields()), [$model->idField])) + : $model->export(null, $model->idField); + + $resAll[$table] = $data; } return $resAll; diff --git a/tests/JoinSqlTest.php b/tests/JoinSqlTest.php index f340afada..7315ba7d9 100644 --- a/tests/JoinSqlTest.php +++ b/tests/JoinSqlTest.php @@ -107,10 +107,10 @@ public function testJoinSaving1(): void $user = new Model($this->db, ['table' => 'user']); $this->setDb([ 'user' => [ - '_' => ['id' => 1, 'name' => 'John', 'contact_id' => 1], + '_types' => ['name' => 'string', 'contact_id' => 'integer'], ], 'contact' => [ - '_' => ['id' => 1, 'contact_phone' => '+123'], + '_types' => ['contact_phone' => 'string'], ], ]); @@ -161,10 +161,10 @@ public function testJoinSaving2(): void { $this->setDb([ 'user' => [ - '_' => ['id' => 1, 'name' => 'John'], + '_types' => ['name' => 'string'], ], 'contact' => [ - '_' => ['id' => 1, 'contact_phone' => '+123', 'test_id' => 0], + '_types' => ['contact_phone' => 'string', 'test_id' => 'integer'], ], ]); @@ -234,10 +234,10 @@ public function testJoinSaving3(): void $user = new Model($this->db, ['table' => 'user']); $this->setDb([ 'user' => [ - '_' => ['id' => 1, 'name' => 'John', 'test_id' => 0], + '_types' => ['name' => 'string', 'test_id' => 'integer'], ], 'contact' => [ - '_' => ['id' => 1, 'contact_phone' => '+123'], + '_types' => ['contact_phone' => 'string'], ], ]); @@ -464,10 +464,10 @@ public function testDoubleSaveHook(): void { $this->setDb([ 'user' => [ - '_' => ['id' => 1, 'name' => 'John'], + '_types' => ['name' => 'string'], ], 'contact' => [ - '_' => ['id' => 1, 'contact_phone' => '+123', 'test_id' => 0], + '_types' => ['contact_phone' => 'string', 'test_id' => 'integer'], ], ]); @@ -828,7 +828,7 @@ public function testJoinActualFieldNamesAndPrefix(): void $j2->allowDangerousForeignTableUpdate = true; $user2->save(); - self::{'assertEquals'}([ + self::assertSame([ 'user' => [ 1 => ['id' => 1, 'first_name' => 'John 2', $contactForeignIdFieldName => 1], ['id' => 2, 'first_name' => 'Peter', $contactForeignIdFieldName => 1], @@ -851,7 +851,7 @@ public function testJoinActualFieldNamesAndPrefix(): void $user3->set('j2_salary', 222); $user3->save(); - self::{'assertEquals'}([ + self::assertSame([ 'user' => [ 1 => ['id' => 1, 'first_name' => 'John 2', $contactForeignIdFieldName => 1], ['id' => 2, 'first_name' => 'Peter', $contactForeignIdFieldName => 1], diff --git a/tests/LookupSqlTest.php b/tests/LookupSqlTest.php index 17476cc35..5c06e7a69 100644 --- a/tests/LookupSqlTest.php +++ b/tests/LookupSqlTest.php @@ -182,43 +182,43 @@ public function testImportCountriesBasic(): void 'id' => 1, 'name' => 'Canada', 'code' => null, - 'is_eu' => '0', + 'is_eu' => false, ], [ 'id' => 2, 'name' => 'Latvia', 'code' => 'LV', - 'is_eu' => '1', + 'is_eu' => true, ], [ 'id' => 3, 'name' => 'Estonia', 'code' => 'ES', - 'is_eu' => '1', + 'is_eu' => true, ], [ 'id' => 4, 'name' => 'Korea', 'code' => 'KR', - 'is_eu' => '0', + 'is_eu' => false, ], [ 'id' => 5, 'name' => 'Japan', 'code' => 'JP', - 'is_eu' => '0', + 'is_eu' => false, ], [ 'id' => 6, 'name' => 'Lithuania', 'code' => 'LT', - 'is_eu' => '1', + 'is_eu' => true, ], [ 'id' => 7, 'name' => 'Russia', 'code' => 'RU', - 'is_eu' => '0', + 'is_eu' => false, ], ], ], $this->getDb(['country'])); @@ -241,38 +241,38 @@ public function testImportInternationalUsers(): void 'id' => 1, 'name' => 'Canada', 'code' => null, - 'is_eu' => '0', + 'is_eu' => false, ], [ 'id' => 2, 'name' => 'Latvia', 'code' => null, - 'is_eu' => '0', + 'is_eu' => false, ], ], 'user' => [ 1 => [ 'id' => 1, 'name' => 'Alain', - 'is_vip' => '0', + 'is_vip' => false, 'country_id' => 1, ], [ 'id' => 2, 'name' => 'Duncan', - 'is_vip' => '1', + 'is_vip' => true, 'country_id' => 1, ], [ 'id' => 3, 'name' => 'imants', - 'is_vip' => '0', + 'is_vip' => false, 'country_id' => 2, ], [ 'id' => 4, 'name' => 'juris', - 'is_vip' => '0', + 'is_vip' => false, 'country_id' => 2, ], ], @@ -309,44 +309,44 @@ public function testImportByLookup(): void 'id' => 1, 'name' => 'Canada', 'code' => 'CA', - 'is_eu' => '0', + 'is_eu' => false, ], [ 'id' => 2, 'name' => 'Latvia', 'code' => 'LV', - 'is_eu' => '1', + 'is_eu' => true, ], [ 'id' => 3, 'name' => 'Japan', 'code' => 'JP', - 'is_eu' => '0', + 'is_eu' => false, ], [ 'id' => 4, 'name' => 'Lithuania', 'code' => 'LT', - 'is_eu' => '1', + 'is_eu' => true, ], [ 'id' => 5, 'name' => 'Russia', 'code' => 'RU', - 'is_eu' => '0', + 'is_eu' => false, ], ], 'user' => [ 1 => [ 'id' => 1, 'name' => 'Alain', - 'is_vip' => '0', + 'is_vip' => false, 'country_id' => 1, ], [ 'id' => 2, 'name' => 'Imants', - 'is_vip' => '0', + 'is_vip' => false, 'country_id' => 2, ], ], diff --git a/tests/ModelNestedSqlTest.php b/tests/ModelNestedSqlTest.php index b72975946..b8912ea71 100644 --- a/tests/ModelNestedSqlTest.php +++ b/tests/ModelNestedSqlTest.php @@ -21,6 +21,7 @@ protected function setUp(): void $this->setDb([ 'user' => [ + '_types' => ['_idField' => '_id'], ['_id' => 1, 'name' => 'John', '_birthday' => '1980-02-01'], ['_id' => 2, 'name' => 'Sue', '_birthday' => '2005-04-03'], ['_id' => 3, 'name' => 'Veronica', '_birthday' => '2005-04-03'], diff --git a/tests/RandomTest.php b/tests/RandomTest.php index 96a465af1..7ee4dbe97 100644 --- a/tests/RandomTest.php +++ b/tests/RandomTest.php @@ -94,7 +94,7 @@ public function testTitleImport(): void { $this->setDb([ 'user' => [ - '_' => ['name' => 'John', 'salary' => 29], + '_types' => ['name' => 'string', 'salary' => 'integer'], ], ]); @@ -108,10 +108,10 @@ public function testTitleImport(): void self::assertSame([ 'user' => [ - 1 => ['id' => 1, 'name' => 'Peter', 'salary' => '10'], - ['id' => 2, 'name' => 'Steve', 'salary' => '30'], - ['id' => 3, 'name' => 'Sue', 'salary' => '10'], - ['id' => 4, 'name' => 'John', 'salary' => '40'], + 1 => ['id' => 1, 'name' => 'Peter', 'salary' => 10], + ['id' => 2, 'name' => 'Steve', 'salary' => 30], + ['id' => 3, 'name' => 'Sue', 'salary' => 10], + ['id' => 4, 'name' => 'John', 'salary' => 40], ], ], $this->getDb()); } diff --git a/tests/Schema/MigratorTest.php b/tests/Schema/MigratorTest.php index 650b08010..cae0722d7 100644 --- a/tests/Schema/MigratorTest.php +++ b/tests/Schema/MigratorTest.php @@ -4,6 +4,7 @@ namespace Atk4\Data\Tests\Schema; +use Atk4\Data\Exception; use Atk4\Data\Field\PasswordField; use Atk4\Data\Model; use Atk4\Data\Persistence\Sql\Expression; @@ -13,7 +14,9 @@ use Doctrine\DBAL\Exception\TableNotFoundException; use Doctrine\DBAL\Platforms\OraclePlatform; use Doctrine\DBAL\Platforms\PostgreSQLPlatform; +use Doctrine\DBAL\Platforms\SQLitePlatform; use Doctrine\DBAL\Platforms\SQLServerPlatform; +use Doctrine\DBAL\Types\Type; use PHPUnit\Framework\Attributes\DataProvider; class MigratorTest extends TestCase @@ -24,14 +27,17 @@ protected function createDemoMigrator(string $table): Migrator ->table($table) ->id() ->field('foo') - ->field('bar', ['type' => 'integer']) + ->field('bar', ['type' => 'integer', 'nullable' => false]) ->field('baz', ['type' => 'text']) + ->field('bin1', ['type' => 'binary']) + ->field('bin2', ['type' => 'blob']) ->field('bl', ['type' => 'boolean']) ->field('tm', ['type' => 'time']) ->field('dt', ['type' => 'date']) ->field('dttm', ['type' => 'datetime']) ->field('fl', ['type' => 'float']) ->field('mn', ['type' => 'atk4_money']) + ->field('json', ['type' => 'json']) ->field('lobj', ['type' => 'atk4_local_object']); } @@ -288,6 +294,73 @@ public function testSetModelCreate(): void ['id' => 1, 'name' => 'john', 'password' => null, 'is_admin' => true, 'notes' => 'some long notes', 'main_role_id' => null], ], $user->export()); } + + public function testIntrospectTableToModelBasic(): void + { + $creatingMigrator = $this->createDemoMigrator('user')->create(); + $introspectedModel = $this->createMigrator()->introspectTableToModel('user'); + + $expectedFields = []; + foreach ($creatingMigrator->table->getColumns() as $column) { + $expectedFields[$column->getName()] = [ + 'type' => Type::getTypeRegistry()->lookupName($column->getType()), // TODO simplify once https://github.com/doctrine/dbal/pull/6130 is merged + 'nullable' => !$column->getNotnull(), + ]; + } + + $introspectedFields = []; + foreach ($introspectedModel->getFields() as $field) { + $introspectedFields[$field->shortName] = [ + 'type' => $field->type, + 'nullable' => $field->nullable && !$field->required, + ]; + } + + // TODO fix DBAL introspection + if ($this->getDatabasePlatform() instanceof SQLitePlatform) { + $expectedFields['bin1']['type'] = 'blob'; + $expectedFields['mn']['type'] = 'float'; + $expectedFields['json']['type'] = 'text'; + $expectedFields['lobj']['type'] = 'string'; + } elseif ($this->getDatabasePlatform() instanceof PostgreSQLPlatform) { + $expectedFields['foo']['type'] = 'text'; + $expectedFields['bin1']['type'] = 'blob'; + } elseif ($this->getDatabasePlatform() instanceof SQLServerPlatform) { + $expectedFields['baz']['type'] = 'string'; + } elseif ($this->getDatabasePlatform() instanceof OraclePlatform) { + $expectedFields['bin1']['type'] = 'string'; + $expectedFields['bin2']['type'] = 'text'; + } + + self::assertSame($expectedFields, $introspectedFields); + } + + public function testIntrospectTableToModelPrimaryKeyNonFirstColumn(): void + { + $this->createMigrator() + ->table('t') + ->field('a') + ->id() + ->field('b') + ->create(); + + $model = $this->createMigrator()->introspectTableToModel('t'); + + self::assertSame(['a', 'id', 'b'], array_keys($model->getFields())); + self::assertSame('id', $model->idField); + } + + public function testIntrospectTableToModelNoPrimaryKeyException(): void + { + $this->createMigrator() + ->table('t') + ->field('a') + ->create(); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('Table must contain exactly one primary key'); + $this->createMigrator()->introspectTableToModel('t'); + } } class TestUser extends Model diff --git a/tests/TypecastingTest.php b/tests/TypecastingTest.php index a2ed79cff..8c388f5ef 100644 --- a/tests/TypecastingTest.php +++ b/tests/TypecastingTest.php @@ -44,8 +44,8 @@ public function testType(): void 'datetime' => '2013-02-20 20:00:12.000000', 'time' => '12:00:50.000000', 'boolean' => true, - 'integer' => '2940', - 'money' => '8.20', + 'integer' => 2940, + 'money' => 8.2, 'float' => 8.20234376757473, 'json' => '[1,2,3]', ], @@ -87,7 +87,7 @@ public function testType(): void 'date' => '2013-02-20', 'datetime' => '2013-02-20 20:00:12.000000', 'time' => '12:00:50.000000', - 'boolean' => 1, + 'boolean' => true, 'integer' => 2940, 'money' => 8.2, 'float' => 8.20234376757473, @@ -99,15 +99,15 @@ public function testType(): void 'date' => '2013-02-20', 'datetime' => '2013-02-20 20:00:12.000000', 'time' => '12:00:50.000000', - 'boolean' => '1', - 'integer' => '2940', + 'boolean' => true, + 'integer' => 2940, 'money' => 8.2, 'float' => 8.20234376757473, 'json' => '[1,2,3]', ], ], ]; - self::{'assertEquals'}($dbData, $this->getDb()); + self::assertSame($dbData, $this->getDb()); [$first, $duplicate] = $m->export(); @@ -258,7 +258,14 @@ public function testTypecastNull(): void $dbData['test'][2] = array_merge(['id' => 2], $row); - self::{'assertEquals'}($dbData, $this->getDb()); + // TODO Oracle always converts empty string to null + // https://stackoverflow.com/questions/13278773/null-vs-empty-string-in-oracle#13278879 + if ($this->getDatabasePlatform() instanceof OraclePlatform) { + $dbData['test'][1]['b'] = null; + $dbData['test'][2]['b'] = null; + } + + self::assertSame($dbData, $this->getDb()); } /** @@ -356,7 +363,7 @@ public function testTypeCustom1(): void 'b1' => true, 'b2' => false, 'integer' => '2940', - 'money' => '8.20', + 'money' => 8.2, 'float' => 8.20234376757473, ], ], @@ -390,10 +397,10 @@ public function testTypeCustom1(): void $m->delete(1); unset($dbData['types'][0]); - $row['money'] = 8.2; // no trailing zero is expected - $dbData['types'][2] = array_merge(['id' => '2'], $row); + $row['money'] = 8.2; + $dbData['types'][2] = array_merge(['id' => 2], $row); - self::{'assertEquals'}($dbData, $this->getDb()); + self::assertSame($dbData, $this->getDb()); } public function testLoad(): void