From 929d763981ebe29fbba66b9866fc90c51d0bc8d3 Mon Sep 17 00:00:00 2001 From: "J. King" Date: Thu, 6 Apr 2017 11:02:47 -0400 Subject: [PATCH] Simplify the creation of arbitrary UPDATEs The type parameters of Db\Driver::prepare() and the parameters of Db\Statement::run() can now be arrays, which will be iterated over recursively to bind scalar values to the SQL statement. This simplifies the construction of arbitrary UPDATE statements (the WHERE clause no longer needs to be taken into account) and should make it clearer what is happening in these cases. It should also simplify the creation of IN() clauses down the road if they become necessary. --- lib/Database.php | 27 +++++++------------ lib/Db/AbstractDriver.php | 2 +- lib/Db/AbstractStatement.php | 15 +++++++---- lib/Db/Driver.php | 2 +- lib/Db/SQLite3/Statement.php | 47 ++++++++++++++++++++------------- lib/REST/NextCloudNews/V1_2.php | 4 +++ 6 files changed, 55 insertions(+), 42 deletions(-) diff --git a/lib/Database.php b/lib/Database.php index e2ada448..075b1a11 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -14,26 +14,19 @@ class Database { public $db; private $driver; - protected function processUpdate(array $props, array $valid, array $where): array { + protected function generateSet(array $props, array $valid): array { $out = [ - 'values' => [], - 'types' => [], - 'set' => [], - 'where' => [], + 'setValues' => [], + 'setTypes' => [], + 'set' => [], ]; foreach($valid as $prop => $type) { if(!array_key_exists($prop, $props)) continue; - $out['values'][] = $props[$prop]; - $out['types'][] = $type; + $out['setValues'][] = $props[$prop]; + $out['setTypes'][] = $type; $out['set'][] = "$prop = ?"; } - foreach($where as $field => $value) { - $out['values'][] = $value[0]; - $out['types'][] = $value[1]; - $out['where'][] = "$field is ?"; - } $out['set'] = implode(", ", $out['set']); - $out['where'] = implode(" and ", $out['where']); return $out; } @@ -256,9 +249,9 @@ class Database { $valid = [ // FIXME: add future properties "name" => "str", ]; - $data = $this->processUpdate($properties, $valid, ['id' => [$user, "str"]]); + $data = $this->generateSet($properties, $valid); extract($data); - $this->db->prepareArray("UPDATE arsse_users set $set where $where", $types)->runArray($values); + $this->db->prepare("UPDATE arsse_users set $set where id is ?", $setTypes, "str")->run($setValues, $user); return $this->userPropertiesGet($user); } @@ -389,9 +382,9 @@ class Database { 'name' => "str", 'parent' => "int", ]; - $data = $this->processUpdate($data, $valid, ['owner' => [$user, "str"], 'id' => [$id, "int"]]); + $data = $this->generateSet($data, $valid); extract($data); - $this->db->prepareArray("UPDATE arsse_folders set $set where $where", $types)->runArray($values); + $this->db->prepare("UPDATE arsse_folders set $set where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id); return true; } diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index ee308a3e..9a9c71eb 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -67,7 +67,7 @@ abstract class AbstractDriver implements Driver { return ($this->query("SELECT count(*) from arsse_settings where key is 'lock'")->getValue() > 0); } - public function prepare(string $query, string ...$paramType): Statement { + public function prepare(string $query, ...$paramType): Statement { return $this->prepareArray($query, $paramType); } } \ No newline at end of file diff --git a/lib/Db/AbstractStatement.php b/lib/Db/AbstractStatement.php index b268eb21..93322b96 100644 --- a/lib/Db/AbstractStatement.php +++ b/lib/Db/AbstractStatement.php @@ -15,12 +15,17 @@ abstract class AbstractStatement implements Statement { return $this->rebindArray($bindings); } - public function rebindArray(array $bindings): bool { - $this->types = []; + public function rebindArray(array $bindings, bool $append = false): bool { + if(!$append) $this->types = []; foreach($bindings as $binding) { - $binding = trim(strtolower($binding)); - if(!array_key_exists($binding, self::TYPES)) throw new Exception("paramTypeInvalid", $binding); - $this->types[] = self::TYPES[$binding]; + if(is_array($binding)) { + // recursively flatten any arrays, which may be provided for SET or IN() clauses + $this->rebindArray($binding, true); + } else { + $binding = trim(strtolower($binding)); + if(!array_key_exists($binding, self::TYPES)) throw new Exception("paramTypeInvalid", $binding); + $this->types[] = self::TYPES[$binding]; + } } return true; } diff --git a/lib/Db/Driver.php b/lib/Db/Driver.php index 0f0f02b1..930f02ce 100644 --- a/lib/Db/Driver.php +++ b/lib/Db/Driver.php @@ -25,5 +25,5 @@ interface Driver { // perform a single unsanitized query and return a result set function query(string $query): Result; // ready a prepared statement for later execution - function prepare(string $query, string ...$paramType): Statement; + function prepare(string $query, ...$paramType): Statement; } \ No newline at end of file diff --git a/lib/Db/SQLite3/Statement.php b/lib/Db/SQLite3/Statement.php index 22d3c57f..e0e5873a 100644 --- a/lib/Db/SQLite3/Statement.php +++ b/lib/Db/SQLite3/Statement.php @@ -48,24 +48,7 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { public function runArray(array $values = null): \JKingWeb\Arsse\Db\Result { $this->st->clear(); - $l = sizeof($values); - for($a = 0; $a < $l; $a++) { - // find the right SQLite binding type for the value/specified type - if($values[$a]===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]); - $type = self::BINDINGS[$this->types[$a]]; - } else { - throw new Exception("paramTypeMissing", $a+1); - } - // cast value if necessary - $values[$a] = $this->cast($values[$a], $this->types[$a]); - // re-adjust for null casts - if($values[$a]===null) $type = \SQLITE3_NULL; - // perform binding - $this->st->bindValue($a+1, $values[$a], $type); - } + if(!is_null($values)) $this->bindValues($values); try { $r = $this->st->execute(); } catch(\Exception $e) { @@ -76,4 +59,32 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { $lastId = $this->db->lastInsertRowID(); return new Result($r, [$changes, $lastId], $this); } + + protected function bindValues(array $values, int $offset = 0): int { + $a = $offset; + foreach($values as $value) { + 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]); + $type = self::BINDINGS[$this->types[$a]]; + } else { + throw new Exception("paramTypeMissing", $a+1); + } + // 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++; + } + } + return $a; + } } \ No newline at end of file diff --git a/lib/REST/NextCloudNews/V1_2.php b/lib/REST/NextCloudNews/V1_2.php index bc5feec1..fd5dc7c2 100644 --- a/lib/REST/NextCloudNews/V1_2.php +++ b/lib/REST/NextCloudNews/V1_2.php @@ -132,24 +132,28 @@ class V1_2 extends \JKingWeb\Arsse\REST\AbstractHandler { return new Response(204); } + // return the server version protected function versionGET(array $url, array $data): Response { // if URL is more than '/version' this is an error if(sizeof($url)) return new Response(404); return new Response(200, ['version' => \JKingWeb\Arsse\VERSION]); } + // invalid function protected function versionPOST(array $url, array $data): Response { // if URL is more than '/version' this is an error if(sizeof($url)) return new Response(404); return new Response(405, "", "", ['Allow: GET']); } + // invalid function protected function versionPUT(array $url, array $data): Response { // if URL is more than '/version' this is an error if(sizeof($url)) return new Response(404); return new Response(405, "", "", ['Allow: GET']); } + // invalid function protected function versionDELETE(array $url, array $data): Response { // if URL is more than '/version' this is an error if(sizeof($url)) return new Response(404);