Remove magic synchronization for relations (issue yiisoft#19788).

PowerGamer1 · Jul 27, 2023 · 2410226 · 2410226
1 parent c8c0ea9
commit 2410226
Show file tree

Hide file tree

Showing 10 changed files with 25 additions and 298 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
 2.0.49 under development
 ------------------------
 
+- 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)
 - Bug #19899: Fixed `GridView` in some cases calling `Model::generateAttributeLabel()` to generate label values that are never used (PowerGamer1)
 - Bug #9899: Fix caching a MSSQL query with BLOB data type (terabytesoftw)
 - Bug #16208: Fix `yii\log\FileTarget` to not export empty messages (terabytesoftw)

diff --git a/framework/UPGRADE.md b/framework/UPGRADE.md
@@ -62,6 +62,29 @@ Upgrade from Yii 2.0.48
 * The function signature for `yii\console\Controller::select()` and `yii\helpers\BaseConsole::select()` have changed.
   They now have an additional `$default = null` parameter. In case those methods are overwritten you will need to
   update your child classes accordingly.
+* The engine no longer attempts to provide an automatic synchronization between ActiveRecord relations and corresponding foreign keys. 
+  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).
+  The new guarantee provided by Yii2 is: once set ActiveRecord relations are never automatically or silently changed/unset by the engine.
+  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:
+    ```php
+    $project = Project::findOne(123);
+    $oldManager = $project->manager;
+    $project->load(Yii::$app->getRequest()->post()); // $project->manager_id may change here. 
+    $project->update();
+    $newManager = $project->manager;
+    // Notify $oldManager and $newManager about the reassignment by email.
+    ```
+  the access to `$project->manager` after update should be preceded by unsetting that relation:  
+    ```PHP
+    // ... (same as above).
+    $project->update();
+    unset($project->manager);
+    $newManager = $project->manager;
+    // Notify $oldManager and $newManager about the reassignment by email.
+    ```
+  Another notable example is using `ActiveRecord::refresh()`. If the refreshed model had relations loaded before the call to `refresh()` 
+  and these relations are expected to change, unset them explicitly with `unset()` before using again.
+
 
 Upgrade from Yii 2.0.46
 -----------------------

diff --git a/framework/db/BaseActiveRecord.php b/framework/db/BaseActiveRecord.php
@@ -295,7 +295,6 @@ public function __get($name)
         }
         $value = parent::__get($name);
         if ($value instanceof ActiveQueryInterface) {
-            $this->setRelationDependencies($name, $value);
             return $this->_related[$name] = $value->findFor($name, $this);
         }
 
@@ -311,12 +310,6 @@ public function __get($name)
     public function __set($name, $value)
     {
         if ($this->hasAttribute($name)) {
-            if (
-                !empty($this->_relationsDependencies[$name])
-                && (!array_key_exists($name, $this->_attributes) || $this->_attributes[$name] !== $value)
-            ) {
-                $this->resetDependentRelations($name);
-            }
             $this->_attributes[$name] = $value;
         } else {
             parent::__set($name, $value);
@@ -350,9 +343,6 @@ public function __unset($name)
     {
         if ($this->hasAttribute($name)) {
             unset($this->_attributes[$name]);
-            if (!empty($this->_relationsDependencies[$name])) {
-                $this->resetDependentRelations($name);
-            }
         } elseif (array_key_exists($name, $this->_related)) {
             unset($this->_related[$name]);
         } elseif ($this->getRelation($name, false) === null) {
@@ -460,10 +450,6 @@ protected function createRelationQuery($class, $link, $multiple)
      */
     public function populateRelation($name, $records)
     {
-        foreach ($this->_relationsDependencies as &$relationNames) {
-            unset($relationNames[$name]);
-        }
-
         $this->_related[$name] = $records;
     }
 
@@ -521,12 +507,6 @@ public function getAttribute($name)
     public function setAttribute($name, $value)
     {
         if ($this->hasAttribute($name)) {
-            if (
-                !empty($this->_relationsDependencies[$name])
-                && (!array_key_exists($name, $this->_attributes) || $this->_attributes[$name] !== $value)
-            ) {
-                $this->resetDependentRelations($name);
-            }
             $this->_attributes[$name] = $value;
         } else {
             throw new InvalidArgumentException(get_class($this) . ' has no attribute named "' . $name . '".');
@@ -1081,8 +1061,6 @@ protected function refreshInternal($record)
             $this->_attributes[$name] = isset($record->_attributes[$name]) ? $record->_attributes[$name] : null;
         }
         $this->_oldAttributes = $record->_oldAttributes;
-        $this->_related = [];
-        $this->_relationsDependencies = [];
         $this->afterRefresh();
 
         return true;
@@ -1196,8 +1174,6 @@ public static function populateRecord($record, $row)
             }
         }
         $record->_oldAttributes = $record->_attributes;
-        $record->_related = [];
-        $record->_relationsDependencies = [];
     }
 
     /**
@@ -1731,41 +1707,6 @@ public function offsetUnset($offset)
         }
     }
 
-    /**
-     * Resets dependent related models checking if their links contain specific attribute.
-     * @param string $attribute The changed attribute name.
-     */
-    private function resetDependentRelations($attribute)
-    {
-        foreach ($this->_relationsDependencies[$attribute] as $relation) {
-            unset($this->_related[$relation]);
-        }
-        unset($this->_relationsDependencies[$attribute]);
-    }
-
-    /**
-     * Sets relation dependencies for a property
-     * @param string $name property name
-     * @param ActiveQueryInterface $relation relation instance
-     * @param string|null $viaRelationName intermediate relation
-     */
-    private function setRelationDependencies($name, $relation, $viaRelationName = null)
-    {
-        if (empty($relation->via) && $relation->link) {
-            foreach ($relation->link as $attribute) {
-                $this->_relationsDependencies[$attribute][$name] = $name;
-                if ($viaRelationName !== null) {
-                    $this->_relationsDependencies[$attribute][] = $viaRelationName;
-                }
-            }
-        } elseif ($relation->via instanceof ActiveQueryInterface) {
-            $this->setRelationDependencies($name, $relation->via);
-        } elseif (is_array($relation->via)) {
-            list($viaRelationName, $viaQuery) = $relation->via;
-            $this->setRelationDependencies($name, $viaQuery, $viaRelationName);
-        }
-    }
-
     /**
      * @param mixed $newValue
      * @param mixed $oldValue

diff --git a/tests/data/ar/Customer.php b/tests/data/ar/Customer.php
@@ -18,6 +18,7 @@
  * @property string $email
  * @property string $address
  * @property int $status
+ * @property int $profile_id
  *
  * @method CustomerQuery findBySql($sql, $params = []) static
  */

diff --git a/tests/data/cubrid.sql b/tests/data/cubrid.sql
@@ -19,9 +19,6 @@ DROP TABLE IF EXISTS "constraints";
 DROP TABLE IF EXISTS "animal";
 DROP TABLE IF EXISTS "default_pk";
 DROP TABLE IF EXISTS "document";
-DROP TABLE IF EXISTS "dossier";
-DROP TABLE IF EXISTS "employee";
-DROP TABLE IF EXISTS "department";
 DROP VIEW IF EXISTS "animal_view";
 DROP TABLE IF EXISTS "T_constraints_4";
 DROP TABLE IF EXISTS "T_constraints_3";
@@ -167,28 +164,6 @@ CREATE TABLE "document" (
   PRIMARY KEY ("id")
 );
 
-CREATE TABLE "department" (
-  "id" int(11) NOT NULL AUTO_INCREMENT,
-  "title" VARCHAR(255) NOT NULL,
-  PRIMARY KEY ("id")
-);
-
-CREATE TABLE "employee" (
-  "id" int(11) NOT NULL,
-  "department_id" int(11) NOT NULL,
-  "first_name" VARCHAR(255) NOT NULL,
-  "last_name" VARCHAR(255) NOT NULL,
-  PRIMARY KEY ("id", "department_id")
-);
-
-CREATE TABLE "dossier" (
-  "id" int(11) NOT NULL AUTO_INCREMENT,
-  "department_id" int(11) NOT NULL,
-  "employee_id" int(11) NOT NULL,
-  "summary" VARCHAR(255) NOT NULL,
-  PRIMARY KEY ("id")
-);
-
 CREATE VIEW "animal_view" AS SELECT * FROM "animal";
 
 INSERT INTO "animal" ("type") VALUES ('yiiunit\data\ar\Cat');
@@ -234,17 +209,6 @@ INSERT INTO "order_item_with_null_fk" (order_id, item_id, quantity, subtotal) VA
 
 INSERT INTO "document" (title, content, version) VALUES ('Yii 2.0 guide', 'This is Yii 2.0 guide', 0);
 
-INSERT INTO "department" (id, title) VALUES (1, 'IT');
-INSERT INTO "department" (id, title) VALUES (2, 'accounting');
-
-INSERT INTO "employee" (id, department_id, first_name, last_name) VALUES (1, 1, 'John', 'Doe');
-INSERT INTO "employee" (id, department_id, first_name, last_name) VALUES (1, 2, 'Ann', 'Smith');
-INSERT INTO "employee" (id, department_id, first_name, last_name) VALUES (2, 2, 'Will', 'Smith');
-
-INSERT INTO "dossier" (id, department_id, employee_id, summary) VALUES (1, 1, 1, 'Excellent employee.');
-INSERT INTO "dossier" (id, department_id, employee_id, summary) VALUES (2, 2, 1, 'Brilliant employee.');
-INSERT INTO "dossier" (id, department_id, employee_id, summary) VALUES (3, 2, 2, 'Good employee.');
-
 /* bit test, see https://github.com/yiisoft/yii2/issues/9006 */
 
 DROP TABLE IF EXISTS `bit_values`;

diff --git a/tests/data/mssql.sql b/tests/data/mssql.sql
@@ -15,9 +15,6 @@ IF OBJECT_ID('[dbo].[negative_default_values]', 'U') IS NOT NULL DROP TABLE [dbo
 IF OBJECT_ID('[dbo].[animal]', 'U') IS NOT NULL DROP TABLE [dbo].[animal];
 IF OBJECT_ID('[dbo].[default_pk]', 'U') IS NOT NULL DROP TABLE [dbo].[default_pk];
 IF OBJECT_ID('[dbo].[document]', 'U') IS NOT NULL DROP TABLE [dbo].[document];
-IF OBJECT_ID('[dbo].[dossier]', 'U') IS NOT NULL DROP TABLE [dbo].[dossier];
-IF OBJECT_ID('[dbo].[employee]', 'U') IS NOT NULL DROP TABLE [dbo].[employee];
-IF OBJECT_ID('[dbo].[department]', 'U') IS NOT NULL DROP TABLE [dbo].[department];
 IF OBJECT_ID('[dbo].[animal_view]', 'V') IS NOT NULL DROP VIEW [dbo].[animal_view];
 IF OBJECT_ID('[T_constraints_4]', 'U') IS NOT NULL DROP TABLE [T_constraints_4];
 IF OBJECT_ID('[T_constraints_3]', 'U') IS NOT NULL DROP TABLE [T_constraints_3];
@@ -179,35 +176,6 @@ CREATE TABLE [dbo].[document] (
     ) ON [PRIMARY]
 );
 
-CREATE TABLE [dbo].[department] (
-    [id] [int] IDENTITY NOT NULL,
-    [title] [varchar](255) NOT NULL,
-    CONSTRAINT [PK_department_pk] PRIMARY KEY CLUSTERED (
-        [id] ASC
-    ) ON [PRIMARY]
-);
-
-CREATE TABLE [dbo].[employee] (
-    [id] [int] NOT NULL,
-    [department_id] [int] NOT NULL,
-    [first_name] [varchar](255) NOT NULL,
-    [last_name] [varchar](255) NOT NULL,
-    CONSTRAINT [PK_employee_pk] PRIMARY KEY CLUSTERED (
-        [id] ASC,
-        [department_id] ASC
-    ) ON [PRIMARY]
-);
-
-CREATE TABLE [dbo].[dossier] (
-    [id] [int] IDENTITY NOT NULL,
-    [department_id] [int] NOT NULL,
-    [employee_id] [int] NOT NULL,
-    [summary] [varchar](255) NOT NULL,
-    CONSTRAINT [PK_dossier_pk] PRIMARY KEY CLUSTERED (
-        [id] ASC
-    ) ON [PRIMARY]
-);
-
 CREATE VIEW [dbo].[animal_view] AS SELECT * FROM [dbo].[animal];
 
 INSERT INTO [dbo].[animal] (type) VALUES ('yiiunit\data\ar\Cat');
@@ -253,21 +221,6 @@ INSERT INTO [dbo].[order_item_with_null_fk] ([order_id], [item_id], [quantity],
 
 INSERT INTO [dbo].[document] ([title], [content], [version]) VALUES ('Yii 2.0 guide', 'This is Yii 2.0 guide', 0);
 
-SET IDENTITY_INSERT [dbo].[department] ON;
-INSERT INTO [dbo].[department] (id, title) VALUES (1, 'IT');
-INSERT INTO [dbo].[department] (id, title) VALUES (2, 'accounting');
-SET IDENTITY_INSERT [dbo].[department] OFF;
-
-INSERT INTO [dbo].[employee] (id, department_id, first_name, last_name) VALUES (1, 1, 'John', 'Doe');
-INSERT INTO [dbo].[employee] (id, department_id, first_name, last_name) VALUES (1, 2, 'Ann', 'Smith');
-INSERT INTO [dbo].[employee] (id, department_id, first_name, last_name) VALUES (2, 2, 'Will', 'Smith');
-
-SET IDENTITY_INSERT [dbo].[dossier] ON;
-INSERT INTO [dbo].[dossier] (id, department_id, employee_id, summary) VALUES (1, 1, 1, 'Excellent employee.');
-INSERT INTO [dbo].[dossier] (id, department_id, employee_id, summary) VALUES (2, 2, 1, 'Brilliant employee.');
-INSERT INTO [dbo].[dossier] (id, department_id, employee_id, summary) VALUES (3, 2, 2, 'Good employee.');
-SET IDENTITY_INSERT [dbo].[dossier] OFF;
-
 /* bit test, see https://github.com/yiisoft/yii2/issues/9006 */
 
 IF OBJECT_ID('[dbo].[bit_values]', 'U') IS NOT NULL DROP TABLE [dbo].[bit_values];

diff --git a/tests/data/mysql.sql b/tests/data/mysql.sql
@@ -20,9 +20,6 @@ DROP TABLE IF EXISTS `animal` CASCADE;
 DROP TABLE IF EXISTS `default_pk` CASCADE;
 DROP TABLE IF EXISTS `document` CASCADE;
 DROP TABLE IF EXISTS `comment` CASCADE;
-DROP TABLE IF EXISTS `dossier`;
-DROP TABLE IF EXISTS `employee`;
-DROP TABLE IF EXISTS `department`;
 DROP TABLE IF EXISTS `storage`;
 DROP TABLE IF EXISTS `alpha`;
 DROP TABLE IF EXISTS `beta`;
@@ -186,28 +183,6 @@ CREATE TABLE `comment` (
   PRIMARY KEY (id)
 ) ENGINE=InnoDB DEFAULT CHARSET=utf8;
 
-CREATE TABLE `department` (
-  `id` INT(11) NOT NULL AUTO_INCREMENT,
-  title VARCHAR(255) NOT NULL,
-  PRIMARY KEY (`id`)
-) ENGINE=InnoDB DEFAULT CHARSET=utf8;
-
-CREATE TABLE `employee` (
-  `id` INT(11) NOT NULL,
-  `department_id` INT(11) NOT NULL,
-  `first_name` VARCHAR(255) NOT NULL,
-  `last_name` VARCHAR(255) NOT NULL,
-  PRIMARY KEY (`id`, `department_id`)
-) ENGINE=InnoDB DEFAULT CHARSET=utf8;
-
-CREATE TABLE `dossier` (
-  `id` INT(11) NOT NULL AUTO_INCREMENT,
-  `department_id` INT(11) NOT NULL,
-  `employee_id` INT(11) NOT NULL,
-  `summary` VARCHAR(255) NOT NULL,
-  PRIMARY KEY (`id`)
-) ENGINE=InnoDB DEFAULT CHARSET=utf8;
-
 CREATE TABLE `storage` (
   `id` INT(11) NOT NULL AUTO_INCREMENT,
   `data` JSON NOT NULL,
@@ -271,17 +246,6 @@ INSERT INTO `order_item_with_null_fk` (order_id, item_id, quantity, subtotal) VA
 
 INSERT INTO `document` (title, content, version) VALUES ('Yii 2.0 guide', 'This is Yii 2.0 guide', 0);
 
-INSERT INTO `department` (id, title) VALUES (1, 'IT');
-INSERT INTO `department` (id, title) VALUES (2, 'accounting');
-
-INSERT INTO `employee` (id, department_id, first_name, last_name) VALUES (1, 1, 'John', 'Doe');
-INSERT INTO `employee` (id, department_id, first_name, last_name) VALUES (1, 2, 'Ann', 'Smith');
-INSERT INTO `employee` (id, department_id, first_name, last_name) VALUES (2, 2, 'Will', 'Smith');
-
-INSERT INTO `dossier` (id, department_id, employee_id, summary) VALUES (1, 1, 1, 'Excellent employee.');
-INSERT INTO `dossier` (id, department_id, employee_id, summary) VALUES (2, 2, 1, 'Brilliant employee.');
-INSERT INTO `dossier` (id, department_id, employee_id, summary) VALUES (3, 2, 2, 'Good employee.');
-
 INSERT INTO `alpha` (id, string_identifier) VALUES (1, '1');
 INSERT INTO `alpha` (id, string_identifier) VALUES (2, '1a');
 INSERT INTO `alpha` (id, string_identifier) VALUES (3, '01');