Merge "Make IndexPager query direction code more readable"

diff --git a/includes/pager/IndexPager.php b/includes/pager/IndexPager.php
index b56ae37..64dbb22 100644 (file)
--- a/includes/pager/IndexPager.php
+++ b/includes/pager/IndexPager.php
@@ -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 {

diff --git a/includes/pager/RangeChronologicalPager.php b/includes/pager/RangeChronologicalPager.php
index 7cbcaa9..5caf1f4 100644 (file)
--- a/includes/pager/RangeChronologicalPager.php
+++ b/includes/pager/RangeChronologicalPager.php
@@ -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 ) {

diff --git a/includes/specials/pagers/ActiveUsersPager.php b/includes/specials/pagers/ActiveUsersPager.php
index 3e1a869..5dbdba0 100644 (file)
--- a/includes/specials/pagers/ActiveUsersPager.php
+++ b/includes/specials/pagers/ActiveUsersPager.php
@@ -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 ? '>=' : '>';

diff --git a/includes/specials/pagers/AllMessagesTablePager.php b/includes/specials/pagers/AllMessagesTablePager.php
index f045333..8120417 100644 (file)
--- a/includes/specials/pagers/AllMessagesTablePager.php
+++ b/includes/specials/pagers/AllMessagesTablePager.php
@@ -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();

diff --git a/includes/specials/pagers/ContribsPager.php b/includes/specials/pagers/ContribsPager.php
index 1220e86..382ba2f 100644 (file)
--- a/includes/specials/pagers/ContribsPager.php
+++ b/includes/specials/pagers/ContribsPager.php
@@ -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 );

diff --git a/includes/specials/pagers/DeletedContribsPager.php b/includes/specials/pagers/DeletedContribsPager.php
index fa0fbf3..56b799b 100644 (file)
--- a/includes/specials/pagers/DeletedContribsPager.php
+++ b/includes/specials/pagers/DeletedContribsPager.php
@@ -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 );

diff --git a/includes/specials/pagers/ImageListPager.php b/includes/specials/pagers/ImageListPager.php
index 1794362..486b0ec 100644 (file)
--- a/includes/specials/pagers/ImageListPager.php
+++ b/includes/specials/pagers/ImageListPager.php
@@ -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 );
        }
 
        /**