Elao / WebProfilerExtraBundle

Adding routing, container, assetic & twig information in the web profiler
http://www.elao.com
MIT License
262 stars 48 forks source link

Loading multiple configs with different environments, is it intentional? #9

Closed srosato closed 11 years ago

srosato commented 13 years ago

Hi there, I am currently using Behat under test environment to run behaviour driven features and the WebProfilerExtraBundle conflicted when the switching from multiple requests. Since I didn't need the extra tools under the dev environment, I went away and added in config_test.yml:

web_profiler_extra:
    routing:    false
    container:  false
    assetic:    false
    twig:       false

from config_dev.yml I have:

web_profiler_extra:
    routing:    true
    container:  true
    assetic:    true
    twig:       true

The thing is, it does not override, each config is parsed as a whole here:

class WebProfilerExtraExtension extends Extension
{
    protected $resources = array(
        'routing'     => array('file' => 'routing.xml'),
        'container'   => array('file' => 'container.xml'),
        'assetic'     => array('file' => 'assetic.xml'), 
        'twig'        => array('file' => 'twig.xml')
    );

    public function load(array $configs, ContainerBuilder $container)
    {
        foreach ($configs as $config) {
            $this->doConfigLoad($config, $container);
        }
    }

    protected function doConfigLoad(array $config, ContainerBuilder $container)
    {
        $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));

        foreach ($this->resources as $key => $resource) 
        {
            if (isset($config[$key]))
            {
                $loader->load($resource['file']);    
            }
        }
    }

Is this intentional? I am able to disable the profiler simply by using one overridable configuration like this:

class WebProfilerExtraExtension extends Extension
{
    protected $resources = array(
        'routing'     => array('file' => 'routing.xml'),
        'container'   => array('file' => 'container.xml'),
        'assetic'     => array('file' => 'assetic.xml'), 
        'twig'        => array('file' => 'twig.xml')
    );

    public function load(array $configs, ContainerBuilder $container)
    {
        $conf = array();
        foreach ($configs as $config) {
            $conf = array_merge($conf, $config);
        }

        $this->doConfigLoad($conf, $container);
    }

    protected function doConfigLoad(array $config, ContainerBuilder $container)
    {
        $loader = new XmlFileLoader($container, new FileLocator(__DIR__.'/../Resources/config'));

        foreach ($this->resources as $key => $resource) 
        {
            if (isset($config[$key]) && $config[$key])
            {
                $loader->load($resource['file']);    
            }
        }
    }

I didn't think of other ways to disable it under the test env while still keeping it enabled for dev without doing the configuration overriding. What are your thoughts on that?

Best regards, Steven Rosato

srosato commented 13 years ago

Of course this is a really simple approach, as the more advanced Symfony\Component\Config\Definition\Processor and Symfony\Component\Config\Definition\ConfigurationInterface could be used as seen there http://symfony.com/doc/current/cookbook/bundles/extension.html#validation-and-merging-with-a-configuration-class

benji07 commented 11 years ago

Fix by PR #20