Skip to content
This repository
Browse code

fixed and refactored YamlFileLoader in the same sense as the XmlFileL…

…oader

some nonsesense configs were not validated correctly like an imported resource with a pattern key. added tests for such things too.
  • Loading branch information...
commit 237bbd057869206a2ffba2c5da209ab672b79bb3 1 parent 392d785
Tobias Schultze authored December 07, 2012
120  src/Symfony/Component/Routing/Loader/YamlFileLoader.php
@@ -21,13 +21,14 @@
21 21
  * YamlFileLoader loads Yaml routing files.
22 22
  *
23 23
  * @author Fabien Potencier <fabien@symfony.com>
  24
+ * @author Tobias Schultze <http://tobion.de>
24 25
  *
25 26
  * @api
26 27
  */
27 28
 class YamlFileLoader extends FileLoader
28 29
 {
29 30
     private static $availableKeys = array(
30  
-        'type', 'resource', 'prefix', 'pattern', 'options', 'defaults', 'requirements', 'hostname_pattern',
  31
+        'resource', 'type', 'prefix', 'pattern', 'hostname_pattern', 'defaults', 'requirements', 'options',
31 32
     );
32 33
 
33 34
     /**
@@ -38,7 +39,7 @@ class YamlFileLoader extends FileLoader
38 39
      *
39 40
      * @return RouteCollection A RouteCollection instance
40 41
      *
41  
-     * @throws \InvalidArgumentException When route can't be parsed
  42
+     * @throws \InvalidArgumentException When a route can't be parsed because YAML is invalid
42 43
      *
43 44
      * @api
44 45
      */
@@ -53,38 +54,19 @@ public function load($file, $type = null)
53 54
 
54 55
         // empty file
55 56
         if (null === $config) {
56  
-            $config = array();
  57
+            return $collection;
57 58
         }
58 59
 
59 60
         // not an array
60 61
         if (!is_array($config)) {
61  
-            throw new \InvalidArgumentException(sprintf('The file "%s" must contain a YAML array.', $file));
  62
+            throw new \InvalidArgumentException(sprintf('The file "%s" must contain a YAML array.', $path));
62 63
         }
63 64
 
64 65
         foreach ($config as $name => $config) {
65  
-            $config = $this->normalizeRouteConfig($config);
  66
+            $this->validate($config, $name, $path);
66 67
 
67 68
             if (isset($config['resource'])) {
68  
-                $type = isset($config['type']) ? $config['type'] : null;
69  
-                $prefix = isset($config['prefix']) ? $config['prefix'] : '';
70  
-                $defaults = isset($config['defaults']) ? $config['defaults'] : array();
71  
-                $requirements = isset($config['requirements']) ? $config['requirements'] : array();
72  
-                $options = isset($config['options']) ? $config['options'] : array();
73  
-                $hostnamePattern = isset($config['hostname_pattern']) ? $config['hostname_pattern'] : null;
74  
-
75  
-                $this->setCurrentDir(dirname($path));
76  
-
77  
-                $subCollection = $this->import($config['resource'], $type, false, $file);
78  
-                /* @var $subCollection RouteCollection */
79  
-                $subCollection->addPrefix($prefix);
80  
-                if (null !== $hostnamePattern) {
81  
-                    $subCollection->setHostnamePattern($hostnamePattern);
82  
-                }
83  
-                $subCollection->addDefaults($defaults);
84  
-                $subCollection->addRequirements($requirements);
85  
-                $subCollection->addOptions($options);
86  
-
87  
-                $collection->addCollection($subCollection);
  69
+                $this->parseImport($collection, $config, $path, $file);
88 70
             } else {
89 71
                 $this->parseRoute($collection, $name, $config, $path);
90 72
             }
@@ -109,46 +91,90 @@ public function supports($resource, $type = null)
109 91
      * @param RouteCollection $collection A RouteCollection instance
110 92
      * @param string          $name       Route name
111 93
      * @param array           $config     Route definition
112  
-     * @param string          $file       A Yaml file path
113  
-     *
114  
-     * @throws \InvalidArgumentException When config pattern is not defined for the given route
  94
+     * @param string          $path       Full path of the YAML file being processed
115 95
      */
116  
-    protected function parseRoute(RouteCollection $collection, $name, $config, $file)
  96
+    protected function parseRoute(RouteCollection $collection, $name, array $config, $path)
117 97
     {
118 98
         $defaults = isset($config['defaults']) ? $config['defaults'] : array();
119 99
         $requirements = isset($config['requirements']) ? $config['requirements'] : array();
120 100
         $options = isset($config['options']) ? $config['options'] : array();
121 101
         $hostnamePattern = isset($config['hostname_pattern']) ? $config['hostname_pattern'] : null;
122 102
 
123  
-        if (!isset($config['pattern'])) {
124  
-            throw new \InvalidArgumentException(sprintf('You must define a "pattern" for the "%s" route.', $name));
125  
-        }
126  
-
127 103
         $route = new Route($config['pattern'], $defaults, $requirements, $options, $hostnamePattern);
128 104
 
129 105
         $collection->add($name, $route);
130 106
     }
131 107
 
132 108
     /**
133  
-     * Normalize route configuration.
  109
+     * Parses an import and adds the routes in the resource to the RouteCollection.
134 110
      *
135  
-     * @param array $config A resource config
  111
+     * @param RouteCollection $collection A RouteCollection instance
  112
+     * @param array           $config     Route definition
  113
+     * @param string          $path       Full path of the YAML file being processed
  114
+     * @param string          $file       Loaded file name
  115
+     */
  116
+    protected function parseImport(RouteCollection $collection, array $config, $path, $file)
  117
+    {
  118
+        $type = isset($config['type']) ? $config['type'] : null;
  119
+        $prefix = isset($config['prefix']) ? $config['prefix'] : '';
  120
+        $defaults = isset($config['defaults']) ? $config['defaults'] : array();
  121
+        $requirements = isset($config['requirements']) ? $config['requirements'] : array();
  122
+        $options = isset($config['options']) ? $config['options'] : array();
  123
+        $hostnamePattern = isset($config['hostname_pattern']) ? $config['hostname_pattern'] : null;
  124
+
  125
+        $this->setCurrentDir(dirname($path));
  126
+
  127
+        $subCollection = $this->import($config['resource'], $type, false, $file);
  128
+        /* @var $subCollection RouteCollection */
  129
+        $subCollection->addPrefix($prefix);
  130
+        if (null !== $hostnamePattern) {
  131
+            $subCollection->setHostnamePattern($hostnamePattern);
  132
+        }
  133
+        $subCollection->addDefaults($defaults);
  134
+        $subCollection->addRequirements($requirements);
  135
+        $subCollection->addOptions($options);
  136
+
  137
+        $collection->addCollection($subCollection);
  138
+    }
  139
+
  140
+    /**
  141
+     * Validates the route configuration.
136 142
      *
137  
-     * @return array
  143
+     * @param array  $config A resource config
  144
+     * @param string $name   The config key
  145
+     * @param string $path   The loaded file path
138 146
      *
139  
-     * @throws \InvalidArgumentException if one of the provided config keys is not supported
  147
+     * @throws \InvalidArgumentException If one of the provided config keys is not supported,
  148
+     *                                   something is missing or the combination is nonesense
140 149
      */
141  
-    private function normalizeRouteConfig(array $config)
  150
+    protected function validate($config, $name, $path)
142 151
     {
143  
-        foreach ($config as $key => $value) {
144  
-            if (!in_array($key, self::$availableKeys)) {
145  
-                throw new \InvalidArgumentException(sprintf(
146  
-                    'Yaml routing loader does not support given key: "%s". Expected one of the (%s).',
147  
-                    $key, implode(', ', self::$availableKeys)
148  
-                ));
149  
-            }
  152
+        if (!is_array($config)) {
  153
+            throw new \InvalidArgumentException(sprintf('The definition of "%s" in "%s" must be a YAML array.', $name, $path));
  154
+        }
  155
+        if ($extraKeys = array_diff(array_keys($config), self::$availableKeys)) {
  156
+            throw new \InvalidArgumentException(sprintf(
  157
+                'The routing file "%s" contains unsupport keys for "%s": "%s". Expected one of: "%s".',
  158
+                $path, $name, implode('", "', $extraKeys), implode('", "', self::$availableKeys)
  159
+            ));
  160
+        }
  161
+        if (isset($config['resource']) && isset($config['pattern'])) {
  162
+            throw new \InvalidArgumentException(sprintf(
  163
+                '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.',
  164
+                $path, $name
  165
+            ));
  166
+        }
  167
+        if (!isset($config['resource']) && isset($config['type'])) {
  168
+            throw new \InvalidArgumentException(sprintf(
  169
+                'The "type" key for the route definition "%s" in "%s" is unsupported. It is only available for imports in combination with the "resource" key.',
  170
+                $name, $path
  171
+            ));
  172
+        }
  173
+        if (!isset($config['resource']) && !isset($config['pattern'])) {
  174
+            throw new \InvalidArgumentException(sprintf(
  175
+                'You must define a "pattern" for the route "%s" in file "%s".',
  176
+                $name, $path
  177
+            ));
150 178
         }
151  
-
152  
-        return $config;
153 179
     }
154 180
 }
3  src/Symfony/Component/Routing/Tests/Fixtures/nonesense_resource_plus_path.yml
... ...
@@ -0,0 +1,3 @@
  1
+blog_show:
  2
+    resource: validpattern.yml
  3
+    pattern:  /test
3  src/Symfony/Component/Routing/Tests/Fixtures/nonesense_type_without_resource.yml
... ...
@@ -0,0 +1,3 @@
  1
+blog_show:
  2
+    pattern: /blog/{slug}
  3
+    type:    custom
1  src/Symfony/Component/Routing/Tests/Fixtures/nonvalid2.yml
... ...
@@ -0,0 +1 @@
  1
+route: string
31  src/Symfony/Component/Routing/Tests/Loader/YamlFileLoaderTest.php
@@ -50,29 +50,17 @@ public function testLoadDoesNothingIfEmpty()
50 50
 
51 51
     /**
52 52
      * @expectedException \InvalidArgumentException
  53
+     * @dataProvider getPathsToInvalidFiles
53 54
      */
54  
-    public function testLoadThrowsExceptionIfNotAnArray()
  55
+    public function testLoadThrowsExceptionWithInvalidFile($filePath)
55 56
     {
56 57
         $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
57  
-        $loader->load('nonvalid.yml');
  58
+        $loader->load($filePath);
58 59
     }
59 60
 
60  
-    /**
61  
-     * @expectedException \InvalidArgumentException
62  
-     */
63  
-    public function testLoadThrowsExceptionIfArrayHasUnsupportedKeys()
64  
-    {
65  
-        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
66  
-        $loader->load('nonvalidkeys.yml');
67  
-    }
68  
-
69  
-    /**
70  
-     * @expectedException \InvalidArgumentException
71  
-     */
72  
-    public function testLoadThrowsExceptionWhenIncomplete()
  61
+    public function getPathsToInvalidFiles()
73 62
     {
74  
-        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
75  
-        $loader->load('incomplete.yml');
  63
+        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'));
76 64
     }
77 65
 
78 66
     public function testLoadSpecialRouteName()
@@ -117,13 +105,4 @@ public function testLoadWithResource()
117 105
         $this->assertEquals('bar', $routes['blog_show']->getOption('foo'));
118 106
         $this->assertEquals('{locale}.example.com', $routes['blog_show']->getHostnamePattern());
119 107
     }
120  
-
121  
-    /**
122  
-     * @expectedException \InvalidArgumentException
123  
-     */
124  
-    public function testParseRouteThrowsExceptionWithMissingPattern()
125  
-    {
126  
-        $loader = new YamlFileLoader(new FileLocator(array(__DIR__.'/../Fixtures')));
127  
-        $loader->load('incomplete.yml');
128  
-    }
129 108
 }

0 notes on commit 237bbd0

Please sign in to comment.
Something went wrong with that request. Please try again.