Skip to content

Commit 45b4146

Browse files
mvorisekmvorisek
authored
Improve join temporary data cleanup (#1120)
1 parent b3e9ea8 commit 45b4146

File tree

7 files changed

+107
-138
lines changed

7 files changed

+107
-138
lines changed

‎src/Model.php

+15-19
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class Model implements \IteratorAggregate
8888
// {{{ Properties of the class
8989

9090
/** @var static|null not-null if and only if this instance is an entity */
91-
private $_model;
91+
private ?self $_model = null;
9292

9393
/** @var mixed once set, loading a different ID will result in an error */
9494
private $_entityId;
@@ -222,13 +222,11 @@ class Model implements \IteratorAggregate
222222
* If this model is "contained into" another entity by using ContainsOne
223223
* or ContainsMany reference, then this property will contain reference
224224
* to owning entity.
225-
*
226-
* @var Model|null
227225
*/
228-
public $containedInEntity;
226+
public ?self $containedInEntity = null;
229227

230-
/** @var Reference Only for Reference class */
231-
public $ownerReference;
228+
/** Only for Reference class */
229+
public ?Reference $ownerReference = null;
232230

233231
// }}}
234232

@@ -346,7 +344,7 @@ protected function getModelOnlyProperties(): array
346344
'_hookIndexCounter',
347345
'_hookOrigThis',
348346

349-
'ownerReference', // should be removed once references/joins are non-entity
347+
'ownerReference', // should be removed once references are non-entity
350348
'userActions', // should be removed once user actions are non-entity
351349

352350
'containedInEntity',
@@ -1495,8 +1493,6 @@ public function save(array $data = [])
14951493
$this->setMulti($data);
14961494

14971495
return $this->atomic(function () {
1498-
$dirtyRef = &$this->getDirtyRef();
1499-
15001496
$errors = $this->validate('save');
15011497
if ($errors !== []) {
15021498
throw new ValidationException($errors, $this);
@@ -1514,9 +1510,7 @@ public function save(array $data = [])
15141510
continue;
15151511
}
15161512

1517-
if ($field->hasJoin()) {
1518-
$field->getJoin()->setSaveBufferValue($this, $name, $value);
1519-
} else {
1513+
if (!$field->hasJoin()) {
15201514
$data[$name] = $value;
15211515
}
15221516
}
@@ -1534,23 +1528,24 @@ public function save(array $data = [])
15341528
} else {
15351529
$data = [];
15361530
$dirtyJoin = false;
1537-
foreach ($dirtyRef as $name => $ignore) {
1531+
foreach ($this->get() as $name => $value) {
1532+
if (!array_key_exists($name, $this->getDirtyRef())) {
1533+
continue;
1534+
}
1535+
15381536
$field = $this->getField($name);
15391537
if ($field->readOnly || $field->neverPersist || $field->neverSave) {
15401538
continue;
15411539
}
15421540

1543-
$value = $this->get($name);
1544-
15451541
if ($field->hasJoin()) {
15461542
$dirtyJoin = true;
1547-
$field->getJoin()->setSaveBufferValue($this, $name, $value);
15481543
} else {
15491544
$data[$name] = $value;
15501545
}
15511546
}
15521547

1553-
// No save needed, nothing was changed
1548+
// no save needed, nothing was changed
15541549
if (count($data) === 0 && !$dirtyJoin) {
15551550
return $this;
15561551
}
@@ -1563,6 +1558,7 @@ public function save(array $data = [])
15631558
$this->hook(self::HOOK_AFTER_UPDATE, [&$data]);
15641559
}
15651560

1561+
$dirtyRef = &$this->getDirtyRef();
15661562
$dirtyRef = [];
15671563

15681564
if ($this->idField && $this->reloadAfterSave) {
@@ -1688,7 +1684,7 @@ public function export(array $fields = null, string $keyField = null, bool $type
16881684
$fields = [];
16891685

16901686
if ($this->onlyFields !== null) {
1691-
// Add requested fields first
1687+
// add requested fields first
16921688
foreach ($this->onlyFields as $field) {
16931689
$fObject = $this->getField($field);
16941690
if ($fObject->neverPersist) {
@@ -1709,7 +1705,7 @@ public function export(array $fields = null, string $keyField = null, bool $type
17091705

17101706
$fields = array_keys($fields);
17111707
} else {
1712-
// Add all model fields
1708+
// add all model fields
17131709
foreach ($this->getFields() as $field => $fObject) {
17141710
if ($fObject->neverPersist) {
17151711
continue;

‎src/Model/Join.php

+61-79
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ abstract class Join
4444
protected $kind;
4545

4646
/** Weak join does not update foreign table. */
47-
protected bool $weak = false;
47+
public bool $weak = false;
4848

4949
/**
5050
* Normally the foreign table is saved first, then it's ID is used in the
@@ -59,7 +59,7 @@ abstract class Join
5959
* of saving and delete needs to be reversed. In this case $reverse
6060
* will be set to `true`. You can specify value of this property.
6161
*/
62-
protected bool $reverse = false;
62+
public bool $reverse = false;
6363

6464
/**
6565
* Field to be used for matching inside master table.
@@ -85,9 +85,6 @@ abstract class Join
8585
*/
8686
protected string $prefix = '';
8787

88-
/** @var mixed ID indexed by spl_object_id(entity) used by a joined table. */
89-
protected $idByOid;
90-
9188
/** @var array<int, array<string, mixed>> Data indexed by spl_object_id(entity) which is populated here as the save/insert progresses. */
9289
private array $saveBufferByOid = [];
9390

@@ -234,7 +231,7 @@ protected function init(): void
234231
->addMoreInfo('model', $this->getOwner());
235232
}
236233

237-
if ($this->reverse === true) {
234+
if ($this->reverse) {
238235
if ($this->masterField && $this->masterField !== $idField) { // TODO not implemented yet, see https://github.com/atk4/data/issues/803
239236
throw (new Exception('Joining tables on non-id fields is not implemented yet'))
240237
->addMoreInfo('masterField', $this->masterField)
@@ -271,16 +268,25 @@ protected function init(): void
271268
protected function initJoinHooks(): void
272269
{
273270
$this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
274-
$this->onHookToOwnerEntity(Model::HOOK_AFTER_UNLOAD, \Closure::fromCallable([$this, 'afterUnload']));
271+
272+
$createHookFxWithCleanup = function (string $methodName): \Closure {
273+
return function (Model $entity, &...$args) use ($methodName): void {
274+
try {
275+
$this->{$methodName}($entity, ...$args);
276+
} finally {
277+
$this->unsetSaveBuffer($entity);
278+
}
279+
};
280+
};
275281

276282
if ($this->reverse) {
277-
$this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, \Closure::fromCallable([$this, 'afterInsert']), [], -5);
278-
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
279-
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_DELETE, \Closure::fromCallable([$this, 'doDelete']), [], -5);
283+
$this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, $createHookFxWithCleanup('afterInsert'), [], -5);
284+
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, $createHookFxWithCleanup('beforeUpdate'), [], -5);
285+
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_DELETE, $createHookFxWithCleanup('beforeDelete'), [], -5);
280286
} else {
281-
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, \Closure::fromCallable([$this, 'beforeInsert']), [], -5);
282-
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
283-
$this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, \Closure::fromCallable([$this, 'doDelete']));
287+
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, $createHookFxWithCleanup('beforeInsert'), [], -5);
288+
$this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, $createHookFxWithCleanup('beforeUpdate'), [], -5);
289+
$this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, $createHookFxWithCleanup('beforeDelete'));
284290
}
285291
}
286292

@@ -418,36 +424,6 @@ protected function assertReferenceIdNotNull($value): void
418424
}
419425
}
420426

421-
/**
422-
* @return mixed
423-
*
424-
* @internal should be not used outside atk4/data
425-
*/
426-
protected function getId(Model $entity)
427-
{
428-
return $this->idByOid[spl_object_id($entity)];
429-
}
430-
431-
/**
432-
* @param mixed $id
433-
*
434-
* @internal should be not used outside atk4/data
435-
*/
436-
protected function setId(Model $entity, $id): void
437-
{
438-
$this->assertReferenceIdNotNull($id);
439-
440-
$this->idByOid[spl_object_id($entity)] = $id;
441-
}
442-
443-
/**
444-
* @internal should be not used outside atk4/data
445-
*/
446-
protected function unsetId(Model $entity): void
447-
{
448-
unset($this->idByOid[spl_object_id($entity)]);
449-
}
450-
451427
/**
452428
* @internal should be not used outside atk4/data
453429
*/
@@ -461,7 +437,7 @@ protected function issetSaveBuffer(Model $entity): bool
461437
*
462438
* @internal should be not used outside atk4/data
463439
*/
464-
protected function getReindexedDataAndUnsetSaveBuffer(Model $entity): array
440+
protected function getAndUnsetReindexedSaveBuffer(Model $entity): array
465441
{
466442
$resOur = $this->saveBufferByOid[spl_object_id($entity)];
467443
$this->unsetSaveBuffer($entity);
@@ -474,18 +450,10 @@ protected function getReindexedDataAndUnsetSaveBuffer(Model $entity): array
474450
return $res;
475451
}
476452

477-
/**
478-
* @internal should be not used outside atk4/data
479-
*/
480-
protected function unsetSaveBuffer(Model $entity): void
481-
{
482-
unset($this->saveBufferByOid[spl_object_id($entity)]);
483-
}
484-
485453
/**
486454
* @param mixed $value
487455
*/
488-
public function setSaveBufferValue(Model $entity, string $fieldName, $value): void
456+
protected function setSaveBufferValue(Model $entity, string $fieldName, $value): void
489457
{
490458
$entity->assertIsEntity($this->getOwner());
491459

@@ -496,26 +464,44 @@ public function setSaveBufferValue(Model $entity, string $fieldName, $value): vo
496464
$this->saveBufferByOid[spl_object_id($entity)][$fieldName] = $value;
497465
}
498466

499-
public function afterLoad(Model $entity): void
467+
/**
468+
* @internal should be not used outside atk4/data
469+
*/
470+
protected function unsetSaveBuffer(Model $entity): void
500471
{
472+
unset($this->saveBufferByOid[spl_object_id($entity)]);
501473
}
502474

503-
protected function afterUnload(Model $entity): void
475+
protected function afterLoad(Model $entity): void
504476
{
505-
$this->unsetId($entity);
506-
$this->unsetSaveBuffer($entity);
477+
}
478+
479+
protected function initSaveBuffer(Model $entity, bool $fromUpdate): void
480+
{
481+
foreach ($entity->get() as $name => $value) {
482+
$field = $entity->getField($name);
483+
if (!$field->hasJoin() || $field->getJoin()->shortName !== $this->shortName || $field->readOnly || $field->neverPersist || $field->neverSave) {
484+
continue;
485+
}
486+
487+
if ($fromUpdate && !$entity->isDirty($name)) {
488+
continue;
489+
}
490+
491+
$field->getJoin()->setSaveBufferValue($entity, $name, $value);
492+
}
507493
}
508494

509495
/**
510496
* @param array<string, mixed> $data
511497
*/
512-
public function beforeInsert(Model $entity, array &$data): void
498+
protected function beforeInsert(Model $entity, array &$data): void
513499
{
514500
if ($this->weak) {
515501
return;
516502
}
517503

518-
$model = $this->getOwner();
504+
$this->initSaveBuffer($entity, false);
519505

520506
// the value for the masterField is set, so we are going to use existing record anyway
521507
if ($entity->get($this->masterField) !== null) {
@@ -524,69 +510,67 @@ public function beforeInsert(Model $entity, array &$data): void
524510

525511
$foreignModel = $this->getForeignModel();
526512
$foreignEntity = $foreignModel->createEntity()
527-
->setMulti($this->getReindexedDataAndUnsetSaveBuffer($entity))
528-
/* ->set($this->foreignField, null) */;
513+
->setMulti($this->getAndUnsetReindexedSaveBuffer($entity))
514+
->setNull($this->foreignField);
529515
$foreignEntity->save();
530516

531-
$this->setId($entity, $foreignEntity->getId());
517+
$foreignId = $foreignEntity->getId();
518+
$this->assertReferenceIdNotNull($foreignId);
532519

533520
if ($this->hasJoin()) {
534-
$this->getJoin()->setSaveBufferValue($entity, $this->masterField, $this->getId($entity));
521+
$this->getJoin()->setSaveBufferValue($entity, $this->masterField, $foreignId);
535522
} else {
536-
$data[$this->masterField] = $this->getId($entity);
523+
$data[$this->masterField] = $foreignId;
537524
}
538-
539-
// $entity->set($this->masterField, $this->getId($entity)); // TODO needed? from array persistence
540525
}
541526

542-
public function afterInsert(Model $entity): void
527+
protected function afterInsert(Model $entity): void
543528
{
544529
if ($this->weak) {
545530
return;
546531
}
547532

548-
$id = $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId();
533+
$this->initSaveBuffer($entity, false);
534+
535+
$id = $entity->getId();
549536
$this->assertReferenceIdNotNull($id);
550-
// $this->setSaveBufferValue($entity, $this->masterField, $id); // TODO needed? from array persistence
551537

552538
$foreignModel = $this->getForeignModel();
553539
$foreignEntity = $foreignModel->createEntity()
554-
->setMulti($this->getReindexedDataAndUnsetSaveBuffer($entity))
540+
->setMulti($this->getAndUnsetReindexedSaveBuffer($entity))
555541
->set($this->foreignField, $id);
556542
$foreignEntity->save();
557-
558-
$this->setId($entity, $entity->getId()); // TODO why is this here? it seems to be not needed
559543
}
560544

561545
/**
562546
* @param array<string, mixed> $data
563547
*/
564-
public function beforeUpdate(Model $entity, array &$data): void
548+
protected function beforeUpdate(Model $entity, array &$data): void
565549
{
566550
if ($this->weak) {
567551
return;
568552
}
569553

554+
$this->initSaveBuffer($entity, true);
555+
570556
if (!$this->issetSaveBuffer($entity)) {
571557
return;
572558
}
573559

574560
$foreignModel = $this->getForeignModel();
575561
$foreignId = $this->reverse ? $entity->getId() : $entity->get($this->masterField);
576562
$this->assertReferenceIdNotNull($foreignId);
577-
$saveBuffer = $this->getReindexedDataAndUnsetSaveBuffer($entity);
563+
$saveBuffer = $this->getAndUnsetReindexedSaveBuffer($entity);
578564
$foreignModel->atomic(function () use ($foreignModel, $foreignId, $saveBuffer) {
579565
$foreignModel = (clone $foreignModel)->addCondition($this->foreignField, $foreignId);
580566
foreach ($foreignModel as $foreignEntity) {
581567
$foreignEntity->setMulti($saveBuffer);
582568
$foreignEntity->save();
583569
}
584570
});
585-
586-
// $this->setId($entity, ??); // TODO needed? from array persistence
587571
}
588572

589-
public function doDelete(Model $entity): void
573+
protected function beforeDelete(Model $entity): void
590574
{
591575
if ($this->weak) {
592576
return;
@@ -601,7 +585,5 @@ public function doDelete(Model $entity): void
601585
$foreignEntity->delete();
602586
}
603587
});
604-
605-
$this->unsetId($entity); // TODO needed? from array persistence
606588
}
607589
}

0 commit comments

Comments
 (0)