Implement cache GC and fix keys

src/Composer/Cache.php

@@ -14,6 +14,7 @@ namespace Composer;
 
 use Composer\IO\IOInterface;
 use Composer\Util\Filesystem;
+use Symfony\Component\Finder\Finder;
 
 /**
  * Reads/writes to a filesystem cache
@@ -28,6 +29,12 @@ class Cache
     private $whitelist;
     private $filesystem;
 
+    /**
+     * @param IOInterface $io
+     * @param string      $cacheDir location of the cache
+     * @param string      $whitelist List of characters that are allowed in path names (used in a regex character class)
+     * @param Filesystem  $filesystem optional filesystem instance
+     */
     public function __construct(IOInterface $io, $cacheDir, $whitelist = 'a-z0-9.', Filesystem $filesystem = null)
     {
         $this->io = $io;
@@ -92,9 +99,17 @@ class Cache
         return false;
     }
 
-    public function gc($expire)
+    public function gc($ttl)
     {
-        // TODO
+        $expire = new \DateTime();
+        $expire->modify('-'.$ttl.' seconds');
+
+        $finder = Finder::create()->files()->in($this->root)->date('until '.$expire->format('Y-m-d H:i:s'));
+        foreach ($finder as $file) {
+            unlink($file->getRealPath());
+        }
+
+        return true;
     }
 
     public function sha1($file)

src/Composer/Config.php

@@ -21,6 +21,7 @@ class Config
 {
     public static $defaultConfig = array(
         'process-timeout' => 300,
+        'cache-ttl' => 15552000, // 6 months
         'vendor-dir' => 'vendor',
         'bin-dir' => '{$vendor-dir}/bin',
         'notify-on-install' => true,

src/Composer/Downloader/FileDownloader.php

@@ -30,6 +30,7 @@ use Composer\Util\RemoteFilesystem;
  */
 class FileDownloader implements DownloaderInterface
 {
+    private static $cacheCollected = false;
     protected $io;
     protected $config;
     protected $rfs;
@@ -41,20 +42,22 @@ class FileDownloader implements DownloaderInterface
      *
      * @param IOInterface      $io         The IO instance
      * @param Config           $config     The config
+     * @param Cache            $cache      Optional cache instance
      * @param RemoteFilesystem $rfs        The remote filesystem
      * @param Filesystem       $filesystem The filesystem
      */
-    public function __construct(IOInterface $io, Config $config, RemoteFilesystem $rfs = null, Filesystem $filesystem = null)
+    public function __construct(IOInterface $io, Config $config, Cache $cache = null, RemoteFilesystem $rfs = null, Filesystem $filesystem = null)
     {
         $this->io = $io;
         $this->config = $config;
         $this->rfs = $rfs ?: new RemoteFilesystem($io);
         $this->filesystem = $filesystem ?: new Filesystem();
-        $this->cache = new Cache($this->io, $config->get('home').'/cache.files/', 'a-z0-9_./');
+        $this->cache = $cache;
 
-        if (!rand(0, 50)) {
+        if ($this->cache && !self::$cacheCollected && !rand(0, 50)) {
-            $this->cache->gc($config->get('cache-ttl') ?: 86400 * 30);
+            $this->cache->gc($config->get('cache-ttl'));
         }
+        self::$cacheCollected = true;
     }
 
     /**
@@ -85,9 +88,11 @@ class FileDownloader implements DownloaderInterface
 
         try {
             try {
-                if (!$this->cache->copyTo($package->getName().'/'.$package->getVersion().'-'.$package->getDistReference().'.'.$package->getDistType(), $fileName)) {
+                if (!$this->cache || !$this->cache->copyTo($this->getCacheKey($package), $fileName)) {
                     $this->rfs->copy(parse_url($processUrl, PHP_URL_HOST), $processUrl, $fileName);
-                    $this->cache->copyFrom($package->getName().'/'.$package->getVersion().'-'.$package->getDistReference().'.'.$package->getDistType(), $fileName);
+                    if ($this->cache) {
+                        $this->cache->copyFrom($this->getCacheKey($package), $fileName);
+                    }
                 }
             } catch (TransportException $e) {
                 if (404 === $e->getCode() && 'github.com' === parse_url($processUrl, PHP_URL_HOST)) {
@@ -169,4 +174,13 @@ class FileDownloader implements DownloaderInterface
 
         return $url;
     }
+
+    private function getCacheKey(PackageInterface $package)
+    {
+        if (preg_match('{^[a-f0-9]{40}$}', $package->getDistReference())) {
+            return $package->getName().'/'.$package->getDistReference().'.'.$package->getDistType();
+        }
+
+        return $package->getName().'/'.$package->getVersion().'-'.$package->getDistReference().'.'.$package->getDistType();
+    }
 }

src/Composer/Downloader/ZipDownloader.php

@@ -13,6 +13,7 @@
 namespace Composer\Downloader;
 
 use Composer\Config;
+use Composer\Cache;
 use Composer\Util\ProcessExecutor;
 use Composer\IO\IOInterface;
 use ZipArchive;
@@ -24,10 +25,10 @@ class ZipDownloader extends ArchiveDownloader
 {
     protected $process;
 
-    public function __construct(IOInterface $io, Config $config, ProcessExecutor $process = null)
+    public function __construct(IOInterface $io, Config $config, Cache $cache = null, ProcessExecutor $process = null)
     {
         $this->process = $process ?: new ProcessExecutor;
-        parent::__construct($io, $config);
+        parent::__construct($io, $config, $cache);
     }
 
     protected function extract($file, $path)

src/Composer/Factory.php

@@ -240,14 +240,16 @@ class Factory
      */
     public function createDownloadManager(IOInterface $io, Config $config)
     {
+        $cache = new Cache($io, $config->get('home').'/cache.files/', 'a-z0-9_./');
+
         $dm = new Downloader\DownloadManager();
         $dm->setDownloader('git', new Downloader\GitDownloader($io, $config));
         $dm->setDownloader('svn', new Downloader\SvnDownloader($io, $config));
         $dm->setDownloader('hg', new Downloader\HgDownloader($io, $config));
-        $dm->setDownloader('zip', new Downloader\ZipDownloader($io, $config));
+        $dm->setDownloader('zip', new Downloader\ZipDownloader($io, $config, $cache));
-        $dm->setDownloader('tar', new Downloader\TarDownloader($io, $config));
+        $dm->setDownloader('tar', new Downloader\TarDownloader($io, $config, $cache));
-        $dm->setDownloader('phar', new Downloader\PharDownloader($io, $config));
+        $dm->setDownloader('phar', new Downloader\PharDownloader($io, $config, $cache));
-        $dm->setDownloader('file', new Downloader\FileDownloader($io, $config));
+        $dm->setDownloader('file', new Downloader\FileDownloader($io, $config, $cache));
 
         return $dm;
     }

tests/Composer/Test/Downloader/FileDownloaderTest.php

@@ -23,7 +23,7 @@ class FileDownloaderTest extends \PHPUnit_Framework_TestCase
         $config = $config ?: $this->getMock('Composer\Config');
         $rfs = $rfs ?: $this->getMockBuilder('Composer\Util\RemoteFilesystem')->disableOriginalConstructor()->getMock();
 
-        return new FileDownloader($io, $config, $rfs);
+        return new FileDownloader($io, $config, null, $rfs);
     }
 
     /**
@@ -56,8 +56,11 @@ class FileDownloaderTest extends \PHPUnit_Framework_TestCase
             $downloader->download($packageMock, $path);
             $this->fail();
         } catch (\Exception $e) {
-            if (file_exists($path)) {
+            if (is_dir($path)) {
-                unset($path);
+                $fs = new Filesystem();
+                $fs->removeDirectory($path);
+            } elseif (is_file($path)) {
+                unlink($path);
             }
             $this->assertInstanceOf('RuntimeException', $e);
             $this->assertContains('exists and is not a directory', $e->getMessage());
@@ -112,7 +115,7 @@ class FileDownloaderTest extends \PHPUnit_Framework_TestCase
                 $fs = new Filesystem();
                 $fs->removeDirectory($path);
             } elseif (is_file($path)) {
-                unset($path);
+                unlink($path);
             }
 
             $this->assertInstanceOf('UnexpectedValueException', $e);
@@ -150,7 +153,7 @@ class FileDownloaderTest extends \PHPUnit_Framework_TestCase
                 $fs = new Filesystem();
                 $fs->removeDirectory($path);
             } elseif (is_file($path)) {
-                unset($path);
+                unlink($path);
             }
 
             $this->assertInstanceOf('UnexpectedValueException', $e);