From c4bf8afdf18b7e70ee023a154c2ec40827f44f27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= Date: Sun, 16 Jan 2022 16:26:02 +0100 Subject: [PATCH] Use Model for Join write operations --- docs/persistence/sql/queries.rst | 4 +- phpstan.neon.dist | 3 -- src/Model.php | 2 +- src/Model/Join.php | 49 +++++++++++++++++---- src/Persistence/Sql/Join.php | 74 ++++++++++++++------------------ 5 files changed, 77 insertions(+), 55 deletions(-) diff --git a/docs/persistence/sql/queries.rst b/docs/persistence/sql/queries.rst index d2528703c..a204add96 100644 --- a/docs/persistence/sql/queries.rst +++ b/docs/persistence/sql/queries.rst @@ -473,11 +473,11 @@ and overwrite this simple method to support expressions like this, for example: Joining with other tables ------------------------- -.. php:method:: join($foreign_table, $master_field, $join_kind) +.. php:method:: join($foreignTable, $master_field, $join_kind) Join results with additional table using "JOIN" statement in your query. - :param string|array $foreign_table: table to join (may include field and alias) + :param string|array $foreignTable: table to join (may include field and alias) :param mixed $master_field: main field (and table) to join on or Expression :param string $join_kind: 'left' (default), 'inner', 'right' etc - which join type to use :returns: $this diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 3adbf85c1..0cb9343bf 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -53,9 +53,6 @@ parameters: # for src/Persistence/Sql.php - '~^Call to an undefined method Atk4\\Data\\Persistence::expr\(\)\.$~' - '~^Call to an undefined method Atk4\\Data\\Persistence::exprNow\(\)\.$~' - # for src/Persistence/Sql/Join.php - - '~^Call to an undefined method Atk4\\Data\\Persistence::initQuery\(\)\.$~' - - '~^Call to an undefined method Atk4\\Data\\Persistence::lastInsertId\(\)\.$~' # for src/Reference/HasMany.php - '~^Call to an undefined method Atk4\\Data\\Model::dsql\(\)\.$~' # for tests/FieldTest.php diff --git a/src/Model.php b/src/Model.php index 75e2da3ae..046fa802f 100644 --- a/src/Model.php +++ b/src/Model.php @@ -1074,7 +1074,7 @@ public function withId($id) */ public function addWith(self $model, string $alias, array $mapping = [], bool $recursive = false) { - if (isset($this->with[$alias])) { + if ($alias === $this->table || $alias === $this->table_alias || isset($this->with[$alias])) { throw (new Exception('With cursor already set with given alias')) ->addMoreInfo('alias', $alias); } diff --git a/src/Model/Join.php b/src/Model/Join.php index f5da760d3..916f30c29 100644 --- a/src/Model/Join.php +++ b/src/Model/Join.php @@ -31,8 +31,7 @@ class Join } /** - * Name of the table (or collection) that can be used to retrieve data from. - * For SQL, This can also be an expression or sub-select. + * Foreign model or WITH/CTE alias when used with SQL persistence. * * @var string */ @@ -120,9 +119,9 @@ class Join /** @var array> Data indexed by spl_object_id(entity) which is populated here as the save/insert progresses. */ private $saveBufferByOid = []; - public function __construct(string $foreign_table = null) + public function __construct(string $foreignTable = null) { - $this->foreign_table = $foreign_table; + $this->foreign_table = $foreignTable; // handle foreign table containing a dot - that will be reverse join if (strpos($this->foreign_table, '.') !== false) { @@ -132,6 +131,38 @@ public function __construct(string $foreign_table = null) } } + /** + * Create fake foreign model, in the future, this method should be removed + * in favor of always requiring an object model. + */ + protected function createFakeForeignModel(): Model + { + $fakeModel = new Model($this->getOwner()->persistence, [ + 'table' => $this->foreign_table, + 'id_field' => $this->foreign_field, + ]); + foreach ($this->getOwner()->getFields() as $ownerField) { + if ($ownerField->hasJoin() && $ownerField->getJoin()->short_name === $this->short_name) { + $fakeModel->addField($ownerField->short_name, [ + 'actual' => $ownerField->actual, + 'type' => $ownerField->type, + ]); + } + } + + return $fakeModel; + } + + public function getForeignModel(): Model + { + // TODO this should be removed in the future + if (!isset($this->getOwner()->with[$this->foreign_table])) { + return $this->createFakeForeignModel(); + } + + return $this->getOwner()->with[$this->foreign_table]['model']; + } + /** * @param Model $owner * @@ -203,6 +234,8 @@ protected function init(): void { $this->_init(); + $this->getForeignModel(); // assert valid foreign_table + // owner model should have id_field set $id_field = $this->getOwner()->id_field; if (!$id_field) { @@ -280,11 +313,11 @@ public function addFields(array $fields = [], array $defaults = []) * * @param array $defaults */ - public function join(string $foreign_table, array $defaults = []): self + public function join(string $foreignTable, array $defaults = []): self { $defaults['joinName'] = $this->short_name; - return $this->getOwner()->join($foreign_table, $defaults); + return $this->getOwner()->join($foreignTable, $defaults); } /** @@ -292,11 +325,11 @@ public function join(string $foreign_table, array $defaults = []): self * * @param array $defaults */ - public function leftJoin(string $foreign_table, array $defaults = []): self + public function leftJoin(string $foreignTable, array $defaults = []): self { $defaults['joinName'] = $this->short_name; - return $this->getOwner()->leftJoin($foreign_table, $defaults); + return $this->getOwner()->leftJoin($foreignTable, $defaults); } /** diff --git a/src/Persistence/Sql/Join.php b/src/Persistence/Sql/Join.php index 8a0ed7e53..e8b59c50a 100644 --- a/src/Persistence/Sql/Join.php +++ b/src/Persistence/Sql/Join.php @@ -71,16 +71,6 @@ protected function init(): void } } - /** - * Returns DSQL query. - */ - public function dsql(): Query - { - $dsql = $this->getOwner()->persistence->initQuery($this->getOwner()); - - return $dsql->reset('table')->table($this->foreign_table, $this->foreign_alias); - } - /** * Before query is executed, this method will be called. */ @@ -137,17 +127,18 @@ public function beforeInsert(Model $entity, array &$data): void $model = $this->getOwner(); - // The value for the master_field is set, so we are going to use existing record anyway - if ($model->hasField($this->master_field) && $entity->get($this->master_field)) { + // the value for the master_field is set, so we are going to use existing record anyway + if ($model->hasField($this->master_field) && $entity->get($this->master_field) !== null) { return; } - $query = $this->dsql(); - $query->mode('insert'); - $query->setMulti($model->persistence->typecastSaveRow($model, $this->getAndUnsetSaveBuffer($entity))); - // $query->set($this->foreign_field, null); - $query->mode('insert')->execute(); // TODO IMPORTANT migrate to Model insert - $this->setId($entity, $model->persistence->lastInsertId(new Model($model->persistence, ['table' => $this->foreign_table]))); + $foreignModel = $this->getForeignModel(); + $foreignEntity = $foreignModel->createEntity() + ->setMulti($this->getAndUnsetSaveBuffer($entity)) + /*->set($this->foreign_field, null)*/; + $foreignEntity->save(); + + $this->setId($entity, $foreignEntity->getId()); if ($this->hasJoin()) { $this->getJoin()->setSaveBufferValue($entity, $this->master_field, $this->getId($entity)); @@ -162,18 +153,13 @@ public function afterInsert(Model $entity): void return; } - $model = $this->getOwner(); + $foreignModel = $this->getForeignModel(); + $foreignEntity = $foreignModel->createEntity() + ->setMulti($this->getAndUnsetSaveBuffer($entity)) + ->set($this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId()); + $foreignEntity->save(); - $query = $this->dsql(); - $query->setMulti($model->persistence->typecastSaveRow($model, $this->getAndUnsetSaveBuffer($entity))); - $query->set($this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId()); - $query->mode('insert')->execute(); // TODO IMPORTANT migrate to Model insert - $modelForLastInsertId = $model; - while (is_object($modelForLastInsertId->table)) { - $modelForLastInsertId = $modelForLastInsertId->table; - } - // assumes same ID field across all nested models (not needed once migrated to Model insert) - $this->setId($entity, $model->persistence->lastInsertId($modelForLastInsertId)); + $this->setId($entity, $entity->getId()); // TODO why is this here? it seems to be not needed } public function beforeUpdate(Model $entity, array &$data): void @@ -186,14 +172,16 @@ public function beforeUpdate(Model $entity, array &$data): void return; } - $model = $this->getOwner(); - - $query = $this->dsql(); - $query->setMulti($model->persistence->typecastSaveRow($model, $this->getAndUnsetSaveBuffer($entity))); - - $id = $this->reverse ? $entity->getId() : $entity->get($this->master_field); - - $query->where($this->foreign_field, $id)->mode('update')->execute(); // TODO IMPORTANT migrate to Model update + $foreignModel = $this->getForeignModel(); + $foreignId = $this->reverse ? $entity->getId() : $entity->get($this->master_field); + $saveBuffer = $this->getAndUnsetSaveBuffer($entity); + $foreignModel->atomic(function () use ($foreignModel, $foreignId, $saveBuffer) { + $foreignModel = (clone $foreignModel)->addCondition($this->foreign_field, $foreignId); + foreach ($foreignModel as $foreignEntity) { + $foreignEntity->setMulti($saveBuffer); + $foreignEntity->save(); + } + }); } public function doDelete(Model $entity): void @@ -202,9 +190,13 @@ public function doDelete(Model $entity): void return; } - $query = $this->dsql(); - $id = $this->reverse ? $entity->getId() : $entity->get($this->master_field); - - $query->where($this->foreign_field, $id)->mode('delete')->execute(); // TODO IMPORTANT migrate to Model delete + $foreignModel = $this->getForeignModel(); + $foreignId = $this->reverse ? $entity->getId() : $entity->get($this->master_field); + $foreignModel->atomic(function () use ($foreignModel, $foreignId) { + $foreignModel = (clone $foreignModel)->addCondition($this->foreign_field, $foreignId); + foreach ($foreignModel as $foreignEntity) { + $foreignEntity->delete(); + } + }); } }