Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 22edf02

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
feature #32845 [HttpKernel][FrameworkBundle] Add alternative convention for bundle directories (yceruto)
This PR was squashed before being merged into the 4.4 branch (closes #32845). Discussion ---------- [HttpKernel][FrameworkBundle] Add alternative convention for bundle directories | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | no | New feature? | yes | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | symfony/symfony#32453 | License | MIT | Doc PR | TODO We already know that bundles must be compatible with many Symfony's versions, so it is very likely that current bundles won't be able to use this feature soon, unless they create symbolic links to support both structures. The point is that this is already happening, so in the future when our bundles stop to support <=4.3 then you'll be sure to change the current directory structure. We have recently added the `getPublicDir()` method in symfony/symfony#31975, here I'm removing it in favor of hardcoding a new convention. I've added some functional tests in which I've changed everything to this structure: ``` -- ModernBundle |-- config/ |-- public/ |-- src/ |-- ModernBundle.php |-- templates/ |-- translations/ ``` WDYT? Commits ------- 6996e1cbe2 [HttpKernel][FrameworkBundle] Add alternative convention for bundle directories
2 parents 454e3d5 + 5d94304 commit 22edf02
Copy full SHA for 22edf02

File tree

Expand file treeCollapse file tree

21 files changed

+248
-24
lines changed
Filter options
Expand file treeCollapse file tree

21 files changed

+248
-24
lines changed

‎Command/AssetsInstallCommand.php

Copy file name to clipboardExpand all lines: Command/AssetsInstallCommand.php
+1-7Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
137137
$validAssetDirs = [];
138138
/** @var BundleInterface $bundle */
139139
foreach ($kernel->getBundles() as $bundle) {
140-
if (!method_exists($bundle, 'getPublicDir')) {
141-
@trigger_error(sprintf('Not defining "getPublicDir()" method in the "%s" class is deprecated since Symfony 4.4 and will not be supported in 5.0.', \get_class($bundle)), E_USER_DEPRECATED);
142-
$publicDir = 'Resources/public';
143-
} else {
144-
$publicDir = ltrim($bundle->getPublicDir(), '\\/');
145-
}
146-
if (!is_dir($originDir = $bundle->getPath().\DIRECTORY_SEPARATOR.$publicDir)) {
140+
if (!is_dir($originDir = $bundle->getPath().'/Resources/public') && !is_dir($originDir = $bundle->getPath().'/public')) {
147141
continue;
148142
}
149143

‎Command/TranslationDebugCommand.php

Copy file name to clipboardExpand all lines: Command/TranslationDebugCommand.php
+6-4Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,9 @@ protected function execute(InputInterface $input, OutputInterface $output)
162162
if (null !== $input->getArgument('bundle')) {
163163
try {
164164
$bundle = $kernel->getBundle($input->getArgument('bundle'));
165-
$transPaths = [$bundle->getPath().'/Resources/translations'];
165+
$bundleDir = $bundle->getPath();
166+
$transPaths = [is_dir($bundleDir.'/Resources/translations') ? $bundleDir.'/Resources/translations' : $bundleDir.'/translations'];
167+
$viewsPaths = [is_dir($bundleDir.'/Resources/views') ? $bundleDir.'/Resources/views' : $bundleDir.'/templates'];
166168
if ($this->defaultTransPath) {
167169
$transPaths[] = $this->defaultTransPath;
168170
}
@@ -171,7 +173,6 @@ protected function execute(InputInterface $input, OutputInterface $output)
171173
$notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $dir, $bundle->getName());
172174
@trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath) : 'configure and use "framework.translator.default_path" instead.'), E_USER_DEPRECATED);
173175
}
174-
$viewsPaths = [$bundle->getPath().'/Resources/views'];
175176
if ($this->defaultViewsPath) {
176177
$viewsPaths[] = $this->defaultViewsPath;
177178
}
@@ -206,13 +207,14 @@ protected function execute(InputInterface $input, OutputInterface $output)
206207
}
207208
} elseif ($input->getOption('all')) {
208209
foreach ($kernel->getBundles() as $bundle) {
209-
$transPaths[] = $bundle->getPath().'/Resources/translations';
210+
$bundleDir = $bundle->getPath();
211+
$transPaths[] = is_dir($bundleDir.'/Resources/translations') ? $bundleDir.'/Resources/translations' : $bundle->getPath().'/translations';
212+
$viewsPaths[] = is_dir($bundleDir.'/Resources/views') ? $bundleDir.'/Resources/views' : $bundle->getPath().'/templates';
210213
if (is_dir($deprecatedPath = sprintf('%s/Resources/%s/translations', $rootDir, $bundle->getName()))) {
211214
$transPaths[] = $deprecatedPath;
212215
$notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $bundle->getName(), $deprecatedPath);
213216
@trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath) : 'configure and use "framework.translator.default_path" instead.'), E_USER_DEPRECATED);
214217
}
215-
$viewsPaths[] = $bundle->getPath().'/Resources/views';
216218
if (is_dir($deprecatedPath = sprintf('%s/Resources/%s/views', $rootDir, $bundle->getName()))) {
217219
$viewsPaths[] = $deprecatedPath;
218220
$notice = sprintf('Loading Twig templates for "%s" from the "%s" directory is deprecated since Symfony 4.2, ', $bundle->getName(), $deprecatedPath);

‎Command/TranslationUpdateCommand.php

Copy file name to clipboardExpand all lines: Command/TranslationUpdateCommand.php
+3-2Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,9 @@ protected function execute(InputInterface $input, OutputInterface $output)
154154
if (null !== $input->getArgument('bundle')) {
155155
try {
156156
$foundBundle = $kernel->getBundle($input->getArgument('bundle'));
157-
$transPaths = [$foundBundle->getPath().'/Resources/translations'];
157+
$bundleDir = $foundBundle->getPath();
158+
$transPaths = [is_dir($bundleDir.'/Resources/translations') ? $bundleDir.'/Resources/translations' : $bundleDir.'/translations'];
159+
$viewsPaths = [is_dir($bundleDir.'/Resources/views') ? $bundleDir.'/Resources/views' : $bundleDir.'/templates'];
158160
if ($this->defaultTransPath) {
159161
$transPaths[] = $this->defaultTransPath;
160162
}
@@ -163,7 +165,6 @@ protected function execute(InputInterface $input, OutputInterface $output)
163165
$notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $foundBundle->getName(), $dir);
164166
@trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath) : 'configure and use "framework.translator.default_path" instead.'), E_USER_DEPRECATED);
165167
}
166-
$viewsPaths = [$foundBundle->getPath().'/Resources/views'];
167168
if ($this->defaultViewsPath) {
168169
$viewsPaths[] = $this->defaultViewsPath;
169170
}

‎DependencyInjection/FrameworkExtension.php

Copy file name to clipboardExpand all lines: DependencyInjection/FrameworkExtension.php
+11-11Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,7 +1156,7 @@ private function registerTranslatorConfiguration(array $config, ContainerBuilder
11561156
$defaultDir = $container->getParameterBag()->resolveValue($config['default_path']);
11571157
$rootDir = $container->getParameter('kernel.root_dir');
11581158
foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) {
1159-
if ($container->fileExists($dir = $bundle['path'].'/Resources/translations')) {
1159+
if ($container->fileExists($dir = $bundle['path'].'/Resources/translations') || $container->fileExists($dir = $bundle['path'].'/translations')) {
11601160
$dirs[] = $dir;
11611161
} else {
11621162
$nonExistingDirs[] = $dir;
@@ -1318,20 +1318,20 @@ private function registerValidatorMapping(ContainerBuilder $container, array $co
13181318
}
13191319

13201320
foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
1321-
$dirname = $bundle['path'];
1321+
$configDir = is_dir($bundle['path'].'/Resources/config') ? $bundle['path'].'/Resources/config' : $bundle['path'].'/config';
13221322

13231323
if (
1324-
$container->fileExists($file = $dirname.'/Resources/config/validation.yaml', false) ||
1325-
$container->fileExists($file = $dirname.'/Resources/config/validation.yml', false)
1324+
$container->fileExists($file = $configDir.'/validation.yaml', false) ||
1325+
$container->fileExists($file = $configDir.'/validation.yml', false)
13261326
) {
13271327
$fileRecorder('yml', $file);
13281328
}
13291329

1330-
if ($container->fileExists($file = $dirname.'/Resources/config/validation.xml', false)) {
1330+
if ($container->fileExists($file = $configDir.'/validation.xml', false)) {
13311331
$fileRecorder('xml', $file);
13321332
}
13331333

1334-
if ($container->fileExists($dir = $dirname.'/Resources/config/validation', '/^$/')) {
1334+
if ($container->fileExists($dir = $configDir.'/validation', '/^$/')) {
13351335
$this->registerMappingFilesFromDir($dir, $fileRecorder);
13361336
}
13371337
}
@@ -1512,20 +1512,20 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
15121512
};
15131513

15141514
foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
1515-
$dirname = $bundle['path'];
1515+
$configDir = is_dir($bundle['path'].'/Resources/config') ? $bundle['path'].'/Resources/config' : $bundle['path'].'/config';
15161516

1517-
if ($container->fileExists($file = $dirname.'/Resources/config/serialization.xml', false)) {
1517+
if ($container->fileExists($file = $configDir.'/serialization.xml', false)) {
15181518
$fileRecorder('xml', $file);
15191519
}
15201520

15211521
if (
1522-
$container->fileExists($file = $dirname.'/Resources/config/serialization.yaml', false) ||
1523-
$container->fileExists($file = $dirname.'/Resources/config/serialization.yml', false)
1522+
$container->fileExists($file = $configDir.'/serialization.yaml', false) ||
1523+
$container->fileExists($file = $configDir.'/serialization.yml', false)
15241524
) {
15251525
$fileRecorder('yml', $file);
15261526
}
15271527

1528-
if ($container->fileExists($dir = $dirname.'/Resources/config/serialization', '/^$/')) {
1528+
if ($container->fileExists($dir = $configDir.'/serialization', '/^$/')) {
15291529
$this->registerMappingFilesFromDir($dir, $fileRecorder);
15301530
}
15311531
}

‎Tests/Functional/Bundle/LegacyBundle/Entity/LegacyPerson.php

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity;
13+
14+
class LegacyPerson
15+
{
16+
public $name;
17+
public $age;
18+
19+
public function __construct(string $name, string $age)
20+
{
21+
$this->name = $name;
22+
$this->age = $age;
23+
}
24+
}

‎Tests/Functional/Bundle/LegacyBundle/LegacyBundle.php

Copy file name to clipboard
+18Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle;
13+
14+
use Symfony\Component\HttpKernel\Bundle\Bundle;
15+
16+
class LegacyBundle extends Bundle
17+
{
18+
}

‎Tests/Functional/Bundle/LegacyBundle/Resources/config/serialization.yaml

Copy file name to clipboard
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity\LegacyPerson:
2+
attributes:
3+
name:
4+
serialized_name: 'full_name'

‎Tests/Functional/Bundle/LegacyBundle/Resources/config/validation.yaml

Copy file name to clipboard
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity\LegacyPerson:
2+
properties:
3+
age:
4+
- GreaterThan:
5+
value: 18

‎Tests/Functional/Bundle/LegacyBundle/Resources/public/legacy.css

Copy file name to clipboardExpand all lines: Tests/Functional/Bundle/LegacyBundle/Resources/public/legacy.css
Whitespace-only changes.

‎Tests/Functional/Bundle/LegacyBundle/Resources/translations/legacy.en.yaml

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ok_label: OK

‎Tests/Functional/Bundle/LegacyBundle/Resources/views/index.html.twig

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
OK

‎Tests/Functional/Bundle/ModernBundle/config/serialization.yaml

Copy file name to clipboard
+4Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity\ModernPerson:
2+
attributes:
3+
name:
4+
serialized_name: 'full_name'

‎Tests/Functional/Bundle/ModernBundle/config/validation.yaml

Copy file name to clipboard
+5Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity\ModernPerson:
2+
properties:
3+
age:
4+
- GreaterThan:
5+
value: 18

‎Tests/Functional/Bundle/ModernBundle/public/modern.css

Copy file name to clipboardExpand all lines: Tests/Functional/Bundle/ModernBundle/public/modern.css
Whitespace-only changes.

‎Tests/Functional/Bundle/ModernBundle/src/Entity/ModernPerson.php

Copy file name to clipboard
+24Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity;
13+
14+
class ModernPerson
15+
{
16+
public $name;
17+
public $age;
18+
19+
public function __construct(string $name, string $age)
20+
{
21+
$this->name = $name;
22+
$this->age = $age;
23+
}
24+
}

‎Tests/Functional/Bundle/ModernBundle/src/ModernBundle.php

Copy file name to clipboard
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src;
13+
14+
use Symfony\Component\HttpKernel\Bundle\Bundle;
15+
16+
class ModernBundle extends Bundle
17+
{
18+
public function getPath(): string
19+
{
20+
return \dirname(__DIR__);
21+
}
22+
}

‎Tests/Functional/Bundle/ModernBundle/templates/index.html.twig

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
OK

‎Tests/Functional/Bundle/ModernBundle/translations/modern.en.yaml

Copy file name to clipboard
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ok_label: OK

‎Tests/Functional/BundlePathsTest.php

Copy file name to clipboard
+84Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Bundle\FrameworkBundle\Tests\Functional;
13+
14+
use Symfony\Bundle\FrameworkBundle\Command\AssetsInstallCommand;
15+
use Symfony\Bundle\FrameworkBundle\Console\Application;
16+
use Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity\LegacyPerson;
17+
use Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity\ModernPerson;
18+
use Symfony\Component\Console\Tester\CommandTester;
19+
use Symfony\Component\Filesystem\Filesystem;
20+
21+
class BundlePathsTest extends AbstractWebTestCase
22+
{
23+
public function testBundlePublicDir()
24+
{
25+
$kernel = static::bootKernel(['test_case' => 'BundlePaths']);
26+
$projectDir = sys_get_temp_dir().'/'.uniqid('sf_bundle_paths', true);
27+
28+
$fs = new Filesystem();
29+
$fs->mkdir($projectDir.'/public');
30+
$command = (new Application($kernel))->add(new AssetsInstallCommand($fs, $projectDir));
31+
$exitCode = (new CommandTester($command))->execute(['target' => $projectDir.'/public']);
32+
33+
$this->assertSame(0, $exitCode);
34+
$this->assertFileExists($projectDir.'/public/bundles/modern/modern.css');
35+
$this->assertFileExists($projectDir.'/public/bundles/legacy/legacy.css');
36+
37+
$fs->remove($projectDir);
38+
}
39+
40+
public function testBundleTwigTemplatesDir()
41+
{
42+
static::bootKernel(['test_case' => 'BundlePaths']);
43+
$twig = static::$container->get('twig');
44+
$bundlesMetadata = static::$container->getParameter('kernel.bundles_metadata');
45+
46+
$this->assertSame([$bundlesMetadata['LegacyBundle']['path'].'/Resources/views'], $twig->getLoader()->getPaths('Legacy'));
47+
$this->assertSame("OK\n", $twig->render('@Legacy/index.html.twig'));
48+
49+
$this->assertSame([$bundlesMetadata['ModernBundle']['path'].'/templates'], $twig->getLoader()->getPaths('Modern'));
50+
$this->assertSame("OK\n", $twig->render('@Modern/index.html.twig'));
51+
}
52+
53+
public function testBundleTranslationsDir()
54+
{
55+
static::bootKernel(['test_case' => 'BundlePaths']);
56+
$translator = static::$container->get('translator');
57+
58+
$this->assertSame('OK', $translator->trans('ok_label', [], 'legacy'));
59+
$this->assertSame('OK', $translator->trans('ok_label', [], 'modern'));
60+
}
61+
62+
public function testBundleValidationConfigDir()
63+
{
64+
static::bootKernel(['test_case' => 'BundlePaths']);
65+
$validator = static::$container->get('validator');
66+
67+
$this->assertTrue($validator->hasMetadataFor(LegacyPerson::class));
68+
$this->assertCount(1, $constraintViolationList = $validator->validate(new LegacyPerson('john', 5)));
69+
$this->assertSame('This value should be greater than 18.', $constraintViolationList->get(0)->getMessage());
70+
71+
$this->assertTrue($validator->hasMetadataFor(ModernPerson::class));
72+
$this->assertCount(1, $constraintViolationList = $validator->validate(new ModernPerson('john', 5)));
73+
$this->assertSame('This value should be greater than 18.', $constraintViolationList->get(0)->getMessage());
74+
}
75+
76+
public function testBundleSerializationConfigDir()
77+
{
78+
static::bootKernel(['test_case' => 'BundlePaths']);
79+
$serializer = static::$container->get('serializer');
80+
81+
$this->assertEquals(['full_name' => 'john', 'age' => 5], $serializer->normalize(new LegacyPerson('john', 5), 'json'));
82+
$this->assertEquals(['full_name' => 'john', 'age' => 5], $serializer->normalize(new ModernPerson('john', 5), 'json'));
83+
}
84+
}

‎Tests/Functional/app/BundlePaths/bundles.php

Copy file name to clipboard
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <fabien@symfony.com>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
use Symfony\Bundle\FrameworkBundle\FrameworkBundle;
13+
use Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\LegacyBundle;
14+
use Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\ModernBundle;
15+
use Symfony\Bundle\TwigBundle\TwigBundle;
16+
17+
return [
18+
new FrameworkBundle(),
19+
new TwigBundle(),
20+
new ModernBundle(),
21+
new LegacyBundle(),
22+
];

‎Tests/Functional/app/BundlePaths/config.yml

Copy file name to clipboard
+11Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
imports:
2+
- { resource: ../config/default.yml }
3+
4+
framework:
5+
translator: true
6+
validation: true
7+
serializer: true
8+
9+
twig:
10+
strict_variables: '%kernel.debug%'
11+
exception_controller: ~

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.