From 392d2af20c258d82e3c7947ab8bcc6658625f2ff Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Tue, 26 Oct 2021 10:17:20 -0700 Subject: [PATCH] ApiQueryImageInfo: don't show empty comments as deleted Bug: T293783 Change-Id: Icddd06c7171bc1d1dba14ec7d6703e082b87836a --- includes/api/ApiQueryImageInfo.php | 65 ++++++++------- .../api/query/ApiQueryImageInfoTest.php | 79 +++++++++++++++++-- 2 files changed, 108 insertions(+), 36 deletions(-) diff --git a/includes/api/ApiQueryImageInfo.php b/includes/api/ApiQueryImageInfo.php index b42408b3d5b..7d23e3247e0 100644 --- a/includes/api/ApiQueryImageInfo.php +++ b/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'] ) { - $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 { + if ( $file->isDeleted( File::DELETED_USER ) ) { $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'] ) { - $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 { + if ( $file->isDeleted( File::DELETED_COMMENT ) ) { $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'] ); diff --git a/tests/phpunit/includes/api/query/ApiQueryImageInfoTest.php b/tests/phpunit/includes/api/query/ApiQueryImageInfoTest.php index 6d8bca1b393..fb5f729668a 100644 --- a/tests/phpunit/includes/api/query/ApiQueryImageInfoTest.php +++ b/tests/phpunit/includes/api/query/ApiQueryImageInfoTest.php @@ -1,5 +1,7 @@ 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() - ->getActorStore() - ->acquireActorId( $this->getTestUser()->getUserIdentity(), $this->db ), + 'img_actor' => $actorId, '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() - ->getActorStore() - ->acquireActorId( $this->getTestUser()->getUserIdentity(), $this->db ), + 'oi_actor' => $actorId, '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'] ); + } }