Skip to content

Commit 5bbd9a4

Browse files
eddielaueddielau
committed
MAGETWO-42265: Fix code that depends on module location
- refactored Populator - fixed i18n unit tests - fixed i18n integration tests
1 parent 9d7d7bb commit 5bbd9a4

File tree

10 files changed

+162
-45
lines changed

10 files changed

+162
-45
lines changed

Diff for: ‎dev/tests/integration/framework/Magento/TestFramework/Application.php

+6-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\TestFramework;
77

88
use Magento\Framework\Autoload\AutoloaderInterface;
9+
use Magento\Framework\Component\ComponentRegistrar;
910
use Magento\Framework\Filesystem;
1011
use Magento\Framework\App\Filesystem\DirectoryList;
1112
use Magento\Framework\App\DeploymentConfig;
@@ -163,7 +164,11 @@ public function __construct(
163164

164165
$customDirs = $this->getCustomDirs();
165166
$this->dirList = new \Magento\Framework\App\Filesystem\DirectoryList(BP, $customDirs);
166-
\Magento\Framework\Autoload\Populator::populateMappings($autoloadWrapper, $this->dirList);
167+
\Magento\Framework\Autoload\Populator::populateMappings(
168+
$autoloadWrapper,
169+
$this->dirList,
170+
new ComponentRegistrar()
171+
);
167172
$this->_initParams = [
168173
\Magento\Framework\App\Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS => $customDirs,
169174
\Magento\Framework\App\State::PARAM_MODE => $appMode

Diff for: ‎dev/tests/integration/testsuite/Magento/Setup/Module/I18n/Dictionary/GeneratorTest.php

+27
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace Magento\Setup\Module\I18n\Dictionary;
77

8+
use Magento\Framework\Component\ComponentRegistrar;
89
use Magento\Setup\Module\I18n\ServiceLocator;
910

1011
class GeneratorTest extends \PHPUnit_Framework_TestCase
@@ -34,6 +35,11 @@ class GeneratorTest extends \PHPUnit_Framework_TestCase
3435
*/
3536
protected $generator;
3637

38+
/**
39+
* @var array
40+
*/
41+
protected $backupRegistrar;
42+
3743
protected function setUp()
3844
{
3945
$this->testDir = realpath(__DIR__ . '/_files');
@@ -42,6 +48,11 @@ protected function setUp()
4248
$this->outputFileName = $this->testDir . '/translate.csv';
4349
$this->generator = ServiceLocator::getDictionaryGenerator();
4450

51+
$reflection = new \ReflectionClass('Magento\Framework\Component\ComponentRegistrar');
52+
$paths = $reflection->getProperty('paths');
53+
$paths->setAccessible(true);
54+
$this->backupRegistrar = $paths->getValue();
55+
$paths->setAccessible(false);
4556
}
4657

4758
protected function tearDown()
@@ -53,6 +64,12 @@ protected function tearDown()
5364
$property->setAccessible(true);
5465
$property->setValue(null);
5566
$property->setAccessible(false);
67+
68+
$reflection = new \ReflectionClass('Magento\Framework\Component\ComponentRegistrar');
69+
$paths = $reflection->getProperty('paths');
70+
$paths->setAccessible(true);
71+
$paths->setValue($this->backupRegistrar);
72+
$paths->setAccessible(false);
5673
}
5774

5875
public function testGenerationWithoutContext()
@@ -64,6 +81,16 @@ public function testGenerationWithoutContext()
6481

6582
public function testGenerationWithContext()
6683
{
84+
ComponentRegistrar::register(
85+
ComponentRegistrar::MODULE,
86+
'Magento_FirstModule',
87+
$this->source . '/app/code/Magento/FirstModule'
88+
);
89+
ComponentRegistrar::register(
90+
ComponentRegistrar::MODULE,
91+
'Magento_SecondModule',
92+
$this->source . '/app/code/Magento/SecondModule'
93+
);
6794
$this->generator->generate($this->source, $this->outputFileName, true);
6895

6996
$this->assertFileEquals($this->expectedDir . '/with_context.csv', $this->outputFileName);

Diff for: ‎dev/tests/integration/testsuite/Magento/Setup/Module/I18n/Pack/GeneratorTest.php

+27
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace Magento\Setup\Module\I18n\Pack;
77

8+
use Magento\Framework\Component\ComponentRegistrar;
89
use Magento\Setup\Module\I18n\ServiceLocator;
910

1011
class GeneratorTest extends \PHPUnit_Framework_TestCase
@@ -44,6 +45,11 @@ class GeneratorTest extends \PHPUnit_Framework_TestCase
4445
*/
4546
protected $_generator;
4647

48+
/**
49+
* @var array
50+
*/
51+
protected $backupRegistrar;
52+
4753
protected function setUp()
4854
{
4955
$this->_testDir = realpath(__DIR__ . '/_files');
@@ -61,17 +67,38 @@ protected function setUp()
6167
$this->_generator = ServiceLocator::getPackGenerator();
6268

6369
\Magento\Framework\System\Dirs::rm($this->_packPath);
70+
71+
$reflection = new \ReflectionClass('Magento\Framework\Component\ComponentRegistrar');
72+
$paths = $reflection->getProperty('paths');
73+
$paths->setAccessible(true);
74+
$this->backupRegistrar = $paths->getValue();
75+
$paths->setAccessible(false);
6476
}
6577

6678
protected function tearDown()
6779
{
6880
\Magento\Framework\System\Dirs::rm($this->_packPath);
81+
$reflection = new \ReflectionClass('Magento\Framework\Component\ComponentRegistrar');
82+
$paths = $reflection->getProperty('paths');
83+
$paths->setAccessible(true);
84+
$paths->setValue($this->backupRegistrar);
85+
$paths->setAccessible(false);
6986
}
7087

7188
public function testGeneration()
7289
{
7390
$this->assertFileNotExists($this->_packPath);
7491

92+
ComponentRegistrar::register(
93+
ComponentRegistrar::MODULE,
94+
'Magento_FirstModule',
95+
BP . '/app/code/Magento/FirstModule'
96+
);
97+
ComponentRegistrar::register(
98+
ComponentRegistrar::MODULE,
99+
'Magento_SecondModule',
100+
BP. '/app/code/Magento/SecondModule'
101+
);
75102
$this->_generator->generate($this->_dictionaryPath, $this->_packPath, $this->_locale);
76103

77104
foreach ($this->_expectedFiles as $file) {

Diff for: ‎lib/internal/Magento/Framework/App/Bootstrap.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Framework\AppInterface;
1111
use Magento\Framework\Autoload\AutoloaderRegistry;
1212
use Magento\Framework\Autoload\Populator;
13+
use Magento\Framework\Component\ComponentRegistrar;
1314
use Magento\Framework\Filesystem\DriverPool;
1415
use Magento\Framework\Profiler;
1516
use Magento\Framework\Config\File\ConfigFilePool;
@@ -134,7 +135,7 @@ public static function populateAutoloader($rootDir, $initParams)
134135
{
135136
$dirList = self::createFilesystemDirectoryList($rootDir, $initParams);
136137
$autoloadWrapper = AutoloaderRegistry::getAutoloader();
137-
Populator::populateMappings($autoloadWrapper, $dirList);
138+
Populator::populateMappings($autoloadWrapper, $dirList, new ComponentRegistrar());
138139
}
139140

140141
/**

Diff for: ‎lib/internal/Magento/Framework/Autoload/Populator.php

+12-6
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
namespace Magento\Framework\Autoload;
77

88
use Magento\Framework\App\Filesystem\DirectoryList;
9-
use Magento\Framework\Autoload\AutoloaderInterface;
9+
use Magento\Framework\Component\ComponentRegistrar;
1010
use Magento\Framework\Filesystem\FileResolver;
1111

1212
/**
@@ -16,18 +16,24 @@
1616
class Populator
1717
{
1818
/**
19-
* @param AutoloaderInterface $registry
19+
* @param AutoloaderInterface $autoloader
2020
* @param DirectoryList $dirList
21+
* @param ComponentRegistrar $componentRegistrar
2122
* @return void
2223
*/
23-
public static function populateMappings(AutoloaderInterface $autoloader, DirectoryList $dirList)
24-
{
25-
$modulesDir = $dirList->getPath(DirectoryList::ROOT) . '/app/code';
24+
public static function populateMappings(
25+
AutoloaderInterface $autoloader,
26+
DirectoryList $dirList,
27+
ComponentRegistrar $componentRegistrar
28+
) {
2629
$generationDir = $dirList->getPath(DirectoryList::GENERATION);
2730
$frameworkDir = $dirList->getPath(DirectoryList::LIB_INTERNAL);
2831

29-
$autoloader->addPsr4('Magento\\', [$modulesDir . '/Magento/', $generationDir . '/Magento/'], true);
32+
foreach ($componentRegistrar->getPaths(ComponentRegistrar::MODULE) as $moduleName => $moduleDir) {
33+
$autoloader->addPsr4(str_replace('_', '\\', $moduleName) . '\\', [$moduleDir . '/'], true);
34+
}
3035

36+
$autoloader->addPsr4('Magento\\', [$generationDir . '/Magento/'], true);
3137
$autoloader->addPsr0('Apache_', $frameworkDir, true);
3238
$autoloader->addPsr0('Cm_', $frameworkDir, true);
3339
$autoloader->addPsr0('Credis_', $frameworkDir, true);

Diff for: ‎lib/internal/Magento/Framework/Autoload/Test/Unit/PopulatorTest.php

+26-13
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ class PopulatorTest extends \PHPUnit_Framework_TestCase
1414
/** @var \Magento\Framework\App\Filesystem\DirectoryList | \PHPUnit_Framework_MockObject_MockObject */
1515
protected $mockDirectoryList;
1616

17+
/**
18+
* @var \Magento\Framework\Component\ComponentRegistrar|\PHPUnit_Framework_MockObject_MockObject
19+
*/
20+
protected $componentRegistrar;
21+
1722
public function setUp()
1823
{
1924
$this->mockDirectoryList = $this->getMockBuilder('\Magento\Framework\App\Filesystem\DirectoryList')
@@ -23,6 +28,8 @@ public function setUp()
2328
$this->mockDirectoryList->expects($this->any())
2429
->method('getPath')
2530
->willReturnArgument(0);
31+
32+
$this->componentRegistrar = $this->getMock('Magento\Framework\Component\ComponentRegistrar', [], [], '', false);
2633
}
2734

2835
public function testPopulateMappings()
@@ -31,32 +38,38 @@ public function testPopulateMappings()
3138
->disableOriginalConstructor()
3239
->getMock();
3340

34-
$mockAutoloader->expects($this->at(0))
41+
$mockAutoloader->expects($this->at(0))->method('addPsr4')->with('Magento\\A\\', ['/path/to/a/'], true);
42+
$mockAutoloader->expects($this->at(1))->method('addPsr4')->with('Magento\\B\\', ['/path/to/b/'], true);
43+
$mockAutoloader->expects($this->at(2))->method('addPsr4')->with('Magento\\C\\', ['/path/to/c/'], true);
44+
$mockAutoloader->expects($this->at(3))
3545
->method('addPsr4')
36-
->with(
37-
'Magento\\',
38-
[DirectoryList::ROOT . '/app/code/Magento/', DirectoryList::GENERATION . '/Magento/'],
39-
true
40-
);
41-
$mockAutoloader->expects($this->at(1))
46+
->with('Magento\\', [DirectoryList::GENERATION . '/Magento/'], true);
47+
$mockAutoloader->expects($this->at(4))
4248
->method('addPsr0')
4349
->with('Apache_', DirectoryList::LIB_INTERNAL, true);
44-
$mockAutoloader->expects($this->at(2))
50+
$mockAutoloader->expects($this->at(5))
4551
->method('addPsr0')
4652
->with('Cm_', DirectoryList::LIB_INTERNAL, true);
47-
$mockAutoloader->expects($this->at(3))
53+
$mockAutoloader->expects($this->at(6))
4854
->method('addPsr0')
4955
->with('Credis_', DirectoryList::LIB_INTERNAL, true);
50-
$mockAutoloader->expects($this->at(4))
56+
$mockAutoloader->expects($this->at(7))
5157
->method('addPsr0')
5258
->with('Less_', DirectoryList::LIB_INTERNAL, true);
53-
$mockAutoloader->expects($this->at(5))
59+
$mockAutoloader->expects($this->at(8))
5460
->method('addPsr0')
5561
->with('Symfony\\', DirectoryList::LIB_INTERNAL, true);
56-
$mockAutoloader->expects($this->at(6))
62+
$mockAutoloader->expects($this->at(9))
5763
->method('addPsr0')
5864
->with('', [DirectoryList::GENERATION]);
5965

60-
Populator::populateMappings($mockAutoloader, $this->mockDirectoryList);
66+
$moduleDirs = [
67+
'Magento_A' => '/path/to/a',
68+
'Magento_B' => '/path/to/b',
69+
'Magento_C' => '/path/to/c',
70+
];
71+
$this->componentRegistrar->expects($this->once())->method('getPaths')->willReturn($moduleDirs);
72+
73+
Populator::populateMappings($mockAutoloader, $this->mockDirectoryList, $this->componentRegistrar);
6174
}
6275
}

Diff for: ‎setup/src/Magento/Setup/Module/I18n/Context.php

+22-4
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,8 @@ public function __construct(ComponentRegistrar $componentRegistrar)
5757
*/
5858
public function getContextByPath($path)
5959
{
60-
$moduleDirs = $this->componentRegistrar->getPaths(ComponentRegistrar::MODULE);
61-
$invertedModuleDirsMap = array_flip($moduleDirs);
62-
if (isset($invertedModuleDirsMap[$path])) {
60+
if ($value = $this->getModuleName($path)) {
6361
$type = self::CONTEXT_TYPE_MODULE;
64-
$value = $invertedModuleDirsMap[$path];
6562
} elseif ($value = strstr($path, '/app/design/')) {
6663
$type = self::CONTEXT_TYPE_THEME;
6764
$value = explode('/', $value);
@@ -75,6 +72,27 @@ public function getContextByPath($path)
7572
return [$type, $value];
7673
}
7774

75+
/**
76+
* Try to get module name by path, return false if not a module
77+
*
78+
* @param string $path
79+
* @return bool|string
80+
*/
81+
private function getModuleName($path)
82+
{
83+
$moduleDirs = $this->componentRegistrar->getPaths(ComponentRegistrar::MODULE);
84+
$invertedModuleDirsMap = array_flip($moduleDirs);
85+
$parts = explode('/', $path);
86+
while (!empty($parts)) {
87+
array_pop($parts);
88+
$partialPath = implode('/', $parts);
89+
if (isset($invertedModuleDirsMap[$partialPath])) {
90+
return $invertedModuleDirsMap[$partialPath];
91+
}
92+
}
93+
return false;
94+
}
95+
7896
/**
7997
* Get paths by context
8098
*

Diff for: ‎setup/src/Magento/Setup/Module/I18n/Dictionary/Options/Resolver.php

+3-9
Original file line numberDiff line numberDiff line change
@@ -33,26 +33,18 @@ class Resolver implements ResolverInterface
3333
*/
3434
protected $componentRegistrar;
3535

36-
/**
37-
* @var DirectoryList
38-
*/
39-
protected $directoryList;
40-
4136
/**
4237
* Resolver construct
4338
*
44-
* @param DirectoryList $directoryList
4539
* @param ComponentRegistrar $componentRegistrar
4640
* @param string $directory
4741
* @param bool $withContext
4842
*/
4943
public function __construct(
50-
DirectoryList $directoryList,
5144
ComponentRegistrar $componentRegistrar,
5245
$directory,
5346
$withContext
5447
) {
55-
$this->directoryList = $directoryList;
5648
$this->componentRegistrar = $componentRegistrar;
5749
$this->directory = $directory;
5850
$this->withContext = $withContext;
@@ -68,7 +60,9 @@ public function getOptions()
6860
$this->directory = rtrim($this->directory, '\\/');
6961
$moduleDirs = [];
7062
foreach ($this->componentRegistrar->getPaths(ComponentRegistrar::MODULE) as $moduleDir) {
71-
$moduleDirs[] = str_replace($this->directoryList->getRoot(), $this->directory, $moduleDir) . '/';
63+
if (strstr($moduleDir, $this->directory)) {
64+
$moduleDirs[] = $moduleDir . '/';
65+
}
7266
}
7367
$this->options = [
7468
[

Diff for: ‎setup/src/Magento/Setup/Module/I18n/Dictionary/Options/ResolverFactory.php

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* See COPYING.txt for license details.
55
*/
66
namespace Magento\Setup\Module\I18n\Dictionary\Options;
7+
use Magento\Framework\Component\ComponentRegistrar;
78

89
/**
910
* Options resolver factory
@@ -36,7 +37,7 @@ public function __construct($resolverClass = self::DEFAULT_RESOLVER)
3637
*/
3738
public function create($directory, $withContext)
3839
{
39-
$resolver = new $this->resolverClass($directory, $withContext);
40+
$resolver = new $this->resolverClass(new ComponentRegistrar(), $directory, $withContext);
4041
if (!$resolver instanceof ResolverInterface) {
4142
throw new \InvalidArgumentException($this->resolverClass . ' doesn\'t implement ResolverInterface');
4243
}

0 commit comments

Comments
 (0)