diff --git a/lib/Db/AbstractDriver.php b/lib/Db/AbstractDriver.php index 45b9476..814159e 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -78,50 +78,63 @@ abstract class AbstractDriver implements Driver { } public function savepointCreate(bool $lock = false): int { + // if no transaction is active and a lock was requested, lock the database using a backend-specific routine if ($lock && !$this->transDepth) { $this->lock(); $this->locked = true; } + // create a savepoint, incrementing the transaction depth $this->exec("SAVEPOINT arsse_".(++$this->transDepth)); + // set the state of the newly created savepoint to pending $this->transStatus[$this->transDepth] = self::TR_PEND; + // return the depth number return $this->transDepth; } public function savepointRelease(int $index = null): bool { + // assume the most recent savepoint if none was specified $index = $index ?? $this->transDepth; if (array_key_exists($index, $this->transStatus)) { switch ($this->transStatus[$index]) { case self::TR_PEND: + // release the requested savepoint and set its state to committed $this->exec("RELEASE SAVEPOINT arsse_".$index); $this->transStatus[$index] = self::TR_COMMIT; + // for any later pending savepoints, set their state to implicitly committed $a = $index; while (++$a && $a <= $this->transDepth) { if ($this->transStatus[$a] <= self::TR_PEND) { $this->transStatus[$a] = self::TR_PEND_COMMIT; } } + // return success $out = true; break; case self::TR_PEND_COMMIT: + // set the state to explicitly committed $this->transStatus[$index] = self::TR_COMMIT; $out = true; break; case self::TR_PEND_ROLLBACK: + // set the state to explicitly committed $this->transStatus[$index] = self::TR_COMMIT; $out = false; break; case self::TR_COMMIT: case self::TR_ROLLBACK: //@codeCoverageIgnore + // savepoint has already been released or rolled back; this is an error throw new Exception("savepointStale", ['action' => "commit", 'index' => $index]); default: throw new Exception("savepointStatusUnknown", $this->transStatus[$index]); // @codeCoverageIgnore } if ($index==$this->transDepth) { + // if we've released the topmost savepoint, clean up all prior savepoints which have already been explicitly committed (or rolled back), if any while ($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { array_pop($this->transStatus); $this->transDepth--; } } + // if no savepoints are pending and the database was locked, unlock it if (!$this->transDepth && $this->locked) { $this->unlock(); $this->locked = false; diff --git a/lib/Db/PostgreSQL/Driver.php b/lib/Db/PostgreSQL/Driver.php index 5e9f637..be8e40e 100644 --- a/lib/Db/PostgreSQL/Driver.php +++ b/lib/Db/PostgreSQL/Driver.php @@ -13,6 +13,8 @@ use JKingWeb\Arsse\Db\ExceptionInput; use JKingWeb\Arsse\Db\ExceptionTimeout; class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { + protected $transStart = 0; + public function __construct(string $user = null, string $pass = null, string $db = null, string $host = null, int $port = null, string $schema = null, string $service = null) { // check to make sure required extension is loaded if (!static::requirementsMet()) { @@ -104,37 +106,44 @@ class Driver extends \JKingWeb\Arsse\Db\AbstractDriver { } public function savepointCreate(bool $lock = false): int { - if (!$this->transDepth) { + if (!$this->transStart) { $this->exec("BEGIN TRANSACTION"); + $this->transStart = parent::savepointCreate($lock); + return $this->transStart; + } else { + return parent::savepointCreate($lock); } - return parent::savepointCreate($lock); } public function savepointRelease(int $index = null): bool { - $out = parent::savepointUndo($index); - if ($out && !$this->transDepth) { - $this->exec("COMMIT TRANSACTION"); + $index = $index ?? $this->transDepth; + $out = parent::savepointRelease($index); + if ($index == $this->transStart) { + $this->exec("COMMIT"); + $this->transStart = 0; } return $out; } public function savepointUndo(int $index = null): bool { + $index = $index ?? $this->transDepth; $out = parent::savepointUndo($index); - if ($out && !$this->transDepth) { - $this->exec("ROLLBACK TRANSACTION"); + if ($index == $this->transStart) { + $this->exec("ROLLBACK"); + $this->transStart = 0; } return $out; } protected function lock(): bool { - if ($this->schemaVersion()) { + if ($this->query("SELECT count(*) from information_schema.tables where table_schema = current_schema() and table_name = 'arsse_meta'")->getValue()) { $this->exec("LOCK TABLE arsse_meta IN EXCLUSIVE MODE NOWAIT"); } return true; } protected function unlock(bool $rollback = false): bool { - $this->exec((!$rollback) ? "COMMIT" : "ROLLBACK"); + // do nothing; transaction is committed or rolled back later return true; } diff --git a/tests/cases/Db/BaseDriver.php b/tests/cases/Db/BaseDriver.php index e918fa9..87f2d17 100644 --- a/tests/cases/Db/BaseDriver.php +++ b/tests/cases/Db/BaseDriver.php @@ -375,7 +375,7 @@ abstract class BaseDriver extends \JKingWeb\Arsse\Test\AbstractTest { // so the effect is usually the same $this->drv->savepointCreate(true); $this->assertException(); - $this->exec(str_replace("#", "3", $this->setVersion)); + $this->exec($this->lock); } public function testUnlockTheDatabase() { diff --git a/tests/cases/Db/PostgreSQL/TestDriver.php b/tests/cases/Db/PostgreSQL/TestDriver.php index 63e73c6..ae4c4b2 100644 --- a/tests/cases/Db/PostgreSQL/TestDriver.php +++ b/tests/cases/Db/PostgreSQL/TestDriver.php @@ -13,6 +13,14 @@ namespace JKingWeb\Arsse\TestCase\Db\PostgreSQL; class TestDriver extends \JKingWeb\Arsse\TestCase\Db\BaseDriver { protected static $implementation = "PDO PostgreSQL"; protected $create = "CREATE TABLE arsse_test(id bigserial primary key)"; - protected $lock = ["BEGIN", "LOCK TABLE arsse_test IN EXCLUSIVE MODE NOWAIT"]; + protected $lock = ["BEGIN", "LOCK TABLE arsse_meta IN EXCLUSIVE MODE NOWAIT"]; protected $setVersion = "UPDATE arsse_meta set value = '#' where key = 'schema_version'"; + + public function tearDown() { + try { + $this->drv->exec("ROLLBACK"); + } catch (\Throwable $e) { + } + parent::tearDown(); + } } diff --git a/tests/lib/DatabaseInformation.php b/tests/lib/DatabaseInformation.php index 0ec4d1d..01cc444 100644 --- a/tests/lib/DatabaseInformation.php +++ b/tests/lib/DatabaseInformation.php @@ -197,9 +197,13 @@ class DatabaseInformation { } }, 'razeFunction' => function($db, array $afterStatements = []) use ($pgObjectList) { + // rollback any pending transaction + try { + $db->exec("ROLLBACK"); + } catch(\Throwable $e) { + } foreach ($pgObjectList($db) as $obj) { $db->exec("DROP {$obj['type']} IF EXISTS {$obj['name']} cascade"); - } foreach ($afterStatements as $st) { $db->exec($st); diff --git a/tests/phpunit.xml b/tests/phpunit.xml index 343f381..06b3d6a 100644 --- a/tests/phpunit.xml +++ b/tests/phpunit.xml @@ -60,7 +60,7 @@ cases/Db/PostgreSQL/TestStatement.php cases/Db/PostgreSQL/TestCreation.php - + cases/Db/PostgreSQL/TestDriver.php