refactored ActiveRecord::unlinkAll() to work will noSQL

issue #3520

framework/db/BaseActiveRecord.php

@@ -1228,15 +1228,15 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
             foreach ($relation->link as $a => $b) {
                 $columns[$b] = $model->$a;
             }
+            $nulls = [];
+            foreach (array_keys($columns) as $a) {
+                $nulls[$a] = null;
+            }
             if (is_array($relation->via)) {
                 /** @var $viaClass ActiveRecordInterface */
                 if ($delete) {
                     $viaClass::deleteAll($columns);
                 } else {
-                    $nulls = [];
-                    foreach (array_keys($columns) as $a) {
-                        $nulls[$a] = null;
-                    }
                     $viaClass::updateAll($nulls, $columns);
                 }
             } else {
@@ -1246,10 +1246,6 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
                 if ($delete) {
                     $command->delete($viaTable, $columns)->execute();
                 } else {
-                    $nulls = [];
-                    foreach (array_keys($columns) as $a) {
-                        $nulls[$a] = null;
-                    }
                     $command->update($viaTable, $nulls, $columns)->execute();
                 }
             }
@@ -1286,10 +1282,10 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
     /**
      * Destroys the relationship in current model.
      *
-     *  The model with the foreign key of the relationship will be deleted if `$delete` is true.
+     * The model with the foreign key of the relationship will be deleted if `$delete` is true.
-     *  Otherwise, the foreign key will be set null and the model will be saved without validation.
+     * Otherwise, the foreign key will be set null and the model will be saved without validation.
      *
-     *  Note to destroy the relationship without removing records make sure your keys can be set to null
+     * Note that to destroy the relationship without removing records make sure your keys can be set to null
      *
      * @param string $name the case sensitive name of the relationship.
      * @param boolean $delete whether to delete the model that contains the foreign key.
@@ -1297,53 +1293,57 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
     public function unlinkAll($name, $delete = false)
     {
         $relation = $this->getRelation($name);
-        $columns = [];
+
-        $condition = [];
+        if ($relation->via !== null) {
-        if (empty($relation->via)) {
+            if (is_array($relation->via)) {
-            /** @var $viaClass \yii\db\ActiveRecord */
+                /** @var ActiveQuery $viaRelation */
-            $viaClass = $relation->modelClass;
+                list($viaName, $viaRelation) = $relation->via;
-            if ($delete) {
+                $viaClass = $viaRelation->modelClass;
-                foreach ($relation->link as $a => $b) {
+                unset($this->_related[$viaName]);
-                    $condition[$a] = $this->$b;
-                }
-                $viaClass::deleteAll($condition);
             } else {
-                foreach ($relation->link as $a => $b) {
+                $viaRelation = $relation->via;
-                    $columns[$a] = null;
+                $viaTable = reset($relation->via->from);
-                    $condition[$a] = $this->$b;
+            }
+            $condition = [];
+            $nulls = [];
+            foreach ($viaRelation->link as $a => $b) {
+                $nulls[$a] = null;
+                $condition[$a] = $this->$b;
+            }
+            if (is_array($relation->via)) {
+                /** @var $viaClass ActiveRecordInterface */
+                if ($delete) {
+                    $viaClass::deleteAll($condition);
+                } else {
+                    $viaClass::updateAll($nulls, $condition);
+                }
+            } else {
+                /** @var $viaTable string */
+                /** @var Command $command */
+                $command = static::getDb()->createCommand();
+                if ($delete) {
+                    $command->delete($viaTable, $condition)->execute();
+                } else {
+                    $command->update($viaTable, $nulls, $condition)->execute();
                 }
-                $viaClass::updateAll($columns, $condition);
             }
         } else {
-            $viaTable = reset($relation->via->from);
+            /** @var $relatedModel ActiveRecordInterface */
-
+            $relatedModel = $relation->modelClass;
-            /** @var ActiveQuery $viaRelation */
+            $nulls = [];
-            $viaRelation = $relation->via;
+            $condition = [];
-
+            foreach ($relation->link as $a => $b) {
-            /** @var Command $command */
+                $nulls[$a] = null;
-            $command = static::getDb()->createCommand();
+                $condition[$a] = $this->$b;
+            }
             if ($delete) {
-                foreach ($viaRelation->link as $a => $b) {
+                $relatedModel::deleteAll($condition);
-                    $condition[$a] = $this->$b;
-                }
-                $command->delete($viaTable, $condition)->execute();
             } else {
-                foreach ($viaRelation->link as $a => $b) {
+                $relatedModel::updateAll($nulls, $condition);
-                    $columns[$a] = null;
-                    $condition[$a] = $this->$b;
-                }
-                $command->update($viaTable, $columns,$condition)->execute();
             }
         }
 
-        if (!$relation->multiple) {
+        unset($this->_related[$name]);
-            unset($this->_related[$name]);
-        } elseif (isset($this->_related[$name])) {
-            /** @var ActiveRecordInterface $b */
-            foreach (array_keys($this->_related[$name]) as $a) {
-                unset($this->_related[$name][$a]);
-            }
-        }
     }
 
     /**

tests/unit/data/ar/Order.php

@@ -65,11 +65,25 @@ class Order extends ActiveRecord
     public function getBooks()
     {
         return $this->hasMany(Item::className(), ['id' => 'item_id'])
-            ->viaTable('order_item', ['order_id' => 'id'])
+            ->via('orderItems')
             ->where(['category_id' => 1]);
     }
 
     public function getBooksWithNullFK()
+    {
+        return $this->hasMany(Item::className(), ['id' => 'item_id'])
+            ->via('orderItemsWithNullFk')
+            ->where(['category_id' => 1]);
+    }
+
+    public function getBooksViaTable()
+    {
+        return $this->hasMany(Item::className(), ['id' => 'item_id'])
+            ->viaTable('order_item', ['order_id' => 'id'])
+            ->where(['category_id' => 1]);
+    }
+
+    public function getBooksWithNullFKViaTable()
     {
         return $this->hasMany(Item::className(), ['id' => 'item_id'])
             ->viaTable('order_item_with_null_fk', ['order_id' => 'id'])

tests/unit/data/ar/elasticsearch/Order.php

@@ -40,6 +40,17 @@ class Order extends ActiveRecord
             ->via('orderItems')->orderBy('id');
     }
 
+    public function getItemsWithNullFK()
+    {
+        return $this->hasMany(Item::className(), ['id' => 'item_id'])
+            ->via('orderItemsWithNullFK');
+    }
+
+    public function getOrderItemsWithNullFK()
+    {
+        return $this->hasMany(OrderItemWithNullFK::className(), ['order_id' => 'id']);
+    }
+
     public function getItemsInOrder1()
     {
         return $this->hasMany(Item::className(), ['id' => 'item_id'])
@@ -56,12 +67,19 @@ class Order extends ActiveRecord
             })->orderBy('name');
     }
 
-//	public function getBooks()
+    public function getBooks()
-//	{
+    {
-//		return $this->hasMany('Item', ['id' => 'item_id'])
+        return $this->hasMany(Item::className(), ['id' => 'item_id'])
-//			->viaTable('order_item', ['order_id' => 'id'])
+            ->via('orderItems')
-//			->where(['category_id' => 1]);
+            ->where(['category_id' => 1]);
-//	}
+    }
+
+    public function getBooksWithNullFK()
+    {
+        return $this->hasMany(Item::className(), ['id' => 'item_id'])
+            ->via('orderItemsWithNullFK')
+            ->where(['category_id' => 1]);
+    }
 
     public function beforeSave($insert)
     {
@@ -90,6 +108,5 @@ class Order extends ActiveRecord
                 ]
             ]
         ]);
-
     }
 }

tests/unit/data/ar/elasticsearch/OrderItemWithNullFK.php

@@ -4,21 +4,7 @@ namespace yiiunit\data\ar\elasticsearch;
 
 /**
  * Class OrderItem
- *
- * @property integer $order_id
- * @property integer $item_id
- * @property integer $quantity
- * @property string $subtotal
  */
-class OrderItemWithNullFK extends ActiveRecord
+class OrderItemWithNullFK extends OrderItem
 {
-    public function attributes()
-    {
-        return ['order_id', 'item_id', 'quantity', 'subtotal'];
-    }
-
-    public static function tableName()
-    {
-        return 'order_item_with_null_fk';
-    }
 }

tests/unit/data/ar/elasticsearch/OrderWithNullFK.php

@@ -4,26 +4,7 @@ namespace yiiunit\data\ar\elasticsearch;
 
 /**
  * Class Order
- *
- * @property integer $id
- * @property integer $customer_id
- * @property integer $created_at
- * @property string $total
  */
-class OrderWithNullFK extends ActiveRecord
+class OrderWithNullFK extends Order
 {
-    public static function primaryKey()
-    {
-        return ['id'];
-    }
-
-    public function attributes()
-    {
-        return ['id', 'customer_id', 'created_at', 'total'];
-    }
-
-    public static function tableName()
-    {
-        return 'order_with_null_fk';
-    }
 }

tests/unit/extensions/elasticsearch/ActiveRecordTest.php

@@ -77,6 +77,8 @@ class ActiveRecordTest extends ElasticSearchTestCase
         Item::setUpMapping($command);
         Order::setUpMapping($command);
         OrderItem::setUpMapping($command);
+        OrderWithNullFK::setUpMapping($command);
+        OrderItemWithNullFK::setUpMapping($command);
 
         $db->createCommand()->flushIndex('yiitest');
 
@@ -149,6 +151,38 @@ class ActiveRecordTest extends ElasticSearchTestCase
         $orderItem->setAttributes(['order_id' => 3, 'item_id' => 2, 'quantity' => 1, 'subtotal' => 40.0], false);
         $orderItem->save(false);
 
+        $order = new OrderWithNullFK();
+        $order->id = 1;
+        $order->setAttributes(['customer_id' => 1, 'created_at' => 1325282384, 'total' => 110.0], false);
+        $order->save(false);
+        $order = new OrderWithNullFK();
+        $order->id = 2;
+        $order->setAttributes(['customer_id' => 2, 'created_at' => 1325334482, 'total' => 33.0], false);
+        $order->save(false);
+        $order = new OrderWithNullFK();
+        $order->id = 3;
+        $order->setAttributes(['customer_id' => 2, 'created_at' => 1325502201, 'total' => 40.0], false);
+        $order->save(false);
+
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 1, 'item_id' => 1, 'quantity' => 1, 'subtotal' => 30.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 1, 'item_id' => 2, 'quantity' => 2, 'subtotal' => 40.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 2, 'item_id' => 4, 'quantity' => 1, 'subtotal' => 10.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 2, 'item_id' => 5, 'quantity' => 1, 'subtotal' => 15.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 2, 'item_id' => 3, 'quantity' => 1, 'subtotal' => 8.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 3, 'item_id' => 2, 'quantity' => 1, 'subtotal' => 40.0], false);
+        $orderItem->save(false);
+
         $db->createCommand()->flushIndex('yiitest');
     }
 

tests/unit/extensions/redis/ActiveRecordTest.php

@@ -111,6 +111,36 @@ class ActiveRecordTest extends RedisTestCase
         $orderItem = new OrderItem();
         $orderItem->setAttributes(['order_id' => 3, 'item_id' => 2, 'quantity' => 1, 'subtotal' => 40.0], false);
         $orderItem->save(false);
+
+        $order = new OrderWithNullFK();
+        $order->setAttributes(['customer_id' => 1, 'created_at' => 1325282384, 'total' => 110.0], false);
+        $order->save(false);
+        $order = new OrderWithNullFK();
+        $order->setAttributes(['customer_id' => 2, 'created_at' => 1325334482, 'total' => 33.0], false);
+        $order->save(false);
+        $order = new OrderWithNullFK();
+        $order->setAttributes(['customer_id' => 2, 'created_at' => 1325502201, 'total' => 40.0], false);
+        $order->save(false);
+
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 1, 'item_id' => 1, 'quantity' => 1, 'subtotal' => 30.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 1, 'item_id' => 2, 'quantity' => 2, 'subtotal' => 40.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 2, 'item_id' => 4, 'quantity' => 1, 'subtotal' => 10.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 2, 'item_id' => 5, 'quantity' => 1, 'subtotal' => 15.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 2, 'item_id' => 3, 'quantity' => 1, 'subtotal' => 8.0], false);
+        $orderItem->save(false);
+        $orderItem = new OrderItemWithNullFK();
+        $orderItem->setAttributes(['order_id' => 3, 'item_id' => 2, 'quantity' => 1, 'subtotal' => 40.0], false);
+        $orderItem->save(false);
+
     }
 
     public function testFindNullValues()
@@ -119,6 +149,18 @@ class ActiveRecordTest extends RedisTestCase
         $this->markTestSkipped('Redis does not store/find null values correctly.');
     }
 
+    public function testUnlinkAll()
+    {
+        // https://github.com/yiisoft/yii2/issues/1311
+        $this->markTestSkipped('Redis does not store/find null values correctly.');
+    }
+
+    public function testUnlink()
+    {
+        // https://github.com/yiisoft/yii2/issues/1311
+        $this->markTestSkipped('Redis does not store/find null values correctly.');
+    }
+
     public function testBooleanAttribute()
     {
         // https://github.com/yiisoft/yii2/issues/1311

tests/unit/framework/ar/ActiveRecordTestTrait.php

@@ -705,6 +705,7 @@ trait ActiveRecordTestTrait
         $customer = $customerClass::findOne(2);
         $this->assertEquals(2, count($customer->orders));
         $customer->unlink('orders', $customer->orders[1], true);
+        $this->afterSave();
 
         $this->assertEquals(1, count($customer->orders));
         $this->assertNull($orderClass::findOne(3));
@@ -714,6 +715,7 @@ trait ActiveRecordTestTrait
         $this->assertEquals(3, count($order->items));
         $this->assertEquals(3, count($order->orderItems));
         $order->unlink('items', $order->items[2], true);
+        $this->afterSave();
 
         $this->assertEquals(2, count($order->items));
         $this->assertEquals(2, count($order->orderItems));
@@ -721,6 +723,7 @@ trait ActiveRecordTestTrait
         // via model without delete
         $this->assertEquals(3, count($order->itemsWithNullFK));
         $order->unlink('itemsWithNullFK', $order->itemsWithNullFK[2], false);
+        $this->afterSave();
 
         $this->assertEquals(2, count($order->itemsWithNullFK));
         $this->assertEquals(2, count($order->orderItems));
@@ -732,9 +735,12 @@ trait ActiveRecordTestTrait
         $customerClass = $this->getCustomerClass();
         /** @var \yii\db\ActiveRecordInterface $orderClass */
         $orderClass = $this->getOrderClass();
-
+        /** @var \yii\db\ActiveRecordInterface $orderItemClass */
+        $orderItemClass = $this->getOrderItemClass();
+        /** @var \yii\db\ActiveRecordInterface $itemClass */
+        $itemClass = $this->getItemClass();
+        /** @var \yii\db\ActiveRecordInterface $orderWithNullFKClass */
         $orderWithNullFKClass = $this->getOrderWithNullFKClass();
-
         /** @var \yii\db\ActiveRecordInterface $orderItemsWithNullFKClass */
         $orderItemsWithNullFKClass = $this->getOrderItemWithNullFKmClass();
 
@@ -742,8 +748,10 @@ trait ActiveRecordTestTrait
         // has many with delete
         $customer = $customerClass::findOne(2);
         $this->assertEquals(2, count($customer->orders));
+        $this->assertEquals(3, $orderClass::find()->count());
         $customer->unlinkAll('orders', true);
-
+        $this->afterSave();
+        $this->assertEquals(1, $orderClass::find()->count());
         $this->assertEquals(0, count($customer->orders));
 
         $this->assertNull($orderClass::findOne(2));
@@ -753,24 +761,38 @@ trait ActiveRecordTestTrait
         // has many without delete
         $customer = $customerClass::findOne(2);
         $this->assertEquals(2, count($customer->ordersWithNullFK));
+        $this->assertEquals(3, $orderWithNullFKClass::find()->count());
         $customer->unlinkAll('ordersWithNullFK', false);
-
+        $this->afterSave();
         $this->assertEquals(0, count($customer->ordersWithNullFK));
+        $this->assertEquals(3, $orderWithNullFKClass::find()->count());
+        $this->assertEquals(2, $orderWithNullFKClass::find()->where(['AND', ['id' => [2, 3]], ['customer_id' => null]])->count());
 
-        $this->assertEquals(2,$orderWithNullFKClass::find()->where('(id=2 OR id=3) AND customer_id IS NULL')->count());
 
         // via model with delete
         /** @var Order $order */
         $order = $orderClass::findOne(1);
         $this->assertEquals(2, count($order->books));
+        $this->assertEquals(6, $orderItemClass::find()->count());
+        $this->assertEquals(5, $itemClass::find()->count());
         $order->unlinkAll('books', true);
+        $this->afterSave();
+        $this->assertEquals(5, $itemClass::find()->count());
+        $this->assertEquals(4, $orderItemClass::find()->count());
         $this->assertEquals(0, count($order->books));
 
         // via model without delete
-        $books = $order->booksWithNullFK;
+        $this->assertEquals(2, count($order->booksWithNullFK));
-        $this->assertEquals(2, count($books));
+        $this->assertEquals(6, $orderItemsWithNullFKClass::find()->count());
+        $this->assertEquals(5, $itemClass::find()->count());
         $order->unlinkAll('booksWithNullFK',false);
-        $this->assertEquals(2,$orderItemsWithNullFKClass::find()->where('(item_id=1 OR item_id=2) AND order_id IS NULL')->count());
+        $this->afterSave();
+        $this->assertEquals(0, count($order->booksWithNullFK));
+        $this->assertEquals(2,$orderItemsWithNullFKClass::find()->where(['AND', ['item_id' => [1, 2]], ['order_id' => null]])->count());
+        $this->assertEquals(6, $orderItemsWithNullFKClass::find()->count());
+        $this->assertEquals(5, $itemClass::find()->count());
+
+        // via table is covered in \yiiunit\framework\db\ActiveRecordTest::testUnlinkAllViaTable()
     }
 
     public static $afterSaveNewRecord;

tests/unit/framework/db/ActiveRecordTest.php

@@ -535,4 +535,38 @@ class ActiveRecordTest extends DatabaseTestCase
         $model->loadDefaultValues(false);
         $this->assertEquals('something', $model->char_col2);
     }
+
+    public function testUnlinkAllViaTable()
+    {
+        /** @var \yii\db\ActiveRecordInterface $orderClass */
+        $orderClass = $this->getOrderClass();
+        /** @var \yii\db\ActiveRecordInterface $orderItemClass */
+        $orderItemClass = $this->getOrderItemClass();
+        /** @var \yii\db\ActiveRecordInterface $itemClass */
+        $itemClass = $this->getOrderItemClass();
+        /** @var \yii\db\ActiveRecordInterface $orderItemsWithNullFKClass */
+        $orderItemsWithNullFKClass = $this->getOrderItemWithNullFKmClass();
+
+        // via table with delete
+        /** @var Order $order */
+        $order = $orderClass::findOne(1);
+        $this->assertEquals(2, count($order->books));
+        $this->assertEquals(6, $orderItemClass::find()->count());
+        $this->assertEquals(5, $itemClass::find()->count());
+        $order->unlinkAll('booksViaTable', true);
+        $this->afterSave();
+        $this->assertEquals(5, $itemClass::find()->count());
+        $this->assertEquals(4, $orderItemClass::find()->count());
+        $this->assertEquals(0, count($order->books));
+
+        // via table without delete
+        $this->assertEquals(2, count($order->booksWithNullFK));
+        $this->assertEquals(6, $orderItemsWithNullFKClass::find()->count());
+        $this->assertEquals(5, $itemClass::find()->count());
+        $order->unlinkAll('booksWithNullFKViaTable',false);
+        $this->assertEquals(0, count($order->booksWithNullFK));
+        $this->assertEquals(2,$orderItemsWithNullFKClass::find()->where(['AND', ['item_id' => [1, 2]], ['order_id' => null]])->count());
+        $this->assertEquals(6, $orderItemsWithNullFKClass::find()->count());
+        $this->assertEquals(5, $itemClass::find()->count());
+    }
 }