Refactor repositories handling in config/factory/loader, fixes #828, fixes #826

src/Composer/Command/InitCommand.php

@@ -235,7 +235,7 @@ EOT
         if (!$this->repos) {
             $this->repos = new CompositeRepository(array_merge(
                 array(new PlatformRepository),
-                Factory::createDefaultRepositories($this->getIO(), Factory::createConfig())
+                Factory::createDefaultRepositories($this->getIO())
             ));
         }
 

src/Composer/Command/SearchCommand.php

@@ -52,7 +52,7 @@ EOT
             $installedRepo = new CompositeRepository(array($localRepo, $platformRepo));
             $repos = new CompositeRepository(array_merge(array($installedRepo), $composer->getRepositoryManager()->getRepositories()));
         } else {
-            $defaultRepos = Factory::createDefaultRepositories($this->getIO(), Factory::createConfig());
+            $defaultRepos = Factory::createDefaultRepositories($this->getIO());
             $output->writeln('No composer.json found in the current directory, showing packages from ' . implode(', ', array_keys($defaultRepos)));
             $installedRepo = $platformRepo;
             $repos = new CompositeRepository(array_merge(array($installedRepo), $defaultRepos));

src/Composer/Command/ShowCommand.php

@@ -63,7 +63,7 @@ EOT
             $installedRepo = new CompositeRepository(array($localRepo, $platformRepo));
             $repos = new CompositeRepository(array_merge(array($installedRepo), $composer->getRepositoryManager()->getRepositories()));
         } else {
-            $defaultRepos = Factory::createDefaultRepositories($this->getIO(), Factory::createConfig());
+            $defaultRepos = Factory::createDefaultRepositories($this->getIO());
             $output->writeln('No composer.json found in the current directory, showing packages from ' . implode(', ', array_keys($defaultRepos)));
             $installedRepo = $platformRepo;
             $repos = new CompositeRepository(array_merge(array($installedRepo), $defaultRepos));

src/Composer/Config.php

@@ -25,7 +25,10 @@ class Config
     );
 
     public static $defaultRepositories = array(
-        'packagist' => 'http://packagist.org',
+        'packagist' => array(
+            'type' => 'composer',
+            'url' => 'http://packagist.org',
+        )
     );
 
     private $config;
@@ -52,15 +55,30 @@ class Config
 
         if (!empty($config['repositories']) && is_array($config['repositories'])) {
             $this->repositories = array_reverse($this->repositories, true);
-            $this->repositories = array_merge($this->repositories, $config['repositories']);
-            $this->repositories = array_reverse($this->repositories, true);
-
-            // filter out disabled ones
-            foreach ($this->repositories as $name => $url) {
-                if (false === $url) {
+            $newRepos = array_reverse($config['repositories'], true);
+            foreach ($newRepos as $name => $repository) {
+                // disable a repository by name
+                if (false === $repository) {
                     unset($this->repositories[$name]);
+                    continue;
+                }
+
+                // disable a repository with an anonymous {"name": false} repo
+                foreach ($this->repositories as $repoName => $repoSpec) {
+                    if (isset($repository[$repoName]) && false === $repository[$repoName]) {
+                        unset($this->repositories[$repoName]);
+                        continue 2;
+                    }
+                }
+
+                // store repo
+                if (is_int($name)) {
+                    $this->repositories[] = $repository;
+                } else {
+                    $this->repositories[$name] = $repository;
                 }
             }
+            $this->repositories = array_reverse($this->repositories, true);
         }
     }
 

src/Composer/Factory.php

@@ -52,14 +52,14 @@ class Factory
 
         $config = new Config();
 
+        // add home dir to the config
+        $config->merge(array('config' => array('home' => $home)));
+
         $file = new JsonFile($home.'/config.json');
         if ($file->exists()) {
             $config->merge($file->read());
         }
 
-        // add home dir to the config
-        $config->merge(array('config' => array('home' => $home)));
-
         return $config;
     }
 
@@ -68,12 +68,33 @@ class Factory
         return getenv('COMPOSER') ?: 'composer.json';
     }
 
-    public static function createDefaultRepositories(IOInterface $io, Config $config)
+    public static function createDefaultRepositories(IOInterface $io = null, Config $config = null, RepositoryManager $rm = null)
     {
         $repos = array();
 
-        foreach ($config->getRepositories() as $repo => $url) {
-            $repos[preg_replace('{^https?://}i', '', $url)] = new ComposerRepository(array('url' => $url), $io, $config);
+        if (!$config) {
+            $config = static::createConfig();
+        }
+        if (!$rm) {
+            if (!$io) {
+                throw new \InvalidArgumentException('This function requires either an IOInterface or a RepositoryManager');
+            }
+            $factory = new static;
+            $rm = $factory->createRepositoryManager($io, $config);
+        }
+
+        foreach ($config->getRepositories() as $index => $repo) {
+            if (!is_array($repo)) {
+                throw new \UnexpectedValueException('Repository '.$index.' ('.json_encode($repo).') should be an array, '.gettype($repo).' given');
+            }
+            if (!isset($repo['type'])) {
+                throw new \UnexpectedValueException('Repository '.$index.' ('.json_encode($repo).') must have a type defined');
+            }
+            $name = is_int($index) && isset($repo['url']) ? preg_replace('{^https?://}i', '', $repo['url']) : $index;
+            while (isset($repos[$name])) {
+                $name .= '2';
+            }
+            $repos[$name] = $rm->createRepository($repo['type'], $repo);
         }
 
         return $repos;
@@ -126,9 +147,6 @@ class Factory
         // initialize repository manager
         $rm = $this->createRepositoryManager($io, $config);
 
-        // load default composer repositories unless they're explicitly disabled
-        $localConfig = $this->addDefaultRepositories($config, $localConfig);
-
         // load local repository
         $this->addLocalRepository($rm, $vendorDir);
 
@@ -194,44 +212,6 @@ class Factory
         $rm->setLocalDevRepository(new Repository\InstalledFilesystemRepository(new JsonFile($vendorDir.'/composer/installed_dev.json')));
     }
 
-    /**
-     * @param  array $localConfig
-     * @return array
-     */
-    protected function addDefaultRepositories(Config $config, array $localConfig)
-    {
-        $defaults = $config->getRepositories();
-
-        if (isset($localConfig['repositories'])) {
-            foreach ($localConfig['repositories'] as $key => $repo) {
-                foreach ($defaults as $name => $url) {
-                    if (isset($repo[$name])) {
-                        if (true === $repo[$name]) {
-                            $localConfig['repositories'][$key] = array(
-                                'type' => 'composer',
-                                'url' => $url,
-                            );
-                        }
-
-                        unset($defaults[$name]);
-                        break;
-                    }
-                }
-            }
-        } else {
-            $localConfig['repositories'] = array();
-        }
-
-        foreach ($defaults as $name => $url) {
-            $localConfig['repositories'][] = array(
-                'type' => 'composer',
-                'url' => $url,
-            );
-        }
-
-        return $localConfig;
-    }
-
     /**
      * @param  IO\IOInterface             $io
      * @return Downloader\DownloadManager

src/Composer/Package/Loader/RootPackageLoader.php

@@ -14,6 +14,7 @@ namespace Composer\Package\Loader;
 
 use Composer\Package\BasePackage;
 use Composer\Config;
+use Composer\Factory;
 use Composer\Package\Version\VersionParser;
 use Composer\Repository\RepositoryManager;
 use Composer\Util\ProcessExecutor;
@@ -82,26 +83,11 @@ class RootPackageLoader extends ArrayLoader
             $package->setMinimumStability(VersionParser::normalizeStability($config['minimum-stability']));
         }
 
-        $defaultRepositories = array_keys($this->config->getRepositories());
-
-        if (isset($config['repositories'])) {
-            foreach ($config['repositories'] as $index => $repo) {
-                foreach ($defaultRepositories as $name) {
-                    if (isset($repo[$name]) && $repo[$name] === false) {
-                        continue 2;
-                    }
-                }
-                if (!is_array($repo)) {
-                    throw new \UnexpectedValueException('Repository '.$index.' should be an array, '.gettype($repo).' given');
-                }
-                if (!isset($repo['type'])) {
-                    throw new \UnexpectedValueException('Repository '.$index.' ('.json_encode($repo).') must have a type defined');
-                }
-                $repository = $this->manager->createRepository($repo['type'], $repo);
-                $this->manager->addRepository($repository);
-            }
-            $package->setRepositories($config['repositories']);
+        $repos = Factory::createDefaultRepositories(null, $this->config, $this->manager);
+        foreach ($repos as $repo) {
+            $this->manager->addRepository($repo);
         }
+        $package->setRepositories($this->config->getRepositories());
 
         return $package;
     }

tests/Composer/Test/ConfigTest.php

@@ -0,0 +1,102 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Test;
+
+use Composer\Config;
+
+class ConfigTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider dataAddPackagistRepository
+     */
+    public function testAddPackagistRepository($expected, $localConfig, $systemConfig = null)
+    {
+        $config = new Config();
+        if ($systemConfig) {
+            $config->merge(array('repositories' => $systemConfig));
+        }
+        $config->merge(array('repositories' => $localConfig));
+
+        $this->assertEquals($expected, $config->getRepositories());
+    }
+
+    public function dataAddPackagistRepository()
+    {
+        $data = array();
+        $data['local config inherits system defaults'] = array(
+            array(
+                'packagist' => array('type' => 'composer', 'url' => 'http://packagist.org')
+            ),
+            array(),
+        );
+
+        $data['local config can disable system config by name'] = array(
+            array(),
+            array(
+                array('packagist' => false),
+            )
+        );
+
+        $data['local config adds above defaults'] = array(
+            array(
+                1 => array('type' => 'vcs', 'url' => 'git://github.com/composer/composer.git'),
+                0 => array('type' => 'pear', 'url' => 'http://pear.composer.org'),
+                'packagist' => array('type' => 'composer', 'url' => 'http://packagist.org'),
+            ),
+            array(
+                array('type' => 'vcs', 'url' => 'git://github.com/composer/composer.git'),
+                array('type' => 'pear', 'url' => 'http://pear.composer.org'),
+            ),
+        );
+
+        $data['system config adds above core defaults'] = array(
+            array(
+                'example.com' => array('type' => 'composer', 'url' => 'http://example.com'),
+                'packagist' => array('type' => 'composer', 'url' => 'http://packagist.org')
+            ),
+            array(),
+            array(
+                'example.com' => array('type' => 'composer', 'url' => 'http://example.com'),
+            ),
+        );
+
+        $data['local config can disable repos by name and re-add them anonymously to bring them above system config'] = array(
+            array(
+                0 => array('type' => 'composer', 'url' => 'http://packagist.org'),
+                'example.com' => array('type' => 'composer', 'url' => 'http://example.com')
+            ),
+            array(
+                array('packagist' => false),
+                array('type' => 'composer', 'url' => 'http://packagist.org')
+            ),
+            array(
+                'example.com' => array('type' => 'composer', 'url' => 'http://example.com'),
+            ),
+        );
+
+        $data['local config can override by name to bring a repo above system config'] = array(
+            array(
+                'packagist' => array('type' => 'composer', 'url' => 'http://packagistnew.org'),
+                'example.com' => array('type' => 'composer', 'url' => 'http://example.com')
+            ),
+            array(
+                'packagist' => array('type' => 'composer', 'url' => 'http://packagistnew.org')
+            ),
+            array(
+                'example.com' => array('type' => 'composer', 'url' => 'http://example.com'),
+            ),
+        );
+
+        return $data;
+    }
+}

tests/Composer/Test/FactoryTest.php

@@ -1,93 +0,0 @@
-<?php
-
-/*
- * This file is part of Composer.
- *
- * (c) Nils Adermann <naderman@naderman.de>
- *     Jordi Boggiano <j.boggiano@seld.be>
- *
- * For the full copyright and license information, please view the LICENSE
- * file that was distributed with this source code.
- */
-
-namespace Composer\Test;
-
-use Composer\Factory;
-use Composer\Config;
-
-class FactoryTest extends \PHPUnit_Framework_TestCase
-{
-    /**
-     * @dataProvider dataAddPackagistRepository
-     */
-    public function testAddPackagistRepository($expected, $composerConfig, $defaults = null)
-    {
-        $factory = new Factory();
-        $config = new Config();
-        if ($defaults) {
-            $config->merge(array('repositories' => $defaults));
-        }
-
-        $ref = new \ReflectionMethod($factory, 'addDefaultRepositories');
-        $ref->setAccessible(true);
-
-        $this->assertEquals($expected, $ref->invoke($factory, $config, $composerConfig));
-    }
-
-    public function dataAddPackagistRepository()
-    {
-        $repos = function() {
-            $repositories = func_get_args();
-
-            return array('repositories' => $repositories);
-        };
-
-        $data = array();
-        $data[] = array(
-            $repos(array('type' => 'composer', 'url' => 'http://packagist.org')),
-            $repos()
-        );
-
-        $data[] = array(
-            $repos(array('packagist' => false)),
-            $repos(array('packagist' => false))
-        );
-
-        $data[] = array(
-            $repos(
-                array('type' => 'vcs', 'url' => 'git://github.com/composer/composer.git'),
-                array('type' => 'composer', 'url' => 'http://packagist.org'),
-                array('type' => 'pear', 'url' => 'http://pear.composer.org')
-            ),
-            $repos(
-                array('type' => 'vcs', 'url' => 'git://github.com/composer/composer.git'),
-                array('packagist' => true),
-                array('type' => 'pear', 'url' => 'http://pear.composer.org')
-            )
-        );
-
-        $multirepo = array(
-            'example.com' => 'http://example.com',
-        );
-
-        $data[] = array(
-            $repos(
-                array('type' => 'composer', 'url' => 'http://example.com'),
-                array('type' => 'composer', 'url' => 'http://packagist.org')
-            ),
-            $repos(),
-            $multirepo,
-        );
-
-        $data[] = array(
-            $repos(
-                array('type' => 'composer', 'url' => 'http://packagist.org'),
-                array('type' => 'composer', 'url' => 'http://example.com')
-            ),
-            $repos(array('packagist' => true)),
-            $multirepo,
-        );
-
-        return $data;
-    }
-}

tests/Composer/Test/Mock/FactoryMock.php

@@ -25,7 +25,10 @@ class FactoryMock extends Factory
     {
         $config = new Config();
 
-        $config->merge(array('config' => array('home' => sys_get_temp_dir().'/composer-test')));
+        $config->merge(array(
+            'config' => array('home' => sys_get_temp_dir().'/composer-test'),
+            'repositories' => array('packagist' => false),
+        ));
 
         return $config;
     }
@@ -34,11 +37,6 @@ class FactoryMock extends Factory
     {
     }
 
-    protected function addDefaultRepositories(Config $config, array $localConfig)
-    {
-        return $localConfig;
-    }
-
     protected function createInstallationManager(Repository\RepositoryManager $rm, Downloader\DownloadManager $dm, $vendorDir, $binDir, IOInterface $io)
     {
         return new InstallationManagerMock;

tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php

@@ -42,25 +42,11 @@ class RootPackageLoaderTest extends \PHPUnit_Framework_TestCase
             return 0;
         });
 
-        $loader = new RootPackageLoader($manager, new Config, null, $processExecutor);
+        $config = new Config;
+        $config->merge(array('repositories' => array('packagist' => false)));
+        $loader = new RootPackageLoader($manager, $config, null, $processExecutor);
         $package = $loader->load(array());
 
         $this->assertEquals("dev-$commitHash", $package->getVersion());
     }
-
-    public function testAllowsDisabledDefaultRepository()
-    {
-        $loader = new RootPackageLoader(
-            new RepositoryManager(
-                $this->getMock('Composer\\IO\\IOInterface'),
-                $this->getMock('Composer\\Config')
-            ),
-            new Config()
-        );
-
-        $repos = array(array('packagist' => false));
-        $package = $loader->load(array('repositories' => $repos));
-
-        $this->assertEquals($repos, $package->getRepositories());
-    }
 }