Merge pull request #1493 from FriendsOfSymfony/CHPICK

Backport #1438

Author: GuilhemN

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/Functional/Bundle/TestBundle/Controller/Version2Controller.php

@@ -0,0 +1,33 @@
+<?php
+
+/*
+ * This file is part of the FOSRestBundle package.
+ *
+ * (c) FriendsOfSymfony <http://friendsofsymfony.github.com/>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace FOS\RestBundle\Tests\Functional\Bundle\TestBundle\Controller;
+
+use FOS\RestBundle\Controller\Annotations\Get;
+use FOS\RestBundle\Controller\Annotations\Version;
+use FOS\RestBundle\Controller\Annotations\View;
+
+/**
+ * @author Ener-Getick <[email protected]>
+ *
+ * @Version({"1.2"})
+ */
+class Version2Controller
+{
+    /**
+     * @View("TestBundle:Version:version.html.twig")
+     * @Get(path="/version")
+     */
+    public function versionAction($version)
+    {
+        return array('version' => 'test annotation');
+    }
+}

Tests/Functional/Bundle/TestBundle/Resources/config/routing.yml

@@ -14,7 +14,12 @@ test_serializer_error_invalid_form:
     path:     /serializer-error/invalid-form.{_format}
     defaults: { _controller: TestBundle:SerializerError:invalidForm }
 
+# Must be defined before test_version
 test_version2:
+  resource: FOS\RestBundle\Tests\Functional\Bundle\TestBundle\Controller\Version2Controller
+  type: rest
+
+test_version:
     path: /version
     defaults: { _controller: TestBundle:Version:version }
     requirements:

Tests/Functional/VersionTest.php

@@ -23,6 +23,15 @@ public function setUp()
         $this->client = $this->createClient(['test_case' => 'Version']);
     }
 
+    public function testVersionAnnotation()
+    {
+        $this->client->request(
+            'GET',
+            '/version?query_version=1.2'
+          );
+        $this->assertContains('test annotation', $this->client->getResponse()->getContent());
+    }
+
     public function testCustomHeaderVersion()
     {
         $this->client->request(

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);