From 4d307e18eb7b39c9f15544849b792377e2ff7474 Mon Sep 17 00:00:00 2001 From: Matthieu Gautier Date: Wed, 1 Jul 2020 16:39:35 +0200 Subject: [PATCH] Add new thread safe suggestion API. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous API were using an internal vector to store the suggestions search results. The new API takes a vector as out argument. So user can call the functions without having to protect the search. We should change the android API to reflect the change but it is a bit more complex to do at JNI level. As android do not call it multithreaded we are safe for now. And we need the new API asap for kiwix-desktop. So we keep the same API on android for now, the new api will be made in next version. --- include/reader.h | 63 ++++++++++++++++++++++++++++---- src/reader.cpp | 53 +++++++++++++++++++-------- src/server.cpp | 10 ++--- src/wrapper/java/kiwixreader.cpp | 6 +++ 4 files changed, 104 insertions(+), 28 deletions(-) diff --git a/include/reader.h b/include/reader.h index dd9f39d94..a8fb2a5f6 100644 --- a/include/reader.h +++ b/include/reader.h @@ -43,6 +43,8 @@ namespace kiwix * The Reader class is the class who allow to get an entry content from a zim * file. */ + +using SuggestionsList_t = std::vector>; class Reader { public: @@ -419,6 +421,10 @@ class Reader * * Suggestions are stored in an internal vector and can be retrieved using * `getNextSuggestion` method. + * This method is not thread safe and is deprecated. Use : + * bool searchSuggestions(const string& prefix, + * unsigned int suggestionsCount, + * SuggestionsList_t& results); * * @param prefix The prefix to search. * @param suggestionsCount How many suggestions to search for. @@ -426,12 +432,49 @@ class Reader * If false, add suggestions to the internal vector * (until internal vector size is suggestionCount (or no more * suggestion)) - * @return True if some suggestions where added to the internal vector. + * @return True if some suggestions have been added to the internal vector. */ - bool searchSuggestions(const string& prefix, + DEPRECATED bool searchSuggestions(const string& prefix, unsigned int suggestionsCount, const bool reset = true); + /** + * Search for entries with title starting with prefix (case sensitive). + * + * Suggestions are added to the `result` vector. + * + * @param prefix The prefix to search. + * @param suggestionsCount How many suggestions to search for. + * @param result The vector where to store the suggestions. + * @return True if some suggestions have been added to the vector. + */ + + bool searchSuggestions(const string& prefix, + unsigned int suggestionsCount, + SuggestionsList_t& resuls); + + /** + * Search for entries for the given prefix. + * + * If the zim file has a internal fulltext index, the suggestions will be + * searched using it. + * Else the suggestions will be search using `searchSuggestions` while trying + * to be smart about case sensitivity (using `getTitleVariants`). + * + * In any case, suggestions are stored in an internal vector and can be + * retrieved using `getNextSuggestion` method. + * The internal vector will be reset. + * This method is not thread safe and is deprecated. Use : + * bool searchSuggestionsSmart(const string& prefix, + * unsigned int suggestionsCount, + * SuggestionsList_t& results); + * + * @param prefix The prefix to search for. + * @param suggestionsCount How many suggestions to search for. + */ + DEPRECATED bool searchSuggestionsSmart(const string& prefix, + unsigned int suggestionsCount); + /** * Search for entries for the given prefix. * @@ -446,9 +489,13 @@ class Reader * * @param prefix The prefix to search for. * @param suggestionsCount How many suggestions to search for. + * @param results The vector where to store the suggestions + * @return True if some suggestions have been added to the results. */ - bool searchSuggestionsSmart(const string& prefix, - unsigned int suggestionsCount); + bool searchSuggestionsSmart(const string& prefix, + unsigned int suggestionsCount, + SuggestionsList_t& results); + /** * Check if the url exists in the zim file. @@ -490,7 +537,7 @@ class Reader * @param[out] title the title of the suggestion. * @return True if title has been set. */ - bool getNextSuggestion(string& title); + DEPRECATED bool getNextSuggestion(string& title); /** * Get the next suggestion title and url. @@ -499,7 +546,7 @@ class Reader * @param[out] url the url of the suggestion. * @return True if title and url have been set. */ - bool getNextSuggestion(string& title, string& url); + DEPRECATED bool getNextSuggestion(string& title, string& url); /** * Get if we can check zim file integrity (has a checksum). @@ -559,8 +606,8 @@ class Reader zim::size_type nsICount; std::string zimFilePath; - std::vector> suggestions; - std::vector>::iterator suggestionsOffset; + SuggestionsList_t suggestions; + SuggestionsList_t::iterator suggestionsOffset; private: std::map parseCounterMetadata() const; diff --git a/src/reader.cpp b/src/reader.cpp index 9cd187f9e..2188cca01 100644 --- a/src/reader.cpp +++ b/src/reader.cpp @@ -709,12 +709,11 @@ bool Reader::hasFulltextIndex() const } /* Search titles by prefix */ + bool Reader::searchSuggestions(const string& prefix, unsigned int suggestionsCount, const bool reset) { - bool retVal = false; - /* Reset the suggestions otherwise check if the suggestions number is less * than the suggestionsCount */ if (reset) { @@ -726,6 +725,21 @@ bool Reader::searchSuggestions(const string& prefix, } } + auto ret = searchSuggestions(prefix, suggestionsCount, this->suggestions); + + /* Set the cursor to the begining */ + this->suggestionsOffset = this->suggestions.begin(); + + return ret; +} + + +bool Reader::searchSuggestions(const string& prefix, + unsigned int suggestionsCount, + SuggestionsList_t& results) +{ + bool retVal = false; + /* Return if no prefix */ if (prefix.size() == 0) { return false; @@ -734,7 +748,7 @@ bool Reader::searchSuggestions(const string& prefix, for (auto articleItr = zimFileHandler->findByTitle('A', prefix); articleItr != zimFileHandler->end() && articleItr->getTitle().compare(0, prefix.size(), prefix) == 0 - && this->suggestions.size() < suggestionsCount; + && results.size() < suggestionsCount; ++articleItr) { /* Extract the interesting part of article title & url */ std::string normalizedArticleTitle @@ -754,8 +768,8 @@ bool Reader::searchSuggestions(const string& prefix, title) */ bool insert = true; std::vector>::iterator suggestionItr; - for (suggestionItr = this->suggestions.begin(); - suggestionItr != this->suggestions.end(); + for (suggestionItr = results.begin(); + suggestionItr != results.end(); suggestionItr++) { int result = normalizedArticleTitle.compare((*suggestionItr)[2]); if (result == 0 && articleFinalUrl.compare((*suggestionItr)[1]) == 0) { @@ -772,16 +786,13 @@ bool Reader::searchSuggestions(const string& prefix, suggestion.push_back(articleItr->getTitle()); suggestion.push_back(articleFinalUrl); suggestion.push_back(normalizedArticleTitle); - this->suggestions.insert(suggestionItr, suggestion); + results.insert(suggestionItr, suggestion); } /* Suggestions where found */ retVal = true; } - /* Set the cursor to the begining */ - this->suggestionsOffset = this->suggestions.begin(); - return retVal; } @@ -796,15 +807,28 @@ std::vector Reader::getTitleVariants( return variants; } -/* Try also a few variations of the prefix to have better results */ + bool Reader::searchSuggestionsSmart(const string& prefix, unsigned int suggestionsCount) +{ + this->suggestions.clear(); + this->suggestionsOffset = this->suggestions.begin(); + + auto ret = searchSuggestionsSmart(prefix, suggestionsCount, this->suggestions); + + this->suggestionsOffset = this->suggestions.begin(); + + return ret; +} + +/* Try also a few variations of the prefix to have better results */ +bool Reader::searchSuggestionsSmart(const string& prefix, + unsigned int suggestionsCount, + SuggestionsList_t& results) { std::vector variants = this->getTitleVariants(prefix); bool retVal = false; - this->suggestions.clear(); - this->suggestionsOffset = this->suggestions.begin(); /* Try to search in the title using fulltext search database */ const auto suggestionSearch = this->getZimFileHandler()->suggestions(prefix, 0, suggestionsCount); @@ -820,15 +844,14 @@ bool Reader::searchSuggestionsSmart(const string& prefix, suggestion.push_back(current->getTitle()); suggestion.push_back("/A/" + current->getUrl()); suggestion.push_back(kiwix::normalize(current->getTitle())); - this->suggestions.push_back(suggestion); + results.push_back(suggestion); } - this->suggestionsOffset = this->suggestions.begin(); retVal = true; } else { for (std::vector::iterator variantsItr = variants.begin(); variantsItr != variants.end(); variantsItr++) { - retVal = this->searchSuggestions(*variantsItr, suggestionsCount, false) + retVal = this->searchSuggestions(*variantsItr, suggestionsCount, results) || retVal; } } diff --git a/src/server.cpp b/src/server.cpp index 783f4a894..2547d2e11 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -543,7 +543,6 @@ Response InternalServer::handle_suggest(const RequestContext& request) std::string mimeType; unsigned int maxSuggestionCount = 10; unsigned int suggestionCount = 0; - std::string suggestion; std::string bookName; std::string bookId; @@ -567,11 +566,12 @@ Response InternalServer::handle_suggest(const RequestContext& request) bool first = true; if (reader != nullptr) { /* Get the suggestions */ - reader->searchSuggestionsSmart(term, maxSuggestionCount); - while (reader->getNextSuggestion(suggestion)) { + SuggestionsList_t suggestions; + reader->searchSuggestionsSmart(term, maxSuggestionCount, suggestions); + for(auto& suggestion:suggestions) { MustacheData result; - result.set("label", suggestion); - result.set("value", suggestion); + result.set("label", suggestion[0]); + result.set("value", suggestion[0]); result.set("first", first); first = false; results.push_back(result); diff --git a/src/wrapper/java/kiwixreader.cpp b/src/wrapper/java/kiwixreader.cpp index 978199ae1..36be34a44 100644 --- a/src/wrapper/java/kiwixreader.cpp +++ b/src/wrapper/java/kiwixreader.cpp @@ -355,9 +355,12 @@ Java_org_kiwix_kiwixlib_JNIKiwixReader_searchSuggestions(JNIEnv* env, unsigned int cCount = jni2c(count, env); try { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" if (READER->searchSuggestionsSmart(cPrefix, cCount)) { retVal = JNI_TRUE; } +#pragma GCC diagnostic pop } catch (std::exception& e) { LOG("Unable to get search results for pattern: %s", cPrefix.c_str()); LOG(e.what()); @@ -377,11 +380,14 @@ Java_org_kiwix_kiwixlib_JNIKiwixReader_getNextSuggestion(JNIEnv* env, std::string cUrl; try { +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" if (READER->getNextSuggestion(cTitle, cUrl)) { setStringObjValue(cTitle, titleObj, env); setStringObjValue(cUrl, urlObj, env); retVal = JNI_TRUE; } +#pragma GCC diagnostic pop } catch (std::exception& e) { LOG("Unable to get next suggestion"); LOG(e.what());