r89001 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r89000‎ | r89001 | r89002 >
Date:22:09, 27 May 2011
Author:catrope
Status:ok (Comments)
Tags:
Comment:
Fix issue on the live site causing RL requests for ext.uploadWizard to consistently take more than 5 seconds: the cache freshness check was failing all the time because ext.uploadWizard listed some messages twice, and duplicates were filtered on the left-hand side of the !== comparison (implicitly, because they're array keys), but not on the right-hand side.
Modified paths:
  • /trunk/phase3/includes/MessageBlobStore.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/MessageBlobStore.php
——@@ -340,7 +340,7 @@
341341 }
342342 // Update the module's blobs if the set of messages changed or if the blob is
343343 // older than $wgCacheEpoch
344 - if ( array_keys( FormatJson::decode( $row->mr_blob, true ) ) !== $module->getMessages() ||
 344+ if ( array_keys( FormatJson::decode( $row->mr_blob, true ) ) !== array_values( array_unique( $module->getMessages() ) ) ||
345345 wfTimestamp( TS_MW, $row->mr_timestamp ) <= $wgCacheEpoch ) {
346346 $retval[$row->mr_resource] = self::updateModule( $row->mr_resource, $module, $lang );
347347 } else {

Follow-up revisions

RevisionCommit summaryAuthorDate
r89005Refactor MessageBlobStore::updateModule() to remove the multi-language update...catrope22:42, 27 May 2011
r890081.17wmf1: MFT r89001 (already deployed)catrope22:55, 27 May 2011

Comments

#Comment by Platonides (talk | contribs)   22:14, 27 May 2011

ResourceLoaderModule;;getMessages() is defined as 'List of message keys. Keys may occur more than once'. Are repeated keys supported or not?

#Comment by Catrope (talk | contribs)   22:15, 27 May 2011

They are now. I guess this takes away any reason I had to blame Neil for this breakage :P

#Comment by Platonides (talk | contribs)   22:22, 27 May 2011

But what is that useful for?

#Comment by Catrope (talk | contribs)   22:23, 27 May 2011

Don't know, don't care. But supporting duplicate message keys with a one-line change is easier than going in and enforcing there be no duplicates in UploadWizard's list of 236 messages.

#Comment by Platonides (talk | contribs)   22:31, 27 May 2011

Then I would have fixed it throwing an exception if there were duplicate messages.

About ensuring that there are no duplicates, that seems a problem for translatewiki?

#Comment by Catrope (talk | contribs)   15:28, 1 June 2011

No. The messages aren't duplicated in the i18n files, they're duplicated in the return value of getMessages(), which is module-specific metadata that's not managed by TWN.

Again, throwing an exception would've been more code than just filtering duplicates :)

Status & tagging log