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

Fix segfault on Model::save() with circular relations #13354

Merged
merged 1 commit into from May 24, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-4.0.md
Expand Up @@ -39,6 +39,7 @@
- Fixed `method` parameter in `Phalcon\Mvc\Model\Manager::getHasOneRecords()`, it's not always a string, null by default. [#14115](https://github.com/phalcon/cphalcon/issues/14115)
- Fixed `method` parameter in `Phalcon\Mvc\Model\Manager::getHasManyRecords()`, it's not always a string, null by default. [#14115](https://github.com/phalcon/cphalcon/issues/14115)
- `handlers` property in `Phalcon\Mvc\Micro\Collection` is now always an array.
- Fixed crash in `Phalcon\Mvc\Model::save()` when saving a circular model relation. [#13354](https://github.com/phalcon/cphalcon/pull/13354)

## Removed
- Removed `Phalcon\Session\Factory`. [#13672](https://github.com/phalcon/cphalcon/issues/13672)
Expand Down
6 changes: 3 additions & 3 deletions phalcon/Mvc/Model.zep
Expand Up @@ -4625,10 +4625,10 @@ abstract class Model implements EntityInterface, ModelInterface, ResultInterface
}

/**
* If dynamic update is enabled, saving the record must not
* take any action
* If dynamic update is enabled, saving the record must not take any action
* Only save if the model is dirty to prevent circular relations causing an infinite loop
*/
if !record->save() {
if record->dirtyState !== Model::DIRTY_STATE_PERSISTENT && !record->save() {
/**
* Get the validation messages generated by the
* referenced model
Expand Down
27 changes: 27 additions & 0 deletions tests/integration/Mvc/Model/SaveCest.php
Expand Up @@ -334,4 +334,31 @@ public function mvcModelSaveAfterWithoutDefaultValues(IntegrationTester $I)
$robot->delete()
);
}

public function mvcModelSaveCircularRelation(IntegrationTester $I)
{
$I->wantToTest('Mvc\Model::save() with circular unsaved relations');

$album = new Albums([
'name' => 'Loopback'
]);
$artist = new Artists([
'name' => 'Evil Robot'
]);

// Assign relationship in both directions on unsaved models
$album->artist = $artist;
$artist->albums = [
$album
];

// Save should handle the circular relation without issue
$I->assertTrue(
$artist->save()
);

// Both should have an ID now
$I->assertNotEquals(null, $album->id);
$I->assertNotEquals(null, $artist->id);
}
}