Skip to content

Commit

Permalink
Use Model for Join write operations
Browse files Browse the repository at this point in the history
  • Loading branch information
mvorisek committed Jan 16, 2022
1 parent 6c64608 commit c4bf8af
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 55 deletions.
4 changes: 2 additions & 2 deletions docs/persistence/sql/queries.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
49 changes: 41 additions & 8 deletions src/Model/Join.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -120,9 +119,9 @@ class Join
/** @var array<int, array<string, mixed>> 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) {
Expand All @@ -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
*
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -280,23 +313,23 @@ public function addFields(array $fields = [], array $defaults = [])
*
* @param array<string, mixed> $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);
}

/**
* Another leftJoin will be attached to a current join.
*
* @param array<string, mixed> $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);
}

/**
Expand Down
74 changes: 33 additions & 41 deletions src/Persistence/Sql/Join.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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));
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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();
}
});
}
}

0 comments on commit c4bf8af

Please sign in to comment.