[FrameworkBundle][HttpKernel] Fix resources loading for bundles with custom structure #21113
Conversation
|
I hope this can be accepted as a bugfix rather than a new feature though :/ |
|
The minimum required version of the HttpKernel component in the FrameworkBundle must be updated from |
a49e87d to
6c87d77
Compare
|
@xabbuh thanks, done |
| $reflection = new \ReflectionClass($class); | ||
| if (is_dir($dir = dirname($reflection->getFileName()).'/Resources/translations')) { | ||
| foreach ($container->getParameter('kernel.bundles_metadata') as $name => $bundle) { | ||
| if (is_dir($dir = dirname($bundle['path']).'/Resources/translations')) { |
There was a problem hiding this comment.
Calling dirname() here (and in all places below) looks wrong to me as the path key should already just refer to directory in which the bundle is stored.
There was a problem hiding this comment.
Maybe it would be good to have a dedicated test case working with a bundle that follows the default behaviour (i.e. extending the base Bundle class without overriding the getPath() method) to detect a similar issue in the future?
There was a problem hiding this comment.
Sorry, I missed this comment. I think the test you have in mind already exists, the one added here is a copy paste: https://github.com/symfony/symfony/pull/21113/files#diff-b0c828e732d3d31f0b2bfb2378bd1011R340
Correct me if I'm wrong
1582a32 to
332aeb8
Compare
|
Tests are green |
| foreach ($this->bundles as $name => $bundle) { | ||
| $bundles[$name] = get_class($bundle); | ||
| $bundlesMetadata[$name] = array('parent' => $bundle->getParent(), 'path' => $bundle->getPath(), 'namespace' => $bundle->getNamespace()); |
There was a problem hiding this comment.
To make this more readable I would change the code style to this:
$bundlesMetadata[$name] = array(
'parent' => $bundle->getParent(),
'path' => $bundle->getPath(),
'namespace' => $bundle->getNamespace(),
);|
👍 Status: Reviewed |
fc6afb3 to
278186f
Compare
a4b54c0 to
e29fdf9
Compare
|
ping @symfony/deciders |
|
This should also update the TwigBundle path registration |
cd5c429 to
6ba13a9
Compare
|
@stof done |
| @@ -21,7 +21,7 @@ | |||
| "symfony/twig-bridge": "~2.7", | |||
| "twig/twig": "~1.28|~2.0", | |||
| "symfony/http-foundation": "~2.5", | |||
| "symfony/http-kernel": "~2.7" | |||
| "symfony/http-kernel": "~2.7.23" | |||
There was a problem hiding this comment.
This forbids using 2.8, you should add |~2.8.16.
…verriding getPath()
6ba13a9 to
fef3146
Compare
|
Thank you @chalasr. |
…ndles with custom structure (chalasr) This PR was merged into the 2.7 branch. Discussion ---------- [FrameworkBundle][HttpKernel] Fix resources loading for bundles with custom structure | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #18563 | License | MIT | Doc PR | n/a This fixes twig/translator/validator/serializer resource loading for bundles overriding `Bundle::getPath()`, adding a kernel parameter containing the bundle metadata (i.e. `path`, `namespace` and `parent`). Fixes #18563 and unlocks #19586 Commits ------- fef3146 Fix serializer/translations/validator resources loading for bundles overriding getPath()
This fixes twig/translator/validator/serializer resource loading for bundles overriding
Bundle::getPath(), adding a kernel parameter containing the bundle metadata (i.e.path,namespaceandparent).Fixes #18563 and unlocks #19586