API: Use ApiBlockInfoTrait in ApiQueryUsers and AllUsers
authorBrad Jorsch <bjorsch@wikimedia.org>
Thu, 5 Sep 2019 18:24:28 +0000 (14:24 -0400)
committerMobrovac <mobrovac@wikimedia.org>
Wed, 18 Sep 2019 19:44:01 +0000 (19:44 +0000)
For efficient bulk querying, this means that
ApiQueryBase::showHiddenUsersAddBlockInfo() needs to return everything
needed by DatabaseBlock::newFromRow().

Since we're rewriting it anyway, we may as well also move
ApiQueryBase::showHiddenUsersAddBlockInfo() out into a trait of its own.

Bug: T232021
Change-Id: I9c5b17a232ecbfbffefc7e40608cf5684ce8a644

RELEASE-NOTES-1.34
autoload.php
includes/api/ApiQueryAllUsers.php
includes/api/ApiQueryBase.php
includes/api/ApiQueryBlockInfoTrait.php [new file with mode: 0644]
includes/api/ApiQueryUsers.php
tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php [new file with mode: 0644]

index 173a5da..af87e2b 100644 (file)
@@ -176,6 +176,9 @@ $wgPasswordPolicy['policies']['default']['PasswordNotInLargeBlacklist'] = false;
   set. Fields that are supported even if the file is missing include:
   'canonicaltitle', ''archivename' (deleted files only), 'descriptionurl',
   'descriptionshorturl'.
+* The 'blockexpiry' result property in list=users and list=allusers will now be
+  returned in the same format used by the rest of the API: ISO 8601 for
+  expiring blocks, and "infinite" for non-expiring blocks.
 
 === Action API internal changes in 1.34 ===
 * The exception thrown in ApiModuleManager::getModule has been changed
@@ -183,7 +186,7 @@ $wgPasswordPolicy['policies']['default']['PasswordNotInLargeBlacklist'] = false;
   ApiModuleManager::getModule now also throws InvalidArgumentExceptions when
   ObjectFactory is presented with an invalid spec or incorrectly constructed
   objects.
-* 
+* Added ApiQueryBlockInfoTrait.
 
 === Languages updated in 1.34 ===
 MediaWiki supports over 350 languages. Many localisations are updated regularly.
@@ -587,6 +590,8 @@ because of Phabricator reports.
 * Global variable $wgSysopEmailBans is deprecated; to allow sysops to ban
   users from sending emails, use
   $wgGroupPermissions['sysop']['blockemail'] = true;
+* ApiQueryBase::showHiddenUsersAddBlockInfo() is deprecated. Use
+  ApiQueryBlockInfoTrait instead.
 
 === Other changes in 1.34 ===
 * …
index 48d5b30..7ff29ce 100644 (file)
@@ -89,6 +89,7 @@ $wgAutoloadLocalClasses = [
        'ApiQueryBacklinks' => __DIR__ . '/includes/api/ApiQueryBacklinks.php',
        'ApiQueryBacklinksprop' => __DIR__ . '/includes/api/ApiQueryBacklinksprop.php',
        'ApiQueryBase' => __DIR__ . '/includes/api/ApiQueryBase.php',
+       'ApiQueryBlockInfoTrait' => __DIR__ . '/includes/api/ApiQueryBlockInfoTrait.php',
        'ApiQueryBlocks' => __DIR__ . '/includes/api/ApiQueryBlocks.php',
        'ApiQueryCategories' => __DIR__ . '/includes/api/ApiQueryCategories.php',
        'ApiQueryCategoryInfo' => __DIR__ . '/includes/api/ApiQueryCategoryInfo.php',
index a7d4fb9..0ea6af3 100644 (file)
  * @file
  */
 
+use MediaWiki\Block\DatabaseBlock;
+
 /**
  * Query module to enumerate all registered users.
  *
  * @ingroup API
  */
 class ApiQueryAllUsers extends ApiQueryBase {
+       use ApiQueryBlockInfoTrait;
+
        public function __construct( ApiQuery $query, $moduleName ) {
                parent::__construct( $query, $moduleName, 'au' );
        }
@@ -153,7 +157,7 @@ class ApiQueryAllUsers extends ApiQueryBase {
                        $this->addWhere( 'user_editcount > 0' );
                }
 
-               $this->showHiddenUsersAddBlockInfo( $fld_blockinfo );
+               $this->addBlockInfoToQuery( $fld_blockinfo );
 
                if ( $fld_groups || $fld_rights ) {
                        $this->addFields( [ 'groups' =>
@@ -263,13 +267,8 @@ class ApiQueryAllUsers extends ApiQueryBase {
                                );
                        }
 
-                       if ( $fld_blockinfo && !is_null( $row->ipb_by_text ) ) {
-                               $data['blockid'] = (int)$row->ipb_id;
-                               $data['blockedby'] = $row->ipb_by_text;
-                               $data['blockedbyid'] = (int)$row->ipb_by;
-                               $data['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $row->ipb_timestamp );
-                               $data['blockreason'] = $commentStore->getComment( 'ipb_reason', $row )->text;
-                               $data['blockexpiry'] = $row->ipb_expiry;
+                       if ( $fld_blockinfo && !is_null( $row->ipb_id ) ) {
+                               $data += $this->getBlockDetails( DatabaseBlock::newFromRow( $row ) );
                        }
                        if ( $row->ipb_deleted ) {
                                $data['hidden'] = true;
index 10db848..8d9cb48 100644 (file)
@@ -31,6 +31,7 @@ use Wikimedia\Rdbms\IResultWrapper;
  * @ingroup API
  */
 abstract class ApiQueryBase extends ApiBase {
+       use ApiQueryBlockInfoTrait;
 
        private $mQueryModule, $mDb, $tables, $where, $fields, $options, $join_conds;
 
@@ -424,47 +425,6 @@ abstract class ApiQueryBase extends ApiBase {
                return Hooks::run( 'ApiQueryBaseProcessRow', [ $this, $row, &$data, &$hookData ] );
        }
 
-       /**
-        * Filters hidden users (where the user doesn't have the right to view them)
-        * Also adds relevant block information
-        *
-        * @param bool $showBlockInfo
-        * @return void
-        */
-       public function showHiddenUsersAddBlockInfo( $showBlockInfo ) {
-               $db = $this->getDB();
-
-               $tables = [ 'ipblocks' ];
-               $fields = [ 'ipb_deleted' ];
-               $joinConds = [
-                       'blk' => [ 'LEFT JOIN', [
-                               'ipb_user=user_id',
-                               'ipb_expiry > ' . $db->addQuotes( $db->timestamp() ),
-                       ] ],
-               ];
-
-               if ( $showBlockInfo ) {
-                       $actorQuery = ActorMigration::newMigration()->getJoin( 'ipb_by' );
-                       $commentQuery = CommentStore::getStore()->getJoin( 'ipb_reason' );
-                       $tables += $actorQuery['tables'] + $commentQuery['tables'];
-                       $joinConds += $actorQuery['joins'] + $commentQuery['joins'];
-                       $fields = array_merge( $fields, [
-                               'ipb_id',
-                               'ipb_expiry',
-                               'ipb_timestamp'
-                       ], $actorQuery['fields'], $commentQuery['fields'] );
-               }
-
-               $this->addTables( [ 'blk' => $tables ] );
-               $this->addFields( $fields );
-               $this->addJoinConds( $joinConds );
-
-               // Don't show hidden names
-               if ( !$this->getPermissionManager()->userHasRight( $this->getUser(), 'hideuser' ) ) {
-                       $this->addWhere( 'ipb_deleted = 0 OR ipb_deleted IS NULL' );
-               }
-       }
-
        /** @} */
 
        /************************************************************************//**
@@ -610,4 +570,24 @@ abstract class ApiQueryBase extends ApiBase {
        }
 
        /** @} */
+
+       /************************************************************************//**
+        * @name   Deprecated methods
+        * @{
+        */
+
+       /**
+        * Filters hidden users (where the user doesn't have the right to view them)
+        * Also adds relevant block information
+        *
+        * @deprecated since 1.34, use ApiQueryBlockInfoTrait instead
+        * @param bool $showBlockInfo
+        * @return void
+        */
+       public function showHiddenUsersAddBlockInfo( $showBlockInfo ) {
+               wfDeprecated( __METHOD__, '1.34' );
+               return $this->addBlockInfoToQuery( $showBlockInfo );
+       }
+
+       /** @} */
 }
diff --git a/includes/api/ApiQueryBlockInfoTrait.php b/includes/api/ApiQueryBlockInfoTrait.php
new file mode 100644 (file)
index 0000000..a3be356
--- /dev/null
@@ -0,0 +1,94 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+use MediaWiki\Block\DatabaseBlock;
+use MediaWiki\Permissions\PermissionManager;
+
+/**
+ * @ingroup API
+ */
+trait ApiQueryBlockInfoTrait {
+       use ApiBlockInfoTrait;
+
+       /**
+        * Filters hidden users (where the user doesn't have the right to view them)
+        * Also adds relevant block information
+        *
+        * @param bool $showBlockInfo
+        * @return void
+        */
+       private function addBlockInfoToQuery( $showBlockInfo ) {
+               $db = $this->getDB();
+
+               if ( $showBlockInfo ) {
+                       $queryInfo = DatabaseBlock::getQueryInfo();
+               } else {
+                       $queryInfo = [
+                               'tables' => [ 'ipblocks' ],
+                               'fields' => [ 'ipb_deleted' ],
+                               'joins' => [],
+                       ];
+               }
+
+               $this->addTables( [ 'blk' => $queryInfo['tables'] ] );
+               $this->addFields( $queryInfo['fields'] );
+               $this->addJoinConds( $queryInfo['joins'] );
+               $this->addJoinConds( [
+                       'blk' => [ 'LEFT JOIN', [
+                               'ipb_user=user_id',
+                               'ipb_expiry > ' . $db->addQuotes( $db->timestamp() ),
+                       ] ],
+               ] );
+
+               // Don't show hidden names
+               if ( !$this->getPermissionManager()->userHasRight( $this->getUser(), 'hideuser' ) ) {
+                       $this->addWhere( 'ipb_deleted = 0 OR ipb_deleted IS NULL' );
+               }
+       }
+
+       /**
+        * @name Methods required from ApiQueryBase
+        * @{
+        */
+
+       /** @see ApiBase::getDB */
+       abstract protected function getDB();
+
+       /** @see ApiBase::getPermissionManager */
+       abstract protected function getPermissionManager(): PermissionManager;
+
+       /** @see IContextSource::getUser */
+       abstract public function getUser();
+
+       /** @see ApiQueryBase::addTables */
+       abstract protected function addTables( $tables, $alias = null );
+
+       /** @see ApiQueryBase::addFields */
+       abstract protected function addFields( $fields );
+
+       /** @see ApiQueryBase::addWhere */
+       abstract protected function addWhere( $conds );
+
+       /** @see ApiQueryBase::addJoinConds */
+       abstract protected function addJoinConds( $conds );
+
+       /**@}*/
+
+}
index ce51a67..0171a37 100644 (file)
  * @file
  */
 
+use MediaWiki\Block\DatabaseBlock;
+
 /**
  * Query module to get information about a list of users
  *
  * @ingroup API
  */
 class ApiQueryUsers extends ApiQueryBase {
+       use ApiQueryBlockInfoTrait;
 
        private $tokenFunctions, $prop;
 
@@ -150,7 +153,7 @@ class ApiQueryUsers extends ApiQueryBase {
                                $this->addWhereFld( 'user_id', $userids );
                        }
 
-                       $this->showHiddenUsersAddBlockInfo( isset( $this->prop['blockinfo'] ) );
+                       $this->addBlockInfoToQuery( isset( $this->prop['blockinfo'] ) );
 
                        $data = [];
                        $res = $this->select( __METHOD__ );
@@ -232,13 +235,7 @@ class ApiQueryUsers extends ApiQueryBase {
                                        $data[$key]['hidden'] = true;
                                }
                                if ( isset( $this->prop['blockinfo'] ) && !is_null( $row->ipb_by_text ) ) {
-                                       $data[$key]['blockid'] = (int)$row->ipb_id;
-                                       $data[$key]['blockedby'] = $row->ipb_by_text;
-                                       $data[$key]['blockedbyid'] = (int)$row->ipb_by;
-                                       $data[$key]['blockedtimestamp'] = wfTimestamp( TS_ISO_8601, $row->ipb_timestamp );
-                                       $data[$key]['blockreason'] = $commentStore->getComment( 'ipb_reason', $row )
-                                               ->text;
-                                       $data[$key]['blockexpiry'] = $row->ipb_expiry;
+                                       $data[$key] += $this->getBlockDetails( DatabaseBlock::newFromRow( $row ) );
                                }
 
                                if ( isset( $this->prop['emailable'] ) ) {
diff --git a/tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php b/tests/phpunit/includes/api/ApiQueryBlockInfoTraitTest.php
new file mode 100644 (file)
index 0000000..cded9dd
--- /dev/null
@@ -0,0 +1,77 @@
+<?php
+
+use MediaWiki\Block\DatabaseBlock;
+use MediaWiki\MediaWikiServices;
+use Wikimedia\TestingAccessWrapper;
+use Wikimedia\Timestamp\ConvertibleTimestamp;
+
+/**
+ * @covers ApiQueryBlockInfoTrait
+ */
+class ApiQueryBlockInfoTraitTest extends MediaWikiTestCase {
+
+       public function testUsesApiBlockInfoTrait() {
+               $this->assertTrue( method_exists( ApiQueryBlockInfoTrait::class, 'getBlockDetails' ),
+                       'ApiQueryBlockInfoTrait::getBlockDetails exists' );
+       }
+
+       /**
+        * @dataProvider provideAddBlockInfoToQuery
+        */
+       public function testAddBlockInfoToQuery( $args, $expect ) {
+               // Fake timestamp to show up in the queries
+               $reset = ConvertibleTimestamp::setFakeTime( '20190101000000' );
+
+               $data = [];
+
+               $mock = $this->getMockForTrait( ApiQueryBlockInfoTrait::class );
+               $mock->method( 'getDB' )->willReturn( wfGetDB( DB_REPLICA ) );
+               $mock->method( 'getPermissionManager' )
+                       ->willReturn( MediaWikiServices::getInstance()->getPermissionManager() );
+               $mock->method( 'getUser' )
+                       ->willReturn( $this->getMutableTestUser()->getUser() );
+               $mock->method( 'addTables' )->willReturnCallback( function ( $v ) use ( &$data ) {
+                       $data['tables'] = array_merge( $data['tables'] ?? [], (array)$v );
+               } );
+               $mock->method( 'addFields' )->willReturnCallback( function ( $v ) use ( &$data ) {
+                       $data['fields'] = array_merge( $data['fields'] ?? [], (array)$v );
+               } );
+               $mock->method( 'addWhere' )->willReturnCallback( function ( $v ) use ( &$data ) {
+                       $data['where'] = array_merge( $data['where'] ?? [], (array)$v );
+               } );
+               $mock->method( 'addJoinConds' )->willReturnCallback( function ( $v ) use ( &$data ) {
+                       $data['joins'] = array_merge( $data['joins'] ?? [], (array)$v );
+               } );
+
+               TestingAccessWrapper::newFromObject( $mock )->addBlockInfoToQuery( ...$args );
+               $this->assertEquals( $expect, $data );
+       }
+
+       public function provideAddBlockInfoToQuery() {
+               $queryInfo = DatabaseBlock::getQueryInfo();
+
+               $db = wfGetDB( DB_REPLICA );
+               $ts = $db->addQuotes( $db->timestamp( '20190101000000' ) );
+
+               return [
+                       [ [ false ], [
+                               'tables' => [ 'blk' => [ 'ipblocks' ] ],
+                               'fields' => [ 'ipb_deleted' ],
+                               'where' => [ 'ipb_deleted = 0 OR ipb_deleted IS NULL' ],
+                               'joins' => [
+                                       'blk' => [ 'LEFT JOIN', [ 'ipb_user=user_id', "ipb_expiry > $ts" ] ]
+                               ],
+                       ] ],
+
+                       [ [ true ], [
+                               'tables' => [ 'blk' => $queryInfo['tables'] ],
+                               'fields' => $queryInfo['fields'],
+                               'where' => [ 'ipb_deleted = 0 OR ipb_deleted IS NULL' ],
+                               'joins' => $queryInfo['joins'] + [
+                                       'blk' => [ 'LEFT JOIN', [ 'ipb_user=user_id', "ipb_expiry > $ts" ] ]
+                               ],
+                       ] ],
+               ];
+       }
+
+}