Merge pull request #50 from byjg/4.9

4.9

.github/workflows/phpunit.yml

@@ -18,14 +18,10 @@ jobs:
     strategy:
       matrix:
         php-version:
+          - "8.2"
           - "8.1"
           - "8.0"
           - "7.4"
-          - "7.3"
-#          - "7.2"
-#          - "7.1"
-#          - "7.0"
-#          - "5.6"
 
     services:
       mysql:
@@ -69,7 +65,7 @@ jobs:
       MSSQL_TEST_HOST: sqlserver
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
 #      - name: Spin up databases
 #        run: |
 #          apk add --no-cache python3 python3-dev py3-pip build-base libffi-dev
@@ -86,11 +82,11 @@ jobs:
 #      - run: ./vendor/bin/phpunit tests/SqlServerDblib*
 
   Documentation:
-    runs-on: 'ubuntu-latest'
-    needs: Build
     if: github.ref == 'refs/heads/master'
-    env:
-      DOC_GITHUB_TOKEN: '${{ secrets.DOC_TOKEN }}'
-    steps:
-      - uses: actions/checkout@v3
-      - run: curl https://opensource.byjg.com/add-doc.sh | bash /dev/stdin php migration
+    needs: Build
+    uses: byjg/byjg.github.io/.github/workflows/add-doc.yaml@master
+    with:
+      folder: php
+      project: ${{ github.event.repository.name }}
+    secrets: inherit
+

README.md

@@ -1,10 +1,10 @@
 # Database Migrations PHP
 
 [![Opensource ByJG](https://img.shields.io/badge/opensource-byjg-success.svg)](http://opensource.byjg.com)
-[![GitHub source](https://img.shields.io/badge/Github-source-informational?logo=github)](https://github.com/byjg/migration/)
-[![GitHub license](https://img.shields.io/github/license/byjg/migration.svg)](https://opensource.byjg.com/opensource/licensing.html)
-[![GitHub release](https://img.shields.io/github/release/byjg/migration.svg)](https://github.com/byjg/migration/releases/)
-[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/byjg/migration/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/byjg/migration/?branch=master)
+[![GitHub source](https://img.shields.io/badge/Github-source-informational?logo=github)](https://github.com/byjg/php-migration/)
+[![GitHub license](https://img.shields.io/github/license/byjg/php-migration.svg)](https://opensource.byjg.com/opensource/licensing.html)
+[![GitHub release](https://img.shields.io/github/release/byjg/php-migration.svg)](https://github.com/byjg/php-migration/releases/)
+[![Scrutinizer Code Quality](https://scrutinizer-ci.com/g/byjg/php-migration/badges/quality-score.png?b=master)](https://scrutinizer-ci.com/g/byjg/php-migration/?branch=master)
 [![Build Status](https://github.com/byjg/migration/actions/workflows/phpunit.yml/badge.svg?branch=master)](https://github.com/byjg/migration/actions/workflows/phpunit.yml)
 
 ## Features
@@ -205,39 +205,28 @@ $migration->getDbDriver();
 
 To use it, please visit: <https://github.com/byjg/anydataset-db>
 
-## Tips on writing SQL migrations for Postgres
-
-### Rely on explicit transactions
+### Avoiding Partial Migration (not available for MySQL)
 
-```sql
--- DO
-BEGIN;
+A partial migration is when the migration script is interrupted in the middle of the process due to an error or a manual interruption.
 
-ALTER TABLE 1;
-UPDATE 1;
-UPDATE 2;
-UPDATE 3;
-ALTER TABLE 2;
+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. 
 
-COMMIT;
+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.
 
+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.
+**NOTE: This feature isn't available for MySQL as it doesn't support DDL commands inside a transaction.**
+If you use this method with MySQL the Migration will ignore it silently. 
+More info: [https://dev.mysql.com/doc/refman/8.0/en/cannot-roll-back.html](https://dev.mysql.com/doc/refman/8.0/en/cannot-roll-back.html)
 
-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
 
@@ -402,5 +391,14 @@ export SQLITE_TEST_HOST=/tmp/test.db      # defaults to /tmp/test.db
 * [PHP Rest Template](https://github.com/byjg/php-rest-template)
 * [USDocker](https://github.com/usdocker/usdocker)
 
+## Dependencies
+
+```mermaid
+flowchart TD
+    byjg/migration --> byjg/anydataset-db
+    byjg/migration --> ext-pdo
+```
+
+
 ----
 [Open source ByJG](http://opensource.byjg.com)

composer.json

@@ -8,7 +8,7 @@
     "ext-pdo": "*"
   },
   "require-dev": {
-    "phpunit/phpunit": "5.7.*|7.4.*|^9.5"
+    "phpunit/phpunit": "5.7.*|7.4.*|^9.6"
   },
   "autoload": {
     "psr-4": {

src/Database/AbstractDatabase.php

@@ -141,4 +141,9 @@ public function isDatabaseVersioned()
     {
         return $this->isTableExists(ltrim($this->getDbDriver()->getUri()->getPath(), "/"), $this->getMigrationTable());
     }
+
+    public function supportsTransaction()
+    {
+        return true;
+    }
 }

src/Database/DatabaseInterface.php

@@ -34,4 +34,6 @@ public function isDatabaseVersioned();
     public function getDbDriver();
 
     public function getMigrationTable();
+
+    public function supportsTransaction();
 }

src/Database/MySqlDatabase.php

@@ -52,4 +52,11 @@ public function executeSql($sql)
     {
         $this->getDbDriver()->execute($sql);
     }
+
+    public function supportsTransaction()
+    {
+        // MySQL doesn't support transaction for DDL commands
+        // https://dev.mysql.com/doc/refman/8.0/en/cannot-roll-back.html
+        return false;
+    }
 }

src/Migration.php

@@ -50,6 +50,8 @@ class Migration
      */
     private $migrationTable;
 
+    private $transaction = false;
+
     /**
      * Migration constructor.
      *
@@ -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)
     {
@@ -161,7 +167,7 @@ public function getMigrationSql($version, $increment)
             . "/*.sql";
 
         $result = array_filter(glob($filePattern), function ($file) use ($version) {
-            return preg_match("/^0*$version(-dev)?\.sql$/", basename($file));
+            return preg_match("/^0*$version(-[\w\d-]*)?\.sql$/", basename($file));
         });
 
         // Valid values are 0 or 1
@@ -327,9 +333,24 @@ 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);
+            $useTransaction = $this->transaction && $this->getDbCommand()->supportsTransaction();
+
+            try {
+                if ($useTransaction) {
+                    $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 ($useTransaction) {
+                    $this->getDbDriver()->commitTransaction();
+                }
+            } catch (\Exception $e) {
+                if ($useTransaction) {
+                    $this->getDbDriver()->rollbackTransaction();
+                }
+                throw $e;
+            }
             $currentVersion = $currentVersion + $increment;
         }
     }

tests/MigrationTest.php

@@ -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;
@@ -63,7 +64,7 @@ public function testGetMigrationSql3()
     public function testGetMigrationSql4()
     {
         $this->expectException(InvalidMigrationFile::class);
-        $this->expectExceptionMessage("version number '13'");
+        $this->expectExceptionMessage("You have two files with the same version number '13'");
         $this->object->getMigrationSql(13, 1);
     }
 
@@ -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')
         );
@@ -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')
         );
@@ -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());
+        }
+    }
 }

tests/dirstructure/migrations/up/00001.sql

@@ -1,3 +1 @@
 -- @description: this is a test
-
-select * from mysql.users;

tests/dirstructure/migrations/up/00002.sql

@@ -1,3 +1 @@
 --   @description:  another test
-
-select * from dual;

tests/dirstructure/migrations/up/00003.sql

@@ -1 +0,0 @@
-select something from sometable;

tests/dirstructure2/migrations/up/00002.sql

@@ -0,0 +1 @@
+error