From 8ebcb01cb55e3e8638d58992cf18561ad3ea438f Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 7 May 2017 18:27:16 -0400 Subject: [PATCH] Made out-of-order transaction handling more predictable --- lib/AbstractException.php | 3 + lib/Db/AbstractDriver.php | 93 ++++++++++++--- lib/Db/Driver.php | 12 +- lib/Db/ExceptionSavepoint.php | 6 + lib/Db/SQLite3/Driver.php | 6 +- lib/Db/Transaction.php | 33 +++--- locale/en.php | 3 + tests/Db/SQLite3/TestDbDriverSQLite3.php | 139 +++++++++++++++++------ 8 files changed, 219 insertions(+), 76 deletions(-) create mode 100644 lib/Db/ExceptionSavepoint.php diff --git a/lib/AbstractException.php b/lib/AbstractException.php index bd7855d..433d5f4 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -32,6 +32,9 @@ 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/ExceptionInput.missing" => 10231, "Db/ExceptionInput.whitespace" => 10232, "Db/ExceptionInput.tooLong" => 10233, diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index a462f75..770c38d 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -5,6 +5,7 @@ use JKingWeb\DrUUID\UUID as UUID; abstract class AbstractDriver implements Driver { protected $transDepth = 0; + protected $transStatus = []; public function schemaVersion(): int { try { @@ -18,33 +19,89 @@ abstract class AbstractDriver implements Driver { return new Transaction($this); } - public function savepointCreate(): bool { + public function savepointCreate(): int { $this->exec("SAVEPOINT arsse_".(++$this->transDepth)); - return true; + $this->transStatus[$this->transDepth] = self::TR_PEND; + return $this->transDepth; } - public function savepointRelease(bool $all = false): bool { - if($this->transDepth==0) return false; - if(!$all) { - $this->exec("RELEASE SAVEPOINT arsse_".($this->transDepth--)); + public function savepointRelease(int $index = null): bool { + if(is_null($index)) $index = $this->transDepth; + if(array_key_exists($index, $this->transStatus)) { + switch($this->transStatus[$index]) { + case self::TR_PEND: + $this->exec("RELEASE SAVEPOINT arsse_".$index); + $this->transStatus[$index] = self::TR_COMMIT; + $a = $index; + while(++$a && $a <= $this->transDepth) { + if($this->transStatus[$a] <= self::TR_PEND) $this->transStatus[$a] = self::TR_PEND_COMMIT; + } + $out = true; + break; + case self::TR_PEND_COMMIT: + $this->transStatus[$index] = self::TR_COMMIT; + $out = true; + break; + case self::TR_PEND_ROLLBACK: + $this->transStatus[$index] = self::TR_COMMIT; + $out = false; + break; + case self::TR_COMMIT: + case self::TR_ROLLBACK: + throw new ExceptionSavepoint("stale", ['action' => "commit", 'index' => $index]); + default: + throw new Exception("unknownSavepointStatus", $this->transStatus[$index]); + } + if($index==$this->transDepth) { + while($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { + array_pop($this->transStatus); + $this->transDepth--; + } + } + return $out; } else { - $this->exec("COMMIT TRANSACTION"); - $this->transDepth = 0; + throw new ExceptionSavepoint("invalid", ['action' => "commit", 'index' => $index]); } - return true; } - public function savepointUndo(bool $all = false): bool { - if($this->transDepth==0) return false; - if(!$all) { - $this->exec("ROLLBACK TRANSACTION TO SAVEPOINT arsse_".($this->transDepth)); - // rollback to savepoint does not collpase the savepoint - $this->exec("RELEASE SAVEPOINT arsse_".($this->transDepth--)); + public function savepointUndo(int $index = null): bool { + if(is_null($index)) $index = $this->transDepth; + if(array_key_exists($index, $this->transStatus)) { + switch($this->transStatus[$index]) { + case self::TR_PEND: + $this->exec("ROLLBACK TRANSACTION TO SAVEPOINT arsse_".$index); + $this->exec("RELEASE SAVEPOINT arsse_".$index); + $this->transStatus[$index] = self::TR_ROLLBACK; + $a = $index; + while(++$a && $a <= $this->transDepth) { + if($this->transStatus[$a] <= self::TR_PEND) $this->transStatus[$a] = self::TR_PEND_ROLLBACK; + } + $out = true; + break; + case self::TR_PEND_COMMIT: + $this->transStatus[$index] = self::TR_ROLLBACK; + $out = false; + break; + case self::TR_PEND_ROLLBACK: + $this->transStatus[$index] = self::TR_ROLLBACK; + $out = true; + break; + case self::TR_COMMIT: + case self::TR_ROLLBACK: + throw new ExceptionSavepoint("stale", ['action' => "rollback", 'index' => $index]); + default: + throw new Exception("unknownSavepointStatus", $this->transStatus[$index]); + } + if($index==$this->transDepth) { + while($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { + array_pop($this->transStatus); + $this->transDepth--; + } + } + return $out; } else { - $this->exec("ROLLBACK TRANSACTION"); - $this->transDepth = 0; + throw new ExceptionSavepoint("invalid", ['action' => "rollback", 'index' => $index]); } - return true; } public function lock(): bool { diff --git a/lib/Db/Driver.php b/lib/Db/Driver.php index cb80aaf..739aafe 100644 --- a/lib/Db/Driver.php +++ b/lib/Db/Driver.php @@ -3,6 +3,12 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Db; interface Driver { + const TR_PEND = 0; + const TR_COMMIT = 1; + const TR_ROLLBACK = 2; + const TR_PEND_COMMIT = -1; + const TR_PEND_ROLLBACK = -2; + function __construct(bool $install = false); // returns a human-friendly name for the driver (for display in installer, for example) static function driverName(): string; @@ -11,11 +17,11 @@ interface Driver { // return a Transaction object function begin(): Transaction; // manually begin a real or synthetic transactions, with real or synthetic nesting - function savepointCreate(): bool; + function savepointCreate(): int; // manually commit either the latest or all pending nested transactions - function savepointRelease(bool $all = false): bool; + function savepointRelease(int $index = null): bool; // manually rollback either the latest or all pending nested transactions - function savepointUndo(bool $all = false): bool; + function savepointUndo(int $index = null): 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/ExceptionSavepoint.php b/lib/Db/ExceptionSavepoint.php new file mode 100644 index 0000000..98a8271 --- /dev/null +++ b/lib/Db/ExceptionSavepoint.php @@ -0,0 +1,6 @@ +dbSchemaBase.$sep."SQLite3".$sep; $this->lock(); - $this->savepointCreate(); + $tr = $this->savepointCreate(); for($a = $ver; $a < $to; $a++) { $this->savepointCreate(); try { @@ -87,9 +87,9 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { } catch(\Throwable $e) { // undo any partial changes from the failed update $this->savepointUndo(); - $this->unlock(); // commit any successful updates if updating by more than one version - $this->savepointRelease(true); + $this->unlock(); + $this->savepointRelease(); // throw the error received throw $e; } diff --git a/lib/Db/Transaction.php b/lib/Db/Transaction.php index 4066688..81f6125 100644 --- a/lib/Db/Transaction.php +++ b/lib/Db/Transaction.php @@ -3,11 +3,12 @@ declare(strict_types=1); namespace JKingWeb\Arsse\Db; class Transaction { + protected $index; protected $pending = false; protected $drv; function __construct(Driver $drv) { - $drv->savepointCreate(); + $this->index = $drv->savepointCreate(); $this->drv = $drv; $this->pending = true; } @@ -15,7 +16,7 @@ class Transaction { function __destruct() { if($this->pending) { try { - $this->drv->savepointUndo(); + $this->drv->savepointUndo($this->index); } catch(\Throwable $e) { // do nothing } @@ -23,22 +24,22 @@ class Transaction { } function commit(): bool { - if($this->pending) { - $this->drv->savepointRelease(); - $this->pending = false; - return true; - } else { - return false; - } + $out = $this->drv->savepointRelease($this->index); + $this->pending = false; + return $out; } function rollback(): bool { - if($this->pending) { - $this->drv->savepointUndo(); - $this->pending = false; - return true; - } else { - return false; - } + $out = $this->drv->savepointUndo($this->index); + $this->pending = false; + return $out; + } + + function getIndex(): int { + return $this->index; + } + + function isPending(): bool { + return $this->pending; } } \ No newline at end of file diff --git a/locale/en.php b/locale/en.php index bfa3461..68aa72f 100644 --- a/locale/en.php +++ b/locale/en.php @@ -57,6 +57,7 @@ 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/ExceptionInput.missing' => 'Required field "{field}" missing while performing action "{action}"', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.whitespace' => 'Required field "{field}" of action "{action}" may not contain only whitespace', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.tooLong' => 'Required field "{field}" of action "{action}" has a maximum length of {max}', @@ -66,6 +67,8 @@ return [ 'Exception.JKingWeb/Arsse/Db/ExceptionInput.constraintViolation' => '{0}', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.typeViolation' => '{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 4d584fc..57bc3f2 100644 --- a/tests/Db/SQLite3/TestDbDriverSQLite3.php +++ b/tests/Db/SQLite3/TestDbDriverSQLite3.php @@ -8,6 +8,7 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { protected $data; protected $drv; + protected $ch; function setUp() { $this->clearData(); @@ -16,10 +17,12 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $conf->dbSQLite3File = tempnam(sys_get_temp_dir(), 'ook'); Data::$conf = $conf; $this->drv = new Db\SQLite3\Driver(true); + $this->ch = new \SQLite3(Data::$conf->dbSQLite3File); } function tearDown() { unset($this->drv); + unset($this->ch); unlink(Data::$conf->dbSQLite3File); $this->clearData(); } @@ -40,13 +43,11 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { function testExecMultipleStatements() { $this->assertTrue($this->drv->exec("CREATE TABLE test(id integer primary key); INSERT INTO test(id) values(2112)")); - $ch = new \SQLite3(Data::$conf->dbSQLite3File); - $this->assertEquals(2112, $ch->querySingle("SELECT id from test")); + $this->assertEquals(2112, $this->ch->querySingle("SELECT id from test")); } function testExecTimeout() { - $ch = new \SQLite3(Data::$conf->dbSQLite3File); - $ch->exec("BEGIN EXCLUSIVE TRANSACTION"); + $this->ch->exec("BEGIN EXCLUSIVE TRANSACTION"); $this->assertException("general", "Db", "ExceptionTimeout"); $this->drv->exec("CREATE TABLE test(id integer primary key)"); } @@ -73,8 +74,7 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { } function testQueryTimeout() { - $ch = new \SQLite3(Data::$conf->dbSQLite3File); - $ch->exec("BEGIN EXCLUSIVE TRANSACTION"); + $this->ch->exec("BEGIN EXCLUSIVE TRANSACTION"); $this->assertException("general", "Db", "ExceptionTimeout"); $this->drv->query("CREATE TABLE test(id integer primary key)"); } @@ -101,122 +101,189 @@ class TestDbDriverSQLite3 extends \PHPUnit\Framework\TestCase { $s = $this->drv->prepare("This is an invalid query", "int", "int"); } - function testBeginTransaction() { + function testCreateASavepoint() { + $this->assertEquals(1, $this->drv->savepointCreate()); + $this->assertEquals(2, $this->drv->savepointCreate()); + $this->assertEquals(3, $this->drv->savepointCreate()); + } + + function testReleaseASavepoint() { + $this->assertEquals(1, $this->drv->savepointCreate()); + $this->assertEquals(true, $this->drv->savepointRelease()); + $this->assertException("invalid", "Db", "ExceptionSavepoint"); + $this->drv->savepointRelease(); + } + + function testUndoASavepoint() { + $this->assertEquals(1, $this->drv->savepointCreate()); + $this->assertEquals(true, $this->drv->savepointUndo()); + $this->assertException("invalid", "Db", "ExceptionSavepoint"); + $this->drv->savepointUndo(); + } + + function testManipulateSavepoints() { + $this->assertEquals(1, $this->drv->savepointCreate()); + $this->assertEquals(2, $this->drv->savepointCreate()); + $this->assertEquals(3, $this->drv->savepointCreate()); + $this->assertEquals(4, $this->drv->savepointCreate()); + $this->assertEquals(5, $this->drv->savepointCreate()); + $this->assertEquals(true, $this->drv->savepointUndo(3)); + $this->assertEquals(false, $this->drv->savepointRelease(4)); + $this->assertEquals(6, $this->drv->savepointCreate()); + $this->assertEquals(false, $this->drv->savepointRelease(5)); + $this->assertEquals(true, $this->drv->savepointRelease(6)); + $this->assertEquals(3, $this->drv->savepointCreate()); + $this->assertEquals(true, $this->drv->savepointRelease(2)); + $this->assertException("stale", "Db", "ExceptionSavepoint"); + $this->drv->savepointRelease(2); + } + + function testBeginATransaction() { $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)"); $tr = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); } - function testCommitTransaction() { + function testCommitATransaction() { $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)"); $tr = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr->commit(); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(1, $ch->querySingle($select)); + $this->assertEquals(1, $this->ch->querySingle($select)); } - function testRollbackTransaction() { + function testRollbackATransaction() { $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)"); $tr = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr->rollback(); $this->assertEquals(0, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); } function testBeginChainedTransactions() { $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)"); $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); } function testCommitChainedTransactions() { $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)"); $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2->commit(); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); + $tr1->commit(); + $this->assertEquals(2, $this->ch->querySingle($select)); + } + + function testCommitChainedTransactionsOutOfOrder() { + $select = "SELECT count(*) FROM test"; + $insert = "INSERT INTO test(id) values(null)"; + $this->drv->exec("CREATE TABLE test(id integer primary key)"); + $tr1 = $this->drv->begin(); + $this->drv->query($insert); + $this->assertEquals(1, $this->drv->query($select)->getValue()); + $this->assertEquals(0, $this->ch->querySingle($select)); + $tr2 = $this->drv->begin(); + $this->drv->query($insert); + $this->assertEquals(2, $this->drv->query($select)->getValue()); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr1->commit(); - $this->assertEquals(2, $ch->querySingle($select)); + $this->assertEquals(2, $this->ch->querySingle($select)); + $tr2->commit(); } function testRollbackChainedTransactions() { $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)"); $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2->rollback(); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); + $tr1->rollback(); + $this->assertEquals(0, $this->drv->query($select)->getValue()); + $this->assertEquals(0, $this->ch->querySingle($select)); + } + + function testRollbackChainedTransactionsOutOfOrder() { + $select = "SELECT count(*) FROM test"; + $insert = "INSERT INTO test(id) values(null)"; + $this->drv->exec("CREATE TABLE test(id integer primary key)"); + $tr1 = $this->drv->begin(); + $this->drv->query($insert); + $this->assertEquals(1, $this->drv->query($select)->getValue()); + $this->assertEquals(0, $this->ch->querySingle($select)); + $tr2 = $this->drv->begin(); + $this->drv->query($insert); + $this->assertEquals(2, $this->drv->query($select)->getValue()); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr1->rollback(); $this->assertEquals(0, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); + $tr2->rollback(); + $this->assertEquals(0, $this->drv->query($select)->getValue()); + $this->assertEquals(0, $this->ch->querySingle($select)); } function testPartiallyRollbackChainedTransactions() { $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)"); $tr1 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2 = $this->drv->begin(); $this->drv->query($insert); $this->assertEquals(2, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr2->rollback(); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(0, $ch->querySingle($select)); + $this->assertEquals(0, $this->ch->querySingle($select)); $tr1->commit(); $this->assertEquals(1, $this->drv->query($select)->getValue()); - $this->assertEquals(1, $ch->querySingle($select)); + $this->assertEquals(1, $this->ch->querySingle($select)); } function testFetchSchemaVersion() {