Refs #36, #40

kleijnweb · Jan 7, 2016 · 0f08202 · 0f08202
1 parent 0dda06a
commit 0f08202
Show file tree

Hide file tree

Showing 24 changed files with 429 additions and 74 deletions.
diff --git a/README.md b/README.md
@@ -203,6 +203,7 @@ public function placeOrder(Request $request)
    //...
 }
 ```
+When a controller action returns `NULL`, SwaggerBundle will return an empty `204` reponse with Content-Type `text/plain`.
 
 #### Using Annotations
 

diff --git a/src/Dev/Test/ApiTestCase.php b/src/Dev/Test/ApiTestCase.php
@@ -66,7 +66,7 @@ public static function initSchemaManager($swaggerPath)
      */
     protected function setUp()
     {
-        $this->client = static::createClient(['environment' => $this->env ?: 'test']);
+        $this->client = static::createClient(['environment' => $this->env ?: 'test', 'debug' => true]);
 
         parent::setUp();
     }
@@ -188,15 +188,14 @@ private function getJsonForLastRequest($fullPath, $method)
         $responseContent = $response->getContent();
 
         if ($response->getStatusCode() === 204 && !$responseContent) {
+            // TODO Validate this
             return null;
         }
 
-        $basePath = isset(self::$document->getDefinition()->basePath) ? self::$document->getDefinition()->basePath : '';
-        $relativePath = !$basePath ? $fullPath : substr($fullPath, strlen($basePath));
         $json = json_decode($responseContent);
 
-        if ($response->getStatusCode() !== 200) {
-            if (!isset($this->validateErrorResponse) || !$this->validateErrorResponse) {
+        if (substr($response->getStatusCode(), 0, 1) != '2') {
+            if (!isset($this->validateErrorResponse) || $this->validateErrorResponse) {
                 $this->assertResponseBodyMatch(
                     $json,
                     self::$schemaManager,
@@ -208,7 +207,9 @@ private function getJsonForLastRequest($fullPath, $method)
             throw new ApiResponseErrorException($json, $response->getStatusCode());
         }
 
-        if (self::$schemaManager->hasPath(['paths', $relativePath, $method, 'responses', '200'])) {
+        $request = $this->client->getRequest();
+
+        if (self::$schemaManager->hasPath(['paths', $request->get('_swagger_path'), $method, 'responses', '200'])) {
             $this->assertNotNull($json, "Not valid JSON: $responseContent");
             $headers = [];
 
@@ -224,7 +225,7 @@ private function getJsonForLastRequest($fullPath, $method)
         // If there is no response definition, the API should return 204 No Content.
         // With the current spec the behavior is undefined, this must be fixed.
         throw new \UnexpectedValueException(
-            "There is no 200 response definition for $relativePath:$method. For empty responses, use 204."
+            "There is no 200 response definition for {$request->get('_swagger_path')}:$method. For empty responses, use 204."
         );
     }
 }
diff --git a/src/Document/SwaggerDocument.php b/src/Document/SwaggerDocument.php
@@ -69,6 +69,14 @@ public function getResourceSchemas()
         return $this->definition->definitions;
     }
 
+    /**
+     * @return string
+     */
+    public function getBasePath()
+    {
+        return $this->definition->basePath;
+    }
+
     /**
      * @param string $path
      * @param string $method
@@ -77,10 +85,6 @@ public function getResourceSchemas()
      */
     public function getOperationDefinition($path, $method)
     {
-        if (isset($this->definition->basePath)) {
-            $path = substr($path, strlen($this->definition->basePath));
-        }
-
         $paths = $this->getPathDefinitions();
         if (!isset($paths[$path])) {
             throw new \InvalidArgumentException("Path '$path' not in Swagger document");

diff --git a/src/EventListener/RequestListener.php b/src/EventListener/RequestListener.php
@@ -29,7 +29,7 @@ class RequestListener
 
     /**
      * @param DocumentRepository $schemaRepository
-     * @param RequestProcessor $processor
+     * @param RequestProcessor   $processor
      */
     public function __construct(DocumentRepository $schemaRepository, RequestProcessor $processor)
     {
@@ -46,11 +46,17 @@ public function onKernelRequest(GetResponseEvent $event)
             return;
         }
         $request = $event->getRequest();
+        if (!$request->get('_definition')) {
+            return;
+        }
+        if (!$request->get('_swagger_path')) {
+            throw new \LogicException("Request does not contain reference to Swagger path");
+        }
         $swaggerDocument = $this->schemaRepository->get($request->get('_definition'));
 
         $operationDefinition = $swaggerDocument
             ->getOperationDefinition(
-                $request->getPathInfo(),
+                $request->get('_swagger_path'),
                 $request->getMethod()
             );
 

diff --git a/src/Response/ResponseFactory.php b/src/Response/ResponseFactory.php
@@ -10,7 +10,6 @@
 
 use KleijnWeb\SwaggerBundle\Document\DocumentRepository;
 use KleijnWeb\SwaggerBundle\Serializer\SerializerAdapter;
-use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 
@@ -49,8 +48,13 @@ public function __construct(DocumentRepository $documentRepository, SerializerAd
      */
     public function createResponse(Request $request, $data)
     {
+        $headers = ['Content-Type' => 'application/json'];
+
+        if ($data === null) {
+            return new Response($data, 204, $headers);
+        }
         $data = $this->serializer->serialize($data, 'json');
 
-        return new Response($data, 200, ['Content-Type' => 'application/json']);
+        return new Response($data, 200, $headers);
     }
 }
diff --git a/src/Routing/SwaggerRouteLoader.php b/src/Routing/SwaggerRouteLoader.php
@@ -64,8 +64,9 @@ public function load($resource, $type = null)
             foreach ($methods as $methodName => $operationSpec) {
                 $operationName = isset($operationSpec['operationId']) ? $operationSpec['operationId'] : $methodName;
                 $defaults = [
-                    '_controller' => "swagger.controller.$resourceName:$operationName",
-                    '_definition' => $resource
+                    '_controller'   => "swagger.controller.$resourceName:$operationName",
+                    '_definition'   => $resource,
+                    '_swagger_path' => $path
                 ];
                 $requirements = [];
                 $route = new Route($path, $defaults, $requirements);

diff --git a/src/Serializer/SerializationTypeResolver.php b/src/Serializer/SerializationTypeResolver.php
@@ -31,7 +31,7 @@ public function resolve(array $definitionFragment)
     {
         if (isset($definitionFragment['parameters'])) {
             foreach ($definitionFragment['parameters'] as $parameterDefinition) {
-                if ($parameterDefinition['in'] == 'body') {
+                if ($parameterDefinition['in'] == 'body' && isset($parameterDefinition['schema'])) {
                     return $this->resolveUsingSchema($parameterDefinition['schema']);
                 }
             }
@@ -55,7 +55,11 @@ public function qualify($typeName)
      */
     public function resolveUsingSchema(array $schema)
     {
-        $reference = isset($schema['$ref']) ? $schema['$ref'] : $schema['id'];
+        $reference = isset($schema['$ref']) ? $schema['$ref'] : (isset($schema['id']) ? $schema['id'] : null);
+
+        if (!$reference) {
+            return null;
+        }
 
         return $this->qualify(substr($reference, strrpos($reference, '/') + 1));
     }

diff --git a/src/Tests/Dev/Command/GenerateResourceClassesCommandTest.php b/src/Tests/Dev/Command/GenerateResourceClassesCommandTest.php
@@ -48,7 +48,7 @@ protected function setUp()
      */
     public function canExecute()
     {
-        $petStoreDocumentPath = __DIR__ . '/../../Functional/PetStore/app/petstore.yml';
+        $petStoreDocumentPath = __DIR__ . '/../../Functional/PetStore/app/swagger/petstore.yml';
         vfsStreamWrapper::register();
         vfsStreamWrapper::setRoot(new vfsStreamDirectory('willAddResponsesToDocument'));
 

diff --git a/src/Tests/Document/DocumentRepositoryTest.php b/src/Tests/Document/DocumentRepositoryTest.php
@@ -31,7 +31,7 @@ public function willFailWhenKeyIsEmpty()
     public function gettingDocumentThatDoestExistWillConstructIt()
     {
         $repository = new DocumentRepository();
-        $document = $repository->get('src/Tests/Functional/PetStore/app/petstore.yml');
+        $document = $repository->get('src/Tests/Functional/PetStore/app/swagger/petstore.yml');
         $this->assertInstanceOf('KleijnWeb\SwaggerBundle\Document\SwaggerDocument', $document);
     }
 
@@ -41,7 +41,7 @@ public function gettingDocumentThatDoestExistWillConstructIt()
     public function canUsePathPrefix()
     {
         $repository = new DocumentRepository('src/Tests/Functional/PetStore');
-        $document = $repository->get('app/petstore.yml');
+        $document = $repository->get('app/swagger/petstore.yml');
         $this->assertInstanceOf('KleijnWeb\SwaggerBundle\Document\SwaggerDocument', $document);
     }
 }
diff --git a/src/Tests/Document/SwaggerDocumentTest.php b/src/Tests/Document/SwaggerDocumentTest.php
@@ -55,7 +55,7 @@ public function canGetPathDefinitions()
      */
     public function getOperationDefinition()
     {
-        $actual = self::getPetStoreDocument()->getOperationDefinition('/v2/store/inventory', 'get');
+        $actual = self::getPetStoreDocument()->getOperationDefinition('/store/inventory', 'get');
         $this->assertInternalType('array', $actual);
 
         // Check a few keys
@@ -69,7 +69,7 @@ public function getOperationDefinition()
      */
     public function getOperationDefinitionHttpMethodIsCaseInsensitive()
     {
-        self::getPetStoreDocument()->getOperationDefinition('/v2/store/inventory', 'GET');
+        self::getPetStoreDocument()->getOperationDefinition('/store/inventory', 'GET');
     }
 
 
@@ -79,7 +79,7 @@ public function getOperationDefinitionHttpMethodIsCaseInsensitive()
      */
     public function getOperationDefinitionWillFailOnUnknownHttpMethod()
     {
-        self::getPetStoreDocument()->getOperationDefinition('/v2/store/inventory', 'post');
+        self::getPetStoreDocument()->getOperationDefinition('/store/inventory', 'post');
     }
 
     /**
@@ -96,12 +96,12 @@ public function getOperationDefinitionWillFailOnUnknownPath()
      */
     public function canWriteValidYamlToFileSystem()
     {
-        $originalHash = md5_file('src/Tests/Functional/PetStore/app/petstore.yml');
+        $originalHash = md5_file('src/Tests/Functional/PetStore/app/swagger/petstore.yml');
 
         $document = self::getPetStoreDocument();
         $document->write();
 
-        $newHash = md5_file('src/Tests/Functional/PetStore/app/petstore.yml');
+        $newHash = md5_file('src/Tests/Functional/PetStore/app/swagger/petstore.yml');
 
         $this->assertSame($originalHash, $newHash);
     }
@@ -126,7 +126,7 @@ public function gettingArrayCopyWillLeaveEmptyArraysAsEmptyArrays()
      */
     public function canWriteModifiedYamlToFileSystem()
     {
-        $originalHash = md5_file('src/Tests/Functional/PetStore/app/petstore.yml');
+        $originalHash = md5_file('src/Tests/Functional/PetStore/app/swagger/petstore.yml');
         vfsStreamWrapper::register();
         vfsStreamWrapper::setRoot(new vfsStreamDirectory('canWriteModifiedYamlToFileSystem'));
 
@@ -181,7 +181,7 @@ public function canResolveResourceSchemaReferences()
      */
     public function canResolveParameterSchemaReferences()
     {
-        $document = new SwaggerDocument('src/Tests/Functional/PetStore/app/instagram.yml');
+        $document = new SwaggerDocument('src/Tests/Functional/PetStore/app/swagger/instagram.yml');
         $pathDefinitions = $document->getPathDefinitions();
         $argumentPseudoSchema = $pathDefinitions['/users/{user-id}']['parameters'][0];
         $this->assertArrayNotHasKey('$ref', $argumentPseudoSchema);
@@ -194,6 +194,6 @@ public function canResolveParameterSchemaReferences()
      */
     public static function getPetStoreDocument()
     {
-        return new SwaggerDocument('src/Tests/Functional/PetStore/app/petstore.yml');
+        return new SwaggerDocument('src/Tests/Functional/PetStore/app/swagger/petstore.yml');
     }
 }
diff --git a/src/Tests/Document/YamlCapableUriRetrieverTest.php b/src/Tests/Document/YamlCapableUriRetrieverTest.php
@@ -21,7 +21,7 @@ class YamlCapableUriRetrieverTest extends \PHPUnit_Framework_TestCase
     public function canRetrieveYamlFileAsJsonString()
     {
         $retriever = new YamlCapableUriRetriever();
-        $result = $retriever->retrieve('file://' . realpath('src/Tests/Functional/PetStore/app/petstore.yml'));
+        $result = $retriever->retrieve('file://' . realpath('src/Tests/Functional/PetStore/app/swagger/petstore.yml'));
         $this->assertInternalType('string', $result);
         $array = json_decode($result, true);
         $this->assertNotNull($array);

diff --git a/src/Tests/EventListener/RequestListenerTest.php b/src/Tests/EventListener/RequestListenerTest.php
@@ -8,18 +8,16 @@
 
 namespace KleijnWeb\SwaggerBundle\Tests\Dev\EventListener;
 
-use KleijnWeb\SwaggerBundle\Document\DocumentRepository;
 use KleijnWeb\SwaggerBundle\EventListener\RequestListener;
-use KleijnWeb\SwaggerBundle\Request\RequestProcessor;
 use Symfony\Component\HttpFoundation\Request;
-use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
 
 /**
  * @author John Kleijn <john@kleijnweb.nl>
  */
 class RequestListenerTest extends \PHPUnit_Framework_TestCase
 {
     const DOCUMENT_PATH = '/what/a/crock';
+    const SWAGGER_PATH = '/a/b/{hello}';
 
     /**
      * @var \PHPUnit_Framework_MockObject_MockObject
@@ -56,7 +54,11 @@ class RequestListenerTest extends \PHPUnit_Framework_TestCase
      */
     protected function setUp()
     {
-        $this->request = new Request([], [], ['_definition' => self::DOCUMENT_PATH]);
+        $this->request = new Request(
+            [],
+            [],
+            ['_definition' => self::DOCUMENT_PATH, '_swagger_path' => self::SWAGGER_PATH]
+        );
 
         $this->documentMock = $this
             ->getMockBuilder('KleijnWeb\SwaggerBundle\Document\SwaggerDocument')
@@ -135,4 +137,84 @@ public function willNotTellTransformerToCoerceRequestWhenNotMasterRequest()
 
         $this->listener->onKernelRequest($this->eventMock);
     }
+
+    /**
+     * @test
+     */
+    public function willIgnoreRequestWithoutDefinition()
+    {
+        $wrongRequest = new Request();
+
+        $this->eventMock
+            ->expects($this->once())
+            ->method('isMasterRequest')
+            ->willReturn(true);
+
+        $this->eventMock
+            ->expects($this->once())
+            ->method('getRequest')
+            ->willReturn($wrongRequest);
+
+        $this->listener->onKernelRequest($this->eventMock);
+    }
+
+    /**
+     * @test
+     * @expectedException \LogicException
+     */
+    public function willFailOnRequestWithDefinitionButWithoutSwaggerPath()
+    {
+        $wrongRequest = new Request(
+            [],
+            [],
+            ['_definition' => self::DOCUMENT_PATH]
+        );
+
+        $this->eventMock
+            ->expects($this->once())
+            ->method('isMasterRequest')
+            ->willReturn(true);
+
+        $this->eventMock
+            ->expects($this->once())
+            ->method('getRequest')
+            ->willReturn($wrongRequest);
+
+        $this->listener->onKernelRequest($this->eventMock);
+    }
+
+    /**
+     * @test
+     */
+    public function canGetOperationDefinitionUsingSwaggerPath()
+    {
+        $this->eventMock
+            ->expects($this->once())
+            ->method('isMasterRequest')
+            ->willReturn(true);
+
+        $this->documentMock
+            ->expects($this->once())
+            ->method('getOperationDefinition')
+            ->with(self::SWAGGER_PATH)
+            ->willReturn([]);
+
+        $this->repositoryMock
+            ->expects($this->once())
+            ->method('get')
+            ->with(self::DOCUMENT_PATH)
+            ->willReturn($this->documentMock);
+
+        $this->eventMock
+            ->expects($this->once())
+            ->method('getRequest')
+            ->willReturn($this->request);
+
+        $this->transformerMock
+            ->expects($this->once())
+            ->method('process')
+            ->with($this->request);
+
+        $this->listener->onKernelRequest($this->eventMock);
+    }
 }