add accept, accept can be multiple things, so 406 response content-ty…

…pe can be correct
chubbyphp · Apr 28, 2019 · ae4c715 · ae4c715
1 parent 33f5aab
commit ae4c715
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 53 deletions.
diff --git a/src/ApiProblem/ClientError/MethodNotAllowed.php b/src/ApiProblem/ClientError/MethodNotAllowed.php
@@ -19,7 +19,7 @@ final class MethodNotAllowed extends AbstractApiProblem
     private $allowedMethods = [];
 
     /**
-     * @param string $method,
+     * @param string      $method,
      * @param string[]    $allowedMethods
      * @param string|null $detail
      * @param string|null $instance

diff --git a/src/ApiProblem/ClientError/NotAcceptable.php b/src/ApiProblem/ClientError/NotAcceptable.php
@@ -8,18 +8,28 @@
 
 final class NotAcceptable extends AbstractApiProblem
 {
+    /**
+     * @var string
+     */
+    private $accept;
+
     /**
      * @var string[]
      */
-    private $acceptableMediaTypes = [];
+    private $acceptables = [];
 
     /**
-     * @param string[]    $acceptableMediaTypes
+     * @param string      $accept
+     * @param string[]    $acceptables
      * @param string|null $detail
      * @param string|null $instance
      */
-    public function __construct(array $acceptableMediaTypes, string $detail = null, string $instance = null)
-    {
+    public function __construct(
+        string $accept,
+        array $acceptables,
+        string $detail = null,
+        string $instance = null
+    ) {
         parent::__construct(
             'https://tools.ietf.org/html/rfc2616#section-10.4.7',
             406,
@@ -28,26 +38,35 @@ public function __construct(array $acceptableMediaTypes, string $detail = null,
             $instance
         );
 
-        $this->acceptableMediaTypes = $acceptableMediaTypes;
+        $this->accept = $accept;
+        $this->acceptables = $acceptables;
     }
 
     /**
      * @return array
      */
     public function getHeaders(): array
     {
-        if ([] === $this->acceptableMediaTypes) {
+        if ([] === $this->acceptables) {
             return [];
         }
 
-        return ['X-Acceptable' => implode(',', $this->acceptableMediaTypes)];
+        return ['X-Acceptables' => implode(',', $this->acceptables)];
+    }
+
+    /**
+     * @return string
+     */
+    public function getAccept(): string
+    {
+        return $this->accept;
     }
 
     /**
      * @return string[]
      */
-    public function getAcceptableMediaTypes(): array
+    public function getAcceptables(): array
     {
-        return $this->acceptableMediaTypes;
+        return $this->acceptables;
     }
 }
diff --git a/src/Manager/ResponseManager.php b/src/Manager/ResponseManager.php
@@ -101,18 +101,12 @@ public function createFromApiProblem(
     ): Response {
         $status = $apiProblem->getStatus();
 
-        $response = $this->responseFactory->createResponse($status);
+        $response = $this->responseFactory->createResponse($status)->withHeader('Content-Type', $accept);
 
         foreach ($apiProblem->getHeaders() as $name => $value) {
             $response = $response->withHeader($name, $value);
         }
 
-        if (406 === $status) {
-            return $response;
-        }
-
-        $response = $response->withHeader('Content-Type', $accept);
-
         $body = $this->serializer->serialize($apiProblem, $accept, $context);
 
         $response->getBody()->write($body);

diff --git a/src/Serialization/ApiProblem/ClientError/NotAcceptableMapping.php b/src/Serialization/ApiProblem/ClientError/NotAcceptableMapping.php
@@ -28,7 +28,8 @@ public function getNormalizationFieldMappings(string $path): array
     {
         $fieldMappings = parent::getNormalizationFieldMappings($path);
 
-        $fieldMappings[] = NormalizationFieldMappingBuilder::create('acceptableMediaTypes')->getMapping();
+        $fieldMappings[] = NormalizationFieldMappingBuilder::create('accept')->getMapping();
+        $fieldMappings[] = NormalizationFieldMappingBuilder::create('acceptables')->getMapping();
 
         return $fieldMappings;
     }

diff --git a/tests/ApiProblem/ClientError/NotAcceptableTest.php b/tests/ApiProblem/ClientError/NotAcceptableTest.php
@@ -12,27 +12,34 @@ final class NotAcceptableTest extends TestCase
 {
     public function testMinimal()
     {
-        $apiProblem = new NotAcceptable([]);
+        $apiProblem = new NotAcceptable('application/x-yaml', []);
 
         self::assertSame(406, $apiProblem->getStatus());
         self::assertSame([], $apiProblem->getHeaders());
         self::assertSame('https://tools.ietf.org/html/rfc2616#section-10.4.7', $apiProblem->getType());
         self::assertSame('Not Acceptable', $apiProblem->getTitle());
         self::assertNull($apiProblem->getDetail());
         self::assertNull($apiProblem->getInstance());
-        self::assertSame([], $apiProblem->getAcceptableMediaTypes());
+        self::assertSame('application/x-yaml', $apiProblem->getAccept());
+        self::assertSame([], $apiProblem->getAcceptables());
     }
 
     public function testMaximal()
     {
-        $apiProblem = new NotAcceptable(['application/json', 'application/xml'], 'detail', '/cccdfd0f-0da3-4070-8e55-61bd832b47c0');
+        $apiProblem = new NotAcceptable(
+            'application/x-yaml',
+            ['application/json', 'application/xml'],
+            'detail',
+            '/cccdfd0f-0da3-4070-8e55-61bd832b47c0'
+        );
 
         self::assertSame(406, $apiProblem->getStatus());
-        self::assertSame(['X-Acceptable' => 'application/json,application/xml'], $apiProblem->getHeaders());
+        self::assertSame(['X-Acceptables' => 'application/json,application/xml'], $apiProblem->getHeaders());
         self::assertSame('https://tools.ietf.org/html/rfc2616#section-10.4.7', $apiProblem->getType());
         self::assertSame('Not Acceptable', $apiProblem->getTitle());
         self::assertSame('detail', $apiProblem->getDetail());
         self::assertSame('/cccdfd0f-0da3-4070-8e55-61bd832b47c0', $apiProblem->getInstance());
-        self::assertSame(['application/json', 'application/xml'], $apiProblem->getAcceptableMediaTypes());
+        self::assertSame('application/x-yaml', $apiProblem->getAccept());
+        self::assertSame(['application/json', 'application/xml'], $apiProblem->getAcceptables());
     }
 }
diff --git a/tests/Manager/ResponseManagerTest.php b/tests/Manager/ResponseManagerTest.php
@@ -225,33 +225,4 @@ public function testCreateFromApiProblem()
 
         self::assertSame($response, $responseManager->createFromApiProblem($apiProblem, 'application/json'));
     }
-
-    public function testCreateFromApiProblemNotAcceptable()
-    {
-        /** @var ApiProblemInterface|MockObject $apiProblem */
-        $apiProblem = $this->getMockByCalls(ApiProblemInterface::class, [
-            Call::create('getStatus')->with()->willReturn(406),
-            Call::create('getHeaders')->with()->willReturn(['X-Acceptable' => 'application/xml']),
-        ]);
-
-        /** @var Response|MockObject $response */
-        $response = $this->getMockByCalls(Response::class, [
-            Call::create('withHeader')->with('X-Acceptable', 'application/xml')->willReturnSelf(),
-        ]);
-
-        /** @var DeserializerInterface|MockObject $deserializer */
-        $deserializer = $this->getMockByCalls(DeserializerInterface::class);
-
-        /** @var ResponseFactoryInterface|MockObject $responseFactory */
-        $responseFactory = $this->getMockByCalls(ResponseFactoryInterface::class, [
-            Call::create('createResponse')->with(406, '')->willReturn($response),
-        ]);
-
-        /** @var SerializerInterface|MockObject $serializer */
-        $serializer = $this->getMockByCalls(SerializerInterface::class);
-
-        $responseManager = new ResponseManager($deserializer, $responseFactory, $serializer);
-
-        self::assertSame($response, $responseManager->createFromApiProblem($apiProblem, 'application/json'));
-    }
 }
diff --git a/tests/Serialization/ApiProblem/ClientError/NotAcceptableMappingTest.php b/tests/Serialization/ApiProblem/ClientError/NotAcceptableMappingTest.php
@@ -37,7 +37,8 @@ public function testGetNormalizationFieldMappings()
             NormalizationFieldMappingBuilder::create('title')->getMapping(),
             NormalizationFieldMappingBuilder::create('detail')->getMapping(),
             NormalizationFieldMappingBuilder::create('instance')->getMapping(),
-            NormalizationFieldMappingBuilder::create('acceptableMediaTypes')->getMapping(),
+            NormalizationFieldMappingBuilder::create('accept')->getMapping(),
+            NormalizationFieldMappingBuilder::create('acceptables')->getMapping(),
         ], $fieldMappings);
     }