Loading a Module (either MVC or CLI) now throws an exception if the p…

…ath does not exists regardless of whether the class is already loaded.
phalcon · May 15, 2019 · dca688f · dca688f
1 parent bd1b05f
commit dca688f
Show file tree

Hide file tree

Showing 6 changed files with 87 additions and 21 deletions.
diff --git a/CHANGELOG-4.0.md b/CHANGELOG-4.0.md
@@ -68,6 +68,7 @@
 - `Phalcon\Logger\Formatter\Line::__construct()` service parameters must be a string or omitted.
 - `Phalcon\Logger\Formatter\Json::__construct()` service parameters must be a string or omitted.
 - Removed deprecated code from `Phalcon\Forms\Form::getMessages()`.
+- Loading a Module (either MVC or CLI) now throws an exception if the path does not exists regardless of whether the class is already loaded.
 
 ## Fixed
 - Fixed `Mvc\Collection::isInitialized()` now works as intended. [#13931](https://github.com/phalcon/cphalcon/pull/13931)

diff --git a/phalcon/Cli/Console.zep b/phalcon/Cli/Console.zep
@@ -98,13 +98,13 @@ class Console extends BaseApplication
             }
 
             if fetch path, module["path"] {
-                if !class_exists(className, false) {
-                    if unlikely !file_exists(path) {
-                        throw new Exception(
-                            "Module definition path '" . path . "' doesn't exist"
-                        );
-                    }
+                if unlikely !file_exists(path) {
+                    throw new Exception(
+                        "Module definition path '" . path . "' doesn't exist"
+                    );
+                }
 
+                if !class_exists(className, false) {
                     require path;
                 }
             }

diff --git a/phalcon/Mvc/Application.zep b/phalcon/Mvc/Application.zep
@@ -207,13 +207,13 @@ class Application extends BaseApplication
                  * If developer specify a path try to include the file
                  */
                 if fetch path, module["path"] {
-                    if !class_exists(className, false) {
-                        if unlikely !file_exists(path) {
-                            throw new Exception(
-                                "Module definition path '" . path . "' doesn't exist"
-                            );
-                        }
+                    if unlikely !file_exists(path) {
+                        throw new Exception(
+                            "Module definition path '" . path . "' doesn't exist"
+                        );
+                    }
 
+                    if !class_exists(className, false) {
                         require path;
                     }
                 }

diff --git a/tests/cli/Cli/Console/GetModuleCest.php b/tests/cli/Cli/Console/GetModuleCest.php
@@ -38,19 +38,19 @@ public function cliConsoleGetModule(CliTester $I)
         $console->registerModules(
             [
                 'frontend' => [
-                    'className' => 'Phalcon\\Test\\Modules\\Frontend\\Module',
-                    'path'      => __DIR__ . '/../../../_data/modules/frontend/Module.php',
+                    'className' => \Phalcon\Test\Modules\Frontend\Module::class,
+                    'path'      => dataDir('fixtures/modules/frontend/Module.php'),
                 ],
                 'backend'  => [
-                    'className' => 'Phalcon\\Test\\Modules\\Backend\\Module',
-                    'path'      => __DIR__ . '/../../../_data/modules/backend/Module.php',
+                    'className' => \Phalcon\Test\Modules\Backend\Module::class,
+                    'path'      => dataDir('fixtures/modules/backend/Module.php'),
                 ],
             ]
         );
 
         $expected = [
-            'className' => 'Phalcon\\Test\\Modules\\Frontend\\Module',
-            'path'      => __DIR__ . '/../../../_data/modules/frontend/Module.php',
+            'className' => \Phalcon\Test\Modules\Frontend\Module::class,
+            'path'      => dataDir('fixtures/modules/frontend/Module.php'),
         ];
 
         $I->assertEquals(

diff --git a/tests/cli/Cli/Console/RegisterModulesCest.php b/tests/cli/Cli/Console/RegisterModulesCest.php
@@ -39,7 +39,7 @@ public function cliConsoleRegisterModules(CliTester $I)
             [
                 'frontend' => [
                     'className' => Module::class,
-                    'path'      => __DIR__ . '/../../../_data/modules/frontend/Module.php',
+                    'path'      => dataDir('fixtures/modules/frontend/Module.php'),
                 ],
             ]
         );
@@ -58,7 +58,7 @@ public function cliConsoleRegisterModules(CliTester $I)
             [
                 'backend' => [
                     'className' => \Phalcon\Test\Modules\Backend\Module::class,
-                    'path'      => __DIR__ . '/../../../_data/modules/backend/Module.php',
+                    'path'      => dataDir('fixtures/modules/backend/Module.php'),
                 ],
             ]
         );
@@ -77,7 +77,7 @@ public function cliConsoleRegisterModules(CliTester $I)
             [
                 'frontend' => [
                     'className' => Module::class,
-                    'path'      => __DIR__ . '/../../../_data/modules/frontend/Module.php',
+                    'path'      => dataDir('fixtures/modules/frontend/Module.php'),
                 ],
             ],
             $merge = true
@@ -98,4 +98,31 @@ public function cliConsoleRegisterModules(CliTester $I)
             $console->getModules()
         );
     }
+
+    public function badPathThrowsAnException(CliTester $I)
+    {
+        $console = $this->newCliConsole();
+
+        $console->registerModules(
+            [
+                'frontend' => [
+                    'path'      => dataDir('not-a-real-file.php'),
+                    'className' => \Phalcon\Test\Modules\Frontend\Module::class,
+                ],
+            ]
+        );
+
+        $I->expectException(
+            new \Phalcon\Cli\Console\Exception(
+                "Module definition path 'not-a-real-file.php' doesn't exist"
+            ),
+            function () use ($console) {
+                $console->handle(
+                    [
+                        'task' => 'echo',
+                    ]
+                );
+            }
+        );
+    }
 }
diff --git a/tests/integration/Mvc/Application/RegisterModulesCest.php b/tests/integration/Mvc/Application/RegisterModulesCest.php
@@ -154,4 +154,42 @@ function () use ($view) {
             $response->getContent()
         );
     }
+
+    public function badPathThrowsAnException(IntegrationTester $I)
+    {
+        Di::reset();
+
+        $di = new FactoryDefault();
+
+        $di->set(
+            'router',
+            function () {
+                $router = new Router(false);
+
+                return $router;
+            }
+        );
+
+        $application = new Application();
+
+        $application->registerModules(
+            [
+                'frontend' => [
+                    'path'      => dataDir('not-a-real-file.php'),
+                    'className' => \Phalcon\Test\Modules\Frontend\Module::class,
+                ],
+            ]
+        );
+
+        $application->setDI($di);
+
+        $I->expectException(
+            new \Phalcon\Mvc\Application\Exception(
+                "Module definition path 'not-a-real-file.php' doesn't exist"
+            ),
+            function () use ($application) {
+                $response = $application->handle('/');
+            }
+        );
+    }
 }