Skip to content

Commit

Permalink
Address issue #49
Browse files Browse the repository at this point in the history
  • Loading branch information
byjg committed Jul 15, 2023
1 parent 7c989f8 commit 2b75aaf
Show file tree
Hide file tree
Showing 13 changed files with 75 additions and 44 deletions.
38 changes: 11 additions & 27 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -205,39 +205,23 @@ $migration->getDbDriver();

To use it, please visit: <https://github.com/byjg/anydataset-db>

## Tips on writing SQL migrations for Postgres
### Avoiding Partial Migration

### Rely on explicit transactions
A partial migration is when the migration script is interrupted in the middle of the process due to an error or a manual interruption.

```sql
-- DO
BEGIN;
The migration table will be with the status `partial up` or `partial down` and it needs to be fixed manually before be able to migrate again.

ALTER TABLE 1;
UPDATE 1;
UPDATE 2;
UPDATE 3;
ALTER TABLE 2;
To avoid this situation you can specify the migration will be run in a transactional context.
If the migration script fails, the transaction will be rolled back and the migration table will be marked as `complete` and
the version will be the immediately previous version before the script that causes the error.

COMMIT;
To enable this feature you need to call the method `withTransactionEnabled` passing `true` as parameter:


-- DON'T
ALTER TABLE 1;
UPDATE 1;
UPDATE 2;
UPDATE 3;
ALTER TABLE 2;
```php
<?php
$migration->withTransactionEnabled(true);
```

It is generally desirable to wrap migration scripts inside a `BEGIN; ... COMMIT;` block.
This way, if _any_ of the inner statements fail, _none_ of them are committed and the
database does not end up in an inconsistent state.

Mind that in case of a failure `byjg/migration` will always mark the migration as `partial`
and warn you when you attempt to run it again. The difference is that with explicit
transactions you know that the database cannot be in an inconsistent state after an
unexpected failure.
## Tips on writing SQL migrations for Postgres

### On creating triggers and SQL functions

Expand Down
31 changes: 25 additions & 6 deletions src/Migration.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ class Migration
*/
private $migrationTable;

private $transaction = false;

/**
* Migration constructor.
*
Expand All @@ -69,10 +71,14 @@ public function __construct(UriInterface $uri, $folder, $requiredBase = true, $m
$this->migrationTable = $migrationTable;
}

public function withTransactionEnabled($enabled = true)
{
$this->transaction = $enabled;
return $this;
}

/**
* @param $scheme
* @param $className
* @return $this
* @param $class
*/
public static function registerDatabase($class)
{
Expand Down Expand Up @@ -327,9 +333,22 @@ protected function migrate($upVersion, $increment, $force)
call_user_func_array($this->callableProgress, ['migrate', $currentVersion, $fileInfo]);
}

$this->getDbCommand()->setVersion($currentVersion, Migration::VERSION_STATUS_PARTIAL . ' ' . ($increment>0 ? 'up' : 'down'));
$this->getDbCommand()->executeSql($fileInfo["content"]);
$this->getDbCommand()->setVersion($currentVersion, Migration::VERSION_STATUS_COMPLETE);
try {
if ($this->transaction) {
$this->getDbDriver()->beginTransaction();
}
$this->getDbCommand()->setVersion($currentVersion, Migration::VERSION_STATUS_PARTIAL . ' ' . ($increment>0 ? 'up' : 'down'));
$this->getDbCommand()->executeSql($fileInfo["content"]);
$this->getDbCommand()->setVersion($currentVersion, Migration::VERSION_STATUS_COMPLETE);
if ($this->transaction) {
$this->getDbDriver()->commitTransaction();
}
} catch (\Exception $e) {
if ($this->transaction) {
$this->getDbDriver()->rollbackTransaction();
}
throw $e;
}
$currentVersion = $currentVersion + $increment;
}
}
Expand Down
44 changes: 38 additions & 6 deletions tests/MigrationTest.php
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<?php
namespace Test;

use ByJG\DbMigration\Database\SqliteDatabase;
use ByJG\DbMigration\Exception\InvalidMigrationFile;
use ByJG\DbMigration\Migration;
use ByJG\Util\Uri;
Expand Down Expand Up @@ -113,8 +114,8 @@ public function testGetFileContent_1()
"file" => __DIR__ . '/dirstructure/migrations/up/00001.sql',
"description" => "this is a test",
"exists" => true,
"checksum" => "b937afa57e363c9244fa30844dd11d312694f697",
"content" => "-- @description: this is a test\n\nselect * from mysql.users;\n",
"checksum" => "55249baf6b70c1d2e9c5362de133b2371d0dc989",
"content" => "-- @description: this is a test\n",
],
$this->object->getFileContent(__DIR__ . '/dirstructure/migrations/up/00001.sql')
);
Expand All @@ -127,8 +128,8 @@ public function testGetFileContent_2()
"file" => __DIR__ . '/dirstructure/migrations/up/00002.sql',
"description" => "another test",
"exists" => true,
"checksum" => "fd8ab8176291c2dcbf0d91564405e0f98f0cd77e",
"content" => "-- @description: another test\n\nselect * from dual;",
"checksum" => "f20c73a5eb4d29e2f8edae4409a2ccc2b02c6f67",
"content" => "-- @description: another test\n",
],
$this->object->getFileContent(__DIR__ . '/dirstructure/migrations/up/00002.sql')
);
Expand All @@ -141,10 +142,41 @@ public function testGetFileContent_3()
"file" => __DIR__ . '/dirstructure/migrations/up/00003.sql',
"description" => "no description provided. Pro tip: use `-- @description:` to define one.",
"exists" => true,
"checksum" => "73faaa68e2f60c11e75a9ccc18528e0ffa15127a",
"content" => "select something from sometable;",
"checksum" => "da39a3ee5e6b4b0d3255bfef95601890afd80709",
"content" => "",
],
$this->object->getFileContent(__DIR__ . '/dirstructure/migrations/up/00003.sql')
);
}

public function testReset()
{
$this->expectException(\PDOException::class);
Migration::registerDatabase(SqliteDatabase::class);
$this->object = new Migration(new Uri('sqlite:///tmp/test.db'), __DIR__ . '/dirstructure2');
$this->object->reset();
}

public function testResetWithoutTransactionCheck()
{
try {
Migration::registerDatabase(SqliteDatabase::class);
$this->object = new Migration(new Uri('sqlite:///tmp/test.db'), __DIR__ . '/dirstructure2');
$this->object->reset();
} catch (\PDOException $ex) {
$this->assertEquals(["version" => '2', "status" => "partial up"], $this->object->getCurrentVersion());
}
}

public function testResetWithTransactionCheck()
{
try {
Migration::registerDatabase(SqliteDatabase::class);
$this->object = new Migration(new Uri('sqlite:///tmp/test.db'), __DIR__ . '/dirstructure2');
$this->object->withTransactionEnabled();
$this->object->reset();
} catch (\PDOException $ex) {
$this->assertEquals(["version" => '1', "status" => "complete"], $this->object->getCurrentVersion());
}
}
}
2 changes: 0 additions & 2 deletions tests/dirstructure/migrations/up/00001.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
-- @description: this is a test

select * from mysql.users;
2 changes: 0 additions & 2 deletions tests/dirstructure/migrations/up/00002.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1 @@
-- @description: another test

select * from dual;
1 change: 0 additions & 1 deletion tests/dirstructure/migrations/up/00003.sql
Original file line number Diff line number Diff line change
@@ -1 +0,0 @@
select something from sometable;
Empty file added tests/dirstructure2/base.sql
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
1 change: 1 addition & 0 deletions tests/dirstructure2/migrations/up/00002.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
error
Empty file.

0 comments on commit 2b75aaf

Please sign in to comment.