From fb3a43b2f0cdb657ebe0b9ebdd5be56df0f484c7 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 1 Nov 2012 18:02:08 +0100 Subject: [PATCH 1/3] Add local cache to dist downloads --- src/Composer/Cache.php | 38 +++++++++++++++++++--- src/Composer/Downloader/FileDownloader.php | 12 ++++++- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/src/Composer/Cache.php b/src/Composer/Cache.php index 9e14ceaff..7ae277089 100644 --- a/src/Composer/Cache.php +++ b/src/Composer/Cache.php @@ -13,6 +13,7 @@ namespace Composer; use Composer\IO\IOInterface; +use Composer\Util\Filesystem; /** * Reads/writes to a filesystem cache @@ -24,11 +25,15 @@ class Cache private $io; private $root; private $enabled = true; + private $whitelist; + private $filesystem; - public function __construct(IOInterface $io, $cacheDir) + public function __construct(IOInterface $io, $cacheDir, $whitelist = 'a-z0-9.', Filesystem $filesystem = null) { $this->io = $io; $this->root = rtrim($cacheDir, '/\\') . '/'; + $this->whitelist = $whitelist; + $this->filesystem = $filesystem ?: new Filesystem(); if (!is_dir($this->root)) { if (!@mkdir($this->root, 0777, true)) { @@ -44,7 +49,7 @@ class Cache public function read($file) { - $file = preg_replace('{[^a-z0-9.]}i', '-', $file); + $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); if ($this->enabled && file_exists($this->root . $file)) { return file_get_contents($this->root . $file); } @@ -53,14 +58,37 @@ class Cache public function write($file, $contents) { if ($this->enabled) { - $file = preg_replace('{[^a-z0-9.]}i', '-', $file); + $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); file_put_contents($this->root . $file, $contents); } } + public function copyFrom($file, $source) + { + if ($this->enabled) { + $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); + $this->filesystem->ensureDirectoryExists(dirname($this->root . $file)); + copy($source, $this->root . $file); + } + } + + public function copyTo($file, $target) + { + $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); + if ($this->enabled && file_exists($this->root . $file)) { + touch($this->root . $file); + return copy($this->root . $file, $target); + } + } + + public function gc($expire) + { + // TODO + } + public function sha1($file) { - $file = preg_replace('{[^a-z0-9.]}i', '-', $file); + $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); if ($this->enabled && file_exists($this->root . $file)) { return sha1_file($this->root . $file); } @@ -68,7 +96,7 @@ class Cache public function sha256($file) { - $file = preg_replace('{[^a-z0-9.]}i', '-', $file); + $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); if ($this->enabled && file_exists($this->root . $file)) { return hash_file('sha256', $this->root . $file); } diff --git a/src/Composer/Downloader/FileDownloader.php b/src/Composer/Downloader/FileDownloader.php index b567e5a5c..ab202ab11 100644 --- a/src/Composer/Downloader/FileDownloader.php +++ b/src/Composer/Downloader/FileDownloader.php @@ -13,6 +13,7 @@ namespace Composer\Downloader; use Composer\Config; +use Composer\Cache; use Composer\IO\IOInterface; use Composer\Package\PackageInterface; use Composer\Package\Version\VersionParser; @@ -33,6 +34,7 @@ class FileDownloader implements DownloaderInterface protected $config; protected $rfs; protected $filesystem; + protected $cache; /** * Constructor. @@ -48,6 +50,11 @@ class FileDownloader implements DownloaderInterface $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_./'); + + if (!rand(0, 50)) { + $this->cache->gc($config->get('cache-ttl') ?: 86400 * 30); + } } /** @@ -78,7 +85,10 @@ class FileDownloader implements DownloaderInterface try { try { - $this->rfs->copy(parse_url($processUrl, PHP_URL_HOST), $processUrl, $fileName); + if (!$this->cache->copyTo($package->getName().'/'.$package->getVersion().'-'.$package->getDistReference().'.'.$package->getDistType(), $fileName)) { + $this->rfs->copy(parse_url($processUrl, PHP_URL_HOST), $processUrl, $fileName); + $this->cache->copyFrom($package->getName().'/'.$package->getVersion().'-'.$package->getDistReference().'.'.$package->getDistType(), $fileName); + } } catch (TransportException $e) { if (404 === $e->getCode() && 'github.com' === parse_url($processUrl, PHP_URL_HOST)) { $message = "\n".'Could not fetch '.$processUrl.', enter your GitHub credentials to access private repos'; From 79bf55e505eb2d2b0787e36b66393f16b5409951 Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 10 Nov 2012 22:17:46 +0100 Subject: [PATCH 2/3] Standardize return valuse of the cache class --- src/Composer/Cache.php | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/Composer/Cache.php b/src/Composer/Cache.php index 7ae277089..5a46b9eb6 100644 --- a/src/Composer/Cache.php +++ b/src/Composer/Cache.php @@ -53,14 +53,19 @@ class Cache if ($this->enabled && file_exists($this->root . $file)) { return file_get_contents($this->root . $file); } + + return false; } public function write($file, $contents) { if ($this->enabled) { $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); - file_put_contents($this->root . $file, $contents); + + return file_put_contents($this->root . $file, $contents); } + + return false; } public function copyFrom($file, $source) @@ -68,8 +73,11 @@ class Cache if ($this->enabled) { $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); $this->filesystem->ensureDirectoryExists(dirname($this->root . $file)); - copy($source, $this->root . $file); + + return copy($source, $this->root . $file); } + + return false; } public function copyTo($file, $target) @@ -77,8 +85,11 @@ class Cache $file = preg_replace('{[^'.$this->whitelist.']}i', '-', $file); if ($this->enabled && file_exists($this->root . $file)) { touch($this->root . $file); + return copy($this->root . $file, $target); } + + return false; } public function gc($expire) @@ -92,6 +103,8 @@ class Cache if ($this->enabled && file_exists($this->root . $file)) { return sha1_file($this->root . $file); } + + return false; } public function sha256($file) @@ -100,5 +113,7 @@ class Cache if ($this->enabled && file_exists($this->root . $file)) { return hash_file('sha256', $this->root . $file); } + + return false; } } From 5a9d986e67d40049b28ffaedfb52f145eba5bbdc Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sat, 10 Nov 2012 23:17:36 +0100 Subject: [PATCH 3/3] 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);