This article could have been titled "Ten things I hate about bundles", but that would be too harsh. The things I am going to describe, are not things I hate, but things I don't like. Besides, when I count them, I don't come to ten things...
Bundle extension discovery
A Symfony2 bundle can have an Extension
class, which allows you to load service definitions from configuration files or define services for the application in a more dynamic way. From the Symfony documentation:
>
[The Extension] class should live in the DependencyInjection
directory of your bundle and its name should be constructed by replacing the Bundle
suffix of the Bundle
class name with Extension
. For example, the Extension
class of AcmeHelloBundle
would be called AcmeHelloExtension
[...]
When you conform to the naming convention, your bundle's Extension
class will automatically be recognized. But how? A quick look in the Kernel
class reveals to us that it calls the method getContainerExtension()
on each registered bundle (which is an object extending implementing BundleInterface
):
namespace Symfony\Component\HttpKernel;
...
abstract class Kernel implements KernelInterface, TerminableInterface
{
protected function prepareContainer(ContainerBuilder $container)
{
...
foreach ($this->bundles as $bundle) {
if ($extension = $bundle->getContainerExtension()) {
$container->registerExtension($extension);
...
}
...
}
...
}
}
If the getContainerExtension()
method of a bundle returns anything, it is assumed to be an instance of ExtensionInterface
, in other words a service container extension.
My bundles almost always look like this:
namespace Matthias\Bundle\DemoBundle;
use Symfony\Component\HttpKernel\Bundle\Bundle;
class MatthiasDemoBundle extends Bundle
{
}
There is no implementation for getContainerExtension()
there, so it must be in the parent class. And it is:
namespace Symfony\Component\HttpKernel\Bundle;
...
abstract class Bundle extends ContainerAware implements BundleInterface
{
...
public function getContainerExtension()
{
if (null === $this->extension) {
$basename = preg_replace('/Bundle$/', '', $this->getName());
$class = $this->getNamespace().'\\DependencyInjection\\'.$basename.'Extension';
if (class_exists($class)) {
$extension = new $class();
// check naming convention
$expectedAlias = Container::underscore($basename);
if ($expectedAlias != $extension->getAlias()) {
throw new \LogicException(sprintf(
'The extension alias for the default extension of a '.
'bundle must be the underscored version of the '.
'bundle name ("%s" instead of "%s")',
$expectedAlias, $extension->getAlias()
));
}
$this->extension = $extension;
} else {
$this->extension = false;
}
}
if ($this->extension) {
return $this->extension;
}
}
}
My goodness, this is some ugly code, but more imporantly: this is magic! My extension class is nowhere explicitly used, nor registered as the one and only service container extension for my bundle. It is simply infered from the existence of a file with the expected name, containing the expected class (there is not even a check for the right interface), that I have a service container extension for my bundle! I don't like that at all.
Naming conventions
Even worse: the naming conventions prevent you from easily moving your Bundle
and Extension
class to another namespace (like you probably have noticed yourself).
I can move Matthias\Bundle\DemoBundle\MatthiasDemoBundle
to Matthias\Bundle\TestBundle\MatthiasTestBundle
with a simple search-and-replace, but I can not just move its DependencyInjection\MatthiasDemoExtension
class to Matthias\Bundle\TestBundle\DependencyInjection
, The class itself has to be renamed to MatthiasTestExtension
, or it won't be recognized anymore.
Also somewhat annoying: the Bundle::getContainerExtension()
puts a constraint on the alias of the service container extension: when my bundle is called MatthiasTestBundle
, its alias should be matthias_test
. But there is no real need for this, it is just a policy to prevent developers from overriding each other's (or worse: the framework's) bundle configuration.
This last rule is enforced in quite a strange way. Remember where the exception is being thrown? Yes, inside my own bundle class! I can easily override getContainerExtension()
and skip the validation of the alias of my service container extension...
Registering service container extensions yourself
Because of the bundle magic described above, I like to implement the getContainerExtension()
method myself and return an instance of the extension. The name of this class can be anything I like.
namespace Matthias\Bundle\TestBundle;
use Matthias\Bundle\TestBundle\DependencyInjection\CanBeAnything;
class MatthiasTestBundle extends Bundle
{
public function getContainerExtension()
{
return new CanBeAnything();
}
}
Now creation logic is also entirely on my side, where I like it to be.
Naming conventions, part two
As mentioned above, an extension has an alias, which you can retrieve by calling its method getAlias()
. The standard implementation of this method is:
namespace Symfony\Component\DependencyInjection\Extension;
...
abstract class Extension implements ExtensionInterface, ConfigurationExtensionInterface
{
...
public function getAlias()
{
$className = get_class($this);
if (substr($className, -9) != 'Extension') {
throw new BadMethodCallException('This extension does not follow the naming convention; you must overwrite the getAlias() method.');
}
$classBaseName = substr(strrchr($className, '\\'), 1, -9);
return Container::underscore($classBaseName);
}
...
}
This function also checks if we have followed the naming convention for extension classes. Since we have chosen to skip the naming convention check in the bundle class, we might just as well skip the check in the extension class too, and just implement the getAlias()
method ourselves:
namespace Matthias\Bundle\TestBundle\DependencyInjection;
use Symfony\Component\HttpKernel\DependencyInjection\Extension;
class CanBeAnything extends Extension
{
public function getAlias()
{
return 'matthias_test';
}
}
Perfectly fine! Now moving a Bundle
or Extension
class won't break any existing configuration under the key matthias_test
in config.yml
and the likes.
Duplicate knowledge: the extension alias
As you may know, when you create a Configuration
class for your bundle, you have to provide the configuration key, also known as the service container alias, as the name of the root node of the TreeBuilder
instance:
namespace Matthias\Bundle\TestBundle\DependencyInjection;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\ConfigurationInterface;
class Configuration implements ConfigurationInterface
{
public function getConfigTreeBuilder()
{
$treeBuilder = new TreeBuilder();
// there it is: the service container alias!
$rootNode = $treeBuilder->root('matthias_test');
...
return $treeBuilder;
}
}
It has always bothered me that this is somehow duplicate knowledge: it should not be necessary to use the exact same string here, which can also be retrieved by calling getAlias()
on the Extension
class. But the Configuration
class can not do this: it has no access to the service container extension. Instead, it has to be the other way around: the Extension
needs to provide its alias to the Configuration
:
namespace Matthias\Bundle\TestBundle\DependencyInjection;
use Symfony\Component\DependencyInjection\ContainerBuilder;
class CanBeAnything extends Extension
{
public function load(array $config, ContainerBuilder $container)
{
$configuration = new Configuration($this->getAlias());
$processedConfig = $this
->processConfiguration($configuration, $config);
}
public function getAlias()
{
return 'matthias_test';
}
}
Then the Configuration
class needs to look something like this:
namespace Matthias\Bundle\TestBundle\DependencyInjection;
class Configuration implements ConfigurationInterface
{
private $alias;
public function __construct($alias)
{
$this->alias = $alias;
}
public function getConfigTreeBuilder()
{
$treeBuilder = new TreeBuilder();
// no more duplication!
$rootNode = $treeBuilder->root($this->alias);
...
return $treeBuilder;
}
}
In conclusion
Well, it takes some extra steps, but only a few, and very easy ones. They will make it much easier to move bundles to another namespace without many other things breaking. They will also free you from naming conventions that would have otherwise been enforced.
Maybe most importantly: they finally remove some magic from bundles, that has been there as some kind of a reminiscence of the old days of symfony 1. It is not very modern to automatically load a file that is located in a certain directory and has a certain name.
(The same goes for console commands, for also registered automagically. See one of my previous post for more about this.)
One final remark: I still recommend you to conform to these conventions:
- Your service container alias/configuration root should correspond to the name of your bundle, to prevent naming collissions.
- Your
Extension
andConfiguration
classes should still be in theDependencyInjection
directory/namespace.
These are good conventions, and they make it easy for any developer to understand and work with your bundle.
Well.. Simply because symfony is not OOP...
here is my 2 cents http://www.craftitonline.co...
Nice post Mattias, reducing the duplicate know of the bundle/extension alias seems like a good idea but I found an issue with the implementation shown in the post which is that it breaks the config:dump-reference command for your bundle unless you implement getConfiguration in your extension. This is because the base Extension->getConfiguration method will fail to return an instance of the Configuration class if it has a constructor (since it has no way of knowing what arguments it might expect).
https://github.com/symfony/...
This is a problem that is easy to miss if you start using the method advocated here since you won't come across it until you try to call the config:dump-reference command on one of your own bundles. You may want to update the extension code to include an implementation of getConfiguration in the extension which returns a new Configuration instance.
Hi Ben,
Thanks, that's an excellent suggestion (also something I didn't know about!).
I think this post could also be titled, Become or Make your Symfony walk the 2 miles or work for you, or become more flexible. I think your 3 or 4 advices can be implemented in every company that is now more mature in Symfony and needs to have deeper understanding and flexibility. To me the commands thing looks as critical. Let's see where this ends in github ticket. I wonder if these tips don't conflict with anything compilerpasses related or other.
Thanks!
Nice :) I think these last couple of posts are indeed the way to go for more advanced users, with long-lasting projects where code has to move sometimes, to reflect changes in the organization. This article is also meant to explain some of the inner workings that developers should know about, instead of just relying on.
Exactly what you describe always makes renaming bundles a messy business
I'd +1 on a PR in 3.0 that would make the Bundle::getContainerExtension and Extension::getAlias methods abstract, and a PR to update the bundle generator command to fill in the blanks with sensible defaults
That would be nice indeed!
This is not the first time someone writes about this topic.
You must know that this "magic" was introduced late in the Symfony2 dev process (in 14aa95ba218c9fd6d02e770e279ed854314fea75 to be precise -- 02/15/2011), so it's not something that is reminiscent from symfony1.
Also, it was introduced after a lot of debates and for very good reasons, some of them are mentioned in this post. IIRC, better error reporting (when someone makes a typo in the extension name in a configuration file), ease of documentation and the possibility to automate things (the configuration entry is auto-discoverable), and ease of bundle creation were the main reasons to introduce this concept. And as you mentioned yourself, changing/overriding/getting rid of this magic is as easy as it can get.
Unfortunately, I'm not able to find the discussion about this topic right now (was it done during an IRC meeting, on the mailing-list or on a PR?), but it took a lot of time to come that that conclusion, which was the best possible compromise.
That said, if someone come up with a better way, I'm open to any suggestions.
Thanks for your elaborate response, Fabien. I think the conventions are great (as I've mentioned at the end of the article) - it would become a mess otherwise. They are probably the best way to keep sanity - I remember these conventions helped me when I started working with Bundles two years ago. It flattens the learning curve quite a lot, since you don't have to worry about extension and configuration classes and how to register them, but you can just start adding service definitions to the service container.
I was surprised to see how easy it was to override the logic from the base classes with something that is also correct, but more explicit and less automatic. This is what I like about Symfony2: that not too many things happen automatically, you need to be explicit about almost everything.
If you happen to find the place and time of the discussion about this subject, I'd be happy to receive it.
Sorry i meant of course "that the HttpKernel component is too much coupled to Bundles."
I think the most problematic point is that the HttpKernel instance is too much coupled to Bundles. For me it would make sense when the Kernel itself (not the HttpKernel) and the Bundle workflow would be extracted to one seperate component or to the FrameworkBundle itself which needs those conventions made in your mentioned base classes. Those conventions are not needed outside of an Symfony application and so should not be included in the component.
Some days ago I had to introduce the HttpKernel and the DIC in some plain PHP project and this was a bit painful. I had to introduce Bundles too that but they were not needed in this project.
The HttpKernel should not rely on any Bundle workflow. It should receive a request and return it into a response. The (application) Kernel should introduce Bundles and extend this workflow.
+1
The HttpKernel has no knowledge of the bundles whatsoever, so you are probably confusing something here. The best proof is that it is used by Silex and Drupal, where there is no notion of bundles.
Yes, sorry about this confusion. I had a typo in my comment and meant the HttpKernel component in common is too coupled with Bundles. The HttpKernel class itself is of course completely fine and has no needless dependencies! The Kernel class however introduces Bundles as they are needed e.g. for building up the complete container. This logic I would not assume in the HttpKernel component but instead in another additional "Kernel" component which relies on HttpKernel. In this way the HttpKernel would be totally free from the Bundle dependency.
https://github.com/symfony/...
Another great CR! Thanks for your efforts!
That's great!
Thanks Frank, I overlooked your other comment...
Thanks for clarifying that point, Fabien. The
Kernel
class implements theHttpKernelInterface
, but it lets HTTP requests be handled by another instance ofHttpKernelInterface
:[php]
namespace Symfony\Component\HttpKernel;
...
class Kernel
{
...
public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQUEST, $catch = true)
{
...
return $this->getHttpKernel()->handle($request, $type, $catch);
}
}
[/php]
Frank, you are right about the component having two major use-cases: an application kernel with bundles and an HTTP kernel. These could have been separated, following the Common Reuse Principle. See also http://butunclebob.com/Arti... and my upcoming book Principles of PHP Package Design.
a rapid count on headings reach to 5 :) but i am reading more slowly this time