brion@svn.wikimedia.org schreef

+// I don't really know what I'm doing in the API and it might explode. ;)

Here's the bomb squad ;)

• 	throw new MWException( "Invalid repo {$args[0]}" );
  

Like with the previous module, you want to throw a regular error here rather than an MWException. MWExceptions are meant for things like weird one-in-a-million race conditions and database errors, not for invalid input. For normal error messages, use $this->dieUsage("Invalid repo ``{$args[0]}''", 'invalidrepo');

Also, since there seems to be an array of predefined repos available somewhere (the [[Special:Code]] main page shows it), you could use that in getAllowedParams() to specify an array of possible values for the repo parameter.

• $rev = intval( $params['rev'] );
  

• if( $rev <= 0 || $rev > $lastStoredRev ) {
  

• 	/// @fixme more sensible error
  

• 	throw new MWException( 'Invalid input revision' );
  

• }
  

Apart from the fact that you want to use dieUsage() here too, you can set the rev parameter's type to integer in getAllowedParams() and st a minimum value of 1. That'll take care of the intval() and the lower bound check for you.

• public function getAllowedParams() {

• return array(
  

• 	'repo' => null,
  

• 	'rev' => null );
  

• }

A better one would probably be:

public function getAllowedParams() { global $wgAvailableRepos; // or whatever it's called return array( 'repo' => array( ApiBase::PARAM_TYPE => $wgAvailableRepos ), 'rev' => array( ApiBase::PARAM_TYPE => 'integer', ApiBase::PARAM_MIN => 1

) ); }

Roan Kattouw (Catrope)