Merge pull request #12 from byjg/2.1.0

2.1.0

.gitignore

@@ -48,3 +48,6 @@ crashlytics-build.properties
 fabric.properties
 /composer.lock
 /vendor/
+node_modules
+package-lock.json
+.usdocker

.travis.yml

@@ -26,13 +26,18 @@ before_install:
 
 install:
   - composer install
-  - node_modules/.bin/usdocker mssql status
   - node_modules/.bin/usdocker postgres status
   - node_modules/.bin/usdocker mysql status
+  - node_modules/.bin/usdocker mssql status
 
 script: 
   - vendor/bin/phpunit
-#  - vendor/bin/phpunit tests/SqlServerDatabaseTest.php
   - vendor/bin/phpunit tests/PostgresDatabaseTest.php
   - vendor/bin/phpunit tests/MysqlDatabaseTest.php
+  - vendor/bin/phpunit tests/SqlServerDatabaseTest.php
+
+  - vendor/bin/phpunit tests/SqliteDatabaseCustomTest.php
+  - vendor/bin/phpunit tests/PostgresDatabaseCustomTest.php
+  - vendor/bin/phpunit tests/MysqlDatabaseCustomTest.php
+  - vendor/bin/phpunit tests/SqlServerDatabaseCustomTest.php
 

README.md

@@ -241,9 +241,158 @@ $migration->reset();
 $migration->up();
 ```
 
-The Migration object controls the database version.  
+The Migration object controls the database version.
 
 
+### Tips on writing SQL migrations
+
+#### Rely on explicit transactions
+
+```sql
+-- DO
+BEGIN;
+
+ALTER TABLE 1;
+UPDATE 1;
+UPDATE 2;
+UPDATE 3;
+ALTER TABLE 2;
+
+COMMIT;
+
+
+-- DON'T
+ALTER TABLE 1;
+UPDATE 1;
+UPDATE 2;
+UPDATE 3;
+ALTER TABLE 2;
+```
+
+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.
+
+#### On creating triggers and SQL functions
+
+```sql
+-- DO
+CREATE FUNCTION emp_stamp() RETURNS trigger AS $emp_stamp$
+    BEGIN
+        -- Check that empname and salary are given
+        IF NEW.empname IS NULL THEN
+            RAISE EXCEPTION 'empname cannot be null'; -- it doesn't matter if these comments are blank or not
+        END IF; --
+        IF NEW.salary IS NULL THEN
+            RAISE EXCEPTION '% cannot have null salary', NEW.empname; --
+        END IF; --
+
+        -- Who works for us when they must pay for it?
+        IF NEW.salary < 0 THEN
+            RAISE EXCEPTION '% cannot have a negative salary', NEW.empname; --
+        END IF; --
+
+        -- Remember who changed the payroll when
+        NEW.last_date := current_timestamp; --
+        NEW.last_user := current_user; --
+        RETURN NEW; --
+    END; --
+$emp_stamp$ LANGUAGE plpgsql;
+
+
+-- DON'T
+CREATE FUNCTION emp_stamp() RETURNS trigger AS $emp_stamp$
+    BEGIN
+        -- Check that empname and salary are given
+        IF NEW.empname IS NULL THEN
+            RAISE EXCEPTION 'empname cannot be null';
+        END IF;
+        IF NEW.salary IS NULL THEN
+            RAISE EXCEPTION '% cannot have null salary', NEW.empname;
+        END IF;
+
+        -- Who works for us when they must pay for it?
+        IF NEW.salary < 0 THEN
+            RAISE EXCEPTION '% cannot have a negative salary', NEW.empname;
+        END IF;
+
+        -- Remember who changed the payroll when
+        NEW.last_date := current_timestamp;
+        NEW.last_user := current_user;
+        RETURN NEW;
+    END;
+$emp_stamp$ LANGUAGE plpgsql;
+```
+
+Since the `PDO` database abstraction layer cannot run batches of SQL statements,
+when `byjg/migration` reads a migration file it has to split up the whole contents of the SQL
+file at the semicolons, and run the statements one by one. However, there is one kind of
+statement that can have multiple semicolons in-between its body: functions.
+
+In order to be able to parse functions correctly, `byjg/migration` 2.1.0 started splitting migration
+files at the `semicolon + EOL` sequence instead of just the semicolon. This way, if you append an empty
+comment after every inner semicolon of a function definition `byjg/migration` will be able to parse it.
+
+Unfortunately, if you forget to add any of these comments the library will split the `CREATE FUNCTION` statement in
+multiple parts and the migration will fail.
+
+#### Avoid the colon character (`:`)
+
+```sql
+-- DO
+CREATE TABLE bookings (
+  booking_id UUID PRIMARY KEY,
+  booked_at  TIMESTAMPTZ NOT NULL CHECK (CAST(booked_at AS DATE) <= check_in),
+  check_in   DATE NOT NULL
+);
+
+
+-- DON'T
+CREATE TABLE bookings (
+  booking_id UUID PRIMARY KEY,
+  booked_at  TIMESTAMPTZ NOT NULL CHECK (booked_at::DATE <= check_in),
+  check_in   DATE NOT NULL
+);
+```
+
+Since `PDO` uses the colon character to prefix named parameters in prepared statements, its use will trip it
+up in other contexts.
+
+For instance, PostgreSQL statements can use `::` to cast values between types. On the other hand `PDO` will
+read this as an invalid named parameter in an invalid context and fail when it tries to run it.
+
+The only way to fix this inconsistency is avoiding colons altogether (in this case, PostgreSQL also has an alternative
+ syntax: `CAST(value AS type)`).
+
+#### Use an SQL editor
+
+Finally, writing manual SQL migrations can be tiresome, but it is significantly easier if
+you use an editor capable of understanding the SQL syntax, providing autocomplete,
+introspecting your current database schema and/or autoformatting your code.
+
+
+### Handle different migration inside one schema
+
+If you need to create different migration scripts and version inside the same schema it is possible
+but is too risky and I do not recommend at all. 
+
+To do this, you need to create different "migration tables" by passing the parameter to the constructor. 
+
+```php
+<?php
+$migration = new \ByJG\DbMigration\Migration("db:/uri", "/path", true, "NEW_MIGRATION_TABLE_NAME");
+```
+
+For security reasons, this feature is not available at command line, but you can use the environment variable
+`MIGRATION_VERSION` to store the name. 
+
+We really recommend do not use this feature. The recommendation is one migration for one schema. 
+
 
 ## Unit Tests
 
@@ -252,10 +401,10 @@ This library has integrated tests and need to be setup for each database you wan
 Basiclly you have the follow tests:
 
 ```
-phpunit tests/SqliteDatabaseTest.php
-phpunit tests/MysqlDatabaseTest.php
-phpunit tests/PostgresDatabaseTest.php
-phpunit tests/SqlServerDatabaseTest.php 
+vendor/bin/phpunit tests/SqliteDatabaseTest.php
+vendor/bin/phpunit tests/MysqlDatabaseTest.php
+vendor/bin/phpunit tests/PostgresDatabaseTest.php
+vendor/bin/phpunit tests/SqlServerDatabaseTest.php 
 ```
 
 ### Using Docker for testing
@@ -264,7 +413,8 @@ phpunit tests/SqlServerDatabaseTest.php
 
 ```bash
 npm i @usdocker/usdocker @usdocker/mysql
-./node_modules/.bin/usdocker --refresh mysql up --home /tmp
+./node_modules/.bin/usdocker --refresh --home /tmp
+./node_modules/.bin/usdocker mysql up --home /tmp
 
 docker run -it --rm \
     --link mysql-container \
@@ -278,7 +428,8 @@ docker run -it --rm \
 
 ```bash
 npm i @usdocker/usdocker @usdocker/postgres
-./node_modules/.bin/usdocker --refresh postgres up --home /tmp
+./node_modules/.bin/usdocker --refresh --home /tmp
+./node_modules/.bin/usdocker postgres up --home /tmp
 
 docker run -it --rm \
     --link postgres-container \
@@ -292,14 +443,15 @@ docker run -it --rm \
 
 ```bash
 npm i @usdocker/usdocker @usdocker/mssql
-./node_modules/.bin/usdocker --refresh mssql up --home /tmp
+./node_modules/.bin/usdocker --refresh --home /tmp
+./node_modules/.bin/usdocker mssql up --home /tmp
 
 docker run -it --rm \
     --link mssql-container \
     -v $PWD:/work \
     -w /work \
     byjg/php:7.2-cli \
-    phpunit tests/SqlserverDatabaseTest
+    phpunit tests/SqlServerDatabaseTest
 ```
 
 ## Related Projects

scripts/migrate

@@ -12,7 +12,7 @@ require_once($autoload);
 
 use Symfony\Component\Console\Application;
 
-$application = new Application('Migrate Script by JG', '2.0.4');
+$application = new Application('Migrate Script by JG', '2.1.0');
 $application->add(new \ByJG\DbMigration\Console\ResetCommand());
 $application->add(new \ByJG\DbMigration\Console\UpCommand());
 $application->add(new \ByJG\DbMigration\Console\DownCommand());

src/Console/ConsoleCommand.php

@@ -29,13 +29,13 @@ protected function configure()
             ->addOption(
                 'path',
                 'p',
-                InputOption::VALUE_OPTIONAL,
+                InputOption::VALUE_REQUIRED,
                 'Define the path where the base.sql resides. If not set assumes the current folder'
             )
             ->addOption(
                 'up-to',
                 'u',
-                InputOption::VALUE_OPTIONAL,
+                InputOption::VALUE_REQUIRED,
                 'Run up to the specified version'
             )
             ->addOption(
@@ -89,8 +89,10 @@ protected function initialize(InputInterface $input, OutputInterface $output)
 
         $requiredBase = !$input->getOption('no-base');
 
+        $migrationTable = (empty(getenv('MIGRATE_TABLE')) ? "migration_version" : getenv('MIGRATE_TABLE'));
+        $this->path = realpath($this->path);
         $uri = new Uri($this->connection);
-        $this->migration = new Migration($uri, $this->path, $requiredBase);
+        $this->migration = new Migration($uri, $this->path, $requiredBase, $migrationTable);
         $this->migration
             ->registerDatabase('sqlite', SqliteDatabase::class)
             ->registerDatabase('mysql', MySqlDatabase::class)

src/Console/ResetCommand.php

@@ -4,6 +4,7 @@
 
 use ByJG\DbMigration\Exception\ResetDisabledException;
 use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Question\ConfirmationQuestion;
 
@@ -15,7 +16,12 @@ protected function configure()
         $this
             ->setName('reset')
             ->setDescription('Create a fresh new database')
-            ->addOption('yes', null, null, 'Answer yes to any interactive question');
+            ->addOption(
+                'yes',
+                null,
+                InputOption::VALUE_NONE,
+                'Answer yes to any interactive question'
+            );
     }
 
     /**

src/Database/AbstractDatabase.php

@@ -19,15 +19,29 @@ abstract class AbstractDatabase implements DatabaseInterface
      * @var \Psr\Http\Message\UriInterface
      */
     private $uri;
+    /**
+     * @var string
+     */
+    private $migrationTable;
 
     /**
      * Command constructor.
      *
      * @param UriInterface $uri
+     * @param string $migrationTable
      */
-    public function __construct(UriInterface $uri)
+    public function __construct(UriInterface $uri, $migrationTable = 'migration_version')
     {
         $this->uri = $uri;
+        $this->migrationTable = $migrationTable;
+    }
+
+    /**
+     * @return string
+     */
+    public function getMigrationTable()
+    {
+        return $this->migrationTable;
     }
 
     /**
@@ -50,13 +64,13 @@ public function getVersion()
     {
         $result = [];
         try {
-            $result['version'] = $this->getDbDriver()->getScalar('SELECT version FROM migration_version');
+            $result['version'] = $this->getDbDriver()->getScalar('SELECT version FROM ' . $this->getMigrationTable());
         } catch (\Exception $ex) {
             throw new DatabaseNotVersionedException('This database does not have a migration version. Please use "migrate reset" or "migrate install" to create one.');
         }
 
         try {
-            $result['status'] = $this->getDbDriver()->getScalar('SELECT status FROM migration_version');
+            $result['status'] = $this->getDbDriver()->getScalar('SELECT status FROM ' . $this->getMigrationTable());
         } catch (\Exception $ex) {
             throw new OldVersionSchemaException('This database does not have a migration version. Please use "migrate install" for update it.');
         }
@@ -71,10 +85,10 @@ public function getVersion()
     public function setVersion($version, $status)
     {
         $this->getDbDriver()->execute(
-            'UPDATE migration_version SET version = :version, status = :status',
+            'UPDATE ' . $this->getMigrationTable() . ' SET version = :version, status = :status',
             [
                 'version' => $version,
-                'status' => $status
+                'status' => $status,
             ]
         );
     }
@@ -88,7 +102,7 @@ protected function checkExistsVersion()
         // Get the version to check if exists
         $versionInfo = $this->getVersion();
         if (empty($versionInfo['version'])) {
-            $this->getDbDriver()->execute("insert into migration_version values(0, 'unknow')");
+            $this->getDbDriver()->execute("insert into " . $this->getMigrationTable() . " values(0, 'unknow')");
         }
     }
 
@@ -97,8 +111,8 @@ protected function checkExistsVersion()
      */
     public function updateVersionTable()
     {
-        $currentVersion = $this->getDbDriver()->getScalar('select version from migration_version');
-        $this->getDbDriver()->execute('drop table migration_version');
+        $currentVersion = $this->getDbDriver()->getScalar('select version from ' . $this->getMigrationTable());
+        $this->getDbDriver()->execute('drop table ' . $this->getMigrationTable());
         $this->createVersion();
         $this->setVersion($currentVersion, 'unknow');
     }