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

pager: Fix navigations when date range is set

Browse source 
The main idea here is to make ReverseChronologicalPager not mess up
mOffset which IndexPager uses, store the offset in another variable
and add separate query conditions.
Also, make RangeChronologicalPager more cooperative with its parent
class, avoid unnecessary inconsistency.

Bug: T228431
Change-Id: Icf76c946770aacee5b038522c066fca33ed1546f
This commit is contained in:
Func 2022-12-03 17:15:04 +08:00
parent 7e531cd553
commit 024a5d4d71
7 changed files with 128 additions and 142 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
includes/actions/pagers/HistoryPager.php
View file

@ -674,15 +674,6 @@ class HistoryPager extends ReverseChronologicalPager {
return parent::isNavigationBarShown();
}
/**
* @inheritDoc
*/
public function getDefaultQuery() {
parent::getDefaultQuery();
unset( $this->mDefaultQuery['date-range-to'] );
return $this->mDefaultQuery;
}
/**
* Set the "prevent clickjacking" flag; for example if a write operation
* if possible from the generated HTML.

78
includes/pager/RangeChronologicalPager.php
View file

@ -27,8 +27,8 @@ use Wikimedia\Timestamp\TimestampException;
*/
abstract class RangeChronologicalPager extends ReverseChronologicalPager {
/** @var string[] */
protected $rangeConds = [];
/** @var string */
protected $startOffset;
/**
* Set and return a date range condition using timestamps provided by the user.
@ -38,73 +38,46 @@ abstract class RangeChronologicalPager extends ReverseChronologicalPager {
*
* @stable to override
*
* @param string $startStamp Timestamp of the beginning of the date range (or empty)
* @param string $endStamp Timestamp of the end of the date range (or empty)
* @param string $startTime Timestamp of the beginning of the date range (or empty)
* @param string $endTime Timestamp of the end of the date range (or empty)
* @return array|null Database conditions to satisfy the specified date range
* or null if dates are invalid
*/
public function getDateRangeCond( $startStamp, $endStamp ) {
$this->rangeConds = [];
public function getDateRangeCond( $startTime, $endTime ) {
// Construct the conds array for compatibility with callers (if any)
$rangeConds = [];
// This is a chronological pager, so the first column should be some kind of timestamp
$timestampField = is_array( $this->mIndexField ) ? $this->mIndexField[0] : $this->mIndexField;
try {
// This is a chronological pager, so the first column should be some kind of timestamp
$timestampField = is_array( $this->mIndexField ) ? $this->mIndexField[0] : $this->mIndexField;
if ( $startStamp !== '' ) {
$startTimestamp = MWTimestamp::getInstance( $startStamp );
$startOffset = $this->mDb->timestamp( $startTimestamp->getTimestamp() );
$this->rangeConds[] = $timestampField . '>=' . $this->mDb->addQuotes( $startOffset );
if ( $startTime !== '' ) {
$startTimestamp = MWTimestamp::getInstance( $startTime );
$this->startOffset = $this->mDb->timestamp( $startTimestamp->getTimestamp() );
$rangeConds[] = $this->mDb->buildComparison( '>=', [ $timestampField => $this->startOffset ] );
}
if ( $endStamp !== '' ) {
$endTimestamp = MWTimestamp::getInstance( $endStamp );
$endOffset = $this->mDb->timestamp( $endTimestamp->getTimestamp() );
$this->rangeConds[] = $timestampField . '<=' . $this->mDb->addQuotes( $endOffset );
if ( $endTime !== '' ) {
$endTimestamp = MWTimestamp::getInstance( $endTime );
// Turned to use '<' for consistency with the parent class,
// add one second for compatibility with existing use cases
$endTimestamp->timestamp = $endTimestamp->timestamp->modify( '+1 second' );
$this->endOffset = $this->mDb->timestamp( $endTimestamp->getTimestamp() );
$rangeConds[] = $this->mDb->buildComparison( '<', [ $timestampField => $this->endOffset ] );
// populate existing variables for compatibility with parent
$this->mYear = (int)$endTimestamp->format( 'Y' );
$this->mMonth = (int)$endTimestamp->format( 'm' );
$this->mDay = (int)$endTimestamp->format( 'd' );
$this->mOffset = $endOffset;
}
} catch ( TimestampException $ex ) {
return null;
}
return $this->rangeConds;
return $rangeConds;
}
/**
* Takes ReverseChronologicalPager::getDateCond parameters and repurposes
* them to work with timestamp-based getDateRangeCond.
*
* @stable to override
*
* @param int $year Year up to which we want revisions
* @param int $month Month up to which we want revisions
* @param int $day [optional] Day up to which we want revisions. Default is end of month.
* @return string|null Timestamp or null if year and month are false/invalid
*/
public function getDateCond( $year, $month, $day = -1 ) {
// run through getDateRangeCond so rangeConds, mOffset, ... are set
$legacyTimestamp = self::getOffsetDate( $year, $month, $day );
// ReverseChronologicalPager uses strict inequality for the end date ('<'),
// but this class uses '<=' and expects extending classes to handle modifying the end date.
// Therefore, we need to subtract one second from the output of getOffsetDate to make it
// work with the '<=' inequality used in this class.
$legacyTimestamp->timestamp = $legacyTimestamp->timestamp->modify( '-1 second' );
$this->getDateRangeCond( '', $legacyTimestamp->getTimestamp( TS_MW ) );
return $this->mOffset;
}
/**
* Build variables to use by the database wrapper.
*
* @stable to override
*
* @param string $offset Index offset, inclusive
* @param int $limit Exact query limit
* @param bool $order IndexPager::QUERY_ASCENDING or IndexPager::QUERY_DESCENDING
* @return array
* @inheritDoc
*/
protected function buildQueryInfo( $offset, $limit, $order ) {
[ $tables, $fields, $conds, $fname, $options, $join_conds ] = parent::buildQueryInfo(
@ -112,9 +85,10 @@ abstract class RangeChronologicalPager extends ReverseChronologicalPager {
$limit,
$order
);
if ( $this->rangeConds ) {
$conds = array_merge( $conds, $this->rangeConds );
// End of the range has been added by ReverseChronologicalPager
if ( $this->startOffset ) {
$timestampField = is_array( $this->mIndexField ) ? $this->mIndexField[0] : $this->mIndexField;
$conds[] = $this->mDb->buildComparison( '>=', [ $timestampField => $this->startOffset ] );
}
return [ $tables, $fields, $conds, $fname, $options, $join_conds ];

30
includes/pager/ReverseChronologicalPager.php
View file

@ -38,6 +38,8 @@ abstract class ReverseChronologicalPager extends IndexPager {
public $mDay;
/** @var string */
private $lastHeaderDate;
/** @var string */
protected $endOffset;
/**
* @param string $date
@ -169,7 +171,7 @@ abstract class ReverseChronologicalPager extends IndexPager {
}
/**
* Set and return the mOffset timestamp such that we can get all revisions with
* Set and return the offset timestamp such that we can get all revisions with
* a timestamp up to the specified parameters.
*
* @stable to override
@ -185,7 +187,7 @@ abstract class ReverseChronologicalPager extends IndexPager {
$day = (int)$day;
// Basic validity checks for year and month
// If year and month are invalid, don't update the mOffset
// If year and month are invalid, don't update the offset
if ( $year <= 0 && ( $month <= 0 || $month >= 13 ) ) {
return null;
}
@ -200,17 +202,18 @@ abstract class ReverseChronologicalPager extends IndexPager {
$this->mYear = (int)$selectedDate->format( 'Y' );
$this->mMonth = (int)$selectedDate->format( 'm' );
$this->mDay = (int)$selectedDate->format( 'd' );
$this->mOffset = $this->mDb->timestamp( $timestamp->getTimestamp() );
// Don't mess with mOffset which IndexPager uses
$this->endOffset = $this->mDb->timestamp( $timestamp->getTimestamp() );
} catch ( TimestampException $e ) {
// Invalid user provided timestamp (T149257)
return null;
}
return $this->mOffset;
return $this->endOffset;
}
/**
* Core logic of determining the mOffset timestamp such that we can get all items with
* Core logic of determining the offset timestamp such that we can get all items with
* a timestamp up to the specified parameters. Given parameters for a day up to which to get
* items, this function finds the timestamp of the day just after the end of the range for use
* in an database strict inequality filter.
@ -286,4 +289,21 @@ abstract class ReverseChronologicalPager extends IndexPager {
return MWTimestamp::getInstance( "{$ymd}000000" );
}
/**
* @inheritDoc
*/
protected function buildQueryInfo( $offset, $limit, $order ) {
[ $tables, $fields, $conds, $fname, $options, $join_conds ] = parent::buildQueryInfo(
$offset,
$limit,
$order
);
if ( $this->endOffset ) {
$timestampField = is_array( $this->mIndexField ) ? $this->mIndexField[0] : $this->mIndexField;
$conds[] = $this->mDb->buildComparison( '<', [ $timestampField => $this->endOffset ] );
}
return [ $tables, $fields, $conds, $fname, $options, $join_conds ];
}
}

13
includes/specials/SpecialContributions.php
View file

@ -196,15 +196,10 @@ class SpecialContributions extends IncludableSpecialPage {
$this->opts['bot'] = '1';
}
$skip = $request->getText( 'offset' ) || $request->getText( 'dir' ) == 'prev';
# Offset overrides year/month selection
if ( !$skip ) {
$this->opts['year'] = $request->getIntOrNull( 'year' );
$this->opts['month'] = $request->getIntOrNull( 'month' );
$this->opts['start'] = $request->getVal( 'start' );
$this->opts['end'] = $request->getVal( 'end' );
}
$this->opts['year'] = $request->getIntOrNull( 'year' );
$this->opts['month'] = $request->getIntOrNull( 'month' );
$this->opts['start'] = $request->getVal( 'start' );
$this->opts['end'] = $request->getVal( 'end' );
$id = 0;
if ( ExternalUserNames::isExternal( $target ) ) {

6
includes/specials/SpecialLog.php
View file

@ -114,12 +114,6 @@ class SpecialLog extends SpecialPage {
}
}
# Don't let the user get stuck with a certain date
if ( $opts->getValue( 'offset' ) || $opts->getValue( 'dir' ) == 'prev' ) {
$opts->setValue( 'year', '' );
$opts->setValue( 'month', '' );
}
// If the user doesn't have the right permission to view the specific
// log type, throw a PermissionsError
// If the log type is invalid, just show all public logs

30
tests/phpunit/includes/pager/RangeChronologicalPagerTest.php
View file

@ -7,7 +7,7 @@
*
* @author Geoffrey Mon <geofbot@gmail.com>
*/
class RangeChronologicalPagerTest extends MediaWikiLangTestCase {
class RangeChronologicalPagerTest extends MediaWikiIntegrationTestCase {
/**
* @covers RangeChronologicalPager::getDateCond
@ -26,14 +26,14 @@ class RangeChronologicalPagerTest extends MediaWikiLangTestCase {
*/
public function getDateCondProvider() {
return [
[ 2016, 12, 5, '20161205235959' ],
[ 2016, 12, 31, '20161231235959' ],
[ 2016, 12, 1337, '20161231235959' ],
[ 2016, 1337, 1337, '20161231235959' ],
[ 2016, 1337, -1, '20161231235959' ],
[ 2016, 12, 32, '20161231235959' ],
[ 2016, 12, -1, '20161231235959' ],
[ 2016, -1, -1, '20161231235959' ],
[ 2016, 12, 5, '20161206000000' ],
[ 2016, 12, 31, '20170101000000' ],
[ 2016, 12, 1337, '20170101000000' ],
[ 2016, 1337, 1337, '20170101000000' ],
[ 2016, 1337, -1, '20170101000000' ],
[ 2016, 12, 32, '20170101000000' ],
[ 2016, 12, -1, '20170101000000' ],
[ 2016, -1, -1, '20170101000000' ],
];
}
@ -55,24 +55,24 @@ class RangeChronologicalPagerTest extends MediaWikiLangTestCase {
return [
[
'20161201000000',
'20161203000000',
'20161202235959',
[
'>=' . $db->addQuotes( $db->timestamp( '20161201000000' ) ),
'<=' . $db->addQuotes( $db->timestamp( '20161203000000' ) ),
$db->buildComparison( '>=', [ '' => $db->timestamp( '20161201000000' ) ] ),
$db->buildComparison( '<', [ '' => $db->timestamp( '20161203000000' ) ] ),
],
],
[
'',
'20161203000000',
'20161202235959',
[
'<=' . $db->addQuotes( $db->timestamp( '20161203000000' ) ),
$db->buildComparison( '<', [ '' => $db->timestamp( '20161203000000' ) ] ),
],
],
[
'20161201000000',
'',
[
'>=' . $db->addQuotes( $db->timestamp( '20161201000000' ) ),
$db->buildComparison( '>=', [ '' => $db->timestamp( '20161201000000' ) ] ),
],
],
[ '', '', [] ],

104
tests/phpunit/includes/pager/ReverseChronologicalPagerTest.php
View file

@ -1,5 +1,7 @@
<?php
use Wikimedia\TestingAccessWrapper;
/**
* Test class for ReverseChronologicalPagerTest methods.
*
@ -7,70 +9,80 @@
*
* @author Geoffrey Mon <geofbot@gmail.com>
*/
class ReverseChronologicalPagerTest extends MediaWikiLangTestCase {
class ReverseChronologicalPagerTest extends MediaWikiIntegrationTestCase {
/**
* @covers ReverseChronologicalPager::getDateCond
* @dataProvider provideGetDateCond
*/
public function testGetDateCond( $params, $expected ) {
$pager = $this->getMockForAbstractClass( ReverseChronologicalPager::class );
$pagerWrapper = TestingAccessWrapper::newFromObject( $pager );
$db = wfGetDB( DB_PRIMARY );
$pager->getDateCond( ...$params );
$this->assertEquals( $pagerWrapper->endOffset, $db->timestamp( $expected ) );
}
/**
* Data provider in description => [ [ param1, ... ], expected output ] format
*/
public function provideGetDateCond() {
yield 'Test year and month' => [
[ 2006, 6 ], '20060701000000'
];
yield 'Test year, month, and day' => [
[ 2006, 6, 5 ], '20060606000000'
];
yield 'Test month overflow into the next year' => [
[ 2006, 12 ], '20070101000000'
];
yield 'Test day overflow to the next month' => [
[ 2006, 6, 30 ], '20060701000000'
];
yield 'Test invalid month (should use end of year)' => [
[ 2006, -1 ], '20070101000000'
];
yield 'Test invalid day (should use end of month)' => [
[ 2006, 6, 1337 ], '20060701000000'
];
yield 'Test last day of year' => [
[ 2006, 12, 31 ], '20070101000000'
];
yield 'Test invalid day that overflows to next year' => [
[ 2006, 12, 32 ], '20070101000000'
];
yield '3-digit year, T287621' => [
[ 720, 1, 5 ], '07200106000000'
];
yield 'Y2K38 bug' => [
[ 2042, 1, 5 ], '20320101000000'
];
}
/**
* @covers ReverseChronologicalPager::getDateCond
*/
public function testGetDateCond() {
public function testGetDateCondSpecial() {
$pager = $this->getMockForAbstractClass( ReverseChronologicalPager::class );
$pagerWrapper = TestingAccessWrapper::newFromObject( $pager );
$timestamp = MWTimestamp::getInstance();
$db = wfGetDB( DB_PRIMARY );
$currYear = $timestamp->format( 'Y' );
$currMonth = $timestamp->format( 'n' );
// Test that getDateCond sets and returns mOffset
$this->assertEquals( $pager->getDateCond( 2006, 6 ), $pager->mOffset );
// Test year and month
$pager->getDateCond( 2006, 6 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20060701000000' ) );
// Test year, month, and day
$pager->getDateCond( 2006, 6, 5 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20060606000000' ) );
// Test month overflow into the next year
$pager->getDateCond( 2006, 12 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20070101000000' ) );
// Test day overflow to the next month
$pager->getDateCond( 2006, 6, 30 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20060701000000' ) );
// Test invalid month (should use end of year)
$pager->getDateCond( 2006, -1 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20070101000000' ) );
// Test invalid day (should use end of month)
$pager->getDateCond( 2006, 6, 1337 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20060701000000' ) );
// Test last day of year
$pager->getDateCond( 2006, 12, 31 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20070101000000' ) );
// Test invalid day that overflows to next year
$pager->getDateCond( 2006, 12, 32 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20070101000000' ) );
// Test that getDateCond sets and returns offset
$this->assertEquals( $pager->getDateCond( 2006, 6 ), $pagerWrapper->endOffset );
// Test month past current month (should use previous year)
if ( $currMonth < 5 ) {
$pager->getDateCond( -1, 5 );
$this->assertEquals( $pager->mOffset, $db->timestamp( $currYear - 1 . '0601000000' ) );
$this->assertEquals( $pagerWrapper->endOffset, $db->timestamp( $currYear - 1 . '0601000000' ) );
}
if ( $currMonth < 12 ) {
$pager->getDateCond( -1, 12 );
$this->assertEquals( $pager->mOffset, $db->timestamp( $currYear . '0101000000' ) );
$this->assertEquals( $pagerWrapper->endOffset, $db->timestamp( $currYear . '0101000000' ) );
}
// 3-digit year, T287621
$pager->getDateCond( 720, 1, 5 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '07200106000000' ) );
// Y2K38 bug
$pager->getDateCond( 2042, 1, 5 );
$this->assertEquals( $pager->mOffset, $db->timestamp( '20320101000000' ) );
}
}