diff --git a/src/Symfony/Component/Routing/Loader/YamlFileLoader.php b/src/Symfony/Component/Routing/Loader/YamlFileLoader.php index 326414e78e..7a5e268b27 100644 --- a/src/Symfony/Component/Routing/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/Routing/Loader/YamlFileLoader.php @@ -21,13 +21,14 @@ use Symfony\Component\Config\Loader\FileLoader; * YamlFileLoader loads Yaml routing files. * * @author Fabien Potencier + * @author Tobias Schultze * * @api */ class YamlFileLoader extends FileLoader { private static $availableKeys = array( - 'type', 'resource', 'prefix', 'pattern', 'options', 'defaults', 'requirements', 'hostname_pattern', + 'resource', 'type', 'prefix', 'pattern', 'hostname_pattern', 'defaults', 'requirements', 'options', ); /** @@ -38,7 +39,7 @@ class YamlFileLoader extends FileLoader * * @return RouteCollection A RouteCollection instance * - * @throws \InvalidArgumentException When route can't be parsed + * @throws \InvalidArgumentException When a route can't be parsed because YAML is invalid * * @api */ @@ -53,38 +54,19 @@ class YamlFileLoader extends FileLoader // empty file if (null === $config) { - $config = array(); + return $collection; } // not an array if (!is_array($config)) { - throw new \InvalidArgumentException(sprintf('The file "%s" must contain a YAML array.', $file)); + throw new \InvalidArgumentException(sprintf('The file "%s" must contain a YAML array.', $path)); } foreach ($config as $name => $config) { - $config = $this->normalizeRouteConfig($config); + $this->validate($config, $name, $path); if (isset($config['resource'])) { - $type = isset($config['type']) ? $config['type'] : null; - $prefix = isset($config['prefix']) ? $config['prefix'] : ''; - $defaults = isset($config['defaults']) ? $config['defaults'] : array(); - $requirements = isset($config['requirements']) ? $config['requirements'] : array(); - $options = isset($config['options']) ? $config['options'] : array(); - $hostnamePattern = isset($config['hostname_pattern']) ? $config['hostname_pattern'] : null; - - $this->setCurrentDir(dirname($path)); - - $subCollection = $this->import($config['resource'], $type, false, $file); - /* @var $subCollection RouteCollection */ - $subCollection->addPrefix($prefix); - if (null !== $hostnamePattern) { - $subCollection->setHostnamePattern($hostnamePattern); - } - $subCollection->addDefaults($defaults); - $subCollection->addRequirements($requirements); - $subCollection->addOptions($options); - - $collection->addCollection($subCollection); + $this->parseImport($collection, $config, $path, $file); } else { $this->parseRoute($collection, $name, $config, $path); } @@ -109,46 +91,90 @@ class YamlFileLoader extends FileLoader * @param RouteCollection $collection A RouteCollection instance * @param string $name Route name * @param array $config Route definition - * @param string $file A Yaml file path - * - * @throws \InvalidArgumentException When config pattern is not defined for the given route + * @param string $path Full path of the YAML file being processed */ - protected function parseRoute(RouteCollection $collection, $name, $config, $file) + protected function parseRoute(RouteCollection $collection, $name, array $config, $path) { $defaults = isset($config['defaults']) ? $config['defaults'] : array(); $requirements = isset($config['requirements']) ? $config['requirements'] : array(); $options = isset($config['options']) ? $config['options'] : array(); $hostnamePattern = isset($config['hostname_pattern']) ? $config['hostname_pattern'] : null; - if (!isset($config['pattern'])) { - throw new \InvalidArgumentException(sprintf('You must define a "pattern" for the "%s" route.', $name)); - } - $route = new Route($config['pattern'], $defaults, $requirements, $options, $hostnamePattern); $collection->add($name, $route); } /** - * Normalize route configuration. + * Parses an import and adds the routes in the resource to the RouteCollection. * - * @param array $config A resource config - * - * @return array - * - * @throws \InvalidArgumentException if one of the provided config keys is not supported + * @param RouteCollection $collection A RouteCollection instance + * @param array $config Route definition + * @param string $path Full path of the YAML file being processed + * @param string $file Loaded file name */ - private function normalizeRouteConfig(array $config) + protected function parseImport(RouteCollection $collection, array $config, $path, $file) { - foreach ($config as $key => $value) { - if (!in_array($key, self::$availableKeys)) { - throw new \InvalidArgumentException(sprintf( - 'Yaml routing loader does not support given key: "%s". Expected one of the (%s).', - $key, implode(', ', self::$availableKeys) - )); - } - } + $type = isset($config['type']) ? $config['type'] : null; + $prefix = isset($config['prefix']) ? $config['prefix'] : ''; + $defaults = isset($config['defaults']) ? $config['defaults'] : array(); + $requirements = isset($config['requirements']) ? $config['requirements'] : array(); + $options = isset($config['options']) ? $config['options'] : array(); + $hostnamePattern = isset($config['hostname_pattern']) ? $config['hostname_pattern'] : null; - return $config; + $this->setCurrentDir(dirname($path)); + + $subCollection = $this->import($config['resource'], $type, false, $file); + /* @var $subCollection RouteCollection */ + $subCollection->addPrefix($prefix); + if (null !== $hostnamePattern) { + $subCollection->setHostnamePattern($hostnamePattern); + } + $subCollection->addDefaults($defaults); + $subCollection->addRequirements($requirements); + $subCollection->addOptions($options); + + $collection->addCollection($subCollection); + } + + /** + * Validates the route configuration. + * + * @param array $config A resource config + * @param string $name The config key + * @param string $path The loaded file path + * + * @throws \InvalidArgumentException If one of the provided config keys is not supported, + * something is missing or the combination is nonesense + */ + protected function validate($config, $name, $path) + { + if (!is_array($config)) { + throw new \InvalidArgumentException(sprintf('The definition of "%s" in "%s" must be a YAML array.', $name, $path)); + } + if ($extraKeys = array_diff(array_keys($config), self::$availableKeys)) { + throw new \InvalidArgumentException(sprintf( + 'The routing file "%s" contains unsupport keys for "%s": "%s". Expected one of: "%s".', + $path, $name, implode('", "', $extraKeys), implode('", "', self::$availableKeys) + )); + } + if (isset($config['resource']) && isset($config['pattern'])) { + throw new \InvalidArgumentException(sprintf( + 'The routing file "%s" must not specify both the "resource" key and the "pattern" key for "%s". Choose between an import and a route definition.', + $path, $name + )); + } + if (!isset($config['resource']) && isset($config['type'])) { + throw new \InvalidArgumentException(sprintf( + 'The "type" key for the route definition "%s" in "%s" is unsupported. It is only available for imports in combination with the "resource" key.', + $name, $path + )); + } + if (!isset($config['resource']) && !isset($config['pattern'])) { + throw new \InvalidArgumentException(sprintf( + 'You must define a "pattern" for the route "%s" in file "%s".', + $name, $path + )); + } } } diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/nonesense_resource_plus_path.yml b/src/Symfony/Component/Routing/Tests/Fixtures/nonesense_resource_plus_path.yml new file mode 100644 index 0000000000..eba64e56bb --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Fixtures/nonesense_resource_plus_path.yml @@ -0,0 +1,3 @@ +blog_show: + resource: validpattern.yml + pattern: /test diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/nonesense_type_without_resource.yml b/src/Symfony/Component/Routing/Tests/Fixtures/nonesense_type_without_resource.yml new file mode 100644 index 0000000000..3b4c76898b --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Fixtures/nonesense_type_without_resource.yml @@ -0,0 +1,3 @@ +blog_show: + pattern: /blog/{slug} + type: custom diff --git a/src/Symfony/Component/Routing/Tests/Fixtures/nonvalid2.yml b/src/Symfony/Component/Routing/Tests/Fixtures/nonvalid2.yml new file mode 100644 index 0000000000..cfa9992bbc --- /dev/null +++ b/src/Symfony/Component/Routing/Tests/Fixtures/nonvalid2.yml @@ -0,0 +1 @@ +route: string diff --git a/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php index d28a499642..f99e789486 100644 --- a/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php @@ -50,29 +50,17 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase /** * @expectedException \InvalidArgumentException + * @dataProvider getPathsToInvalidFiles */ - public function testLoadThrowsExceptionIfNotAnArray() + public function testLoadThrowsExceptionWithInvalidFile($filePath) { $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures'))); - $loader->load('nonvalid.yml'); + $loader->load($filePath); } - /** - * @expectedException \InvalidArgumentException - */ - public function testLoadThrowsExceptionIfArrayHasUnsupportedKeys() + public function getPathsToInvalidFiles() { - $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures'))); - $loader->load('nonvalidkeys.yml'); - } - - /** - * @expectedException \InvalidArgumentException - */ - public function testLoadThrowsExceptionWhenIncomplete() - { - $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures'))); - $loader->load('incomplete.yml'); + return array(array('nonvalid.yml'), array('nonvalid2.yml'), array('incomplete.yml'), array('nonvalidkeys.yml'), array('nonesense_resource_plus_path.yml'), array('nonesense_type_without_resource.yml')); } public function testLoadSpecialRouteName() @@ -117,13 +105,4 @@ class YamlFileLoaderTest extends \PHPUnit_Framework_TestCase $this->assertEquals('bar', $routes['blog_show']->getOption('foo')); $this->assertEquals('{locale}.example.com', $routes['blog_show']->getHostnamePattern()); } - - /** - * @expectedException \InvalidArgumentException - */ - public function testParseRouteThrowsExceptionWithMissingPattern() - { - $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures'))); - $loader->load('incomplete.yml'); - } }