From 5a9d986e67d40049b28ffaedfb52f145eba5bbdc Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 10 Nov 2012 23:17:36 +0100 Subject: [PATCH] Implement cache GC and fix keys --- src/Composer/Cache.php | 19 ++++++++++++-- src/Composer/Config.php | 1 + src/Composer/Downloader/FileDownloader.php | 26 ++++++++++++++----- src/Composer/Downloader/ZipDownloader.php | 5 ++-- src/Composer/Factory.php | 10 ++++--- .../Test/Downloader/FileDownloaderTest.php | 13 ++++++---- 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/src/Composer/Cache.php b/src/Composer/Cache.php index 5a46b9eb6..eb9548a17 100644 --- a/src/Composer/Cache.php +++ b/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) diff --git a/src/Composer/Config.php b/src/Composer/Config.php index df8563e81..803b3cbf3 100644 --- a/src/Composer/Config.php +++ b/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, diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index ab202ab11..12c5d2471 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/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)) { - $this->cache->gc($config->get('cache-ttl') ?: 86400 * 30); + if ($this->cache && !self::$cacheCollected && !rand(0, 50)) { + $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(); + } } diff --git a/src/Composer/Downloader/ZipDownloader.php b/src/Composer/Downloader/ZipDownloader.php index dff84c504..027d63033 100644 --- a/src/Composer/Downloader/ZipDownloader.php +++ b/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) diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index 96d30aee8..7a49131cb 100644 --- a/src/Composer/Factory.php +++ b/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('tar', new Downloader\TarDownloader($io, $config)); - $dm->setDownloader('phar', new Downloader\PharDownloader($io, $config)); - $dm->setDownloader('file', new Downloader\FileDownloader($io, $config)); + $dm->setDownloader('zip', new Downloader\ZipDownloader($io, $config, $cache)); + $dm->setDownloader('tar', new Downloader\TarDownloader($io, $config, $cache)); + $dm->setDownloader('phar', new Downloader\PharDownloader($io, $config, $cache)); + $dm->setDownloader('file', new Downloader\FileDownloader($io, $config, $cache)); return $dm; } diff --git a/tests/Composer/Test/Downloader/FileDownloaderTest.php b/tests/Composer/Test/Downloader/FileDownloaderTest.php index e68fe32f1..83a1c7b5f 100644 --- a/tests/Composer/Test/Downloader/FileDownloaderTest.php +++ b/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)) { - unset($path); + if (is_dir($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);