jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/393925 )
Change subject: Sort lists and entries by name and last updated timestamp
......................................................................
Sort lists and entries by name and last updated timestamp
Add sort options to the read methods:
* by name/updated for getAllLists, getListEntries and
getListsByDateUpdated
* getListEntriesByDateUpdated is always sorted by update time
(to do otherwise would require an extra index and there is
no use case for it)
* getListsByPage has no human-comprehensible sorting (results
are assumed to be small).
Ensure stable paging by using the primary key as secondary sortkey
everywhere.
Improve performance by aligning indexes with sort options
(and getting rid of the old offset-based paging):
* getAllLists, getListsByDateUpdated, getListEntries, and
purgeOldDeleted have a matching index (for both sort methods,
in the case of the first three);
* getListEntriesByDateUpdated has an almost matching index (both
tables have to be filtered for the deleted flag so no way to cram
everything in a single index).
* getListsByPage is an efficient select with rle_user_project_title
(use index to find all lists with that page, then skip where
the list or the page is deleted) which then has to be sorted in
a temporary table for the GROUP BY / ORDER BY. Items can appear
only once per list (deleted flag notwithstanding) and list count
is capped at 100 so this is a filesort on 100 rows max and the
number of scanned rows is the number of lists containing the page
(could be >100 due to deletions).
May have gone a bit overboard with the indexes but in previous
code review there was a preference for avoiding filesorts even
on smallish result sets and it takes this many to accomplish that.
Bug: T180402
Bug: T182053
Change-Id: Ia04a6c42ed7eca2a1c49d6ef109f0359db06ccd4
---
M extension.json
M i18n/en.json
M i18n/qqq.json
A sql/patches/03-add-sort-indexes.sql
M sql/readinglists.sql
M src/Api/ApiQueryReadingListEntries.php
M src/Api/ApiQueryReadingLists.php
A src/Api/ApiQueryTrait.php
M src/HookHandler.php
M src/ReadingListRepository.php
M tests/src/ReadingListRepositoryTest.php
11 files changed, 701 insertions(+), 287 deletions(-)
Approvals:
jenkins-bot: Verified
Mholloway: Looks good to me, approved
diff --git a/extension.json b/extension.json
index 1f5e374..84ceb09 100644
--- a/extension.json
+++ b/extension.json
@@ -17,6 +17,7 @@
"MediaWiki\\Extensions\\ReadingLists\\ReverseInterwikiLookup":
"src/ReverseInterwikiLookup.php",
"MediaWiki\\Extensions\\ReadingLists\\Api\\ApiQueryReadingListEntries":
"src/Api/ApiQueryReadingListEntries.php",
"MediaWiki\\Extensions\\ReadingLists\\Api\\ApiQueryReadingLists":
"src/Api/ApiQueryReadingLists.php",
+ "MediaWiki\\Extensions\\ReadingLists\\Api\\ApiQueryTrait":
"src/Api/ApiQueryTrait.php",
"MediaWiki\\Extensions\\ReadingLists\\Api\\ApiReadingLists":
"src/Api/ApiReadingLists.php",
"MediaWiki\\Extensions\\ReadingLists\\Api\\ApiReadingListsCreateEntry":
"src/Api/ApiReadingListsCreateEntry.php",
"MediaWiki\\Extensions\\ReadingLists\\Api\\ApiReadingListsCreate":
"src/Api/ApiReadingListsCreate.php",
diff --git a/i18n/en.json b/i18n/en.json
index 82e84ff..f3acfe8 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -24,11 +24,16 @@
"readinglists-db-error-no-such-project": "'$1' is not a recognized
project.",
"readinglists-apierror-project-title-param": "<var>$1project</var> and
<var>$1title</var> must be used together.",
"readinglists-apierror-too-old": "Timestamps passed to
<var>$1changedsince</var> cannot be older than <kbd>$2</kbd>.",
+ "readinglists-apierror-invalidsort-notbyname": "The <kbd>name</kbd>
sort option cannot be used together with <var>$1changedsince</var>.",
"apihelp-query+readinglists-summary": "List or filter the user's
reading lists and show metadata about them.",
"apihelp-query+readinglists-extended-description": "This module has
three modes of operation. With the <var>$1changedsince</var> parameter, it
returns all lists of the current user which have been changed since the given
date. (This is meant for device sync and, unlike the other modes, includes
deleted lists. Only changes to list metadata are considered, not changes to
list items.) With the <var>$1project</var> and <var>$1title</var> parameters,
it returns all lists that include that page. Without any of those parameters,
it returns all lists.",
"apihelp-query+readinglists-param-changedsince": "Show lists that have
been changed since this timestamp. Must be after <kbd>$1</kbd>.",
"apihelp-query+readinglists-param-project": "Project of the page to
filter on. Must be used together with <var>$1title</var>.",
"apihelp-query+readinglists-param-title": "Title of the page to filter
on. Must be used together with <var>$1project</var>.",
+ "apihelp-query+readinglists-param-sort": "Property to sort by. Ignored
when <var>$1project</var> and <var>$1title</var> is set (results are returned
in DB order). Defaults to <kbd>updated</kbd> when <var>$1changedsince</var> is
set, and to <kbd>name</kbd> otherwise.",
+ "apihelp-query+readinglists-paramvalue-sort-name": "List name. (Sorting
is by binary value; e.g. any uppercase ASCII character will sort before any
lowercase one.)",
+ "apihelp-query+readinglists-paramvalue-sort-updated": "Last update
timestamp. (Updates include list metadata changes but not changes to list
items.)",
+ "apihelp-query+readinglists-param-dir": "Sort direction:
<kbd>ascending</kbd> (A to Z, oldest to newest) or <kbd>descending</kbd>.
Ignored when <var>$1project</var> and <var>$1title</var> is set. ",
"apihelp-query+readinglists-param-limit": "Number of result items to
return.",
"apihelp-query+readinglists-example-1": "Get the reading lists of the
current user.",
"apihelp-query+readinglists-example-2": "Get the reading lists of the
current user which have changed since <kbd>2013-01-01T00:00:00Z</kbd>.",
@@ -37,6 +42,10 @@
"apihelp-query+readinglistentries-extended-description": "This module
has two modes of operation. With the <var>$1lists</var> parameter, it returns
the pages in the given list(s). With the <var>$1changedsince</var> parameter,
it returns all list entries from any list of the current user which have been
changed since the given date. (This is meant for device sync and, unlike the
other modes, includes deleted entries, although not entries of deleted lists.)",
"apihelp-query+readinglistentries-param-lists": "The list IDs for which
to return pages.",
"apihelp-query+readinglistentries-param-changedsince": "Show list
entries that have been changed since this timestamp. Must be after
<kbd>$1</kbd>.",
+ "apihelp-query+readinglistentries-param-sort": "Property to sort by.
<kbd>name</kbd> cannot be used together with <var>$1changedsince</var>.
Defaults to <kbd>updated</kbd> when <var>$1changedsince</var> is set, and to
<kbd>name</kbd> otherwise.",
+ "apihelp-query+readinglistentries-paramvalue-sort-name": "Article
title. (Project name is ignored. Sorting is by binary value; e.g. any uppercase
ASCII character will sort before any lowercase one.)",
+ "apihelp-query+readinglistentries-paramvalue-sort-updated": "Last
update timestamp.",
+ "apihelp-query+readinglistentries-param-dir": "Sort direction:
<kbd>ascending</kbd> (A to Z, oldest to newest) or <kbd>descending</kbd>.",
"apihelp-query+readinglistentries-param-limit": "Number of result items
to return.",
"apihelp-query+readinglistentries-example-1": "Get the pages from the
reading lists with ID <kbd>10</kbd>, <kbd>11</kbd> and <kbd>12</kbd>.",
"apihelp-query+readinglistentries-example-2": "Get the list entries of
the current user which have changed since <kbd>2013-01-01T00:00:00Z</kbd>.",
diff --git a/i18n/qqq.json b/i18n/qqq.json
index af4ad6a..b79480e 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -25,11 +25,16 @@
"readinglists-db-error-no-such-project": "Error message used when the
user tries to add a new list entry, but the project does not match any of the
known ones.\n\nParameters:\n* $1 - the project.",
"readinglists-apierror-project-title-param": "{{doc-apierror}}\n$1 is
the module prefix.",
"readinglists-apierror-too-old": "{{doc-apierror}}\n$1 is the module
prefix, $2 is the expiry date for deleted lists/entries.",
+ "readinglists-apierror-invalidsort-notbyname": "{{doc-apierror}}\n$1 is
the module prefix.",
"apihelp-query+readinglists-summary":
"{{doc-apihelp-summary|query+readinglists}}",
"apihelp-query+readinglists-extended-description":
"{{doc-apihelp-extended-description|query+readinglists}}",
"apihelp-query+readinglists-param-changedsince":
"{{doc-apihelp-param|query+readinglists|changedsince|paramstart=2|params=* $1 -
Oldest allowed value}}",
"apihelp-query+readinglists-param-project":
"{{doc-apihelp-param|query+readinglists|project}}",
"apihelp-query+readinglists-param-title":
"{{doc-apihelp-param|query+readinglists|title}}",
+ "apihelp-query+readinglists-param-sort":
"{{doc-apihelp-param|query+readinglists|sort}}",
+ "apihelp-query+readinglists-paramvalue-sort-name":
"{{doc-apihelp-paramvalue|query+readinglists|sort|name}}",
+ "apihelp-query+readinglists-paramvalue-sort-updated":
"{{doc-apihelp-paramvalue|query+readinglists|sort|updated}}",
+ "apihelp-query+readinglists-param-dir":
"{{doc-apihelp-param|query+readinglists|dir}}",
"apihelp-query+readinglists-param-limit":
"{{doc-apihelp-param|query+readinglists|limit}}",
"apihelp-query+readinglists-example-1":
"{{doc-apihelp-example|query+readinglists}}",
"apihelp-query+readinglists-example-2":
"{{doc-apihelp-example|query+readinglists}}",
@@ -38,6 +43,10 @@
"apihelp-query+readinglistentries-extended-description":
"{{doc-apihelp-extended-description|query+readinglistentries}}",
"apihelp-query+readinglistentries-param-lists":
"{{doc-apihelp-param|query+readinglistentries|lists}}",
"apihelp-query+readinglistentries-param-changedsince":
"{{doc-apihelp-param|query+readinglistentries|changedsince|paramstart=2|params=*
$1 - Oldest allowed value}}",
+ "apihelp-query+readinglistentries-param-sort":
"{{doc-apihelp-param|query+readinglistentries|sort}}",
+ "apihelp-query+readinglistentries-paramvalue-sort-name":
"{{doc-apihelp-paramvalue|query+readinglistentries|sort|name}}",
+ "apihelp-query+readinglistentries-paramvalue-sort-updated":
"{{doc-apihelp-paramvalue|query+readinglistentries|sort|updated}}",
+ "apihelp-query+readinglistentries-param-dir":
"{{doc-apihelp-param|query+readinglistentries|dir}}",
"apihelp-query+readinglistentries-param-limit":
"{{doc-apihelp-param|query+readinglistentries|limit}}",
"apihelp-query+readinglistentries-example-1":
"{{doc-apihelp-example|query+readinglistentries}}",
"apihelp-query+readinglistentries-example-2":
"{{doc-apihelp-example|query+readinglistentries}}",
diff --git a/sql/patches/03-add-sort-indexes.sql
b/sql/patches/03-add-sort-indexes.sql
new file mode 100644
index 0000000..e840caa
--- /dev/null
+++ b/sql/patches/03-add-sort-indexes.sql
@@ -0,0 +1,15 @@
+DROP INDEX /*i*/rl_user_updated ON /*_*/reading_list;
+DROP INDEX /*i*/rl_user_deleted ON /*_*/reading_list;
+DROP INDEX /*i*/rle_list_updated ON /*_*/reading_list_entry;
+DROP INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry;;
+
+CREATE INDEX /*i*/rl_user_default ON /*_*/reading_list (rl_user_id,
rl_is_default);
+CREATE UNIQUE INDEX /*i*/rl_user_deleted_name_id ON /*_*/reading_list
(rl_user_id, rl_deleted, rl_name, rl_id);
+CREATE UNIQUE INDEX /*i*/rl_user_deleted_updated_id ON /*_*/reading_list
(rl_user_id, rl_deleted, rl_date_updated, rl_id);
+CREATE INDEX /*i*/rl_deleted_updated ON /*_*/reading_list (rl_deleted,
rl_date_updated);
+
+CREATE UNIQUE INDEX /*i*/rle_list_deleted_title_id ON /*_*/reading_list_entry
(rle_rl_id, rle_deleted, rle_title, rle_id);
+CREATE UNIQUE INDEX /*i*/rle_list_deleted_updated_id ON
/*_*/reading_list_entry (rle_rl_id, rle_deleted, rle_date_updated, rle_id);
+CREATE UNIQUE INDEX /*i*/rle_user_updated_id ON /*_*/reading_list_entry
(rle_user_id, rle_date_updated, rle_id);
+CREATE UNIQUE INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry
(rle_user_id, rle_rlp_id, rle_title, rle_rl_id);
+CREATE INDEX /*i*/rle_deleted_updated ON /*_*/reading_list_entry (rle_deleted,
rle_date_updated);
diff --git a/sql/readinglists.sql b/sql/readinglists.sql
index 055c135..11ced2e 100644
--- a/sql/readinglists.sql
+++ b/sql/readinglists.sql
@@ -29,11 +29,21 @@
-- Lists will be hard-deleted eventually but kept around for a while for
sync.
rl_deleted TINYINT NOT NULL DEFAULT 0
) /*$wgDBTableOptions*/;
--- For syncing lists that changed since a given date.
-CREATE INDEX /*i*/rl_user_updated ON /*_*/reading_list (rl_user_id,
rl_date_updated);
--- For getting all non-deleted items.
-CREATE INDEX /*i*/rl_user_deleted ON /*_*/reading_list (rl_user_id,
rl_deleted);
--- TODO date_updated + deleted for cleanup?
+-- For isSetupForUser() which is called a lot and used for row locks. Will
only ever be used
+-- with rl_is_default = 1 so effectively a unique index.
+CREATE INDEX /*i*/rl_user_default ON /*_*/reading_list (rl_user_id,
rl_is_default);
+-- For querying lists of a user by name (and then id as a tiebreaker). Covers
getAllLists() with
+-- SORT_BY_NAME.
+CREATE UNIQUE INDEX /*i*/rl_user_deleted_name_id ON /*_*/reading_list
(rl_user_id, rl_deleted, rl_name, rl_id);
+-- For querying lists of a user by last updated timestamp (and then id as a
tiebreaker). Covers
+-- getAllLists() with SORT_BY_UPDATED.
+CREATE UNIQUE INDEX /*i*/rl_user_deleted_updated_id ON /*_*/reading_list
(rl_user_id, rl_deleted, rl_date_updated, rl_id);
+-- Like the previous two, except for getListsByDateUpdated() which does not
have rl_deleted=0.
+-- TODO this is the lazy option, the previous indexes could be made to work if
the sort matched the condition.
+CREATE UNIQUE INDEX /*i*/rl_user_name_id ON /*_*/reading_list (rl_user_id,
rl_name, rl_id);
+CREATE UNIQUE INDEX /*i*/rl_user_updated_id ON /*_*/reading_list (rl_user_id,
rl_deleted, rl_date_updated, rl_id);
+-- For getting all deleted items older than a given date. Covers
purgeOldDeleted().
+CREATE INDEX /*i*/rl_deleted_updated ON /*_*/reading_list (rl_deleted,
rl_date_updated);
-- List items.
CREATE TABLE /*_*/reading_list_entry (
@@ -56,12 +66,28 @@
-- Entries will be hard-deleted eventually but kept around for a while for
sync.
rle_deleted TINYINT NOT NULL DEFAULT 0
) /*$wgDBTableOptions*/;
--- For getting all entries in a list and for syncing list entries that changed
since a given date.
-CREATE INDEX /*i*/rle_list_updated ON /*_*/reading_list_entry (rle_rl_id,
rle_date_updated);
--- For getting all lists of a given user which contain a specified page.
-CREATE INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry
(rle_user_id, rle_rlp_id, rle_title);
-- For ensuring there are no duplicate pages on a single list.
+-- (What we actually need is "no duplicate non-deleted items" but there is no
way to turn that
+-- into an index condition so the software will work around it by repurposing
deleted items as needed.)
CREATE UNIQUE INDEX /*i*/rle_list_project_title ON /*_*/reading_list_entry
(rle_rl_id, rle_rlp_id, rle_title);
+-- For querying list entries in a given list by title (and then id as a
tiebreaker). Covers
+-- getListEntries() with SORT_BY_NAME.
+CREATE UNIQUE INDEX /*i*/rle_list_deleted_title_id ON /*_*/reading_list_entry
(rle_rl_id, rle_deleted, rle_title, rle_id);
+-- For querying list entries in a given list by last updated timestamp (and
then id as a tiebreaker).
+-- Covers getListEntries() with SORT_BY_UPDATED.
+CREATE UNIQUE INDEX /*i*/rle_list_deleted_updated_id ON
/*_*/reading_list_entry (rle_rl_id, rle_deleted, rle_date_updated, rle_id);
+-- For querying all list entries of a given user by last updated timestamp
(and then id as a
+-- tiebreaker). Covers getListEntriesByDateUpdated() (almost; the results
still have to be
+-- filtered on rl_deleted).
+CREATE UNIQUE INDEX /*i*/rle_user_updated_id ON /*_*/reading_list_entry
(rle_user_id, rle_date_updated, rle_id);
+-- For getting all lists of a given user which contain a specified page.
Covers getListsByPage().
+-- rle_rl_id is included to ensure consistent sorting within a fully covered
query (the assumption
+-- being that result sets for this query will typically be small enough that
we don't care about
+-- server-side sorting by name etc; if we do end up with an anomalously huge
resultset, performance
+-- is preferred over returning the results in order.)
+CREATE UNIQUE INDEX /*i*/rle_user_project_title ON /*_*/reading_list_entry
(rle_user_id, rle_rlp_id, rle_title, rle_rl_id);
+-- For getting all deleted items older than a given date. Covers
purgeOldDeleted().
+CREATE INDEX /*i*/rle_deleted_updated ON /*_*/reading_list_entry (rle_deleted,
rle_date_updated);
-- Table for storing projects (domains) efficiently.
CREATE TABLE /*_*/reading_list_project (
diff --git a/src/Api/ApiQueryReadingListEntries.php
b/src/Api/ApiQueryReadingListEntries.php
index eadac97..34aba86 100644
--- a/src/Api/ApiQueryReadingListEntries.php
+++ b/src/Api/ApiQueryReadingListEntries.php
@@ -17,21 +17,7 @@
class ApiQueryReadingListEntries extends ApiQueryGeneratorBase {
use ApiTrait;
-
- /**
- * Return all entries of the given list(s).
- * Intended for initial copy of data to a new device, or for devices
which have information
- * that's too outdated for normal sync. Might also be useful for
devices with limited storage
- * capacity, such as web clients.
- */
- const MODE_ALL = 'all';
-
- /**
- * Return list entries (from any list of the user) which have been
changed (or deleted) recently.
- * Intended for syncing updates to a device which has an older snapshot
of the data.
- * "Recently" is defined by the changedsince parameter.
- */
- const MODE_CHANGES = 'changes';
+ use ApiQueryTrait;
/** @var string API module prefix */
private static $prefix = 'rle';
@@ -74,13 +60,26 @@
$lists = $this->getParameter( 'lists' );
$changedSince = $this->getParameter( 'changedsince' );
+ $sort = $this->getParameter( 'sort' );
+ $dir = $this->getParameter( 'dir' );
$limit = $this->getParameter( 'limit' );
- $offset = $this->getParameter( 'continue' );
+ $continue = $this->getParameter( 'continue' );
- $mode = $changedSince !== null ? self::MODE_CHANGES :
self::MODE_ALL;
+ $mode = $changedSince !== null ? self::$MODE_CHANGES :
self::$MODE_ALL;
+ if ( $sort === null ) {
+ $sort = ( $mode === self::$MODE_CHANGES ) ? 'updated' :
'name';
+ }
+ if ( $mode === self::$MODE_CHANGES && $sort === 'name' ) {
+ // We don't have the right DB index for this. Wouldn't
make much sense anyways.
+ $errorMessage = $this->msg(
'readinglists-apierror-invalidsort-notbyname', static::$prefix );
+ $this->dieWithError( $errorMessage, 'invalidparammix' );
+ }
+ $sort = self::$sortParamMap[$sort];
+ $dir = self::$sortParamMap[$dir];
+ $continue = $this->decodeContinuationParameter( $continue,
$mode, $sort );
$this->requireOnlyOneParameter( $this->extractRequestParams(),
'lists', 'changedsince' );
- if ( $mode === self::MODE_CHANGES ) {
+ if ( $mode === self::$MODE_CHANGES ) {
$expiry = Utils::getDeletedExpiry();
if ( $changedSince < $expiry ) {
$errorMessage = $this->msg(
'readinglists-apierror-too-old', static::$prefix,
@@ -94,23 +93,29 @@
$result->addIndexedTagName( $path, 'entry' );
$repository = $this->getReadingListRepository( $this->getUser()
);
- if ( $mode === self::MODE_CHANGES ) {
- $res = $repository->getListEntriesByDateUpdated(
$changedSince, $limit + 1, $offset );
+ if ( $mode === self::$MODE_CHANGES ) {
+ $res = $repository->getListEntriesByDateUpdated(
$changedSince, $dir, $limit + 1, $continue );
} else {
- $res = $repository->getListEntries( $lists, $limit + 1,
$offset );
+ $res = $repository->getListEntries( $lists, $sort,
$dir, $limit + 1, $continue );
}
$titles = [];
- $resultOffset = 0;
$fits = true;
- foreach ( $res as $row ) {
- $isLastRow = ( $resultOffset === $res->numRows() - 1 );
+ foreach ( $res as $i => $row ) {
+ $item = $this->getResultItem( $row, $mode );
+ if ( $i > $limit ) {
+ // $i is 1-based so this means we reached the
extra row.
+ $this->setContinueEnumParameter( 'continue',
+ $this->encodeContinuationParameter(
$item, $mode, $sort ) );
+ break;
+ }
if ( $resultPageSet ) {
$titles[] = $this->getResultTitle( $row );
} else {
- $fits = $result->addValue( $path, null,
$this->getResultItem( $row, $mode ) );
+ $fits = $result->addValue( $path, null, $item );
}
- if ( !$fits || ++$resultOffset >= $limit && !$isLastRow
) {
- $this->setContinueEnumParameter( 'continue',
$offset + $resultOffset );
+ if ( !$fits ) {
+ $this->setContinueEnumParameter( 'continue',
+ $this->encodeContinuationParameter(
$item, $mode, $sort ) );
break;
}
}
@@ -134,19 +139,7 @@
self::PARAM_HELP_MSG => $this->msg(
'apihelp-query+readinglistentries-param-changedsince',
wfTimestamp( TS_ISO_8601,
Utils::getDeletedExpiry() ) ),
],
- 'limit' => [
- self::PARAM_DFLT => 10,
- self::PARAM_TYPE => 'limit',
- self::PARAM_MIN => 1,
- self::PARAM_MAX => self::LIMIT_BIG1,
- self::PARAM_MAX2 => self::LIMIT_BIG2,
- ],
- 'continue' => [
- self::PARAM_TYPE => 'integer',
- self::PARAM_DFLT => 0,
- self::PARAM_HELP_MSG =>
'api-help-param-continue',
- ],
- ];
+ ] + $this->getAllowedSortParams();
}
/**
@@ -204,7 +197,7 @@
'title' => $row->rle_title,
'created' => wfTimestamp( TS_ISO_8601,
$row->rle_date_created ),
'updated' => wfTimestamp( TS_ISO_8601,
$row->rle_date_updated ),
- ] + ( $mode === self::MODE_CHANGES ? [ 'deleted' =>
(bool)$row->rle_deleted ] : [] );
+ ] + ( $mode === self::$MODE_CHANGES ? [ 'deleted' =>
(bool)$row->rle_deleted ] : [] );
}
/**
diff --git a/src/Api/ApiQueryReadingLists.php b/src/Api/ApiQueryReadingLists.php
index c924489..cd024c8 100644
--- a/src/Api/ApiQueryReadingLists.php
+++ b/src/Api/ApiQueryReadingLists.php
@@ -13,28 +13,7 @@
class ApiQueryReadingLists extends ApiQueryBase {
use ApiTrait;
-
- /**
- * Return all lists.
- * Intended for initial copy of data to a new device, or for devices
which have information
- * that's too outdated for normal sync. Might also be useful for
devices with limited storage
- * capacity, such as web clients.
- */
- const MODE_ALL = 'all';
-
- /**
- * Return lists which have been changed (or deleted) recently.
- * Intended for syncing updates to a device which has an older snapshot
of the data.
- * "Recently" is defined by the changedsince parameter.
- */
- const MODE_CHANGES = 'changes';
-
- /**
- * Return lists which include a given page.
- * Intended for status indicators and such (e.g. showing a star on the
current page if it's
- * included in some list).
- */
- const MODE_PAGE = 'page';
+ use ApiQueryTrait;
/** @var string API module prefix */
private static $prefix = 'rl';
@@ -54,8 +33,10 @@
$changedSince = $this->getParameter( 'changedsince' );
$project = $this->getParameter( 'project' );
$title = $this->getParameter( 'title' );
+ $sort = $this->getParameter( 'sort' );
+ $dir = $this->getParameter( 'dir' );
$limit = $this->getParameter( 'limit' );
- $offset = $this->getParameter( 'continue' );
+ $continue = $this->getParameter( 'continue' );
$path = [ 'query', $this->getModuleName() ];
$result = $this->getResult();
@@ -65,7 +46,7 @@
$mode = null;
$this->requireMaxOneParameter(
$this->extractRequestParams(), 'title', 'changedsince' );
if ( $project !== null && $title !== null ) {
- $mode = self::MODE_PAGE;
+ $mode = self::$MODE_PAGE;
} elseif ( $project !== null || $title !== null ) {
$errorMessage = $this->msg(
'readinglists-apierror-project-title-param', static::$prefix );
$this->dieWithError( $errorMessage,
'missingparam' );
@@ -76,25 +57,37 @@
wfTimestamp( TS_ISO_8601,
$expiry ) );
$this->dieWithError( $errorMessage );
}
- $mode = self::MODE_CHANGES;
+ $mode = self::$MODE_CHANGES;
} else {
- $mode = self::MODE_ALL;
+ $mode = self::$MODE_ALL;
}
- if ( $mode === self::MODE_PAGE ) {
- $res = $repository->getListsByPage( $project,
$title, $limit + 1, $offset );
- } elseif ( $mode === self::MODE_CHANGES ) {
- $res = $repository->getListsByDateUpdated(
$changedSince, $limit + 1, $offset );
- } else {
- $res = $repository->getAllLists( $limit + 1,
$offset );
+ if ( $sort === null ) {
+ $sort = ( $mode === self::$MODE_CHANGES ) ?
'updated' : 'name';
}
- $resultOffset = 0;
- foreach ( $res as $row ) {
- $isLastRow = ( $resultOffset ===
$res->numRows() - 1 );
+ $sort = self::$sortParamMap[$sort];
+ $dir = self::$sortParamMap[$dir];
+ $continue = $this->decodeContinuationParameter(
$continue, $mode, $sort );
+
+ if ( $mode === self::$MODE_PAGE ) {
+ $res = $repository->getListsByPage( $project,
$title, $limit + 1, $continue );
+ } elseif ( $mode === self::$MODE_CHANGES ) {
+ $res = $repository->getListsByDateUpdated(
$changedSince, $sort, $dir, $limit + 1, $continue );
+ } else {
+ $res = $repository->getAllLists( $sort, $dir,
$limit + 1, $continue );
+ }
+ foreach ( $res as $i => $row ) {
$item = $this->getResultItem( $row, $mode );
+ if ( $i > $limit ) {
+ // $i is 1-based so this means we
reached the extra row.
+ $this->setContinueEnumParameter(
'continue',
+
$this->encodeContinuationParameter( $item, $mode, $sort ) );
+ break;
+ }
$fits = $result->addValue( $path, null, $item );
- if ( !$fits || ++$resultOffset >= $limit &&
!$isLastRow ) {
- $this->setContinueEnumParameter(
'continue', $offset + $resultOffset );
+ if ( !$fits ) {
+ $this->setContinueEnumParameter(
'continue',
+
$this->encodeContinuationParameter( $item, $mode, $sort ) );
break;
}
}
@@ -120,19 +113,7 @@
self::PARAM_HELP_MSG => $this->msg(
'apihelp-query+readinglists-param-changedsince',
wfTimestamp( TS_ISO_8601,
Utils::getDeletedExpiry() ) ),
],
- 'limit' => [
- self::PARAM_DFLT => 10,
- self::PARAM_TYPE => 'limit',
- self::PARAM_MIN => 1,
- self::PARAM_MAX => self::LIMIT_BIG1,
- self::PARAM_MAX2 => self::LIMIT_BIG2,
- ],
- 'continue' => [
- self::PARAM_TYPE => 'integer',
- self::PARAM_DFLT => 0,
- self::PARAM_HELP_MSG =>
'api-help-param-continue',
- ],
- ];
+ ] + $this->getAllowedSortParams();
}
/**
@@ -188,7 +169,7 @@
'created' => wfTimestamp( TS_ISO_8601,
$row->rl_date_created ),
'updated' => wfTimestamp( TS_ISO_8601,
$row->rl_date_updated ),
];
- if ( $mode === self::MODE_CHANGES ) {
+ if ( $mode === self::$MODE_CHANGES ) {
$item['deleted'] = (bool)$row->rl_deleted;
}
return $item;
diff --git a/src/Api/ApiQueryTrait.php b/src/Api/ApiQueryTrait.php
new file mode 100644
index 0000000..f090fa0
--- /dev/null
+++ b/src/Api/ApiQueryTrait.php
@@ -0,0 +1,135 @@
+<?php
+
+namespace MediaWiki\Extensions\ReadingLists\Api;
+
+use ApiUsageException;
+use MediaWiki\Extensions\ReadingLists\ReadingListRepository;
+
+/**
+ * Shared sorting / paging for the query APIs.
+ */
+trait ApiQueryTrait {
+
+ // Mode constants, to support different sorting / paging / deleted item
behavior for different
+ // parameter combinations. For no particular reason, PHP does not allow
constants in traits,
+ // so we'll use statics instead.
+
+ /**
+ * Return all lists, or all entries of the specified list(s).
+ * Intended for initial copy of data to a new device, or for devices
which have information
+ * that's too outdated for normal sync. Might also be useful for
devices with limited storage
+ * capacity, such as web clients.
+ */
+ private static $MODE_ALL = 'all';
+
+ /**
+ * Return lists/entries which have been changed (or deleted) recently.
+ * Intended for syncing updates to a device which has an older snapshot
of the data.
+ * "Recently" is defined by the changedsince parameter.
+ */
+ private static $MODE_CHANGES = 'changes';
+
+ /**
+ * Return lists which include a given page.
+ * Intended for status indicators and such (e.g. showing a star on the
current page if it's
+ * included in some list).
+ */
+ private static $MODE_PAGE = 'page';
+
+ /** @var string[] Map of sort keywords used by the API to sort keywords
used by the repo. */
+ private static $sortParamMap = [
+ 'name' => ReadingListRepository::SORT_BY_NAME,
+ 'updated' => ReadingListRepository::SORT_BY_UPDATED,
+ 'ascending' => ReadingListRepository::SORT_DIR_ASC,
+ 'descending' => ReadingListRepository::SORT_DIR_DESC,
+ ];
+
+ /**
+ * Extract continuation data from item position and serialize it into a
string.
+ * @param array $item Result item to continue from.
+ * @param string $mode One of the MODE_* constants.
+ * @param string $sort One of the SORT_BY_* constants.
+ * @return string
+ */
+ private function encodeContinuationParameter( array $item, $mode, $sort
) {
+ if ( $mode === self::$MODE_PAGE ) {
+ return $item['id'];
+ } elseif ( $sort === ReadingListRepository::SORT_BY_NAME ) {
+ if ( self::$prefix === 'rl' ) {
+ $name = $item['name'];
+ } else {
+ $name = $item['title'];
+ }
+ return $name . '|' . $item['id'];
+ } else {
+ return $item['updated'] . '|' . $item['id'];
+ }
+ }
+
+ /**
+ * Recover continuation data after it has been roundtripped to the
client.
+ * @param string|null $continue Continuation parameter returned by the
client.
+ * @param string $mode One of the MODE_* constants.
+ * @param string $sort One of the SORT_BY_* constants.
+ * @return null|int|string[]
+ * - null if there was no continuation parameter;
+ * - [ rl(e)_name, rl(e)_id ] for MODE_ALL/MODE_CHANGES when sorting
by name;
+ * - [ rl(e)_date_updated, rl(e)_id ] for MODE_ALL/MODE_CHANGES when
sorting by updated time;
+ * - rle_id for MODE_PAGE.
+ * @throws ApiUsageException
+ */
+ private function decodeContinuationParameter( $continue, $mode, $sort )
{
+ if ( $continue === null ) {
+ return null;
+ }
+
+ if ( $mode === self::$MODE_PAGE ) {
+ $this->dieContinueUsageIf( $continue !==
(string)(int)$continue );
+ return (int)$continue;
+ } else {
+ // Continue token format is '<name|timestamp>|<id>';
name can contain '|'.
+ $separatorPosition = strrpos( $continue, '|' );
+ $this->dieContinueUsageIf( $separatorPosition === false
);
+ $continue = [
+ substr( $continue, 0, $separatorPosition ),
+ substr( $continue, $separatorPosition + 1 ),
+ ];
+ $this->dieContinueUsageIf( $continue[1] !==
(string)(int)$continue[1] );
+ $continue[1] = (int)$continue[1];
+ if ( $sort === ReadingListRepository::SORT_BY_UPDATED )
{
+ $this->dieContinueUsageIf( wfTimestamp( TS_MW,
$continue[0] ) === false );
+ }
+ return $continue;
+ }
+ }
+
+ /**
+ * Get common sorting/paging related params for getAllowedParams().
+ * @return array
+ */
+ private function getAllowedSortParams() {
+ return [
+ 'sort' => [
+ self::PARAM_TYPE => [ 'name', 'updated' ],
+ self::PARAM_HELP_MSG_PER_VALUE => [],
+ ],
+ 'dir' => [
+ self::PARAM_DFLT => 'ascending',
+ self::PARAM_TYPE => [ 'ascending', 'descending'
],
+ ],
+ 'limit' => [
+ self::PARAM_DFLT => 10,
+ self::PARAM_TYPE => 'limit',
+ self::PARAM_MIN => 1,
+ self::PARAM_MAX => self::LIMIT_BIG1,
+ self::PARAM_MAX2 => self::LIMIT_BIG2,
+ ],
+ 'continue' => [
+ self::PARAM_TYPE => 'string',
+ self::PARAM_DFLT => null,
+ self::PARAM_HELP_MSG =>
'api-help-param-continue',
+ ],
+ ];
+ }
+
+}
diff --git a/src/HookHandler.php b/src/HookHandler.php
index f8333b8..c73d078 100644
--- a/src/HookHandler.php
+++ b/src/HookHandler.php
@@ -47,6 +47,8 @@
$updater->dropExtensionTable( 'reading_list_sortkey',
"$patchDir/01-drop-sortkeys.sql" );
$updater->addExtensionTable( 'reading_list_project',
"$patchDir/02-add-reading_list_project.sql" );
+ $updater->addExtensionIndex( 'reading_list',
'rl_user_deleted_name_id',
+ "$patchDir/03-add-sort-indexes.sql" );
}
return true;
}
diff --git a/src/ReadingListRepository.php b/src/ReadingListRepository.php
index ed4351e..dda6e72 100644
--- a/src/ReadingListRepository.php
+++ b/src/ReadingListRepository.php
@@ -33,6 +33,15 @@
*/
class ReadingListRepository implements IDBAccessObject, LoggerAwareInterface {
+ /** Sort lists / entries alphabetically by name / title. */
+ const SORT_BY_NAME = 'name';
+ /** Sort lists / entries chronologically by last updated timestamp. */
+ const SORT_BY_UPDATED = 'updated';
+ /** Sort ascendingly (first letter / oldest date first). */
+ const SORT_DIR_ASC = 'asc';
+ /** Sort descendingly (last letter / newest date first). */
+ const SORT_DIR_DESC = 'desc';
+
/** @var array Database field lengths in bytes (only for the string
types). */
public static $fieldLength = [
'rl_name' => 255,
@@ -234,27 +243,28 @@
/**
* Get all lists of the user.
+ * @param string $sortBy One of the SORT_BY_* constants.
+ * @param string $sortDir One of the SORT_DIR_* constants.
* @param int $limit
- * @param int $offset
- * @return IResultWrapper<ReadingListRow>
+ * @param array|null $from DB position to continue from (or null to
start at the beginning/end).
+ * When sorting by name, this should be the name and id of a list;
when sorting by update time,
+ * the updated timestamp (in some form accepted by MWTimestamp) and
the id.
+ * @return IResultWrapper <ReadingListRow>
* @throws ReadingListRepositoryException
*/
- public function getAllLists( $limit = 1000, $offset = 0 ) {
+ public function getAllLists( $sortBy, $sortDir, $limit = 1000, array
$from = null ) {
$this->assertUser();
+ list( $conditions, $options ) = $this->processSort( 'rl',
$sortBy, $sortDir, $limit, $from );
$res = $this->dbr->select(
'reading_list',
$this->getListFields(),
- [
+ array_merge( [
'rl_user_id' => $this->userId,
'rl_deleted' => 0,
- ],
+ ], $conditions ),
__METHOD__,
- [
- 'LIMIT' => $limit,
- 'OFFSET' => $offset,
- 'ORDER BY' => 'rl_id',
- ]
+ $options
);
if (
@@ -431,16 +441,23 @@
/**
* Get the entries of one or more lists.
* @param array $ids List ids
+ * @param string $sortBy One of the SORT_BY_* constants.
+ * @param string $sortDir One of the SORT_DIR_* constants.
* @param int $limit
- * @param int $offset
+ * @param array|null $from DB position to continue from (or null to
start at the beginning/end).
+ * When sorting by name, this should be the name and id of a list;
when sorting by update time,
+ * the updated timestamp (in some form accepted by MWTimestamp) and
the id.
* @return IResultWrapper<ReadingListEntryRow>
* @throws ReadingListRepositoryException
*/
- public function getListEntries( array $ids, $limit = 1000, $offset = 0
) {
+ public function getListEntries(
+ array $ids, $sortBy, $sortDir, $limit = 1000, array $from = null
+ ) {
$this->assertUser();
if ( !$ids ) {
throw new ReadingListRepositoryException(
'readinglists-db-error-empty-list-ids' );
}
+ list( $conditions, $options ) = $this->processSort( 'rle',
$sortBy, $sortDir, $limit, $from );
// sanity check for nice error messages
$res = $this->dbr->select(
@@ -469,18 +486,14 @@
$res = $this->dbr->select(
[ 'reading_list_entry', 'reading_list_project' ],
$this->getListEntryFields(),
- [
+ array_merge( [
'rle_rlp_id = rlp_id',
'rle_rl_id' => $ids,
'rle_user_id' => $this->userId,
'rle_deleted' => 0,
- ],
+ ], $conditions ),
__METHOD__,
- [
- 'LIMIT' => $limit,
- 'OFFSET' => $offset,
- 'ORDER BY' => [ 'rle_rl_id', 'rle_id' ],
- ]
+ $options
);
return $res;
@@ -542,26 +555,29 @@
* Unlike other methods this returns deleted lists as well. Only
changes to list metadata
* (including deletion) are considered, not changes to list entries.
* @param string $date The cutoff date in TS_MW format
+ * @param string $sortBy One of the SORT_BY_* constants.
+ * @param string $sortDir One of the SORT_DIR_* constants.
* @param int $limit
- * @param int $offset
+ * @param array|null $from DB position to continue from (or null to
start at the beginning/end).
+ * When sorting by name, this should be the name and id of a list;
when sorting by update time,
+ * the updated timestamp (in some form accepted by MWTimestamp) and
the id.
* @throws ReadingListRepositoryException
* @return IResultWrapper<ReadingListRow>
*/
- public function getListsByDateUpdated( $date, $limit = 1000, $offset =
0 ) {
+ public function getListsByDateUpdated( $date, $sortBy =
self::SORT_BY_UPDATED,
+ $sortDir = self::SORT_DIR_ASC, $limit = 1000, array $from = null
+ ) {
$this->assertUser();
+ list( $conditions, $options ) = $this->processSort( 'rl',
$sortBy, $sortDir, $limit, $from );
$res = $this->dbr->select(
'reading_list',
$this->getListFields(),
- [
+ array_merge( [
'rl_user_id' => $this->userId,
'rl_date_updated > ' . $this->dbr->addQuotes(
$this->dbr->timestamp( $date ) ),
- ],
+ ], $conditions ),
__METHOD__,
- [
- 'LIMIT' => $limit,
- 'OFFSET' => $offset,
- 'ORDER BY' => 'rl_id',
- ]
+ $options
);
if (
@@ -579,29 +595,32 @@
* Unlike other methods this returns deleted entries as well (but not
entries inside deleted
* lists).
* @param string $date The cutoff date in TS_MW format
+ * @param string $sortDir One of the SORT_DIR_* constants.
* @param int $limit
- * @param int $offset
+ * @param array|null $from DB position to continue from (or null to
start at the beginning/end).
+ * Should contain the updated timestamp (in some form accepted by
MWTimestamp) and the id.
* @throws ReadingListRepositoryException
* @return IResultWrapper<ReadingListEntryRow>
*/
- public function getListEntriesByDateUpdated( $date, $limit = 1000,
$offset = 0 ) {
+ public function getListEntriesByDateUpdated(
+ $date, $sortDir = self::SORT_DIR_ASC, $limit = 1000, array
$from = null
+ ) {
$this->assertUser();
+ // Always sort by last updated; there is no supporting index
for sorting by name.
+ list( $conditions, $options ) = $this->processSort( 'rle',
self::SORT_BY_UPDATED,
+ $sortDir, $limit, $from );
$res = $this->dbr->select(
[ 'reading_list', 'reading_list_entry',
'reading_list_project' ],
$this->getListEntryFields(),
- [
+ array_merge( [
'rl_id = rle_rl_id',
'rle_rlp_id = rlp_id',
- 'rl_user_id' => $this->userId,
+ 'rle_user_id' => $this->userId,
'rl_deleted' => 0,
'rle_date_updated > ' . $this->dbr->addQuotes(
$this->dbr->timestamp( $date ) ),
- ],
+ ], $conditions ),
__METHOD__,
- [
- 'LIMIT' => $limit,
- 'OFFSET' => $offset,
- 'ORDER BY' => [ 'rle_rl_id', 'rle_id' ],
- ]
+ $options
);
return $res;
}
@@ -671,34 +690,39 @@
* @param string $project Project identifier (typically a domain name)
* @param string $title Page title (in localized prefixed DBkey format)
* @param int $limit
- * @param int $offset
+ * @param int|null $from List ID to continue from (or null to start at
the beginning/end).
+ *
* @throws ReadingListRepositoryException
* @return IResultWrapper<ReadingListRow>
*/
- public function getListsByPage( $project, $title, $limit = 1000,
$offset = 0 ) {
+ public function getListsByPage( $project, $title, $limit = 1000, $from
= null ) {
$this->assertUser();
$projectId = $this->getProjectId( $project );
if ( !$projectId ) {
return new FakeResultWrapper( [] );
}
+ $conditions = [
+ 'rl_id = rle_rl_id',
+ 'rle_user_id' => $this->userId,
+ 'rle_rlp_id' => $projectId,
+ 'rle_title' => $title,
+ 'rl_deleted' => 0,
+ 'rle_deleted' => 0,
+ ];
+ if ( $from !== null ) {
+ $conditions[] = 'rle_rl_id >= ' . (int)$from;
+ }
$res = $this->dbr->select(
[ 'reading_list', 'reading_list_entry' ],
$this->getListFields(),
- [
- 'rl_id = rle_rl_id',
- 'rl_user_id' => $this->userId,
- 'rle_rlp_id' => $projectId,
- 'rle_title' => $title,
- 'rl_deleted' => 0,
- 'rle_deleted' => 0,
- ],
+ $conditions,
__METHOD__,
[
- 'LIMIT' => $limit,
- 'OFFSET' => $offset,
'GROUP BY' => $this->getListFields(),
- 'ORDER BY' => 'rl_id',
+ 'ORDER BY' => 'rle_rl_id ASC',
+ 'LIMIT' => (int)$limit,
+
]
);
@@ -778,6 +802,63 @@
[ $field, self::$fieldLength[$field] ] );
}
}
+
+ /**
+ * Validate sort paramters.
+ * @param string $tablePrefix 'rl' or 'rle', depending on whether we
are sorting lists or entries.
+ * @param string $sortBy
+ * @param string $sortDir
+ * @param int $limit
+ * @param array|null $from
+ * @return array [ conditions, options ] Merge these into the
corresponding IDatabase::select
+ * parameters.
+ */
+ private function processSort( $tablePrefix, $sortBy, $sortDir, $limit,
$from ) {
+ if ( !in_array( $sortBy, [ self::SORT_BY_NAME,
self::SORT_BY_UPDATED ], true ) ) {
+ throw new LogicException( 'Invalid $sortBy parameter: '
. $sortBy );
+ }
+ if ( !in_array( $sortDir, [ self::SORT_DIR_ASC,
self::SORT_DIR_DESC ], true ) ) {
+ throw new LogicException( 'Invalid $sortDir parameter:
' . $sortDir );
+ }
+ if ( is_array( $from ) ) {
+ if ( count( $from ) !== 2 || !is_string( $from[0] ) ||
!is_numeric( $from[1] ) ) {
+ throw new LogicException( 'Invalid $from
parameter' );
+ }
+ } elseif ( $from !== null ) {
+ throw new LogicException( 'Invalid $from parameter
type: ' . gettype( $from ) );
+ }
+
+ if ( $tablePrefix === 'rl' ) {
+ $mainField = ( $sortBy === self::SORT_BY_NAME ) ?
'rl_name' : 'rl_date_updated';
+ } else {
+ $mainField = ( $sortBy === self::SORT_BY_NAME ) ?
'rle_title' : 'rle_date_updated';
+ }
+ $idField = "${tablePrefix}_id";
+ $conditions = [];
+ $options = [
+ 'ORDER BY' => [ "$mainField $sortDir", "$idField
$sortDir" ],
+ 'LIMIT' => (int)$limit,
+ ];
+
+ if ( $from !== null ) {
+ $op = ( $sortDir === self::SORT_DIR_ASC ) ? '>' : '<';
+ $safeFromMain = ( $sortBy === self::SORT_BY_NAME )
+ ? $this->dbr->addQuotes( $from[0] )
+ : $this->dbr->addQuotes( $this->dbr->timestamp(
$from[0] ) );
+ $safeFromId = (int)$from[1];
+ $conditions[] = $this->dbr->makeList( [
+ "$mainField $op $safeFromMain",
+ $this->dbr->makeList( [
+ "$mainField = $safeFromMain",
+ "$idField $op= $safeFromId",
+ ], IDatabase::LIST_AND ),
+ ], IDatabase::LIST_OR );
+ }
+
+ // note: $conditions will be array_merge-d so it should not
contain non-numeric keys
+ return [ $conditions, $options ];
+ }
+
/**
* Get list data, and optionally lock the list.
* List must exist, belong to the current user and not be deleted.
diff --git a/tests/src/ReadingListRepositoryTest.php
b/tests/src/ReadingListRepositoryTest.php
index 94a968d..fcd1c5c 100644
--- a/tests/src/ReadingListRepositoryTest.php
+++ b/tests/src/ReadingListRepositoryTest.php
@@ -51,11 +51,13 @@
[ 'teardownForUser' ],
[ 'isSetupForUser' ],
[ 'addList', 'foo' ],
- [ 'getAllLists' ],
+ [ 'getAllLists', ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC ],
[ 'updateList', 1, 'foo' ],
[ 'deleteList', 1 ],
[ 'addListEntry', 1, 'foo', 'bar' ],
- [ 'getListEntries', [ 1 ] ],
+ [ 'getListEntries', [ 1 ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC ],
[ 'deleteListEntry', 1 ],
[ 'getListsByDateUpdated', wfTimestampNow() ],
[ 'getListsByPage', 'foo', 'bar' ],
@@ -87,7 +89,8 @@
return [
[ 'teardownForUser' ],
[ 'addList', 'foo' ],
- [ 'getAllLists' ],
+ [ 'getAllLists', ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC ],
[ 'getListsByDateUpdated', wfTimestampNow() ],
[ 'getListsByPage', 'foo', 'bar' ],
];
@@ -184,7 +187,12 @@
} );
}
- public function testGetAllLists() {
+ /**
+ * @dataProvider provideGetAllLists
+ * @param array $args
+ * @param array $expected
+ */
+ public function testGetAllLists( array $args, array $expected ) {
$this->addDataForAnotherUser();
$repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
$repository->setupForUser();
@@ -198,8 +206,8 @@
[
'rl_name' => 'foo',
'rl_description' => 'this is the second foo',
- 'rl_date_created' => wfTimestampNow(),
- 'rl_date_updated' => wfTimestampNow(),
+ 'rl_date_created' => '20170101000000',
+ 'rl_date_updated' => '20170101000000',
'rl_deleted' => '0',
],
[
@@ -210,14 +218,16 @@
],
[
'rl_name' => 'baz',
- 'rl_date_created' => wfTimestampNow(),
- 'rl_date_updated' => wfTimestampNow(),
+ 'rl_date_created' => '20170101000000',
+ 'rl_date_updated' => '20170101000000',
'rl_deleted' => '1',
],
] );
$compareResultItems = function ( array $expected, array $actual
) {
- $this->assertTimestampEquals(
$expected['rl_date_created'], $actual['rl_date_created'] );
- $this->assertTimestampEquals(
$expected['rl_date_updated'], $actual['rl_date_updated'] );
+ $this->assertTimestampEquals(
$expected['rl_date_created'], $actual['rl_date_created'],
+ "expected: {$expected['rl_name']}; actual:
{$actual['rl_name']}" );
+ $this->assertTimestampEquals(
$expected['rl_date_updated'], $actual['rl_date_updated'],
+ "expected: {$expected['rl_name']}; actual:
{$actual['rl_name']}" );
unset( $expected['rl_date_created'],
$expected['rl_date_updated'] );
unset( $actual['rl_id'], $actual['rl_date_created'],
$actual['rl_date_updated'] );
$this->assertArrayEquals( $expected, $actual, false,
true );
@@ -228,9 +238,13 @@
array_map( $compareResultItems, $expected, $data );
};
- $res = $repository->getAllLists();
- $expectedData = [
- [
+ $res = call_user_func_array( [ $repository, 'getAllLists' ],
$args );
+ $compare( $expected, $res );
+ }
+
+ public function provideGetAllLists() {
+ $entries = [
+ 'default' => [
'rl_name' => 'default',
'rl_description' => '',
'rl_color' => '',
@@ -241,7 +255,7 @@
'rl_date_updated' => wfTimestampNow(),
'rl_deleted' => '0',
],
- [
+ 'foo' => [
'rl_name' => 'foo',
'rl_description' => '',
'rl_color' => '',
@@ -252,18 +266,18 @@
'rl_date_updated' => '20120101000000',
'rl_deleted' => '0',
],
- [
+ 'foo_2' => [
'rl_name' => 'foo',
'rl_description' => 'this is the second foo',
'rl_color' => '',
'rl_image' => '',
'rl_icon' => '',
'rl_is_default' => '0',
- 'rl_date_created' => wfTimestampNow(),
- 'rl_date_updated' => wfTimestampNow(),
+ 'rl_date_created' => '20170101000000',
+ 'rl_date_updated' => '20170101000000',
'rl_deleted' => '0',
],
- [
+ 'bar' => [
'rl_name' => 'bar',
'rl_description' => '',
'rl_color' => '',
@@ -275,16 +289,42 @@
'rl_deleted' => '0',
],
];
- $compare( $expectedData, $res );
+ // 1 list from addDataForAnotherUser, 1 from setupForUser,
second list in addLists call
+ $foo2Id = 4;
- $res = $repository->getAllLists( 1 );
- $compare( array_slice( $expectedData, 0, 1 ), $res );
-
- $res = $repository->getAllLists( 1, 1 );
- $compare( array_slice( $expectedData, 1, 1 ), $res );
-
- $res = $repository->getAllLists( 1, 10 );
- $this->assertSame( 0, iterator_count( $res ) );
+ return [
+ 'name, basic' => [
+ [ ReadingListRepository::SORT_BY_NAME,
ReadingListRepository::SORT_DIR_ASC ],
+ [ $entries['bar'], $entries['default'],
$entries['foo'], $entries['foo_2'] ],
+ ],
+ 'name, reverse' => [
+ [ ReadingListRepository::SORT_BY_NAME,
ReadingListRepository::SORT_DIR_DESC ],
+ [ $entries['foo_2'], $entries['foo'],
$entries['default'], $entries['bar'] ],
+ ],
+ 'name, limit' => [
+ [ ReadingListRepository::SORT_BY_NAME,
ReadingListRepository::SORT_DIR_ASC, 1 ],
+ [ $entries['bar'] ],
+ ],
+ 'name, limit + offset' => [
+ [ ReadingListRepository::SORT_BY_NAME,
ReadingListRepository::SORT_DIR_ASC,
+ 1, [ 'default', 1 ] ],
+ [ $entries['default'] ],
+ ],
+ 'name, limit + other offset' => [
+ [ ReadingListRepository::SORT_BY_NAME,
ReadingListRepository::SORT_DIR_ASC,
+ 1, [ 'foo', $foo2Id ] ],
+ [ $entries['foo_2'] ],
+ ],
+ 'updated, basic' => [
+ [ ReadingListRepository::SORT_BY_UPDATED,
ReadingListRepository::SORT_DIR_ASC ],
+ [ $entries['bar'], $entries['foo'],
$entries['foo_2'], $entries['default'] ],
+ ],
+ 'updated, limit + offset' => [
+ [ ReadingListRepository::SORT_BY_UPDATED,
ReadingListRepository::SORT_DIR_ASC,
+ 1, [ '20170101000000', $foo2Id ] ],
+ [ $entries['foo_2'] ],
+ ],
+ ];
}
public function testUpdateList() {
@@ -493,11 +533,15 @@
);
}
- public function testGetListEntries() {
+ /**
+ * @dataProvider provideGetListEntries
+ * @param array $args
+ * @param array $expected
+ */
+ public function testGetListEntries( array $args, array $expected ) {
$repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
$repository->setupForUser();
- $defaultId = $this->db->selectField( 'reading_list', 'rl_id',
- [ 'rl_user_id' => 1, 'rl_is_default' => 1 ] );
+ $defaultId = 1;
$this->addListEntries( $defaultId, 1, [
[
'rle_user_id' => 1,
@@ -508,52 +552,49 @@
'rle_deleted' => 0,
],
] );
- list( $listId, $deletedListId ) = $this->addLists( 1, [
+ $this->addLists( 1, [
[
'rl_is_default' => 0,
'rl_name' => 'test',
'entries' => [
[
'rlp_project' => 'foo',
- 'rle_title' => 'bar',
- 'rle_date_created' =>
wfTimestampNow(),
- 'rle_date_updated' =>
wfTimestampNow(),
+ 'rle_title' => 'Bar',
+ 'rle_date_created' =>
'20100101000000',
+ 'rle_date_updated' =>
'20150101000000',
'rle_deleted' => 0,
],
[
'rlp_project' => 'foo2',
- 'rle_title' => 'bar2',
+ 'rle_title' => 'Bar2',
'rle_date_created' =>
'20100101000000',
'rle_date_updated' =>
'20120101000000',
'rle_deleted' => 0,
],
[
'rlp_project' => 'foo3',
- 'rle_title' => 'bar3',
- 'rle_date_created' =>
wfTimestampNow(),
- 'rle_date_updated' =>
wfTimestampNow(),
+ 'rle_title' => 'Bar2',
+ 'rle_date_created' =>
'20100101000000',
+ 'rle_date_updated' =>
'20170101000000',
'rle_deleted' => 0,
],
[
'rlp_project' => 'foo4',
- 'rle_title' => 'bar4',
- 'rle_date_created' =>
wfTimestampNow(),
- 'rle_date_updated' =>
wfTimestampNow(),
+ 'rle_title' => 'Bar4',
+ 'rle_date_created' =>
'20100101000000',
+ 'rle_date_updated' =>
'20160101000000',
'rle_deleted' => 1,
],
],
],
- [
- 'rl_is_default' => 0,
- 'rl_name' => 'test-deleted',
- 'rl_deleted' => 1,
- ],
] );
$compareResultItems = function ( $expected, $actual, $n ) {
+ $error = "Mismatch in item $n (expected project/title:
{$expected['rlp_project']}"
+ . "/{$expected['rle_title']}; actual:
{$actual['rlp_project']}/{$actual['rle_title']})";
$this->assertTimestampEquals(
$expected['rle_date_created'], $actual['rle_date_created'],
- "Mismatch in item $n" );
+ $error );
$this->assertTimestampEquals(
$expected['rle_date_updated'], $actual['rle_date_updated'],
- "Mismatch in item $n" );
+ $error );
unset( $expected['rle_date_created'],
$expected['rle_date_updated'] );
unset( $actual['rle_id'], $actual['rle_rlp_id'],
$actual['rle_date_created'],
$actual['rle_date_updated'] );
@@ -565,9 +606,15 @@
array_map( $compareResultItems, $expected, $data,
range( 1, count( $expected ) ) );
};
- $res = $repository->getListEntries( [ $defaultId, $listId ] );
- $expectedData = [
- [
+ $res = call_user_func_array( [ $repository, 'getListEntries' ],
$args );
+ $compare( $expected, $res );
+ }
+
+ public function provideGetListEntries() {
+ $defaultId = 1;
+ $testId = 2;
+ $expected = [
+ 'default-foo' => [
'rle_rl_id' => $defaultId,
'rlp_project' => 'foo',
'rle_title' => 'Foo',
@@ -575,61 +622,105 @@
'rle_date_updated' => wfTimestampNow(),
'rle_deleted' => 0,
],
- [
- 'rle_rl_id' => $listId,
+ 'list-foo' => [
+ 'rle_rl_id' => $testId,
'rlp_project' => 'foo',
- 'rle_title' => 'bar',
- 'rle_date_created' => wfTimestampNow(),
- 'rle_date_updated' => wfTimestampNow(),
+ 'rle_title' => 'Bar',
+ 'rle_date_created' => '20100101000000',
+ 'rle_date_updated' => '20150101000000',
'rle_deleted' => 0,
],
- [
- 'rle_rl_id' => $listId,
+ 'list-foo2' => [
+ 'rle_rl_id' => $testId,
'rlp_project' => 'foo2',
- 'rle_title' => 'bar2',
+ 'rle_title' => 'Bar2',
'rle_date_created' => '20100101000000',
'rle_date_updated' => '20120101000000',
'rle_deleted' => 0,
],
- [
- 'rle_rl_id' => $listId,
+ 'list-foo3' => [
+ 'rle_rl_id' => $testId,
'rlp_project' => 'foo3',
- 'rle_title' => 'bar3',
- 'rle_date_created' => wfTimestampNow(),
- 'rle_date_updated' => wfTimestampNow(),
+ 'rle_title' => 'Bar2',
+ 'rle_date_created' => '20100101000000',
+ 'rle_date_updated' => '20170101000000',
'rle_deleted' => 0,
],
];
- $compare( $expectedData, $res );
- $res = $repository->getListEntries( [ $listId ] );
- $compare( array_slice( $expectedData, 1 ), $res );
+ return [
+ 'name, basic' => [
+ [ [ $defaultId, $testId ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC ],
+ [ $expected['list-foo'],
$expected['list-foo2'], $expected['list-foo3'],
+ $expected['default-foo'] ],
+ ],
+ 'name, desc' => [
+ [ [ $defaultId, $testId ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_DESC ],
+ [ $expected['default-foo'],
$expected['list-foo3'], $expected['list-foo2'],
+ $expected['list-foo'] ],
+ ],
+ 'name, offset + limit' => [
+ [ [ $defaultId, $testId ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC, 2,
[ 'Bar2', 3 ] ],
+ [ $expected['list-foo2'],
$expected['list-foo3'] ],
+ ],
+ 'tiebreaker' => [
+ [ [ $defaultId, $testId ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC, 2,
[ 'Bar2', 4 ] ],
+ [ $expected['list-foo3'],
$expected['default-foo'] ],
+ ],
+ 'updated' => [
+ [ [ $defaultId, $testId ],
ReadingListRepository::SORT_BY_UPDATED,
+ ReadingListRepository::SORT_DIR_ASC ],
+ [ $expected['list-foo2'],
$expected['list-foo'], $expected['list-foo3'],
+ $expected['default-foo'] ],
+ ],
+ 'filter by list id' => [
+ [ [ $defaultId ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC ],
+ [ $expected['default-foo'] ],
+ ],
+ ];
+ }
- $res = $repository->getListEntries( [ $defaultId, $listId ], 2
);
- $compare( array_slice( $expectedData, 0, 2 ), $res );
-
- $res = $repository->getListEntries( [ $defaultId, $listId ], 2,
2 );
- $compare( array_slice( $expectedData, 2, 2 ), $res );
+ // @codingStandardsIgnoreLine
MediaWiki.NamingConventions.LowerCamelFunctionsName.FunctionName
+ public function testGetListEntries_error() {
+ $repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
+ $repository->setupForUser();
+ $defaultId = 1;
+ list( $deletedListId ) = $this->addLists( 1, [
+ [
+ 'rl_is_default' => 0,
+ 'rl_name' => 'test-deleted',
+ 'rl_deleted' => 1,
+ ],
+ ] );
$this->assertFailsWith( 'readinglists-db-error-empty-list-ids',
function () use ( $repository ) {
- $repository->getListEntries( [] );
+ $repository->getListEntries( [],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC );
}
);
$this->assertFailsWith( 'readinglists-db-error-no-such-list',
function () use ( $repository ) {
- $repository->getListEntries( [ 123 ] );
+ $repository->getListEntries( [ 123 ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC );
}
);
$this->assertFailsWith( 'readinglists-db-error-not-own-list',
function () use ( $defaultId ) {
$repository = new ReadingListRepository( 123,
$this->db, $this->db, $this->lbFactory );
- $repository->getListEntries( [ $defaultId ] );
+ $repository->getListEntries( [ $defaultId ],
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC );
}
);
$this->assertFailsWith( 'readinglists-db-error-list-deleted',
function () use ( $repository, $deletedListId ) {
- $repository->getListEntries( [ $deletedListId ]
);
+ $repository->getListEntries( [ $deletedListId
], ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC );
}
);
}
@@ -716,61 +807,106 @@
);
}
- public function testGetListsByDateUpdated() {
+ /**
+ * @dataProvider provideGetListsByDateUpdated
+ * @param array $args
+ * @param array $expected
+ */
+ public function testGetListsByDateUpdated( array $args, array $expected
) {
$repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
$this->addLists( 1, [
[
- 'rl_name' => 'new',
+ 'rl_name' => 'foo',
+ 'rl_description' => 'list1',
'rl_date_updated' => '20150101000000',
],
[
- 'rl_name' => 'deleted',
+ 'rl_name' => 'foo',
+ 'rl_description' => 'list2',
+ 'rl_date_updated' => '20120101000000',
+ ],
+ [
+ 'rl_name' => 'foo2',
+ 'rl_description' => 'list3',
+ 'rl_date_updated' => '20150101000000',
+ ],
+ [
+ 'rl_name' => 'foo3',
+ 'rl_description' => 'list4',
+ 'rl_date_updated' => '20170101000000',
+ ],
+ [
+ 'rl_name' => 'foo',
+ 'rl_description' => 'too-old',
+ 'rl_date_updated' => '20080101000000',
+ ],
+ [
+ 'rl_name' => 'foo',
+ 'rl_description' => 'deleted',
'rl_deleted' => 1,
'rl_date_updated' => '20150102000000',
],
- [
- 'rl_name' => 'old',
- 'rl_date_updated' => '20080101000000',
- ],
] );
- $expected = [ 'new', 'deleted' ];
- $res = $repository->getListsByDateUpdated( '20100101000000' );
- $data = $this->resultWrapperToArray( $res, 'rl_name' );
+ $res = call_user_func_array( [ $repository,
'getListsByDateUpdated' ], $args );
+ $data = $this->resultWrapperToArray( $res, 'rl_description' );
$this->assertArrayEquals( $expected, $data );
-
- $res = $repository->getListsByDateUpdated( '20100101000000', 1
);
- $data = $this->resultWrapperToArray( $res, 'rl_name' );
- $this->assertCount( 1, $data );
- $this->assertSubset( $data, $expected );
-
- $res = $repository->getListsByDateUpdated( '20100101000000', 1,
1 );
- $data2 = $this->resultWrapperToArray( $res, 'rl_name' );
- $this->assertCount( 1, $data2 );
- $this->assertSubset( $data2, $expected );
- $this->assertNotEquals( $data, $data2 );
}
- public function testGetListEntriesByDateUpdated() {
+ public function provideGetListsByDateUpdated() {
+ return [
+ 'basic' => [
+ [ '20100101000000' ],
+ [ 'list2', 'list1', 'list3', 'deleted', 'list4'
],
+ ],
+ 'desc' => [
+ [ '20100101000000',
ReadingListRepository::SORT_BY_UPDATED,
+ ReadingListRepository::SORT_DIR_DESC ],
+ [ 'list4', 'deleted', 'list3', 'list1', 'list2'
],
+ ],
+ 'limit + offset' => [
+ [ '20100101000000',
ReadingListRepository::SORT_BY_UPDATED,
+ ReadingListRepository::SORT_DIR_ASC, 2,
[ '20150101000000', 3 ] ],
+ [ 'list3', 'deleted' ],
+ ],
+ 'name' => [
+ [ '20100101000000',
ReadingListRepository::SORT_BY_NAME,
+ ReadingListRepository::SORT_DIR_ASC ],
+ [ 'list1', 'list2', 'deleted', 'list3', 'list4'
],
+ ]
+ ];
+ }
+
+ /**
+ * @dataProvider provideGetListEntriesByDateUpdated
+ * @param array $args
+ * @param array $expected
+ */
+ public function testGetListEntriesByDateUpdated( array $args, array
$expected ) {
$repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
$this->addLists( 1, [
[
'rl_name' => 'one',
'entries' => [
[
- 'rlp_project' => 'new',
- 'rle_title' => 'new',
+ 'rlp_project' => 'foo',
+ 'rle_title' => 'foo',
'rle_date_updated' =>
'20150101000000',
],
[
- 'rlp_project' => 'deleted',
- 'rle_title' => 'deleted',
+ 'rlp_project' => 'foo-deleted',
+ 'rle_title' => 'foo',
'rle_deleted' => 1,
'rle_date_updated' =>
'20150101000000',
],
[
- 'rlp_project' => 'old',
- 'rle_title' => 'old',
+ 'rlp_project' => 'foo2',
+ 'rle_title' => 'foo',
+ 'rle_date_updated' =>
'20170101000000',
+ ],
+ [
+ 'rlp_project' => 'too-old',
+ 'rle_title' => 'foo',
'rle_date_updated' =>
'20080101000000',
],
],
@@ -779,8 +915,8 @@
'rl_name' => 'two',
'entries' => [
[
- 'rlp_project' => 'other',
- 'rle_title' => 'other',
+ 'rlp_project' => 'bar',
+ 'rle_title' => 'bar',
'rle_date_updated' =>
'20150101000000',
],
],
@@ -791,28 +927,40 @@
'entries' => [
[
'rlp_project' => 'parent
deleted',
- 'rle_title' => 'parent deleted',
+ 'rle_title' => 'bar',
'rle_date_updated' =>
'20150101000000',
],
],
],
] );
-
- $expected = [ 'new', 'deleted', 'other' ];
- $res = $repository->getListEntriesByDateUpdated(
'20100101000000' );
- $data = $this->resultWrapperToArray( $res, 'rle_title' );
+ $res = call_user_func_array( [ $repository,
'getListEntriesByDateUpdated' ], $args );
+ $data = $this->resultWrapperToArray( $res, 'rlp_project' );
$this->assertArrayEquals( $expected, $data );
+ }
- $res = $repository->getListEntriesByDateUpdated(
'20100101000000', 1 );
- $data = $this->resultWrapperToArray( $res, 'rle_title' );
- $this->assertCount( 1, $data );
- $this->assertSubset( $data, $expected );
-
- $res = $repository->getListEntriesByDateUpdated(
'20100101000000', 1, 1 );
- $data2 = $this->resultWrapperToArray( $res, 'rle_title' );
- $this->assertCount( 1, $data2 );
- $this->assertSubset( $data2, $expected );
- $this->assertNotEquals( $data, $data2 );
+ public function provideGetListEntriesByDateUpdated() {
+ return [
+ 'basic' => [
+ [ '20100101000000' ],
+ [ 'foo', 'foo-deleted', 'bar', 'foo2' ],
+ ],
+ 'desc' => [
+ [ '20100101000000',
ReadingListRepository::SORT_DIR_DESC ],
+ [ 'foo2', 'bar', 'foo-deleted', 'foo' ],
+ ],
+ 'limit + offset' => [
+ [ '20100101000000',
ReadingListRepository::SORT_DIR_ASC, 2, [ '20150101000000', 2 ] ],
+ [ 'foo-deleted', 'bar' ],
+ ],
+ 'limit + offset 2' => [
+ [ '20100101000000',
ReadingListRepository::SORT_DIR_ASC, 2, [ '20150101000000', 5 ] ],
+ [ 'bar', 'foo2' ],
+ ],
+ 'limit + offset 3' => [
+ [ '20100101000000',
ReadingListRepository::SORT_DIR_ASC, 2, [ '20170101000000', 3 ] ],
+ [ 'foo2' ],
+ ],
+ ];
}
public function testPurgeOldDeleted() {
@@ -871,7 +1019,12 @@
'kept-parent-deleted-new',
'deleted-new-parent-deleted-new' ], $keptEntries );
}
- public function testGetListsByPage() {
+ /**
+ * @dataProvider provideGetListsByPage
+ * @param array $args
+ * @param array $expected
+ */
+ public function testGetListsByPage( array $args, array $expected ) {
$repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
$this->addLists( 1, [
[
@@ -936,21 +1089,30 @@
],
] );
- $expected = [ 'fourth', 'fifth' ];
- $res = $repository->getListsByPage( '-', 'x' );
+ $res = call_user_func_array( [ $repository, 'getListsByPage' ],
$args );
$data = $this->resultWrapperToArray( $res, 'rl_name' );
$this->assertArrayEquals( $expected, $data );
+ }
- $res = $repository->getListsByPage( '-', 'x', 1 );
- $data = $this->resultWrapperToArray( $res, 'rl_name' );
- $this->assertCount( 1, $data );
- $this->assertSubset( $data, $expected );
-
- $res = $repository->getListsByPage( '-', 'x', 1, 1 );
- $data2 = $this->resultWrapperToArray( $res, 'rl_name' );
- $this->assertCount( 1, $data2 );
- $this->assertSubset( $data2, $expected );
- $this->assertNotEquals( $data, $data2 );
+ public function provideGetListsByPage() {
+ return [
+ 'basic' => [
+ [ '-', 'x' ],
+ [ 'fourth', 'fifth' ],
+ ],
+ 'limit' => [
+ [ '-', 'x', 1 ],
+ [ 'fourth' ],
+ ],
+ 'limit + offset' => [
+ [ '-', 'x', 1, 4 ],
+ [ 'fourth' ],
+ ],
+ 'limit + offset 2' => [
+ [ '-', 'x', 1, 5 ],
+ [ 'fifth' ],
+ ],
+ ];
}
// -------------------------------------------
@@ -981,7 +1143,7 @@
}
$delta = abs( wfTimestamp( TS_UNIX, $expectedTimestamp )
- wfTimestamp( TS_UNIX, $actualTimestamp ) );
- $this->assertLessThanOrEqual( 3, $delta,
+ $this->assertLessThanOrEqual( 30, $delta,
"${msg}Difference between expected timestamp
($expectedTimestamp) "
. "and actual timetamp ($actualTimestamp) is too large"
);
}
--
To view, visit https://gerrit.wikimedia.org/r/393925
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia04a6c42ed7eca2a1c49d6ef109f0359db06ccd4
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/ReadingLists
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Gergő Tisza <[email protected]>
Gerrit-Reviewer: Mholloway <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
