Skip to content

[Kernel] Move the cache compiler passes into the Kernel component #20250

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

[Kernel] Move the cache compiler passes into the Kernel component #20250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5ee08d2
[Kernel] Move the cache compiler passes into the Kernel component
Taluu Oct 19, 2016
6330bfa
[FrameworkBundle] Add BC for cache passes
Taluu Oct 19, 2016
06aed12
[HttpKernel] Add arguments for cache passes
Taluu Oct 19, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 8 additions & 18 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheClearerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,31 +11,21 @@

namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\HttpKernel\DependencyInjection\AddCacheClearerPass as BaseAddCacheClearerPass;

@trigger_error('The '.AddCacheClearerPass::class.' class is deprecated since version 3.3 and will be removed in 4.0. Use the '.BaseAddCacheClearerPass::class.' class instead.', E_USER_DEPRECATED);

/**
* Registers the cache clearers.
*
* @deprecated This class is deprecated since 3.3, and will be removed in 4.0. Use Symfony\Component\HttpKernel\DependencyInjection\AddCacheClearerPass instead.
*
* @author Dustin Dobervich <ddobervich@gmail.com>
*/
class AddCacheClearerPass implements CompilerPassInterface
class AddCacheClearerPass extends BaseAddCacheClearerPass
{
/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
public function __construct()
{
if (!$container->hasDefinition('cache_clearer')) {
return;
}

$clearers = array();
foreach ($container->findTaggedServiceIds('kernel.cache_clearer') as $id => $attributes) {
$clearers[] = new Reference($id);
}

$container->getDefinition('cache_clearer')->replaceArgument(0, $clearers);
parent::__construct('cache_clearer', 'kernel.cache_clearer');
}
}
29 changes: 8 additions & 21 deletions src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/AddCacheWarmerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,21 @@

namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\HttpKernel\DependencyInjection\AddCacheWarmerPass as BaseAddCacheWarmerPass;

@trigger_error('The '.AddCacheWarmerPass::class.' class is deprecated since version 3.3 and will be removed in 4.0. Use the '.BaseAddCacheWarmerPass::class.' class instead.', E_USER_DEPRECATED);

/**
* Registers the cache warmers.
*
* @deprecated This class is deprecated since 3.3, and will be removed in 4.0. Use Symfony\Component\HttpKernel\DependencyInjection\AddCacheWarmerPass instead.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class AddCacheWarmerPass implements CompilerPassInterface
class AddCacheWarmerPass extends BaseAddCacheWarmerPass
{
use PriorityTaggedServiceTrait;

/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
public function __construct()
{
if (!$container->hasDefinition('cache_warmer')) {
return;
}

$warmers = $this->findAndSortTaggedServices('kernel.cache_warmer', $container);

if (empty($warmers)) {
return;
}

$container->getDefinition('cache_warmer')->replaceArgument(0, $warmers);
parent::__construct('cache_warmer', 'kernel.cache_warmer');
}
}
8 changes: 4 additions & 4 deletions src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ProfilerPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\TranslatorPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\LoggingTranslatorPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddCacheWarmerPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddCacheClearerPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddExpressionLanguageProvidersPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ContainerBuilderDebugDumpPass;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\CompilerDebugDumpPass;
Expand All @@ -44,6 +42,8 @@
use Symfony\Component\EventDispatcher\DependencyInjection\RegisterListenersPass;
use Symfony\Component\HttpKernel\DependencyInjection\FragmentRendererPass;
use Symfony\Component\Form\DependencyInjection\FormPass;
use Symfony\Component\HttpKernel\DependencyInjection\AddCacheWarmerPass;
use Symfony\Component\HttpKernel\DependencyInjection\AddCacheClearerPass;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\Config\Resource\ClassExistenceResource;
Expand Down Expand Up @@ -88,8 +88,8 @@ public function build(ContainerBuilder $container)
$this->addCompilerPassIfExists($container, AddConsoleCommandPass::class);
$container->addCompilerPass(new TranslatorPass());
$container->addCompilerPass(new LoggingTranslatorPass());
$container->addCompilerPass(new AddCacheWarmerPass());
$container->addCompilerPass(new AddCacheClearerPass());
$container->addCompilerPass(new AddCacheWarmerPass('cache_warmer', 'kernel.cache_warmer'));
$container->addCompilerPass(new AddCacheClearerPass('cache_clearer', 'kernel.cache_clearer'));
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
$container->addCompilerPass(new TranslationExtractorPass());
$container->addCompilerPass(new TranslationDumperPass());
Expand Down
50 changes: 50 additions & 0 deletions src/Symfony/Component/HttpKernel/DependencyInjection/AddCacheClearerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?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;

use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
use Symfony\Component\DependencyInjection\Reference;

/**
* Registers the cache clearers.
*
* @author Dustin Dobervich <ddobervich@gmail.com>
*/
class AddCacheClearerPass implements CompilerPassInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides of the sort, this pass and the AddCacheWarmerPass are rather identical now (and there's probably more).

A lot of times, a compiler pass for tags is used to collect a bunch of references and that's it. What about a more generic class?

Just a brain fart:

class CollectAndInjectTaggedServicePass implements CompilerPassInterface
{
    const NO_FLAGS = 0;
    const SORT_BY_PRIORITY = 1;

    public function __construct($targetService, $serviceArgumentIndex, $tag, $flags = self::NO_FLAGS);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, then this would need to be in the DependencyInjection component, but I am not sure this should be the goal of this PR (but a later one probably ?)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR would introduce 2 new classes in the component while it might be solved with a (re-usable) generic solution, which means those 2 new classes would not be added unless you use them for inheritance templating

Copy link
Contributor

@ro0NL ro0NL Oct 20, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iltar

A lot of times, a compiler pass for tags is used to collect a bunch of references and that's it.

Dont forget the handling of collected references, this is probably the true problem. Currently the passes still make assumptions about what to do with it (pass it as constructor arguments, whereas others add a method call (addSomething(new Ref())), ie. we assume argument index 0 is available on the implementation side, that's why i still think this belongs to the application/framework. Although i can live with it like this in the component as well.

Nevertheless, a generic approach should solve this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But yeah, i thought about a TagCollectingPass many, many times :)

{
private $tag;
private $serviceName;

public function __construct($serviceName, $tag)
{
$this->serviceName = $serviceName;
$this->tag = $tag;
}

/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
{
if (!$container->hasDefinition($this->serviceName)) {
return;
}

$clearers = array();
foreach ($container->findTaggedServiceIds($this->tag) as $id => $attributes) {
$clearers[] = new Reference($id);
}

$container->getDefinition($this->serviceName)->replaceArgument(0, $clearers);
}
}
53 changes: 53 additions & 0 deletions src/Symfony/Component/HttpKernel/DependencyInjection/AddCacheWarmerPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?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;

use Symfony\Component\DependencyInjection\Compiler\PriorityTaggedServiceTrait;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;

/**
* Registers the cache warmers.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class AddCacheWarmerPass implements CompilerPassInterface
{
use PriorityTaggedServiceTrait;

private $tag;
private $serviceName;

public function __construct($serviceName, $tag)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

our other compiler passes in the component don't make these argument required

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the discussion on #20250 (comment)

{
$this->serviceName = $serviceName;
$this->tag = $tag;
}

/**
* {@inheritdoc}
*/
public function process(ContainerBuilder $container)
{
if (!$container->hasDefinition($this->serviceName)) {
return;
}

$warmers = $this->findAndSortTaggedServices($this->tag, $container);

if (empty($warmers)) {
return;
}

$container->getDefinition($this->serviceName)->replaceArgument(0, $warmers);
}
}
10 changes: 5 additions & 5 deletions ...ction/Compiler/AddCacheWarmerPassTest.php → ...dencyInjection/AddCacheWarmerPassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
* file that was distributed with this source code.
*/

namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler;
namespace Symfony\Component\HttpKernel\Tests\DependencyInjection;

use PHPUnit\Framework\TestCase;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\AddCacheWarmerPass;
use Symfony\Component\HttpKernel\DependencyInjection\AddCacheWarmerPass;

class AddCacheWarmerPassTest extends TestCase
{
Expand Down Expand Up @@ -48,7 +48,7 @@ public function testThatCacheWarmersAreProcessedInPriorityOrder()
new Reference('my_cache_warmer_service3'),
));

$addCacheWarmerPass = new AddCacheWarmerPass();
$addCacheWarmerPass = new AddCacheWarmerPass('cache_warmer', 'kernel.cache_warmer');
$addCacheWarmerPass->process($container);
}

Expand All @@ -65,7 +65,7 @@ public function testThatCompilerPassIsIgnoredIfThereIsNoCacheWarmerDefinition()
->will($this->returnValue(false));
$definition->expects($this->never())->method('replaceArgument');

$addCacheWarmerPass = new AddCacheWarmerPass();
$addCacheWarmerPass = new AddCacheWarmerPass('cache_warmer', 'kernel.cache_warmer');
$addCacheWarmerPass->process($container);
}

Expand All @@ -85,7 +85,7 @@ public function testThatCacheWarmersMightBeNotDefined()

$definition->expects($this->never())->method('replaceArgument');

$addCacheWarmerPass = new AddCacheWarmerPass();
$addCacheWarmerPass = new AddCacheWarmerPass('cache_warmer', 'kernel.cache_warmer');
$addCacheWarmerPass->process($container);
}
}