feature #32845 [HttpKernel][FrameworkBundle] Add alternative conventi…

…on 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 | #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 #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 ------- 6996e1c [HttpKernel][FrameworkBundle] Add alternative convention for bundle directories
symfony · Aug 13, 2019 · 32e0a25 · 32e0a25
2 parents d6773bc + 6996e1c
commit 32e0a25
Show file tree

Hide file tree

Showing 29 changed files with 308 additions and 42 deletions.
diff --git a/UPGRADE-4.4.md b/UPGRADE-4.4.md
@@ -113,9 +113,33 @@ HttpFoundation
 HttpKernel
 ----------
 
- * Implementing the `BundleInterface` without implementing the `getPublicDir()` method is deprecated.
-   This method will be added to the interface in 5.0.
  * The `DebugHandlersListener` class has been marked as `final`
+ * Added new Bundle directory convention consistent with standard skeletons:
+
+    ```
+    └── MyBundle/
+        ├── config/
+        ├── public/
+        ├── src/
+        │   └── MyBundle.php
+        ├── templates/
+        └── translations/
+    ```
+
+   To make this work properly, it is necessary to change the root path of the bundle:
+
+    ```php
+    class MyBundle extends Bundle
+    {
+        public function getPath(): string
+        {
+            return \dirname(__DIR__);
+        }
+    }
+    ```
+
+   As many bundles must be compatible with a range of Symfony versions, the current 
+   directory convention is not deprecated yet, but it will be in the future.
 
 Lock
 ----

diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md
@@ -291,7 +291,6 @@ HttpFoundation
 HttpKernel
 ----------
 
- * The `getPublicDir()` method has been added to the `BundleInterface`.
  * Removed `Client`, use `HttpKernelBrowser` instead
  * The `Kernel::getRootDir()` and the `kernel.root_dir` parameter have been removed
  * The `KernelInterface::getName()` and the `kernel.name` parameter have been removed
@@ -308,6 +307,32 @@ HttpKernel
  * Removed `TranslatorListener` in favor of `LocaleAwareListener`
  * The `DebugHandlersListener` class has been made `final`
  * Removed `SaveSessionListener` in favor of `AbstractSessionListener`
+ * Added new Bundle directory convention consistent with standard skeletons:
+
+    ```
+    └── MyBundle/
+        ├── config/
+        ├── public/
+        ├── src/
+        │   └── MyBundle.php
+        ├── templates/
+        └── translations/
+    ```
+
+   To make this work properly, it is necessary to change the root path of the bundle:
+
+    ```php
+    class MyBundle extends Bundle
+    {
+        public function getPath(): string
+        {
+            return \dirname(__DIR__);
+        }
+    }
+    ```
+
+   As many bundles must be compatible with a range of Symfony versions, the current 
+   directory convention is not deprecated yet, but it will be in the future.
 
 Intl
 ----

diff --git a/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/DoctrineValidationPass.php b/src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/DoctrineValidationPass.php
@@ -48,11 +48,10 @@ private function updateValidatorMappingFiles(ContainerBuilder $container, string
         }
 
         $files = $container->getParameter('validator.mapping.loader.'.$mapping.'_files_loader.mapping_files');
-        $validationPath = 'Resources/config/validation.'.$this->managerType.'.'.$extension;
+        $validationPath = '/config/validation.'.$this->managerType.'.'.$extension;
 
-        foreach ($container->getParameter('kernel.bundles') as $bundle) {
-            $reflection = new \ReflectionClass($bundle);
-            if ($container->fileExists($file = \dirname($reflection->getFileName()).'/'.$validationPath)) {
+        foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
+            if ($container->fileExists($file = $bundle['path'].'/Resources'.$validationPath) || $container->fileExists($file = $bundle['path'].$validationPath)) {
                 $files[] = $file;
             }
         }

diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/AssetsInstallCommand.php
@@ -137,13 +137,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
         $validAssetDirs = [];
         /** @var BundleInterface $bundle */
         foreach ($kernel->getBundles() as $bundle) {
-            if (!method_exists($bundle, 'getPublicDir')) {
-                @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);
-                $publicDir = 'Resources/public';
-            } else {
-                $publicDir = ltrim($bundle->getPublicDir(), '\\/');
-            }
-            if (!is_dir($originDir = $bundle->getPath().\DIRECTORY_SEPARATOR.$publicDir)) {
+            if (!is_dir($originDir = $bundle->getPath().'/Resources/public') && !is_dir($originDir = $bundle->getPath().'/public')) {
                 continue;
             }
 

diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.php
@@ -162,7 +162,9 @@ protected function execute(InputInterface $input, OutputInterface $output)
         if (null !== $input->getArgument('bundle')) {
             try {
                 $bundle = $kernel->getBundle($input->getArgument('bundle'));
-                $transPaths = [$bundle->getPath().'/Resources/translations'];
+                $bundleDir = $bundle->getPath();
+                $transPaths = [is_dir($bundleDir.'/Resources/translations') ? $bundleDir.'/Resources/translations' : $bundleDir.'/translations'];
+                $viewsPaths = [is_dir($bundleDir.'/Resources/views') ? $bundleDir.'/Resources/views' : $bundleDir.'/templates'];
                 if ($this->defaultTransPath) {
                     $transPaths[] = $this->defaultTransPath;
                 }
@@ -171,7 +173,6 @@ protected function execute(InputInterface $input, OutputInterface $output)
                     $notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $dir, $bundle->getName());
                     @trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath) : 'configure and use "framework.translator.default_path" instead.'), E_USER_DEPRECATED);
                 }
-                $viewsPaths = [$bundle->getPath().'/Resources/views'];
                 if ($this->defaultViewsPath) {
                     $viewsPaths[] = $this->defaultViewsPath;
                 }
@@ -206,13 +207,14 @@ protected function execute(InputInterface $input, OutputInterface $output)
             }
         } elseif ($input->getOption('all')) {
             foreach ($kernel->getBundles() as $bundle) {
-                $transPaths[] = $bundle->getPath().'/Resources/translations';
+                $bundleDir = $bundle->getPath();
+                $transPaths[] = is_dir($bundleDir.'/Resources/translations') ? $bundleDir.'/Resources/translations' : $bundle->getPath().'/translations';
+                $viewsPaths[] = is_dir($bundleDir.'/Resources/views') ? $bundleDir.'/Resources/views' : $bundle->getPath().'/templates';
                 if (is_dir($deprecatedPath = sprintf('%s/Resources/%s/translations', $rootDir, $bundle->getName()))) {
                     $transPaths[] = $deprecatedPath;
                     $notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $bundle->getName(), $deprecatedPath);
                     @trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath) : 'configure and use "framework.translator.default_path" instead.'), E_USER_DEPRECATED);
                 }
-                $viewsPaths[] = $bundle->getPath().'/Resources/views';
                 if (is_dir($deprecatedPath = sprintf('%s/Resources/%s/views', $rootDir, $bundle->getName()))) {
                     $viewsPaths[] = $deprecatedPath;
                     $notice = sprintf('Loading Twig templates for "%s" from the "%s" directory is deprecated since Symfony 4.2, ', $bundle->getName(), $deprecatedPath);

diff --git a/src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php b/src/Symfony/Bundle/FrameworkBundle/Command/TranslationUpdateCommand.php
@@ -154,7 +154,9 @@ protected function execute(InputInterface $input, OutputInterface $output)
         if (null !== $input->getArgument('bundle')) {
             try {
                 $foundBundle = $kernel->getBundle($input->getArgument('bundle'));
-                $transPaths = [$foundBundle->getPath().'/Resources/translations'];
+                $bundleDir = $foundBundle->getPath();
+                $transPaths = [is_dir($bundleDir.'/Resources/translations') ? $bundleDir.'/Resources/translations' : $bundleDir.'/translations'];
+                $viewsPaths = [is_dir($bundleDir.'/Resources/views') ? $bundleDir.'/Resources/views' : $bundleDir.'/templates'];
                 if ($this->defaultTransPath) {
                     $transPaths[] = $this->defaultTransPath;
                 }
@@ -163,7 +165,6 @@ protected function execute(InputInterface $input, OutputInterface $output)
                     $notice = sprintf('Storing translations files for "%s" in the "%s" directory is deprecated since Symfony 4.2, ', $foundBundle->getName(), $dir);
                     @trigger_error($notice.($this->defaultTransPath ? sprintf('use the "%s" directory instead.', $this->defaultTransPath) : 'configure and use "framework.translator.default_path" instead.'), E_USER_DEPRECATED);
                 }
-                $viewsPaths = [$foundBundle->getPath().'/Resources/views'];
                 if ($this->defaultViewsPath) {
                     $viewsPaths[] = $this->defaultViewsPath;
                 }

diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php
@@ -1156,7 +1156,7 @@ private function registerTranslatorConfiguration(array $config, ContainerBuilder
         $defaultDir = $container->getParameterBag()->resolveValue($config['default_path']);
         $rootDir = $container->getParameter('kernel.root_dir');
         foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) {
-            if ($container->fileExists($dir = $bundle['path'].'/Resources/translations')) {
+            if ($container->fileExists($dir = $bundle['path'].'/Resources/translations') || $container->fileExists($dir = $bundle['path'].'/translations')) {
                 $dirs[] = $dir;
             } else {
                 $nonExistingDirs[] = $dir;
@@ -1318,20 +1318,20 @@ private function registerValidatorMapping(ContainerBuilder $container, array $co
         }
 
         foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
-            $dirname = $bundle['path'];
+            $configDir = is_dir($bundle['path'].'/Resources/config') ? $bundle['path'].'/Resources/config' : $bundle['path'].'/config';
 
             if (
-                $container->fileExists($file = $dirname.'/Resources/config/validation.yaml', false) ||
-                $container->fileExists($file = $dirname.'/Resources/config/validation.yml', false)
+                $container->fileExists($file = $configDir.'/validation.yaml', false) ||
+                $container->fileExists($file = $configDir.'/validation.yml', false)
             ) {
                 $fileRecorder('yml', $file);
             }
 
-            if ($container->fileExists($file = $dirname.'/Resources/config/validation.xml', false)) {
+            if ($container->fileExists($file = $configDir.'/validation.xml', false)) {
                 $fileRecorder('xml', $file);
             }
 
-            if ($container->fileExists($dir = $dirname.'/Resources/config/validation', '/^$/')) {
+            if ($container->fileExists($dir = $configDir.'/validation', '/^$/')) {
                 $this->registerMappingFilesFromDir($dir, $fileRecorder);
             }
         }
@@ -1512,20 +1512,20 @@ private function registerSerializerConfiguration(array $config, ContainerBuilder
         };
 
         foreach ($container->getParameter('kernel.bundles_metadata') as $bundle) {
-            $dirname = $bundle['path'];
+            $configDir = is_dir($bundle['path'].'/Resources/config') ? $bundle['path'].'/Resources/config' : $bundle['path'].'/config';
 
-            if ($container->fileExists($file = $dirname.'/Resources/config/serialization.xml', false)) {
+            if ($container->fileExists($file = $configDir.'/serialization.xml', false)) {
                 $fileRecorder('xml', $file);
             }
 
             if (
-                $container->fileExists($file = $dirname.'/Resources/config/serialization.yaml', false) ||
-                $container->fileExists($file = $dirname.'/Resources/config/serialization.yml', false)
+                $container->fileExists($file = $configDir.'/serialization.yaml', false) ||
+                $container->fileExists($file = $configDir.'/serialization.yml', false)
             ) {
                 $fileRecorder('yml', $file);
             }
 
-            if ($container->fileExists($dir = $dirname.'/Resources/config/serialization', '/^$/')) {
+            if ($container->fileExists($dir = $configDir.'/serialization', '/^$/')) {
                 $this->registerMappingFilesFromDir($dir, $fileRecorder);
             }
         }

diff --git a/...mfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Entity/LegacyPerson.php b/...mfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Entity/LegacyPerson.php
@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity;
+
+class LegacyPerson
+{
+    public $name;
+    public $age;
+
+    public function __construct(string $name, string $age)
+    {
+        $this->name = $name;
+        $this->age = $age;
+    }
+}
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/LegacyBundle.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/LegacyBundle.php
@@ -0,0 +1,18 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle;
+
+use Symfony\Component\HttpKernel\Bundle\Bundle;
+
+class LegacyBundle extends Bundle
+{
+}
diff --git a/.../FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/config/serialization.yaml b/.../FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/config/serialization.yaml
@@ -0,0 +1,4 @@
+Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity\LegacyPerson:
+    attributes:
+        name:
+            serialized_name: 'full_name'
diff --git a/...dle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/config/validation.yaml b/...dle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/config/validation.yaml
@@ -0,0 +1,5 @@
+Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\LegacyBundle\Entity\LegacyPerson:
+    properties:
+        age:
+            - GreaterThan:
+                  value: 18
diff --git a/...y/Bundle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/public/legacy.css b/...y/Bundle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/public/legacy.css
diff --git a/...rameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/translations/legacy.en.yaml b/...rameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/translations/legacy.en.yaml
@@ -0,0 +1 @@
+ok_label: OK
diff --git a/...ndle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/views/index.html.twig b/...ndle/FrameworkBundle/Tests/Functional/Bundle/LegacyBundle/Resources/views/index.html.twig
@@ -0,0 +1 @@
+OK
diff --git a/...ony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/config/serialization.yaml b/...ony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/config/serialization.yaml
@@ -0,0 +1,4 @@
+Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity\ModernPerson:
+    attributes:
+        name:
+            serialized_name: 'full_name'
diff --git a/...ymfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/config/validation.yaml b/...ymfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/config/validation.yaml
@@ -0,0 +1,5 @@
+Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity\ModernPerson:
+    properties:
+        age:
+            - GreaterThan:
+                  value: 18
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/public/modern.css b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/public/modern.css
diff --git a/...y/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/src/Entity/ModernPerson.php b/...y/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/src/Entity/ModernPerson.php
@@ -0,0 +1,24 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src\Entity;
+
+class ModernPerson
+{
+    public $name;
+    public $age;
+
+    public function __construct(string $name, string $age)
+    {
+        $this->name = $name;
+        $this->age = $age;
+    }
+}
diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/src/ModernBundle.php b/src/Symfony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/src/ModernBundle.php
@@ -0,0 +1,22 @@
+<?php
+
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Symfony\Bundle\FrameworkBundle\Tests\Functional\Bundle\ModernBundle\src;
+
+use Symfony\Component\HttpKernel\Bundle\Bundle;
+
+class ModernBundle extends Bundle
+{
+    public function getPath(): string
+    {
+        return \dirname(__DIR__);
+    }
+}
diff --git a/...ony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/templates/index.html.twig b/...ony/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/templates/index.html.twig
@@ -0,0 +1 @@
+OK
diff --git a/...y/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/translations/modern.en.yaml b/...y/Bundle/FrameworkBundle/Tests/Functional/Bundle/ModernBundle/translations/modern.en.yaml
@@ -0,0 +1 @@
+ok_label: OK