Permalink
Browse files

bug #27501 [FrameworkBundle] Fix test-container on kernel reboot, rev…

…ert to returning the real container from Client::getContainer() (nicolas-grekas)

This PR was merged into the 4.1 branch.

Discussion
----------

[FrameworkBundle] Fix test-container on kernel reboot, revert to returning the real container from Client::getContainer()

| Q             | A
| ------------- | ---
| Branch?       | 4.1
| Bug fix?      | yes
| New feature?  | -
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27494
| License       | MIT
| Doc PR        | -

By making `Client::getContainer()` return the new test container, we broke BC, as spotted in linked issue.

Always use `static::$container` in your tests instead.

While reverting to returning the real container, I noticed we have a serious design issue in the way the test container currently works: because the kernel can be rebooted, we cannot inject the container directly, but have to go through the kernel all the time. Fixing this forces doing a BC break on the constructor of `TestContainer`. Since this is a new class and since it's mostly internal, I think we should do it now. I've marked the class as internal to further strengthen this.

Commits
-------

6764d4e [FrameworkBundle] Fix test-container on kernel reboot, revert to returning the real container from Client::getContainer()
  • Loading branch information...
fabpot committed Jun 6, 2018
2 parents 8130f22 + 6764d4e commit 6770630ceeccfcab7bf61875dbc65af102d76a38
@@ -30,15 +30,13 @@ class Client extends BaseClient
private $hasPerformedRequest = false;
private $profiler = false;
private $reboot = true;
private $testContainerId;
/**
* {@inheritdoc}
*/
public function __construct(KernelInterface $kernel, array $server = array(), History $history = null, CookieJar $cookieJar = null, string $testContainerId = null)
public function __construct(KernelInterface $kernel, array $server = array(), History $history = null, CookieJar $cookieJar = null)
{
parent::__construct($kernel, $server, $history, $cookieJar);
$this->testContainerId = $testContainerId;
}
/**
@@ -48,9 +46,7 @@ public function __construct(KernelInterface $kernel, array $server = array(), Hi
*/
public function getContainer()
{
$container = $this->kernel->getContainer();
return null !== $this->testContainerId ? $container->get($this->testContainerId) : $container;
return $this->kernel->getContainer();
}
/**
@@ -22,15 +22,11 @@ class TestServiceContainerRealRefPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if (!$container->hasDefinition('test.service_container')) {
if (!$container->hasDefinition('test.private_services_locator')) {
return;
}
$testContainer = $container->getDefinition('test.service_container');
$privateContainer = $testContainer->getArgument(2);
if ($privateContainer instanceof Reference) {
$privateContainer = $container->getDefinition((string) $privateContainer);
}
$privateContainer = $container->getDefinition('test.private_services_locator');
$definitions = $container->getDefinitions();
$privateServices = $privateContainer->getArgument(0);
@@ -23,7 +23,7 @@ class TestServiceContainerWeakRefPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
if (!$container->hasDefinition('test.service_container')) {
if (!$container->hasDefinition('test.private_services_locator')) {
return;
}
@@ -50,7 +50,7 @@ public function process(ContainerBuilder $container)
}
if ($privateServices) {
$definitions[(string) $definitions['test.service_container']->getArgument(2)]->replaceArgument(0, $privateServices);
$definitions['test.private_services_locator']->replaceArgument(0, $privateServices);
}
}
}
@@ -16,7 +16,6 @@
<argument>%test.client.parameters%</argument>
<argument type="service" id="test.client.history" />
<argument type="service" id="test.client.cookiejar" />
<argument>test.service_container</argument>
</service>
<service id="test.client.history" class="Symfony\Component\BrowserKit\History" shared="false" />
@@ -36,13 +35,12 @@
</service>
<service id="test.service_container" class="Symfony\Bundle\FrameworkBundle\Test\TestContainer" public="true">
<argument type="service" id="parameter_bag" on-invalid="null" />
<argument type="service" id="service_container" />
<argument type="service">
<service class="Symfony\Component\DependencyInjection\ServiceLocator">
<argument type="collection" />
</service>
</argument>
<argument type="service" id="kernel" />
<argument>test.private_services_locator</argument>
</service>
<service id="test.private_services_locator" class="Symfony\Component\DependencyInjection\ServiceLocator" public="true">
<argument type="collection" />
</service>
</services>
</container>
@@ -11,95 +11,140 @@
namespace Symfony\Bundle\FrameworkBundle\Test;
use Psr\Container\ContainerInterface as PsrContainerInterface;
use Symfony\Component\DependencyInjection\Container;
use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface;
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
use Symfony\Component\HttpKernel\KernelInterface;
/**
* @author Nicolas Grekas <p@tchwork.com>
*
* @internal
*/
class TestContainer extends Container
{
private $publicContainer;
private $privateContainer;
private $kernel;
private $privateServicesLocatorId;
public function __construct(?ParameterBagInterface $parameterBag, SymfonyContainerInterface $publicContainer, PsrContainerInterface $privateContainer)
public function __construct(KernelInterface $kernel, string $privateServicesLocatorId)
{
$this->parameterBag = $parameterBag ?? $publicContainer->getParameterBag();
$this->publicContainer = $publicContainer;
$this->privateContainer = $privateContainer;
$this->kernel = $kernel;
$this->privateServicesLocatorId = $privateServicesLocatorId;
}
/**
* {@inheritdoc}
*/
public function compile()
{
$this->publicContainer->compile();
$this->getPublicContainer()->compile();
}
/**
* {@inheritdoc}
*/
public function isCompiled()
{
return $this->publicContainer->isCompiled();
return $this->getPublicContainer()->isCompiled();
}
/**
* {@inheritdoc}
*/
public function getParameterBag()
{
return $this->getPublicContainer()->getParameterBag();
}
/**
* {@inheritdoc}
*/
public function getParameter($name)
{
return $this->getPublicContainer()->getParameter($name);
}
/**
* {@inheritdoc}
*/
public function hasParameter($name)
{
return $this->getPublicContainer()->hasParameter($name);
}
/**
* {@inheritdoc}
*/
public function setParameter($name, $value)
{
$this->getPublicContainer()->setParameter($name, $value);
}
/**
* {@inheritdoc}
*/
public function set($id, $service)
{
$this->publicContainer->set($id, $service);
$this->getPublicContainer()->set($id, $service);
}
/**
* {@inheritdoc}
*/
public function has($id)
{
return $this->publicContainer->has($id) || $this->privateContainer->has($id);
return $this->getPublicContainer()->has($id) || $this->getPrivateContainer()->has($id);
}
/**
* {@inheritdoc}
*/
public function get($id, $invalidBehavior = /* self::EXCEPTION_ON_INVALID_REFERENCE */ 1)
{
return $this->privateContainer->has($id) ? $this->privateContainer->get($id) : $this->publicContainer->get($id, $invalidBehavior);
return $this->getPrivateContainer()->has($id) ? $this->getPrivateContainer()->get($id) : $this->getPublicContainer()->get($id, $invalidBehavior);
}
/**
* {@inheritdoc}
*/
public function initialized($id)
{
return $this->publicContainer->initialized($id);
return $this->getPublicContainer()->initialized($id);
}
/**
* {@inheritdoc}
*/
public function reset()
{
$this->publicContainer->reset();
$this->getPublicContainer()->reset();
}
/**
* {@inheritdoc}
*/
public function getServiceIds()
{
return $this->publicContainer->getServiceIds();
return $this->getPublicContainer()->getServiceIds();
}
/**
* {@inheritdoc}
*/
public function getRemovedIds()
{
return $this->publicContainer->getRemovedIds();
return $this->getPublicContainer()->getRemovedIds();
}
private function getPublicContainer()
{
if (null === $container = $this->kernel->getContainer()) {
throw new \LogicException('Cannot access the container on a non-booted kernel. Did you forget to boot it?');
}
return $container;
}
private function getPrivateContainer()
{
return $this->getPublicContainer()->get($this->privateServicesLocatorId);
}
}
@@ -20,13 +20,13 @@ public function testContainerCompilationInDebug()
{
$client = $this->createClient(array('test_case' => 'ContainerDump', 'root_config' => 'config.yml'));
$this->assertTrue($client->getContainer()->has('serializer'));
$this->assertTrue(static::$container->has('serializer'));
}
public function testContainerCompilation()
{
$client = $this->createClient(array('test_case' => 'ContainerDump', 'root_config' => 'config.yml', 'debug' => false));
$this->assertTrue($client->getContainer()->has('serializer'));
$this->assertTrue(static::$container->has('serializer'));
}
}
@@ -35,18 +35,18 @@ public function testSessionLessRememberMeLogout()
public function testCsrfTokensAreClearedOnLogout()
{
$client = $this->createClient(array('test_case' => 'LogoutWithoutSessionInvalidation', 'root_config' => 'config.yml'));
$client->getContainer()->get('security.csrf.token_storage')->setToken('foo', 'bar');
static::$container->get('security.csrf.token_storage')->setToken('foo', 'bar');
$client->request('POST', '/login', array(
'_username' => 'johannes',
'_password' => 'test',
));
$this->assertTrue($client->getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertSame('bar', $client->getContainer()->get('security.csrf.token_storage')->getToken('foo'));
$this->assertTrue(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertSame('bar', static::$container->get('security.csrf.token_storage')->getToken('foo'));
$client->request('GET', '/logout');
$this->assertFalse($client->getContainer()->get('security.csrf.token_storage')->hasToken('foo'));
$this->assertFalse(static::$container->get('security.csrf.token_storage')->hasToken('foo'));
}
}
@@ -47,7 +47,7 @@
"symfony/security": "4.1.0-beta1|4.1.0-beta2",
"symfony/var-dumper": "<3.4",
"symfony/event-dispatcher": "<3.4",
"symfony/framework-bundle": "<=4.1-beta2",
"symfony/framework-bundle": "<4.1.1",
"symfony/console": "<3.4"
},
"autoload": {

0 comments on commit 6770630

Please sign in to comment.