r58196 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r58195‎ | r58196 | r58197 >
Date:15:11, 27 October 2009
Author:ashley
Status:deferred (Comments)
Tags:
Comment:
cleanup to GeoIP extension:
*use tabs for indentation, not spaces
*add (more) doxygen comments
*add extension credits
*NULL -> null, FALSE -> false
*removed wfLoadExtensionMessages() call in SpecialPage's constructor since no i18n file has been defined for the extension
*removed unused $wgRequest global
*added __METHOD__ to database calls
*changed $dbw->immediateBegin() and $dbw->immediateCommit() to $dbw->begin() and $dbw->commit(); the former two are deprecated
Modified paths:
  • /trunk/extensions/DonationInterface/GeoIP/GeoIP.php (modified) (history)
  • /trunk/extensions/DonationInterface/GeoIP/RebuildGeoIP.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DonationInterface/GeoIP/RebuildGeoIP.php
——@@ -1,5 +1,4 @@
22 <?php
3 -
43 /**
54 * This program is free software; you can redistribute it and/or modify
65 * it under the terms of the GNU General Public License as published by
——@@ -16,62 +15,65 @@
1716 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
1817 * http://www.gnu.org/copyleft/gpl.html
1918 *
 19+ * @file
2020 * @ingroup Maintenance
2121 */
2222
2323 require_once( dirname(__FILE__) . '/../../maintenance/Maintenance.php' );
2424
2525 class RebuildGeoIP extends Maintenance {
26 - public function __construct() {
27 - parent::__construct();
28 - $this->mDescription = 'Rebuild GeoIP data from a CSV file.';
29 - $this->addArg( 'csv-file', 'CSV-formatted file with geographic IP data.', true );
30 - }
31 -
32 - public function memoryLimit() {
33 - return '200M';
34 - }
 26+ public function __construct() {
 27+ parent::__construct();
 28+ $this->mDescription = 'Rebuild GeoIP data from a CSV file.';
 29+ $this->addArg( 'csv-file', 'CSV-formatted file with geographic IP data.', true );
 30+ }
3531
36 - public function execute() {
37 - $dbw = wfGetDB( DB_MASTER );
38 -
39 - // Perform this in a transaction so an failed load doesn't erase current data.
40 - $dbw->immediateBegin();
41 -
42 - // Clear existing GeoIP data.
43 - try {
44 - $dbw->delete( 'geoip', '*' );
45 - }
46 - catch (DBQueryError $e) {
47 - $this->error('ERROR: Could not delete existing geographic data. Is the GeoIP schema loaded?', true);
48 - }
49 -
50 - // Load fresh data from the first (and only) argument.
51 - $filename = $this->getArg(0);
52 - $lines = exec( 'wc -l ' . $filename );
53 - $handle = fopen( $filename, 'r' );
54 -
55 - $count = 0;
56 - while (($data = fgetcsv( $handle, 256, ',' )) !== FALSE) {
57 - // Output a nice progress bar.
58 - if ($count % 1000 == 0) {
59 - $progress = ceil(($count / $lines) * 50);
60 - $this->output('[' . str_repeat('=', $progress) . str_repeat(' ', 50 - $progress) . '] ' . ceil(($count / $lines) * 100) . '%' . "\r");
61 - }
62 - ++$count;
 32+ public function memoryLimit() {
 33+ return '200M';
 34+ }
6335
64 - $record = array(
65 - 'begin_ip_long' => IP::toUnsigned( $data[0] ),
66 - 'end_ip_long' => IP::toUnsigned( $data[1] ),
67 - 'country_code' => $data[4],
68 - );
69 - $dbw->insert( 'geoip', $record );
70 - }
71 - $this->output("\n");
72 -
73 - $dbw->immediateCommit();
74 - $this->output('Successfully loaded ' . $count . ' geographic IP ranges.' . "\n");
75 - }
 36+ public function execute() {
 37+ $dbw = wfGetDB( DB_MASTER );
 38+
 39+ // Perform this in a transaction so an failed load doesn't erase current data.
 40+ $dbw->begin();
 41+
 42+ // Clear existing GeoIP data.
 43+ try {
 44+ $dbw->delete( 'geoip', '*', __METHOD__ );
 45+ } catch ( DBQueryError $e ) {
 46+ $this->error( 'ERROR: Could not delete existing geographic data. Is the GeoIP schema loaded?', true );
 47+ }
 48+
 49+ // Load fresh data from the first (and only) argument.
 50+ $filename = $this->getArg( 0 );
 51+ $lines = exec( 'wc -l ' . $filename );
 52+ $handle = fopen( $filename, 'r' );
 53+
 54+ $count = 0;
 55+ while( ( $data = fgetcsv( $handle, 256, ',' ) ) !== false ) {
 56+ // Output a nice progress bar.
 57+ if( $count % 1000 == 0 ) {
 58+ $progress = ceil( ( $count / $lines ) * 50 );
 59+ $this->output( '[' . str_repeat( '=', $progress ) .
 60+ str_repeat( ' ', 50 - $progress ) . '] ' .
 61+ ceil( ( $count / $lines ) * 100 ) . '%' . "\r"
 62+ );
 63+ }
 64+ ++$count;
 65+
 66+ $record = array(
 67+ 'begin_ip_long' => IP::toUnsigned( $data[0] ),
 68+ 'end_ip_long' => IP::toUnsigned( $data[1] ),
 69+ 'country_code' => $data[4],
 70+ );
 71+ $dbw->insert( 'geoip', $record, __METHOD__ );
 72+ }
 73+ $this->output( "\n" );
 74+
 75+ $dbw->commit();
 76+ $this->output( 'Successfully loaded ' . $count . ' geographic IP ranges.' . "\n" );
 77+ }
7678 }
7779
7880 $maintClass = 'RebuildGeoIP';
Index: trunk/extensions/DonationInterface/GeoIP/GeoIP.php
——@@ -1,5 +1,4 @@
22 <?php
3 -
43 /**
54 * This program is free software; you can redistribute it and/or modify
65 * it under the terms of the GNU General Public License as published by
——@@ -15,78 +14,101 @@
1615 * with this program; if not, write to the Free Software Foundation, Inc.,
1716 * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
1817 * http://www.gnu.org/copyleft/gpl.html
 18+ *
 19+ * @file
 20+ * @ingroup Extensions
 21+ * @version 1.0
 22+ * @author David Strauss <david@fourkitchens.com>
 23+ * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
1924 */
2025
21 -// Alert the user that this is not a valid entry point to MediaWiki if they try to access the special pages file directly.
22 -if (!defined('MEDIAWIKI')) {
23 - echo <<<EOT
24 -To install my extension, put the following line in LocalSettings.php:
 26+// Alert the user that this is not a valid entry point to MediaWiki if they try to access this file directly.
 27+if ( !defined ('MEDIAWIKI' ) ) {
 28+ echo <<<EOT
 29+To install GeoIP extension, put the following line in LocalSettings.php:
2530 require_once( "\$IP/extensions/GeoIP/GeoIP.php" );
2631 EOT;
27 - exit( 1 );
 32+ exit( 1 );
2833 }
29 -
 34+
 35+// Extension credits that will show up on Special:Version
 36+$wgExtensionCredits['specialpage'][] = array(
 37+ 'name' => 'GeoIP',
 38+ 'version' => '1.0',
 39+ 'author' => 'David Strauss',
 40+ 'description' => "[[Special:GeoIP|Shows user's geographical location, based on their IP address]]",
 41+ 'url' => 'http://www.mediawiki.org/wiki/Extension:GeoIP'
 42+);
 43+
3044 $wgHooks['LoadExtensionSchemaUpdates'][] = 'fnGeoIPSchema';
3145 $wgSpecialPages['GeoIP'] = 'GeoIP';
3246
3347 function fnGeoIPSchema() {
34 - global $wgExtNewTables;
35 - $wgExtNewTables[] = array(
36 - 'geoip',
37 - dirname( __FILE__ ) . '/GeoIP.sql' );
38 - return true;
 48+ global $wgExtNewTables;
 49+ $wgExtNewTables[] = array(
 50+ 'geoip',
 51+ dirname( __FILE__ ) . '/GeoIP.sql'
 52+ );
 53+ return true;
3954 }
4055
4156 class UnsupportedGeoIP extends Exception { }
4257 class NotFoundGeoIP extends Exception { }
4358
44 -function fnGetGeoIP($ip_address = NULL) {
45 - if (!isset($ip_address)) {
46 - $ip_address = IP::sanitizeIP(wfGetIP());
47 - }
 59+function fnGetGeoIP( $ip_address = null ) {
 60+ if( !isset( $ip_address ) ) {
 61+ $ip_address = IP::sanitizeIP( wfGetIP() );
 62+ }
4863
49 - if (isset($_GET['ip'])) {
50 - $ip_address = IP::sanitizeIP($_GET['ip']);
51 - }
 64+ if( isset( $_GET['ip'] ) ) {
 65+ $ip_address = IP::sanitizeIP( $_GET['ip'] );
 66+ }
5267
53 - if (!IP::isIPv4($ip_address)) {
54 - throw new UnsupportedGeoIP('Only IPv4 addresses are supported.');
55 - }
56 -
57 - $country_code = NULL;
58 -
59 - $dbr = wfGetDB( DB_SLAVE );
60 - $long_ip = IP::toUnsigned( $ip_address );
61 - $conditions = array(
62 - 'begin_ip_long <= ' . $long_ip,
63 - $long_ip . ' <= end_ip_long',
64 - );
65 -
66 - $country_code = $dbr->selectField( 'geoip', 'country_code', $conditions);
67 -
68 - if ( !$country_code ) {
69 - throw new NotFoundGeoIP('Could not identify the country for the provided IP address.');
70 - }
71 -
72 - return $country_code;
 68+ if( !IP::isIPv4( $ip_address ) ) {
 69+ throw new UnsupportedGeoIP( 'Only IPv4 addresses are supported.' );
 70+ }
 71+
 72+ $country_code = null;
 73+
 74+ $dbr = wfGetDB( DB_SLAVE );
 75+ $long_ip = IP::toUnsigned( $ip_address );
 76+ $conditions = array(
 77+ 'begin_ip_long <= ' . $long_ip,
 78+ $long_ip . ' <= end_ip_long',
 79+ );
 80+
 81+ $country_code = $dbr->selectField( 'geoip', 'country_code', $conditions, __METHOD__ );
 82+
 83+ if ( !$country_code ) {
 84+ throw new NotFoundGeoIP( 'Could not identify the country for the provided IP address.' );
 85+ }
 86+
 87+ return $country_code;
7388 }
7489
7590 class GeoIP extends SpecialPage {
76 - function __construct() {
 91+
 92+ /**
 93+ * Constructor -- set up the new special page
 94+ */
 95+ public function __construct() {
7796 parent::__construct( 'GeoIP' );
78 - wfLoadExtensionMessages('GeoIP');
7997 }
80 -
81 - function execute( $par ) {
82 - global $wgRequest, $wgOut;
83 -
 98+
 99+ /**
 100+ * Show the special page
 101+ *
 102+ * @param $par Mixed: parameter passed to the page or null
 103+ */
 104+ public function execute( $par ) {
 105+ global $wgOut;
 106+
84107 $this->setHeaders();
85 -
86 - try {
87 - $wgOut->addHTML( '<p>' . fnGetGeoIP() . '</p>' );
88 - }
89 - catch (Exception $e) {
90 - $wgOut->addHTML( '<p>Unknown</p>' );
91 - }
 108+
 109+ try {
 110+ $wgOut->addHTML( '<p>' . fnGetGeoIP() . '</p>' );
 111+ } catch ( Exception $e ) {
 112+ $wgOut->addHTML( '<p>Unknown</p>' );
 113+ }
92114 }
93 -}
 115+}
\ No newline at end of file

Comments

#Comment by Tim Starling (talk | contribs)   09:02, 30 December 2009

Marking deferred since it's superseded by GeoLite and I assume it's not going to be used now.

Status & tagging log