Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

fixes #1 (multiple calls to safely*() trashed the condition on the lo…

…ck_version).

documentation updates
added abstract static method getCreatorUserId($id) that is an aux method that is intended to use caching.
  • Loading branch information...
commit ba34fcbd30206f9543dc2fec0978aa19a3a8e14c 1 parent e0f7dee
authored July 05, 2012

Showing 1 changed file with 51 additions and 6 deletions. Show diff stats Hide diff stats

  1. 57  PcBaseArModel.php
57  PcBaseArModel.php
@@ -60,7 +60,7 @@ public function safelyUpdateByPk($pk, array $attributes, $condition = '', $param
60 60
 		// first, check and explode if needed on incompatible condition given
61 61
 		$this->explodeOnNonSupportedCondition($condition);
62 62
 
63  
-		/* now, apply the locking condition which will protect again deletion if object was already updated by someone else */
  63
+		/* now, apply the locking condition which will protect against update if the model was already updated by someone else */
64 64
 		$this->applyLockingCondition($condition);
65 65
 
66 66
 		//increment object version
@@ -77,6 +77,8 @@ public function safelyUpdateByPk($pk, array $attributes, $condition = '', $param
77 77
 		if ($affectedRows != 1) {
78 78
 			throw new PcStaleObjectErrorException(Yii::t('PcBaseArModel', 'Data has been updated by another user so avoiding the update'));
79 79
 		}
  80
+		// reflect the updated lock version in the model. It might be used down the road in the same request so it must to be
  81
+		// updated to reflect the updated lock version.
80 82
 		$this->$lockingAttribute = $this->$lockingAttribute + 1;
81 83
 		return $affectedRows;
82 84
 	}
@@ -118,6 +120,20 @@ private function applyLockingCondition($condition = "") {
118 120
 		$lockingAttribute = self::LOCKING_ATTRIBUTE;
119 121
 		$expectedLockVersion = $this->$lockingAttribute;
120 122
 
  123
+		/*
  124
+		 * if safelyUpdate*...() is called several times in a single request, which is very much ok, this could cause a fatal bug since the
  125
+		 * $this->condition_string will be applied several times, causing a complete mess in the condition a total failure of the sql statement.
  126
+		 * Now, since during such several calls to update the model, the actual update is performed and the lock_version is advanced (by us) so
  127
+		 * to avoid this bug we need to calculate again the expected lock version and apply it in the condition.
  128
+		 */
  129
+		if (strpos($this->condition_string, "$lockingAttribute = ") !== false) {
  130
+			// condition already in. strip it. must be a reminiscence of past 'updates' to this model, in the same request
  131
+			$this->condition_string = preg_replace("/{$lockingAttribute} = \d+/", "", $this->condition_string);
  132
+			// strip any dangling "AND" words (the lock version condition is always concatenated at the end of the condition string so trim \
  133
+			// from end only)
  134
+			$this->condition_string = rtrim($this->condition_string, " ");
  135
+			$this->condition_string = preg_replace('/AND$/', '', $this->condition_string);
  136
+		}
121 137
 		// add to an existing condition, if such exists:
122 138
 		if (!empty($condition)) {
123 139
 			$this->condition_string .= ' AND ';
@@ -244,20 +260,49 @@ public function trimStringForBreadcrumbs($str) {
244 260
 	 *
245 261
 	 * Child classes should be able to return the relation name that relate this model to 'User' model (=users table)
246 262
 	 * What is it good for? Sometimes, we will be handling AR models of unknown types, such as in the case of handling
247  
-	 * 'inappropriate content', or any other case in which we get the AR model but do not know its type in advance.
248  
-	 * In those cases, we'd like to cache the loaded AR records. Those AR objects relate (i.e. via
249  
-	 * relations() method) to the 'users' table and in since we cache, we need to eagerly load the author username as well.
  263
+	 * a report of an 'inappropriate content', or any other case in which we get the AR model but do not know its type
  264
+	 * in advance. In those cases, we'd like to cache the loaded AR records for future use of the same record. Those AR
  265
+	 * objects relate (i.e. via relations() method) to the 'users' table and in since we cache the objects, we need to
  266
+	 * eagerly load some of the creator details (like username) as well.
250 267
 	 * This is where this method comes in handy.
251  
-	 * By each AR class implementing this method we can ask for the class for the relation name, rather than knowing that
252  
-	 * in advance.
  268
+	 *
  269
+	 * By forcing each AR class that extends this base class to implement this method we can ask for the extending class
  270
+	 * for the relation name, rather than knowing that in advance.
253 271
 	 *
254 272
 	 * For a working use case PcReportContent extension: PcReportContent._getContentCreatorUserId() method.
255 273
 	 *
  274
+	 * If this method is irrelevant to your extending class either return null or you can throw an exception (whatever fits the
  275
+	 * case and the reasonable logic)
  276
+	 *
256 277
 	 * @static
257 278
 	 * @abstract
258 279
 	 * @return string
259 280
 	 */
260 281
 	abstract public static function getCreatorRelationName();
  282
+	/**
  283
+	 * Method returns the model's creator user id. this is a common task that is useful. This method also caches the loaded
  284
+	 * object (or uses a cached object).
  285
+	 * Its highly recommended to use the following content for the method implementation within your model:
  286
+	 * > public static function getCreatorUserId($id) {
  287
+	 * >    $model = self::model()->cache(3600)->with('user')->findByPk($id);
  288
+	 * >    return $model->user_id;
  289
+	 * > }
  290
+	 * Notice that we cache the model with the (eagerly loaded) relating 'User' model. In some of my implementations I needed
  291
+	 * to load the username of the relating user. Using the same query here and in other locations enable actual re-use of
  292
+	 * the cached model. This way we get the entire relating user object at hand, of course at the
  293
+	 * price of possibly high cache usage - depending on your specific site. Consider lowering the lifetime used above (3600)
  294
+	 * if you exhaust your cache capacity.
  295
+	 *
  296
+	 * If this method is irrelevant to your extending class either return null or you can throw an exception (whatever fits the
  297
+	 * case and the reasonable logic)
  298
+	 *
  299
+	 * @static
  300
+	 * @abstract
  301
+	 *
  302
+	 * @param int $id the primary key for the model in question
  303
+	 * @return mixed
  304
+	 */
  305
+	abstract public static function getCreatorUserId($id);
261 306
 }
262 307
 
263 308
 /**

0 notes on commit ba34fcb

Please sign in to comment.
Something went wrong with that request. Please try again.