From 9b8c0a4560fa1d87cab1529099b1b4677e92e265 Mon Sep 17 00:00:00 2001 From: ArthurHoaro Date: Wed, 20 Jan 2021 14:45:59 +0100 Subject: Handle pagination through BookmarkService Handle all search results through SearchResult object. This is a required step toward implementing a BookmarkService based on SQL database. Related to #953 --- application/api/controllers/Links.php | 38 +++--- application/api/controllers/Tags.php | 8 +- application/bookmark/BookmarkFileService.php | 32 +++-- application/bookmark/BookmarkServiceInterface.php | 8 +- application/bookmark/SearchResult.php | 136 +++++++++++++++++++++ application/feed/FeedBuilder.php | 25 ++-- .../front/controller/admin/ManageTagController.php | 11 +- .../controller/admin/ThumbnailsController.php | 2 +- .../controller/visitor/BookmarkListController.php | 63 ++++------ .../front/controller/visitor/DailyController.php | 2 +- .../controller/visitor/PictureWallController.php | 12 +- application/netscape/NetscapeBookmarkUtils.php | 2 +- application/updater/Updater.php | 2 +- 13 files changed, 234 insertions(+), 107 deletions(-) create mode 100644 application/bookmark/SearchResult.php (limited to 'application') diff --git a/application/api/controllers/Links.php b/application/api/controllers/Links.php index b83b2260..fe4bdc9f 100644 --- a/application/api/controllers/Links.php +++ b/application/api/controllers/Links.php @@ -36,13 +36,6 @@ class Links extends ApiController public function getLinks($request, $response) { $private = $request->getParam('visibility'); - $bookmarks = $this->bookmarkService->search( - [ - 'searchtags' => $request->getParam('searchtags', ''), - 'searchterm' => $request->getParam('searchterm', ''), - ], - $private - ); // Return bookmarks from the {offset}th link, starting from 0. $offset = $request->getParam('offset'); @@ -50,9 +43,6 @@ class Links extends ApiController throw new ApiBadParametersException('Invalid offset'); } $offset = ! empty($offset) ? intval($offset) : 0; - if ($offset > count($bookmarks)) { - return $response->withJson([], 200, $this->jsonStyle); - } // limit parameter is either a number of bookmarks or 'all' for everything. $limit = $request->getParam('limit'); @@ -61,23 +51,33 @@ class Links extends ApiController } elseif (ctype_digit($limit)) { $limit = intval($limit); } elseif ($limit === 'all') { - $limit = count($bookmarks); + $limit = null; } else { throw new ApiBadParametersException('Invalid limit'); } + $searchResult = $this->bookmarkService->search( + [ + 'searchtags' => $request->getParam('searchtags', ''), + 'searchterm' => $request->getParam('searchterm', ''), + ], + $private, + false, + false, + false, + [ + 'limit' => $limit, + 'offset' => $offset, + 'allowOutOfBounds' => true, + ] + ); + // 'environment' is set by Slim and encapsulate $_SERVER. $indexUrl = index_url($this->ci['environment']); $out = []; - $index = 0; - foreach ($bookmarks as $bookmark) { - if (count($out) >= $limit) { - break; - } - if ($index++ >= $offset) { - $out[] = ApiUtils::formatLink($bookmark, $indexUrl); - } + foreach ($searchResult->getBookmarks() as $bookmark) { + $out[] = ApiUtils::formatLink($bookmark, $indexUrl); } return $response->withJson($out, 200, $this->jsonStyle); diff --git a/application/api/controllers/Tags.php b/application/api/controllers/Tags.php index e60e00a7..5a23f6db 100644 --- a/application/api/controllers/Tags.php +++ b/application/api/controllers/Tags.php @@ -122,12 +122,12 @@ class Tags extends ApiController throw new ApiBadParametersException('New tag name is required in the request body'); } - $bookmarks = $this->bookmarkService->search( + $searchResult = $this->bookmarkService->search( ['searchtags' => $args['tagName']], BookmarkFilter::$ALL, true ); - foreach ($bookmarks as $bookmark) { + foreach ($searchResult->getBookmarks() as $bookmark) { $bookmark->renameTag($args['tagName'], $data['name']); $this->bookmarkService->set($bookmark, false); $this->history->updateLink($bookmark); @@ -157,12 +157,12 @@ class Tags extends ApiController throw new ApiTagNotFoundException(); } - $bookmarks = $this->bookmarkService->search( + $searchResult = $this->bookmarkService->search( ['searchtags' => $args['tagName']], BookmarkFilter::$ALL, true ); - foreach ($bookmarks as $bookmark) { + foreach ($searchResult->getBookmarks() as $bookmark) { $bookmark->deleteTag($args['tagName']); $this->bookmarkService->set($bookmark, false); $this->history->updateLink($bookmark); diff --git a/application/bookmark/BookmarkFileService.php b/application/bookmark/BookmarkFileService.php index 6666a251..8ea37427 100644 --- a/application/bookmark/BookmarkFileService.php +++ b/application/bookmark/BookmarkFileService.php @@ -55,8 +55,12 @@ class BookmarkFileService implements BookmarkServiceInterface /** * @inheritDoc */ - public function __construct(ConfigManager $conf, History $history, Mutex $mutex, bool $isLoggedIn) - { + public function __construct( + ConfigManager $conf, + History $history, + Mutex $mutex, + bool $isLoggedIn + ) { $this->conf = $conf; $this->history = $history; $this->mutex = $mutex; @@ -129,8 +133,9 @@ class BookmarkFileService implements BookmarkServiceInterface string $visibility = null, bool $caseSensitive = false, bool $untaggedOnly = false, - bool $ignoreSticky = false - ) { + bool $ignoreSticky = false, + array $pagination = [] + ): SearchResult { if ($visibility === null) { $visibility = $this->isLoggedIn ? BookmarkFilter::$ALL : BookmarkFilter::$PUBLIC; } @@ -143,13 +148,20 @@ class BookmarkFileService implements BookmarkServiceInterface $this->bookmarks->reorder('DESC', true); } - return $this->bookmarkFilter->filter( + $bookmarks = $this->bookmarkFilter->filter( BookmarkFilter::$FILTER_TAG | BookmarkFilter::$FILTER_TEXT, [$searchTags, $searchTerm], $caseSensitive, $visibility, $untaggedOnly ); + + return SearchResult::getSearchResult( + $bookmarks, + $pagination['offset'] ?? 0, + $pagination['limit'] ?? null, + $pagination['allowOutOfBounds'] ?? false + ); } /** @@ -282,7 +294,7 @@ class BookmarkFileService implements BookmarkServiceInterface */ public function count(string $visibility = null): int { - return count($this->search([], $visibility)); + return $this->search([], $visibility)->getResultCount(); } /** @@ -305,10 +317,10 @@ class BookmarkFileService implements BookmarkServiceInterface */ public function bookmarksCountPerTag(array $filteringTags = [], string $visibility = null): array { - $bookmarks = $this->search(['searchtags' => $filteringTags], $visibility); + $searchResult = $this->search(['searchtags' => $filteringTags], $visibility); $tags = []; $caseMapping = []; - foreach ($bookmarks as $bookmark) { + foreach ($searchResult->getBookmarks() as $bookmark) { foreach ($bookmark->getTags() as $tag) { if ( empty($tag) @@ -357,7 +369,7 @@ class BookmarkFileService implements BookmarkServiceInterface $previous = null; $next = null; - foreach ($this->search([], null, false, false, true) as $bookmark) { + foreach ($this->search([], null, false, false, true)->getBookmarks() as $bookmark) { if ($to < $bookmark->getCreated()) { $next = $bookmark->getCreated(); } elseif ($from < $bookmark->getCreated() && $to > $bookmark->getCreated()) { @@ -378,7 +390,7 @@ class BookmarkFileService implements BookmarkServiceInterface */ public function getLatest(): ?Bookmark { - foreach ($this->search([], null, false, false, true) as $bookmark) { + foreach ($this->search([], null, false, false, true)->getBookmarks() as $bookmark) { return $bookmark; } diff --git a/application/bookmark/BookmarkServiceInterface.php b/application/bookmark/BookmarkServiceInterface.php index 08cdbb4e..4b1f0daa 100644 --- a/application/bookmark/BookmarkServiceInterface.php +++ b/application/bookmark/BookmarkServiceInterface.php @@ -44,16 +44,18 @@ interface BookmarkServiceInterface * @param bool $caseSensitive * @param bool $untaggedOnly * @param bool $ignoreSticky + * @param array $pagination This array can contain the following keys for pagination: limit, offset. * - * @return Bookmark[] + * @return SearchResult */ public function search( array $request = [], string $visibility = null, bool $caseSensitive = false, bool $untaggedOnly = false, - bool $ignoreSticky = false - ); + bool $ignoreSticky = false, + array $pagination = [] + ): SearchResult; /** * Get a single bookmark by its ID. diff --git a/application/bookmark/SearchResult.php b/application/bookmark/SearchResult.php new file mode 100644 index 00000000..c0bce311 --- /dev/null +++ b/application/bookmark/SearchResult.php @@ -0,0 +1,136 @@ +bookmarks = $bookmarks; + $this->resultCount = count($bookmarks); + $this->totalCount = $totalCount; + $this->limit = $limit; + $this->offset = $offset; + } + + /** + * Build a SearchResult from provided full result set and pagination settings. + * + * @param Bookmark[] $bookmarks Full set of result which will be filtered + * @param int $offset Start recording results from $offset + * @param int|null $limit End recording results after $limit bookmarks is reached + * @param bool $allowOutOfBounds Set to false to display the last page if the offset is out of bound, + * return empty result set otherwise (default: false) + * + * @return SearchResult + */ + public static function getSearchResult( + $bookmarks, + int $offset = 0, + ?int $limit = null, + bool $allowOutOfBounds = false + ): self { + $totalCount = count($bookmarks); + if (!$allowOutOfBounds && $offset > $totalCount) { + $offset = $limit === null ? 0 : $limit * -1; + } + + if ($bookmarks instanceof BookmarkArray) { + $buffer = []; + foreach ($bookmarks as $key => $value) { + $buffer[$key] = $value; + } + $bookmarks = $buffer; + } + + return new static( + array_slice($bookmarks, $offset, $limit, true), + $totalCount, + $offset, + $limit + ); + } + + /** @return Bookmark[] List of result bookmarks with pagination applied */ + public function getBookmarks(): array + { + return $this->bookmarks; + } + + /** @return int number of Bookmarks found, with pagination applied */ + public function getResultCount(): int + { + return $this->resultCount; + } + + /** @return int total number of result found */ + public function getTotalCount(): int + { + return $this->totalCount; + } + + /** @return int pagination: limit number of result bookmarks */ + public function getLimit(): ?int + { + return $this->limit; + } + + /** @return int pagination: offset to apply to complete result list */ + public function getOffset(): int + { + return $this->offset; + } + + /** @return int Current page of result set in complete results */ + public function getPage(): int + { + if (empty($this->limit)) { + return $this->offset === 0 ? 1 : 2; + } + $base = $this->offset >= 0 ? $this->offset : $this->totalCount + $this->offset; + + return (int) ceil($base / $this->limit) + 1; + } + + /** @return int Get the # of the last page */ + public function getLastPage(): int + { + if (empty($this->limit)) { + return $this->offset === 0 ? 1 : 2; + } + + return (int) ceil($this->totalCount / $this->limit); + } + + /** @return bool Either the current page is the last one or not */ + public function isLastPage(): bool + { + return $this->getPage() === $this->getLastPage(); + } + + /** @return bool Either the current page is the first one or not */ + public function isFirstPage(): bool + { + return $this->offset === 0; + } +} diff --git a/application/feed/FeedBuilder.php b/application/feed/FeedBuilder.php index ed62af26..d5d74fd1 100644 --- a/application/feed/FeedBuilder.php +++ b/application/feed/FeedBuilder.php @@ -102,22 +102,16 @@ class FeedBuilder $userInput['searchtags'] = false; } - // Optionally filter the results: - $linksToDisplay = $this->linkDB->search($userInput ?? [], null, false, false, true); - - $nblinksToDisplay = $this->getNbLinks(count($linksToDisplay), $userInput); + $limit = $this->getLimit($userInput); - // Can't use array_keys() because $link is a LinkDB instance and not a real array. - $keys = []; - foreach ($linksToDisplay as $key => $value) { - $keys[] = $key; - } + // Optionally filter the results: + $searchResult = $this->linkDB->search($userInput ?? [], null, false, false, true, ['limit' => $limit]); $pageaddr = escape(index_url($this->serverInfo)); $this->formatter->addContextData('index_url', $pageaddr); - $linkDisplayed = []; - for ($i = 0; $i < $nblinksToDisplay && $i < count($keys); $i++) { - $linkDisplayed[$keys[$i]] = $this->buildItem($feedType, $linksToDisplay[$keys[$i]], $pageaddr); + $links = []; + foreach ($searchResult->getBookmarks() as $key => $bookmark) { + $links[$key] = $this->buildItem($feedType, $bookmark, $pageaddr); } $data['language'] = $this->getTypeLanguage($feedType); @@ -128,7 +122,7 @@ class FeedBuilder $data['self_link'] = $pageaddr . $requestUri; $data['index_url'] = $pageaddr; $data['usepermalinks'] = $this->usePermalinks === true; - $data['links'] = $linkDisplayed; + $data['links'] = $links; return $data; } @@ -268,19 +262,18 @@ class FeedBuilder * If 'nb' not set or invalid, default value: $DEFAULT_NB_LINKS. * If 'nb' is set to 'all', display all filtered bookmarks (max parameter). * - * @param int $max maximum number of bookmarks to display. * @param array $userInput $_GET. * * @return int number of bookmarks to display. */ - protected function getNbLinks($max, ?array $userInput) + protected function getLimit(?array $userInput) { if (empty($userInput['nb'])) { return self::$DEFAULT_NB_LINKS; } if ($userInput['nb'] == 'all') { - return $max; + return null; } $intNb = intval($userInput['nb']); diff --git a/application/front/controller/admin/ManageTagController.php b/application/front/controller/admin/ManageTagController.php index 8675a0c5..1333cce7 100644 --- a/application/front/controller/admin/ManageTagController.php +++ b/application/front/controller/admin/ManageTagController.php @@ -57,9 +57,12 @@ class ManageTagController extends ShaarliAdminController } // TODO: move this to bookmark service - $count = 0; - $bookmarks = $this->container->bookmarkService->search(['searchtags' => $fromTag], BookmarkFilter::$ALL, true); - foreach ($bookmarks as $bookmark) { + $searchResult = $this->container->bookmarkService->search( + ['searchtags' => $fromTag], + BookmarkFilter::$ALL, + true + ); + foreach ($searchResult->getBookmarks() as $bookmark) { if (false === $isDelete) { $bookmark->renameTag($fromTag, $toTag); } else { @@ -68,11 +71,11 @@ class ManageTagController extends ShaarliAdminController $this->container->bookmarkService->set($bookmark, false); $this->container->history->updateLink($bookmark); - $count++; } $this->container->bookmarkService->save(); + $count = $searchResult->getResultCount(); if (true === $isDelete) { $alert = sprintf( t('The tag was removed from %d bookmark.', 'The tag was removed from %d bookmarks.', $count), diff --git a/application/front/controller/admin/ThumbnailsController.php b/application/front/controller/admin/ThumbnailsController.php index 94d97d4b..5dfea096 100644 --- a/application/front/controller/admin/ThumbnailsController.php +++ b/application/front/controller/admin/ThumbnailsController.php @@ -22,7 +22,7 @@ class ThumbnailsController extends ShaarliAdminController public function index(Request $request, Response $response): Response { $ids = []; - foreach ($this->container->bookmarkService->search() as $bookmark) { + foreach ($this->container->bookmarkService->search()->getBookmarks() as $bookmark) { // A note or not HTTP(S) if ($bookmark->isNote() || !startsWith(strtolower($bookmark->getUrl()), 'http')) { continue; diff --git a/application/front/controller/visitor/BookmarkListController.php b/application/front/controller/visitor/BookmarkListController.php index fe8231be..321ca813 100644 --- a/application/front/controller/visitor/BookmarkListController.php +++ b/application/front/controller/visitor/BookmarkListController.php @@ -36,7 +36,6 @@ class BookmarkListController extends ShaarliVisitorController $searchTags = normalize_spaces($request->getParam('searchtags') ?? ''); $searchTerm = escape(normalize_spaces($request->getParam('searchterm') ?? '')); - ; // Filter bookmarks according search parameters. $visibility = $this->container->sessionManager->getSessionParameter('visibility'); @@ -44,39 +43,26 @@ class BookmarkListController extends ShaarliVisitorController 'searchtags' => $searchTags, 'searchterm' => $searchTerm, ]; - $linksToDisplay = $this->container->bookmarkService->search( - $search, - $visibility, - false, - !!$this->container->sessionManager->getSessionParameter('untaggedonly') - ) ?? []; - - // ---- Handle paging. - $keys = []; - foreach ($linksToDisplay as $key => $value) { - $keys[] = $key; - } - - $linksPerPage = $this->container->sessionManager->getSessionParameter('LINKS_PER_PAGE', 20) ?: 20; // Select articles according to paging. - $pageCount = (int) ceil(count($keys) / $linksPerPage) ?: 1; - $page = (int) $request->getParam('page') ?? 1; + $page = (int) ($request->getParam('page') ?? 1); $page = $page < 1 ? 1 : $page; - $page = $page > $pageCount ? $pageCount : $page; + $linksPerPage = $this->container->sessionManager->getSessionParameter('LINKS_PER_PAGE', 20) ?: 20; - // Start index. - $i = ($page - 1) * $linksPerPage; - $end = $i + $linksPerPage; + $searchResult = $this->container->bookmarkService->search( + $search, + $visibility, + false, + !!$this->container->sessionManager->getSessionParameter('untaggedonly'), + false, + ['offset' => $linksPerPage * ($page - 1), 'limit' => $linksPerPage] + ) ?? []; - $linkDisp = []; $save = false; - while ($i < $end && $i < count($keys)) { - $save = $this->updateThumbnail($linksToDisplay[$keys[$i]], false) || $save; - $link = $formatter->format($linksToDisplay[$keys[$i]]); - - $linkDisp[$keys[$i]] = $link; - $i++; + $links = []; + foreach ($searchResult->getBookmarks() as $key => $bookmark) { + $save = $this->updateThumbnail($bookmark, false) || $save; + $links[$key] = $formatter->format($bookmark); } if ($save) { @@ -86,15 +72,10 @@ class BookmarkListController extends ShaarliVisitorController // Compute paging navigation $searchtagsUrl = $searchTags === '' ? '' : '&searchtags=' . urlencode($searchTags); $searchtermUrl = $searchTerm === '' ? '' : '&searchterm=' . urlencode($searchTerm); + $page = $searchResult->getPage(); - $previous_page_url = ''; - if ($i !== count($keys)) { - $previous_page_url = '?page=' . ($page + 1) . $searchtermUrl . $searchtagsUrl; - } - $next_page_url = ''; - if ($page > 1) { - $next_page_url = '?page=' . ($page - 1) . $searchtermUrl . $searchtagsUrl; - } + $previousPageUrl = !$searchResult->isLastPage() ? '?page=' . ($page + 1) . $searchtermUrl . $searchtagsUrl : ''; + $nextPageUrl = !$searchResult->isFirstPage() ? '?page=' . ($page - 1) . $searchtermUrl . $searchtagsUrl : ''; $tagsSeparator = $this->container->conf->get('general.tags_separator', ' '); $searchTagsUrlEncoded = array_map('urlencode', tags_str2array($searchTags, $tagsSeparator)); @@ -104,16 +85,16 @@ class BookmarkListController extends ShaarliVisitorController $data = array_merge( $this->initializeTemplateVars(), [ - 'previous_page_url' => $previous_page_url, - 'next_page_url' => $next_page_url, + 'previous_page_url' => $previousPageUrl, + 'next_page_url' => $nextPageUrl, 'page_current' => $page, - 'page_max' => $pageCount, - 'result_count' => count($linksToDisplay), + 'page_max' => $searchResult->getLastPage(), + 'result_count' => $searchResult->getTotalCount(), 'search_term' => escape($searchTerm), 'search_tags' => escape($searchTags), 'search_tags_url' => $searchTagsUrlEncoded, 'visibility' => $visibility, - 'links' => $linkDisp, + 'links' => $links, ] ); diff --git a/application/front/controller/visitor/DailyController.php b/application/front/controller/visitor/DailyController.php index 29492a5f..3739ec16 100644 --- a/application/front/controller/visitor/DailyController.php +++ b/application/front/controller/visitor/DailyController.php @@ -100,7 +100,7 @@ class DailyController extends ShaarliVisitorController $days = []; $format = DailyPageHelper::getFormatByType($type); $length = DailyPageHelper::getRssLengthByType($type); - foreach ($this->container->bookmarkService->search() as $bookmark) { + foreach ($this->container->bookmarkService->search()->getBookmarks() as $bookmark) { $day = $bookmark->getCreated()->format($format); // Stop iterating after DAILY_RSS_NB_DAYS entries diff --git a/application/front/controller/visitor/PictureWallController.php b/application/front/controller/visitor/PictureWallController.php index 23553ee6..9c8f07d7 100644 --- a/application/front/controller/visitor/PictureWallController.php +++ b/application/front/controller/visitor/PictureWallController.php @@ -30,19 +30,19 @@ class PictureWallController extends ShaarliVisitorController ); // Optionally filter the results: - $links = $this->container->bookmarkService->search($request->getQueryParams()); - $linksToDisplay = []; + $bookmarks = $this->container->bookmarkService->search($request->getQueryParams())->getBookmarks(); + $links = []; // Get only bookmarks which have a thumbnail. // Note: we do not retrieve thumbnails here, the request is too heavy. $formatter = $this->container->formatterFactory->getFormatter('raw'); - foreach ($links as $key => $link) { - if (!empty($link->getThumbnail())) { - $linksToDisplay[] = $formatter->format($link); + foreach ($bookmarks as $key => $bookmark) { + if (!empty($bookmark->getThumbnail())) { + $links[] = $formatter->format($bookmark); } } - $data = ['linksToDisplay' => $linksToDisplay]; + $data = ['linksToDisplay' => $links]; $this->executePageHooks('render_picwall', $data, TemplatePage::PICTURE_WALL); foreach ($data as $key => $value) { diff --git a/application/netscape/NetscapeBookmarkUtils.php b/application/netscape/NetscapeBookmarkUtils.php index 2d97b4c8..20715bd0 100644 --- a/application/netscape/NetscapeBookmarkUtils.php +++ b/application/netscape/NetscapeBookmarkUtils.php @@ -64,7 +64,7 @@ class NetscapeBookmarkUtils } $bookmarkLinks = []; - foreach ($this->bookmarkService->search([], $selection) as $bookmark) { + foreach ($this->bookmarkService->search([], $selection)->getBookmarks() as $bookmark) { $link = $formatter->format($bookmark); $link['taglist'] = implode(',', $bookmark->getTags()); if ($bookmark->isNote() && $prependNoteUrl) { diff --git a/application/updater/Updater.php b/application/updater/Updater.php index 4f557d0f..11b6c051 100644 --- a/application/updater/Updater.php +++ b/application/updater/Updater.php @@ -152,7 +152,7 @@ class Updater { $updated = false; - foreach ($this->bookmarkService->search() as $bookmark) { + foreach ($this->bookmarkService->search()->getBookmarks() as $bookmark) { if ( $bookmark->isNote() && startsWith($bookmark->getUrl(), '?') -- cgit v1.2.3