From 17d1abcec03c02710fa12420a87b47d5618af740 Mon Sep 17 00:00:00 2001 From: everzet Date: Sun, 25 Sep 2011 18:30:54 +0300 Subject: [PATCH] Refactored DownloadManager --- src/Composer/Downloader/DownloadManager.php | 86 ++++++---- src/Composer/Package/MemoryPackage.php | 17 ++ src/Composer/Package/PackageInterface.php | 14 ++ .../Test/Downloader/DownloadManagerTest.php | 159 +++++++++++++++++- 4 files changed, 235 insertions(+), 41 deletions(-) diff --git a/src/Composer/Downloader/DownloadManager.php b/src/Composer/Downloader/DownloadManager.php index 5b15f54cf..96c18c4a5 100644 --- a/src/Composer/Downloader/DownloadManager.php +++ b/src/Composer/Downloader/DownloadManager.php @@ -77,35 +77,33 @@ class DownloadManager /** * Downloads package into target dir. * - * @param PackageInterface $package package instance - * @param string $targetDir target dir + * @param PackageInterface $package package instance + * @param string $targetDir target dir + * @param Boolean $preferSource prefer installation from source * - * @return string downloader type (source/dist) + * @return string downloader type (source/dist) * - * @throws InvalidArgumentException if package have no urls to download from + * @throws InvalidArgumentException if package have no urls to download from */ - public function download(PackageInterface $package, $targetDir) + public function download(PackageInterface $package, $targetDir, $preferSource = null) { - $sourceType = $package->getSourceType(); - $distType = $package->getDistType(); + $preferSource = null !== $preferSource ? $preferSource : $this->preferSource; + $sourceType = $package->getSourceType(); + $distType = $package->getDistType(); - if (!($this->preferSource && $sourceType) && $distType) { + if (!($preferSource && $sourceType) && $distType) { $downloader = $this->getDownloader($distType); $downloader->download( $package, $targetDir, $package->getDistUrl(), $package->getDistSha1Checksum() ); - - return 'dist'; - } - - if ($sourceType) { + $package->setInstallationSource('dist'); + } elseif ($sourceType) { $downloader = $this->getDownloader($sourceType); $downloader->download($package, $targetDir, $package->getSourceUrl()); - - return 'source'; + $package->setInstallationSource('source'); + } else { + throw new \InvalidArgumentException('Package should have dist or source specified'); } - - throw new \InvalidArgumentException('Package should have dist or source specified'); } /** @@ -114,20 +112,33 @@ class DownloadManager * @param PackageInterface $initial initial package version * @param PackageInterface $target target package version * @param string $targetDir target dir - * @param string $type downloader type (source/dist) * * @throws InvalidArgumentException if initial package is not installed */ - public function update(PackageInterface $initial, PackageInterface $target, $targetDir, $type) + public function update(PackageInterface $initial, PackageInterface $target, $targetDir) { - if ('dist' === $type) { - $downloader = $this->getDownloader($initial->getDistType()); - $downloader->update($initial, $target, $targetDir); - } elseif ('source' === $type) { - $downloader = $this->getDownloader($initial->getSourceType()); + if (null === $installationType = $initial->getInstallationSource()) { + throw new \InvalidArgumentException( + 'Package '.$initial.' was not been installed propertly and can not be updated' + ); + } + $useSource = 'source' === $installationType; + + if (!$useSource) { + $initialType = $initial->getDistType(); + $targetType = $target->getDistType(); + } else { + $initialType = $initial->getSourceType(); + $targetType = $target->getSourceType(); + } + + $downloader = $this->getDownloader($initialType); + + if ($initialType === $targetType) { $downloader->update($initial, $target, $targetDir); } else { - throw new \InvalidArgumentException('Package should have dist or source specified'); + $downloader->remove($initial, $targetDir); + $this->download($target, $targetDir, $useSource); } } @@ -136,18 +147,23 @@ class DownloadManager * * @param PackageInterface $package package instance * @param string $targetDir target dir - * @param string $type downloader type (source/dist) */ - public function remove(PackageInterface $package, $targetDir, $type) + public function remove(PackageInterface $package, $targetDir) { - if ('dist' === $type) { - $downloader = $this->getDownloader($package->getDistType()); - $downloader->remove($package, $targetDir); - } elseif ('source' === $type) { - $downloader = $this->getDownloader($package->getSourceType()); - $downloader->remove($package, $targetDir); - } else { - throw new \InvalidArgumentException('Package should have dist or source specified'); + if (null === $installationType = $package->getInstallationSource()) { + throw new \InvalidArgumentException( + 'Package '.$package.' was not been installed propertly and can not be removed' + ); } + $useSource = 'source' === $installationType; + + // get proper downloader + if (!$useSource) { + $downloader = $this->getDownloader($package->getDistType()); + } else { + $downloader = $this->getDownloader($package->getSourceType()); + } + + $downloader->remove($package, $targetDir); } } diff --git a/src/Composer/Package/MemoryPackage.php b/src/Composer/Package/MemoryPackage.php index 0476f86e6..535b73eb1 100644 --- a/src/Composer/Package/MemoryPackage.php +++ b/src/Composer/Package/MemoryPackage.php @@ -20,6 +20,7 @@ namespace Composer\Package; class MemoryPackage extends BasePackage { protected $type; + protected $installationSource; protected $sourceType; protected $sourceUrl; protected $distType; @@ -84,6 +85,22 @@ class MemoryPackage extends BasePackage return $this->extra; } + /** + * {@inheritDoc} + */ + public function setInstallationSource($type) + { + $this-> installationSource = $type; + } + + /** + * {@inheritDoc} + */ + public function getInstallationSource() + { + return $this->installationSource; + } + /** * @param string $type */ diff --git a/src/Composer/Package/PackageInterface.php b/src/Composer/Package/PackageInterface.php index 47b81814c..69e2ed497 100644 --- a/src/Composer/Package/PackageInterface.php +++ b/src/Composer/Package/PackageInterface.php @@ -82,6 +82,20 @@ interface PackageInterface */ function getExtra(); + /** + * Sets source from which this package was installed (source/dist). + * + * @param string $type source/dist + */ + function setInstallationSource($type); + + /** + * Returns source from which this package was installed (source/dist). + * + * @param string $type source/dist + */ + function getInstallationSource(); + /** * Returns the repository type of this package, e.g. git, svn * diff --git a/tests/Composer/Test/Downloader/DownloadManagerTest.php b/tests/Composer/Test/Downloader/DownloadManagerTest.php index efac5f0ba..bf39a377a 100644 --- a/tests/Composer/Test/Downloader/DownloadManagerTest.php +++ b/tests/Composer/Test/Downloader/DownloadManagerTest.php @@ -49,6 +49,11 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistSha1Checksum') ->will($this->returnValue('sha1')); + $package + ->expects($this->once()) + ->method('setInstallationSource') + ->with('dist'); + $pearDownloader = $this->createDownloaderMock(); $pearDownloader ->expects($this->once()) @@ -100,6 +105,11 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistSha1Checksum') ->will($this->returnValue('sha1')); + $package + ->expects($this->once()) + ->method('setInstallationSource') + ->with('dist'); + $pearDownloader = $this->createDownloaderMock(); $pearDownloader ->expects($this->once()) @@ -129,6 +139,11 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getSourceUrl') ->will($this->returnValue('source_url')); + $package + ->expects($this->once()) + ->method('setInstallationSource') + ->with('source'); + $gitDownloader = $this->createDownloaderMock(); $gitDownloader ->expects($this->once()) @@ -158,6 +173,11 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getSourceUrl') ->will($this->returnValue('source_url')); + $package + ->expects($this->once()) + ->method('setInstallationSource') + ->with('source'); + $gitDownloader = $this->createDownloaderMock(); $gitDownloader ->expects($this->once()) @@ -192,6 +212,11 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getDistSha1Checksum') ->will($this->returnValue('sha1')); + $package + ->expects($this->once()) + ->method('setInstallationSource') + ->with('dist'); + $pearDownloader = $this->createDownloaderMock(); $pearDownloader ->expects($this->once()) @@ -222,6 +247,11 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase ->method('getSourceUrl') ->will($this->returnValue('source_url')); + $package + ->expects($this->once()) + ->method('setInstallationSource') + ->with('source'); + $gitDownloader = $this->createDownloaderMock(); $gitDownloader ->expects($this->once()) @@ -254,15 +284,23 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager->download($package, 'target_dir'); } - public function testUpdateDist() + public function testUpdateDistWithEqualTypes() { $initial = $this->createPackageMock(); + $initial + ->expects($this->once()) + ->method('getInstallationSource') + ->will($this->returnValue('dist')); $initial ->expects($this->once()) ->method('getDistType') ->will($this->returnValue('pear')); $target = $this->createPackageMock(); + $target + ->expects($this->once()) + ->method('getDistType') + ->will($this->returnValue('pear')); $pearDownloader = $this->createDownloaderMock(); $pearDownloader @@ -273,18 +311,62 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager = new DownloadManager(); $manager->setDownloader('pear', $pearDownloader); - $manager->update($initial, $target, 'vendor/bundles/FOS/UserBundle', 'dist'); + $manager->update($initial, $target, 'vendor/bundles/FOS/UserBundle'); } - public function testUpdateSource() + public function testUpdateDistWithNotEqualTypes() { $initial = $this->createPackageMock(); + $initial + ->expects($this->once()) + ->method('getInstallationSource') + ->will($this->returnValue('dist')); + $initial + ->expects($this->once()) + ->method('getDistType') + ->will($this->returnValue('pear')); + + $target = $this->createPackageMock(); + $target + ->expects($this->once()) + ->method('getDistType') + ->will($this->returnValue('composer')); + + $pearDownloader = $this->createDownloaderMock(); + $pearDownloader + ->expects($this->once()) + ->method('remove') + ->with($initial, 'vendor/bundles/FOS/UserBundle'); + + $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') + ->setMethods(array('download')) + ->getMock(); + $manager + ->expects($this->once()) + ->method('download') + ->with($target, 'vendor/bundles/FOS/UserBundle', false); + + $manager->setDownloader('pear', $pearDownloader); + $manager->update($initial, $target, 'vendor/bundles/FOS/UserBundle'); + } + + public function testUpdateSourceWithEqualTypes() + { + $initial = $this->createPackageMock(); + $initial + ->expects($this->once()) + ->method('getInstallationSource') + ->will($this->returnValue('source')); $initial ->expects($this->once()) ->method('getSourceType') ->will($this->returnValue('svn')); $target = $this->createPackageMock(); + $target + ->expects($this->once()) + ->method('getSourceType') + ->will($this->returnValue('svn')); $svnDownloader = $this->createDownloaderMock(); $svnDownloader @@ -295,12 +377,63 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager = new DownloadManager(); $manager->setDownloader('svn', $svnDownloader); - $manager->update($initial, $target, 'vendor/pkg', 'source'); + $manager->update($initial, $target, 'vendor/pkg'); + } + + public function testUpdateSourceWithNotEqualTypes() + { + $initial = $this->createPackageMock(); + $initial + ->expects($this->once()) + ->method('getInstallationSource') + ->will($this->returnValue('source')); + $initial + ->expects($this->once()) + ->method('getSourceType') + ->will($this->returnValue('svn')); + + $target = $this->createPackageMock(); + $target + ->expects($this->once()) + ->method('getSourceType') + ->will($this->returnValue('git')); + + $svnDownloader = $this->createDownloaderMock(); + $svnDownloader + ->expects($this->once()) + ->method('remove') + ->with($initial, 'vendor/pkg'); + + $manager = $this->getMockBuilder('Composer\Downloader\DownloadManager') + ->setMethods(array('download')) + ->getMock(); + $manager + ->expects($this->once()) + ->method('download') + ->with($target, 'vendor/pkg', true); + $manager->setDownloader('svn', $svnDownloader); + + $manager->update($initial, $target, 'vendor/pkg'); + } + + public function testUpdateBadlyInstalledPackage() + { + $initial = $this->createPackageMock(); + $target = $this->createPackageMock(); + + $this->setExpectedException('InvalidArgumentException'); + + $manager = new DownloadManager(); + $manager->update($initial, $target, 'vendor/pkg'); } public function testRemoveDist() { $package = $this->createPackageMock(); + $package + ->expects($this->once()) + ->method('getInstallationSource') + ->will($this->returnValue('dist')); $package ->expects($this->once()) ->method('getDistType') @@ -315,12 +448,16 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager = new DownloadManager(); $manager->setDownloader('pear', $pearDownloader); - $manager->remove($package, 'vendor/bundles/FOS/UserBundle', 'dist'); + $manager->remove($package, 'vendor/bundles/FOS/UserBundle'); } public function testRemoveSource() { $package = $this->createPackageMock(); + $package + ->expects($this->once()) + ->method('getInstallationSource') + ->will($this->returnValue('source')); $package ->expects($this->once()) ->method('getSourceType') @@ -335,7 +472,17 @@ class DownloadManagerTest extends \PHPUnit_Framework_TestCase $manager = new DownloadManager(); $manager->setDownloader('svn', $svnDownloader); - $manager->remove($package, 'vendor/pkg', 'source'); + $manager->remove($package, 'vendor/pkg'); + } + + public function testRemoveBadlyInstalledPackage() + { + $package = $this->createPackageMock(); + $manager = new DownloadManager(); + + $this->setExpectedException('InvalidArgumentException'); + + $manager->remove($package, 'vendor/pkg'); } private function createDownloaderMock()