brion(a)svn.wikimedia.org schreef:
> Revision: 41458
> Author: brion
> Date: 2008-09-30 23:04:24 +0000 (Tue, 30 Sep 2008)
>
> Log Message:
> -----------
> Add an API module that can be POSTed to to trigger a codereview repo update
>
> + $repo = CodeRepository::newFromName( $params['repo'] );
> + if( !$repo ){
> + throw new MWException( "Invalid repo {$args[0]}" );
> + }
Throwing an MWException on invalid input is not very nice. You should
really use something like:
$this->dieUsage("Invalid repo ``{$args[0]}''", 'invalidrepo');
If you have some kind of array containing all valid repos lying around,
you could use that in getAllowedParams() so the repo parameter will
automatically be validated by extractRequestParams().
> + $endRev = intval( $params['rev'] );
You can also set the rev parameter to be an integer in
getAllowedParams(), so you won't have to use intval() (and so
non-integers will be rejected instead of being silently converted to 0).
> + if( $lastStoredRev >= $endRev ) {
> + // Nothing to do, we're up to date.
> + return;
> + }
You should still output *something* here (see also below).
> + // fixme: this could be a lot?
Yeah, you gotta thing about max exec time here. What you could do is
just process the first 50 or so and tell the client where you stopped,
so they can issue another request for the next 50.
> + $log = $svn->getLog( '', $lastStoredRev + 1, $endRev );
> + if( !$log ) {
> + throw new MWException( "Something awry..." );
> + }
I don't know in which circumstances $log would be empty or false, but
unless it only happens in really crazy cases you should use dieUsage()
here as well.
> + foreach( $log as $data ) {
> + $codeRev = CodeRevision::newFromSvn( $repo, $data );
> + $codeRev->save();
> + // would be nice to output something but the api code is weird
> + // and i don't feel like figuring it out right now :)
> + }
You probably wanna do something like this:
$data = array();
foreach($log as $entry)
{
// Do whatever you need to do
$data['saved'][] = array('revid' => $revid, /* More properties
here */);
}
$data['continue'] = $revidToContinueFrom;
// Tell the XML formatter to use <rev> tags for the $data['saved'] array
// You need to call this function on every array with integer indices,
// or ApiFormatXml will scream and die
$this->getResult()->setIndexedTagName($data['saved'], 'rev');
// Add $data on the top level (that's what the null does) in a
// <codeupdate> tag (which is what getModuleName() returns)
$this->getResult()->addValue(null, $this->getModuleName(), $data);
The XML this will return will look like:
<api>
<codeupdate continue="56">
<saved>
<rev revid="54" />
<rev revid="55" />
<rev revid="56" />
</saved>
</codeupdate>
</api>
> + public function getAllowedParams() {
> + return array(
> + 'repo' => null,
> + 'rev' => null );
> + }
A better getAllowedParams() array would probably be:
// This assumes $wgAvailableRepos is an array of
// (surprise) available repos. If there is no such
// array, you should still use 'repo' => null like
// you did initially.
array(
'repo' => array(
ApiBase::PARAM_TYPE => $wgAvailableRepos
),
'rev' => array(
ApiBase::PARAM_TYPE => 'integer'
)
)
Roan Kattouw (Catrope)