Skip to content

Commit

Permalink
Drop support for silencing errors about DDL in transactions
Browse files Browse the repository at this point in the history
I hesitated between this and removing the transaction helper entirely,
but I think this solution will result in less support for us.
  • Loading branch information
greg0ire committed Jan 11, 2023
1 parent 53303eb commit fe8e7b8
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 35 deletions.
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
25 changes: 8 additions & 17 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 @@ -19,19 +18,15 @@ final class TransactionHelper
public static function commitIfInTransaction(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();
Expand All @@ -40,19 +35,15 @@ public static function commitIfInTransaction(Connection $connection): void
public static function rollbackIfInTransaction(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
36 changes: 18 additions & 18 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'
);
$this->expectException(LogicException::class);
TransactionHelper::commitIfInTransaction($connection);
}

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

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

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

$this->expectException(LogicException::class);
TransactionHelper::rollbackIfInTransaction($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);
}
}

0 comments on commit fe8e7b8

Please sign in to comment.