Skip to content

Commit

Permalink
Test for a Module class before testing for a class named after the …
Browse files Browse the repository at this point in the history
…module

Per zendframework#5, non-namespaced classes can create issues within applications if
they conflict with a module. In particular, `Generator` is one (as the
class is non-instantiable via `new`), but this can also happen in legacy
applications where a new module may exist under the same name as a
defined class.

This patch adds tests to ensure that the resolver prefers `Module`
classes if they exist, and never loads known uninstantiable classes that
always exist (specifically, `Generator`).
  • Loading branch information
weierophinney committed Nov 1, 2017
1 parent 25c4bc2 commit d6ae1b4
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 8 deletions.
4 changes: 3 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,12 @@
},
"autoload-dev": {
"files": [
"test/autoload.php"
"test/autoload.php",
"test/TestAsset/ModuleAsClass.php"
],
"psr-4": {
"ListenerTestModule\\": "test/TestAsset/ListenerTestModule/",
"ModuleAsClass\\": "test/TestAsset/ModuleAsClass/",
"ZendTest\\ModuleManager\\": "test/"
}
},
Expand Down
5 changes: 5 additions & 0 deletions phpcs.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,9 @@
<!-- Paths to check -->
<file>src</file>
<file>test</file>

<!-- Exclusions -->
<rule ref="PSR1.Classes.ClassDeclaration">
<exclude-pattern>*/test/TestAsset/ModuleAsClass.php</exclude-pattern>
</rule>
</ruleset>
25 changes: 18 additions & 7 deletions src/Listener/ModuleResolverListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,23 @@

namespace Zend\ModuleManager\Listener;

use Generator;
use Zend\ModuleManager\ModuleEvent;

/**
* Module resolver listener
*/
class ModuleResolverListener extends AbstractListener
{
/**
* Class names that are invalid as module classes, due to inability to instantiate.
*
* @var string[]
*/
protected $invalidClassNames = [
Generator::class,
];

/**
* @param ModuleEvent $e
* @return object|false False if module class does not exist
Expand All @@ -24,16 +34,17 @@ public function __invoke(ModuleEvent $e)
{
$moduleName = $e->getModuleName();

if (class_exists($moduleName)) {
return new $moduleName;
$class = sprintf('%s\Module', $moduleName);
if (class_exists($class)) {
return new $class;
}

$class = $moduleName . '\Module';

if (! class_exists($class)) {
return false;
if (class_exists($moduleName)
&& ! in_array($moduleName, $this->invalidClassNames, true)
) {
return new $moduleName;
}

return new $class;
return false;
}
}
19 changes: 19 additions & 0 deletions test/Listener/ModuleResolverListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
namespace ZendTest\ModuleManager\Listener;

use ListenerTestModule;
use ModuleAsClass;
use Zend\ModuleManager\Listener\ModuleResolverListener;
use Zend\ModuleManager\ModuleEvent;

Expand Down Expand Up @@ -49,4 +50,22 @@ public function testModuleResolverListenerReturnFalseIfCannotResolveModuleClasse
$e->setModuleName('DoesNotExist');
$this->assertFalse($moduleResolver($e));
}

public function testModuleResolverListenerPrefersModuleClassesInModuleNamespaceOverNamedClasses()
{
$moduleResolver = new ModuleResolverListener;
$e = new ModuleEvent;

$e->setModuleName('ModuleAsClass');
$this->assertInstanceOf(ModuleAsClass\Module::class, $moduleResolver($e));
}

public function testModuleResolverListenerWillNotAttemptToResolveModuleAsClassNameGenerator()
{
$moduleResolver = new ModuleResolverListener;
$e = new ModuleEvent;

$e->setModuleName('Generator');
$this->assertFalse($moduleResolver($e));
}
}
10 changes: 10 additions & 0 deletions test/TestAsset/ModuleAsClass.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<?php
/**
* @see https://github.com/zendframework/zend-modulemanager for the canonical source repository
* @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)
* @license https://github.com/zendframework/zend-modulemanager/blob/master/LICENSE.md New BSD License
*/

class ModuleAsClass
{
}
12 changes: 12 additions & 0 deletions test/TestAsset/ModuleAsClass/Module.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php
/**
* @see https://github.com/zendframework/zend-modulemanager for the canonical source repository
* @copyright Copyright (c) 2017 Zend Technologies USA Inc. (http://www.zend.com)
* @license https://github.com/zendframework/zend-modulemanager/blob/master/LICENSE.md New BSD License
*/

namespace ModuleAsClass;

class Module
{
}

0 comments on commit d6ae1b4

Please sign in to comment.