Refactored DownloadManager

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) {
+        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());
-            $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');
+            $downloader = $this->getDownloader($package->getSourceType());
         }
+
+        $downloader->remove($package, $targetDir);
     }
 }

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
      */

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
      *

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 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 testUpdateSource()
+    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()