brion(a)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)