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 38b3dc8

Browse filesBrowse files
fabpotfabpot
committed
bug #32308 [Messenger] DoctrineTransport: ensure auto setup is only done once (bendavies)
This PR was squashed before being merged into the 4.3 branch (closes #32308). Discussion ---------- [Messenger] DoctrineTransport: ensure auto setup is only done once | Q | A | ------------- | --- | Branch? | 4.3 | Bug fix? | yes | New feature? | no <!-- please update src/**/CHANGELOG.md files --> | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? |no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files --> | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Setup is done for every invocation of `get`, `find`, and `findAll`. For `get`, this causes message consumption to be very slow, as slow as 1 message per second on my moderately sized schema, because a schema diff is done in `setup` every `get`. I'm not sure what the desired behaviour is here, but it seems like we should try performing a query, fail once, `setup`, and retry. This will the same approach taken by the PDO Cache. However, we still need auto setup in `get`, as `get` starts a transaction, so the auto setup in `executeQuery` won't be called. Further more, the same problem seems to exist for the AMPQ Transport, but the performance impact should be less there, but i have not tried. Commits ------- 0241402 [Messenger] DoctrineTransport: ensure auto setup is only done once
2 parents 05ab863 + 0241402 commit 38b3dc8
Copy full SHA for 38b3dc8

File tree

2 files changed

+33
-32
lines changed
Filter options

2 files changed

+33
-32
lines changed

‎src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineIntegrationTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Tests/Transport/Doctrine/DoctrineIntegrationTest.php
+18-20Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -21,29 +21,27 @@
2121
*/
2222
class DoctrineIntegrationTest extends TestCase
2323
{
24+
/** @var \Doctrine\DBAL\Connection */
2425
private $driverConnection;
26+
/** @var Connection */
2527
private $connection;
28+
/** @var string */
29+
private $sqliteFile;
2630

27-
/**
28-
* @after
29-
*/
30-
public function cleanup()
31+
protected function setUp(): void
3132
{
32-
@unlink(sys_get_temp_dir().'/symfony.messenger.sqlite');
33+
$this->sqliteFile = sys_get_temp_dir().'/symfony.messenger.sqlite';
34+
$dsn = getenv('MESSENGER_DOCTRINE_DSN') ?: 'sqlite:///'.$this->sqliteFile;
35+
$this->driverConnection = DriverManager::getConnection(['url' => $dsn]);
36+
$this->connection = new Connection([], $this->driverConnection);
3337
}
3438

35-
/**
36-
* @before
37-
*/
38-
public function createConnection()
39+
protected function tearDown(): void
3940
{
40-
$dsn = getenv('MESSENGER_DOCTRINE_DSN') ?: 'sqlite:///'.sys_get_temp_dir().'/symfony.messenger.sqlite';
41-
$this->driverConnection = DriverManager::getConnection(['url' => $dsn]);
42-
$this->connection = new Connection([], $this->driverConnection);
43-
// call send to auto-setup the table
44-
$this->connection->setup();
45-
// ensure the table is clean for tests
46-
$this->driverConnection->exec('DELETE FROM messenger_messages');
41+
$this->driverConnection->close();
42+
if (file_exists($this->sqliteFile)) {
43+
unlink($this->sqliteFile);
44+
}
4745
}
4846

4947
public function testConnectionSendAndGet()
@@ -75,6 +73,7 @@ public function testSendWithDelay()
7573

7674
public function testItRetrieveTheFirstAvailableMessage()
7775
{
76+
$this->connection->setup();
7877
// insert messages
7978
// one currently handled
8079
$this->driverConnection->insert('messenger_messages', [
@@ -108,6 +107,7 @@ public function testItRetrieveTheFirstAvailableMessage()
108107

109108
public function testItCountMessages()
110109
{
110+
$this->connection->setup();
111111
// insert messages
112112
// one currently handled
113113
$this->driverConnection->insert('messenger_messages', [
@@ -148,6 +148,7 @@ public function testItCountMessages()
148148

149149
public function testItRetrieveTheMessageThatIsOlderThanRedeliverTimeout()
150150
{
151+
$this->connection->setup();
151152
$twoHoursAgo = new \DateTime('now');
152153
$twoHoursAgo->modify('-2 hours');
153154
$this->driverConnection->insert('messenger_messages', [
@@ -173,10 +174,7 @@ public function testItRetrieveTheMessageThatIsOlderThanRedeliverTimeout()
173174

174175
public function testTheTransportIsSetupOnGet()
175176
{
176-
// If the table does not exist and we call the get (i.e run messenger:consume) the table must be setup
177-
// so first delete the tables
178-
$this->driverConnection->exec('DROP TABLE messenger_messages');
179-
177+
$this->assertFalse($this->driverConnection->getSchemaManager()->tablesExist('messenger_messages'));
180178
$this->assertNull($this->connection->get());
181179

182180
$this->connection->send('the body', ['my' => 'header']);

‎src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Messenger/Transport/Doctrine/Connection.php
+15-12Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,14 @@ class Connection
5252
private $configuration = [];
5353
private $driverConnection;
5454
private $schemaSynchronizer;
55+
private $autoSetup;
5556

5657
public function __construct(array $configuration, DBALConnection $driverConnection, SchemaSynchronizer $schemaSynchronizer = null)
5758
{
5859
$this->configuration = array_replace_recursive(self::DEFAULT_OPTIONS, $configuration);
5960
$this->driverConnection = $driverConnection;
6061
$this->schemaSynchronizer = $schemaSynchronizer ?? new SingleDatabaseSynchronizer($this->driverConnection);
62+
$this->autoSetup = $this->configuration['auto_setup'];
6163
}
6264

6365
public function getConfiguration(): array
@@ -131,9 +133,7 @@ public function send(string $body, array $headers, int $delay = 0): string
131133

132134
public function get(): ?array
133135
{
134-
if ($this->configuration['auto_setup']) {
135-
$this->setup();
136-
}
136+
get:
137137
$this->driverConnection->beginTransaction();
138138
try {
139139
$query = $this->createAvailableMessagesQueryBuilder()
@@ -170,6 +170,11 @@ public function get(): ?array
170170
} catch (\Throwable $e) {
171171
$this->driverConnection->rollBack();
172172

173+
if ($this->autoSetup && $e instanceof TableNotFoundException) {
174+
$this->setup();
175+
goto get;
176+
}
177+
173178
throw $e;
174179
}
175180
}
@@ -213,6 +218,8 @@ public function setup(): void
213218
} else {
214219
$this->driverConnection->getConfiguration()->setFilterSchemaAssetsExpression($assetFilter);
215220
}
221+
222+
$this->autoSetup = false;
216223
}
217224

218225
public function getMessageCount(): int
@@ -226,10 +233,6 @@ public function getMessageCount(): int
226233

227234
public function findAll(int $limit = null): array
228235
{
229-
if ($this->configuration['auto_setup']) {
230-
$this->setup();
231-
}
232-
233236
$queryBuilder = $this->createAvailableMessagesQueryBuilder();
234237
if (null !== $limit) {
235238
$queryBuilder->setMaxResults($limit);
@@ -244,10 +247,6 @@ public function findAll(int $limit = null): array
244247

245248
public function find($id): ?array
246249
{
247-
if ($this->configuration['auto_setup']) {
248-
$this->setup();
249-
}
250-
251250
$queryBuilder = $this->createQueryBuilder()
252251
->where('m.id = ?');
253252

@@ -288,8 +287,12 @@ private function executeQuery(string $sql, array $parameters = [])
288287
$stmt = $this->driverConnection->prepare($sql);
289288
$stmt->execute($parameters);
290289
} catch (TableNotFoundException $e) {
290+
if ($this->driverConnection->isTransactionActive()) {
291+
throw $e;
292+
}
293+
291294
// create table
292-
if (!$this->driverConnection->isTransactionActive() && $this->configuration['auto_setup']) {
295+
if ($this->autoSetup) {
293296
$this->setup();
294297
}
295298
// statement not prepared ? SQLite throw on exception on prepare if the table does not exist

0 commit comments

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