Merge 5453816 into f6d41a4

zendframework · Oct 14, 2016 · 1419005 · 1419005
2 parents f6d41a4 + 5453816
commit 1419005
Show file tree

Hide file tree

Showing 12 changed files with 197 additions and 40 deletions.
diff --git a/phpcs.xml b/phpcs.xml
@@ -19,4 +19,5 @@
     <file>src</file>
     <file>test</file>
     <exclude-pattern>test/Injector/TestAsset/*.php</exclude-pattern>
+    <exclude-pattern>test/TestAsset/Module*.php</exclude-pattern>
 </ruleset>
diff --git a/src/ComponentInstaller.php b/src/ComponentInstaller.php
@@ -178,7 +178,7 @@ public function onPostPackageInstall(PackageEvent $event)
             return;
         }
 
-        $this->includeModuleClasses($package);
+        $dependencies = $this->loadModuleClassesDependencies($package);
         $applicationModules = $this->findApplicationModules();
 
         $this->marshalInstallableModules($extra, $options)
@@ -190,26 +190,32 @@ public function onPostPackageInstall(PackageEvent $event)
                 return $injectors;
             }, new Collection([]))
             // Inject modules into configuration
-            ->each(function ($injector, $module) use ($name, $packageTypes, $applicationModules) {
+            ->each(function ($injector, $module) use ($name, $packageTypes, $applicationModules, $dependencies) {
+                if (isset($dependencies[$module])) {
+                    $injector->setModuleDependencies($dependencies[$module]);
+                }
+
                 $injector->setApplicationModules($applicationModules);
                 $this->injectModuleIntoConfig($name, $module, $injector, $packageTypes[$module]);
             });
     }
 
     /**
-     * Find all Module classes in the package and include them.
-     * Module classes is used later
+     * Find all Module classes in the package and their dependencies
+     * - method `getModuleDependencies` of Module class.
+     *
+     * These dependencies are used later
      * @see \Zend\ComponentInstaller\Injector\AbstractInjector::injectAfterDependencies
-     * - to get package dependencies - module method `getModuleDependencies`
-     * and add component in a correct order on the module list.
+     * to add component in a correct order on the module list - after dependencies.
      *
      * It works with PSR-0, PSR-4, 'classmap' and 'files' composer autoloading.
      *
      * @param PackageInterface $package
-     * @return void
+     * @return array
      */
-    private function includeModuleClasses(PackageInterface $package)
+    private function loadModuleClassesDependencies(PackageInterface $package)
     {
+        $dependencies = [];
         $installer = $this->composer->getInstallationManager();
         $packagePath = $installer->getInstallPath($package);
 
@@ -255,10 +261,41 @@ private function includeModuleClasses(PackageInterface $package)
                 }
 
                 if (file_exists($modulePath)) {
-                    include $modulePath;
+                    if ($result = $this->getModuleDependencies($modulePath)) {
+                        $dependencies += $result;
+                    }
+                }
+            }
+        }
+
+        return $dependencies;
+    }
+
+    /**
+     * @param string $file
+     * @return array
+     */
+    private function getModuleDependencies($file)
+    {
+        $content = file_get_contents($file);
+        if (preg_match('/namespace\s+([^\s]+)\s*;/', $content, $m)) {
+            $moduleName = $m[1];
+
+            // @codingStandardsIgnoreStart
+            $regExp = '/public\s+function\s+getModuleDependencies\s*\(\s*\)\s*{[^}]*return\s*(?:array\(|\[)([^})\]]*)(\)|\])/';
+            // @codingStandardsIgnoreEnd
+            if (preg_match($regExp, $content, $m)) {
+                $dependencies = array_filter(
+                    explode(',', stripslashes(rtrim(preg_replace('/[\s"\']/', '', $m[1]), ',')))
+                );
+
+                if ($dependencies) {
+                    return [$moduleName => $dependencies];
                 }
             }
         }
+
+        return [];
     }
 
     /**

diff --git a/src/Injector/AbstractInjector.php b/src/Injector/AbstractInjector.php
@@ -108,6 +108,13 @@ abstract class AbstractInjector implements InjectorInterface
      */
     protected $applicationModules = [];
 
+    /**
+     * Dependencies of the module.
+     *
+     * @var array
+     */
+    protected $moduleDependencies = [];
+
     /**
      * Constructor
      *
@@ -161,6 +168,7 @@ public function inject($package, $type, IOInterface $io)
         }
 
         if ($type == self::TYPE_COMPONENT
+            && $this->moduleDependencies
             && $this->injectAfterDependencies($package, $config, $io)
         ) {
             return;
@@ -195,42 +203,32 @@ public function inject($package, $type, IOInterface $io)
      */
     private function injectAfterDependencies($package, $config, IOInterface $io)
     {
-        $moduleClass = sprintf('%s\\%s', $package, 'Module');
-        if (class_exists($moduleClass) && method_exists($moduleClass, 'getModuleDependencies')) {
-            $module = new $moduleClass();
-            $dependencies = $module->getModuleDependencies();
-
-            if ($dependencies) {
-                foreach ($dependencies as $dependency) {
-                    if (! $this->isRegisteredInConfig($dependency, $config)) {
-                        $io->write(sprintf(
-                            '<error>    Dependency %s is not registered in the configuration</error>',
-                            $dependency
-                        ));
-
-                        return true;
-                    }
-                }
-
-                $lastDependency = $this->findLastDependency($dependencies, $config);
-
-                $pattern = sprintf(
-                    $this->injectionPatterns[self::TYPE_DEPENDENCY]['pattern'],
-                    preg_quote($lastDependency, '/')
-                );
-                $replacement = sprintf(
-                    $this->injectionPatterns[self::TYPE_DEPENDENCY]['replacement'],
-                    $package
-                );
-
-                $config = preg_replace($pattern, $replacement, $config);
-                file_put_contents($this->configFile, $config);
+        foreach ($this->moduleDependencies as $dependency) {
+            if (! $this->isRegisteredInConfig($dependency, $config)) {
+                $io->write(sprintf(
+                    '<error>    Dependency %s is not registered in the configuration</error>',
+                    $dependency
+                ));
 
                 return true;
             }
         }
 
-        return false;
+        $lastDependency = $this->findLastDependency($this->moduleDependencies, $config);
+
+        $pattern = sprintf(
+            $this->injectionPatterns[self::TYPE_DEPENDENCY]['pattern'],
+            preg_quote($lastDependency, '/')
+        );
+        $replacement = sprintf(
+            $this->injectionPatterns[self::TYPE_DEPENDENCY]['replacement'],
+            $package
+        );
+
+        $config = preg_replace($pattern, $replacement, $config);
+        file_put_contents($this->configFile, $config);
+
+        return true;
     }
 
     /**
@@ -331,6 +329,16 @@ public function setApplicationModules(array $modules)
         return $this;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    public function setModuleDependencies(array $modules)
+    {
+        $this->moduleDependencies = $modules;
+
+        return $this;
+    }
+
     /**
      * Remove a package from the configuration.
      *

diff --git a/src/Injector/ConfigInjectorChain.php b/src/Injector/ConfigInjectorChain.php
@@ -137,4 +137,12 @@ public function setApplicationModules(array $modules)
     {
         return $this;
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function setModuleDependencies(array $modules)
+    {
+        return $this;
+    }
 }
diff --git a/src/Injector/InjectorInterface.php b/src/Injector/InjectorInterface.php
@@ -65,4 +65,12 @@ public function remove($package, IOInterface $io);
      * @return self
      */
     public function setApplicationModules(array $modules);
+
+    /**
+     * Set dependencies for the module.
+     *
+     * @param array $modules
+     * @return self
+     */
+    public function setModuleDependencies(array $modules);
 }
diff --git a/src/Injector/NoopInjector.php b/src/Injector/NoopInjector.php
@@ -58,4 +58,12 @@ public function setApplicationModules(array $modules)
     {
         return $this;
     }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function setModuleDependencies(array $modules)
+    {
+        return $this;
+    }
 }
diff --git a/test/ComponentInstallerTest.php b/test/ComponentInstallerTest.php
@@ -1445,4 +1445,32 @@ public function testPrependComponentAndAppendModule()
             'Some\Module',
         ], $modules);
     }
+
+    public function moduleClass()
+    {
+        return [
+            [__DIR__ . '/TestAsset/ModuleBadlyFormatted.php', ['BadlyFormatted\Application' => ['Dependency1']]],
+            [__DIR__ . '/TestAsset/ModuleWithDependencies.php', ['MyNamespace' => ['Dependency']]],
+            [__DIR__ . '/TestAsset/ModuleWithInterface.php', ['LongArray\Application' => ['Foo\D1', 'Bar\D2']]],
+            [__DIR__ . '/TestAsset/ModuleWithoutDependencies.php', []],
+            [__DIR__ . '/TestAsset/ModuleWithEmptyArrayDependencies.php', []],
+        ];
+    }
+
+    /**
+     * @dataProvider moduleClass
+     *
+     * @param string $file
+     * @param array $result
+     */
+    public function testGetModuleDependenciesFromModuleClass($file, $result)
+    {
+        $r = new \ReflectionObject($this->installer);
+        $rm = $r->getMethod('getModuleDependencies');
+        $rm->setAccessible(true);
+
+        $dependencies = $rm->invoke($this->installer, $file);
+
+        $this->assertEquals($result, $dependencies);
+    }
 }
diff --git a/test/TestAsset/ModuleBadlyFormatted.php b/test/TestAsset/ModuleBadlyFormatted.php
@@ -0,0 +1,14 @@
+<?php
+namespace
+  BadlyFormatted\Application
+
+;
+
+class Module{
+    public function
+    getModuleDependencies(){
+        return
+
+        [    "Dependency1" ];
+    }
+}
diff --git a/test/TestAsset/ModuleWithDependencies.php b/test/TestAsset/ModuleWithDependencies.php
@@ -0,0 +1,10 @@
+<?php
+namespace MyNamespace;
+
+class Module
+{
+    public function getModuleDependencies()
+    {
+        return ['Dependency'];
+    }
+}
diff --git a/test/TestAsset/ModuleWithEmptyArrayDependencies.php b/test/TestAsset/ModuleWithEmptyArrayDependencies.php
@@ -0,0 +1,10 @@
+<?php
+namespace Foo\NewApplication;
+
+class Module
+{
+    public function getModuleDependencies()
+    {
+        return [];
+    }
+}
diff --git a/test/TestAsset/ModuleWithInterface.php b/test/TestAsset/ModuleWithInterface.php
@@ -0,0 +1,15 @@
+<?php
+namespace LongArray\Application;
+
+use ClassMap\SomeInterface;
+
+class Module implements SomeInterface
+{
+    public function getModuleDependencies()
+    {
+        return array(
+            'Foo\\D1',
+            'Bar\\D2',
+        );
+    }
+}
diff --git a/test/TestAsset/ModuleWithoutDependencies.php b/test/TestAsset/ModuleWithoutDependencies.php
@@ -0,0 +1,10 @@
+<?php
+namespace MyNamespace;
+
+class Module
+{
+    public function getConfig()
+    {
+        return [];
+    }
+}