Merge pull request #1997 from cebe/1993-fixed-ar-afterFind

extensions/elasticsearch/ActiveQuery.php

@@ -116,6 +116,11 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 		if (!empty($this->with)) {
 			$this->findWith($this->with, $models);
 		}
+		if (!$this->asArray) {
+			foreach($models as $model) {
+				$model->afterFind();
+			}
+		}
 		return $models;
 	}
 
@@ -151,6 +156,9 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 			$this->findWith($this->with, $models);
 			$model = $models[0];
 		}
+		if (!$this->asArray) {
+			$model->afterFind();
+		}
 		return $model;
 	}
 
@@ -177,6 +185,11 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 			if (!empty($this->with)) {
 				$this->findWith($this->with, $models);
 			}
+			if (!$this->asArray) {
+				foreach($models as $model) {
+					$model->afterFind();
+				}
+			}
 			$result['hits']['hits'] = $models;
 		}
 		return $result;

extensions/elasticsearch/ActiveRecord.php

@@ -91,7 +91,9 @@ class ActiveRecord extends BaseActiveRecord
 		$command = static::getDb()->createCommand();
 		$result = $command->get(static::index(), static::type(), $primaryKey, $options);
 		if ($result['exists']) {
-			return static::create($result);
+			$model = static::create($result);
+			$model->afterFind();
+			return $model;
 		}
 		return null;
 	}
@@ -118,7 +120,9 @@ class ActiveRecord extends BaseActiveRecord
 		$models = [];
 		foreach($result['docs'] as $doc) {
 			if ($doc['exists']) {
-				$models[] = static::create($doc);
+				$model = static::create($doc);
+				$model->afterFind();
+				$models[] = $model;
 			}
 		}
 		return $models;
@@ -267,7 +271,9 @@ class ActiveRecord extends BaseActiveRecord
 	{
 		$record = parent::create($row['_source']);
 		$pk = static::primaryKey()[0];
+		if ($pk === '_id') {
 			$record->$pk = $row['_id'];
+		}
 		$record->_score = isset($row['_score']) ? $row['_score'] : null;
 		$record->_version = isset($row['_version']) ? $row['_version'] : null; // TODO version should always be available...
 		return $record;

extensions/elasticsearch/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 elasticsearch extension Change Log
 2.0.0 beta under development
 ----------------------------
 
+- Bug #1993: afterFind event in AR is now called after relations have been populated (cebe, creocoder)
 - Enh #1382: Added a debug toolbar panel for elasticsearch (cebe)
 - Enh #1765: Added support for primary key path mapping, pk can now be part of the attributes when mapping is defined (cebe)
 - Chg #1765: Changed handling of ActiveRecord primary keys, removed getId(), use getPrimaryKey() instead (cebe)

extensions/mongodb/ActiveQuery.php

@@ -53,6 +53,11 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 			if (!empty($this->with)) {
 				$this->findWith($this->with, $models);
 			}
+			if (!$this->asArray) {
+				foreach($models as $model) {
+					$model->afterFind();
+				}
+			}
 			return $models;
 		} else {
 			return [];
@@ -83,6 +88,9 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 				$this->findWith($this->with, $models);
 				$model = $models[0];
 			}
+			if (!$this->asArray) {
+				$model->afterFind();
+			}
 			return $model;
 		} else {
 			return null;

extensions/mongodb/file/ActiveQuery.php

@@ -53,6 +53,11 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 			if (!empty($this->with)) {
 				$this->findWith($this->with, $models);
 			}
+			if (!$this->asArray) {
+				foreach($models as $model) {
+					$model->afterFind();
+				}
+			}
 			return $models;
 		} else {
 			return [];
@@ -83,6 +88,9 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 				$this->findWith($this->with, $models);
 				$model = $models[0];
 			}
+			if (!$this->asArray) {
+				$model->afterFind();
+			}
 			return $model;
 		} else {
 			return null;
@@ -91,7 +99,7 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 
 	/**
 	 * Returns the Mongo collection for this query.
-	 * @param \yii\mongo\Connection $db Mongo connection.
+	 * @param \yii\mongodb\Connection $db Mongo connection.
 	 * @return Collection collection instance.
 	 */
 	public function getCollection($db = null)

extensions/redis/ActiveQuery.php

@@ -76,6 +76,11 @@ class ActiveQuery extends \yii\base\Component implements ActiveQueryInterface
 			if (!empty($this->with)) {
 				$this->findWith($this->with, $models);
 			}
+			if (!$this->asArray) {
+				foreach($models as $model) {
+					$model->afterFind();
+				}
+			}
 			return $models;
 		} else {
 			return [];
@@ -114,6 +119,9 @@ class ActiveQuery extends \yii\base\Component implements ActiveQueryInterface
 			$this->findWith($this->with, $models);
 			$model = $models[0];
 		}
+		if (!$this->asArray) {
+			$model->afterFind();
+		}
 		return $model;
 	}
 

extensions/redis/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 redis extension Change Log
 2.0.0 beta under development
 ----------------------------
 
+- Bug #1993: afterFind event in AR is now called after relations have been populated (cebe, creocoder)
 - Enh #1773: keyPrefix property of Session and Cache is not restricted to alnum characters anymore (cebe)
 
 2.0.0 alpha, December 1, 2013

extensions/sphinx/ActiveQuery.php

@@ -110,6 +110,11 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 				$this->findWith($this->with, $models);
 			}
 			$models = $this->fillUpSnippets($models);
+			if (!$this->asArray) {
+				foreach($models as $model) {
+					$model->afterFind();
+				}
+			}
 			return $models;
 		} else {
 			return [];
@@ -142,6 +147,9 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 				$model = $models[0];
 			}
 			list ($model) = $this->fillUpSnippets([$model]);
+			if (!$this->asArray) {
+				$model->afterFind();
+			}
 			return $model;
 		} else {
 			return null;

extensions/sphinx/ActiveRecord.php

@@ -641,7 +641,6 @@ abstract class ActiveRecord extends BaseActiveRecord
 			}
 		}
 		$record->setOldAttributes($record->getAttributes());
-		$record->afterFind();
 		return $record;
 	}
 

extensions/sphinx/CHANGELOG.md

@@ -4,6 +4,7 @@ Yii Framework 2 sphinx extension Change Log
 2.0.0 beta under development
 ----------------------------
 
+- Bug #1993: afterFind event in AR is now called after relations have been populated (cebe, creocoder)
 - Enh #1398: Refactor ActiveRecord to use BaseActiveRecord class of the framework (klimov-paul)
 
 2.0.0 alpha, December 1, 2013

extensions/sphinx/Query.php

@@ -668,7 +668,7 @@ class Query extends Component implements QueryInterface
 	 * Fills the query result rows with the snippets built from source determined by
 	 * [[snippetCallback]] result.
 	 * @param array $rows raw query result rows.
-	 * @return array query result rows with filled up snippets.
+	 * @return array|ActiveRecord[] query result rows with filled up snippets.
 	 */
 	protected function fillUpSnippets($rows)
 	{

framework/CHANGELOG.md

@@ -30,6 +30,7 @@ Yii Framework 2 Change Log
 - Bug #1937: Fixed wrong behavior or advanced app's `init --env` when called without parameter actually specified (samdark)
 - Bug #1959: `Html::activeCheckbox` wasn't respecting custom values for checked/unchecked state (klevron, samdark)
 - Bug #1965: `Controller::findLayoutFile()` returns incorrect file path when layout name starts with a slash (qiangxue)
+- Bug #1993: afterFind event in AR is now called after relations have been populated (cebe, creocoder)
 - Bug: Fixed `Call to a member function registerAssetFiles() on a non-object` in case of wrong `sourcePath` for an asset bundle (samdark)
 - Bug: Fixed incorrect event name for `yii\jui\Spinner` (samdark)
 - Bug: Json::encode() did not handle objects that implement JsonSerializable interface correctly (cebe)

framework/db/ActiveQuery.php

@@ -74,6 +74,11 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 			if (!empty($this->with)) {
 				$this->findWith($this->with, $models);
 			}
+			if (!$this->asArray) {
+				foreach($models as $model) {
+					$model->afterFind();
+				}
+			}
 			return $models;
 		} else {
 			return [];
@@ -146,6 +151,9 @@ class ActiveQuery extends Query implements ActiveQueryInterface
 				$this->findWith($this->with, $models);
 				$model = $models[0];
 			}
+			if (!$this->asArray) {
+				$model->afterFind();
+			}
 			return $model;
 		} else {
 			return null;

framework/db/BaseActiveRecord.php

@@ -983,11 +983,19 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
 	}
 
 	/**
-	 * Creates an active record object using a row of data.
-	 * This method is called by [[ActiveQuery]] to populate the query results
-	 * into Active Records. It is not meant to be used to create new records.
+	 * Creates an active record object using a row of data from the database/storage.
+	 *
+	 * This method is *not* meant to be used to create new records.
+	 *
+	 * It is an internal method meant to be called to create active record objects after
+	 * fetching data from the database. It is mainly used by [[ActiveQuery]] to populate
+	 * the query results into Active Records.
+	 *
+	 * When calling this method manually you should call [[afterFind()]] on the created
+	 * record to trigger the [[EVENT_AFTER_FIND|afterFind Event]].
+	 *
 	 * @param array $row attribute values (name => value)
-	 * @return ActiveRecord the newly created active record.
+	 * @return static the newly created active record.
 	 */
 	public static function create($row)
 	{
@@ -1001,7 +1009,6 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
 			}
 		}
 		$record->_oldAttributes = $record->_attributes;
-		$record->afterFind();
 		return $record;
 	}
 
@@ -1013,7 +1020,7 @@ abstract class BaseActiveRecord extends Model implements ActiveRecordInterface
 	 * For example, by creating a record based on the value of a column,
 	 * you may implement the so-called single-table inheritance mapping.
 	 * @param array $row row data to be populated into the record.
-	 * @return ActiveRecord the newly created active record
+	 * @return static the newly created active record
 	 */
 	public static function instantiate($row)
 	{

framework/db/Exception.php

@@ -41,4 +41,10 @@ class Exception extends \yii\base\Exception
 	{
 		return 'Database Exception';
 	}
+
+	public function __toString()
+	{
+		return parent::__toString() . PHP_EOL
+		. 'Additional Information:' . PHP_EOL . print_r($this->errorInfo, true);
+	}
 }

tests/unit/extensions/elasticsearch/ActiveRecordTest.php

@@ -2,6 +2,9 @@
 
 namespace yiiunit\extensions\elasticsearch;
 
+use yii\base\Event;
+use yii\base\Exception;
+use yii\db\BaseActiveRecord;
 use yii\elasticsearch\Connection;
 use yii\helpers\Json;
 use yiiunit\framework\ar\ActiveRecordTestTrait;
@@ -10,6 +13,7 @@ use yiiunit\data\ar\elasticsearch\Customer;
 use yiiunit\data\ar\elasticsearch\OrderItem;
 use yiiunit\data\ar\elasticsearch\Order;
 use yiiunit\data\ar\elasticsearch\Item;
+use yiiunit\TestCase;
 
 /**
  * @group elasticsearch
@@ -341,8 +345,8 @@ class ActiveRecordTest extends ElasticSearchTestCase
 	public function testBooleanAttribute()
 	{
 		$db = $this->getConnection();
-		Customer::setUpMapping($db->createCommand(), true);
 		Customer::deleteAll();
+		Customer::setUpMapping($db->createCommand(), true);
 
 		$customerClass = $this->getCustomerClass();
 		$customer = new $customerClass();
@@ -375,7 +379,7 @@ class ActiveRecordTest extends ElasticSearchTestCase
 		$this->assertEquals(2, count($customers));
 	}
 
-	public function testfindAsArrayFields()
+	public function testFindAsArrayFields()
 	{
 		$customerClass = $this->getCustomerClass();
 		/** @var TestCase|ActiveRecordTestTrait $this */
@@ -399,7 +403,7 @@ class ActiveRecordTest extends ElasticSearchTestCase
 		$this->assertArrayNotHasKey('status', $customers[2]);
 	}
 
-	public function testfindIndexByFields()
+	public function testFindIndexByFields()
 	{
 		$customerClass = $this->getCustomerClass();
 		/** @var TestCase|ActiveRecordTestTrait $this */
@@ -450,7 +454,7 @@ class ActiveRecordTest extends ElasticSearchTestCase
 		$this->assertNull($customers['3-user3']->status);
 	}
 
-	public function testfindIndexByAsArrayFields()
+	public function testFindIndexByAsArrayFields()
 	{
 		$customerClass = $this->getCustomerClass();
 		/** @var TestCase|ActiveRecordTestTrait $this */
@@ -495,5 +499,34 @@ class ActiveRecordTest extends ElasticSearchTestCase
 		$this->assertArrayNotHasKey('status', $customers['3-user3']);
 	}
 
+	public function testAfterFindGet()
+	{
+		/** @var BaseActiveRecord $customerClass */
+		$customerClass = $this->getCustomerClass();
+
+		$afterFindCalls = [];
+		Event::on(BaseActiveRecord::className(), BaseActiveRecord::EVENT_AFTER_FIND, function($event) use (&$afterFindCalls) {
+			/** @var BaseActiveRecord $ar */
+			$ar = $event->sender;
+			$afterFindCalls[] = [get_class($ar), $ar->getIsNewRecord(), $ar->getPrimaryKey(), $ar->isRelationPopulated('orders')];
+		});
+
+		$customer = Customer::get(1);
+		$this->assertNotNull($customer);
+		$this->assertEquals([[$customerClass, false, 1, false]], $afterFindCalls);
+		$afterFindCalls = [];
+
+		$customer = Customer::mget([1, 2]);
+		$this->assertNotNull($customer);
+		$this->assertEquals([
+			[$customerClass, false, 1, false],
+			[$customerClass, false, 2, false],
+		], $afterFindCalls);
+		$afterFindCalls = [];
+
+		Event::off(BaseActiveRecord::className(), BaseActiveRecord::EVENT_AFTER_FIND);
+	}
+
 
+	// TODO test AR with not mapped PK
 }
\ No newline at end of file

tests/unit/framework/ar/ActiveRecordTestTrait.php

@@ -7,7 +7,9 @@
 
 namespace yiiunit\framework\ar;
 
+use yii\base\Event;
 use yii\db\ActiveQueryInterface;
+use yii\db\BaseActiveRecord;
 use yiiunit\TestCase;
 use yiiunit\data\ar\Customer;
 use yiiunit\data\ar\Order;
@@ -835,4 +837,62 @@ trait ActiveRecordTestTrait
 		$customers = $this->callCustomerFind()->where(['status' => false])->all();
 		$this->assertEquals(1, count($customers));
 	}
+
+	public function testAfterFind()
+	{
+		/** @var BaseActiveRecord $customerClass */
+		$customerClass = $this->getCustomerClass();
+		/** @var BaseActiveRecord $orderClass */
+		$orderClass = $this->getOrderClass();
+		/** @var TestCase|ActiveRecordTestTrait $this */
+
+		$afterFindCalls = [];
+		Event::on(BaseActiveRecord::className(), BaseActiveRecord::EVENT_AFTER_FIND, function($event) use (&$afterFindCalls) {
+			/** @var BaseActiveRecord $ar */
+			$ar = $event->sender;
+			$afterFindCalls[] = [get_class($ar), $ar->getIsNewRecord(), $ar->getPrimaryKey(), $ar->isRelationPopulated('orders')];
+		});
+
+		$customer = $this->callCustomerFind(1);
+		$this->assertNotNull($customer);
+		$this->assertEquals([[$customerClass, false, 1, false]], $afterFindCalls);
+		$afterFindCalls = [];
+
+		$customer = $this->callCustomerFind()->where(['id' => 1])->one();
+		$this->assertNotNull($customer);
+		$this->assertEquals([[$customerClass, false, 1, false]], $afterFindCalls);
+		$afterFindCalls = [];
+
+		$customer = $this->callCustomerFind()->where(['id' => 1])->all();
+		$this->assertNotNull($customer);
+		$this->assertEquals([[$customerClass, false, 1, false]], $afterFindCalls);
+		$afterFindCalls = [];
+
+		$customer = $this->callCustomerFind()->where(['id' => 1])->with('orders')->all();
+		$this->assertNotNull($customer);
+		$this->assertEquals([
+			[$this->getOrderClass(), false, 1, false],
+			[$customerClass, false, 1, true],
+		], $afterFindCalls);
+		$afterFindCalls = [];
+
+		if ($this instanceof \yiiunit\extensions\redis\ActiveRecordTest) { // TODO redis does not support orderBy() yet
+			$customer = $this->callCustomerFind()->where(['id' => [1, 2]])->with('orders')->all();
+		} else {
+			// orderBy is needed to avoid random test failure
+			$customer = $this->callCustomerFind()->where(['id' => [1, 2]])->with('orders')->orderBy('name')->all();
+		}
+		$this->assertNotNull($customer);
+		$this->assertEquals([
+			[$orderClass, false, 1, false],
+			[$orderClass, false, 2, false],
+			[$orderClass, false, 3, false],
+			[$customerClass, false, 1, true],
+			[$customerClass, false, 2, true],
+		], $afterFindCalls);
+		$afterFindCalls = [];
+
+		Event::off(BaseActiveRecord::className(), BaseActiveRecord::EVENT_AFTER_FIND);
+	}
+
 }