Use 'cache-files-ttl' for cache gc, fixes #2441

The configuration option 'cache-ttl' was used instead of 'cache-files-ttl' to determine
whether or not a cache gc should be performed.

* changed 'cache-ttl' to 'cache-files-ttl' to determine if a gc should be performed
* refactored FileDownloader to allow for easier testing
* added test to ensure that the gc is called with the proper config option

src/Composer/Cache.php

@@ -23,6 +23,7 @@ use Symfony\Component\Finder\Finder;
  */
 class Cache
 {
+    private static $cacheCollected = false;
     private $io;
     private $root;
     private $enabled = true;
@@ -126,6 +127,11 @@ class Cache
         return false;
     }
 
+    public function gcIsNecessary()
+    {
+       return (!self::$cacheCollected && !mt_rand(0, 50));
+    }
+
     public function remove($file)
     {
         $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file);
@@ -157,6 +163,8 @@ class Cache
             }
         }
 
+        self::$cacheCollected = true;
+
         return true;
     }
 

src/Composer/Downloader/FileDownloader.php

@@ -34,7 +34,6 @@ use Composer\Util\RemoteFilesystem;
  */
 class FileDownloader implements DownloaderInterface
 {
-    private static $cacheCollected = false;
     protected $io;
     protected $config;
     protected $rfs;
@@ -61,10 +60,10 @@ class FileDownloader implements DownloaderInterface
         $this->filesystem = $filesystem ?: new Filesystem();
         $this->cache = $cache;
 
-        if ($this->cache && !self::$cacheCollected && !mt_rand(0, 50)) {
-            $this->cache->gc($config->get('cache-ttl'), $config->get('cache-files-maxsize'));
+
+        if ($this->cache && $this->cache->gcIsNecessary()) {
+            $this->cache->gc($config->get('cache-files-ttl'), $config->get('cache-files-maxsize'));
         }
-        self::$cacheCollected = true;
     }
 
     /**

tests/Composer/Test/Downloader/FileDownloaderTest.php

@@ -17,13 +17,13 @@ use Composer\Util\Filesystem;
 
 class FileDownloaderTest extends \PHPUnit_Framework_TestCase
 {
-    protected function getDownloader($io = null, $config = null, $rfs = null, $filesystem = null)
+    protected function getDownloader($io = null, $config = null, $eventDispatcher = null, $cache = null, $rfs = null, $filesystem = null)
     {
         $io = $io ?: $this->getMock('Composer\IO\IOInterface');
         $config = $config ?: $this->getMock('Composer\Config');
         $rfs = $rfs ?: $this->getMockBuilder('Composer\Util\RemoteFilesystem')->disableOriginalConstructor()->getMock();
 
-        return new FileDownloader($io, $config, null, null, $rfs, $filesystem);
+        return new FileDownloader($io, $config, $eventDispatcher, $cache, $rfs, $filesystem);
     }
 
     /**
@@ -123,6 +123,37 @@ class FileDownloaderTest extends \PHPUnit_Framework_TestCase
         }
     }
 
+    public function testCacheGarbageCollectionIsCalled()
+    {
+        $expectedTtl = '99999999';
+
+        $configMock = $this->getMock('Composer\Config');
+        $configMock
+            ->expects($this->at(0))
+            ->method('get')
+            ->with('cache-files-ttl')
+            ->will($this->returnValue($expectedTtl));
+        $configMock
+            ->expects($this->at(1))
+            ->method('get')
+            ->with('cache-files-maxsize')
+            ->will($this->returnValue('500M'));
+
+        $cacheMock = $this->getMockBuilder('Composer\Cache')
+                     ->disableOriginalConstructor()
+                     ->getMock();
+        $cacheMock
+            ->expects($this->any())
+            ->method('gcIsNecessary')
+            ->will($this->returnValue(true));
+        $cacheMock
+            ->expects($this->once())
+            ->method('gc')
+            ->with($expectedTtl, $this->anything());
+
+        $downloader = $this->getDownloader(null, $configMock, null, $cacheMock, null, null);
+    }
+
     public function testDownloadFileWithInvalidChecksum()
     {
         $packageMock = $this->getMock('Composer\Package\PackageInterface');
@@ -140,7 +171,7 @@ class FileDownloaderTest extends \PHPUnit_Framework_TestCase
             $path = sys_get_temp_dir().'/'.md5(time().mt_rand());
         } while (file_exists($path));
 
-        $downloader = $this->getDownloader(null, null, null, $filesystem);
+        $downloader = $this->getDownloader(null, null, null, null, null, $filesystem);
 
         // make sure the file expected to be downloaded is on disk already
         mkdir($path, 0777, true);