From: Jeremy Benoist <jeremy.benoist@gmail.com>
Date: Fri, 5 May 2017 10:05:50 +0000 (+0200)
Subject: Little refacto and send 400 on reaching urls limit
X-Git-Tag: 2.3.0~31^2~107^2
X-Git-Url: https://git.immae.eu/?a=commitdiff_plain;h=72db15ca5d7950a604f359056fc6a627f25e4ee4;p=github%2Fwallabag%2Fwallabag.git

Little refacto and send 400 on reaching urls limit
---

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());
     }
 }