From 9304f990327950c83de8beaea624e28af16927eb Mon Sep 17 00:00:00 2001 From: "J. King" Date: Sun, 5 Nov 2017 22:13:44 -0500 Subject: [PATCH] Make result sets single-use; change savewepoint exceptions - Result sets are now single-use; this is required for PDO drivers (PDO result sets are not rewindable) - Change savepoint exceptions to be simple database exceptions; codes remain the same --- lib/AbstractException.php | 7 ++++--- lib/Db/AbstractDriver.php | 12 ++++++------ lib/Db/ExceptionSavepoint.php | 6 ------ lib/Db/SQLite3/Result.php | 13 +++++-------- locale/en.php | 7 ++++--- tests/Db/SQLite3/TestDbDriverSQLite3.php | 8 ++++---- tests/Db/SQLite3/TestDbResultSQLite3.php | 12 +++++++++++- tests/Db/TestTransaction.php | 2 +- 8 files changed, 35 insertions(+), 32 deletions(-) delete mode 100644 lib/Db/ExceptionSavepoint.php diff --git a/lib/AbstractException.php b/lib/AbstractException.php index 5d5ae3d3..da0fc7f4 100644 --- a/lib/AbstractException.php +++ b/lib/AbstractException.php @@ -33,9 +33,10 @@ 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/Exception.savepointStatusUnknown" => 10225, + "Db/Exception.savepointInvalid" => 10226, + "Db/Exception.savepointStale" => 10227, + "Db/Exception.resultReused" => 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 b49ceb14..6ae59929 100644 --- a/lib/Db/AbstractDriver.php +++ b/lib/Db/AbstractDriver.php @@ -60,9 +60,9 @@ abstract class AbstractDriver implements Driver { break; case self::TR_COMMIT: case self::TR_ROLLBACK: //@codeCoverageIgnore - throw new ExceptionSavepoint("stale", ['action' => "commit", 'index' => $index]); + throw new Exception("savepointStale", ['action' => "commit", 'index' => $index]); default: - throw new Exception("unknownSavepointStatus", $this->transStatus[$index]); //@codeCoverageIgnore + throw new Exception("savepointStatusUnknown", $this->transStatus[$index]); // @codeCoverageIgnore } if ($index==$this->transDepth) { while ($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { @@ -76,7 +76,7 @@ abstract class AbstractDriver implements Driver { } return $out; } else { - throw new ExceptionSavepoint("invalid", ['action' => "commit", 'index' => $index]); + throw new Exception("savepointInvalid", ['action' => "commit", 'index' => $index]); } } @@ -106,9 +106,9 @@ abstract class AbstractDriver implements Driver { break; case self::TR_COMMIT: case self::TR_ROLLBACK: //@codeCoverageIgnore - throw new ExceptionSavepoint("stale", ['action' => "rollback", 'index' => $index]); + throw new Exception("savepointStale", ['action' => "rollback", 'index' => $index]); default: - throw new Exception("unknownSavepointStatus", $this->transStatus[$index]); //@codeCoverageIgnore + throw new Exception("savepointStatusUnknown", $this->transStatus[$index]); // @codeCoverageIgnore } if ($index==$this->transDepth) { while ($this->transDepth > 0 && $this->transStatus[$this->transDepth] > self::TR_PEND) { @@ -122,7 +122,7 @@ abstract class AbstractDriver implements Driver { } return $out; } else { - throw new ExceptionSavepoint("invalid", ['action' => "rollback", 'index' => $index]); + throw new Exception("savepointInvalid", ['action' => "rollback", 'index' => $index]); } } diff --git a/lib/Db/ExceptionSavepoint.php b/lib/Db/ExceptionSavepoint.php deleted file mode 100644 index a90839ea..00000000 --- a/lib/Db/ExceptionSavepoint.php +++ /dev/null @@ -1,6 +0,0 @@ -pos = 0; - $this->cur = null; - $this->set->reset(); + if ($this->pos) { + throw new Exception("resultReused"); + } } } diff --git a/locale/en.php b/locale/en.php index d232f1f8..e7f9e293 100644 --- a/locale/en.php +++ b/locale/en.php @@ -123,7 +123,10 @@ 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/Exception.savepointStatusUnknown' => 'Savepoint status code {0} not implemented', + 'Exception.JKingWeb/Arsse/Db/Exception.savepointInvalid' => 'Tried to {action} invalid savepoint {index}', + 'Exception.JKingWeb/Arsse/Db/Exception.savepointStale' => 'Tried to {action} stale savepoint {index}', + 'Exception.JKingWeb/Arsse/Db/Exception.resultReused' => 'Result set already iterated', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.missing' => 'Required field "{field}" missing while performing action "{action}"', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.whitespace' => 'Field "{field}" of action "{action}" may not contain only whitespace', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.tooLong' => 'Field "{field}" of action "{action}" has a maximum length of {max}', @@ -136,8 +139,6 @@ return [ 'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineConstraintViolation' => '{0}', 'Exception.JKingWeb/Arsse/Db/ExceptionInput.engineTypeViolation' => '{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 0c80b5b3..59936b9e 100644 --- a/tests/Db/SQLite3/TestDbDriverSQLite3.php +++ b/tests/Db/SQLite3/TestDbDriverSQLite3.php @@ -117,14 +117,14 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { public function testReleaseASavepoint() { $this->assertEquals(1, $this->drv->savepointCreate()); $this->assertEquals(true, $this->drv->savepointRelease()); - $this->assertException("invalid", "Db", "ExceptionSavepoint"); + $this->assertException("savepointInvalid", "Db"); $this->drv->savepointRelease(); } public function testUndoASavepoint() { $this->assertEquals(1, $this->drv->savepointCreate()); $this->assertEquals(true, $this->drv->savepointUndo()); - $this->assertException("invalid", "Db", "ExceptionSavepoint"); + $this->assertException("savepointInvalid", "Db"); $this->drv->savepointUndo(); } @@ -141,7 +141,7 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { $this->assertTrue($this->drv->savepointRelease(6)); $this->assertEquals(3, $this->drv->savepointCreate()); $this->assertTrue($this->drv->savepointRelease(2)); - $this->assertException("stale", "Db", "ExceptionSavepoint"); + $this->assertException("savepointStale", "Db"); $this->drv->savepointRelease(2); } @@ -152,7 +152,7 @@ class TestDbDriverSQLite3 extends Test\AbstractTest { $this->assertEquals(4, $this->drv->savepointCreate()); $this->assertTrue($this->drv->savepointRelease(2)); $this->assertFalse($this->drv->savepointUndo(3)); - $this->assertException("stale", "Db", "ExceptionSavepoint"); + $this->assertException("savepointStale", "Db"); $this->drv->savepointUndo(2); } diff --git a/tests/Db/SQLite3/TestDbResultSQLite3.php b/tests/Db/SQLite3/TestDbResultSQLite3.php index 3d92b540..2fd187ab 100644 --- a/tests/Db/SQLite3/TestDbResultSQLite3.php +++ b/tests/Db/SQLite3/TestDbResultSQLite3.php @@ -51,10 +51,11 @@ class TestDbResultSQLite3 extends Test\AbstractTest { foreach ($test as $row) { $rows[] = $row['col']; } + $this->assertEquals([1,2,3], $rows); + $this->assertException("resultReused", "Db"); foreach ($test as $row) { $rows[] = $row['col']; } - $this->assertEquals([1,2,3,1,2,3], $rows); } public function testGetSingleValues() { @@ -85,6 +86,15 @@ class TestDbResultSQLite3 extends Test\AbstractTest { $this->assertEquals($rows[0], $test->getRow()); $this->assertEquals($rows[1], $test->getRow()); $this->assertSame(null, $test->getRow()); + } + + public function testGetAllRows() { + $set = $this->c->query("SELECT '2112' as album, '2112' as track union select 'Clockwork Angels' as album, 'The Wreckers' as track"); + $rows = [ + ['album' => '2112', 'track' => '2112'], + ['album' => 'Clockwork Angels', 'track' => 'The Wreckers'], + ]; + $test = new Db\SQLite3\Result($set); $this->assertEquals($rows, $test->getAll()); } } diff --git a/tests/Db/TestTransaction.php b/tests/Db/TestTransaction.php index 721b17d4..9c435101 100644 --- a/tests/Db/TestTransaction.php +++ b/tests/Db/TestTransaction.php @@ -47,7 +47,7 @@ class TestTransaction extends Test\AbstractTest { } public function testIgnoreRollbackErrors() { - Phake::when($this->drv)->savepointUndo->thenThrow(new Db\ExceptionSavepoint("stale")); + Phake::when($this->drv)->savepointUndo->thenThrow(new Db\Exception("savepointStale")); $tr1 = new Transaction($this->drv); $tr2 = new Transaction($this->drv); unset($tr1, $tr2); // no exception should bubble up