diff --git a/.codeclimate.yml b/.codeclimate.yml index 6b68128..a0762fc 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -4,7 +4,7 @@ plugins: phpcodesniffer: enabled: false config: - standard: "PSR1,PSR2" + standard: "PSR12" ignore_warnings: true encoding: utf-8 phpmd: diff --git a/.github/workflows/bundler.yml b/.github/workflows/bundler.yml index 29c092b..7b38480 100644 --- a/.github/workflows/bundler.yml +++ b/.github/workflows/bundler.yml @@ -14,26 +14,25 @@ jobs: - run: echo ${{ github.ref }} - name: Update to stable dependencies run: | - jq 'del(.require["atk4/dsql"]) | del(.["require-dev"]["atk4/ui"]) | del(.["require-dev"]["atk4/data"])' < composer.json > tmp && mv tmp composer.json - - composer require --no-progress --no-suggest --prefer-dist --optimize-autoloader atk4/dsql - #composer require --dev atk4/data # atk4/ui - removed temporarily until atk4/ui is released - composer update --no-suggest --prefer-dist --optimize-autoloader + # replaces X keys with X-release keys + jq '. as $in | reduce (keys_unsorted[] | select(endswith("-release")|not)) as $k ({}; . + {($k) : (($k + "-release") as $kr | $in | if has($kr) then .[$kr] else .[$k] end) } )' < composer.json > tmp && mv tmp composer.json + v=$(echo ${{ github.ref }} | cut -d / -f 4) + echo "::set-env name=version::$v" - uses: teaminkling/autocommit@master with: - commit-message: Setting current dependencies + commit-message: Setting release dependencies - uses: ad-m/github-push-action@master with: branch: ${{ github.ref }} github_token: ${{ secrets.GITHUB_TOKEN }} - name: pull-request - uses: repo-sync/pull-request@v2 + uses: romaninsh/pull-request@master with: - source_branch: "" # If blank, default: triggered branch + source_branch: "release/${{ env.version }}" destination_branch: "master" # If blank, default: master - pr_title: "Releasing ${{ github.ref }} into master" + pr_title: "Releasing ${{ env.version }} into master" pr_body: | - [ ] Review changes (must include stable dependencies) - [ ] Merge this PR into master (will delete ${{ github.ref }}) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 77685ce..02dee9a 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -48,7 +48,7 @@ jobs: run: vendor/bin/phpunit --configuration phpunit.xml --coverage-text --exclude-group dns - name: MySQL Testing - run: vendor/bin/phpunit --configuration phpunit-mysql.xml --exclude-group dns + run: vendor/bin/phpunit --configuration phpunit-mysql-workflow.xml --exclude-group dns - name: Merge coverage logs run: vendor/bin/phpcov merge build/logs/ --clover build/logs/cc.xml; diff --git a/.phpcs.xml.dist b/.phpcs.xml.dist new file mode 100644 index 0000000..3b6a920 --- /dev/null +++ b/.phpcs.xml.dist @@ -0,0 +1,6 @@ + + . + /vendor/*$ + + + diff --git a/composer.json b/composer.json index c3816b9..e7d1e9e 100644 --- a/composer.json +++ b/composer.json @@ -26,16 +26,19 @@ "php": ">=7.2.0", "atk4/dsql": "^2.0" }, - "suggest": { - "atk4/data": "*", - "atk4/ui": "*", - "jdorn/sql-formatter": "*" - }, "require-dev": { + "atk4/data": "^2.0", + "atk4/ui": "^2.0", "phpunit/phpunit": "<6", "phpunit/dbunit": ">=1.2", "phpunit/phpcov": "*", - "codeclimate/php-test-reporter": "*" + "codeclimate/php-test-reporter": "*", + "squizlabs/php_codesniffer": "^3.5" + }, + "suggest": { + "atk4/data": "^2.0", + "atk4/ui": "^2.0", + "jdorn/sql-formatter": "*" }, "autoload": { "psr-4": { diff --git a/demos/init.php b/demos/init.php index 1634aee..0480e83 100644 --- a/demos/init.php +++ b/demos/init.php @@ -3,4 +3,4 @@ include '../vendor/autoload.php'; $db = \atk4\data\Persistence::connect('mysql://root:root@localhost/test'); -$db->connection = new \atk4\dsql\Connection_Dumper(['connection'=>$db->connection]); +$db->connection = new \atk4\dsql\Connection_Dumper(['connection' => $db->connection]); diff --git a/demos/modelmigrator.php b/demos/modelmigrator.php index 3a2d285..bc438b5 100644 --- a/demos/modelmigrator.php +++ b/demos/modelmigrator.php @@ -21,7 +21,7 @@ public function init() // ok, now we surely have DB! $m->save([ - 'name'=> 'John'.rand(1, 100), + 'name' => 'John'.rand(1, 100), ]); } catch (\atk4\core\Exception $e) { echo $e->getColorfulText(); diff --git a/phpunit-mysql-workflow.xml b/phpunit-mysql-workflow.xml new file mode 100644 index 0000000..4986a6e --- /dev/null +++ b/phpunit-mysql-workflow.xml @@ -0,0 +1,24 @@ + + + + + + + + + + ./vendor + + + ./src + + + + + tests + + + + + + diff --git a/phpunit-mysql.xml b/phpunit-mysql.xml index 4986a6e..ad17758 100644 --- a/phpunit-mysql.xml +++ b/phpunit-mysql.xml @@ -1,8 +1,8 @@ - - - + + + diff --git a/src/Migration.php b/src/Migration.php index 13c1872..481131b 100644 --- a/src/Migration.php +++ b/src/Migration.php @@ -3,6 +3,7 @@ namespace atk4\schema; use atk4\core\Exception; +use atk4\data\Field; use atk4\data\Field_SQL_Expression; use atk4\Data\Model; use atk4\data\Persistence; @@ -12,6 +13,10 @@ class Migration extends Expression { + public const REF_TYPE_NONE = 0; + public const REF_TYPE_LINK = 1; + public const REF_TYPE_PRIMARY = 2; + /** @var string Expression mode. See $templates. */ public $mode = 'create'; @@ -35,13 +40,13 @@ class Migration extends Expression protected $escape_char = '"'; /** @var string Expression to create primary key */ - public $primary_key_expr = 'integer primary key autoincrement'; + public $primary_key_expr = 'primary key autoincrement'; /** @var array Conversion mapping from Agile Data types to persistence types */ protected $defaultMapToPersistence = [ ['varchar', 255], // default 'boolean' => ['tinyint', 1], - 'integer' => ['int'], + 'integer' => ['bigint'], 'money' => ['decimal', 12, 2], 'float' => ['decimal', 16, 6], 'date' => ['date'], @@ -60,6 +65,8 @@ class Migration extends Expression [null], // default 'tinyint' => ['boolean'], 'int' => ['integer'], + 'integer' => ['integer'], + 'bigint' => ['integer'], 'decimal' => ['float'], 'numeric' => ['float'], 'date' => ['date'], @@ -81,7 +88,7 @@ class Migration extends Expression * * @return Migration Subclass */ - public static function getMigration($source, $params = []) : self + public static function getMigration($source, $params = []): self { $c = static::getConnection($source); @@ -112,7 +119,7 @@ public static function getMigration($source, $params = []) : self * * @return Connection */ - public static function getConnection($source) : Connection + public static function getConnection($source): Connection { if ($source instanceof Connection) { return $source; @@ -178,7 +185,7 @@ public function setSource($source) * * @return Model */ - public function setModel(Model $m) :Model + public function setModel(Model $m): Model { $this->table($m->table); @@ -193,46 +200,63 @@ public function setModel(Model $m) :Model } if ($field->short_name == $m->id_field) { - $this->id($field->actual ?: $field->short_name); - continue; + $ref_type = self::REF_TYPE_PRIMARY; + $persist_field = $field; + } else { + $ref_field = $this->getReferenceField($field); + $ref_type = $ref_field !== null ? self::REF_TYPE_LINK : $ref_type = self::REF_TYPE_NONE; + $persist_field = $ref_field ?? $field; } - // get field type from field - $type = $field->type; - - // if the field is a hasOne relation - // Don't have the right FieldType - // FieldType is stored in the reference field - if ($field->reference instanceof HasOne) { - - // @TODO if this can be done better? - - // i don't want to : - // - change the isolation of relation link - // - expose the protected property ->their_field - // i need the type of the field to be used in this table - $reflection = new \ReflectionClass($field->reference); - $property = $reflection->getProperty('their_field'); - $property->setAccessible(true); - - /** @var string $reference_their_field get Reflection protected property Reference->their_field */ - $reference_their_field = $property->getValue($field->reference); - - /** @var string $reference_field reference field name */ - $reference_field = $reference_their_field ?? $field->reference->owner->id_field; - - /** @var string $reference_model_class reference class fqcn */ - $reference_model_class = $field->reference->model; + $options = [ + 'type' => $ref_type !== self::REF_TYPE_NONE && empty($persist_field->type) ? 'integer' : $persist_field->type, + 'ref_type' => $ref_type, + 'mandatory' => ($field->mandatory || $field->required) && ($persist_field->mandatory || $persist_field->required), + // todo add more options here + ]; - $type = (new $reference_model_class($m->persistence))->getField($reference_field)->type ?? 'integer'; - } - - $this->field($field->actual ?: $field->short_name, ['type' => $type]); // todo add more options here + $this->field($field->actual ?: $field->short_name, $options); } return $m; } + protected function getReferenceField(Field $field): ?Field + { + // if the field is a hasOne relation + // Don't have the right FieldType + // FieldType is stored in the reference field + if ($field->reference instanceof HasOne) { + // @TODO if this can be done better? + + // i don't want to : + // - change the isolation of relation link + // - expose the protected property ->their_field + // i need the type of the field to be used in this table + $reflection = new \ReflectionClass($field->reference); + $property = $reflection->getProperty('their_field'); + $property->setAccessible(true); + + /** @var string $reference_their_field get Reflection protected property Reference->their_field */ + $reference_their_field = $property->getValue($field->reference); + + /** @var string $reference_field reference field name */ + $reference_field = $reference_their_field ?? $field->reference->owner->id_field; + + /** @var string $reference_model_class reference class fqcn */ + $reference_model_class = $field->reference->model; + + // @TODO fix, but without the dummy persistence, the following is shown: + // Uncaught atk4\core\Exception: Element is not found in collection + // for ID column + $dummyPersistence = new Persistence\SQL($this->connection); + + return (new $reference_model_class($dummyPersistence))->getField($reference_field); + } else { + return null; + } + } + /** * Set SQL expression template. * @@ -242,7 +266,7 @@ public function setModel(Model $m) :Model * * @return $this */ - public function mode(string $mode) :self + public function mode(string $mode): self { if (!isset($this->templates[$mode])) { throw new Exception(['Structure builder does not have this mode', 'mode' => $mode]); @@ -262,7 +286,7 @@ public function mode(string $mode) :self * * @return $this */ - public function create() :self + public function create(): self { $this->mode('create')->execute(); @@ -277,7 +301,7 @@ public function create() :self * * @return $this */ - public function drop() :self + public function drop(): self { $this->mode('drop')->execute(); @@ -292,7 +316,7 @@ public function drop() :self * * @return $this */ - public function alter() :self + public function alter(): self { $this->mode('alter')->execute(); @@ -307,7 +331,7 @@ public function alter() :self * * @return $this */ - public function rename() :self + public function rename(): self { $this->mode('rename')->execute(); @@ -323,7 +347,7 @@ public function rename() :self * * @return string Returns short textual info for logging purposes */ - public function migrate() :string + public function migrate(): string { $changes = $added = $altered = $dropped = 0; @@ -348,13 +372,16 @@ public function migrate() :string } if (isset($old[$field])) { - // compare options and if needed alter field // @todo add more options here like 'len' - if (array_key_exists('type', $old[$field]) && array_key_exists('type', $options) && $old[$field]['type'] != $options['type']) { - $this->alterField($field, $options); - $altered++; - $changes++; + if (array_key_exists('type', $old[$field]) && array_key_exists('type', $options)) { + $oldSQLFieldType = $this->getSQLFieldType($old[$field]['type']); + $newSQLFieldType = $this->getSQLFieldType($options['type']); + if ($oldSQLFieldType !== $newSQLFieldType) { + $this->alterField($field, $options); + $altered++; + $changes++; + } } unset($old[$field]); @@ -381,9 +408,9 @@ public function migrate() :string if ($changes) { $this->alter(); - return 'added '.$added.' field'.($added % 10 == 1 ? '' : 's').', '. - 'changed '.$altered.' field'.($altered % 10 == 1 ? '' : 's').' and '. - 'deleted '.$dropped.' field'.($dropped % 10 == 1 ? '' : 's'); + return 'added '.$added.' field'.($added == 1 ? '' : 's').', '. + 'changed '.$altered.' field'.($altered == 1 ? '' : 's').' and '. + 'deleted '.$dropped.' field'.($dropped == 1 ? '' : 's'); } return 'no changes'; @@ -394,7 +421,7 @@ public function migrate() :string * * @return string */ - public function _render_statements() :string + public function _render_statements(): string { $result = []; @@ -431,11 +458,11 @@ public function _render_statements() :string * * @return Model */ - public function createModel($persistence, $table = null) : Model + public function createModel($persistence, $table = null): Model { $this['table'] = $table ?? $this['table']; - $m = new Model([$persistence, 'table'=> $this['table']]); + $m = new Model([$persistence, 'table' => $this['table']]); $this->importTable($this['table']); @@ -469,7 +496,7 @@ public function createModel($persistence, $table = null) : Model * * @return $this */ - public function newField($field, $options = []) :self + public function newField($field, $options = []): self { $this->_set_args('newField', $field, $options); @@ -486,7 +513,7 @@ public function newField($field, $options = []) :self * * @return $this */ - public function alterField(string $field, $options = []) :self + public function alterField(string $field, $options = []): self { $this->_set_args('alterField', $field, $options); @@ -502,7 +529,7 @@ public function alterField(string $field, $options = []) :self * * @return $this */ - public function dropField($field) :self + public function dropField($field): self { $this->_set_args('dropField', $field, true); @@ -519,7 +546,7 @@ public function dropField($field) :self * * @return array */ - public function describeTable(string $table) : array + public function describeTable(string $table): array { return $this->connection->expr('pragma table_info({})', [$table])->get(); } @@ -531,7 +558,7 @@ public function describeTable(string $table) : array * * @return string|null */ - public function getModelFieldType(string $type) :?string + public function getModelFieldType(string $type): ?string { // remove parenthesis $type = trim(preg_replace('/\(.*/', '', strtolower($type))); @@ -550,14 +577,34 @@ public function getModelFieldType(string $type) :?string * * @return string|null */ - public function getSQLFieldType(?string $type, ?array $options = null) :?string + public function getSQLFieldType(?string $type, array $options = []): ?string { $type = strtolower($type); $map = array_merge($this->defaultMapToPersistence, $this->mapToPersistence); $a = array_key_exists($type, $map) ? $map[$type] : $map[0]; - return $a[0].(count($a) > 1 ? ' ('.implode(',', array_slice($a, 1)).')' : ''); + $res = $a[0]; + if (count($a) > 1) { + $res .= ' ('.implode(',', array_slice($a, 1)).')'; + } + + if (!empty($options['ref_type']) && $options['ref_type'] !== self::REF_TYPE_NONE && $type === 'integer') { + $res .= ' unsigned'; + } + + if ( + !empty($options['mandatory']) + || (!empty($options['ref_type']) && $options['ref_type'] === self::REF_TYPE_PRIMARY) + ) { + $res .= ' not null'; + } + + if (!empty($options['ref_type']) && $options['ref_type'] === self::REF_TYPE_PRIMARY) { + $res .= ' '.$this->primary_key_expr; + } + + return $res; } /** @@ -569,20 +616,22 @@ public function getSQLFieldType(?string $type, ?array $options = null) :?string * * @return bool */ - public function importTable(string $table) :bool + public function importTable(string $table): bool { $this->table($table); $has_fields = false; foreach ($this->describeTable($table) as $row) { $has_fields = true; - if ($row['pk']) { - $this->id($row['name']); - continue; - } $type = $this->getModelFieldType($row['type']); + $ref_type = $row['pk'] ? self::REF_TYPE_PRIMARY : self::REF_TYPE_NONE; - $this->field($row['name'], ['type'=>$type]); + $options = [ + 'type' => $type, + 'ref_type' => $ref_type, + ]; + + $this->field($row['name'], $options); } return $has_fields; @@ -643,14 +692,12 @@ public function field($name, $options = []) */ public function id($name = null) { - if (!$name) { - $name = 'id'; - } + $options = [ + 'type' => 'integer', + 'ref_type' => self::REF_TYPE_PRIMARY, + ]; - $val = $this->connection->expr($this->primary_key_expr); - - $this->args['field'] = - [$name => $val] + (isset($this->args['field']) ? $this->args['field'] : []); + $this->field($name ?? 'id', $options); return $this; } @@ -693,7 +740,7 @@ public function _render_field() * * @return string */ - protected function _render_one_field(string $field, array $options) :string + protected function _render_one_field(string $field, array $options): string { $name = $options['name'] ?? $field; $type = $this->getSQLFieldType($options['type'] ?? null, $options); @@ -706,7 +753,7 @@ protected function _render_one_field(string $field, array $options) :string * * @return array */ - public function _getFields() :array + public function _getFields(): array { return $this->args['field']; } @@ -726,7 +773,6 @@ protected function _set_args(string $what, string $alias, $value) if ($alias === null) { $this->args[$what][] = $value; } else { - // don't allow multiple values with same alias if (isset($this->args[$what][$alias])) { throw new Exception([ diff --git a/src/Migration/MySQL.php b/src/Migration/MySQL.php index 7e8664b..a91c363 100644 --- a/src/Migration/MySQL.php +++ b/src/Migration/MySQL.php @@ -13,7 +13,7 @@ class MySQL extends \atk4\schema\Migration protected $escape_char = '`'; /** @var string Expression to create primary key */ - public $primary_key_expr = 'integer primary key auto_increment'; + public $primary_key_expr = 'primary key auto_increment'; /** @var array use this array in extended classes to overwrite or extend values of default mapping */ public $mapToPersistence = [ @@ -37,7 +37,7 @@ class MySQL extends \atk4\schema\Migration * * @return array */ - public function describeTable(string $table) : array + public function describeTable(string $table): array { if (!$this->connection->expr('show tables like []', [$table])->get()) { return []; // no such table diff --git a/src/Migration/PgSQL.php b/src/Migration/PgSQL.php index 6821f8e..f30cd84 100644 --- a/src/Migration/PgSQL.php +++ b/src/Migration/PgSQL.php @@ -6,7 +6,7 @@ class PgSQL extends \atk4\schema\Migration { /** @var string Expression to create primary key */ - public $primary_key_expr = 'bigint generated by default as identity(start with 1) primary key'; + public $primary_key_expr = 'generated by default as identity(start with 1) primary key'; /** @var array use this array in extended classes to overwrite or extend values of default mapping */ public $mapToPersistence = [ @@ -33,7 +33,7 @@ class PgSQL extends \atk4\schema\Migration * * @return array */ - public function describeTable(string $table) : array + public function describeTable(string $table): array { $columns = $this->connection->expr('SELECT * FROM information_schema.COLUMNS WHERE TABLE_NAME = []', [$table])->get(); @@ -60,7 +60,7 @@ public function describeTable(string $table) : array * * @return string */ - public function _render_statements() :string + public function _render_statements(): string { $result = []; diff --git a/src/Migration/SQLite.php b/src/Migration/SQLite.php index e92bb78..4ada0ed 100644 --- a/src/Migration/SQLite.php +++ b/src/Migration/SQLite.php @@ -5,7 +5,7 @@ class SQLite extends \atk4\schema\Migration { /** @var string Expression to create primary key */ - public $primary_key_expr = 'integer primary key autoincrement'; + public $primary_key_expr = 'primary key autoincrement'; public $mapToAgile = [ 0 => ['string'], @@ -19,8 +19,30 @@ class SQLite extends \atk4\schema\Migration * * @return array */ - public function describeTable(string $table) : array + public function describeTable(string $table): array { return $this->connection->expr('pragma table_info({})', [$table])->get(); } + + /** + * Convert Agile Data field types to SQL field types. + * + * @param string $type Agile Data field type + * @param array $options More options + * + * @return string|null + */ + public function getSQLFieldType(?string $type, array $options = []): ?string + { + $res = parent::getSQLFieldType($type, $options); + + // fix PK datatype to "integer primary key" + // see https://www.sqlite.org/lang_createtable.html#rowid + // all other datatypes (like "bigint", "integer unsinged", "integer not null") are not supported + if (!empty($options['ref_type']) && $options['ref_type'] === self::REF_TYPE_PRIMARY) { + $res = preg_replace('~(?:big)?int(?:eger)?\s+(unsigned\s+)?(not null\s+)?~', 'integer ', $res); + } + + return $res; + } } diff --git a/tests/ModelTest.php b/tests/ModelTest.php index b9bc13f..dc9461d 100644 --- a/tests/ModelTest.php +++ b/tests/ModelTest.php @@ -13,7 +13,7 @@ public function testSetModelCreate() $migration->create(); // now we can use user - $user->save(['name'=>'john', 'is_admin'=>true, 'notes'=>'some long notes']); + $user->save(['name' => 'john', 'is_admin' => true, 'notes' => 'some long notes']); } public function testImportTable() @@ -23,17 +23,17 @@ public function testImportTable() $m = $this->getMigration(); $m->table('user')->id() ->field('foo') - ->field('str', ['type'=>'string']) - ->field('bool', ['type'=>'boolean']) - ->field('int', ['type'=>'integer']) - ->field('mon', ['type'=>'money']) - ->field('flt', ['type'=>'float']) - ->field('date', ['type'=>'date']) - ->field('datetime', ['type'=>'datetime']) - ->field('time', ['type'=>'time']) - ->field('txt', ['type'=>'text']) - ->field('arr', ['type'=>'array']) - ->field('obj', ['type'=>'object']) + ->field('str', ['type' => 'string']) + ->field('bool', ['type' => 'boolean']) + ->field('int', ['type' => 'integer']) + ->field('mon', ['type' => 'money']) + ->field('flt', ['type' => 'float']) + ->field('date', ['type' => 'date']) + ->field('datetime', ['type' => 'datetime']) + ->field('time', ['type' => 'time']) + ->field('txt', ['type' => 'text']) + ->field('arr', ['type' => 'array']) + ->field('obj', ['type' => 'object']) ->create(); $this->db->dsql()->table('user') ->set([ @@ -74,8 +74,8 @@ public function testMigrateTable() $m = $this->getMigration($this->db); $m->table('user')->id() ->field('foo') - ->field('bar', ['type'=>'integer']) - ->field('baz', ['type'=>'text']) + ->field('bar', ['type' => 'integer']) + ->field('baz', ['type' => 'text']) ->create(); $this->db->dsql()->table('user') ->set([ @@ -88,7 +88,7 @@ public function testMigrateTable() $m2 = $this->getMigration($this->db); $m2->table('user')->id() ->field('xx') - ->field('bar', ['type'=>'integer']) + ->field('bar', ['type' => 'integer']) ->field('baz') ->migrate(); } @@ -101,11 +101,13 @@ public function testCreateModel() $m = $this->getMigration($this->db); $user_model = $m->createModel($this->db, 'user'); - $this->assertEquals([ + $this->assertEquals( + [ 'name', 'password', 'is_admin', 'notes', + 'main_role_id', // our_field here not role_id (reference name) ], array_keys($user_model->getFields()) ); @@ -121,8 +123,23 @@ public function init() parent::init(); $this->addField('name'); - $this->addField('password', ['type'=>'password']); - $this->addField('is_admin', ['type'=>'boolean']); - $this->addField('notes', ['type'=>'text']); + $this->addField('password', ['type' => 'password']); + $this->addField('is_admin', ['type' => 'boolean']); + $this->addField('notes', ['type' => 'text']); + + $this->hasOne('role_id', [TestRole::class, 'our_field' => 'main_role_id', 'their_field' => 'id']); + } +} + +class TestRole extends \atk4\data\Model +{ + public $table = 'role'; + + public function init() + { + parent::init(); + + $this->addField('name'); + $this->hasMany('Users', [TestUser::class, 'our_field' => 'id', 'their_field' => 'main_role_id']); } }