Gergő Tisza has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/390833 )
Change subject: Improve DB assertions & fix related tests
......................................................................
Improve DB assertions & fix related tests
* make sure user ids are cast to integer and user id 0 (often used
for anonymous users) is treated as no user
* make sure calls other than setup never succeed if the user has
not set up lists (see T180189#3749953)
* fix setup tests (the last few of which I apparently forgot to write)
* fix assertUser tests (forgot to remove old code after rewriting)
* convert those tests into dataProvider-based for nicer error messages
Change-Id: I60d5d70d446ff3e0bab667e95938b2d3b1fde5ce
---
M src/ReadingListRepository.php
M tests/src/ReadingListRepositoryTest.php
2 files changed, 71 insertions(+), 59 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/ReadingLists
refs/changes/33/390833/1
diff --git a/src/ReadingListRepository.php b/src/ReadingListRepository.php
index dcf499b..ef32dbf 100644
--- a/src/ReadingListRepository.php
+++ b/src/ReadingListRepository.php
@@ -71,7 +71,7 @@
* @param LBFactory $lbFactory
*/
public function __construct( $userId, IDatabase $dbw, IDatabase $dbr,
LBFactory $lbFactory ) {
- $this->userId = $userId;
+ $this->userId = (int)$userId ?: null;
$this->dbw = $dbw;
$this->dbr = $dbr;
$this->lbFactory = $lbFactory;
@@ -267,7 +267,11 @@
]
);
- if ( $res->numRows() === 0 && !$this->isSetupForUser() ) {
+ if (
+ $res->numRows() === 0
+ && !$this->isSetupForUser()
+ && !$this->isSetupForUser( self::READ_LATEST )
+ ) {
throw new ReadingListRepositoryException(
'readinglists-db-error-not-set-up' );
}
return $res;
@@ -845,6 +849,14 @@
'ORDER BY' => 'rl_id',
]
);
+
+ if (
+ $res->numRows() === 0
+ && !$this->isSetupForUser()
+ && !$this->isSetupForUser( self::READ_LATEST )
+ ) {
+ throw new ReadingListRepositoryException(
'readinglists-db-error-not-set-up' );
+ }
return $res;
}
@@ -969,6 +981,14 @@
'ORDER BY' => 'rl_id',
]
);
+
+ if (
+ $res->numRows() === 0
+ && !$this->isSetupForUser()
+ && !$this->isSetupForUser( self::READ_LATEST )
+ ) {
+ throw new ReadingListRepositoryException(
'readinglists-db-error-not-set-up' );
+ }
return $res;
}
diff --git a/tests/src/ReadingListRepositoryTest.php
b/tests/src/ReadingListRepositoryTest.php
index e6d4e12..dc188de 100644
--- a/tests/src/ReadingListRepositoryTest.php
+++ b/tests/src/ReadingListRepositoryTest.php
@@ -25,9 +25,28 @@
$this->lbFactory =
MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
}
- public function testAssertUser() {
+ /**
+ * @dataProvider provideAssertUser
+ * @param string $method ReadingListRepository method name
+ * @param mixed $param Method parameters
+ */
+ public function testAssertUser( $method /*, $param... */ ) {
$repository = new ReadingListRepository( null, $this->db,
$this->db, $this->lbFactory );
- $calls = [
+ $call = func_get_args();
+ $this->assertFailsWith( 'readinglists-db-error-user-required',
+ function () use ( $repository, $call ) {
+ $method = array_shift( $call );
+ $params = $call;
+ call_user_func_array( [ $repository, $method ],
$params );
+ }
+ );
+ }
+
+ /**
+ * @return array
+ */
+ public function provideAssertUser() {
+ return [
[ 'setupForUser' ],
[ 'teardownForUser' ],
[ 'isSetupForUser' ],
@@ -45,66 +64,39 @@
[ 'getListsByDateUpdated', wfTimestampNow() ],
[ 'getListsByPage', 'foo', 'bar' ],
];
- foreach ( $calls as $call ) {
- $this->assertFailsWith(
'readinglists-db-error-user-required',
- function () use ( $repository, $call ) {
- $method = array_shift( $call );
- $params = $call;
- call_user_func_array( [ $repository,
$method ], $params );
- }
- );
- }
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->teardownForUser();
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->isSetupForUser();
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-user-required',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
}
- public function testUninitializedErrors() {
+ /**
+ * @dataProvider provideUninitializedErrors
+ * @param string $method ReadingListRepository method name
+ * @param mixed $param Method parameters
+ */
+ public function testUninitializedErrors( $method /*, $param... */ ) {
$this->addDataForAnotherUser();
$repository = new ReadingListRepository( 1, $this->db,
$this->db, $this->lbFactory );
+ $call = func_get_args();
+ $this->assertFailsWith( 'readinglists-db-error-not-set-up',
+ function () use ( $repository, $call ) {
+ $method = array_shift( $call );
+ $params = $call;
+ call_user_func_array( [ $repository, $method ],
$params );
+ }
+ );
+ }
- $this->assertFailsWith( 'readinglists-db-error-not-set-up',
function () use ( $repository ) {
- $repository->addList( 'foo' );
- } );
- $this->assertFailsWith( 'readinglists-db-error-not-set-up',
function () use ( $repository ) {
- $repository->getAllLists();
- } );
- $this->assertFailsWith( 'readinglists-db-error-not-set-up',
function () use ( $repository ) {
- $repository->getListOrder();
- } );
- $this->assertFailsWith( 'readinglists-db-error-not-set-up',
function () use ( $repository ) {
- $repository->teardownForUser();
- } );
- $this->assertFailsWith( 'readinglists-db-error-not-set-up',
function () use ( $repository ) {
- $repository->teardownForUser();
- } );
- $this->assertFailsWith( 'readinglists-db-error-not-set-up',
function () use ( $repository ) {
- $repository->teardownForUser();
- } );
+ /**
+ * @return array
+ */
+ public function provideUninitializedErrors() {
+ return [
+ [ 'teardownForUser' ],
+ [ 'addList', 'foo' ],
+ [ 'getAllLists' ],
+ [ 'getListOrder' ],
+ [ 'setListOrder', [ 1 ] ],
+ [ 'getListsByDateUpdated', wfTimestampNow() ],
+ [ 'getListsByPage', 'foo', 'bar' ],
+ ];
}
public function testSetupAndTeardown() {
--
To view, visit https://gerrit.wikimedia.org/r/390833
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I60d5d70d446ff3e0bab667e95938b2d3b1fde5ce
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/ReadingLists
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
