From e9d449a8ba76373f88c8cb785093683b48b00978 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Mon, 7 Dec 2020 00:07:10 -0500 Subject: [PATCH] Fix user manager and tests --- lib/AbstractException.php | 5 +++-- lib/User.php | 9 ++++++--- locale/en.php | 14 +++++++++++++- tests/cases/User/TestUser.php | 19 +++++++++++-------- 4 files changed, 33 insertions(+), 14 deletions(-) diff --git a/lib/AbstractException.php b/lib/AbstractException.php index d26b3cd3..73a17076 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -74,8 +74,9 @@ abstract class AbstractException extends \Exception { "User/ExceptionConflict.alreadyExists" => 10403, "User/ExceptionSession.invalid" => 10431, "User/ExceptionInput.invalidTimezone" => 10441, - "User/ExceptionInput.invalidBoolean" => 10442, - "User/ExceptionInput.invalidUsername" => 10443, + "User/ExceptionInput.invalidValue" => 10442, + "User/ExceptionInput.invalidNonZeroInteger" => 10443, + "User/ExceptionInput.invalidUsername" => 10444, "Feed/Exception.internalError" => 10500, "Feed/Exception.invalidCertificate" => 10501, "Feed/Exception.invalidUrl" => 10502, diff --git a/lib/User.php b/lib/User.php index 124fd238..bf457a95 100644 --- a/lib/User.php +++ b/lib/User.php @@ -136,7 +136,7 @@ class User { Arsse::$db->userPropertiesSet($user, $extra); } // retrieve from the database to get at least the user number, and anything else the driver does not provide - $meta = Arsse::$db->userPropertiesGet($user); + $meta = Arsse::$db->userPropertiesGet($user, $includeLarge); // combine all the data $out = ['num' => $meta['num']]; foreach (self::PROPERTIES as $k => $t) { @@ -156,8 +156,11 @@ class User { $in = []; foreach (self::PROPERTIES as $k => $t) { if (array_key_exists($k, $data)) { - // TODO: handle type mistmatch exception - $in[$k] = V::normalize($data[$k], $t | V::M_NULL | V::M_STRICT); + try { + $in[$k] = V::normalize($data[$k], $t | V::M_NULL | V::M_STRICT); + } catch (\JKingWeb\Arsse\ExceptionType $e) { + throw new User\ExceptionInput("invalidValue", ['field' => $k, 'type' => $t], $e); + } } } if (isset($in['tz']) && !@timezone_open($in['tz'])) { diff --git a/locale/en.php b/locale/en.php index f58d7b49..cbf3d793 100644 --- a/locale/en.php +++ b/locale/en.php @@ -148,8 +148,20 @@ return [ 'Exception.JKingWeb/Arsse/User/Exception.authFailed' => 'Authentication failed', 'Exception.JKingWeb/Arsse/User/ExceptionSession.invalid' => 'Session with ID {0} does not exist', 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidUsername' => 'User names may not contain the Unicode character {0}', - 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidBoolean' => 'User property "{0}" must be a boolean value (true or false)', + 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidValue' => + 'User property "{field}" must be {type, select, + 1 {null} + 2 {true or false} + 3 {an integer} + 4 {a real number} + 5 {a DateTime object} + 6 {a string} + 7 {an array} + 8 {a DateInterval object} + other {another type} + }', 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidTimezone' => 'User property "{field}" must be a valid zoneinfo timezone', + 'Exception.JKingWeb/Arsse/User/ExceptionInput.invalidNonZeroInteger' => 'User property "{field}" must be greater than zero', 'Exception.JKingWeb/Arsse/Feed/Exception.internalError' => 'Could not download feed "{url}" because of an internal error which is probably a bug', 'Exception.JKingWeb/Arsse/Feed/Exception.invalidCertificate' => 'Could not download feed "{url}" because its server is serving an invalid SSL certificate', 'Exception.JKingWeb/Arsse/Feed/Exception.invalidUrl' => 'Feed URL "{url}" is invalid', diff --git a/tests/cases/User/TestUser.php b/tests/cases/User/TestUser.php index 8863d5fc..84228cac 100644 --- a/tests/cases/User/TestUser.php +++ b/tests/cases/User/TestUser.php @@ -331,13 +331,14 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { /** @dataProvider provideProperties */ public function testGetThePropertiesOfAUser(array $exp, array $base, array $extra): void { $user = "john.doe@example.com"; + $exp = array_merge(['num' => null], array_combine(array_keys(User::PROPERTIES), array_fill(0, sizeof(User::PROPERTIES), null)), $exp); $u = new User($this->drv); \Phake::when($this->drv)->userPropertiesGet->thenReturn($extra); \Phake::when(Arsse::$db)->userPropertiesGet->thenReturn($base); \Phake::when(Arsse::$db)->userExists->thenReturn(true); $this->assertSame($exp, $u->propertiesGet($user)); - \Phake::verify($this->drv)->userPropertiesGet($user); - \Phake::verify(Arsse::$db)->userPropertiesGet($user); + \Phake::verify($this->drv)->userPropertiesGet($user, true); + \Phake::verify(Arsse::$db)->userPropertiesGet($user, true); \Phake::verify(Arsse::$db)->userExists($user); } @@ -356,14 +357,15 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { $extra = ['tz' => "Europe/Istanbul"]; $base = ['num' => 47, 'admin' => false, 'lang' => null, 'tz' => "Etc/UTC", 'sort_asc' => false]; $exp = ['num' => 47, 'admin' => false, 'lang' => null, 'tz' => "Europe/Istanbul", 'sort_asc' => false]; + $exp = array_merge(['num' => null], array_combine(array_keys(User::PROPERTIES), array_fill(0, sizeof(User::PROPERTIES), null)), $exp); $u = new User($this->drv); \Phake::when($this->drv)->userPropertiesGet->thenReturn($extra); \Phake::when(Arsse::$db)->userPropertiesGet->thenReturn($base); \Phake::when(Arsse::$db)->userAdd->thenReturn(true); \Phake::when(Arsse::$db)->userExists->thenReturn(false); $this->assertSame($exp, $u->propertiesGet($user)); - \Phake::verify($this->drv)->userPropertiesGet($user); - \Phake::verify(Arsse::$db)->userPropertiesGet($user); + \Phake::verify($this->drv)->userPropertiesGet($user, true); + \Phake::verify(Arsse::$db)->userPropertiesGet($user, true); \Phake::verify(Arsse::$db)->userPropertiesSet($user, $extra); \Phake::verify(Arsse::$db)->userAdd($user, null); \Phake::verify(Arsse::$db)->userExists($user); @@ -377,7 +379,7 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { try { $u->propertiesGet($user); } finally { - \Phake::verify($this->drv)->userPropertiesGet($user); + \Phake::verify($this->drv)->userPropertiesGet($user, true); } } @@ -421,13 +423,14 @@ class TestUser extends \JKingWeb\Arsse\Test\AbstractTest { public function providePropertyChanges(): iterable { return [ [['admin' => true], ['admin' => true]], - [['admin' => 2], new ExceptionInput("invalidBoolean")], - [['sort_asc' => 2], new ExceptionInput("invalidBoolean")], + [['admin' => 2], new ExceptionInput("invalidValue")], + [['sort_asc' => 2], new ExceptionInput("invalidValue")], [['tz' => "Etc/UTC"], ['tz' => "Etc/UTC"]], [['tz' => "Etc/blah"], new ExceptionInput("invalidTimezone")], - [['tz' => false], new ExceptionInput("invalidTimezone")], + [['tz' => false], new ExceptionInput("invalidValue")], [['lang' => "en-ca"], ['lang' => "en-CA"]], [['lang' => null], ['lang' => null]], + [['page_size' => 0], new ExceptionInput("invalidNonZeroInteger")] ]; }