r92182 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r92181‎ | r92182 | r92183 >
Date:19:22, 14 July 2011
Author:aaron
Status:deferred (Comments)
Tags:
Comment:
* Code style and comments tweaks
* Moved instance functions in MWMultiVersion up to top of file
* Protected __clone function (class is a singleton)
Modified paths:
  • /trunk/tools/mwmultiversion/live-1.5/MWVersion.php (modified) (history)
  • /trunk/tools/mwmultiversion/wmf-config/MWMultiVersion.php (modified) (history)

Diff [purge]

Index: trunk/tools/mwmultiversion/live-1.5/MWVersion.php
——@@ -1,27 +1,36 @@
22 <?php
3 -
 3+/*
 4+ * Get the location of the correct version of a MediaWiki web
 5+ * entry-point file given environmental variables such as the server name.
 6+ * This also sets the MW_INSTALL_PATH environmental variable and changes
 7+ * PHP's current directory to the directory of this file.
 8+ *
 9+ * @return string File path
 10+ */
411 function getMediaWiki( $file ) {
512 $secure = getenv( 'MW_SECURE_HOST' );
6 - $host = $secure ? $secure : $_SERVER['HTTP_HOST'];
7 -
 13+ $host = $secure ? $secure : $_SERVER['HTTP_HOST'];
 14+
815 require( dirname( __FILE__ ) . '/../wmf-config/MWMultiVersion.php' );
9 - $multiVersion = MWMultiVersion::getInstanceForWiki( $_SERVER['SERVER_NAME'], $_SERVER['DOCUMENT_ROOT'] );
 16+ $multiVersion = MWMultiVersion::getInstanceForWiki(
 17+ $_SERVER['SERVER_NAME'], $_SERVER['DOCUMENT_ROOT'] );
1018
1119 $version = $multiVersion->getVersion();
12 -
 20+
1321 if ( $host == 'test.wikipedia.org' && !$secure &&
14 - !preg_match( '!thumb\.php!', $_SERVER['REQUEST_URI'] ) ) {
 22+ !preg_match( '!thumb\.php!', $_SERVER['REQUEST_URI'] ) )
 23+ {
1524 define( 'TESTWIKI', 1 );
16 - // As horrible hack for NFS-less iamge scalers, use regular docroot for thumbs?
 25+ # Test wiki mostly runs off the version of MediaWiki on /home.
 26+ # As horrible hack for NFS-less image scalers, use regular docroot for thumbs?
1727 # $IP = '/home/wikipedia/common/php-1.5';
1828 $IP = "/home/wikipedia/common/$version";
1929 } else {
2030 $IP = "/usr/local/apache/common/$version";
2131 }
22 -
 32+
2333 chdir( $IP );
2434 putenv( "MW_INSTALL_PATH=$IP" );
 35+
2536 return "$IP/$file";
2637 }
27 -
28 -
Index: trunk/tools/mwmultiversion/wmf-config/MWMultiVersion.php
——@@ -1,7 +1,11 @@
22 <?php
33
44 class MWMultiVersion {
 5+ /**
 6+ * @var MWMultiVersion
 7+ */
58 private static $mwversion;
 9+
610 private $site;
711 private $lang;
812
——@@ -10,25 +14,66 @@
1115 * @see getInstanceForWiki
1216 * @see getInstanceForUploadWiki
1317 */
14 - private function __construct() {
 18+ private function __construct() {}
 19+ private function __clone() {}
 20+
 21+ /**
 22+ * Factory method to get an instance of MWMultiVersion.
 23+ * Use this for all wikis except calls to /w/thumb.php on upload.wikmedia.org.
 24+ * @param $serverName the ServerName for this wiki -- $_SERVER['SERVER_NAME']
 25+ * @param $docroot the DocumentRoot for this wiki -- $_SERVER['DOCUMENT_ROOT']
 26+ * @return An MWMultiVersion object for this wiki
 27+ */
 28+ public static function getInstanceForWiki( $serverName, $docRoot ) {
 29+ if ( !isset( self::$mwversion ) ) {
 30+ self::$mwversion = new self;
 31+ self::$mwversion->setSiteInfoForWiki( $serverName, $docRoot );
 32+ }
 33+ return self::$mwversion;
1534 }
1635
1736 /**
 37+ * Factory method to get an instance of MWMultiVersion used for calls to /w/thumb.php on upload.wikmedia.org.
 38+ * @param $pathInfo the PathInfo -- $_SERVER['PATH_INFO']
 39+ * @return An MWMultiVersion object for the wiki derived from the pathinfo
 40+ */
 41+ public static function getInstanceForUploadWiki( $pathInfo ) {
 42+ if ( !isset( self::$mwversion ) ) {
 43+ self::$mwversion = new self;
 44+ self::$mwversion->setSiteInfoForUploadWiki( $PathInfo );
 45+ }
 46+ return self::$mwversion;
 47+ }
 48+
 49+ /**
 50+ * Factory method to get an instance of MWMultiVersion
 51+ * via maintenance scripts since they need to set site and lang.
 52+ * @return An MWMultiVersion object for the wiki derived from the env variables set by Maintenance.php
 53+ */
 54+ public static function getInstanceForMaintenance() {
 55+ if ( !isset( self::$mwversion ) ) {
 56+ self::$mwversion = new self;
 57+ self::$mwversion->setSiteInfoForMaintenance();
 58+ }
 59+ return self::$mwversion;
 60+ }
 61+
 62+ /**
1863 * Derives site and lang from the parameters and sets $site and $lang on the instance
1964 * @param $serverName the ServerName for this wiki -- $_SERVER['SERVER_NAME']
2065 * @param $docroot the DocumentRoot for this wiki -- $_SERVER['DOCUMENT_ROOT']
2166 */
22 - private function setSiteInfoForWiki( $serverName, $docRoot) {
 67+ private function setSiteInfoForWiki( $serverName, $docRoot ) {
2368 $secure = getenv( 'MW_SECURE_HOST' );
2469 $matches = array();
25 - if( $secure ) {
 70+ if ( $secure ) {
2671 if ( !preg_match('/^([^.]+).([^.]+).*$/', $secure, $matches ) ) {
2772 die("invalid hostname");
2873 }
29 -
3074 $this->lang = $matches[1];
3175 $this->site = $matches[2];
32 -
 76+
 77+ // @TODO: move/use some dblist?
3378 if (in_array($this->lang, array("commons", "grants", "sources", "wikimania", "wikimania2006", "foundation", "meta")))
3479 $this->site = "wikipedia";
3580 } else {
——@@ -51,15 +96,14 @@
5297 die( "Invalid host name (docroot=" . $_SERVER['DOCUMENT_ROOT'] . "), can't determine language" );
5398 }
5499 }
55 -
56100 }
57 -
 101+
58102 /**
59103 * Derives site and lang from the parameter and sets $site and $lang on the instance
60104 * @param $pathInfo the PathInfo -- $_SERVER['PATH_INFO']
61105 */
62 - private function setSiteInfoForUploadWiki( $pathInfo) {
63 - //TODO: error if we don't get what we expect
 106+ private function setSiteInfoForUploadWiki( $pathInfo ) {
 107+ // @TODO: error if we don't get what we expect
64108 $pathBits = explode( '/', $pathInfo );
65109 $this->site = $pathBits[1];
66110 $this->lang = $pathBits[2];
——@@ -69,11 +113,11 @@
70114 * Gets the site and lang from env variables
71115 */
72116 private function setSiteInfoForMaintenance() {
73 - //TODO: some error checking
 117+ // @TODO: some error checking
74118 $this->site = getenv( 'wikisite' );
75119 $this->lang = getenv( 'wikilang' );
76120 }
77 -
 121+
78122 /**
79123 * Get the site for this wiki
80124 * @return String site. Eg: wikipedia, wikinews, wikiversity
——@@ -81,7 +125,7 @@
82126 public function getSite() {
83127 return $this->site;
84128 }
85 -
 129+
86130 /**
87131 * Get the lang for this wiki
88132 * @return String lang Eg: en, de, ar, hi
——@@ -127,46 +171,4 @@
128172 }
129173 return $version;
130174 }
131 -
132 - /**
133 - * Factory method to get an instance of MWMultiVersion.
134 - * Use this for all wikis except calls to /w/thumb.php on upload.wikmedia.org.
135 - * @param $serverName the ServerName for this wiki -- $_SERVER['SERVER_NAME']
136 - * @param $docroot the DocumentRoot for this wiki -- $_SERVER['DOCUMENT_ROOT']
137 - * @return An MWMultiVersion object for this wiki
138 - */
139 - public static function getInstanceForWiki( $serverName, $docRoot ) {
140 - if (!isset(self::$mwversion)) {
141 - self::$mwversion = new self;
142 - self::$mwversion->setSiteInfoForWiki( $serverName, $docRoot );
143 - }
144 - return self::$mwversion;
145 - }
146 -
147 - /**
148 - * Factory method to get an instance of MWMultiVersion used for calls to /w/thumb.php on upload.wikmedia.org.
149 - * @param $pathInfo the PathInfo -- $_SERVER['PATH_INFO']
150 - * @return An MWMultiVersion object for the wiki derived from the pathinfo
151 - */
152 - public static function getInstanceForUploadWiki( $pathInfo ) {
153 - if (!isset(self::$mwversion)) {
154 - self::$mwversion = new self;
155 - self::$mwversion->setSiteInfoForUploadWiki( $PathInfo );
156 - }
157 - return self::$mwversion;
158 - }
159 -
160 - /**
161 - * Factory method to get an instance of MWMultiVersion via maintenance scripts since they need to set site and lang.
162 - * @return An MWMultiVersion object for the wiki derived from the env variables set by Maintenance.php
163 - */
164 - public static function getInstanceForMaintenance( ) {
165 - if (!isset(self::$mwversion)) {
166 - self::$mwversion = new self;
167 - self::$mwversion->setSiteInfoForMaintenance();
168 - }
169 - return self::$mwversion;
170 - }
171175 }
172 -
173 -?>
\ No newline at end of file

Comments

#Comment by Catrope (talk | contribs)   02:30, 21 July 2011 
+	$host = $secure ? $secure : $_SERVER['HTTP_HOST'];

Not introduced here, but commenting here so you get the e-mail instead of Priyanka: $secure is a global variable that needs to be set globally for various things in CommonSettings.php to work. It's currently an unused local variable.

#Comment by 😂 (talk | contribs)   22:23, 10 August 2011

$secure does seem used here locally. He's not changing whether it's getting set, he's just pulling the ENV value for it and working from that.

Status & tagging log