r81039 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r81038‎ | r81039 | r81040 >
Date:17:41, 26 January 2011
Author:ashley
Status:ok (Comments)
Tags:
Comment:
MessageCache.php: fixed a typo, tweaked spacing, added braces where needed, etc.
Modified paths:
  • /trunk/phase3/includes/MessageCache.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/MessageCache.php
——@@ -7,9 +7,9 @@
88 /**
99 *
1010 */
11 -define( 'MSG_LOAD_TIMEOUT', 60);
12 -define( 'MSG_LOCK_TIMEOUT', 10);
13 -define( 'MSG_WAIT_TIMEOUT', 10);
 11+define( 'MSG_LOAD_TIMEOUT', 60 );
 12+define( 'MSG_LOCK_TIMEOUT', 10 );
 13+define( 'MSG_WAIT_TIMEOUT', 10 );
1414 define( 'MSG_CACHE_VERSION', 1 );
1515
1616 /**
——@@ -82,7 +82,7 @@
8383 }
8484
8585 /**
86 - * Destroy the signleton instance
 86+ * Destroy the singleton instance
8787 *
8888 * @since 1.18
8989 */
——@@ -100,7 +100,6 @@
101101 $this->mExpiry = $expiry;
102102 }
103103
104 -
105104 /**
106105 * ParserOptions is lazy initialised.
107106 */
——@@ -149,9 +148,9 @@
150149 return false; // Wrong hash
151150 }
152151 } else {
153 - $localHash=substr(fread($file,40),8);
154 - fclose($file);
155 - if ($hash!=$localHash) {
 152+ $localHash = substr( fread( $file, 40 ), 8 );
 153+ fclose( $file );
 154+ if ( $hash != $localHash ) {
156155 return false; // Wrong hash
157156 }
158157
——@@ -192,7 +191,7 @@
193192 wfMkdirParents( $wgCacheDirectory ); // might fail
194193
195194 wfSuppressWarnings();
196 - $file = fopen( $tempFilename, 'w');
 195+ $file = fopen( $tempFilename, 'w' );
197196 wfRestoreWarnings();
198197
199198 if ( !$file ) {
——@@ -200,20 +199,20 @@
201200 return;
202201 }
203202
204 - fwrite($file,"<?php\n//$hash\n\n \$this->mCache = array(");
 203+ fwrite( $file, "<?php\n//$hash\n\n \$this->mCache = array(" );
205204
206205 foreach ( $array as $key => $message ) {
207 - $key = $this->escapeForScript($key);
208 - $message = $this->escapeForScript($message);
209 - fwrite($file, "'$key' => '$message',\n");
 206+ $key = $this->escapeForScript( $key );
 207+ $message = $this->escapeForScript( $message );
 208+ fwrite( $file, "'$key' => '$message',\n" );
210209 }
211210
212 - fwrite($file,");\n?>");
213 - fclose($file);
214 - rename($tempFilename, $filename);
 211+ fwrite( $file, ");\n?>" );
 212+ fclose( $file);
 213+ rename( $tempFilename, $filename );
215214 }
216215
217 - function escapeForScript($string) {
 216+ function escapeForScript( $string ) {
218217 $string = str_replace( '\\', '\\\\', $string );
219218 $string = str_replace( '\'', '\\\'', $string );
220219 return $string;
——@@ -260,7 +259,9 @@
261260 }
262261
263262 # Don't do double loading...
264 - if ( isset($this->mLoadedLanguages[$code]) ) return true;
 263+ if ( isset( $this->mLoadedLanguages[$code] ) ) {
 264+ return true;
 265+ }
265266
266267 # 8 lines of code just to say (once) that message cache is disabled
267268 if ( $this->mDisable ) {
——@@ -278,7 +279,6 @@
279280 $where = array(); # Debug info, delayed to avoid spamming debug log too much
280281 $cacheKey = wfMemcKey( 'messages', $code ); # Key in memc for messages
281282
282 -
283283 # (1) local cache
284284 # Hash of the contents is stored in memcache, to detect if local cache goes
285285 # out of date (due to update in other thread?)
——@@ -306,14 +306,13 @@
307307 wfProfileOut( __METHOD__ . '-fromcache' );
308308 }
309309
310 -
311310 # (3)
312311 # Nothing in caches... so we need create one and store it in caches
313312 if ( !$success ) {
314313 $where[] = 'cache is empty';
315314 $where[] = 'loading from database';
316315
317 - $this->lock($cacheKey);
 316+ $this->lock( $cacheKey );
318317
319318 # Limit the concurrency of loadFromDB to a single process
320319 # This prevents the site from going down when the cache expires
——@@ -328,7 +327,7 @@
329328 if ( $success ) {
330329 $this->mMemc->delete( $statusKey );
331330 } else {
332 - $this->mMemc->set( $statusKey, 'error', 60*5 );
 331+ $this->mMemc->set( $statusKey, 'error', 60 * 5 );
333332 wfDebug( "MemCached set error in MessageCache: restart memcached server!\n" );
334333 }
335334 }
——@@ -357,8 +356,8 @@
358357 * $wgMaxMsgCacheEntrySize are assigned a special value, and are loaded
359358 * on-demand from the database later.
360359 *
361 - * @param $code \string Language code.
362 - * @return \array Loaded messages for storing in caches.
 360+ * @param $code String: language code.
 361+ * @return Array: loaded messages for storing in caches.
363362 */
364363 function loadFromDB( $code ) {
365364 wfProfileIn( __METHOD__ );
——@@ -376,7 +375,9 @@
377376 if ( $wgAdaptiveMessageCache ) {
378377 $mostused = $this->getMostUsedMessages();
379378 if ( $code !== $wgLanguageCode ) {
380 - foreach ( $mostused as $key => $value ) $mostused[$key] = "$value/$code";
 379+ foreach ( $mostused as $key => $value ) {
 380+ $mostused[$key] = "$value/$code";
 381+ }
381382 }
382383 }
383384
——@@ -406,9 +407,12 @@
407408 $smallConds[] = 'rev_text_id=old_id';
408409 $smallConds[] = 'page_len <= ' . intval( $wgMaxMsgCacheEntrySize );
409410
410 - $res = $dbr->select( array( 'page', 'revision', 'text' ),
 411+ $res = $dbr->select(
 412+ array( 'page', 'revision', 'text' ),
411413 array( 'page_title', 'old_text', 'old_flags' ),
412 - $smallConds, __METHOD__ . "($code)-small" );
 414+ $smallConds,
 415+ __METHOD__ . "($code)-small"
 416+ );
413417
414418 foreach ( $res as $row ) {
415419 $cache[$row->page_title] = ' ' . Revision::getRevisionText( $row );
——@@ -451,12 +455,10 @@
452456 # Article was deleted
453457 $this->mCache[$code][$title] = '!NONEXISTENT';
454458 $this->mMemc->delete( $titleKey );
455 -
456459 } elseif ( strlen( $text ) > $wgMaxMsgCacheEntrySize ) {
457460 # Check for size
458461 $this->mCache[$code][$title] = '!TOO BIG';
459462 $this->mMemc->set( $titleKey, ' ' . $text, $this->mExpiry );
460 -
461463 } else {
462464 $this->mCache[$code][$title] = ' ' . $text;
463465 $this->mMemc->delete( $titleKey );
——@@ -479,12 +481,12 @@
480482 $sidebarKey = wfMemcKey( 'sidebar', $code );
481483 $parserMemc->delete( $sidebarKey );
482484 }
483 -
 485+
484486 // Update the message in the message blob store
485487 global $wgContLang;
486488 MessageBlobStore::updateMessage( $wgContLang->lcfirst( $msg ) );
487489
488 - wfRunHooks( "MessageCacheReplace", array( $title, $text ) );
 490+ wfRunHooks( 'MessageCacheReplace', array( $title, $text ) );
489491
490492 wfProfileOut( __METHOD__ );
491493 }
——@@ -530,16 +532,16 @@
531533 *
532534 * @return Boolean: success
533535 */
534 - function lock($key) {
 536+ function lock( $key ) {
535537 $lockKey = $key . ':lock';
536 - for ($i=0; $i < MSG_WAIT_TIMEOUT && !$this->mMemc->add( $lockKey, 1, MSG_LOCK_TIMEOUT ); $i++ ) {
537 - sleep(1);
 538+ for ( $i = 0; $i < MSG_WAIT_TIMEOUT && !$this->mMemc->add( $lockKey, 1, MSG_LOCK_TIMEOUT ); $i++ ) {
 539+ sleep( 1 );
538540 }
539541
540542 return $i >= MSG_WAIT_TIMEOUT;
541543 }
542544
543 - function unlock($key) {
 545+ function unlock( $key ) {
544546 $lockKey = $key . ':lock';
545547 $this->mMemc->delete( $lockKey );
546548 }
——@@ -565,7 +567,7 @@
566568 global $wgLanguageCode, $wgContLang;
567569
568570 if ( !is_string( $key ) ) {
569 - throw new MWException( "Non-string key given" );
 571+ throw new MWException( 'Non-string key given' );
570572 }
571573
572574 if ( strval( $key ) === '' ) {
——@@ -594,15 +596,17 @@
595597 $uckey = $wgContLang->ucfirst( $lckey );
596598 }
597599
598 - /* Record each message request, but only once per request.
 600+ /**
 601+ * Record each message request, but only once per request.
599602 * This information is not used unless $wgAdaptiveMessageCache
600 - * is enabled. */
 603+ * is enabled.
 604+ */
601605 $this->mRequestedMessages[$uckey] = true;
602606
603607 # Try the MediaWiki namespace
604608 if( !$this->mDisable && $useDB ) {
605609 $title = $uckey;
606 - if(!$isFullKey && ( $langcode != $wgLanguageCode ) ) {
 610+ if( !$isFullKey && ( $langcode != $wgLanguageCode ) ) {
607611 $title .= '/' . $langcode;
608612 }
609613 $message = $this->getMsgFromNamespace( $title, $langcode );
——@@ -631,9 +635,9 @@
632636 }
633637
634638 # Is this a custom message? Try the default language in the db...
635 - if( ($message === false || $message === '-' ) &&
 639+ if( ( $message === false || $message === '-' ) &&
636640 !$this->mDisable && $useDB &&
637 - !$isFullKey && ($langcode != $wgLanguageCode) ) {
 641+ !$isFullKey && ( $langcode != $wgLanguageCode ) ) {
638642 $message = $this->getMsgFromNamespace( $uckey, $wgLanguageCode );
639643 }
640644
——@@ -678,12 +682,13 @@
679683 } else {
680684 // XXX: This is not cached in process cache, should it?
681685 $message = false;
682 - wfRunHooks('MessagesPreLoad', array( $title, &$message ) );
 686+ wfRunHooks( 'MessagesPreLoad', array( $title, &$message ) );
683687 if ( $message !== false ) {
684688 return $message;
685689 }
686690
687 - /* If message cache is in normal mode, it is guaranteed
 691+ /**
 692+ * If message cache is in normal mode, it is guaranteed
688693 * (except bugs) that there is always entry (or placeholder)
689694 * in the cache if message exists. Thus we can do minor
690695 * performance improvement and return false early.
——@@ -754,9 +759,14 @@
755760 return $message;
756761 }
757762
758 - function disable() { $this->mDisable = true; }
759 - function enable() { $this->mDisable = false; }
 763+ function disable() {
 764+ $this->mDisable = true;
 765+ }
760766
 767+ function enable() {
 768+ $this->mDisable = false;
 769+ }
 770+
761771 /**
762772 * Clear all stored messages. Mainly used after a mass rebuild.
763773 */
——@@ -774,13 +784,15 @@
775785 public function figureMessage( $key ) {
776786 global $wgLanguageCode;
777787 $pieces = explode( '/', $key );
778 - if( count( $pieces ) < 2 )
 788+ if( count( $pieces ) < 2 ) {
779789 return array( $key, $wgLanguageCode );
 790+ }
780791
781792 $lang = array_pop( $pieces );
782793 $validCodes = Language::getLanguageNames();
783 - if( !array_key_exists( $lang, $validCodes ) )
 794+ if( !array_key_exists( $lang, $validCodes ) ) {
784795 return array( $key, $wgLanguageCode );
 796+ }
785797
786798 $message = implode( '/', $pieces );
787799 return array( $message, $lang );
——@@ -796,19 +808,27 @@
797809 $cache = wfGetCache( CACHE_DB );
798810 $data = $cache->get( $cachekey );
799811
800 - if ( !$data ) $data = array();
 812+ if ( !$data ) {
 813+ $data = array();
 814+ }
801815
802816 $age = self::$mAdaptiveDataAge;
803 - $filterDate = substr( wfTimestamp( TS_MW, time()-$age ), 0, 8 );
 817+ $filterDate = substr( wfTimestamp( TS_MW, time() - $age ), 0, 8 );
804818 foreach ( array_keys( $data ) as $key ) {
805 - if ( $key < $filterDate ) unset( $data[$key] );
 819+ if ( $key < $filterDate ) {
 820+ unset( $data[$key] );
 821+ }
806822 }
807823
808824 $index = substr( wfTimestampNow(), 0, 8 );
809 - if ( !isset( $data[$index] ) ) $data[$index] = array();
 825+ if ( !isset( $data[$index] ) ) {
 826+ $data[$index] = array();
 827+ }
810828
811829 foreach ( self::$instance->mRequestedMessages as $message => $_ ) {
812 - if ( !isset( $data[$index][$message] ) ) $data[$index][$message] = 0;
 830+ if ( !isset( $data[$index][$message] ) ) {
 831+ $data[$index][$message] = 0;
 832+ }
813833 $data[$index][$message]++;
814834 }
815835
——@@ -816,17 +836,21 @@
817837 }
818838
819839 public function getMostUsedMessages() {
820 - $cachekey = wfMemckey( 'message-profiling' );
 840+ $cachekey = wfMemcKey( 'message-profiling' );
821841 $cache = wfGetCache( CACHE_DB );
822842 $data = $cache->get( $cachekey );
823 - if ( !$data ) return array();
 843+ if ( !$data ) {
 844+ return array();
 845+ }
824846
825847 $list = array();
826848
827849 foreach( $data as $messages ) {
828850 foreach( $messages as $message => $count ) {
829851 $key = $message;
830 - if ( !isset( $list[$key] ) ) $list[$key] = 0;
 852+ if ( !isset( $list[$key] ) ) {
 853+ $list[$key] = 0;
 854+ }
831855 $list[$key] += $count;
832856 }
833857 }

Comments

#Comment by Hashar (talk | contribs)   07:28, 22 February 2011

I tend to prefer the former form in the following code: 

-	function disable() { $this->mDisable = true; }
-	function enable() { $this->mDisable = false; }
+	function disable() {
+		$this->mDisable = true;
+	}
 
+	function enable() {
+		$this->mDisable = false;
+	}

Looks easier to read when properly indented, that is cosmetic though.

Marked 'ok' 

+

Status & tagging log