Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add onMigrationsQueryExecuting and onMigrationsQueryExecuted events #906

Open
wants to merge 1 commit into
base: 3.1.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions docs/en/reference/events.rst
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ there are no versions to be executed.
- ``onMigrationsVersionExecuting``: dispatched before a single version executes.
- ``onMigrationsVersionExecuted``: dispatched after a single version executes.
- ``onMigrationsVersionSkipped``: dispatched when a single version is skipped.
- ``onMigrationsQueryExecuting``: dispatched before a single query executes.
- ``onMigrationsQueryExecuted``: dispatched after a single query executes.
- ``onMigrationsMigrated``: dispatched when all versions have been executed.

All of these events are emitted via the DBAL connection's event manager. Here's an example event subscriber that
Expand All @@ -19,6 +21,7 @@ listens for all possible migrations events.

use Doctrine\Common\EventSubscriber;
use Doctrine\Migrations\Event\MigrationsEventArgs;
use Doctrine\Migrations\Event\MigrationsQueryEventArgs;
use Doctrine\Migrations\Event\MigrationsVersionEventArgs;
use Doctrine\Migrations\Events;

Expand All @@ -32,6 +35,8 @@ listens for all possible migrations events.
Events::onMigrationsVersionExecuting,
Events::onMigrationsVersionExecuted,
Events::onMigrationsVersionSkipped,
Events::onMigrationsQueryExecuting,
Events::onMigrationsQueryExecuted,
];
}

Expand Down Expand Up @@ -59,6 +64,16 @@ listens for all possible migrations events.
{
// ...
}

public function onMigrationsQueryExecuting(MigrationsQueryEventArgs $args) : void
{
// ...
}

public function onMigrationsQueryExecuted(MigrationsQueryEventArgs $args) : void
{
// ...
}
}

To add an event subscriber to a connections event manager, use the ``Connection::getEventManager()`` method
Expand Down
61 changes: 61 additions & 0 deletions lib/Doctrine/Migrations/Event/MigrationsQueryEventArgs.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Event;

use Doctrine\Common\EventArgs;
use Doctrine\DBAL\Connection;
use Doctrine\Migrations\Metadata\MigrationPlan;
use Doctrine\Migrations\MigratorConfiguration;
use Doctrine\Migrations\Query\Query;

/**
* The MigrationsQueryEventArgs class is passed to events related to a single migration query from a version.
*/
final class MigrationsQueryEventArgs extends EventArgs
{
/** @var Connection */
private $connection;

/** @var MigrationPlan */
private $plan;

/** @var MigratorConfiguration */
private $migratorConfiguration;

/** @var Query */
private $query;

public function __construct(
Connection $connection,
MigrationPlan $plan,
MigratorConfiguration $migratorConfiguration,
Query $query
) {
$this->connection = $connection;
$this->plan = $plan;
$this->migratorConfiguration = $migratorConfiguration;
$this->query = $query;
}

public function getConnection() : Connection
{
return $this->connection;
}

public function getPlan() : MigrationPlan
{
return $this->plan;
}

public function getMigratorConfiguration() : MigratorConfiguration
{
return $this->migratorConfiguration;
}

public function getQuery() : Query
{
return $this->query;
}
}
30 changes: 30 additions & 0 deletions lib/Doctrine/Migrations/EventDispatcher.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@
use Doctrine\Common\EventManager;
use Doctrine\DBAL\Connection;
use Doctrine\Migrations\Event\MigrationsEventArgs;
use Doctrine\Migrations\Event\MigrationsQueryEventArgs;
use Doctrine\Migrations\Event\MigrationsVersionEventArgs;
use Doctrine\Migrations\Metadata\MigrationPlan;
use Doctrine\Migrations\Metadata\MigrationPlanList;
use Doctrine\Migrations\Query\Query;

/**
* The EventDispatcher class is responsible for dispatching events internally that a user can listen for.
Expand Down Expand Up @@ -54,6 +56,21 @@ public function dispatchVersionEvent(
$this->dispatchEvent($eventName, $event);
}

public function dispatchQueryExecuteEvent(
string $eventName,
MigrationPlan $plan,
MigratorConfiguration $migratorConfiguration,
Query $query
) : void {
$event = $this->createMigrationsQueryEventArgs(
$plan,
$migratorConfiguration,
$query
);

$this->dispatchEvent($eventName, $event);
}

private function dispatchEvent(string $eventName, ?EventArgs $args = null) : void
{
$this->eventManager->dispatchEvent($eventName, $args);
Expand All @@ -76,4 +93,17 @@ private function createMigrationsVersionEventArgs(
$migratorConfiguration
);
}

private function createMigrationsQueryEventArgs(
MigrationPlan $plan,
MigratorConfiguration $migratorConfiguration,
Query $query
) : MigrationsQueryEventArgs {
return new MigrationsQueryEventArgs(
$this->connection,
$plan,
$migratorConfiguration,
$query
);
}
}
2 changes: 2 additions & 0 deletions lib/Doctrine/Migrations/Events.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ final class Events
{
public const onMigrationsMigrating = 'onMigrationsMigrating';
public const onMigrationsMigrated = 'onMigrationsMigrated';
public const onMigrationsQueryExecuting = 'onMigrationsQueryExecuting';
public const onMigrationsQueryExecuted = 'onMigrationsQueryExecuted';
public const onMigrationsVersionExecuting = 'onMigrationsVersionExecuting';
public const onMigrationsVersionExecuted = 'onMigrationsVersionExecuted';
public const onMigrationsVersionSkipped = 'onMigrationsVersionSkipped';
Expand Down
48 changes: 48 additions & 0 deletions lib/Doctrine/Migrations/Query/Query.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Query;

/**
* The Query wraps the sql query, parameters and types. It used in MigrationsQueryEventArgs.
*/
final class Query
{
/** @var string */
private $statement;

/** @var mixed[] */
private $parameters;

/** @var mixed[] */
private $types;

/**
* @param mixed[] $parameters
* @param mixed[] $types
*/
public function __construct(string $statement, array $parameters = [], array $types = [])
{
$this->statement = $statement;
$this->parameters = $parameters;
$this->types = $types;
}

public function getStatement() : string
{
return $this->statement;
}

/** @return mixed[] */
public function getParameters() : array
{
return $this->parameters;
}

/** @return mixed[] */
public function getTypes() : array
{
return $this->types;
}
}
19 changes: 17 additions & 2 deletions lib/Doctrine/Migrations/Version/DbalExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Doctrine\Migrations\MigratorConfiguration;
use Doctrine\Migrations\ParameterFormatter;
use Doctrine\Migrations\Provider\SchemaDiffProvider;
use Doctrine\Migrations\Query\Query;
use Doctrine\Migrations\Stopwatch;
use Doctrine\Migrations\Tools\BytesFormatter;
use Psr\Log\LoggerInterface;
Expand Down Expand Up @@ -206,7 +207,7 @@ private function executeMigration(

if (count($this->sql) !== 0) {
if (! $configuration->isDryRun()) {
$this->executeResult($configuration);
$this->executeResult($configuration, $plan);
} else {
foreach ($this->sql as $idx => $query) {
$this->outputSqlQuery($idx, $query);
Expand Down Expand Up @@ -324,9 +325,16 @@ private function logResult(Throwable $e, ExecutionResult $result, MigrationPlan
}
}

private function executeResult(MigratorConfiguration $configuration) : void
private function executeResult(MigratorConfiguration $configuration, MigrationPlan $plan) : void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally don't like that the check is done here. It is already too late, the migration process already started. Databases as MYSQL are not that good at transactions and altering the schema.

I would suggest to trigger a new event type right before the if (count($this->sql) !== 0) { check in the executeMigration method.
In that moment the execution of SQL commands did not start yet and is much safer to interrupt it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also expect that, when I consider the example use case, if a big table was the reason why an migration was aborted, that the previous executed queries of the same version on the smaller tables are rolled back. Maybe a test can cover that.

{
foreach ($this->sql as $key => $query) {
$migrationQuery = new Query($query, $this->params[$key] ?? [], $this->types[$key] ?? []);
$this->dispatcher->dispatchQueryExecuteEvent(
Events::onMigrationsQueryExecuting,
$plan,
$configuration,
$migrationQuery
);
$stopwatchEvent = $this->stopwatch->start('query');

$this->outputSqlQuery($key, $query);
Expand All @@ -339,6 +347,13 @@ private function executeResult(MigratorConfiguration $configuration) : void

$stopwatchEvent->stop();

$this->dispatcher->dispatchQueryExecuteEvent(
Events::onMigrationsQueryExecuted,
$plan,
$configuration,
$migrationQuery
);

if (! $configuration->getTimeAllQueries()) {
continue;
}
Expand Down
13 changes: 13 additions & 0 deletions tests/Doctrine/Migrations/Tests/Event/EventArgsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
use Doctrine\DBAL\Connection;
use Doctrine\Migrations\AbstractMigration;
use Doctrine\Migrations\Event\MigrationsEventArgs;
use Doctrine\Migrations\Event\MigrationsQueryEventArgs;
use Doctrine\Migrations\Event\MigrationsVersionEventArgs;
use Doctrine\Migrations\Metadata\MigrationPlan;
use Doctrine\Migrations\Metadata\MigrationPlanList;
use Doctrine\Migrations\MigratorConfiguration;
use Doctrine\Migrations\Query\Query;
use Doctrine\Migrations\Version\Direction;
use Doctrine\Migrations\Version\Version;
use PHPUnit\Framework\MockObject\MockObject;
Expand Down Expand Up @@ -44,6 +46,17 @@ public function testMigrationsVersionEventArgs() : void
self::assertSame($this->plan, $event->getPlan());
}

public function testMigrationsQueryEventArgs() : void
{
$query = new Query('SELECT 1');
$event = new MigrationsQueryEventArgs($this->connection, $this->plan, $this->config, $query);

self::assertSame($this->connection, $event->getConnection());
self::assertSame($this->config, $event->getMigratorConfiguration());
self::assertSame($this->plan, $event->getPlan());
self::assertSame($query, $event->getQuery());
}

public function testMigrationsEventArgs() : void
{
$plan = new MigrationPlanList([], Direction::UP);
Expand Down
20 changes: 20 additions & 0 deletions tests/Doctrine/Migrations/Tests/Query/QueryTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Doctrine\Migrations\Tests\Query;

use Doctrine\Migrations\Query\Query;
use Doctrine\Migrations\Tests\MigrationTestCase;

final class QueryTest extends MigrationTestCase
{
public function testGetQuery() : void
{
$query = new Query('foo', ['bar', 'baz'], ['qux', 'quux']);

self::assertSame('foo', $query->getStatement());
self::assertSame(['bar', 'baz'], $query->getParameters());
self::assertSame(['qux', 'quux'], $query->getTypes());
}
}