From 6c6460870332ff12b2abef5ecc52653d674256c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 02:30:00 +0100
Subject: [PATCH 01/19] improve Model::$with phpdoc type

---
 src/Model.php           |  4 ++--
 src/Persistence/Sql.php | 11 +++++------
 2 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/Model.php b/src/Model.php
index bd1bed894..75e2da3ae 100644
--- a/src/Model.php
+++ b/src/Model.php
@@ -147,7 +147,7 @@ class Model implements \IteratorAggregate
     /** @var array Array of set order by. */
     public $order = [];
 
-    /** @var array Array of WITH cursors set. */
+    /** @var array<string, array{'model': Model, 'mapping': array<int|string, string>, 'recursive': bool}> */
     public $with = [];
 
     /**
@@ -1075,7 +1075,7 @@ public function withId($id)
     public function addWith(self $model, string $alias, array $mapping = [], bool $recursive = false)
     {
         if (isset($this->with[$alias])) {
-            throw (new Exception('With cursor already set with this alias'))
+            throw (new Exception('With cursor already set with given alias'))
                 ->addMoreInfo('alias', $alias);
         }
 
diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php
index 70adfa514..62e5f6ba7 100644
--- a/src/Persistence/Sql.php
+++ b/src/Persistence/Sql.php
@@ -212,7 +212,6 @@ public function initQuery(Model $model): Query
             );
         }
 
-        // add With cursors
         $this->initWithCursors($model, $query);
 
         return $query;
@@ -223,11 +222,12 @@ public function initQuery(Model $model): Query
      */
     public function initWithCursors(Model $model, Query $query): void
     {
-        if (!$with = $model->with) {
+        $with = $model->with;
+        if (count($with) === 0) {
             return;
         }
 
-        foreach ($with as $alias => ['model' => $withModel, 'mapping' => $withMapping, 'recursive' => $recursive]) {
+        foreach ($with as $alias => ['model' => $withModel, 'mapping' => $withMapping, 'recursive' => $withRecursive]) {
             // prepare field names
             $fieldsFrom = $fieldsTo = [];
             foreach ($withMapping as $from => $to) {
@@ -237,14 +237,13 @@ public function initWithCursors(Model $model, Query $query): void
 
             // prepare sub-query
             if ($fieldsFrom) {
-                $withModel->setOnlyFields($fieldsFrom);
+                $withModel->setOnlyFields($fieldsFrom); // TODO this mutates model state
             }
             // 2nd parameter here strictly define which fields should be selected
             // as result system fields will not be added if they are not requested
             $subQuery = $withModel->action('select', [$fieldsFrom]);
 
-            // add With cursor
-            $query->with($subQuery, $alias, $fieldsTo ?: null, $recursive);
+            $query->with($subQuery, $alias, $fieldsTo ?: null, $withRecursive);
         }
     }
 

From c4bf8afdf18b7e70ee023a154c2ec40827f44f27 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 16:26:02 +0100
Subject: [PATCH 02/19] 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<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) {
@@ -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<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);
     }
 
     /**
@@ -292,11 +325,11 @@ public function join(string $foreign_table, array $defaults = []): self
      *
      * @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);
     }
 
     /**
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();
+            }
+        });
     }
 }

From 23014c550796cda29ce47e665fa4a2e491cbb560 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 16:30:03 +0100
Subject: [PATCH 03/19] simplify join code for array, no functional change

---
 src/Persistence/Array_/Join.php | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/Persistence/Array_/Join.php b/src/Persistence/Array_/Join.php
index 900d1b201..dd55079ab 100644
--- a/src/Persistence/Array_/Join.php
+++ b/src/Persistence/Array_/Join.php
@@ -88,10 +88,11 @@ public function beforeInsert(Model $entity, array &$data): void
         // Figure out where are we going to save data
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
-        $this->setId($entity, $persistence->insert(
+        $lastInsertedId = $persistence->insert(
             $this->makeFakeModelWithForeignTable(),
             $this->getAndUnsetSaveBuffer($entity)
-        ));
+        );
+        $this->setId($entity, $lastInsertedId);
 
         $data[$this->master_field] = $this->getId($entity);
 
@@ -108,10 +109,11 @@ public function afterInsert(Model $entity): void
 
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
-        $this->setId($entity, $persistence->insert(
+        $lastInsertedId = $persistence->insert(
             $this->makeFakeModelWithForeignTable(),
             $this->getAndUnsetSaveBuffer($entity)
-        ));
+        );
+        $this->setId($entity, $lastInsertedId);
     }
 
     public function beforeUpdate(Model $entity, array &$data): void
@@ -122,12 +124,12 @@ public function beforeUpdate(Model $entity, array &$data): void
 
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
-        // @phpstan-ignore-next-line TODO this cannot work, Persistence::update() returns void
-        $this->setId($entity, $persistence->update(
+        $persistence->update(
             $this->makeFakeModelWithForeignTable(),
             $this->getId($entity),
             $this->getAndUnsetSaveBuffer($entity)
-        ));
+        );
+        // $this->setId($entity, ??);
     }
 
     public function doDelete(Model $entity): void
@@ -142,7 +144,6 @@ public function doDelete(Model $entity): void
             $this->makeFakeModelWithForeignTable(),
             $this->getId($entity)
         );
-
         $this->unsetId($entity);
     }
 }

From 4f71fbccc1e37ee2b1e68e8405541081860e1dfb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 17:44:45 +0100
Subject: [PATCH 04/19] improve SQLLogger types

---
 src/Schema/TestCase.php | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Schema/TestCase.php b/src/Schema/TestCase.php
index 21ff2fca2..da1dcabf3 100644
--- a/src/Schema/TestCase.php
+++ b/src/Schema/TestCase.php
@@ -52,7 +52,7 @@ public function __construct(TestCase $testCase)
                     $this->testCaseWeakRef = \WeakReference::create($testCase);
                 }
 
-                public function startQuery($sql, $params = null, $types = null): void
+                public function startQuery($sql, array $params = null, array $types = null): void
                 {
                     if (!$this->testCaseWeakRef->get()->debug) {
                         return;

From 18006a9616dc24a83e48b005eb7e75a127121f1b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 18:05:34 +0100
Subject: [PATCH 05/19] dedup Array_\Join::makeFakeModelWithForeignTable()

---
 src/Model/Join.php              |  8 ++++++--
 src/Persistence/Array_/Join.php | 30 +++++-------------------------
 2 files changed, 11 insertions(+), 27 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 916f30c29..5546c9410 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -139,10 +139,14 @@ protected function createFakeForeignModel(): Model
     {
         $fakeModel = new Model($this->getOwner()->persistence, [
             'table' => $this->foreign_table,
-            'id_field' => $this->foreign_field,
         ]);
+        if ($fakeModel->id_field !== $this->foreign_field && $this->foreign_field !== null) {
+            $fakeModel->addField($this->foreign_field);
+        }
         foreach ($this->getOwner()->getFields() as $ownerField) {
-            if ($ownerField->hasJoin() && $ownerField->getJoin()->short_name === $this->short_name) {
+            if ($ownerField->hasJoin() && $ownerField->getJoin()->short_name === $this->short_name
+                && $ownerField->short_name !== $fakeModel->id_field
+                && $ownerField->short_name !== $this->foreign_field) {
                 $fakeModel->addField($ownerField->short_name, [
                     'actual' => $ownerField->actual,
                     'type' => $ownerField->type,
diff --git a/src/Persistence/Array_/Join.php b/src/Persistence/Array_/Join.php
index dd55079ab..2bda790e6 100644
--- a/src/Persistence/Array_/Join.php
+++ b/src/Persistence/Array_/Join.php
@@ -33,26 +33,6 @@ protected function init(): void
         }
     }
 
-    protected function makeFakeModelWithForeignTable(): Model
-    {
-        $this->getOwner()->assertIsModel();
-
-        $modelCloned = clone $this->getOwner();
-        foreach ($modelCloned->getFields() as $field) {
-            if ($field->hasJoin() && $field->getJoin()->foreign_table === $this->foreign_table) {
-                \Closure::bind(fn () => $field->joinName = null, null, \Atk4\Data\Field::class)();
-            } else {
-                $modelCloned->removeField($field->short_name);
-            }
-        }
-        $modelCloned->addField($this->id_field, ['type' => 'integer']);
-        $modelCloned->table = $this->foreign_table;
-
-        // @TODO hooks will be fixed on a cloned model, foreign_table string name should be replaced with object model
-
-        return $modelCloned;
-    }
-
     public function afterLoad(Model $entity): void
     {
         // we need to collect ID
@@ -63,7 +43,7 @@ public function afterLoad(Model $entity): void
 
         try {
             $data = Persistence\Array_::assertInstanceOf($this->getOwner()->persistence)
-                ->load($this->makeFakeModelWithForeignTable(), $this->getId($entity));
+                ->load($this->createFakeForeignModel(), $this->getId($entity));
         } catch (Exception $e) {
             throw (new Exception('Unable to load joined record', $e->getCode(), $e))
                 ->addMoreInfo('table', $this->foreign_table)
@@ -89,7 +69,7 @@ public function beforeInsert(Model $entity, array &$data): void
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
         $lastInsertedId = $persistence->insert(
-            $this->makeFakeModelWithForeignTable(),
+            $this->createFakeForeignModel(),
             $this->getAndUnsetSaveBuffer($entity)
         );
         $this->setId($entity, $lastInsertedId);
@@ -110,7 +90,7 @@ public function afterInsert(Model $entity): void
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
         $lastInsertedId = $persistence->insert(
-            $this->makeFakeModelWithForeignTable(),
+            $this->createFakeForeignModel(),
             $this->getAndUnsetSaveBuffer($entity)
         );
         $this->setId($entity, $lastInsertedId);
@@ -125,7 +105,7 @@ public function beforeUpdate(Model $entity, array &$data): void
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
         $persistence->update(
-            $this->makeFakeModelWithForeignTable(),
+            $this->createFakeForeignModel(),
             $this->getId($entity),
             $this->getAndUnsetSaveBuffer($entity)
         );
@@ -141,7 +121,7 @@ public function doDelete(Model $entity): void
         $persistence = $this->persistence ?: $this->getOwner()->persistence;
 
         $persistence->delete(
-            $this->makeFakeModelWithForeignTable(),
+            $this->createFakeForeignModel(),
             $this->getId($entity)
         );
         $this->unsetId($entity);

From e8a04d13c8deae331342b0d602e145e3e5f97e69 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 18:07:42 +0100
Subject: [PATCH 06/19] guess type only if needed

---
 src/Model/Join.php | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 5546c9410..8837a016d 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -140,9 +140,6 @@ protected function createFakeForeignModel(): Model
         $fakeModel = new Model($this->getOwner()->persistence, [
             'table' => $this->foreign_table,
         ]);
-        if ($fakeModel->id_field !== $this->foreign_field && $this->foreign_field !== null) {
-            $fakeModel->addField($this->foreign_field);
-        }
         foreach ($this->getOwner()->getFields() as $ownerField) {
             if ($ownerField->hasJoin() && $ownerField->getJoin()->short_name === $this->short_name
                 && $ownerField->short_name !== $fakeModel->id_field
@@ -153,6 +150,9 @@ protected function createFakeForeignModel(): Model
                 ]);
             }
         }
+        if ($fakeModel->id_field !== $this->foreign_field && $this->foreign_field !== null) {
+            $fakeModel->addField($this->foreign_field, ['type' => 'integer']);
+        }
 
         return $fakeModel;
     }

From 0a3b04f86bb7783c3d16638bbfe8e3565737a80d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 18:19:49 +0100
Subject: [PATCH 07/19] init hooks in initJoinHooks() method

---
 src/Model/Join.php              |  9 +++++++--
 src/Persistence/Array_/Join.php |  8 ++------
 src/Persistence/Sql/Join.php    | 35 +++++++++++++++++----------------
 3 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 8837a016d..4c4cdcd70 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -273,12 +273,17 @@ protected function init(): void
             }
         }
 
-        $this->onHookToOwnerEntity(Model::HOOK_AFTER_UNLOAD, \Closure::fromCallable([$this, 'afterUnload']));
-
         // if kind is not specified, figure out join type
         if (!$this->kind) {
             $this->kind = $this->weak ? 'left' : 'inner';
         }
+
+        $this->initJoinHooks();
+    }
+
+    protected function initJoinHooks(): void
+    {
+        $this->onHookToOwnerEntity(Model::HOOK_AFTER_UNLOAD, \Closure::fromCallable([$this, 'afterUnload']));
     }
 
     /**
diff --git a/src/Persistence/Array_/Join.php b/src/Persistence/Array_/Join.php
index 2bda790e6..9bc789f31 100644
--- a/src/Persistence/Array_/Join.php
+++ b/src/Persistence/Array_/Join.php
@@ -13,14 +13,10 @@
  */
 class Join extends Model\Join
 {
-    /**
-     * This method is to figure out stuff.
-     */
-    protected function init(): void
+    protected function initJoinHooks(): void
     {
-        parent::init();
+        parent::initJoinHooks();
 
-        // add necessary hooks
         if ($this->reverse) {
             $this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, \Closure::fromCallable([$this, 'afterInsert']), [], -5);
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
diff --git a/src/Persistence/Sql/Join.php b/src/Persistence/Sql/Join.php
index e8b59c50a..ccceed521 100644
--- a/src/Persistence/Sql/Join.php
+++ b/src/Persistence/Sql/Join.php
@@ -28,9 +28,6 @@ public function getDesiredName(): string
         return '_' . ($this->foreign_alias ?: $this->foreign_table);
     }
 
-    /**
-     * This method is to figure out stuff.
-     */
     protected function init(): void
     {
         parent::init();
@@ -42,28 +39,32 @@ protected function init(): void
             $this->foreign_alias = ($this->getOwner()->table_alias ?: '') . $this->short_name;
         }
 
+        // Master field indicates ID of the joined item. In the past it had to be
+        // defined as a physical field in the main table. Now it is a model field
+        // so you can use expressions or fields inside joined entities.
+        // If string specified here does not point to an existing model field
+        // a new basic field is inserted and marked hidden.
+        if (!$this->reverse && !$this->getOwner()->hasField($this->master_field)) {
+            $owner = $this->hasJoin() ? $this->getJoin() : $this->getOwner();
+
+            $field = $owner->addField($this->master_field, ['system' => true, 'read_only' => true]);
+
+            $this->master_field = $field->short_name;
+        }
+    }
+
+    protected function initJoinHooks(): void
+    {
+        parent::initJoinHooks();
+
         $this->onHookToOwnerBoth(Persistence\Sql::HOOK_INIT_SELECT_QUERY, \Closure::fromCallable([$this, 'initSelectQuery']));
 
-        // add necessary hooks
         if ($this->reverse) {
             $this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, \Closure::fromCallable([$this, 'afterInsert']));
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']));
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_DELETE, \Closure::fromCallable([$this, 'doDelete']), [], -5);
             $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
         } else {
-            // Master field indicates ID of the joined item. In the past it had to be
-            // defined as a physical field in the main table. Now it is a model field
-            // so you can use expressions or fields inside joined entities.
-            // If string specified here does not point to an existing model field
-            // a new basic field is inserted and marked hidden.
-            if (!$this->getOwner()->hasField($this->master_field)) {
-                $owner = $this->hasJoin() ? $this->getJoin() : $this->getOwner();
-
-                $field = $owner->addField($this->master_field, ['system' => true, 'read_only' => true]);
-
-                $this->master_field = $field->short_name;
-            }
-
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, \Closure::fromCallable([$this, 'beforeInsert']), [], -5);
             $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']));
             $this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, \Closure::fromCallable([$this, 'doDelete']));

From 47e461af5391565b45c2dbb407e74a53513ff84b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 18:41:56 +0100
Subject: [PATCH 08/19] move hooks init into abstract Join

---
 src/Model/Join.php              | 27 +++++++++++++++++++++++----
 src/Persistence/Array_/Join.php | 16 ----------------
 src/Persistence/Sql/Join.php    | 12 ------------
 3 files changed, 23 insertions(+), 32 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 4c4cdcd70..1b39c613c 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -19,7 +19,7 @@
  *
  * @method Model getOwner()
  */
-class Join
+abstract class Join
 {
     use DiContainerTrait;
     use InitializerTrait {
@@ -284,6 +284,18 @@ protected function init(): void
     protected function initJoinHooks(): void
     {
         $this->onHookToOwnerEntity(Model::HOOK_AFTER_UNLOAD, \Closure::fromCallable([$this, 'afterUnload']));
+
+        if ($this->reverse) {
+            $this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, \Closure::fromCallable([$this, 'afterInsert']), [], -5);
+            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
+            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_DELETE, \Closure::fromCallable([$this, 'doDelete']), [], -5);
+        } else {
+            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, \Closure::fromCallable([$this, 'beforeInsert']), [], -5);
+            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
+            $this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, \Closure::fromCallable([$this, 'doDelete']));
+
+            $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
+        }
     }
 
     /**
@@ -494,12 +506,19 @@ public function setSaveBufferValue(Model $entity, string $fieldName, $value): vo
         $this->saveBufferByOid[spl_object_id($entity)][$fieldName] = $value;
     }
 
-    /**
-     * Clears id and save buffer.
-     */
     protected function afterUnload(Model $entity): void
     {
         $this->unsetId($entity);
         $this->unsetSaveBuffer($entity);
     }
+
+    abstract public function afterLoad(Model $entity): void;
+
+    abstract public function beforeInsert(Model $entity, array &$data): void;
+
+    abstract public function afterInsert(Model $entity): void;
+
+    abstract public function beforeUpdate(Model $entity, array &$data): void;
+
+    abstract public function doDelete(Model $entity): void;
 }
diff --git a/src/Persistence/Array_/Join.php b/src/Persistence/Array_/Join.php
index 9bc789f31..b39646c48 100644
--- a/src/Persistence/Array_/Join.php
+++ b/src/Persistence/Array_/Join.php
@@ -13,22 +13,6 @@
  */
 class Join extends Model\Join
 {
-    protected function initJoinHooks(): void
-    {
-        parent::initJoinHooks();
-
-        if ($this->reverse) {
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, \Closure::fromCallable([$this, 'afterInsert']), [], -5);
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']), [], -5);
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_DELETE, \Closure::fromCallable([$this, 'doDelete']), [], -5);
-        } else {
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, \Closure::fromCallable([$this, 'beforeInsert']));
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']));
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, \Closure::fromCallable([$this, 'doDelete']));
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
-        }
-    }
-
     public function afterLoad(Model $entity): void
     {
         // we need to collect ID
diff --git a/src/Persistence/Sql/Join.php b/src/Persistence/Sql/Join.php
index ccceed521..670f79888 100644
--- a/src/Persistence/Sql/Join.php
+++ b/src/Persistence/Sql/Join.php
@@ -58,18 +58,6 @@ protected function initJoinHooks(): void
         parent::initJoinHooks();
 
         $this->onHookToOwnerBoth(Persistence\Sql::HOOK_INIT_SELECT_QUERY, \Closure::fromCallable([$this, 'initSelectQuery']));
-
-        if ($this->reverse) {
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_INSERT, \Closure::fromCallable([$this, 'afterInsert']));
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']));
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_DELETE, \Closure::fromCallable([$this, 'doDelete']), [], -5);
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
-        } else {
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_INSERT, \Closure::fromCallable([$this, 'beforeInsert']), [], -5);
-            $this->onHookToOwnerEntity(Model::HOOK_BEFORE_UPDATE, \Closure::fromCallable([$this, 'beforeUpdate']));
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_DELETE, \Closure::fromCallable([$this, 'doDelete']));
-            $this->onHookToOwnerEntity(Model::HOOK_AFTER_LOAD, \Closure::fromCallable([$this, 'afterLoad']));
-        }
     }
 
     /**

From b363b30ca7e2ea8e244ef12a5b056e59fe772d0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 18:56:39 +0100
Subject: [PATCH 09/19] dedup all join methods excl. afterLoad

---
 src/Model/Join.php              | 89 +++++++++++++++++++++++++++++++--
 src/Persistence/Array_/Join.php | 74 ---------------------------
 src/Persistence/Sql/Join.php    | 81 ------------------------------
 3 files changed, 85 insertions(+), 159 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 1b39c613c..5b1034bb8 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -514,11 +514,92 @@ protected function afterUnload(Model $entity): void
 
     abstract public function afterLoad(Model $entity): void;
 
-    abstract public function beforeInsert(Model $entity, array &$data): void;
+    public function beforeInsert(Model $entity, array &$data): void
+    {
+        if ($this->weak) {
+            return;
+        }
+
+        $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) !== null) {
+            return;
+        }
+
+        $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));
+        } else {
+            $data[$this->master_field] = $this->getId($entity);
+        }
+
+        // $entity->set($this->master_field, $this->getId($entity));
+    }
+
+    public function afterInsert(Model $entity): void
+    {
+        if ($this->weak) {
+            return;
+        }
+
+        $this->setSaveBufferValue($entity, $this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId()); // from array persistence...
 
-    abstract public function afterInsert(Model $entity): void;
+        $foreignModel = $this->getForeignModel();
+        $foreignEntity = $foreignModel->createEntity()
+            ->setMulti($this->getAndUnsetSaveBuffer($entity))
+            ->set($this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId());
+        $foreignEntity->save();
 
-    abstract public function beforeUpdate(Model $entity, array &$data): void;
+        $this->setId($entity, $entity->getId()); // TODO why is this here? it seems to be not needed
+    }
+
+    public function beforeUpdate(Model $entity, array &$data): void
+    {
+        if ($this->weak) {
+            return;
+        }
+
+        if (!$this->issetSaveBuffer($entity)) {
+            return;
+        }
+
+        $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();
+            }
+        });
 
-    abstract public function doDelete(Model $entity): void;
+        // $this->setId($entity, ??); // TODO needed? from array persistence
+    }
+
+    public function doDelete(Model $entity): void
+    {
+        if ($this->weak) {
+            return;
+        }
+
+        $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();
+            }
+        });
+
+        $this->unsetId($entity); // TODO needed? from array persistence
+    }
 }
diff --git a/src/Persistence/Array_/Join.php b/src/Persistence/Array_/Join.php
index b39646c48..6861303b9 100644
--- a/src/Persistence/Array_/Join.php
+++ b/src/Persistence/Array_/Join.php
@@ -32,78 +32,4 @@ public function afterLoad(Model $entity): void
         $dataRef = &$entity->getDataRef();
         $dataRef = array_merge($data, $entity->getDataRef());
     }
-
-    public function beforeInsert(Model $entity, array &$data): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        if ($entity->hasField($this->master_field) && $entity->get($this->master_field)) {
-            // The value for the master_field is set,
-            // we are going to use existing record.
-            return;
-        }
-
-        // Figure out where are we going to save data
-        $persistence = $this->persistence ?: $this->getOwner()->persistence;
-
-        $lastInsertedId = $persistence->insert(
-            $this->createFakeForeignModel(),
-            $this->getAndUnsetSaveBuffer($entity)
-        );
-        $this->setId($entity, $lastInsertedId);
-
-        $data[$this->master_field] = $this->getId($entity);
-
-        // $entity->set($this->master_field, $this->getId($entity));
-    }
-
-    public function afterInsert(Model $entity): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        $this->setSaveBufferValue($entity, $this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId());
-
-        $persistence = $this->persistence ?: $this->getOwner()->persistence;
-
-        $lastInsertedId = $persistence->insert(
-            $this->createFakeForeignModel(),
-            $this->getAndUnsetSaveBuffer($entity)
-        );
-        $this->setId($entity, $lastInsertedId);
-    }
-
-    public function beforeUpdate(Model $entity, array &$data): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        $persistence = $this->persistence ?: $this->getOwner()->persistence;
-
-        $persistence->update(
-            $this->createFakeForeignModel(),
-            $this->getId($entity),
-            $this->getAndUnsetSaveBuffer($entity)
-        );
-        // $this->setId($entity, ??);
-    }
-
-    public function doDelete(Model $entity): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        $persistence = $this->persistence ?: $this->getOwner()->persistence;
-
-        $persistence->delete(
-            $this->createFakeForeignModel(),
-            $this->getId($entity)
-        );
-        $this->unsetId($entity);
-    }
 }
diff --git a/src/Persistence/Sql/Join.php b/src/Persistence/Sql/Join.php
index 670f79888..d41aecdd2 100644
--- a/src/Persistence/Sql/Join.php
+++ b/src/Persistence/Sql/Join.php
@@ -107,85 +107,4 @@ public function afterLoad(Model $entity): void
             unset($entity->getDataRef()[$this->short_name]);
         }
     }
-
-    public function beforeInsert(Model $entity, array &$data): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        $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) !== null) {
-            return;
-        }
-
-        $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));
-        } else {
-            $data[$this->master_field] = $this->getId($entity);
-        }
-    }
-
-    public function afterInsert(Model $entity): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        $foreignModel = $this->getForeignModel();
-        $foreignEntity = $foreignModel->createEntity()
-            ->setMulti($this->getAndUnsetSaveBuffer($entity))
-            ->set($this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId());
-        $foreignEntity->save();
-
-        $this->setId($entity, $entity->getId()); // TODO why is this here? it seems to be not needed
-    }
-
-    public function beforeUpdate(Model $entity, array &$data): void
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        if (!$this->issetSaveBuffer($entity)) {
-            return;
-        }
-
-        $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
-    {
-        if ($this->weak) {
-            return;
-        }
-
-        $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();
-            }
-        });
-    }
 }

From 8af65a99c416643613bde7c5896aa33bfd475660 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Sun, 16 Jan 2022 21:38:24 +0100
Subject: [PATCH 10/19] initialize filter iterator by default

---
 src/Persistence/Array_.php        | 2 --
 src/Persistence/Array_/Action.php | 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/Persistence/Array_.php b/src/Persistence/Array_.php
index 697fe8f07..32b8f9dbe 100644
--- a/src/Persistence/Array_.php
+++ b/src/Persistence/Array_.php
@@ -214,7 +214,6 @@ public function tryLoad(Model $model, $id): ?array
     {
         if ($id === self::ID_LOAD_ONE || $id === self::ID_LOAD_ANY) {
             $action = $this->action($model, 'select');
-            $action->generator->rewind(); // TODO needed for some reasons!
 
             $selectRow = $action->getRow();
             if ($selectRow === null) {
@@ -238,7 +237,6 @@ public function tryLoad(Model $model, $id): ?array
             $condition->key = $model->getField($model->id_field);
             $condition->setOwner($model->createEntity()); // TODO needed for typecasting to apply
             $action->filter($condition);
-            $action->generator->rewind(); // TODO needed for some reasons!
 
             $rowData = $action->getRow();
             if ($rowData === null) {
diff --git a/src/Persistence/Array_/Action.php b/src/Persistence/Array_/Action.php
index 4b692458a..9e432fd53 100644
--- a/src/Persistence/Array_/Action.php
+++ b/src/Persistence/Array_/Action.php
@@ -45,6 +45,9 @@ public function filter(Model\Scope\AbstractScope $condition)
             } else {
                 $this->generator = new \CallbackFilterIterator($this->generator, $filterFx);
             }
+            // initialize filter iterator, it is not rewound by default
+            // https://github.com/php/php-src/issues/7952
+            $this->generator->rewind();
         }
 
         return $this;

From 8e96c327dc5d68d07ab4023e6c3a45aaae07979f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 17 Jan 2022 12:13:52 +0100
Subject: [PATCH 11/19] use LimitIterator

---
 src/Persistence/Array_/Action.php | 39 ++++++++++---------------------
 1 file changed, 12 insertions(+), 27 deletions(-)

diff --git a/src/Persistence/Array_/Action.php b/src/Persistence/Array_/Action.php
index 9e432fd53..b181a84cf 100644
--- a/src/Persistence/Array_/Action.php
+++ b/src/Persistence/Array_/Action.php
@@ -56,13 +56,9 @@ public function filter(Model\Scope\AbstractScope $condition)
     /**
      * Calculates SUM|AVG|MIN|MAX aggregate values for $field.
      *
-     * @param string $fx
-     * @param string $field
-     * @param bool   $coalesce
-     *
      * @return $this
      */
-    public function aggregate($fx, $field, $coalesce = false)
+    public function aggregate(string $fx, string $field, bool $coalesce = false)
     {
         $result = 0;
         $column = array_column($this->getRows(), $field);
@@ -100,15 +96,10 @@ public function aggregate($fx, $field, $coalesce = false)
 
     /**
      * Checks if $row matches $condition.
-     *
-     * @return bool
      */
-    protected function match(array $row, Model\Scope\AbstractScope $condition)
+    protected function match(array $row, Model\Scope\AbstractScope $condition): bool
     {
-        $match = false;
-
-        // simple condition
-        if ($condition instanceof Model\Scope\Condition) {
+        if ($condition instanceof Model\Scope\Condition) { // simple condition
             $args = $condition->toQueryArguments();
 
             $field = $args[0];
@@ -126,11 +117,8 @@ protected function match(array $row, Model\Scope\AbstractScope $condition)
                     ->addMoreInfo('condition', $condition);
             }
 
-            $match = $this->evaluateIf($row[$field->short_name] ?? null, $operator, $value);
-        }
-
-        // nested conditions
-        if ($condition instanceof Model\Scope) {
+            return $this->evaluateIf($row[$field->short_name] ?? null, $operator, $value);
+        } elseif ($condition instanceof Model\Scope) { // nested conditions
             $matches = [];
 
             foreach ($condition->getNestedConditions() as $nestedCondition) {
@@ -143,10 +131,11 @@ protected function match(array $row, Model\Scope\AbstractScope $condition)
             }
 
             // any matches && all matches the same (if all required)
-            $match = array_filter($matches) && ($condition->isAnd() ? count(array_unique($matches)) === 1 : true);
+            return array_filter($matches) && ($condition->isAnd() ? count(array_unique($matches)) === 1 : true);
         }
 
-        return $match;
+        throw (new Exception('Unexpected condition type'))
+            ->addMoreInfo('class', get_class($condition));
     }
 
     /**
@@ -160,7 +149,8 @@ protected function evaluateIf($v1, string $operator, $v2): bool
         }
 
         if ($v2 instanceof \Traversable) {
-            throw new \Exception('Unexpected v2 type');
+            throw (new Exception('Unexpected v2 type'))
+                ->addMoreInfo('class', get_class($v2));
         }
 
         switch (strtoupper($operator)) {
@@ -233,11 +223,9 @@ protected function evaluateIf($v1, string $operator, $v2): bool
     /**
      * Applies sorting on Iterator.
      *
-     * @param array $fields
-     *
      * @return $this
      */
-    public function order($fields)
+    public function order(array $fields)
     {
         $data = $this->getRows();
 
@@ -265,10 +253,7 @@ public function order($fields)
      */
     public function limit(int $limit = null, int $offset = 0)
     {
-        $data = array_slice($this->getRows(), $offset, $limit, true);
-
-        // put data back in generator
-        $this->generator = new \ArrayIterator($data);
+        $this->generator = new \LimitIterator($this->generator, $offset, $limit ?? -1);
 
         return $this;
     }

From f14d6f48fdc453fa0ee2c5f873c8d1d379176fb4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 17 Jan 2022 12:27:28 +0100
Subject: [PATCH 12/19] rewind before exists

---
 src/Persistence/Array_/Action.php | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/Persistence/Array_/Action.php b/src/Persistence/Array_/Action.php
index b181a84cf..e5af81c75 100644
--- a/src/Persistence/Array_/Action.php
+++ b/src/Persistence/Array_/Action.php
@@ -277,6 +277,7 @@ public function count()
      */
     public function exists()
     {
+        $this->generator->rewind();
         $this->generator = new \ArrayIterator([[$this->generator->valid() ? 1 : 0]]);
 
         return $this;

From 9f753d1c0a6069129513006de962a6a4c38e152d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 17 Jan 2022 13:02:27 +0100
Subject: [PATCH 13/19] add phpstan/extension-installer allow plugin

---
 composer.json | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/composer.json b/composer.json
index c12ef94f1..c1d36b1d0 100644
--- a/composer.json
+++ b/composer.json
@@ -73,6 +73,9 @@
         }
     },
     "config": {
-        "sort-packages": true
+        "sort-packages": true,
+        "allow-plugins": {
+            "phpstan/extension-installer": true
+        }
     }
 }

From c85d7532c169ac507eebe9d372454444467f6693 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 17 Jan 2022 12:53:06 +0100
Subject: [PATCH 14/19] prefer limit 2 and getRows()

---
 src/Persistence/Array_.php        | 12 +++++++-----
 src/Persistence/Array_/Action.php |  1 +
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/Persistence/Array_.php b/src/Persistence/Array_.php
index 32b8f9dbe..637f1bdb7 100644
--- a/src/Persistence/Array_.php
+++ b/src/Persistence/Array_.php
@@ -215,18 +215,20 @@ public function tryLoad(Model $model, $id): ?array
         if ($id === self::ID_LOAD_ONE || $id === self::ID_LOAD_ANY) {
             $action = $this->action($model, 'select');
 
-            $selectRow = $action->getRow();
-            if ($selectRow === null) {
+            $action->limit($id === self::ID_LOAD_ANY ? 1 : 2);
+
+            $rowsRaw = $action->getRows();
+            if (count($rowsRaw) === 0) {
                 return null;
-            } elseif ($id === self::ID_LOAD_ONE && $action->getRow() !== null) {
+            } elseif (count($rowsRaw) !== 1) {
                 throw (new Exception('Ambiguous conditions, more than one record can be loaded.'))
                     ->addMoreInfo('model', $model)
                     ->addMoreInfo('id', null);
             }
 
-            $id = $selectRow[$model->id_field];
+            $idRaw = reset($rowsRaw)[$model->id_field];
 
-            $row = $this->tryLoad($model, $id);
+            $row = $this->tryLoad($model, $idRaw);
 
             return $row;
         }
diff --git a/src/Persistence/Array_/Action.php b/src/Persistence/Array_/Action.php
index e5af81c75..e4f83323d 100644
--- a/src/Persistence/Array_/Action.php
+++ b/src/Persistence/Array_/Action.php
@@ -296,6 +296,7 @@ public function getRows(): array
      */
     public function getRow(): ?array
     {
+        $this->generator->rewind(); // TODO alternatively allow to fetch only once
         $row = $this->generator->current();
         $this->generator->next();
 

From 057247407f80565541c9adef8a829fdff3789159 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 17 Jan 2022 14:05:09 +0100
Subject: [PATCH 15/19] fix leak by not using LimitIterator

---
 src/Persistence/Array_/Action.php | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/Persistence/Array_/Action.php b/src/Persistence/Array_/Action.php
index e4f83323d..5c1cc0263 100644
--- a/src/Persistence/Array_/Action.php
+++ b/src/Persistence/Array_/Action.php
@@ -251,9 +251,16 @@ public function order(array $fields)
      *
      * @return $this
      */
-    public function limit(int $limit = null, int $offset = 0)
+    public function limit(?int $limit, int $offset = 0)
     {
-        $this->generator = new \LimitIterator($this->generator, $offset, $limit ?? -1);
+        // LimitIterator with circular reference is not GCed in PHP 7.4 - ???, see
+        // https://github.com/php/php-src/issues/7958
+        if (\PHP_MAJOR_VERSION < 20) { // TODO update condition once fixed in php-src
+            $data = array_slice($this->getRows(), $offset, $limit, true);
+            $this->generator = new \ArrayIterator($data);
+        } else {
+            $this->generator = new \LimitIterator($this->generator, $offset, $limit ?? -1);
+        }
 
         return $this;
     }

From b74220ed61b2cec81157beb852b42e29415a846a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Mon, 17 Jan 2022 13:48:03 +0100
Subject: [PATCH 16/19] fix allow-plugins for composer 2.2.5+

---
 composer.json | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/composer.json b/composer.json
index c1d36b1d0..214c124e6 100644
--- a/composer.json
+++ b/composer.json
@@ -73,9 +73,10 @@
         }
     },
     "config": {
-        "sort-packages": true,
         "allow-plugins": {
+            "ergebnis/composer-normalize": true,
             "phpstan/extension-installer": true
-        }
+        },
+        "sort-packages": true
     }
 }

From 0d38d467b8395a67903a3db2f6bf89e38afc673d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Fri, 21 Jan 2022 14:08:16 +0100
Subject: [PATCH 17/19] fix DBAL 3.3 stan

---
 phpstan.neon.dist                   |  2 +-
 src/Model/Scope/RootScope.php       | 10 ++++++++--
 src/Persistence/GenericPlatform.php |  5 -----
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/phpstan.neon.dist b/phpstan.neon.dist
index 0cb9343bf..f85d3cbda 100644
--- a/phpstan.neon.dist
+++ b/phpstan.neon.dist
@@ -33,7 +33,7 @@ parameters:
         # for Doctrine DBAL 2.x, remove the support once Doctrine ORM 2.10 is released
         # see https://github.com/doctrine/orm/issues/8526
         -
-            message: '~^(Call to an undefined method Doctrine\\DBAL\\Driver\\Connection::getWrappedConnection\(\)\.|Call to an undefined method Doctrine\\DBAL\\Connection::createSchemaManager\(\)\.|Call to an undefined static method Doctrine\\DBAL\\Exception::invalidPdoInstance\(\)\.|Call to method (getCreateTableSQL|getClobTypeDeclarationSQL|initializeCommentedDoctrineTypes)\(\) of deprecated class Doctrine\\DBAL\\Platforms\\\w+Platform:\n.+|Anonymous class extends deprecated class Doctrine\\DBAL\\Platforms\\(PostgreSQL94Platform|SQLServer2012Platform):\n.+|Call to deprecated method fetch(|All)\(\) of class Doctrine\\DBAL\\Result:\n.+|Call to deprecated method getSchemaManager\(\) of class Doctrine\\DBAL\\Connection:\n.+|Access to an undefined property Doctrine\\DBAL\\Driver\\PDO\\Connection::\$connection\.|Parameter #1 \$dsn of class Doctrine\\DBAL\\Driver\\PDO\\SQLSrv\\Connection constructor expects string, Doctrine\\DBAL\\Driver\\PDO\\Connection given\.|Method Atk4\\Data\\Persistence\\Sql\\Expression::execute\(\) should return Doctrine\\DBAL\\Result\|PDOStatement but returns bool\.|PHPDoc tag @return contains generic type Doctrine\\DBAL\\Schema\\AbstractSchemaManager<Doctrine\\DBAL\\Platforms\\AbstractPlatform> but class Doctrine\\DBAL\\Schema\\AbstractSchemaManager is not generic\.|Class Doctrine\\DBAL\\Platforms\\(MySqlPlatform|PostgreSqlPlatform) referenced with incorrect case: Doctrine\\DBAL\\Platforms\\(MySQLPlatform|PostgreSQLPlatform)\.)$~'
+            message: '~^(Call to an undefined method Doctrine\\DBAL\\Driver\\Connection::getWrappedConnection\(\)\.|Call to an undefined method Doctrine\\DBAL\\Connection::createSchemaManager\(\)\.|Call to an undefined static method Doctrine\\DBAL\\Exception::invalidPdoInstance\(\)\.|Call to method (getCreateTableSQL|getClobTypeDeclarationSQL|initializeCommentedDoctrineTypes)\(\) of deprecated class Doctrine\\DBAL\\Platforms\\\w+Platform:\n.+|Anonymous class extends deprecated class Doctrine\\DBAL\\Platforms\\(PostgreSQL94Platform|SQLServer2012Platform):\n.+|Call to deprecated method fetch(|All)\(\) of class Doctrine\\DBAL\\Result:\n.+|Call to deprecated method getSchemaManager\(\) of class Doctrine\\DBAL\\Connection:\n.+|Call to deprecated method getWrappedConnection\(\) of class Doctrine\\DBAL\\Connection:\n.+getNativeConnection\(\).+|Call to deprecated method getWrappedResourceHandle\(\) of class Doctrine\\DBAL\\Driver\\Mysqli\\Connection:\n.+getNativeConnection\(\).+|Access to an undefined property Doctrine\\DBAL\\Driver\\PDO\\Connection::\$connection\.|Parameter #1 \$dsn of class Doctrine\\DBAL\\Driver\\PDO\\SQLSrv\\Connection constructor expects string, Doctrine\\DBAL\\Driver\\PDO\\Connection given\.|Method Atk4\\Data\\Persistence\\Sql\\Expression::execute\(\) should return Doctrine\\DBAL\\Result\|PDOStatement but returns bool\.|PHPDoc tag @return contains generic type Doctrine\\DBAL\\Schema\\AbstractSchemaManager<Doctrine\\DBAL\\Platforms\\AbstractPlatform> but class Doctrine\\DBAL\\Schema\\AbstractSchemaManager is not generic\.|Class Doctrine\\DBAL\\Platforms\\(MySqlPlatform|PostgreSqlPlatform) referenced with incorrect case: Doctrine\\DBAL\\Platforms\\(MySQLPlatform|PostgreSQLPlatform)\.)$~'
             path: '*'
             # count for DBAL 3.x matched in "src/Persistence/GenericPlatform.php" file
             count: 40
diff --git a/src/Model/Scope/RootScope.php b/src/Model/Scope/RootScope.php
index 0476a02f4..5b7b298b4 100644
--- a/src/Model/Scope/RootScope.php
+++ b/src/Model/Scope/RootScope.php
@@ -48,12 +48,18 @@ public function negate()
         throw new Exception('Model Scope cannot be negated!');
     }
 
-    public static function createAnd(...$conditions)
+    /**
+     * @return Model\Scope
+     */
+    public static function createAnd(...$conditions) // @phpstan-ignore-line
     {
         return (parent::class)::createAnd(...$conditions);
     }
 
-    public static function createOr(...$conditions)
+    /**
+     * @return Model\Scope
+     */
+    public static function createOr(...$conditions) // @phpstan-ignore-line
     {
         return (parent::class)::createOr(...$conditions);
     }
diff --git a/src/Persistence/GenericPlatform.php b/src/Persistence/GenericPlatform.php
index da6dc4c48..3fe2df090 100644
--- a/src/Persistence/GenericPlatform.php
+++ b/src/Persistence/GenericPlatform.php
@@ -43,11 +43,6 @@ private function createNotSupportedException(): \Exception
                 $connection->getSchemaManager();
                 $connection->getSchemaManager();
                 $connection->getSchemaManager();
-                $connection->getSchemaManager();
-                $connection->getSchemaManager();
-                $connection->getSchemaManager();
-                $connection->getSchemaManager();
-                $connection->getSchemaManager();
             }
         }
 

From 501eac2f270f08ba319606465ad6043319455c2e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Fri, 21 Jan 2022 14:29:21 +0100
Subject: [PATCH 18/19] join persistence is determined by joined model

---
 src/Model/Join.php              | 19 ++++---------------
 src/Persistence/Array_/Join.php |  3 ---
 src/Persistence/Sql/Join.php    |  3 ---
 3 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 5b1034bb8..27ff859b2 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -11,7 +11,6 @@
 use Atk4\Data\Exception;
 use Atk4\Data\Field;
 use Atk4\Data\Model;
-use Atk4\Data\Persistence;
 use Atk4\Data\Reference;
 
 /**
@@ -37,20 +36,10 @@ abstract class Join
      */
     protected $foreign_table;
 
-    /**
-     * If $persistence is set, then it's used for loading
-     * and storing the values, instead $owner->persistence.
-     *
-     * @var Persistence|Persistence\Sql|null
-     */
-    protected $persistence;
-
     /**
      * Field that is used as native "ID" in the foreign table.
      * When deleting record, this field will be conditioned.
      *
-     * ->where($join->id_field, $join->id)->delete();
-     *
      * @var string
      */
     protected $id_field = 'id';
@@ -64,7 +53,7 @@ abstract class Join
      */
     protected $kind;
 
-    /** @var bool Is our join weak? Weak join will stop you from touching foreign table. */
+    /** @var bool Weak join does not update foreign table. */
     protected $weak = false;
 
     /**
@@ -74,7 +63,7 @@ abstract class Join
      *
      * If you are using the following syntax:
      *
-     * $user->join('contact','default_contact_id');
+     * $user->join('contact', 'default_contact_id');
      *
      * Then the ID connecting tables is stored in foreign table and the order
      * of saving and delete needs to be reversed. In this case $reverse
@@ -541,7 +530,7 @@ public function beforeInsert(Model $entity, array &$data): void
             $data[$this->master_field] = $this->getId($entity);
         }
 
-        // $entity->set($this->master_field, $this->getId($entity));
+        // $entity->set($this->master_field, $this->getId($entity)); // TODO needed? from array persistence
     }
 
     public function afterInsert(Model $entity): void
@@ -550,7 +539,7 @@ public function afterInsert(Model $entity): void
             return;
         }
 
-        $this->setSaveBufferValue($entity, $this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId()); // from array persistence...
+        $this->setSaveBufferValue($entity, $this->foreign_field, $this->hasJoin() ? $this->getJoin()->getId($entity) : $entity->getId()); // TODO needed? from array persistence
 
         $foreignModel = $this->getForeignModel();
         $foreignEntity = $foreignModel->createEntity()
diff --git a/src/Persistence/Array_/Join.php b/src/Persistence/Array_/Join.php
index 6861303b9..517e82e0d 100644
--- a/src/Persistence/Array_/Join.php
+++ b/src/Persistence/Array_/Join.php
@@ -8,9 +8,6 @@
 use Atk4\Data\Model;
 use Atk4\Data\Persistence;
 
-/**
- * @property Persistence\Array_|null $persistence
- */
 class Join extends Model\Join
 {
     public function afterLoad(Model $entity): void
diff --git a/src/Persistence/Sql/Join.php b/src/Persistence/Sql/Join.php
index d41aecdd2..787a1003c 100644
--- a/src/Persistence/Sql/Join.php
+++ b/src/Persistence/Sql/Join.php
@@ -7,9 +7,6 @@
 use Atk4\Data\Model;
 use Atk4\Data\Persistence;
 
-/**
- * @property Persistence\Sql $persistence
- */
 class Join extends Model\Join
 {
     /**

From edf81f19c2e68126a82b7704e9e1ad2fdcf17e3d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Michael=20Vo=C5=99=C3=AD=C5=A1ek?= <mvorisek@mvorisek.cz>
Date: Fri, 21 Jan 2022 14:36:38 +0100
Subject: [PATCH 19/19] no Join::id_field

---
 src/Model/Join.php | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/src/Model/Join.php b/src/Model/Join.php
index 27ff859b2..0b597888d 100644
--- a/src/Model/Join.php
+++ b/src/Model/Join.php
@@ -36,14 +36,6 @@ abstract class Join
      */
     protected $foreign_table;
 
-    /**
-     * Field that is used as native "ID" in the foreign table.
-     * When deleting record, this field will be conditioned.
-     *
-     * @var string
-     */
-    protected $id_field = 'id';
-
     /**
      * By default this will be either "inner" (for strong) or "left" for weak joins.
      * You can specify your own type of join by passing ['kind' => 'right']
@@ -63,7 +55,7 @@ abstract class Join
      *
      * If you are using the following syntax:
      *
-     * $user->join('contact', 'default_contact_id');
+     * $user->join('contact', 'default_contact_id')
      *
      * Then the ID connecting tables is stored in foreign table and the order
      * of saving and delete needs to be reversed. In this case $reverse
@@ -74,18 +66,16 @@ abstract class Join
     protected $reverse;
 
     /**
-     * Field to be used for matching inside master table. By default
-     * it's $foreign_table.'_id'.
-     * Note that it should be actual field name in master table.
+     * Field to be used for matching inside master table.
+     * By default it's $foreign_table.'_id'.
      *
      * @var string
      */
     protected $master_field;
 
     /**
-     * Field to be used for matching in a foreign table. By default
-     * it's 'id'.
-     * Note that it should be actual field name in foreign table.
+     * Field to be used for matching in a foreign table.
+     * By default it's 'id'.
      *
      * @var string
      */
@@ -240,7 +230,7 @@ protected function init(): void
             if ($this->master_field && $this->master_field !== $id_field) { // TODO not implemented yet, see https://github.com/atk4/data/issues/803
                 throw (new Exception('Joining tables on non-id fields is not implemented yet'))
                     ->addMoreInfo('master_field', $this->master_field)
-                    ->addMoreInfo('id_field', $this->id_field);
+                    ->addMoreInfo('id_field', $id_field);
             }
 
             if (!$this->master_field) {
@@ -361,9 +351,10 @@ public function hasOne(string $link, array $defaults = [])
      */
     public function hasMany(string $link, array $defaults = [])
     {
+        $id_field = $this->getOwner()->id_field;
         $defaults = array_merge([
-            'our_field' => $this->id_field,
-            'their_field' => $this->getModelTableString($this->getOwner()) . '_' . $this->id_field,
+            'our_field' => $id_field,
+            'their_field' => $this->getModelTableString($this->getOwner()) . '_' . $id_field,
         ], $defaults);
 
         return $this->getOwner()->hasMany($link, $defaults);