Skip to content

Commit

Permalink
bug #2255 do not override the status code if it is already set (xabbuh)
Browse files Browse the repository at this point in the history
This PR was merged into the 2.x branch.

Discussion
----------

do not override the status code if it is already set

Commits
-------

e8d0314 do not override the status code if it is already set
  • Loading branch information
xabbuh committed Jul 14, 2020
2 parents ae1db28 + e8d0314 commit 88470cc
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 20 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
2.8.2
-----

* fixed the `ViewHandler` to not override an already set `status_code` in the serialization context
* fixed embedding status codes in the response body when a mapping of exception classes to status
codes is configured

Expand Down
48 changes: 29 additions & 19 deletions Tests/View/ViewHandlerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@

namespace FOS\RestBundle\Tests\View;

use FOS\RestBundle\Context\Context;
use FOS\RestBundle\Serializer\Serializer;
use FOS\RestBundle\View\View;
use FOS\RestBundle\View\ViewHandler;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Form\Extension\Core\Type\TextType;
use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\Forms;
use Symfony\Component\Form\FormView;
use Symfony\Component\HttpFoundation\Request;
Expand Down Expand Up @@ -97,24 +99,16 @@ public function testGetStatusCode(
$data,
$isSubmitted,
$isValid,
$isSubmittedCalled,
$isValidCalled,
$noContentCode,
$actualStatusCode = null
) {
$reflectionMethod = new \ReflectionMethod(ViewHandler::class, 'getStatusCode');
$reflectionMethod->setAccessible(true);

$form = $this->getMockBuilder(Form::class)
->disableOriginalConstructor()
->setMethods(array('isSubmitted', 'isValid'))
->getMock();
$form = $this->createMock(FormInterface::class);
$form
->expects($this->exactly($isSubmittedCalled))
->expects($this->any())
->method('isSubmitted')
->will($this->returnValue($isSubmitted));
$form
->expects($this->exactly($isValidCalled))
->expects($this->any())
->method('isValid')
->will($this->returnValue($isValid));

Expand All @@ -123,20 +117,36 @@ public function testGetStatusCode(
}
$view = new View($data ?: null, $actualStatusCode ?: null);

if ($data) {
$expectedContext = new Context();
$expectedContext->setAttribute('template_data', []);

if (null !== $actualStatusCode || $form->isSubmitted() && !$form->isValid()) {
$expectedContext->setAttribute('status_code', $expected);
}

$this->serializer
->expects($this->once())
->method('serialize')
->with($form, 'json', $expectedContext)
->willReturn(json_encode(['code' => $expected, 'message' => $isValid ? 'OK' : 'Validation failed']));
}

$viewHandler = $this->createViewHandler([], $expected, $noContentCode);
$this->assertEquals($expected, $reflectionMethod->invoke($viewHandler, $view, $view->getData()));
$this->assertEquals($expected, $viewHandler->createResponse($view, new Request(), 'json')->getStatusCode());
}

public static function getStatusCodeDataProvider()
{
return [
'no data' => [Response::HTTP_OK, false, false, false, 0, 0, Response::HTTP_OK],
'no data with 204' => [Response::HTTP_NO_CONTENT, false, false, false, 0, 0, Response::HTTP_NO_CONTENT],
'no data, but custom response code' => [Response::HTTP_OK, false, false, false, 0, 0, Response::HTTP_NO_CONTENT, Response::HTTP_OK],
'form key form not bound' => [Response::HTTP_OK, true, false, true, 1, 0, Response::HTTP_OK],
'form key form is bound and invalid' => [Response::HTTP_FORBIDDEN, true, true, false, 1, 1, Response::HTTP_OK],
'form key form bound and valid' => [Response::HTTP_OK, true, true, true, 1, 1, Response::HTTP_OK],
'form key null form bound and valid' => [Response::HTTP_OK, true, true, true, 1, 1, Response::HTTP_OK],
'no data' => [Response::HTTP_OK, false, false, false, Response::HTTP_OK],
'no data with 204' => [Response::HTTP_NO_CONTENT, false, false, false, Response::HTTP_NO_CONTENT],
'no data, but custom response code' => [Response::HTTP_OK, false, false, false, Response::HTTP_NO_CONTENT, Response::HTTP_OK],
'form key form not bound' => [Response::HTTP_OK, true, false, true, Response::HTTP_OK],
'form key form is bound and invalid' => [Response::HTTP_FORBIDDEN, true, true, false, Response::HTTP_OK],
'form key form is bound and invalid and status code in view is set' => [Response::HTTP_FORBIDDEN, true, true, false, Response::HTTP_OK, Response::HTTP_CREATED],
'form key form bound and valid' => [Response::HTTP_OK, true, true, true, Response::HTTP_OK],
'form key null form bound and valid' => [Response::HTTP_OK, true, true, true, Response::HTTP_OK],
];
}

Expand Down
2 changes: 1 addition & 1 deletion View/ViewHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ protected function getSerializationContext(View $view)
$context->setSerializeNull($this->serializeNullStrategy);
}

if (null !== $view->getStatusCode()) {
if (null !== $view->getStatusCode() && !$context->hasAttribute('status_code')) {
$context->setAttribute('status_code', $view->getStatusCode());
}

Expand Down

0 comments on commit 88470cc

Please sign in to comment.