[Cache][Lock] Fix PDO store not creating table + add tests

symfony · Nov 20, 2023 · a6d7d82 · a6d7d82
1 parent 23146b4
commit a6d7d82
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 16 deletions.
diff --git a/Store/PdoStore.php b/Store/PdoStore.php
@@ -115,7 +115,7 @@ public function save(Key $key)
         try {
             $stmt = $conn->prepare($sql);
         } catch (\PDOException $e) {
-            if (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true)) {
+            if ($this->isTableMissing($e) && (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true))) {
                 $this->createTable();
             }
             $stmt = $conn->prepare($sql);
@@ -127,8 +127,18 @@ public function save(Key $key)
         try {
             $stmt->execute();
         } catch (\PDOException $e) {
-            // the lock is already acquired. It could be us. Let's try to put off.
-            $this->putOffExpiration($key, $this->initialTtl);
+            if ($this->isTableMissing($e) && (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true))) {
+                $this->createTable();
+
+                try {
+                    $stmt->execute();
+                } catch (\PDOException $e) {
+                    $this->putOffExpiration($key, $this->initialTtl);
+                }
+            } else {
+                // the lock is already acquired. It could be us. Let's try to put off.
+                $this->putOffExpiration($key, $this->initialTtl);
+            }
         }
 
         $this->randomlyPrune();
@@ -316,4 +326,21 @@ private function getCurrentTimestampStatement(): string
                 return (string) time();
         }
     }
+
+    private function isTableMissing(\PDOException $exception): bool
+    {
+        $driver = $this->getDriver();
+        $code = $exception->getCode();
+
+        switch (true) {
+            case 'pgsql' === $driver && '42P01' === $code:
+            case 'sqlite' === $driver && str_contains($exception->getMessage(), 'no such table:'):
+            case 'oci' === $driver && 942 === $code:
+            case 'sqlsrv' === $driver && 208 === $code:
+            case 'mysql' === $driver && 1146 === $code:
+                return true;
+            default:
+                return false;
+        }
+    }
 }
diff --git a/Tests/Store/DoctrineDbalStoreTest.php b/Tests/Store/DoctrineDbalStoreTest.php
@@ -79,9 +79,9 @@ public function testAbortAfterExpiration()
     }
 
     /**
-     * @dataProvider provideDsn
+     * @dataProvider provideDsnWithSQLite
      */
-    public function testDsn(string $dsn, string $file = null)
+    public function testDsnWithSQLite(string $dsn, string $file = null)
     {
         $key = new Key(uniqid(__METHOD__, true));
 
@@ -97,12 +97,36 @@ public function testDsn(string $dsn, string $file = null)
         }
     }
 
-    public static function provideDsn()
+    public static function provideDsnWithSQLite()
     {
         $dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
-        yield ['sqlite://localhost/'.$dbFile.'1', $dbFile.'1'];
-        yield ['sqlite3:///'.$dbFile.'3', $dbFile.'3'];
-        yield ['sqlite://localhost/:memory:'];
+        yield 'SQLite file' => ['sqlite://localhost/'.$dbFile.'1', $dbFile.'1'];
+        yield 'SQLite3 file' => ['sqlite3:///'.$dbFile.'3', $dbFile.'3'];
+        yield 'SQLite in memory' => ['sqlite://localhost/:memory:'];
+    }
+
+    /**
+     * @requires extension pdo_pgsql
+     *
+     * @group integration
+     */
+    public function testDsnWithPostgreSQL()
+    {
+        if (!$host = getenv('POSTGRES_HOST')) {
+            $this->markTestSkipped('Missing POSTGRES_HOST env variable');
+        }
+
+        $key = new Key(uniqid(__METHOD__, true));
+
+        try {
+            $store = new DoctrineDbalStore('pgsql://postgres:password@'.$host);
+
+            $store->save($key);
+            $this->assertTrue($store->exists($key));
+        } finally {
+            $pdo = new \PDO('pgsql:host='.$host.';user=postgres;password=password');
+            $pdo->exec('DROP TABLE IF EXISTS lock_keys');
+        }
     }
 
     /**

diff --git a/Tests/Store/PdoStoreTest.php b/Tests/Store/PdoStoreTest.php
@@ -20,8 +20,6 @@
  * @author Jérémy Derussé <jeremy@derusse.com>
  *
  * @requires extension pdo_sqlite
- *
- * @group integration
  */
 class PdoStoreTest extends AbstractStoreTestCase
 {
@@ -78,9 +76,9 @@ public function testInvalidTtlConstruct()
     }
 
     /**
-     * @dataProvider provideDsn
+     * @dataProvider provideDsnWithSQLite
      */
-    public function testDsn(string $dsn, string $file = null)
+    public function testDsnWithSQLite(string $dsn, string $file = null)
     {
         $key = new Key(uniqid(__METHOD__, true));
 
@@ -96,10 +94,36 @@ public function testDsn(string $dsn, string $file = null)
         }
     }
 
-    public static function provideDsn()
+    public static function provideDsnWithSQLite()
     {
         $dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
-        yield ['sqlite:'.$dbFile.'2', $dbFile.'2'];
-        yield ['sqlite::memory:'];
+        yield 'SQLite file' => ['sqlite:'.$dbFile.'2', $dbFile.'2'];
+        yield 'SQLite in memory' => ['sqlite::memory:'];
+    }
+
+    /**
+     * @requires extension pdo_pgsql
+     *
+     * @group integration
+     */
+    public function testDsnWithPostgreSQL()
+    {
+        if (!$host = getenv('POSTGRES_HOST')) {
+            $this->markTestSkipped('Missing POSTGRES_HOST env variable');
+        }
+
+        $key = new Key(uniqid(__METHOD__, true));
+
+        $dsn = 'pgsql:host='.$host.';user=postgres;password=password';
+
+        try {
+            $store = new PdoStore($dsn);
+
+            $store->save($key);
+            $this->assertTrue($store->exists($key));
+        } finally {
+            $pdo = new \PDO($dsn);
+            $pdo->exec('DROP TABLE IF EXISTS lock_keys');
+        }
     }
 }