From ffecd39d33bc41234963b6f9ac9d9a2c6286edcc Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Sun, 24 Jun 2012 13:03:55 +0200 Subject: [PATCH] Refactor repositories handling in config/factory/loader, fixes #828, fixes #826 --- src/Composer/Command/InitCommand.php | 2 +- src/Composer/Command/SearchCommand.php | 2 +- src/Composer/Command/ShowCommand.php | 2 +- src/Composer/Config.php | 32 ++++-- src/Composer/Factory.php | 74 +++++-------- .../Package/Loader/RootPackageLoader.php | 24 +---- tests/Composer/Test/ConfigTest.php | 102 ++++++++++++++++++ tests/Composer/Test/FactoryTest.php | 93 ---------------- tests/Composer/Test/Mock/FactoryMock.php | 10 +- .../Package/Loader/RootPackageLoaderTest.php | 20 +--- 10 files changed, 169 insertions(+), 192 deletions(-) create mode 100644 tests/Composer/Test/ConfigTest.php delete mode 100644 tests/Composer/Test/FactoryTest.php diff --git a/src/Composer/Command/InitCommand.php b/src/Composer/Command/InitCommand.php index e1c70d0c2..1a4ac9948 100644 --- a/src/Composer/Command/InitCommand.php +++ b/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()) )); } diff --git a/src/Composer/Command/SearchCommand.php b/src/Composer/Command/SearchCommand.php index d61c2dc8d..c09b5d4f1 100644 --- a/src/Composer/Command/SearchCommand.php +++ b/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)); diff --git a/src/Composer/Command/ShowCommand.php b/src/Composer/Command/ShowCommand.php index 314fcfa34..ef30c5bb0 100644 --- a/src/Composer/Command/ShowCommand.php +++ b/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)); diff --git a/src/Composer/Config.php b/src/Composer/Config.php index e507bd75d..e9bc28199 100644 --- a/src/Composer/Config.php +++ b/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); } } diff --git a/src/Composer/Factory.php b/src/Composer/Factory.php index de2f67e0f..01f8cc390 100644 --- a/src/Composer/Factory.php +++ b/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 diff --git a/src/Composer/Package/Loader/RootPackageLoader.php b/src/Composer/Package/Loader/RootPackageLoader.php index dfc59fc17..51c676112 100644 --- a/src/Composer/Package/Loader/RootPackageLoader.php +++ b/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; } diff --git a/tests/Composer/Test/ConfigTest.php b/tests/Composer/Test/ConfigTest.php new file mode 100644 index 000000000..dc950e20e --- /dev/null +++ b/tests/Composer/Test/ConfigTest.php @@ -0,0 +1,102 @@ + + * Jordi Boggiano + * + * 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; + } +} diff --git a/tests/Composer/Test/FactoryTest.php b/tests/Composer/Test/FactoryTest.php deleted file mode 100644 index 55ec0e920..000000000 --- a/tests/Composer/Test/FactoryTest.php +++ /dev/null @@ -1,93 +0,0 @@ - - * Jordi Boggiano - * - * 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; - } -} diff --git a/tests/Composer/Test/Mock/FactoryMock.php b/tests/Composer/Test/Mock/FactoryMock.php index 85644f54c..c98b378cb 100644 --- a/tests/Composer/Test/Mock/FactoryMock.php +++ b/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; diff --git a/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php b/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php index d5d3a0b03..6f4f53e19 100644 --- a/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php +++ b/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()); - } }