Fix PDO insert ID errors in PHP 7.1

lib/Db/PDODriver.php

@@ -26,13 +26,7 @@ trait PDODriver {
             list($excClass, $excMsg, $excData) = $this->exceptionBuild();
             throw new $excClass($excMsg, $excData);
         }
-        $changes = $r->rowCount();
+        return new PDOResult($this->db, $r);
-        try {
-            $lastId = 0;
-            $lastId = ($changes) ? $this->db->lastInsertId() : 0;
-        } catch (\PDOException $e) { // @codeCoverageIgnore
-        }
-        return new PDOResult($r, [$changes, $lastId]);
     }
 
     public function prepareArray(string $query, array $paramTypes): Statement {

lib/Db/PDOResult.php

@@ -10,26 +10,28 @@ use JKingWeb\Arsse\Db\Exception;
 
 class PDOResult extends AbstractResult {
     protected $set;
+    protected $db;
     protected $cur = null;
-    protected $rows = 0;
-    protected $id = 0;
 
     // actual public methods
 
     public function changes(): int {
-        return $this->rows;
+        return $this->set->rowCount();
     }
 
     public function lastId(): int {
-        return $this->id;
+        try {
+            return (int) $this->db->lastInsertId();
+        } catch (\PDOException $e) {
+            return 0;
+        }
     }
 
     // constructor/destructor
 
-    public function __construct(\PDOStatement $result, array $changes = [0,0]) {
+    public function __construct(\PDO $db, \PDOStatement $result) {
         $this->set = $result;
-        $this->rows = (int) $changes[0];
+        $this->db = $db;
-        $this->id = (int) $changes[1];
     }
 
     public function __destruct() {
@@ -38,6 +40,7 @@ class PDOResult extends AbstractResult {
         } catch (\PDOException $e) { // @codeCoverageIgnore
         }
         unset($this->set);
+        unset($this->db);
     }
 
     // PHP iterator methods

lib/Db/PDOStatement.php

@@ -40,13 +40,7 @@ class PDOStatement extends AbstractStatement {
             list($excClass, $excMsg, $excData) = $this->exceptionBuild();
             throw new $excClass($excMsg, $excData);
         }
-        $changes = $this->st->rowCount();
+        return new PDOResult($this->db, $this->st);
-        try {
-            $lastId = 0;
-            $lastId = ($changes) ? $this->db->lastInsertId() : 0;
-        } catch (\PDOException $e) { // @codeCoverageIgnore
-        }
-        return new PDOResult($this->st, [$changes, $lastId]);
     }
 
     protected function bindValue($value, string $type, int $position): bool {

lib/Db/PostgreSQL/PDODriver.php

@@ -22,7 +22,10 @@ class PDODriver extends Driver {
 
     protected function makeConnection(string $user, string $pass, string $db, string $host, int $port, string $service) {
         $dsn = $this->makeconnectionString(true, $user, $pass, $db, $host, $port, $service);
-        $this->db = new \PDO("pgsql:$dsn", $user, $pass, [\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION]);
+        $this->db = new \PDO("pgsql:$dsn", $user, $pass, [
+            \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
+            //\PDO::ATTR_PERSISTENT => true,
+        ]);
     }
 
     public function __destruct() {

lib/Db/SQLite3/PDODriver.php

@@ -21,7 +21,9 @@ class PDODriver extends Driver {
     }
 
     protected function makeConnection(string $file, string $key) {
-        $this->db = new \PDO("sqlite:".$file, "", "", [\PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION]);
+        $this->db = new \PDO("sqlite:".$file, "", "", [
+            \PDO::ATTR_ERRMODE => \PDO::ERRMODE_EXCEPTION,
+        ]);
     }
 
     public function __destruct() {

tests/cases/Db/BaseDriver.php

@@ -57,7 +57,7 @@ abstract class BaseDriver extends \JKingWeb\Arsse\Test\AbstractTest {
     }
 
     public static function tearDownAfterClass() {
-        static::$implementation = null;
+        static::$interface = null;
         static::$dbInfo = null;
         self::clearData();
     }

tests/cases/Db/BaseResult.php

@@ -56,13 +56,20 @@ abstract class BaseResult extends \JKingWeb\Arsse\Test\AbstractTest {
     }
 
     public function testGetChangeCountAndLastInsertId() {
-        $this->makeResult("CREATE TABLE arsse_meta(key varchar(255) primary key not null, value text)");
+        $this->makeResult(static::$createMeta);
-        $out = $this->makeResult("INSERT INTO arsse_meta(key,value) values('test', 1)");
+        $r = new $this->resultClass(...$this->makeResult("INSERT INTO arsse_meta(key,value) values('test', 1)"));
-        $rows = $out[1][0];
+        $this->assertSame(1, $r->changes());
-        $id = $out[1][1];
+        $this->assertSame(0, $r->lastId());
-        $r = new $this->resultClass(...$out);
+    }
-        $this->assertSame((int) $rows, $r->changes());
+
-        $this->assertSame((int) $id, $r->lastId());
+    public function testGetChangeCountAndLastInsertIdBis() {
+        $this->makeResult(static::$createTest);
+        $r = new $this->resultClass(...$this->makeResult("INSERT INTO arsse_test default values"));
+        $this->assertSame(1, $r->changes());
+        $this->assertSame(1, $r->lastId());
+        $r = new $this->resultClass(...$this->makeResult("INSERT INTO arsse_test default values"));
+        $this->assertSame(1, $r->changes());
+        $this->assertSame(2, $r->lastId());
     }
 
     public function testIterateOverResults() {
@@ -91,7 +98,7 @@ abstract class BaseResult extends \JKingWeb\Arsse\Test\AbstractTest {
     public function testGetSingleValues() {
         $exp = [1867, 1970, 2112];
         $exp = $this->stringOutput ? $this->stringify($exp) : $exp;
-        $test = new $this->resultClass(...$this->makeResult("SELECT 1867 as year union select 1970 as year union select 2112 as year"));
+        $test = new $this->resultClass(...$this->makeResult("SELECT 1867 as year union all select 1970 as year union all select 2112 as year"));
         $this->assertSame($exp[0], $test->getValue());
         $this->assertSame($exp[1], $test->getValue());
         $this->assertSame($exp[2], $test->getValue());
@@ -101,7 +108,7 @@ abstract class BaseResult extends \JKingWeb\Arsse\Test\AbstractTest {
     public function testGetFirstValuesOnly() {
         $exp = [1867, 1970, 2112];
         $exp = $this->stringOutput ? $this->stringify($exp) : $exp;
-        $test = new $this->resultClass(...$this->makeResult("SELECT 1867 as year, 19 as century union select 1970 as year, 20 as century union select 2112 as year, 22 as century"));
+        $test = new $this->resultClass(...$this->makeResult("SELECT 1867 as year, 19 as century union all select 1970 as year, 20 as century union all select 2112 as year, 22 as century"));
         $this->assertSame($exp[0], $test->getValue());
         $this->assertSame($exp[1], $test->getValue());
         $this->assertSame($exp[2], $test->getValue());

tests/cases/Db/PostgreSQL/TestResult.php

@@ -0,0 +1,23 @@
+<?php
+/** @license MIT
+ * Copyright 2017 J. King, Dustin Wilson et al.
+ * See LICENSE and AUTHORS files for details */
+
+declare(strict_types=1);
+namespace JKingWeb\Arsse\TestCase\Db\PostgreSQL;
+
+use JKingWeb\Arsse\Test\DatabaseInformation;
+
+/**
+ * @covers \JKingWeb\Arsse\Db\ResultPDO<extended>
+ */
+class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult {
+    protected static $implementation = "PDO PostgreSQL";
+    protected static $createMeta = "CREATE TABLE arsse_meta(key text primary key not null, value text)";
+    protected static $createTest = "CREATE TABLE arsse_test(id bigserial primary key)";
+
+    protected function makeResult(string $q): array {
+        $set = static::$interface->query($q);
+        return [static::$interface, $set];
+    }
+}

tests/cases/Db/SQLite3/TestResult.php

@@ -13,6 +13,8 @@ use JKingWeb\Arsse\Test\DatabaseInformation;
  */
 class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult {
     protected static $implementation = "SQLite 3";
+    protected static $createMeta = "CREATE TABLE arsse_meta(key text primary key not null, value text) without rowid";
+    protected static $createTest = "CREATE TABLE arsse_test(id integer primary key)";
 
     public static function tearDownAfterClass() {
         if (static::$interface) {

tests/cases/Db/SQLite3PDO/TestResult.php

@@ -0,0 +1,23 @@
+<?php
+/** @license MIT
+ * Copyright 2017 J. King, Dustin Wilson et al.
+ * See LICENSE and AUTHORS files for details */
+
+declare(strict_types=1);
+namespace JKingWeb\Arsse\TestCase\Db\SQLite3PDO;
+
+use JKingWeb\Arsse\Test\DatabaseInformation;
+
+/**
+ * @covers \JKingWeb\Arsse\Db\ResultPDO<extended>
+ */
+class TestResult extends \JKingWeb\Arsse\TestCase\Db\BaseResult {
+    protected static $implementation = "PDO SQLite 3";
+    protected static $createMeta = "CREATE TABLE arsse_meta(key text primary key not null, value text) without rowid";
+    protected static $createTest = "CREATE TABLE arsse_test(id integer primary key)";
+
+    protected function makeResult(string $q): array {
+        $set = static::$interface->query($q);
+        return [static::$interface, $set];
+    }
+}

tests/cases/Db/TestResultPDO.php

@@ -1,41 +0,0 @@
-<?php
-/** @license MIT
- * Copyright 2017 J. King, Dustin Wilson et al.
- * See LICENSE and AUTHORS files for details */
-
-declare(strict_types=1);
-namespace JKingWeb\Arsse\TestCase\Db;
-
-use JKingWeb\Arsse\Test\DatabaseInformation;
-
-/**
- * @covers \JKingWeb\Arsse\Db\PDOResult<extended>
- */
-class TestResultPDO extends \JKingWeb\Arsse\TestCase\Db\BaseResult {
-    protected static $implementation;
-
-    public static function setUpBeforeClass() {
-        self::setConf();
-        // we only need to test one PDO implementation (they all use the same result class), so we find the first usable one
-        $drivers = DatabaseInformation::listPDO();
-        self::$implementation = $drivers[0];
-        foreach ($drivers as $driver) {
-            $info = new DatabaseInformation($driver);
-            $interface = ($info->interfaceConstructor)();
-            if ($interface) {
-                self::$implementation = $driver;
-                break;
-            }
-        }
-        unset($interface);
-        unset($info);
-        parent::setUpBeforeClass();
-    }
-
-    protected function makeResult(string $q): array {
-        $set = static::$interface->query($q);
-        $rows = $set->rowCount();
-        $id = static::$interface->lastInsertID();
-        return [$set, [$rows, $id]];
-    }
-}

tests/phpunit.xml

@@ -45,7 +45,6 @@
         <file>cases/Db/TestTransaction.php</file>
         <file>cases/Db/TestResultAggregate.php</file>
         <file>cases/Db/TestResultEmpty.php</file>
-        <file>cases/Db/TestResultPDO.php</file>
 
         <file>cases/Db/SQLite3/TestResult.php</file>
         <file>cases/Db/SQLite3/TestStatement.php</file>
@@ -53,11 +52,13 @@
         <file>cases/Db/SQLite3/TestDriver.php</file>
         <file>cases/Db/SQLite3/TestUpdate.php</file>
 
+        <file>cases/Db/SQLite3PDO/TestResult.php</file>
         <file>cases/Db/SQLite3PDO/TestStatement.php</file>
         <file>cases/Db/SQLite3PDO/TestCreation.php</file>
         <file>cases/Db/SQLite3PDO/TestDriver.php</file>
         <file>cases/Db/SQLite3PDO/TestUpdate.php</file>
 
+        <file>cases/Db/PostgreSQL/TestResult.php</file>
         <file>cases/Db/PostgreSQL/TestStatement.php</file>
         <file>cases/Db/PostgreSQL/TestCreation.php</file>
         <file>cases/Db/PostgreSQL/TestDriver.php</file>