diff --git a/lib/Database.php b/lib/Database.php index 1af1d6df..1b2ddf2b 100644 --- a/lib/Database.php +++ b/lib/Database.php @@ -458,7 +458,7 @@ class Database { 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(); + $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("idMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $id]); @@ -470,118 +470,112 @@ class Database { '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(); + $out = (bool) $this->db->prepare("UPDATE arsse_subscriptions set $setClause where owner is ? and id is ?", $setTypes, "str", "int")->run($setValues, $user, $id)->changes(); + $tr->commit(); + return $out; } public function feedUpdate(int $feedID, bool $throwError = false): bool { - $this->db->begin(); + $tr = $this->db->begin(); + // check to make sure the feed exists + $f = $this->db->prepare('SELECT url, username, password, DATEFORMAT("http", modified) AS lastmodified, etag, err_count FROM arsse_feeds where id is ?', "int")->run($feedID)->getRow(); + if(!$f) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID]); + // the Feed object throws an exception when there are problems, but that isn't ideal + // 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 { - // check to make sure the feed exists - $f = $this->db->prepare('SELECT url, username, password, DATEFORMAT("http", modified) AS lastmodified, etag, err_count FROM arsse_feeds where id is ?', "int")->run($feedID)->getRow(); - if(!$f) throw new Db\ExceptionInput("idMissing", ["action" => __FUNCTION__, "field" => "feed", 'id' => $feedID]); - // the Feed object throws an exception when there are problems, but that isn't ideal - // 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)$f['lastmodified'], $f['etag'], $f['username'], $f['password']); - 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); - $this->db->commit(); - return false; - } - } catch (Feed\Exception $e) { - // update the database with the resultant error and the next fetch time, incrementing the error count - $this->db->prepare( - 'UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ?, err_count = err_count + 1, err_msg = ? WHERE id is ?', - 'datetime', 'str', 'int' - )->run(Feed::nextFetchOnError($f['err_count']), $e->getMessage(),$feedID); - $this->db->commit(); - if($throwError) throw $e; + $feed = new Feed($feedID, $f['url'], (string)$f['lastmodified'], $f['etag'], $f['username'], $f['password']); + 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); + $tr->commit(); return false; - } catch(\Throwable $e) { - $this->db->rollback(); - throw $e; } - //prepare the necessary statements to perform the update - if(sizeof($feed->newItems) || sizeof($feed->changedItems)) { - $qInsertCategory = $this->db->prepare('INSERT INTO arsse_categories(article,name) values(?,?)', 'int', 'str'); - $qInsertEdition = $this->db->prepare('INSERT INTO arsse_editions(article) values(?)', 'int'); - } - if(sizeof($feed->newItems)) { - $qInsertArticle = $this->db->prepare( - 'INSERT INTO arsse_articles(url,title,author,published,edited,guid,content,url_title_hash,url_content_hash,title_content_hash,feed) values(?,?,?,?,?,?,?,?,?,?,?)', - 'str', 'str', 'str', 'datetime', 'datetime', 'str', 'str', 'str', 'str', 'str', 'int' - ); - } - if(sizeof($feed->changedItems)) { - $qDeleteCategories = $this->db->prepare('DELETE FROM arsse_categories WHERE article is ?', 'int'); - $qClearReadMarks = $this->db->prepare('UPDATE arsse_subscription_articles SET read = 0, modified = CURRENT_TIMESTAMP WHERE article is ?', 'int'); - $qUpdateArticle = $this->db->prepare( - 'UPDATE arsse_articles SET url = ?, title = ?, author = ?, published = ?, edited = ?, modified = CURRENT_TIMESTAMP, guid = ?, content = ?, url_title_hash = ?, url_content_hash = ?, title_content_hash = ? WHERE id is ?', - 'str', 'str', 'str', 'datetime', 'datetime', 'str', 'str', 'str', 'str', 'str', 'int' - ); - } - // actually perform updates - foreach($feed->newItems as $article) { - $articleID = $qInsertArticle->run( - $article->url, - $article->title, - $article->author, - $article->publishedDate, - $article->updatedDate, - $article->id, - $article->content, - $article->urlTitleHash, - $article->urlContentHash, - $article->titleContentHash, - $feedID - )->lastId(); - foreach($article->getTag('category') as $c) { - $qInsertCategory->run($articleID, $c); - } - $qInsertEdition->run($articleID); - } - foreach($feed->changedItems as $articleID => $article) { - $qUpdateArticle->run( - $article->url, - $article->title, - $article->author, - $article->publishedDate, - $article->updatedDate, - $article->id, - $article->content, - $article->urlTitleHash, - $article->urlContentHash, - $article->titleContentHash, - $articleID - ); - $qDeleteCategories->run($articleID); - foreach($article->getTag('category') as $c) { - $qInsertCategory->run($articleID, $c); - } - $qInsertEdition->run($articleID); - $qClearReadMarks->run($articleID); - } - // lastly update the feed database itself with updated information. + } catch (Feed\Exception $e) { + // update the database with the resultant error and the next fetch time, incrementing the error count $this->db->prepare( - 'UPDATE arsse_feeds SET url = ?, title = ?, favicon = ?, source = ?, updated = CURRENT_TIMESTAMP, modified = ?, etag = ?, err_count = 0, err_msg = "", next_fetch = ? WHERE id is ?', - 'str', 'str', 'str', 'str', 'datetime', 'str', 'datetime', 'int' - )->run( - $feed->data->feedUrl, - $feed->data->title, - $feed->favicon, - $feed->data->siteUrl, - $feed->lastModified, - $feed->resource->getEtag(), - $feed->nextFetch, - $feedID - ); - } catch(\Throwable $e) { - $this->db->rollback(); - throw $e; + 'UPDATE arsse_feeds SET updated = CURRENT_TIMESTAMP, next_fetch = ?, err_count = err_count + 1, err_msg = ? WHERE id is ?', + 'datetime', 'str', 'int' + )->run(Feed::nextFetchOnError($f['err_count']), $e->getMessage(),$feedID); + $tr->commit(); + if($throwError) throw $e; + return false; } - $this->db->commit(); + //prepare the necessary statements to perform the update + if(sizeof($feed->newItems) || sizeof($feed->changedItems)) { + $qInsertCategory = $this->db->prepare('INSERT INTO arsse_categories(article,name) values(?,?)', 'int', 'str'); + $qInsertEdition = $this->db->prepare('INSERT INTO arsse_editions(article) values(?)', 'int'); + } + if(sizeof($feed->newItems)) { + $qInsertArticle = $this->db->prepare( + 'INSERT INTO arsse_articles(url,title,author,published,edited,guid,content,url_title_hash,url_content_hash,title_content_hash,feed) values(?,?,?,?,?,?,?,?,?,?,?)', + 'str', 'str', 'str', 'datetime', 'datetime', 'str', 'str', 'str', 'str', 'str', 'int' + ); + } + if(sizeof($feed->changedItems)) { + $qDeleteCategories = $this->db->prepare('DELETE FROM arsse_categories WHERE article is ?', 'int'); + $qClearReadMarks = $this->db->prepare('UPDATE arsse_subscription_articles SET read = 0, modified = CURRENT_TIMESTAMP WHERE article is ?', 'int'); + $qUpdateArticle = $this->db->prepare( + 'UPDATE arsse_articles SET url = ?, title = ?, author = ?, published = ?, edited = ?, modified = CURRENT_TIMESTAMP, guid = ?, content = ?, url_title_hash = ?, url_content_hash = ?, title_content_hash = ? WHERE id is ?', + 'str', 'str', 'str', 'datetime', 'datetime', 'str', 'str', 'str', 'str', 'str', 'int' + ); + } + // actually perform updates + foreach($feed->newItems as $article) { + $articleID = $qInsertArticle->run( + $article->url, + $article->title, + $article->author, + $article->publishedDate, + $article->updatedDate, + $article->id, + $article->content, + $article->urlTitleHash, + $article->urlContentHash, + $article->titleContentHash, + $feedID + )->lastId(); + foreach($article->getTag('category') as $c) { + $qInsertCategory->run($articleID, $c); + } + $qInsertEdition->run($articleID); + } + foreach($feed->changedItems as $articleID => $article) { + $qUpdateArticle->run( + $article->url, + $article->title, + $article->author, + $article->publishedDate, + $article->updatedDate, + $article->id, + $article->content, + $article->urlTitleHash, + $article->urlContentHash, + $article->titleContentHash, + $articleID + ); + $qDeleteCategories->run($articleID); + foreach($article->getTag('category') as $c) { + $qInsertCategory->run($articleID, $c); + } + $qInsertEdition->run($articleID); + $qClearReadMarks->run($articleID); + } + // lastly update the feed database itself with updated information. + $this->db->prepare( + 'UPDATE arsse_feeds SET url = ?, title = ?, favicon = ?, source = ?, updated = CURRENT_TIMESTAMP, modified = ?, etag = ?, err_count = 0, err_msg = "", next_fetch = ? WHERE id is ?', + 'str', 'str', 'str', 'str', 'datetime', 'str', 'datetime', 'int' + )->run( + $feed->data->feedUrl, + $feed->data->title, + $feed->favicon, + $feed->data->siteUrl, + $feed->lastModified, + $feed->resource->getEtag(), + $feed->nextFetch, + $feedID + ); + $tr->commit(); return true; } diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index 9a9c71eb..a462f759 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -14,12 +14,16 @@ abstract class AbstractDriver implements Driver { } } - public function begin(): bool { + public function begin(): Transaction { + return new Transaction($this); + } + + public function savepointCreate(): bool { $this->exec("SAVEPOINT arsse_".(++$this->transDepth)); return true; } - public function commit(bool $all = false): bool { + public function savepointRelease(bool $all = false): bool { if($this->transDepth==0) return false; if(!$all) { $this->exec("RELEASE SAVEPOINT arsse_".($this->transDepth--)); @@ -30,7 +34,7 @@ abstract class AbstractDriver implements Driver { return true; } - public function rollback(bool $all = false): bool { + public function savepointUndo(bool $all = false): bool { if($this->transDepth==0) return false; if(!$all) { $this->exec("ROLLBACK TRANSACTION TO SAVEPOINT arsse_".($this->transDepth)); diff --git a/lib/Db/Driver.php b/lib/Db/Driver.php index 930f02ce..cb80aaff 100644 --- a/lib/Db/Driver.php +++ b/lib/Db/Driver.php @@ -8,12 +8,14 @@ interface Driver { static function driverName(): string; // returns the version of the scheme of the opened database; if uninitialized should return 0 function schemaVersion(): int; - // begin a real or synthetic transactions, with real or synthetic nesting - function begin(): bool; - // commit either the latest or all pending nested transactions; use of this method should assume a partial commit is a no-op - function commit(bool $all = false): bool; - // rollback either the latest or all pending nested transactions; use of this method should assume a partial rollback will not work - function rollback(bool $all = false): bool; + // return a Transaction object + function begin(): Transaction; + // manually begin a real or synthetic transactions, with real or synthetic nesting + function savepointCreate(): bool; + // manually commit either the latest or all pending nested transactions + function savepointRelease(bool $all = false): bool; + // manually rollback either the latest or all pending nested transactions + function savepointUndo(bool $all = false): bool; // attempt to advise other processes that they should not attempt to access the database; used during live upgrades function lock(): bool; function unlock(): bool; diff --git a/lib/Db/SQLite3/Driver.php b/lib/Db/SQLite3/Driver.php index 37c4c57c..fe5aa967 100644 --- a/lib/Db/SQLite3/Driver.php +++ b/lib/Db/SQLite3/Driver.php @@ -69,9 +69,9 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { $sep = \DIRECTORY_SEPARATOR; $path = Data::$conf->dbSchemaBase.$sep."SQLite3".$sep; $this->lock(); - $this->begin(); + $this->savepointCreate(); for($a = $ver; $a < $to; $a++) { - $this->begin(); + $this->savepointCreate(); try { $file = $path.$a.".sql"; if(!file_exists($file)) throw new Exception("updateFileMissing", ['file' => $file, 'driver_name' => $this->driverName(), 'current' => $a]); @@ -86,17 +86,17 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { if($this->schemaVersion() != $a+1) throw new Exception("updateFileIncomplete", ['file' => $file, 'driver_name' => $this->driverName(), 'current' => $a]); } catch(\Throwable $e) { // undo any partial changes from the failed update - $this->rollback(); + $this->savepointUndo(); $this->unlock(); // commit any successful updates if updating by more than one version - $this->commit(true); + $this->savepointRelease(true); // throw the error received throw $e; } - $this->commit(); + $this->savepointRelease(); } $this->unlock(); - $this->commit(); + $this->savepointRelease(); return true; } diff --git a/lib/Db/Transaction.php b/lib/Db/Transaction.php new file mode 100644 index 00000000..40666881 --- /dev/null +++ b/lib/Db/Transaction.php @@ -0,0 +1,44 @@ +savepointCreate(); + $this->drv = $drv; + $this->pending = true; + } + + function __destruct() { + if($this->pending) { + try { + $this->drv->savepointUndo(); + } catch(\Throwable $e) { + // do nothing + } + } + } + + function commit(): bool { + if($this->pending) { + $this->drv->savepointRelease(); + $this->pending = false; + return true; + } else { + return false; + } + } + + function rollback(): bool { + if($this->pending) { + $this->drv->savepointUndo(); + $this->pending = false; + return true; + } else { + return false; + } + } +} \ No newline at end of file diff --git a/tests/Db/SQLite3/TestDbDriverSQLite3.php b/tests/Db/SQLite3/TestDbDriverSQLite3.php index 54047b9a..4d584fcb 100644 --- a/tests/Db/SQLite3/TestDbDriverSQLite3.php +++ b/tests/Db/SQLite3/TestDbDriverSQLite3.php @@ -106,7 +106,7 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); @@ -120,11 +120,11 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->commit(); + $tr->commit(); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(1, $ch->querySingle($select)); } @@ -134,11 +134,11 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->rollback(); + $tr->rollback(); $this->assertEquals(0, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); } @@ -148,11 +148,11 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->begin(); + $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); @@ -163,17 +163,17 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->begin(); + $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->commit(); + $tr2->commit(); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->commit(); + $tr1->commit(); $this->assertEquals(2, $ch->querySingle($select)); } @@ -182,18 +182,18 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->begin(); + $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->rollback(); + $tr2->rollback(); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->rollback(); + $tr1->rollback(); $this->assertEquals(0, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); } @@ -203,58 +203,22 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $insert = "INSERT INTO test(id) values(null)"; $ch = new \SQLite3(Data::$conf->dbSQLite3File); $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); + $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->begin(); + $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->rollback(); + $tr2->rollback(); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->commit(); + $tr1->commit(); $this->assertEquals(1, $this->drv->query($select)->getValue()); $this->assertEquals(1, $ch->querySingle($select)); } - function testFullyRollbackChainedTransactions() { - $select = "SELECT count(*) FROM test"; - $insert = "INSERT INTO test(id) values(null)"; - $ch = new \SQLite3(Data::$conf->dbSQLite3File); - $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); - $this->drv->query($insert); - $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->begin(); - $this->drv->query($insert); - $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->rollback(true); - $this->assertEquals(0, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); - } - - function testFullyCommitChainedTransactions() { - $select = "SELECT count(*) FROM test"; - $insert = "INSERT INTO test(id) values(null)"; - $ch = new \SQLite3(Data::$conf->dbSQLite3File); - $this->drv->exec("CREATE TABLE test(id integer primary key)"); - $this->drv->begin(); - $this->drv->query($insert); - $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->begin(); - $this->drv->query($insert); - $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); - $this->drv->commit(true); - $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(2, $ch->querySingle($select)); - } - function testFetchSchemaVersion() { $this->assertSame(0, $this->drv->schemaVersion()); $this->drv->exec("PRAGMA user_version=1"); diff --git a/tests/lib/Database/Setup.php b/tests/lib/Database/Setup.php index 2d74860b..4e628d76 100644 --- a/tests/lib/Database/Setup.php +++ b/tests/lib/Database/Setup.php @@ -52,7 +52,7 @@ trait Setup { } function primeDatabase(array $data): bool { - $this->drv->begin(); + $tr = $this->drv->begin(); foreach($data as $table => $info) { $cols = implode(",", array_keys($info['columns'])); $bindings = array_values($info['columns']); @@ -62,7 +62,7 @@ trait Setup { $this->assertEquals(1, $s->runArray($row)->changes()); } } - $this->drv->commit(); + $tr->commit(); return true; }