Write in the json directly without reformatting the whole file - skip validation since that is not really the job of the config command

src/Composer/Command/ConfigCommand.php

@@ -16,11 +16,10 @@ use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputArgument;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
-use JsonSchema\Validator;
 use Composer\Config;
 use Composer\Factory;
 use Composer\Json\JsonFile;
-use Composer\Json\JsonValidationException;
+use Composer\Json\JsonManipulator;
 
 /**
  * @author Joshua Estes <Joshua.Estes@iostudio.com>
@@ -146,118 +145,133 @@ EOT
             throw new \RuntimeException('You must include a setting value or pass --unset to clear the value');
         }
 
-        /**
-         * The user needs the ability to add a repository with one command.
-         * For example "config -g repository.foo 'vcs http://example.com'
-         */
-        $configSettings = $this->configFile->read(); // what is current in the config
-        $values         = $input->getArgument('setting-value'); // what the user is trying to add/change
+        $values = $input->getArgument('setting-value'); // what the user is trying to add/change
 
         // handle repositories
         if (preg_match('/^repos?(?:itories)?\.(.+)/', $input->getArgument('setting-key'), $matches)) {
             if ($input->getOption('unset')) {
-                unset($configSettings['repositories'][$matches[1]]);
-            } else {
-                $settingKey = 'repositories.'.$matches[1];
-                if (2 !== count($values)) {
-                    throw new \RuntimeException('You must pass the type and a url. Example: php composer.phar config repositories.foo vcs http://bar.com');
-                }
-                $setting = $this->parseSetting($settingKey, array(
-                    'type' => $values[0],
-                    'url'  => $values[1],
-                ));
+                return $this->manipulateJson('removeRepository', $matches[1], function (&$config, $repo) {
+                    unset($config['repositories'][$repo]);
+                });
+            }
 
-                // Could there be a better way to do this?
-                $configSettings = array_merge_recursive($configSettings, $setting);
-                $this->validateSchema($configSettings);
+            if (2 !== count($values)) {
+                throw new \RuntimeException('You must pass the type and a url. Example: php composer.phar config repositories.foo vcs http://bar.com');
             }
-        } else {
-            // handle config values
-            $uniqueConfigValues = array(
-                'process-timeout' => array('is_numeric', 'intval'),
-                'vendor-dir' => array('is_string', function ($val) { return $val; }),
-                'bin-dir' => array('is_string', function ($val) { return $val; }),
-                'notify-on-install' => array(
-                    function ($val) { return true; },
-                    function ($val) { return $val !== 'false' && (bool) $val; }
-                ),
-            );
-            $multiConfigValues = array(
-                'github-protocols' => array(
-                    function ($vals) {
-                        if (!is_array($vals)) {
-                            return 'array expected';
-                        }
 
-                        foreach ($vals as $val) {
-                            if (!in_array($val, array('git', 'https', 'http'))) {
-                                return 'valid protocols include: git, https, http';
-                            }
-                        }
+            return $this->manipulateJson(
+                'addRepository',
+                $matches[1],
+                array(
+                    'type' => $values[0],
+                    'url'  => $values[1],
+                ), function (&$config, $repo, $repoConfig) {
+                    $config['repositories'][$repo] = $repoConfig;
+                }
+            );
+        }
 
-                        return true;
-                    },
-                    function ($vals) {
-                        return $vals;
+        // handle config values
+        $uniqueConfigValues = array(
+            'process-timeout' => array('is_numeric', 'intval'),
+            'vendor-dir' => array('is_string', function ($val) { return $val; }),
+            'bin-dir' => array('is_string', function ($val) { return $val; }),
+            'notify-on-install' => array(
+                function ($val) { return true; },
+                function ($val) { return $val !== 'false' && (bool) $val; }
+            ),
+        );
+        $multiConfigValues = array(
+            'github-protocols' => array(
+                function ($vals) {
+                    if (!is_array($vals)) {
+                        return 'array expected';
                     }
-                ),
-            );
 
-            $settingKey = $input->getArgument('setting-key');
-            foreach ($uniqueConfigValues as $name => $callbacks) {
-                 if ($settingKey === $name) {
-                    list($validator, $normalizer) = $callbacks;
-                    if ($input->getOption('unset')) {
-                        unset($configSettings['config'][$settingKey]);
-                    } else {
-                        if (1 !== count($values)) {
-                            throw new \RuntimeException('You can only pass one value. Example: php composer.phar config process-timeout 300');
+                    foreach ($vals as $val) {
+                        if (!in_array($val, array('git', 'https', 'http'))) {
+                            return 'valid protocols include: git, https, http';
                         }
+                    }
 
-                        if (true !== $validation = $validator($values[0])) {
-                            throw new \RuntimeException(sprintf(
-                                '"%s" is an invalid value'.($validation ? ' ('.$validation.')' : ''),
-                                $values[0]
-                            ));
-                        }
+                    return true;
+                },
+                function ($vals) {
+                    return $vals;
+                }
+            ),
+        );
 
-                        $setting = $this->parseSetting('config.'.$settingKey, $normalizer($values[0]));
-                        $configSettings = array_merge($configSettings, $setting);
-                        $this->validateSchema($configSettings);
-                    }
+        $settingKey = $input->getArgument('setting-key');
+        foreach ($uniqueConfigValues as $name => $callbacks) {
+             if ($settingKey === $name) {
+                if ($input->getOption('unset')) {
+                    return $this->manipulateJson('removeConfigSetting', $settingKey, function (&$config, $key) {
+                        unset($config['config'][$key]);
+                    });
                 }
-            }
 
-            foreach ($multiConfigValues as $name => $callbacks) {
-                if ($settingKey === $name) {
-                    list($validator, $normalizer) = $callbacks;
-                    if ($input->getOption('unset')) {
-                        unset($configSettings['config'][$settingKey]);
-                    } else {
-                        if (true !== $validation = $validator($values)) {
-                            throw new \RuntimeException(sprintf(
-                                '%s is an invalid value'.($validation ? ' ('.$validation.')' : ''),
-                                json_encode($values)
-                            ));
-                        }
+                list($validator, $normalizer) = $callbacks;
+                if (1 !== count($values)) {
+                    throw new \RuntimeException('You can only pass one value. Example: php composer.phar config process-timeout 300');
+                }
 
-                        $setting = $this->parseSetting('config.'.$settingKey, $normalizer($values));
-                        $configSettings = array_merge($configSettings, $setting);
-                        $this->validateSchema($configSettings);
-                    }
+                if (true !== $validation = $validator($values[0])) {
+                    throw new \RuntimeException(sprintf(
+                        '"%s" is an invalid value'.($validation ? ' ('.$validation.')' : ''),
+                        $values[0]
+                    ));
                 }
+
+                return $this->manipulateJson('addConfigSetting', $settingKey, $normalizer($values[0]), function (&$config, $key, $val) {
+                    $config['config'][$key] = $val;
+                });
             }
         }
 
-        // clean up empty sections
-        if (empty($configSettings['repositories'])) {
-            unset($configSettings['repositories']);
-        }
-        if (empty($configSettings['config'])) {
-            unset($configSettings['config']);
+        foreach ($multiConfigValues as $name => $callbacks) {
+            if ($settingKey === $name) {
+                if ($input->getOption('unset')) {
+                    return $this->manipulateJson('removeConfigSetting', $settingKey, function (&$config, $key) {
+                        unset($config['config'][$key]);
+                    });
+                }
+
+                list($validator, $normalizer) = $callbacks;
+                if (true !== $validation = $validator($values)) {
+                    throw new \RuntimeException(sprintf(
+                        '%s is an invalid value'.($validation ? ' ('.$validation.')' : ''),
+                        json_encode($values)
+                    ));
+                }
+
+                return $this->manipulateJson('addConfigSetting', $settingKey, $normalizer($values), function (&$config, $key, $val) {
+                    $config['config'][$key] = $val;
+                });
+            }
         }
+    }
+
+    protected function manipulateJson($method, $args, $fallback)
+    {
+        $args = func_get_args();
+        // remove method & fallback
+        array_shift($args);
+        $fallback = array_pop($args);
+
+        $contents = file_get_contents($this->configFile->getPath());
+        $manipulator = new JsonManipulator($contents);
 
-        $this->configFile->write($configSettings);
+        // try to update cleanly
+        if (call_user_func_array(array($manipulator, $method), $args)) {
+            file_put_contents($this->configFile->getPath(), $manipulator->getContents());
+        } else {
+            // on failed clean update, call the fallback and rewrite the whole file
+            $config = $this->configFile->read();
+            array_unshift($args, $config);
+            call_user_func_array($fallback, $args);
+            $this->configFile->write($config);
+        }
     }
 
     /**
@@ -291,61 +305,4 @@ EOT
             $output->writeln('[<comment>' . $k . $key . '</comment>] <info>' . $value . '</info>');
         }
     }
-
-    /**
-     * This function will take a setting key (a.b.c) and return an
-     * array that matches this
-     *
-     * @param string $key
-     * @param string $value
-     * @return array
-     */
-    protected function parseSetting($key, $value)
-    {
-        $parts = array_reverse(explode('.', $key));
-        $tmp = array();
-        for ($i = 0; $i < count($parts); $i++) {
-            $tmp[$parts[$i]] = (0 === $i) ? $value : $tmp;
-            if (0 < $i) {
-                unset($tmp[$parts[$i - 1]]);
-            }
-        }
-
-        return $tmp;
-    }
-
-    /**
-     * After the command sets a new config value, this will parse it writes
-     * it to disk to make sure that it is valid according the the composer.json
-     * schema.
-     *
-     * @param array $data
-     * @throws JsonValidationException
-     * @return boolean
-     */
-    protected function validateSchema(array $data)
-    {
-        // TODO Figure out what should be excluded from the validation check
-        // TODO validation should vary based on if it's global or local
-        $schemaFile = __DIR__ . '/../../../res/composer-schema.json';
-        $schemaData = json_decode(file_get_contents($schemaFile));
-
-        unset(
-            $schemaData->properties->name,
-            $schemaData->properties->description
-        );
-
-        $validator = new Validator();
-        $validator->check(json_decode(json_encode($data)), $schemaData);
-
-        if (!$validator->isValid()) {
-            $errors = array();
-            foreach ((array) $validator->getErrors() as $error) {
-                $errors[] = ($error['property'] ? $error['property'].' : ' : '').$error['message'];
-            }
-            throw new JsonValidationException('"'.$this->configFile->getPath().'" does not match the expected JSON schema'."\n". implode("\n",$errors));
-        }
-
-        return true;
-    }
 }

src/Composer/Json/JsonManipulator.php

@@ -17,6 +17,8 @@ namespace Composer\Json;
  */
 class JsonManipulator
 {
+    private static $RECURSE_BLOCKS = '(?:[^{}]*|\{(?:[^{}]*|\{(?:[^{}]*|\{(?:[^{}]*|\{[^{}]*\})*\})*\})*\})*';
+
     private $contents;
     private $newline;
     private $indent;
@@ -56,7 +58,7 @@ class JsonManipulator
 
         // link exists already
         if (preg_match('{"'.$packageRegex.'"\s*:}i', $links)) {
-            $links = preg_replace('{"'.$packageRegex.'"(\s*:\s*)"[^"]+"}i', JsonFile::encode($package).'$1"'.$constraint.'"', $links);
+            $links = preg_replace('{"'.$packageRegex.'"(\s*:\s*)"[^"]+"}i', JsonFile::encode($package).'${1}"'.$constraint.'"', $links);
         } elseif (preg_match('#[^\s](\s*)$#', $links, $match)) {
             // link missing but non empty links
             $links = preg_replace(
@@ -69,7 +71,118 @@ class JsonManipulator
             $links = $this->newline . $this->indent . $this->indent . JsonFile::encode($package).': '.JsonFile::encode($constraint) . $links;
         }
 
-        $this->contents = preg_replace($linksRegex, '$1'.$links.'$3', $this->contents);
+        $this->contents = preg_replace($linksRegex, '${1}'.$links.'$3', $this->contents);
+
+        return true;
+    }
+
+    public function addRepository($name, $config)
+    {
+        return $this->addSubNode('repositories', $name, $config);
+    }
+
+    public function removeRepository($name)
+    {
+        return $this->removeSubNode('repositories', $name);
+    }
+
+    public function addConfigSetting($name, $value)
+    {
+        return $this->addSubNode('config', $name, $value);
+    }
+
+    public function removeConfigSetting($name)
+    {
+        return $this->removeSubNode('config', $name);
+    }
+
+    public function addSubNode($mainNode, $name, $value)
+    {
+        // no main node yet
+        if (!preg_match('#"'.$mainNode.'":\s*\{#', $this->contents)) {
+            $this->addMainKey(''.$mainNode.'', $this->format(array($name => $value)));
+
+            return true;
+        }
+
+        // main node content not match-able
+        $nodeRegex = '#("'.$mainNode.'":\s*\{)('.self::$RECURSE_BLOCKS.')(\})#s';
+        if (!preg_match($nodeRegex, $this->contents, $match)) {
+            return false;
+        }
+
+        $children = $match[2];
+
+        // invalid match due to un-regexable content, abort
+        if (!json_decode('{'.$children.'}')) {
+            return false;
+        }
+
+        // child exists
+        if (preg_match('{("'.preg_quote($name).'"\s*:\s*)([0-9.]+|null|true|false|"[^"]+"|\{'.self::$RECURSE_BLOCKS.'\})(,?)}', $children, $matches)) {
+            $children = preg_replace('{("'.preg_quote($name).'"\s*:\s*)([0-9.]+|null|true|false|"[^"]+"|\{'.self::$RECURSE_BLOCKS.'\})(,?)}', '${1}'.$this->format($value, 1).'$3', $children);
+        } elseif (preg_match('#[^\s](\s*)$#', $children, $match)) {
+            // child missing but non empty children
+            $children = preg_replace(
+                '#'.$match[1].'$#',
+                ',' . $this->newline . $this->indent . $this->indent . JsonFile::encode($name).': '.$this->format($value, 1) . $match[1],
+                $children
+            );
+        } else {
+            // children present but empty
+            $children = $this->newline . $this->indent . $this->indent . JsonFile::encode($name).': '.$this->format($value, 1) . $children;
+        }
+
+        $this->contents = preg_replace($nodeRegex, '${1}'.$children.'$3', $this->contents);
+
+        return true;
+    }
+
+    public function removeSubNode($mainNode, $name)
+    {
+        // no node
+        if (!preg_match('#"'.$mainNode.'":\s*\{#', $this->contents)) {
+            return true;
+        }
+
+        // empty node
+        if (preg_match('#"'.$mainNode.'":\s*\{\s*\}#s', $this->contents)) {
+            return true;
+        }
+
+        // no node content match-able
+        $nodeRegex = '#("'.$mainNode.'":\s*\{)('.self::$RECURSE_BLOCKS.')(\})#s';
+        if (!preg_match($nodeRegex, $this->contents, $match)) {
+            return false;
+        }
+
+        $children = $match[2];
+
+        // invalid match due to un-regexable content, abort
+        if (!json_decode('{'.$children.'}')) {
+            return false;
+        }
+
+        if (preg_match('{"'.preg_quote($name).'"\s*:}i', $children)) {
+            if (preg_match('{"'.preg_quote($name).'"\s*:\s*(?:[0-9.]+|null|true|false|"[^"]+"|\{'.self::$RECURSE_BLOCKS.'\})}', $children, $matches)) {
+                $children = preg_replace('{,\s*'.preg_quote($matches[0]).'}i', '', $children, -1, $count);
+                if (1 !== $count) {
+                    $children = preg_replace('{'.preg_quote($matches[0]).'\s*,?\s*}i', '', $children, -1, $count);
+                    if (1 !== $count) {
+                        return false;
+                    }
+                }
+            }
+
+        }
+
+        if (!trim($children)) {
+            $this->contents = preg_replace($nodeRegex, '$1'.$this->newline.$this->indent.'}', $this->contents);
+
+            return true;
+        }
+
+        $this->contents = preg_replace($nodeRegex, '${1}'.$children.'$3', $this->contents);
 
         return true;
     }
@@ -91,7 +204,7 @@ class JsonManipulator
         }
     }
 
-    protected function format($data)
+    protected function format($data, $depth = 0)
     {
         if (is_array($data)) {
             reset($data);
@@ -102,10 +215,10 @@ class JsonManipulator
 
             $out = '{' . $this->newline;
             foreach ($data as $key => $val) {
-                $elems[] = $this->indent . $this->indent . JsonFile::encode($key). ': '.$this->format($val);
+                $elems[] = str_repeat($this->indent, $depth + 2) . JsonFile::encode($key). ': '.$this->format($val, $depth + 1);
             }
 
-            return $out . implode(','.$this->newline, $elems) . $this->newline . $this->indent . '}';
+            return $out . implode(','.$this->newline, $elems) . $this->newline . str_repeat($this->indent, $depth + 1) . '}';
         }
 
         return JsonFile::encode($data);

tests/Composer/Test/Json/JsonManipulatorTest.php

@@ -131,4 +131,319 @@ class JsonManipulatorTest extends \PHPUnit_Framework_TestCase
             ),
         );
     }
+
+    /**
+     * @dataProvider removeSubNodeProvider
+     */
+    public function testRemoveSubNode($json, $name, $expected, $expectedContent = null)
+    {
+        $manipulator = new JsonManipulator($json);
+
+        $this->assertEquals($expected, $manipulator->removeSubNode('repositories', $name));
+        if (null !== $expectedContent) {
+            $this->assertEquals($expectedContent, $manipulator->getContents());
+        }
+    }
+
+    public function removeSubNodeProvider()
+    {
+        return array(
+            'works on simple ones first' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "foo": "bar",
+            "bar": "baz"
+        },
+        "bar": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}',
+                'foo',
+                true,
+                '{
+    "repositories": {
+        "bar": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}
+'
+            ),
+            'works on simple ones last' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "foo": "bar",
+            "bar": "baz"
+        },
+        "bar": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}',
+                'bar',
+                true,
+                '{
+    "repositories": {
+        "foo": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}
+'
+            ),
+            'works on simple ones unique' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}',
+                'foo',
+                true,
+                '{
+    "repositories": {
+    }
+}
+'
+            ),
+            'works on simple ones middle' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "foo": "bar",
+            "bar": "baz"
+        },
+        "bar": {
+            "foo": "bar",
+            "bar": "baz"
+        },
+        "baz": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}',
+                'bar',
+                true,
+                '{
+    "repositories": {
+        "foo": {
+            "foo": "bar",
+            "bar": "baz"
+        },
+        "baz": {
+            "foo": "bar",
+            "bar": "baz"
+        }
+    }
+}
+'
+            ),
+            'works on empty repos' => array(
+                '{
+    "repositories": {
+    }
+}',
+                'bar',
+                true
+            ),
+            'works on empty repos2' => array(
+                '{
+    "repositories": {}
+}',
+                'bar',
+                true
+            ),
+            'works on missing repos' => array(
+                "{\n}",
+                'bar',
+                true
+            ),
+            'works on deep repos' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "package": { "bar": "baz" }
+        }
+    }
+}',
+                'foo',
+                true,
+                '{
+    "repositories": {
+    }
+}
+'
+            ),
+            'fails on deep repos with borked texts' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "package": { "bar": "ba{z" }
+        }
+    }
+}',
+                'bar',
+                false
+            ),
+            'fails on deep repos with borked texts2' => array(
+                '{
+    "repositories": {
+        "foo": {
+            "package": { "bar": "ba}z" }
+        }
+    }
+}',
+                'bar',
+                false
+            ),
+        );
+    }
+
+    public function testAddRepositoryCanInitializeEmptyRepositories()
+    {
+        $manipulator = new JsonManipulator('{
+    "repositories": {
+    }
+}');
+
+        $this->assertTrue($manipulator->addRepository('bar', array('type' => 'composer')));
+        $this->assertEquals('{
+    "repositories": {
+        "bar": {
+            "type": "composer"
+        }
+    }
+}
+', $manipulator->getContents());
+    }
+
+    public function testAddRepositoryCanInitializeFromScratch()
+    {
+        $manipulator = new JsonManipulator('{
+    "a": "b"
+}');
+
+        $this->assertTrue($manipulator->addRepository('bar2', array('type' => 'composer')));
+        $this->assertEquals('{
+    "a": "b",
+    "repositories": {
+        "bar2": {
+            "type": "composer"
+        }
+    }
+}
+', $manipulator->getContents());
+    }
+
+    public function testAddRepositoryCanAdd()
+    {
+        $manipulator = new JsonManipulator('{
+    "repositories": {
+        "foo": {
+            "type": "vcs",
+            "url": "lala"
+        }
+    }
+}');
+
+        $this->assertTrue($manipulator->addRepository('bar', array('type' => 'composer')));
+        $this->assertEquals('{
+    "repositories": {
+        "foo": {
+            "type": "vcs",
+            "url": "lala"
+        },
+        "bar": {
+            "type": "composer"
+        }
+    }
+}
+', $manipulator->getContents());
+    }
+
+    public function testAddRepositoryCanOverrideDeepRepos()
+    {
+        $manipulator = new JsonManipulator('{
+    "repositories": {
+        "baz": {
+            "type": "package",
+            "package": {}
+        }
+    }
+}');
+
+        $this->assertTrue($manipulator->addRepository('baz', array('type' => 'composer')));
+        $this->assertEquals('{
+    "repositories": {
+        "baz": {
+            "type": "composer"
+        }
+    }
+}
+', $manipulator->getContents());
+    }
+
+    public function testAddConfigSettingCanAdd()
+    {
+        $manipulator = new JsonManipulator('{
+    "config": {
+        "foo": "bar"
+    }
+}');
+
+        $this->assertTrue($manipulator->addConfigSetting('bar', 'baz'));
+        $this->assertEquals('{
+    "config": {
+        "foo": "bar",
+        "bar": "baz"
+    }
+}
+', $manipulator->getContents());
+    }
+
+    public function testAddConfigSettingCanOverwrite()
+    {
+        $manipulator = new JsonManipulator('{
+    "config": {
+        "foo": "bar",
+        "bar": "baz"
+    }
+}');
+
+        $this->assertTrue($manipulator->addConfigSetting('foo', 'zomg'));
+        $this->assertEquals('{
+    "config": {
+        "foo": "zomg",
+        "bar": "baz"
+    }
+}
+', $manipulator->getContents());
+    }
+
+    public function testAddConfigSettingCanOverwriteNumbers()
+    {
+        $manipulator = new JsonManipulator('{
+    "config": {
+        "foo": 500
+    }
+}');
+
+        $this->assertTrue($manipulator->addConfigSetting('foo', 50));
+        $this->assertEquals('{
+    "config": {
+        "foo": 50
+    }
+}
+', $manipulator->getContents());
+    }
 }