Skip to content

Commit

Permalink
Move facade clearing logic to ClearCommand instead of FileStore
Browse files Browse the repository at this point in the history
  • Loading branch information
@calebporzio @taylorotwell
calebporzio authored and taylorotwell committed Sep 19, 2017
1 parent e058944 commit 489c9f9
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 28 deletions.
28 changes: 27 additions & 1 deletion src/Illuminate/Cache/Console/ClearCommand.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Console\Command;
use Illuminate\Cache\CacheManager;
use Illuminate\Filesystem\Filesystem;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Input\InputArgument;

Expand All @@ -30,17 +31,26 @@ class ClearCommand extends Command
*/
protected $cache;

/**
* The filesystem instance.
*
* @var \Illuminate\Filesystem\Filesystem
*/
protected $files;

/**
* Create a new cache clear command instance.
*
* @param \Illuminate\Cache\CacheManager $cache
* @param \Illuminate\Filesystem\Filesystem $files
* @return void
*/
public function __construct(CacheManager $cache)
public function __construct(CacheManager $cache, Filesystem $files)
{
parent::__construct();

$this->cache = $cache;
$this->files = $files;
}

/**
Expand All @@ -54,6 +64,8 @@ public function handle()

$this->cache()->flush();

$this->clearRealTimeFacades();

$this->laravel['events']->fire('cache:cleared', [$this->argument('store'), $this->tags()]);

$this->info('Cache cleared successfully.');
Expand Down Expand Up @@ -104,4 +116,18 @@ protected function getOptions()
['tags', null, InputOption::VALUE_OPTIONAL, 'The cache tags you would like to clear.', null],
];
}

/**
* Clear real-time facades stored in the framework's cache directory.
*
* @return void
*/
public function clearRealTimeFacades()
{
foreach ($this->files->files(storage_path('framework/cache')) as $file) {
if (preg_match('/facade-.*\.php$/', $file)) {
$this->files->delete($file);
}
}
}
}
8 changes: 0 additions & 8 deletions src/Illuminate/Cache/FileStore.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,14 +151,6 @@ public function flush()
}
}

foreach ($this->files->files($this->directory) as $file) {
if (preg_match('/facade-.*\.php$/', $file)) {
if (! $this->files->delete($file)) {
return false;
}
}
}

return true;
}

Expand Down
14 changes: 0 additions & 14 deletions tests/Cache/CacheFileStoreTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ public function testFlushCleansDirectory()
$files = $this->mockFilesystem();
$files->expects($this->once())->method('isDirectory')->with($this->equalTo(__DIR__))->will($this->returnValue(true));
$files->expects($this->once())->method('directories')->with($this->equalTo(__DIR__))->will($this->returnValue(['foo']));
$files->expects($this->once())->method('files')->with($this->equalTo(__DIR__))->will($this->returnValue([]));
$files->expects($this->once())->method('deleteDirectory')->with($this->equalTo('foo'))->will($this->returnValue(true));

$store = new FileStore($files, __DIR__);
Expand Down Expand Up @@ -168,19 +167,6 @@ public function testFlushIgnoreNonExistingDirectory()
$this->assertFalse($result, 'Flush should not clean directory');
}

public function testFlushCleansRealTimeFacades()
{
$files = $this->mockFilesystem();
$files->expects($this->once())->method('isDirectory')->with($this->equalTo(__DIR__))->will($this->returnValue(true));
$files->expects($this->once())->method('directories')->with($this->equalTo(__DIR__))->will($this->returnValue([]));
$files->expects($this->once())->method('files')->with($this->equalTo(__DIR__))->will($this->returnValue(['/facade-XXX.php']));
$files->expects($this->once())->method('delete')->with($this->equalTo('/facade-XXX.php'))->will($this->returnValue(true));

$store = new FileStore($files, __DIR__);
$result = $store->flush();
$this->assertTrue($result, 'Flush failed');
}

protected function mockFilesystem()
{
return $this->createMock('Illuminate\Filesystem\Filesystem');
Expand Down
46 changes: 41 additions & 5 deletions tests/Cache/ClearCommandTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,16 @@ public function tearDown()
public function testClearWithNoStoreArgument()
{
$command = new ClearCommandTestStub(
$cacheManager = m::mock('Illuminate\Cache\CacheManager')
$cacheManager = m::mock('Illuminate\Cache\CacheManager'),
$files = m::mock('Illuminate\Filesystem\Filesystem')
);

$cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');

$app = new Application;
$app['path.storage'] = __DIR__;
$command->setLaravel($app);
$files->shouldReceive('files')->andReturn([]);

$cacheManager->shouldReceive('store')->once()->with(null)->andReturn($cacheRepository);
$cacheRepository->shouldReceive('flush')->once();
Expand All @@ -34,13 +37,16 @@ public function testClearWithNoStoreArgument()
public function testClearWithStoreArgument()
{
$command = new ClearCommandTestStub(
$cacheManager = m::mock('Illuminate\Cache\CacheManager')
$cacheManager = m::mock('Illuminate\Cache\CacheManager'),
$files = m::mock('Illuminate\Filesystem\Filesystem')
);

$cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');

$app = new Application;
$app['path.storage'] = __DIR__;
$command->setLaravel($app);
$files->shouldReceive('files')->andReturn([]);

$cacheManager->shouldReceive('store')->once()->with('foo')->andReturn($cacheRepository);
$cacheRepository->shouldReceive('flush')->once();
Expand All @@ -55,13 +61,16 @@ public function testClearWithStoreArgument()
public function testClearWithInvalidStoreArgument()
{
$command = new ClearCommandTestStub(
$cacheManager = m::mock('Illuminate\Cache\CacheManager')
$cacheManager = m::mock('Illuminate\Cache\CacheManager'),
$files = m::mock('Illuminate\Filesystem\Filesystem')
);

$cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');

$app = new Application;
$app['path.storage'] = __DIR__;
$command->setLaravel($app);
$files->shouldReceive('files')->andReturn([]);

$cacheManager->shouldReceive('store')->once()->with('bar')->andThrow('InvalidArgumentException');
$cacheRepository->shouldReceive('flush')->never();
Expand All @@ -72,13 +81,16 @@ public function testClearWithInvalidStoreArgument()
public function testClearWithTagsOption()
{
$command = new ClearCommandTestStub(
$cacheManager = m::mock('Illuminate\Cache\CacheManager')
$cacheManager = m::mock('Illuminate\Cache\CacheManager'),
$files = m::mock('Illuminate\Filesystem\Filesystem')
);

$cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');

$app = new Application;
$app['path.storage'] = __DIR__;
$command->setLaravel($app);
$files->shouldReceive('files')->andReturn([]);

$cacheManager->shouldReceive('store')->once()->with(null)->andReturn($cacheRepository);
$cacheRepository->shouldReceive('tags')->once()->with(['foo', 'bar'])->andReturn($cacheRepository);
Expand All @@ -90,13 +102,16 @@ public function testClearWithTagsOption()
public function testClearWithStoreArgumentAndTagsOption()
{
$command = new ClearCommandTestStub(
$cacheManager = m::mock('Illuminate\Cache\CacheManager')
$cacheManager = m::mock('Illuminate\Cache\CacheManager'),
$files = m::mock('Illuminate\Filesystem\Filesystem')
);

$cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');

$app = new Application;
$app['path.storage'] = __DIR__;
$command->setLaravel($app);
$files->shouldReceive('files')->andReturn([]);

$cacheManager->shouldReceive('store')->once()->with('redis')->andReturn($cacheRepository);
$cacheRepository->shouldReceive('tags')->once()->with(['foo'])->andReturn($cacheRepository);
Expand All @@ -105,6 +120,27 @@ public function testClearWithStoreArgumentAndTagsOption()
$this->runCommand($command, ['store' => 'redis', '--tags' => 'foo']);
}

public function testClearWillClearsRealTimeFacades()
{
$command = new ClearCommandTestStub(
$cacheManager = m::mock('Illuminate\Cache\CacheManager'),
$files = m::mock('Illuminate\Filesystem\Filesystem')
);

$cacheRepository = m::mock('Illuminate\Contracts\Cache\Repository');

$app = new Application;
$app['path.storage'] = __DIR__;
$command->setLaravel($app);
$cacheManager->shouldReceive('store')->once()->with(null)->andReturn($cacheRepository);
$cacheRepository->shouldReceive('flush')->once();

$files->shouldReceive('files')->andReturn(['/facade-XXXX.php']);
$files->shouldReceive('delete')->with('/facade-XXXX.php')->once();

$this->runCommand($command);
}

protected function runCommand($command, $input = [])
{
return $command->run(new \Symfony\Component\Console\Input\ArrayInput($input), new \Symfony\Component\Console\Output\NullOutput);
Expand Down

2 comments on commit 489c9f9

@Jalle19
Copy link
Contributor

Choose a reason for hiding this comment

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

@taylorotwell @calebporzio this breaks php artisan cache:clear when using e.g. Redis as the cache driver and the framework/cache directory doesn't exist.

@Rah1x
Copy link

@Rah1x Rah1x commented on 489c9f9 Mar 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

As mentioned by Jalle19, this assumes we have a directory framework/cache without even checking if we even have that directory and hence it breaks the artisan cache:clear command. (Took me hours and hours to finally come here and realise it).

Solution:
Manually create the storage/framework/cache directory.

Please sign in to comment.