From db132c0c2401a5cf4c60bdc3b705883d4a95b878 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 4 May 2017 19:12:33 -0400 Subject: [PATCH] Various changes to Database - Introduced "strict" binding types for use when inserting into NOT NULL columns: any null value supplied is always cast to the supplied type rather than passing through - Fixed feed updating further - Filled out full complement of subscription manipulation functions - made folderPropertiesSet possibly return false --- lib/Database.php | 68 +++++++++++++++++++++++++++++------- lib/Db/AbstractStatement.php | 14 +++++++- lib/Db/SQLite3/Statement.php | 30 ++++++++-------- tests/test.php | 8 ++++- 4 files changed, 90 insertions(+), 30 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index 853700d..1af1d6d 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -352,10 +352,10 @@ class Database { public function folderPropertiesSet(string $user, int $id, array $data): bool { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); - // layer the existing folder properties onto the new desired one + // layer the existing folder properties onto the new desired ones; this also has the effect of checking whether the folder is valid $data = array_merge($this->folderPropertiesGet($user, $id), $data); // if the desired folder name is missing or invalid, throw an exception - if(!array_key_exists("name", $data) || $data['name']=="") { + if($data['name']=="") { throw new Db\ExceptionInput("missing", ["action" => __FUNCTION__, "field" => "name"]); } else if(!strlen(trim($data['name']))) { throw new Db\ExceptionInput("whitespace", ["action" => __FUNCTION__, "field" => "name"]); @@ -389,8 +389,7 @@ class Database { 'parent' => "int", ]; list($setClause, $setTypes, $setValues) = $this->generateSet($data, $valid); - $this->db->prepare("UPDATE arsse_folders set $setClause where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id); - return true; + return (bool) $this->db->prepare("UPDATE arsse_folders set $setClause where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id)->changes(); } public function subscriptionAdd(string $user, string $url, string $fetchUser = "", string $fetchPassword = ""): int { @@ -414,21 +413,64 @@ class Database { return $this->db->prepare('INSERT INTO arsse_subscriptions(owner,feed) values(?,?)', 'str', 'int')->run($user, $feedID)->lastId(); } - public function subscriptionRemove(string $user, int $id): bool { - if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); - return (bool) $this->db->prepare("DELETE from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); - } - - public function subscriptionList(string $user, int $folder = null): Db\Result { + public function subscriptionList(string $user, int $folder = null, int $id = null): Db\Result { if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); // check to make sure the folder exists, if one is specified + $query = + "SELECT + arsse_subscriptions.id, + url,favicon,source,folder,added,pinned,err_count,err_msg,order_type, + CASE WHEN arsse_subscriptions.title THEN arsse_subscriptions.title ELSE arsse_feeds.title END as title, + (SELECT count(*) from arsse_articles where feed is arsse_subscriptions.feed) - (SELECT count(*) from (SELECT article,feed from arsse_subscription_articles join arsse_articles on article = arsse_articles.id where owner is ? and feed is arsse_feeds.id and read is 1)) as unread + from arsse_subscriptions join arsse_feeds on feed = arsse_feeds.id where owner is ?"; if(!is_null($folder)) { if(!$this->db->prepare("SELECT count(*) from arsse_folders where owner is ? and id is ?", "str", "int")->run($user, $folder)->getValue()) { throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "folder", 'id' => $folder]); } + return $this->db->prepare( + "WITH RECURSIVE folders(folder) as (SELECT ? union select id from arsse_folders join folders on parent is folder) $query and folder in (select folder from folders)", + "int", "str", "str" + )->run($folder, $user, $user); + } else if(!is_null($id)) { + // this condition facilitates the implementation of subscriptionPropertiesGet, which would otherwise have to duplicate the complex query + return $this->db->prepare("$query and arsse_subscriptions.id is ?", "str", "str", "int")->run($user, $user, $id); + } else { + return $this->db->prepare($query, "str", "str")->run($user, $user); + } + } + + public function subscriptionRemove(string $user, int $id): bool { + if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + return (bool) $this->db->prepare("DELETE from arsse_subscriptions where owner is ? and id is ?", "str", "int")->run($user, $id)->changes(); + } + + public function subscriptionPropertiesGet(string $user, int $id): array { + if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + // disable authorization checks for the list call + Data::$user->authorizationEnabled(false); + $sub = $this->subscriptionList($user, null, $id)->getRow(); + Data::$user->authorizationEnabled(true); + if(!$sub) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); + return $sub; + } + + public function subscriptionPropertiesSet(string $user, int $id, array $data): bool { + if(!Data::$user->authorize($user, __FUNCTION__)) throw new User\ExceptionAuthz("notAuthorized", ["action" => __FUNCTION__, "user" => $user]); + if(!$this->userExists($user)) throw new User\Exception("doesNotExist", ["user" => $user, "action" => __FUNCTION__]); + $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("idMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); } - return $this->db->prepare("SELECT arsse_subscriptions.id, arsse_feeds.url, arsse_feeds.title from arsse_subscriptions join arsse_feeds on feed = arsse_feeds.id where arsse_subscriptions.owner is ?", "str")->run($user); + $valid = [ + 'title' => "str", + 'folder' => "int", + 'order_type' => "strict int", + 'pinned' => "strict bool", + ]; + list($setClause, $setTypes, $setValues) = $this->generateSet($data, $valid); + return (bool) $this->db->prepare("UPDATE arsse_subscriptions set $setClause where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id)->changes(); } public function feedUpdate(int $feedID, bool $throwError = false): bool { @@ -496,7 +538,7 @@ class Database { $feedID )->lastId(); foreach($article->getTag('category') as $c) { - $qInsertCategories->run($articleID, $c); + $qInsertCategory->run($articleID, $c); } $qInsertEdition->run($articleID); } @@ -516,7 +558,7 @@ class Database { ); $qDeleteCategories->run($articleID); foreach($article->getTag('category') as $c) { - $qInsertCategories->run($articleID, $c); + $qInsertCategory->run($articleID, $c); } $qInsertEdition->run($articleID); $qClearReadMarks->run($articleID); diff --git a/lib/Db/AbstractStatement.php b/lib/Db/AbstractStatement.php index 28d02ba..54ec1cf 100644 --- a/lib/Db/AbstractStatement.php +++ b/lib/Db/AbstractStatement.php @@ -3,6 +3,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Db; abstract class AbstractStatement implements Statement { + protected $types = []; + protected $isNullable = []; abstract function runArray(array $values): Result; abstract static function dateFormat(int $part = self::TS_BOTH): string; @@ -23,6 +25,13 @@ abstract class AbstractStatement implements Statement { $this->rebindArray($binding, true); } else { $binding = trim(strtolower($binding)); + if(strpos($binding, "strict ")===0) { + // "strict" types' values may never be null; null values will later be cast to the type specified + $this->isNullable[] = false; + $binding = substr($binding, 7); + } else { + $this->isNullable[] = true; + } if(!array_key_exists($binding, self::TYPES)) throw new Exception("paramTypeInvalid", $binding); $this->types[] = self::TYPES[$binding]; } @@ -30,13 +39,16 @@ abstract class AbstractStatement implements Statement { return true; } - protected function cast($v, string $t) { + protected function cast($v, string $t, bool $nullable) { switch($t) { case "date": + if(is_null($v) && !$nullable) $v = 0; return $this->formatDate($v, self::TS_DATE); case "time": + if(is_null($v) && !$nullable) $v = 0; return $this->formatDate($v, self::TS_TIME); case "datetime": + if(is_null($v) && !$nullable) $v = 0; return $this->formatDate($v, self::TS_BOTH); case "null": case "integer": diff --git a/lib/Db/SQLite3/Statement.php b/lib/Db/SQLite3/Statement.php index 023d55e..ba868b4 100644 --- a/lib/Db/SQLite3/Statement.php +++ b/lib/Db/SQLite3/Statement.php @@ -25,7 +25,6 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { protected $db; protected $st; - protected $types; public function __construct(\SQLite3 $db, \SQLite3Stmt $st, array $bindings = []) { $this->db = $db; @@ -66,23 +65,24 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { if(is_array($value)) { // recursively flatten any arrays, which may be provided for SET or IN() clauses $a += $this->bindValues($value, $a); - } else { - // find the right SQLite binding type for the value/specified type - if($value===null) { - $type = \SQLITE3_NULL; - } else if(array_key_exists($a,$this->types)) { - if(!array_key_exists($this->types[$a], self::BINDINGS)) throw new Exception("paramTypeUnknown", $this->types[$a]); + } else if(array_key_exists($a,$this->types)) { + // if the parameter type is something other than the known values, this is an error + if(!array_key_exists($this->types[$a], self::BINDINGS)) throw new Exception("paramTypeUnknown", $this->types[$a]); + // if the parameter type is null or the value is null (and the type is nullable), just bind null + if($this->types[$a]=="null" || ($this->isNullable[$a] && is_null($value))) { + $this->st->bindValue($a+1, null, \SQLITE3_NULL); + } else { + // otherwise cast the value to the right type and bind the result $type = self::BINDINGS[$this->types[$a]]; - } else { - throw new Exception("paramTypeMissing", $a+1); + $value = $this->cast($value, $this->types[$a], $this->isNullable[$a]); + // re-adjust for null casts + if($value===null) $type = \SQLITE3_NULL; + // perform binding + $this->st->bindValue($a+1, $value, $type); } - // cast value if necessary - $value = $this->cast($value, $this->types[$a]); - // re-adjust for null casts - if($value===null) $type = \SQLITE3_NULL; - // perform binding - $this->st->bindValue($a+1, $value, $type); $a++; + } else { + throw new Exception("paramTypeMissing", $a+1); } } return $a - $offset; diff --git a/tests/test.php b/tests/test.php index cf7507c..bf19bd1 100644 --- a/tests/test.php +++ b/tests/test.php @@ -1,7 +1,7 @@ rightsSet($user, User\Driver::RIGHTS_GLOBAL_ADMIN); Data::$user->authorizationEnabled(true); Data::$db->folderAdd($user, ['name' => 'ook']); Data::$db->subscriptionAdd($user, "https://jkingweb.ca/test.atom"); +Data::$db->subscriptionPropertiesSet($user, 1, [ + 'title' => "OOOOOOOOK!", + 'folder' => null, + 'order_type' => null, + 'pinned' => null, +]); var_export(Data::$db->subscriptionList($user)->getAll()); \ No newline at end of file