From 5ed18d9aa2d08931cb9b4a61cb8beab388011e8d Mon Sep 17 00:00:00 2001 From: Nils Adermann Date: Mon, 24 Feb 2014 18:40:33 +0100 Subject: [PATCH] Fail over from source to dist and vice versa when downloads fail Any RuntimeException descendent will be caught and cause another download attempt using either source or dist depending on what was attempted first. --- src/Composer/Downloader/DownloadManager.php | 47 ++++++-- src/Composer/Factory.php | 2 +- tests/Composer/Test/ComposerTest.php | 3 +- .../Test/Downloader/DownloadManagerTest.php | 104 ++++++++++++++---- tests/Composer/Test/InstallerTest.php | 2 +- 5 files changed, 123 insertions(+), 35 deletions(-) diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index 51649cc3a..39a16611e 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -14,6 +14,7 @@ namespace Composer\Downloader; use Composer\Package\PackageInterface; use Composer\Downloader\DownloaderInterface; +use Composer\IO\IOInterface; use Composer\Util\Filesystem; /** @@ -23,6 +24,7 @@ use Composer\Util\Filesystem; */ class DownloadManager { + private $io; private $preferDist = false; private $preferSource = false; private $filesystem; @@ -31,11 +33,13 @@ class DownloadManager /** * Initializes download manager. * + * @param IOInterface $io The Input Output Interface * @param bool $preferSource prefer downloading from source * @param Filesystem|null $filesystem custom Filesystem object */ - public function __construct($preferSource = false, Filesystem $filesystem = null) + public function __construct(IOInterface $io, $preferSource = false, Filesystem $filesystem = null) { + $this->io = $io; $this->preferSource = $preferSource; $this->filesystem = $filesystem ?: new Filesystem(); } @@ -168,19 +172,44 @@ class DownloadManager $sourceType = $package->getSourceType(); $distType = $package->getDistType(); - if ((!$package->isDev() || $this->preferDist || !$sourceType) && !($preferSource && $sourceType) && $distType) { - $package->setInstallationSource('dist'); - } elseif ($sourceType) { - $package->setInstallationSource('source'); - } else { + $wantDist = !$package->isDev() || $this->preferDist || !$sourceType; + $wantSource = $preferSource && $sourceType; + + $types = array(); + if ($sourceType) { + $types[] = 'source'; + } + if ($distType) { + $types[] = 'dist'; + } + + if (empty($types)) { throw new \InvalidArgumentException('Package '.$package.' must have a source or dist specified'); } + if ($wantDist && !$wantSource) { + $types = array_reverse($types); + } + $this->filesystem->ensureDirectoryExists($targetDir); - $downloader = $this->getDownloaderForInstalledPackage($package); - if ($downloader) { - $downloader->download($package, $targetDir); + foreach ($types as $source) { + $package->setInstallationSource($source); + try { + $downloader = $this->getDownloaderForInstalledPackage($package); + if ($downloader) { + $downloader->download($package, $targetDir); + } + break; + } catch (\RuntimeException $e) { + $this->io->write( + 'Caught an exception while trying to download '. + $package->getPrettyString(). + ': '. + $e->getMessage().'' + ); + continue; + } } } diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index f829cb459..27f83dd87 100644 --- a/src/Composer/Factory.php +++ b/src/Composer/Factory.php @@ -344,7 +344,7 @@ class Factory $cache = new Cache($io, $config->get('cache-files-dir'), 'a-z0-9_./'); } - $dm = new Downloader\DownloadManager(); + $dm = new Downloader\DownloadManager($io); switch ($config->get('preferred-install')) { case 'dist': $dm->setPreferDist(true); diff --git a/tests/Composer/Test/ComposerTest.php b/tests/Composer/Test/ComposerTest.php index f667e88e5..df00c36f7 100644 --- a/tests/Composer/Test/ComposerTest.php +++ b/tests/Composer/Test/ComposerTest.php @@ -47,7 +47,8 @@ class ComposerTest extends TestCase public function testSetGetDownloadManager() { $composer = new Composer(); - $manager = $this->getMock('Composer\Downloader\DownloadManager'); + $io = $this->getMock('Composer\IO\IOInterface'); + $manager = $this->getMock('Composer\Downloader\DownloadManager', array(), array($io)); $composer->setDownloadManager($manager); $this->assertSame($manager, $composer->getDownloadManager()); diff --git a/tests/Composer/Test/Downloader/DownloadManagerTest.php b/tests/Composer/Test/Downloader/DownloadManagerTest.php index 48242a818..1728c583e 100644 --- a/tests/Composer/Test/Downloader/DownloadManagerTest.php +++ b/tests/Composer/Test/Downloader/DownloadManagerTest.php @@ -17,16 +17,18 @@ use Composer\Downloader\DownloadManager; class DownloadManagerTest extends \PHPUnit_Framework_TestCase { protected $filesystem; + protected $io; public function setUp() { $this->filesystem = $this->getMock('Composer\Util\Filesystem'); + $this->io = $this->getMock('Composer\IO\IOInterface'); } public function testSetGetDownloader() { $downloader = $this->createDownloaderMock(); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $manager->setDownloader('test', $downloader); $this->assertSame($downloader, $manager->getDownloader('test')); @@ -43,7 +45,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getInstallationSource') ->will($this->returnValue(null)); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $this->setExpectedException('InvalidArgumentException'); @@ -69,7 +71,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('dist')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -101,7 +103,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('source')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -135,7 +137,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('source')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -167,7 +169,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->will($this->returnValue('dist')); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloader')) ->getMock(); @@ -190,7 +192,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getType') ->will($this->returnValue('metapackage')); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $this->assertNull($manager->getDownloaderForInstalledPackage($package)); } @@ -219,7 +221,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -231,6 +233,62 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager->download($package, 'target_dir'); } + public function testFullPackageDownloadFailover() + { + $package = $this->createPackageMock(); + $package + ->expects($this->once()) + ->method('getSourceType') + ->will($this->returnValue('git')); + $package + ->expects($this->once()) + ->method('getDistType') + ->will($this->returnValue('pear')); + $package + ->expects($this->any()) + ->method('getPrettyString') + ->will($this->returnValue('prettyPackage')); + + $package + ->expects($this->at(3)) + ->method('setInstallationSource') + ->with('dist'); + $package + ->expects($this->at(5)) + ->method('setInstallationSource') + ->with('source'); + + $downloaderFail = $this->createDownloaderMock(); + $downloaderFail + ->expects($this->once()) + ->method('download') + ->with($package, 'target_dir') + ->will($this->throwException(new \RuntimeException("Foo"))); + + $downloaderSuccess = $this->createDownloaderMock(); + $downloaderSuccess + ->expects($this->once()) + ->method('download') + ->with($package, 'target_dir'); + + $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') + ->setConstructorArgs(array($this->io, false, $this->filesystem)) + ->setMethods(array('getDownloaderForInstalledPackage')) + ->getMock(); + $manager + ->expects($this->at(0)) + ->method('getDownloaderForInstalledPackage') + ->with($package) + ->will($this->returnValue($downloaderFail)); + $manager + ->expects($this->at(1)) + ->method('getDownloaderForInstalledPackage') + ->with($package) + ->will($this->returnValue($downloaderSuccess)); + + $manager->download($package, 'target_dir'); + } + public function testBadPackageDownload() { $package = $this->createPackageMock(); @@ -243,7 +301,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistType') ->will($this->returnValue(null)); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $this->setExpectedException('InvalidArgumentException'); $manager->download($package, 'target_dir'); @@ -273,7 +331,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -309,7 +367,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -339,7 +397,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with('source'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -375,7 +433,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -412,7 +470,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -449,7 +507,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'target_dir'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -474,7 +532,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistType') ->will($this->returnValue(null)); - $manager = new DownloadManager(false, $this->filesystem); + $manager = new DownloadManager($this->io, false, $this->filesystem); $manager->setPreferSource(true); $this->setExpectedException('InvalidArgumentException'); @@ -510,7 +568,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, $target, 'vendor/bundles/FOS/UserBundle'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -547,7 +605,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, 'vendor/bundles/FOS/UserBundle'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage', 'download')) ->getMock(); $manager @@ -588,7 +646,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, $target, 'vendor/pkg'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage', 'download')) ->getMock(); $manager @@ -625,7 +683,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($initial, 'vendor/pkg'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage', 'download')) ->getMock(); $manager @@ -647,7 +705,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $target = $this->createPackageMock(); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -670,7 +728,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->with($package, 'vendor/bundles/FOS/UserBundle'); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager @@ -687,7 +745,7 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $package = $this->createPackageMock(); $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') - ->setConstructorArgs(array(false, $this->filesystem)) + ->setConstructorArgs(array($this->io, false, $this->filesystem)) ->setMethods(array('getDownloaderForInstalledPackage')) ->getMock(); $manager diff --git a/tests/Composer/Test/InstallerTest.php b/tests/Composer/Test/InstallerTest.php index 4e992c577..cc17973bc 100644 --- a/tests/Composer/Test/InstallerTest.php +++ b/tests/Composer/Test/InstallerTest.php @@ -51,7 +51,7 @@ class InstallerTest extends TestCase { $io = $this->getMock('Composer\IO\IOInterface'); - $downloadManager = $this->getMock('Composer\Downloader\DownloadManager'); + $downloadManager = $this->getMock('Composer\Downloader\DownloadManager', array(), array($io)); $config = $this->getMock('Composer\Config'); $repositoryManager = new RepositoryManager($io, $config);