From: Timo Tijhof Date: Thu, 27 Sep 2018 21:23:48 +0000 (+0100) Subject: skins: Remove 'usemsgcache' and deprecate getDynamicStylesheetQuery X-Git-Tag: 1.34.0-rc.0~3960^2 X-Git-Url: https://git.heureux-cyclage.org/?p=lhc%2Fweb%2Fwiklou.git;a=commitdiff_plain;h=cba0fb1c1576324e87b27b6d31ee4359195025c8;ds=sidebyside skins: Remove 'usemsgcache' and deprecate getDynamicStylesheetQuery For several years now, action=raw urls are automatically purged after edits, and receive the same cache allowance as page views. The vast majority of action=raw requests come from the importScript() function in wikibits.js, which only specififes 'title', 'action' and 'ctype'. The 'usemsgcache' parameter is a relic (pre-2009) for optimising something we haven't used in a very long time. Nothing uses the 'usemsgcache' parameter, except for the one edge-case of third-party wikis loading Filepage.css from Commons on local file description pages. This commit removes the handling logic, in favour of letting the normal Revision-logic happen instead, which is compatible and produces the same response. Before 2009, this probably made sense as an optimisation because we didn't have Revision text cache in Memcached yet at that time, to avoid DB-load from InstantCommons-based file page views. The way FileRepo was using it needlessly allowed Filepage.css to return stale responses (given non-canonical query parameters variations are not purged on-edit). It also caused responses to be cached longer or shorter than they should be, due to it being based on the client wiki's configuration, instead of the repo's configuration (e.g. Wikimedia Commons). By omitting these 'maxage' parameters, the $wgSquidMaxage config of the repo is used instead, which makes more sense. Bug: T193271 Change-Id: Ie52195b56b7f8fc17dec0cbedd6d77968e3bc040 --- diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index 60f5e6abf1..4ece1153fe 100644 --- a/RELEASE-NOTES-1.32 +++ b/RELEASE-NOTES-1.32 @@ -466,6 +466,8 @@ because of Phabricator reports. * QuickTemplate::msgHtml() and BaseTemplate::msgHtml() have been deprecated as they promote bad practises. I18n messages should always be properly escaped. +* Skin::getDynamicStylesheetQuery() has been deprecated. It always + returns action=raw&ctype=text/css which callers should use directly. === Other changes in 1.32 === * (T198811) The following tables have had their UNIQUE indexes turned into diff --git a/includes/actions/RawAction.php b/includes/actions/RawAction.php index 463019f517..b5a6d3ac25 100644 --- a/includes/actions/RawAction.php +++ b/includes/actions/RawAction.php @@ -163,64 +163,35 @@ class RawAction extends FormlessAction { $title = $this->getTitle(); $request = $this->getRequest(); - // If it's a page in the MediaWiki namespace, we can just hit the message cache - if ( $request->getBool( 'usemsgcache' ) && $title->getNamespace() == NS_MEDIAWIKI ) { - // FIXME: The overhead and complexity of using MessageCache for serving - // source code is not worth the marginal gain in performance. This should - // instead use Revision::getRevisionText, which already has its own caching - // layer, which is good enough fine given action=raw only responds with - // a single page (no need for batch). - // - // Use of MessageCache: - // - is unsustainable (T193271), - // - can cause bugs due to "post-processing" (see MessageCache::get) not - // intending to apply to program source code, - // - causes uncertaintly around whether or not localisation default - // placeholders are, can, and should be used, or not. - $text = MessageCache::singleton()->get( - $title->getDBkey(), - // Yes, use the database. - true, - // Yes, use the content language. - true, - // Yes, the message key already contains the language in it ("/de", etc.) - true - ); - // If the local page doesn't exist, return a blank (not the default) - if ( $text === false ) { - $text = ''; - } - } else { - // Get it from the DB - $rev = Revision::newFromTitle( $title, $this->getOldId() ); - if ( $rev ) { - $lastmod = wfTimestamp( TS_RFC2822, $rev->getTimestamp() ); - $request->response()->header( "Last-modified: $lastmod" ); + // Get it from the DB + $rev = Revision::newFromTitle( $title, $this->getOldId() ); + if ( $rev ) { + $lastmod = wfTimestamp( TS_RFC2822, $rev->getTimestamp() ); + $request->response()->header( "Last-modified: $lastmod" ); + + // Public-only due to cache headers + $content = $rev->getContent(); - // Public-only due to cache headers - $content = $rev->getContent(); + if ( $content === null ) { + // revision not found (or suppressed) + $text = false; + } elseif ( !$content instanceof TextContent ) { + // non-text content + wfHttpError( 415, "Unsupported Media Type", "The requested page uses the content model `" + . $content->getModel() . "` which is not supported via this interface." ); + die(); + } else { + // want a section? + $section = $request->getIntOrNull( 'section' ); + if ( $section !== null ) { + $content = $content->getSection( $section ); + } - if ( $content === null ) { - // revision not found (or suppressed) + if ( $content === null || $content === false ) { + // section not found (or section not supported, e.g. for JS, JSON, and CSS) $text = false; - } elseif ( !$content instanceof TextContent ) { - // non-text content - wfHttpError( 415, "Unsupported Media Type", "The requested page uses the content model `" - . $content->getModel() . "` which is not supported via this interface." ); - die(); } else { - // want a section? - $section = $request->getIntOrNull( 'section' ); - if ( $section !== null ) { - $content = $content->getSection( $section ); - } - - if ( $content === null || $content === false ) { - // section not found (or section not supported, e.g. for JS, JSON, and CSS) - $text = false; - } else { - $text = $content->getNativeData(); - } + $text = $content->getNativeData(); } } } diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index 858e124af4..455d38f243 100644 --- a/includes/filerepo/FileRepo.php +++ b/includes/filerepo/FileRepo.php @@ -810,8 +810,9 @@ class FileRepo { */ public function getDescriptionStylesheetUrl() { if ( isset( $this->scriptDirUrl ) ) { - return $this->makeUrl( 'title=MediaWiki:Filepage.css&' . - wfArrayToCgi( Skin::getDynamicStylesheetQuery() ) ); + // Must match canonical query parameter order for optimum caching + // See Title::getCdnUrls + return $this->makeUrl( 'title=MediaWiki:Filepage.css&action=raw&ctype=text/css' ); } return false; diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php index e426f7f8c5..ed4045d487 100644 --- a/includes/skins/Skin.php +++ b/includes/skins/Skin.php @@ -414,17 +414,13 @@ abstract class Skin extends ContextSource { /** * Get the query to generate a dynamic stylesheet * + * @deprecated since 1.32 Use action=raw&ctype=text/css directly. * @return array */ public static function getDynamicStylesheetQuery() { - global $wgSquidMaxage; - return [ 'action' => 'raw', - 'maxage' => $wgSquidMaxage, - 'usemsgcache' => 'yes', 'ctype' => 'text/css', - 'smaxage' => $wgSquidMaxage, ]; } diff --git a/maintenance/dictionary/mediawiki.dic b/maintenance/dictionary/mediawiki.dic index ff06e49d4d..a0177b1737 100644 --- a/maintenance/dictionary/mediawiki.dic +++ b/maintenance/dictionary/mediawiki.dic @@ -4322,7 +4322,6 @@ useemail uselang uselivepreview usemod -usemsgcache usenewrc user useragent