Merge pull request #190 from edefimov/sf4_migrations_fix

Fixed issues with SF4
liip · Aug 13, 2018 · 6c67280 · 6c67280
2 parents 7e7d2be + c6efe9a
commit 6c67280
Show file tree

Hide file tree

Showing 5 changed files with 139 additions and 36 deletions.
diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php
@@ -165,12 +165,10 @@ private function createGroupsNode()
                     ->end()
                     ->arrayNode('doctrine_migrations')
                         ->useAttributeAsKey('name')
-                        ->info('Check if doctrine migrations are applied')
+                        ->info('Checks to see if migrations from specified configuration file are applied')
                         ->prototype('array')
                             ->children()
                                 ->scalarNode('configuration_file')
-                                    ->isRequired()
-                                    ->cannotBeEmpty()
                                     ->info('Absolute path to doctrine migrations configuration')
                                 ->end()
                                 ->scalarNode('connection')
@@ -179,13 +177,36 @@ private function createGroupsNode()
                                     ->info('Connection name from doctrine DBAL configuration')
                                 ->end()
                             ->end()
+                            ->beforeNormalization()
+                                ->ifString()
+                                ->then(function ($value) {
+                                    if (is_string($value)) {
+                                        $value = [ 'connection' => $value ];
+                                    }
+
+                                    return $value;
+                                })
+                            ->end()
+                            ->validate()
+                                ->always(function ($value) {
+                                    if (is_array($value) && !isset($value['configuration_file']) && !class_exists('Doctrine\\Bundle\\MigrationsBundle\\Command\\DoctrineCommand')) {
+                                        throw new \InvalidArgumentException('You should explicitly define "configuration_file" parameter or install doctrine/doctrine-migrations-bundle to use empty parameter.');
+                                    }
+
+                                    return $value;
+                                })
+                            ->end()
                         ->end()
                         ->example(
                             [
                                 'application_migrations' => [
                                     'configuration_file' => '%kernel.root_dir%/Resources/config/migrations.yml',
                                     'connection'         => 'default'
-                                ]
+                                ],
+                                'migrations_with_doctrine_bundle' => [
+                                    'connection' => 'default'
+                                ],
+                                'migrations_with_doctrine_bundle_v2' => 'default',
                             ]
                         )
                     ->end()

diff --git a/DependencyInjection/LiipMonitorExtension.php b/DependencyInjection/LiipMonitorExtension.php
@@ -5,17 +5,35 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Driver\PDOSqlite\Driver;
 use Doctrine\DBAL\Migrations\Configuration\AbstractFileConfiguration;
-use Doctrine\DBAL\Migrations\Configuration\Configuration as MigrationConfiguration;
+use Doctrine\DBAL\Migrations\Configuration\Configuration as DoctrineMigrationConfiguration;
+use Doctrine\DBAL\Migrations\MigrationException;
+use Liip\MonitorBundle\DoctrineMigrations\Configuration as LiipMigrationConfiguration;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\DependencyInjection\ChildDefinition;
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\DefinitionDecorator;
 use Symfony\Component\DependencyInjection\Loader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\HttpKernel\DependencyInjection\Extension;
 
-class LiipMonitorExtension extends Extension
+class LiipMonitorExtension extends Extension implements CompilerPassInterface
 {
+    /**
+     * Tuple (migrationsConfiguration, tempConfiguration) for doctrine migrations check
+     *
+     * @var array
+     */
+    private $migrationConfigurationsServices = [];
+
+    /**
+     * Connection object needed for correct migration loading
+     *
+     * @var Connection
+     */
+    private $fakeConnection;
+
     /**
      * Loads the services based on your application configuration.
      *
@@ -24,6 +42,7 @@ class LiipMonitorExtension extends Extension
      */
     public function load(array $configs, ContainerBuilder $container)
     {
+        $this->fakeConnection = new Connection([], new Driver());
         $loader = new Loader\XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));
         $loader->load('runner.xml');
         $loader->load('helper.xml');
@@ -93,6 +112,22 @@ public function load(array $configs, ContainerBuilder $container)
         $this->configureDoctrineMigrationsCheck($container, $containerParams);
     }
 
+    /**
+     * @inheritDoc
+     */
+    public function process(ContainerBuilder $container)
+    {
+        foreach ($this->migrationConfigurationsServices as $services) {
+            list($configurationService, $configuration) = $services;
+            /** @var Definition $configurationService */
+            /** @var DoctrineMigrationConfiguration $configuration */
+            $versions = $this->getPredefinedMigrations($container, $configuration, $this->fakeConnection);
+            if ($versions) {
+                $configurationService->addMethodCall('registerMigrations', [ $versions ]);
+            }
+        }
+    }
+
     /**
      * @param ContainerBuilder $container
      * @param string           $checkName
@@ -142,10 +177,6 @@ private function setParameters(ContainerBuilder $container, $checkName, $group,
                     throw new \InvalidArgumentException('Please require at least "v1.0.6" of "ZendDiagnostics"');
                 }
 
-                if (!class_exists('Doctrine\Bundle\MigrationsBundle\Command\DoctrineCommand')) {
-                    throw new \InvalidArgumentException('Please require at least "v1.0.0" of "DoctrineMigrationsBundle"');
-                }
-
                 if (!class_exists('Doctrine\DBAL\Migrations\Configuration\Configuration')) {
                     throw new \InvalidArgumentException('Please require at least "v1.1.0" of "Doctrine Migrations Library"');
                 }
@@ -190,13 +221,26 @@ private function configureDoctrineMigrationsCheck(ContainerBuilder $container, a
 
             $services = [];
             foreach ($groupChecks['doctrine_migrations'] as $key => $config) {
-                $serviceConfiguration =
-                    $this->createMigrationConfigurationService($container, $config['configuration_file'], $config[ 'connection' ]);
-
-                $serviceId = sprintf('liip_monitor.check.doctrine_migrations.configuration.%s.%s', $groupName, $key);
-                $container->setDefinition($serviceId, $serviceConfiguration);
-
-                $services[$key] = $serviceId;
+                try {
+                    $serviceConfiguration =
+                        $this->createMigrationConfigurationService($container, $config[ 'connection' ], $config['configuration_file'] ?? null);
+
+                    $serviceId = sprintf('liip_monitor.check.doctrine_migrations.configuration.%s.%s', $groupName, $key);
+                    $container->setDefinition($serviceId, $serviceConfiguration);
+
+                    $services[$key] = $serviceId;
+                } catch (MigrationException $e) {
+                    throw new MigrationException(
+                        sprintf(
+                            'Invalid doctrine migration check under "%s.%s": %s',
+                            $groupName,
+                            $key,
+                            $e->getMessage()
+                        ),
+                        $e->getCode(),
+                        $e
+                    );
+                }
             }
 
             $parameter = sprintf('%s.check.%s.%s', $this->getAlias(), 'doctrine_migrations', $groupName);
@@ -207,18 +251,22 @@ private function configureDoctrineMigrationsCheck(ContainerBuilder $container, a
     /**
      * Return key-value array with migration version as key and class as a value defined in config file
      *
-     * @param AbstractFileConfiguration $config Current configuration
-     * @param Connection                $connection Fake connections
+     * @param ContainerBuilder               $container  The container
+     * @param DoctrineMigrationConfiguration $config     Current configuration
+     * @param Connection                     $connection Fake connections
      *
      * @return array[]
      */
-    private function getPredefinedMigrations(AbstractFileConfiguration $config, Connection $connection)
+    private function getPredefinedMigrations(ContainerBuilder $container, DoctrineMigrationConfiguration $config, Connection $connection)
     {
         $result = array();
 
-        $diff = new MigrationConfiguration($connection);
+        $diff = new LiipMigrationConfiguration($connection);
         $diff->setMigrationsNamespace($config->getMigrationsNamespace());
         $diff->setMigrationsDirectory($config->getMigrationsDirectory());
+        $diff->setContainer($container);
+        $diff->configure();
+
         foreach ($config->getMigrations() as $version) {
             $result[$version->getVersion()] = get_class($version->getMigration());
         }
@@ -234,23 +282,23 @@ private function getPredefinedMigrations(AbstractFileConfiguration $config, Conn
      * Creates migration configuration service definition
      *
      * @param ContainerBuilder $container      DI Container
-     * @param string           $filename       File name with migration configuration
      * @param string           $connectionName Connection name for container service
+     * @param string           $filename       File name with migration configuration
      *
      * @return DefinitionDecorator|ChildDefinition
      */
-    private function createMigrationConfigurationService(ContainerBuilder $container, $filename, $connectionName)
+    private function createMigrationConfigurationService(ContainerBuilder $container, string $connectionName, string $filename = null)
     {
-        /** @var AbstractFileConfiguration $configuration */
-        $connection    = new Connection([], new Driver()); // needed for correct migration loading
-        $configuration = $this->createTemporaryConfiguration($container, $connection, $filename);
+        $configuration = $this->createTemporaryConfiguration($container, $this->fakeConnection, $filename);
 
         $configurationServiceName = 'liip_monitor.check.doctrine_migrations.abstract_configuration';
         $serviceConfiguration = class_exists('Symfony\Component\DependencyInjection\ChildDefinition')
             ? new ChildDefinition($configurationServiceName)
             : new DefinitionDecorator($configurationServiceName)
         ;
 
+        $this->migrationConfigurationsServices[] = [$serviceConfiguration, $configuration];
+
         $serviceConfiguration->replaceArgument(
             0,
             new Reference(sprintf('doctrine.dbal.%s_connection', $connectionName))
@@ -292,19 +340,12 @@ private function createMigrationConfigurationService(ContainerBuilder $container
                 }
             }
 
-
             $serviceConfiguration->addMethodCall(
                 'setMigrationsDirectory',
                 [ $directory ]
             );
         }
 
-        /** @var AbstractFileConfiguration $diff */
-        $versions = $this->getPredefinedMigrations($configuration, $connection);
-        if ($versions) {
-            $serviceConfiguration->addMethodCall('registerMigrations', [ $versions ]);
-        }
-
         $serviceConfiguration->addMethodCall('configure', []);
 
         if ($configuration->areMigrationsOrganizedByYear()) {
@@ -327,10 +368,18 @@ private function createMigrationConfigurationService(ContainerBuilder $container
      * @param Connection       $connection Fake connection
      * @param string           $filename   Migrations configuration file
      *
-     * @return AbstractFileConfiguration
+     * @return DoctrineMigrationConfiguration
      */
-    private function createTemporaryConfiguration(ContainerBuilder $container, Connection $connection, $filename)
-    {
+    private function createTemporaryConfiguration(
+        ContainerBuilder $container,
+        Connection $connection,
+        string $filename = null
+    ): DoctrineMigrationConfiguration {
+        if ($filename === null) {
+            // this is configured from migrations bundle
+            return new DoctrineMigrationConfiguration($connection);
+        }
+
         // -------
         // This part must be in sync with Doctrine\DBAL\Migrations\Tools\Console\Helper\ConfigurationHelper::loadConfig
         $map = [

diff --git a/DoctrineMigrations/Configuration.php b/DoctrineMigrations/Configuration.php
@@ -11,6 +11,13 @@
  */
 class Configuration extends BaseConfiguration
 {
+    /**
+     * Flag whether doctrine migrations bundle is installed
+     *
+     * @var bool
+     */
+    private static $haveMigrationBundle;
+
     /**
      * Service container
      *
@@ -37,6 +44,14 @@ public function setContainer(ContainerInterface $container)
      */
     public function configure()
     {
+        if (self::$haveMigrationBundle === null) {
+            self::$haveMigrationBundle = class_exists(DoctrineCommand::class);
+        }
+
+        if (!self::$haveMigrationBundle) {
+            return;
+        }
+
         DoctrineCommand::configureMigrations($this->container, $this);
     }
 }
diff --git a/README.md b/README.md
@@ -316,6 +316,23 @@ liip_monitor:
                 # Connection name or an array of connection names
                 doctrine_dbal:        null # Example: [default, crm]
 
+                # Checks to see if migrations from specified configuration file are applied
+                doctrine_migrations:
+                    # Examples:
+                    application_migrations: 
+                        configuration_file:  %kernel.root_dir%/Resources/config/migrations.yml
+                        connection:          default
+                    migrations_with_doctrine_bundle: 
+                        connection:          default
+                    migrations_with_doctrine_bundle_v2: default
+
+                    # Prototype
+                    name:
+                        # Absolute path to doctrine migrations configuration
+                        configuration_file:   ~
+                        # Connection name from doctrine DBAL configuration
+                        connection:           ~ # Required
+
                 # Connection name or an array of connection names
                 doctrine_mongodb:        null # Example: [default, crm]
 

diff --git a/Resources/config/checks/doctrine_migrations.xml b/Resources/config/checks/doctrine_migrations.xml
@@ -15,6 +15,7 @@
 
         <service id="liip_monitor.check.doctrine_migrations.abstract_configuration"
                  class="Liip\MonitorBundle\DoctrineMigrations\Configuration"
+                 public="true"
                  abstract="true">
             <argument /> <!-- Connection -->