User "Bryan" posted a comment on MediaWiki.r92009.
Full URL:
https://secure.wikimedia.org/wikipedia/mediawiki/wiki/Special:Code/MediaWiki/92009#c19537
Commit summary:
Refactored UploadStash and related classes to use the database for file
metadata storage instead of the session, see bug 26179
Tweaked the UploadWizard to work properly with the new backend code, updated
tests
Comment:
<pre>
+ public function __construct( $stash = false, $repo = false ) {
+ if( !$this->repo ) {
+ $this->repo = RepoGroup::singleton()->getLocalRepo();
+ }
</pre>
This is not what you meant I think. You want if ( $repo ) { $this->repo = $repo
} else { $this->repo = RepoGroup::singleton()->getLocalRepo(); }
<pre>
public function getFile( $key ) {
+ global $wgUser;
<pre>
We should avoid new uses of $wgUser. Pass a user object as parameter to
getFile().
<pre>
+ $lag = $dbr->getLag();
</pre>
$dbr is undefined at this point.
<pre>
+ 'us_timestamp' => wfTimestamp( TS_MW )
</pre>
This breaks postgres, use $dbw->timestamp()
<pre>
+ $dbw = wfGetDB( DB_MASTER );
+ $dbw->insert(
+ 'uploadstash',
+ $this->fileMetadata[$key],
+ __METHOD__
+ );
</pre>
The uploadstash table is repobound, so you should use $repo->getMasterDb().
<pre>
+ $dbw->delete(
+ 'uploadstash',
+ array( 'us_key' => $key, 'us_user' => $userId ),
+ __METHOD__
+ );
</pre>
us_key is a UNIQUE INDEX, so you don't need the condition on us_user.
<pre>
+ while( $row = $dbr->fetchRow( $res ) ) {
+ array_push( $keys, $row['us_key'] );
+ }
</pre>
fetchRow returns objects, not arrays iirc. You should not use while {}, but
iterate over $res with foreach.
<pre>
+ us_media_type varchar(255),
</pre>
This is an enum in image, so for consistency this should also be an enum, but
on the other hand enums suck. Hmmm...
_______________________________________________
MediaWiki-CodeReview mailing list
mediawiki-coderev...@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
