[FrameworkBundle][HttpKernel] Fix resources loading for bundles with custom structure #21113
Conversation
chalasr
changed the title
|
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 |
chalasr
force-pushed
the
kernel/bundle_metadata
branch
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
chalasr
force-pushed
the
kernel/bundle_metadata
branch
2 times, most recently
from
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 |
chalasr
force-pushed
the
kernel/bundle_metadata
branch
2 times, most recently
from
fc6afb3 to
278186f
Compare
chalasr
force-pushed
the
kernel/bundle_metadata
branch
2 times, most recently
from
a4b54c0 to
e29fdf9
Compare
|
ping @symfony/deciders |
|
This should also update the TwigBundle path registration |
chalasr
force-pushed
the
kernel/bundle_metadata
branch
2 times, most recently
from
cd5c429 to
6ba13a9
Compare
|
@stof done |
| "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()
chalasr
force-pushed
the
kernel/bundle_metadata
branch
from
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()
8 participants
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