Allow Model::isset/getPersistence() calls on model only (#1088)

atk4 · Feb 10, 2023 · dfa5de5 · dfa5de5
1 parent 856f742
commit dfa5de5
Show file tree

Hide file tree

Showing 14 changed files with 47 additions and 52 deletions.
diff --git a/docs/advanced.rst b/docs/advanced.rst
@@ -81,7 +81,7 @@ hook. Place the following inside Transaction::init()::
     $this->onHookShort(Model::HOOK_AFTER_LOAD, function () {
         if (get_class($this) != $this->getClassName()) {
             $cl = $this->getClassName();
-            $m = new $cl($this->getPersistence());
+            $m = new $cl($this->getModel()->getPersistence());
             $m = $m->load($this->getId());
 
             $this->breakHook($m);

diff --git a/docs/sql.rst b/docs/sql.rst
@@ -169,7 +169,7 @@ This method allows you to execute code within a 'START TRANSACTION / COMMIT' blo
     {
         public function applyPayment(Payment $p)
         {
-            $this->getPersistence()->atomic(function () use ($p) {
+            $this->getModel()->getPersistence()->atomic(function () use ($p) {
                 $this->set('paid', true);
                 $this->save();
 

diff --git a/src/Model.php b/src/Model.php
@@ -271,20 +271,20 @@ public function isEntity(): bool
     public function assertIsModel(self $expectedModelInstance = null): void
     {
         if ($this->_model !== null) {
-            throw new Exception('Expected model, but instance is an entity');
+            throw new \TypeError('Expected model, but instance is an entity');
         }
 
         if ($expectedModelInstance !== null && $expectedModelInstance !== $this) {
             $expectedModelInstance->assertIsModel();
 
-            throw new Exception('Model instance does not match');
+            throw new \TypeError('Model instance does not match');
         }
     }
 
     public function assertIsEntity(self $expectedModelInstance = null): void
     {
         if ($this->_model === null) {
-            throw new Exception('Expected entity, but instance is a model');
+            throw new \TypeError('Expected entity, but instance is a model');
         }
 
         if ($expectedModelInstance !== null) {
@@ -336,7 +336,7 @@ protected function getModelOnlyProperties(): array
                 }
             }
 
-            foreach ([
+            $modelOnlyProperties = array_diff_key($modelOnlyProperties, array_flip([
                 '_model',
                 '_entityId',
                 'data',
@@ -350,9 +350,7 @@ protected function getModelOnlyProperties(): array
                 'userActions', // should be removed once user actions are non-entity
 
                 'containedInEntity',
-            ] as $name) {
-                unset($modelOnlyProperties[$name]);
-            }
+            ]));
 
             self::$_modelOnlyProperties = $modelOnlyProperties;
         }
@@ -422,7 +420,7 @@ private function initEntityIdAndAssertUnchanged(): void
         if ($this->_entityId === null) {
             // set entity ID to the first seen ID
             $this->_entityId = $id;
-        } elseif (!$this->compare($this->idField, $this->_entityId)) {
+        } elseif ($this->_entityId !== $id && !$this->compare($this->idField, $this->_entityId)) {
             $this->unload(); // data for different ID were loaded, make sure to discard them
 
             throw (new Exception('Model instance is an entity, ID cannot be changed to a different one'))
@@ -1102,11 +1100,15 @@ public function setLimit(int $count = null, int $offset = 0)
 
     public function issetPersistence(): bool
     {
+        $this->assertIsModel();
+
         return $this->_persistence !== null;
     }
 
     public function getPersistence(): Persistence
     {
+        $this->assertIsModel();
+
         return $this->_persistence;
     }
 
@@ -1115,8 +1117,6 @@ public function getPersistence(): Persistence
      */
     public function setPersistence(Persistence $persistence)
     {
-        $this->assertIsModel();
-
         if ($this->issetPersistence()) {
             throw new Exception('Persistence is already set');
         }
@@ -1205,8 +1205,7 @@ private function remapIdLoadToPersistence($id)
      */
     private function _load(bool $fromReload, bool $fromTryLoad, $id)
     {
-        $this->assertIsEntity();
-        $this->assertHasPersistence();
+        $this->getModel()->assertHasPersistence();
         if ($this->isLoaded()) {
             throw new Exception('Entity must be unloaded');
         }
@@ -1228,7 +1227,7 @@ private function _load(bool $fromReload, bool $fromTryLoad, $id)
             return $res;
         }
 
-        $data = $this->getPersistence()->{$fromTryLoad ? 'tryLoad' : 'load'}($this->getModel(), $this->remapIdLoadToPersistence($id));
+        $data = $this->getModel()->getPersistence()->{$fromTryLoad ? 'tryLoad' : 'load'}($this->getModel(), $this->remapIdLoadToPersistence($id));
         if ($data === null) {
             return null; // $fromTryLoad is always true here
         }
@@ -1480,7 +1479,7 @@ public function tryLoadBy(string $fieldName, $value)
     protected function validateEntityScope(): void
     {
         if (!$this->getModel()->scope()->isEmpty()) {
-            $this->getPersistence()->load($this->getModel(), $this->getId());
+            $this->getModel()->getPersistence()->load($this->getModel(), $this->getId());
         }
     }
 
@@ -1494,10 +1493,10 @@ protected function validateEntityScope(): void
     public function save(array $data = [])
     {
         if ($this->readOnly) {
-            throw new Exception('Model is read-only and cannot be saved');
+            throw new Exception('Model is read-only');
         }
 
-        $this->assertHasPersistence();
+        $this->getModel()->assertHasPersistence();
 
         $this->setMulti($data);
 
@@ -1532,7 +1531,7 @@ public function save(array $data = [])
                     return $this;
                 }
 
-                $id = $this->getPersistence()->insert($this->getModel(), $data);
+                $id = $this->getModel()->getPersistence()->insert($this->getModel(), $data);
                 if ($this->idField) {
                     $this->setId($id, false);
                 }
@@ -1566,7 +1565,7 @@ public function save(array $data = [])
                     return $this;
                 }
                 $this->validateEntityScope();
-                $this->getPersistence()->update($this->getModel(), $this->getId(), $data);
+                $this->getModel()->getPersistence()->update($this->getModel(), $this->getId(), $data);
                 $this->hook(self::HOOK_AFTER_UPDATE, [&$data]);
             }
 
@@ -1682,7 +1681,6 @@ public function import(array $rows)
      */
     public function export(array $fields = null, string $keyField = null, bool $typecast = true): array
     {
-        $this->assertIsModel();
         $this->assertHasPersistence('export');
 
         // no key field - then just do export
@@ -1802,8 +1800,6 @@ public function getIterator(): \Traversable
      */
     public function getRawIterator(): \Traversable
     {
-        $this->assertIsModel();
-
         return $this->getPersistence()->prepareIterator($this);
     }
 
@@ -1826,18 +1822,18 @@ public function delete($id = null)
         }
 
         if ($this->readOnly) {
-            throw new Exception('Model is read-only and cannot be deleted');
+            throw new Exception('Model is read-only');
         }
 
-        $this->assertHasPersistence();
+        $this->getModel()->assertHasPersistence();
         $this->assertIsLoaded();
 
         $this->atomic(function () {
             if ($this->hook(self::HOOK_BEFORE_DELETE) === false) {
                 return;
             }
             $this->validateEntityScope();
-            $this->getPersistence()->delete($this->getModel(), $this->getId());
+            $this->getModel()->getPersistence()->delete($this->getModel(), $this->getId());
             $this->hook(self::HOOK_AFTER_DELETE);
         });
         $this->unload();
@@ -1855,7 +1851,7 @@ public function delete($id = null)
     public function atomic(\Closure $fx)
     {
         try {
-            return $this->getPersistence()->atomic($fx);
+            return $this->getModel(true)->getPersistence()->atomic($fx);
         } catch (\Throwable $e) {
             if ($this->hook(self::HOOK_ROLLBACK, [$e]) === false) {
                 return false;
@@ -1882,9 +1878,9 @@ public function atomic(\Closure $fx)
      */
     public function action(string $mode, array $args = [])
     {
-        $this->assertHasPersistence('action');
+        $this->getModel(true)->assertHasPersistence('action');
 
-        return $this->getPersistence()->action($this, $mode, $args);
+        return $this->getModel(true)->getPersistence()->action($this, $mode, $args);
     }
 
     public function executeCountQuery(): int

diff --git a/src/Persistence/Array_.php b/src/Persistence/Array_.php
@@ -102,6 +102,8 @@ private function seedDataAndGetTable(Model $model): Table
      */
     public function getRawDataByTable(Model $model, string $table): array
     {
+        $model->assertIsModel();
+
         if (!is_object($model->table)) {
             $this->seedData($model);
         }

diff --git a/src/Persistence/Sql.php b/src/Persistence/Sql.php
@@ -134,18 +134,12 @@ public function add(Model $model, array $defaults = []): void
     protected function initPersistence(Model $model): void
     {
         $model->addMethod('expr', static function (Model $m, ...$args) {
-            $m->assertIsModel();
-
             return $m->getPersistence()->expr($m, ...$args);
         });
         $model->addMethod('dsql', static function (Model $m, ...$args) {
-            $m->assertIsModel();
-
             return $m->getPersistence()->dsql($m, ...$args); // @phpstan-ignore-line
         });
         $model->addMethod('exprNow', static function (Model $m, ...$args) {
-            $m->assertIsModel();
-
             return $m->getPersistence()->exprNow($m, ...$args);
         });
     }

diff --git a/src/Reference.php b/src/Reference.php
@@ -295,8 +295,8 @@ protected function getDefaultPersistence(Model $theirModel)
         // this is useful for ContainsOne/Many implementation in case when you have
         // SQL_Model->containsOne()->hasOne() structure to get back to SQL persistence
         // from Array persistence used in ContainsOne model
-        if ($ourModel->containedInEntity && $ourModel->containedInEntity->issetPersistence()) {
-            return $ourModel->containedInEntity->getPersistence();
+        if ($ourModel->containedInEntity && $ourModel->containedInEntity->getModel()->issetPersistence()) {
+            return $ourModel->containedInEntity->getModel()->getPersistence();
         }
 
         return $ourModel->issetPersistence() ? $ourModel->getPersistence() : false;

diff --git a/src/Reference/ContainsMany.php b/src/Reference/ContainsMany.php
@@ -28,13 +28,13 @@ public function ref(Model $ourModel, array $defaults = []): Model
         ]));
 
         foreach ([Model::HOOK_AFTER_SAVE, Model::HOOK_AFTER_DELETE] as $spot) {
-            $this->onHookToTheirModel($theirModel, $spot, function (Model $theirModel) {
-                $ourModel = $this->getOurModel($theirModel->containedInEntity);
+            $this->onHookToTheirModel($theirModel, $spot, function (Model $theirEntity) {
+                $ourModel = $this->getOurModel($theirEntity->containedInEntity);
                 $ourModel->assertIsEntity();
 
                 /** @var Persistence\Array_ */
-                $persistence = $theirModel->getPersistence();
-                $rows = $persistence->getRawDataByTable($theirModel, $this->tableAlias); // @phpstan-ignore-line
+                $persistence = $theirEntity->getModel()->getPersistence();
+                $rows = $persistence->getRawDataByTable($theirEntity->getModel(), $this->tableAlias); // @phpstan-ignore-line
                 $ourModel->save([$this->getOurFieldName() => $rows !== [] ? $rows : null]);
             });
         }

diff --git a/src/Reference/ContainsOne.php b/src/Reference/ContainsOne.php
@@ -28,13 +28,13 @@ public function ref(Model $ourModel, array $defaults = []): Model
         ]));
 
         foreach ([Model::HOOK_AFTER_SAVE, Model::HOOK_AFTER_DELETE] as $spot) {
-            $this->onHookToTheirModel($theirModel, $spot, function (Model $theirModel) {
-                $ourModel = $this->getOurModel($theirModel->containedInEntity);
+            $this->onHookToTheirModel($theirModel, $spot, function (Model $theirEntity) {
+                $ourModel = $this->getOurModel($theirEntity->containedInEntity);
                 $ourModel->assertIsEntity();
 
                 /** @var Persistence\Array_ */
-                $persistence = $theirModel->getPersistence();
-                $row = $persistence->getRawDataByTable($theirModel, $this->tableAlias); // @phpstan-ignore-line
+                $persistence = $theirEntity->getModel()->getPersistence();
+                $row = $persistence->getRawDataByTable($theirEntity->getModel(), $this->tableAlias); // @phpstan-ignore-line
                 $row = $row ? array_shift($row) : null; // get first and only one record from array persistence
                 $ourModel->save([$this->getOurFieldName() => $row]);
             });

diff --git a/src/Util/DeepCopy.php b/src/Util/DeepCopy.php
@@ -86,7 +86,7 @@ public function to(Model $destination)
         $this->destination = $destination;
 
         if (!$this->destination->issetPersistence()) {
-            $this->destination->setPersistence($this->source->getPersistence());
+            $this->destination->setPersistence($this->source->getModel()->getPersistence());
         }
 
         return $this;

diff --git a/tests/ConditionSqlTest.php b/tests/ConditionSqlTest.php
@@ -49,7 +49,8 @@ public function testEntityNoScopeCloning(): void
         $scope = $m->scope();
         static::assertSame($scope, $m->createEntity()->getModel()->scope());
 
-        $this->expectException(Exception::class);
+        $this->expectException(\TypeError::class);
+        $this->expectExceptionMessage('Expected model, but instance is an entity');
         $m->createEntity()->scope();
     }
 
@@ -73,7 +74,7 @@ public function testEntityReloadWithDifferentIdException(): void
         }, null, Model::class)();
 
         $this->expectException(Exception::class);
-        $this->expectExceptionMessageMatches('~entity.+different~');
+        $this->expectExceptionMessage('Model instance is an entity, ID cannot be changed to a different one');
         $m->reload();
     }
 

diff --git a/tests/IteratorTest.php b/tests/IteratorTest.php
@@ -73,7 +73,8 @@ public function testException6(): void
     {
         $m = new Model();
 
-        $this->expectException(Exception::class);
+        $this->expectException(\TypeError::class);
+        $this->expectExceptionMessage('Expected entity, but instance is a model');
         $m->save();
     }
 

diff --git a/tests/Model/Smbo/Account.php b/tests/Model/Smbo/Account.php
@@ -25,7 +25,7 @@ protected function init(): void
      */
     public function transfer(self $a, float $amount): Transfer
     {
-        $t = new Transfer($this->getPersistence(), ['detached' => true]);
+        $t = new Transfer($this->getModel()->getPersistence(), ['detached' => true]);
         $t = $t->createEntity();
         $t->set('account_id', $this->getId());
 

diff --git a/tests/RandomTest.php b/tests/RandomTest.php
@@ -385,7 +385,8 @@ public function testGetTitle(): void
         $mm = $m->load(2);
         static::assertSame('2', $mm->getTitle()); // loaded returns id value
 
-        $this->expectException(Exception::class);
+        $this->expectException(\TypeError::class);
+        $this->expectExceptionMessage('Expected model, but instance is an entity');
         $mm->getTitles();
     }
 

diff --git a/tests/SubTypesTest.php b/tests/SubTypesTest.php
@@ -91,7 +91,7 @@ protected function init(): void
         $this->onHookShort(Model::HOOK_AFTER_LOAD, function () {
             if (static::class !== $this->getClassName()) {
                 $cl = $this->getClassName();
-                $cl = new $cl($this->getPersistence());
+                $cl = new $cl($this->getModel()->getPersistence());
                 $cl = $cl->load($this->getId());
 
                 $this->breakHook($cl);