r96716 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r96715‎ | r96716 | r96717 >
Date:06:50, 10 September 2011
Author:demon
Status:ok (Comments)
Tags:
Comment:
Refactor deferrable updates into classes & interfaces, also add helper method for the most common use case:

$wgDeferredUpdateList[] = new HTMLCacheUpdate( $title, "sometable" );

I kept b/c with $wgDeferredUpdateList for now, but seeing as only 3 exts in svn use it (FileSearch, FlaggedRevs and WikiScripts), I'd like to deprecate it pretty soon :)
Modified paths:
  • /trunk/phase3/includes/AutoLoader.php (modified) (history)
  • /trunk/phase3/includes/DeferredUpdates.php (added) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/SiteStats.php (modified) (history)
  • /trunk/phase3/includes/ViewCountUpdate.php (modified) (history)
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/includes/WikiPage.php (modified) (history)
  • /trunk/phase3/includes/cache/HTMLCacheUpdate.php (modified) (history)
  • /trunk/phase3/includes/search/SearchUpdate.php (modified) (history)
  • /trunk/phase3/maintenance/importDump.php (modified) (history)
  • /trunk/phase3/tests/parser/parserTest.inc (modified) (history)
  • /trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/importDump.php
——@@ -209,7 +209,7 @@
210210 }
211211 wfWaitForSlaves();
212212 // XXX: Don't let deferred jobs array get absurdly large (bug 24375)
213 - wfDoUpdates( 'commit' );
 213+ DeferredUpdates::doUpdates( 'commit' );
214214 }
215215
216216 function progress( $string ) {
Index: trunk/phase3/tests/phpunit/suites/UploadFromUrlTestSuite.php
——@@ -14,7 +14,7 @@
1515 }
1616
1717 function setUp() {
18 - global $wgParser, $wgParserConf, $IP, $messageMemc, $wgMemc, $wgDeferredUpdateList,
 18+ global $wgParser, $wgParserConf, $IP, $messageMemc, $wgMemc,
1919 $wgUser, $wgLang, $wgOut, $wgRequest, $wgStyleDirectory, $wgEnableParserCache,
2020 $wgNamespaceAliases, $wgNamespaceProtection, $wgLocalFileRepo,
2121 $parserMemc, $wgThumbnailScriptPath, $wgScriptPath,
——@@ -41,7 +41,7 @@
4242
4343
4444 $wgEnableParserCache = false;
45 - $wgDeferredUpdateList = array();
 45+ DeferredUpdates::clearPendingUpdates();
4646 $wgMemc = wfGetMainCache();
4747 $messageMemc = wfGetMessageCacheStorage();
4848 $parserMemc = wfGetParserCacheStorage();
Index: trunk/phase3/tests/parser/parserTest.inc
——@@ -132,7 +132,7 @@
133133 }
134134
135135 static function setUp() {
136 - global $wgParser, $wgParserConf, $IP, $messageMemc, $wgMemc, $wgDeferredUpdateList,
 136+ global $wgParser, $wgParserConf, $IP, $messageMemc, $wgMemc,
137137 $wgUser, $wgLang, $wgOut, $wgRequest, $wgStyleDirectory, $wgEnableParserCache,
138138 $wgNamespaceAliases, $wgNamespaceProtection, $wgLocalFileRepo,
139139 $parserMemc, $wgThumbnailScriptPath, $wgScriptPath,
——@@ -160,7 +160,7 @@
161161
162162
163163 $wgEnableParserCache = false;
164 - $wgDeferredUpdateList = array();
 164+ DeferredUpdates::clearPendingUpdates();
165165 $wgMemc = wfGetMainCache();
166166 $messageMemc = wfGetMessageCacheStorage();
167167 $parserMemc = wfGetParserCacheStorage();
Index: trunk/phase3/includes/search/SearchUpdate.php
——@@ -13,7 +13,7 @@
1414 *
1515 * @ingroup Search
1616 */
17 -class SearchUpdate {
 17+class SearchUpdate implements DeferrableUpdate {
1818
1919 private $mId = 0, $mNamespace, $mTitle, $mText;
2020 private $mTitleWords;
Index: trunk/phase3/includes/SiteStats.php
——@@ -220,9 +220,9 @@
221221 }
222222
223223 /**
224 - *
 224+ * Class for handling updates to the site_stats table
225225 */
226 -class SiteStatsUpdate {
 226+class SiteStatsUpdate implements DeferrableUpdate {
227227
228228 var $mViews, $mEdits, $mGood, $mPages, $mUsers;
229229
Index: trunk/phase3/includes/GlobalFunctions.php
——@@ -2918,35 +2918,11 @@
29192919 /**
29202920 * Do any deferred updates and clear the list
29212921 *
2922 - * @param $commit String: set to 'commit' to commit after every update to
2923 - * prevent lock contention
 2922+ * @deprecated since 1.19
 2923+ * @see DeferredUpdates::doUpdate()
29242924 */
29252925 function wfDoUpdates( $commit = '' ) {
2926 - global $wgDeferredUpdateList;
2927 -
2928 - wfProfileIn( __METHOD__ );
2929 -
2930 - // No need to get master connections in case of empty updates array
2931 - if ( !count( $wgDeferredUpdateList ) ) {
2932 - wfProfileOut( __METHOD__ );
2933 - return;
2934 - }
2935 -
2936 - $doCommit = $commit == 'commit';
2937 - if ( $doCommit ) {
2938 - $dbw = wfGetDB( DB_MASTER );
2939 - }
2940 -
2941 - foreach ( $wgDeferredUpdateList as $update ) {
2942 - $update->doUpdate();
2943 -
2944 - if ( $doCommit && $dbw->trxLevel() ) {
2945 - $dbw->commit();
2946 - }
2947 - }
2948 -
2949 - $wgDeferredUpdateList = array();
2950 - wfProfileOut( __METHOD__ );
 2926+ DeferredUpdates::doUpdates( $commit );
29512927 }
29522928
29532929 /**
Index: trunk/phase3/includes/DeferredUpdates.php
——@@ -0,0 +1,83 @@
 2+<?php
 3+/**
 4+ * Interface that deferrable updates should implement. Basically required so we
 5+ * can validate input on DeferredUpdates::addUpdate()
 6+ */
 7+interface DeferrableUpdate {
 8+ /**
 9+ * Perform the actual work
 10+ */
 11+ function doUpdate();
 12+}
 13+
 14+/**
 15+ * Class for mananging the deferred updates.
 16+ */
 17+class DeferredUpdates {
 18+ /**
 19+ * Store of updates to be deferred until the end of the request.
 20+ */
 21+ private static $updates = array();
 22+
 23+ /**
 24+ * Add an update to the deferred list
 25+ * @param $update DeferrableUpdate Some object that implements doUpdate()
 26+ */
 27+ public static function addUpdate( DeferrableUpdate $update ) {
 28+ array_push( self::$updates, $update );
 29+ }
 30+
 31+ /**
 32+ * HTMLCacheUpdates are the most common deferred update people use. This
 33+ * is a shortcut method for that.
 34+ * @see HTMLCacheUpdate::__construct()
 35+ */
 36+ public static function addHTMLCacheUpdate( $title, $table ) {
 37+ self::addUpdate( new HTMLCacheUpdate( $title, $table ) );
 38+ }
 39+
 40+ /**
 41+ * Do any deferred updates and clear the list
 42+ *
 43+ * @param $commit String: set to 'commit' to commit after every update to
 44+ * prevent lock contention
 45+ */
 46+ public static function doUpdates( $commit = '' ) {
 47+ global $wgDeferredUpdateList;
 48+
 49+ wfProfileIn( __METHOD__ );
 50+
 51+ $updates = array_merge( $wgDeferredUpdateList, self::$updates );
 52+
 53+ // No need to get master connections in case of empty updates array
 54+ if ( !count( $updates ) ) {
 55+ wfProfileOut( __METHOD__ );
 56+ return;
 57+ }
 58+
 59+ $doCommit = $commit == 'commit';
 60+ if ( $doCommit ) {
 61+ $dbw = wfGetDB( DB_MASTER );
 62+ }
 63+
 64+ foreach ( $updates as $update ) {
 65+ $update->doUpdate();
 66+
 67+ if ( $doCommit && $dbw->trxLevel() ) {
 68+ $dbw->commit();
 69+ }
 70+ }
 71+
 72+ self::clearPendingUpdates();
 73+ wfProfileOut( __METHOD__ );
 74+ }
 75+
 76+ /**
 77+ * Clear all pending updates without performing them. Generally, you don't
 78+ * want or need to call this. Unit tests need it though.
 79+ */
 80+ public static function clearPendingUpdates() {
 81+ global $wgDeferredUpdateList;
 82+ $wgDeferredUpdateList = self::$updates = array();
 83+ }
 84+}
Property changes on: trunk/phase3/includes/DeferredUpdates.php
___________________________________________________________________
Added: svn:eol-style
185 + native
Index: trunk/phase3/includes/AutoLoader.php
——@@ -49,6 +49,8 @@
5050 'ContextSource' => 'includes/RequestContext.php',
5151 'Cookie' => 'includes/Cookie.php',
5252 'CookieJar' => 'includes/Cookie.php',
 53+ 'DeferrableUpdate' => 'includes/DeferredUpdates.php',
 54+ 'DeferredUpdates' => 'includes/DeferredUpdates.php',
5355 'DiffHistoryBlob' => 'includes/HistoryBlob.php',
5456 'DjVuImage' => 'includes/DjVuImage.php',
5557 'DoubleReplacer' => 'includes/StringUtils.php',
Index: trunk/phase3/includes/Wiki.php
——@@ -377,7 +377,7 @@
378378 // Output everything!
379379 $this->context->getOutput()->output();
380380 // Do any deferred jobs
381 - wfDoUpdates( 'commit' );
 381+ DeferredUpdates::doUpdates( 'commit' );
382382 $this->doJobs();
383383 wfProfileOut( __METHOD__ );
384384 }
Index: trunk/phase3/includes/cache/HTMLCacheUpdate.php
——@@ -23,8 +23,7 @@
2424 *
2525 * @ingroup Cache
2626 */
27 -class HTMLCacheUpdate
28 -{
 27+class HTMLCacheUpdate implements DeferrableUpdate {
2928 /**
3029 * @var Title
3130 */
Index: trunk/phase3/includes/WikiPage.php
——@@ -1275,7 +1275,7 @@
12761276
12771277 # Do updates right now unless deferral was requested
12781278 if ( !( $flags & EDIT_DEFER_UPDATES ) ) {
1279 - wfDoUpdates();
 1279+ DeferredUpdates::doUpdates();
12801280 }
12811281
12821282 // Return the new revision (or null) to the caller
——@@ -1604,7 +1604,7 @@
16051605 public function doDeleteArticle(
16061606 $reason, $suppress = false, $id = 0, $commit = true, &$error = '', User $user = null
16071607 ) {
1608 - global $wgDeferredUpdateList, $wgUseTrackbacks, $wgEnableInterwikiTemplatesTracking, $wgGlobalDatabase, $wgUser;
 1608+ global $wgUseTrackbacks, $wgEnableInterwikiTemplatesTracking, $wgGlobalDatabase, $wgUser;
16091609 $user = is_null( $user ) ? $wgUser : $user;
16101610
16111611 wfDebug( __METHOD__ . "\n" );
——@@ -1620,8 +1620,9 @@
16211621 return false;
16221622 }
16231623
1624 - $u = new SiteStatsUpdate( 0, 1, - (int)$this->isCountable(), -1 );
1625 - array_push( $wgDeferredUpdateList, $u );
 1624+ DeferredUpdates::addUpdate(
 1625+ new SiteStatsUpdate( 0, 1, - (int)$this->isCountable(), -1 )
 1626+ );
16261627
16271628 // Bitfields to further suppress the content
16281629 if ( $suppress ) {
——@@ -1939,15 +1940,15 @@
19401941 * @param $user User The relevant user
19411942 */
19421943 public function doViewUpdates( User $user ) {
1943 - global $wgDeferredUpdateList, $wgDisableCounters;
 1944+ global $wgDisableCounters;
19441945 if ( wfReadOnly() ) {
19451946 return;
19461947 }
19471948
19481949 # Don't update page view counters on views from bot users (bug 14044)
19491950 if ( !$wgDisableCounters && !$user->isAllowed( 'bot' ) && $this->getId() ) {
1950 - $wgDeferredUpdateList[] = new ViewCountUpdate( $this->getId() );
1951 - $wgDeferredUpdateList[] = new SiteStatsUpdate( 1, 0, 0 );
 1951+ DeferredUpdates::addUpdate( new ViewCountUpdate( $this->getId() ) );
 1952+ DeferredUpdates::addUpdate( new SiteStatsUpdate( 1, 0, 0 ) );
19521953 }
19531954
19541955 # Update newtalk / watchlist notification status
——@@ -2004,7 +2005,7 @@
20052006 * - null: don't change the article count
20062007 */
20072008 public function doEditUpdates( Revision $revision, User $user, array $options = array() ) {
2008 - global $wgDeferredUpdateList, $wgEnableParserCache;
 2009+ global $wgEnableParserCache;
20092010
20102011 wfProfileIn( __METHOD__ );
20112012
——@@ -2072,8 +2073,8 @@
20732074 $total = 0;
20742075 }
20752076
2076 - $wgDeferredUpdateList[] = new SiteStatsUpdate( 0, 1, $good, $total );
2077 - $wgDeferredUpdateList[] = new SearchUpdate( $id, $title, $text );
 2077+ DeferredUpdates::addUpdate( new SiteStatsUpdate( 0, 1, $good, $total ) );
 2078+ DeferredUpdates::addUpdate( new SearchUpdate( $id, $title, $text ) );
20782079
20792080 # If this is another user's talk page, update newtalk.
20802081 # Don't do this if $options['changed'] = false (null-edits) nor if
——@@ -2221,8 +2222,6 @@
22222223 * @param $title Title object
22232224 */
22242225 public static function onArticleCreate( $title ) {
2225 - global $wgDeferredUpdateList;
2226 -
22272226 # Update existence markers on article/talk tabs...
22282227 if ( $title->isTalkPage() ) {
22292228 $other = $title->getSubjectPage();
——@@ -2238,7 +2237,7 @@
22392238 $title->deleteTitleProtection();
22402239
22412240 # Invalidate caches of distant articles which transclude this page
2242 - $wgDeferredUpdateList[] = new HTMLCacheUpdate( $title, 'globaltemplatelinks' );
 2241+ DeferredUpdates::addHTMLCacheUpdate( $title, 'globaltemplatelinks' );
22432242 }
22442243
22452244 /**
——@@ -2247,8 +2246,6 @@
22482247 * @param $title Title
22492248 */
22502249 public static function onArticleDelete( $title ) {
2251 - global $wgDeferredUpdateList;
2252 -
22532250 # Update existence markers on article/talk tabs...
22542251 if ( $title->isTalkPage() ) {
22552252 $other = $title->getSubjectPage();
——@@ -2286,7 +2283,7 @@
22872284 RepoGroup::singleton()->getLocalRepo()->invalidateImageRedirect( $title );
22882285
22892286 # Invalidate caches of distant articles which transclude this page
2290 - $wgDeferredUpdateList[] = new HTMLCacheUpdate( $title, 'globaltemplatelinks' );
 2287+ DeferredUpdates::addHTMLCacheUpdate( $title, 'globaltemplatelinks' );
22912288 }
22922289
22932290 /**
——@@ -2296,16 +2293,14 @@
22972294 * @todo: verify that $title is always a Title object (and never false or null), add Title hint to parameter $title
22982295 */
22992296 public static function onArticleEdit( $title ) {
2300 - global $wgDeferredUpdateList;
2301 -
23022297 // Invalidate caches of articles which include this page
2303 - $wgDeferredUpdateList[] = new HTMLCacheUpdate( $title, 'templatelinks' );
 2298+ DeferredUpdates::addHTMLCacheUpdate( $title, 'templatelinks' );
23042299
23052300 // Invalidate caches of distant articles which transclude this page
2306 - $wgDeferredUpdateList[] = new HTMLCacheUpdate( $title, 'globaltemplatelinks' );
 2301+ DeferredUpdates::addHTMLCacheUpdate( $title, 'globaltemplatelinks' );
23072302
23082303 // Invalidate the caches of all pages which redirect here
2309 - $wgDeferredUpdateList[] = new HTMLCacheUpdate( $title, 'redirect' );
 2304+ DeferredUpdates::addHTMLCacheUpdate( $title, 'redirect' );
23102305
23112306 # Purge squid for this page only
23122307 $title->purgeSquid();
Index: trunk/phase3/includes/ViewCountUpdate.php
——@@ -27,7 +27,7 @@
2828 * 'page_counter' field or use the 'hitcounter' table and then collect the data
2929 * from that table to update the 'page_counter' field in a batch operation.
3030 */
31 -class ViewCountUpdate {
 31+class ViewCountUpdate implements DeferrableUpdate {
3232 protected $id;
3333
3434 /**

Sign-offs

UserFlagDate
Nikerabbitinspected15:16, 10 September 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r96725Add @since to the Deferred stuff, ping r96716demon17:16, 10 September 2011

Comments

#Comment by Nikerabbit (talk | contribs)   15:15, 10 September 2011

@since annotations would be nice.

Just wondering, could DeferredUpdates::addUpdate() be shortened to DeferredUpdates::add()?

#Comment by 😂 (talk | contribs)   17:17, 10 September 2011

Added @since in the followup. I guess you could shorten it if that really bothers you...but I'm lazy, it's a Saturday and I have a headache.

#Comment by Nikerabbit (talk | contribs)   17:18, 10 September 2011

/me too.

Status & tagging log