Merge pull request #4944 from yiisoft/redis-fixes

extensions/redis/ActiveQuery.php

@@ -123,6 +123,7 @@ class ActiveQuery extends Component implements ActiveQueryInterface
             for ($i = 0; $i < $c;) {
                 $row[$dataRow[$i++]] = $dataRow[$i++];
             }
+
             $rows[] = $row;
         }
         if (!empty($rows)) {

extensions/redis/ActiveRecord.php

@@ -119,12 +119,21 @@ class ActiveRecord extends BaseActiveRecord
 
         $key = static::keyPrefix() . ':a:' . static::buildKey($pk);
         // save attributes
-        $args = [$key];
+        $setArgs = [$key];
         foreach ($values as $attribute => $value) {
-            $args[] = $attribute;
-            $args[] = $value;
+            // only insert attributes that are not null
+            if ($value !== null) {
+                if (is_bool($value)) {
+                    $value = (int)$value;
+                }
+                $setArgs[] = $attribute;
+                $setArgs[] = $value;
+            }
+        }
+
+        if (count($setArgs) > 1) {
+            $db->executeCommand('HMSET', $setArgs);
         }
-        $db->executeCommand('HMSET', $args);
 
         $changedAttributes = array_fill_keys(array_keys($values), null);
         $this->setOldAttributes($values);
@@ -158,26 +167,44 @@ class ActiveRecord extends BaseActiveRecord
             $pk = static::buildKey($pk);
             $key = static::keyPrefix() . ':a:' . $pk;
             // save attributes
-            $args = [$key];
+            $delArgs = [$key];
+            $setArgs = [$key];
             foreach ($attributes as $attribute => $value) {
                 if (isset($newPk[$attribute])) {
                     $newPk[$attribute] = $value;
                 }
-                $args[] = $attribute;
-                $args[] = $value;
+                if ($value !== null) {
+                    if (is_bool($value)) {
+                        $value = (int)$value;
+                    }
+                    $setArgs[] = $attribute;
+                    $setArgs[] = $value;
+                } else {
+                    $delArgs[] = $attribute;
+                }
             }
             $newPk = static::buildKey($newPk);
             $newKey = static::keyPrefix() . ':a:' . $newPk;
             // rename index if pk changed
             if ($newPk != $pk) {
                 $db->executeCommand('MULTI');
-                $db->executeCommand('HMSET', $args);
+                if (count($setArgs) > 1) {
+                    $db->executeCommand('HMSET', $setArgs);
+                }
+                if (count($delArgs) > 1) {
+                    $db->executeCommand('HDEL', $delArgs);
+                }
                 $db->executeCommand('LINSERT', [static::keyPrefix(), 'AFTER', $pk, $newPk]);
                 $db->executeCommand('LREM', [static::keyPrefix(), 0, $pk]);
                 $db->executeCommand('RENAME', [$key, $newKey]);
                 $db->executeCommand('EXEC');
             } else {
-                $db->executeCommand('HMSET', $args);
+                if (count($setArgs) > 1) {
+                    $db->executeCommand('HMSET', $setArgs);
+                }
+                if (count($delArgs) > 1) {
+                    $db->executeCommand('HDEL', $delArgs);
+                }
             }
             $n++;
         }

extensions/redis/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 redis extension Change Log
 2.0.0-rc under development
 --------------------------
 
+- Bug #1311: Fixed storage and finding of `null` and boolean values (samdark, cebe)
 - Enh #3520: Added `unlinkAll()`-method to active record to remove all records of a model relation (NmDimas, samdark, cebe)
 - Enh #4048: Added `init` event to `ActiveQuery` classes (qiangxue)
 - Enh #4086: changedAttributes of afterSave Event now contain old values (dizews)

extensions/redis/LuaScriptBuilder.php

@@ -173,6 +173,7 @@ local pks={}
 local n=0
 local v=nil
 local i=0
+local key=$key
 for k,pk in ipairs(allpks) do
     $loadColumnValues
     if $condition then
@@ -268,12 +269,16 @@ EOF;
             if (is_array($value)) { // IN condition
                 $parts[] = $this->buildInCondition('in', [$column, $value], $columns);
             } else {
-                $column = $this->addColumn($column, $columns);
+                if (is_bool($value)) {
+                    $value = (int)$value;
+                }
                 if ($value === null) {
-                    $parts[] = "$column==nil";
+                    $parts[] = "redis.call('HEXISTS',key .. ':a:' .. pk, ".$this->quoteValue($column).")==0";
                 } elseif ($value instanceof Expression) {
+                    $column = $this->addColumn($column, $columns);
                     $parts[] = "$column==" . $value->expression;
                 } else {
+                    $column = $this->addColumn($column, $columns);
                     $value = $this->quoteValue($value);
                     $parts[] = "$column==$value";
                 }
@@ -356,7 +361,7 @@ EOF;
                 $value = isset($value[$column]) ? $value[$column] : null;
             }
             if ($value === null) {
-                $parts[] = "$columnAlias==nil";
+                $parts[] = "redis.call('HEXISTS',key .. ':a:' .. pk, ".$this->quoteValue($column).")==0";
             } elseif ($value instanceof Expression) {
                 $parts[] = "$columnAlias==" . $value->expression;
             } else {
@@ -375,11 +380,11 @@ EOF;
         foreach ($values as $value) {
             $vs = [];
             foreach ($inColumns as $column) {
-                $column = $this->addColumn($column, $columns);
                 if (isset($value[$column])) {
-                    $vs[] = "$column==" . $this->quoteValue($value[$column]);
+                    $columnAlias = $this->addColumn($column, $columns);
+                    $vs[] = "$columnAlias==" . $this->quoteValue($value[$column]);
                 } else {
-                    $vs[] = "$column==nil";
+                    $vs[] = "redis.call('HEXISTS',key .. ':a:' .. pk, ".$this->quoteValue($column).")==0";
                 }
             }
             $vss[] = '(' . implode(' and ', $vs) . ')';

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

@@ -33,6 +33,17 @@ class Order extends ActiveRecord
             ->via('orderItems')->indexBy('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'])
@@ -52,8 +63,15 @@ class Order extends ActiveRecord
     public function getBooks()
     {
         return $this->hasMany(Item::className(), ['id' => 'item_id'])
-            ->via('orderItems', ['order_id' => 'id']);
-            //->where(['category_id' => 1]);
+            ->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 beforeSave($insert)

tests/unit/extensions/redis/ActiveRecordTest.php

@@ -143,30 +143,6 @@ class ActiveRecordTest extends RedisTestCase
 
     }
 
-    public function testFindNullValues()
-    {
-        // https://github.com/yiisoft/yii2/issues/1311
-        $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
-        $this->markTestSkipped('Redis does not store/find boolean values correctly.');
-    }
-
     public function testFindEagerViaRelationPreserveOrder()
     {
         $this->markTestSkipped('Redis does not support orderBy.');
@@ -177,6 +153,44 @@ class ActiveRecordTest extends RedisTestCase
         $this->markTestSkipped('Redis does not support orderBy.');
     }
 
+    /**
+     * overridden because null values are not part of the asArray result in redis
+     */
+    public function testFindAsArray()
+    {
+        /* @var $customerClass \yii\db\ActiveRecordInterface */
+        $customerClass = $this->getCustomerClass();
+
+        // asArray
+        $customer = $customerClass::find()->where(['id' => 2])->asArray()->one();
+        $this->assertEquals([
+            'id' => 2,
+            'email' => 'user2@example.com',
+            'name' => 'user2',
+            'address' => 'address2',
+            'status' => 1,
+        ], $customer);
+
+        // find all asArray
+        $customers = $customerClass::find()->asArray()->all();
+        $this->assertEquals(3, count($customers));
+        $this->assertArrayHasKey('id', $customers[0]);
+        $this->assertArrayHasKey('name', $customers[0]);
+        $this->assertArrayHasKey('email', $customers[0]);
+        $this->assertArrayHasKey('address', $customers[0]);
+        $this->assertArrayHasKey('status', $customers[0]);
+        $this->assertArrayHasKey('id', $customers[1]);
+        $this->assertArrayHasKey('name', $customers[1]);
+        $this->assertArrayHasKey('email', $customers[1]);
+        $this->assertArrayHasKey('address', $customers[1]);
+        $this->assertArrayHasKey('status', $customers[1]);
+        $this->assertArrayHasKey('id', $customers[2]);
+        $this->assertArrayHasKey('name', $customers[2]);
+        $this->assertArrayHasKey('email', $customers[2]);
+        $this->assertArrayHasKey('address', $customers[2]);
+        $this->assertArrayHasKey('status', $customers[2]);
+    }
+
     public function testStatisticalFind()
     {
         // find count, sum, average, min, max, scalar
@@ -273,6 +287,7 @@ class ActiveRecordTest extends RedisTestCase
 
     public function testFindColumn()
     {
+        // TODO this test is duplicated because of missing orderBy support in redis
         $this->assertEquals(['user1', 'user2', 'user3'], Customer::find()->column('name'));
         // TODO $this->assertEquals(['user3', 'user2', 'user1'], Customer::find()->orderBy(['name' => SORT_DESC])->column('name'));
     }