refactored redis AR pk and findByPk

framework/yii/redis/ActiveQuery.php

@@ -90,20 +90,22 @@ class ActiveQuery extends \yii\base\Component
 	public $indexBy;
 
 	/**
-	 * Executes a script created by [[LuaScriptBuilder]]
+	 * PHP magic method.
-	 * @param $type
+	 * This method allows calling static method defined in [[modelClass]] via this query object.
-	 * @param null $column
+	 * It is mainly implemented for supporting the feature of scope.
-	 * @return array|bool|null|string
+	 * @param string $name the method name to be called
+	 * @param array $params the parameters passed to the method
+	 * @return mixed the method return result
 	 */
-	private function executeScript($type, $column=null)
+	public function __call($name, $params)
 	{
-		$modelClass = $this->modelClass;
+		if (method_exists($this->modelClass, $name)) {
-		/** @var Connection $db */
+			array_unshift($params, $this);
-		$db = $modelClass::getDb();
+			call_user_func_array(array($this->modelClass, $name), $params);
-
+			return $this;
-		$method = 'build' . $type;
+		} else {
-		$script = $db->getLuaScriptBuilder()->$method($this, $column);
+			return parent::__call($name, $params);
-		return $db->executeCommand('EVAL', array($script, 0));
+		}
 	}
 
 	/**
@@ -262,6 +264,130 @@ class ActiveQuery extends \yii\base\Component
 		return $this->one() !== null;
 	}
 
+	/**
+	 * Executes a script created by [[LuaScriptBuilder]]
+	 * @param string $type
+	 * @param null $column
+	 * @return array|bool|null|string
+	 */
+	protected function executeScript($type, $columnName=null)
+	{
+		if (($data = $this->findByPk($type)) === false) {
+			$modelClass = $this->modelClass;
+			/** @var Connection $db */
+			$db = $modelClass::getDb();
+
+			$method = 'build' . $type;
+			$script = $db->getLuaScriptBuilder()->$method($this, $columnName);
+			return $db->executeCommand('EVAL', array($script, 0));
+		}
+		return $data;
+	}
+
+	/**
+	 * Fetch by pk if possible as this is much faster
+	 */
+	private function findByPk($type, $columnName = null)
+	{
+		$modelClass = $this->modelClass;
+		if (is_array($this->where) && !isset($this->where[0]) && $modelClass::isPrimaryKey(array_keys($this->where))) {
+			/** @var Connection $db */
+			$db = $modelClass::getDb();
+
+			if (count($this->where) == 1) {
+				$pks = (array) reset($this->where);
+			} else {
+				// TODO support IN for composite PK
+				return false;
+			}
+
+			$start = $this->offset === null ? 0 : $this->offset;
+			$i = 0;
+			$data = array();
+			foreach($pks as $pk) {
+				if (++$i > $start && ($this->limit === null || $i <= $start + $this->limit)) {
+					$key = $modelClass::tableName() . ':a:' . $modelClass::buildKey($pk);
+					$data[] = $db->executeCommand('HGETALL', array($key));
+					if ($type === 'One' && $this->orderBy === null) {
+						break;
+					}
+				}
+			}
+			// TODO support orderBy
+
+			switch($type) {
+				case 'All':
+					return $data;
+				case 'One':
+					return reset($data);
+				case 'Column':
+					// TODO support indexBy
+					$column = array();
+					foreach($data as $dataRow) {
+						$row = array();
+						$c = count($dataRow);
+						for($i = 0; $i < $c; ) {
+							$row[$dataRow[$i++]] = $dataRow[$i++];
+						}
+						$column[] = $row[$columnName];
+					}
+					return $column;
+				case 'Count':
+					return count($data);
+				case 'Sum':
+					$sum = 0;
+					foreach($data as $dataRow) {
+						$c = count($dataRow);
+						for($i = 0; $i < $c; ) {
+							if ($dataRow[$i++] == $columnName) {
+								$sum += $dataRow[$i];
+								break;
+							}
+						}
+					}
+					return $sum;
+				case 'Average':
+					$sum = 0;
+					$count = 0;
+					foreach($data as $dataRow) {
+						$count++;
+						$c = count($dataRow);
+						for($i = 0; $i < $c; ) {
+							if ($dataRow[$i++] == $columnName) {
+								$sum += $dataRow[$i];
+								break;
+							}
+						}
+					}
+					return $sum / $count;
+				case 'Min':
+					$min = null;
+					foreach($data as $dataRow) {
+						$c = count($dataRow);
+						for($i = 0; $i < $c; ) {
+							if ($dataRow[$i++] == $columnName && ($min == null || $dataRow[$i] < $min)) {
+								$min = $dataRow[$i];
+								break;
+							}
+						}
+					}
+					return $min;
+				case 'Max':
+					$max = null;
+					foreach($data as $dataRow) {
+						$c = count($dataRow);
+						for($i = 0; $i < $c; ) {
+							if ($dataRow[$i++] == $columnName && ($max == null || $dataRow[$i] > $max)) {
+								$max = $dataRow[$i];
+								break;
+							}
+						}
+					}
+					return $max;
+			}
+		}
+		return false;
+	}
 
 	/**
 	 * Sets the [[asArray]] property.

framework/yii/redis/ActiveRecord.php

@@ -16,6 +16,7 @@ use yii\base\InvalidParamException;
 use yii\base\NotSupportedException;
 use yii\base\UnknownMethodException;
 use yii\db\TableSchema;
+use yii\helpers\StringHelper;
 
 /**
  * ActiveRecord is the base class for classes representing relational data in terms of objects.
@@ -25,6 +26,11 @@ use yii\db\TableSchema;
  */
 abstract class ActiveRecord extends \yii\db\ActiveRecord
 {
+	/**
+	 * @var array cache for TableSchema instances
+	 */
+	private static $_tables = array();
+
 	/**
 	 * Returns the database connection used by this AR class.
 	 * By default, the "redis" application component is used as the database connection.
@@ -36,11 +42,6 @@ abstract class ActiveRecord extends \yii\db\ActiveRecord
 		return \Yii::$app->redis;
 	}
 
-	public static function hashPk($pk)
-	{
-		return is_array($pk) ? implode('-', $pk) : $pk; // TODO escape PK glue
-	}
-
 	/**
 	 * @inheritdoc
 	 */
@@ -72,14 +73,27 @@ abstract class ActiveRecord extends \yii\db\ActiveRecord
 		return static::getTableSchema()->name;
 	}
 
+	/**
+	 * This method is ment to be overridden in redis ActiveRecord subclasses to return a [[RecordSchema]] instance.
+	 * @return RecordSchema
+	 * @throws \yii\base\InvalidConfigException
+	 */
+	public static function getRecordSchema()
+	{
+		throw new InvalidConfigException(__CLASS__.'::getRecordSchema() needs to be overridden in subclasses and return a RecordSchema.');
+	}
+
 	/**
 	 * Returns the schema information of the DB table associated with this AR class.
 	 * @return TableSchema the schema information of the DB table associated with this AR class.
 	 */
 	public static function getTableSchema()
 	{
-		// TODO should be cached
+		$class = get_called_class();
-		throw new InvalidConfigException(__CLASS__.'::getTableSchema() needs to be overridden in subclasses and return a TableSchema.');
+		if (isset(self::$_tables[$class])) {
+			return self::$_tables[$class];
+		}
+		return self::$_tables[$class] = static::getRecordSchema();
 	}
 
 	/**
@@ -138,9 +152,9 @@ abstract class ActiveRecord extends \yii\db\ActiveRecord
 				}
 //			}
 			// save pk in a findall pool
-			$db->executeCommand('RPUSH', array(static::tableName(), static::hashPk($pk)));
+			$db->executeCommand('RPUSH', array(static::tableName(), static::buildKey($pk)));
 
-			$key = static::tableName() . ':a:' . static::hashPk($pk);
+			$key = static::tableName() . ':a:' . static::buildKey($pk);
 			// save attributes
 			$args = array($key);
 			foreach($values as $attribute => $value) {
@@ -161,34 +175,45 @@ abstract class ActiveRecord extends \yii\db\ActiveRecord
 	 * For example, to change the status to be 1 for all customers whose status is 2:
 	 *
 	 * ~~~
-	 * Customer::updateAll(array('status' => 1), 'status = 2');
+	 * Customer::updateAll(array('status' => 1), array('id' => 2));
 	 * ~~~
 	 *
 	 * @param array $attributes attribute values (name-value pairs) to be saved into the table
-	 * @param string|array $condition the conditions that will be put in the WHERE part of the UPDATE SQL.
+	 * @param array $condition the conditions that will be put in the WHERE part of the UPDATE SQL.
-	 * Please refer to [[Query::where()]] on how to specify this parameter.
+	 * Please refer to [[ActiveQuery::where()]] on how to specify this parameter.
-	 * @param array $params the parameters (name=>value) to be bound to the query.
+	 * @param array $params this parameter is ignored in redis implementation.
 	 * @return integer the number of rows updated
 	 */
-	public static function updateAll($attributes, $condition = '', $params = array())
+	public static function updateAll($attributes, $condition = null, $params = array())
 	{
 		$db = static::getDb();
-		if ($condition==='') {
-			$condition = $db->executeCommand('LRANGE', array(static::tableName(), 0, -1));
-		}
 		if (empty($attributes)) {
 			return 0;
 		}
 		$n=0;
-		foreach($condition as $pk) {
+		foreach(static::fetchPks($condition) as $pk) {
-			$key = static::tableName() . ':a:' . static::hashPk($pk);
+			$newPk = $pk;
+			$pk = static::buildKey($pk);
+			$key = static::tableName() . ':a:' . $pk;
 			// save attributes
 			$args = array($key);
 			foreach($attributes as $attribute => $value) {
+				if (isset($newPk[$attribute])) {
+					$newPk[$attribute] = $value;
+				}
 				$args[] = $attribute;
 				$args[] = $value;
 			}
+			$newPk = static::buildKey($newPk);
+			$newKey = static::tableName() . ':a:' . $newPk;
 			$db->executeCommand('HMSET', $args);
+			// rename index
+			if ($newPk != $pk) {
+				// TODO make this atomic
+				$db->executeCommand('LINSERT', array(static::tableName(), 'AFTER', $pk, $newPk));
+				$db->executeCommand('LREM', array(static::tableName(), 0, $pk));
+				$db->executeCommand('RENAME', array($key, $newKey));
+			}
 			$n++;
 		}
 
@@ -205,24 +230,17 @@ abstract class ActiveRecord extends \yii\db\ActiveRecord
 	 *
 	 * @param array $counters the counters to be updated (attribute name => increment value).
 	 * Use negative values if you want to decrement the counters.
-	 * @param string|array $condition the conditions that will be put in the WHERE part of the UPDATE SQL.
+	 * @param array $condition the conditions that will be put in the WHERE part of the UPDATE SQL.
-	 * Please refer to [[Query::where()]] on how to specify this parameter.
+	 * Please refer to [[ActiveQuery::where()]] on how to specify this parameter.
-	 * @param array $params the parameters (name=>value) to be bound to the query.
+	 * @param array $params this parameter is ignored in redis implementation.
-	 * Do not name the parameters as `:bp0`, `:bp1`, etc., because they are used internally by this method.
 	 * @return integer the number of rows updated
 	 */
-	public static function updateAllCounters($counters, $condition = '', $params = array())
+	public static function updateAllCounters($counters, $condition = null, $params = array())
 	{
-		if (is_array($condition) && !isset($condition[0])) { // TODO do this in all *All methods
-			$condition = array($condition);
-		}
 		$db = static::getDb();
-		if ($condition==='') {
-			$condition = $db->executeCommand('LRANGE', array(static::tableName(), 0, -1));
-		}
 		$n=0;
-		foreach($condition as $pk) { // TODO allow multiple pks as condition
+		foreach(static::fetchPks($condition) as $pk) {
-			$key = static::tableName() . ':a:' . static::hashPk($pk);
+			$key = static::tableName() . ':a:' . static::buildKey($pk);
 			foreach($counters as $attribute => $value) {
 				$db->executeCommand('HINCRBY', array($key, $attribute, $value));
 			}
@@ -241,29 +259,74 @@ abstract class ActiveRecord extends \yii\db\ActiveRecord
 	 * Customer::deleteAll('status = 3');
 	 * ~~~
 	 *
-	 * @param string|array $condition the conditions that will be put in the WHERE part of the DELETE SQL.
+	 * @param array $condition the conditions that will be put in the WHERE part of the DELETE SQL.
-	 * Please refer to [[Query::where()]] on how to specify this parameter.
+	 * Please refer to [[ActiveQuery::where()]] on how to specify this parameter.
-	 * @param array $params the parameters (name=>value) to be bound to the query.
+	 * @param array $params this parameter is ignored in redis implementation.
 	 * @return integer the number of rows deleted
 	 */
-	public static function deleteAll($condition = '', $params = array())
+	public static function deleteAll($condition = null, $params = array())
 	{
 		$db = static::getDb();
-		if ($condition==='') {
-			$condition = $db->executeCommand('LRANGE', array(static::tableName(), 0, -1));
-		}
-		if (empty($condition)) {
-			return 0;
-		}
 		$attributeKeys = array();
-		foreach($condition as $pk) {
+		foreach(static::fetchPks($condition) as $pk) {
-			$pk = static::hashPk($pk);
+			$pk = static::buildKey($pk);
 			$db->executeCommand('LREM', array(static::tableName(), 0, $pk));
 			$attributeKeys[] = static::tableName() . ':a:' . $pk;
 		}
+		if (empty($attributeKeys)) {
+			return 0;
+		}
 		return $db->executeCommand('DEL', $attributeKeys);// TODO make this atomic or document as NOT
 	}
 
+	private static function fetchPks($condition)
+	{
+		$query = static::createQuery();
+		$query->where($condition);
+		$records = $query->asArray()->all(); // TODO limit fetched columns to pk
+		$primaryKey = static::primaryKey();
+
+		$pks = array();
+		foreach($records as $record) {
+			$pk = array();
+			foreach($primaryKey as $key) {
+				$pk[$key] = $record[$key];
+			}
+			$pks[] = $pk;
+		}
+		return $pks;
+	}
+
+
+	/**
+	 * Builds a normalized key from a given primary key value.
+	 *
+	 * @param mixed $key the key to be normalized
+	 * @return string the generated key
+	 */
+	public static function buildKey($key)
+	{
+		if (is_numeric($key)) {
+			return $key;
+		} elseif (is_string($key)) {
+			return ctype_alnum($key) && StringHelper::strlen($key) <= 32 ? $key : md5($key);
+		} elseif (is_array($key)) {
+			if (count($key) == 1) {
+				return self::buildKey(reset($key));
+			}
+			$isNumeric = true;
+			foreach($key as $value) {
+				if (!is_numeric($value)) {
+					$isNumeric = false;
+				}
+			}
+			if ($isNumeric) {
+				return implode('-', $key);
+			}
+		}
+		return md5(json_encode($key));
+	}
+
 	/**
 	 * Declares a `has-one` relation.
 	 * The declaration is returned in terms of an [[ActiveRelation]] instance

framework/yii/redis/LuaScriptBuilder.php

@@ -29,7 +29,7 @@ class LuaScriptBuilder extends \yii\base\Object
 		// TODO add support for orderBy
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "n=n+1 pks[n]=redis.call('HGETALL',$key .. pk)", 'pks'); // TODO properly hash pk
+		return $this->build($query, "n=n+1 pks[n]=redis.call('HGETALL',$key .. pk)", 'pks');
 	}
 
 	/**
@@ -42,7 +42,7 @@ class LuaScriptBuilder extends \yii\base\Object
 		// TODO add support for orderBy
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "do return redis.call('HGETALL',$key .. pk) end", 'pks'); // TODO properly hash pk
+		return $this->build($query, "do return redis.call('HGETALL',$key .. pk) end", 'pks');
 	}
 
 	/**
@@ -56,7 +56,7 @@ class LuaScriptBuilder extends \yii\base\Object
 		// TODO add support for orderBy and indexBy
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "n=n+1 pks[n]=redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ")", 'pks'); // TODO properly hash pk
+		return $this->build($query, "n=n+1 pks[n]=redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ")", 'pks');
 	}
 
 	/**
@@ -79,7 +79,7 @@ class LuaScriptBuilder extends \yii\base\Object
 	{
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "n=n+redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ")", 'n'); // TODO properly hash pk
+		return $this->build($query, "n=n+redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ")", 'n');
 	}
 
 	/**
@@ -92,7 +92,7 @@ class LuaScriptBuilder extends \yii\base\Object
 	{
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "n=n+1 if v==nil then v=0 end v=v+redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ")", 'v/n'); // TODO properly hash pk
+		return $this->build($query, "n=n+1 if v==nil then v=0 end v=v+redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ")", 'v/n');
 	}
 
 	/**
@@ -105,7 +105,7 @@ class LuaScriptBuilder extends \yii\base\Object
 	{
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "n=redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ") if v==nil or n<v then v=n end", 'v'); // TODO properly hash pk
+		return $this->build($query, "n=redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ") if v==nil or n<v then v=n end", 'v');
 	}
 
 	/**
@@ -118,7 +118,7 @@ class LuaScriptBuilder extends \yii\base\Object
 	{
 		$modelClass = $query->modelClass;
 		$key = $this->quoteValue($modelClass::tableName() . ':a:');
-		return $this->build($query, "n=redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ") if v==nil or n>v then v=n end", 'v'); // TODO properly hash pk
+		return $this->build($query, "n=redis.call('HGET',$key .. pk," . $this->quoteValue($column) . ") if v==nil or n>v then v=n end", 'v');
 	}
 
 	/**
@@ -140,14 +140,14 @@ class LuaScriptBuilder extends \yii\base\Object
 		$limitCondition = 'i>' . $start . ($query->limit === null ? '' : ' and i<=' . ($start + $query->limit));
 
 		$modelClass = $query->modelClass;
-		$key = $this->quoteValue($modelClass::tableName() . ':a:');
+		$key = $this->quoteValue($modelClass::tableName());
 		$loadColumnValues = '';
 		foreach($columns as $column => $alias) {
-			$loadColumnValues .= "local $alias=redis.call('HGET',$key .. pk, '$column')\n"; // TODO properly hash pk
+			$loadColumnValues .= "local $alias=redis.call('HGET',$key .. ':a:' .. pk, '$column')\n";
 		}
 
 		return <<<EOF
-local allpks=redis.call('LRANGE','$key',0,-1)
+local allpks=redis.call('LRANGE',$key,0,-1)
 local pks={}
 local n=0
 local v=nil

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

@@ -19,7 +19,12 @@ class Customer extends ActiveRecord
 		return $this->hasMany('Order', array('customer_id' => 'id'));
 	}
 
-	public static function getTableSchema()
+	public static function active($query)
+	{
+		$query->andWhere(array('status' => 1));
+	}
+
+	public static function getRecordSchema()
 	{
 		return new RecordSchema(array(
 			'name' => 'customer',

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

@@ -6,15 +6,12 @@ use yii\redis\RecordSchema;
 
 class Item extends ActiveRecord
 {
-	public static function getTableSchema()
+	public static function getRecordSchema()
 	{
 		return new RecordSchema(array(
 			'name' => 'item',
 			'primaryKey' => array('id'),
 			'sequenceName' => 'id',
-			'foreignKeys' => array(
-				// TODO for defining relations
-			),
 			'columns' => array(
 				'id' => 'integer',
 				'name' => 'string',

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

@@ -42,7 +42,7 @@ class Order extends ActiveRecord
 	}
 
 
-	public static function getTableSchema()
+	public static function getRecordSchema()
 	{
 		return new RecordSchema(array(
 			'name' => 'orders',

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

@@ -16,7 +16,7 @@ class OrderItem extends ActiveRecord
 		return $this->hasOne('Item', array('id' => 'item_id'));
 	}
 
-	public static function getTableSchema()
+	public static function getRecordSchema()
 	{
 		return new RecordSchema(array(
 			'name' => 'order_item',

tests/unit/framework/db/ActiveRecordTest.php

@@ -40,6 +40,12 @@ class ActiveRecordTest extends DatabaseTestCase
 		$customer = Customer::find(2);
 		$this->assertTrue($customer instanceof Customer);
 		$this->assertEquals('user2', $customer->name);
+		$customer = Customer::find(5);
+		$this->assertNull($customer);
+
+		// query scalar
+		$customerName = Customer::find()->where(array('id' => 2))->scalar('name');
+		$this->assertEquals('user2', $customerName);
 
 		// find by column values
 		$customer = Customer::find(array('id' => 2, 'name' => 'user2'));