r81871 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81870‎ | r81871 | r81872 >
Date:02:43, 10 February 2011
Author:aaron
Status:deferred
Tags:
Comment:
* Refactored and cleaned up getPageStabilitySettings()
* Handle required cache purging in purgeExpiredConfigurations() and added wfReadOnly() check
* Made FlaggedArticle load stable rev and config in one query
* Cleanup getArticleID() usage and some vars in FlaggedRevision
* Various comment tweaks
Modified paths:
  • /trunk/extensions/FlaggedRevs/FlaggedArticle.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/FlaggedRevision.php (modified) (history)
  • /trunk/extensions/FlaggedRevs/FlaggedRevs.class.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FlaggedRevs/FlaggedArticle.php
——@@ -279,6 +279,16 @@
280280 }
281281
282282 /**
 283+ * Get the stable revision ID
 284+ * @param int $flags
 285+ * @return int
 286+ */
 287+ public function getStable( $flags = 0 ) {
 288+ $srev = $this->getStableRev( $flags );
 289+ return $srev ? $srev->getRevId() : 0;
 290+ }
 291+
 292+ /**
283293 * Get the stable revision
284294 * @param int $flags
285295 * @return mixed (FlaggedRevision/null)
——@@ -286,29 +296,15 @@
287297 public function getStableRev( $flags = 0 ) {
288298 # Cached results available?
289299 if ( $this->stableRev === null || ( $flags & FR_MASTER ) ) {
290 - $srev = null;
291 - if ( $this->isReviewable( $flags ) ) { // short-circuit if not reviewable
292 - $srev = FlaggedRevision::newFromStable( $this->getTitle(), $flags );
293 - }
294 - $this->stableRev = $srev ? $srev : false; // false => "found nothing"
 300+ $this->loadStableRevAndConfig();
295301 }
296302 if ( $this->stableRev ) {
297303 return $this->stableRev;
298304 }
299 - return null;
 305+ return null; // false => null
300306 }
301307
302308 /**
303 - * Get the stable revision ID
304 - * @param int $flags
305 - * @return int
306 - */
307 - public function getStable( $flags = 0 ) {
308 - $srev = $this->getStableRev( $flags );
309 - return $srev ? $srev->getRevId() : 0;
310 - }
311 -
312 - /**
313309 * Get visiblity restrictions on page
314310 * @param int $flags, FR_MASTER
315311 * @return array (select,override)
——@@ -317,7 +313,48 @@
318314 if ( !( $flags & FR_MASTER ) && $this->pageConfig !== null ) {
319315 return $this->pageConfig; // use process cache
320316 }
321 - $this->pageConfig = FlaggedRevs::getPageStabilitySettings( $this->getTitle(), $flags );
 317+ $this->loadStableRevAndConfig();
322318 return $this->pageConfig;
323319 }
 320+
 321+ /**
 322+ * Get a DB row of the stable version and page config of a title.
 323+ * @param Title $title, page title
 324+ * @param int $flags FR_MASTER
 325+ */
 326+ protected function loadStableRevAndConfig( $flags = 0 ) {
 327+ $this->stableRev = false; // false => "found nothing"
 328+ $this->pageConfig = FlaggedRevs::getDefaultVisibilitySettings(); // default
 329+ if ( !FlaggedRevs::inReviewNamespace( $this->getTitle() ) ) {
 330+ return; // short-circuit
 331+ }
 332+ # User master/slave as appropriate...
 333+ $db = ( $flags & FR_MASTER ) ?
 334+ wfGetDB( DB_MASTER ) : wfGetDB( DB_SLAVE );
 335+ $row = $db->selectRow(
 336+ array( 'page', 'flaggedpages', 'flaggedrevs', 'flaggedpage_config' ),
 337+ array_merge( FlaggedRevision::selectFields(),
 338+ array( 'fpc_override', 'fpc_level', 'fpc_expiry' ) ),
 339+ array( 'page_id' => $this->getID() ),
 340+ __METHOD__,
 341+ array(),
 342+ array(
 343+ 'flaggedpages' => array( 'LEFT JOIN', 'fp_page_id = page_id' ),
 344+ 'flaggedrevs' => array( 'LEFT JOIN',
 345+ 'fr_page_id = fp_page_id AND fr_rev_id = fp_stable' ),
 346+ 'flaggedpage_config' => array( 'LEFT JOIN', 'fpc_page_id = page_id' ) )
 347+ );
 348+ if ( !$row ) {
 349+ return; // no page found at all
 350+ }
 351+ if ( $row->fpc_override !== null ) { // page config row found
 352+ $this->pageConfig = FlaggedRevs::getVisibilitySettingsFromRow( $row );
 353+ }
 354+ if ( $row->fr_rev_id !== null ) { // stable rev row found
 355+ // Page may not reviewable, which implies no stable version
 356+ if ( !FlaggedRevs::useOnlyIfProtected() || $this->pageConfig['override'] ) {
 357+ $this->stableRev = new FlaggedRevision( $row );
 358+ }
 359+ }
 360+ }
324361 }
Index: trunk/extensions/FlaggedRevs/FlaggedRevs.class.php
——@@ -967,6 +967,13 @@
968968 array( 'fpc_page_id' => $title->getArticleID() ),
969969 __METHOD__
970970 );
 971+ return self::getVisibilitySettingsFromRow( $row );
 972+ }
 973+
 974+ /**
 975+ * Get page configuration settings from a DB row
 976+ */
 977+ public static function getVisibilitySettingsFromRow( $row ) {
971978 if ( $row ) {
972979 # This code should be refactored, now that it's being used more generally.
973980 $expiry = Block::decodeExpiry( $row->fpc_expiry );
——@@ -974,7 +981,6 @@
975982 if ( !$expiry || $expiry < wfTimestampNow() ) {
976983 $row = null; // expired
977984 self::purgeExpiredConfigurations();
978 - self::stableVersionUpdates( $title ); // re-find stable version
979985 }
980986 }
981987 // Is there a non-expired row?
——@@ -1059,36 +1065,54 @@
10601066 * The stable version of pages may change and invalidation may be required.
10611067 */
10621068 public static function purgeExpiredConfigurations() {
 1069+ if ( wfReadOnly() ) return;
 1070+
10631071 $dbw = wfGetDB( DB_MASTER );
1064 - $pageIds = array();
1065 - $pagesClearTracking = array();
10661072 $config = self::getDefaultVisibilitySettings(); // config is to be reset
10671073 $encCutoff = $dbw->addQuotes( $dbw->timestamp() );
1068 - $ret = $dbw->select( 'flaggedpage_config',
1069 - array( 'fpc_page_id' ),
1070 - array( 'fpc_expiry < ' . $encCutoff ),
 1074+ $ret = $dbw->select(
 1075+ array( 'flaggedpage_config', 'page' ),
 1076+ array( 'fpc_page_id', 'page_namespace', 'page_title' ),
 1077+ array( 'page_id = fpc_page_id', 'fpc_expiry < ' . $encCutoff ),
10711078 __METHOD__
10721079 // array( 'FOR UPDATE' )
10731080 );
1074 - foreach( $ret as $row ) {
1075 - // If FlaggedRevs got "turned off" for this page (due to not
1076 - // having the stable version as the default), then clear it
1077 - // from the tracking tables...
1078 - if ( !$config['override'] && self::useOnlyIfProtected() ) {
 1081+ $pagesClearConfig = array();
 1082+ $pagesClearTracking = $titlesClearTracking = array();
 1083+ foreach ( $ret as $row ) {
 1084+ # If FlaggedRevs got "turned off" (in protection config)
 1085+ # for this page, then clear it from the tracking tables...
 1086+ if ( self::useOnlyIfProtected() && !$config['override'] ) {
10791087 $pagesClearTracking[] = $row->fpc_page_id; // no stable version
 1088+ $titlesClearTracking[] = Title::newFromRow( $row ); // no stable version
10801089 }
1081 - $pageIds[] = $row->fpc_page_id; // page with expired config
 1090+ $pagesClearConfig[] = $row->fpc_page_id; // page with expired config
10821091 }
1083 - // Clear the expired config for these pages
1084 - if ( count( $pageIds ) ) {
 1092+ # Clear the expired config for these pages...
 1093+ if ( count( $pagesClearConfig ) ) {
10851094 $dbw->delete( 'flaggedpage_config',
1086 - array( 'fpc_page_id' => $pageIds, 'fpc_expiry < ' . $encCutoff ),
1087 - __METHOD__ );
 1095+ array( 'fpc_page_id' => $pagesClearConfig, 'fpc_expiry < ' . $encCutoff ),
 1096+ __METHOD__
 1097+ );
10881098 }
1089 - // Clear the tracking rows where needed
 1099+ # Clear the tracking rows and update page_touched for the
 1100+ # pages in $pagesClearConfig that do now have a stable version...
10901101 if ( count( $pagesClearTracking ) ) {
10911102 self::clearTrackingRows( $pagesClearTracking );
 1103+ $dbw->update( 'page',
 1104+ array( 'page_touched' => $dbw->timestamp() ),
 1105+ array( 'page_id' => $pagesClearTracking ),
 1106+ __METHOD__
 1107+ );
10921108 }
 1109+ # Also, clear their squid caches and purge other pages that use this page.
 1110+ # NOTE: all of these updates are deferred via $wgDeferredUpdateList.
 1111+ foreach ( $titlesClearTracking as $title ) {
 1112+ self::purgeSquid( $title );
 1113+ if ( FlaggedRevs::inclusionSetting() == FR_INCLUDES_STABLE ) {
 1114+ FlaggedRevs::HTMLCacheUpdates( $title ); // purge pages that use this page
 1115+ }
 1116+ }
10931117 }
10941118
10951119 # ################ Other utility functions #################
Index: trunk/extensions/FlaggedRevs/FlaggedRevision.php
——@@ -80,30 +80,29 @@
8181 * Note: will return NULL if the revision is deleted.
8282 * @param Title $title
8383 * @param int $revId
84 - * @param int $flags FR_MASTER
 84+ * @param int $flags (FR_MASTER, FR_FOR_UPDATE)
8585 * @return mixed FlaggedRevision (null on failure)
8686 */
8787 public static function newFromTitle( Title $title, $revId, $flags = 0 ) {
8888 if ( !FlaggedRevs::inReviewNamespace( $title ) ) {
8989 return null; // short-circuit
9090 }
91 - $columns = self::selectFields();
9291 $options = array();
93 - # User master/slave as appropriate
 92+ # User master/slave as appropriate...
9493 if ( $flags & FR_FOR_UPDATE || $flags & FR_MASTER ) {
9594 $db = wfGetDB( DB_MASTER );
9695 if ( $flags & FR_FOR_UPDATE ) $options[] = 'FOR UPDATE';
 96+ $pageId = $title->getArticleID( Title::GAID_FOR_UPDATE );
9797 } else {
9898 $db = wfGetDB( DB_SLAVE );
 99+ $pageId = $title->getArticleID();
99100 }
100 - $pageId = $title->getArticleID( $flags & FR_FOR_UPDATE ? Title::GAID_FOR_UPDATE : 0 );
101 - # Short-circuit query
102101 if ( !$pageId ) {
103 - return null;
 102+ return null; // short-circuit query
104103 }
105104 # Skip deleted revisions
106105 $row = $db->selectRow( array( 'flaggedrevs', 'revision' ),
107 - $columns,
 106+ self::selectFields(),
108107 array( 'fr_page_id' => $pageId,
109108 'fr_rev_id' => $revId,
110109 'rev_id = fr_rev_id',
——@@ -125,30 +124,30 @@
126125 /**
127126 * Get a FlaggedRevision of the stable version of a title.
128127 * @param Title $title, page title
129 - * @param int $flags FR_MASTER
 128+ * @param int $flags (FR_MASTER, FR_FOR_UPDATE)
130129 * @return mixed FlaggedRevision (null on failure)
131130 */
132131 public static function newFromStable( Title $title, $flags = 0 ) {
133132 if ( !FlaggedRevs::inReviewNamespace( $title ) ) {
134133 return null; // short-circuit
135134 }
136 - $columns = self::selectFields();
137135 $options = array();
138 - $pageId = $title->getArticleID( $flags & FR_MASTER ? Title::GAID_FOR_UPDATE : 0 );
139 - if ( !$pageId ) {
140 - return null; // short-circuit query
141 - }
142 - # User master/slave as appropriate
 136+ # User master/slave as appropriate...
143137 if ( $flags & FR_FOR_UPDATE || $flags & FR_MASTER ) {
144138 $db = wfGetDB( DB_MASTER );
145139 if ( $flags & FR_FOR_UPDATE ) $options[] = 'FOR UPDATE';
 140+ $pageId = $title->getArticleID( Title::GAID_FOR_UPDATE );
146141 } else {
147142 $db = wfGetDB( DB_SLAVE );
 143+ $pageId = $title->getArticleID();
148144 }
 145+ if ( !$pageId ) {
 146+ return null; // short-circuit query
 147+ }
149148 # Check tracking tables
150149 $row = $db->selectRow(
151150 array( 'flaggedpages', 'flaggedrevs' ),
152 - $columns,
 151+ self::selectFields(),
153152 array( 'fp_page_id' => $pageId,
154153 'fr_page_id = fp_page_id',
155154 'fr_rev_id = fp_stable'
——@@ -168,7 +167,7 @@
169168 * Get a FlaggedRevision of the stable version of a title.
170169 * Skips tracking tables to figure out new stable version.
171170 * @param Title $title, page title
172 - * @param int $flags FR_MASTER
 171+ * @param int $flags (FR_MASTER, FR_FOR_UPDATE)
173172 * @param array $config, optional page config (use to skip queries)
174173 * @param string $precedence (latest,quality,pristine)
175174 * @return mixed FlaggedRevision (null on failure)
——@@ -179,19 +178,19 @@
180179 if ( !FlaggedRevs::inReviewNamespace( $title ) ) {
181180 return null; // short-circuit
182181 }
183 - $columns = self::selectFields();
184182 $options = array();
185 - $pageId = $title->getArticleID( $flags & FR_FOR_UPDATE ? Title::GAID_FOR_UPDATE : 0 );
186 - if ( !$pageId ) {
187 - return null; // short-circuit query
188 - }
189 - # User master/slave as appropriate
 183+ # User master/slave as appropriate...
190184 if ( $flags & FR_FOR_UPDATE || $flags & FR_MASTER ) {
191185 $db = wfGetDB( DB_MASTER );
192186 if ( $flags & FR_FOR_UPDATE ) $options[] = 'FOR UPDATE';
 187+ $pageId = $title->getArticleID( Title::GAID_FOR_UPDATE );
193188 } else {
194189 $db = wfGetDB( DB_SLAVE );
 190+ $pageId = $title->getArticleID();
195191 }
 192+ if ( !$pageId ) {
 193+ return null; // short-circuit query
 194+ }
196195 # Get visiblity settings...
197196 if ( empty( $config ) ) {
198197 $config = FlaggedRevs::getPageStabilitySettings( $title, $flags );
——@@ -200,6 +199,7 @@
201200 return null; // page is not reviewable; no stable version
202201 }
203202 $row = null;
 203+ $columns = self::selectFields();
204204 $options['ORDER BY'] = 'fr_rev_id DESC';
205205 # Look for the latest pristine revision...
206206 if ( FlaggedRevs::pristineVersions() && $precedence !== 'latest' ) {
——@@ -331,9 +331,9 @@
332332 }
333333
334334 /**
335 - * @return Array basic select fields (not including text/text flags)
 335+ * @return Array basic select fields for FlaggedRevision DB row
336336 */
337 - protected static function selectFields() {
 337+ public static function selectFields() {
338338 return array(
339339 'fr_rev_id', 'fr_page_id', 'fr_user', 'fr_timestamp',
340340 'fr_comment', 'fr_quality', 'fr_tags', 'fr_img_name',

Follow-up revisions

RevisionCommit summaryAuthorDate
r82883Fix for r81871: pass $flags too loadStableRevAndConfig()aaron07:43, 27 February 2011

Status & tagging log