From a54c1cd6841a3148c6da462b62c94b9a5e5df62c Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 20:38:03 +0600 Subject: [PATCH 01/11] Fixes #226: atomic operations and transaction support in AR. --- docs/api/db/ActiveRecord.md | 4 + docs/internals/ar.md | 43 +++++++++- yii/base/Model.php | 39 +++++---- yii/db/ActiveRecord.php | 205 +++++++++++++++++++++++++++++++++----------- 4 files changed, 221 insertions(+), 70 deletions(-) diff --git a/docs/api/db/ActiveRecord.md b/docs/api/db/ActiveRecord.md index 4e82793..d8bedb4 100644 --- a/docs/api/db/ActiveRecord.md +++ b/docs/api/db/ActiveRecord.md @@ -446,3 +446,7 @@ $customers = Customer::find()->olderThan(50)->all(); The parameters should follow after the `$query` parameter when defining the scope method, and they can take default values like shown above. + +### Atomic operations and scenarios + +TBD diff --git a/docs/internals/ar.md b/docs/internals/ar.md index c493269..8b55dcb 100644 --- a/docs/internals/ar.md +++ b/docs/internals/ar.md @@ -1,15 +1,50 @@ ActiveRecord ============ +Scenarios +--------- + +All possible scenario formats supported by ActiveRecord: + +```php +public function scenarios() +{ + return array( + // 1. attributes array + 'scenario1' => array('attribute1', 'attribute2'), + + // 2. insert, update and delete operations won't be wrapped with transaction (default mode) + 'scenario2' => array( + 'attributes' => array('attribute1', 'attribute2'), + 'atomic' => array(), // default value + ), + + // 3. all three operations (insert, update and delete) will be wrapped with transaction + 'scenario3' => array( + 'attributes' => array('attribute1', 'attribute2'), + 'atomic', + ), + + // 4. insert and update operations will be wrapped with transaction, delete won't + 'scenario4' => array( + 'attributes' => array('attribute1', 'attribute2'), + 'atomic' => array(self::INSERT, self::UPDATE), + ), + + // 5. insert and update operations won't be wrapped with transaction, delete will + 'scenario5' => array( + 'attributes' => array('attribute1', 'attribute2'), + 'atomic' => array(self::DELETE), + ), + ); +} +``` + Query ----- ### Basic Queries - - ### Relational Queries ### Scopes - - diff --git a/yii/base/Model.php b/yii/base/Model.php index 8348d97..683bfc7 100644 --- a/yii/base/Model.php +++ b/yii/base/Model.php @@ -590,18 +590,22 @@ class Model extends Component implements \IteratorAggregate, \ArrayAccess /** * Returns the attribute names that are safe to be massively assigned in the current scenario. - * @return array safe attribute names + * @return string[] safe attribute names */ public function safeAttributes() { $scenario = $this->getScenario(); $scenarios = $this->scenarios(); + if (!isset($scenarios[$scenario])) { + return array(); + } $attributes = array(); - if (isset($scenarios[$scenario])) { - foreach ($scenarios[$scenario] as $attribute) { - if ($attribute[0] !== '!') { - $attributes[] = $attribute; - } + if (isset($scenarios[$scenario]['attributes']) && is_array($scenarios[$scenario]['attributes'])) { + $scenarios[$scenario] = $scenarios[$scenario]['attributes']; + } + foreach ($scenarios[$scenario] as $attribute) { + if ($attribute[0] !== '!') { + $attributes[] = $attribute; } } return $attributes; @@ -609,23 +613,26 @@ class Model extends Component implements \IteratorAggregate, \ArrayAccess /** * Returns the attribute names that are subject to validation in the current scenario. - * @return array safe attribute names + * @return string[] safe attribute names */ public function activeAttributes() { $scenario = $this->getScenario(); $scenarios = $this->scenarios(); - if (isset($scenarios[$scenario])) { - $attributes = $scenarios[$this->getScenario()]; - foreach ($attributes as $i => $attribute) { - if ($attribute[0] === '!') { - $attributes[$i] = substr($attribute, 1); - } - } - return $attributes; - } else { + if (!isset($scenarios[$scenario])) { return array(); } + if (isset($scenarios[$scenario]['attributes']) && is_array($scenarios[$scenario]['attributes'])) { + $attributes = $scenarios[$scenario]['attributes']; + } else { + $attributes = $scenarios[$scenario]; + } + foreach ($attributes as $i => $attribute) { + if ($attribute[0] === '!') { + $attributes[$i] = substr($attribute, 1); + } + } + return $attributes; } /** diff --git a/yii/db/ActiveRecord.php b/yii/db/ActiveRecord.php index 41ce84c..286edca 100644 --- a/yii/db/ActiveRecord.php +++ b/yii/db/ActiveRecord.php @@ -74,6 +74,22 @@ class ActiveRecord extends Model const EVENT_AFTER_DELETE = 'afterDelete'; /** + * Represents insert ActiveRecord operation. This constant is used for specifying set of atomic operations + * for particular scenario in the [[scenarios()]] method. + */ + const OPERATION_INSERT = 'insert'; + /** + * Represents update ActiveRecord operation. This constant is used for specifying set of atomic operations + * for particular scenario in the [[scenarios()]] method. + */ + const OPERATION_UPDATE = 'update'; + /** + * Represents delete ActiveRecord operation. This constant is used for specifying set of atomic operations + * for particular scenario in the [[scenarios()]] method. + */ + const OPERATION_DELETE = 'delete'; + + /** * @var array attribute values indexed by attribute names */ private $_attributes = array(); @@ -664,10 +680,39 @@ class ActiveRecord extends Model * @param array $attributes list of attributes that need to be saved. Defaults to null, * meaning all attributes that are loaded from DB will be saved. * @return boolean whether the attributes are valid and the record is inserted successfully. + * @throws \Exception in case insert failed. */ public function insert($runValidation = true, $attributes = null) { - if ($runValidation && !$this->validate($attributes) || !$this->beforeSave(true)) { + if ($runValidation && !$this->validate($attributes)) { + return false; + } + $db = static::getDb(); + $transaction = $this->isOperationAtomic(self::OPERATION_INSERT) && null === $db->getTransaction() ? $db->beginTransaction() : null; + try { + $result = $this->internalInsert($attributes); + if (null !== $transaction) { + if (false === $result) { + $transaction->rollback(); + } else { + $transaction->commit(); + } + } + } catch (\Exception $e) { + if (null !== $transaction) { + $transaction->rollback(); + } + throw $e; + } + return $result; + } + + /** + * @see ActiveRecord::insert() + */ + private function internalInsert($attributes = null) + { + if (!$this->beforeSave(true)) { return false; } $values = $this->getDirtyAttributes($attributes); @@ -678,22 +723,23 @@ class ActiveRecord extends Model } $db = static::getDb(); $command = $db->createCommand()->insert($this->tableName(), $values); - if ($command->execute()) { - $table = $this->getTableSchema(); - if ($table->sequenceName !== null) { - foreach ($table->primaryKey as $name) { - if (!isset($this->_attributes[$name])) { - $this->_oldAttributes[$name] = $this->_attributes[$name] = $db->getLastInsertID($table->sequenceName); - break; - } + if (!$command->execute()) { + return false; + } + $table = $this->getTableSchema(); + if ($table->sequenceName !== null) { + foreach ($table->primaryKey as $name) { + if (!isset($this->_attributes[$name])) { + $this->_oldAttributes[$name] = $this->_attributes[$name] = $db->getLastInsertID($table->sequenceName); + break; } } - foreach ($values as $name => $value) { - $this->_oldAttributes[$name] = $value; - } - $this->afterSave(true); - return true; } + foreach ($values as $name => $value) { + $this->_oldAttributes[$name] = $value; + } + $this->afterSave(true); + return true; } /** @@ -744,39 +790,67 @@ class ActiveRecord extends Model * or [[beforeSave()]] stops the updating process. * @throws StaleObjectException if [[optimisticLock|optimistic locking]] is enabled and the data * being updated is outdated. + * @throws \Exception in case update failed. */ public function update($runValidation = true, $attributes = null) { - if ($runValidation && !$this->validate($attributes) || !$this->beforeSave(false)) { + if ($runValidation && !$this->validate($attributes)) { return false; } - $values = $this->getDirtyAttributes($attributes); - if (!empty($values)) { - $condition = $this->getOldPrimaryKey(true); - $lock = $this->optimisticLock(); - if ($lock !== null) { - if (!isset($values[$lock])) { - $values[$lock] = $this->$lock + 1; + $db = static::getDb(); + $transaction = $this->isOperationAtomic(self::OPERATION_UPDATE) && null === $db->getTransaction() ? $db->beginTransaction() : null; + try { + $result = $this->internalUpdate($attributes); + if (null !== $transaction) { + if (false === $result) { + $transaction->rollback(); + } else { + $transaction->commit(); } - $condition[$lock] = $this->$lock; } - // We do not check the return value of updateAll() because it's possible - // that the UPDATE statement doesn't change anything and thus returns 0. - $rows = $this->updateAll($values, $condition); - - if ($lock !== null && !$rows) { - throw new StaleObjectException('The object being updated is outdated.'); + } catch (\Exception $e) { + if (null !== $transaction) { + $transaction->rollback(); } + throw $e; + } + return $result; + } - foreach ($values as $name => $value) { - $this->_oldAttributes[$name] = $this->_attributes[$name]; + /** + * @see CActiveRecord::update() + * @throws StaleObjectException + */ + private function internalUpdate($attributes = null) + { + if (!$this->beforeSave(false)) { + return false; + } + $values = $this->getDirtyAttributes($attributes); + if (empty($values)) { + return 0; + } + $condition = $this->getOldPrimaryKey(true); + $lock = $this->optimisticLock(); + if ($lock !== null) { + if (!isset($values[$lock])) { + $values[$lock] = $this->$lock + 1; } + $condition[$lock] = $this->$lock; + } + // We do not check the return value of updateAll() because it's possible + // that the UPDATE statement doesn't change anything and thus returns 0. + $rows = $this->updateAll($values, $condition); - $this->afterSave(false); - return $rows; - } else { - return 0; + if ($lock !== null && !$rows) { + throw new StaleObjectException('The object being updated is outdated.'); + } + + foreach ($values as $name => $value) { + $this->_oldAttributes[$name] = $this->_attributes[$name]; } + $this->afterSave(false); + return $rows; } /** @@ -826,27 +900,43 @@ class ActiveRecord extends Model * Note that it is possible the number of rows deleted is 0, even though the deletion execution is successful. * @throws StaleObjectException if [[optimisticLock|optimistic locking]] is enabled and the data * being deleted is outdated. + * @throws \Exception in case delete failed. */ public function delete() { - if ($this->beforeDelete()) { - // we do not check the return value of deleteAll() because it's possible - // the record is already deleted in the database and thus the method will return 0 - $condition = $this->getOldPrimaryKey(true); - $lock = $this->optimisticLock(); - if ($lock !== null) { - $condition[$lock] = $this->$lock; + $db = static::getDb(); + $transaction = $this->isOperationAtomic(self::OPERATION_DELETE) && null === $db->getTransaction() ? $db->beginTransaction() : null; + try { + $result = false; + if ($this->beforeDelete()) { + // we do not check the return value of deleteAll() because it's possible + // the record is already deleted in the database and thus the method will return 0 + $condition = $this->getOldPrimaryKey(true); + $lock = $this->optimisticLock(); + if (null !== $lock) { + $condition[$lock] = $this->$lock; + } + $result = $this->deleteAll($condition); + if (null !== $lock && !$result) { + throw new StaleObjectException('The object being deleted is outdated.'); + } + $this->_oldAttributes = null; + $this->afterDelete(); } - $rows = $this->deleteAll($condition); - if ($lock !== null && !$rows) { - throw new StaleObjectException('The object being deleted is outdated.'); + if (null !== $transaction) { + if (false === $result) { + $transaction->rollback(); + } else { + $transaction->commit(); + } } - $this->_oldAttributes = null; - $this->afterDelete(); - return $rows; - } else { - return false; + } catch (\Exception $e) { + if (null !== $transaction) { + $transaction->rollback(); + } + throw $e; } + return $result; } /** @@ -1336,4 +1426,19 @@ class ActiveRecord extends Model } return true; } + + /** + * @param string $operation possible values are ActiveRecord::INSERT, ActiveRecord::UPDATE and ActiveRecord::DELETE. + * @return boolean whether given operation is atomic. Currently active scenario is taken into account. + */ + private function isOperationAtomic($operation) + { + $scenario = $this->getScenario(); + $scenarios = $this->scenarios(); + if (!isset($scenarios[$scenario]) || !isset($scenarios[$scenario]['attributes']) || !is_array($scenarios[$scenario]['attributes'])) { + return false; + } + return in_array('atomic', $scenarios[$scenario]) || + isset($scenarios[$scenario]['atomic']) && is_array($scenarios[$scenario]['atomic']) && in_array($operation, $scenarios[$scenario]['atomic']); + } } From 7fa9f113eee8c482baed82ee8ff4300d09984f26 Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 20:40:09 +0600 Subject: [PATCH 02/11] Fixed typo in internals/ar.md. --- docs/internals/ar.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/internals/ar.md b/docs/internals/ar.md index 8b55dcb..dea40de 100644 --- a/docs/internals/ar.md +++ b/docs/internals/ar.md @@ -28,13 +28,13 @@ public function scenarios() // 4. insert and update operations will be wrapped with transaction, delete won't 'scenario4' => array( 'attributes' => array('attribute1', 'attribute2'), - 'atomic' => array(self::INSERT, self::UPDATE), + 'atomic' => array(self::OPERATION_INSERT, self::OPERATION_UPDATE), ), // 5. insert and update operations won't be wrapped with transaction, delete will 'scenario5' => array( 'attributes' => array('attribute1', 'attribute2'), - 'atomic' => array(self::DELETE), + 'atomic' => array(self::OPERATION_DELETE), ), ); } From 31a46f5bcf1384c129e5c041db540f8514ab2828 Mon Sep 17 00:00:00 2001 From: Alexander Makarov Date: Mon, 13 May 2013 19:55:07 +0400 Subject: [PATCH 03/11] fixed minor JS codestyle issues --- yii/assets/yii.activeForm.js | 2 +- yii/assets/yii.captcha.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/yii/assets/yii.activeForm.js b/yii/assets/yii.activeForm.js index d987879..483df96 100644 --- a/yii/assets/yii.activeForm.js +++ b/yii/assets/yii.activeForm.js @@ -57,7 +57,7 @@ // whether to perform validation when a change is detected on the input validateOnChange: false, // whether to perform validation when the user is typing. - validateOnType: false, + validateOnType: false, // number of milliseconds that the validation should be delayed when a user is typing in the input field. validationDelay: 200, // whether to enable AJAX-based validation. diff --git a/yii/assets/yii.captcha.js b/yii/assets/yii.captcha.js index 9211edb..7dfe3cc 100644 --- a/yii/assets/yii.captcha.js +++ b/yii/assets/yii.captcha.js @@ -51,8 +51,8 @@ dataType: 'json', cache: false, success: function(data) { - $e.attr('src', data['url']); - $('body').data(settings.hashKey, [data['hash1'], data['hash2']]); + $e.attr('src', data.url); + $('body').data(settings.hashKey, [data.hash1, data.hash2]); } }); }, From 35688b327b7ddd0144d6c58b8a9f0e12eecd8b7e Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 22:02:15 +0600 Subject: [PATCH 04/11] AR atomic scenarios fixes. --- docs/internals/ar.md | 10 ++-------- yii/db/ActiveRecord.php | 52 ++++++++++++++++++++++++------------------------- 2 files changed, 28 insertions(+), 34 deletions(-) diff --git a/docs/internals/ar.md b/docs/internals/ar.md index dea40de..9bb54d3 100644 --- a/docs/internals/ar.md +++ b/docs/internals/ar.md @@ -19,19 +19,13 @@ public function scenarios() 'atomic' => array(), // default value ), - // 3. all three operations (insert, update and delete) will be wrapped with transaction - 'scenario3' => array( - 'attributes' => array('attribute1', 'attribute2'), - 'atomic', - ), - - // 4. insert and update operations will be wrapped with transaction, delete won't + // 3. insert and update operations will be wrapped with transaction, delete won't be wrapped 'scenario4' => array( 'attributes' => array('attribute1', 'attribute2'), 'atomic' => array(self::OPERATION_INSERT, self::OPERATION_UPDATE), ), - // 5. insert and update operations won't be wrapped with transaction, delete will + // 5. insert and update operations won't be wrapped with transaction, delete will be wrapped 'scenario5' => array( 'attributes' => array('attribute1', 'attribute2'), 'atomic' => array(self::OPERATION_DELETE), diff --git a/yii/db/ActiveRecord.php b/yii/db/ActiveRecord.php index 286edca..d11823d 100644 --- a/yii/db/ActiveRecord.php +++ b/yii/db/ActiveRecord.php @@ -77,17 +77,17 @@ class ActiveRecord extends Model * Represents insert ActiveRecord operation. This constant is used for specifying set of atomic operations * for particular scenario in the [[scenarios()]] method. */ - const OPERATION_INSERT = 'insert'; + const OP_INSERT = 'insert'; /** * Represents update ActiveRecord operation. This constant is used for specifying set of atomic operations * for particular scenario in the [[scenarios()]] method. */ - const OPERATION_UPDATE = 'update'; + const OP_UPDATE = 'update'; /** * Represents delete ActiveRecord operation. This constant is used for specifying set of atomic operations * for particular scenario in the [[scenarios()]] method. */ - const OPERATION_DELETE = 'delete'; + const OP_DELETE = 'delete'; /** * @var array attribute values indexed by attribute names @@ -688,18 +688,18 @@ class ActiveRecord extends Model return false; } $db = static::getDb(); - $transaction = $this->isOperationAtomic(self::OPERATION_INSERT) && null === $db->getTransaction() ? $db->beginTransaction() : null; + $transaction = $this->isOpAtomic(self::OP_INSERT) && $db->getTransaction() === null ? $db->beginTransaction() : null; try { - $result = $this->internalInsert($attributes); - if (null !== $transaction) { - if (false === $result) { + $result = $this->insertInternal($attributes); + if ($transaction !== null) { + if ($result === false) { $transaction->rollback(); } else { $transaction->commit(); } } } catch (\Exception $e) { - if (null !== $transaction) { + if ($transaction !== null) { $transaction->rollback(); } throw $e; @@ -710,7 +710,7 @@ class ActiveRecord extends Model /** * @see ActiveRecord::insert() */ - private function internalInsert($attributes = null) + private function insertInternal($attributes = null) { if (!$this->beforeSave(true)) { return false; @@ -798,18 +798,18 @@ class ActiveRecord extends Model return false; } $db = static::getDb(); - $transaction = $this->isOperationAtomic(self::OPERATION_UPDATE) && null === $db->getTransaction() ? $db->beginTransaction() : null; + $transaction = $this->isOpAtomic(self::OP_UPDATE) && $db->getTransaction() === null ? $db->beginTransaction() : null; try { - $result = $this->internalUpdate($attributes); - if (null !== $transaction) { - if (false === $result) { + $result = $this->updateInternal($attributes); + if ($transaction !== null) { + if ($result === false) { $transaction->rollback(); } else { $transaction->commit(); } } } catch (\Exception $e) { - if (null !== $transaction) { + if ($transaction !== null) { $transaction->rollback(); } throw $e; @@ -821,7 +821,7 @@ class ActiveRecord extends Model * @see CActiveRecord::update() * @throws StaleObjectException */ - private function internalUpdate($attributes = null) + private function updateInternal($attributes = null) { if (!$this->beforeSave(false)) { return false; @@ -905,7 +905,7 @@ class ActiveRecord extends Model public function delete() { $db = static::getDb(); - $transaction = $this->isOperationAtomic(self::OPERATION_DELETE) && null === $db->getTransaction() ? $db->beginTransaction() : null; + $transaction = $this->isOpAtomic(self::OP_DELETE) && $db->getTransaction() === null ? $db->beginTransaction() : null; try { $result = false; if ($this->beforeDelete()) { @@ -913,25 +913,25 @@ class ActiveRecord extends Model // the record is already deleted in the database and thus the method will return 0 $condition = $this->getOldPrimaryKey(true); $lock = $this->optimisticLock(); - if (null !== $lock) { + if ($lock !== null) { $condition[$lock] = $this->$lock; } $result = $this->deleteAll($condition); - if (null !== $lock && !$result) { + if ($lock !== null && !$result) { throw new StaleObjectException('The object being deleted is outdated.'); } $this->_oldAttributes = null; $this->afterDelete(); } - if (null !== $transaction) { - if (false === $result) { + if ($transaction !== null) { + if ($result === false) { $transaction->rollback(); } else { $transaction->commit(); } } } catch (\Exception $e) { - if (null !== $transaction) { + if ($transaction !== null) { $transaction->rollback(); } throw $e; @@ -1428,17 +1428,17 @@ class ActiveRecord extends Model } /** - * @param string $operation possible values are ActiveRecord::INSERT, ActiveRecord::UPDATE and ActiveRecord::DELETE. + * @param string $op possible values are ActiveRecord::INSERT, ActiveRecord::UPDATE and ActiveRecord::DELETE. * @return boolean whether given operation is atomic. Currently active scenario is taken into account. */ - private function isOperationAtomic($operation) + private function isOpAtomic($op) { $scenario = $this->getScenario(); $scenarios = $this->scenarios(); - if (!isset($scenarios[$scenario]) || !isset($scenarios[$scenario]['attributes']) || !is_array($scenarios[$scenario]['attributes'])) { + if (isset($scenarios[$scenario], $scenario[$scenario]['atomic']) && is_array($scenarios[$scenario]['atomic'])) { + return in_array($op, $scenarios[$scenario]['atomic']); + } else { return false; } - return in_array('atomic', $scenarios[$scenario]) || - isset($scenarios[$scenario]['atomic']) && is_array($scenarios[$scenario]['atomic']) && in_array($operation, $scenarios[$scenario]['atomic']); } } From 601407067670684334a971257a35759ed81d918e Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 22:30:05 +0600 Subject: [PATCH 05/11] AR atomic scenarios internals doc fix. --- docs/internals/ar.md | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/docs/internals/ar.md b/docs/internals/ar.md index 9bb54d3..f6d2b72 100644 --- a/docs/internals/ar.md +++ b/docs/internals/ar.md @@ -10,25 +10,13 @@ All possible scenario formats supported by ActiveRecord: public function scenarios() { return array( - // 1. attributes array + // attributes array, all operations won't be wrapped with transaction 'scenario1' => array('attribute1', 'attribute2'), - // 2. insert, update and delete operations won't be wrapped with transaction (default mode) + // insert and update operations will be wrapped with transaction, delete won't be wrapped 'scenario2' => array( 'attributes' => array('attribute1', 'attribute2'), - 'atomic' => array(), // default value - ), - - // 3. insert and update operations will be wrapped with transaction, delete won't be wrapped - 'scenario4' => array( - 'attributes' => array('attribute1', 'attribute2'), - 'atomic' => array(self::OPERATION_INSERT, self::OPERATION_UPDATE), - ), - - // 5. insert and update operations won't be wrapped with transaction, delete will be wrapped - 'scenario5' => array( - 'attributes' => array('attribute1', 'attribute2'), - 'atomic' => array(self::OPERATION_DELETE), + 'atomic' => array(self::OP_INSERT, self::OP_UPDATE), ), ); } From 944e1fc0fe237a4ca6dfd1ceb2d8abbc88ed4bda Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 22:32:15 +0600 Subject: [PATCH 06/11] =?UTF-8?q?isOpAtomic=20=E2=86=92=20isOperationAtomi?= =?UTF-8?q?c?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- yii/db/ActiveRecord.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/yii/db/ActiveRecord.php b/yii/db/ActiveRecord.php index d11823d..8da2c90 100644 --- a/yii/db/ActiveRecord.php +++ b/yii/db/ActiveRecord.php @@ -688,7 +688,7 @@ class ActiveRecord extends Model return false; } $db = static::getDb(); - $transaction = $this->isOpAtomic(self::OP_INSERT) && $db->getTransaction() === null ? $db->beginTransaction() : null; + $transaction = $this->isOperationAtomic(self::OP_INSERT) && $db->getTransaction() === null ? $db->beginTransaction() : null; try { $result = $this->insertInternal($attributes); if ($transaction !== null) { @@ -798,7 +798,7 @@ class ActiveRecord extends Model return false; } $db = static::getDb(); - $transaction = $this->isOpAtomic(self::OP_UPDATE) && $db->getTransaction() === null ? $db->beginTransaction() : null; + $transaction = $this->isOperationAtomic(self::OP_UPDATE) && $db->getTransaction() === null ? $db->beginTransaction() : null; try { $result = $this->updateInternal($attributes); if ($transaction !== null) { @@ -905,7 +905,7 @@ class ActiveRecord extends Model public function delete() { $db = static::getDb(); - $transaction = $this->isOpAtomic(self::OP_DELETE) && $db->getTransaction() === null ? $db->beginTransaction() : null; + $transaction = $this->isOperationAtomic(self::OP_DELETE) && $db->getTransaction() === null ? $db->beginTransaction() : null; try { $result = false; if ($this->beforeDelete()) { @@ -1431,7 +1431,7 @@ class ActiveRecord extends Model * @param string $op possible values are ActiveRecord::INSERT, ActiveRecord::UPDATE and ActiveRecord::DELETE. * @return boolean whether given operation is atomic. Currently active scenario is taken into account. */ - private function isOpAtomic($op) + private function isOperationAtomic($op) { $scenario = $this->getScenario(); $scenarios = $this->scenarios(); From 4973838b69df20e97f9b9f990e5d90937ac34553 Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 22:33:32 +0600 Subject: [PATCH 07/11] Better argument name in the ActiveRecord::isOperationAtomic() method. --- yii/db/ActiveRecord.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yii/db/ActiveRecord.php b/yii/db/ActiveRecord.php index 8da2c90..2b7d2b8 100644 --- a/yii/db/ActiveRecord.php +++ b/yii/db/ActiveRecord.php @@ -1428,15 +1428,15 @@ class ActiveRecord extends Model } /** - * @param string $op possible values are ActiveRecord::INSERT, ActiveRecord::UPDATE and ActiveRecord::DELETE. + * @param string $operation possible values are ActiveRecord::INSERT, ActiveRecord::UPDATE and ActiveRecord::DELETE. * @return boolean whether given operation is atomic. Currently active scenario is taken into account. */ - private function isOperationAtomic($op) + private function isOperationAtomic($operation) { $scenario = $this->getScenario(); $scenarios = $this->scenarios(); if (isset($scenarios[$scenario], $scenario[$scenario]['atomic']) && is_array($scenarios[$scenario]['atomic'])) { - return in_array($op, $scenarios[$scenario]['atomic']); + return in_array($operation, $scenarios[$scenario]['atomic']); } else { return false; } From 472902fa6607a3f543cff3ee838f4fd60e671f41 Mon Sep 17 00:00:00 2001 From: resurtm Date: Mon, 13 May 2013 23:05:41 +0600 Subject: [PATCH 08/11] AR internals doc fix. --- docs/internals/ar.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/internals/ar.md b/docs/internals/ar.md index f6d2b72..d59ba6a 100644 --- a/docs/internals/ar.md +++ b/docs/internals/ar.md @@ -4,7 +4,7 @@ ActiveRecord Scenarios --------- -All possible scenario formats supported by ActiveRecord: +Possible scenario formats supported by ActiveRecord: ```php public function scenarios() From 2d8a465c2fe6c6b35980b89e2b36f780256ed24a Mon Sep 17 00:00:00 2001 From: Qiang Xue Date: Tue, 14 May 2013 07:24:37 -0400 Subject: [PATCH 09/11] Fixes issue #257: removed the $view parameter from widget methods. --- apps/bootstrap/protected/views/layouts/main.php | 6 +-- apps/bootstrap/protected/views/site/contact.php | 4 +- apps/bootstrap/protected/views/site/login.php | 2 +- docs/guide/upgrade-from-v1.md | 2 +- yii/base/Controller.php | 1 + yii/base/View.php | 9 ++-- yii/base/Widget.php | 66 +++++++++++++------------ yii/web/Pagination.php | 2 +- yii/widgets/Breadcrumbs.php | 4 +- yii/widgets/Menu.php | 2 +- 10 files changed, 53 insertions(+), 45 deletions(-) diff --git a/apps/bootstrap/protected/views/layouts/main.php b/apps/bootstrap/protected/views/layouts/main.php index 05f7259..635e118 100644 --- a/apps/bootstrap/protected/views/layouts/main.php +++ b/apps/bootstrap/protected/views/layouts/main.php @@ -27,7 +27,7 @@ $this->registerAssetBundle('app'); - + endPage(); ?> diff --git a/apps/bootstrap/protected/views/site/contact.php b/apps/bootstrap/protected/views/site/contact.php index 4fa0db5..e740d0f 100644 --- a/apps/bootstrap/protected/views/site/contact.php +++ b/apps/bootstrap/protected/views/site/contact.php @@ -23,7 +23,7 @@ $this->params['breadcrumbs'][] = $this->title; If you have business inquiries or other questions, please fill out the following form to contact us. Thank you.

- array('class' => 'form-horizontal'), 'fieldConfig' => array('inputOptions' => array('class' => 'input-xlarge')), )); ?> @@ -35,7 +35,7 @@ $this->params['breadcrumbs'][] = $this->title; $field = $form->field($model, 'verifyCode'); echo $field->begin() . $field->label() - . Captcha::widget($this) + . Captcha::widget() . Html::activeTextInput($model, 'verifyCode', array('class' => 'input-medium')) . $field->error() . $field->end(); diff --git a/apps/bootstrap/protected/views/site/login.php b/apps/bootstrap/protected/views/site/login.php index 7d2fed2..f676b98 100644 --- a/apps/bootstrap/protected/views/site/login.php +++ b/apps/bootstrap/protected/views/site/login.php @@ -14,7 +14,7 @@ $this->params['breadcrumbs'][] = $this->title;

Please fill out the following fields to login:

- array('class' => 'form-horizontal'))); ?> + array('class' => 'form-horizontal'))); ?> field($model, 'username')->textInput(); ?> field($model, 'password')->passwordInput(); ?> field($model, 'rememberMe')->checkbox(); ?> diff --git a/docs/guide/upgrade-from-v1.md b/docs/guide/upgrade-from-v1.md index f02f825..c89b3e8 100644 --- a/docs/guide/upgrade-from-v1.md +++ b/docs/guide/upgrade-from-v1.md @@ -218,7 +218,7 @@ methods of the `Widget` class. For example, ```php // $this refers to the View object // Note that you have to "echo" the result to display it -echo \yii\widgets\Menu::widget($this, array('items' => $items)); +echo \yii\widgets\Menu::widget(array('items' => $items)); // $this refers to the View object $form = \yii\widgets\ActiveForm::begin($this); diff --git a/yii/base/Controller.php b/yii/base/Controller.php index 6b6c926..55db4ed 100644 --- a/yii/base/Controller.php +++ b/yii/base/Controller.php @@ -410,6 +410,7 @@ class Controller extends Component * Returns the view object that can be used to render views or view files. * The [[render()]], [[renderPartial()]] and [[renderFile()]] methods will use * this view object to implement the actual view rendering. + * If not set, it will default to the "view" application component. * @return View the view object that can be used to render views or view files. */ public function getView() diff --git a/yii/base/View.php b/yii/base/View.php index 2a6f71f..59b2c85 100644 --- a/yii/base/View.php +++ b/yii/base/View.php @@ -373,9 +373,10 @@ class View extends Component */ public function beginBlock($id, $renderInPlace = false) { - return Block::begin($this, array( + return Block::begin(array( 'id' => $id, 'renderInPlace' => $renderInPlace, + 'view' => $this, )); } @@ -406,9 +407,10 @@ class View extends Component */ public function beginContent($viewFile, $params = array()) { - return ContentDecorator::begin($this, array( + return ContentDecorator::begin(array( 'viewFile' => $viewFile, 'params' => $params, + 'view' => $this, )); } @@ -442,8 +444,9 @@ class View extends Component public function beginCache($id, $properties = array()) { $properties['id'] = $id; + $properties['view'] = $this; /** @var $cache FragmentCache */ - $cache = FragmentCache::begin($this, $properties); + $cache = FragmentCache::begin($properties); if ($cache->getCachedContent() !== false) { $this->endCache(); return false; diff --git a/yii/base/Widget.php b/yii/base/Widget.php index 9ec690c..0c948e9 100644 --- a/yii/base/Widget.php +++ b/yii/base/Widget.php @@ -18,16 +18,6 @@ use Yii; class Widget extends Component { /** - * @var View the view object that this widget is associated with. - * The widget will use this view object to register any needed assets. - * This property is also required by [[render()]] and [[renderFile()]]. - */ - public $view; - /** - * @var string id of the widget. - */ - private $_id; - /** * @var integer a counter used to generate [[id]] for widgets. * @internal */ @@ -39,32 +29,19 @@ class Widget extends Component */ public static $_stack = array(); - /** - * Constructor. - * @param View $view the view object that this widget is associated with. - * The widget will use this view object to register any needed assets. - * It is also required by [[render()]] and [[renderFile()]]. - * @param array $config name-value pairs that will be used to initialize the object properties - */ - public function __construct($view, $config = array()) - { - $this->view = $view; - parent::__construct($config); - } - + /** * Begins a widget. * This method creates an instance of the calling class. It will apply the configuration * to the created instance. A matching [[end()]] call should be called later. - * @param View $view the view object that the newly created widget is associated with. * @param array $config name-value pairs that will be used to initialize the object properties * @return Widget the newly created widget instance */ - public static function begin($view, $config = array()) + public static function begin($config = array()) { $config['class'] = get_called_class(); /** @var Widget $widget */ - $widget = Yii::createObject($config, $view); + $widget = Yii::createObject($config); self::$_stack[] = $widget; return $widget; } @@ -93,21 +70,22 @@ class Widget extends Component /** * Creates a widget instance and runs it. * The widget rendering result is returned by this method. - * @param View $view the view object that the newly created widget is associated with. * @param array $config name-value pairs that will be used to initialize the object properties * @return string the rendering result of the widget. */ - public static function widget($view, $config = array()) + public static function widget($config = array()) { ob_start(); ob_implicit_flush(false); /** @var Widget $widget */ $config['class'] = get_called_class(); - $widget = Yii::createObject($config, $view); + $widget = Yii::createObject($config); $widget->run(); return ob_get_clean(); } + private $_id; + /** * Returns the ID of the widget. * @param boolean $autoGenerate whether to generate an ID if it is not set previously @@ -130,6 +108,32 @@ class Widget extends Component $this->_id = $value; } + private $_view; + + /** + * Returns the view object that can be used to render views or view files. + * The [[render()]] and [[renderFile()]] methods will use + * this view object to implement the actual view rendering. + * If not set, it will default to the "view" application component. + * @return View the view object that can be used to render views or view files. + */ + public function getView() + { + if ($this->_view === null) { + $this->_view = Yii::$app->getView(); + } + return $this->_view; + } + + /** + * Sets the view object to be used by this widget. + * @param View $view the view object that can be used to render views or view files. + */ + public function setView($view) + { + $this->_view = $view; + } + /** * Executes the widget. */ @@ -159,7 +163,7 @@ class Widget extends Component public function render($view, $params = array()) { $viewFile = $this->findViewFile($view); - return $this->view->renderFile($viewFile, $params, $this); + return $this->getView()->renderFile($viewFile, $params, $this); } /** @@ -171,7 +175,7 @@ class Widget extends Component */ public function renderFile($file, $params = array()) { - return $this->view->renderFile($file, $params, $this); + return $this->getView()->renderFile($file, $params, $this); } /** diff --git a/yii/web/Pagination.php b/yii/web/Pagination.php index 73c0adb..52ff517 100644 --- a/yii/web/Pagination.php +++ b/yii/web/Pagination.php @@ -47,7 +47,7 @@ use Yii; * } * * // display pagination - * LinkPager::widget($this, array( + * LinkPager::widget(array( * 'pages' => $pages, * )); * ~~~ diff --git a/yii/widgets/Breadcrumbs.php b/yii/widgets/Breadcrumbs.php index 35772e0..5d15689 100644 --- a/yii/widgets/Breadcrumbs.php +++ b/yii/widgets/Breadcrumbs.php @@ -23,7 +23,7 @@ use yii\helpers\Html; * * ~~~ * // $this is the view object currently being used - * echo Breadcrumbs::widget($this, array( + * echo Breadcrumbs::widget(array( * 'links' => array( * array('label' => 'Sample Post', 'url' => array('post/edit', 'id' => 1)), * 'Edit', @@ -37,7 +37,7 @@ use yii\helpers\Html; * * ~~~ * // $this is the view object currently being used - * echo Breadcrumbs::widget($this, array( + * echo Breadcrumbs::widget(array( * 'links' => isset($this->params['breadcrumbs']) ? $this->params['breadcrumbs'] : array(), * )); * ~~~ diff --git a/yii/widgets/Menu.php b/yii/widgets/Menu.php index 3da0f31..e76f11f 100644 --- a/yii/widgets/Menu.php +++ b/yii/widgets/Menu.php @@ -27,7 +27,7 @@ use yii\helpers\Html; * * ~~~ * // $this is the view object currently being used - * echo Menu::widget($this, array( + * echo Menu::widget(array( * 'items' => array( * // Important: you need to specify url as 'controller/action', * // not just as 'controller' even if default action is used. From 15c2a2eed996aa168f25bfd5187a07ec94df1d55 Mon Sep 17 00:00:00 2001 From: Qiang Xue Date: Tue, 14 May 2013 07:32:42 -0400 Subject: [PATCH 10/11] Removed unneeded code. --- yii/web/CaptchaAction.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/yii/web/CaptchaAction.php b/yii/web/CaptchaAction.php index e3d6eaa..cff2314 100644 --- a/yii/web/CaptchaAction.php +++ b/yii/web/CaptchaAction.php @@ -260,10 +260,6 @@ class CaptchaAction extends Action (int)($this->foreColor % 0x10000 / 0x100), $this->foreColor % 0x100); - if ($this->fontFile === null) { - $this->fontFile = dirname(__FILE__) . '/SpicyRice.ttf'; - } - $length = strlen($code); $box = imagettfbbox(30, 0, $this->fontFile, $code); $w = $box[4] - $box[0] + $this->offset * ($length - 1); @@ -302,10 +298,6 @@ class CaptchaAction extends Action $image = new \Imagick(); $image->newImage($this->width, $this->height, $backColor); - if ($this->fontFile === null) { - $this->fontFile = dirname(__FILE__) . '/SpicyRice.ttf'; - } - $draw = new \ImagickDraw(); $draw->setFont($this->fontFile); $draw->setFontSize(30); From 5e888260d8df353a81b1d64d3b17117ba77c8f5d Mon Sep 17 00:00:00 2001 From: Oleksii Strutsynskyi Date: Tue, 14 May 2013 17:12:21 +0000 Subject: [PATCH 11/11] Fixed issue with getting Object component --- yii/base/Module.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yii/base/Module.php b/yii/base/Module.php index b1597e2..f2e67a9 100644 --- a/yii/base/Module.php +++ b/yii/base/Module.php @@ -449,7 +449,7 @@ abstract class Module extends Component public function getComponent($id, $load = true) { if (isset($this->_components[$id])) { - if ($this->_components[$id] instanceof Component) { + if ($this->_components[$id] instanceof Object) { return $this->_components[$id]; } elseif ($load) { Yii::trace("Loading component: $id", __METHOD__);