add version condition to route even without any routing annotations (#1438)

* add version condition to route also without any @route annotations

* review changes

Author: dmaicher

Routing/Loader/Reader/RestActionReader.php

@@ -292,7 +292,7 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,
         $requirements = [];
         $options = [];
         $host = '';
-        $condition = null;
+        $versionCondition = $this->getVersionCondition();
 
         $annotations = $this->readRouteAnnotation($method);
         if (!empty($annotations)) {
@@ -316,13 +316,13 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,
                 $defaults = array_merge($defaults, $annotation->getDefaults());
                 $host = $annotation->getHost();
                 $schemes = $annotation->getSchemes();
-                $condition = $this->getCondition($method, $annotation);
+                $combinedCondition = $this->combineConditions($versionCondition, $annotation->getCondition());
 
                 $this->includeFormatIfNeeded($path, $requirements);
 
                 // add route to collection
                 $route = new Route(
-                    $path, $defaults, $requirements, $options, $host, $schemes, $methods, $condition
+                    $path, $defaults, $requirements, $options, $host, $schemes, $methods, $combinedCondition
                 );
                 $this->addRoute($collection, $routeName, $route, $isCollection, $isInflectable, $annotation);
             }
@@ -333,39 +333,41 @@ public function read(RestRouteCollection $collection, \ReflectionMethod $method,
 
             // add route to collection
             $route = new Route(
-                $path, $defaults, $requirements, $options, $host, [], $methods, $condition
+                $path, $defaults, $requirements, $options, $host, [], $methods, $versionCondition
             );
             $this->addRoute($collection, $routeName, $route, $isCollection, $isInflectable);
         }
     }
 
     /**
-     * Determine the Route condition by combining Route annotations with Version annotation.
-     *
-     * @param \ReflectionMethod $method
-     * @param RouteAnnotation   $annotation
+     * @return string|null
+     */
+    private function getVersionCondition()
+    {
+        if (empty($this->versions)) {
+            return;
+        }
+
+        return sprintf("request.attributes.get('version') in ['%s']", implode("', '", $this->versions));
+    }
+
+    /**
+     * @param string|null $conditionOne
+     * @param string|null $conditionTwo
      *
-     * @return string
+     * @return string|null
      */
-    private function getCondition(\ReflectionMethod $method, RouteAnnotation $annotation)
+    private function combineConditions($conditionOne, $conditionTwo)
     {
-        $condition = $annotation->getCondition();
-
-        if (!empty($this->versions)) {
-            $versionCondition = "request.attributes.get('version') == (";
-            $first = true;
-            foreach ($this->versions as $version) {
-                if (!$first) {
-                    $versionCondition .= ' or ';
-                }
-                $versionCondition .= '\''.$version.'\'';
-                $first = false;
-            }
-            $versionCondition .= ')';
-            $condition = $condition ? '('.$condition.') and '.$versionCondition : $versionCondition;
+        if (null === $conditionOne) {
+            return $conditionTwo;
+        }
+
+        if (null === $conditionTwo) {
+            return $conditionOne;
         }
 
-        return $condition;
+        return sprintf('(%s) and (%s)', $conditionOne, $conditionTwo);
     }
 
     /**

Tests/Fixtures/Controller/AnnotatedVersionUserController.php

@@ -37,4 +37,8 @@ public function v1UserAction()
     public function conditionalUserAction()
     {
     }
+
+    public function v3UserAction()
+    {
+    }
 }

Tests/Fixtures/Etalon/annotated_version_controller.yml

@@ -2,10 +2,16 @@ v1_user:
   path:      /users/v1.{_format}
   controller:   ::v1UserAction
   methods: [GET]
-  condition: "request.attributes.get('version') == ('v1' or 'v3')"
+  condition: "request.attributes.get('version') in ['v1', 'v3']"
 
 conditional_user:
   path:      /users/conditional.{_format}
   controller:   ::conditionalUserAction
   methods: [GET]
-  condition: "(context.getMethod() in ['GET', 'HEAD'] and request.headers.get('User-Agent') matches '/firefox/i') and request.attributes.get('version') == ('v1' or 'v3')"
+  condition: "(request.attributes.get('version') in ['v1', 'v3']) and (context.getMethod() in ['GET', 'HEAD'] and request.headers.get('User-Agent') matches '/firefox/i')"
+
+v3_user:
+  path:     /users/v3.{_format}
+  controller:   ::v3UserAction
+  methods: [GET]
+  condition: "request.attributes.get('version') in ['v1', 'v3']"

Tests/Routing/Loader/RestRouteLoaderTest.php

@@ -173,7 +173,7 @@ public function testAnnotatedVersionUserFixture()
         $etalonRoutes = $this->loadEtalonRoutesInfo('annotated_version_controller.yml');
 
         $this->assertTrue($collection instanceof RestRouteCollection);
-        $this->assertEquals(2, count($collection->all()));
+        $this->assertEquals(3, count($collection->all()));
 
         foreach ($etalonRoutes as $name => $params) {
             $route = $collection->get($name);