]> git.immae.eu Git - github/wallabag/wallabag.git/commitdiff
EntryRestController::getEntriesExistsAction: always find by hashed url
authorOlivier Mehani <shtrom@ssji.net>
Thu, 2 May 2019 11:19:20 +0000 (21:19 +1000)
committerJeremy Benoist <jeremy.benoist@gmail.com>
Fri, 24 May 2019 13:17:45 +0000 (15:17 +0200)
Simplify the logic from #3158 by hashing all the urls from the request,
and only doing a search by hash. This allows to get performance benefits
from the new indexed hash column even when using older clients that do
not hash the URL in the request.

Fixes: #3158, #3919
Signed-off-by: Olivier Mehani <shtrom@ssji.net>
src/Wallabag/ApiBundle/Controller/EntryRestController.php

index aff0534a056fcf0f08a124d3d093956e129ee6de..17b53a0189d6284e849332d373511fc520e5e391 100644 (file)
@@ -43,50 +43,59 @@ class EntryRestController extends WallabagRestController
 
         $returnId = (null === $request->query->get('return_id')) ? false : (bool) $request->query->get('return_id');
 
-        $urls = $request->query->get('urls', []);
         $hashedUrls = $request->query->get('hashed_urls', []);
+        $hashedUrl = $request->query->get('hashed_url', '');
+        if (!empty($hashedUrl)) {
+            $hashedUrls[] = $hashedUrl;
+        }
 
-        // handle multiple urls first
-        if (!empty($hashedUrls)) {
-            $results = [];
-            foreach ($hashedUrls as $hashedUrl) {
-                $res = $repo->findByHashedUrlAndUserId($hashedUrl, $this->getUser()->getId());
-
-                $results[$hashedUrl] = $this->returnExistInformation($res, $returnId);
-            }
+        $urls = $request->query->get('urls', []);
+        $url = $request->query->get('url', '');
+        if (!empty($url)) {
+            $urls[] = $url;
+        }
 
-            return $this->sendResponse($results);
+        $urlHashMap = [];
+        foreach($urls as $urlToHash) {
+            $urlHash = hash('sha1', $urlToHash); // XXX: the hash logic would better be in a separate util to avoid duplication with GenerateUrlHashesCommand::generateHashedUrls
+            $hashedUrls[] = $urlHash;
+            $urlHashMap[$urlHash] = $urlToHash;
         }
 
-        // @deprecated, to be remove in 3.0
-        if (!empty($urls)) {
-            $results = [];
-            foreach ($urls as $url) {
-                $res = $repo->findByUrlAndUserId($url, $this->getUser()->getId());
+        if (empty($hashedUrls)) {
+            throw $this->createAccessDeniedException('URL is empty?, logged user id: ' . $this->getUser()->getId());
+        }
 
-                $results[$url] = $this->returnExistInformation($res, $returnId);
-            }
+        $results = [];
+        foreach ($hashedUrls as $hashedUrlToSearch) {
+            $res = $repo->findByHashedUrlAndUserId($hashedUrlToSearch, $this->getUser()->getId());
 
-            return $this->sendResponse($results);
+            $results[$hashedUrlToSearch] = $this->returnExistInformation($res, $returnId);
         }
 
-        // let's see if it is a simple url?
-        $url = $request->query->get('url', '');
-        $hashedUrl = $request->query->get('hashed_url', '');
+        $results = $this->replaceUrlHashes($results, $urlHashMap);
 
-        if (empty($url) && empty($hashedUrl)) {
-            throw $this->createAccessDeniedException('URL is empty?, logged user id: ' . $this->getUser()->getId());
+        if (!empty($url) || !empty($hashedUrl)) {
+            $hu = array_keys($results)[0];
+            return $this->sendResponse(['exists' => $results[$hu]]);
         }
+        return $this->sendResponse($results);
+    }
 
-        $method = 'findByUrlAndUserId';
-        if (!empty($hashedUrl)) {
-            $method = 'findByHashedUrlAndUserId';
-            $url = $hashedUrl;
+    /**
+     * Replace the hashedUrl keys in $results with the unhashed URL from the
+     * request, as recorded in $urlHashMap.
+     */
+    private function replaceUrlHashes(array $results, array $urlHashMap) {
+        $newResults = [];
+        foreach($results as $hash => $res) {
+            if (isset($urlHashMap[$hash])) {
+                $newResults[$urlHashMap[$hash]] = $res;
+            } else {
+                $newResults[$hash] = $res;
+            }
         }
-
-        $res = $repo->$method($url, $this->getUser()->getId());
-
-        return $this->sendResponse(['exists' => $this->returnExistInformation($res, $returnId)]);
+        return $newResults;
     }
 
     /**