r72177 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r72176‎ | r72177 | r72178 >
Date:23:18, 1 September 2010
Author:kaldari
Status:resolved (Comments)
Tags:
Comment:
initial version of BannerLoader
Modified paths:
  • /trunk/extensions/CentralNotice/BannerLoader.php (added) (history)
  • /trunk/extensions/CentralNotice/CentralNotice.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CentralNotice/CentralNotice.php
@@ -138,6 +138,9 @@
139139 $wgSpecialPages['CentralNotice'] = 'CentralNotice';
140140 $wgSpecialPageGroups['CentralNotice'] = 'wiki'; // Wiki data and tools"
141141 $wgAutoloadClasses['CentralNotice'] = $dir . 'SpecialCentralNotice.php';
 142+
 143+ $wgSpecialPages['BannerLoader'] = 'BannerLoader';
 144+ $wgAutoloadClasses['BannerLoader'] = $dir . 'BannerLoader.php';
142145
143146 $wgSpecialPages['NoticeText'] = 'SpecialNoticeText';
144147 $wgAutoloadClasses['SpecialNoticeText'] = $dir . 'SpecialNoticeText.php';
Index: trunk/extensions/CentralNotice/BannerLoader.php
@@ -0,0 +1,163 @@
 2+<?php
 3+
 4+/**
 5+ * Generates content for static HTML files
 6+ */
 7+class BannerLoader extends UnlistedSpecialPage {
 8+ var $project = 'wikipedia';
 9+ var $language = 'en';
 10+ protected $sharedMaxAge = 22; // Cache for ? hours on the server side
 11+ protected $maxAge = 0; // No client-side banner caching so we get all impressions
 12+ protected $contentType = 'text/html';
 13+
 14+ function __construct() {
 15+ // Register special page
 16+ parent::__construct( "BannerLoader" );
 17+ }
 18+
 19+ function execute( $par ) {
 20+ global $wgOut, $wgRequest;
 21+
 22+ $wgOut->disable();
 23+ $this->sendHeaders();
 24+
 25+ // If a language is specified in the query string, use it
 26+ if ( $wgRequest->getText( 'language' ) ) {
 27+ $this->language = htmlspecialchars( $wgRequest->getText( 'language' ) );
 28+ }
 29+
 30+ if ( $wgRequest->getText( 'banner') ) {
 31+ $bannerName = htmlspecialchars( $wgRequest->getText( 'banner' ) );
 32+ $content = $this->getHtmlNotice( $bannerName );
 33+ if ( strlen( $content ) == 0 ) {
 34+ // Hack for IE/Mac 0-length keepalive problem, see RawPage.php
 35+ echo "/* Empty */";
 36+ } else {
 37+ echo $content;
 38+ }
 39+ } else {
 40+ echo "/* No banner specified */";
 41+ }
 42+ }
 43+
 44+ /**
 45+ * Generate the HTTP response headers for the banner file
 46+ */
 47+ function sendHeaders() {
 48+ header( "Content-type: $this->contentType; charset=utf-8" );
 49+ header( "Cache-Control: public, s-maxage=$this->sharedMaxAge, max-age=$this->maxAge" );
 50+ }
 51+
 52+ /**
 53+ * Generate the HTML for the requested banner
 54+ */
 55+ function getHtmlNotice( $bannerName ) {
 56+ // Make sure the banner exists
 57+ if ( SpecialNoticeTemplate::templateExists( $bannerName ) ) {
 58+ $this->bannerName = $bannerName;
 59+ return preg_replace_callback(
 60+ '/{{{(.*?)}}}/',
 61+ array( $this, 'getNoticeField' ),
 62+ $this->getNoticeTemplate()
 63+ );
 64+ }
 65+ }
 66+
 67+ function getNoticeTemplate() {
 68+ return $this->getMessage( "centralnotice-template-{$this->bannerName}" );
 69+ }
 70+
 71+ function getNoticeField( $matches ) {
 72+ $field = $matches[1];
 73+ $params = array();
 74+ if ( $field == 'amount' ) {
 75+ $params = array( $this->formatNum( $this->getDonationAmount() ) );
 76+ }
 77+ $message = "centralnotice-{$this->bannerName}-$field";
 78+ $source = $this->getMessage( $message, $params );
 79+ return $source;
 80+ }
 81+
 82+ private function formatNum( $num ) {
 83+ $num = sprintf( "%.1f", $num / 1e6 );
 84+ if ( substr( $num, - 2 ) == '.0' ) {
 85+ $num = substr( $num, 0, - 2 );
 86+ }
 87+ $lang = Language::factory( $this->language );
 88+ return $lang->formatNum( $num );
 89+ }
 90+
 91+ private function getMessage( $msg, $params = array() ) {
 92+ // A god-damned dirty hack! :D
 93+ $old = array();
 94+ $old['wgSitename'] = $GLOBALS['wgSitename'];
 95+ $old['wgLang'] = $GLOBALS['wgLang'];
 96+
 97+ $GLOBALS['wgSitename'] = $this->projectName();
 98+ $GLOBALS['wgLang'] = Language::factory( $this->language ); // hack for {{int:...}}
 99+
 100+ $options = array(
 101+ 'language' => $this->language,
 102+ 'parsemag',
 103+ );
 104+ array_unshift( $params, $options );
 105+ array_unshift( $params, $msg );
 106+ $out = call_user_func_array( 'wfMsgExt', $params );
 107+
 108+ // Restore globals
 109+ $GLOBALS['wgSitename'] = $old['wgSitename'];
 110+ $GLOBALS['wgLang'] = $old['wgLang'];
 111+
 112+ return $out;
 113+ }
 114+
 115+ private function projectName() {
 116+ global $wgConf;
 117+
 118+ $wgConf->loadFullData();
 119+
 120+ // Special cases for commons and meta who have no lang
 121+ if ( $this->project == 'commons' )
 122+ return "Commons";
 123+ else if ( $this->project == 'meta' )
 124+ return "Wikimedia";
 125+
 126+ // Guess dbname since we don't have it atm
 127+ $dbname = $this->language .
 128+ ( ( $this->project == 'wikipedia' ) ? "wiki" : $this->project );
 129+ $name = $wgConf->get( 'wgSitename', $dbname, $this->project,
 130+ array( 'lang' => $this->language, 'site' => $this->project ) );
 131+
 132+ if ( $name ) {
 133+ return $name;
 134+ } else {
 135+ global $wgLang;
 136+ return $wgLang->ucfirst( $this->project );
 137+ }
 138+ }
 139+
 140+ /**
 141+ * Pull the current amount raised during a fundraiser
 142+ */
 143+ private function getDonationAmount() {
 144+ global $wgNoticeCounterSource, $wgMemc;
 145+ // Pull short-cached amount
 146+ $count = intval( $wgMemc->get( 'centralnotice:counter' ) );
 147+ if ( !$count ) {
 148+ // Pull from dynamic counter
 149+ $count = intval( @file_get_contents( $wgNoticeCounterSource ) );
 150+ if ( !$count ) {
 151+ // Pull long-cached amount
 152+ $count = intval( $wgMemc->get( 'centralnotice:counter:fallback' ) );
 153+ if ( !$count ) {
 154+ // Return hard-coded amount if all else fails
 155+ return 100; // Update as needed during fundraiser
 156+ }
 157+ }
 158+ $wgMemc->set( 'centralnotice:counter', $count, 60 ); // Expire in 60 seconds
 159+ $wgMemc->set( 'centralnotice:counter:fallback', $count ); // No expiration
 160+ }
 161+ return $count;
 162+ }
 163+
 164+}

Follow-up revisions

RevisionCommit summaryAuthorDate
r72204replacing var and @, removing htmlspecialchars() per comments at r72177kaldari17:30, 2 September 2010
r72231removing htmlspecialchars() per comment at r72177kaldari21:25, 2 September 2010
r72610switching from BannerLoader to SpecialBannerLoader per comment at r72177kaldari21:23, 8 September 2010
r72611fixing html comments and wfMemc per comments at r72177kaldari21:41, 8 September 2010
r72666correct fix for r72177kaldari17:57, 9 September 2010

Comments

#Comment by Nikerabbit (talk | contribs)   06:57, 2 September 2010
+$this->language = htmlspecialchars( $wgRequest->getText( 'language' ) );

I see no need to escape that there. Besides, escaping should happen close to the output.

In projectName() your are using $wgLang before it is replaced with your hack.

+$count = intval( @file_get_contents( $wgNoticeCounterSource ) );
+var $language = 'en';

Use of @ and var is discouraged.

#Comment by Ryan Kaldari (WMF) (talk | contribs)   17:31, 2 September 2010

Fixed in r72204. I still need to look into the projectName() issue.

#Comment by Kaldari (talk | contribs)   19:07, 3 September 2010

The projectName code has been replaced in r72304.

#Comment by Catrope (talk | contribs)   17:19, 8 September 2010

BannerLoader should be called SpecialBannerLoader: classes implementing special pages all have names starting with Special.

+				echo "/* Empty */";
[...]
+			echo "/* No banner specified */";

Given that you're outputting HTML, I think it'd make more sense to use <!-- Empty --> and <!-- No banner specified --> instead.

+		if ( substr( $num, - 2 ) == '.0' ) {
+		$num = substr( $num, 0, - 2 );
+		}

This code is misindented.

+		$old['wgSitename'] = $GLOBALS['wgSitename'];
+		$old['wgLang'] = $GLOBALS['wgLang'];

You don't need to use $GLOBALS here, just using global $wgLang; then accessing $wgLang directly works just fine.

+		$count = intval( $wgMemc->get( 'centralnotice:counter' ) );

Use wfMemcKey( 'centralnotice', 'counter' ) instead of rolling your own. One helpful thing wfMemcKey() does is prefix the wiki name so wikis don't overwrite each other's cache entries.

#Comment by Catrope (talk | contribs)   17:48, 8 September 2010

Misindent and $GLOBALS addressed in r72304.

#Comment by Kaldari (talk | contribs)   21:26, 8 September 2010

class name fixed in r72610.

#Comment by Kaldari (talk | contribs)   21:41, 8 September 2010

remaining issues fixed in r72611.

#Comment by Ryan Kaldari (WMF) (talk | contribs)   18:03, 9 September 2010

correct fix for memcache code in r72666.

Status & tagging log