From 8fd15902a45fb6168a7af461c3bfdc2048b4f918 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Tue, 6 Feb 2024 22:07:28 +0100 Subject: [PATCH 01/15] minor improvements --- src/Field.php | 8 ++++---- src/Persistence.php | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Field.php b/src/Field.php index 5e7f9187f..0a1064eb5 100644 --- a/src/Field.php +++ b/src/Field.php @@ -231,15 +231,15 @@ public function normalize($value) throw $e; } + if ($e->getPrevious() !== null && $e instanceof Exception && $e->getMessage() === 'Typecast save error') { + $e = $e->getPrevious(); + } + $messages = []; do { $messages[] = $e->getMessage(); } while ($e = $e->getPrevious()); - if (count($messages) >= 2 && $messages[0] === 'Typecast save error') { - array_shift($messages); - } - throw (new ValidationException([$this->shortName => implode(': ', $messages)], $this->issetOwner() ? $this->getOwner() : null)) ->addMoreInfo('field', $this); } diff --git a/src/Persistence.php b/src/Persistence.php index 21c7ac998..5a50a1608 100644 --- a/src/Persistence.php +++ b/src/Persistence.php @@ -434,7 +434,7 @@ public function typecastSaveField(Field $field, $value) } throw (new Exception('Typecast save error', 0, $e)) - ->addMoreInfo('field', $field->shortName); + ->addMoreInfo('field', $field); } } @@ -462,7 +462,7 @@ public function typecastLoadField(Field $field, $value) } throw (new Exception('Typecast parse error', 0, $e)) - ->addMoreInfo('field', $field->shortName); + ->addMoreInfo('field', $field); } } From 07b85e3e71ce35ae561b7fdde8fe55cb1d430ba8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sun, 18 Feb 2024 14:48:10 +0100 Subject: [PATCH 02/15] create generic platform only once --- src/Field.php | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Field.php b/src/Field.php index 0a1064eb5..943709ae2 100644 --- a/src/Field.php +++ b/src/Field.php @@ -27,6 +27,8 @@ class Field implements Expressionable setOwner as private _setOwner; } + private static $genericPersistence; + // {{{ Core functionality /** @@ -101,9 +103,7 @@ private function normalizeUsingTypecast($value) { $persistence = $this->issetOwner() && $this->getOwner()->issetPersistence() ? $this->getOwner()->getPersistence() - : new class() extends Persistence { - public function __construct() {} - }; + : $this->getGenericPersistence(); $persistenceSetSkipNormalizeFx = \Closure::bind(static function (bool $value) use ($persistence) { $persistence->typecastSaveSkipNormalize = $value; @@ -283,6 +283,15 @@ final public function setNull(Model $entity): self return $this; } + private function getGenericPersistence(): Persistence + { + if ((self::$genericPersistence ?? null) === null) { + self::$genericPersistence = new class() extends Persistence {}; + } + + return self::$genericPersistence; + } + /** * @param mixed $value * @@ -291,9 +300,7 @@ final public function setNull(Model $entity): self private function typecastSaveField($value, bool $allowGenericPersistence = false) { if (!$this->getOwner()->issetPersistence() && $allowGenericPersistence) { - $persistence = new class() extends Persistence { - public function __construct() {} - }; + $persistence = $this->getGenericPersistence(); } else { $this->getOwner()->assertHasPersistence(); $persistence = $this->getOwner()->getPersistence(); From 3280c981cb845757539081f9a33904d916f02fdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 11:41:01 +0100 Subject: [PATCH 03/15] minor --- tests/Persistence/GenericPlatformTest.php | 2 +- tests/ReferenceTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Persistence/GenericPlatformTest.php b/tests/Persistence/GenericPlatformTest.php index 24b3ff3e1..913c478e1 100644 --- a/tests/Persistence/GenericPlatformTest.php +++ b/tests/Persistence/GenericPlatformTest.php @@ -13,7 +13,7 @@ class GenericPlatformTest extends TestCase public function testGetName(): void { $genericPlatform = new GenericPlatform(); - self::assertSame($genericPlatform->getName(), 'atk4_data_generic'); // @phpstan-ignore-line + self::assertSame('atk4_data_generic', $genericPlatform->getName()); // @phpstan-ignore-line } public function testInitializeDoctrineTypeMappings(): void diff --git a/tests/ReferenceTest.php b/tests/ReferenceTest.php index eb976377c..5b0d7217a 100644 --- a/tests/ReferenceTest.php +++ b/tests/ReferenceTest.php @@ -119,7 +119,7 @@ public function testTheirFieldNameGuessTableWithSchema(): void $order->addField('user_id', ['type' => 'integer']); $user->hasMany('Orders', ['model' => $order, 'caption' => 'My Orders']); - self::assertSame($user->getReference('Orders')->getTheirFieldName(), 'user_id'); + self::assertSame('user_id', $user->getReference('Orders')->getTheirFieldName()); } public function testRefTypeMismatchOneException(): void From f49426f01d465419da24889e59b742f9cf03cea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 11:53:03 +0100 Subject: [PATCH 04/15] minor --- src/Model.php | 2 +- src/Persistence/Sql.php | 2 +- src/Persistence/Sql/Expression.php | 2 +- tests/SmboTransferTest.php | 8 ++------ 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/Model.php b/src/Model.php index 41b45d278..44c46a838 100644 --- a/src/Model.php +++ b/src/Model.php @@ -1686,7 +1686,7 @@ public function import(array $rows) * @param string $keyField Optional name of field which value we will use as array key * @param bool $typecast Should we typecast exported data * - * @return ($keyField is string ? array> : array>) + * @return ($keyField is string ? array> : list>) */ public function export(array $fields = null, string $keyField = null, bool $typecast = true): array { diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php index 977a6b94e..2daa9fd6f 100644 --- a/src/Persistence/Sql.php +++ b/src/Persistence/Sql.php @@ -499,7 +499,7 @@ public function tryLoad(Model $model, $id): ?array * * @param array|null $fields * - * @return array> + * @return list> */ public function export(Model $model, array $fields = null, bool $typecast = true): array { diff --git a/src/Persistence/Sql/Expression.php b/src/Persistence/Sql/Expression.php index 279f6c5dc..645ebc0b4 100644 --- a/src/Persistence/Sql/Expression.php +++ b/src/Persistence/Sql/Expression.php @@ -739,7 +739,7 @@ public function getRowsIterator(): \Traversable /** * Executes expression and return whole result-set in form of array of hashes. * - * @return array> + * @return list> */ public function getRows(): array { diff --git a/tests/SmboTransferTest.php b/tests/SmboTransferTest.php index 35f464230..946e8450d 100644 --- a/tests/SmboTransferTest.php +++ b/tests/SmboTransferTest.php @@ -50,14 +50,10 @@ public function testTransferBetweenAccounts(): void self::assertSame(100.0, $boi->reload()->get('balance')); $t = new Transfer($this->db); - $data = $t->export(['id', 'transfer_document_id']); - usort($data, static function ($e1, $e2) { - return $e1['id'] < $e2['id'] ? -1 : 1; - }); - self::assertSame([ + self::assertSameExportUnordered([ ['id' => 1, 'transfer_document_id' => 2], ['id' => 2, 'transfer_document_id' => 1], - ], $data); + ], $t->export(['id', 'transfer_document_id'])); } public function testRef(): void From ef6dce52c14b5f349b1083fe830f4070d8a22f2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 11:53:45 +0100 Subject: [PATCH 05/15] minor --- src/Model/AggregateModel.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Model/AggregateModel.php b/src/Model/AggregateModel.php index c94c58cae..626e887a2 100644 --- a/src/Model/AggregateModel.php +++ b/src/Model/AggregateModel.php @@ -22,9 +22,8 @@ * 'salary' => ['expr' => 'sum([])', 'type' => 'atk4_money'], * ]; * - * your resulting model will have 3 fields: first, last, salary - * - * but when querying it will use the original model to calculate the query, then add grouping and aggregates. + * Your resulting model will have 3 fields: first, last, salary, but when querying + * it will use the original model to calculate the query, then add grouping and aggregates. * * If you wish you can add more fields, which will be passed through: * $aggregate->addField('middle'); From 60802bc1cea41095c386307861070e1c98dc79ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 12:12:06 +0100 Subject: [PATCH 06/15] minor --- tests/Persistence/Sql/WithDb/SelectTest.php | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Persistence/Sql/WithDb/SelectTest.php b/tests/Persistence/Sql/WithDb/SelectTest.php index 1fb876a28..5cfef5fa9 100644 --- a/tests/Persistence/Sql/WithDb/SelectTest.php +++ b/tests/Persistence/Sql/WithDb/SelectTest.php @@ -616,6 +616,7 @@ public function testImportAndAutoincrement(): void // TODO workaround SQLite to be consistent with other databases // https://stackoverflow.com/questions/27947712/sqlite-repeats-primary-key-autoincrement-value-after-rollback + // https://github.com/atk4/data/issues/1162 if (!$this->getDatabasePlatform() instanceof SQLitePlatform) { self::assertSame(103, $getLastAiFx()); self::assertSame($expectedRows, $m->export()); From cd85df1a2aa47f2d8bd5d9b41c0a38cd891295a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 12:12:21 +0100 Subject: [PATCH 07/15] prevent object ID casting as much as possible --- src/Model.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Model.php b/src/Model.php index 44c46a838..13a0bd318 100644 --- a/src/Model.php +++ b/src/Model.php @@ -2051,7 +2051,11 @@ public function __debugInfo(): array if ($this->isEntity()) { return [ 'entityId' => $this->idField && $this->hasField($this->idField) - ? ($this->_entityId !== null ? $this->_entityId . ($this->getId() !== null ? '' : ' (unloaded)') : 'null') + ? ($this->_entityId !== null + ? ($this->getId() !== null + ? $this->_entityId + : $this->_entityId . ' (unloaded)') + : 'null') : 'no id field', 'model' => $this->getModel()->__debugInfo(), ]; From 14fbc7b1ec8ff7d8878e0d93e4e32a692fc422e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 12:15:41 +0100 Subject: [PATCH 08/15] improve SqlExpressionField dump --- src/Field/SqlExpressionField.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Field/SqlExpressionField.php b/src/Field/SqlExpressionField.php index 7651260f4..7cb78c089 100644 --- a/src/Field/SqlExpressionField.php +++ b/src/Field/SqlExpressionField.php @@ -63,4 +63,12 @@ public function getDsqlExpression(Expression $expression): Expression return $expr; } + + #[\Override] + public function __debugInfo(): array + { + return array_merge(parent::__debugInfo(), [ + 'expr' => $this->expr, + ]); + } } From fa5ef9756590045c84afea22f33f5d72ff753fa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 12:19:41 +0100 Subject: [PATCH 09/15] better Model::$data/$dirty init --- src/Model.php | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Model.php b/src/Model.php index 13a0bd318..224095afd 100644 --- a/src/Model.php +++ b/src/Model.php @@ -144,7 +144,7 @@ class Model implements \IteratorAggregate * * @var array */ - private array $data = []; + private array $data; /** * After loading an entity the data will be stored in @@ -157,7 +157,7 @@ class Model implements \IteratorAggregate * * @var array */ - private array $dirty = []; + private array $dirty; /** * Setting model as readOnly will protect you from accidentally @@ -374,6 +374,9 @@ public function createEntity(): self unset($entity->{$name}); } + $entity->data = []; + $entity->dirty = []; + return $entity; } From d9827da7ff895f67906b57cd212012942fe00dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 12:20:12 +0100 Subject: [PATCH 10/15] rm obsolete doc --- src/Model.php | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/Model.php b/src/Model.php index 224095afd..2987f78dc 100644 --- a/src/Model.php +++ b/src/Model.php @@ -235,11 +235,6 @@ class Model implements \IteratorAggregate * $m = new Model(); * $m->setPersistence($db); * - * The second use actually calls add() but is preferred usage because: - * - it's shorter - * - type hinting will work; - * - you can specify string for a table - * * @param array $defaults */ public function __construct(Persistence $persistence = null, array $defaults = []) From 651b42ee68f10234a25bf21315ef79bf8cc6ea91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 12:55:12 +0100 Subject: [PATCH 11/15] impl. Model::getIdField() --- src/Model.php | 21 +++++++++++++++++++-- src/Persistence.php | 14 +++++++------- src/Persistence/Array_.php | 12 ++++++------ src/Persistence/Array_/Db/Table.php | 2 +- src/Persistence/Sql.php | 22 +++++++++++----------- src/Persistence/Static_.php | 2 +- src/Reference.php | 6 ++++-- tests/ModelWithoutIdTest.php | 7 +++++++ 8 files changed, 56 insertions(+), 30 deletions(-) diff --git a/src/Model.php b/src/Model.php index 2987f78dc..bd4bb477f 100644 --- a/src/Model.php +++ b/src/Model.php @@ -605,6 +605,21 @@ public function getField(string $name): Field } } + public function getIdField(): Field + { + if ($this->isEntity()) { + return $this->getModel()->getIdField(); + } + + try { + return $this->getField($this->idField); + } catch (\Throwable $e) { + $this->assertHasIdField(); + + throw $e; + } + } + /** * Sets which fields we will select. * @@ -861,7 +876,7 @@ public function getModelCaption(): string } /** - * Return value of $model->get($model->titleField). If not set, returns id value. + * Return value of $model->get($model->titleField). If not set, returns ID value. * * @return mixed */ @@ -883,7 +898,9 @@ public function getTitles(): array { $this->assertIsModel(); - $field = $this->titleField && $this->hasField($this->titleField) ? $this->titleField : $this->idField; + $field = $this->titleField && $this->hasField($this->titleField) + ? $this->titleField + : $this->idField; return array_map(static function (array $row) use ($field) { return $row[$field]; diff --git a/src/Persistence.php b/src/Persistence.php index 5a50a1608..c21b9ad40 100644 --- a/src/Persistence.php +++ b/src/Persistence.php @@ -176,11 +176,11 @@ public function insert(Model $model, array $data) return false; } - $idField = $model->getField($model->idField); + $idField = $model->getIdField(); $insertId = $this->typecastLoadField( $idField, $idField->getPersistenceName() === $model->table->idField - ? $this->typecastSaveField($model->table->getField($model->table->idField), $innerInsertId) + ? $this->typecastSaveField($model->table->getIdField(), $innerInsertId) : $dataRaw[$idField->getPersistenceName()] ); @@ -192,7 +192,7 @@ public function insert(Model $model, array $data) return false; } - $id = $this->typecastLoadField($model->getField($model->idField), $idRaw); + $id = $this->typecastLoadField($model->getIdField(), $idRaw); return $id; } @@ -217,7 +217,7 @@ public function update(Model $model, $id, array $data): void { $model->assertIsModel(); - $idRaw = $model->idField ? $this->typecastSaveField($model->getField($model->idField), $id) : null; + $idRaw = $model->idField ? $this->typecastSaveField($model->getIdField(), $id) : null; unset($id); if ($idRaw === null || (array_key_exists($model->idField, $data) && $data[$model->idField] === null)) { throw new Exception('Unable to update record: Model idField is not set'); @@ -231,7 +231,7 @@ public function update(Model $model, $id, array $data): void } if (is_object($model->table)) { - $idPersistenceName = $model->getField($model->idField)->getPersistenceName(); + $idPersistenceName = $model->getIdField()->getPersistenceName(); $innerId = $this->typecastLoadField($model->table->getField($idPersistenceName), $idRaw); $innerEntity = $model->table->loadBy($idPersistenceName, $innerId); @@ -261,14 +261,14 @@ public function delete(Model $model, $id): void { $model->assertIsModel(); - $idRaw = $model->idField ? $this->typecastSaveField($model->getField($model->idField), $id) : null; + $idRaw = $model->idField ? $this->typecastSaveField($model->getIdField(), $id) : null; unset($id); if ($idRaw === null) { throw new Exception('Unable to delete record: Model idField is not set'); } if (is_object($model->table)) { - $idPersistenceName = $model->getField($model->idField)->getPersistenceName(); + $idPersistenceName = $model->getIdField()->getPersistenceName(); $innerId = $this->typecastLoadField($model->table->getField($idPersistenceName), $idRaw); $innerEntity = $model->table->loadBy($idPersistenceName, $innerId); diff --git a/src/Persistence/Array_.php b/src/Persistence/Array_.php index b35495d55..c024862e2 100644 --- a/src/Persistence/Array_.php +++ b/src/Persistence/Array_.php @@ -122,7 +122,7 @@ public function getRawDataByTable(Model $model, string $table): array */ private function assertNoIdMismatch(Model $model, $idFromRow, $id): void { - if ($idFromRow !== null && !$model->getField($model->idField)->compare($idFromRow, $id)) { + if ($idFromRow !== null && !$model->getIdField()->compare($idFromRow, $id)) { throw (new Exception('Row contains ID column, but it does not match the row ID')) ->addMoreInfo('idFromKey', $id) ->addMoreInfo('idFromData', $idFromRow); @@ -136,7 +136,7 @@ private function assertNoIdMismatch(Model $model, $idFromRow, $id): void private function saveRow(Model $model, array $rowData, $id): void { if ($model->idField) { - $idField = $model->getField($model->idField); + $idField = $model->getIdField(); $id = $idField->normalize($id); $idColumnName = $idField->getPersistenceName(); if (array_key_exists($idColumnName, $rowData)) { @@ -249,7 +249,7 @@ public function tryLoad(Model $model, $id): ?array if (is_object($model->table)) { $action = $this->action($model, 'select'); $condition = new Model\Scope\Condition('', $id); - $condition->key = $model->getField($model->idField); + $condition->key = $model->getIdField(); $condition->setOwner($model->createEntity()); // TODO needed for typecasting to apply $action->filter($condition); @@ -308,7 +308,7 @@ public function generateNewId(Model $model) { $this->seedData($model); - $type = $model->idField ? $model->getField($model->idField)->type : 'integer'; + $type = $model->idField ? $model->getIdField()->type : 'integer'; switch ($type) { case 'integer': @@ -389,7 +389,7 @@ public function initAction(Model $model, array $fields = null): Action $rows = []; foreach ($table->getRows() as $row) { - $rows[$row->getValue($model->getField($model->idField)->getPersistenceName())] = $row->getData(); + $rows[$row->getValue($model->getIdField()->getPersistenceName())] = $row->getData(); } } @@ -432,7 +432,7 @@ protected function applyScope(Model $model, Action $action): void // add entity ID to scope to allow easy traversal if ($model->isEntity() && $model->idField && $model->getId() !== null) { $scope = new Model\Scope([$scope]); - $scope->addCondition($model->getField($model->idField), $model->getId()); + $scope->addCondition($model->getIdField(), $model->getId()); } $action->filter($scope); diff --git a/src/Persistence/Array_/Db/Table.php b/src/Persistence/Array_/Db/Table.php index d6d523025..8a19132f7 100644 --- a/src/Persistence/Array_/Db/Table.php +++ b/src/Persistence/Array_/Db/Table.php @@ -190,7 +190,7 @@ protected function beforeValuesSet(Row $childRow, $newRowData): void public function getRowById(Model $model, $idRaw): ?Row { foreach ($this->getRows() as $row) { - if ($row->getValue($model->getField($model->idField)->getPersistenceName()) === $idRaw) { + if ($row->getValue($model->getIdField()->getPersistenceName()) === $idRaw) { return $row; } } diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php index 2daa9fd6f..274391581 100644 --- a/src/Persistence/Sql.php +++ b/src/Persistence/Sql.php @@ -293,9 +293,9 @@ public function initQueryConditions(Model $model, Query $query): void // add entity ID to scope to allow easy traversal if ($model->isEntity() && $model->idField && $model->getId() !== null) { - $query->group($model->getField($model->idField)); + $query->group($model->getIdField()); $this->fixMssqlOracleMissingFieldsInGroup($model, $query); - $query->having($model->getField($model->idField), $model->getId()); + $query->having($model->getIdField(), $model->getId()); } } @@ -305,7 +305,7 @@ private function fixMssqlOracleMissingFieldsInGroup(Model $model, Query $query): || $this->getDatabasePlatform() instanceof OraclePlatform) { $isIdFieldInGroup = false; foreach ($query->args['group'] ?? [] as $v) { - if ($model->idField && $v === $model->getField($model->idField)) { + if ($model->idField && $v === $model->getIdField()) { $isIdFieldInGroup = true; break; @@ -396,8 +396,8 @@ public function action(Model $model, string $type, array $args = []) $this->fixMssqlOracleMissingFieldsInGroup($model, $query); if ($model->isEntity() && $model->isLoaded()) { - $idRaw = $this->typecastSaveField($model->getField($model->idField), $model->getId()); - $query->where($model->getField($model->idField), $idRaw); + $idRaw = $this->typecastSaveField($model->getIdField(), $model->getId()); + $query->where($model->getIdField(), $idRaw); } return $query; @@ -456,8 +456,8 @@ public function tryLoad(Model $model, $id): ?array ->addMoreInfo('id', $id); } - $idRaw = $this->typecastSaveField($model->getField($model->idField), $id); - $query->where($model->getField($model->idField), $idRaw); + $idRaw = $this->typecastSaveField($model->getIdField(), $id); + $query->where($model->getIdField(), $idRaw); } $query->limit( min($id === self::ID_LOAD_ANY ? 1 : 2, $query->args['limit']['cnt'] ?? \PHP_INT_MAX), @@ -568,7 +568,7 @@ protected function insertRaw(Model $model, array $dataRaw) $this->assertExactlyOneRecordUpdated($model, null, $c, 'insert'); if ($model->idField) { - $idRaw = $dataRaw[$model->getField($model->idField)->getPersistenceName()] ?? null; + $idRaw = $dataRaw[$model->getIdField()->getPersistenceName()] ?? null; if ($idRaw === null) { $idRaw = $this->lastInsertId($model); } @@ -592,7 +592,7 @@ protected function updateRaw(Model $model, $idRaw, array $dataRaw): void // only apply fields that has been modified $update->setMulti($dataRaw); - $update->where($model->getField($model->idField)->getPersistenceName(), $idRaw); + $update->where($model->getIdField()->getPersistenceName(), $idRaw); $model->hook(self::HOOK_BEFORE_UPDATE_QUERY, [$update]); @@ -614,7 +614,7 @@ protected function deleteRaw(Model $model, $idRaw): void { $delete = $this->initQuery($model); $delete->mode('delete'); - $delete->where($model->getField($model->idField)->getPersistenceName(), $idRaw); + $delete->where($model->getIdField()->getPersistenceName(), $idRaw); $model->hook(self::HOOK_BEFORE_DELETE_QUERY, [$delete]); try { @@ -707,7 +707,7 @@ public function lastInsertId(Model $model): string if ($this->getConnection()->getDatabasePlatform() instanceof PostgreSQLPlatform) { $sequenceName = $this->getConnection()->getDatabasePlatform()->getIdentitySequenceName( $model->table, - $model->getField($model->idField)->getPersistenceName() + $model->getIdField()->getPersistenceName() ); } elseif ($this->getConnection()->getDatabasePlatform() instanceof OraclePlatform) { $sequenceName = $model->table . '_SEQ'; diff --git a/src/Persistence/Static_.php b/src/Persistence/Static_.php index ba42ad6f4..ee27d3408 100644 --- a/src/Persistence/Static_.php +++ b/src/Persistence/Static_.php @@ -149,7 +149,7 @@ public function add(Model $model, array $defaults = []): void }, null, Model::class)(); if (isset($this->fieldsForModel[$model->idField])) { - $model->getField($model->idField)->type = $this->fieldsForModel[$model->idField]['type']; + $model->getIdField()->type = $this->fieldsForModel[$model->idField]['type']; } } $this->addMissingFieldsToModel($model); diff --git a/src/Reference.php b/src/Reference.php index 1c8d2a992..23ffdfcb5 100644 --- a/src/Reference.php +++ b/src/Reference.php @@ -112,7 +112,8 @@ protected function assertReferenceValueNotNull($value): void public function getOurFieldName(): string { - return $this->ourField ?? $this->getOurModel(null)->idField; + return $this->ourField + ?? $this->getOurModel(null)->idField; } final protected function getOurField(): Field @@ -132,7 +133,8 @@ final protected function getOurFieldValue(Model $ourEntity) public function getTheirFieldName(Model $theirModel = null): string { - return $this->theirField ?? ($theirModel ?? Model::assertInstanceOf($this->model))->idField; + return $this->theirField + ?? ($theirModel ?? Model::assertInstanceOf($this->model))->idField; } /** diff --git a/tests/ModelWithoutIdTest.php b/tests/ModelWithoutIdTest.php index f96081f2d..40f462417 100644 --- a/tests/ModelWithoutIdTest.php +++ b/tests/ModelWithoutIdTest.php @@ -52,6 +52,13 @@ public function testBasic(): void self::assertSame(['Sue', 'John'], $names); } + public function testGetIdFieldException(): void + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('ID field is not defined'); + $this->m->getIdField(); + } + public function testGetIdException(): void { $m = $this->m->loadAny(); From 646a10ed3164f421b4f62a2261e0e1eda45c76ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 13:00:06 +0100 Subject: [PATCH 12/15] fix stan --- src/Field.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Field.php b/src/Field.php index 943709ae2..9d7f47c62 100644 --- a/src/Field.php +++ b/src/Field.php @@ -27,7 +27,7 @@ class Field implements Expressionable setOwner as private _setOwner; } - private static $genericPersistence; + private static Persistence $genericPersistence; // {{{ Core functionality From de73c3a589361c1a5461705a6aa95ec6f408832b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 13:03:58 +0100 Subject: [PATCH 13/15] improve coverage measurement --- src/Model.php | 4 +++- src/Persistence.php | 8 ++++++-- src/Persistence/Array_.php | 4 +++- src/Persistence/Csv.php | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/src/Model.php b/src/Model.php index bd4bb477f..acd3356d3 100644 --- a/src/Model.php +++ b/src/Model.php @@ -1671,7 +1671,9 @@ public function insert(array $row) }); } - return $this->idField ? $entity->getId() : null; + return $this->idField + ? $entity->getId() + : null; } /** diff --git a/src/Persistence.php b/src/Persistence.php index c21b9ad40..07a9c63cb 100644 --- a/src/Persistence.php +++ b/src/Persistence.php @@ -217,7 +217,9 @@ public function update(Model $model, $id, array $data): void { $model->assertIsModel(); - $idRaw = $model->idField ? $this->typecastSaveField($model->getIdField(), $id) : null; + $idRaw = $model->idField + ? $this->typecastSaveField($model->getIdField(), $id) + : null; unset($id); if ($idRaw === null || (array_key_exists($model->idField, $data) && $data[$model->idField] === null)) { throw new Exception('Unable to update record: Model idField is not set'); @@ -261,7 +263,9 @@ public function delete(Model $model, $id): void { $model->assertIsModel(); - $idRaw = $model->idField ? $this->typecastSaveField($model->getIdField(), $id) : null; + $idRaw = $model->idField + ? $this->typecastSaveField($model->getIdField(), $id) + : null; unset($id); if ($idRaw === null) { throw new Exception('Unable to delete record: Model idField is not set'); diff --git a/src/Persistence/Array_.php b/src/Persistence/Array_.php index c024862e2..b8846093d 100644 --- a/src/Persistence/Array_.php +++ b/src/Persistence/Array_.php @@ -308,7 +308,9 @@ public function generateNewId(Model $model) { $this->seedData($model); - $type = $model->idField ? $model->getIdField()->type : 'integer'; + $type = $model->idField + ? $model->getIdField()->type + : 'integer'; switch ($type) { case 'integer': diff --git a/src/Persistence/Csv.php b/src/Persistence/Csv.php index df7933d7d..8505aa611 100644 --- a/src/Persistence/Csv.php +++ b/src/Persistence/Csv.php @@ -260,7 +260,9 @@ protected function insertRaw(Model $model, array $dataRaw) $this->putLine($line); - return $model->idField ? $dataRaw[$model->idField] : null; + return $model->idField + ? $dataRaw[$model->idField] + : null; } /** From 07b918cc1298065c9f55d9893c11e6407cfea6ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 15:22:11 +0100 Subject: [PATCH 14/15] cover more dumps --- src/Model.php | 10 +++--- tests/ExpressionSqlTest.php | 11 +++++++ tests/Persistence/Sql/ExpressionTest.php | 11 ++++--- tests/Persistence/Sql/QueryTest.php | 27 +++------------- tests/RandomTest.php | 39 ++++++++++++++++++++++++ 5 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/Model.php b/src/Model.php index acd3356d3..e06da1cdc 100644 --- a/src/Model.php +++ b/src/Model.php @@ -2067,14 +2067,12 @@ public function __debugInfo(): array { if ($this->isEntity()) { return [ + 'model' => $this->getModel()->__debugInfo(), 'entityId' => $this->idField && $this->hasField($this->idField) - ? ($this->_entityId !== null - ? ($this->getId() !== null - ? $this->_entityId - : $this->_entityId . ' (unloaded)') - : 'null') + ? ($this->_entityId === null || $this->getId() !== null + ? $this->getId() + : 'unloaded (' . (is_object($this->_entityId) ? get_class($this->_entityId) : $this->_entityId) . ')') : 'no id field', - 'model' => $this->getModel()->__debugInfo(), ]; } diff --git a/tests/ExpressionSqlTest.php b/tests/ExpressionSqlTest.php index d6faae115..47af46cf6 100644 --- a/tests/ExpressionSqlTest.php +++ b/tests/ExpressionSqlTest.php @@ -154,6 +154,17 @@ public function testExpressions(): void self::assertSame('Sue', $mm->get('name')); } + public function testVarDump(): void + { + $m = new Model($this->db, ['table' => false]); + $m->addExpression('x', ['expr' => '2 + 3']); + + self::assertSame( + '2 + 3', + $m->getField('x')->__debugInfo()['expr'] + ); + } + public function testReloading(): void { $this->setDb($dbData = [ diff --git a/tests/Persistence/Sql/ExpressionTest.php b/tests/Persistence/Sql/ExpressionTest.php index be0436eac..b78d40c45 100644 --- a/tests/Persistence/Sql/ExpressionTest.php +++ b/tests/Persistence/Sql/ExpressionTest.php @@ -393,16 +393,19 @@ public function escapeParam($value): string ], $e->render()); } - public function testVarDump(): void + public function testVarDumpBasic(): void { self::assertSame( 'test', $this->e('test')->__debugInfo()['R'] ); + } - self::assertStringContainsString( - 'Expression could not render tag', - $this->e(' [nosuchtag] ')->__debugInfo()['R'] + public function testVarDumpException(): void + { + self::assertSame( + Exception::class . ': Expression could not render tag', + $this->e('test [noSuchTag] ')->__debugInfo()['R'] ); } diff --git a/tests/Persistence/Sql/QueryTest.php b/tests/Persistence/Sql/QueryTest.php index b4974aad0..4bb7c07b9 100644 --- a/tests/Persistence/Sql/QueryTest.php +++ b/tests/Persistence/Sql/QueryTest.php @@ -458,35 +458,18 @@ public function testGetDebugQuery(): void ); } - public function testVarDump(): void + public function testVarDumpBasic(): void { self::assertMatchesRegularExpression( - '~select\s+\*\s+from\s*"user"~', + '~^select\s+\*\s+from\s*"user"$~', $this->q()->table('user')->__debugInfo()['R'] ); } - public function testVarDump2(): void + public function testVarDumpException(): void { - self::assertStringContainsString( - 'Expression could not render tag', - $this->e('Hello [world]')->__debugInfo()['R'] - ); - } - - public function testVarDump3(): void - { - self::assertStringContainsString( - 'Hello \'php\'', - $this->e('Hello [world]', ['world' => 'php'])->__debugInfo()['R'] - ); - } - - public function testVarDump4(): void - { - // should throw exception "Table cannot be Query in UPDATE, INSERT etc. query modes" - self::assertStringContainsString( - 'Table cannot be Query', + self::assertSame( + Exception::class . ': Table cannot be Query in UPDATE, INSERT etc. query modes', $this->q() ->mode('update') ->table($this->q()->table('test'), 'foo')->__debugInfo()['R'] diff --git a/tests/RandomTest.php b/tests/RandomTest.php index 870c683a5..7e4af31ea 100644 --- a/tests/RandomTest.php +++ b/tests/RandomTest.php @@ -489,6 +489,45 @@ public function testDuplicateSaveNew(): void ], $m->export()); } + public function testVarDumpModel(): void + { + $m = new Model($this->db, ['table' => 'user']); + + $dump = $m->__debugInfo(); + self::assertSame('user', $dump['table']); + self::assertArrayNotHasKey('entityId', $dump); + } + + public function testVarDumpEntityBasic(): void + { + $m = new Model($this->db, ['table' => 'user']); + $entity = $m->createEntity(); + + $dump = $entity->__debugInfo(); + self::assertSame('user', $dump['model']['table']); + self::assertArrayNotHasKey('table', $dump); + self::assertSame(null, $dump['entityId']); + + $entity->setId(10); + self::assertSame(10, $entity->__debugInfo()['entityId']); + + $entity->setId(null); + self::assertSame('unloaded (10)', $entity->__debugInfo()['entityId']); + } + + public function testVarDumpEntityWithObjectId(): void + { + $m = new Model($this->db, ['table' => 'user']); + $m->getIdField()->type = 'datetime'; + $entity = $m->createEntity(); + + $entity->setId(new \DateTime()); + self::assertSame($entity->getId(), $entity->__debugInfo()['entityId']); + + $entity->setId(null); + self::assertSame('unloaded (DateTime)', $entity->__debugInfo()['entityId']); + } + public function testNoWriteActionInsert(): void { $this->expectException(Exception::class); From 9aab780abf019fb18b34cdc4a33f65fd189b057f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Mon, 19 Feb 2024 15:40:28 +0100 Subject: [PATCH 15/15] fix cs --- tests/RandomTest.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/RandomTest.php b/tests/RandomTest.php index 7e4af31ea..5d09e95ad 100644 --- a/tests/RandomTest.php +++ b/tests/RandomTest.php @@ -506,7 +506,7 @@ public function testVarDumpEntityBasic(): void $dump = $entity->__debugInfo(); self::assertSame('user', $dump['model']['table']); self::assertArrayNotHasKey('table', $dump); - self::assertSame(null, $dump['entityId']); + self::assertNull($dump['entityId']); $entity->setId(10); self::assertSame(10, $entity->__debugInfo()['entityId']); @@ -521,6 +521,8 @@ public function testVarDumpEntityWithObjectId(): void $m->getIdField()->type = 'datetime'; $entity = $m->createEntity(); + self::assertNull($entity->__debugInfo()['entityId']); + $entity->setId(new \DateTime()); self::assertSame($entity->getId(), $entity->__debugInfo()['entityId']);