Skip to content

Commit bfe2ef9

Browse files
PowerGamer1PowerGamer1
committed
Remove magic synchronization for relations (issue #19788).
1 parent c8c0ea9 commit bfe2ef9

File tree

4 files changed

+43
-77
lines changed

4 files changed

+43
-77
lines changed

‎framework/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
44
2.0.49 under development
55
------------------------
66

7+
- Chg #19788: Removed all (often non-functional) attempts of Yii2 to automatically synchronize ActiveRecord relations with corresponding foreign key values. The new guarantee provided by Yii2 is: once set ActiveRecord relations are never automatically or silently changed/unset by the engine (PowerGamer1)
78
- Bug #19899: Fixed `GridView` in some cases calling `Model::generateAttributeLabel()` to generate label values that are never used (PowerGamer1)
89
- Bug #9899: Fix caching a MSSQL query with BLOB data type (terabytesoftw)
910
- Bug #16208: Fix `yii\log\FileTarget` to not export empty messages (terabytesoftw)

‎framework/UPGRADE.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,29 @@ Upgrade from Yii 2.0.48
6262
* The function signature for `yii\console\Controller::select()` and `yii\helpers\BaseConsole::select()` have changed.
6363
They now have an additional `$default = null` parameter. In case those methods are overwritten you will need to
6464
update your child classes accordingly.
65+
* The engine no longer attempts to provide an automatic synchronization between ActiveRecord relations and corresponding foreign keys.
66+
Such synchronization never worked in many cases, came with ActiveRecord performance and memory costs and in some cases is impossible to achieve (see https://github.com/yiisoft/yii2/issues/19788 for details).
67+
The new guarantee provided by Yii2 is: once set ActiveRecord relations are never automatically or silently changed/unset by the engine.
68+
All places in existing code that use already loaded relation after it is expected to change need to manually unset such relation. For example, in the code below:
69+
```php
70+
$project = Project::findOne(123);
71+
$oldManager = $project->manager;
72+
$project->load(Yii::$app->getRequest()->post()); // $project->manager_id may change here.
73+
$project->update();
74+
$newManager = $project->manager;
75+
// Notify $oldManager and $newManager about the reassignment by email.
76+
```
77+
the access to `$project->manager` after update should be preceded by unsetting that relation:
78+
```PHP
79+
// ... (same as above).
80+
$project->update();
81+
unset($project->manager);
82+
$newManager = $project->manager;
83+
// Notify $oldManager and $newManager about the reassignment by email.
84+
```
85+
Another notable example is using `ActiveRecord::refresh()`. If the refreshed model had relations loaded before the call to `refresh()`
86+
and these relations are expected to change, unset them explicitly with `unset()` before using again.
87+
6588

6689
Upgrade from Yii 2.0.46
6790
-----------------------

‎framework/db/BaseActiveRecord.php‎

Lines changed: 0 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,6 @@ public function __get($name)
295295
}
296296
$value = parent::__get($name);
297297
if ($value instanceof ActiveQueryInterface) {
298-
$this->setRelationDependencies($name, $value);
299298
return $this->_related[$name] = $value->findFor($name, $this);
300299
}
301300

@@ -311,12 +310,6 @@ public function __get($name)
311310
public function __set($name, $value)
312311
{
313312
if ($this->hasAttribute($name)) {
314-
if (
315-
!empty($this->_relationsDependencies[$name])
316-
&& (!array_key_exists($name, $this->_attributes) || $this->_attributes[$name] !== $value)
317-
) {
318-
$this->resetDependentRelations($name);
319-
}
320313
$this->_attributes[$name] = $value;
321314
} else {
322315
parent::__set($name, $value);
@@ -350,9 +343,6 @@ public function __unset($name)
350343
{
351344
if ($this->hasAttribute($name)) {
352345
unset($this->_attributes[$name]);
353-
if (!empty($this->_relationsDependencies[$name])) {
354-
$this->resetDependentRelations($name);
355-
}
356346
} elseif (array_key_exists($name, $this->_related)) {
357347
unset($this->_related[$name]);
358348
} elseif ($this->getRelation($name, false) === null) {
@@ -460,10 +450,6 @@ protected function createRelationQuery($class, $link, $multiple)
460450
*/
461451
public function populateRelation($name, $records)
462452
{
463-
foreach ($this->_relationsDependencies as &$relationNames) {
464-
unset($relationNames[$name]);
465-
}
466-
467453
$this->_related[$name] = $records;
468454
}
469455

@@ -521,12 +507,6 @@ public function getAttribute($name)
521507
public function setAttribute($name, $value)
522508
{
523509
if ($this->hasAttribute($name)) {
524-
if (
525-
!empty($this->_relationsDependencies[$name])
526-
&& (!array_key_exists($name, $this->_attributes) || $this->_attributes[$name] !== $value)
527-
) {
528-
$this->resetDependentRelations($name);
529-
}
530510
$this->_attributes[$name] = $value;
531511
} else {
532512
throw new InvalidArgumentException(get_class($this) . ' has no attribute named "' . $name . '".');
@@ -1081,8 +1061,6 @@ protected function refreshInternal($record)
10811061
$this->_attributes[$name] = isset($record->_attributes[$name]) ? $record->_attributes[$name] : null;
10821062
}
10831063
$this->_oldAttributes = $record->_oldAttributes;
1084-
$this->_related = [];
1085-
$this->_relationsDependencies = [];
10861064
$this->afterRefresh();
10871065

10881066
return true;
@@ -1196,8 +1174,6 @@ public static function populateRecord($record, $row)
11961174
}
11971175
}
11981176
$record->_oldAttributes = $record->_attributes;
1199-
$record->_related = [];
1200-
$record->_relationsDependencies = [];
12011177
}
12021178

12031179
/**
@@ -1731,41 +1707,6 @@ public function offsetUnset($offset)
17311707
}
17321708
}
17331709

1734-
/**
1735-
* Resets dependent related models checking if their links contain specific attribute.
1736-
* @param string $attribute The changed attribute name.
1737-
*/
1738-
private function resetDependentRelations($attribute)
1739-
{
1740-
foreach ($this->_relationsDependencies[$attribute] as $relation) {
1741-
unset($this->_related[$relation]);
1742-
}
1743-
unset($this->_relationsDependencies[$attribute]);
1744-
}
1745-
1746-
/**
1747-
* Sets relation dependencies for a property
1748-
* @param string $name property name
1749-
* @param ActiveQueryInterface $relation relation instance
1750-
* @param string|null $viaRelationName intermediate relation
1751-
*/
1752-
private function setRelationDependencies($name, $relation, $viaRelationName = null)
1753-
{
1754-
if (empty($relation->via) && $relation->link) {
1755-
foreach ($relation->link as $attribute) {
1756-
$this->_relationsDependencies[$attribute][$name] = $name;
1757-
if ($viaRelationName !== null) {
1758-
$this->_relationsDependencies[$attribute][] = $viaRelationName;
1759-
}
1760-
}
1761-
} elseif ($relation->via instanceof ActiveQueryInterface) {
1762-
$this->setRelationDependencies($name, $relation->via);
1763-
} elseif (is_array($relation->via)) {
1764-
list($viaRelationName, $viaQuery) = $relation->via;
1765-
$this->setRelationDependencies($name, $viaQuery, $viaRelationName);
1766-
}
1767-
}
1768-
17691710
/**
17701711
* @param mixed $newValue
17711712
* @param mixed $oldValue

‎tests/framework/db/ActiveRecordTest.php‎

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ public function testRelationWhereParams($orderTableName, $orderItemTableName)
11651165
OrderItem::$tableName = null;
11661166
}
11671167

1168-
public function testOutdatedRelationsAreResetForNewRecords()
1168+
public function testOutdatedRelationsAreNotResetForNewRecords()
11691169
{
11701170
$orderItem = new OrderItem();
11711171
$orderItem->order_id = 1;
@@ -1176,17 +1176,17 @@ public function testOutdatedRelationsAreResetForNewRecords()
11761176
// Test `__set()`.
11771177
$orderItem->order_id = 2;
11781178
$orderItem->item_id = 1;
1179-
$this->assertEquals(2, $orderItem->order->id);
1180-
$this->assertEquals(1, $orderItem->item->id);
1179+
$this->assertEquals(1, $orderItem->order->id);
1180+
$this->assertEquals(3, $orderItem->item->id);
11811181

11821182
// Test `setAttribute()`.
11831183
$orderItem->setAttribute('order_id', 2);
11841184
$orderItem->setAttribute('item_id', 2);
1185-
$this->assertEquals(2, $orderItem->order->id);
1186-
$this->assertEquals(2, $orderItem->item->id);
1185+
$this->assertEquals(1, $orderItem->order->id);
1186+
$this->assertEquals(3, $orderItem->item->id);
11871187
}
11881188

1189-
public function testOutdatedRelationsAreResetForExistingRecords()
1189+
public function testOutdatedRelationsAreNotResetForExistingRecords()
11901190
{
11911191
$orderItem = OrderItem::findOne(1);
11921192
$this->assertEquals(1, $orderItem->order->id);
@@ -1195,17 +1195,17 @@ public function testOutdatedRelationsAreResetForExistingRecords()
11951195
// Test `__set()`.
11961196
$orderItem->order_id = 2;
11971197
$orderItem->item_id = 1;
1198-
$this->assertEquals(2, $orderItem->order->id);
1198+
$this->assertEquals(1, $orderItem->order->id);
11991199
$this->assertEquals(1, $orderItem->item->id);
12001200

12011201
// Test `setAttribute()`.
12021202
$orderItem->setAttribute('order_id', 3);
12031203
$orderItem->setAttribute('item_id', 1);
1204-
$this->assertEquals(3, $orderItem->order->id);
1204+
$this->assertEquals(1, $orderItem->order->id);
12051205
$this->assertEquals(1, $orderItem->item->id);
12061206
}
12071207

1208-
public function testOutdatedCompositeKeyRelationsAreReset()
1208+
public function testOutdatedCompositeKeyRelationsAreNotReset()
12091209
{
12101210
$dossier = Dossier::findOne([
12111211
'department_id' => 1,
@@ -1214,26 +1214,26 @@ public function testOutdatedCompositeKeyRelationsAreReset()
12141214
$this->assertEquals('John Doe', $dossier->employee->fullName);
12151215

12161216
$dossier->department_id = 2;
1217-
$this->assertEquals('Ann Smith', $dossier->employee->fullName);
1217+
$this->assertEquals('John Doe', $dossier->employee->fullName);
12181218

12191219
$dossier->employee_id = 2;
1220-
$this->assertEquals('Will Smith', $dossier->employee->fullName);
1220+
$this->assertEquals('John Doe', $dossier->employee->fullName);
12211221

12221222
unset($dossier->employee_id);
1223-
$this->assertNull($dossier->employee);
1223+
$this->assertEquals('John Doe', $dossier->employee->fullName);
12241224

12251225
$dossier = new Dossier();
12261226
$this->assertNull($dossier->employee);
12271227

12281228
$dossier->employee_id = 1;
12291229
$dossier->department_id = 2;
1230-
$this->assertEquals('Ann Smith', $dossier->employee->fullName);
1230+
$this->assertNull($dossier->employee);
12311231

12321232
$dossier->employee_id = 2;
1233-
$this->assertEquals('Will Smith', $dossier->employee->fullName);
1233+
$this->assertNull($dossier->employee);
12341234
}
12351235

1236-
public function testOutdatedViaTableRelationsAreReset()
1236+
public function testOutdatedViaTableRelationsAreNotReset()
12371237
{
12381238
$order = Order::findOne(1);
12391239
$orderItemIds = ArrayHelper::getColumn($order->items, 'id');
@@ -1243,17 +1243,18 @@ public function testOutdatedViaTableRelationsAreReset()
12431243
$order->id = 2;
12441244
sort($orderItemIds);
12451245
$orderItemIds = ArrayHelper::getColumn($order->items, 'id');
1246-
$this->assertSame([3, 4, 5], $orderItemIds);
1246+
$this->assertSame([1, 2], $orderItemIds);
12471247

12481248
unset($order->id);
1249-
$this->assertSame([], $order->items);
1249+
$orderItemIds = ArrayHelper::getColumn($order->items, 'id');
1250+
$this->assertSame([1, 2], $orderItemIds);
12501251

12511252
$order = new Order();
12521253
$this->assertSame([], $order->items);
12531254

12541255
$order->id = 3;
12551256
$orderItemIds = ArrayHelper::getColumn($order->items, 'id');
1256-
$this->assertSame([2], $orderItemIds);
1257+
$this->assertSame([], $orderItemIds);
12571258
}
12581259

12591260
public function testAlias()

0 commit comments

Comments
 (0)