Build the bookSelection query string when we parse the query.

We have to reuse the query the user give us to generate the
pagination links.
At search result rendering step we don't have access to the query object.
The best place to know which arguments are used to select books
(and so which arguments to keep in the pagination links) is when we
parse the query to select books.

Fix tests (pagination links) with book selector other than "books.id="
(pattern=jazz&books.query.lang=eng)
This commit is contained in:
Matthieu Gautier 2022-05-30 23:41:30 +02:00
parent b483a8e4e4
commit b857293cfd
4 changed files with 27 additions and 22 deletions

View File

@ -81,9 +81,9 @@ class SearchRenderer
void setSearchPattern(const std::string& pattern); void setSearchPattern(const std::string& pattern);
/** /**
* Set the book names used to do the search. * Set the querystring used to select books
*/ */
void setSearchBookIds(const std::set<std::string>& bookIds); void setSearchBookQuery(const std::string& bookQuery);
/** /**
* Set protocol prefix. * Set protocol prefix.
@ -112,7 +112,7 @@ class SearchRenderer
zim::SearchResultSet m_srs; zim::SearchResultSet m_srs;
NameMapper* mp_nameMapper; NameMapper* mp_nameMapper;
Library* mp_library; Library* mp_library;
std::set<std::string> searchBookIds; std::string searchBookQuery;
std::string searchPattern; std::string searchPattern;
std::string protocolPrefix; std::string protocolPrefix;
std::string searchProtocolPrefix; std::string searchProtocolPrefix;

View File

@ -71,9 +71,9 @@ void SearchRenderer::setSearchPattern(const std::string& pattern)
searchPattern = pattern; searchPattern = pattern;
} }
void SearchRenderer::setSearchBookIds(const std::set<std::string>& bookIds) void SearchRenderer::setSearchBookQuery(const std::string& bookQuery)
{ {
searchBookIds = bookIds; searchBookQuery = bookQuery;
} }
void SearchRenderer::setProtocolPrefix(const std::string& prefix) void SearchRenderer::setProtocolPrefix(const std::string& prefix)
@ -90,15 +90,13 @@ kainjow::mustache::data buildQueryData
( (
const std::string& searchProtocolPrefix, const std::string& searchProtocolPrefix,
const std::string& pattern, const std::string& pattern,
const std::set<std::string>& bookIds const std::string& bookQuery
) { ) {
kainjow::mustache::data query; kainjow::mustache::data query;
query.set("pattern", kiwix::encodeDiples(pattern)); query.set("pattern", kiwix::encodeDiples(pattern));
std::ostringstream ss; std::ostringstream ss;
ss << searchProtocolPrefix << "?pattern=" << urlEncode(pattern, true); ss << searchProtocolPrefix << "?pattern=" << urlEncode(pattern, true);
for (auto& bookId: bookIds) { ss << "&" << bookQuery;
ss << "&books.id="<<urlEncode(bookId, true);
}
query.set("unpaginatedQuery", ss.str()); query.set("unpaginatedQuery", ss.str());
return query; return query;
} }
@ -199,7 +197,7 @@ std::string SearchRenderer::getHtml()
kainjow::mustache::data query = buildQueryData( kainjow::mustache::data query = buildQueryData(
searchProtocolPrefix, searchProtocolPrefix,
searchPattern, searchPattern,
searchBookIds searchBookQuery
); );
std::string template_str = RESOURCE::templates::search_result_html; std::string template_str = RESOURCE::templates::search_result_html;

View File

@ -214,13 +214,15 @@ void checkBookNumber(const Library::BookIdSet& bookIds, size_t limit) {
} // unnamed namespace } // unnamed namespace
Library::BookIdSet InternalServer::selectBooks(const RequestContext& request) const std::pair<std::string, Library::BookIdSet> InternalServer::selectBooks(const RequestContext& request) const
{ {
// Try old API // Try old API
try { try {
auto bookName = request.get_argument("content"); auto bookName = request.get_argument("content");
try { try {
return {mp_nameMapper->getIdForName(bookName)}; const auto bookIds = Library::BookIdSet{mp_nameMapper->getIdForName(bookName)};
const auto queryString = request.get_query([&](const std::string& key){return key == "content";});
return {queryString, bookIds};
} catch (const std::out_of_range&) { } catch (const std::out_of_range&) {
throw Error(noSuchBookErrorMsg(bookName)); throw Error(noSuchBookErrorMsg(bookName));
} }
@ -236,7 +238,8 @@ Library::BookIdSet InternalServer::selectBooks(const RequestContext& request) co
throw Error(noValueForArgMsg("books.id")); throw Error(noValueForArgMsg("books.id"));
} }
const auto bookIds = Library::BookIdSet(id_vec.begin(), id_vec.end()); const auto bookIds = Library::BookIdSet(id_vec.begin(), id_vec.end());
return bookIds; const auto queryString = request.get_query([&](const std::string& key){return key == "books.id";});
return {queryString, bookIds};
} catch(const std::out_of_range&) {} } catch(const std::out_of_range&) {}
// Use the names // Use the names
@ -253,7 +256,8 @@ Library::BookIdSet InternalServer::selectBooks(const RequestContext& request) co
throw Error(noSuchBookErrorMsg(bookName)); throw Error(noSuchBookErrorMsg(bookName));
} }
} }
return bookIds; const auto queryString = request.get_query([&](const std::string& key){return key == "books.name";});
return {queryString, bookIds};
} catch(const std::out_of_range&) {} } catch(const std::out_of_range&) {}
// Check for filtering // Check for filtering
@ -263,13 +267,14 @@ Library::BookIdSet InternalServer::selectBooks(const RequestContext& request) co
throw Error(nonParameterizedMessage("no-book-found")); throw Error(nonParameterizedMessage("no-book-found"));
} }
const auto bookIds = Library::BookIdSet(id_vec.begin(), id_vec.end()); const auto bookIds = Library::BookIdSet(id_vec.begin(), id_vec.end());
return bookIds; const auto queryString = request.get_query([&](const std::string& key){return startsWith(key, "books.filter.");});
return {queryString, bookIds};
} }
SearchInfo InternalServer::getSearchInfo(const RequestContext& request) const SearchInfo InternalServer::getSearchInfo(const RequestContext& request) const
{ {
auto bookIds = selectBooks(request); auto bookIds = selectBooks(request);
checkBookNumber(bookIds, m_multizimSearchLimit); checkBookNumber(bookIds.second, m_multizimSearchLimit);
auto pattern = request.get_optional_param<std::string>("pattern", ""); auto pattern = request.get_optional_param<std::string>("pattern", "");
GeoQuery geoQuery; GeoQuery geoQuery;
@ -286,13 +291,14 @@ SearchInfo InternalServer::getSearchInfo(const RequestContext& request) const
throw Error(nonParameterizedMessage("no-query")); throw Error(nonParameterizedMessage("no-query"));
} }
return SearchInfo(pattern, geoQuery, bookIds); return SearchInfo(pattern, geoQuery, bookIds.second, bookIds.first);
} }
SearchInfo::SearchInfo(const std::string& pattern, GeoQuery geoQuery, const Library::BookIdSet& bookIds) SearchInfo::SearchInfo(const std::string& pattern, GeoQuery geoQuery, const Library::BookIdSet& bookIds, const std::string& bookFilterQuery)
: pattern(pattern), : pattern(pattern),
geoQuery(geoQuery), geoQuery(geoQuery),
bookIds(bookIds) bookIds(bookIds),
bookFilterQuery(bookFilterQuery)
{} {}
zim::Query SearchInfo::getZimQuery(bool verbose) const { zim::Query SearchInfo::getZimQuery(bool verbose) const {
@ -751,7 +757,7 @@ std::unique_ptr<Response> InternalServer::handle_search(const RequestContext& re
SearchRenderer renderer(search->getResults(start, pageLength), mp_nameMapper, mp_library, start, SearchRenderer renderer(search->getResults(start, pageLength), mp_nameMapper, mp_library, start,
search->getEstimatedMatches()); search->getEstimatedMatches());
renderer.setSearchPattern(searchInfo.pattern); renderer.setSearchPattern(searchInfo.pattern);
renderer.setSearchBookIds(bookIds); renderer.setSearchBookQuery(searchInfo.bookFilterQuery);
renderer.setProtocolPrefix(m_root + "/"); renderer.setProtocolPrefix(m_root + "/");
renderer.setSearchProtocolPrefix(m_root + "/search"); renderer.setSearchProtocolPrefix(m_root + "/search");
renderer.setPageLength(pageLength); renderer.setPageLength(pageLength);

View File

@ -68,7 +68,7 @@ struct GeoQuery {
class SearchInfo { class SearchInfo {
public: public:
SearchInfo(const std::string& pattern, GeoQuery geoQuery, const Library::BookIdSet& bookIds); SearchInfo(const std::string& pattern, GeoQuery geoQuery, const Library::BookIdSet& bookIds, const std::string& bookFilterString);
zim::Query getZimQuery(bool verbose) const; zim::Query getZimQuery(bool verbose) const;
const Library::BookIdSet& getBookIds() const { return bookIds; } const Library::BookIdSet& getBookIds() const { return bookIds; }
@ -83,6 +83,7 @@ class SearchInfo {
std::string pattern; std::string pattern;
GeoQuery geoQuery; GeoQuery geoQuery;
Library::BookIdSet bookIds; Library::BookIdSet bookIds;
std::string bookFilterQuery;
}; };
@ -149,7 +150,7 @@ class InternalServer {
bool etag_not_needed(const RequestContext& r) const; bool etag_not_needed(const RequestContext& r) const;
ETag get_matching_if_none_match_etag(const RequestContext& request) const; ETag get_matching_if_none_match_etag(const RequestContext& request) const;
Library::BookIdSet selectBooks(const RequestContext& r) const; std::pair<std::string, Library::BookIdSet> selectBooks(const RequestContext& r) const;
SearchInfo getSearchInfo(const RequestContext& r) const; SearchInfo getSearchInfo(const RequestContext& r) const;
private: // data private: // data