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

Drop support for silencing errors about DDL in transactions #1300

Open
wants to merge 1 commit into
base: 4.0.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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
# Upgrade to 4.0

It is no longer possible to relying on silencing to enable transactional
migrations when the platform does not allow DDL inside transactions.

# Upgrade to 3.1

- The "version" is the FQCN of the migration class (existing entries in the migrations table will be automatically updated).
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/Migrations/DbalMigrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,14 @@ private function executeMigrations(
$this->dispatcher->dispatchMigrationEvent(Events::onMigrationsMigrated, $migrationsPlan, $migratorConfiguration);
} catch (Throwable $e) {
if ($allOrNothing) {
TransactionHelper::rollbackIfInTransaction($this->connection);
TransactionHelper::rollback($this->connection);
}

throw $e;
}

if ($allOrNothing) {
TransactionHelper::commitIfInTransaction($this->connection);
TransactionHelper::commit($this->connection);
}

return $sql;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public function onMigrationsMigrated(MigrationsEventArgs $args): void
return;
}

TransactionHelper::commitIfInTransaction($conn);
TransactionHelper::commit($conn);
}

/** {@inheritdoc} */
Expand Down
29 changes: 10 additions & 19 deletions lib/Doctrine/Migrations/Tools/TransactionHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
namespace Doctrine\Migrations\Tools;

use Doctrine\DBAL\Connection;
use Doctrine\Deprecations\Deprecation;
use LogicException;
use PDO;

Expand All @@ -16,43 +15,35 @@
*/
final class TransactionHelper
{
public static function commitIfInTransaction(Connection $connection): void
public static function commit(Connection $connection): void
{
if (! self::inTransaction($connection)) {
Deprecation::trigger(
'doctrine/migrations',
'https://github.com/doctrine/migrations/issues/1169',
<<<'DEPRECATION'
throw new LogicException(
<<<'EXCEPTION'
Context: trying to commit a transaction
Problem: the transaction is already committed, relying on silencing is deprecated.
Problem: the transaction is already committed.
Solution: override `AbstractMigration::isTransactional()` so that it returns false.
Automate that by setting `transactional` to false in the configuration.
More details at https://www.doctrine-project.org/projects/doctrine-migrations/en/3.2/explanation/implicit-commits.html
DEPRECATION
EXCEPTION
);

return;
}

$connection->commit();
}

public static function rollbackIfInTransaction(Connection $connection): void
public static function rollback(Connection $connection): void
{
if (! self::inTransaction($connection)) {
Deprecation::trigger(
'doctrine/migrations',
'https://github.com/doctrine/migrations/issues/1169',
<<<'DEPRECATION'
throw new LogicException(
<<<'EXCEPTION'
Context: trying to rollback a transaction
Problem: the transaction is already rolled back, relying on silencing is deprecated.
Problem: the transaction is already rolled back.
Solution: override `AbstractMigration::isTransactional()` so that it returns false.
Automate that by setting `transactional` to false in the configuration.
More details at https://www.doctrine-project.org/projects/doctrine-migrations/en/3.2/explanation/implicit-commits.html
DEPRECATION
EXCEPTION
);

return;
}

$connection->rollBack();
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/Migrations/Version/DbalExecutor.php
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ private function executeMigration(
}

if ($migration->isTransactional()) {
TransactionHelper::commitIfInTransaction($this->connection);
TransactionHelper::commit($this->connection);
}

$plan->markAsExecuted($result);
Expand Down Expand Up @@ -252,7 +252,7 @@ private function migrationEnd(Throwable $e, MigrationPlan $plan, ExecutionResult
$migration = $plan->getMigration();
if ($migration->isTransactional()) {
//only rollback transaction if in transactional mode
TransactionHelper::rollbackIfInTransaction($this->connection);
TransactionHelper::rollback($this->connection);
}

$plan->markAsExecuted($result);
Expand Down
44 changes: 22 additions & 22 deletions tests/Doctrine/Migrations/Tests/Tools/TransactionHelperTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,14 @@
namespace Doctrine\Migrations\Tests\Tools;

use Doctrine\DBAL\Connection;
use Doctrine\Deprecations\PHPUnit\VerifyDeprecations;
use Doctrine\Migrations\Tools\TransactionHelper;
use LogicException;
use PDO;
use PHPUnit\Framework\TestCase;

final class TransactionHelperTest extends TestCase
{
use VerifyDeprecations;

public function testItTriggersADeprecationWhenUseful(): void
public function testItThrowsAnExceptionWhenAttemptingToCommitWhileNotInsideATransaction(): void
{
$connection = $this->createStub(Connection::class);
$wrappedConnection = $this->createStub(PDO::class);
Expand All @@ -23,18 +21,27 @@ public function testItTriggersADeprecationWhenUseful(): void

$wrappedConnection->method('inTransaction')->willReturn(false);

$this->expectDeprecationWithIdentifier(
'https://github.com/doctrine/migrations/issues/1169'
);
TransactionHelper::commitIfInTransaction($connection);
$this->expectException(LogicException::class);
TransactionHelper::commit($connection);
}

public function testItThrowsAnExceptionWhenAttemptingToRollbackWhileNotInsideATransaction(): void
{
$connection = $this->createStub(Connection::class);
$wrappedConnection = $this->createStub(PDO::class);

$this->expectDeprecationWithIdentifier(
'https://github.com/doctrine/migrations/issues/1169'
);
TransactionHelper::rollbackIfInTransaction($connection);
$connection->method('getNativeConnection')->willReturn($wrappedConnection);

$wrappedConnection->method('inTransaction')->willReturn(false);

$this->expectException(LogicException::class);
TransactionHelper::rollback($connection);
}

public function testItDoesNotTriggerADeprecationWhenUseless(): void
/**
* @doesNotPerformAssertions
*/
public function testItDoesNotThrowAnExceptionWhenUseless(): void
{
$connection = $this->createStub(Connection::class);
$wrappedConnection = $this->createStub(PDO::class);
Expand All @@ -43,14 +50,7 @@ public function testItDoesNotTriggerADeprecationWhenUseless(): void

$wrappedConnection->method('inTransaction')->willReturn(true);

$this->expectNoDeprecationWithIdentifier(
'https://github.com/doctrine/migrations/issues/1169'
);
TransactionHelper::commitIfInTransaction($connection);

$this->expectNoDeprecationWithIdentifier(
'https://github.com/doctrine/migrations/issues/1169'
);
TransactionHelper::rollbackIfInTransaction($connection);
TransactionHelper::commit($connection);
TransactionHelper::rollback($connection);
}
}