From 72db15ca5d7950a604f359056fc6a627f25e4ee4 Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 5 May 2017 12:05:50 +0200 Subject: Little refacto and send 400 on reaching urls limit --- .../ApiBundle/Controller/EntryRestController.php | 196 ++++++++++----------- .../Controller/EntryRestControllerTest.php | 7 +- 2 files changed, 97 insertions(+), 106 deletions(-) diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 7c3e778e..dbff6065 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -5,7 +5,7 @@ namespace Wallabag\ApiBundle\Controller; use Hateoas\Configuration\Route; use Hateoas\Representation\Factory\PagerfantaFactory; use Nelmio\ApiDocBundle\Annotation\ApiDoc; -use Symfony\Component\Config\Definition\Exception\Exception; +use Symfony\Component\HttpKernel\Exception\HttpException; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\JsonResponse; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; @@ -45,9 +45,7 @@ class EntryRestController extends WallabagRestController $results[$url] = $res instanceof Entry ? $res->getId() : false; } - $json = $this->get('serializer')->serialize($results, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($results); } // let's see if it is a simple url? @@ -63,9 +61,7 @@ class EntryRestController extends WallabagRestController $exists = $res instanceof Entry ? $res->getId() : false; - $json = $this->get('serializer')->serialize(['exists' => $exists], 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse(['exists' => $exists]); } /** @@ -125,9 +121,7 @@ class EntryRestController extends WallabagRestController ) ); - $json = $this->get('serializer')->serialize($paginatedCollection, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($paginatedCollection); } /** @@ -146,9 +140,7 @@ class EntryRestController extends WallabagRestController $this->validateAuthentication(); $this->validateUserAccess($entry->getUser()->getId()); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -189,35 +181,35 @@ class EntryRestController extends WallabagRestController $this->validateAuthentication(); $urls = json_decode($request->query->get('urls', [])); + + if (empty($urls)) { + return $this->sendResponse([]); + } + $results = []; // handle multiple urls - if (!empty($urls)) { - $results = []; - foreach ($urls as $key => $url) { - $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( - $url, - $this->getUser()->getId() - ); - - $results[$key]['url'] = $url; + foreach ($urls as $key => $url) { + $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( + $url, + $this->getUser()->getId() + ); - if (false !== $entry) { - $em = $this->getDoctrine()->getManager(); - $em->remove($entry); - $em->flush(); + $results[$key]['url'] = $url; - // entry deleted, dispatch event about it! - $this->get('event_dispatcher')->dispatch(EntryDeletedEvent::NAME, new EntryDeletedEvent($entry)); - } + if (false !== $entry) { + $em = $this->getDoctrine()->getManager(); + $em->remove($entry); + $em->flush(); - $results[$key]['entry'] = $entry instanceof Entry ? true : false; + // entry deleted, dispatch event about it! + $this->get('event_dispatcher')->dispatch(EntryDeletedEvent::NAME, new EntryDeletedEvent($entry)); } - } - $json = $this->get('serializer')->serialize($results, 'json'); + $results[$key]['entry'] = $entry instanceof Entry ? true : false; + } - return (new JsonResponse())->setJson($json); + return $this->sendResponse($results); } /** @@ -231,7 +223,7 @@ class EntryRestController extends WallabagRestController * * @return JsonResponse * - * @throws Symfony\Component\Config\Definition\Exception\Exception When limit is reached + * @throws HttpException When limit is reached */ public function postEntriesListAction(Request $request) { @@ -243,7 +235,7 @@ class EntryRestController extends WallabagRestController $limit = $this->container->getParameter('wallabag_core.api_limit_mass_actions'); if (count($urls) > $limit) { - throw new Exception('API limit reached'); + throw new HttpException(400, 'API limit reached'); } // handle multiple urls @@ -274,9 +266,7 @@ class EntryRestController extends WallabagRestController } } - $json = $this->get('serializer')->serialize($results, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($results); } /** @@ -336,9 +326,7 @@ class EntryRestController extends WallabagRestController // entry saved, dispatch event about it! $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -387,9 +375,7 @@ class EntryRestController extends WallabagRestController $em = $this->getDoctrine()->getManager(); $em->flush(); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -432,9 +418,7 @@ class EntryRestController extends WallabagRestController // entry saved, dispatch event about it! $this->get('event_dispatcher')->dispatch(EntrySavedEvent::NAME, new EntrySavedEvent($entry)); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -460,9 +444,7 @@ class EntryRestController extends WallabagRestController // entry deleted, dispatch event about it! $this->get('event_dispatcher')->dispatch(EntryDeletedEvent::NAME, new EntryDeletedEvent($entry)); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -481,9 +463,7 @@ class EntryRestController extends WallabagRestController $this->validateAuthentication(); $this->validateUserAccess($entry->getUser()->getId()); - $json = $this->get('serializer')->serialize($entry->getTags(), 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry->getTags()); } /** @@ -514,9 +494,7 @@ class EntryRestController extends WallabagRestController $em->persist($entry); $em->flush(); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -541,9 +519,7 @@ class EntryRestController extends WallabagRestController $em->persist($entry); $em->flush(); - $json = $this->get('serializer')->serialize($entry, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($entry); } /** @@ -562,45 +538,46 @@ class EntryRestController extends WallabagRestController $this->validateAuthentication(); $list = json_decode($request->query->get('list', [])); - $results = []; + + if (empty($list)) { + return $this->sendResponse([]); + } // handle multiple urls - if (!empty($list)) { - foreach ($list as $key => $element) { - $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( - $element->url, - $this->getUser()->getId() - ); + $results = []; - $results[$key]['url'] = $element->url; - $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; + foreach ($list as $key => $element) { + $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( + $element->url, + $this->getUser()->getId() + ); - $tags = $element->tags; + $results[$key]['url'] = $element->url; + $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; - if (false !== $entry && !(empty($tags))) { - $tags = explode(',', $tags); - foreach ($tags as $label) { - $label = trim($label); + $tags = $element->tags; - $tag = $this->getDoctrine() - ->getRepository('WallabagCoreBundle:Tag') - ->findOneByLabel($label); + if (false !== $entry && !(empty($tags))) { + $tags = explode(',', $tags); + foreach ($tags as $label) { + $label = trim($label); - if (false !== $tag) { - $entry->removeTag($tag); - } - } + $tag = $this->getDoctrine() + ->getRepository('WallabagCoreBundle:Tag') + ->findOneByLabel($label); - $em = $this->getDoctrine()->getManager(); - $em->persist($entry); - $em->flush(); + if (false !== $tag) { + $entry->removeTag($tag); + } } + + $em = $this->getDoctrine()->getManager(); + $em->persist($entry); + $em->flush(); } } - $json = $this->get('serializer')->serialize($results, 'json'); - - return (new JsonResponse())->setJson($json); + return $this->sendResponse($results); } /** @@ -619,32 +596,47 @@ class EntryRestController extends WallabagRestController $this->validateAuthentication(); $list = json_decode($request->query->get('list', [])); + + if (empty($list)) { + return $this->sendResponse([]); + } + $results = []; // handle multiple urls - if (!empty($list)) { - foreach ($list as $key => $element) { - $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( - $element->url, - $this->getUser()->getId() - ); + foreach ($list as $key => $element) { + $entry = $this->get('wallabag_core.entry_repository')->findByUrlAndUserId( + $element->url, + $this->getUser()->getId() + ); - $results[$key]['url'] = $element->url; - $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; + $results[$key]['url'] = $element->url; + $results[$key]['entry'] = $entry instanceof Entry ? $entry->getId() : false; - $tags = $element->tags; + $tags = $element->tags; - if (false !== $entry && !(empty($tags))) { - $this->get('wallabag_core.content_proxy')->assignTagsToEntry($entry, $tags); + if (false !== $entry && !(empty($tags))) { + $this->get('wallabag_core.content_proxy')->assignTagsToEntry($entry, $tags); - $em = $this->getDoctrine()->getManager(); - $em->persist($entry); - $em->flush(); - } + $em = $this->getDoctrine()->getManager(); + $em->persist($entry); + $em->flush(); } } - $json = $this->get('serializer')->serialize($results, 'json'); + return $this->sendResponse($results); + } + + /** + * Shortcut to send data serialized in json. + * + * @param mixed $data + * + * @return JsonResponse + */ + private function sendResponse($data) + { + $json = $this->get('serializer')->serialize($data, 'json'); return (new JsonResponse())->setJson($json); } diff --git a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php index 8594ad0b..34a2f894 100644 --- a/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php +++ b/tests/Wallabag/ApiBundle/Controller/EntryRestControllerTest.php @@ -810,10 +810,6 @@ class EntryRestControllerTest extends WallabagApiTestCase $this->assertEquals('http://0.0.0.0/entry3', $content[1]['url']); } - /** - * @expectedException Symfony\Component\Config\Definition\Exception\Exception - * @expectedExceptionMessage API limit reached - */ public function testLimitBulkAction() { $list = [ @@ -831,5 +827,8 @@ class EntryRestControllerTest extends WallabagApiTestCase ]; $this->client->request('POST', '/api/entries/lists?urls='.json_encode($list)); + + $this->assertEquals(400, $this->client->getResponse()->getStatusCode()); + $this->assertContains('API limit reached', $this->client->getResponse()->getContent()); } } -- cgit v1.2.3