From d3655294938742c18038bdfa8509edc33731049c Mon Sep 17 00:00:00 2001 From: "J. King" Date: Wed, 27 Sep 2017 22:25:45 -0400 Subject: [PATCH 01/20] Multiple fixes to input sanitization - Database functions now accept any input, but throw typeViolation exceptions where appropriate instead of idMissing or subjectMissing - Added unit tests for the new Misc\ValueInfo static class - Added ValueInfo::id() method to centrally validate database IDs, and made use of it consistently - Made use of PHP's filter_var() function where appropriate when validating or sanitizing input - Made the NCN protocol handler reject most invalid IDs before handing off to method handlers - Made NCN's feedUpdate and subscriptionMove methods return 422 on invalid input - Adjusted several tests to handler type violations --- lib/Database.php | 95 ++++++----- lib/Misc/Context.php | 2 +- lib/Misc/ValueInfo.php | 53 ++++--- lib/REST/AbstractHandler.php | 37 ++--- lib/REST/NextCloudNews/V1_2.php | 53 ++++--- tests/Misc/TestContext.php | 4 +- tests/Misc/TestValueInfo.php | 185 ++++++++++++++++++++++ tests/REST/NextCloudNews/TestNCNV1_2.php | 99 +++++++++--- tests/lib/AbstractTest.php | 4 +- tests/lib/Database/SeriesFeed.php | 5 + tests/lib/Database/SeriesFolder.php | 21 ++- tests/lib/Database/SeriesSubscription.php | 17 +- tests/phpunit.xml | 7 +- 13 files changed, 446 insertions(+), 136 deletions(-) create mode 100644 tests/Misc/TestValueInfo.php diff --git a/lib/Database.php b/lib/Database.php index 9accd6e..3a97177 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -238,17 +238,13 @@ class Database { return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,name) values(?,?,?)", "str", "int", "str")->run($user, $parent, $name)->lastId(); } - public function folderList(string $user, int $parent = null, bool $recursive = true): Db\Result { + public function folderList(string $user, $parent = null, bool $recursive = true): Db\Result { // if the user isn't authorized to perform this action then throw an exception. if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } // check to make sure the parent exists, if one is specified - if (!is_null($parent)) { - if (!$this->db->prepare("SELECT count(*) from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $parent)->getValue()) { - throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "parent", 'id' => $parent]); - } - } + $parent = $this->folderValidateId($user, $parent)['id']; // if we're not returning a recursive list we can use a simpler query if (!$recursive) { return $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and parent is ?", "str", "int")->run($user, $parent); @@ -260,10 +256,13 @@ class Database { } } - public function folderRemove(string $user, int $id): bool { + public function folderRemove(string $user, $id): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id, 'type' => "int > 0"]); + } $changes = $this->db->prepare("DELETE FROM arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); if (!$changes) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); @@ -271,10 +270,13 @@ class Database { return true; } - public function folderPropertiesGet(string $user, int $id): array { + public function folderPropertiesGet(string $user, $id): array { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id, 'type' => "int > 0"]); + } $props = $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow(); if (!$props) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); @@ -282,7 +284,7 @@ class Database { return $props; } - public function folderPropertiesSet(string $user, int $id, array $data): bool { + public function folderPropertiesSet(string $user, $id, array $data): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } @@ -294,14 +296,18 @@ class Database { // if a new name and parent are specified, validate both together $this->folderValidateName($data['name']); $in['name'] = $data['name']; - $in['parent'] = $this->folderValidateMove($user, $id, $data['parent'], $data['name']); + $in['parent'] = $this->folderValidateMove($user, (int) $id, $data['parent'], $data['name']); } elseif ($name) { + // if we're trying to rename the root folder, this simply fails + if (!$id) { + return false; + } // if a new name is specified, validate it $this->folderValidateName($data['name'], true, $in['parent']); $in['name'] = $data['name']; } elseif ($parent) { // if a new parent is specified, validate it - $in['parent'] = $this->folderValidateMove($user, $id, $data['parent']); + $in['parent'] = $this->folderValidateMove($user, (int) $id, $data['parent']); } else { // if neither was specified, do nothing return false; @@ -315,14 +321,13 @@ class Database { } protected function folderValidateId(string $user, $id = null, bool $subject = false): array { - $idInfo = ValueInfo::int($id); - if ($idInfo & (ValueInfo::NULL | ValueInfo::ZERO)) { - // if a null or zero ID is specified this is a no-op - return ['id' => null, 'name' => null, 'parent' => null]; + // if the specified ID is not a non-negative integer (or null), this will always fail + if(!ValueInfo::id($id, true)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "folder", 'type' => "int >= 0"]); } - // if a negative integer or non-integer is specified this will always fail - if (!($idInfo & ValueInfo::VALID) || (($idInfo & ValueInfo::NEG))) { - throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "folder", 'id' => $id]); + // if a null or zero ID is specified this is a no-op + if (!$id) { + return ['id' => null, 'name' => null, 'parent' => null]; } // check whether the folder exists and is owned by the user $f = $this->db->prepare("SELECT id,name,parent from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $id)->getRow(); @@ -417,10 +422,12 @@ class Database { return $this->db->prepare('INSERT INTO arsse_subscriptions(owner,feed) values(?,?)', 'str', 'int')->run($user, $feedID)->lastId(); } - public function subscriptionList(string $user, int $folder = null, int $id = null): Db\Result { + public function subscriptionList(string $user, $folder = null, int $id = null): Db\Result { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + // validate inputs + $folder = $this->folderValidateId($user, $folder)['id']; // create a complex query $q = new Query( "SELECT @@ -439,13 +446,11 @@ class Database { $q->setCTE("user(user)", "SELECT ?", "str", $user); // the subject user; this way we only have to pass it to prepare() once // topmost folders belonging to the user $q->setCTE("topmost(f_id,top)", "SELECT id,id from arsse_folders join user on owner is user where parent is null union select id,top from arsse_folders join topmost on parent=f_id"); - if (!is_null($id)) { + if ($id) { // this condition facilitates the implementation of subscriptionPropertiesGet, which would otherwise have to duplicate the complex query; it takes precedence over a specified folder // if an ID is specified, add a suitable WHERE condition and bindings $q->setWhere("arsse_subscriptions.id is ?", "int", $id); } elseif ($folder) { - // if a folder is specified, make sure it exists - $this->folderValidateId($user, $folder); // if it does exist, add a common table expression to list it and its children so that we select from the entire subtree $q->setCTE("folders(folder)", "SELECT ? union select id from arsse_folders join folders on parent is folder", "int", $folder); // add a suitable WHERE condition @@ -454,24 +459,30 @@ class Database { return $this->db->prepare($q->getQuery(), $q->getTypes())->run($q->getValues()); } - public function subscriptionRemove(string $user, int $id): bool { + public function subscriptionRemove(string $user, $id): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id, 'type' => "int > 0"]); + } $changes = $this->db->prepare("DELETE from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); if (!$changes) { - throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $id]); + throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); } return true; } - public function subscriptionPropertiesGet(string $user, int $id): array { + public function subscriptionPropertiesGet(string $user, $id): array { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id, 'type' => "int > 0"]); + } // disable authorization checks for the list call Arsse::$user->authorizationEnabled(false); - $sub = $this->subscriptionList($user, null, $id)->getRow(); + $sub = $this->subscriptionList($user, null, (int) $id)->getRow(); Arsse::$user->authorizationEnabled(true); if (!$sub) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); @@ -479,15 +490,13 @@ class Database { return $sub; } - public function subscriptionPropertiesSet(string $user, int $id, array $data): bool { + public function subscriptionPropertiesSet(string $user, $id, array $data): bool { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } $tr = $this->db->begin(); - if (!$this->db->prepare("SELECT count(*) from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->getValue()) { - // if the ID doesn't exist or doesn't belong to the user, throw an exception - throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); - } + // validate the ID + $id = $this->subscriptionValidateId($user, $id, true)['id']; if (array_key_exists("folder", $data)) { // ensure the target folder exists and belong to the user $data['folder'] = $this->folderValidateId($user, $data['folder'])['id']; @@ -517,10 +526,13 @@ class Database { return $out; } - protected function subscriptionValidateId(string $user, int $id): array { - $out = $this->db->prepare("SELECT feed from arsse_subscriptions where id is ? and owner is ?", "int", "str")->run($id, $user)->getRow(); + protected function subscriptionValidateId(string $user, $id, bool $subject = false): array { + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "feed", 'id' => $id, 'type' => "int > 0"]); + } + $out = $this->db->prepare("SELECT id,feed from arsse_subscriptions where id is ? and owner is ?", "int", "str")->run($id, $user)->getRow(); if (!$out) { - throw new Db\ExceptionInput("idMissing", ["action" => $this->caller(), "field" => "subscription", 'id' => $id]); + throw new Db\ExceptionInput($subject ? "subjectMissing" : "idMissing", ["action" => $this->caller(), "field" => "subscription", 'id' => $id]); } return $out; } @@ -530,9 +542,12 @@ class Database { return array_column($feeds, 'id'); } - public function feedUpdate(int $feedID, bool $throwError = false): bool { + public function feedUpdate($feedID, bool $throwError = false): bool { $tr = $this->db->begin(); // check to make sure the feed exists + if (!ValueInfo::id($feedID)) { + throw new Db\ExceptionInput("typeViolation", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID, 'type' => "int > 0"]); + } $f = $this->db->prepare("SELECT url, username, password, modified, etag, err_count, scrape FROM arsse_feeds where id is ?", "int")->run($feedID)->getRow(); if (!$f) { throw new Db\ExceptionInput("subjectMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID]); @@ -543,7 +558,7 @@ class Database { // here. When an exception is thrown it should update the database with the // error instead of failing; if other exceptions are thrown, we should simply roll back try { - $feed = new Feed($feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); + $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); if (!$feed->modified) { // if the feed hasn't changed, just compute the next fetch time and record it $this->db->prepare("UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ? WHERE id is ?", 'datetime', 'int')->run($feed->nextFetch, $feedID); @@ -951,7 +966,10 @@ class Database { return true; } - protected function articleValidateId(string $user, int $id): array { + protected function articleValidateId(string $user, $id): array { + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "article", 'id' => $id, 'type' => "int > 0"]); // @codeCoverageIgnore + } $out = $this->db->prepare( "SELECT arsse_articles.id as article, @@ -970,6 +988,9 @@ class Database { } protected function articleValidateEdition(string $user, int $id): array { + if (!ValueInfo::id($id)) { + throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "edition", 'id' => $id, 'type' => "int > 0"]); // @codeCoverageIgnore + } $out = $this->db->prepare( "SELECT arsse_editions.id as edition, diff --git a/lib/Misc/Context.php b/lib/Misc/Context.php index 566d928..01e0528 100644 --- a/lib/Misc/Context.php +++ b/lib/Misc/Context.php @@ -37,7 +37,7 @@ class Context { protected function cleanArray(array $spec): array { $spec = array_values($spec); for ($a = 0; $a < sizeof($spec); $a++) { - if(ValueInfo::int($spec[$a])===ValueInfo::VALID) { + if(ValueInfo::id($spec[$a])) { $spec[$a] = (int) $spec[$a]; } else { $spec[$a] = 0; diff --git a/lib/Misc/ValueInfo.php b/lib/Misc/ValueInfo.php index f6f85e9..bc7e9d3 100644 --- a/lib/Misc/ValueInfo.php +++ b/lib/Misc/ValueInfo.php @@ -15,31 +15,31 @@ class ValueInfo { static public function int($value): int { $out = 0; - // check if the input is null if (is_null($value)) { - $out += self::NULL; - } - // normalize the value to an integer or float if possible - if (is_string($value)) { - if (strval(@intval($value))===$value) { - $value = (int) $value; - } elseif (strval(@floatval($value))===$value) { - $value = (float) $value; - } - // the empty string is equivalent to null when evaluating an integer + // check if the input is null + return self::NULL; + } elseif (is_string($value)) { + // normalize a string an integer or float if possible if (!strlen((string) $value)) { - $out += self::NULL; + // the empty string is equivalent to null when evaluating an integer + return self::NULL; + } elseif (filter_var($value, \FILTER_VALIDATE_FLOAT) !== false && !fmod((float) $value, 1)) { + // an integral float is acceptable + $value = (int) $value; + } else { + return $out; } - } - // if the value is not an integer or integral float, stop - if (!is_int($value) && (!is_float($value) || fmod($value, 1))) { + } elseif (is_float($value) && !fmod($value, 1)) { + // an integral float is acceptable + $value = (int) $value; + } elseif (!is_int($value)) { + // if the value is not an integer or integral float, stop return $out; } // mark validity - $value = (int) $value; $out += self::VALID; // mark zeroness - if(!$value) { + if($value==0) { $out += self::ZERO; } // mark negativeness @@ -55,8 +55,8 @@ class ValueInfo { if (is_null($value)) { $out += self::NULL; } - // if the value is not scalar, it cannot be valid - if (!is_scalar($value)) { + // if the value is not scalar, is a boolean, or is infinity or NaN, it cannot be valid + if (!is_scalar($value) || is_bool($value) || (is_float($value) && !is_finite($value))) { return $out; } // mark validity @@ -70,4 +70,19 @@ class ValueInfo { } return $out; } + + static public function id($value, bool $allowNull = false): bool { + $info = self::int($value); + if ($allowNull && ($info & self::NULL)) { // null (and allowed) + return true; + } elseif (!($info & self::VALID)) { // not an integer + return false; + } elseif ($info & self::NEG) { // negative integer + return false; + } elseif (!$allowNull && ($info & self::ZERO)) { // zero (and not allowed) + return false; + } else { // non-negative integer + return true; + } + } } \ No newline at end of file diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php index 281c7c5..fbc42d4 100644 --- a/lib/REST/AbstractHandler.php +++ b/lib/REST/AbstractHandler.php @@ -32,10 +32,6 @@ abstract class AbstractHandler implements Handler { return $data; } - protected function validateInt($id): bool { - return (bool) (ValueInfo::int($id) & ValueInfo::VALID); - } - protected function NormalizeInput(array $data, array $types, string $dateFormat = null): array { $out = []; foreach ($data as $key => $value) { @@ -49,34 +45,29 @@ abstract class AbstractHandler implements Handler { } switch ($types[$key]) { case "int": - if ($this->validateInt($value)) { + if (valueInfo::int($value) & ValueInfo::VALID) { $out[$key] = (int) $value; } break; case "string": - $out[$key] = (string) $value; + if(is_bool($value)) { + $out[$key] = var_export($value, true); + } elseif (!is_scalar($value)) { + break; + } else { + $out[$key] = (string) $value; + } break; case "bool": - if (is_bool($value)) { - $out[$key] = $value; - } elseif ($this->validateInt($value)) { - $value = (int) $value; - if ($value > -1 && $value < 2) { - $out[$key] = $value; - } - } elseif (is_string($value)) { - $value = trim(strtolower($value)); - if ($value=="false") { - $out[$key] = false; - } - if ($value=="true") { - $out[$key] = true; - } + $test = filter_var($value, \FILTER_VALIDATE_BOOLEAN, \FILTER_NULL_ON_FAILURE); + if (!is_null($test)) { + $out[$key] = $test; } break; case "float": - if (is_numeric($value)) { - $out[$key] = (float) $value; + $test = filter_var($value, \FILTER_VALIDATE_FLOAT); + if ($test !== false) { + $out[$key] = $test; } break; case "datetime": diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index f17a298..caf3d40 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -6,6 +6,7 @@ use JKingWeb\Arsse\Arsse; use JKingWeb\Arsse\User; use JKingWeb\Arsse\Service; use JKingWeb\Arsse\Misc\Context; +use JKingWeb\Arsse\Misc\ValueInfo; use JKingWeb\Arsse\AbstractException; use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Feed\Exception as FeedException; @@ -94,15 +95,15 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { 'items' => [], 'folders' => [ '' => ['GET' => "folderList", 'POST' => "folderAdd"], - '0' => ['PUT' => "folderRename", 'DELETE' => "folderRemove"], - '0/read' => ['PUT' => "folderMarkRead"], + '1' => ['PUT' => "folderRename", 'DELETE' => "folderRemove"], + '1/read' => ['PUT' => "folderMarkRead"], ], 'feeds' => [ '' => ['GET' => "subscriptionList", 'POST' => "subscriptionAdd"], - '0' => ['DELETE' => "subscriptionRemove"], - '0/move' => ['PUT' => "subscriptionMove"], - '0/rename' => ['PUT' => "subscriptionRename"], - '0/read' => ['PUT' => "subscriptionMarkRead"], + '1' => ['DELETE' => "subscriptionRemove"], + '1/move' => ['PUT' => "subscriptionMove"], + '1/rename' => ['PUT' => "subscriptionRename"], + '1/read' => ['PUT' => "subscriptionMarkRead"], 'all' => ['GET' => "feedListStale"], 'update' => ['GET' => "feedUpdate"], ], @@ -110,12 +111,12 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { '' => ['GET' => "articleList"], 'updated' => ['GET' => "articleList"], 'read' => ['PUT' => "articleMarkReadAll"], - '0/read' => ['PUT' => "articleMarkRead"], - '0/unread' => ['PUT' => "articleMarkRead"], + '1/read' => ['PUT' => "articleMarkRead"], + '1/unread' => ['PUT' => "articleMarkRead"], 'read/multiple' => ['PUT' => "articleMarkReadMulti"], 'unread/multiple' => ['PUT' => "articleMarkReadMulti"], - '0/0/star' => ['PUT' => "articleMarkStarred"], - '0/0/unstar' => ['PUT' => "articleMarkStarred"], + '1/1/star' => ['PUT' => "articleMarkStarred"], + '1/1/unstar' => ['PUT' => "articleMarkStarred"], 'star/multiple' => ['PUT' => "articleMarkStarredMulti"], 'unstar/multiple' => ['PUT' => "articleMarkStarredMulti"], ], @@ -135,10 +136,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { ]; // the first path element is the overall scope of the request $scope = $url[0]; - // any URL components which are only digits should be replaced with "0", for easier comparison (integer segments are IDs, and we don't care about the specific ID) + // any URL components which are database IDs (integers greater than zero) should be replaced with "1", for easier comparison (we don't care about the specific ID) for ($a = 0; $a < sizeof($url); $a++) { - if ($this->validateInt($url[$a])) { - $url[$a] = "0"; + if (ValueInfo::id($url[$a])) { + $url[$a] = "1"; } } // normalize the HTTP method to uppercase @@ -336,7 +337,14 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { try { Arsse::$db->feedUpdate($data['feedId']); } catch (ExceptionInput $e) { - return new Response(404); + switch ($e->getCode()) { + case 10239: // feed does not exist + return new Response(404); + case 10237: // feed ID invalid + return new Response(422); + default: // other errors related to input + return new Response(400); // @codeCoverageIgnore + } } return new Response(204); } @@ -347,8 +355,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { if (!isset($data['url'])) { return new Response(422); } - // normalize the folder ID, if specified; zero should be transformed to null - $folder = (isset($data['folderId']) && $data['folderId']) ? $data['folderId'] : null; + // normalize the folder ID, if specified + $folder = isset($data['folderId']) ? $data['folderId'] : null; // try to add the feed $tr = Arsse::$db->begin(); try { @@ -446,12 +454,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], $in); } catch (ExceptionInput $e) { switch ($e->getCode()) { - // subscription does not exist - case 10239: return new Response(404); - // folder does not exist - case 10235: return new Response(422); - // other errors related to input - default: return new Response(400); // @codeCoverageIgnore + case 10239: // subscription does not exist + return new Response(404); + case 10235: // folder does not exist + case 10237: // folder ID is invalid + return new Response(422); + default: // other errors related to input + return new Response(400); // @codeCoverageIgnore } } return new Response(204); diff --git a/tests/Misc/TestContext.php b/tests/Misc/TestContext.php index f03ca41..49ab9bd 100644 --- a/tests/Misc/TestContext.php +++ b/tests/Misc/TestContext.php @@ -47,9 +47,9 @@ class TestContext extends Test\AbstractTest { $this->assertInstanceOf(Context::class, $c->$method($v[$method])); $this->assertTrue($c->$method()); if (in_array($method, $times)) { - $this->assertTime($c->$method, $v[$method]); + $this->assertTime($c->$method, $v[$method], "Context method $method did not return the expected results"); } else { - $this->assertSame($c->$method, $v[$method]); + $this->assertSame($c->$method, $v[$method], "Context method $method did not return the expected results"); } } } diff --git a/tests/Misc/TestValueInfo.php b/tests/Misc/TestValueInfo.php new file mode 100644 index 0000000..8a59055 --- /dev/null +++ b/tests/Misc/TestValueInfo.php @@ -0,0 +1,185 @@ +assertSame($exp, I::int($value), "Test returned ".decbin(I::int($value))." for value: ".var_export($value, true)); + } + } + public function testGetStringInfo() { + $tests = [ + [null, I::NULL], + ["", I::VALID | I::EMPTY], + [1, I::VALID], + [PHP_INT_MAX, I::VALID], + [1.0, I::VALID], + ["1.0", I::VALID], + ["001.0", I::VALID], + ["1.0e2", I::VALID], + ["1", I::VALID], + ["001", I::VALID], + ["1e2", I::VALID], + ["+1.0", I::VALID], + ["+001.0", I::VALID], + ["+1.0e2", I::VALID], + ["+1", I::VALID], + ["+001", I::VALID], + ["+1e2", I::VALID], + [0, I::VALID], + ["0", I::VALID], + ["000", I::VALID], + [0.0, I::VALID], + ["0.0", I::VALID], + ["000.000", I::VALID], + ["+0", I::VALID], + ["+000", I::VALID], + ["+0.0", I::VALID], + ["+000.000", I::VALID], + [-1, I::VALID], + [-1.0, I::VALID], + ["-1.0", I::VALID], + ["-001.0", I::VALID], + ["-1.0e2", I::VALID], + ["-1", I::VALID], + ["-001", I::VALID], + ["-1e2", I::VALID], + [-0, I::VALID], + ["-0", I::VALID], + ["-000", I::VALID], + [-0.0, I::VALID], + ["-0.0", I::VALID], + ["-000.000", I::VALID], + [false, 0], + [true, 0], + [INF, 0], + [-INF, 0], + [NAN, 0], + [[], 0], + ["some string", I::VALID], + [" ", I::VALID | I::WHITE], + [new \StdClass, 0], + ]; + foreach ($tests as $test) { + list($value, $exp) = $test; + $this->assertSame($exp, I::str($value), "Test returned ".decbin(I::str($value))." for value: ".var_export($value, true)); + } + } + + public function testValidateDatabaseIdentifier() { + $tests = [ + [null, false, true], + ["", false, true], + [1, true, true], + [PHP_INT_MAX, true, true], + [1.0, true, true], + ["1.0", true, true], + ["001.0", true, true], + ["1.0e2", true, true], + ["1", true, true], + ["001", true, true], + ["1e2", true, true], + ["+1.0", true, true], + ["+001.0", true, true], + ["+1.0e2", true, true], + ["+1", true, true], + ["+001", true, true], + ["+1e2", true, true], + [0, false, true], + ["0", false, true], + ["000", false, true], + [0.0, false, true], + ["0.0", false, true], + ["000.000", false, true], + ["+0", false, true], + ["+000", false, true], + ["+0.0", false, true], + ["+000.000", false, true], + [-1, false, false], + [-1.0, false, false], + ["-1.0", false, false], + ["-001.0", false, false], + ["-1.0e2", false, false], + ["-1", false, false], + ["-001", false, false], + ["-1e2", false, false], + [-0, false, true], + ["-0", false, true], + ["-000", false, true], + [-0.0, false, true], + ["-0.0", false, true], + ["-000.000", false, true], + [false, false, false], + [true, false, false], + [INF, false, false], + [-INF, false, false], + [NAN, false, false], + [[], false, false], + ["some string", false, false], + [" ", false, false], + [new \StdClass, false, false], + ]; + foreach ($tests as $test) { + list($value, $exp, $expNull) = $test; + $this->assertSame($exp, I::id($value), "Non-null test failed for value: ".var_export($value, true)); + $this->assertSame($expNull, I::id($value, true), "Null test failed for value: ".var_export($value, true)); + } + } +} diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index a137df9..2466599 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -46,6 +46,21 @@ class TestNCNV1_2 extends Test\AbstractTest { 'title' => 'Second example feed', 'unread' => 23, ], + [ + 'id' => 47, + 'url' => 'http://example.net/news.atom', + 'favicon' => 'http://example.net/favicon.png', + 'source' => 'http://example.net/', + 'folder' => null, + 'top_folder' => null, + 'pinned' => 0, + 'err_count' => 0, + 'err_msg' => '', + 'order_type' => 1, + 'added' => '2017-05-20 13:35:54', + 'title' => 'Third example feed', + 'unread' => 0, + ], ], 'rest' => [ [ @@ -76,6 +91,20 @@ class TestNCNV1_2 extends Test\AbstractTest { 'title' => 'Second example feed', 'unreadCount' => 23, ], + [ + 'id' => 47, + 'url' => 'http://example.net/news.atom', + 'faviconLink' => 'http://example.net/favicon.png', + 'link' => 'http://example.net/', + 'folderId' => 0, + 'pinned' => false, + 'updateErrorCount' => 0, + 'lastUpdateError' => '', + 'ordering' => 1, + 'added' => 1495287354, + 'title' => 'Third example feed', + 'unreadCount' => 0, + ], ], ]; protected $articles = [ @@ -331,7 +360,7 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/folders/1", '', 'application/json'))); } - public function testReceiveAuthenticationChallenge() { + public function testSendAuthenticationChallenge() { Phake::when(Arsse::$user)->authHTTP->thenReturn(false); $exp = new Response(401, "", "", ['WWW-Authenticate: Basic realm="'.REST\NextCloudNews\V1_2::REALM.'"']); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/"))); @@ -381,6 +410,7 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders"))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":""}', 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":" "}', 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders", '{"name":{}}', 'application/json'))); // try adding the same two folders again $exp = new Response(409); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/folders?name=Software"))); @@ -457,26 +487,29 @@ class TestNCNV1_2 extends Test\AbstractTest { $in = [ ['url' => "http://example.com/news.atom", 'folderId' => 3], ['url' => "http://example.org/news.atom", 'folderId' => 8], - ['url' => "http://example.net/news.atom", 'folderId' => 0], + ['url' => "http://example.net/news.atom", 'folderId' => 8], + ['url' => "http://example.net/news.atom", 'folderId' => -1], [], ]; $out = [ ['feeds' => [$this->feeds['rest'][0]]], ['feeds' => [$this->feeds['rest'][1]], 'newestItemId' => 4758915], - [], - [], + ['feeds' => [$this->feeds['rest'][2]], 'newestItemId' => 2112], ]; // set up the necessary mocks Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.com/news.atom")->thenReturn(2112)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.org/news.atom")->thenReturn(42)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 2112)->thenReturn($this->feeds['db'][0]); - Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]); + Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]); + Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 47)->thenReturn($this->feeds['db'][2]); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(2112))->thenReturn(0); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(42))->thenReturn(4758915); - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 2112, ['folder' => 3])->thenThrow(new ExceptionInput("idMissing")); // folder ID 3 does not exist - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, ['folder' => 8])->thenReturn(true); - // set up a mock for a bad feed - Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.net/news.atom")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.net/news.atom", new \PicoFeed\Client\InvalidUrlException())); + Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(47))->thenReturn(2112); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 2112, ['folder' => 3])->thenThrow(new ExceptionInput("idMissing")); // folder ID 3 does not exist + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, ['folder' => 8])->thenReturn(true); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 47, ['folder' => -1])->thenThrow(new ExceptionInput("typeViolation")); // folder ID -1 is invalid + // set up a mock for a bad feed which succeeds the second time + Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.net/news.atom")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.net/news.atom", new \PicoFeed\Client\InvalidUrlException()))->thenReturn(47); // add the subscriptions $exp = new Response(200, $out[0]); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[0]), 'application/json'))); @@ -485,14 +518,16 @@ class TestNCNV1_2 extends Test\AbstractTest { // try to add them a second time $exp = new Response(409); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[0]), 'application/json'))); - $exp = new Response(409); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[1]), 'application/json'))); // try to add a bad feed $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[2]), 'application/json'))); + // try again (this will succeed), with an invalid folder ID + $exp = new Response(200, $out[2]); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[3]), 'application/json'))); // try to add no feed $exp = new Response(422); - $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[3]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("POST", "/feeds", json_encode($in[4]), 'application/json'))); } public function testRemoveASubscription() { @@ -511,11 +546,13 @@ class TestNCNV1_2 extends Test\AbstractTest { ['folderId' => 42], ['folderId' => 2112], ['folderId' => 42], + ['folderId' => -1], [], ]; - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => 42])->thenReturn(true); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => 42])->thenReturn(true); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => null])->thenReturn(true); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => 2112])->thenThrow(new ExceptionInput("idMissing")); // folder does not exist + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, ['folder' => -1])->thenThrow(new ExceptionInput("typeViolation")); // folder is invalid Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, $this->anything())->thenThrow(new ExceptionInput("subjectMissing")); // subscription does not exist $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/move", json_encode($in[0]), 'application/json'))); @@ -527,6 +564,8 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/42/move", json_encode($in[3]), 'application/json'))); $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/move", json_encode($in[4]), 'application/json'))); + $exp = new Response(422); + $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/move", json_encode($in[5]), 'application/json'))); } public function testRenameASubscription() { @@ -584,18 +623,20 @@ class TestNCNV1_2 extends Test\AbstractTest { ['feedId' => 42], // valid ['feedId' => 2112], // feed does not exist ['feedId' => "ook"], // invalid ID + ['feedId' => -1], // invalid ID ['feed' => 42], // invalid input ]; Phake::when(Arsse::$db)->feedUpdate(42)->thenReturn(true); Phake::when(Arsse::$db)->feedUpdate(2112)->thenThrow(new ExceptionInput("subjectMissing")); + Phake::when(Arsse::$db)->feedUpdate(-1)->thenThrow(new ExceptionInput("typeViolation")); $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[0]), 'application/json'))); $exp = new Response(404); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[1]), 'application/json'))); $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[2]), 'application/json'))); - $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[3]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[4]), 'application/json'))); // updating a feed when not an admin fails Phake::when(Arsse::$user)->rightsGet->thenReturn(0); $exp = new Response(403); @@ -606,13 +647,15 @@ class TestNCNV1_2 extends Test\AbstractTest { $res = new Result($this->articles['db']); $t = new \DateTime; $in = [ - ['type' => 0, 'id' => 42], - ['type' => 1, 'id' => 2112], - ['type' => 2, 'id' => 0], - ['type' => 3, 'id' => 0], + ['type' => 0, 'id' => 42], // type=0 => subscription/feed + ['type' => 1, 'id' => 2112], // type=1 => folder + ['type' => 0, 'id' => -1], // type=0 => subscription/feed; invalid ID + ['type' => 1, 'id' => -1], // type=1 => folder; invalid ID + ['type' => 2, 'id' => 0], // type=2 => starred + ['type' => 3, 'id' => 0], // type=3 => all (default); base context ['oldestFirst' => true, 'batchSize' => 10, 'offset' => 5], ['oldestFirst' => false, 'batchSize' => 5, 'offset' => 5], - ['getRead' => true], + ['getRead' => true], // base context ['getRead' => false], ['lastModified' => $t->getTimestamp()], ['oldestFirst' => false, 'batchSize' => 5, 'offset' => 0], // offset=0 should not set the latestEdition context @@ -620,6 +663,8 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->articleList(Arsse::$user->id, $this->anything())->thenReturn($res); Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(42))->thenThrow(new ExceptionInput("idMissing")); Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(2112))->thenThrow(new ExceptionInput("idMissing")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(-1))->thenThrow(new ExceptionInput("typeViolation")); + Phake::when(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(-1))->thenThrow(new ExceptionInput("typeViolation")); $exp = new Response(200, ['items' => $this->articles['rest']]); // check the contents of the response $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items"))); // first instance of base context @@ -628,22 +673,26 @@ class TestNCNV1_2 extends Test\AbstractTest { $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[0]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[1]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[2]), 'application/json'))); + $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/items", json_encode($in[3]), 'application/json'))); // simply run through the remainder of the input for later method verification - $this->h->dispatch(new Request("GET", "/items", json_encode($in[2]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[3]), 'application/json')); // third instance of base context $this->h->dispatch(new Request("GET", "/items", json_encode($in[4]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[5]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[6]), 'application/json')); // fourth instance of base context + $this->h->dispatch(new Request("GET", "/items", json_encode($in[5]), 'application/json')); // third instance of base context + $this->h->dispatch(new Request("GET", "/items", json_encode($in[6]), 'application/json')); $this->h->dispatch(new Request("GET", "/items", json_encode($in[7]), 'application/json')); - $this->h->dispatch(new Request("GET", "/items", json_encode($in[8]), 'application/json')); + $this->h->dispatch(new Request("GET", "/items", json_encode($in[8]), 'application/json')); // fourth instance of base context $this->h->dispatch(new Request("GET", "/items", json_encode($in[9]), 'application/json')); + $this->h->dispatch(new Request("GET", "/items", json_encode($in[10]), 'application/json')); + $this->h->dispatch(new Request("GET", "/items", json_encode($in[11]), 'application/json')); // perform method verifications Phake::verify(Arsse::$db, Phake::times(4))->articleList(Arsse::$user->id, (new Context)->reverse(true)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(42)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(2112)); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->subscription(-1)); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->folder(-1)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->starred(true)); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->limit(10)->oldestEdition(6)); - Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)->latestEdition(4)); + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(false)->limit(10)->oldestEdition(6)); // offset is one more than specified + Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)->latestEdition(4)); // offset is one less than specified Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->unread(true)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->modifiedSince($t)); Phake::verify(Arsse::$db)->articleList(Arsse::$user->id, (new Context)->reverse(true)->limit(5)); diff --git a/tests/lib/AbstractTest.php b/tests/lib/AbstractTest.php index 391ae7a..a310cda 100644 --- a/tests/lib/AbstractTest.php +++ b/tests/lib/AbstractTest.php @@ -25,10 +25,10 @@ abstract class AbstractTest extends \PHPUnit\Framework\TestCase { } } - public function assertTime($exp, $test) { + public function assertTime($exp, $test, string $msg = null) { $exp = Date::transform($exp, "iso8601"); $test = Date::transform($test, "iso8601"); - $this->assertSame($exp, $test); + $this->assertSame($exp, $test, $msg); } public function clearData(bool $loadLang = true): bool { diff --git a/tests/lib/Database/SeriesFeed.php b/tests/lib/Database/SeriesFeed.php index d2cd7ed..09312e5 100644 --- a/tests/lib/Database/SeriesFeed.php +++ b/tests/lib/Database/SeriesFeed.php @@ -221,6 +221,11 @@ trait SeriesFeed { Arsse::$db->feedUpdate(2112); } + public function testUpdateAnInvalidFeed() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->feedUpdate(-1); + } + public function testUpdateAFeedThrowingExceptions() { $this->assertException("invalidUrl", "Feed"); Arsse::$db->feedUpdate(3, true); diff --git a/tests/lib/Database/SeriesFolder.php b/tests/lib/Database/SeriesFolder.php index 16312f2..5c0d15b 100644 --- a/tests/lib/Database/SeriesFolder.php +++ b/tests/lib/Database/SeriesFolder.php @@ -77,7 +77,7 @@ trait SeriesFolder { } public function testAddANestedFolderToAnInvalidParent() { - $this->assertException("idMissing", "Db", "ExceptionInput"); + $this->assertException("typeViolation", "Db", "ExceptionInput"); Arsse::$db->folderAdd("john.doe@example.com", ['name' => "Sociology", 'parent' => "stringFolderId"]); } @@ -184,6 +184,11 @@ trait SeriesFolder { Arsse::$db->folderRemove("john.doe@example.com", 2112); } + public function testRemoveAnInvalidFolder() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->folderRemove("john.doe@example.com", -1); + } + public function testRemoveAFolderOfTheWrongOwner() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->folderRemove("john.doe@example.com", 4); // folder ID 4 belongs to Jane @@ -210,6 +215,11 @@ trait SeriesFolder { Arsse::$db->folderPropertiesGet("john.doe@example.com", 2112); } + public function testGetThePropertiesOfAnInvalidFolder() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->folderPropertiesGet("john.doe@example.com", -1); + } + public function testGetThePropertiesOfAFolderOfTheWrongOwner() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->folderPropertiesGet("john.doe@example.com", 4); // folder ID 4 belongs to Jane @@ -233,6 +243,10 @@ trait SeriesFolder { $this->compareExpectations($state); } + public function testRenameTheRootFolder() { + $this->assertFalse(Arsse::$db->folderPropertiesSet("john.doe@example.com", null, ['name' => "Opinion"])); + } + public function testRenameAFolderToTheEmptyString() { $this->assertException("missing", "Db", "ExceptionInput"); $this->assertTrue(Arsse::$db->folderPropertiesSet("john.doe@example.com", 6, ['name' => ""])); @@ -296,6 +310,11 @@ trait SeriesFolder { Arsse::$db->folderPropertiesSet("john.doe@example.com", 2112, ['parent' => null]); } + public function testSetThePropertiesOfAnInvalidFolder() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->folderPropertiesSet("john.doe@example.com", -1, ['parent' => null]); + } + public function testSetThePropertiesOfAFolderForTheWrongOwner() { $this->assertException("subjectMissing", "Db", "ExceptionInput"); Arsse::$db->folderPropertiesSet("john.doe@example.com", 4, ['parent' => null]); // folder ID 4 belongs to Jane diff --git a/tests/lib/Database/SeriesSubscription.php b/tests/lib/Database/SeriesSubscription.php index 12f5760..b68ecb4 100644 --- a/tests/lib/Database/SeriesSubscription.php +++ b/tests/lib/Database/SeriesSubscription.php @@ -193,6 +193,11 @@ trait SeriesSubscription { Arsse::$db->subscriptionRemove($this->user, 2112); } + public function testRemoveAnInvalidSubscription() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->subscriptionRemove($this->user, -1); + } + public function testRemoveASubscriptionForTheWrongOwner() { $this->user = "jane.doe@example.com"; $this->assertException("subjectMissing", "Db", "ExceptionInput"); @@ -264,6 +269,11 @@ trait SeriesSubscription { Arsse::$db->subscriptionPropertiesGet($this->user, 2112); } + public function testGetThePropertiesOfAnInvalidSubscription() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->subscriptionPropertiesGet($this->user, -1); + } + public function testGetThePropertiesOfASubscriptionWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); @@ -311,7 +321,7 @@ trait SeriesSubscription { } public function testRenameASubscriptionToFalse() { - $this->assertException("missing", "Db", "ExceptionInput"); + $this->assertException("typeViolation", "Db", "ExceptionInput"); Arsse::$db->subscriptionPropertiesSet($this->user, 1, ['title' => false]); } @@ -329,6 +339,11 @@ trait SeriesSubscription { Arsse::$db->subscriptionPropertiesSet($this->user, 2112, ['folder' => null]); } + public function testSetThePropertiesOfAnInvalidSubscription() { + $this->assertException("typeViolation", "Db", "ExceptionInput"); + Arsse::$db->subscriptionPropertiesSet($this->user, -1, ['folder' => null]); + } + public function testSetThePropertiesOfASubscriptionWithoutAuthority() { Phake::when(Arsse::$user)->authorize->thenReturn(false); $this->assertException("notAuthorized", "User", "ExceptionAuthz"); diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 2a2b443..1e30465 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -31,6 +31,10 @@ Conf/TestConf.php + + Misc/TestValueInfo.php + Misc/TestContext.php + User/TestUserMockInternal.php User/TestUserMockExternal.php @@ -41,9 +45,6 @@ Feed/TestFeedFetching.php Feed/TestFeed.php - - Misc/TestContext.php - Db/TestTransaction.php Db/SQLite3/TestDbResultSQLite3.php From 5ebf6cb689bad3d9762350b7829b7ba28ca89c8b Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 28 Sep 2017 08:55:47 -0400 Subject: [PATCH 02/20] Treat objects which are convertible to strings the same as actual strings in ValueInfo --- lib/Misc/ValueInfo.php | 12 ++++++++---- tests/Misc/TestValueInfo.php | 19 +++++++++++++++++++ tests/lib/Misc/StrClass.php | 15 +++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) create mode 100644 tests/lib/Misc/StrClass.php diff --git a/lib/Misc/ValueInfo.php b/lib/Misc/ValueInfo.php index bc7e9d3..dd784b1 100644 --- a/lib/Misc/ValueInfo.php +++ b/lib/Misc/ValueInfo.php @@ -18,9 +18,10 @@ class ValueInfo { if (is_null($value)) { // check if the input is null return self::NULL; - } elseif (is_string($value)) { + } elseif (is_string($value) || (is_object($value) && method_exists($value, "__toString"))) { + $value = (string) $value; // normalize a string an integer or float if possible - if (!strlen((string) $value)) { + if (!strlen($value)) { // the empty string is equivalent to null when evaluating an integer return self::NULL; } elseif (filter_var($value, \FILTER_VALIDATE_FLOAT) !== false && !fmod((float) $value, 1)) { @@ -55,8 +56,11 @@ class ValueInfo { if (is_null($value)) { $out += self::NULL; } - // if the value is not scalar, is a boolean, or is infinity or NaN, it cannot be valid - if (!is_scalar($value) || is_bool($value) || (is_float($value) && !is_finite($value))) { + if (is_object($value) && method_exists($value, "__toString")) { + // if the value is an object which has a __toString method, this is acceptable + $value = (string) $value; + } elseif (!is_scalar($value) || is_bool($value) || (is_float($value) && !is_finite($value))) { + // otherwise if the value is not scalar, is a boolean, or is infinity or NaN, it cannot be valid return $out; } // mark validity diff --git a/tests/Misc/TestValueInfo.php b/tests/Misc/TestValueInfo.php index 8a59055..6fac75c 100644 --- a/tests/Misc/TestValueInfo.php +++ b/tests/Misc/TestValueInfo.php @@ -3,6 +3,7 @@ declare(strict_types=1); namespace JKingWeb\Arsse; use JKingWeb\Arsse\Misc\ValueInfo as I; +use JKingWeb\Arsse\Test\Misc\StrClass; /** @covers \JKingWeb\Arsse\Misc\ValueInfo */ class TestValueInfo extends Test\AbstractTest { @@ -58,6 +59,12 @@ class TestValueInfo extends Test\AbstractTest { ["some string", 0], [" ", 0], [new \StdClass, 0], + [new StrClass(""), I::NULL], + [new StrClass("1"), I::VALID], + [new StrClass("0"), I::VALID | I::ZERO], + [new StrClass("-1"), I::VALID | I::NEG], + [new StrClass("Msg"), 0], + [new StrClass(" "), 0], ]; foreach ($tests as $test) { list($value, $exp) = $test; @@ -116,6 +123,12 @@ class TestValueInfo extends Test\AbstractTest { ["some string", I::VALID], [" ", I::VALID | I::WHITE], [new \StdClass, 0], + [new StrClass(""), I::VALID | I::EMPTY], + [new StrClass("1"), I::VALID], + [new StrClass("0"), I::VALID], + [new StrClass("-1"), I::VALID], + [new StrClass("Msg"), I::VALID], + [new StrClass(" "), I::VALID | I::WHITE], ]; foreach ($tests as $test) { list($value, $exp) = $test; @@ -175,6 +188,12 @@ class TestValueInfo extends Test\AbstractTest { ["some string", false, false], [" ", false, false], [new \StdClass, false, false], + [new StrClass(""), false, true], + [new StrClass("1"), true, true], + [new StrClass("0"), false, true], + [new StrClass("-1"), false, false], + [new StrClass("Msg"), false, false], + [new StrClass(" "), false, false], ]; foreach ($tests as $test) { list($value, $exp, $expNull) = $test; diff --git a/tests/lib/Misc/StrClass.php b/tests/lib/Misc/StrClass.php new file mode 100644 index 0000000..d93f2c5 --- /dev/null +++ b/tests/lib/Misc/StrClass.php @@ -0,0 +1,15 @@ +str = (string) $str; + } + + public function __toString() { + return $this->str; + } +} \ No newline at end of file From 96ebf936e489ca03c61d406b151e5e8b78250e61 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 28 Sep 2017 09:01:43 -0400 Subject: [PATCH 03/20] CS fixes --- lib/Database.php | 16 +++++++++++----- lib/Db/SQLite3/Driver.php | 2 +- lib/Misc/Context.php | 2 +- lib/Misc/ValueInfo.php | 10 +++++----- lib/REST/AbstractHandler.php | 2 +- lib/REST/NextCloudNews/V1_2.php | 4 ++-- tests/Misc/TestValueInfo.php | 2 +- tests/REST/NextCloudNews/TestNCNV1_2.php | 8 ++++---- tests/lib/Misc/StrClass.php | 2 +- 9 files changed, 27 insertions(+), 21 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 3a97177..757188d 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -232,7 +232,7 @@ class Database { // normalize folder's parent, if there is one $parent = array_key_exists("parent", $data) ? $this->folderValidateId($user, $data['parent'])['id'] : null; // validate the folder name and parent (if specified); this also checks for duplicates - $name = array_key_exists("name", $data) ? $data['name'] : ""; + $name = array_key_exists("name", $data) ? $data['name'] : ""; $this->folderValidateName($name, true, $parent); // actually perform the insert return $this->db->prepare("INSERT INTO arsse_folders(owner,parent,name) values(?,?,?)", "str", "int", "str")->run($user, $parent, $name)->lastId(); @@ -322,7 +322,7 @@ class Database { protected function folderValidateId(string $user, $id = null, bool $subject = false): array { // if the specified ID is not a non-negative integer (or null), this will always fail - if(!ValueInfo::id($id, true)) { + if (!ValueInfo::id($id, true)) { throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "folder", 'type' => "int >= 0"]); } // if a null or zero ID is specified this is a no-op @@ -358,7 +358,9 @@ class Database { if ($id==$parent) { throw new Db\ExceptionInput("circularDependence", $errData); } - // make sure both that the prospective parent exists, and that the it is not one of its children (a circular dependence) + // make sure both that the prospective parent exists, and that the it is not one of its children (a circular dependence); + // also make sure that a folder with the same prospective name and parent does not already exist: if the parent is null, + // SQL will happily accept duplicates (null is not unique), so we must do this check ourselves $p = $this->db->prepare( "WITH RECURSIVE target as (select ? as user, ? as source, ? as dest, ? as rename), @@ -368,7 +370,7 @@ class Database { ((select dest from target) is null or exists(select id from arsse_folders join target on owner is user and id is dest)) as extant, not exists(select id from folders where id is (select dest from target)) as valid, not exists(select id from arsse_folders join target on parent is dest and name is coalesce((select rename from target),(select name from arsse_folders join target on id is source))) as available - ", "str", "int", "int","str" + ", "str", "int", "int", "str" )->run($user, $id, $parent, $name)->getRow(); if (!$p['extant']) { // if the parent doesn't exist or doesn't below to the user, throw an exception @@ -377,6 +379,7 @@ class Database { // if using the desired parent would create a circular dependence, throw a different exception throw new Db\ExceptionInput("circularDependence", $errData); } elseif (!$p['available']) { + // if a folder with the same parent and name already exists, throw another different exception throw new Db\ExceptionInput("constraintViolation", ["action" => $this->caller(), "field" => (is_null($name) ? "parent" : "name")]); } return $parent; @@ -390,7 +393,10 @@ class Database { throw new Db\ExceptionInput("whitespace", ["action" => $this->caller(), "field" => "name"]); } elseif (!($info & ValueInfo::VALID)) { throw new Db\ExceptionInput("typeViolation", ["action" => $this->caller(), "field" => "name", 'type' => "string"]); - } elseif($checkDuplicates) { + } elseif ($checkDuplicates) { + // make sure that a folder with the same prospective name and parent does not already exist: if the parent is null, + // SQL will happily accept duplicates (null is not unique), so we must do this check ourselves + $parent = $parent ? $parent : null; if ($this->db->prepare("SELECT exists(select id from arsse_folders where parent is ? and name is ?)", "int", "str")->run($parent, $name)->getValue()) { throw new Db\ExceptionInput("constraintViolation", ["action" => $this->caller(), "field" => "name"]); } diff --git a/lib/Db/SQLite3/Driver.php b/lib/Db/SQLite3/Driver.php index b47339e..d65ff2e 100644 --- a/lib/Db/SQLite3/Driver.php +++ b/lib/Db/SQLite3/Driver.php @@ -20,7 +20,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { // check to make sure required extension is loaded if (!class_exists("SQLite3")) { throw new Exception("extMissing", self::driverName()); // @codeCoverageIgnore - } + } // if no database file is specified in the configuration, use a suitable default $dbFile = Arsse::$conf->dbSQLite3File ?? \JKingWeb\Arsse\BASE."arsse.db"; $mode = \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE; diff --git a/lib/Misc/Context.php b/lib/Misc/Context.php index 01e0528..ca37b3d 100644 --- a/lib/Misc/Context.php +++ b/lib/Misc/Context.php @@ -37,7 +37,7 @@ class Context { protected function cleanArray(array $spec): array { $spec = array_values($spec); for ($a = 0; $a < sizeof($spec); $a++) { - if(ValueInfo::id($spec[$a])) { + if (ValueInfo::id($spec[$a])) { $spec[$a] = (int) $spec[$a]; } else { $spec[$a] = 0; diff --git a/lib/Misc/ValueInfo.php b/lib/Misc/ValueInfo.php index dd784b1..040243d 100644 --- a/lib/Misc/ValueInfo.php +++ b/lib/Misc/ValueInfo.php @@ -13,7 +13,7 @@ class ValueInfo { const EMPTY = 1 << 2; const WHITE = 1 << 3; - static public function int($value): int { + public static function int($value): int { $out = 0; if (is_null($value)) { // check if the input is null @@ -40,7 +40,7 @@ class ValueInfo { // mark validity $out += self::VALID; // mark zeroness - if($value==0) { + if ($value==0) { $out += self::ZERO; } // mark negativeness @@ -50,7 +50,7 @@ class ValueInfo { return $out; } - static public function str($value): int { + public static function str($value): int { $out = 0; // check if the input is null if (is_null($value)) { @@ -75,7 +75,7 @@ class ValueInfo { return $out; } - static public function id($value, bool $allowNull = false): bool { + public static function id($value, bool $allowNull = false): bool { $info = self::int($value); if ($allowNull && ($info & self::NULL)) { // null (and allowed) return true; @@ -89,4 +89,4 @@ class ValueInfo { return true; } } -} \ No newline at end of file +} diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php index fbc42d4..8c0988d 100644 --- a/lib/REST/AbstractHandler.php +++ b/lib/REST/AbstractHandler.php @@ -50,7 +50,7 @@ abstract class AbstractHandler implements Handler { } break; case "string": - if(is_bool($value)) { + if (is_bool($value)) { $out[$key] = var_export($value, true); } elseif (!is_scalar($value)) { break; diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index caf3d40..8ece274 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -79,7 +79,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // dispatch try { return $this->$func($req->paths, $data); - // @codeCoverageIgnoreStart + // @codeCoverageIgnoreStart } catch (Exception $e) { // if there was a REST exception return 400 return new Response(400); @@ -340,7 +340,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { switch ($e->getCode()) { case 10239: // feed does not exist return new Response(404); - case 10237: // feed ID invalid + case 10237: // feed ID invalid return new Response(422); default: // other errors related to input return new Response(400); // @codeCoverageIgnore diff --git a/tests/Misc/TestValueInfo.php b/tests/Misc/TestValueInfo.php index 6fac75c..1a75099 100644 --- a/tests/Misc/TestValueInfo.php +++ b/tests/Misc/TestValueInfo.php @@ -197,7 +197,7 @@ class TestValueInfo extends Test\AbstractTest { ]; foreach ($tests as $test) { list($value, $exp, $expNull) = $test; - $this->assertSame($exp, I::id($value), "Non-null test failed for value: ".var_export($value, true)); + $this->assertSame($exp, I::id($value), "Non-null test failed for value: ".var_export($value, true)); $this->assertSame($expNull, I::id($value, true), "Null test failed for value: ".var_export($value, true)); } } diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 2466599..713267b 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -500,14 +500,14 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.com/news.atom")->thenReturn(2112)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.org/news.atom")->thenReturn(42)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 2112)->thenReturn($this->feeds['db'][0]); - Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]); - Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 47)->thenReturn($this->feeds['db'][2]); + Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]); + Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 47)->thenReturn($this->feeds['db'][2]); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(2112))->thenReturn(0); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(42))->thenReturn(4758915); Phake::when(Arsse::$db)->editionLatest(Arsse::$user->id, (new Context)->subscription(47))->thenReturn(2112); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 2112, ['folder' => 3])->thenThrow(new ExceptionInput("idMissing")); // folder ID 3 does not exist - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, ['folder' => 8])->thenReturn(true); - Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 47, ['folder' => -1])->thenThrow(new ExceptionInput("typeViolation")); // folder ID -1 is invalid + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, ['folder' => 8])->thenReturn(true); + Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 47, ['folder' => -1])->thenThrow(new ExceptionInput("typeViolation")); // folder ID -1 is invalid // set up a mock for a bad feed which succeeds the second time Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.net/news.atom")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("http://example.net/news.atom", new \PicoFeed\Client\InvalidUrlException()))->thenReturn(47); // add the subscriptions diff --git a/tests/lib/Misc/StrClass.php b/tests/lib/Misc/StrClass.php index d93f2c5..905194f 100644 --- a/tests/lib/Misc/StrClass.php +++ b/tests/lib/Misc/StrClass.php @@ -12,4 +12,4 @@ class StrClass { public function __toString() { return $this->str; } -} \ No newline at end of file +} From 73d284e101ab4ac7f7973ebee6a20917265c7a36 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 28 Sep 2017 14:17:18 -0400 Subject: [PATCH 04/20] Clean up Git metadata --- .gitattributes | 31 +++++++++++------------------ .gitignore | 53 ++++++++++++++++++++++++-------------------------- 2 files changed, 36 insertions(+), 48 deletions(-) diff --git a/.gitattributes b/.gitattributes index 2431c40..7df9c77 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,22 +1,13 @@ -# Auto detect text files and perform LF normalization -* text=auto +* text=auto encoding=utf-8 -# Custom for Visual Studio -*.cs diff=csharp -*.sln merge=union -*.csproj merge=union -*.vbproj merge=union -*.fsproj merge=union -*.dbproj merge=union +*.html diff=html +*.php diff=php +*.bat eol=crlf +.gitignore -eol -# Standard to msysgit -*.doc diff=astextplain -*.DOC diff=astextplain -*.docx diff=astextplain -*.DOCX diff=astextplain -*.dot diff=astextplain -*.DOT diff=astextplain -*.pdf diff=astextplain -*.PDF diff=astextplain -*.rtf diff=astextplain -*.RTF diff=astextplain + +tests/ export-ignore +.* export-ignore +build.xml export-ignore +composer.* export-ignore +phpdoc.* export-ignore diff --git a/.gitignore b/.gitignore index 9ac5f60..005d958 100644 --- a/.gitignore +++ b/.gitignore @@ -1,48 +1,45 @@ -#dependencies -vendor/ -#temp files +# Temporary files and dependencies + +vendor/ documentation/ -tests/coverage +tests/coverage/ +build/ arsse.db* config.php .php_cs.cache -build -# Windows image file caches + + + +# Windows files + Thumbs.db ehthumbs.db - -# Folder config file Desktop.ini - -# Recycle Bin used on file shares $RECYCLE.BIN/ -# Windows Installer files -*.cab -*.msi -*.msm -*.msp - -# ========================= -# Operating System Files -# ========================= -# OSX -# ========================= +# macOS files .DS_Store .AppleDouble .LSOverride - -# Icon must ends with two \r. Icon - - -# Thumbnails ._* - -# Files that might appear on external disk .Spotlight-V100 .Trashes + +# archives + +*.zip +*.7z +*.tar.gz +*.tgz +*.deb +*.rpm +*.dmg +*.cab +*.msi +*.msm +*.msp From 1ca5b4b4563511b3a2476e7084f69478249287b5 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 28 Sep 2017 19:25:31 -0400 Subject: [PATCH 05/20] Add newline after password in CLI --- lib/CLI.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/CLI.php b/lib/CLI.php index 56dfbad..971197f 100644 --- a/lib/CLI.php +++ b/lib/CLI.php @@ -84,7 +84,7 @@ USAGE_TEXT; public function userAdd(string $user, string $password = null): int { $passwd = Arsse::$user->add($user, $password); if (is_null($password)) { - echo $passwd; + echo $passwd.\PHP_EOL; } return 0; } From 00bc7a00a00df2bb82407180d95cf4e3d628093c Mon Sep 17 00:00:00 2001 From: "J. King" Date: Fri, 29 Sep 2017 17:40:07 -0400 Subject: [PATCH 06/20] Transparently handle HEAD requests; fixes #114 --- lib/REST.php | 8 +++++++- lib/REST/Request.php | 5 +++++ lib/REST/Response.php | 26 +++++++++++++++++--------- 3 files changed, 29 insertions(+), 10 deletions(-) diff --git a/lib/REST.php b/lib/REST.php index 68f5e0b..e102376 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -39,7 +39,13 @@ class REST { $req->refreshURL(); $class = $this->apis[$api]['class']; $drv = new $class(); - return $drv->dispatch($req); + if ($req->head) { + $res = $drv->dispatch($req); + $res->head = true; + return $res; + } else { + return $drv->dispatch($req); + } } public function apiMatch(string $url, array $map): string { diff --git a/lib/REST/Request.php b/lib/REST/Request.php index bdd3a39..2eda047 100644 --- a/lib/REST/Request.php +++ b/lib/REST/Request.php @@ -4,6 +4,7 @@ namespace JKingWeb\Arsse\REST; class Request { public $method = "GET"; + public $head = false; public $url = ""; public $path =""; public $paths = []; @@ -26,6 +27,10 @@ class Request { $this->url = $url; $this->body = $body; $this->type = $contentType; + if($this->method=="HEAD") { + $this->head = true; + $this->method = "GET"; + } $this->refreshURL(); } diff --git a/lib/REST/Response.php b/lib/REST/Response.php index fc18723..5323695 100644 --- a/lib/REST/Response.php +++ b/lib/REST/Response.php @@ -9,6 +9,7 @@ class Response { const T_XML = "application/xml"; const T_TEXT = "text/plain"; + public $head = false; public $code; public $payload; public $type; @@ -24,15 +25,11 @@ class Response { public function output() { if (!headers_sent()) { - try { - $statusText = Arsse::$lang->msg("HTTP.Status.".$this->code); - } catch (\JKingWeb\Arsse\Lang\Exception $e) { - $statusText = ""; + foreach ($this->fields as $field) { + header($field); } - header("Status: ".$this->code." ".$statusText); $body = ""; if (!is_null($this->payload)) { - header("Content-Type: ".$this->type); switch ($this->type) { case self::T_JSON: $body = (string) json_encode($this->payload, \JSON_PRETTY_PRINT); @@ -42,10 +39,21 @@ class Response { break; } } - foreach ($this->fields as $field) { - header($field); + if (strlen($body)) { + header("Content-Type: ".$this->type); + header("Content-Length: ".strlen($body)); + } elseif ($this->code==200) { + $this->code = 204; + } + try { + $statusText = Arsse::$lang->msg("HTTP.Status.".$this->code); + } catch (\JKingWeb\Arsse\Lang\Exception $e) { + $statusText = ""; + } + header("Status: ".$this->code." ".$statusText); + if (!$this->head) { + echo $body; } - echo $body; } else { throw new REST\Exception("headersSent"); } From d1e4c6eed32d9f9578ce16bb4ae6b2a182f57a79 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Fri, 29 Sep 2017 18:11:39 -0400 Subject: [PATCH 07/20] Fix 405 response of NCN version lister --- lib/REST/NextCloudNews/Versions.php | 2 +- tests/REST/NextCloudNews/TestNCNVersionDiscovery.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/REST/NextCloudNews/Versions.php b/lib/REST/NextCloudNews/Versions.php index df260cc..9d5029d 100644 --- a/lib/REST/NextCloudNews/Versions.php +++ b/lib/REST/NextCloudNews/Versions.php @@ -11,7 +11,7 @@ class Versions implements \JKingWeb\Arsse\REST\Handler { public function dispatch(\JKingWeb\Arsse\REST\Request $req): Response { // if a method other than GET was used, this is an error if ($req->method != "GET") { - return new Response(405); + return new Response(405, "", "", ["Allow: GET"]); } if (preg_match("<^/?$>", $req->path)) { // if the request path is an empty string or just a slash, return the supported versions diff --git a/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php b/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php index ad3f15a..ddef865 100644 --- a/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php +++ b/tests/REST/NextCloudNews/TestNCNVersionDiscovery.php @@ -26,7 +26,7 @@ class TestNCNVersionDiscovery extends Test\AbstractTest { } public function testUseIncorrectMethod() { - $exp = new Response(405); + $exp = new Response(405, "", "", ["Allow: GET"]); $h = new REST\NextCloudNews\Versions(); $req = new Request("POST", "/"); $res = $h->dispatch($req); From 3482a35e549370fcff6b4685af83ba74a55fe99d Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sat, 30 Sep 2017 11:43:43 -0400 Subject: [PATCH 08/20] Implement feed discovery; fixes #110 --- lib/Database.php | 8 ++++---- lib/Feed.php | 13 +++++++------ tests/Feed/TestFeed.php | 9 +++++++++ tests/docroot/Feed/Discovery/Feed.php | 12 ++++++++++++ tests/docroot/Feed/Discovery/Invalid.php | 8 ++++++++ tests/docroot/Feed/Discovery/Valid.php | 9 +++++++++ tests/lib/Database/SeriesSubscription.php | 4 ++-- 7 files changed, 51 insertions(+), 12 deletions(-) create mode 100644 tests/docroot/Feed/Discovery/Feed.php create mode 100644 tests/docroot/Feed/Discovery/Invalid.php create mode 100644 tests/docroot/Feed/Discovery/Valid.php diff --git a/lib/Database.php b/lib/Database.php index 757188d..e952404 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -406,7 +406,7 @@ class Database { } } - public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = ""): int { + public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = "", bool $discover = true): int { if (!Arsse::$user->authorize($user, __FUNCTION__)) { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } @@ -417,7 +417,7 @@ class Database { $feedID = $this->db->prepare('INSERT INTO arsse_feeds(url,username,password) values(?,?,?)', 'str', 'str', 'str')->run($url, $fetchUser, $fetchPassword)->lastId(); try { // perform an initial update on the newly added feed - $this->feedUpdate($feedID, true); + $this->feedUpdate($feedID, true, $discover); } catch (\Throwable $e) { // if the update fails, delete the feed we just added $this->db->prepare('DELETE from arsse_feeds where id is ?', 'int')->run($feedID); @@ -548,7 +548,7 @@ class Database { return array_column($feeds, 'id'); } - public function feedUpdate($feedID, bool $throwError = false): bool { + public function feedUpdate($feedID, bool $throwError = false, bool $discover = false): bool { $tr = $this->db->begin(); // check to make sure the feed exists if (!ValueInfo::id($feedID)) { @@ -564,7 +564,7 @@ class Database { // here. When an exception is thrown it should update the database with the // error instead of failing; if other exceptions are thrown, we should simply roll back try { - $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); + $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape, $discover); if (!$feed->modified) { // if the feed hasn't changed, just compute the next fetch time and record it $this->db->prepare("UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ? WHERE id is ?", 'datetime', 'int')->run($feed->nextFetch, $feedID); diff --git a/lib/Feed.php b/lib/Feed.php index a4dc476..ee6114c 100644 --- a/lib/Feed.php +++ b/lib/Feed.php @@ -21,7 +21,7 @@ class Feed { public $newItems = []; public $changedItems = []; - public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false) { + public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false, bool $discover = false) { // set the configuration $userAgent = Arsse::$conf->fetchUserAgentString ?? sprintf('Arsse/%s (%s %s; %s; https://code.jkingweb.ca/jking/arsse) PicoFeed (https://github.com/fguillot/picoFeed)', VERSION, // Arsse version @@ -36,7 +36,7 @@ class Feed { $this->config->setClientUserAgent($userAgent); $this->config->setGrabberUserAgent($userAgent); // fetch the feed - $this->download($url, $lastModified, $etag, $username, $password); + $this->download($url, $lastModified, $etag, $username, $password, $discover); // format the HTTP Last-Modified date returned $lastMod = $this->resource->getLastModified(); if (strlen($lastMod)) { @@ -65,10 +65,11 @@ class Feed { $this->nextFetch = $this->computeNextFetch(); } - protected function download(string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = ''): bool { + protected function download(string $url, string $lastModified, string $etag, string $username, string $password, bool $discover): bool { + $action = $discover ? "discover" : "download"; try { $this->reader = new Reader($this->config); - $this->resource = $this->reader->download($url, $lastModified, $etag, $username, $password); + $this->resource = $this->reader->$action($url, $lastModified, $etag, $username, $password); } catch (PicoFeedException $e) { throw new Feed\Exception($url, $e); } @@ -361,13 +362,13 @@ class Feed { protected function computeLastModified() { if (!$this->modified) { - return $this->lastModified; + return $this->lastModified; // @codeCoverageIgnore } $dates = $this->gatherDates(); if (sizeof($dates)) { return Date::normalize($dates[0]); } else { - return null; + return null; // @codeCoverageIgnore } } diff --git a/tests/Feed/TestFeed.php b/tests/Feed/TestFeed.php index 74c2426..8957ba8 100644 --- a/tests/Feed/TestFeed.php +++ b/tests/Feed/TestFeed.php @@ -133,6 +133,15 @@ class TestFeed extends Test\AbstractTest { $this->assertSame($categories, $f->data->items[5]->categories); } + public function testDiscoverAFeedSuccessfully() { + $this->assertInstanceOf(Feed::class, new Feed(null, $this->base."Discovery/Valid", "", "", "", "", false, true)); + } + + public function testDiscoverAFeedUnsuccessfully() { + $this->assertException("subscriptionNotFound", "Feed"); + new Feed(null, $this->base."Discovery/Invalid", "", "", "", "", false, true); + } + public function testParseEntityExpansionAttack() { $this->assertException("xmlEntity", "Feed"); new Feed(null, $this->base."Parsing/XEEAttack"); diff --git a/tests/docroot/Feed/Discovery/Feed.php b/tests/docroot/Feed/Discovery/Feed.php new file mode 100644 index 0000000..a13398a --- /dev/null +++ b/tests/docroot/Feed/Discovery/Feed.php @@ -0,0 +1,12 @@ + "application/rss+xml", + 'content' => << + + Test feed + http://example.com/ + Example newsfeed title + + +MESSAGE_BODY +]; diff --git a/tests/docroot/Feed/Discovery/Invalid.php b/tests/docroot/Feed/Discovery/Invalid.php new file mode 100644 index 0000000..59f7e6e --- /dev/null +++ b/tests/docroot/Feed/Discovery/Invalid.php @@ -0,0 +1,8 @@ + "text/html", + 'content' => << +Example article + +MESSAGE_BODY +]; \ No newline at end of file diff --git a/tests/docroot/Feed/Discovery/Valid.php b/tests/docroot/Feed/Discovery/Valid.php new file mode 100644 index 0000000..293ea19 --- /dev/null +++ b/tests/docroot/Feed/Discovery/Valid.php @@ -0,0 +1,9 @@ + "text/html", + 'content' => << +Example article + + +MESSAGE_BODY +]; \ No newline at end of file diff --git a/tests/lib/Database/SeriesSubscription.php b/tests/lib/Database/SeriesSubscription.php index b68ecb4..fc528c3 100644 --- a/tests/lib/Database/SeriesSubscription.php +++ b/tests/lib/Database/SeriesSubscription.php @@ -135,7 +135,7 @@ trait SeriesSubscription { Phake::when(Arsse::$db)->feedUpdate->thenReturn(true); $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url)); Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); - Phake::verify(Arsse::$db)->feedUpdate($feedID, true); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true, true); $state = $this->primeExpectations($this->data, [ 'arsse_feeds' => ['id','url','username','password'], 'arsse_subscriptions' => ['id','owner','feed'], @@ -153,7 +153,7 @@ trait SeriesSubscription { Arsse::$db->subscriptionAdd($this->user, $url); } catch (FeedException $e) { Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); - Phake::verify(Arsse::$db)->feedUpdate($feedID, true); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true, true); $state = $this->primeExpectations($this->data, [ 'arsse_feeds' => ['id','url','username','password'], 'arsse_subscriptions' => ['id','owner','feed'], From 474f7fc2f6e75acfa4898a4d2c561f6f73c80ff1 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sat, 30 Sep 2017 12:05:57 -0400 Subject: [PATCH 09/20] Changelog; CS fixes --- CHANGELOG | 12 ++++++++++++ build.xml | 1 + dist/nginx-fcgi.conf | 3 ++- lib/REST.php | 6 +++--- lib/REST/Request.php | 2 +- tests/docroot/Feed/Discovery/Invalid.php | 2 +- tests/docroot/Feed/Discovery/Valid.php | 2 +- 7 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 CHANGELOG diff --git a/CHANGELOG b/CHANGELOG new file mode 100644 index 0000000..451ce8a --- /dev/null +++ b/CHANGELOG @@ -0,0 +1,12 @@ +Version 0.1.1 (2017-09-30) +========================== + +Bug fixes: +- Perform feed discovery like NextCloud News does +- Respond correctly to HEAD requests +- Various minor fixes + +Version 0.1.0 (2017-08-29) +========================== + +Initial release \ No newline at end of file diff --git a/build.xml b/build.xml index 5524305..25753f7 100644 --- a/build.xml +++ b/build.xml @@ -11,6 +11,7 @@ + diff --git a/dist/nginx-fcgi.conf b/dist/nginx-fcgi.conf index fc76d31..fb37825 100644 --- a/dist/nginx-fcgi.conf +++ b/dist/nginx-fcgi.conf @@ -9,4 +9,5 @@ fastcgi_param REQUEST_METHOD $request_method; fastcgi_param CONTENT_TYPE $content_type; fastcgi_param CONTENT_LENGTH $content_length; fastcgi_param REQUEST_URI $request_uri; -fastcgi_param HTTPS $https if_not_empty; \ No newline at end of file +fastcgi_param HTTPS $https if_not_empty; +fastcgi_param REMOTE_USER $remote_user; \ No newline at end of file diff --git a/lib/REST.php b/lib/REST.php index e102376..f0a5b37 100644 --- a/lib/REST.php +++ b/lib/REST.php @@ -40,9 +40,9 @@ class REST { $class = $this->apis[$api]['class']; $drv = new $class(); if ($req->head) { - $res = $drv->dispatch($req); - $res->head = true; - return $res; + $res = $drv->dispatch($req); + $res->head = true; + return $res; } else { return $drv->dispatch($req); } diff --git a/lib/REST/Request.php b/lib/REST/Request.php index 2eda047..c21ca48 100644 --- a/lib/REST/Request.php +++ b/lib/REST/Request.php @@ -27,7 +27,7 @@ class Request { $this->url = $url; $this->body = $body; $this->type = $contentType; - if($this->method=="HEAD") { + if ($this->method=="HEAD") { $this->head = true; $this->method = "GET"; } diff --git a/tests/docroot/Feed/Discovery/Invalid.php b/tests/docroot/Feed/Discovery/Invalid.php index 59f7e6e..9a2f49f 100644 --- a/tests/docroot/Feed/Discovery/Invalid.php +++ b/tests/docroot/Feed/Discovery/Invalid.php @@ -5,4 +5,4 @@ Example article MESSAGE_BODY -]; \ No newline at end of file +]; diff --git a/tests/docroot/Feed/Discovery/Valid.php b/tests/docroot/Feed/Discovery/Valid.php index 293ea19..9f34f71 100644 --- a/tests/docroot/Feed/Discovery/Valid.php +++ b/tests/docroot/Feed/Discovery/Valid.php @@ -6,4 +6,4 @@ MESSAGE_BODY -]; \ No newline at end of file +]; From bf35e0ecc88d5b3244f13b827cb8a7a1dbae3dee Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sat, 30 Sep 2017 12:08:05 -0400 Subject: [PATCH 10/20] Version bump --- bootstrap.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bootstrap.php b/bootstrap.php index 8ba3a73..04b4f0b 100644 --- a/bootstrap.php +++ b/bootstrap.php @@ -4,7 +4,7 @@ namespace JKingWeb\Arsse; const BASE = __DIR__.DIRECTORY_SEPARATOR; const NS_BASE = __NAMESPACE__."\\"; -const VERSION = "0.1.0"; +const VERSION = "0.1.1"; require_once BASE."vendor".DIRECTORY_SEPARATOR."autoload.php"; ignore_user_abort(true); \ No newline at end of file From 1b72d45adf4cf1f4fe89843a0651e8e2a44c67b2 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 1 Oct 2017 09:33:49 -0400 Subject: [PATCH 11/20] Relegate bootstrap.php to testing; fixes #117 --- arsse.php | 7 ++++++- bootstrap.php | 10 ---------- build.xml | 1 - lib/Arsse.php | 2 ++ lib/CLI.php | 2 +- lib/Feed.php | 4 ++-- lib/REST/NextCloudNews/V1_2.php | 4 ++-- tests/REST/NextCloudNews/TestNCNV1_2.php | 4 ++-- tests/bootstrap.php | 8 ++++++++ tests/phpunit.xml | 2 +- tests/server.php | 2 +- 11 files changed, 25 insertions(+), 21 deletions(-) delete mode 100644 bootstrap.php create mode 100644 tests/bootstrap.php diff --git a/arsse.php b/arsse.php index 8634bf5..c1fd508 100644 --- a/arsse.php +++ b/arsse.php @@ -1,7 +1,12 @@ - diff --git a/lib/Arsse.php b/lib/Arsse.php index 2475ed6..a453bd7 100644 --- a/lib/Arsse.php +++ b/lib/Arsse.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse; class Arsse { + const VERSION = "0.1.1"; + /** @var Lang */ public static $lang; /** @var Conf */ diff --git a/lib/CLI.php b/lib/CLI.php index 971197f..36aa8db 100644 --- a/lib/CLI.php +++ b/lib/CLI.php @@ -27,7 +27,7 @@ USAGE_TEXT; $this->args = \Docopt::handle($this->usage(), [ 'argv' => $argv, 'help' => true, - 'version' => VERSION, + 'version' => Arsse::VERSION, ]); } diff --git a/lib/Feed.php b/lib/Feed.php index ee6114c..f256016 100644 --- a/lib/Feed.php +++ b/lib/Feed.php @@ -20,11 +20,11 @@ class Feed { public $nextFetch; public $newItems = []; public $changedItems = []; - + public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false, bool $discover = false) { // set the configuration $userAgent = Arsse::$conf->fetchUserAgentString ?? sprintf('Arsse/%s (%s %s; %s; https://code.jkingweb.ca/jking/arsse) PicoFeed (https://github.com/fguillot/picoFeed)', - VERSION, // Arsse version + Arsse::VERSION, // Arsse version php_uname('s'), // OS php_uname('r'), // OS version php_uname('m') // platform architecture diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index 8ece274..bab902e 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -691,14 +691,14 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { protected function serverVersion(array $url, array $data): Response { return new Response(200, [ 'version' => self::VERSION, - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, ]); } protected function serverStatus(array $url, array $data): Response { return new Response(200, [ 'version' => self::VERSION, - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, 'warnings' => [ 'improperlyConfiguredCron' => !Service::hasCheckedIn(), ] diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 713267b..0d8e08e 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -458,7 +458,7 @@ class TestNCNV1_2 extends Test\AbstractTest { public function testRetrieveServerVersion() { $exp = new Response(200, [ - 'arsse_version' => \JKingWeb\Arsse\VERSION, + 'arsse_version' => Arsse::VERSION, 'version' => REST\NextCloudNews\V1_2::VERSION, ]); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/version"))); @@ -842,7 +842,7 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->metaGet("service_last_checkin")->thenReturn(Date::transform($valid, "sql"))->thenReturn(Date::transform($invalid, "sql")); $arr1 = $arr2 = [ 'version' => REST\NextCloudNews\V1_2::VERSION, - 'arsse_version' => VERSION, + 'arsse_version' => Arsse::VERSION, 'warnings' => [ 'improperlyConfiguredCron' => false, ] diff --git a/tests/bootstrap.php b/tests/bootstrap.php new file mode 100644 index 0000000..c74e551 --- /dev/null +++ b/tests/bootstrap.php @@ -0,0 +1,8 @@ + Date: Mon, 2 Oct 2017 11:53:52 -0400 Subject: [PATCH 12/20] Perform feed discovery correctly; fixes #118 --- lib/Database.php | 16 +++-- lib/Feed.php | 75 +++++++++++++++-------- tests/Feed/TestFeed.php | 5 +- tests/lib/Database/SeriesSubscription.php | 26 ++++++-- 4 files changed, 84 insertions(+), 38 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index e952404..593d5e0 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -411,13 +411,19 @@ class Database { throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); } // check to see if the feed exists - $feedID = $this->db->prepare("SELECT id from arsse_feeds where url is ? and username is ? and password is ?", "str", "str", "str")->run($url, $fetchUser, $fetchPassword)->getValue(); + $check = $this->db->prepare("SELECT id from arsse_feeds where url is ? and username is ? and password is ?", "str", "str", "str"); + $feedID = $check->run($url, $fetchUser, $fetchPassword)->getValue(); + if ($discover && is_null($feedID)) { + // if the feed doesn't exist, first perform discovery if requested and check for the existence of that URL + $url = Feed::discover($url, $fetchUser, $fetchPassword); + $feedID = $check->run($url, $fetchUser, $fetchPassword)->getValue(); + } if (is_null($feedID)) { - // if the feed doesn't exist add it to the database; we do this unconditionally so as to lock SQLite databases for as little time as possible + // if the feed still doesn't exist in the database, add it to the database; we do this unconditionally so as to lock SQLite databases for as little time as possible $feedID = $this->db->prepare('INSERT INTO arsse_feeds(url,username,password) values(?,?,?)', 'str', 'str', 'str')->run($url, $fetchUser, $fetchPassword)->lastId(); try { // perform an initial update on the newly added feed - $this->feedUpdate($feedID, true, $discover); + $this->feedUpdate($feedID, true); } catch (\Throwable $e) { // if the update fails, delete the feed we just added $this->db->prepare('DELETE from arsse_feeds where id is ?', 'int')->run($feedID); @@ -548,7 +554,7 @@ class Database { return array_column($feeds, 'id'); } - public function feedUpdate($feedID, bool $throwError = false, bool $discover = false): bool { + public function feedUpdate($feedID, bool $throwError = false): bool { $tr = $this->db->begin(); // check to make sure the feed exists if (!ValueInfo::id($feedID)) { @@ -564,7 +570,7 @@ class Database { // here. When an exception is thrown it should update the database with the // error instead of failing; if other exceptions are thrown, we should simply roll back try { - $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape, $discover); + $feed = new Feed((int) $feedID, $f['url'], (string) Date::transform($f['modified'], "http", "sql"), $f['etag'], $f['username'], $f['password'], $scrape); if (!$feed->modified) { // if the feed hasn't changed, just compute the next fetch time and record it $this->db->prepare("UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ? WHERE id is ?", 'datetime', 'int')->run($feed->nextFetch, $feedID); diff --git a/lib/Feed.php b/lib/Feed.php index f256016..a390659 100644 --- a/lib/Feed.php +++ b/lib/Feed.php @@ -5,6 +5,7 @@ namespace JKingWeb\Arsse; use JKingWeb\Arsse\Misc\Date; use PicoFeed\PicoFeedException; use PicoFeed\Config\Config; +use PicoFeed\Client\Client; use PicoFeed\Reader\Reader; use PicoFeed\Reader\Favicon; use PicoFeed\Scraper\Scraper; @@ -12,31 +13,37 @@ use PicoFeed\Scraper\Scraper; class Feed { public $data = null; public $favicon; - public $parser; - public $reader; public $resource; public $modified = false; public $lastModified; public $nextFetch; public $newItems = []; public $changedItems = []; + + public static function discover(string $url, string $username = '', string $password = ''): string { + // fetch the candidate feed + $f = self::download($url, "", "", $username, $password); + if ($f->reader->detectFormat($f->getContent())) { + // if the prospective URL is a feed, use it + $out = $url; + } else { + $links = $f->reader->find($f->getUrl(), $f->getContent()); + if (!$links) { + // work around a PicoFeed memory leak FIXME: remove this hack (or not) once PicoFeed stops leaking memory + libxml_use_internal_errors(false); + throw new Feed\Exception($url, new \PicoFeed\Reader\SubscriptionNotFoundException('Unable to find a subscription')); + } else { + $out = $links[0]; + } + } + // work around a PicoFeed memory leak FIXME: remove this hack (or not) once PicoFeed stops leaking memory + libxml_use_internal_errors(false); + return $out; + } - public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false, bool $discover = false) { - // set the configuration - $userAgent = Arsse::$conf->fetchUserAgentString ?? sprintf('Arsse/%s (%s %s; %s; https://code.jkingweb.ca/jking/arsse) PicoFeed (https://github.com/fguillot/picoFeed)', - Arsse::VERSION, // Arsse version - php_uname('s'), // OS - php_uname('r'), // OS version - php_uname('m') // platform architecture - ); - $this->config = new Config; - $this->config->setMaxBodySize(Arsse::$conf->fetchSizeLimit); - $this->config->setClientTimeout(Arsse::$conf->fetchTimeout); - $this->config->setGrabberTimeout(Arsse::$conf->fetchTimeout); - $this->config->setClientUserAgent($userAgent); - $this->config->setGrabberUserAgent($userAgent); + public function __construct(int $feedID = null, string $url, string $lastModified = '', string $etag = '', string $username = '', string $password = '', bool $scrape = false) { // fetch the feed - $this->download($url, $lastModified, $etag, $username, $password, $discover); + $this->resource = self::download($url, $lastModified, $etag, $username, $password); // format the HTTP Last-Modified date returned $lastMod = $this->resource->getLastModified(); if (strlen($lastMod)) { @@ -65,26 +72,40 @@ class Feed { $this->nextFetch = $this->computeNextFetch(); } - protected function download(string $url, string $lastModified, string $etag, string $username, string $password, bool $discover): bool { - $action = $discover ? "discover" : "download"; + protected static function configure(): Config { + $userAgent = Arsse::$conf->fetchUserAgentString ?? sprintf('Arsse/%s (%s %s; %s; https://thearsse.com/) PicoFeed (https://github.com/miniflux/picoFeed)', + Arsse::VERSION, // Arsse version + php_uname('s'), // OS + php_uname('r'), // OS version + php_uname('m') // platform architecture + ); + $config = new Config; + $config->setMaxBodySize(Arsse::$conf->fetchSizeLimit); + $config->setClientTimeout(Arsse::$conf->fetchTimeout); + $config->setGrabberTimeout(Arsse::$conf->fetchTimeout); + $config->setClientUserAgent($userAgent); + $config->setGrabberUserAgent($userAgent); + return $config; + } + + protected static function download(string $url, string $lastModified, string $etag, string $username, string $password): Client { try { - $this->reader = new Reader($this->config); - $this->resource = $this->reader->$action($url, $lastModified, $etag, $username, $password); + $reader = new Reader(self::configure()); + $client = $reader->download($url, $lastModified, $etag, $username, $password); + $client->reader = $reader; + return $client; } catch (PicoFeedException $e) { throw new Feed\Exception($url, $e); } - return true; } protected function parse(): bool { try { - $this->parser = $this->reader->getParser( + $feed = $this->resource->reader->getParser( $this->resource->getUrl(), $this->resource->getContent(), $this->resource->getEncoding() - ); - $feed = $this->parser->execute(); - + )->execute(); // Grab the favicon for the feed; returns an empty string if it cannot find one. // Some feeds might use a different domain (eg: feedburner), so the site url is // used instead of the feed's url. @@ -388,7 +409,7 @@ class Feed { } protected function scrape(): bool { - $scraper = new Scraper($this->config); + $scraper = new Scraper(self::configure()); foreach (array_merge($this->newItems, $this->changedItems) as $item) { $scraper->setUrl($item->url); $scraper->execute(); diff --git a/tests/Feed/TestFeed.php b/tests/Feed/TestFeed.php index 8957ba8..0ef1eb4 100644 --- a/tests/Feed/TestFeed.php +++ b/tests/Feed/TestFeed.php @@ -134,12 +134,13 @@ class TestFeed extends Test\AbstractTest { } public function testDiscoverAFeedSuccessfully() { - $this->assertInstanceOf(Feed::class, new Feed(null, $this->base."Discovery/Valid", "", "", "", "", false, true)); + $this->assertSame($this->base."Discovery/Feed", Feed::discover($this->base."Discovery/Valid")); + $this->assertSame($this->base."Discovery/Feed", Feed::discover($this->base."Discovery/Feed")); } public function testDiscoverAFeedUnsuccessfully() { $this->assertException("subscriptionNotFound", "Feed"); - new Feed(null, $this->base."Discovery/Invalid", "", "", "", "", false, true); + Feed::discover($this->base."Discovery/Invalid"); } public function testParseEntityExpansionAttack() { diff --git a/tests/lib/Database/SeriesSubscription.php b/tests/lib/Database/SeriesSubscription.php index fc528c3..a40e509 100644 --- a/tests/lib/Database/SeriesSubscription.php +++ b/tests/lib/Database/SeriesSubscription.php @@ -133,9 +133,9 @@ trait SeriesSubscription { $feedID = $this->nextID("arsse_feeds"); $subID = $this->nextID("arsse_subscriptions"); Phake::when(Arsse::$db)->feedUpdate->thenReturn(true); - $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url)); + $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url, "", "", false)); Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); - Phake::verify(Arsse::$db)->feedUpdate($feedID, true, true); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true); $state = $this->primeExpectations($this->data, [ 'arsse_feeds' => ['id','url','username','password'], 'arsse_subscriptions' => ['id','owner','feed'], @@ -145,15 +145,33 @@ trait SeriesSubscription { $this->compareExpectations($state); } + public function testAddASubscriptionToANewFeedViaDiscovery() { + $url = "http://localhost:8000/Feed/Discovery/Valid"; + $discovered = "http://localhost:8000/Feed/Discovery/Feed"; + $feedID = $this->nextID("arsse_feeds"); + $subID = $this->nextID("arsse_subscriptions"); + Phake::when(Arsse::$db)->feedUpdate->thenReturn(true); + $this->assertSame($subID, Arsse::$db->subscriptionAdd($this->user, $url, "", "", true)); + Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true); + $state = $this->primeExpectations($this->data, [ + 'arsse_feeds' => ['id','url','username','password'], + 'arsse_subscriptions' => ['id','owner','feed'], + ]); + $state['arsse_feeds']['rows'][] = [$feedID,$discovered,"",""]; + $state['arsse_subscriptions']['rows'][] = [$subID,$this->user,$feedID]; + $this->compareExpectations($state); + } + public function testAddASubscriptionToAnInvalidFeed() { $url = "http://example.org/feed1"; $feedID = $this->nextID("arsse_feeds"); Phake::when(Arsse::$db)->feedUpdate->thenThrow(new FeedException($url, new \PicoFeed\Client\InvalidUrlException())); try { - Arsse::$db->subscriptionAdd($this->user, $url); + Arsse::$db->subscriptionAdd($this->user, $url, "", "", false); } catch (FeedException $e) { Phake::verify(Arsse::$user)->authorize($this->user, "subscriptionAdd"); - Phake::verify(Arsse::$db)->feedUpdate($feedID, true, true); + Phake::verify(Arsse::$db)->feedUpdate($feedID, true); $state = $this->primeExpectations($this->data, [ 'arsse_feeds' => ['id','url','username','password'], 'arsse_subscriptions' => ['id','owner','feed'], From 661515590e66c2b1ef662631a9c029314982f15c Mon Sep 17 00:00:00 2001 From: "J. King" Date: Tue, 17 Oct 2017 17:25:27 -0400 Subject: [PATCH 13/20] Update dependencies --- composer.lock | 275 +++++++++++++++++++++++++++++++------------------- 1 file changed, 169 insertions(+), 106 deletions(-) diff --git a/composer.lock b/composer.lock index ca0b748..3cd05e4 100644 --- a/composer.lock +++ b/composer.lock @@ -310,6 +310,68 @@ ], "time": "2012-12-19T10:50:58+00:00" }, + { + "name": "composer/semver", + "version": "1.4.2", + "source": { + "type": "git", + "url": "https://github.com/composer/semver.git", + "reference": "c7cb9a2095a074d131b65a8a0cd294479d785573" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/composer/semver/zipball/c7cb9a2095a074d131b65a8a0cd294479d785573", + "reference": "c7cb9a2095a074d131b65a8a0cd294479d785573", + "shasum": "" + }, + "require": { + "php": "^5.3.2 || ^7.0" + }, + "require-dev": { + "phpunit/phpunit": "^4.5 || ^5.0.5", + "phpunit/phpunit-mock-objects": "2.3.0 || ^3.0" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "1.x-dev" + } + }, + "autoload": { + "psr-4": { + "Composer\\Semver\\": "src" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "authors": [ + { + "name": "Nils Adermann", + "email": "naderman@naderman.de", + "homepage": "http://www.naderman.de" + }, + { + "name": "Jordi Boggiano", + "email": "j.boggiano@seld.be", + "homepage": "http://seld.be" + }, + { + "name": "Rob Bast", + "email": "rob.bast@gmail.com", + "homepage": "http://robbast.nl" + } + ], + "description": "Semver library that offers utilities, version constraint parsing and validation.", + "keywords": [ + "semantic", + "semver", + "validation", + "versioning" + ], + "time": "2016-08-30T16:08:34+00:00" + }, { "name": "container-interop/container-interop", "version": "1.2.0", @@ -561,24 +623,25 @@ }, { "name": "friendsofphp/php-cs-fixer", - "version": "v2.2.6", + "version": "v2.2.8", "source": { "type": "git", "url": "https://github.com/FriendsOfPHP/PHP-CS-Fixer.git", - "reference": "c1cc52c242f17c4d52d9601159631da488fac7a4" + "reference": "aca23e791784eade7b377d578d6dfc6fcf1398d2" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/FriendsOfPHP/PHP-CS-Fixer/zipball/c1cc52c242f17c4d52d9601159631da488fac7a4", - "reference": "c1cc52c242f17c4d52d9601159631da488fac7a4", + "url": "https://api.github.com/repos/FriendsOfPHP/PHP-CS-Fixer/zipball/aca23e791784eade7b377d578d6dfc6fcf1398d2", + "reference": "aca23e791784eade7b377d578d6dfc6fcf1398d2", "shasum": "" }, "require": { + "composer/semver": "^1.4", "doctrine/annotations": "^1.2", "ext-json": "*", "ext-tokenizer": "*", "gecko-packages/gecko-php-unit": "^2.0", - "php": "^5.3.6 || >=7.0 <7.2", + "php": "^5.3.6 || >=7.0 <7.3", "sebastian/diff": "^1.4", "symfony/console": "^2.4 || ^3.0", "symfony/event-dispatcher": "^2.1 || ^3.0", @@ -641,7 +704,7 @@ } ], "description": "A tool to automatically fix PHP code style", - "time": "2017-08-22T14:08:16+00:00" + "time": "2017-09-29T15:07:49+00:00" }, { "name": "gecko-packages/gecko-php-unit", @@ -936,16 +999,16 @@ }, { "name": "jms/serializer", - "version": "1.8.1", + "version": "1.9.0", "source": { "type": "git", "url": "https://github.com/schmittjoh/serializer.git", - "reference": "ce65798f722c836f16d5d7d2e3ca9d21e0fb4331" + "reference": "f4683f41ebf21e60667447bb49939bee35807c3c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/schmittjoh/serializer/zipball/ce65798f722c836f16d5d7d2e3ca9d21e0fb4331", - "reference": "ce65798f722c836f16d5d7d2e3ca9d21e0fb4331", + "url": "https://api.github.com/repos/schmittjoh/serializer/zipball/f4683f41ebf21e60667447bb49939bee35807c3c", + "reference": "f4683f41ebf21e60667447bb49939bee35807c3c", "shasum": "" }, "require": { @@ -984,7 +1047,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.8-dev" + "dev-master": "1.9-dev" } }, "autoload": { @@ -1015,7 +1078,7 @@ "serialization", "xml" ], - "time": "2017-07-13T11:23:56+00:00" + "time": "2017-09-28T15:17:28+00:00" }, { "name": "justinrainbow/json-schema", @@ -1339,16 +1402,16 @@ }, { "name": "paragonie/random_compat", - "version": "v2.0.10", + "version": "v2.0.11", "source": { "type": "git", "url": "https://github.com/paragonie/random_compat.git", - "reference": "634bae8e911eefa89c1abfbf1b66da679ac8f54d" + "reference": "5da4d3c796c275c55f057af5a643ae297d96b4d8" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/paragonie/random_compat/zipball/634bae8e911eefa89c1abfbf1b66da679ac8f54d", - "reference": "634bae8e911eefa89c1abfbf1b66da679ac8f54d", + "url": "https://api.github.com/repos/paragonie/random_compat/zipball/5da4d3c796c275c55f057af5a643ae297d96b4d8", + "reference": "5da4d3c796c275c55f057af5a643ae297d96b4d8", "shasum": "" }, "require": { @@ -1383,7 +1446,7 @@ "pseudorandom", "random" ], - "time": "2017-03-13T16:27:32+00:00" + "time": "2017-09-27T21:40:39+00:00" }, { "name": "pear/archive_tar", @@ -1599,16 +1662,16 @@ }, { "name": "phake/phake", - "version": "v3.0.0", + "version": "v3.0.1", "source": { "type": "git", "url": "https://github.com/mlively/Phake.git", - "reference": "c242d6a8376bd3280d903d95725d3e1e2f9efadc" + "reference": "949340efc3cd99b401a0dd1a5ffeac690a3c3967" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/mlively/Phake/zipball/c242d6a8376bd3280d903d95725d3e1e2f9efadc", - "reference": "c242d6a8376bd3280d903d95725d3e1e2f9efadc", + "url": "https://api.github.com/repos/mlively/Phake/zipball/949340efc3cd99b401a0dd1a5ffeac690a3c3967", + "reference": "949340efc3cd99b401a0dd1a5ffeac690a3c3967", "shasum": "" }, "require": { @@ -1653,7 +1716,7 @@ "mock", "testing" ], - "time": "2017-07-04T20:09:48+00:00" + "time": "2017-09-06T12:09:44+00:00" }, { "name": "phar-io/manifest", @@ -2226,22 +2289,22 @@ }, { "name": "phpspec/prophecy", - "version": "v1.7.0", + "version": "v1.7.2", "source": { "type": "git", "url": "https://github.com/phpspec/prophecy.git", - "reference": "93d39f1f7f9326d746203c7c056f300f7f126073" + "reference": "c9b8c6088acd19d769d4cc0ffa60a9fe34344bd6" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/phpspec/prophecy/zipball/93d39f1f7f9326d746203c7c056f300f7f126073", - "reference": "93d39f1f7f9326d746203c7c056f300f7f126073", + "url": "https://api.github.com/repos/phpspec/prophecy/zipball/c9b8c6088acd19d769d4cc0ffa60a9fe34344bd6", + "reference": "c9b8c6088acd19d769d4cc0ffa60a9fe34344bd6", "shasum": "" }, "require": { "doctrine/instantiator": "^1.0.2", "php": "^5.3|^7.0", - "phpdocumentor/reflection-docblock": "^2.0|^3.0.2", + "phpdocumentor/reflection-docblock": "^2.0|^3.0.2|^4.0", "sebastian/comparator": "^1.1|^2.0", "sebastian/recursion-context": "^1.0|^2.0|^3.0" }, @@ -2252,7 +2315,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.6.x-dev" + "dev-master": "1.7.x-dev" } }, "autoload": { @@ -2285,7 +2348,7 @@ "spy", "stub" ], - "time": "2017-03-02T20:05:34+00:00" + "time": "2017-09-04T11:05:03+00:00" }, { "name": "phpunit/php-code-coverage", @@ -3432,16 +3495,16 @@ }, { "name": "symfony/config", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/config.git", - "reference": "0b8541d18507d10204a08384640ff6df3c739ebe" + "reference": "1dbeaa8e2db4b29159265867efff075ad961558c" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/config/zipball/0b8541d18507d10204a08384640ff6df3c739ebe", - "reference": "0b8541d18507d10204a08384640ff6df3c739ebe", + "url": "https://api.github.com/repos/symfony/config/zipball/1dbeaa8e2db4b29159265867efff075ad961558c", + "reference": "1dbeaa8e2db4b29159265867efff075ad961558c", "shasum": "" }, "require": { @@ -3484,20 +3547,20 @@ ], "description": "Symfony Config Component", "homepage": "https://symfony.com", - "time": "2017-04-12T14:07:15+00:00" + "time": "2017-10-04T18:56:36+00:00" }, { "name": "symfony/console", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/console.git", - "reference": "c0807a2ca978e64d8945d373a9221a5c35d1a253" + "reference": "f81549d2c5fdee8d711c9ab3c7e7362353ea5853" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/console/zipball/c0807a2ca978e64d8945d373a9221a5c35d1a253", - "reference": "c0807a2ca978e64d8945d373a9221a5c35d1a253", + "url": "https://api.github.com/repos/symfony/console/zipball/f81549d2c5fdee8d711c9ab3c7e7362353ea5853", + "reference": "f81549d2c5fdee8d711c9ab3c7e7362353ea5853", "shasum": "" }, "require": { @@ -3545,7 +3608,7 @@ ], "description": "Symfony Console Component", "homepage": "https://symfony.com", - "time": "2017-08-27T14:29:03+00:00" + "time": "2017-10-01T21:00:16+00:00" }, { "name": "symfony/debug", @@ -3606,16 +3669,16 @@ }, { "name": "symfony/event-dispatcher", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/event-dispatcher.git", - "reference": "1377400fd641d7d1935981546aaef780ecd5bf6d" + "reference": "7fe089232554357efb8d4af65ce209fc6e5a2186" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/event-dispatcher/zipball/1377400fd641d7d1935981546aaef780ecd5bf6d", - "reference": "1377400fd641d7d1935981546aaef780ecd5bf6d", + "url": "https://api.github.com/repos/symfony/event-dispatcher/zipball/7fe089232554357efb8d4af65ce209fc6e5a2186", + "reference": "7fe089232554357efb8d4af65ce209fc6e5a2186", "shasum": "" }, "require": { @@ -3662,7 +3725,7 @@ ], "description": "Symfony EventDispatcher Component", "homepage": "https://symfony.com", - "time": "2017-06-02T07:47:27+00:00" + "time": "2017-10-01T21:00:16+00:00" }, { "name": "symfony/filesystem", @@ -3715,16 +3778,16 @@ }, { "name": "symfony/finder", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/finder.git", - "reference": "4f4e84811004e065a3bb5ceeb1d9aa592630f9ad" + "reference": "a945724b201f74d543e356f6059c930bb8d10c92" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/finder/zipball/4f4e84811004e065a3bb5ceeb1d9aa592630f9ad", - "reference": "4f4e84811004e065a3bb5ceeb1d9aa592630f9ad", + "url": "https://api.github.com/repos/symfony/finder/zipball/a945724b201f74d543e356f6059c930bb8d10c92", + "reference": "a945724b201f74d543e356f6059c930bb8d10c92", "shasum": "" }, "require": { @@ -3760,11 +3823,11 @@ ], "description": "Symfony Finder Component", "homepage": "https://symfony.com", - "time": "2017-06-01T20:52:29+00:00" + "time": "2017-10-01T21:00:16+00:00" }, { "name": "symfony/options-resolver", - "version": "v3.3.8", + "version": "v3.3.10", "source": { "type": "git", "url": "https://github.com/symfony/options-resolver.git", @@ -3818,16 +3881,16 @@ }, { "name": "symfony/polyfill-mbstring", - "version": "v1.5.0", + "version": "v1.6.0", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-mbstring.git", - "reference": "7c8fae0ac1d216eb54349e6a8baa57d515fe8803" + "reference": "2ec8b39c38cb16674bbf3fea2b6ce5bf117e1296" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-mbstring/zipball/7c8fae0ac1d216eb54349e6a8baa57d515fe8803", - "reference": "7c8fae0ac1d216eb54349e6a8baa57d515fe8803", + "url": "https://api.github.com/repos/symfony/polyfill-mbstring/zipball/2ec8b39c38cb16674bbf3fea2b6ce5bf117e1296", + "reference": "2ec8b39c38cb16674bbf3fea2b6ce5bf117e1296", "shasum": "" }, "require": { @@ -3839,7 +3902,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.5-dev" + "dev-master": "1.6-dev" } }, "autoload": { @@ -3873,20 +3936,20 @@ "portable", "shim" ], - "time": "2017-06-14T15:44:48+00:00" + "time": "2017-10-11T12:05:26+00:00" }, { "name": "symfony/polyfill-php54", - "version": "v1.5.0", + "version": "v1.6.0", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-php54.git", - "reference": "b7763422a5334c914ef0298ed21b253d25913a6e" + "reference": "d7810a14b2c6c1aff415e1bb755f611b3d5327bc" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php54/zipball/b7763422a5334c914ef0298ed21b253d25913a6e", - "reference": "b7763422a5334c914ef0298ed21b253d25913a6e", + "url": "https://api.github.com/repos/symfony/polyfill-php54/zipball/d7810a14b2c6c1aff415e1bb755f611b3d5327bc", + "reference": "d7810a14b2c6c1aff415e1bb755f611b3d5327bc", "shasum": "" }, "require": { @@ -3895,7 +3958,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.5-dev" + "dev-master": "1.6-dev" } }, "autoload": { @@ -3931,20 +3994,20 @@ "portable", "shim" ], - "time": "2017-06-14T15:44:48+00:00" + "time": "2017-10-11T12:05:26+00:00" }, { "name": "symfony/polyfill-php55", - "version": "v1.5.0", + "version": "v1.6.0", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-php55.git", - "reference": "29b1381d66f16e0581aab0b9f678ccf073288f68" + "reference": "b64e7f0c37ecf144ecc16668936eef94e628fbfd" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php55/zipball/29b1381d66f16e0581aab0b9f678ccf073288f68", - "reference": "29b1381d66f16e0581aab0b9f678ccf073288f68", + "url": "https://api.github.com/repos/symfony/polyfill-php55/zipball/b64e7f0c37ecf144ecc16668936eef94e628fbfd", + "reference": "b64e7f0c37ecf144ecc16668936eef94e628fbfd", "shasum": "" }, "require": { @@ -3954,7 +4017,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.5-dev" + "dev-master": "1.6-dev" } }, "autoload": { @@ -3987,20 +4050,20 @@ "portable", "shim" ], - "time": "2017-06-14T15:44:48+00:00" + "time": "2017-10-11T12:05:26+00:00" }, { "name": "symfony/polyfill-php70", - "version": "v1.5.0", + "version": "v1.6.0", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-php70.git", - "reference": "b6482e68974486984f59449ecea1fbbb22ff840f" + "reference": "0442b9c0596610bd24ae7b5f0a6cdbbc16d9fcff" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php70/zipball/b6482e68974486984f59449ecea1fbbb22ff840f", - "reference": "b6482e68974486984f59449ecea1fbbb22ff840f", + "url": "https://api.github.com/repos/symfony/polyfill-php70/zipball/0442b9c0596610bd24ae7b5f0a6cdbbc16d9fcff", + "reference": "0442b9c0596610bd24ae7b5f0a6cdbbc16d9fcff", "shasum": "" }, "require": { @@ -4010,7 +4073,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.5-dev" + "dev-master": "1.6-dev" } }, "autoload": { @@ -4046,20 +4109,20 @@ "portable", "shim" ], - "time": "2017-06-14T15:44:48+00:00" + "time": "2017-10-11T12:05:26+00:00" }, { "name": "symfony/polyfill-php72", - "version": "v1.5.0", + "version": "v1.6.0", "source": { "type": "git", "url": "https://github.com/symfony/polyfill-php72.git", - "reference": "8abc9097f5001d310f0edba727469c988acc6ea7" + "reference": "6de4f4884b97abbbed9f0a84a95ff2ff77254254" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/polyfill-php72/zipball/8abc9097f5001d310f0edba727469c988acc6ea7", - "reference": "8abc9097f5001d310f0edba727469c988acc6ea7", + "url": "https://api.github.com/repos/symfony/polyfill-php72/zipball/6de4f4884b97abbbed9f0a84a95ff2ff77254254", + "reference": "6de4f4884b97abbbed9f0a84a95ff2ff77254254", "shasum": "" }, "require": { @@ -4068,7 +4131,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.5-dev" + "dev-master": "1.6-dev" } }, "autoload": { @@ -4101,20 +4164,20 @@ "portable", "shim" ], - "time": "2017-07-11T13:25:55+00:00" + "time": "2017-10-11T12:05:26+00:00" }, { "name": "symfony/process", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/process.git", - "reference": "57e52a0a6a80ea0aec4fc1b785a7920a95cb88a8" + "reference": "26c9fb02bf06bd6b90f661a5bd17e510810d0176" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/process/zipball/57e52a0a6a80ea0aec4fc1b785a7920a95cb88a8", - "reference": "57e52a0a6a80ea0aec4fc1b785a7920a95cb88a8", + "url": "https://api.github.com/repos/symfony/process/zipball/26c9fb02bf06bd6b90f661a5bd17e510810d0176", + "reference": "26c9fb02bf06bd6b90f661a5bd17e510810d0176", "shasum": "" }, "require": { @@ -4150,20 +4213,20 @@ ], "description": "Symfony Process Component", "homepage": "https://symfony.com", - "time": "2017-07-03T08:04:30+00:00" + "time": "2017-10-01T21:00:16+00:00" }, { "name": "symfony/stopwatch", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/stopwatch.git", - "reference": "e02577b841394a78306d7b547701bb7bb705bad5" + "reference": "28ee62ea4736431ca817cdaebcb005663e9cd1cb" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/stopwatch/zipball/e02577b841394a78306d7b547701bb7bb705bad5", - "reference": "e02577b841394a78306d7b547701bb7bb705bad5", + "url": "https://api.github.com/repos/symfony/stopwatch/zipball/28ee62ea4736431ca817cdaebcb005663e9cd1cb", + "reference": "28ee62ea4736431ca817cdaebcb005663e9cd1cb", "shasum": "" }, "require": { @@ -4199,7 +4262,7 @@ ], "description": "Symfony Stopwatch Component", "homepage": "https://symfony.com", - "time": "2017-04-12T14:07:15+00:00" + "time": "2017-10-01T21:00:16+00:00" }, { "name": "symfony/translation", @@ -4267,16 +4330,16 @@ }, { "name": "symfony/validator", - "version": "v2.8.27", + "version": "v2.8.28", "source": { "type": "git", "url": "https://github.com/symfony/validator.git", - "reference": "864ba6865e253a7ffc3db5629af676cfdc3bd104" + "reference": "1531ddfd96efd1b2c231cbf45f22e652a8f67925" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/validator/zipball/864ba6865e253a7ffc3db5629af676cfdc3bd104", - "reference": "864ba6865e253a7ffc3db5629af676cfdc3bd104", + "url": "https://api.github.com/repos/symfony/validator/zipball/1531ddfd96efd1b2c231cbf45f22e652a8f67925", + "reference": "1531ddfd96efd1b2c231cbf45f22e652a8f67925", "shasum": "" }, "require": { @@ -4336,20 +4399,20 @@ ], "description": "Symfony Validator Component", "homepage": "https://symfony.com", - "time": "2017-08-27T14:29:03+00:00" + "time": "2017-10-01T21:00:16+00:00" }, { "name": "symfony/yaml", - "version": "v3.3.8", + "version": "v3.3.10", "source": { "type": "git", "url": "https://github.com/symfony/yaml.git", - "reference": "1d8c2a99c80862bdc3af94c1781bf70f86bccac0" + "reference": "8c7bf1e7d5d6b05a690b715729cb4cd0c0a99c46" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/symfony/yaml/zipball/1d8c2a99c80862bdc3af94c1781bf70f86bccac0", - "reference": "1d8c2a99c80862bdc3af94c1781bf70f86bccac0", + "url": "https://api.github.com/repos/symfony/yaml/zipball/8c7bf1e7d5d6b05a690b715729cb4cd0c0a99c46", + "reference": "8c7bf1e7d5d6b05a690b715729cb4cd0c0a99c46", "shasum": "" }, "require": { @@ -4391,7 +4454,7 @@ ], "description": "Symfony Yaml Component", "homepage": "https://symfony.com", - "time": "2017-07-29T21:54:42+00:00" + "time": "2017-10-05T14:43:42+00:00" }, { "name": "theseer/tokenizer", @@ -4435,16 +4498,16 @@ }, { "name": "twig/twig", - "version": "v1.34.4", + "version": "v1.35.0", "source": { "type": "git", "url": "https://github.com/twigphp/Twig.git", - "reference": "f878bab48edb66ad9c6ed626bf817f60c6c096ee" + "reference": "daa657073e55b0a78cce8fdd22682fddecc6385f" }, "dist": { "type": "zip", - "url": "https://api.github.com/repos/twigphp/Twig/zipball/f878bab48edb66ad9c6ed626bf817f60c6c096ee", - "reference": "f878bab48edb66ad9c6ed626bf817f60c6c096ee", + "url": "https://api.github.com/repos/twigphp/Twig/zipball/daa657073e55b0a78cce8fdd22682fddecc6385f", + "reference": "daa657073e55b0a78cce8fdd22682fddecc6385f", "shasum": "" }, "require": { @@ -4458,7 +4521,7 @@ "type": "library", "extra": { "branch-alias": { - "dev-master": "1.34-dev" + "dev-master": "1.35-dev" } }, "autoload": { @@ -4496,7 +4559,7 @@ "keywords": [ "templating" ], - "time": "2017-07-04T13:19:31+00:00" + "time": "2017-09-27T18:06:46+00:00" }, { "name": "webmozart/assert", From 1271a0c8c0d7cb076bd45fa1c4290f0cd031a953 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 19 Oct 2017 15:18:58 -0400 Subject: [PATCH 14/20] Add ValueInfo::normalize method This method provides generalized, consistent type casting more versatile than PHP's basic type juggling while hiding the significant complexity in achieving this. While this commit does not change any existing code to use the new method, the intent is for both API handlers and database drivers to use the same basic rules for type conversion while still allowing for differing failure modes. --- lib/AbstractException.php | 2 + lib/ExceptionType.php | 6 + lib/Misc/Date.php | 16 +- lib/Misc/ValueInfo.php | 267 ++++++++++++++++++++++++++++- locale/en.php | 11 ++ tests/Misc/TestValueInfo.php | 320 ++++++++++++++++++++++++++++++++++- 6 files changed, 608 insertions(+), 14 deletions(-) create mode 100644 lib/ExceptionType.php diff --git a/lib/AbstractException.php b/lib/AbstractException.php index 4d630c7..5d5ae3d 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -6,6 +6,8 @@ abstract class AbstractException extends \Exception { const CODES = [ "Exception.uncoded" => -1, "Exception.unknown" => 10000, + "ExceptionType.strictFailure" => 10011, + "ExceptionType.typeUnknown" => 10012, "Lang/Exception.defaultFileMissing" => 10101, "Lang/Exception.fileMissing" => 10102, "Lang/Exception.fileUnreadable" => 10103, diff --git a/lib/ExceptionType.php b/lib/ExceptionType.php new file mode 100644 index 0000000..16094ce --- /dev/null +++ b/lib/ExceptionType.php @@ -0,0 +1,6 @@ + ["!Y-m-d\TH:i:s", "Y-m-d\TH:i:s\Z" ], // NOTE: ISO 8601 dates require special input processing because of varying formats for timezone offsets + 'iso8601m' => ["!Y-m-d\TH:i:s.u", "Y-m-d\TH:i:s.u\Z" ], // NOTE: ISO 8601 dates require special input processing because of varying formats for timezone offsets + 'microtime' => ["U.u", "0.u00 U" ], // NOTE: the actual input format at the user level matches the output format; pre-processing is required for PHP not to fail + 'http' => ["!D, d M Y H:i:s \G\M\T", "D, d M Y H:i:s \G\M\T"], + 'sql' => ["!Y-m-d H:i:s", "Y-m-d H:i:s" ], + 'date' => ["!Y-m-d", "Y-m-d" ], + 'time' => ["!H:i:s", "H:i:s" ], + 'unix' => ["U", "U" ], + 'float' => ["U.u", "U.u" ], + ]; + public static function transform($date, string $outFormat = null, string $inFormat = null, bool $inLocal = false) { $date = self::normalize($date, $inFormat, $inLocal); if (is_null($date) || is_null($outFormat)) { @@ -14,7 +26,7 @@ class Date { } switch ($outFormat) { case 'http': $f = "D, d M Y H:i:s \G\M\T"; break; - case 'iso8601': $f = "Y-m-d\TH:i:s"; break; + case 'iso8601': $f = "Y-m-d\TH:i:s"; break; case 'sql': $f = "Y-m-d H:i:s"; break; case 'date': $f = "Y-m-d"; break; case 'time': $f = "H:i:s"; break; @@ -36,7 +48,7 @@ class Date { if (!is_null($inFormat)) { switch ($inFormat) { case 'http': $f = "D, d M Y H:i:s \G\M\T"; break; - case 'iso8601': $f = "Y-m-d\TH:i:sP"; break; + case 'iso8601': $f = "Y-m-d\TH:i:sP"; break; case 'sql': $f = "Y-m-d H:i:s"; break; case 'date': $f = "Y-m-d"; break; case 'time': $f = "H:i:s"; break; diff --git a/lib/Misc/ValueInfo.php b/lib/Misc/ValueInfo.php index 040243d..09dca6d 100644 --- a/lib/Misc/ValueInfo.php +++ b/lib/Misc/ValueInfo.php @@ -2,6 +2,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Misc; +use JKingWeb\Arsse\ExceptionType; + class ValueInfo { // universal const VALID = 1 << 0; @@ -9,9 +11,232 @@ class ValueInfo { // integers const ZERO = 1 << 2; const NEG = 1 << 3; + const FLOAT = 1 << 4; // strings const EMPTY = 1 << 2; const WHITE = 1 << 3; + //normalization types + const T_MIXED = 0; // pass through unchanged + const T_NULL = 1; // convert to null + const T_BOOL = 2; // convert to boolean + const T_INT = 3; // convert to integer + const T_FLOAT = 4; // convert to floating point + const T_DATE = 5; // convert to DateTimeInterface instance + const T_STRING = 6; // convert to string + const T_ARRAY = 7; // convert to array + //normalization modes + const M_NULL = 1 << 28; // pass nulls through regardless of target type + const M_DROP = 1 << 29; // drop the value (return null) if the type doesn't match + const M_STRICT = 1 << 30; // throw an exception if the type doesn't match + const M_ARRAY = 1 << 31; // the value should be a flat array of values of the specified type; indexed and associative are both acceptable + + public function normalize($value, int $type, string $dateFormat = null) { + $allowNull = ($type & self::M_NULL); + $strict = ($type & (self::M_STRICT | self::M_DROP)); + $drop = ($type & self::M_DROP); + $arrayVal = ($type & self::M_ARRAY); + $type = ($type & ~(self::M_NULL | self::M_DROP | self::M_STRICT | self::M_ARRAY)); + // if the value is null and this is allowed, simply return + if ($allowNull && is_null($value)) { + return null; + } + // if the value is supposed to be an array, handle it specially + if ($arrayVal) { + $value = self::normalize($value, self::T_ARRAY); + foreach ($value as $key => $v) { + $value[$key] = self::normalize($v, $type | ($allowNull ? self::M_NULL : 0) | ($strict ? self::M_STRICT : 0) | ($drop ? self::M_DROP : 0), $dateFormat); + } + return $value; + } + switch ($type) { + case self::T_MIXED: + return $value; + case self::T_NULL: + return null; + case self::T_BOOL: + if (is_bool($value)) { + return $value; + } + $out = self::bool($value); + if ($strict && is_null($out)) { + // if strict and input is not a boolean, this is an error + if ($drop) { + return null; + } + throw new ExceptionType("strictFailure", $type); + } elseif (is_float($value) && is_nan($value)) { + return false; + } elseif (is_null($out)) { + // if not strict and input is not a boolean, return a simple type-cast + return (bool) $value; + } + return $out; + case self::T_INT: + if (is_int($value)) { + return $value; + } elseif ($value instanceof \DateTimeInterface) { + if ($strict && !$drop) { + throw new ExceptionType("strictFailure", $type); + } + return (!$drop) ? (int) $value->getTimestamp(): null; + } + $info = self::int($value); + if ($strict && !($info & self::VALID)) { + // if strict and input is not an integer, this is an error + if ($drop) { + return null; + } + throw new ExceptionType("strictFailure", $type); + } elseif (is_bool($value)) { + return (int) $value; + } elseif ($info & (self::VALID | self::FLOAT)) { + $out = strtolower((string) $value); + if (strpos($out, "e")) { + return (int) (float) $out; + } else { + return (int) $out; + } + } else { + return 0; + } + case self::T_FLOAT: + if (is_float($value)) { + return $value; + } elseif ($value instanceof \DateTimeInterface) { + if ($strict && !$drop) { + throw new ExceptionType("strictFailure", $type); + } + return (!$drop) ? (float) $value->getTimestamp(): null; + } elseif (is_bool($value) && $strict) { + if ($drop) { + return null; + } + throw new ExceptionType("strictFailure", $type); + } + $out = filter_var($value, \FILTER_VALIDATE_FLOAT); + if ($strict && $out===false) { + // if strict and input is not a float, this is an error + if ($drop) { + return null; + } + throw new ExceptionType("strictFailure", $type); + } + return (float) $out; + case self::T_STRING: + if (is_string($value)) { + return $value; + } + if ($value instanceof \DateTimeImmutable) { + return $value->setTimezone(new \DateTimeZone("UTC"))->format(Date::FORMAT['iso8601'][1]); + } elseif ($value instanceof \DateTime) { + $out = clone $value; + $out->setTimezone(new \DateTimeZone("UTC")); + return $out->format(Date::FORMAT['iso8601'][1]); + } elseif (is_float($value) && is_finite($value)) { + $out = (string) $value; + if(!strpos($out, "E")) { + return $out; + } else { + $out = sprintf("%F", $value); + return substr($out, -2)==".0" ? (string) (int) $out : $out; + } + } + $info = self::str($value); + if (!($info & self::VALID)) { + if ($drop) { + return null; + } elseif ($strict) { + // if strict and input is not a string, this is an error + throw new ExceptionType("strictFailure", $type); + } elseif (!is_scalar($value)) { + return ""; + } else { + return (string) $value; + } + } else { + return (string) $value; + } + case self::T_DATE: + if ($value instanceof \DateTimeImmutable) { + return $value->setTimezone(new \DateTimeZone("UTC")); + } elseif ($value instanceof \DateTime) { + $out = clone $value; + $out->setTimezone(new \DateTimeZone("UTC")); + return $out; + } elseif (is_int($value)) { + return \DateTime::createFromFormat("U", (string) $value, new \DateTimeZone("UTC")); + } elseif (is_float($value)) { + return \DateTime::createFromFormat("U.u", sprintf("%F", $value), new \DateTimeZone("UTC")); + } elseif (is_string($value)) { + try { + if (!is_null($dateFormat)) { + $out = false; + if ($dateFormat=="microtime") { + // PHP is not able to correctly handle the output of microtime() as the input of DateTime::createFromFormat(), so we fudge it to look like a float + if (preg_match("<^0\.\d{6}00 \d+$>", $value)) { + $value = substr($value,11).".".substr($value,2,6); + } else { + throw new \Exception; + } + } + $f = isset(Date::FORMAT[$dateFormat]) ? Date::FORMAT[$dateFormat][0] : $dateFormat; + if ($dateFormat=="iso8601" || $dateFormat=="iso8601m") { + // DateTime::createFromFormat() doesn't provide one catch-all for ISO 8601 timezone specifiers, so we try all of them till one works + if ($dateFormat=="iso8601m") { + $f2 = Date::FORMAT["iso8601"][0]; + $zones = [$f."", $f."\Z", $f."P", $f."O", $f2."", $f2."\Z", $f2."P", $f2."O"]; + } else { + $zones = [$f."", $f."\Z", $f."P", $f."O"]; + } + do { + $ftz = array_shift($zones); + $out = \DateTime::createFromFormat($ftz, $value, new \DateTimeZone("UTC")); + } while (!$out && $zones); + } else { + $out = \DateTime::createFromFormat($f, $value, new \DateTimeZone("UTC")); + } + if (!$out) { + throw new \Exception; + } + return $out; + } else { + return new \DateTime($value, new \DateTimeZone("UTC")); + } + } catch (\Exception $e) { + if ($strict && !$drop) { + throw new ExceptionType("strictFailure", $type); + } + return null; + } + } elseif ($strict && !$drop) { + throw new ExceptionType("strictFailure", $type); + } + return null; + case self::T_ARRAY: + if (is_array($value)) { + return $value; + } elseif ($value instanceof \Traversable) { + $out = []; + foreach ($value as $k => $v) { + $out[$k] = $v; + } + return $out; + } else { + if ($drop) { + return null; + } elseif ($strict) { + // if strict and input is not a string, this is an error + throw new ExceptionType("strictFailure", $type); + } elseif (is_null($value) || (is_float($value) && is_nan($value))) { + return []; + } else { + return [$value]; + } + } + default: + throw new ExceptionType("typeUnknown", $type); // @codeCoverageIgnore + } + } public static function int($value): int { $out = 0; @@ -19,28 +244,42 @@ class ValueInfo { // check if the input is null return self::NULL; } elseif (is_string($value) || (is_object($value) && method_exists($value, "__toString"))) { - $value = (string) $value; + $value = strtolower((string) $value); // normalize a string an integer or float if possible if (!strlen($value)) { // the empty string is equivalent to null when evaluating an integer return self::NULL; - } elseif (filter_var($value, \FILTER_VALIDATE_FLOAT) !== false && !fmod((float) $value, 1)) { + } + // interpret the value as a float + $float = filter_var($value, \FILTER_VALIDATE_FLOAT); + if ($float !== false) { + if (!fmod($float, 1)) { + // an integral float is acceptable + $value = (int) (!strpos($value, "e") ? $value : $float); + } else { + $out += self::FLOAT; + $value = $float; + } + } else { + return $out; + } + } elseif (is_float($value)) { + if (!fmod($value, 1)) { // an integral float is acceptable $value = (int) $value; } else { - return $out; + $out += self::FLOAT; } - } elseif (is_float($value) && !fmod($value, 1)) { - // an integral float is acceptable - $value = (int) $value; } elseif (!is_int($value)) { // if the value is not an integer or integral float, stop return $out; } // mark validity - $out += self::VALID; + if (is_int($value)) { + $out += self::VALID; + } // mark zeroness - if ($value==0) { + if (!$value) { $out += self::ZERO; } // mark negativeness @@ -89,4 +328,16 @@ class ValueInfo { return true; } } + + public static function bool($value, bool $default = null) { + if (is_null($value) || ValueInfo::str($value) & ValueInfo::WHITE) { + return $default; + } + $out = filter_var($value, \FILTER_VALIDATE_BOOLEAN, \FILTER_NULL_ON_FAILURE); + if (is_null($out) && (ValueInfo::int($value) & ValueInfo::VALID)) { + $out = (int) filter_var($value, \FILTER_VALIDATE_FLOAT); + return ($out==1 || $out==0) ? (bool) $out : $default; + } + return !is_null($out) ? $out : $default; + } } diff --git a/locale/en.php b/locale/en.php index 03cf5f6..d232f1f 100644 --- a/locale/en.php +++ b/locale/en.php @@ -70,6 +70,17 @@ return [ 'Exception.JKingWeb/Arsse/Exception.uncoded' => 'The specified exception symbol {0} has no code specified in AbstractException.php', // this should not usually be encountered 'Exception.JKingWeb/Arsse/Exception.unknown' => 'An unknown error has occurred', + 'Exception.JKingWeb/Arsse/ExceptionType.strictFailure' => 'Supplied value could not be normalized to {0, select, + 1 {null} + 2 {boolean} + 3 {integer} + 4 {float} + 5 {datetime} + 6 {string} + 7 {array} + other {requested type} + }', + 'Exception.JKingWeb/Arsse/ExceptionType.typeUnknown' => 'Normalization type {0} is not implemented', 'Exception.JKingWeb/Arsse/Lang/Exception.defaultFileMissing' => 'Default language file "{0}" missing', 'Exception.JKingWeb/Arsse/Lang/Exception.fileMissing' => 'Language file "{0}" is not available', 'Exception.JKingWeb/Arsse/Lang/Exception.fileUnreadable' => 'Insufficient permissions to read language file "{0}"', diff --git a/tests/Misc/TestValueInfo.php b/tests/Misc/TestValueInfo.php index 1a75099..a6f8d97 100644 --- a/tests/Misc/TestValueInfo.php +++ b/tests/Misc/TestValueInfo.php @@ -7,6 +7,10 @@ use JKingWeb\Arsse\Test\Misc\StrClass; /** @covers \JKingWeb\Arsse\Misc\ValueInfo */ class TestValueInfo extends Test\AbstractTest { + public function setUp() { + $this->clearData(); + } + public function testGetIntegerInfo() { $tests = [ [null, I::NULL], @@ -52,9 +56,15 @@ class TestValueInfo extends Test\AbstractTest { ["-000.000", I::VALID | I::ZERO], [false, 0], [true, 0], - [INF, 0], - [-INF, 0], - [NAN, 0], + ["on", 0], + ["off", 0], + ["yes", 0], + ["no", 0], + ["true", 0], + ["false", 0], + [INF, I::FLOAT], + [-INF, I::FLOAT | I::NEG], + [NAN, I::FLOAT], [[], 0], ["some string", 0], [" ", 0], @@ -65,6 +75,10 @@ class TestValueInfo extends Test\AbstractTest { [new StrClass("-1"), I::VALID | I::NEG], [new StrClass("Msg"), 0], [new StrClass(" "), 0], + [2.5, I::FLOAT], + [0.5, I::FLOAT], + ["2.5", I::FLOAT], + ["0.5", I::FLOAT], ]; foreach ($tests as $test) { list($value, $exp) = $test; @@ -116,6 +130,12 @@ class TestValueInfo extends Test\AbstractTest { ["-000.000", I::VALID], [false, 0], [true, 0], + ["on", I::VALID], + ["off", I::VALID], + ["yes", I::VALID], + ["no", I::VALID], + ["true", I::VALID], + ["false", I::VALID], [INF, 0], [-INF, 0], [NAN, 0], @@ -181,6 +201,12 @@ class TestValueInfo extends Test\AbstractTest { ["-000.000", false, true], [false, false, false], [true, false, false], + ["on", false, false], + ["off", false, false], + ["yes", false, false], + ["no", false, false], + ["true", false, false], + ["false", false, false], [INF, false, false], [-INF, false, false], [NAN, false, false], @@ -201,4 +227,290 @@ class TestValueInfo extends Test\AbstractTest { $this->assertSame($expNull, I::id($value, true), "Null test failed for value: ".var_export($value, true)); } } -} + + public function testValidateBoolean() { + $tests = [ + [null, null], + ["", false], + [1, true], + [PHP_INT_MAX, null], + [1.0, true], + ["1.0", true], + ["001.0", true], + ["1.0e2", null], + ["1", true], + ["001", true], + ["1e2", null], + ["+1.0", true], + ["+001.0", true], + ["+1.0e2", null], + ["+1", true], + ["+001", true], + ["+1e2", null], + [0, false], + ["0", false], + ["000", false], + [0.0, false], + ["0.0", false], + ["000.000", false], + ["+0", false], + ["+000", false], + ["+0.0", false], + ["+000.000", false], + [-1, null], + [-1.0, null], + ["-1.0", null], + ["-001.0", null], + ["-1.0e2", null], + ["-1", null], + ["-001", null], + ["-1e2", null], + [-0, false], + ["-0", false], + ["-000", false], + [-0.0, false], + ["-0.0", false], + ["-000.000", false], + [false, false], + [true, true], + ["on", true], + ["off", false], + ["yes", true], + ["no", false], + ["true", true], + ["false", false], + [INF, null], + [-INF, null], + [NAN, null], + [[], null], + ["some string", null], + [" ", null], + [new \StdClass, null], + [new StrClass(""), false], + [new StrClass("1"), true], + [new StrClass("0"), false], + [new StrClass("-1"), null], + [new StrClass("Msg"), null], + [new StrClass(" "), null], + ]; + foreach ($tests as $test) { + list($value, $exp) = $test; + $this->assertSame($exp, I::bool($value), "Null Test failed for value: ".var_export($value, true)); + if (is_null($exp)) { + $this->assertTrue(I::bool($value, true), "True Test failed for value: ".var_export($value, true)); + $this->assertFalse(I::bool($value, false), "False Test failed for value: ".var_export($value, true)); + } + } + } + + public function testNormalizeValues() { + $tests = [ + /* The test data are very dense for this set. Each value is normalized to each of the following types: + + - mixed (no normalization performed) + - null + - boolean + - integer + - float + - string + - array + + For each of these types, there is an expected output value, as well as a boolean indicating whether + the value should pass or fail a strict normalization. Conversion to DateTime is covered below by a different data set + */ + /* Input value null bool int float string array */ + [null, [null,true], [false,false], [0, false], [0.0, false], ["", false], [[], false]], + ["", [null,true], [false,true], [0, false], [0.0, false], ["", true], [[""], false]], + [1, [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[1], false]], + [PHP_INT_MAX, [null,true], [true, false], [PHP_INT_MAX, true], [(float) PHP_INT_MAX,true], [(string) PHP_INT_MAX, true], [[PHP_INT_MAX], false]], + [1.0, [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[1.0], false]], + ["1.0", [null,true], [true, true], [1, true], [1.0, true], ["1.0", true], [["1.0"], false]], + ["001.0", [null,true], [true, true], [1, true], [1.0, true], ["001.0", true], [["001.0"], false]], + ["1.0e2", [null,true], [true, false], [100, true], [100.0, true], ["1.0e2", true], [["1.0e2"], false]], + ["1", [null,true], [true, true], [1, true], [1.0, true], ["1", true], [["1"], false]], + ["001", [null,true], [true, true], [1, true], [1.0, true], ["001", true], [["001"], false]], + ["1e2", [null,true], [true, false], [100, true], [100.0, true], ["1e2", true], [["1e2"], false]], + ["+1.0", [null,true], [true, true], [1, true], [1.0, true], ["+1.0", true], [["+1.0"], false]], + ["+001.0", [null,true], [true, true], [1, true], [1.0, true], ["+001.0", true], [["+001.0"], false]], + ["+1.0e2", [null,true], [true, false], [100, true], [100.0, true], ["+1.0e2", true], [["+1.0e2"], false]], + ["+1", [null,true], [true, true], [1, true], [1.0, true], ["+1", true], [["+1"], false]], + ["+001", [null,true], [true, true], [1, true], [1.0, true], ["+001", true], [["+001"], false]], + ["+1e2", [null,true], [true, false], [100, true], [100.0, true], ["+1e2", true], [["+1e2"], false]], + [0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[0], false]], + ["0", [null,true], [false,true], [0, true], [0.0, true], ["0", true], [["0"], false]], + ["000", [null,true], [false,true], [0, true], [0.0, true], ["000", true], [["000"], false]], + [0.0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[0.0], false]], + ["0.0", [null,true], [false,true], [0, true], [0.0, true], ["0.0", true], [["0.0"], false]], + ["000.000", [null,true], [false,true], [0, true], [0.0, true], ["000.000", true], [["000.000"], false]], + ["+0", [null,true], [false,true], [0, true], [0.0, true], ["+0", true], [["+0"], false]], + ["+000", [null,true], [false,true], [0, true], [0.0, true], ["+000", true], [["+000"], false]], + ["+0.0", [null,true], [false,true], [0, true], [0.0, true], ["+0.0", true], [["+0.0"], false]], + ["+000.000", [null,true], [false,true], [0, true], [0.0, true], ["+000.000", true], [["+000.000"], false]], + [-1, [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[-1], false]], + [-1.0, [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[-1.0], false]], + ["-1.0", [null,true], [true, false], [-1, true], [-1.0, true], ["-1.0", true], [["-1.0"], false]], + ["-001.0", [null,true], [true, false], [-1, true], [-1.0, true], ["-001.0", true], [["-001.0"], false]], + ["-1.0e2", [null,true], [true, false], [-100, true], [-100.0, true], ["-1.0e2", true], [["-1.0e2"], false]], + ["-1", [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [["-1"], false]], + ["-001", [null,true], [true, false], [-1, true], [-1.0, true], ["-001", true], [["-001"], false]], + ["-1e2", [null,true], [true, false], [-100, true], [-100.0, true], ["-1e2", true], [["-1e2"], false]], + [-0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[-0], false]], + ["-0", [null,true], [false,true], [0, true], [-0.0, true], ["-0", true], [["-0"], false]], + ["-000", [null,true], [false,true], [0, true], [-0.0, true], ["-000", true], [["-000"], false]], + [-0.0, [null,true], [false,true], [0, true], [-0.0, true], ["-0", true], [[-0.0], false]], + ["-0.0", [null,true], [false,true], [0, true], [-0.0, true], ["-0.0", true], [["-0.0"], false]], + ["-000.000", [null,true], [false,true], [0, true], [-0.0, true], ["-000.000", true], [["-000.000"], false]], + [false, [null,true], [false,true], [0, false], [0.0, false], ["", false], [[false], false]], + [true, [null,true], [true, true], [1, false], [1.0, false], ["1", false], [[true], false]], + ["on", [null,true], [true, true], [0, false], [0.0, false], ["on", true], [["on"], false]], + ["off", [null,true], [false,true], [0, false], [0.0, false], ["off", true], [["off"], false]], + ["yes", [null,true], [true, true], [0, false], [0.0, false], ["yes", true], [["yes"], false]], + ["no", [null,true], [false,true], [0, false], [0.0, false], ["no", true], [["no"], false]], + ["true", [null,true], [true, true], [0, false], [0.0, false], ["true", true], [["true"], false]], + ["false", [null,true], [false,true], [0, false], [0.0, false], ["false", true], [["false"], false]], + [INF, [null,true], [true, false], [0, false], [INF, true], ["INF", false], [[INF], false]], + [-INF, [null,true], [true, false], [0, false], [-INF, true], ["-INF", false], [[-INF], false]], + [NAN, [null,true], [false,false], [0, false], [NAN, true], ["NAN", false], [[], false]], + [[], [null,true], [false,false], [0, false], [0.0, false], ["", false], [[], true] ], + ["some string", [null,true], [true, false], [0, false], [0.0, false], ["some string", true], [["some string"], false]], + [" ", [null,true], [true, false], [0, false], [0.0, false], [" ", true], [[" "], false]], + [new \StdClass, [null,true], [true, false], [0, false], [0.0, false], ["", false], [[new \StdClass], false]], + [new StrClass(""), [null,true], [false,true], [0, false], [0.0, false], ["", true], [[new StrClass("")], false]], + [new StrClass("1"), [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[new StrClass("1")], false]], + [new StrClass("0"), [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[new StrClass("0")], false]], + [new StrClass("-1"), [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[new StrClass("-1")], false]], + [new StrClass("Msg"), [null,true], [true, false], [0, false], [0.0, false], ["Msg", true], [[new StrClass("Msg")], false]], + [new StrClass(" "), [null,true], [true, false], [0, false], [0.0, false], [" ", true], [[new StrClass(" ")], false]], + [2.5, [null,true], [true, false], [2, false], [2.5, true], ["2.5", true], [[2.5], false]], + [0.5, [null,true], [true, false], [0, false], [0.5, true], ["0.5", true], [[0.5], false]], + ["2.5", [null,true], [true, false], [2, false], [2.5, true], ["2.5", true], [["2.5"], false]], + ["0.5", [null,true], [true, false], [0, false], [0.5, true], ["0.5", true], [["0.5"], false]], + [$this->d("2010-01-01T00:00:00",0,0), [null,true], [true, false], [1262304000, false], [1262304000.0, false], ["2010-01-01T00:00:00Z",true], [[$this->d("2010-01-01T00:00:00",0,0)],false]], + [$this->d("2010-01-01T00:00:00",0,1), [null,true], [true, false], [1262304000, false], [1262304000.0, false], ["2010-01-01T00:00:00Z",true], [[$this->d("2010-01-01T00:00:00",0,1)],false]], + [$this->d("2010-01-01T00:00:00",1,0), [null,true], [true, false], [1262322000, false], [1262322000.0, false], ["2010-01-01T05:00:00Z",true], [[$this->d("2010-01-01T00:00:00",1,0)],false]], + [$this->d("2010-01-01T00:00:00",1,1), [null,true], [true, false], [1262322000, false], [1262322000.0, false], ["2010-01-01T05:00:00Z",true], [[$this->d("2010-01-01T00:00:00",1,1)],false]], + [1e14, [null,true], [true, false], [100000000000000,true], [1e14, true], ["100000000000000", true], [[1e14], false]], + [1e-6, [null,true], [true, false], [0, false], [1e-6, true], ["0.000001", true], [[1e-6], false]], + [[1,2,3], [null,true], [true, false], [0, false], [0.0, false], ["", false], [[1,2,3], true] ], + [['a'=>1,'b'=>2], [null,true], [true, false], [0, false], [0.0, false], ["", false], [['a'=>1,'b'=>2], true] ], + [new Test\Result([['a'=>1,'b'=>2]]), [null,true], [true, false], [0, false], [0.0, false], ["", false], [[['a'=>1,'b'=>2]], true] ], + ]; + $params = [ + [I::T_MIXED, "Mixed" ], + [I::T_NULL, "Null", ], + [I::T_BOOL, "Boolean", ], + [I::T_INT, "Integer", ], + [I::T_FLOAT, "Floating point"], + [I::T_STRING, "String", ], + [I::T_ARRAY, "Array", ], + ]; + foreach ($params as $index => $param) { + list($type, $name) = $param; + $this->assertNull(I::normalize(null, $type | I::M_STRICT | I::M_NULL), $name." null-passthrough test failed"); + foreach ($tests as $test) { + list($exp, $pass) = $index ? $test[$index] : [$test[$index], true]; + $value = $test[0]; + $assert = (is_float($exp) && is_nan($exp) ? "assertNan" : (is_scalar($exp) ? "assertSame" : "assertEquals")); + $this->$assert($exp, I::normalize($value, $type), $name." test failed for value: ".var_export($value, true)); + if ($pass) { + $this->$assert($exp, I::normalize($value, $type | I::M_DROP), $name." drop test failed for value: ".var_export($value, true)); + $this->$assert($exp, I::normalize($value, $type | I::M_STRICT), $name." error test failed for value: ".var_export($value, true)); + } else { + $this->assertNull(I::normalize($value, $type | I::M_DROP), $name." drop test failed for value: ".var_export($value, true)); + $exc = new ExceptionType("strictFailure", $type); + try { + $act = I::normalize($value, $type | I::M_STRICT); + } catch (ExceptionType $e) { + $act = $e; + } finally { + $this->assertEquals($exc, $act, $name." error test failed for value: ".var_export($value, true)); + } + } + } + } + // DateTimeInterface tests + $tests = [ + /* Input value microtime iso8601 iso8601m http sql date time unix float '!M j, Y (D)' *strtotime* (null) */ + [null, null, null, null, null, null, null, null, null, null, null, null, ], + [$this->d("2010-01-01T00:00:00",0,0), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], + [$this->d("2010-01-01T00:00:00",0,1), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], + [$this->d("2010-01-01T00:00:00",1,0), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), ], + [$this->d("2010-01-01T00:00:00",1,1), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), ], + [1262304000, $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], + [1262304000.123456, $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), ], + [1262304000.42, $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), ], + ["0.12345600 1262304000", $this->t(1262304000.123456), null, null, null, null, null, null, null, null, null, null, ], + ["0.42 1262304000", null, null, null, null, null, null, null, null, null, null, null, ], + ["2010-01-01T00:00:00", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00Z", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00+0000", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00-0000", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00+00:00", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00-05:00", null, $this->t(1262322000), $this->t(1262322000), null, null, null, null, null, null, null, $this->t(1262322000), ], + ["2010-01-01T00:00:00.123456Z", null, null, $this->t(1262304000.123456), null, null, null, null, null, null, null, $this->t(1262304000.123456), ], + ["Fri, 01 Jan 2010 00:00:00 GMT", null, null, null, $this->t(1262304000), null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01 00:00:00", null, null, null, null, $this->t(1262304000), null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01", null, null, null, null, null, $this->t(1262304000), null, null, null, null, $this->t(1262304000), ], + ["12:34:56", null, null, null, null, null, null, $this->t(45296), null, null, null, $this->t(strtotime("today")+45296), ], + ["1262304000", null, null, null, null, null, null, null, $this->t(1262304000), null, null, null, ], + ["1262304000.123456", null, null, null, null, null, null, null, null, $this->t(1262304000.123456), null, null, ], + ["1262304000.42", null, null, null, null, null, null, null, null, $this->t(1262304000.42), null, null, ], + ["Jan 1, 2010 (Fri)", null, null, null, null, null, null, null, null, null, $this->t(1262304000), null, ], + ["First day of Jan 2010 12AM", null, null, null, null, null, null, null, null, null, null, $this->t(1262304000), ], + ]; + $formats = [ + "microtime", + "iso8601", + "iso8601m", + "http", + "sql", + "date", + "time", + "unix", + "float", + "!M j, Y (D)", + null, + ]; + $exc = new ExceptionType("strictFailure", I::T_DATE); + foreach ($formats as $index => $format) { + foreach ($tests as $test) { + $value = $test[0]; + $exp = $test[$index+1]; + $this->assertEquals($exp, I::normalize($value, I::T_DATE, $format), "Test failed for format ".var_export($format, true)." using value ".var_export($value, true)); + $this->assertEquals($exp, I::normalize($value, I::T_DATE | I::M_DROP, $format), "Drop test failed for format ".var_export($format, true)." using value ".var_export($value, true)); + // test for exception in case of errors + $exp = $exp ?? $exc; + try { + $act = I::normalize($value, I::T_DATE | I::M_STRICT, $format); + } catch (ExceptionType $e) { + $act = $e; + } finally { + $this->assertEquals($exp, $act, "Error test failed for format ".var_export($format, true)." using value ".var_export($value, true)); + } + } + } + // Array-mode tests + $tests = [ + [I::T_INT | I::M_DROP, new Test\Result([1, 2, 2.2, 3]), [1,2,null,3] ], + [I::T_INT, new Test\Result([1, 2, 2.2, 3]), [1,2,2,3] ], + [I::T_STRING | I::M_STRICT, "Bare string", ["Bare string"]], + ]; + foreach ($tests as $index => $test) { + list($type, $value, $exp) = $test; + $this->assertEquals($exp, I::normalize($value, $type | I::M_ARRAY, "iso8601"), "Failed test #$index"); + } + } + + protected function d($spec, $local, $immutable): \DateTimeInterface { + $tz = $local ? new \DateTimeZone("America/Toronto") : new \DateTimeZone("UTC"); + if ($immutable) { + return \DateTimeImmutable::createFromFormat("!Y-m-d\TH:i:s", $spec, $tz); + } else { + return \DateTime::createFromFormat("!Y-m-d\TH:i:s", $spec, $tz); + } + } + + protected function t(float $spec): \DateTime { + return \DateTime::createFromFormat("U.u", sprintf("%F", $spec), new \DateTimeZone("UTC")); + } +} \ No newline at end of file From cc875be57e9ac6dec89e6ca92b1379920109a059 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 19 Oct 2017 15:32:18 -0400 Subject: [PATCH 15/20] Backport testing improvements from ttrss branch --- lib/Db/SQLite3/Driver.php | 4 ++-- tests/Db/SQLite3/TestDbDriverSQLite3.php | 3 ++- tests/Db/SQLite3/TestDbUpdateSQLite3.php | 2 +- tests/lib/Database/DriverSQLite3.php | 2 +- tests/lib/Database/Setup.php | 30 +++++++++++++++++------- 5 files changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/Db/SQLite3/Driver.php b/lib/Db/SQLite3/Driver.php index d65ff2e..46b1110 100644 --- a/lib/Db/SQLite3/Driver.php +++ b/lib/Db/SQLite3/Driver.php @@ -16,13 +16,13 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { protected $db; - public function __construct() { + public function __construct(string $dbFile = null) { // check to make sure required extension is loaded if (!class_exists("SQLite3")) { throw new Exception("extMissing", self::driverName()); // @codeCoverageIgnore } // if no database file is specified in the configuration, use a suitable default - $dbFile = Arsse::$conf->dbSQLite3File ?? \JKingWeb\Arsse\BASE."arsse.db"; + $dbFile = $dbFile ?? Arsse::$conf->dbSQLite3File ?? \JKingWeb\Arsse\BASE."arsse.db"; $mode = \SQLITE3_OPEN_READWRITE | \SQLITE3_OPEN_CREATE; $timeout = Arsse::$conf->dbSQLite3Timeout * 1000; try { diff --git a/tests/Db/SQLite3/TestDbDriverSQLite3.php b/tests/Db/SQLite3/TestDbDriverSQLite3.php index 4a2058d..0c80b5b 100644 --- a/tests/Db/SQLite3/TestDbDriverSQLite3.php +++ b/tests/Db/SQLite3/TestDbDriverSQLite3.php @@ -18,8 +18,9 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { $conf = new Conf(); Arsse::$conf = $conf; $conf->dbDriver = Db\SQLite3\Driver::class; + $conf->dbSQLite3Timeout = 0; $conf->dbSQLite3File = tempnam(sys_get_temp_dir(), 'ook'); - $this->drv = new Db\SQLite3\Driver(true); + $this->drv = new Db\SQLite3\Driver(); $this->ch = new \SQLite3(Arsse::$conf->dbSQLite3File); $this->ch->enableExceptions(true); } diff --git a/tests/Db/SQLite3/TestDbUpdateSQLite3.php b/tests/Db/SQLite3/TestDbUpdateSQLite3.php index a71c605..3441263 100644 --- a/tests/Db/SQLite3/TestDbUpdateSQLite3.php +++ b/tests/Db/SQLite3/TestDbUpdateSQLite3.php @@ -30,7 +30,7 @@ class TestDbUpdateSQLite3 extends Test\AbstractTest { Arsse::$conf = $conf; $this->base = $this->vfs->url(); $this->path = $this->base."/SQLite3/"; - $this->drv = new Db\SQLite3\Driver(true); + $this->drv = new Db\SQLite3\Driver(); } public function tearDown() { diff --git a/tests/lib/Database/DriverSQLite3.php b/tests/lib/Database/DriverSQLite3.php index a898a08..ca096e4 100644 --- a/tests/lib/Database/DriverSQLite3.php +++ b/tests/lib/Database/DriverSQLite3.php @@ -11,7 +11,7 @@ trait DriverSQLite3 { $this->markTestSkipped("SQLite extension not loaded"); } Arsse::$conf->dbSQLite3File = ":memory:"; - $this->drv = new Driver(true); + $this->drv = new Driver(); } public function nextID(string $table): int { diff --git a/tests/lib/Database/Setup.php b/tests/lib/Database/Setup.php index 3dfba36..8267b0c 100644 --- a/tests/lib/Database/Setup.php +++ b/tests/lib/Database/Setup.php @@ -48,15 +48,16 @@ trait Setup { $this->clearData(); } - public function primeDatabase(array $data): bool { - $tr = $this->drv->begin(); + public function primeDatabase(array $data, \JKingWeb\Arsse\Db\Driver $drv = null): bool { + $drv = $drv ?? $this->drv; + $tr = $drv->begin(); foreach ($data as $table => $info) { $cols = implode(",", array_keys($info['columns'])); $bindings = array_values($info['columns']); $params = implode(",", array_fill(0, sizeof($info['columns']), "?")); - $s = $this->drv->prepareArray("INSERT INTO $table($cols) values($params)", $bindings); + $s = $drv->prepareArray("INSERT INTO $table($cols) values($params)", $bindings); foreach ($info['rows'] as $row) { - $this->assertEquals(1, $s->runArray($row)->changes()); + $s->runArray($row); } } $tr->commit(); @@ -64,6 +65,16 @@ trait Setup { return true; } + public function primeFile(string $file, array $data = null): bool { + $data = $data ?? $this->data; + $primed = $this->primed; + $drv = new \JKingWeb\Arsse\Db\SQLite3\Driver($file); + $drv->schemaUpdate(\JKingWeb\Arsse\Database::SCHEMA_VERSION); + $this->primeDatabase($data, $drv); + $this->primed = $primed; + return true; + } + public function compareExpectations(array $expected): bool { foreach ($expected as $table => $info) { $cols = implode(",", array_keys($info['columns'])); @@ -126,11 +137,12 @@ trait Setup { $rows[] = array_intersect_key($row, $keys); } // compare the result set to the expectations - foreach ($expected as $index => $exp) { - $this->assertContains($exp, $rows, "Result set does not contain record at array index $index."); - $found = array_search($exp, $rows, true); - unset($rows[$found]); + foreach ($rows as $row) { + $this->assertContains($row, $expected, "Result set contains unexpected record."); + $found = array_search($row, $expected); + unset($expected[$found]); } + $this->assertArraySubset($expected, [], "Expectations not in result set."); } } -} +} \ No newline at end of file From d45401fb8b9dd1c8f26db5d56820c45f55ebd18a Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 19 Oct 2017 20:35:45 -0400 Subject: [PATCH 16/20] Adapt NCN to new type converter This has the side-effect of removing the ability to reset a feed's title by passing null explicitly. As a non-standard behaviour it was simpler to just remove it. --- lib/REST/AbstractHandler.php | 49 +------- lib/REST/NextCloudNews/V1_2.php | 139 ++++++++--------------- tests/REST/NextCloudNews/TestNCNV1_2.php | 18 +-- 3 files changed, 62 insertions(+), 144 deletions(-) diff --git a/lib/REST/AbstractHandler.php b/lib/REST/AbstractHandler.php index 8c0988d..7cc7871 100644 --- a/lib/REST/AbstractHandler.php +++ b/lib/REST/AbstractHandler.php @@ -32,52 +32,13 @@ abstract class AbstractHandler implements Handler { return $data; } - protected function NormalizeInput(array $data, array $types, string $dateFormat = null): array { + protected function normalizeInput(array $data, array $types, string $dateFormat = null, int $mode = 0): array { $out = []; - foreach ($data as $key => $value) { - if (!isset($types[$key])) { - $out[$key] = $value; - continue; - } - if (is_null($value)) { + foreach ($types as $key => $type) { + if (isset($data[$key])) { + $out[$key] = ValueInfo::normalize($data[$key], $type | $mode, $dateFormat); + } else { $out[$key] = null; - continue; - } - switch ($types[$key]) { - case "int": - if (valueInfo::int($value) & ValueInfo::VALID) { - $out[$key] = (int) $value; - } - break; - case "string": - if (is_bool($value)) { - $out[$key] = var_export($value, true); - } elseif (!is_scalar($value)) { - break; - } else { - $out[$key] = (string) $value; - } - break; - case "bool": - $test = filter_var($value, \FILTER_VALIDATE_BOOLEAN, \FILTER_NULL_ON_FAILURE); - if (!is_null($test)) { - $out[$key] = $test; - } - break; - case "float": - $test = filter_var($value, \FILTER_VALIDATE_FLOAT); - if ($test !== false) { - $out[$key] = $test; - } - break; - case "datetime": - $t = Date::normalize($value, $dateFormat); - if ($t) { - $out[$key] = $t; - } - break; - default: - throw new Exception("typeUnknown", $types[$key]); } } return $out; diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index bab902e..0074037 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -21,21 +21,21 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { protected $dateFormat = "unix"; protected $validInput = [ - 'name' => "string", - 'url' => "string", - 'folderId' => "int", - 'feedTitle' => "string", - 'userId' => "string", - 'feedId' => "int", - 'newestItemId' => "int", - 'batchSize' => "int", - 'offset' => "int", - 'type' => "int", - 'id' => "int", - 'getRead' => "bool", - 'oldestFirst' => "bool", - 'lastModified' => "datetime", - // 'items' => "array int", // just pass these through + 'name' => ValueInfo::T_STRING, + 'url' => ValueInfo::T_STRING, + 'folderId' => ValueInfo::T_INT, + 'feedTitle' => ValueInfo::T_STRING, + 'userId' => ValueInfo::T_STRING, + 'feedId' => ValueInfo::T_INT, + 'newestItemId' => ValueInfo::T_INT, + 'batchSize' => ValueInfo::T_INT, + 'offset' => ValueInfo::T_INT, + 'type' => ValueInfo::T_INT, + 'id' => ValueInfo::T_INT, + 'getRead' => ValueInfo::T_BOOL, + 'oldestFirst' => ValueInfo::T_BOOL, + 'lastModified' => ValueInfo::T_DATE, + 'items' => ValueInfo::T_MIXED | ValueInfo::M_ARRAY, ]; public function __construct() { @@ -61,10 +61,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { $data = []; } // FIXME: Do query parameters take precedence in NextCloud? Is there a conflict error when values differ? - $data = $this->normalizeInput($data, $this->validInput, "U"); - $query = $this->normalizeInput($req->query, $this->validInput, "U"); - $data = array_merge($data, $query); - unset($query); + $data = $this->normalizeInput(array_merge($data, $req->query), $this->validInput, "unix"); // check to make sure the requested function is implemented try { $func = $this->chooseCall($req->paths, $req->method); @@ -233,7 +230,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // create a folder protected function folderAdd(array $url, array $data): Response { try { - $folder = Arsse::$db->folderAdd(Arsse::$user->id, $data); + $folder = Arsse::$db->folderAdd(Arsse::$user->id, ['name' => $data['name']]); } catch (ExceptionInput $e) { switch ($e->getCode()) { // folder already exists @@ -263,13 +260,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // rename a folder (also supports moving nesting folders, but this is not a feature of the API) protected function folderRename(array $url, array $data): Response { - // there must be some change to be made - if (!sizeof($data)) { - return new Response(422); - } - // perform the edit try { - Arsse::$db->folderPropertiesSet(Arsse::$user->id, (int) $url[1], $data); + Arsse::$db->folderPropertiesSet(Arsse::$user->id, (int) $url[1], ['name' => $data['name']]); } catch (ExceptionInput $e) { switch ($e->getCode()) { // folder does not exist @@ -288,15 +280,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // mark all articles associated with a folder as read protected function folderMarkRead(array $url, array $data): Response { - $c = new Context; - if (isset($data['newestItemId'])) { - // if the item ID is valid (i.e. an integer), add it to the context - $c->latestEdition($data['newestItemId']); - } else { - // otherwise return an error + if (!ValueInfo::id($data['newestItemId'])) { + // if the item ID is invalid (i.e. not a positive integer), this is an error return new Response(422); } - // add the folder ID to the context + // build the context + $c = new Context; + $c->latestEdition((int) $data['newestItemId']); $c->folder((int) $url[1]); // perform the operation try { @@ -330,10 +320,6 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { if (Arsse::$user->rightsGet(Arsse::$user->id)==User::RIGHTS_NONE) { return new Response(403); } - // perform an update of a single feed - if (!isset($data['feedId'])) { - return new Response(422); - } try { Arsse::$db->feedUpdate($data['feedId']); } catch (ExceptionInput $e) { @@ -351,16 +337,10 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // add a new feed protected function subscriptionAdd(array $url, array $data): Response { - // normalize the feed URL - if (!isset($data['url'])) { - return new Response(422); - } - // normalize the folder ID, if specified - $folder = isset($data['folderId']) ? $data['folderId'] : null; // try to add the feed $tr = Arsse::$db->begin(); try { - $id = Arsse::$db->subscriptionAdd(Arsse::$user->id, $data['url']); + $id = Arsse::$db->subscriptionAdd(Arsse::$user->id, (string) $data['url']); } catch (ExceptionInput $e) { // feed already exists return new Response(409); @@ -369,9 +349,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { return new Response(422); } // if a folder was specified, move the feed to the correct folder; silently ignore errors - if ($folder) { + if ($data['folderId']) { try { - Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, $id, ['folder' => $folder]); + Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, $id, ['folder' => $data['folderId']]); } catch (ExceptionInput $e) { } } @@ -416,16 +396,8 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // rename a feed protected function subscriptionRename(array $url, array $data): Response { - // normalize input - $in = []; - if (array_key_exists('feedTitle', $data)) { // we use array_key_exists because null is a valid input - $in['title'] = $data['feedTitle']; - } else { - return new Response(422); - } - // perform the renaming try { - Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], $in); + Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], ['title' => (string) $data['feedTitle']]); } catch (ExceptionInput $e) { switch ($e->getCode()) { // subscription does not exist @@ -442,16 +414,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // move a feed to a folder protected function subscriptionMove(array $url, array $data): Response { - // normalize input - $in = []; - if (isset($data['folderId'])) { - $in['folder'] = $data['folderId'] ? $data['folderId'] : null; - } else { + // if no folder is specified this is an error + if (!isset($data['folderId'])) { return new Response(422); } // perform the move try { - Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], $in); + Arsse::$db->subscriptionPropertiesSet(Arsse::$user->id, (int) $url[1], ['folder' => $data['folderId']]); } catch (ExceptionInput $e) { switch ($e->getCode()) { case 10239: // subscription does not exist @@ -468,14 +437,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // mark all articles associated with a subscription as read protected function subscriptionMarkRead(array $url, array $data): Response { - $c = new Context; - if (isset($data['newestItemId'])) { - $c->latestEdition($data['newestItemId']); - } else { - // otherwise return an error + if (!ValueInfo::id($data['newestItemId'])) { + // if the item ID is invalid (i.e. not a positive integer), this is an error return new Response(422); } - // add the subscription ID to the context + // build the context + $c = new Context; + $c->latestEdition((int) $data['newestItemId']); $c->subscription((int) $url[1]); // perform the operation try { @@ -492,17 +460,17 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // set the context options supplied by the client $c = new Context; // set the batch size - if (isset($data['batchSize']) && $data['batchSize'] > 0) { + if ($data['batchSize'] > 0) { $c->limit($data['batchSize']); } // set the order of returned items - if (isset($data['oldestFirst']) && $data['oldestFirst']) { + if ($data['oldestFirst']) { $c->reverse(false); } else { $c->reverse(true); } // set the edition mark-off; the database uses an or-equal comparison for internal consistency, but the protocol does not, so we must adjust by one - if (isset($data['offset']) && $data['offset'] > 0) { + if ($data['offset'] > 0) { if ($c->reverse) { $c->latestEdition($data['offset'] - 1); } else { @@ -510,13 +478,11 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { } } // set whether to only return unread - if (isset($data['getRead']) && !$data['getRead']) { + if (!ValueInfo::bool($data['getRead'], true)) { $c->unread(true); } // if no type is specified assume 3 (All) - if (!isset($data['type'])) { - $data['type'] = 3; - } + $data['type'] = $data['type'] ?? 3; switch ($data['type']) { case 0: // feed if (isset($data['id'])) { @@ -535,7 +501,7 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // return all items } // whether to return only updated items - if (isset($data['lastModified'])) { + if ($data['lastModified']) { $c->modifiedSince($data['lastModified']); } // perform the fetch @@ -555,14 +521,13 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { // mark all articles as read protected function articleMarkReadAll(array $url, array $data): Response { - $c = new Context; - if (isset($data['newestItemId'])) { - // set the newest item ID as specified - $c->latestEdition($data['newestItemId']); - } else { - // otherwise return an error + if (!ValueInfo::id($data['newestItemId'])) { + // if the item ID is invalid (i.e. not a positive integer), this is an error return new Response(422); } + // build the context + $c = new Context; + $c->latestEdition((int) $data['newestItemId']); // perform the operation Arsse::$db->articleMark(Arsse::$user->id, ['read' => true], $c); return new Response(204); @@ -604,13 +569,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { protected function articleMarkReadMulti(array $url, array $data): Response { // determine whether to mark read or unread $set = ($url[1]=="read"); - // if the input data is not at all valid, return an error - if (!isset($data['items']) || !is_array($data['items'])) { - return new Response(422); - } // start a transaction and loop through the items $t = Arsse::$db->begin(); - $in = array_chunk($data['items'], 50); + $in = array_chunk($data['items'] ?? [], 50); for ($a = 0; $a < sizeof($in); $a++) { // initialize the matching context $c = new Context; @@ -628,13 +589,9 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { protected function articleMarkStarredMulti(array $url, array $data): Response { // determine whether to mark starred or unstarred $set = ($url[1]=="star"); - // if the input data is not at all valid, return an error - if (!isset($data['items']) || !is_array($data['items'])) { - return new Response(422); - } // start a transaction and loop through the items $t = Arsse::$db->begin(); - $in = array_chunk(array_column($data['items'], "guidHash"), 50); + $in = array_chunk(array_column($data['items'] ?? [], "guidHash"), 50); for ($a = 0; $a < sizeof($in); $a++) { // initialize the matching context $c = new Context; diff --git a/tests/REST/NextCloudNews/TestNCNV1_2.php b/tests/REST/NextCloudNews/TestNCNV1_2.php index 0d8e08e..956a249 100644 --- a/tests/REST/NextCloudNews/TestNCNV1_2.php +++ b/tests/REST/NextCloudNews/TestNCNV1_2.php @@ -388,6 +388,7 @@ class TestNCNV1_2 extends Test\AbstractTest { ['id' => 2, 'name' => "Hardware", 'parent' => null], ]; // set of various mocks for testing + Phake::when(Arsse::$db)->folderAdd($this->anything(), $this->anything())->thenThrow(new \Exception); Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $in[0])->thenReturn(1)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->folderAdd(Arsse::$user->id, $in[1])->thenReturn(2)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->folderPropertiesGet(Arsse::$user->id, 1)->thenReturn($out[0]); @@ -499,6 +500,7 @@ class TestNCNV1_2 extends Test\AbstractTest { // set up the necessary mocks Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.com/news.atom")->thenReturn(2112)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "http://example.org/news.atom")->thenReturn(42)->thenThrow(new ExceptionInput("constraintViolation")); // error on the second call + Phake::when(Arsse::$db)->subscriptionAdd(Arsse::$user->id, "")->thenThrow(new \JKingWeb\Arsse\Feed\Exception("", new \PicoFeed\Reader\SubscriptionNotFoundException)); Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 2112)->thenReturn($this->feeds['db'][0]); Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 42)->thenReturn($this->feeds['db'][1]); Phake::when(Arsse::$db)->subscriptionPropertiesGet(Arsse::$user->id, 47)->thenReturn($this->feeds['db'][2]); @@ -584,7 +586,7 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, $this->identicalTo(['title' => ""]))->thenThrow(new ExceptionInput("missing")); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 1, $this->identicalTo(['title' => false]))->thenThrow(new ExceptionInput("missing")); Phake::when(Arsse::$db)->subscriptionPropertiesSet(Arsse::$user->id, 42, $this->anything())->thenThrow(new ExceptionInput("subjectMissing")); - $exp = new Response(204); + $exp = new Response(422); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/rename", json_encode($in[0]), 'application/json'))); $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/feeds/1/rename", json_encode($in[1]), 'application/json'))); @@ -628,7 +630,7 @@ class TestNCNV1_2 extends Test\AbstractTest { ]; Phake::when(Arsse::$db)->feedUpdate(42)->thenReturn(true); Phake::when(Arsse::$db)->feedUpdate(2112)->thenThrow(new ExceptionInput("subjectMissing")); - Phake::when(Arsse::$db)->feedUpdate(-1)->thenThrow(new ExceptionInput("typeViolation")); + Phake::when(Arsse::$db)->feedUpdate($this->lessThan(1))->thenThrow(new ExceptionInput("typeViolation")); $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("GET", "/feeds/update", json_encode($in[0]), 'application/json'))); $exp = new Response(404); @@ -788,7 +790,7 @@ class TestNCNV1_2 extends Test\AbstractTest { Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->editions($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles([]))->thenThrow(new ExceptionInput("tooShort")); // data model function requires one valid integer for multiples Phake::when(Arsse::$db)->articleMark(Arsse::$user->id, $this->anything(), (new Context)->articles($in[1]))->thenThrow(new ExceptionInput("tooLong")); // data model function limited to 50 items for multiples - $exp = new Response(422); + $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple"))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple"))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple"))); @@ -797,14 +799,12 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => "ook"]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => "ook"]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unstar/multiple", json_encode(['items' => "ook"]), 'application/json'))); - $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple", json_encode(['items' => []]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => []]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple", json_encode(['items' => $in[0]]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => $in[0]]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/read/multiple", json_encode(['items' => $in[1]]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unread/multiple", json_encode(['items' => $in[1]]), 'application/json'))); - $exp = new Response(204); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => []]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unstar/multiple", json_encode(['items' => []]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => $inStar[0]]), 'application/json'))); @@ -812,13 +812,13 @@ class TestNCNV1_2 extends Test\AbstractTest { $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/star/multiple", json_encode(['items' => $inStar[1]]), 'application/json'))); $this->assertEquals($exp, $this->h->dispatch(new Request("PUT", "/items/unstar/multiple", json_encode(['items' => $inStar[1]]), 'application/json'))); // ensure the data model was queried appropriately for read/unread - Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions([])); - Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[0])); + Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $read, (new Context)->editions([])); + Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[0])); Phake::verify(Arsse::$db, Phake::times(0))->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[1])); Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[2])); Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $read, (new Context)->editions($in[3])); - Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions([])); - Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[0])); + Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $unread, (new Context)->editions([])); + Phake::verify(Arsse::$db, Phake::times(2))->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[0])); Phake::verify(Arsse::$db, Phake::times(0))->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[1])); Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[2])); Phake::verify(Arsse::$db)->articleMark(Arsse::$user->id, $unread, (new Context)->editions($in[3])); From f690bbb3a94b899776c390b688093a0b14266cd4 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 19 Oct 2017 22:39:39 -0400 Subject: [PATCH 17/20] Alias Date::normalize to ValueInfo::normalize --- lib/Misc/Date.php | 37 ++----------------------------------- 1 file changed, 2 insertions(+), 35 deletions(-) diff --git a/lib/Misc/Date.php b/lib/Misc/Date.php index cbde60f..37a9587 100644 --- a/lib/Misc/Date.php +++ b/lib/Misc/Date.php @@ -35,41 +35,8 @@ class Date { return $date->format($f); } - public static function normalize($date, string $inFormat = null, bool $inLocal = false) { - if ($date instanceof \DateTimeInterface) { - return $date; - } elseif (is_numeric($date)) { - $time = (int) $date; - } elseif ($date===null) { - return null; - } elseif (is_string($date)) { - try { - $tz = (!$inLocal) ? new \DateTimeZone("UTC") : null; - if (!is_null($inFormat)) { - switch ($inFormat) { - case 'http': $f = "D, d M Y H:i:s \G\M\T"; break; - case 'iso8601': $f = "Y-m-d\TH:i:sP"; break; - case 'sql': $f = "Y-m-d H:i:s"; break; - case 'date': $f = "Y-m-d"; break; - case 'time': $f = "H:i:s"; break; - default: $f = $inFormat; break; - } - return \DateTime::createFromFormat("!".$f, $date, $tz); - } else { - return new \DateTime($date, $tz); - } - } catch (\Throwable $e) { - return null; - } - } elseif (is_bool($date)) { - return null; - } else { - $time = (int) $date; - } - $tz = (!$inLocal) ? new \DateTimeZone("UTC") : null; - $d = new \DateTime("now", $tz); - $d->setTimestamp($time); - return $d; + public static function normalize($date, string $inFormat = null) { + return ValueInfo::normalize($date, ValueInfo::T_DATE, $inFormat); } public static function add(string $interval, $date = null): \DateTimeInterface { From 65963f228f578f7ea1bea155d349bb33181cf8f0 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Fri, 20 Oct 2017 18:41:21 -0400 Subject: [PATCH 18/20] CS fixes --- .php_cs.dist | 2 +- lib/Misc/ValueInfo.php | 6 +- tests/Misc/TestValueInfo.php | 210 +++++++++++++++++------------------ tests/bootstrap.php | 2 +- tests/lib/Database/Setup.php | 2 +- 5 files changed, 111 insertions(+), 111 deletions(-) diff --git a/.php_cs.dist b/.php_cs.dist index f2e2fc2..1410543 100644 --- a/.php_cs.dist +++ b/.php_cs.dist @@ -1,7 +1,7 @@ format(Date::FORMAT['iso8601'][1]); } elseif (is_float($value) && is_finite($value)) { $out = (string) $value; - if(!strpos($out, "E")) { + if (!strpos($out, "E")) { return $out; } else { $out = sprintf("%F", $value); @@ -174,7 +174,7 @@ class ValueInfo { if ($dateFormat=="microtime") { // PHP is not able to correctly handle the output of microtime() as the input of DateTime::createFromFormat(), so we fudge it to look like a float if (preg_match("<^0\.\d{6}00 \d+$>", $value)) { - $value = substr($value,11).".".substr($value,2,6); + $value = substr($value, 11).".".substr($value, 2, 6); } else { throw new \Exception; } @@ -251,7 +251,7 @@ class ValueInfo { return self::NULL; } // interpret the value as a float - $float = filter_var($value, \FILTER_VALIDATE_FLOAT); + $float = filter_var($value, \FILTER_VALIDATE_FLOAT); if ($float !== false) { if (!fmod($float, 1)) { // an integral float is acceptable diff --git a/tests/Misc/TestValueInfo.php b/tests/Misc/TestValueInfo.php index a6f8d97..3d7e364 100644 --- a/tests/Misc/TestValueInfo.php +++ b/tests/Misc/TestValueInfo.php @@ -315,85 +315,85 @@ class TestValueInfo extends Test\AbstractTest { - string - array - For each of these types, there is an expected output value, as well as a boolean indicating whether + For each of these types, there is an expected output value, as well as a boolean indicating whether the value should pass or fail a strict normalization. Conversion to DateTime is covered below by a different data set */ - /* Input value null bool int float string array */ - [null, [null,true], [false,false], [0, false], [0.0, false], ["", false], [[], false]], - ["", [null,true], [false,true], [0, false], [0.0, false], ["", true], [[""], false]], - [1, [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[1], false]], - [PHP_INT_MAX, [null,true], [true, false], [PHP_INT_MAX, true], [(float) PHP_INT_MAX,true], [(string) PHP_INT_MAX, true], [[PHP_INT_MAX], false]], - [1.0, [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[1.0], false]], - ["1.0", [null,true], [true, true], [1, true], [1.0, true], ["1.0", true], [["1.0"], false]], - ["001.0", [null,true], [true, true], [1, true], [1.0, true], ["001.0", true], [["001.0"], false]], - ["1.0e2", [null,true], [true, false], [100, true], [100.0, true], ["1.0e2", true], [["1.0e2"], false]], - ["1", [null,true], [true, true], [1, true], [1.0, true], ["1", true], [["1"], false]], - ["001", [null,true], [true, true], [1, true], [1.0, true], ["001", true], [["001"], false]], - ["1e2", [null,true], [true, false], [100, true], [100.0, true], ["1e2", true], [["1e2"], false]], - ["+1.0", [null,true], [true, true], [1, true], [1.0, true], ["+1.0", true], [["+1.0"], false]], - ["+001.0", [null,true], [true, true], [1, true], [1.0, true], ["+001.0", true], [["+001.0"], false]], - ["+1.0e2", [null,true], [true, false], [100, true], [100.0, true], ["+1.0e2", true], [["+1.0e2"], false]], - ["+1", [null,true], [true, true], [1, true], [1.0, true], ["+1", true], [["+1"], false]], - ["+001", [null,true], [true, true], [1, true], [1.0, true], ["+001", true], [["+001"], false]], - ["+1e2", [null,true], [true, false], [100, true], [100.0, true], ["+1e2", true], [["+1e2"], false]], - [0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[0], false]], - ["0", [null,true], [false,true], [0, true], [0.0, true], ["0", true], [["0"], false]], - ["000", [null,true], [false,true], [0, true], [0.0, true], ["000", true], [["000"], false]], - [0.0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[0.0], false]], - ["0.0", [null,true], [false,true], [0, true], [0.0, true], ["0.0", true], [["0.0"], false]], - ["000.000", [null,true], [false,true], [0, true], [0.0, true], ["000.000", true], [["000.000"], false]], - ["+0", [null,true], [false,true], [0, true], [0.0, true], ["+0", true], [["+0"], false]], - ["+000", [null,true], [false,true], [0, true], [0.0, true], ["+000", true], [["+000"], false]], - ["+0.0", [null,true], [false,true], [0, true], [0.0, true], ["+0.0", true], [["+0.0"], false]], - ["+000.000", [null,true], [false,true], [0, true], [0.0, true], ["+000.000", true], [["+000.000"], false]], - [-1, [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[-1], false]], - [-1.0, [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[-1.0], false]], - ["-1.0", [null,true], [true, false], [-1, true], [-1.0, true], ["-1.0", true], [["-1.0"], false]], - ["-001.0", [null,true], [true, false], [-1, true], [-1.0, true], ["-001.0", true], [["-001.0"], false]], - ["-1.0e2", [null,true], [true, false], [-100, true], [-100.0, true], ["-1.0e2", true], [["-1.0e2"], false]], - ["-1", [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [["-1"], false]], - ["-001", [null,true], [true, false], [-1, true], [-1.0, true], ["-001", true], [["-001"], false]], - ["-1e2", [null,true], [true, false], [-100, true], [-100.0, true], ["-1e2", true], [["-1e2"], false]], - [-0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[-0], false]], - ["-0", [null,true], [false,true], [0, true], [-0.0, true], ["-0", true], [["-0"], false]], - ["-000", [null,true], [false,true], [0, true], [-0.0, true], ["-000", true], [["-000"], false]], - [-0.0, [null,true], [false,true], [0, true], [-0.0, true], ["-0", true], [[-0.0], false]], - ["-0.0", [null,true], [false,true], [0, true], [-0.0, true], ["-0.0", true], [["-0.0"], false]], - ["-000.000", [null,true], [false,true], [0, true], [-0.0, true], ["-000.000", true], [["-000.000"], false]], - [false, [null,true], [false,true], [0, false], [0.0, false], ["", false], [[false], false]], - [true, [null,true], [true, true], [1, false], [1.0, false], ["1", false], [[true], false]], - ["on", [null,true], [true, true], [0, false], [0.0, false], ["on", true], [["on"], false]], - ["off", [null,true], [false,true], [0, false], [0.0, false], ["off", true], [["off"], false]], - ["yes", [null,true], [true, true], [0, false], [0.0, false], ["yes", true], [["yes"], false]], - ["no", [null,true], [false,true], [0, false], [0.0, false], ["no", true], [["no"], false]], - ["true", [null,true], [true, true], [0, false], [0.0, false], ["true", true], [["true"], false]], - ["false", [null,true], [false,true], [0, false], [0.0, false], ["false", true], [["false"], false]], - [INF, [null,true], [true, false], [0, false], [INF, true], ["INF", false], [[INF], false]], - [-INF, [null,true], [true, false], [0, false], [-INF, true], ["-INF", false], [[-INF], false]], - [NAN, [null,true], [false,false], [0, false], [NAN, true], ["NAN", false], [[], false]], - [[], [null,true], [false,false], [0, false], [0.0, false], ["", false], [[], true] ], - ["some string", [null,true], [true, false], [0, false], [0.0, false], ["some string", true], [["some string"], false]], - [" ", [null,true], [true, false], [0, false], [0.0, false], [" ", true], [[" "], false]], - [new \StdClass, [null,true], [true, false], [0, false], [0.0, false], ["", false], [[new \StdClass], false]], - [new StrClass(""), [null,true], [false,true], [0, false], [0.0, false], ["", true], [[new StrClass("")], false]], - [new StrClass("1"), [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[new StrClass("1")], false]], - [new StrClass("0"), [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[new StrClass("0")], false]], - [new StrClass("-1"), [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[new StrClass("-1")], false]], - [new StrClass("Msg"), [null,true], [true, false], [0, false], [0.0, false], ["Msg", true], [[new StrClass("Msg")], false]], - [new StrClass(" "), [null,true], [true, false], [0, false], [0.0, false], [" ", true], [[new StrClass(" ")], false]], - [2.5, [null,true], [true, false], [2, false], [2.5, true], ["2.5", true], [[2.5], false]], - [0.5, [null,true], [true, false], [0, false], [0.5, true], ["0.5", true], [[0.5], false]], - ["2.5", [null,true], [true, false], [2, false], [2.5, true], ["2.5", true], [["2.5"], false]], - ["0.5", [null,true], [true, false], [0, false], [0.5, true], ["0.5", true], [["0.5"], false]], - [$this->d("2010-01-01T00:00:00",0,0), [null,true], [true, false], [1262304000, false], [1262304000.0, false], ["2010-01-01T00:00:00Z",true], [[$this->d("2010-01-01T00:00:00",0,0)],false]], - [$this->d("2010-01-01T00:00:00",0,1), [null,true], [true, false], [1262304000, false], [1262304000.0, false], ["2010-01-01T00:00:00Z",true], [[$this->d("2010-01-01T00:00:00",0,1)],false]], - [$this->d("2010-01-01T00:00:00",1,0), [null,true], [true, false], [1262322000, false], [1262322000.0, false], ["2010-01-01T05:00:00Z",true], [[$this->d("2010-01-01T00:00:00",1,0)],false]], - [$this->d("2010-01-01T00:00:00",1,1), [null,true], [true, false], [1262322000, false], [1262322000.0, false], ["2010-01-01T05:00:00Z",true], [[$this->d("2010-01-01T00:00:00",1,1)],false]], - [1e14, [null,true], [true, false], [100000000000000,true], [1e14, true], ["100000000000000", true], [[1e14], false]], - [1e-6, [null,true], [true, false], [0, false], [1e-6, true], ["0.000001", true], [[1e-6], false]], - [[1,2,3], [null,true], [true, false], [0, false], [0.0, false], ["", false], [[1,2,3], true] ], - [['a'=>1,'b'=>2], [null,true], [true, false], [0, false], [0.0, false], ["", false], [['a'=>1,'b'=>2], true] ], - [new Test\Result([['a'=>1,'b'=>2]]), [null,true], [true, false], [0, false], [0.0, false], ["", false], [[['a'=>1,'b'=>2]], true] ], + /* Input value null bool int float string array */ + [null, [null,true], [false,false], [0, false], [0.0, false], ["", false], [[], false]], + ["", [null,true], [false,true], [0, false], [0.0, false], ["", true], [[""], false]], + [1, [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[1], false]], + [PHP_INT_MAX, [null,true], [true, false], [PHP_INT_MAX, true], [(float) PHP_INT_MAX,true], [(string) PHP_INT_MAX, true], [[PHP_INT_MAX], false]], + [1.0, [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[1.0], false]], + ["1.0", [null,true], [true, true], [1, true], [1.0, true], ["1.0", true], [["1.0"], false]], + ["001.0", [null,true], [true, true], [1, true], [1.0, true], ["001.0", true], [["001.0"], false]], + ["1.0e2", [null,true], [true, false], [100, true], [100.0, true], ["1.0e2", true], [["1.0e2"], false]], + ["1", [null,true], [true, true], [1, true], [1.0, true], ["1", true], [["1"], false]], + ["001", [null,true], [true, true], [1, true], [1.0, true], ["001", true], [["001"], false]], + ["1e2", [null,true], [true, false], [100, true], [100.0, true], ["1e2", true], [["1e2"], false]], + ["+1.0", [null,true], [true, true], [1, true], [1.0, true], ["+1.0", true], [["+1.0"], false]], + ["+001.0", [null,true], [true, true], [1, true], [1.0, true], ["+001.0", true], [["+001.0"], false]], + ["+1.0e2", [null,true], [true, false], [100, true], [100.0, true], ["+1.0e2", true], [["+1.0e2"], false]], + ["+1", [null,true], [true, true], [1, true], [1.0, true], ["+1", true], [["+1"], false]], + ["+001", [null,true], [true, true], [1, true], [1.0, true], ["+001", true], [["+001"], false]], + ["+1e2", [null,true], [true, false], [100, true], [100.0, true], ["+1e2", true], [["+1e2"], false]], + [0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[0], false]], + ["0", [null,true], [false,true], [0, true], [0.0, true], ["0", true], [["0"], false]], + ["000", [null,true], [false,true], [0, true], [0.0, true], ["000", true], [["000"], false]], + [0.0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[0.0], false]], + ["0.0", [null,true], [false,true], [0, true], [0.0, true], ["0.0", true], [["0.0"], false]], + ["000.000", [null,true], [false,true], [0, true], [0.0, true], ["000.000", true], [["000.000"], false]], + ["+0", [null,true], [false,true], [0, true], [0.0, true], ["+0", true], [["+0"], false]], + ["+000", [null,true], [false,true], [0, true], [0.0, true], ["+000", true], [["+000"], false]], + ["+0.0", [null,true], [false,true], [0, true], [0.0, true], ["+0.0", true], [["+0.0"], false]], + ["+000.000", [null,true], [false,true], [0, true], [0.0, true], ["+000.000", true], [["+000.000"], false]], + [-1, [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[-1], false]], + [-1.0, [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[-1.0], false]], + ["-1.0", [null,true], [true, false], [-1, true], [-1.0, true], ["-1.0", true], [["-1.0"], false]], + ["-001.0", [null,true], [true, false], [-1, true], [-1.0, true], ["-001.0", true], [["-001.0"], false]], + ["-1.0e2", [null,true], [true, false], [-100, true], [-100.0, true], ["-1.0e2", true], [["-1.0e2"], false]], + ["-1", [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [["-1"], false]], + ["-001", [null,true], [true, false], [-1, true], [-1.0, true], ["-001", true], [["-001"], false]], + ["-1e2", [null,true], [true, false], [-100, true], [-100.0, true], ["-1e2", true], [["-1e2"], false]], + [-0, [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[-0], false]], + ["-0", [null,true], [false,true], [0, true], [-0.0, true], ["-0", true], [["-0"], false]], + ["-000", [null,true], [false,true], [0, true], [-0.0, true], ["-000", true], [["-000"], false]], + [-0.0, [null,true], [false,true], [0, true], [-0.0, true], ["-0", true], [[-0.0], false]], + ["-0.0", [null,true], [false,true], [0, true], [-0.0, true], ["-0.0", true], [["-0.0"], false]], + ["-000.000", [null,true], [false,true], [0, true], [-0.0, true], ["-000.000", true], [["-000.000"], false]], + [false, [null,true], [false,true], [0, false], [0.0, false], ["", false], [[false], false]], + [true, [null,true], [true, true], [1, false], [1.0, false], ["1", false], [[true], false]], + ["on", [null,true], [true, true], [0, false], [0.0, false], ["on", true], [["on"], false]], + ["off", [null,true], [false,true], [0, false], [0.0, false], ["off", true], [["off"], false]], + ["yes", [null,true], [true, true], [0, false], [0.0, false], ["yes", true], [["yes"], false]], + ["no", [null,true], [false,true], [0, false], [0.0, false], ["no", true], [["no"], false]], + ["true", [null,true], [true, true], [0, false], [0.0, false], ["true", true], [["true"], false]], + ["false", [null,true], [false,true], [0, false], [0.0, false], ["false", true], [["false"], false]], + [INF, [null,true], [true, false], [0, false], [INF, true], ["INF", false], [[INF], false]], + [-INF, [null,true], [true, false], [0, false], [-INF, true], ["-INF", false], [[-INF], false]], + [NAN, [null,true], [false,false], [0, false], [NAN, true], ["NAN", false], [[], false]], + [[], [null,true], [false,false], [0, false], [0.0, false], ["", false], [[], true] ], + ["some string", [null,true], [true, false], [0, false], [0.0, false], ["some string", true], [["some string"], false]], + [" ", [null,true], [true, false], [0, false], [0.0, false], [" ", true], [[" "], false]], + [new \StdClass, [null,true], [true, false], [0, false], [0.0, false], ["", false], [[new \StdClass], false]], + [new StrClass(""), [null,true], [false,true], [0, false], [0.0, false], ["", true], [[new StrClass("")], false]], + [new StrClass("1"), [null,true], [true, true], [1, true], [1.0, true], ["1", true], [[new StrClass("1")], false]], + [new StrClass("0"), [null,true], [false,true], [0, true], [0.0, true], ["0", true], [[new StrClass("0")], false]], + [new StrClass("-1"), [null,true], [true, false], [-1, true], [-1.0, true], ["-1", true], [[new StrClass("-1")], false]], + [new StrClass("Msg"), [null,true], [true, false], [0, false], [0.0, false], ["Msg", true], [[new StrClass("Msg")], false]], + [new StrClass(" "), [null,true], [true, false], [0, false], [0.0, false], [" ", true], [[new StrClass(" ")], false]], + [2.5, [null,true], [true, false], [2, false], [2.5, true], ["2.5", true], [[2.5], false]], + [0.5, [null,true], [true, false], [0, false], [0.5, true], ["0.5", true], [[0.5], false]], + ["2.5", [null,true], [true, false], [2, false], [2.5, true], ["2.5", true], [["2.5"], false]], + ["0.5", [null,true], [true, false], [0, false], [0.5, true], ["0.5", true], [["0.5"], false]], + [$this->d("2010-01-01T00:00:00", 0, 0), [null,true], [true, false], [1262304000, false], [1262304000.0, false], ["2010-01-01T00:00:00Z",true], [[$this->d("2010-01-01T00:00:00", 0, 0)],false]], + [$this->d("2010-01-01T00:00:00", 0, 1), [null,true], [true, false], [1262304000, false], [1262304000.0, false], ["2010-01-01T00:00:00Z",true], [[$this->d("2010-01-01T00:00:00", 0, 1)],false]], + [$this->d("2010-01-01T00:00:00", 1, 0), [null,true], [true, false], [1262322000, false], [1262322000.0, false], ["2010-01-01T05:00:00Z",true], [[$this->d("2010-01-01T00:00:00", 1, 0)],false]], + [$this->d("2010-01-01T00:00:00", 1, 1), [null,true], [true, false], [1262322000, false], [1262322000.0, false], ["2010-01-01T05:00:00Z",true], [[$this->d("2010-01-01T00:00:00", 1, 1)],false]], + [1e14, [null,true], [true, false], [100000000000000,true], [1e14, true], ["100000000000000", true], [[1e14], false]], + [1e-6, [null,true], [true, false], [0, false], [1e-6, true], ["0.000001", true], [[1e-6], false]], + [[1,2,3], [null,true], [true, false], [0, false], [0.0, false], ["", false], [[1,2,3], true] ], + [['a'=>1,'b'=>2], [null,true], [true, false], [0, false], [0.0, false], ["", false], [['a'=>1,'b'=>2], true] ], + [new Test\Result([['a'=>1,'b'=>2]]), [null,true], [true, false], [0, false], [0.0, false], ["", false], [[['a'=>1,'b'=>2]], true] ], ]; $params = [ [I::T_MIXED, "Mixed" ], @@ -430,33 +430,33 @@ class TestValueInfo extends Test\AbstractTest { } // DateTimeInterface tests $tests = [ - /* Input value microtime iso8601 iso8601m http sql date time unix float '!M j, Y (D)' *strtotime* (null) */ - [null, null, null, null, null, null, null, null, null, null, null, null, ], - [$this->d("2010-01-01T00:00:00",0,0), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], - [$this->d("2010-01-01T00:00:00",0,1), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], - [$this->d("2010-01-01T00:00:00",1,0), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), ], - [$this->d("2010-01-01T00:00:00",1,1), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), ], - [1262304000, $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], - [1262304000.123456, $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), ], - [1262304000.42, $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), ], - ["0.12345600 1262304000", $this->t(1262304000.123456), null, null, null, null, null, null, null, null, null, null, ], - ["0.42 1262304000", null, null, null, null, null, null, null, null, null, null, null, ], - ["2010-01-01T00:00:00", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01T00:00:00Z", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01T00:00:00+0000", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01T00:00:00-0000", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01T00:00:00+00:00", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01T00:00:00-05:00", null, $this->t(1262322000), $this->t(1262322000), null, null, null, null, null, null, null, $this->t(1262322000), ], - ["2010-01-01T00:00:00.123456Z", null, null, $this->t(1262304000.123456), null, null, null, null, null, null, null, $this->t(1262304000.123456), ], - ["Fri, 01 Jan 2010 00:00:00 GMT", null, null, null, $this->t(1262304000), null, null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01 00:00:00", null, null, null, null, $this->t(1262304000), null, null, null, null, null, $this->t(1262304000), ], - ["2010-01-01", null, null, null, null, null, $this->t(1262304000), null, null, null, null, $this->t(1262304000), ], - ["12:34:56", null, null, null, null, null, null, $this->t(45296), null, null, null, $this->t(strtotime("today")+45296), ], - ["1262304000", null, null, null, null, null, null, null, $this->t(1262304000), null, null, null, ], - ["1262304000.123456", null, null, null, null, null, null, null, null, $this->t(1262304000.123456), null, null, ], - ["1262304000.42", null, null, null, null, null, null, null, null, $this->t(1262304000.42), null, null, ], - ["Jan 1, 2010 (Fri)", null, null, null, null, null, null, null, null, null, $this->t(1262304000), null, ], - ["First day of Jan 2010 12AM", null, null, null, null, null, null, null, null, null, null, $this->t(1262304000), ], + /* Input value microtime iso8601 iso8601m http sql date time unix float '!M j, Y (D)' *strtotime* (null) */ + [null, null, null, null, null, null, null, null, null, null, null, null, ], + [$this->d("2010-01-01T00:00:00", 0, 0), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], + [$this->d("2010-01-01T00:00:00", 0, 1), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], + [$this->d("2010-01-01T00:00:00", 1, 0), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), ], + [$this->d("2010-01-01T00:00:00", 1, 1), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), $this->t(1262322000), ], + [1262304000, $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), $this->t(1262304000), ], + [1262304000.123456, $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), $this->t(1262304000.123456), ], + [1262304000.42, $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), $this->t(1262304000.42), ], + ["0.12345600 1262304000", $this->t(1262304000.123456), null, null, null, null, null, null, null, null, null, null, ], + ["0.42 1262304000", null, null, null, null, null, null, null, null, null, null, null, ], + ["2010-01-01T00:00:00", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00Z", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00+0000", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00-0000", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00+00:00", null, $this->t(1262304000), $this->t(1262304000), null, null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01T00:00:00-05:00", null, $this->t(1262322000), $this->t(1262322000), null, null, null, null, null, null, null, $this->t(1262322000), ], + ["2010-01-01T00:00:00.123456Z", null, null, $this->t(1262304000.123456), null, null, null, null, null, null, null, $this->t(1262304000.123456), ], + ["Fri, 01 Jan 2010 00:00:00 GMT", null, null, null, $this->t(1262304000), null, null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01 00:00:00", null, null, null, null, $this->t(1262304000), null, null, null, null, null, $this->t(1262304000), ], + ["2010-01-01", null, null, null, null, null, $this->t(1262304000), null, null, null, null, $this->t(1262304000), ], + ["12:34:56", null, null, null, null, null, null, $this->t(45296), null, null, null, $this->t(strtotime("today")+45296), ], + ["1262304000", null, null, null, null, null, null, null, $this->t(1262304000), null, null, null, ], + ["1262304000.123456", null, null, null, null, null, null, null, null, $this->t(1262304000.123456), null, null, ], + ["1262304000.42", null, null, null, null, null, null, null, null, $this->t(1262304000.42), null, null, ], + ["Jan 1, 2010 (Fri)", null, null, null, null, null, null, null, null, null, $this->t(1262304000), null, ], + ["First day of Jan 2010 12AM", null, null, null, null, null, null, null, null, null, null, $this->t(1262304000), ], ]; $formats = [ "microtime", @@ -513,4 +513,4 @@ class TestValueInfo extends Test\AbstractTest { protected function t(float $spec): \DateTime { return \DateTime::createFromFormat("U.u", sprintf("%F", $spec), new \DateTimeZone("UTC")); } -} \ No newline at end of file +} diff --git a/tests/bootstrap.php b/tests/bootstrap.php index c74e551..66f69a8 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -5,4 +5,4 @@ namespace JKingWeb\Arsse; const NS_BASE = __NAMESPACE__."\\"; define(NS_BASE."BASE", dirname(__DIR__).DIRECTORY_SEPARATOR); -require_once BASE."vendor".DIRECTORY_SEPARATOR."autoload.php"; \ No newline at end of file +require_once BASE."vendor".DIRECTORY_SEPARATOR."autoload.php"; diff --git a/tests/lib/Database/Setup.php b/tests/lib/Database/Setup.php index 8267b0c..edd9de4 100644 --- a/tests/lib/Database/Setup.php +++ b/tests/lib/Database/Setup.php @@ -145,4 +145,4 @@ trait Setup { $this->assertArraySubset($expected, [], "Expectations not in result set."); } } -} \ No newline at end of file +} From 9304f990327950c83de8beaea624e28af16927eb Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 5 Nov 2017 22:13:44 -0500 Subject: [PATCH 19/20] Make result sets single-use; change savewepoint exceptions - Result sets are now single-use; this is required for PDO drivers (PDO result sets are not rewindable) - Change savepoint exceptions to be simple database exceptions; codes remain the same --- lib/AbstractException.php | 7 ++++--- lib/Db/AbstractDriver.php | 12 ++++++------ lib/Db/ExceptionSavepoint.php | 6 ------ lib/Db/SQLite3/Result.php | 13 +++++-------- locale/en.php | 7 ++++--- tests/Db/SQLite3/TestDbDriverSQLite3.php | 8 ++++---- tests/Db/SQLite3/TestDbResultSQLite3.php | 12 +++++++++++- tests/Db/TestTransaction.php | 2 +- 8 files changed, 35 insertions(+), 32 deletions(-) delete mode 100644 lib/Db/ExceptionSavepoint.php diff --git a/lib/AbstractException.php b/lib/AbstractException.php index 5d5ae3d..da0fc7f 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -33,9 +33,10 @@ abstract class AbstractException extends \Exception { "Db/Exception.paramTypeUnknown" => 10222, "Db/Exception.paramTypeMissing" => 10223, "Db/Exception.engineErrorGeneral" => 10224, // this symbol may have engine-specific duplicates to accomodate engine-specific error string construction - "Db/Exception.unknownSavepointStatus" => 10225, - "Db/ExceptionSavepoint.invalid" => 10226, - "Db/ExceptionSavepoint.stale" => 10227, + "Db/Exception.savepointStatusUnknown" => 10225, + "Db/Exception.savepointInvalid" => 10226, + "Db/Exception.savepointStale" => 10227, + "Db/Exception.resultReused" => 10227, "Db/ExceptionInput.missing" => 10231, "Db/ExceptionInput.whitespace" => 10232, "Db/ExceptionInput.tooLong" => 10233, diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index b49ceb1..6ae5992 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -60,9 +60,9 @@ abstract class AbstractDriver implements Driver { break; case self::TR_COMMIT: case self::TR_ROLLBACK: //@codeCoverageIgnore - throw new ExceptionSavepoint("stale", ['action' => "commit", 'index' => $index]); + throw new Exception("savepointStale", ['action' => "commit", 'index' => $index]); default: - throw new Exception("unknownSavepointStatus", $this->transStatus[$index]); //@codeCoverageIgnore + throw new Exception("savepointStatusUnknown", $this->transStatus[$index]); // @codeCoverageIgnore } if ($index==$this->transDepth) { while ($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { @@ -76,7 +76,7 @@ abstract class AbstractDriver implements Driver { } return $out; } else { - throw new ExceptionSavepoint("invalid", ['action' => "commit", 'index' => $index]); + throw new Exception("savepointInvalid", ['action' => "commit", 'index' => $index]); } } @@ -106,9 +106,9 @@ abstract class AbstractDriver implements Driver { break; case self::TR_COMMIT: case self::TR_ROLLBACK: //@codeCoverageIgnore - throw new ExceptionSavepoint("stale", ['action' => "rollback", 'index' => $index]); + throw new Exception("savepointStale", ['action' => "rollback", 'index' => $index]); default: - throw new Exception("unknownSavepointStatus", $this->transStatus[$index]); //@codeCoverageIgnore + throw new Exception("savepointStatusUnknown", $this->transStatus[$index]); // @codeCoverageIgnore } if ($index==$this->transDepth) { while ($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { @@ -122,7 +122,7 @@ abstract class AbstractDriver implements Driver { } return $out; } else { - throw new ExceptionSavepoint("invalid", ['action' => "rollback", 'index' => $index]); + throw new Exception("savepointInvalid", ['action' => "rollback", 'index' => $index]); } } diff --git a/lib/Db/ExceptionSavepoint.php b/lib/Db/ExceptionSavepoint.php deleted file mode 100644 index a90839e..0000000 --- a/lib/Db/ExceptionSavepoint.php +++ /dev/null @@ -1,6 +0,0 @@ -pos = 0; - $this->cur = null; - $this->set->reset(); + if ($this->pos) { + throw new Exception("resultReused"); + } } } diff --git a/locale/en.php b/locale/en.php index d232f1f..e7f9e29 100644 --- a/locale/en.php +++ b/locale/en.php @@ -123,7 +123,10 @@ return [ other {Automatic updating of the {driver_name} database failed because its version, {current}, is newer than the requested version, {target}} }', 'Exception.JKingWeb/Arsse/Db/Exception.engineErrorGeneral' => '{0}', - 'Exception.JKingWeb/Arsse/Db/Exception.unknownSavepointStatus' => 'Savepoint status code {0} not implemented', + 'Exception.JKingWeb/Arsse/Db/Exception.savepointStatusUnknown' => 'Savepoint status code {0} not implemented', + 'Exception.JKingWeb/Arsse/Db/Exception.savepointInvalid' => 'Tried to {action} invalid savepoint {index}', + 'Exception.JKingWeb/Arsse/Db/Exception.savepointStale' => 'Tried to {action} stale savepoint {index}', + 'Exception.JKingWeb/Arsse/Db/Exception.resultReused' => 'Result set already iterated', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.missing' => 'Required field "{field}" missing while performing action "{action}"', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.whitespace' => 'Field "{field}" of action "{action}" may not contain only whitespace', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.tooLong' => 'Field "{field}" of action "{action}" has a maximum length of {max}', @@ -136,8 +139,6 @@ return [ 'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineConstraintViolation' => '{0}', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineTypeViolation' => '{0}', 'Exception.JKingWeb/Arsse/Db/ExceptionTimeout.general' => '{0}', - 'Exception.JKingWeb/Arsse/Db/ExceptionSavepoint.invalid' => 'Tried to {action} invalid savepoint {index}', - 'Exception.JKingWeb/Arsse/Db/ExceptionSavepoint.stale' => 'Tried to {action} stale savepoint {index}', '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/Exception.authMissing' => 'Please log in to proceed', diff --git a/tests/Db/SQLite3/TestDbDriverSQLite3.php b/tests/Db/SQLite3/TestDbDriverSQLite3.php index 0c80b5b..59936b9 100644 --- a/tests/Db/SQLite3/TestDbDriverSQLite3.php +++ b/tests/Db/SQLite3/TestDbDriverSQLite3.php @@ -117,14 +117,14 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { public function testReleaseASavepoint() { $this->assertEquals(1, $this->drv->savepointCreate()); $this->assertEquals(true, $this->drv->savepointRelease()); - $this->assertException("invalid", "Db", "ExceptionSavepoint"); + $this->assertException("savepointInvalid", "Db"); $this->drv->savepointRelease(); } public function testUndoASavepoint() { $this->assertEquals(1, $this->drv->savepointCreate()); $this->assertEquals(true, $this->drv->savepointUndo()); - $this->assertException("invalid", "Db", "ExceptionSavepoint"); + $this->assertException("savepointInvalid", "Db"); $this->drv->savepointUndo(); } @@ -141,7 +141,7 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { $this->assertTrue($this->drv->savepointRelease(6)); $this->assertEquals(3, $this->drv->savepointCreate()); $this->assertTrue($this->drv->savepointRelease(2)); - $this->assertException("stale", "Db", "ExceptionSavepoint"); + $this->assertException("savepointStale", "Db"); $this->drv->savepointRelease(2); } @@ -152,7 +152,7 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { $this->assertEquals(4, $this->drv->savepointCreate()); $this->assertTrue($this->drv->savepointRelease(2)); $this->assertFalse($this->drv->savepointUndo(3)); - $this->assertException("stale", "Db", "ExceptionSavepoint"); + $this->assertException("savepointStale", "Db"); $this->drv->savepointUndo(2); } diff --git a/tests/Db/SQLite3/TestDbResultSQLite3.php b/tests/Db/SQLite3/TestDbResultSQLite3.php index 3d92b54..2fd187a 100644 --- a/tests/Db/SQLite3/TestDbResultSQLite3.php +++ b/tests/Db/SQLite3/TestDbResultSQLite3.php @@ -51,10 +51,11 @@ class TestDbResultSQLite3 extends Test\AbstractTest { foreach ($test as $row) { $rows[] = $row['col']; } + $this->assertEquals([1,2,3], $rows); + $this->assertException("resultReused", "Db"); foreach ($test as $row) { $rows[] = $row['col']; } - $this->assertEquals([1,2,3,1,2,3], $rows); } public function testGetSingleValues() { @@ -85,6 +86,15 @@ class TestDbResultSQLite3 extends Test\AbstractTest { $this->assertEquals($rows[0], $test->getRow()); $this->assertEquals($rows[1], $test->getRow()); $this->assertSame(null, $test->getRow()); + } + + public function testGetAllRows() { + $set = $this->c->query("SELECT '2112' as album, '2112' as track union select 'Clockwork Angels' as album, 'The Wreckers' as track"); + $rows = [ + ['album' => '2112', 'track' => '2112'], + ['album' => 'Clockwork Angels', 'track' => 'The Wreckers'], + ]; + $test = new Db\SQLite3\Result($set); $this->assertEquals($rows, $test->getAll()); } } diff --git a/tests/Db/TestTransaction.php b/tests/Db/TestTransaction.php index 721b17d..9c43510 100644 --- a/tests/Db/TestTransaction.php +++ b/tests/Db/TestTransaction.php @@ -47,7 +47,7 @@ class TestTransaction extends Test\AbstractTest { } public function testIgnoreRollbackErrors() { - Phake::when($this->drv)->savepointUndo->thenThrow(new Db\ExceptionSavepoint("stale")); + Phake::when($this->drv)->savepointUndo->thenThrow(new Db\Exception("savepointStale")); $tr1 = new Transaction($this->drv); $tr2 = new Transaction($this->drv); unset($tr1, $tr2); // no exception should bubble up From f51152980cbfd598e43e2e3a0949533f852a2d73 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 5 Nov 2017 22:28:19 -0500 Subject: [PATCH 20/20] Introduce abstract class for result sets Most of the SQLite3 Result class' methods are fully generic, so it should be helpful for future result sets --- lib/Db/AbstractResult.php | 55 +++++++++++++++++++++++++++++++++++++++ lib/Db/SQLite3/Result.php | 40 ++-------------------------- 2 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 lib/Db/AbstractResult.php diff --git a/lib/Db/AbstractResult.php b/lib/Db/AbstractResult.php new file mode 100644 index 0000000..dce2926 --- /dev/null +++ b/lib/Db/AbstractResult.php @@ -0,0 +1,55 @@ +next(); + if ($this->valid()) { + $keys = array_keys($this->cur); + return $this->cur[array_shift($keys)]; + } + return null; + } + + public function getRow() { + $this->next(); + return ($this->valid() ? $this->cur : null); + } + + public function getAll(): array { + return iterator_to_array($this, false); + } + + abstract public function changes(); + + abstract public function lastId(); + + // PHP iterator methods + + abstract public function valid(); + + public function next() { + $this->cur = null; + $this->pos += 1; + } + + public function current() { + return $this->cur; + } + + public function key() { + return $this->pos; + } + + public function rewind() { + if ($this->pos) { + throw new Exception("resultReused"); + } + } +} diff --git a/lib/Db/SQLite3/Result.php b/lib/Db/SQLite3/Result.php index df82390..9aed7a9 100644 --- a/lib/Db/SQLite3/Result.php +++ b/lib/Db/SQLite3/Result.php @@ -1,9 +1,10 @@ next(); - if ($this->valid()) { - $keys = array_keys($this->cur); - return $this->cur[array_shift($keys)]; - } - return null; - } - - public function getRow() { - $this->next(); - return ($this->valid() ? $this->cur : null); - } - - public function getAll(): array { - return iterator_to_array($this, false); - } - public function changes() { return $this->rows; } @@ -62,23 +45,4 @@ class Result implements \JKingWeb\Arsse\Db\Result { $this->cur = $this->set->fetchArray(\SQLITE3_ASSOC); return ($this->cur !== false); } - - public function next() { - $this->cur = null; - $this->pos += 1; - } - - public function current() { - return $this->cur; - } - - public function key() { - return $this->pos; - } - - public function rewind() { - if ($this->pos) { - throw new Exception("resultReused"); - } - } }