Skip to content

Commit

Permalink
Add hook method for migrations to skip creating a transaction
Browse files Browse the repository at this point in the history
There are schema operations that you don't want a wrapping transaction,
like if you're modifying a sqlite database. This new method allows
migrations to opt-out of having a transaction wrapped around each
migration automatically.

Refs #741
  • Loading branch information
markstory committed Sep 13, 2024
1 parent 2fb93e5 commit 4e013b8
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 14 deletions.
13 changes: 13 additions & 0 deletions src/AbstractMigration.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,19 @@ class AbstractMigration extends BaseAbstractMigration
*/
public bool $autoId = true;

/**
* Hook method to decide if this migration should use transactions
*
* By default if your driver supports transactions, a transaction will be opened
* before the migration begins, and commit when the migration completes.
*
* @return bool
*/
public function useTransactions(): bool
{
return $this->getAdapter()->hasTransactions();
}

/**
* Returns an instance of the Table class.
*
Expand Down
8 changes: 0 additions & 8 deletions src/Db/Adapter/SqliteAdapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -1046,15 +1046,7 @@ protected function copyAndDropTmpTable(AlterInstructions $instructions, string $
$state['selectColumns']
);

$result = $this->fetchRow('PRAGMA foreign_keys');
$foreignKeysEnabled = $result ? (bool)$result['foreign_keys'] : false;
if ($foreignKeysEnabled) {
$this->execute('PRAGMA foreign_keys = OFF');
}
$this->execute(sprintf('DROP TABLE %s', $this->quoteTableName($tableName)));
if ($foreignKeysEnabled) {
$this->execute('PRAGMA foreign_keys = ON');
}
$this->execute(sprintf(
'ALTER TABLE %s RENAME TO %s',
$this->quoteTableName($state['tmpTableName']),
Expand Down
14 changes: 9 additions & 5 deletions src/Migration/Environment.php
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,12 @@ public function executeMigration(MigrationInterface $migration, string $directio
$migration->{MigrationInterface::INIT}();
}

$atomic = $adapter->hasTransactions();
if (method_exists($migration, 'useTransactions')) {
$atomic = $migration->useTransactions();
}
// begin the transaction if the adapter supports it
if ($adapter->hasTransactions()) {
if ($atomic) {
$adapter->beginTransaction();
}

Expand Down Expand Up @@ -121,7 +125,7 @@ public function executeMigration(MigrationInterface $migration, string $directio
$adapter->migrated($migration, $direction, date('Y-m-d H:i:s', $startTime), date('Y-m-d H:i:s', time()));

// commit the transaction if the adapter supports it
if ($adapter->hasTransactions()) {
if ($atomic) {
$adapter->commitTransaction();
}

Expand All @@ -143,9 +147,9 @@ public function executeSeed(SeedInterface $seed): void
if (method_exists($seed, SeedInterface::INIT)) {
$seed->{SeedInterface::INIT}();
}

// begin the transaction if the adapter supports it
if ($adapter->hasTransactions()) {
$atomic = $adapter->hasTransactions();
if ($atomic) {
$adapter->beginTransaction();
}

Expand All @@ -155,7 +159,7 @@ public function executeSeed(SeedInterface $seed): void
}

// commit the transaction if the adapter supports it
if ($adapter->hasTransactions()) {
if ($atomic) {
$adapter->commitTransaction();
}
}
Expand Down
37 changes: 36 additions & 1 deletion tests/TestCase/Migration/EnvironmentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ public function testExecutingAMigrationWithTransactions()
$adapterStub->expects($this->once())
->method('commitTransaction');

$adapterStub->expects($this->exactly(2))
$adapterStub->expects($this->exactly(1))
->method('hasTransactions')
->willReturn(true);

Expand All @@ -189,6 +189,41 @@ public function up(): void
$this->assertTrue($migration->executed);
}

public function testExecutingAMigrationWithUseTransactions()
{
// stub adapter
$adapterStub = $this->getMockBuilder(PdoAdapter::class)
->setConstructorArgs([[]])
->getMock();
$adapterStub->expects($this->never())
->method('beginTransaction');

$adapterStub->expects($this->never())
->method('commitTransaction');

$adapterStub->expects($this->exactly(1))
->method('hasTransactions')
->willReturn(true);

$this->environment->setAdapter($adapterStub);

// migrate
$migration = new class ('mockenv', 20110301080000) extends AbstractMigration {
public bool $executed = false;
public function useTransactions(): bool

Check failure on line 213 in tests/TestCase/Migration/EnvironmentTest.php

View workflow job for this annotation

GitHub Actions / cs-stan / Coding Standard & Static Analysis

Every function/method needs a newline before
{
return false;
}

Check failure on line 216 in tests/TestCase/Migration/EnvironmentTest.php

View workflow job for this annotation

GitHub Actions / cs-stan / Coding Standard & Static Analysis

Every function/method needs a newline afterwards
public function up(): void
{
$this->executed = true;
}
};

$this->environment->executeMigration($migration, MigrationInterface::UP);
$this->assertTrue($migration->executed);
}

public function testExecutingAChangeMigrationUp()
{
// stub adapter
Expand Down

0 comments on commit 4e013b8

Please sign in to comment.