diff --git a/lib/Database.php b/lib/Database.php index ec1345ad..dd94cf36 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -1054,9 +1054,9 @@ class Database { if ($data['read'] || $data['starred'] || strlen($data['note'] ?? "")) { // first prepare a query to insert any missing marks rows for the articles we want to mark // but only insert new mark records if we're setting at least one "positive" mark - $q = $this->articleQuery($user, $context, ["id", "subscription"]); + $q = $this->articleQuery($user, $context, ["id", "subscription", "note"]); $q->setWhere("arsse_marks.starred is null"); // null means there is no marks row for the article - $this->db->prepare("INSERT INTO arsse_marks(article,subscription) ".$q->getQuery(), $q->getTypes())->run($q->getValues()); + $this->db->prepare("INSERT INTO arsse_marks(article,subscription,note) ".$q->getQuery(), $q->getTypes())->run($q->getValues()); } if (isset($data['read']) && (isset($data['starred']) || isset($data['note'])) && ($context->edition() || $context->editions())) { // if marking by edition both read and something else, do separate marks for starred and note than for read diff --git a/lib/Db/AbstractStatement.php b/lib/Db/AbstractStatement.php index acc9650f..55c6cf63 100644 --- a/lib/Db/AbstractStatement.php +++ b/lib/Db/AbstractStatement.php @@ -15,6 +15,7 @@ abstract class AbstractStatement implements Statement { abstract public function runArray(array $values = []): Result; abstract protected function bindValue($value, string $type, int $position): bool; + abstract protected function prepare(string $query): bool; public function run(...$values): Result { return $this->runArray($values); @@ -24,6 +25,10 @@ abstract class AbstractStatement implements Statement { return $this->retypeArray($bindings); } + public static function mungeQuery(string $query, array $types, ...$extraData): string { + return $query; + } + public function retypeArray(array $bindings, bool $append = false): bool { if (!$append) { $this->types = []; @@ -47,6 +52,9 @@ abstract class AbstractStatement implements Statement { $this->types[] = self::TYPES[$binding]; } } + if (!$append) { + $this->prepare(static::mungeQuery($this->query, $this->types)); + } return true; } diff --git a/lib/Db/MySQL/PDOStatement.php b/lib/Db/MySQL/PDOStatement.php new file mode 100644 index 00000000..f388de4f --- /dev/null +++ b/lib/Db/MySQL/PDOStatement.php @@ -0,0 +1,21 @@ +db->prepare($query); - } catch (\PDOException $e) { - list($excClass, $excMsg, $excData) = $this->exceptionBuild(); - throw new $excClass($excMsg, $excData); - } - return new PDOStatement($this->db, $s, $paramTypes); + return new PDOStatement($this->db, $query, $paramTypes); } } diff --git a/lib/Db/PDOError.php b/lib/Db/PDOError.php index dc3baab4..25b2a08f 100644 --- a/lib/Db/PDOError.php +++ b/lib/Db/PDOError.php @@ -9,8 +9,8 @@ namespace JKingWeb\Arsse\Db; use JKingWeb\Arsse\Db\SQLite3\Driver as SQLite3; trait PDOError { - public function exceptionBuild(bool $statementError = null): array { - if ($statementError ?? ($this instanceof Statement)) { + public function exceptionBuild(bool $statementError = false): array { + if ($statementError) { $err = $this->st->errorInfo(); } else { $err = $this->db->errorInfo(); diff --git a/lib/Db/PDOStatement.php b/lib/Db/PDOStatement.php index 3a090150..2115ce5c 100644 --- a/lib/Db/PDOStatement.php +++ b/lib/Db/PDOStatement.php @@ -20,13 +20,24 @@ class PDOStatement extends AbstractStatement { protected $st; protected $db; + protected $query; - public function __construct(\PDO $db, \PDOStatement $st, array $bindings = []) { + public function __construct(\PDO $db, string $query, array $bindings = []) { $this->db = $db; - $this->st = $st; + $this->query = $query; $this->retypeArray($bindings); } + protected function prepare(string $query): bool { + try { + $this->st = $this->db->prepare($query); + return true; + } catch (\PDOException $e) { + list($excClass, $excMsg, $excData) = $this->exceptionBuild(); + throw new $excClass($excMsg, $excData); + } + } + public function __destruct() { unset($this->st, $this->db); } @@ -37,7 +48,7 @@ class PDOStatement extends AbstractStatement { try { $this->st->execute(); } catch (\PDOException $e) { - list($excClass, $excMsg, $excData) = $this->exceptionBuild(); + list($excClass, $excMsg, $excData) = $this->exceptionBuild(true); throw new $excClass($excMsg, $excData); } return new PDOResult($this->db, $this->st); diff --git a/lib/Db/PostgreSQL/PDOStatement.php b/lib/Db/PostgreSQL/PDOStatement.php index 534efbcb..c255eaef 100644 --- a/lib/Db/PostgreSQL/PDOStatement.php +++ b/lib/Db/PostgreSQL/PDOStatement.php @@ -6,51 +6,8 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Db\PostgreSQL; -class PDOStatement extends Statement { - use \JKingWeb\Arsse\Db\PDOError; - - protected $db; - protected $st; - protected $qOriginal; - protected $qMunged; - protected $bindings; - - public function __construct(\PDO $db, string $query, array $bindings = []) { - $this->db = $db; - $this->qOriginal = $query; - $this->retypeArray($bindings); - } - - public function __destruct() { - unset($this->db, $this->st); - } - - public function retypeArray(array $bindings, bool $append = false): bool { - if ($append) { - return parent::retypeArray($bindings, $append); - } else { - $this->bindings = $bindings; - parent::retypeArray($bindings, $append); - $this->qMunged = self::mungeQuery($this->qOriginal, $this->types, false); - try { - // statement creation with PostgreSQL should never fail (it is not evaluated at creation time) - $s = $this->db->prepare($this->qMunged); - } catch (\PDOException $e) { // @codeCoverageIgnore - list($excClass, $excMsg, $excData) = $this->exceptionBuild(true); // @codeCoverageIgnore - throw new $excClass($excMsg, $excData); // @codeCoverageIgnore - } - $this->st = new \JKingWeb\Arsse\Db\PDOStatement($this->db, $s, $this->bindings); - } - return true; - } - - public function runArray(array $values = []): \JKingWeb\Arsse\Db\Result { - return $this->st->runArray($values); - } - - /** @codeCoverageIgnore */ - protected function bindValue($value, string $type, int $position): bool { - // stub required by abstract parent, but never used - return true; +class PDOStatement extends \JKingWeb\Arsse\Db\PDOStatement { + public static function mungeQuery(string $query, array $types, ...$extraData): string { + return Statement::mungeQuery($query, $types, false); } } diff --git a/lib/Db/PostgreSQL/Statement.php b/lib/Db/PostgreSQL/Statement.php index bccd0fb0..64d4ddc8 100644 --- a/lib/Db/PostgreSQL/Statement.php +++ b/lib/Db/PostgreSQL/Statement.php @@ -24,27 +24,16 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { protected $db; protected $in = []; - protected $qOriginal; + protected $query; protected $qMunged; protected $bindings; public function __construct($db, string $query, array $bindings = []) { $this->db = $db; - $this->qOriginal = $query; + $this->query = $query; $this->retypeArray($bindings); } - public function retypeArray(array $bindings, bool $append = false): bool { - if ($append) { - return parent::retypeArray($bindings, $append); - } else { - $this->bindings = $bindings; - parent::retypeArray($bindings, $append); - $this->qMunged = self::mungeQuery($this->qOriginal, $this->types, true); - } - return true; - } - public function runArray(array $values = []): \JKingWeb\Arsse\Db\Result { $this->in = []; $this->bindValues($values); @@ -62,7 +51,8 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { return true; } - protected static function mungeQuery(string $q, array $types, bool $mungeParamMarkers = true): string { + public static function mungeQuery(string $q, array $types, ...$extraData): string { + $mungeParamMarkers = (bool) ($extraData[0] ?? true); $q = explode("?", $q); $out = ""; for ($b = 1; $b < sizeof($q); $b++) { @@ -74,4 +64,9 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { $out .= array_pop($q); return $out; } + + protected function prepare(string $query): bool { + $this->qMunged = $query; + return true; + } } diff --git a/lib/Db/SQLite3/Driver.php b/lib/Db/SQLite3/Driver.php index 95fd7d9d..62e6efb9 100644 --- a/lib/Db/SQLite3/Driver.php +++ b/lib/Db/SQLite3/Driver.php @@ -166,13 +166,7 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { } public function prepareArray(string $query, array $paramTypes): \JKingWeb\Arsse\Db\Statement { - try { - $s = $this->db->prepare($query); - } catch (\Exception $e) { - list($excClass, $excMsg, $excData) = $this->exceptionBuild(); - throw new $excClass($excMsg, $excData); - } - return new Statement($this->db, $s, $paramTypes); + return new Statement($this->db, $query, $paramTypes); } protected function lock(): bool { diff --git a/lib/Db/SQLite3/Statement.php b/lib/Db/SQLite3/Statement.php index ab07b47b..2de3e3bd 100644 --- a/lib/Db/SQLite3/Statement.php +++ b/lib/Db/SQLite3/Statement.php @@ -27,13 +27,24 @@ class Statement extends \JKingWeb\Arsse\Db\AbstractStatement { protected $db; protected $st; + protected $query; - public function __construct(\SQLite3 $db, \SQLite3Stmt $st, array $bindings = []) { + public function __construct(\SQLite3 $db, string $query, array $bindings = []) { $this->db = $db; - $this->st = $st; + $this->query = $query; $this->retypeArray($bindings); } + protected function prepare(string $query): bool { + try { + $this->st = $this->db->prepare($query); + return true; + } catch (\Exception $e) { + list($excClass, $excMsg, $excData) = $this->exceptionBuild(); + throw new $excClass($excMsg, $excData); + } + } + public function __destruct() { try { $this->st->close(); diff --git a/tests/cases/Db/MySQLPDO/TestStatement.php b/tests/cases/Db/MySQLPDO/TestStatement.php index 6be598a5..a0727ea8 100644 --- a/tests/cases/Db/MySQLPDO/TestStatement.php +++ b/tests/cases/Db/MySQLPDO/TestStatement.php @@ -14,7 +14,7 @@ class TestStatement extends \JKingWeb\Arsse\TestCase\Db\BaseStatement { protected static $implementation = "PDO MySQL"; protected function makeStatement(string $q, array $types = []): array { - return [static::$interface, static::$interface->prepare($q), $types]; + return [static::$interface, $q, $types]; } protected function decorateTypeSyntax(string $value, string $type): string { diff --git a/tests/cases/Db/SQLite3/TestStatement.php b/tests/cases/Db/SQLite3/TestStatement.php index 7471b411..ede2225a 100644 --- a/tests/cases/Db/SQLite3/TestStatement.php +++ b/tests/cases/Db/SQLite3/TestStatement.php @@ -19,7 +19,7 @@ class TestStatement extends \JKingWeb\Arsse\TestCase\Db\BaseStatement { } protected function makeStatement(string $q, array $types = []): array { - return [static::$interface, static::$interface->prepare($q), $types]; + return [static::$interface, $q, $types]; } protected function decorateTypeSyntax(string $value, string $type): string { diff --git a/tests/cases/Db/SQLite3PDO/TestStatement.php b/tests/cases/Db/SQLite3PDO/TestStatement.php index 9e2e06c6..36f2eb9b 100644 --- a/tests/cases/Db/SQLite3PDO/TestStatement.php +++ b/tests/cases/Db/SQLite3PDO/TestStatement.php @@ -13,7 +13,7 @@ class TestStatement extends \JKingWeb\Arsse\TestCase\Db\BaseStatement { protected static $implementation = "PDO SQLite 3"; protected function makeStatement(string $q, array $types = []): array { - return [static::$interface, static::$interface->prepare($q), $types]; + return [static::$interface, $q, $types]; } protected function decorateTypeSyntax(string $value, string $type): string { diff --git a/tests/lib/DatabaseInformation.php b/tests/lib/DatabaseInformation.php index 28a342fd..9f5788c2 100644 --- a/tests/lib/DatabaseInformation.php +++ b/tests/lib/DatabaseInformation.php @@ -290,7 +290,7 @@ class DatabaseInformation { 'PDO MySQL' => [ 'pdo' => true, 'backend' => "MySQL", - 'statementClass' => \JKingWeb\Arsse\Db\PDOStatement::class, + 'statementClass' => \JKingWeb\Arsse\Db\MySQL\PDOStatement::class, 'resultClass' => \JKingWeb\Arsse\Db\PDOResult::class, 'driverClass' => \JKingWeb\Arsse\Db\MySQL\PDODriver::class, 'stringOutput' => true,