From 31e276fc1636b41b03b7c29127681de257c16b06 Mon Sep 17 00:00:00 2001 From: Olivier Mehani Date: Thu, 2 May 2019 21:19:20 +1000 Subject: EntryRestController::getEntriesExistsAction: always find by hashed url 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 --- .../ApiBundle/Controller/EntryRestController.php | 71 ++++++++++++---------- 1 file changed, 40 insertions(+), 31 deletions(-) (limited to 'src/Wallabag/ApiBundle') diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index aff0534a..17b53a01 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -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; } /** -- cgit v1.2.3 From 4a5516376bf4c8b0cdc1e81d24ce1cca68425785 Mon Sep 17 00:00:00 2001 From: Olivier Mehani Date: Fri, 10 May 2019 22:07:55 +1000 Subject: Add Wallabag\CoreBundle\Helper\UrlHasher Signed-off-by: Olivier Mehani --- .../ApiBundle/Controller/EntryRestController.php | 41 ++++++++++++---------- 1 file changed, 23 insertions(+), 18 deletions(-) (limited to 'src/Wallabag/ApiBundle') diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 17b53a01..77eb489e 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -14,6 +14,7 @@ use Wallabag\CoreBundle\Entity\Entry; use Wallabag\CoreBundle\Entity\Tag; use Wallabag\CoreBundle\Event\EntryDeletedEvent; use Wallabag\CoreBundle\Event\EntrySavedEvent; +use Wallabag\CoreBundle\Helper\UrlHasher; class EntryRestController extends WallabagRestController { @@ -56,8 +57,8 @@ class EntryRestController extends WallabagRestController } $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 + foreach ($urls as $urlToHash) { + $urlHash = UrlHasher::hashUrl($urlToHash); $hashedUrls[] = $urlHash; $urlHashMap[$urlHash] = $urlToHash; } @@ -77,25 +78,11 @@ class EntryRestController extends WallabagRestController if (!empty($url) || !empty($hashedUrl)) { $hu = array_keys($results)[0]; + return $this->sendResponse(['exists' => $results[$hu]]); } - return $this->sendResponse($results); - } - /** - * 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; - } - } - return $newResults; + return $this->sendResponse($results); } /** @@ -815,6 +802,24 @@ class EntryRestController extends WallabagRestController return $this->sendResponse($results); } + /** + * 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; + } + } + + return $newResults; + } + /** * Retrieve value from the request. * Used for POST & PATCH on a an entry. -- cgit v1.2.3 From 629a3797bcef33943df8ef5631328e05d12634ed Mon Sep 17 00:00:00 2001 From: Jeremy Benoist Date: Fri, 24 May 2019 15:43:30 +0200 Subject: Remove useless methods Also fix a phpdoc block --- src/Wallabag/ApiBundle/Controller/EntryRestController.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/Wallabag/ApiBundle') diff --git a/src/Wallabag/ApiBundle/Controller/EntryRestController.php b/src/Wallabag/ApiBundle/Controller/EntryRestController.php index 77eb489e..bdd02129 100644 --- a/src/Wallabag/ApiBundle/Controller/EntryRestController.php +++ b/src/Wallabag/ApiBundle/Controller/EntryRestController.php @@ -848,8 +848,8 @@ class EntryRestController extends WallabagRestController /** * Return information about the entry if it exist and depending on the id or not. * - * @param Entry|null $entry - * @param bool $returnId + * @param Entry|bool|null $entry + * @param bool $returnId * * @return bool|int */ -- cgit v1.2.3