Review fixes

src/Composer/Repository/Pear/BaseChannelReader.php

@@ -26,17 +26,17 @@ abstract class BaseChannelReader
     /**
      * PEAR REST Interface namespaces
      */
-    const channelNS =               'http://pear.php.net/channel-1.0';
-    const allCategoriesNS =         'http://pear.php.net/dtd/rest.allcategories';
-    const categoryPackagesInfoNS =  'http://pear.php.net/dtd/rest.categorypackageinfo';
-    const allPackagesNS =           'http://pear.php.net/dtd/rest.allpackages';
-    const allReleasesNS =           'http://pear.php.net/dtd/rest.allreleases';
-    const packageInfoNS =           'http://pear.php.net/dtd/rest.package';
+    const CHANNEL_NS = 'http://pear.php.net/channel-1.0';
+    const ALL_CATEGORIES_NS = 'http://pear.php.net/dtd/rest.allcategories';
+    const CATEGORY_PACKAGES_INFO_NS = 'http://pear.php.net/dtd/rest.categorypackageinfo';
+    const ALL_PACKAGES_NS = 'http://pear.php.net/dtd/rest.allpackages';
+    const ALL_RELEASES_NS = 'http://pear.php.net/dtd/rest.allreleases';
+    const PACKAGE_INFO_NS = 'http://pear.php.net/dtd/rest.package';
 
     /** @var RemoteFilesystem */
     private $rfs;
 
-    protected function __construct($rfs)
+    protected function __construct(RemoteFilesystem $rfs)
     {
         $this->rfs = $rfs;
     }
@@ -53,7 +53,7 @@ abstract class BaseChannelReader
         $url = rtrim($origin, '/') . '/' . ltrim($path, '/');
         $content = $this->rfs->getContents($origin, $url, false);
         if (!$content) {
-            throw new \UnexpectedValueException('The PEAR channel at '.$url.' did not respond.');
+            throw new \UnexpectedValueException('The PEAR channel at ' . $url . ' did not respond.');
         }
 
         return $content;
@@ -73,7 +73,7 @@ abstract class BaseChannelReader
 
         if (false == $xml) {
             $url = rtrim($origin, '/') . '/' . ltrim($path, '/');
-            throw new \UnexpectedValueException('The PEAR channel at '.$origin.' is broken.');
+            throw new \UnexpectedValueException('The PEAR channel at ' . $origin . ' is broken.');
         }
 
         return $xml;

src/Composer/Repository/Pear/ChannelReader.php

@@ -43,28 +43,6 @@ class ChannelReader extends BaseChannelReader
         );
     }
 
-    /**
-     * Reads channel supported REST interfaces and selects one of them
-     *
-     * @param $channelXml \SimpleXMLElement
-     * @param $supportedVersions string[] supported PEAR REST protocols
-     * @return array|bool hash with selected version and baseUrl
-     */
-    private function selectRestVersion($channelXml, $supportedVersions)
-    {
-        $channelXml->registerXPathNamespace('ns', self::channelNS);
-
-        foreach ($supportedVersions as $version) {
-            $xpathTest = "ns:servers/ns:primary/ns:rest/ns:baseurl[@type='{$version}']";
-            $testResult = $channelXml->xpath($xpathTest);
-            if (count($testResult) > 0) {
-                return array('version' => $version, 'baseUrl' => (string) $testResult[0]);
-            }
-        }
-
-        return false;
-    }
-
     /**
      * Reads PEAR channel through REST interface and builds list of packages
      *
@@ -81,7 +59,7 @@ class ChannelReader extends BaseChannelReader
 
         $supportedVersions = array_keys($this->readerMap);
         $selectedRestVersion = $this->selectRestVersion($xml, $supportedVersions);
-        if (false === $selectedRestVersion) {
+        if (!$selectedRestVersion) {
             throw new \UnexpectedValueException(sprintf('PEAR repository $s does not supports any of %s protocols.', $url, implode(', ', $supportedVersions)));
         }
 
@@ -92,109 +70,24 @@ class ChannelReader extends BaseChannelReader
     }
 
     /**
-     * Builds MemoryPackages from PEAR package definition data.
+     * Reads channel supported REST interfaces and selects one of them
      *
-     * @param $channelName          string channel name
-     * @param $channelAlias         string channel alias
-     * @param $packageDefinitions   PackageInfo[]  package definition
-     * @return array
+     * @param $channelXml \SimpleXMLElement
+     * @param $supportedVersions string[] supported PEAR REST protocols
+     * @return array|null hash with selected version and baseUrl
      */
-    private function buildComposerPackages($channelName, $channelAlias, $packageDefinitions)
+    private function selectRestVersion($channelXml, $supportedVersions)
     {
-        $versionParser = new \Composer\Package\Version\VersionParser();
-        $result = array();
-        foreach ($packageDefinitions as $packageDefinition) {
-            foreach ($packageDefinition->getReleases() as $version => $releaseInfo) {
-                $normalizedVersion = $this->parseVersion($version);
-                if (false === $normalizedVersion) {
-                    continue; // skip packages with unparsable versions
-                }
+        $channelXml->registerXPathNamespace('ns', self::CHANNEL_NS);
 
-                $composerPackageName = $this->buildComposerPackageName($packageDefinition->getChannelName(), $packageDefinition->getPackageName());
-
-                // distribution url must be read from /r/{packageName}/{version}.xml::/r/g:text()
-                $distUrl = "http://{$packageDefinition->getChannelName()}/get/{$packageDefinition->getPackageName()}-{$version}.tgz";
-
-                $requires = array();
-                $suggests = array();
-                $conflicts = array();
-                $replaces = array();
-
-                // alias package only when its channel matches repository channel,
-                // cause we've know only repository channel alias
-                if ($channelName == $packageDefinition->getChannelName()) {
-                    $composerPackageAlias = $this->buildComposerPackageName($channelAlias, $packageDefinition->getPackageName());
-                    $aliasConstraint = new VersionConstraint('==', $normalizedVersion);
-                    $aliasLink = new Link($composerPackageName, $composerPackageAlias, $aliasConstraint, 'replaces', (string) $aliasConstraint);
-                    $replaces[] = $aliasLink;
-                }
-
-                $dependencyInfo = $releaseInfo->getDependencyInfo();
-                foreach ($dependencyInfo->getRequires() as $dependencyConstraint) {
-                    $dependencyPackageName = $this->buildComposerPackageName($dependencyConstraint->getChannelName(), $dependencyConstraint->getPackageName());
-                    $constraint = $versionParser->parseConstraints($dependencyConstraint->getConstraint());
-                    $link = new Link($composerPackageName, $dependencyPackageName, $constraint, $dependencyConstraint->getType(), $dependencyConstraint->getConstraint());
-                    switch ($dependencyConstraint->getType()) {
-                        case 'required':
-                            $requires[] = $link;
-                            break;
-                        case 'conflicts':
-                            $conflicts[] = $link;
-                            break;
-                        case 'replaces':
-                            $replaces[] = $link;
-                            break;
-                    }
-                }
-
-                foreach ($dependencyInfo->getOptionals() as $groupName => $dependencyConstraints) {
-                    foreach ($dependencyConstraints as $dependencyConstraint) {
-                        $dependencyPackageName = $this->buildComposerPackageName($dependencyConstraint->getChannelName(), $dependencyConstraint->getPackageName());
-                        $suggests[$groupName.'-'.$dependencyPackageName] = $dependencyConstraint->getConstraint();
-                    }
-                }
-
-                $package = new \Composer\Package\MemoryPackage($composerPackageName, $normalizedVersion, $version);
-                $package->setType('library');
-                $package->setDescription($packageDefinition->getDescription());
-                $package->setDistType('pear');
-                $package->setDistUrl($distUrl);
-                $package->setAutoload(array('classmap' => array('')));
-                $package->setIncludePaths(array('/'));
-                $package->setRequires($requires);
-                $package->setConflicts($conflicts);
-                $package->setSuggests($suggests);
-                $package->setReplaces($replaces);
-                $result[] = $package;
+        foreach ($supportedVersions as $version) {
+            $xpathTest = "ns:servers/ns:primary/ns:rest/ns:baseurl[@type='{$version}']";
+            $testResult = $channelXml->xpath($xpathTest);
+            if (count($testResult) > 0) {
+                return array('version' => $version, 'baseUrl' => (string) $testResult[0]);
             }
         }
 
-        return $result;
-    }
-
-    private function buildComposerPackageName($pearChannelName, $pearPackageName)
-    {
-        if ($pearChannelName == 'php') {
-            return "php";
-        }
-        if ($pearChannelName == 'ext') {
-            return "ext-{$pearPackageName}";
-        }
-
-        return "pear-{$pearChannelName}/{$pearPackageName}";
-    }
-
-    protected function parseVersion($version)
-    {
-        if (preg_match('{^v?(\d{1,3})(\.\d+)?(\.\d+)?(\.\d+)?}i', $version, $matches)) {
-            $version = $matches[1]
-                .(!empty($matches[2]) ? $matches[2] : '.0')
-                .(!empty($matches[3]) ? $matches[3] : '.0')
-                .(!empty($matches[4]) ? $matches[4] : '.0');
-
-            return $version;
-        } else {
-            return false;
-        }
+        return null;
     }
 }

src/Composer/Repository/Pear/ChannelRest10Reader.php

@@ -61,7 +61,7 @@ class ChannelRest10Reader extends BaseChannelReader
 
         $xmlPath = '/p/packages.xml';
         $xml = $this->requestXml($baseUrl, $xmlPath);
-        $xml->registerXPathNamespace('ns', self::allPackagesNS);
+        $xml->registerXPathNamespace('ns', self::ALL_PACKAGES_NS);
         foreach ($xml->xpath('ns:p') as $node) {
             $packageName = (string) $node;
             $packageInfo = $this->readPackage($baseUrl, $packageName);
@@ -83,7 +83,7 @@ class ChannelRest10Reader extends BaseChannelReader
     {
         $xmlPath = '/p/' . strtolower($packageName) . '/info.xml';
         $xml = $this->requestXml($baseUrl, $xmlPath);
-        $xml->registerXPathNamespace('ns', self::packageInfoNS);
+        $xml->registerXPathNamespace('ns', self::PACKAGE_INFO_NS);
 
         $channelName = (string) $xml->c;
         $packageName = (string) $xml->n;
@@ -116,7 +116,7 @@ class ChannelRest10Reader extends BaseChannelReader
         try {
             $xmlPath = '/r/' . strtolower($packageName) . '/allreleases.xml';
             $xml = $this->requestXml($baseUrl, $xmlPath);
-            $xml->registerXPathNamespace('ns', self::allReleasesNS);
+            $xml->registerXPathNamespace('ns', self::ALL_RELEASES_NS);
             foreach ($xml->xpath('ns:r') as $node) {
                 $releaseVersion = (string) $node->v;
                 $releaseStability = (string) $node->s;

src/Composer/Repository/Pear/ChannelRest11Reader.php

@@ -56,7 +56,7 @@ class ChannelRest11Reader extends BaseChannelReader
         $result = array();
 
         $xml = $this->requestXml($baseUrl, "/c/categories.xml");
-        $xml->registerXPathNamespace('ns', self::allCategoriesNS);
+        $xml->registerXPathNamespace('ns', self::ALL_CATEGORIES_NS);
         foreach ($xml->xpath('ns:c') as $node) {
             $categoryName = (string) $node;
             $categoryPackages = $this->readCategoryPackages($baseUrl, $categoryName);
@@ -80,7 +80,7 @@ class ChannelRest11Reader extends BaseChannelReader
 
         $categoryPath = '/c/'.urlencode($categoryName).'/packagesinfo.xml';
         $xml = $this->requestXml($baseUrl, $categoryPath);
-        $xml->registerXPathNamespace('ns', self::categoryPackagesInfoNS);
+        $xml->registerXPathNamespace('ns', self::CATEGORY_PACKAGES_INFO_NS);
         foreach ($xml->xpath('ns:pi') as $node) {
             $packageInfo = $this->parsePackage($node);
             $result[] = $packageInfo;
@@ -97,7 +97,7 @@ class ChannelRest11Reader extends BaseChannelReader
      */
     private function parsePackage($packageInfo)
     {
-        $packageInfo->registerXPathNamespace('ns', self::categoryPackagesInfoNS);
+        $packageInfo->registerXPathNamespace('ns', self::CATEGORY_PACKAGES_INFO_NS);
         $channelName = (string) $packageInfo->p->c;
         $packageName = (string) $packageInfo->p->n;
         $license = (string) $packageInfo->p->l;

src/Composer/Repository/Pear/PackageDependencyParser.php

@@ -19,6 +19,23 @@ namespace Composer\Repository\Pear;
  */
 class PackageDependencyParser
 {
+    /**
+     * Builds dependency information. It detects used package.xml format.
+     *
+     * @param $depArray array
+     * @return DependencyInfo
+     */
+    public function buildDependencyInfo($depArray)
+    {
+        if (!is_array($depArray)) {
+            return new DependencyInfo(array(), array());
+        }
+        if (!$this->isHash($depArray)) {
+            return new DependencyInfo($this->buildDependency10Info($depArray), array());
+        }
+        return $this->buildDependency20Info($depArray);
+    }
+
     /**
      * Builds dependency information from package.xml 1.0 format
      *
@@ -229,23 +246,6 @@ class PackageDependencyParser
         return $result;
     }
 
-    /**
-     * Builds dependency information. It detects used package.xml format.
-     *
-     * @param $depArray array
-     * @return DependencyInfo
-     */
-    public function buildDependencyInfo($depArray)
-    {
-        if (!is_array($depArray)) {
-            return new DependencyInfo(array(), array());
-        } elseif (!$this->isHash($depArray)) {
-            return new DependencyInfo($this->buildDependency10Info($depArray), array());
-        } else {
-            return $this->buildDependency20Info($depArray);
-        }
-    }
-
     /**
      * Parses version constraint
      *
@@ -260,7 +260,8 @@ class PackageDependencyParser
         $values = array_intersect_key($data, $dep20toOperatorMap);
         if (0 == count($values)) {
             return '*';
-        } elseif (isset($values['min']) && isset($values['exclude']) && $data['min'] == $data['exclude']) {
+        }
+        if (isset($values['min']) && isset($values['exclude']) && $data['min'] == $data['exclude']) {
             $versions[] = '>' . $this->parseVersion($values['min']);
         } elseif (isset($values['max']) && isset($values['exclude']) && $data['max'] == $data['exclude']) {
             $versions[] = '<' . $this->parseVersion($values['max']);
@@ -283,7 +284,7 @@ class PackageDependencyParser
      * Softened version parser
      *
      * @param $version
-     * @return bool|string
+     * @return null|string
      */
     private function parseVersion($version)
     {
@@ -294,9 +295,9 @@ class PackageDependencyParser
                 .(!empty($matches[4]) ? $matches[4] : '.0');
 
             return $version;
-        } else {
-            return false;
         }
+
+        return null;
     }
 
     /**

src/Composer/Repository/PearRepository.php

@@ -13,6 +13,8 @@
 namespace Composer\Repository;
 
 use Composer\IO\IOInterface;
+use Composer\Package\Version\VersionParser;
+use Composer\Repository\Pear\ChannelReader;
 use Composer\Package\MemoryPackage;
 use Composer\Repository\Pear\ChannelInfo;
 use Composer\Package\Link;
@@ -34,6 +36,7 @@ class PearRepository extends ArrayRepository
     private $url;
     private $io;
     private $rfs;
+    private $versionParser;
 
     /** @var string vendor makes additional alias for each channel as {prefix}/{packagename}. It allows smoother
      * package transition to composer-like repositories.
@@ -54,6 +57,7 @@ class PearRepository extends ArrayRepository
         $this->io = $io;
         $this->rfs = $rfs ?: new RemoteFilesystem($this->io);
         $this->vendorAlias = isset($repoConfig['vendor-alias']) ? $repoConfig['vendor-alias'] : null;
+        $this->versionParser = new VersionParser();
     }
 
     protected function initialize()
@@ -62,14 +66,14 @@ class PearRepository extends ArrayRepository
 
         $this->io->write('Initializing PEAR repository '.$this->url);
 
-        $reader = new \Composer\Repository\Pear\ChannelReader($this->rfs);
+        $reader = new ChannelReader($this->rfs);
         try {
             $channelInfo = $reader->read($this->url);
         } catch (\Exception $e) {
             $this->io->write('<warning>PEAR repository from '.$this->url.' could not be loaded. '.$e->getMessage().'</warning>');
             return;
         }
-        $packages = $this->buildComposerPackages($channelInfo);
+        $packages = $this->buildComposerPackages($channelInfo, $this->versionParser);
         foreach ($packages as $package) {
             $this->addPackage($package);
         }
@@ -81,14 +85,13 @@ class PearRepository extends ArrayRepository
      * @param  ChannelInfo   $channelInfo
      * @return MemoryPackage
      */
-    private function buildComposerPackages(ChannelInfo $channelInfo)
+    private function buildComposerPackages(ChannelInfo $channelInfo, VersionParser $versionParser)
     {
-        $versionParser = new \Composer\Package\Version\VersionParser();
         $result = array();
         foreach ($channelInfo->getPackages() as $packageDefinition) {
             foreach ($packageDefinition->getReleases() as $version => $releaseInfo) {
                 $normalizedVersion = $this->parseVersion($version);
-                if (false === $normalizedVersion) {
+                if (!$normalizedVersion) {
                     continue; // skip packages with unparsable versions
                 }
 
@@ -160,18 +163,25 @@ class PearRepository extends ArrayRepository
         return $result;
     }
 
-    private function buildComposerPackageName($pearChannelName, $pearPackageName)
+    private function buildComposerPackageName($channelName, $packageName)
     {
-        if ($pearChannelName == 'php') {
+        if ('php' === $channelName) {
             return "php";
-        } elseif ($pearChannelName == 'ext') {
-            return "ext-{$pearPackageName}";
-        } else {
-            return "pear-{$pearChannelName}/{$pearPackageName}";
         }
+        if ('ext' === $channelName) {
+            return "ext-{$packageName}";
+        }
+
+        return "pear-{$channelName}/{$packageName}";
     }
 
-    protected function parseVersion($version)
+    /**
+     * Softened version parser.
+     *
+     * @param string $version
+     * @return null|string
+     */
+    private function parseVersion($version)
     {
         if (preg_match('{^v?(\d{1,3})(\.\d+)?(\.\d+)?(\.\d+)?}i', $version, $matches)) {
             $version = $matches[1]
@@ -180,8 +190,8 @@ class PearRepository extends ArrayRepository
                 .(!empty($matches[4]) ? $matches[4] : '.0');
 
             return $version;
-        } else {
-            return false;
         }
+
+        return null;
     }
 }

tests/Composer/Test/Repository/Pear/ChannelReaderTest.php

@@ -13,6 +13,7 @@
 namespace Composer\Repository\Pear;
 
 use Composer\Test\TestCase;
+use Composer\Package\Version\VersionParser;
 use Composer\Package\LinkConstraint\VersionConstraint;
 use Composer\Package\Link;
 use Composer\Package\MemoryPackage;
@@ -62,45 +63,51 @@ class ChannelReaderTest extends TestCase
 
     public function testShouldCreatePackages()
     {
-        $reader = $this->getMockBuilder('\Composer\Repository\Pear\ChannelReader')
+        $reader = $this->getMockBuilder('\Composer\Repository\PearRepository')
             ->disableOriginalConstructor()
             ->getMock();
 
         $ref = new \ReflectionMethod($reader, 'buildComposerPackages');
         $ref->setAccessible(true);
 
-        $packageInfo = new PackageInfo(
+        $channelInfo = new ChannelInfo(
             'test.loc',
-            'sample',
-            'license',
-            'shortDescription',
-            'description',
+            'test',
             array(
-                '1.0.0.1' => new ReleaseInfo(
-                    'stable',
-                    new DependencyInfo(
-                        array(
-                            new DependencyConstraint(
-                                'required',
-                                '> 5.2.0.0',
-                                'php',
-                                ''
-                            ),
-                            new DependencyConstraint(
-                                'conflicts',
-                                '== 2.5.6.0',
-                                'pear.php.net',
-                                'broken'
-                            ),
-                        ),
-                        array(
-                            '*' => array(
-                                new DependencyConstraint(
-                                    'optional',
-                                    '*',
-                                    'ext',
-                                    'xml'
+                new PackageInfo(
+                    'test.loc',
+                    'sample',
+                    'license',
+                    'shortDescription',
+                    'description',
+                    array(
+                        '1.0.0.1' => new ReleaseInfo(
+                            'stable',
+                            new DependencyInfo(
+                                array(
+                                    new DependencyConstraint(
+                                        'required',
+                                        '> 5.2.0.0',
+                                        'php',
+                                        ''
+                                    ),
+                                    new DependencyConstraint(
+                                        'conflicts',
+                                        '== 2.5.6.0',
+                                        'pear.php.net',
+                                        'broken'
+                                    ),
                                 ),
+                                array(
+                                    '*' => array(
+                                        new DependencyConstraint(
+                                            'optional',
+                                            '*',
+                                            'ext',
+                                            'xml'
+                                        ),
+                                    )
+                                )
                             )
                         )
                     )
@@ -108,7 +115,7 @@ class ChannelReaderTest extends TestCase
             )
         );
 
-        $packages = $ref->invoke($reader, 'test.loc', 'test', array($packageInfo));
+        $packages = $ref->invoke($reader, $channelInfo, new VersionParser());
 
         $expectedPackage = new MemoryPackage('pear-test.loc/sample', '1.0.0.1' , '1.0.0.1');
         $expectedPackage->setType('library');