-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Adding a MicroKernel that holds service configuration #15820
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,91 @@ | ||
<?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\Component\DependencyInjection\Loader; | ||
|
||
use Symfony\Component\Config\Loader\LoaderInterface; | ||
use Symfony\Component\Config\Loader\LoaderResolverInterface; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
|
||
/** | ||
* Configuration loader that holds the ContainerBuilder inside. | ||
* | ||
* It can be useful to pass this to KernelInterface::registerContainerConfiguration() | ||
* instead of a normal Loader if that method will need the ContainerBuilder. | ||
* | ||
* @author Ryan Weaver <ryan@knpuniversity.com> | ||
*/ | ||
class ContainerBuilderAwareLoader implements LoaderInterface | ||
{ | ||
/** | ||
* @var ContainerBuilder | ||
*/ | ||
private $containerBuilder; | ||
|
||
/** | ||
* @var LoaderInterface | ||
*/ | ||
private $resourceLoader; | ||
|
||
public function __construct(ContainerBuilder $builder, LoaderInterface $resourceLoader) | ||
{ | ||
$this->containerBuilder = $builder; | ||
$this->resourceLoader = $resourceLoader; | ||
} | ||
|
||
/** | ||
* @return ContainerBuilder | ||
*/ | ||
public function getContainerBuilder() | ||
{ | ||
return $this->containerBuilder; | ||
} | ||
|
||
/** | ||
* @return LoaderInterface | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should the 2 getters be marked as internal as they are meant to be used only in the MicroKernel ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a big enough precedent for that in core? If I were to create an interface for this class, these are the two methods that I'd create. Granted, you don't need to use them if you use the MicroKernel, but you could use it with the normal |
||
*/ | ||
public function getResourceLoader() | ||
{ | ||
return $this->resourceLoader; | ||
} | ||
|
||
/** | ||
* @see {@inheritdoc} | ||
*/ | ||
public function load($resource, $type = null) | ||
{ | ||
return $this->resourceLoader->load($resource, $type); | ||
} | ||
|
||
/** | ||
* @see {@inheritdoc} | ||
*/ | ||
public function supports($resource, $type = null) | ||
{ | ||
return $this->resourceLoader->supports($resource, $type); | ||
} | ||
|
||
/** | ||
* @see {@inheritdoc} | ||
*/ | ||
public function getResolver() | ||
{ | ||
return $this->resourceLoader->getResolver(); | ||
} | ||
|
||
/** | ||
* @see {@inheritdoc} | ||
*/ | ||
public function setResolver(LoaderResolverInterface $resolver) | ||
{ | ||
return $this->resourceLoader->setResolver($resolver); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
<?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\Component\HttpKernel; | ||
|
||
use Symfony\Component\Config\Loader\Loader; | ||
use Symfony\Component\Config\Loader\LoaderInterface; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\ContainerInterface; | ||
use Symfony\Component\DependencyInjection\Loader\ContainerBuilderAwareLoader; | ||
|
||
/** | ||
* A Kernel that allows you to configure services. | ||
* | ||
* @author Ryan Weaver <ryan@knpuniversity.com> | ||
*/ | ||
abstract class MicroKernel extends Kernel | ||
{ | ||
/** | ||
* Applies the bundle configuration and calls configureServices() for continued building. | ||
* | ||
* @param LoaderInterface $loader | ||
*/ | ||
public function registerContainerConfiguration(LoaderInterface $loader) | ||
{ | ||
if (!$loader instanceof ContainerBuilderAwareLoader) { | ||
throw new \LogicException('registerContainerConfiguration requires the LoaderInterface to be a ContainerBuilderAwareLoader.'); | ||
} | ||
|
||
$this->configureExtensions($loader->getContainerBuilder(), $loader->getResourceLoader()); | ||
$this->configureServices($loader->getContainerBuilder(), $loader->getResourceLoader()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably add: $loader->addResource(new FileResource(__FILE__)); |
||
} | ||
|
||
/** | ||
* Configure dependency injection extensions that have been added to the container. | ||
* | ||
* $c->loadFromExtension('framework', array( | ||
* 'secret' => '%secret%' | ||
* )); | ||
* | ||
* @param ContainerBuilder $c | ||
* @param LoaderInterface $loader | ||
*/ | ||
protected function configureExtensions(ContainerBuilder $c, LoaderInterface $loader) | ||
{ | ||
} | ||
|
||
/** | ||
* Add any service definitions to your container. | ||
* | ||
* @param ContainerBuilder $c | ||
* @param LoaderInterface $loader | ||
*/ | ||
protected function configureServices(ContainerBuilder $c, LoaderInterface $loader) | ||
{ | ||
} | ||
|
||
/** | ||
* Returns a loader with the ContainerBuilder embedded inside of it. | ||
* | ||
* @param ContainerInterface $container | ||
* | ||
* @return ContainerBuilderAwareLoader | ||
*/ | ||
protected function getContainerLoader(ContainerInterface $container) | ||
{ | ||
if (!$container instanceof ContainerBuilder) { | ||
throw new \LogicException('Only ContainerBuilder instances are supported.'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. By the way, should we add the same check in the parent There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right - the |
||
} | ||
|
||
$loader = parent::getContainerLoader($container); | ||
|
||
return new ContainerBuilderAwareLoader($container, $loader); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
<?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\Component\HttpKernel\Tests\Fixtures; | ||
|
||
use Symfony\Component\HttpKernel\MicroKernel; | ||
use Symfony\Component\Config\Loader\LoaderInterface; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\ContainerInterface; | ||
use Symfony\Component\HttpKernel\Bundle\BundleInterface; | ||
|
||
class MicroKernelForTest extends MicroKernel | ||
{ | ||
private $configureServicesCalled = false; | ||
|
||
private $configureExtensionsCalled = false; | ||
|
||
private $configureServicesArgs = array(); | ||
|
||
public function registerBundles() | ||
{ | ||
return array(); | ||
} | ||
|
||
public function getContainerLoaderExternally(ContainerInterface $container) | ||
{ | ||
return $this->getContainerLoader($container); | ||
} | ||
|
||
protected function configureExtensions(ContainerBuilder $c, LoaderInterface $loader) | ||
{ | ||
$this->configureExtensionsCalled = true; | ||
} | ||
|
||
protected function configureServices(ContainerBuilder $c, LoaderInterface $loader) | ||
{ | ||
$this->configureServicesArgs = array($c, $loader); | ||
|
||
$this->configureServicesCalled = true; | ||
} | ||
|
||
public function wasConfigureServicesCalled() | ||
{ | ||
return $this->configureServicesCalled; | ||
} | ||
|
||
public function wasConfigureExtensionsCalled() | ||
{ | ||
return $this->configureExtensionsCalled; | ||
} | ||
|
||
public function getConfigureServicesArguments() | ||
{ | ||
return $this->configureServicesArgs; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,70 @@ | ||
<?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\Component\HttpKernel\Tests; | ||
|
||
use Symfony\Component\DependencyInjection\Loader\ContainerBuilderAwareLoader; | ||
use Symfony\Component\HttpKernel\Tests\Fixtures\MicroKernelForTest; | ||
use Symfony\Component\DependencyInjection\ContainerBuilder; | ||
use Symfony\Component\DependencyInjection\Container; | ||
|
||
class MicroKernelTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
public function testGetContainerLoader() | ||
{ | ||
$containerBuilder = new ContainerBuilder(); | ||
$kernel = new MicroKernelForTest('test', false); | ||
|
||
$loader = $kernel->getContainerLoaderExternally($containerBuilder); | ||
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Loader\ContainerBuilderAwareLoader', $loader); | ||
$this->assertSame($containerBuilder, $loader->getContainerBuilder()); | ||
} | ||
|
||
/** | ||
* @expectedException \LogicException | ||
*/ | ||
public function testGetContainerLoaderFailsUnlessBuilder() | ||
{ | ||
$containerBuilder = new Container(); | ||
$kernel = new MicroKernelForTest('test', false); | ||
|
||
$kernel->getContainerLoaderExternally($containerBuilder); | ||
} | ||
|
||
/** | ||
* @expectedException \LogicException | ||
*/ | ||
public function testRegisterContainerConfigurationOnlyAcceptsContainerAwareBuilderLoader() | ||
{ | ||
$loader = $this->getMock('Symfony\Component\Config\Loader\LoaderInterface'); | ||
$kernel = new MicroKernelForTest('test', false); | ||
$kernel->registerContainerConfiguration($loader); | ||
} | ||
|
||
public function testRegisterContainerConfiguration() | ||
{ | ||
$loader = $this->getContainerBuilderAwareLoader(); | ||
$kernel = new MicroKernelForTest('test', false); | ||
$kernel->registerContainerConfiguration($loader); | ||
|
||
$this->assertTrue($kernel->wasConfigureServicesCalled()); | ||
$configureServicesArgs = $kernel->getConfigureServicesArguments(); | ||
$this->assertSame($loader->getResourceLoader(), $configureServicesArgs[1], 'The original loader is sent to configureServices'); | ||
} | ||
|
||
private function getContainerBuilderAwareLoader() | ||
{ | ||
$loader = $this->getMock('Symfony\Component\Config\Loader\LoaderInterface'); | ||
$builder = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder'); | ||
|
||
return new ContainerBuilderAwareLoader($builder, $loader); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently, we have a circular reference between the loaders and the loader resolver. What about passing a
LoaderResolverInterface
instance here instead? This way the micro kernel would not hold us back breaking this circular reference at a later time (at least, we could get rid of it in the usual FileLoaders and would have to keep it in theDelegatingLoader
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point - and I can make this change if others agree. I'd prefer to push it off until we make that change. The main reason is so that
registerContainerConfiguration()
andconfigureServices()
/configureExtensions()
receive the exact sameLoaderInterface
object as an argument, making these two new methods look and act like what we're used to.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just found that we already have an issue for this: #12207