Skip to content

Commit

Permalink
bug #24709 [HttpKernel] Move services reset to Kernel::handle()+boot(…
Browse files Browse the repository at this point in the history
…) (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpKernel] Move services reset to Kernel::handle()+boot()

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

This is an alternative to #24697 (which uses middlewares).
This PR adds a new `services_resetter` service that the Kernel calls on 2nd root requests to reset services.
Instead of #24697 which plans for optional enabling of the services reset, this approach moves the responsibility of calling the services resetter to the core Kernel class, so that no configuration/middleware/etc. is required at all, and no overhead exists at all for regular requests.

Commits
-------

4501a36 [HttpKernel] Move services reset to Kernel
  • Loading branch information
fabpot committed Oct 30, 2017
2 parents 344e4b0 + 4501a36 commit 039250a
Show file tree
Hide file tree
Showing 9 changed files with 164 additions and 170 deletions.
Expand Up @@ -75,10 +75,6 @@
<tag name="config_cache.resource_checker" priority="-990" />
</service>

<service id="Symfony\Component\HttpKernel\EventListener\ServiceResetListener">
<argument /> <!-- ResettableServicePass will inject an iterator of initialized services here ($serviceId => $serviceInstance) -->
<argument type="collection" /> <!-- ResettableServicePass will inject an array of reset methods here ($serviceId => $method) -->
<tag name="kernel.event_subscriber" />
</service>
<service id="services_resetter" class="Symfony\Component\HttpKernel\DependencyInjection\ServicesResetter" public="true" />
</services>
</container>
Expand Up @@ -17,7 +17,6 @@
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\EventListener\ServiceResetListener;

/**
* @author Alexander M. Turek <me@derrabus.de>
Expand All @@ -26,9 +25,6 @@ class ResettableServicePass implements CompilerPassInterface
{
private $tagName;

/**
* @param string $tagName
*/
public function __construct($tagName = 'kernel.reset')
{
$this->tagName = $tagName;
Expand All @@ -39,7 +35,7 @@ public function __construct($tagName = 'kernel.reset')
*/
public function process(ContainerBuilder $container)
{
if (!$container->has(ServiceResetListener::class)) {
if (!$container->has('services_resetter')) {
return;
}

Expand All @@ -57,13 +53,13 @@ public function process(ContainerBuilder $container)
}

if (empty($services)) {
$container->removeDefinition(ServiceResetListener::class);
$container->removeDefinition('services_resetter');

return;
}

$container->findDefinition(ServiceResetListener::class)
->replaceArgument(0, new IteratorArgument($services))
->replaceArgument(1, $methods);
$container->findDefinition('services_resetter')
->setArgument(0, new IteratorArgument($services))
->setArgument(1, $methods);
}
}
@@ -0,0 +1,39 @@
<?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\DependencyInjection;

/**
* Resets provided services.
*
* @author Alexander M. Turek <me@derrabus.de>
* @author Nicolas Grekas <p@tchwork.com>
*
* @internal
*/
class ServicesResetter
{
private $resettableServices;
private $resetMethods;

public function __construct(\Traversable $resettableServices, array $resetMethods)
{
$this->resettableServices = $resettableServices;
$this->resetMethods = $resetMethods;
}

public function reset()
{
foreach ($this->resettableServices as $id => $service) {
$service->{$this->resetMethods[$id]}();
}
}
}

This file was deleted.

34 changes: 25 additions & 9 deletions src/Symfony/Component/HttpKernel/Kernel.php
Expand Up @@ -64,6 +64,8 @@ abstract class Kernel implements KernelInterface, RebootableInterface, Terminabl

private $projectDir;
private $warmupDir;
private $requestStackSize = 0;
private $resetServices = false;

const VERSION = '3.4.0-DEV';
const VERSION_ID = 30400;
Expand Down Expand Up @@ -99,6 +101,8 @@ public function __clone()

$this->booted = false;
$this->container = null;
$this->requestStackSize = 0;
$this->resetServices = false;
}

/**
Expand All @@ -107,8 +111,20 @@ public function __clone()
public function boot()
{
if (true === $this->booted) {
if (!$this->requestStackSize && $this->resetServices) {
if ($this->container->has('services_resetter')) {
$this->container->get('services_resetter')->reset();
}
$this->resetServices = false;
}

return;
}
if ($this->debug && !isset($_SERVER['SHELL_VERBOSITY'])) {
putenv('SHELL_VERBOSITY=3');
$_ENV['SHELL_VERBOSITY'] = 3;
$_SERVER['SHELL_VERBOSITY'] = 3;
}

if ($this->loadClassCache) {
$this->doLoadClassCache($this->loadClassCache[0], $this->loadClassCache[1]);
Expand Down Expand Up @@ -169,24 +185,24 @@ public function shutdown()
}

$this->container = null;
$this->requestStackSize = 0;
$this->resetServices = false;
}

/**
* {@inheritdoc}
*/
public function handle(Request $request, $type = HttpKernelInterface::MASTER_REQUEST, $catch = true)
{
if (false === $this->booted) {
if ($this->debug && !isset($_SERVER['SHELL_VERBOSITY'])) {
putenv('SHELL_VERBOSITY=3');
$_ENV['SHELL_VERBOSITY'] = 3;
$_SERVER['SHELL_VERBOSITY'] = 3;
}
$this->boot();
++$this->requestStackSize;
$this->resetServices = true;

$this->boot();
try {
return $this->getHttpKernel()->handle($request, $type, $catch);
} finally {
--$this->requestStackSize;
}

return $this->getHttpKernel()->handle($request, $type, $catch);
}

/**
Expand Down
Expand Up @@ -8,7 +8,7 @@
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\ResettableServicePass;
use Symfony\Component\HttpKernel\EventListener\ServiceResetListener;
use Symfony\Component\HttpKernel\DependencyInjection\ServicesResetter;
use Symfony\Component\HttpKernel\Tests\Fixtures\ClearableService;
use Symfony\Component\HttpKernel\Tests\Fixtures\ResettableService;

Expand All @@ -24,14 +24,14 @@ public function testCompilerPass()
->setPublic(true)
->addTag('kernel.reset', array('method' => 'clear'));

$container->register(ServiceResetListener::class)
$container->register('services_resetter', ServicesResetter::class)
->setPublic(true)
->setArguments(array(null, array()));
$container->addCompilerPass(new ResettableServicePass('kernel.reset'));
$container->addCompilerPass(new ResettableServicePass());

$container->compile();

$definition = $container->getDefinition(ServiceResetListener::class);
$definition = $container->getDefinition('services_resetter');

$this->assertEquals(
array(
Expand All @@ -57,32 +57,22 @@ public function testMissingMethod()
$container = new ContainerBuilder();
$container->register(ResettableService::class)
->addTag('kernel.reset');
$container->register(ServiceResetListener::class)
$container->register('services_resetter', ServicesResetter::class)
->setArguments(array(null, array()));
$container->addCompilerPass(new ResettableServicePass('kernel.reset'));
$container->addCompilerPass(new ResettableServicePass());

$container->compile();
}

public function testCompilerPassWithoutResetters()
{
$container = new ContainerBuilder();
$container->register(ServiceResetListener::class)
$container->register('services_resetter', ServicesResetter::class)
->setArguments(array(null, array()));
$container->addCompilerPass(new ResettableServicePass());

$container->compile();

$this->assertFalse($container->has(ServiceResetListener::class));
}

public function testCompilerPassWithoutListener()
{
$container = new ContainerBuilder();
$container->addCompilerPass(new ResettableServicePass());

$container->compile();

$this->assertFalse($container->has(ServiceResetListener::class));
$this->assertFalse($container->has('services_resetter'));
}
}
@@ -0,0 +1,42 @@
<?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\DependencyInjection;

use PHPUnit\Framework\TestCase;
use Symfony\Component\HttpKernel\DependencyInjection\ServicesResetter;
use Symfony\Component\HttpKernel\Tests\Fixtures\ClearableService;
use Symfony\Component\HttpKernel\Tests\Fixtures\ResettableService;

class ServicesResetterTest extends TestCase
{
protected function setUp()
{
ResettableService::$counter = 0;
ClearableService::$counter = 0;
}

public function testResetServices()
{
$resetter = new ServicesResetter(new \ArrayIterator(array(
'id1' => new ResettableService(),
'id2' => new ClearableService(),
)), array(
'id1' => 'reset',
'id2' => 'clear',
));

$resetter->reset();

$this->assertEquals(1, ResettableService::$counter);
$this->assertEquals(1, ClearableService::$counter);
}
}

0 comments on commit 039250a

Please sign in to comment.