Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "Make IndexPager query direction code more readable"

Browse source
This commit is contained in:
jenkins-bot 2019-03-08 00:05:31 +00:00 committed by Gerrit Code Review
commit 9c5d5606fc
7 changed files with 63 additions and 44 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

54
includes/pager/IndexPager.php
View file

@ -67,14 +67,16 @@ use Wikimedia\Rdbms\IDatabase;
* @ingroup Pager
*/
abstract class IndexPager extends ContextSource implements Pager {
/**
* Constants for the $mDefaultDirection field.
*
* These are boolean for historical reasons and should stay boolean for backwards-compatibility.
*/
/** Backwards-compatible constant for $mDefaultDirection field (do not change) */
const DIR_ASCENDING = false;
/** Backwards-compatible constant for $mDefaultDirection field (do not change) */
const DIR_DESCENDING = true;
/** Backwards-compatible constant for reallyDoQuery() (do not change) */
const QUERY_ASCENDING = true;
/** Backwards-compatible constant for reallyDoQuery() (do not change) */
const QUERY_DESCENDING = false;
/** @var WebRequest */
public $mRequest;
/** @var int[] List of default entry limit options to be presented to clients */
@ -224,11 +226,13 @@ abstract class IndexPager extends ContextSource implements Pager {
public function doQuery() {
# Use the child class name for profiling
$fname = __METHOD__ . ' (' . static::class . ')';
/** @noinspection PhpUnusedLocalVariableInspection */
$section = Profiler::instance()->scopedProfileIn( $fname );
$descending = $this->mIsBackwards
? ( $this->mDefaultDirection === self::DIR_DESCENDING )
: ( $this->mDefaultDirection === self::DIR_ASCENDING );
$defaultOrder = ( $this->mDefaultDirection === self::DIR_ASCENDING )
? self::QUERY_ASCENDING
: self::QUERY_DESCENDING;
$order = $this->mIsBackwards ? self::oppositeOrder( $defaultOrder ) : $defaultOrder;
# Plus an extra row so that we can tell the "next" link should be shown
$queryLimit = $this->mLimit + 1;
@ -241,14 +245,15 @@ abstract class IndexPager extends ContextSource implements Pager {
// direction see if we get a row.
$oldIncludeOffset = $this->mIncludeOffset;
$this->mIncludeOffset = !$this->mIncludeOffset;
$isFirst = !$this->reallyDoQuery( $this->mOffset, 1, !$descending )->numRows();
$oppositeOrder = self::oppositeOrder( $order );
$isFirst = !$this->reallyDoQuery( $this->mOffset, 1, $oppositeOrder )->numRows();
$this->mIncludeOffset = $oldIncludeOffset;
}
$this->mResult = $this->reallyDoQuery(
$this->mOffset,
$queryLimit,
$descending
$order
);
$this->extractResultInfo( $isFirst, $queryLimit, $this->mResult );
@ -258,6 +263,16 @@ abstract class IndexPager extends ContextSource implements Pager {
$this->mResult->rewind(); // Paranoia
}
/**
* @param bool $order One of the IndexPager::QUERY_* class constants
* @return bool The opposite query order as an IndexPager::QUERY_ constant
*/
final protected static function oppositeOrder( $order ) {
return ( $order === self::QUERY_ASCENDING )
? self::QUERY_DESCENDING
: self::QUERY_ASCENDING;
}
/**
* @return IResultWrapper The result wrapper.
*/
@ -380,17 +395,18 @@ abstract class IndexPager extends ContextSource implements Pager {
}
/**
* Do a query with specified parameters, rather than using the object
* context
* Do a query with specified parameters, rather than using the object context
*
* @note For b/c, query direction is true for ascending and false for descending
*
* @param string $offset Index offset, inclusive
* @param int $limit Exact query limit
* @param bool $descending Query direction, false for ascending, true for descending
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return IResultWrapper
*/
public function reallyDoQuery( $offset, $limit, $descending ) {
public function reallyDoQuery( $offset, $limit, $order ) {
list( $tables, $fields, $conds, $fname, $options, $join_conds ) =
$this->buildQueryInfo( $offset, $limit, $descending );
$this->buildQueryInfo( $offset, $limit, $order );
return $this->mDb->select( $tables, $fields, $conds, $fname, $options, $join_conds );
}
@ -398,12 +414,14 @@ abstract class IndexPager extends ContextSource implements Pager {
/**
* Build variables to use by the database wrapper.
*
* @note For b/c, query direction is true for ascending and false for descending
*
* @param string $offset Index offset, inclusive
* @param int $limit Exact query limit
* @param bool $descending Query direction, false for ascending, true for descending
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return array
*/
protected function buildQueryInfo( $offset, $limit, $descending ) {
protected function buildQueryInfo( $offset, $limit, $order ) {
$fname = __METHOD__ . ' (' . $this->getSqlComment() . ')';
$info = $this->getQueryInfo();
$tables = $info['tables'];
@ -412,7 +430,7 @@ abstract class IndexPager extends ContextSource implements Pager {
$options = $info['options'] ?? [];
$join_conds = $info['join_conds'] ?? [];
$sortColumns = array_merge( [ $this->mIndexField ], $this->mExtraSortFields );
if ( $descending ) {
if ( $order === self::QUERY_ASCENDING ) {
$options['ORDER BY'] = $sortColumns;
$operator = $this->mIncludeOffset ? '>=' : '>';
} else {

6
includes/pager/RangeChronologicalPager.php
View file

@ -94,14 +94,14 @@ abstract class RangeChronologicalPager extends ReverseChronologicalPager {
*
* @param string $offset Index offset, inclusive
* @param int $limit Exact query limit
* @param bool $descending Query direction, false for ascending, true for descending
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return array
*/
protected function buildQueryInfo( $offset, $limit, $descending ) {
protected function buildQueryInfo( $offset, $limit, $order ) {
list( $tables, $fields, $conds, $fname, $options, $join_conds ) = parent::buildQueryInfo(
$offset,
$limit,
$descending
$order
);
if ( $this->rangeConds ) {

4
includes/specials/pagers/ActiveUsersPager.php
View file

@ -161,11 +161,11 @@ class ActiveUsersPager extends UsersPager {
];
}
protected function buildQueryInfo( $offset, $limit, $descending ) {
protected function buildQueryInfo( $offset, $limit, $order ) {
$fname = __METHOD__ . ' (' . $this->getSqlComment() . ')';
$sortColumns = array_merge( [ $this->mIndexField ], $this->mExtraSortFields );
if ( $descending ) {
if ( $order === self::QUERY_ASCENDING ) {
$dir = 'ASC';
$orderBy = $sortColumns;
$operator = $this->mIncludeOffset ? '>=' : '>';

11
includes/specials/pagers/AllMessagesTablePager.php
View file

@ -167,24 +167,25 @@ class AllMessagesTablePager extends TablePager {
}
/**
* This function normally does a database query to get the results; we need
* This function normally does a database query to get the results; we need
* to make a pretend result using a FakeResultWrapper.
* @param string $offset
* @param int $limit
* @param bool $descending
* @param bool $order
* @return FakeResultWrapper
*/
function reallyDoQuery( $offset, $limit, $descending ) {
function reallyDoQuery( $offset, $limit, $order ) {
$asc = ( $order === self::QUERY_ASCENDING );
$result = new FakeResultWrapper( [] );
$messageNames = $this->getAllMessages( $descending );
$messageNames = $this->getAllMessages( $order );
$statuses = self::getCustomisedStatuses( $messageNames, $this->langcode, $this->foreign );
$count = 0;
foreach ( $messageNames as $key ) {
$customised = isset( $statuses['pages'][$key] );
if ( $customised !== $this->custom &&
( $descending && ( $key < $offset || !$offset ) || !$descending && $key > $offset ) &&
( $asc && ( $key < $offset || !$offset ) || !$asc && $key > $offset ) &&
( ( $this->prefix && preg_match( $this->prefix, $key ) ) || $this->prefix === false )
) {
$actual = wfMessage( $key )->inLanguage( $this->lang )->plain();

10
includes/specials/pagers/ContribsPager.php
View file

@ -160,14 +160,14 @@ class ContribsPager extends RangeChronologicalPager {
*
* @param string $offset Index offset, inclusive
* @param int $limit Exact query limit
* @param bool $descending Query direction, false for ascending, true for descending
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return IResultWrapper
*/
function reallyDoQuery( $offset, $limit, $descending ) {
function reallyDoQuery( $offset, $limit, $order ) {
list( $tables, $fields, $conds, $fname, $options, $join_conds ) = $this->buildQueryInfo(
$offset,
$limit,
$descending
$order
);
/*
@ -193,7 +193,7 @@ class ContribsPager extends RangeChronologicalPager {
) ];
Hooks::run(
'ContribsPager::reallyDoQuery',
[ &$data, $this, $offset, $limit, $descending ]
[ &$data, $this, $offset, $limit, $order ]
);
$result = [];
@ -207,7 +207,7 @@ class ContribsPager extends RangeChronologicalPager {
}
// sort results
if ( $descending ) {
if ( $order === self::QUERY_ASCENDING ) {
ksort( $result );
} else {
krsort( $result );

10
includes/specials/pagers/DeletedContribsPager.php
View file

@ -115,17 +115,17 @@ class DeletedContribsPager extends IndexPager {
*
* @param string $offset Index offset, inclusive
* @param int $limit Exact query limit
* @param bool $descending Query direction, false for ascending, true for descending
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return IResultWrapper
*/
function reallyDoQuery( $offset, $limit, $descending ) {
$data = [ parent::reallyDoQuery( $offset, $limit, $descending ) ];
function reallyDoQuery( $offset, $limit, $order ) {
$data = [ parent::reallyDoQuery( $offset, $limit, $order ) ];
// This hook will allow extensions to add in additional queries, nearly
// identical to ContribsPager::reallyDoQuery.
Hooks::run(
'DeletedContribsPager::reallyDoQuery',
[ &$data, $this, $offset, $limit, $descending ]
[ &$data, $this, $offset, $limit, $order ]
);
$result = [];
@ -139,7 +139,7 @@ class DeletedContribsPager extends IndexPager {
}
// sort results
if ( $descending ) {
if ( $order === self::QUERY_ASCENDING ) {
ksort( $result );
} else {
krsort( $result );

12
includes/specials/pagers/ImageListPager.php
View file

@ -321,15 +321,15 @@ class ImageListPager extends TablePager {
* is descending, so I renamed it to $asc here.
* @param int $offset
* @param int $limit
* @param bool $asc
* @return array
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return FakeResultWrapper
* @throws MWException
*/
function reallyDoQuery( $offset, $limit, $asc ) {
function reallyDoQuery( $offset, $limit, $order ) {
$prevTableName = $this->mTableName;
$this->mTableName = 'image';
list( $tables, $fields, $conds, $fname, $options, $join_conds ) =
$this->buildQueryInfo( $offset, $limit, $asc );
$this->buildQueryInfo( $offset, $limit, $order );
$imageRes = $this->mDb->select( $tables, $fields, $conds, $fname, $options, $join_conds );
$this->mTableName = $prevTableName;
@ -347,13 +347,13 @@ class ImageListPager extends TablePager {
$this->mIndexField = 'oi_' . substr( $this->mIndexField, 4 );
list( $tables, $fields, $conds, $fname, $options, $join_conds ) =
$this->buildQueryInfo( $offset, $limit, $asc );
$this->buildQueryInfo( $offset, $limit, $order );
$oldimageRes = $this->mDb->select( $tables, $fields, $conds, $fname, $options, $join_conds );
$this->mTableName = $prevTableName;
$this->mIndexField = $oldIndex;
return $this->combineResult( $imageRes, $oldimageRes, $limit, $asc );
return $this->combineResult( $imageRes, $oldimageRes, $limit, $order );
}
/**