From 1782bc7fd481e29a98ba105b1515a7757342391e Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Mon, 28 Jun 2021 12:49:47 -0700 Subject: [PATCH] Remove hard-deprecated LogEntry::getPerformer Change-Id: Ia2c4819848f5d23d5ceb74aae9c6c5920b8851ba --- RELEASE-NOTES-1.37 | 2 + includes/logging/DatabaseLogEntry.php | 43 ++++++++++--------- includes/logging/LogEntry.php | 7 --- includes/logging/ManualLogEntry.php | 8 ---- includes/logging/RCDatabaseLogEntry.php | 26 ++++++++--- .../includes/logging/DatabaseLogEntryTest.php | 37 ++++++++++++++++ .../includes/logging/LogFormatterTestCase.php | 4 +- 7 files changed, 82 insertions(+), 45 deletions(-) diff --git a/RELEASE-NOTES-1.37 b/RELEASE-NOTES-1.37 index 443828d8561..21a75e48c8a 100644 --- a/RELEASE-NOTES-1.37 +++ b/RELEASE-NOTES-1.37 @@ -297,6 +297,8 @@ because of Phabricator reports. - ::loadRevisionFromTitle, - ::loadRevisionFromTimestamp, - ::listRevisionSizes +* LogEntry::getPerformer, deprecated since 1.36, was removed along with methods + in subclasses: DatabaseLogEntry, ManualLogEntry, RCDatabaseLogEntry. * … === Deprecations in 1.37 === diff --git a/includes/logging/DatabaseLogEntry.php b/includes/logging/DatabaseLogEntry.php index 960fd591c3b..5f366e71893 100644 --- a/includes/logging/DatabaseLogEntry.php +++ b/includes/logging/DatabaseLogEntry.php @@ -23,6 +23,7 @@ * @since 1.19 */ +use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; use MediaWiki\User\UserIdentity; use Wikimedia\Rdbms\IDatabase; @@ -129,7 +130,7 @@ class DatabaseLogEntry extends LogEntryBase { /** @var stdClass Database result row. */ protected $row; - /** @var User */ + /** @var UserIdentity */ protected $performer; /** @var array Parameters for log entry */ @@ -151,7 +152,7 @@ class DatabaseLogEntry extends LogEntryBase { * @return int */ public function getId() { - return (int)$this->row->log_id; + return (int)( $this->row->log_id ?? 0 ); } /** @@ -206,26 +207,26 @@ class DatabaseLogEntry extends LogEntryBase { return $this->revId; } - protected function getPerformerUser(): User { - if ( !$this->performer ) { - $this->performer = MediaWikiServices::getInstance()->getUserFactory() - ->newFromAnyId( - $this->row->user_id ?? 0, // left join failure means anonymous - $this->row->log_user_text, - $this->row->log_actor - ); - } - - return $this->performer; - } - - public function getPerformer() { - wfDeprecated( __METHOD__, '1.36' ); - return $this->getPerformerUser(); - } - public function getPerformerIdentity(): UserIdentity { - return $this->getPerformerUser(); + if ( !$this->performer ) { + $actorStore = MediaWikiServices::getInstance()->getActorStore(); + try { + $this->performer = $actorStore->newActorFromRowFields( + $this->row->user_id ?? 0, + $this->row->log_user_text ?? null, + $this->row->log_actor ?? null + ); + } catch ( InvalidArgumentException $e ) { + LoggerFactory::getInstance( 'logentry' )->warning( + 'Failed to instantiate log entry performer', [ + 'exception' => $e, + 'log_id' => $this->getId() + ] + ); + $this->performer = $actorStore->getUnknownActor(); + } + } + return $this->performer; } public function getTarget() { diff --git a/includes/logging/LogEntry.php b/includes/logging/LogEntry.php index 9be27afeafb..c5e5a258746 100644 --- a/includes/logging/LogEntry.php +++ b/includes/logging/LogEntry.php @@ -61,13 +61,6 @@ interface LogEntry { */ public function getParameters(); - /** - * Get the user who performed this action. - * @deprecated since 1.36 use ::getPerformerIdentity instead - * @return User - */ - public function getPerformer(); - /** * @since 1.36 * @return UserIdentity diff --git a/includes/logging/ManualLogEntry.php b/includes/logging/ManualLogEntry.php index 85620864fc4..aa2cc802061 100644 --- a/includes/logging/ManualLogEntry.php +++ b/includes/logging/ManualLogEntry.php @@ -455,14 +455,6 @@ class ManualLogEntry extends LogEntryBase implements Taggable { return $this->parameters; } - /** - * @return User - */ - public function getPerformer() { - wfDeprecated( __METHOD__, '1.36' ); - return User::newFromIdentity( $this->performer ); - } - /** * @return UserIdentity */ diff --git a/includes/logging/RCDatabaseLogEntry.php b/includes/logging/RCDatabaseLogEntry.php index 5b9ed7353f3..3b2a04ca00f 100644 --- a/includes/logging/RCDatabaseLogEntry.php +++ b/includes/logging/RCDatabaseLogEntry.php @@ -23,7 +23,9 @@ * @since 1.19 */ +use MediaWiki\Logger\LoggerFactory; use MediaWiki\MediaWikiServices; +use MediaWiki\User\UserIdentity; use Wikimedia\Rdbms\IDatabase; /** @@ -64,15 +66,25 @@ class RCDatabaseLogEntry extends DatabaseLogEntry { return $this->row->rc_log_action; } - protected function getPerformerUser(): User { + public function getPerformerIdentity(): UserIdentity { if ( !$this->performer ) { - $this->performer = MediaWikiServices::getInstance()->getUserFactory()->newFromAnyId( - $this->row->rc_user ?? 0, - $this->row->rc_user_text, - $this->row->rc_actor ?? null - ); + $actorStore = MediaWikiServices::getInstance()->getActorStore(); + try { + $this->performer = $actorStore->newActorFromRowFields( + $this->row->rc_user ?? 0, + $this->row->rc_user_text, + $this->row->rc_actor ?? null + ); + } catch ( InvalidArgumentException $e ) { + LoggerFactory::getInstance( 'logentry' )->warning( + 'Failed to instantiate RC log entry performer', [ + 'exception' => $e, + 'log_id' => $this->getId() + ] + ); + $this->performer = $actorStore->getUnknownActor(); + } } - return $this->performer; } diff --git a/tests/phpunit/includes/logging/DatabaseLogEntryTest.php b/tests/phpunit/includes/logging/DatabaseLogEntryTest.php index 267b20bed6d..09e364c235e 100644 --- a/tests/phpunit/includes/logging/DatabaseLogEntryTest.php +++ b/tests/phpunit/includes/logging/DatabaseLogEntryTest.php @@ -1,6 +1,9 @@ [ + 'actor_row_fields' => [ + 'user_id' => 42, + 'log_user_text' => 'Testing', + 'log_actor' => 24, + ], + UserIdentityValue::newRegistered( 42, 'Testing' ), + ]; + yield 'anon actor' => [ + 'actor_row_fields' => [ + 'log_user_text' => '127.0.0.1', + 'log_actor' => 24, + ], + UserIdentityValue::newAnonymous( '127.0.0.1' ), + ]; + yield 'unknown actor' => [ + 'actor_row_fields' => [], + new UserIdentityValue( 0, ActorStore::UNKNOWN_USER_NAME ), + ]; + } + + /** + * @dataProvider provideGetPerformerIdentity + * @covers DatabaseLogEntry::getPerformerIdentity + */ + public function testGetPerformer( array $actorRowFields, UserIdentity $expected ) { + $logEntry = DatabaseLogEntry::newFromRow( [ + 'log_id' => 1, + ] + $actorRowFields ); + $performer = $logEntry->getPerformerIdentity(); + $this->assertTrue( $expected->equals( $performer ) ); + } } diff --git a/tests/phpunit/includes/logging/LogFormatterTestCase.php b/tests/phpunit/includes/logging/LogFormatterTestCase.php index 8067202b01b..20a73d34d9c 100644 --- a/tests/phpunit/includes/logging/LogFormatterTestCase.php +++ b/tests/phpunit/includes/logging/LogFormatterTestCase.php @@ -56,9 +56,9 @@ abstract class LogFormatterTestCase extends MediaWikiLangTestCase { 'log_type' => $data['type'], 'log_action' => $data['action'], 'log_timestamp' => $data['timestamp'] ?? wfTimestampNow(), - 'log_user' => $data['user'] ?? 0, + 'log_user' => $data['user'] ?? 42, 'log_user_text' => $data['user_text'] ?? 'User', - 'log_actor' => $data['actor'] ?? 0, + 'log_actor' => $data['actor'] ?? 24, 'log_namespace' => $data['namespace'] ?? NS_MAIN, 'log_title' => $data['title'] ?? 'Main_Page', 'log_page' => $data['page'] ?? 0,