From 7427187647523f79c31712a0fb2224183f49d921 Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Sun, 28 Mar 2021 11:30:35 +0200 Subject: [PATCH] fix api 404 on page parameter (#2468) --- config/packages/fos_rest.yaml | 1 + tests/API/APIControllerBaseTest.php | 60 ++++++++++----------------- tests/API/TimesheetControllerTest.php | 21 +++++++++- 3 files changed, 42 insertions(+), 40 deletions(-) diff --git a/config/packages/fos_rest.yaml b/config/packages/fos_rest.yaml index 7dad5697..8c7b8699 100644 --- a/config/packages/fos_rest.yaml +++ b/config/packages/fos_rest.yaml @@ -26,6 +26,7 @@ fos_rest: 'Symfony\Component\Routing\Exception\ResourceNotFoundException': 404 'Symfony\Component\HttpKernel\Exception\NotFoundHttpException': 404 'App\API\NotFoundException': 404 + 'Pagerfanta\Exception\OutOfRangeCurrentPageException': 404 body_listener: enabled: true decoders: diff --git a/tests/API/APIControllerBaseTest.php b/tests/API/APIControllerBaseTest.php index 23221217..1272e64f 100644 --- a/tests/API/APIControllerBaseTest.php +++ b/tests/API/APIControllerBaseTest.php @@ -120,17 +120,10 @@ abstract class APIControllerBaseTest extends ControllerBaseTest sprintf('The secure URL %s is not protected for role %s', $url, $role) ); - $expected = [ + $this->assertApiException($client->getResponse(), [ 'code' => 403, 'message' => 'Access denied.' - ]; - - self::assertEquals(403, $client->getResponse()->getStatusCode()); - - self::assertEquals( - $expected, - json_decode($client->getResponse()->getContent(), true) - ); + ]); } protected function request(HttpKernelBrowser $client, string $url, $method = 'GET', array $parameters = [], string $content = null): Crawler @@ -144,23 +137,15 @@ abstract class APIControllerBaseTest extends ControllerBaseTest { $client = $this->getClientForAuthenticatedUser($role); $this->request($client, $url, $method); - - $expected = [ + $this->assertApiException($client->getResponse(), [ 'code' => 404, 'message' => 'Not found' - ]; - - self::assertEquals(404, $client->getResponse()->getStatusCode()); - - self::assertEquals( - $expected, - json_decode($client->getResponse()->getContent(), true) - ); + ]); } protected function assertNotFoundForDelete(HttpKernelBrowser $client, string $url) { - return $this->assertExceptionForMethod($client, $url, 'DELETE', [], [ + $this->assertExceptionForMethod($client, $url, 'DELETE', [], [ 'code' => 404, 'message' => 'Not found' ]); @@ -168,7 +153,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest protected function assertEntityNotFoundForDelete(string $role, string $url) { - return $this->assertExceptionForDeleteAction($role, $url, [], [ + $this->assertExceptionForDeleteAction($role, $url, [], [ 'code' => 404, 'message' => 'Not found' ]); @@ -176,7 +161,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest protected function assertEntityNotFoundForPatch(string $role, string $url, array $data) { - return $this->assertExceptionForPatchAction($role, $url, $data, [ + $this->assertExceptionForPatchAction($role, $url, $data, [ 'code' => 404, 'message' => 'Not found' ]); @@ -184,7 +169,7 @@ abstract class APIControllerBaseTest extends ControllerBaseTest protected function assertEntityNotFoundForPost(string $role, string $url, array $data, ?string $message = null) { - return $this->assertExceptionForPostAction($role, $url, $data, [ + $this->assertExceptionForPostAction($role, $url, $data, [ 'code' => 404, 'message' => $message ?? 'Not found' ]); @@ -208,15 +193,14 @@ abstract class APIControllerBaseTest extends ControllerBaseTest protected function assertExceptionForMethod(HttpKernelBrowser $client, string $url, string $method, array $data, array $expectedErrors) { $this->request($client, $url, $method, [], json_encode($data)); - $response = $client->getResponse(); + $this->assertApiException($client->getResponse(), $expectedErrors); + } + + protected function assertApiException(Response $response, array $expectedErrors) + { self::assertFalse($response->isSuccessful()); - - self::assertEquals($expectedErrors['code'], $client->getResponse()->getStatusCode()); - - self::assertEquals( - $expectedErrors, - json_decode($client->getResponse()->getContent(), true) - ); + self::assertEquals($expectedErrors['code'], $response->getStatusCode()); + self::assertEquals($expectedErrors, json_decode($response->getContent(), true)); } protected function assertExceptionForRole(string $role, string $url, string $method, array $data, array $expectedErrors) @@ -225,11 +209,9 @@ abstract class APIControllerBaseTest extends ControllerBaseTest $this->assertExceptionForMethod($client, $url, $method, $data, $expectedErrors); } - protected function assertApiException(Response $response, string $message) + protected function assertApi500Exception(Response $response, string $message) { - self::assertFalse($response->isSuccessful()); - self::assertEquals(500, $response->getStatusCode()); - self::assertEquals(['code' => 500, 'message' => $message], json_decode($response->getContent(), true)); + $this->assertApiException($response, ['code' => 500, 'message' => $message]); } protected function assertApiAccessDenied(HttpKernelBrowser $client, string $url, string $message) @@ -240,10 +222,10 @@ abstract class APIControllerBaseTest extends ControllerBaseTest protected function assertApiResponseAccessDenied(Response $response, string $message) { - self::assertFalse($response->isSuccessful()); - self::assertEquals(Response::HTTP_FORBIDDEN, $response->getStatusCode()); - $expected = ['code' => Response::HTTP_FORBIDDEN, 'message' => $message]; - self::assertEquals($expected, json_decode($response->getContent(), true)); + $this->assertApiException($response, [ + 'code' => Response::HTTP_FORBIDDEN, + 'message' => $message + ]); } /** diff --git a/tests/API/TimesheetControllerTest.php b/tests/API/TimesheetControllerTest.php index 80b9fae4..92a73f0d 100644 --- a/tests/API/TimesheetControllerTest.php +++ b/tests/API/TimesheetControllerTest.php @@ -177,6 +177,25 @@ class TimesheetControllerTest extends APIControllerBaseTest self::assertApiResponseTypeStructure('TimesheetCollection', $result[0]); } + public function testGetCollectionWithQueryFailsWith404OnOutOfRangedPage() + { + $modifiedAfter = new \DateTime('-1 hour'); + $begin = new \DateTime('first day of this month'); + $begin->setTime(0, 0, 0); + $end = new \DateTime('last day of this month'); + $end->setTime(23, 59, 59); + + $query = [ + 'page' => 19, + 'size' => 50, + ]; + + $client = $this->getClientForAuthenticatedUser(User::ROLE_USER); + $this->importFixtureForUser(User::ROLE_USER); + $this->request($client, '/api/timesheets', 'GET', $query); + $this->assertApiException($client->getResponse(), ['code' => 404, 'message' => 'Page "19" does not exist. The currentPage must be inferior to "1"']); + } + public function testGetCollectionWithSingleParamsQuery() { $begin = new \DateTime('first day of this month'); @@ -760,7 +779,7 @@ class TimesheetControllerTest extends APIControllerBaseTest $id = $timesheets[0]->getId(); $this->request($client, '/api/timesheets/' . $id . '/stop', 'PATCH'); - $this->assertApiException($client->getResponse(), 'Timesheet entry already stopped'); + $this->assertApi500Exception($client->getResponse(), 'Timesheet entry already stopped'); } public function testStopThrowsNotFound()