r52727 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r52726‎ | r52727 | r52728 >
Date:06:19, 3 July 2009
Author:tstarling
Status:resolved (Comments)
Tags:
Comment:
* Re-added $wgMessageCache->addMessages(), there are still some extensions (in and out of subversion) that rely on it. Call wfDeprecated().
* Made wfDeprecated() issue a notice only once for each function name instead of every time the function is called.
* Added exceptions for malformed keys input to $wgMessageCache->get(), per CR comments on r52503
* Fixed notice from MWException::useMessageCache()
Modified paths:
  • /trunk/phase3/RELEASE-NOTES (modified) (history)
  • /trunk/phase3/includes/Exception.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/LocalisationCache.php (modified) (history)
  • /trunk/phase3/includes/MessageCache.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/GlobalFunctions.php
@@ -2975,7 +2975,11 @@
29762976 * @return null
29772977 */
29782978 function wfDeprecated( $function ) {
2979 - wfWarn( "Use of $function is deprecated.", 2 );
 2979+ static $functionsWarned = array();
 2980+ if ( !isset( $functionsWarned[$function] ) ) {
 2981+ $functionsWarned[$function] = true;
 2982+ wfWarn( "Use of $function is deprecated.", 2 );
 2983+ }
29802984 }
29812985
29822986 function wfWarn( $msg, $callerOffset = 1, $level = E_USER_NOTICE ) {
Index: trunk/phase3/includes/LocalisationCache.php
@@ -78,6 +78,12 @@
7979 var $recachedLangs = array();
8080
8181 /**
 82+ * Data added by extensions using the deprecated $wgMessageCache->addMessages()
 83+ * interface.
 84+ */
 85+ var $legacyData = array();
 86+
 87+ /**
8288 * All item keys
8389 */
8490 static public $allKeys = array(
@@ -137,10 +143,6 @@
138144 global $wgCacheDirectory;
139145
140146 $this->conf = $conf;
141 - $this->data = array();
142 - $this->loadedItems = array();
143 - $this->loadedSubitems = array();
144 - $this->initialisedLangs = array();
145147 if ( !empty( $conf['storeClass'] ) ) {
146148 $storeClass = $conf['storeClass'];
147149 } else {
@@ -208,6 +210,9 @@
209211 * Get a subitem, for instance a single message for a given language.
210212 */
211213 public function getSubitem( $code, $key, $subkey ) {
 214+ if ( isset( $this->legacyData[$code][$key][$subkey] ) ) {
 215+ return $this->legacyData[$code][$key][$subkey];
 216+ }
212217 if ( !isset( $this->loadedSubitems[$code][$key][$subkey] ) ) {
213218 if ( isset( $this->loadedItems[$code][$key] ) ) {
214219 if ( isset( $this->data[$code][$key][$subkey] ) ) {
@@ -619,12 +624,30 @@
620625 unset( $this->loadedItems[$code] );
621626 unset( $this->loadedSubitems[$code] );
622627 unset( $this->initialisedLangs[$code] );
 628+ // We don't unload legacyData because there's no way to get it back
 629+ // again, it's not really a cache
623630 foreach ( $this->shallowFallbacks as $shallowCode => $fbCode ) {
624631 if ( $fbCode === $code ) {
625632 $this->unload( $shallowCode );
626633 }
627634 }
628635 }
 636+
 637+ /**
 638+ * Add messages to the cache, from an extension that has not yet been
 639+ * migrated to $wgExtensionMessages or the LocalisationCacheRecache hook.
 640+ * Called by deprecated function $wgMessageCache->addMessages().
 641+ */
 642+ public function addLegacyMessages( $messages ) {
 643+ foreach ( $messages as $lang => $langMessages ) {
 644+ if ( isset( $this->legacyData[$lang]['messages'] ) ) {
 645+ $this->legacyData[$lang]['messages'] =
 646+ $langMessages + $this->legacyData[$lang]['messages'];
 647+ } else {
 648+ $this->legacyData[$lang]['messages'] = $langMessages;
 649+ }
 650+ }
 651+ }
629652 }
630653
631654 /**
Index: trunk/phase3/includes/MessageCache.php
@@ -501,6 +501,12 @@
502502 function get( $key, $useDB = true, $langcode = true, $isFullKey = false ) {
503503 global $wgContLanguageCode, $wgContLang;
504504
 505+ if ( !is_string( $key ) ) {
 506+ throw new MWException( __METHOD__.': Invalid message key of type ' . gettype( $key ) );
 507+ } elseif ( $key === '' ) {
 508+ throw new MWException( __METHOD__.': Invaild message key: empty string' );
 509+ }
 510+
505511 $lang = wfGetLangObj( $langcode );
506512 $langcode = $lang->getCode();
507513
@@ -701,7 +707,53 @@
702708 }
703709 }
704710
 711+ /**
 712+ * Add a message to the cache
 713+ * @deprecated Use $wgExtensionMessagesFiles
 714+ *
 715+ * @param mixed $key
 716+ * @param mixed $value
 717+ * @param string $lang The messages language, English by default
 718+ */
 719+ function addMessage( $key, $value, $lang = 'en' ) {
 720+ wfDeprecated( __METHOD__ );
 721+ $lc = Language::getLocalisationCache();
 722+ $lc->addLegacyMessages( array( $lang => array( $key => $value ) ) );
 723+ }
 724+
705725 /**
 726+ * Add an associative array of message to the cache
 727+ * @deprecated Use $wgExtensionMessagesFiles
 728+ *
 729+ * @param array $messages An associative array of key => values to be added
 730+ * @param string $lang The messages language, English by default
 731+ */
 732+ function addMessages( $messages, $lang = 'en' ) {
 733+ wfDeprecated( __METHOD__ );
 734+ $lc = Language::getLocalisationCache();
 735+ $lc->addLegacyMessages( array( $lang => $messages ) );
 736+ }
 737+
 738+ /**
 739+ * Add a 2-D array of messages by lang. Useful for extensions.
 740+ * @deprecated Use $wgExtensionMessagesFiles
 741+ *
 742+ * @param array $messages The array to be added
 743+ */
 744+ function addMessagesByLang( $messages ) {
 745+ wfDeprecated( __METHOD__ );
 746+ $lc = Language::getLocalisationCache();
 747+ $lc->addLegacyMessages( $messages );
 748+ }
 749+
 750+ /**
 751+ * Set a hook for addMessagesByLang()
 752+ */
 753+ function setExtensionMessagesHook( $callback ) {
 754+ $this->mAddMessagesHook = $callback;
 755+ }
 756+
 757+ /**
706758 * @deprecated
707759 */
708760 function loadAllMessages( $lang = false ) {
Index: trunk/phase3/includes/Exception.php
@@ -26,7 +26,7 @@
2727 function useMessageCache() {
2828 global $wgLang;
2929 foreach ( $this->getTrace() as $frame ) {
30 - if ( $frame['class'] == 'LocalisationCache' ) {
 30+ if ( isset( $frame['class'] ) && $frame['class'] === 'LocalisationCache' ) {
3131 return false;
3232 }
3333 }
Index: trunk/phase3/RELEASE-NOTES
@@ -50,6 +50,8 @@
5151 to true
5252 * Removed $wgEnableSerializedMessages and $wgCheckSerialized. Similar
5353 functionality is now available via $wgLocalisationCacheConf.
 54+* $wgMessageCache->addMessages() is deprecated. Messages added via this
 55+ interface will not appear in Special:AllMessages.
5456
5557 === New features in 1.16 ===
5658

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r52503* Introduced a new system for localisation caching. The system is based aroun...tstarling07:11, 28 June 2009

Comments

#Comment by Simetrical (talk | contribs)   20:53, 7 July 2009

The "empty string" exception was getting hit on my development wiki. If the first line of MediaWiki:Sidebar starts with "**" (so there's no top-level heading), the code in Skin::buildSidebar() will use an empty string for its array key, and Monobook will try to load a message whose name is the empty string. Previously I guess this failed silently; now a screwup by a sysop could cause an exception to be thrown on every page of the wiki, messing up the interface.

This is why exceptions aren't such a great idea for production code, maybe, unless the error is totally unrecoverable? At the very least, this specific problem should be fixed before this revision goes live. Alternatively, don't throw an exception here (someone should still fix the bug uncovered in buildSidebar(), but it's not urgent).

#Comment by Tim Starling (talk | contribs)   11:02, 8 July 2009

If the developers who comment on CR would isolate bugs before they report them, as you have done, instead of implying that every notice that's issued from my code is my sole responsibility, then the exceptions wouldn't have been necessary. They're a debugging hack, they obviously aren't appropriate for production code.

#Comment by Tim Starling (talk | contribs)   11:56, 10 July 2009

Graceful fail in r53043, compatible with existing code in Skin::buildSidebar().

Status & tagging log