r89542 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r89541‎ | r89542 | r89543 >
Date:23:40, 5 June 2011
Author:reedy
Status:resolved (Comments)
Tags:
Comment:
* (bug 21346) Make deleted images searchable by hash (disabled in Miser Mode)

Effectively reverts r83411, but with the addition of it only works in !$wgMiserMode
Modified paths:
  • /trunk/phase3/RELEASE-NOTES-1.19 (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryFilearchive.php (modified) (history)

Diff [purge]

Index: trunk/phase3/RELEASE-NOTES-1.19
@@ -103,6 +103,7 @@
104104 action=login
105105 * (bug 29237) add interwiki target url attribute to api/query/interwiki
106106 * (bug 28392) mark action=undelete&timestamps as type "timestamp"
 107+* (bug 21346) Make deleted images searchable by hash (disabled in Miser Mode)
107108
108109 === Languages updated in 1.19 ===
109110
Index: trunk/phase3/includes/api/ApiQueryFilearchive.php
@@ -85,6 +85,25 @@
8686 $this->addWhere( 'fa_name' . $db->buildLike( $this->titlePartToKey( $params['prefix'] ), $db->anyString() ) );
8787 }
8888
 89+ $sha1Set = isset( $params['sha1'] );
 90+ $sha1base36Set = isset( $params['sha1base36'] );
 91+ if ( $sha1Set || $sha1base36Set ) {
 92+ global $wgMiserMode;
 93+ if ( $wgMiserMode ) {
 94+ $this->dieUsage( 'Search by hash disabled in Miser Mode', 'hashsearchdisabled' );
 95+ }
 96+
 97+ $sha1 = false;
 98+ if ( $sha1Set ) {
 99+ $sha1 = wfBaseConvert( $params['sha1'], 16, 36, 31 );
 100+ } elseif ( $sha1base36Set ) {
 101+ $sha1 = $params['sha1base36'];
 102+ }
 103+ if ( $sha1 ) {
 104+ $this->addWhere( 'fa_storage_key=' . $db->addQuotes( $sha1 ) );
 105+ }
 106+ }
 107+
89108 if ( !$wgUser->isAllowed( 'suppressrevision' ) ) {
90109 // Filter out revisions that the user is not allowed to see. There
91110 // is no way to indicate that we have skipped stuff because the
@@ -201,6 +220,8 @@
202221 'descending'
203222 )
204223 ),
 224+ 'sha1' => null,
 225+ 'sha1base36' => null,
205226 'prop' => array(
206227 ApiBase::PARAM_DFLT => 'timestamp',
207228 ApiBase::PARAM_ISMULTI => true,
@@ -227,6 +248,8 @@
228249 'prefix' => 'Search for all image titles that begin with this value',
229250 'dir' => 'The direction in which to list',
230251 'limit' => 'How many images to return in total',
 252+ 'sha1' => "SHA1 hash of image. Overrides {$this->getModulePrefix()}sha1base36. Disabled in Miser Mode",
 253+ 'sha1base36' => 'SHA1 hash of image in base 36 (used in MediaWiki). Disabled in Miser Mode',
231254 'prop' => array(
232255 'What image information to get:',
233256 ' sha1 - Adds SHA-1 hash for the image',
@@ -250,6 +273,7 @@
251274 public function getPossibleErrors() {
252275 return array_merge( parent::getPossibleErrors(), array(
253276 array( 'code' => 'permissiondenied', 'info' => 'You don\'t have permission to view deleted file information' ),
 277+ array( 'code' => 'hashsearchdisabled', 'info' => 'Search by hash disabled in Miser Mode' ),
254278 ) );
255279 }
256280

Follow-up revisions

RevisionCommit summaryAuthorDate
r89544* (bug 27595) sha1 search of list=filearchive does not work...reedy23:48, 5 June 2011
r89576Followup r89542, validate hashesreedy16:06, 6 June 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r66168* (bug 21346) Make deleted images searchable by hash...reedy22:41, 10 May 2010
r83411Kill filtering by hash because the query is unindexed. We need a condition on...btongminh22:16, 6 March 2011

Comments

#Comment by Nikerabbit (talk | contribs)   06:20, 6 June 2011

There is no predicate (verb) in the error message. Are user supposed to know what is Miser Mode? Is it documented anywhere?

#Comment by Reedy (talk | contribs)   09:44, 6 June 2011

I don't think it is on the api documentation, but it wasn't already (and there are 2 or 3 usages)

Maybe we could document it at the top

Would "Searching is disabled in Miser Mode" be better?

#Comment by Duplicatebug (talk | contribs)   14:11, 6 June 2011

You can also validate the hash input, see r88174. Thanks.

#Comment by Aaron Schulz (talk | contribs)   01:25, 25 June 2011

Don't we only use 'deleted' as a value for fa_storage_group here? So you should be able to use the INDEX.

Status & tagging log