Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 48be4b3

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #52459 [Cache][HttpFoundation][Lock] Fix PDO store not creating table + add tests (HypeMC)
This PR was merged into the 5.4 branch. Discussion ---------- [Cache][HttpFoundation][Lock] Fix PDO store not creating table + add tests | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT Unlike e.g. SQLite, PostgreSQL doesn't throw an exception on `$conn->prepare()` if the table is missing, it instead throws it on `$stmt->execute()`, so the table never gets created. The table used to get created, but the behavior was broken with #43332. Commits ------- cb5d832 [Cache][Lock] Fix PDO store not creating table + add tests
2 parents b5376c7 + cb5d832 commit 48be4b3
Copy full SHA for 48be4b3

File tree

9 files changed

+177
-42
lines changed
Filter options

9 files changed

+177
-42
lines changed

‎src/Symfony/Component/Cache/Adapter/DoctrineDbalAdapter.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Adapter/DoctrineDbalAdapter.php
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -420,7 +420,8 @@ private function getServerVersion(): string
420420
return $this->serverVersion;
421421
}
422422

423-
$conn = $this->conn->getWrappedConnection();
423+
// The condition should be removed once support for DBAL <3.3 is dropped
424+
$conn = method_exists($this->conn, 'getNativeConnection') ? $this->conn->getNativeConnection() : $this->conn->getWrappedConnection();
424425
if ($conn instanceof ServerInfoAwareConnection) {
425426
return $this->serverVersion = $conn->getServerVersion();
426427
}

‎src/Symfony/Component/Cache/Adapter/PdoAdapter.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Adapter/PdoAdapter.php
+19-2Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ protected function doSave(array $values, int $lifetime)
507507
try {
508508
$stmt = $conn->prepare($sql);
509509
} catch (\PDOException $e) {
510-
if (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true)) {
510+
if ($this->isTableMissing($e) && (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true))) {
511511
$this->createTable();
512512
}
513513
$stmt = $conn->prepare($sql);
@@ -542,7 +542,7 @@ protected function doSave(array $values, int $lifetime)
542542
try {
543543
$stmt->execute();
544544
} catch (\PDOException $e) {
545-
if (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true)) {
545+
if ($this->isTableMissing($e) && (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true))) {
546546
$this->createTable();
547547
}
548548
$stmt->execute();
@@ -596,4 +596,21 @@ private function getServerVersion(): string
596596

597597
return $this->serverVersion;
598598
}
599+
600+
private function isTableMissing(\PDOException $exception): bool
601+
{
602+
$driver = $this->driver;
603+
$code = $exception->getCode();
604+
605+
switch (true) {
606+
case 'pgsql' === $driver && '42P01' === $code:
607+
case 'sqlite' === $driver && str_contains($exception->getMessage(), 'no such table:'):
608+
case 'oci' === $driver && 942 === $code:
609+
case 'sqlsrv' === $driver && 208 === $code:
610+
case 'mysql' === $driver && 1146 === $code:
611+
return true;
612+
default:
613+
return false;
614+
}
615+
}
599616
}

‎src/Symfony/Component/Cache/Tests/Adapter/DoctrineDbalAdapterTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Tests/Adapter/DoctrineDbalAdapterTest.php
+31-12Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,13 @@
1818
use Doctrine\DBAL\DriverManager;
1919
use Doctrine\DBAL\Schema\DefaultSchemaManagerFactory;
2020
use Doctrine\DBAL\Schema\Schema;
21-
use PHPUnit\Framework\SkippedTestSuiteError;
2221
use Psr\Cache\CacheItemPoolInterface;
2322
use Symfony\Component\Cache\Adapter\DoctrineDbalAdapter;
2423
use Symfony\Component\Cache\Tests\Fixtures\DriverWrapper;
2524

2625
/**
26+
* @requires extension pdo_sqlite
27+
*
2728
* @group time-sensitive
2829
*/
2930
class DoctrineDbalAdapterTest extends AdapterTestCase
@@ -32,10 +33,6 @@ class DoctrineDbalAdapterTest extends AdapterTestCase
3233

3334
public static function setUpBeforeClass(): void
3435
{
35-
if (!\extension_loaded('pdo_sqlite')) {
36-
throw new SkippedTestSuiteError('Extension pdo_sqlite required.');
37-
}
38-
3936
self::$dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
4037
}
4138

@@ -107,13 +104,12 @@ public function testConfigureSchemaTableExists()
107104
}
108105

109106
/**
110-
* @dataProvider provideDsn
107+
* @dataProvider provideDsnWithSQLite
111108
*/
112-
public function testDsn(string $dsn, string $file = null)
109+
public function testDsnWithSQLite(string $dsn, string $file = null)
113110
{
114111
try {
115112
$pool = new DoctrineDbalAdapter($dsn);
116-
$pool->createTable();
117113

118114
$item = $pool->getItem('key');
119115
$item->set('value');
@@ -125,12 +121,35 @@ public function testDsn(string $dsn, string $file = null)
125121
}
126122
}
127123

128-
public static function provideDsn()
124+
public static function provideDsnWithSQLite()
129125
{
130126
$dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
131-
yield ['sqlite://localhost/'.$dbFile.'1', $dbFile.'1'];
132-
yield ['sqlite3:///'.$dbFile.'3', $dbFile.'3'];
133-
yield ['sqlite://localhost/:memory:'];
127+
yield 'SQLite file' => ['sqlite://localhost/'.$dbFile.'1', $dbFile.'1'];
128+
yield 'SQLite3 file' => ['sqlite3:///'.$dbFile.'3', $dbFile.'3'];
129+
yield 'SQLite in memory' => ['sqlite://localhost/:memory:'];
130+
}
131+
132+
/**
133+
* @requires extension pdo_pgsql
134+
*
135+
* @group integration
136+
*/
137+
public function testDsnWithPostgreSQL()
138+
{
139+
if (!$host = getenv('POSTGRES_HOST')) {
140+
$this->markTestSkipped('Missing POSTGRES_HOST env variable');
141+
}
142+
143+
try {
144+
$pool = new DoctrineDbalAdapter('pgsql://postgres:password@'.$host);
145+
146+
$item = $pool->getItem('key');
147+
$item->set('value');
148+
$this->assertTrue($pool->save($item));
149+
} finally {
150+
$pdo = new \PDO('pgsql:host='.$host.';user=postgres;password=password');
151+
$pdo->exec('DROP TABLE IF EXISTS cache_items');
152+
}
134153
}
135154

136155
protected function isPruned(DoctrineDbalAdapter $cache, string $name): bool

‎src/Symfony/Component/Cache/Tests/Adapter/PdoAdapterTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Cache/Tests/Adapter/PdoAdapterTest.php
+32-11Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@
1111

1212
namespace Symfony\Component\Cache\Tests\Adapter;
1313

14-
use PHPUnit\Framework\SkippedTestSuiteError;
1514
use Psr\Cache\CacheItemPoolInterface;
1615
use Symfony\Component\Cache\Adapter\PdoAdapter;
1716

1817
/**
18+
* @requires extension pdo_sqlite
19+
*
1920
* @group time-sensitive
2021
*/
2122
class PdoAdapterTest extends AdapterTestCase
@@ -24,10 +25,6 @@ class PdoAdapterTest extends AdapterTestCase
2425

2526
public static function setUpBeforeClass(): void
2627
{
27-
if (!\extension_loaded('pdo_sqlite')) {
28-
throw new SkippedTestSuiteError('Extension pdo_sqlite required.');
29-
}
30-
3128
self::$dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
3229

3330
$pool = new PdoAdapter('sqlite:'.self::$dbFile);
@@ -71,13 +68,12 @@ public function testCleanupExpiredItems()
7168
}
7269

7370
/**
74-
* @dataProvider provideDsn
71+
* @dataProvider provideDsnSQLite
7572
*/
76-
public function testDsn(string $dsn, string $file = null)
73+
public function testDsnWithSQLite(string $dsn, string $file = null)
7774
{
7875
try {
7976
$pool = new PdoAdapter($dsn);
80-
$pool->createTable();
8177

8278
$item = $pool->getItem('key');
8379
$item->set('value');
@@ -89,11 +85,36 @@ public function testDsn(string $dsn, string $file = null)
8985
}
9086
}
9187

92-
public static function provideDsn()
88+
public static function provideDsnSQLite()
9389
{
9490
$dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
95-
yield ['sqlite:'.$dbFile.'2', $dbFile.'2'];
96-
yield ['sqlite::memory:'];
91+
yield 'SQLite file' => ['sqlite:'.$dbFile.'2', $dbFile.'2'];
92+
yield 'SQLite in memory' => ['sqlite::memory:'];
93+
}
94+
95+
/**
96+
* @requires extension pdo_pgsql
97+
*
98+
* @group integration
99+
*/
100+
public function testDsnWithPostgreSQL()
101+
{
102+
if (!$host = getenv('POSTGRES_HOST')) {
103+
$this->markTestSkipped('Missing POSTGRES_HOST env variable');
104+
}
105+
106+
$dsn = 'pgsql:host='.$host.';user=postgres;password=password';
107+
108+
try {
109+
$pool = new PdoAdapter($dsn);
110+
111+
$item = $pool->getItem('key');
112+
$item->set('value');
113+
$this->assertTrue($pool->save($item));
114+
} finally {
115+
$pdo = new \PDO($dsn);
116+
$pdo->exec('DROP TABLE IF EXISTS cache_items');
117+
}
97118
}
98119

99120
protected function isPruned(PdoAdapter $cache, string $name): bool

‎src/Symfony/Component/HttpFoundation/Session/Storage/Handler/SessionHandlerFactory.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpFoundation/Session/Storage/Handler/SessionHandlerFactory.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public static function createHandler($connection): AbstractSessionHandler
8282
}
8383

8484
$connection = DriverManager::getConnection($params, $config);
85+
// The condition should be removed once support for DBAL <3.3 is dropped
8586
$connection = method_exists($connection, 'getNativeConnection') ? $connection->getNativeConnection() : $connection->getWrappedConnection();
8687
// no break;
8788

‎src/Symfony/Component/Lock/Store/PdoStore.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Lock/Store/PdoStore.php
+30-3Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ public function save(Key $key)
115115
try {
116116
$stmt = $conn->prepare($sql);
117117
} catch (\PDOException $e) {
118-
if (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true)) {
118+
if ($this->isTableMissing($e) && (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true))) {
119119
$this->createTable();
120120
}
121121
$stmt = $conn->prepare($sql);
@@ -127,8 +127,18 @@ public function save(Key $key)
127127
try {
128128
$stmt->execute();
129129
} catch (\PDOException $e) {
130-
// the lock is already acquired. It could be us. Let's try to put off.
131-
$this->putOffExpiration($key, $this->initialTtl);
130+
if ($this->isTableMissing($e) && (!$conn->inTransaction() || \in_array($this->driver, ['pgsql', 'sqlite', 'sqlsrv'], true))) {
131+
$this->createTable();
132+
133+
try {
134+
$stmt->execute();
135+
} catch (\PDOException $e) {
136+
$this->putOffExpiration($key, $this->initialTtl);
137+
}
138+
} else {
139+
// the lock is already acquired. It could be us. Let's try to put off.
140+
$this->putOffExpiration($key, $this->initialTtl);
141+
}
132142
}
133143

134144
$this->randomlyPrune();
@@ -316,4 +326,21 @@ private function getCurrentTimestampStatement(): string
316326
return (string) time();
317327
}
318328
}
329+
330+
private function isTableMissing(\PDOException $exception): bool
331+
{
332+
$driver = $this->getDriver();
333+
$code = $exception->getCode();
334+
335+
switch (true) {
336+
case 'pgsql' === $driver && '42P01' === $code:
337+
case 'sqlite' === $driver && str_contains($exception->getMessage(), 'no such table:'):
338+
case 'oci' === $driver && 942 === $code:
339+
case 'sqlsrv' === $driver && 208 === $code:
340+
case 'mysql' === $driver && 1146 === $code:
341+
return true;
342+
default:
343+
return false;
344+
}
345+
}
319346
}

‎src/Symfony/Component/Lock/Tests/Store/DoctrineDbalStoreTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Lock/Tests/Store/DoctrineDbalStoreTest.php
+30-6Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ public function testAbortAfterExpiration()
7979
}
8080

8181
/**
82-
* @dataProvider provideDsn
82+
* @dataProvider provideDsnWithSQLite
8383
*/
84-
public function testDsn(string $dsn, string $file = null)
84+
public function testDsnWithSQLite(string $dsn, string $file = null)
8585
{
8686
$key = new Key(uniqid(__METHOD__, true));
8787

@@ -97,12 +97,36 @@ public function testDsn(string $dsn, string $file = null)
9797
}
9898
}
9999

100-
public static function provideDsn()
100+
public static function provideDsnWithSQLite()
101101
{
102102
$dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
103-
yield ['sqlite://localhost/'.$dbFile.'1', $dbFile.'1'];
104-
yield ['sqlite3:///'.$dbFile.'3', $dbFile.'3'];
105-
yield ['sqlite://localhost/:memory:'];
103+
yield 'SQLite file' => ['sqlite://localhost/'.$dbFile.'1', $dbFile.'1'];
104+
yield 'SQLite3 file' => ['sqlite3:///'.$dbFile.'3', $dbFile.'3'];
105+
yield 'SQLite in memory' => ['sqlite://localhost/:memory:'];
106+
}
107+
108+
/**
109+
* @requires extension pdo_pgsql
110+
*
111+
* @group integration
112+
*/
113+
public function testDsnWithPostgreSQL()
114+
{
115+
if (!$host = getenv('POSTGRES_HOST')) {
116+
$this->markTestSkipped('Missing POSTGRES_HOST env variable');
117+
}
118+
119+
$key = new Key(uniqid(__METHOD__, true));
120+
121+
try {
122+
$store = new DoctrineDbalStore('pgsql://postgres:password@'.$host);
123+
124+
$store->save($key);
125+
$this->assertTrue($store->exists($key));
126+
} finally {
127+
$pdo = new \PDO('pgsql:host='.$host.';user=postgres;password=password');
128+
$pdo->exec('DROP TABLE IF EXISTS lock_keys');
129+
}
106130
}
107131

108132
/**

‎src/Symfony/Component/Lock/Tests/Store/PdoStoreTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Lock/Tests/Store/PdoStoreTest.php
+31-7Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@
2020
* @author Jérémy Derussé <jeremy@derusse.com>
2121
*
2222
* @requires extension pdo_sqlite
23-
*
24-
* @group integration
2523
*/
2624
class PdoStoreTest extends AbstractStoreTestCase
2725
{
@@ -78,9 +76,9 @@ public function testInvalidTtlConstruct()
7876
}
7977

8078
/**
81-
* @dataProvider provideDsn
79+
* @dataProvider provideDsnWithSQLite
8280
*/
83-
public function testDsn(string $dsn, string $file = null)
81+
public function testDsnWithSQLite(string $dsn, string $file = null)
8482
{
8583
$key = new Key(uniqid(__METHOD__, true));
8684

@@ -96,10 +94,36 @@ public function testDsn(string $dsn, string $file = null)
9694
}
9795
}
9896

99-
public static function provideDsn()
97+
public static function provideDsnWithSQLite()
10098
{
10199
$dbFile = tempnam(sys_get_temp_dir(), 'sf_sqlite_cache');
102-
yield ['sqlite:'.$dbFile.'2', $dbFile.'2'];
103-
yield ['sqlite::memory:'];
100+
yield 'SQLite file' => ['sqlite:'.$dbFile.'2', $dbFile.'2'];
101+
yield 'SQLite in memory' => ['sqlite::memory:'];
102+
}
103+
104+
/**
105+
* @requires extension pdo_pgsql
106+
*
107+
* @group integration
108+
*/
109+
public function testDsnWithPostgreSQL()
110+
{
111+
if (!$host = getenv('POSTGRES_HOST')) {
112+
$this->markTestSkipped('Missing POSTGRES_HOST env variable');
113+
}
114+
115+
$key = new Key(uniqid(__METHOD__, true));
116+
117+
$dsn = 'pgsql:host='.$host.';user=postgres;password=password';
118+
119+
try {
120+
$store = new PdoStore($dsn);
121+
122+
$store->save($key);
123+
$this->assertTrue($store->exists($key));
124+
} finally {
125+
$pdo = new \PDO($dsn);
126+
$pdo->exec('DROP TABLE IF EXISTS lock_keys');
127+
}
104128
}
105129
}

‎src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/PostgreSqlConnection.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Bridge/Doctrine/Transport/PostgreSqlConnection.php
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public function get(): ?array
6464
// https://www.postgresql.org/docs/current/sql-syntax-lexical.html#SQL-SYNTAX-IDENTIFIERS
6565
$this->executeStatement(sprintf('LISTEN "%s"', $this->configuration['table_name']));
6666

67+
// The condition should be removed once support for DBAL <3.3 is dropped
6768
if (method_exists($this->driverConnection, 'getNativeConnection')) {
6869
$wrappedConnection = $this->driverConnection->getNativeConnection();
6970
} else {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.