8000 feature #36655 Automatically provide Messenger Doctrine schema to "di… · symfony/cache@9cb7985 · GitHub
[go: up one dir, main page]
Skip to content

Commit 9cb7985

Browse files
fabpotfabpot
committed
feature #36655 Automatically provide Messenger Doctrine schema to "diff" (weaverryan)
This PR was squashed before being merged into the 5.1-dev branch. Discussion ---------- Automatically provide Messenger Doctrine schema to "diff" | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Alternative to #36629 | License | MIT | Doc PR | TODO - WILL be needed This follows this conversation: symfony/symfony#36629 (comment) - it automatically adds SQL to Doctrine's migration/diff system when features are added the require a database table: The new feature works for: ### A) Messenger Doctrine transport **FULL support** Works perfectly: configure a doctrine transport and run `make:migration` **Note**: There is no current way to disable this. So if you have `auto_setup` ON and you run `make:migration` before trying Messenger, it will generate the table SQL. Adding a flag to disable it might be very complicated, because we need to know (in DoctrineBundle, at compile time) whether or not this feature is enabled/disabled so that we can decide *not* to add `messenger_messages` to the `schema_filter`. ### B) `PdoAdapter` from Cache **FULL support** Works perfectly: configure a doctrine transport and run `make:migration` ### C) `PdoStore` from Lock **PARTIAL support** I added `PdoStore::configureSchema()` but did NOT add a listener. While `PdoStore` *does* accept a DBAL `Connection`, I don't think it's possible via the `framework.lock` config to create a `PdoStore` that is passed a `Connection`. In other words: if we added a listener that called `PdoStore::configureSchema` if the user configured a `pdo` lock, that service will *never* have a `Connection` object... so it's kind of worthless. **NEED**: A proper way to inject a DBAL `Connection` into `PdoStore` via `framework.lock` config. ### D) `PdoSessionHandler` **NO support** This class doesn't accept a DBAL `Connection` object. And so, we can't reliably create a listener to add the schema because (if there are multiple connections) we wouldn't know which Connection to use. We could compare (`===`) the `PDO` instance inside `PdoSessionHandler` to the wrapped `PDO` connection in Doctrine. That would only work if the user has configured their `PdoSessionHandler` to re-use the Doctrine PDO connection. The `PdoSessionHandler` *already* has a `createTable()` method on it to help with manual migration. But... it's not easy to call from a migration because you would need to fetch the `PdoSessionHandler` service from the container. Adding something **NEED**: Either: A) A way for `PdoSessionHandler` to use a DBAL Connection or B) We try to hack this feature by comparing the `PDO` instances in the event subscriber or C) We add an easier way to access the `createTable()` method from inside a migration. TODOs * [X] Determine service injection XML needed for getting all PdoAdapter pools * [ ] Finish DoctrineBundle PR: doctrine/DoctrineBundle#1163 Commits ------- 2dd9c3c3c8 Automatically provide Messenger Doctrine schema to "diff"
2 parents f438f8e + a5062f3 commit 9cb7985

File tree

2 files changed

+88
-17
lines changed

2 files changed

+88
-17
lines changed

‎Adapter/PdoAdapter.php

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -115,24 +115,8 @@ public function createTable()
115115
$conn = $this->getConnection();
116116

117117
if ($conn instanceof Connection) {
118-
$types = [
119-
'mysql' => 'binary',
120-
'sqlite' => 'text',
121-
'pgsql' => 'string',
122-
'oci' => 'string',
123-
'sqlsrv' => 'string',
124-
];
125-
if (!isset($types[$this->driver])) {
126-
throw new \DomainException(sprintf('Creating the cache table is currently not implemented for PDO driver "%s".', $this->driver));
127-
}
128-
129118
$schema = new Schema();
130-
$table = $schema->createTable($this->table);
131-
$table->addColumn($this->idCol, $types[$this->driver], ['length' => 255]);
132-
$table->addColumn($this->dataCol, 'blob', ['length' => 16777215]);
133-
$table->addColumn($this->lifetimeCol, 'integer', ['unsigned' => true, 'notnull' => false]);
134-
$table->addColumn($this->timeCol, 'integer', ['unsigned' => true]);
135-
$table->setPrimaryKey([$this->idCol]);
119+
$this->addTableToSchema($schema);
136120

137121
foreach ($schema->toSql($conn->getDatabasePlatform()) as $sql) {
138122
$conn->exec($sql);
@@ -169,6 +153,23 @@ public function createTable()
169153
$conn->exec($sql);
170154
}
171155

156+
/**
157+
* Adds the Table to the Schema if the adapter uses this Connection.
158+
*/
159+
public function configureSchema(Schema $schema, Connection $forConnection): void
160+
{
161+
// only update the schema for this connection
162+
if ($forConnection !== $this->getConnection()) {
163+
return;
164+
}
165+
166+
if ($schema->hasTable($this->table)) {
167+
return;
168+
}
169+
170+
$this->addTableToSchema($schema);
171+
}
172+
172173
/**
173174
* {@inheritdoc}
174175
*/
@@ -467,4 +468,25 @@ private function getServerVersion(): string
467468

468469
return $this->serverVersion;
469470
}
471+
472+
private function addTableToSchema(Schema $schema): void
473+
{
474+
$types = [
475+
'mysql' => 'binary',
476+
'sqlite' => 'text',
477+
'pgsql' => 'string',
478+
'oci' => 'string',
479+
'sqlsrv' => 'string',
480+
];
481+
if (!isset($types[$this->driver])) {
482+
throw new \DomainException(sprintf('Creating the cache table is currently not implemented for PDO driver "%s".', $this->driver));
483+
}
484+
485+
$table = $schema->createTable($this->table);
486+
$table->addColumn($this->idCol, $types[$this->driver], ['length' => 255]);
487+
$table->addColumn($this->dataCol, 'blob', ['length' => 16777215]);
488+
$table->addColumn($this->lifetimeCol, 'integer', ['unsigned' => true, 'notnull' => false]);
489+
$table->addColumn($this->timeCol, 'integer', ['unsigned' => true]);
490+
$table->setPrimaryKey([$this->idCol]);
491+
}
470492
}

‎Tests/Adapter/PdoDbalAdapterTest.php

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@
1111

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

14+
use Doctrine\DBAL\Connection;
15+
use Doctrine\DBAL\Driver;
1416
use Doctrine\DBAL\DriverManager;
17+
use Doctrine\DBAL\Schema\Schema;
1518
use Psr\Cache\CacheItemPoolInterface;
1619
use Symfony\Component\Cache\Adapter\PdoAdapter;
1720
use Symfony\Component\Cache\Tests\Traits\PdoPruneableTrait;
@@ -43,4 +46,50 @@ public function createCachePool(int $defaultLifetime = 0): CacheItemPoolInterfac
4346
{
4447
return new PdoAdapter(DriverManager::getConnection(['driver' => 'pdo_sqlite', 'path' => self::$dbFile]), '', $defaultLifetime);
4548
}
49+
50+
public function testConfigureSchema()
51+
{
52+
$connection = DriverManager::getConnection(['driver' => 'pdo_sqlite', 'path' => self::$dbFile]);
53+
$schema = new Schema();
54+
55+
$adapter = new PdoAdapter($connection);
56+
$adapter->configureSchema($schema, $connection);
57+
$this->assertTrue($schema->hasTable('cache_items'));
58+
}
59+
60+
public function testConfigureSchemaDifferentDbalConnection()
61+
{
62+
$otherConnection = $this->createConnectionMock();
63+
$schema = new Schema();
64+
65+
$adapter = $this->createCachePool();
66+
$adapter->configureSchema($schema, $otherConnection);
67+
$this->assertFalse($schema->hasTable('cache_items'));
68+
}
69+
70+
public function testConfigureSchemaTableExists()
71+
{
72+
$connection = DriverManager::getConnection(['driver' => 'pdo_sqlite', 'path' => self::$dbFile]);
73+
$schema = new Schema();
74+
$schema->createTable('cache_items');
75+
76+
$adapter = new PdoAdapter($connection);
77+
$adapter->configureSchema($schema, $connection);
78+
$table = $schema->getTable('cache_items');
79+
$this->assertEmpty($table->getColumns(), 'The table was not overwritten');
80+
}
81+
82+
private function createConnectionMock()
83+
{
84+
$connection = $this->createMock(Connection::class);
85+
$driver = $this->createMock(Driver::class);
86+
$driver->expects($this->any())
87+
->method('getName')
88+
->willReturn('pdo_mysql');
89+
$connection->expects($this->any())
90+
->method('getDriver')
91+
->willReturn($driver);
92+
93+
return $connection;
94+
}
4695
}

0 commit comments

Comments
 (0)
0