Merge "ApiQueryImageInfo: don't show empty comments as deleted"

includes/api/ApiQueryImageInfo.php

@@ -433,29 +433,39 @@ class ApiQueryImageInfo extends ApiQueryBase {
 			$vals['timestamp'] = wfTimestamp( TS_ISO_8601, $file->getTimestamp() );
 		}
 
+		// Handle external callers who don't pass revdelUser
+		if ( isset( $opts['revdelUser'] ) && $opts['revdelUser'] ) {
+			$revdelUser = $opts['revdelUser'];
+			$canShowField = static function ( $field ) use ( $file, $revdelUser ) {
+				return $file->userCan( $field, $revdelUser );
+			};
+		} else {
+			$canShowField = static function ( $field ) use ( $file ) {
+				return !$file->isDeleted( $field );
+			};
+		}
+
 		$user = isset( $prop['user'] );
 		$userid = isset( $prop['userid'] );
 
 		if ( ( $user || $userid ) && $exists ) {
-			if ( isset( $opts['revdelUser'] ) && $opts['revdelUser'] ) {
+			if ( $file->isDeleted( File::DELETED_USER ) ) {
-				$uploader = $file->getUploader( File::FOR_THIS_USER, $opts['revdelUser'] );
-			} else {
-				$uploader = $file->getUploader( File::FOR_PUBLIC );
-			}
-			if ( $uploader ) {
-				if ( $user ) {
-					$vals['user'] = $uploader->getName();
-				}
-				if ( $userid ) {
-					$vals['userid'] = $uploader->getId();
-				}
-				if ( !$uploader->isRegistered() ) {
-					$vals['anon'] = true;
-				}
-			} else {
 				$vals['userhidden'] = true;
 				$anyHidden = true;
 			}
+			if ( $canShowField( File::DELETED_USER ) ) {
+				// Already checked if the field can be show
+				$uploader = $file->getUploader( File::RAW );
+				if ( $user ) {
+					$vals['user'] = $uploader ? $uploader->getName() : '';
+				}
+				if ( $userid ) {
+					$vals['userid'] = $uploader ? $uploader->getId() : 0;
+				}
+				if ( $uploader && !$uploader->isRegistered() ) {
+					$vals['anon'] = true;
+				}
+			}
 		}
 
 		// This is shown even if the file is revdelete'd in interface
@@ -482,22 +492,19 @@ class ApiQueryImageInfo extends ApiQueryBase {
 		$comment = isset( $prop['comment'] );
 
 		if ( ( $pcomment || $comment ) && $exists ) {
-			if ( isset( $opts['revdelUser'] ) && $opts['revdelUser'] ) {
+			if ( $file->isDeleted( File::DELETED_COMMENT ) ) {
-				$description = $file->getDescription( File::FOR_THIS_USER, $opts['revdelUser'] );
-			} else {
-				$description = $file->getDescription( File::FOR_PUBLIC );
-			}
-			if ( $description ) {
-				if ( $pcomment ) {
-					$vals['parsedcomment'] = Linker::formatComment( $description, $file->getTitle() );
-				}
-				if ( $comment ) {
-					$vals['comment'] = $description;
-				}
-			} else {
 				$vals['commenthidden'] = true;
 				$anyHidden = true;
 			}
+			if ( $canShowField( File::DELETED_COMMENT ) ) {
+				if ( $pcomment ) {
+					$vals['parsedcomment'] = Linker::formatComment(
+						$file->getDescription( File::RAW ), $file->getTitle() );
+				}
+				if ( $comment ) {
+					$vals['comment'] = $file->getDescription( File::RAW );
+				}
+			}
 		}
 
 		$canonicaltitle = isset( $prop['canonicaltitle'] );

tests/phpunit/includes/api/query/ApiQueryImageInfoTest.php

@@ -1,5 +1,7 @@
 <?php
 
+use MediaWiki\Tests\Unit\Permissions\MockAuthorityTrait;
+
 /**
  * @covers ApiQueryImageInfo
  * @group API
@@ -7,6 +9,7 @@
  * @group Database
  */
 class ApiQueryImageInfoTest extends ApiTestCase {
+	use MockAuthorityTrait;
 
 	private const IMAGE_NAME = 'Random-11m.png';
 
@@ -18,6 +21,8 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 
 	private const NEW_IMAGE_SIZE = 54321;
 
+	private const NO_COMMENT_TIMESTAMP = '20201105235239';
+
 	protected function setUp(): void {
 		parent::setUp();
 		$this->tablesUsed[] = 'image';
@@ -26,6 +31,9 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 
 	public function addDBData() {
 		parent::addDBData();
+		$actorId = $this->getServiceContainer()
+			->getActorStore()
+			->acquireActorId( $this->getTestUser()->getUserIdentity(), $this->db );
 		$this->db->insert(
 			'image',
 			[
@@ -41,9 +49,7 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 				'img_description_id' => $this->getServiceContainer()
 					->getCommentStore()
 					->createComment( $this->db, "'''comment'''" )->id,
-				'img_actor' => $this->getServiceContainer()
+				'img_actor' => $actorId,
-					->getActorStore()
-					->acquireActorId( $this->getTestUser()->getUserIdentity(), $this->db ),
 				'img_timestamp' => $this->db->timestamp( self::NEW_IMAGE_TIMESTAMP ),
 				'img_sha1' => 'sy02psim0bgdh0jt4vdltuzoh7j80ru',
 			]
@@ -52,7 +58,7 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 			'oldimage',
 			[
 				'oi_name' => 'Random-11m.png',
-				'oi_archive_name' => 'Random-11m.png',
+				'oi_archive_name' => self::OLD_IMAGE_TIMESTAMP . 'Random-11m.png',
 				'oi_size' => self::OLD_IMAGE_SIZE,
 				'oi_width' => 1000,
 				'oi_height' => 1800,
@@ -64,14 +70,34 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 				'oi_description_id' => $this->getServiceContainer()
 					->getCommentStore()
 					->createComment( $this->db, 'deleted comment' )->id,
-				'oi_actor' => $this->getServiceContainer()
+				'oi_actor' => $actorId,
-					->getActorStore()
-					->acquireActorId( $this->getTestUser()->getUserIdentity(), $this->db ),
 				'oi_timestamp' => $this->db->timestamp( self::OLD_IMAGE_TIMESTAMP ),
 				'oi_sha1' => 'sy02psim0bgdh0jt4vdltuzoh7j80ru',
 				'oi_deleted' => File::DELETED_FILE | File::DELETED_COMMENT | File::DELETED_USER,
 			]
 		);
+		$this->db->insert(
+			'oldimage',
+			[
+				'oi_name' => 'Random-11m.png',
+				'oi_archive_name' => self::NO_COMMENT_TIMESTAMP . 'Random-11m.png',
+				'oi_size' => self::OLD_IMAGE_SIZE,
+				'oi_width' => 1000,
+				'oi_height' => 1800,
+				'oi_metadata' => '',
+				'oi_bits' => 16,
+				'oi_media_type' => 'BITMAP',
+				'oi_major_mime' => 'image',
+				'oi_minor_mime' => 'png',
+				'oi_description_id' => $this->getServiceContainer()
+					->getCommentStore()
+					->createComment( $this->db, '' )->id,
+				'oi_actor' => $actorId,
+				'oi_timestamp' => $this->db->timestamp( self::NO_COMMENT_TIMESTAMP ),
+				'oi_sha1' => 'sy02psim0bgdh0jt4vdltuzoh7j80ru',
+				'oi_deleted' => 0,
+			]
+		);
 	}
 
 	private function getImageInfoFromResult( array $result ) {
@@ -106,6 +132,21 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 		$this->assertSame( self::NEW_IMAGE_SIZE, $image['size'] );
 	}
 
+	public function testGetImageEmptyComment() {
+		[ $result, ] = $this->doApiRequest( [
+			'action' => 'query',
+			'prop' => 'imageinfo',
+			'titles' => 'File:' . self::IMAGE_NAME,
+			'iiprop' => implode( '|', ApiQueryImageInfo::getPropertyNames() ),
+			'iistart' => self::NO_COMMENT_TIMESTAMP,
+			'iiend' => self::NO_COMMENT_TIMESTAMP,
+		] );
+		$image = $this->getImageInfoFromResult( $result );
+		$this->assertSame( MWTimestamp::convert( TS_ISO_8601, self::NO_COMMENT_TIMESTAMP ), $image['timestamp'] );
+		$this->assertSame( '', $image['comment'] );
+		$this->assertArrayNotHasKey( 'commenthidden', $image );
+	}
+
 	public function testGetImageInfoOldRestrictedImage() {
 		[ $result, ] = $this->doApiRequest( [
 				'action' => 'query',
@@ -129,4 +170,28 @@ class ApiQueryImageInfoTest extends ApiTestCase {
 		$this->assertTrue( $image['filehidden'] );
 		$this->assertSame( self::OLD_IMAGE_SIZE, $image['size'] );
 	}
+
+	public function testGetImageInfoOldRestrictedImage_sysop() {
+		[ $result, ] = $this->doApiRequest( [
+			'action' => 'query',
+			'prop' => 'imageinfo',
+			'titles' => 'File:' . self::IMAGE_NAME,
+			'iiprop' => implode( '|', ApiQueryImageInfo::getPropertyNames() ),
+			'iistart' => self::OLD_IMAGE_TIMESTAMP,
+			'iiend' => self::OLD_IMAGE_TIMESTAMP,
+		],
+			null,
+			false,
+			$this->mockRegisteredUltimateAuthority()
+		);
+		$image = $this->getImageInfoFromResult( $result );
+		$this->assertSame( MWTimestamp::convert( TS_ISO_8601, self::OLD_IMAGE_TIMESTAMP ), $image['timestamp'] );
+		$this->assertTrue( $image['commenthidden'] );
+		$this->assertSame( 'deleted comment', $image['comment'] );
+		$this->assertTrue( $image['userhidden'] );
+		$this->assertSame( $this->getTestUser()->getUserIdentity()->getName(), $image['user'] );
+		$this->assertSame( $this->getTestUser()->getUserIdentity()->getId(), $image['userid'] );
+		$this->assertTrue( $image['filehidden'] );
+		$this->assertSame( self::OLD_IMAGE_SIZE, $image['size'] );
+	}
 }