fix: isolate statement cache and fix SQLite inserts (#20)

* Fix SQLite inserts and statement cache isolation

* Cover SQLite in integration tests

* Handle SQLite review feedback

Author: davebarnwell

.github/workflows/ci.yml

@@ -17,7 +17,7 @@ jobs:
         with:
           php-version: "8.3"
           coverage: none
-          extensions: pdo, pdo_mysql, pdo_pgsql
+          extensions: pdo, pdo_mysql, pdo_pgsql, pdo_sqlite
 
       - name: Install dependencies
         run: composer install --no-interaction --no-progress --prefer-dist
@@ -34,7 +34,7 @@ jobs:
       fail-fast: false
       matrix:
         php: ["8.3", "8.4"]
-        db: ["mysql", "postgres"]
+        db: ["mysql", "postgres", "sqlite"]
 
     services:
       mysql:
@@ -71,7 +71,7 @@ jobs:
         with:
           php-version: ${{ matrix.php }}
           coverage: none
-          extensions: pdo, pdo_mysql, pdo_pgsql
+          extensions: pdo, pdo_mysql, pdo_pgsql, pdo_sqlite
 
       - name: Install dependencies
         run: composer install --no-interaction --no-progress --prefer-dist
@@ -82,10 +82,14 @@ jobs:
             echo "MODEL_ORM_TEST_DSN=mysql:host=127.0.0.1;port=3306" >> "$GITHUB_ENV"
             echo "MODEL_ORM_TEST_USER=root" >> "$GITHUB_ENV"
             echo "MODEL_ORM_TEST_PASS=" >> "$GITHUB_ENV"
-          else
+          elif [ "${{ matrix.db }}" = "postgres" ]; then
             echo "MODEL_ORM_TEST_DSN=pgsql:host=127.0.0.1;port=5432;dbname=categorytest" >> "$GITHUB_ENV"
             echo "MODEL_ORM_TEST_USER=postgres" >> "$GITHUB_ENV"
             echo "MODEL_ORM_TEST_PASS=postgres" >> "$GITHUB_ENV"
+          else
+            echo "MODEL_ORM_TEST_DSN=sqlite::memory:" >> "$GITHUB_ENV"
+            echo "MODEL_ORM_TEST_USER=" >> "$GITHUB_ENV"
+            echo "MODEL_ORM_TEST_PASS=" >> "$GITHUB_ENV"
           fi
 
       - name: Run tests

README.md

@@ -14,7 +14,7 @@ It is designed for projects that value straightforward PHP, direct PDO access, a
 - PDO-first: use the ORM helpers when they help and drop down to raw SQL when they do not.
 - Familiar model flow: create, hydrate, validate, save, update, count, find, and delete.
 - Dynamic finders: call methods such as `findByName()`, `findOneByName()`, `countByName()`, and more.
-- Multi-database support: tested against MySQL/MariaDB and PostgreSQL, with SQLite code paths also supported.
+- Multi-database support: tested against MySQL/MariaDB, PostgreSQL, and SQLite.
 
 ## Installation
 
@@ -193,7 +193,7 @@ Freshsauce\Model\Model::connectDb(
 );
 ```
 
-SQLite is supported in the library code paths, but the automated test suite currently covers MySQL/MariaDB and PostgreSQL.
+SQLite is supported in the library and covered by the automated test suite alongside MySQL/MariaDB and PostgreSQL.
 
 ## Quality
 

src/Model/Model.php

@@ -41,9 +41,9 @@ class Model
     // sub class must also redeclare public static $_db;
 
     /**
-     * @var \PDOStatement[]
+     * @var array<int, array<string, \PDOStatement>>
      */
-    protected static $_stmt = array(); // prepared statements cache
+    protected static $_stmt = array(); // prepared statements cache keyed by PDO connection and SQL
 
     /**
      * @var string|null
@@ -226,10 +226,13 @@ public function __isset($name)
      */
     public static function connectDb(string $dsn, string $username, string $password, array $driverOptions = array()): void
     {
+        $previousDb = static::$_db;
+        if ($previousDb instanceof \PDO) {
+            unset(static::$_stmt[spl_object_id($previousDb)]);
+        }
         static::$_db = new \PDO($dsn, $username, $password, $driverOptions);
         static::$_db->setAttribute(\PDO::ATTR_ERRMODE, \PDO::ERRMODE_EXCEPTION); // Set Errorhandling to Exception
         static::$_identifier_quote_character = null;
-        static::$_stmt = array();
         self::$_tableColumns = array();
         static::_setup_identifier_quote_character();
     }
@@ -966,7 +969,7 @@ public function insert($autoTimestamp = true, $allowSetPrimaryKey = false)
             return false;
         }
 
-        if ($set['sql'] === '') {
+        if (count($set['columns']) === 0) {
             if ($driver === 'sqlite' || $driver === 'sqlite2') {
                 $query = 'INSERT INTO ' . static::_quote_identifier(static::$_tableName) . ' DEFAULT VALUES';
                 $st = static::execute($query);
@@ -975,7 +978,9 @@ public function insert($autoTimestamp = true, $allowSetPrimaryKey = false)
                 $st = static::execute($query);
             }
         } else {
-            $query = 'INSERT INTO ' . static::_quote_identifier(static::$_tableName) . ' SET ' . $set['sql'];
+            $query = 'INSERT INTO ' . static::_quote_identifier(static::$_tableName) .
+                ' (' . implode(', ', $set['columns']) . ')' .
+                ' VALUES (' . implode(', ', $set['values']) . ')';
             $st    = static::execute($query, $set['params']);
         }
         if ($st->rowCount() == 1) {
@@ -1051,11 +1056,15 @@ protected static function _prepare($query)
         if (!$db) {
             throw new \Exception('No database connection setup');
         }
-        if (!isset(static::$_stmt[$query])) {
+        $connectionId = spl_object_id($db);
+        if (!isset(static::$_stmt[$connectionId])) {
+            static::$_stmt[$connectionId] = array();
+        }
+        if (!isset(static::$_stmt[$connectionId][$query])) {
             // cache prepared query if not seen before
-            static::$_stmt[$query] = $db->prepare($query);
+            static::$_stmt[$connectionId][$query] = $db->prepare($query);
         }
-        return static::$_stmt[$query]; // return cache copy
+        return static::$_stmt[$connectionId][$query]; // return cache copy
     }
 
     /**
@@ -1138,7 +1147,7 @@ protected function setString($ignorePrimary = true)
     protected static function supportsUpdateLimit()
     {
         $driver = static::getDriverName();
-        return ($driver === 'mysql' || $driver === 'sqlite' || $driver === 'sqlite2');
+        return ($driver === 'mysql');
     }
 
     /**
@@ -1150,7 +1159,7 @@ protected static function supportsUpdateLimit()
     protected static function supportsDeleteLimit()
     {
         $driver = static::getDriverName();
-        return ($driver === 'mysql' || $driver === 'sqlite' || $driver === 'sqlite2');
+        return ($driver === 'mysql');
     }
 
     /**

test-src/Model/IsolatedConnectionCategoryA.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Model;
+
+/**
+ * @property int|null $id
+ * @property string|null $name
+ */
+class IsolatedConnectionCategoryA extends \Freshsauce\Model\Model
+{
+    public static $_db;
+
+    protected static $_tableName = 'items';
+}

test-src/Model/IsolatedConnectionCategoryB.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace App\Model;
+
+/**
+ * @property int|null $id
+ * @property string|null $name
+ */
+class IsolatedConnectionCategoryB extends \Freshsauce\Model\Model
+{
+    public static $_db;
+
+    protected static $_tableName = 'items';
+}

test-src/Model/SqliteCategory.php

@@ -0,0 +1,16 @@
+<?php
+
+namespace App\Model;
+
+/**
+ * @property int|null $id
+ * @property string|null $name
+ * @property string|null $updated_at
+ * @property string|null $created_at
+ */
+class SqliteCategory extends \Freshsauce\Model\Model
+{
+    public static $_db;
+
+    protected static $_tableName = 'categories';
+}

tests/Model/CategoryTest.php

@@ -8,6 +8,7 @@
 class CategoryTest extends TestCase
 {
     private const TEST_DB_NAME = 'categorytest';
+    private const SQLITE_SEQUENCE_TABLE = 'sqlite_sequence';
     private static ?string $driverName = null;
 
     /**
@@ -53,6 +54,16 @@ public static function setUpBeforeClass(): void
                  "created_at" TIMESTAMP NULL
                )',
             ];
+        } elseif (self::$driverName === 'sqlite') {
+            $sql_setup = [
+                'DROP TABLE IF EXISTS `categories`',
+                'CREATE TABLE `categories` (
+                 `id` INTEGER PRIMARY KEY AUTOINCREMENT,
+                 `name` VARCHAR(120) NULL,
+                 `updated_at` TEXT NULL,
+                 `created_at` TEXT NULL
+               )',
+            ];
         } else {
             throw new RuntimeException('Unsupported PDO driver for tests: ' . self::$driverName);
         }
@@ -81,6 +92,12 @@ public function setUp(): void
             Freshsauce\Model\Model::execute('TRUNCATE TABLE `categories`');
         } elseif (self::$driverName === 'pgsql') {
             Freshsauce\Model\Model::execute('TRUNCATE TABLE "categories" RESTART IDENTITY');
+        } elseif (self::$driverName === 'sqlite') {
+            Freshsauce\Model\Model::execute('DELETE FROM `categories`');
+            Freshsauce\Model\Model::execute(
+                'DELETE FROM `' . self::SQLITE_SEQUENCE_TABLE . '` WHERE `name` = ?',
+                ['categories']
+            );
         }
     }
 

tests/Model/SqliteModelTest.php

@@ -0,0 +1,82 @@
+<?php
+
+use PHPUnit\Framework\TestCase;
+use PHPUnit\Framework\SkippedTestSuiteError;
+
+class SqliteModelTest extends TestCase
+{
+    public static function setUpBeforeClass(): void
+    {
+        if (!in_array('sqlite', \PDO::getAvailableDrivers(), true)) {
+            throw new SkippedTestSuiteError('The pdo_sqlite extension is required to run SQLite-specific tests.');
+        }
+
+        App\Model\SqliteCategory::connectDb('sqlite::memory:', '', '');
+        App\Model\SqliteCategory::execute(
+            'CREATE TABLE categories (
+                id INTEGER PRIMARY KEY AUTOINCREMENT,
+                name TEXT NULL,
+                updated_at TEXT NULL,
+                created_at TEXT NULL
+            )'
+        );
+    }
+
+    protected function setUp(): void
+    {
+        App\Model\SqliteCategory::execute('DELETE FROM `categories`');
+        App\Model\SqliteCategory::execute('DELETE FROM sqlite_sequence WHERE name = ?', ['categories']);
+    }
+
+    public function testSqliteInsertWithDirtyFields(): void
+    {
+        /** @var App\Model\SqliteCategory $category */
+        $category = new App\Model\SqliteCategory([
+            'name' => 'SQLite Fiction',
+        ]);
+
+        $this->assertTrue($category->save());
+        $this->assertSame('SQLite Fiction', $category->name);
+        $this->assertSame('1', (string) $category->id);
+        $this->assertNotEmpty($category->created_at);
+        $this->assertNotEmpty($category->updated_at);
+
+        /** @var App\Model\SqliteCategory|null $reloaded */
+        $reloaded = App\Model\SqliteCategory::getById((int) $category->id);
+
+        $this->assertNotNull($reloaded);
+        $this->assertSame('SQLite Fiction', $reloaded->name);
+    }
+
+    public function testPreparedStatementsStayBoundToTheirOwnConnection(): void
+    {
+        App\Model\IsolatedConnectionCategoryA::connectDb('sqlite::memory:', '', '');
+        App\Model\IsolatedConnectionCategoryB::connectDb('sqlite::memory:', '', '');
+
+        App\Model\IsolatedConnectionCategoryA::execute(
+            'CREATE TABLE items (
+                id INTEGER PRIMARY KEY AUTOINCREMENT,
+                name TEXT NULL
+            )'
+        );
+        App\Model\IsolatedConnectionCategoryB::execute(
+            'CREATE TABLE items (
+                id INTEGER PRIMARY KEY AUTOINCREMENT,
+                name TEXT NULL
+            )'
+        );
+
+        App\Model\IsolatedConnectionCategoryA::execute('INSERT INTO `items` (`name`) VALUES (?)', ['from-a']);
+        App\Model\IsolatedConnectionCategoryB::execute('INSERT INTO `items` (`name`) VALUES (?)', ['from-b']);
+
+        /** @var App\Model\IsolatedConnectionCategoryA|null $fromA */
+        $fromA = App\Model\IsolatedConnectionCategoryA::fetchOneWhere('`id` = ?', [1]);
+        /** @var App\Model\IsolatedConnectionCategoryB|null $fromB */
+        $fromB = App\Model\IsolatedConnectionCategoryB::fetchOneWhere('`id` = ?', [1]);
+
+        $this->assertNotNull($fromA);
+        $this->assertNotNull($fromB);
+        $this->assertSame('from-a', $fromA->name);
+        $this->assertSame('from-b', $fromB->name);
+    }
+}