diff --git a/lib/AbstractException.php b/lib/AbstractException.php index 93798ca1..22b6eb52 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -68,11 +68,10 @@ abstract class AbstractException extends \Exception { "Conf/Exception.typeMismatch" => 10311, "Conf/Exception.semanticMismatch" => 10312, "Conf/Exception.ambiguousDefault" => 10313, - "User/Exception.functionNotImplemented" => 10401, - "User/Exception.doesNotExist" => 10402, - "User/Exception.alreadyExists" => 10403, "User/Exception.authMissing" => 10411, "User/Exception.authFailed" => 10412, + "User/ExceptionConflict.doesNotExist" => 10402, + "User/ExceptionConflict.alreadyExists" => 10403, "User/ExceptionSession.invalid" => 10431, "User/ExceptionInput.invalidTimezone" => 10441, "User/ExceptionInput.invalidBoolean" => 10442, diff --git a/lib/Database.php b/lib/Database.php index a531f23d..660fbc4a 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -248,11 +248,11 @@ class Database { /** Adds a user to the database * * @param string $user The user to add - * @param string $passwordThe user's password in cleartext. It will be stored hashed + * @param string|null $passwordThe user's password in cleartext. It will be stored hashed */ - public function userAdd(string $user, string $password): bool { + public function userAdd(string $user, ?string $password): bool { if ($this->userExists($user)) { - throw new User\Exception("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("alreadyExists", ["action" => __FUNCTION__, "user" => $user]); } $hash = (strlen($password) > 0) ? password_hash($password, \PASSWORD_DEFAULT) : ""; // NOTE: This roundabout construction (with 'select' rather than 'values') is required by MySQL, because MySQL is riddled with pitfalls and exceptions @@ -263,7 +263,7 @@ class Database { /** Removes a user from the database */ public function userRemove(string $user): bool { if ($this->db->prepare("DELETE from arsse_users where id = ?", "str")->run($user)->changes() < 1) { - throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } return true; } @@ -280,7 +280,7 @@ class Database { /** Retrieves the hashed password of a user */ public function userPasswordGet(string $user): ?string { if (!$this->userExists($user)) { - throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } return $this->db->prepare("SELECT password from arsse_users where id = ?", "str")->run($user)->getValue(); } @@ -288,11 +288,11 @@ class Database { /** Sets the password of an existing user * * @param string $user The user for whom to set the password - * @param string $password The new password, in cleartext. The password will be stored hashed. If null is passed, the password is unset and authentication not possible + * @param string|null $password The new password, in cleartext. The password will be stored hashed. If null is passed, the password is unset and authentication not possible */ - public function userPasswordSet(string $user, string $password = null): bool { + public function userPasswordSet(string $user, ?string $password): bool { if (!$this->userExists($user)) { - throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } $hash = (strlen($password ?? "") > 0) ? password_hash($password, \PASSWORD_DEFAULT) : $password; $this->db->prepare("UPDATE arsse_users set password = ? where id = ?", "str", "str")->run($hash, $user); @@ -302,7 +302,7 @@ class Database { public function userPropertiesGet(string $user): array { $out = $this->db->prepare("SELECT num, admin, lang, tz, sort_asc from arsse_users where id = ?", "str")->run($user)->getRow(); if (!$out) { - throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } settype($out['num'], "int"); settype($out['admin'], "bool"); @@ -312,7 +312,7 @@ class Database { public function userPropertiesSet(string $user, array $data): bool { if (!$this->userExists($user)) { - throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } $allowed = [ 'admin' => "strict bool", @@ -402,7 +402,7 @@ class Database { */ public function tokenCreate(string $user, string $class, string $id = null, \DateTimeInterface $expires = null): string { if (!$this->userExists($user)) { - throw new User\Exception("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); + throw new User\ExceptionConflict("doesNotExist", ["action" => __FUNCTION__, "user" => $user]); } // generate a token if it's not provided $id = $id ?? UUID::mint()->hex; diff --git a/lib/ImportExport/AbstractImportExport.php b/lib/ImportExport/AbstractImportExport.php index 72064825..6f0496fd 100644 --- a/lib/ImportExport/AbstractImportExport.php +++ b/lib/ImportExport/AbstractImportExport.php @@ -9,7 +9,7 @@ namespace JKingWeb\Arsse\ImportExport; use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Db\ExceptionInput as InputException; -use JKingWeb\Arsse\User\Exception as UserException; +use JKingWeb\Arsse\User\ExceptionConflict as UserException; abstract class AbstractImportExport { public function import(string $user, string $data, bool $flat = false, bool $replace = false): bool { diff --git a/lib/ImportExport/OPML.php b/lib/ImportExport/OPML.php index 30cb4f56..85d136cf 100644 --- a/lib/ImportExport/OPML.php +++ b/lib/ImportExport/OPML.php @@ -7,7 +7,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse\ImportExport; use JKingWeb\Arsse\Arsse; -use JKingWeb\Arsse\User\Exception as UserException; +use JKingWeb\Arsse\User\ExceptionConflict as UserException; class OPML extends AbstractImportExport { protected function parse(string $opml, bool $flat): array { diff --git a/lib/REST/Fever/API.php b/lib/REST/Fever/API.php index 1901397f..3382e6cb 100644 --- a/lib/REST/Fever/API.php +++ b/lib/REST/Fever/API.php @@ -241,7 +241,7 @@ class API extends \JKingWeb\Arsse\REST\AbstractHandler { try { // verify the supplied hash is valid $s = Arsse::$db->TokenLookup("fever.login", $hash); - } catch (\JKingWeb\Arsse\Db\ExceptionInput $e) { + } catch (ExceptionInput $e) { return false; } // set the user name diff --git a/lib/User.php b/lib/User.php index 8ebee8a5..ffb6d4aa 100644 --- a/lib/User.php +++ b/lib/User.php @@ -48,11 +48,14 @@ class User { return $this->u->userList(); } - public function add($user, $password = null): string { - $out = $this->u->userAdd($user, $password) ?? $this->u->userAdd($user, $this->generatePassword()); - // synchronize the internal database - if (!Arsse::$db->userExists($user)) { - Arsse::$db->userAdd($user, $out); + public function add(string $user, ?string $password = null): string { + try { + $out = $this->u->userAdd($user, $password) ?? $this->u->userAdd($user, $this->generatePassword()); + } finally { + // synchronize the internal database + if (!Arsse::$db->userExists($user)) { + Arsse::$db->userAdd($user, $out ?? null); + } } return $out; } @@ -68,7 +71,7 @@ class User { } } - public function passwordSet(string $user, string $newPassword = null, $oldPassword = null): string { + public function passwordSet(string $user, ?string $newPassword, $oldPassword = null): string { $out = $this->u->userPasswordSet($user, $newPassword, $oldPassword) ?? $this->u->userPasswordSet($user, $this->generatePassword(), $oldPassword); if (Arsse::$db->userExists($user)) { // if the password change was successful and the user exists, set the internal password to the same value diff --git a/lib/User/ExceptionConflict.php b/lib/User/ExceptionConflict.php new file mode 100644 index 00000000..4fa1bbfd --- /dev/null +++ b/lib/User/ExceptionConflict.php @@ -0,0 +1,10 @@ +userExists($user)) { - throw new Exception("doesNotExist", ['action' => "userPasswordUnset", 'user' => $user]); + throw new ExceptionConflict("doesNotExist", ['action' => "userPasswordUnset", 'user' => $user]); } else { return true; } @@ -74,7 +74,7 @@ class Driver implements \JKingWeb\Arsse\User\Driver { public function userPropertiesGet(string $user): array { // do nothing: the internal database will retrieve everything for us if (!$this->userExists($user)) { - throw new Exception("doesNotExist", ['action' => "userPasswordUnset", 'user' => $user]); + throw new ExceptionConflict("doesNotExist", ['action' => "userPasswordUnset", 'user' => $user]); } else { return []; } @@ -83,7 +83,7 @@ class Driver implements \JKingWeb\Arsse\User\Driver { public function userPropertiesSet(string $user, array $data): array { // do nothing: the internal database will set everything for us if (!$this->userExists($user)) { - throw new Exception("doesNotExist", ['action' => "userPasswordUnset", 'user' => $user]); + throw new ExceptionConflict("doesNotExist", ['action' => "userPasswordUnset", 'user' => $user]); } else { return $data; } diff --git a/locale/en.php b/locale/en.php index bcc71db1..e5ada184 100644 --- a/locale/en.php +++ b/locale/en.php @@ -134,8 +134,8 @@ return [ 'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineTypeViolation' => '{0}', 'Exception.JKingWeb/Arsse/Db/ExceptionTimeout.general' => '{0}', 'Exception.JKingWeb/Arsse/Db/ExceptionTimeout.logicalLock' => 'Database is locked', - 'Exception.JKingWeb/Arsse/User/Exception.alreadyExists' => 'Could not perform action "{action}" because the user {user} already exists', - 'Exception.JKingWeb/Arsse/User/Exception.doesNotExist' => 'Could not perform action "{action}" because the user {user} does not exist', + 'Exception.JKingWeb/Arsse/User/ExceptionConflict.alreadyExists' => 'Could not perform action "{action}" because the user {user} already exists', + 'Exception.JKingWeb/Arsse/User/ExceptionConflict.doesNotExist' => 'Could not perform action "{action}" because the user {user} does not exist', 'Exception.JKingWeb/Arsse/User/Exception.authMissing' => 'Please log in to proceed', 'Exception.JKingWeb/Arsse/User/Exception.authFailed' => 'Authentication failed', 'Exception.JKingWeb/Arsse/User/ExceptionSession.invalid' => 'Session with ID {0} does not exist', diff --git a/tests/cases/CLI/TestCLI.php b/tests/cases/CLI/TestCLI.php index e6c19e2d..671fbb91 100644 --- a/tests/cases/CLI/TestCLI.php +++ b/tests/cases/CLI/TestCLI.php @@ -142,7 +142,7 @@ class TestCLI extends \JKingWeb\Arsse\Test\AbstractTest { Arsse::$user->method("add")->will($this->returnCallback(function($user, $pass = null) { switch ($user) { case "john.doe@example.com": - throw new \JKingWeb\Arsse\User\Exception("alreadyExists"); + throw new \JKingWeb\Arsse\User\ExceptionConflict("alreadyExists"); case "jane.doe@example.com": return is_null($pass) ? "random password" : $pass; } @@ -200,7 +200,7 @@ class TestCLI extends \JKingWeb\Arsse\Test\AbstractTest { if ($user === "john.doe@example.com") { return true; } - throw new \JKingWeb\Arsse\User\Exception("doesNotExist"); + throw new \JKingWeb\Arsse\User\ExceptionConflict("doesNotExist"); })); $this->assertConsole($this->cli, $cmd, $exitStatus, $output); } @@ -217,7 +217,7 @@ class TestCLI extends \JKingWeb\Arsse\Test\AbstractTest { $passwordChange = function($user, $pass = null) { switch ($user) { case "jane.doe@example.com": - throw new \JKingWeb\Arsse\User\Exception("doesNotExist"); + throw new \JKingWeb\Arsse\User\ExceptionConflict("doesNotExist"); case "john.doe@example.com": return is_null($pass) ? "random password" : $pass; } @@ -247,7 +247,7 @@ class TestCLI extends \JKingWeb\Arsse\Test\AbstractTest { $passwordClear = function($user) { switch ($user) { case "jane.doe@example.com": - throw new \JKingWeb\Arsse\User\Exception("doesNotExist"); + throw new \JKingWeb\Arsse\User\ExceptionConflict("doesNotExist"); case "john.doe@example.com": return true; } diff --git a/tests/cases/Database/SeriesToken.php b/tests/cases/Database/SeriesToken.php index 29977b3f..3f766aa3 100644 --- a/tests/cases/Database/SeriesToken.php +++ b/tests/cases/Database/SeriesToken.php @@ -99,7 +99,7 @@ trait SeriesToken { } public function testCreateATokenForAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); Arsse::$db->tokenCreate("fever.login", "jane.doe@example.biz"); } diff --git a/tests/cases/Database/SeriesUser.php b/tests/cases/Database/SeriesUser.php index 10bd2582..350fa272 100644 --- a/tests/cases/Database/SeriesUser.php +++ b/tests/cases/Database/SeriesUser.php @@ -47,7 +47,7 @@ trait SeriesUser { } public function testGetThePasswordOfAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); Arsse::$db->userPasswordGet("john.doe@example.org"); } @@ -59,7 +59,7 @@ trait SeriesUser { } public function testAddAnExistingUser(): void { - $this->assertException("alreadyExists", "User"); + $this->assertException("alreadyExists", "User", "ExceptionConflict"); Arsse::$db->userAdd("john.doe@example.com", ""); } @@ -71,7 +71,7 @@ trait SeriesUser { } public function testRemoveAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); Arsse::$db->userRemove("john.doe@example.org"); } @@ -101,7 +101,7 @@ trait SeriesUser { } public function testSetThePasswordOfAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); Arsse::$db->userPasswordSet("john.doe@example.org", "secret"); } @@ -110,7 +110,7 @@ trait SeriesUser { $this->assertSame($exp, Arsse::$db->userPropertiesGet($user)); } - public function provideMetadata() { + public function provideMetadata(): iterable { return [ ["admin@example.net", ['num' => 1, 'admin' => true, 'lang' => "en", 'tz' => "America/Toronto", 'sort_asc' => false]], ["jane.doe@example.com", ['num' => 2, 'admin' => false, 'lang' => "fr", 'tz' => "Asia/Kuala_Lumpur", 'sort_asc' => true]], @@ -119,7 +119,7 @@ trait SeriesUser { } public function testGetTheMetadataOfAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); Arsse::$db->userPropertiesGet("john.doe@example.org"); } @@ -147,7 +147,7 @@ trait SeriesUser { } public function testSetTheMetadataOfAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); Arsse::$db->userPropertiesSet("john.doe@example.org", ['admin' => true]); } } diff --git a/tests/cases/ImportExport/TestImportExport.php b/tests/cases/ImportExport/TestImportExport.php index 1a899c4c..e113d83c 100644 --- a/tests/cases/ImportExport/TestImportExport.php +++ b/tests/cases/ImportExport/TestImportExport.php @@ -146,7 +146,7 @@ class TestImportExport extends \JKingWeb\Arsse\Test\AbstractTest { } public function testImportForAMissingUser(): void { - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); $this->proc->import("no.one@example.com", "", false, false); } diff --git a/tests/cases/ImportExport/TestOPML.php b/tests/cases/ImportExport/TestOPML.php index 36caa77c..3c61688e 100644 --- a/tests/cases/ImportExport/TestOPML.php +++ b/tests/cases/ImportExport/TestOPML.php @@ -101,7 +101,7 @@ OPML_EXPORT_SERIALIZATION; public function testExportToOpmlAMissingUser(): void { \Phake::when(Arsse::$db)->userExists->thenReturn(false); - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); (new OPML)->export("john.doe@example.com"); } diff --git a/tests/cases/REST/Fever/TestUser.php b/tests/cases/REST/Fever/TestUser.php index d6bab6df..2764c420 100644 --- a/tests/cases/REST/Fever/TestUser.php +++ b/tests/cases/REST/Fever/TestUser.php @@ -10,7 +10,7 @@ use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\Db\ExceptionInput; -use JKingWeb\Arsse\User\Exception as UserException; +use JKingWeb\Arsse\User\ExceptionConflict as UserException; use JKingWeb\Arsse\Db\Transaction; use JKingWeb\Arsse\REST\Fever\User as FeverUser; diff --git a/tests/cases/User/TestInternal.php b/tests/cases/User/TestInternal.php index cd231a0a..21b38b5f 100644 --- a/tests/cases/User/TestInternal.php +++ b/tests/cases/User/TestInternal.php @@ -37,7 +37,7 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest { \Phake::when(Arsse::$db)->userPasswordGet("john.doe@example.com")->thenReturn('$2y$10$1zbqRJhxM8uUjeSBPp4IhO90xrqK0XjEh9Z16iIYEFRV4U.zeAFom'); // hash of "secret" \Phake::when(Arsse::$db)->userPasswordGet("jane.doe@example.com")->thenReturn('$2y$10$bK1ljXfTSyc2D.NYvT.Eq..OpehLRXVbglW.23ihVuyhgwJCd.7Im'); // hash of "superman" \Phake::when(Arsse::$db)->userPasswordGet("owen.hardy@example.com")->thenReturn(""); - \Phake::when(Arsse::$db)->userPasswordGet("kira.nerys@example.com")->thenThrow(new \JKingWeb\Arsse\User\Exception("doesNotExist")); + \Phake::when(Arsse::$db)->userPasswordGet("kira.nerys@example.com")->thenThrow(new \JKingWeb\Arsse\User\ExceptionConflict("doesNotExist")); \Phake::when(Arsse::$db)->userPasswordGet("007@example.com")->thenReturn(null); $this->assertSame($exp, (new Driver)->auth($user, $password)); } @@ -90,11 +90,11 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest { public function testRemoveAUser(): void { $john = "john.doe@example.com"; - \Phake::when(Arsse::$db)->userRemove->thenReturn(true)->thenThrow(new \JKingWeb\Arsse\User\Exception("doesNotExist")); + \Phake::when(Arsse::$db)->userRemove->thenReturn(true)->thenThrow(new \JKingWeb\Arsse\User\ExceptionConflict("doesNotExist")); $driver = new Driver; $this->assertTrue($driver->userRemove($john)); \Phake::verify(Arsse::$db, \Phake::times(1))->userRemove($john); - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); try { $this->assertFalse($driver->userRemove($john)); } finally { @@ -118,7 +118,7 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest { public function testUnsetAPasswordForAMssingUser(): void { \Phake::when(Arsse::$db)->userExists->thenReturn(false); - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); (new Driver)->userPasswordUnset("john.doe@example.com"); } @@ -131,7 +131,7 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest { public function testGetPropertiesForAMissingUser(): void { \Phake::when(Arsse::$db)->userExists->thenReturn(false); - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); try { (new Driver)->userPropertiesGet("john.doe@example.com"); } finally { @@ -150,7 +150,7 @@ class TestInternal extends \JKingWeb\Arsse\Test\AbstractTest { public function testSetPropertiesForAMissingUser(): void { \Phake::when(Arsse::$db)->userExists->thenReturn(false); - $this->assertException("doesNotExist", "User"); + $this->assertException("doesNotExist", "User", "ExceptionConflict"); try { (new Driver)->userPropertiesSet("john.doe@example.com", ['admin' => true]); } finally { diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php index 759241c2..97a93523 100644 --- a/tests/cases/User/TestUser.php +++ b/tests/cases/User/TestUser.php @@ -10,6 +10,7 @@ use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\Database; use JKingWeb\Arsse\User; use JKingWeb\Arsse\AbstractException as Exception; +use JKingWeb\Arsse\User\ExceptionConflict; use JKingWeb\Arsse\User\Driver; /** @covers \JKingWeb\Arsse\User */ @@ -23,6 +24,11 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { // create a mock user driver $this->drv = \Phake::mock(Driver::class); } + + public function tearDown(): void { + \Phake::verifyNoOtherInteractions($this->drv); + \Phake::verifyNoOtherInteractions(Arsse::$db); + } public function testConstruct(): void { $this->assertInstanceOf(User::class, new User($this->drv)); @@ -49,6 +55,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { $u = new User($this->drv); $this->assertSame($exp, $u->auth($user, $password)); $this->assertNull($u->id); + \Phake::verify($this->drv, \Phake::times((int) !$preAuth))->auth($user, $password); \Phake::verify(Arsse::$db, \Phake::times($exp ? 1 : 0))->userExists($user); \Phake::verify(Arsse::$db, \Phake::times($exp && $user === "jane.doe@example.com" ? 1 : 0))->userAdd($user, $password); } @@ -68,190 +75,65 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { ]; } - /** @dataProvider provideUserList */ - public function testListUsers($exp): void { + public function testListUsers(): void { + $exp = ["john.doe@example.com", "jane.doe@example.com"]; $u = new User($this->drv); \Phake::when($this->drv)->userList->thenReturn(["john.doe@example.com", "jane.doe@example.com"]); $this->assertSame($exp, $u->list()); + \Phake::verify($this->drv)->userList(); } - public function provideUserList(): iterable { - $john = "john.doe@example.com"; - $jane = "jane.doe@example.com"; - return [ - [[$john, $jane]], - ]; - } - - /** @dataProvider provideAdditions */ - public function testAddAUser(string $user, $password, $exp): void { + public function testAddAUser(): void { + $user = "ohn.doe@example.com"; + $pass = "secret"; $u = new User($this->drv); - \Phake::when($this->drv)->userAdd("john.doe@example.com", $this->anything())->thenThrow(new \JKingWeb\Arsse\User\Exception("alreadyExists")); - \Phake::when($this->drv)->userAdd("jane.doe@example.com", $this->anything())->thenReturnCallback(function($user, $pass) { - return $pass ?? "random password"; - }); - if ($exp instanceof Exception) { - $this->assertException("alreadyExists", "User"); - } - $this->assertSame($exp, $u->add($user, $password)); + \Phake::when($this->drv)->userAdd->thenReturn($pass); + \Phake::when(Arsse::$db)->userExists->thenReturn(true); + $this->assertSame($pass, $u->add($user, $pass)); + \Phake::verify($this->drv)->userAdd($user, $pass); + \Phake::verify(Arsse::$db)->userExists($user); } - /** @dataProvider provideAdditions */ - public function testAddAUserWithARandomPassword(string $user, $password, $exp): void { - $u = \Phake::partialMock(User::class, $this->drv); - \Phake::when($this->drv)->userAdd($this->anything(), $this->isNull())->thenReturn(null); - \Phake::when($this->drv)->userAdd("john.doe@example.com", $this->logicalNot($this->isNull()))->thenThrow(new \JKingWeb\Arsse\User\Exception("alreadyExists")); - \Phake::when($this->drv)->userAdd("jane.doe@example.com", $this->logicalNot($this->isNull()))->thenReturnCallback(function($user, $pass) { - return $pass; - }); - if ($exp instanceof Exception) { - $this->assertException("alreadyExists", "User"); - $calls = 2; - } else { - $calls = 4; - } - try { - $pass1 = $u->add($user, null); - $pass2 = $u->add($user, null); - $this->assertNotEquals($pass1, $pass2); - } finally { - \Phake::verify($this->drv, \Phake::times($calls))->userAdd; - \Phake::verify($u, \Phake::times($calls / 2))->generatePassword; - } - } - - public function provideAdditions(): iterable { - $john = "john.doe@example.com"; - $jane = "jane.doe@example.com"; - return [ - [$john, "secret", new \JKingWeb\Arsse\User\Exception("alreadyExists")], - [$jane, "superman", "superman"], - [$jane, null, "random password"], - ]; - } - - /** @dataProvider provideRemovals */ - public function testRemoveAUser(string $user, bool $exists, $exp): void { + public function testAddAUserWeDoNotKnow(): void { + $user = "ohn.doe@example.com"; + $pass = "secret"; $u = new User($this->drv); - \Phake::when($this->drv)->userRemove("john.doe@example.com")->thenReturn(true); - \Phake::when($this->drv)->userRemove("jane.doe@example.com")->thenThrow(new \JKingWeb\Arsse\User\Exception("doesNotExist")); - \Phake::when(Arsse::$db)->userExists->thenReturn($exists); - \Phake::when(Arsse::$db)->userRemove->thenReturn(true); - if ($exp instanceof Exception) { - $this->assertException("doesNotExist", "User"); - } - try { - $this->assertSame($exp, $u->remove($user)); - } finally { - \Phake::verify(Arsse::$db, \Phake::times(1))->userExists($user); - \Phake::verify(Arsse::$db, \Phake::times((int) $exists))->userRemove($user); - } + \Phake::when($this->drv)->userAdd->thenReturn($pass); + \Phake::when(Arsse::$db)->userExists->thenReturn(false); + $this->assertSame($pass, $u->add($user, $pass)); + \Phake::verify($this->drv)->userAdd($user, $pass); + \Phake::verify(Arsse::$db)->userExists($user); + \Phake::verify(Arsse::$db)->userAdd($user, $pass); } - public function provideRemovals(): iterable { - $john = "john.doe@example.com"; - $jane = "jane.doe@example.com"; - return [ - [$john, true, true], - [$john, false, true], - [$jane, true, new \JKingWeb\Arsse\User\Exception("doesNotExist")], - [$jane, false, new \JKingWeb\Arsse\User\Exception("doesNotExist")], - ]; - } - - /** @dataProvider providePasswordChanges */ - public function testChangeAPassword(string $user, $password, bool $exists, $exp): void { + public function testAddADuplicateUser(): void { + $user = "ohn.doe@example.com"; + $pass = "secret"; $u = new User($this->drv); - \Phake::when($this->drv)->userPasswordSet("john.doe@example.com", $this->anything(), $this->anything())->thenReturnCallback(function($user, $pass, $old) { - return $pass ?? "random password"; - }); - \Phake::when($this->drv)->userPasswordSet("jane.doe@example.com", $this->anything(), $this->anything())->thenThrow(new \JKingWeb\Arsse\User\Exception("doesNotExist")); - \Phake::when(Arsse::$db)->userExists->thenReturn($exists); - if ($exp instanceof Exception) { - $this->assertException("doesNotExist", "User"); - $calls = 0; - } else { - $calls = 1; - } + \Phake::when($this->drv)->userAdd->thenThrow(new ExceptionConflict("alreadyExists")); + \Phake::when(Arsse::$db)->userExists->thenReturn(true); + $this->assertException("alreadyExists", "User", "ExceptionConflict"); try { - $this->assertSame($exp, $u->passwordSet($user, $password)); + $u->add($user, $pass); } finally { - \Phake::verify(Arsse::$db, \Phake::times($calls))->userExists($user); - \Phake::verify(Arsse::$db, \Phake::times($exists ? $calls : 0))->userPasswordSet($user, $password ?? "random password", null); + \Phake::verify(Arsse::$db)->userExists($user); + \Phake::verify($this->drv)->userAdd($user, $pass); } } - /** @dataProvider providePasswordChanges */ - public function testChangeAPasswordToARandomPassword(string $user, $password, bool $exists, $exp): void { - $u = \Phake::partialMock(User::class, $this->drv); - \Phake::when($this->drv)->userPasswordSet($this->anything(), $this->isNull(), $this->anything())->thenReturn(null); - \Phake::when($this->drv)->userPasswordSet("john.doe@example.com", $this->logicalNot($this->isNull()), $this->anything())->thenReturnCallback(function($user, $pass, $old) { - return $pass ?? "random password"; - }); - \Phake::when($this->drv)->userPasswordSet("jane.doe@example.com", $this->logicalNot($this->isNull()), $this->anything())->thenThrow(new \JKingWeb\Arsse\User\Exception("doesNotExist")); - \Phake::when(Arsse::$db)->userExists->thenReturn($exists); - if ($exp instanceof Exception) { - $this->assertException("doesNotExist", "User"); - $calls = 2; - } else { - $calls = 4; - } - try { - $pass1 = $u->passwordSet($user, null); - $pass2 = $u->passwordSet($user, null); - $this->assertNotEquals($pass1, $pass2); - } finally { - \Phake::verify($this->drv, \Phake::times($calls))->userPasswordSet; - \Phake::verify($u, \Phake::times($calls / 2))->generatePassword; - \Phake::verify(Arsse::$db, \Phake::times($calls == 4 ? 2 : 0))->userExists($user); - if ($calls == 4) { - \Phake::verify(Arsse::$db, \Phake::times($exists ? 1 : 0))->userPasswordSet($user, $pass1, null); - \Phake::verify(Arsse::$db, \Phake::times($exists ? 1 : 0))->userPasswordSet($user, $pass2, null); - } else { - \Phake::verify(Arsse::$db, \Phake::times(0))->userPasswordSet; - } - } - } - - public function providePasswordChanges(): iterable { - $john = "john.doe@example.com"; - $jane = "jane.doe@example.com"; - return [ - [$john, "superman", true, "superman"], - [$john, null, true, "random password"], - [$john, "superman", false, "superman"], - [$john, null, false, "random password"], - [$jane, "secret", true, new \JKingWeb\Arsse\User\Exception("doesNotExist")], - ]; - } - - /** @dataProvider providePasswordClearings */ - public function testClearAPassword(bool $exists, string $user, $exp): void { - \Phake::when($this->drv)->userPasswordUnset->thenReturn(true); - \Phake::when($this->drv)->userPasswordUnset("jane.doe@example.net", null)->thenThrow(new \JKingWeb\Arsse\User\Exception("doesNotExist")); - \Phake::when(Arsse::$db)->userExists->thenReturn($exists); + public function testAddADuplicateUserWeDoNotKnow(): void { + $user = "ohn.doe@example.com"; + $pass = "secret"; $u = new User($this->drv); + \Phake::when($this->drv)->userAdd->thenThrow(new ExceptionConflict("alreadyExists")); + \Phake::when(Arsse::$db)->userExists->thenReturn(false); + $this->assertException("alreadyExists", "User", "ExceptionConflict"); try { - if ($exp instanceof \JKingWeb\Arsse\AbstractException) { - $this->assertException($exp); - $u->passwordUnset($user); - } else { - $this->assertSame($exp, $u->passwordUnset($user)); - } + $u->add($user, $pass); } finally { - \Phake::verify(Arsse::$db, \Phake::times((int) ($exists && is_bool($exp))))->userPasswordSet($user, null); + \Phake::verify(Arsse::$db)->userExists($user); + \Phake::verify(Arsse::$db)->userAdd($user, null); + \Phake::verify($this->drv)->userAdd($user, $pass); } } - - public function providePasswordClearings(): iterable { - $missing = new \JKingWeb\Arsse\User\Exception("doesNotExist"); - return [ - [true, "jane.doe@example.com", true], - [true, "john.doe@example.com", true], - [true, "jane.doe@example.net", $missing], - [false, "jane.doe@example.com", true], - [false, "john.doe@example.com", true], - [false, "jane.doe@example.net", $missing], - ]; - } }