From 18696f77fdc76f7055d84460d459051a6bb7253a Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 23 Jun 2017 08:30:23 +0200 Subject: Avoid BC on exists endpoint --- .../ApiBundle/Controller/EntryRestController.php | 8 +++-- .../Controller/EntryRestControllerTest.php | 41 +++++++++++++++++++--- 2 files changed, 43 insertions(+), 6 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 768c4fdc..793b91ba 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -18,9 +18,12 @@ class EntryRestController extends WallabagRestController { /** * Check if an entry exist by url. + * Return ID if entry(ies) exist (and if you give the return_id parameter). + * Otherwise it returns false. * * @ApiDoc( * parameters={ + * {"name"="return_id", "dataType"="string", "required"=false, "format"="1 or 0", "description"="Set 1 if you want to retrieve ID in case entry(ies) exists, 0 by default"}, * {"name"="url", "dataType"="string", "required"=true, "format"="An url", "description"="Url to check if it exists"}, * {"name"="urls", "dataType"="string", "required"=false, "format"="An array of urls (?urls[]=http...&urls[]=http...)", "description"="Urls (as an array) to check if it exists"} * } @@ -32,6 +35,7 @@ class EntryRestController extends WallabagRestController { $this->validateAuthentication(); + $returnId = (null === $request->query->get('return_id')) ? 0 : (bool) $request->query->get('return_id'); $urls = $request->query->get('urls', []); // handle multiple urls first @@ -42,7 +46,7 @@ class EntryRestController extends WallabagRestController ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId($url, $this->getUser()->getId()); - $results[$url] = $res instanceof Entry ? $res->getId() : false; + $results[$url] = $res instanceof Entry ? ($returnId ? $res->getId() : true) : false; } return $this->sendResponse($results); @@ -59,7 +63,7 @@ class EntryRestController extends WallabagRestController ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId($url, $this->getUser()->getId()); - $exists = $res instanceof Entry ? $res->getId() : false; + $exists = $res instanceof Entry ? ($returnId ? $res->getId() : true) : false; return $this->sendResponse(['exists' => $exists]); } diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php index 067aed2c..dc21bd32 100644 --- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php @@ -759,18 +759,51 @@ class EntryRestControllerTest extends WallabagApiTestCase $this->assertEquals(true, $content['is_starred']); } - public function testGetEntriesExists() + public function dataForEntriesExistWithUrl() { - $this->client->request('GET', '/api/entries/exists?url=http://0.0.0.0/entry2'); + return [ + 'with_id' => [ + 'url' => '/api/entries/exists?url=http://0.0.0.0/entry2&return_id=1', + 'expectedValue' => 2, + ], + 'without_id' => [ + 'url' => '/api/entries/exists?url=http://0.0.0.0/entry2', + 'expectedValue' => true, + ], + ]; + } + + /** + * @dataProvider dataForEntriesExistWithUrl + */ + public function testGetEntriesExists($url, $expectedValue) + { + $this->client->request('GET', $url); $this->assertEquals(200, $this->client->getResponse()->getStatusCode()); $content = json_decode($this->client->getResponse()->getContent(), true); - $this->assertEquals(2, $content['exists']); + $this->assertSame($expectedValue, $content['exists']); } public function testGetEntriesExistsWithManyUrls() + { + $url1 = 'http://0.0.0.0/entry2'; + $url2 = 'http://0.0.0.0/entry10'; + $this->client->request('GET', '/api/entries/exists?urls[]='.$url1.'&urls[]='.$url2.'&return_id=1'); + + $this->assertEquals(200, $this->client->getResponse()->getStatusCode()); + + $content = json_decode($this->client->getResponse()->getContent(), true); + + $this->assertArrayHasKey($url1, $content); + $this->assertArrayHasKey($url2, $content); + $this->assertSame(2, $content[$url1]); + $this->assertSame(false, $content[$url2]); + } + + public function testGetEntriesExistsWithManyUrlsReturnBool() { $url1 = 'http://0.0.0.0/entry2'; $url2 = 'http://0.0.0.0/entry10'; @@ -782,7 +815,7 @@ class EntryRestControllerTest extends WallabagApiTestCase $this->assertArrayHasKey($url1, $content); $this->assertArrayHasKey($url2, $content); - $this->assertEquals(2, $content[$url1]); + $this->assertEquals(true, $content[$url1]); $this->assertEquals(false, $content[$url2]); } -- cgit v1.2.3 From 39ffaba3232b6378f47883615c58a9ffba668af3 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Wed, 28 Jun 2017 08:15:06 +0200 Subject: Return null instead of false --- .../ApiBundle/Controller/EntryRestController.php | 30 +++++++++++++++++++--- .../Controller/EntryRestControllerTest.php | 6 ++--- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 793b91ba..9786c4d3 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -4,6 +4,7 @@ namespace Wallabag\ApiBundle\Controller; use Hateoas\Configuration\Route; use Hateoas\Representation\Factory\PagerfantaFactory; +use JMS\Serializer\SerializationContext; use Nelmio\ApiDocBundle\Annotation\ApiDoc; use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpFoundation\Request; @@ -21,6 +22,8 @@ class EntryRestController extends WallabagRestController * Return ID if entry(ies) exist (and if you give the return_id parameter). * Otherwise it returns false. * + * @todo Remove that `return_id` in the next major release + * * @ApiDoc( * parameters={ * {"name"="return_id", "dataType"="string", "required"=false, "format"="1 or 0", "description"="Set 1 if you want to retrieve ID in case entry(ies) exists, 0 by default"}, @@ -46,7 +49,7 @@ class EntryRestController extends WallabagRestController ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId($url, $this->getUser()->getId()); - $results[$url] = $res instanceof Entry ? ($returnId ? $res->getId() : true) : false; + $results[$url] = $this->returnExistInformation($res, $returnId); } return $this->sendResponse($results); @@ -63,7 +66,7 @@ class EntryRestController extends WallabagRestController ->getRepository('WallabagCoreBundle:Entry') ->findByUrlAndUserId($url, $this->getUser()->getId()); - $exists = $res instanceof Entry ? ($returnId ? $res->getId() : true) : false; + $exists = $this->returnExistInformation($res, $returnId); return $this->sendResponse(['exists' => $exists]); } @@ -621,7 +624,11 @@ class EntryRestController extends WallabagRestController */ private function sendResponse($data) { - $json = $this->get('serializer')->serialize($data, 'json'); + // https://github.com/schmittjoh/JMSSerializerBundle/issues/293 + $context = new SerializationContext(); + $context->setSerializeNull(true); + + $json = $this->get('serializer')->serialize($data, 'json', $context); return (new JsonResponse())->setJson($json); } @@ -698,4 +705,21 @@ class EntryRestController extends WallabagRestController // entry saved, dispatch event about it! $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); } + + /** + * Return information about the entry if it exist and depending on the id or not. + * + * @param Entry|null $entry + * @param bool $returnId + * + * @return bool|int + */ + private function returnExistInformation($entry, $returnId) + { + if ($returnId) { + return $entry instanceof Entry ? $entry->getId() : null; + } + + return $entry instanceof Entry ? true : false; + } } diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php index dc21bd32..c0391d87 100644 --- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php @@ -800,7 +800,7 @@ class EntryRestControllerTest extends WallabagApiTestCase $this->assertArrayHasKey($url1, $content); $this->assertArrayHasKey($url2, $content); $this->assertSame(2, $content[$url1]); - $this->assertSame(false, $content[$url2]); + $this->assertNull($content[$url2]); } public function testGetEntriesExistsWithManyUrlsReturnBool() @@ -815,8 +815,8 @@ class EntryRestControllerTest extends WallabagApiTestCase $this->assertArrayHasKey($url1, $content); $this->assertArrayHasKey($url2, $content); - $this->assertEquals(true, $content[$url1]); - $this->assertEquals(false, $content[$url2]); + $this->assertTrue($content[$url1]); + $this->assertFalse($content[$url2]); } public function testGetEntriesExistsWhichDoesNotExists() -- cgit v1.2.3 From 331e5b026189ff6433014659f32304b1706015e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Benoist?= Date: Wed, 28 Jun 2017 09:18:22 +0200 Subject: CS --- src/Wallabag/ApiBundle/Controller/EntryRestController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 9786c4d3..c7938633 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -38,7 +38,7 @@ class EntryRestController extends WallabagRestController { $this->validateAuthentication(); - $returnId = (null === $request->query->get('return_id')) ? 0 : (bool) $request->query->get('return_id'); + $returnId = (null === $request->query->get('return_id')) ? false : (bool) $request->query->get('return_id'); $urls = $request->query->get('urls', []); // handle multiple urls first @@ -720,6 +720,6 @@ class EntryRestController extends WallabagRestController return $entry instanceof Entry ? $entry->getId() : null; } - return $entry instanceof Entry ? true : false; + return $entry instanceof Entry; } } -- cgit v1.2.3