From 9cff130fbb6325883e97ee5a11ebe42835de4234 Mon Sep 17 00:00:00 2001 From: Maneesh P M Date: Wed, 4 Aug 2021 17:24:30 +0530 Subject: [PATCH] Fix usage of zim::Searcher::getResults() in libkiwix The correct usage does not require the user to calculate an `end` using the `pageLength`. We can directly use getResults(start, pageLength) --- include/searcher.h | 14 +++++------ src/searcher.cpp | 22 ++++++++--------- src/server/internalServer.cpp | 4 +-- test/searcher.cpp | 46 +++++++++++++++++------------------ 4 files changed, 42 insertions(+), 44 deletions(-) diff --git a/include/searcher.h b/include/searcher.h index f9cdb4a4c..b4aa4fbf5 100644 --- a/include/searcher.h +++ b/include/searcher.h @@ -85,12 +85,12 @@ class Searcher * * @param search The search query. * @param resultStart the start offset of the search results (used for pagination). - * @param resultEnd the end offset of the search results (used for pagination). + * @param maxResultCount Maximum results to get from start (used for pagination). * @param verbose print some info on stdout if true. */ void search(const std::string& search, unsigned int resultStart, - unsigned int resultEnd, + unsigned int maxResultCount, const bool verbose = false); /** @@ -104,12 +104,12 @@ class Searcher * @param longitude The longitude of the center point. * @param distance The radius of the disc. * @param resultStart the start offset of the search results (used for pagination). - * @param resultEnd the end offset of the search results (used for pagination). + * @param maxResultCount Maximum number of results to get from start (used for pagination). * @param verbose print some info on stdout if true. */ void geo_search(float latitude, float longitude, float distance, unsigned int resultStart, - unsigned int resultEnd, + unsigned int maxResultCount, const bool verbose = false); /** @@ -148,14 +148,14 @@ class Searcher zim::SearchResultSet getSearchResultSet(); unsigned int getResultStart() { return resultStart; } - unsigned int getResultEnd() { return resultEnd; } + unsigned int getMaxResultCount() { return maxResultCount; } protected: std::string beautifyInteger(const unsigned int number); void closeIndex(); void searchInIndex(string& search, const unsigned int resultStart, - const unsigned int resultEnd, + const unsigned int maxResultCount, const bool verbose = false); std::vector readers; @@ -163,7 +163,7 @@ class Searcher std::string searchPattern; unsigned int estimatedResultCount; unsigned int resultStart; - unsigned int resultEnd; + unsigned int maxResultCount; private: void reset(); diff --git a/src/searcher.cpp b/src/searcher.cpp index 56ffb3d4e..1d0b492c6 100644 --- a/src/searcher.cpp +++ b/src/searcher.cpp @@ -67,7 +67,7 @@ Searcher::Searcher() : searchPattern(""), estimatedResultCount(0), resultStart(0), - resultEnd(0) + maxResultCount(0) { loadICUExternalTables(); } @@ -95,7 +95,7 @@ Reader* Searcher::get_reader(int readerIndex) /* Search strings in the database */ void Searcher::search(const std::string& search, unsigned int resultStart, - unsigned int resultEnd, + unsigned int maxResultCount, const bool verbose) { this->reset(); @@ -106,9 +106,9 @@ void Searcher::search(const std::string& search, this->searchPattern = search; this->resultStart = resultStart; - this->resultEnd = resultEnd; + this->maxResultCount = maxResultCount; /* Try to find results */ - if (resultStart != resultEnd) { + if (maxResultCount != 0) { /* Perform the search */ string unaccentedSearch = removeAccents(search); std::vector archives; @@ -123,7 +123,7 @@ void Searcher::search(const std::string& search, query.setQuery(unaccentedSearch, false); query.setVerbose(verbose); zim::Search search = searcher.search(query); - internal.reset(new SearcherInternal(search.getResults(resultStart, resultEnd))); + internal.reset(new SearcherInternal(search.getResults(resultStart, maxResultCount))); this->estimatedResultCount = search.getEstimatedMatches(); } @@ -133,7 +133,7 @@ void Searcher::search(const std::string& search, void Searcher::geo_search(float latitude, float longitude, float distance, unsigned int resultStart, - unsigned int resultEnd, + unsigned int maxResultCount, const bool verbose) { this->reset(); @@ -147,10 +147,10 @@ void Searcher::geo_search(float latitude, float longitude, float distance, oss << "Articles located less than " << distance << " meters of " << latitude << ";" << longitude; this->searchPattern = oss.str(); this->resultStart = resultStart; - this->resultEnd = resultEnd; + this->maxResultCount = maxResultCount; /* Try to find results */ - if (resultStart == resultEnd) { + if (maxResultCount == 0) { return; } @@ -165,7 +165,7 @@ void Searcher::geo_search(float latitude, float longitude, float distance, query.setQuery("", false); query.setGeorange(latitude, longitude, distance); zim::Search search = searcher.search(query); - internal.reset(new SearcherInternal(search.getResults(resultStart, resultEnd))); + internal.reset(new SearcherInternal(search.getResults(resultStart, maxResultCount))); this->estimatedResultCount = search.getEstimatedMatches(); } @@ -206,7 +206,7 @@ void Searcher::suggestions(std::string& searchPattern, const bool verbose) this->searchPattern = searchPattern; this->resultStart = 0; - this->resultEnd = 10; + this->maxResultCount = 10; string unaccentedSearch = removeAccents(searchPattern); std::vector archives; @@ -219,7 +219,7 @@ void Searcher::suggestions(std::string& searchPattern, const bool verbose) query.setVerbose(verbose); query.setQuery(unaccentedSearch, true); zim::Search search = searcher.search(query); - internal.reset(new SearcherInternal(search.getResults(resultStart, resultEnd))); + internal.reset(new SearcherInternal(search.getResults(resultStart, maxResultCount))); this->estimatedResultCount = search.getEstimatedMatches(); } diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 689ba7732..957b128f8 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -589,8 +589,6 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re pageLength = 25; } - auto end = start + pageLength; - /* Get the results */ try { zim::Query query; @@ -615,7 +613,7 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re } zim::Search search = searcher->search(query); - SearchRenderer renderer(search.getResults(start, end), mp_nameMapper, start, + SearchRenderer renderer(search.getResults(start, pageLength), mp_nameMapper, start, search.getEstimatedMatches()); renderer.setSearchPattern(patternString); renderer.setSearchContent(bookName); diff --git a/test/searcher.cpp b/test/searcher.cpp index 7722eaa4c..d89523b62 100644 --- a/test/searcher.cpp +++ b/test/searcher.cpp @@ -6,36 +6,36 @@ namespace kiwix { TEST(Searcher, search) { - Reader reader("./test/example.zim"); + Reader reader("./test/example.zim"); - Searcher searcher; - searcher.add_reader(&reader); - ASSERT_EQ(searcher.get_reader(0)->getTitle(), reader.getTitle()); + Searcher searcher; + searcher.add_reader(&reader); + ASSERT_EQ(searcher.get_reader(0)->getTitle(), reader.getTitle()); - searcher.search("wiki", 0, 2); - searcher.restart_search(); - ASSERT_EQ(searcher.getEstimatedResultCount(), (unsigned int)2); + searcher.search("wiki", 0, 2); + searcher.restart_search(); + ASSERT_EQ(searcher.getEstimatedResultCount(), (unsigned int)2); - auto result = searcher.getNextResult(); - ASSERT_EQ(result->get_title(), "FreedomBox for Communities/Offline Wikipedia - Wikibooks, open books for an open world"); - result = searcher.getNextResult(); - ASSERT_EQ(result->get_title(), "Wikibooks"); + auto result = searcher.getNextResult(); + ASSERT_EQ(result->get_title(), "FreedomBox for Communities/Offline Wikipedia - Wikibooks, open books for an open world"); + result = searcher.getNextResult(); + ASSERT_EQ(result->get_title(), "Wikibooks"); } TEST(Searcher, incrementalRange) { - // Attempt to get 50 results in steps of 5 - zim::Archive archive("./test/zimfile.zim"); - zim::Searcher ftsearcher(archive); - zim::Query query; - query.setQuery("ray", false); - auto search = ftsearcher.search(query); - - int suggCount = 0; - for (int i = 0; i < 10; i++) { - auto srs = search.getResults(i*5, (i + 1)*5); // get 5 results - ASSERT_EQ(srs.size(), 5); + // Attempt to get 50 results in steps of 5 + zim::Archive archive("./test/zimfile.zim"); + zim::Searcher ftsearcher(archive); + zim::Query query; + query.setQuery("ray", false); + auto search = ftsearcher.search(query); + + int suggCount = 0; + for (int i = 0; i < 10; i++) { + auto srs = search.getResults(i*5, 5); // get 5 results + ASSERT_EQ(srs.size(), 5); suggCount += srs.size(); - } + } ASSERT_EQ(suggCount, 50); }