From 1cca62dc97249a7c62d90ba4bd973ed3f011a173 Mon Sep 17 00:00:00 2001 From: everzet Date: Sat, 1 Oct 2011 16:01:33 +0300 Subject: [PATCH 1/2] move json parsing instructions into single class object --- bin/composer | 6 +- src/Composer/Json/JsonFile.php | 41 ++++++++---- src/Composer/Package/Loader/JsonLoader.php | 42 ++---------- .../Repository/ComposerRepository.php | 4 +- .../Repository/FilesystemRepository.php | 24 ++----- src/Composer/Repository/GitRepository.php | 4 +- .../Repository/FilesystemRepositoryTest.php | 67 +++++++++++-------- 7 files changed, 89 insertions(+), 99 deletions(-) diff --git a/bin/composer b/bin/composer index a2fba3ad0..e2d36b3c9 100755 --- a/bin/composer +++ b/bin/composer @@ -13,7 +13,9 @@ use Composer\Console\Application as ComposerApplication; // initialize repository manager $rm = new Repository\RepositoryManager(); -$rm->setLocalRepository(new Repository\PlatformRepository(new Repository\FilesystemRepository('.composer/installed.json'))); +$rm->setLocalRepository(new Repository\PlatformRepository( + new Repository\FilesystemRepository(new JsonFile('.composer/installed.json')) +)); $rm->setRepository('Packagist', new Repository\ComposerRepository('http://packagist.org')); // initialize download manager @@ -28,7 +30,7 @@ $im->setInstaller('library', new Installer\LibraryInstaller('vendor', $dm, $rm-> // load package $loader = new Package\Loader\JsonLoader(); -$package = $loader->load('composer.json'); +$package = $loader->load(new JsonFile('composer.json')); // init locker diff --git a/src/Composer/Json/JsonFile.php b/src/Composer/Json/JsonFile.php index 719e8ba5c..08a91d2f3 100644 --- a/src/Composer/Json/JsonFile.php +++ b/src/Composer/Json/JsonFile.php @@ -52,9 +52,32 @@ class JsonFile */ public function read() { - $json = file_get_contents($this->path); - $config = json_decode($json, true); - if (!$config) { + $json = file_get_contents($this->path); + + return static::parseJson($json); + } + + /** + * Writes json file. + * + * @param array $hash writes hash into json file + */ + public function write(array $hash) + { + file_put_contents($this->path, json_encode($hash)); + } + + /** + * Parses json string and returns hash. + * + * @param string $json json string + * + * @return array + */ + public static function parseJson($json) + { + $hash = json_decode($json, true); + if (!$hash) { switch (json_last_error()) { case JSON_ERROR_NONE: $msg = 'No error has occurred, is your composer.json file empty?'; @@ -78,16 +101,6 @@ class JsonFile throw new \UnexpectedValueException('Incorrect composer.json file: '.$msg); } - return $config; - } - - /** - * Writes json file. - * - * @param array $hash writes hash into json file - */ - public function write(array $hash) - { - file_put_contents($this->path, json_encode($hash)); + return $hash; } } diff --git a/src/Composer/Package/Loader/JsonLoader.php b/src/Composer/Package/Loader/JsonLoader.php index b0fcbd52a..4efb8e862 100644 --- a/src/Composer/Package/Loader/JsonLoader.php +++ b/src/Composer/Package/Loader/JsonLoader.php @@ -12,6 +12,8 @@ namespace Composer\Package\Loader; +use Composer\Json\JsonFile; + /** * @author Konstantin Kudryashiv */ @@ -19,42 +21,12 @@ class JsonLoader extends ArrayLoader { public function load($json) { - $config = $this->loadJsonConfig($json); + if ($json instanceof JsonFile) { + $json = $json->read(); + } elseif (is_string($json)) { + $json = JsonFile::parseJson($json); + } return parent::load($config); } - - private function loadJsonConfig($json) - { - if (is_file($json)) { - $json = file_get_contents($json); - } - - $config = json_decode($json, true); - if (!$config) { - switch (json_last_error()) { - case JSON_ERROR_NONE: - $msg = 'No error has occurred, is your composer.json file empty?'; - break; - case JSON_ERROR_DEPTH: - $msg = 'The maximum stack depth has been exceeded'; - break; - case JSON_ERROR_STATE_MISMATCH: - $msg = 'Invalid or malformed JSON'; - break; - case JSON_ERROR_CTRL_CHAR: - $msg = 'Control character error, possibly incorrectly encoded'; - break; - case JSON_ERROR_SYNTAX: - $msg = 'Syntax error'; - break; - case JSON_ERROR_UTF8: - $msg = 'Malformed UTF-8 characters, possibly incorrectly encoded'; - break; - } - throw new \UnexpectedValueException('Incorrect composer.json file: '.$msg); - } - - return $config; - } } diff --git a/src/Composer/Repository/ComposerRepository.php b/src/Composer/Repository/ComposerRepository.php index 36f2e0a18..81ecc005c 100644 --- a/src/Composer/Repository/ComposerRepository.php +++ b/src/Composer/Repository/ComposerRepository.php @@ -14,6 +14,7 @@ namespace Composer\Repository; use Composer\Package\Loader\ArrayLoader; use Composer\Package\LinkConstraint\VersionConstraint; +use Composer\Json\JsonFile; /** * @author Jordi Boggiano @@ -36,7 +37,8 @@ class ComposerRepository extends ArrayRepository protected function initialize() { parent::initialize(); - $packages = @json_decode(file_get_contents($this->url.'/packages.json'), true); + $json = new JsonFile($this->url.'/packages.json'); + $packages = $json->read(); if (!$packages) { throw new \UnexpectedValueException('Could not parse package list from the '.$this->url.' repository'); } diff --git a/src/Composer/Repository/FilesystemRepository.php b/src/Composer/Repository/FilesystemRepository.php index e5de4e5cb..a49765c8d 100644 --- a/src/Composer/Repository/FilesystemRepository.php +++ b/src/Composer/Repository/FilesystemRepository.php @@ -12,6 +12,7 @@ namespace Composer\Repository; +use Composer\Json\JsonFile; use Composer\Package\PackageInterface; use Composer\Package\Loader\ArrayLoader; use Composer\Package\Dumper\ArrayDumper; @@ -28,25 +29,11 @@ class FilesystemRepository extends ArrayRepository implements WritableRepository /** * Initializes filesystem repository. * - * @param string $group registry (installer) group + * @param JsonFile $repositoryFile repository json file */ - public function __construct($repositoryFile) + public function __construct(JsonFile $repositoryFile) { $this->file = $repositoryFile; - $path = dirname($this->file); - - if (!is_dir($path)) { - if (file_exists($path)) { - throw new \UnexpectedValueException( - $path.' exists and is not a directory.' - ); - } - if (!mkdir($path, 0777, true)) { - throw new \UnexpectedValueException( - $path.' does not exist and could not be created.' - ); - } - } } /** @@ -56,8 +43,7 @@ class FilesystemRepository extends ArrayRepository implements WritableRepository { parent::initialize(); - $packages = @json_decode(file_get_contents($this->file), true); - + $packages = $this->file->read(); if (is_array($packages)) { $loader = new ArrayLoader(); foreach ($packages as $package) { @@ -77,6 +63,6 @@ class FilesystemRepository extends ArrayRepository implements WritableRepository $packages[] = $dumper->dump($package); } - file_put_contents($this->file, json_encode($packages)); + $this->file->write($packages); } } diff --git a/src/Composer/Repository/GitRepository.php b/src/Composer/Repository/GitRepository.php index ed382538e..a2780f0d4 100644 --- a/src/Composer/Repository/GitRepository.php +++ b/src/Composer/Repository/GitRepository.php @@ -16,6 +16,7 @@ use Composer\Package\MemoryPackage; use Composer\Package\BasePackage; use Composer\Package\Link; use Composer\Package\LinkConstraint\VersionConstraint; +use Composer\Json\JsonFile; /** * FIXME This is majorly broken and incomplete, it was an experiment @@ -45,7 +46,8 @@ class GitRepository extends ArrayRepository if (!file_exists($this->url.'/composer.json')) { throw new \InvalidArgumentException('The repository at url '.$this->url.' does not contain a composer.json file.'); } - $config = json_decode(file_get_contents($this->url.'/composer.json'), true); + $json = new JsonFile($this->url.'/composer.json'); + $fonfig = $json->read(); if (!$config) { throw new \UnexpectedValueException('Config file could not be parsed: '.$this->url.'/composer.json. Probably a JSON syntax error.'); } diff --git a/tests/Composer/Test/Repository/FilesystemRepositoryTest.php b/tests/Composer/Test/Repository/FilesystemRepositoryTest.php index 048c9c8d7..24e092b37 100644 --- a/tests/Composer/Test/Repository/FilesystemRepositoryTest.php +++ b/tests/Composer/Test/Repository/FilesystemRepositoryTest.php @@ -13,43 +13,56 @@ namespace Composer\Repository; use Composer\Repository\FilesystemRepository; +use Composer\Package\MemoryPackage; class FilesystemRepositoryTest extends \PHPUnit_Framework_TestCase { - private $dir; - private $repositoryFile; - - protected function setUp() + public function testRepositoryRead() { - $this->dir = sys_get_temp_dir().'/.composer'; - $this->repositoryFile = $this->dir.'/some_registry-reg.json'; + $json = $this->createJsonFileMock(); - if (file_exists($this->repositoryFile)) { - unlink($this->repositoryFile); - } + $repository = new FilesystemRepository($json); + + $json + ->expects($this->once()) + ->method('read') + ->will($this->returnValue(array( + array('name' => 'package1', 'version' => '1.0.0-beta', 'type' => 'vendor') + ))); + + $packages = $repository->getPackages(); + + $this->assertSame(1, count($packages)); + $this->assertSame('package1', $packages[0]->getName()); + $this->assertSame('1.0.0.0-beta', $packages[0]->getVersion()); + $this->assertSame('vendor', $packages[0]->getType()); } - public function testRepositoryReadWrite() + public function testRepositoryWrite() { - $this->assertFileNotExists($this->repositoryFile); - $repository = new FilesystemRepository($this->repositoryFile); + $json = $this->createJsonFileMock(); - $repository->getPackages(); + $repository = new FilesystemRepository($json); + + $json + ->expects($this->once()) + ->method('read') + ->will($this->returnValue(array())); + $json + ->expects($this->once()) + ->method('write') + ->with(array( + array('name' => 'mypkg', 'type' => 'library', 'names' => array('mypkg'), 'version' => '0.1.10') + )); + + $repository->addPackage(new MemoryPackage('mypkg', '0.1.10')); $repository->write(); - $this->assertFileExists($this->repositoryFile); + } - file_put_contents($this->repositoryFile, json_encode(array( - array('name' => 'package1', 'version' => '1.0.0-beta', 'type' => 'vendor') - ))); - - $repository = new FilesystemRepository($this->repositoryFile); - $repository->getPackages(); - $repository->write(); - $this->assertFileExists($this->repositoryFile); - - $data = json_decode(file_get_contents($this->repositoryFile), true); - $this->assertEquals(array( - array('name' => 'package1', 'type' => 'vendor', 'version' => '1.0.0.0-beta', 'names' => array('package1')) - ), $data); + private function createJsonFileMock() + { + return $this->getMockBuilder('Composer\Json\JsonFile') + ->disableOriginalConstructor() + ->getMock(); } } From d5ba884f8bc92423c15c315a756111c4ec76c1ef Mon Sep 17 00:00:00 2001 From: everzet Date: Sat, 1 Oct 2011 17:06:14 +0300 Subject: [PATCH 2/2] fixed potential error --- src/Composer/Repository/FilesystemRepository.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Composer/Repository/FilesystemRepository.php b/src/Composer/Repository/FilesystemRepository.php index a49765c8d..5a5534df7 100644 --- a/src/Composer/Repository/FilesystemRepository.php +++ b/src/Composer/Repository/FilesystemRepository.php @@ -43,7 +43,11 @@ class FilesystemRepository extends ArrayRepository implements WritableRepository { parent::initialize(); - $packages = $this->file->read(); + $packages = null; + if ($this->file->exists()) { + $packages = $this->file->read(); + } + if (is_array($packages)) { $loader = new ArrayLoader(); foreach ($packages as $package) {