Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/advanced.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion docs/sql.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
50 changes: 23 additions & 27 deletions src/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -336,7 +336,7 @@ protected function getModelOnlyProperties(): array
}
}

foreach ([
$modelOnlyProperties = array_diff_key($modelOnlyProperties, array_flip([
'_model',
'_entityId',
'data',
Expand All @@ -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;
}
Expand Down Expand Up @@ -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'))
Expand Down Expand Up @@ -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;
}

Expand All @@ -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');
}
Expand Down Expand Up @@ -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');
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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());
}
}

Expand All @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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]);
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1802,8 +1800,6 @@ public function getIterator(): \Traversable
*/
public function getRawIterator(): \Traversable
{
$this->assertIsModel();

return $this->getPersistence()->prepareIterator($this);
}

Expand All @@ -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();
Expand All @@ -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;
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/Persistence/Array_.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
6 changes: 0 additions & 6 deletions src/Persistence/Sql.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
}
Expand Down
4 changes: 2 additions & 2 deletions src/Reference.php
Original file line number Diff line number Diff line change
Expand Up @@ -279,8 +279,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;
Expand Down
8 changes: 4 additions & 4 deletions src/Reference/ContainsMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/Reference/ContainsOne.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
});
Expand Down
2 changes: 1 addition & 1 deletion src/Util/DeepCopy.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 3 additions & 2 deletions tests/ConditionSqlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand All @@ -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();
}

Expand Down
3 changes: 2 additions & 1 deletion tests/IteratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/Model/Smbo/Account.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down
3 changes: 2 additions & 1 deletion tests/RandomTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
2 changes: 1 addition & 1 deletion tests/SubTypesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down