MediaWiki r44145 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:r44144‎ | r44145 (on ViewVC)‎ | r44146 >
Date:13:00, 2 December 2008
Author:werdna
Status:resolved (Comments)
Tags:
Comment:
Configure extension updates:
* Fix some register_globals stuff.
* Fix recaching per-request.
* Move timestamp from the keys of a data array to an element in the values. It's *probably* okay as a unique index, but I guess I shouldn't be lazy.
* Ignore metadata when getting settings (file handler).
* Kill a random unused message.
* Hack around the problem of duplicate files with the same timestamp by fudging forward a second.
Modified paths:

Diff [purge]

Index: trunk/extensions/Configure/Configure.page.php
===================================================================
--- trunk/extensions/Configure/Configure.page.php	(revision 44144)
+++ trunk/extensions/Configure/Configure.page.php	(revision 44145)
@@ -355,7 +355,8 @@
 
 		ksort($versions); ## Put in ascending order for now.
 
-		foreach ( $versions as $ts => $data ) {
+		foreach ( $versions as $data ) {
+			$ts = $data['timestamp'];
 			if ( in_array( $this->mWiki, $wgConf->getWikisInVersion( $ts ) ) ) {
 				$count++;
 				$link = $skin->makeKnownLinkObj( $title, $wgLang->timeAndDate( $ts ), "version=$ts" );
Index: trunk/extensions/Configure/Configure.handler-files.php
===================================================================
--- trunk/extensions/Configure/Configure.handler-files.php	(revision 44144)
+++ trunk/extensions/Configure/Configure.handler-files.php	(revision 44145)
@@ -123,10 +123,10 @@
 				if (isset( $settings['__metadata'] )) {
 					$metadata = $settings['__metadata'];
 					
-					$files[$m[1]] = array( 'username' => $metadata['user_name'], 
-						'userwiki' => $metadata['user_wiki'], 'reason' => $metadata['reason'] );
+					$files[] = array( 'username' => $metadata['user_name'], 
+						'userwiki' => $metadata['user_wiki'], 'reason' => $metadata['reason'], 'timestamp' => $m[1] );
 				} else {
-					$files[$m[1]] = array( 'username' => false, 'userwiki' => false, 'reason' => false );
+					$files[] = array( 'username' => false, 'userwiki' => false, 'reason' => false, 'timestamp' => $m[1] );
 				}
 			}
 		}
@@ -190,6 +190,13 @@
 	}
 	
 	public function listArchiveVersions() {
-		return array_keys( $this->getArchiveVersions() );
+		$versions = $this->getArchiveVersions();
+		$ret = array();
+		
+		foreach( $versions as $data ) {
+			$ret[] = $data['timestamp'];
+		}
+		
+		return $ret;
 	}
 }
Index: trunk/extensions/Configure/Configure.handler-db.php
===================================================================
--- trunk/extensions/Configure/Configure.handler-db.php	(revision 44144)
+++ trunk/extensions/Configure/Configure.handler-db.php	(revision 44145)
@@ -98,14 +98,13 @@
 
 		# Check filesystem cache
 		if ( $useCache && ( $cached = $this->getFSCached() ) ) {
-			# FIXME: why does this always recache?
-			$this->cacheToFS( $cached );
 			return $ipCached = $cached;
 		}
 
 		$cacheKey = $this->cacheKey( 'configure', 'current' );
 		$cached = $this->getCache()->get( $cacheKey );
 		if ( is_array( $cached ) && $useCache ) {
+			$this->cacheToFS( $cached );
 			return $ipCached = $cached;
 		}
 
@@ -257,7 +256,7 @@
 		);
 		$arr = array();
 		foreach ( $ret as $row ) {
-			$arr[$row->cv_timestamp] = array( 'username' => $row->cv_user_text, 'userwiki' => $row->cv_user_wiki, 'reason' => $row->cv_reason );
+			$arr[] = array( 'username' => $row->cv_user_text, 'userwiki' => $row->cv_user_wiki, 'reason' => $row->cv_reason, 'timestamp' => $row->cv_timestamp );
 		}
 		return $arr;
 	}
Index: trunk/extensions/Configure/Configure.diff.php
===================================================================
--- trunk/extensions/Configure/Configure.diff.php	(revision 44144)
+++ trunk/extensions/Configure/Configure.diff.php	(revision 44145)
@@ -337,6 +337,9 @@
 			
 			## This is kinda annoying. We can't copy ALL settings over, because not all settings are stored.
 			foreach( $new as $wiki => $newSettings ) {
+				if ($wiki == '__metadata') ## Ignore metadata.
+					continue;
+				
 				$defaultSettings[$wiki] = array();
 				
 				foreach( $newSettings as $key => $value ) {
Index: trunk/extensions/Configure/SpecialExtensions.php
===================================================================
--- trunk/extensions/Configure/SpecialExtensions.php	(revision 44144)
+++ trunk/extensions/Configure/SpecialExtensions.php	(revision 44145)
@@ -84,7 +84,6 @@
 
 	/**
 	 * Build the content of the form
-	 * FIXME: register_globals
 	 *
 	 * @return xhtml
 	 */
@@ -96,20 +95,21 @@
 			foreach ( $settings as $setting => $type ) {
 				if ( !isset( $GLOBALS[$setting] ) && !isset( $this->conf[$setting] ) && file_exists( $ext->getFile() ) ) {
 					if ( !$globalDone ) {
-						extract( $GLOBALS, EXTR_REFS );
-						$__hooks__ = $wgHooks;
+						global $wgHooks;
+						
+						$oldHooks = $wgHooks;
 						$globalDone = true;
 					}
 					require_once( $ext->getFile() );
-					if ( isset( $$setting ) )
-						$this->conf[$setting] = $$setting;
+					if ( isset( $GLOBALS[$setting] ) )
+						$this->conf[$setting] = $GLOBALS[$setting];
 				}
 			}
 			$ext->setPageObj( $this );
 			$ret .= $ext->getHtml();
 		}
 		if ( isset( $__hooks__ ) )
-			$GLOBALS['wgHooks'] = $__hooks__;
+			$GLOBALS['wgHooks'] = $oldHooks;
 		return $ret;
 	}
 }

Comments

#Comment by Aaron Schulz (Talk | contribs)   14:59, 2 December 2008

register_globals issue seems to remain. It can fool the 'if ( !isset( $GLOBALS[$setting] )' line.

#Comment by Aaron Schulz (Talk | contribs)   15:00, 2 December 2008

Partial revert in r44146

Status & tagging log

Personal tools
Namespaces
Variants
Views
Actions
Site
Support
Download
Development
Communication
Toolbox