Skip to content

Commit

Permalink
feature #36655 Automatically provide Messenger Doctrine schema to "di…
Browse files Browse the repository at this point in the history
…ff" (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: #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
-------

2dd9c3c Automatically provide Messenger Doctrine schema to "diff"
  • Loading branch information
fabpot committed May 5, 2020
2 parents 3d30ff7 + 2dd9c3c commit b9d4149
Show file tree
Hide file tree
Showing 16 changed files with 566 additions and 27 deletions.
@@ -0,0 +1,96 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\SchemaListener;

use Doctrine\Common\EventSubscriber;
use Doctrine\DBAL\Event\SchemaCreateTableEventArgs;
use Doctrine\DBAL\Events;
use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;
use Doctrine\ORM\Tools\ToolEvents;
use Symfony\Component\Messenger\Bridge\Doctrine\Transport\DoctrineTransport;
use Symfony\Component\Messenger\Transport\TransportInterface;

/**
* Automatically adds any required database tables to the Doctrine Schema.
*
* @author Ryan Weaver <ryan@symfonycasts.com>
*/
final class MessengerTransportDoctrineSchemaSubscriber implements EventSubscriber
{
private const PROCESSING_TABLE_FLAG = self::class.':processing';

private $transports;

/**
* @param iterable|TransportInterface[] $transports
*/
public function __construct(iterable $transports)
{
$this->transports = $transports;
}

public function postGenerateSchema(GenerateSchemaEventArgs $event): void
{
$dbalConnection = $event->getEntityManager()->getConnection();
foreach ($this->transports as $transport) {
if (!$transport instanceof DoctrineTransport) {
continue;
}

$transport->configureSchema($event->getSchema(), $dbalConnection);
}
}

public function onSchemaCreateTable(SchemaCreateTableEventArgs $event): void
{
$table = $event->getTable();

// if this method triggers a nested create table below, allow Doctrine to work like normal
if ($table->hasOption(self::PROCESSING_TABLE_FLAG)) {
return;
}

foreach ($this->transports as $transport) {
if (!$transport instanceof DoctrineTransport) {
continue;
}

$extraSql = $transport->getExtraSetupSqlForTable($table);
if (null === $extraSql) {
continue;
}

// avoid this same listener from creating a loop on this table
$table->addOption(self::PROCESSING_TABLE_FLAG, true);
$createTableSql = $event->getPlatform()->getCreateTableSQL($table);

/*
* Add all the SQL needed to create the table and tell Doctrine
* to "preventDefault" so that only our SQL is used. This is
* the only way to inject some extra SQL.
*/
$event->addSql($createTableSql);
$event->addSql($extraSql);
$event->preventDefault();

return;
}
}

public function getSubscribedEvents(): array
{
return [
ToolEvents::postGenerateSchema,
Events::onSchemaCreateTable,
];
}
}
@@ -0,0 +1,50 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\SchemaListener;

use Doctrine\Common\EventSubscriber;
use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;
use Doctrine\ORM\Tools\ToolEvents;
use Symfony\Component\Cache\Adapter\PdoAdapter;

/**
* Automatically adds the cache table needed for the PdoAdapter.
*
* @author Ryan Weaver <ryan@symfonycasts.com>
*/
final class PdoCacheAdapterDoctrineSchemaSubscriber implements EventSubscriber
{
private $pdoAdapters;

/**
* @param iterable|PdoAdapter[] $pdoAdapters
*/
public function __construct(iterable $pdoAdapters)
{
$this->pdoAdapters = $pdoAdapters;
}

public function postGenerateSchema(GenerateSchemaEventArgs $event): void
{
$dbalConnection = $event->getEntityManager()->getConnection();
foreach ($this->pdoAdapters as $pdoAdapter) {
$pdoAdapter->configureSchema($event->getSchema(), $dbalConnection);
}
}

public function getSubscribedEvents(): array
{
return [
ToolEvents::postGenerateSchema,
];
}
}
@@ -0,0 +1,99 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Tests\SchemaListener;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Event\SchemaCreateTableEventArgs;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\DBAL\Schema\Table;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\SchemaListener\MessengerTransportDoctrineSchemaSubscriber;
use Symfony\Component\Messenger\Bridge\Doctrine\Transport\DoctrineTransport;
use Symfony\Component\Messenger\Transport\TransportInterface;

class MessengerTransportDoctrineSchemaSubscriberTest extends TestCase
{
public function testPostGenerateSchema()
{
$schema = new Schema();
$dbalConnection = $this->createMock(Connection::class);
$entityManager = $this->createMock(EntityManagerInterface::class);
$entityManager->expects($this->once())
->method('getConnection')
->willReturn($dbalConnection);
$event = new GenerateSchemaEventArgs($entityManager, $schema);

$doctrineTransport = $this->createMock(DoctrineTransport::class);
$doctrineTransport->expects($this->once())
->method('configureSchema')
->with($schema, $dbalConnection);
$otherTransport = $this->createMock(TransportInterface::class);
$otherTransport->expects($this->never())
->method($this->anything());

$subscriber = new MessengerTransportDoctrineSchemaSubscriber([$doctrineTransport, $otherTransport]);
$subscriber->postGenerateSchema($event);
}

public function testOnSchemaCreateTable()
{
$platform = $this->createMock(AbstractPlatform::class);
$table = new Table('queue_table');
$event = new SchemaCreateTableEventArgs($table, [], [], $platform);

$otherTransport = $this->createMock(TransportInterface::class);
$otherTransport->expects($this->never())
->method($this->anything());

$doctrineTransport = $this->createMock(DoctrineTransport::class);
$doctrineTransport->expects($this->once())
->method('getExtraSetupSqlForTable')
->with($table)
->willReturn('ALTER TABLE pizza ADD COLUMN extra_cheese boolean');

// we use the platform to generate the full create table sql
$platform->expects($this->once())
->method('getCreateTableSQL')
->with($table)
->willReturn('CREATE TABLE pizza (id integer NOT NULL)');

$subscriber = new MessengerTransportDoctrineSchemaSubscriber([$otherTransport, $doctrineTransport]);
$subscriber->onSchemaCreateTable($event);
$this->assertTrue($event->isDefaultPrevented());
$this->assertSame([
'CREATE TABLE pizza (id integer NOT NULL)',
'ALTER TABLE pizza ADD COLUMN extra_cheese boolean',
], $event->getSql());
}

public function testOnSchemaCreateTableNoExtraSql()
{
$platform = $this->createMock(AbstractPlatform::class);
$table = new Table('queue_table');
$event = new SchemaCreateTableEventArgs($table, [], [], $platform);

$doctrineTransport = $this->createMock(DoctrineTransport::class);
$doctrineTransport->expects($this->once())
->method('getExtraSetupSqlForTable')
->willReturn(null);

$platform->expects($this->never())
->method('getCreateTableSQL');

$subscriber = new MessengerTransportDoctrineSchemaSubscriber([$doctrineTransport]);
$subscriber->onSchemaCreateTable($event);
$this->assertFalse($event->isDefaultPrevented());
}
}
@@ -0,0 +1,42 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <fabien@symfony.com>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\Bridge\Doctrine\Tests\SchemaListener;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Schema\Schema;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Tools\Event\GenerateSchemaEventArgs;
use PHPUnit\Framework\TestCase;
use Symfony\Bridge\Doctrine\SchemaListener\PdoCacheAdapterDoctrineSchemaSubscriber;
use Symfony\Component\Cache\Adapter\PdoAdapter;

class PdoCacheAdapterDoctrineSchemaSubscriberTest extends TestCase
{
public function testPostGenerateSchema()
{
$schema = new Schema();
$dbalConnection = $this->createMock(Connection::class);
$entityManager = $this->createMock(EntityManagerInterface::class);
$entityManager->expects($this->once())
->method('getConnection')
->willReturn($dbalConnection);
$event = new GenerateSchemaEventArgs($entityManager, $schema);

$pdoAdapter = $this->createMock(PdoAdapter::class);
$pdoAdapter->expects($this->once())
->method('configureSchema')
->with($schema, $dbalConnection);

$subscriber = new PdoCacheAdapterDoctrineSchemaSubscriber([$pdoAdapter]);
$subscriber->postGenerateSchema($event);
}
}
2 changes: 2 additions & 0 deletions src/Symfony/Bridge/Doctrine/composer.json
Expand Up @@ -26,11 +26,13 @@
},
"require-dev": {
"symfony/stopwatch": "^4.4|^5.0",
"symfony/cache": "^5.1",
"symfony/config": "^4.4|^5.0",
"symfony/dependency-injection": "^4.4|^5.0",
"symfony/form": "^5.1",
"symfony/http-kernel": "^5.0",
"symfony/messenger": "^4.4|^5.0",
"symfony/doctrine-messenger": "^5.1",
"symfony/property-access": "^4.4|^5.0",
"symfony/property-info": "^5.0",
"symfony/proxy-manager-bridge": "^4.4|^5.0",
Expand Down
56 changes: 39 additions & 17 deletions src/Symfony/Component/Cache/Adapter/PdoAdapter.php
Expand Up @@ -115,24 +115,8 @@ public function createTable()
$conn = $this->getConnection();

if ($conn instanceof Connection) {
$types = [
'mysql' => 'binary',
'sqlite' => 'text',
'pgsql' => 'string',
'oci' => 'string',
'sqlsrv' => 'string',
];
if (!isset($types[$this->driver])) {
throw new \DomainException(sprintf('Creating the cache table is currently not implemented for PDO driver "%s".', $this->driver));
}

$schema = new Schema();
$table = $schema->createTable($this->table);
$table->addColumn($this->idCol, $types[$this->driver], ['length' => 255]);
$table->addColumn($this->dataCol, 'blob', ['length' => 16777215]);
$table->addColumn($this->lifetimeCol, 'integer', ['unsigned' => true, 'notnull' => false]);
$table->addColumn($this->timeCol, 'integer', ['unsigned' => true]);
$table->setPrimaryKey([$this->idCol]);
$this->addTableToSchema($schema);

foreach ($schema->toSql($conn->getDatabasePlatform()) as $sql) {
$conn->exec($sql);
Expand Down Expand Up @@ -169,6 +153,23 @@ public function createTable()
$conn->exec($sql);
}

/**
* Adds the Table to the Schema if the adapter uses this Connection.
*/
public function configureSchema(Schema $schema, Connection $forConnection): void
{
// only update the schema for this connection
if ($forConnection !== $this->getConnection()) {
return;
}

if ($schema->hasTable($this->table)) {
return;
}

$this->addTableToSchema($schema);
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -467,4 +468,25 @@ private function getServerVersion(): string

return $this->serverVersion;
}

private function addTableToSchema(Schema $schema): void
{
$types = [
'mysql' => 'binary',
'sqlite' => 'text',
'pgsql' => 'string',
'oci' => 'string',
'sqlsrv' => 'string',
];
if (!isset($types[$this->driver])) {
throw new \DomainException(sprintf('Creating the cache table is currently not implemented for PDO driver "%s".', $this->driver));
}

$table = $schema->createTable($this->table);
$table->addColumn($this->idCol, $types[$this->driver], ['length' => 255]);
$table->addColumn($this->dataCol, 'blob', ['length' => 16777215]);
$table->addColumn($this->lifetimeCol, 'integer', ['unsigned' => true, 'notnull' => false]);
$table->addColumn($this->timeCol, 'integer', ['unsigned' => true]);
$table->setPrimaryKey([$this->idCol]);
}
}

0 comments on commit b9d4149

Please sign in to comment.