8000 fixes per nicolas-grekas reviews · symfony/symfony@faa5c1a · GitHub
[go: up one dir, main page]
Skip to content

Commit faa5c1a

Browse files
robfrawleyrobfrawley
committed
fixes per nicolas-grekas reviews
1 parent 6bdebd2 commit faa5c1a

File tree

13 files changed

+157
-326
lines changed

13 files changed

+157
-326
lines changed

‎src/Symfony/Bundle/FrameworkBundle/Command/CachePoolPruneCommand.php

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Bundle\FrameworkBundle\Command;
1313

14+
use Symfony\Component\Cache\PruneableInterface;
1415
use Symfony\Component\Console\Command\Command;
1516
use Symfony\Component\Console\Input\InputInterface;
1617
use Symfony\Component\Console\Output\OutputInterface;
@@ -21,13 +22,10 @@
2122
*/
2223
final class CachePoolPruneCommand extends Command
2324
{
24-
/**
25-
* @var \IteratorAggregate
26-
*/
2725
private $pools;
2826

2927
/**
30-
* @param \IteratorAggregate $pools
28+
* @param iterable|PruneableInterface[] $pools
3129
*/
3230
public function __construct($pools)
3331
{
@@ -45,7 +43,7 @@ protected function configure()
4543
->setName('cache:pool:prune')
4644
->setDescription('Prun F438 e cache pools')
4745
->setHelp(<<<'EOF'
48-
The <info>%command.name%</info> command prunes all or the given cache pools.
46+
The <info>%command.name%</info> command deletes all expired items from all pruneable pools.
4947
5048
%command.full_name%
5149
EOF

‎src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/CachePoolPrunerPass.php

Lines changed: 9 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -13,28 +13,16 @@
1313

1414
use Symfony\Component\Cache\PruneableInterface;
1515
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
16-
use Symfony\Component\DependencyInjection\ChildDefinition;
1716
use Symfony\Component\DependencyInjection\ContainerBuilder;
1817
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1918
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
2019
use Symfony\Component\DependencyInjection\Reference;
2120

2221
class CachePoolPrunerPass implements CompilerPassInterface
2322
{
24-
/**
25-
* @var string
26-
*/
2723
private $cacheCommandServiceId;
28-
29-
/**
30-
* @var string
31-
*/
3224
private $cachePoolTag;
3325

34-
/**
35-
* @param string $cacheCommandServiceId
36-
* @param string $cachePoolTag
37-
*/
3826
public function __construct($cacheCommandServiceId = 'cache.command.pool_pruner', $cachePoolTag = 'cache.pool')
3927
{
4028
$this->cacheCommandServiceId = $cacheCommandServiceId;
@@ -46,82 +34,24 @@ public function __construct($cacheCommandServiceId = 'cache.command.pool_pruner'
4634
*/
4735
public function process(ContainerBuilder $container)
4836
{
49-
if ($container->hasDefinition($this->cacheCommandServiceId)) {
50-
$container->getDefinition($this->cacheCommandServiceId)->replaceArgument(0, $this->getCachePoolIteratorArgument($container));
37+
if (!$container->hasDefinition($this->cacheCommandServiceId)) {
38+
return;
5139
}
52-
}
53-
54-
/**
55-
* @param ContainerBuilder $container
56-
*
57-
* @return IteratorArgument
58-
*/
59-
private function getCachePoolIteratorArgument(ContainerBuilder $container)
60-
{
61-
$services = $this->getCachePoolServiceIds($container);
62-
63-
return new IteratorArgument(array_combine($services, array_map(function ($id) {
64-
return new Reference($id);
65-
}, $services)));
66-
}
6740

68-
/**
69-
* @param ContainerBuilder $container
70-
*
71-
* @return string[]
72-
*/
73-
private function getCachePoolServiceIds(ContainerBuilder $container)
74-
{
7541
$services = array();
7642

7743
foreach ($container->findTaggedServiceIds($this->cachePoolTag) as $id => $tags) {
78-
if ($container->getDefinition($id)->isAbstract()) {
79-
continue;
80-
}
44+
$class = $container->getParameterBag()->resolveValue($container->getDefinition($id)->getClass());
8145

82-
if (!$this->resolveServiceReflectionClass($container, $id)->implementsInterface(PruneableInterface::class)) {
83-
continue;
46+
if (!$reflection = $container->getReflectionClass($class)) {
47+
throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $id));
8448
}
8549

86-
$services[] = $id;
87-
}
88-
89-
return $services;
90-
}
91-
92-
/**
93-
* @param ContainerBuilder $container
94-
* @param string $id
95-
*
96-
* @return \ReflectionClass
97-
*/
98-
private function resolveServiceReflectionClass(ContainerBuilder $container, $id)
99-
{
100-
$class = $this->resolveServiceClassName($container, $id);
101-
102-
if (null === $reflection = $container->getReflectionClass($class)) {
103-
throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $id));
104-
}
105-
106-
return $reflection;
107-
}
108-
109-
/**
110-
* @param ContainerBuilder $container
111-
* @param string $id
112-
*
113-
* @return string
114-
*/
115-
private function resolveServiceClassName(ContainerBuilder $container, $id)
116-
{
117-
$definition = $container->getDefinition($id);
118-
$class = $definition->getClass();
119-
120-
while (!$class && $definition instanceof ChildDefinition) {
121-
$definition = $container->findDefinition($definition->getParent());
122-
$class = $definition->getClass();
50+
if ($reflection->implementsInterface(PruneableInterface::class)) {
51+
$services[$id] = new Reference($id);
52+
}
12353
}
12454

125-
return $container->getParameterBag()->resolveValue($class);
55+
$container->getDefinition($this->cacheCommandServiceId)->replaceArgument(0, new IteratorArgument($services));
12656
}
12757
}

‎src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,6 @@ public function build(ContainerBuilder $container)
9797
$this->addCompilerPassIfExists($container, TranslatorPass::class);
9898
$container->addCompilerPass(new LoggingTranslatorPass());
9999
$container->addCompilerPass(new AddCacheWarmerPass());
100-
$container->addCompilerPass(new CachePoolPrunerPass());
101100
$container->addCompilerPass(new AddCacheClearerPass());
102101
$container->addCompilerPass(new AddExpressionLanguageProvidersPass());
103102
$this->addCompilerPassIfExists($container, TranslationExtractorPass::class);
@@ -110,6 +109,7 @@ public function build(ContainerBuilder $container)
110109
$container->addCompilerPass(new CachePoolPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, 32);
111110
$this->addCompilerPassIfExists($container, ValidateWorkflowsPass::class);
112111
$container->addCompilerPass(new CachePoolClearerPass(), PassConfig::TYPE_AFTER_REMOVING);
112+
$container->addCompilerPass(new CachePoolPrunerPass(), PassConfig::TYPE_AFTER_REMOVING);
113113
$this->addCompilerPassIfExists($container, FormPass::class);
114114

115115
if ($container->getParameter('kernel.debug')) {

‎src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/CachePoolPrunerPassTest.php

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
use Symfony\Component\Cache\Adapter\FilesystemAdapter;
1717
use Symfony\Component\Cache\Adapter\PhpFilesAdapter;
1818
use Symfony\Component\DependencyInjection\Argument\IteratorArgument;
19-
use Symfony\Component\DependencyInjection\ChildDefinition;
2019
use Symfony\Component\DependencyInjection\ContainerBuilder;
2120
use Symfony\Component\DependencyInjection\Reference;
2221

@@ -42,25 +41,6 @@ public function testCompilerPassReplacesCommandArgument()
4241
$this->assertEquals($expected, $argument->getValues());
4342
}
4443

45-
public function testCompilerPassReplacesCommandArgumentWithDecorated()
46-
{
47-
$container = new ContainerBuilder();
48-
$container->register('cache.command.pool_pruner')->addArgument(array());
49-
$container->register('pool.foo', FilesystemAdapter::class);
50-
$container->setDefinition('pool.bar', new ChildDefinition('pool.foo'))->addTag('cache.pool');
51-
52-
$pass = new CachePoolPrunerPass();
53-
$pass->process($container);
54-
55-
$expected = array(
56-
'pool.bar' => new Reference('pool.bar'),
57-
);
58-
$argument = $container->getDefinition('cache.command.pool_pruner')->getArgument(0);
59-
60-
$this->assertInstanceOf(IteratorArgument::class, $argument);
61-
$this->assertEquals($expected, $argument->getValues());
62-
}
63-
6444
public function testCompilePassIsIgnoredIfCommandDoesNotExist()
6545
{
6646
$container = $this

‎src/Symfony/Component/Cache/PruneableInterface.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
namespace Symfony\Component\Cache;
1313

1414
/**
15-
* Interface for adapters and simple cache implementations that allow pruning entries.
15+
* Interface for adapters and simple cache implementations that allow pruning expired items.
1616
*/
1717
interface PruneableInterface
1818
{

‎src/Symfony/Component/Cache/Tests/Adapter/AdapterTestCase.php

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\Cache\Tests\Adapter;
1313

1414
use Cache\IntegrationTests\CachePoolTest;
15+
use Symfony\Component\Cache\PruneableInterface;
1516

1617
abstract class AdapterTestCase extends CachePoolTest
1718
{
@@ -22,6 +23,10 @@ protected function setUp()
2223
if (!array_key_exists('testDeferredSaveWithoutCommit', $this->skippedTests) && defined('HHVM_VERSION')) {
2324
$this->skippedTests['testDeferredSaveWithoutCommit'] = 'Destructors are called late on HHVM.';
2425
}
26+
27+
if (!array_key_exists('testPrune', $this->skippedTests) && !$this->createCachePool() instanceof PruneableInterface) {
28+
$this->skippedTests['testPrune'] = 'Not a pruneable cache pool.';
29+
}
2530
}
2631

2732
public function testDefaultLifeTime()
@@ -67,6 +72,59 @@ public function testNotUnserializable()
6772
}
6873
$this->assertFalse($item->isHit());
6974
}
75+
76+
public function testPrune()
77+
{
78+
if (isset($this->skippedTests[__FUNCTION__])) {
79+
$this->markTestSkipped($this->skippedTests[__FUNCTION__]);
80+
}
81+
82+
if (!method_exists($this, 'isPruned')) {
83+
$this->fail('Test classes for pruneable caches must implement `isPruned($cache, $name)` method.');
84+
}
85+
86+
$cache = $this->createCachePool();
87+
88+
$doSet = function ($name, $value, \DateInterval $expiresAfter = null) use ($cache) {
89+
$item = $cache->getItem($name);
90+
$item->set($value);
91+
92+
if ($expiresAfter) {
93+
$item->expiresAfter($expiresAfter);
94+
}
95+
96+
$cache->save($item);
97+
};
98+
99+
$doSet('foo', 'foo-val');
100+
$doSet('bar', 'bar-val', new \DateInterval('PT20S'));
101+
$doSet('baz', 'baz-val', new \DateInterval('PT40S'));
102+
$doSet('qux', 'qux-val', new \DateInterval('PT80S'));
103+
104+
$cache->prune();
105+
$this->assertTrue($this->isPruned($cache, 'foo'));
106+
$this->assertTrue($this->isPruned($cache, 'bar'));
107+
$this->assertTrue($this->isPruned($cache, 'baz'));
108+
$this->assertTrue($this->isPruned($cache, 'qux'));
109+
110+
sleep(30);
111+
$cache->prune();
112+
$this->assertTrue($this->isPruned($cache, 'foo'));
113+
$this->assertFalse($this->isPruned($cache, 'bar'));
114+
$this->assertTrue($this->isPruned($cache, 'baz'));
115+
$this->assertTrue($this->isPruned($cache, 'qux'));
116+
117+
sleep(30);
118+
$cache->prune();
119+
$this->assertTrue($this->isPruned($cache, 'foo'));
120+
$this->assertFalse($this->isPruned($cache, 'baz'));
121+
$this->assertTrue($this->isPruned($cache, 'qux'));
122+
123+
sleep(30);
124+
$cache->prune();
125+
$this->assertTrue($this->isPruned($cache, 'foo'));
126+
$this->assertFalse($this->isPruned($cache, 'qux'));
127+
}
70128
}
71129

72130
class NotUnserializable implements \Serializable

‎src/Symfony/Component/Cache/Tests/Adapter/FilesystemAdapterTest.php

Lines changed: 5 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\Cache\Tests\Adapter;
1313

14+
use Psr\Cache\CacheItemPoolInterface;
1415
use Symfony\Component\Cache\Adapter\FilesystemAdapter;
1516

1617
/**
@@ -50,61 +51,11 @@ public static function rmdir($dir)
5051
rmdir($dir);
5152
}
5253

53-
public function testPrune()
54+
protected function isPruned(CacheItemPoolInterface $cache, $name)
5455
{
55-
if (isset($this->skippedTests[__FUNCTION__])) {
56-
$this->markTestSkipped($this->skippedTests[__FUNCTION__]);
56+
$getFileMethod = (new \ReflectionObject($cache))->getMethod('getFile');
57+
$getFileMethod->setAccessible(true);
5758

58-
return;
59-
}
60-
61-
$cache = $this->createCachePool();
62-
63-
$isHit = function ($name) use ($cache) {
64-
$getFileMethod = (new \ReflectionObject($cache))->getMethod('getFile');
65-
$getFileMethod->setAccessible(true);
66-
67-
return $cache->getItem($name)->isHit() && file_exists($getFileMethod->invoke($cache, $name));
68-
};
69-
70-
$doSet = function ($name, $value, \DateInterval $expiresAfter = null) use ($cache) {
71-
$item = $cache->getItem($name);
72-
$item->set($value);
73-
74-
if ($expiresAfter) {
75-
$item->expiresAfter($expiresAfter);
76-
}
77-
78-
$cache->save($item);
79-
};
80-
81-
$doSet('foo', 'foo-val');
82-
$doSet('bar', 'bar-val', new \DateInterval('PT20S'));
83-
$doSet('baz', 'baz-val', new \DateInterval('PT40S'));
84-
$doSet('qux', 'qux-val', new \DateInterval('PT80S'));
85-
86-
$cache->prune();
87-
$this->assertTrue($isHit('foo'));
88-
$this->assertTrue($isHit('bar'));
89-
$this->assertTrue($isHit('baz'));
90-
$this->assertTrue($isHit('qux'));
91-
92-
sleep(30);
93-
$cache->prune();
94-
$this->assertTrue($isHit('foo'));
95-
$this->assertFalse($isHit('bar'));
96-
$this->assertTrue($isHit('baz'));
97-
$this->assertTrue($isHit('qux'));
98-
99-
sleep(30);
100-
$cache->prune();
101-
$this->assertTrue($isHit('foo'));
102-
$this->assertFalse($isHit('baz'));
103-
$this->assertTrue($isHit('qux'));
104-
105-
sleep(30);
106-
$cache->prune();
107-
$this->assertTrue($isHit('foo'));
108-
$this->assertFalse($isHit('qux'));
59+
return $cache->getItem($name)->isHit() && file_exists($getFileMethod->invoke($cache, $name));
10960
}
11061
}

0 commit comments

Comments
 (0)
0