diff --git a/include/library.h b/include/library.h index dc4463f62..1763de7d5 100644 --- a/include/library.h +++ b/include/library.h @@ -140,51 +140,16 @@ private: // functions bool accept(const Book& book) const; }; -/** - * This class is not part of the libkiwix API. Its only purpose is - * to simplify the implementation of the Library's move operations - * and avoid bugs should new data members be added to Library. - */ -class LibraryBase -{ -protected: // types - typedef uint64_t LibraryRevision; - - struct Entry : Book - { - LibraryRevision lastUpdatedRevision = 0; - - // May also keep the Archive and Reader pointers here and get - // rid of the m_readers and m_archives data members in Library - }; - -protected: // data - LibraryRevision m_revision; - std::map m_books; - std::map> m_readers; - std::map> m_archives; - std::vector m_bookmarks; - class BookDB; - std::unique_ptr m_bookDB; - -protected: // functions - LibraryBase(); - ~LibraryBase(); - - LibraryBase(LibraryBase&& ); - LibraryBase& operator=(LibraryBase&& ); -}; - /** * A Library store several books. */ -class Library : private LibraryBase +class Library { // all data fields must be added in LibraryBase mutable std::mutex m_mutex; public: - typedef LibraryRevision Revision; + typedef uint64_t Revision; typedef std::vector BookIdCollection; typedef std::map AttributeCounts; @@ -351,7 +316,7 @@ class Library : private LibraryBase * * @return Current revision of the library. */ - LibraryRevision getRevision() const; + Revision getRevision() const; /** * Remove books that have not been updated since the specified revision. @@ -359,13 +324,14 @@ class Library : private LibraryBase * @param rev the library revision to use * @return Count of books that were removed by this operation. */ - uint32_t removeBooksNotUpdatedSince(LibraryRevision rev); + uint32_t removeBooksNotUpdatedSince(Revision rev); friend class OPDSDumper; friend class libXMLDumper; private: // types typedef const std::string& (Book::*BookStrPropMemFn)() const; + struct Impl; private: // functions AttributeCounts getBookAttributeCounts(BookStrPropMemFn p) const; @@ -373,6 +339,9 @@ private: // functions BookIdCollection filterViaBookDB(const Filter& filter) const; void updateBookDB(const Book& book); void dropReader(const std::string& bookId); + +private: //data + std::unique_ptr mp_impl; }; } diff --git a/include/search_renderer.h b/include/search_renderer.h index 627699a69..cd499d13d 100644 --- a/include/search_renderer.h +++ b/include/search_renderer.h @@ -75,6 +75,9 @@ class SearchRenderer ~SearchRenderer(); + /** + * Set the search pattern used to do the search + */ void setSearchPattern(const std::string& pattern); /** diff --git a/src/library.cpp b/src/library.cpp index bf62c4183..f77059404 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -58,66 +58,98 @@ bool booksReferToTheSameArchive(const Book& book1, const Book& book2) } // unnamed namespace -class LibraryBase::BookDB : public Xapian::WritableDatabase +struct Library::Impl { -public: - BookDB() : Xapian::WritableDatabase("", Xapian::DB_BACKEND_INMEMORY) {} + struct Entry : Book + { + Library::Revision lastUpdatedRevision = 0; + + // May also keep the Archive and Reader pointers here and get + // rid of the m_readers and m_archives data members in Library + }; + + Library::Revision m_revision; + std::map m_books; + std::map> m_readers; + std::map> m_archives; + std::vector m_bookmarks; + Xapian::WritableDatabase m_bookDB; + + unsigned int getBookCount(const bool localBooks, const bool remoteBooks) const; + + Impl(); + ~Impl(); + + Impl(Impl&& ); + Impl& operator=(Impl&& ); }; -LibraryBase::LibraryBase() - : m_bookDB(new BookDB) +Library::Impl::Impl() + : m_bookDB("", Xapian::DB_BACKEND_INMEMORY) { } -LibraryBase::~LibraryBase() +Library::Impl::~Impl() { } -LibraryBase::LibraryBase(LibraryBase&& ) = default; -LibraryBase& LibraryBase::operator=(LibraryBase&& ) = default; +Library::Impl::Impl(Library::Impl&& ) = default; +Library::Impl& Library::Impl::operator=(Library::Impl&& ) = default; + +unsigned int +Library::Impl::getBookCount(const bool localBooks, const bool remoteBooks) const +{ + unsigned int result = 0; + for (auto& pair: m_books) { + auto& book = pair.second; + if ((!book.getPath().empty() && localBooks) + || (!book.getUrl().empty() && remoteBooks)) { + result++; + } + } + return result; +} /* Constructor */ Library::Library() + : mp_impl(new Library::Impl) { } Library::Library(Library&& other) - : LibraryBase(std::move(other)) + : mp_impl(std::move(other.mp_impl)) { } Library& Library::operator=(Library&& other) { - LibraryBase::operator=(std::move(other)); + mp_impl = std::move(other.mp_impl); return *this; } /* Destructor */ -Library::~Library() -{ -} - +Library::~Library() = default; bool Library::addBook(const Book& book) { std::lock_guard lock(m_mutex); - ++m_revision; + ++mp_impl->m_revision; /* Try to find it */ updateBookDB(book); try { - auto& oldbook = m_books.at(book.getId()); + auto& oldbook = mp_impl->m_books.at(book.getId()); if ( ! booksReferToTheSameArchive(oldbook, book) ) { dropReader(book.getId()); } oldbook.update(book); // XXX: This may have no effect if oldbook is readonly // XXX: Then m_bookDB will become out-of-sync with // XXX: the real contents of the library. - oldbook.lastUpdatedRevision = m_revision; + oldbook.lastUpdatedRevision = mp_impl->m_revision; return false; } catch (std::out_of_range&) { - Entry& newEntry = m_books[book.getId()]; + auto& newEntry = mp_impl->m_books[book.getId()]; static_cast(newEntry) = book; - newEntry.lastUpdatedRevision = m_revision; + newEntry.lastUpdatedRevision = mp_impl->m_revision; return true; } } @@ -125,15 +157,15 @@ bool Library::addBook(const Book& book) void Library::addBookmark(const Bookmark& bookmark) { std::lock_guard lock(m_mutex); - m_bookmarks.push_back(bookmark); + mp_impl->m_bookmarks.push_back(bookmark); } bool Library::removeBookmark(const std::string& zimId, const std::string& url) { std::lock_guard lock(m_mutex); - for(auto it=m_bookmarks.begin(); it!=m_bookmarks.end(); it++) { + for(auto it=mp_impl->m_bookmarks.begin(); it!=mp_impl->m_bookmarks.end(); it++) { if (it->getBookId() == zimId && it->getUrl() == url) { - m_bookmarks.erase(it); + mp_impl->m_bookmarks.erase(it); return true; } } @@ -143,30 +175,30 @@ bool Library::removeBookmark(const std::string& zimId, const std::string& url) void Library::dropReader(const std::string& id) { - m_readers.erase(id); - m_archives.erase(id); + mp_impl->m_readers.erase(id); + mp_impl->m_archives.erase(id); } bool Library::removeBookById(const std::string& id) { std::lock_guard lock(m_mutex); - m_bookDB->delete_document("Q" + id); + mp_impl->m_bookDB.delete_document("Q" + id); dropReader(id); - return m_books.erase(id) == 1; + return mp_impl->m_books.erase(id) == 1; } Library::Revision Library::getRevision() const { std::lock_guard lock(m_mutex); - return m_revision; + return mp_impl->m_revision; } -uint32_t Library::removeBooksNotUpdatedSince(LibraryRevision libraryRevision) +uint32_t Library::removeBooksNotUpdatedSince(Revision libraryRevision) { BookIdCollection booksToRemove; { std::lock_guard lock(m_mutex); - for ( const auto& entry : m_books) { + for ( const auto& entry : mp_impl->m_books) { if ( entry.second.lastUpdatedRevision <= libraryRevision ) { booksToRemove.push_back(entry.first); } @@ -185,7 +217,7 @@ const Book& Library::getBookById(const std::string& id) const { // XXX: Doesn't make sense to lock this operation since it cannot // XXX: guarantee thread-safety because of its return type - return m_books.at(id); + return mp_impl->m_books.at(id); } Book Library::getBookByIdThreadSafe(const std::string& id) const @@ -198,7 +230,7 @@ const Book& Library::getBookByPath(const std::string& path) const { // XXX: Doesn't make sense to lock this operation since it cannot // XXX: guarantee thread-safety because of its return type - for(auto& it: m_books) { + for(auto& it: mp_impl->m_books) { auto& book = it.second; if (book.getPath() == path) return book; @@ -212,7 +244,7 @@ std::shared_ptr Library::getReaderById(const std::string& id) { try { std::lock_guard lock(m_mutex); - return m_readers.at(id); + return mp_impl->m_readers.at(id); } catch (std::out_of_range& e) {} const auto archive = getArchiveById(id); @@ -221,7 +253,7 @@ std::shared_ptr Library::getReaderById(const std::string& id) const shared_ptr reader(new Reader(archive, true)); std::lock_guard lock(m_mutex); - m_readers[id] = reader; + mp_impl->m_readers[id] = reader; return reader; } @@ -229,7 +261,7 @@ std::shared_ptr Library::getArchiveById(const std::string& id) { std::lock_guard lock(m_mutex); try { - return m_archives.at(id); + return mp_impl->m_archives.at(id); } catch (std::out_of_range& e) {} auto book = getBookById(id); @@ -237,7 +269,7 @@ std::shared_ptr Library::getArchiveById(const std::string& id) return nullptr; auto sptr = make_shared(book.getPath()); - m_archives[id] = sptr; + mp_impl->m_archives[id] = sptr; return sptr; } @@ -245,15 +277,7 @@ unsigned int Library::getBookCount(const bool localBooks, const bool remoteBooks) const { std::lock_guard lock(m_mutex); - unsigned int result = 0; - for (auto& pair: m_books) { - auto& book = pair.second; - if ((!book.getPath().empty() && localBooks) - || (book.getPath().empty() && remoteBooks)) { - result++; - } - } - return result; + return mp_impl->getBookCount(localBooks, remoteBooks); } bool Library::writeToFile(const std::string& path) const @@ -284,7 +308,7 @@ Library::AttributeCounts Library::getBookAttributeCounts(BookStrPropMemFn p) con std::lock_guard lock(m_mutex); AttributeCounts propValueCounts; - for (const auto& pair: m_books) { + for (const auto& pair: mp_impl->m_books) { const auto& book = pair.second; if (book.getOrigId().empty()) { propValueCounts[(book.*p)()] += 1; @@ -317,7 +341,7 @@ std::vector Library::getBooksCategories() const std::lock_guard lock(m_mutex); std::set categories; - for (const auto& pair: m_books) { + for (const auto& pair: mp_impl->m_books) { const auto& book = pair.second; const auto& c = book.getCategory(); if ( !c.empty() ) { @@ -341,12 +365,12 @@ std::vector Library::getBooksPublishers() const const std::vector Library::getBookmarks(bool onlyValidBookmarks) const { if (!onlyValidBookmarks) { - return m_bookmarks; + return mp_impl->m_bookmarks; } std::vector validBookmarks; auto booksId = getBooksIds(); std::lock_guard lock(m_mutex); - for(auto& bookmark:m_bookmarks) { + for(auto& bookmark:mp_impl->m_bookmarks) { if (std::find(booksId.begin(), booksId.end(), bookmark.getBookId()) != booksId.end()) { validBookmarks.push_back(bookmark); } @@ -359,7 +383,7 @@ Library::BookIdCollection Library::getBooksIds() const std::lock_guard lock(m_mutex); BookIdCollection bookIds; - for (auto& pair: m_books) { + for (auto& pair: mp_impl->m_books) { bookIds.push_back(pair.first); } @@ -405,7 +429,7 @@ void Library::updateBookDB(const Book& book) doc.set_data(book.getId()); - m_bookDB->replace_document(idterm, doc); + mp_impl->m_bookDB.replace_document(idterm, doc); } namespace @@ -538,9 +562,9 @@ Library::BookIdCollection Library::filterViaBookDB(const Filter& filter) const BookIdCollection bookIds; std::lock_guard lock(m_mutex); - Xapian::Enquire enquire(*m_bookDB); + Xapian::Enquire enquire(mp_impl->m_bookDB); enquire.set_query(query); - const auto results = enquire.get_mset(0, m_books.size()); + const auto results = enquire.get_mset(0, mp_impl->m_books.size()); for ( auto it = results.begin(); it != results.end(); ++it ) { bookIds.push_back(it.get_document().get_data()); } @@ -554,7 +578,7 @@ Library::BookIdCollection Library::filter(const Filter& filter) const const auto preliminaryResult = filterViaBookDB(filter); std::lock_guard lock(m_mutex); for(auto id : preliminaryResult) { - if(filter.accept(m_books.at(id))) { + if(filter.accept(mp_impl->m_books.at(id))) { result.push_back(id); } } diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index f40b45e77..8f9685563 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -58,7 +58,7 @@ SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, Lib mp_nameMapper(mapper), mp_library(library), protocolPrefix("zim://"), - searchProtocolPrefix("search://?"), + searchProtocolPrefix("search://"), estimatedResultCount(estimatedResultCount), resultStart(start) {} @@ -68,12 +68,12 @@ SearchRenderer::~SearchRenderer() = default; void SearchRenderer::setSearchPattern(const std::string& pattern) { - this->searchPattern = pattern; + searchPattern = pattern; } -void SearchRenderer::setSearchContent(const std::string& name) +void SearchRenderer::setSearchContent(const std::string& content) { - this->searchContent = name; + searchContent = content; } void SearchRenderer::setProtocolPrefix(const std::string& prefix) @@ -86,84 +86,133 @@ void SearchRenderer::setSearchProtocolPrefix(const std::string& prefix) this->searchProtocolPrefix = prefix; } -std::string SearchRenderer::getHtml() -{ - kainjow::mustache::data results{kainjow::mustache::data::type::list}; +kainjow::mustache::data buildQueryData +( + const std::string& searchProtocolPrefix, + const std::string& pattern, + const std::string& searchContent +) { + kainjow::mustache::data query; + query.set("pattern", kiwix::encodeDiples(pattern)); + std::ostringstream ss; + ss << searchProtocolPrefix << "?pattern=" << urlEncode(pattern, true); + ss << "&content=" << urlEncode(searchContent, true); + query.set("unpaginatedQuery", ss.str()); + return query; +} - for (auto it = m_srs.begin(); it != m_srs.end(); it++) { - kainjow::mustache::data result; - result.set("title", it.getTitle()); - result.set("url", it.getPath()); - result.set("snippet", it.getSnippet()); - std::string zim_id(it.getZimId()); - result.set("resultContentId", mp_nameMapper->getNameForId(zim_id)); - if (!mp_library) { - result.set("bookTitle", kainjow::mustache::data(false)); - } else { - result.set("bookTitle", mp_library->getBookById(zim_id).getTitle()); +kainjow::mustache::data buildPagination( + unsigned int pageLength, + unsigned int resultsCount, + unsigned int resultsStart +) +{ + assert(pageLength!=0); + kainjow::mustache::data pagination; + kainjow::mustache::data pages{kainjow::mustache::data::type::list}; + + if (resultsCount == 0) { + // Easy case + pagination.set("itemsPerPage", to_string(pageLength)); + pagination.set("hasPages", false); + pagination.set("pages", pages); + return pagination; + } + + // First we want to display pages starting at a multiple of `pageLength` + // so, let's calculate the start index of the current page. + auto currentPage = resultsStart/pageLength; + auto lastPage = ((resultsCount-1)/pageLength); + auto lastPageStart = lastPage*pageLength; + auto nbPages = lastPage + 1; + + auto firstPageGenerated = currentPage > 4 ? currentPage-4 : 0; + auto lastPageGenerated = min(currentPage+4, lastPage); + + if (nbPages != 1) { + if (firstPageGenerated!=0) { + kainjow::mustache::data page; + page.set("label", "◀"); + page.set("start", to_string(0)); + page.set("current", false); + pages.push_back(page); } + for (auto i=firstPageGenerated; i<=lastPageGenerated; i++) { + kainjow::mustache::data page; + page.set("label", to_string(i+1)); + page.set("start", to_string(i*pageLength)); + page.set("current", bool(i == currentPage)); + pages.push_back(page); + } + + if (lastPageGenerated!=lastPage) { + kainjow::mustache::data page; + page.set("label", "▶"); + page.set("start", to_string(lastPageStart)); + page.set("current", false); + pages.push_back(page); + } + } + + pagination.set("itemsPerPage", to_string(pageLength)); + pagination.set("hasPages", firstPageGenerated < lastPageGenerated); + pagination.set("pages", pages); + return pagination; +} + +std::string SearchRenderer::getHtml() +{ + // Build the results list + kainjow::mustache::data items{kainjow::mustache::data::type::list}; + for (auto it = m_srs.begin(); it != m_srs.end(); it++) { + kainjow::mustache::data result; + std::string zim_id(it.getZimId()); + result.set("title", it.getTitle()); + result.set("absolutePath", protocolPrefix + urlEncode(mp_nameMapper->getNameForId(zim_id), true) + "/" + urlEncode(it.getPath())); + result.set("snippet", it.getSnippet()); + if (mp_library) { + result.set("bookTitle", mp_library->getBookById(zim_id).getTitle()); + } if (it.getWordCount() >= 0) { result.set("wordCount", kiwix::beautifyInteger(it.getWordCount())); } - results.push_back(result); + items.push_back(result); } + kainjow::mustache::data results; + results.set("items", items); + results.set("count", kiwix::beautifyInteger(estimatedResultCount)); + results.set("hasResults", estimatedResultCount != 0); + results.set("start", kiwix::beautifyInteger(resultStart+1)); + results.set("end", kiwix::beautifyInteger(min(resultStart+pageLength, estimatedResultCount))); - // pages - kainjow::mustache::data pages{kainjow::mustache::data::type::list}; + // pagination + auto pagination = buildPagination( + pageLength, + estimatedResultCount, + resultStart + ); - auto resultEnd = 0U; - auto currentPage = 0U; - auto pageStart = 0U; - auto pageEnd = 0U; - auto lastPageStart = 0U; - if (pageLength) { - currentPage = resultStart/pageLength; - pageStart = currentPage > 4 ? currentPage-4 : 0; - pageEnd = currentPage + 5; - if (pageEnd > estimatedResultCount / pageLength) { - pageEnd = (estimatedResultCount + pageLength - 1) / pageLength; - } - if (estimatedResultCount > pageLength) { - lastPageStart = ((estimatedResultCount-1)/pageLength)*pageLength; - } - } - - resultEnd = resultStart+pageLength; //setting result end - - for (unsigned int i = pageStart; i < pageEnd; i++) { - kainjow::mustache::data page; - page.set("label", to_string(i + 1)); - page.set("start", to_string(i * pageLength)); - - if (i == currentPage) { - page.set("selected", true); - } - pages.push_back(page); - } + kainjow::mustache::data query = buildQueryData( + searchProtocolPrefix, + searchPattern, + searchContent + ); std::string template_str = RESOURCE::templates::search_result_html; kainjow::mustache::mustache tmpl(template_str); kainjow::mustache::data allData; allData.set("results", results); - allData.set("pages", pages); - allData.set("hasResults", estimatedResultCount != 0); - allData.set("hasPages", pageStart + 1 < pageEnd); - allData.set("count", kiwix::beautifyInteger(estimatedResultCount)); - allData.set("searchPattern", kiwix::encodeDiples(this->searchPattern)); - allData.set("searchPatternEncoded", urlEncode(this->searchPattern)); - allData.set("resultStart", to_string(resultStart + 1)); - allData.set("resultEnd", to_string(min(resultEnd, estimatedResultCount))); - allData.set("pageLength", to_string(pageLength)); - allData.set("resultLastPageStart", to_string(lastPageStart)); - allData.set("protocolPrefix", this->protocolPrefix); - allData.set("searchProtocolPrefix", this->searchProtocolPrefix); - allData.set("contentId", this->searchContent); + allData.set("pagination", pagination); + allData.set("query", query); std::stringstream ss; tmpl.render(allData, [&ss](const std::string& str) { ss << str; }); + if (!tmpl.is_valid()) { + throw std::runtime_error("Error while rendering search results: " + tmpl.error_message()); + } return ss.str(); } diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 95f999f99..0290d79a7 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -633,12 +633,13 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re // Searcher->search will throw a runtime error if there is no valid xapian database to do the search. // (in case of zim file not containing a index) const auto cssUrl = renderUrl(m_root, RESOURCE::templates::url_of_search_results_css); - return HTTPErrorHtmlResponse(*this, request, MHD_HTTP_NOT_FOUND, - "fulltext-search-unavailable", - "404-page-heading", - cssUrl) - + nonParameterizedMessage("no-search-results") - + TaskbarInfo(searchInfo.bookName, archive.get()); + HTTPErrorHtmlResponse response(*this, request, MHD_HTTP_NOT_FOUND, + "fulltext-search-unavailable", + "404-page-heading", + cssUrl); + response += nonParameterizedMessage("no-search-results"); + response += TaskbarInfo(searchInfo.bookName, archive.get()); + return response; } @@ -664,7 +665,7 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re renderer.setSearchPattern(searchInfo.pattern); renderer.setSearchContent(searchInfo.bookName); renderer.setProtocolPrefix(m_root + "/"); - renderer.setSearchProtocolPrefix(m_root + "/search?"); + renderer.setSearchProtocolPrefix(m_root + "/search"); renderer.setPageLength(pageLength); auto response = ContentResponse::build(*this, renderer.getHtml(), "text/html; charset=utf-8"); response->set_taskbar(searchInfo.bookName, archive.get()); @@ -673,10 +674,6 @@ std::unique_ptr InternalServer::handle_search(const RequestContext& re return HTTP400HtmlResponse(*this, request) + invalidUrlMsg + std::string(e.what()); - } catch (const std::exception& e) { - std::cerr << e.what() << std::endl; - return HTTP500HtmlResponse(*this, request) - + e.what(); } } diff --git a/src/server/request_context.h b/src/server/request_context.h index 79b0a60e9..2c4c8902e 100644 --- a/src/server/request_context.h +++ b/src/server/request_context.h @@ -28,6 +28,7 @@ #include #include "byte_range.h" +#include "tools/stringTools.h" extern "C" { #include "microhttpd_wrapper.h" @@ -68,10 +69,7 @@ class RequestContext { std::string get_header(const std::string& name) const; template T get_argument(const std::string& name) const { - std::istringstream stream(arguments.at(name)); - T v; - stream >> v; - return v; + return extractFromString(arguments.at(name)); } template diff --git a/src/server/response.cpp b/src/server/response.cpp index ae80a5846..5543499fd 100644 --- a/src/server/response.cpp +++ b/src/server/response.cpp @@ -194,6 +194,12 @@ HTTPErrorHtmlResponse& HTTPErrorHtmlResponse::operator+(const ParameterizedMessa return *this + details.getText(m_request.get_user_language()); } +HTTPErrorHtmlResponse& HTTPErrorHtmlResponse::operator+=(const ParameterizedMessage& details) +{ + // operator+() is already a state-modifying operator (akin to operator+=) + return *this + details; +} + HTTP400HtmlResponse::HTTP400HtmlResponse(const InternalServer& server, const RequestContext& request) @@ -246,6 +252,13 @@ ContentResponseBlueprint& ContentResponseBlueprint::operator+(const TaskbarInfo& return *this; } +ContentResponseBlueprint& ContentResponseBlueprint::operator+=(const TaskbarInfo& taskbarInfo) +{ + // operator+() is already a state-modifying operator (akin to operator+=) + return *this + taskbarInfo; +} + + std::unique_ptr Response::build_416(const InternalServer& server, size_t resourceLength) { auto response = Response::build(server); diff --git a/src/server/response.h b/src/server/response.h index 2eba46d2b..5c14e7786 100644 --- a/src/server/response.h +++ b/src/server/response.h @@ -165,6 +165,7 @@ public: // functions ContentResponseBlueprint& operator+(const TaskbarInfo& taskbarInfo); + ContentResponseBlueprint& operator+=(const TaskbarInfo& taskbarInfo); protected: // functions std::string getMessage(const std::string& msgId) const; @@ -190,8 +191,10 @@ struct HTTPErrorHtmlResponse : ContentResponseBlueprint const std::string& cssUrl = ""); using ContentResponseBlueprint::operator+; + using ContentResponseBlueprint::operator+=; HTTPErrorHtmlResponse& operator+(const std::string& msg); HTTPErrorHtmlResponse& operator+(const ParameterizedMessage& errorDetails); + HTTPErrorHtmlResponse& operator+=(const ParameterizedMessage& errorDetails); }; class UrlNotFoundMsg {}; diff --git a/src/tools/stringTools.cpp b/src/tools/stringTools.cpp index 6d6e1d39e..e7cb851ec 100644 --- a/src/tools/stringTools.cpp +++ b/src/tools/stringTools.cpp @@ -405,3 +405,8 @@ std::vector kiwix::getTitleVariants(const std::string& title) { variants.push_back(kiwix::toTitle(title)); return variants; } + +template<> +std::string kiwix::extractFromString(const std::string& str) { + return str; +} diff --git a/src/tools/stringTools.h b/src/tools/stringTools.h index 9b5cf3622..337548578 100644 --- a/src/tools/stringTools.h +++ b/src/tools/stringTools.h @@ -25,6 +25,7 @@ #include #include #include +#include namespace kiwix { @@ -65,9 +66,15 @@ T extractFromString(const std::string& str) { std::istringstream iss(str); T ret; iss >> ret; + if(iss.fail() || !iss.eof()) { + throw std::invalid_argument("no conversion"); + } return ret; } +template<> +std::string extractFromString(const std::string& str); + bool startsWith(const std::string& base, const std::string& start); std::vector getTitleVariants(const std::string& title); diff --git a/static/templates/search_result.html b/static/templates/search_result.html index ddf0ff2ab..93133c584 100644 --- a/static/templates/search_result.html +++ b/static/templates/search_result.html @@ -102,30 +102,30 @@ } - Search: {{searchPattern}} + Search: {{query.pattern}}
- {{#hasResults}} + {{#results.hasResults}} Results - {{resultStart}}-{{resultEnd}} + {{results.start}}-{{results.end}} of - {{count}} + {{results.count}} for - "{{{searchPattern}}}" + "{{{query.pattern}}}" - {{/hasResults}} - {{^hasResults}} - No results were found for "{{{searchPattern}}}" - {{/hasResults}} + {{/results.hasResults}} + {{^results.hasResults}} + No results were found for "{{{query.pattern}}}" + {{/results.hasResults}}
    - {{#results}} + {{#results.items}}
  • - + {{title}} {{#snippet}} @@ -138,39 +138,23 @@
    {{wordCount}} words
    {{/wordCount}}
    • - {{/results}} + {{/results.items}}
- {{#hasPages}} + {{#pagination.hasPages}}
    - {{#resultLastPageStart}} + {{#pagination.pages}}
  • - - ◀ - -
    • - {{/resultLastPageStart}} - {{#pages}} -
  • - + {{label}}
    • - {{/pages}} - {{#resultLastPageStart}} -
  • - - ▶ - -
    • - {{/resultLastPageStart}} + {{/pagination.pages}}
- {{/hasPages}} + {{/pagination.hasPages}}
diff --git a/test/meson.build b/test/meson.build index 1047ff0c7..368144c57 100644 --- a/test/meson.build +++ b/test/meson.build @@ -9,7 +9,8 @@ tests = [ 'book', 'manager', 'name_mapper', - 'opds_catalog' + 'opds_catalog', + 'server_helper' ] if build_machine.system() != 'windows' @@ -59,6 +60,7 @@ if gtest_dep.found() and not meson.is_cross_build() # XXX: '#include ' includes the regex unit test binary test_exe = executable(test_name, [test_name+'.cpp'], implicit_include_directories: false, + include_directories : inc, link_with : kiwixlib, link_args: extra_link_args, dependencies : all_deps + [gtest_dep], diff --git a/test/server.cpp b/test/server.cpp index 72725a67b..4054a49ba 100644 --- a/test/server.cpp +++ b/test/server.cpp @@ -1158,7 +1158,7 @@ R"SEARCHRESULT( )SEARCHRESULT", R"SEARCHRESULT( - + Catchin' Some Rays: The Music of Ray Charles ...jazz singer Roseanna Vitro, released in August 1997 on the Telarc Jazz label. Catchin' Some Rays: The Music of Ray Charles Studio album by Roseanna Vitro Released August 1997 Recorded March 26, 1997 at Sound on Sound, NYC April 4,1997 at Quad Recording Studios, NYC Genre Vocal jazz Length 61:00 Label Telarc Jazz CD-83419 Producer Paul Wickliffe Roseanna Vitro chronology Passion Dance (1996) Catchin' Some Rays: The Music of Ray Charles (1997) The Time of My Life: Roseanna Vitro Sings the Songs of...... @@ -1167,7 +1167,7 @@ R"SEARCHRESULT( )SEARCHRESULT", R"SEARCHRESULT( - + That's What I Say: John Scofield Plays the Music of Ray Charles That's What I Say: John Scofield Plays the Music of Ray Charles Studio album by John Scofield Released June 7, 2005 (2005-06-07) Recorded December 2004 Studio Avatar Studios, New York City Genre Jazz Length 65:21 Label Verve Producer Steve Jordan John Scofield chronology EnRoute: John Scofield Trio LIVE (2004) That's What I Say: John Scofield Plays the Music of Ray Charles (2005) Out Louder (2006) Professional ratings Review scores Source Rating Allmusic All About Jazz All About Jazz... @@ -1437,7 +1437,7 @@ R"SEARCHRESULT( )SEARCHRESULT", R"SEARCHRESULT( - + Don't Let the Sun Catch You Cryin' ...R&B Sides" and No. 95 on the Billboard Hot 100. It was also recorded by Jackie DeShannon on her 1965 album This is Jackie De Shannon, Paul McCartney on his 1990 live album Tripping the Live Fantastic, Jex Saarelaht and Kate Ceberano on their album Open the Door - Live at Mietta's (1992) and jazz singer Roseanna Vitro on her 1997 album Catchin’ Some Rays: The Music of Ray Charles. Karin Krog and Steve Kuhn include it on their 2005 album, Together Again. Steve Alaimo released a version in 1963... @@ -1446,7 +1446,7 @@ R"SEARCHRESULT( )SEARCHRESULT", R"SEARCHRESULT( - + I Don't Need No Doctor ...jazz guitar player John Scofield recorded a version for his album That's What I Say: John Scofield Plays the Music of Ray Charles in 2005, featuring the blues guitarist John Mayer on additional guitar and vocals. Mayer covered the song again with his band during his tour in summer 2007. A recorded live version from a Los Angeles show during that tour is available on Mayer's CD/DVD release Where the Light Is. A Ray Charles tribute album also provided the impetus for jazz singer Roseanna Vitro's...... @@ -1849,7 +1849,6 @@ R"SEARCHRESULT( }, /* pagination */ { - { "◀", 0, false }, { "1", 0, true }, { "2", 5, false }, { "3", 10, false }, @@ -1874,7 +1873,6 @@ R"SEARCHRESULT( }, /* pagination */ { - { "◀", 0, false }, { "1", 0, false }, { "2", 5, true }, { "3", 10, false }, @@ -1900,7 +1898,6 @@ R"SEARCHRESULT( }, /* pagination */ { - { "◀", 0, false }, { "1", 0, false }, { "2", 5, false }, { "3", 10, true }, @@ -1927,7 +1924,6 @@ R"SEARCHRESULT( }, /* pagination */ { - { "◀", 0, false }, { "1", 0, false }, { "2", 5, false }, { "3", 10, false }, @@ -1955,7 +1951,6 @@ R"SEARCHRESULT( }, /* pagination */ { - { "◀", 0, false }, { "1", 0, false }, { "2", 5, false }, { "3", 10, false }, @@ -1965,7 +1960,6 @@ R"SEARCHRESULT( { "7", 30, false }, { "8", 35, false }, { "9", 40, false }, - { "▶", 40, false }, } }, @@ -1993,7 +1987,6 @@ R"SEARCHRESULT( { "7", 30, false }, { "8", 35, false }, { "9", 40, false }, - { "▶", 40, false }, } }, @@ -2020,7 +2013,6 @@ R"SEARCHRESULT( { "7", 30, true }, { "8", 35, false }, { "9", 40, false }, - { "▶", 40, false }, } }, @@ -2046,7 +2038,6 @@ R"SEARCHRESULT( { "7", 30, false }, { "8", 35, true }, { "9", 40, false }, - { "▶", 40, false }, } }, @@ -2070,7 +2061,6 @@ R"SEARCHRESULT( { "7", 30, false }, { "8", 35, false }, { "9", 40, true }, - { "▶", 40, false }, } }, @@ -2117,7 +2107,6 @@ R"SEARCHRESULT( { "7", 30, false }, { "8", 35, false }, { "9", 40, false }, - { "▶", 40, false }, } }, }; diff --git a/test/server_helper.cpp b/test/server_helper.cpp new file mode 100644 index 000000000..c8e04deab --- /dev/null +++ b/test/server_helper.cpp @@ -0,0 +1,215 @@ + +#include +#include "gtest/gtest.h" +#include "../src/tools/stringTools.h" + +namespace { + struct ExpectedPage { + ExpectedPage(const std::string& label, unsigned int start, bool current) + : label(label), + start(start), + current(current) + {} + + testing::AssertionResult isEqual(const kainjow::mustache::data& data) { + if (!data.is_object() + || data.get("label") == nullptr + || data.get("start") == nullptr + || data.get("current") == nullptr) { + return testing::AssertionFailure() << "data is not a valid object"; + } + + if (data.get("label")->string_value() != label) { + return testing::AssertionFailure() << data.get("label")->string_value() + << " is not equal to " + << label; + } + + if (data.get("start")->string_value() != kiwix::to_string(start)) { + return testing::AssertionFailure() << data.get("start")->string_value() + << " is not equal to " + << kiwix::to_string(start); + } + + if (current && !data.get("current")->is_true()) { + return testing::AssertionFailure() << "data is not true"; + } + if (!current && !data.get("current")->is_false()) { + return testing::AssertionFailure() << "data is not false"; + } + return testing::AssertionSuccess(); + } + + std::string label; + unsigned int start; + bool current; + }; + + class ExpectedPages : public std::vector + { + public: + ExpectedPages(std::vector&& v) + : std::vector(v) + {} + + testing::AssertionResult isEqual(const kainjow::mustache::data& data) { + if (empty()) { + if (data.is_empty_list()) { + return testing::AssertionSuccess(); + } else { + return testing::AssertionFailure() << "data is not an empty list."; + } + } + + if (! data.is_non_empty_list()) { + return testing::AssertionFailure() << "data is not a non empty list."; + } + auto& data_pages = data.list_value(); + if (size() != data_pages.size()) { + return testing::AssertionFailure() << "data (size " << data_pages.size() << ")" + << " and expected (size " << size() << ")" + << " must have the same size"; + } + auto it1 = begin(); + auto it2 = data_pages.begin(); + while (it1!=end()) { + auto result = it1->isEqual(*it2); + if(!result) { + return result; + } + it1++; it2++; + } + return testing::AssertionSuccess(); + } + }; +} + +namespace kiwix { +kainjow::mustache::data buildPagination( + unsigned int pageLength, + unsigned int resultsCount, + unsigned int resultsStart +); +} + +TEST(SearchRenderer, buildPagination) { + { + auto pagination = kiwix::buildPagination(10, 0, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_false()); + ASSERT_TRUE(ExpectedPages({}).isEqual(*pagination.get("pages"))); + } + { + auto pagination = kiwix::buildPagination(10, 1, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_false()); + ASSERT_TRUE(ExpectedPages({}).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 10, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_false()); + ASSERT_TRUE(ExpectedPages({}).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 11, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"1", 0, true}, + {"2", 10, false}, + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 20, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"1", 0, true}, + {"2", 10, false}, + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 21, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"1", 0, true}, + {"2", 10, false}, + {"3", 20, false} + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 21, 11); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"1", 0, false}, + {"2", 10, true}, + {"3", 20, false} + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 21, 21); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"1", 0, false}, + {"2", 10, false}, + {"3", 20, true} + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 200, 0); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"1", 0, true}, + {"2", 10, false}, + {"3", 20, false}, + {"4", 30, false}, + {"5", 40, false}, + {"▶", 190, false} + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 200, 105); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"◀", 0, false}, + {"7", 60, false}, + {"8", 70, false}, + {"9", 80, false}, + {"10", 90, false}, + {"11", 100, true}, + {"12", 110, false}, + {"13", 120, false}, + {"14", 130, false}, + {"15", 140, false}, + {"▶", 190, false} + }).isEqual(*pagination.get("pages"))); + } + + { + auto pagination = kiwix::buildPagination(10, 200, 199); + ASSERT_EQ(pagination.get("itemsPerPage")->string_value(), "10"); + ASSERT_TRUE(pagination.get("hasPages")->is_true()); + ASSERT_TRUE(ExpectedPages({ + {"◀", 0, false}, + {"16", 150, false}, + {"17", 160, false}, + {"18", 170, false}, + {"19", 180, false}, + {"20", 190, true} + }).isEqual(*pagination.get("pages"))); + } +} diff --git a/test/stringTools.cpp b/test/stringTools.cpp index af9c25bdf..d91670933 100644 --- a/test/stringTools.cpp +++ b/test/stringTools.cpp @@ -18,11 +18,11 @@ */ #include "gtest/gtest.h" +#include "../src/tools/stringTools.h" #include #include namespace kiwix { -std::string join(const std::vector& list, const std::string& sep); std::vector split(const std::string& base, const std::string& sep, bool trimEmpty, bool keepDelim); }; @@ -58,4 +58,25 @@ TEST(stringTools, split) ASSERT_EQ(split(";a;b=;c=d;", ";=", false, true), list6); } +TEST(stringTools, extractFromString) +{ + ASSERT_EQ(extractFromString("55"), 55); + ASSERT_EQ(extractFromString("-55"), -55); + ASSERT_EQ(extractFromString("-55.0"), -55.0); + ASSERT_EQ(extractFromString("1"), true); + ASSERT_EQ(extractFromString("0"), false); + ASSERT_EQ(extractFromString("55"), "55"); + ASSERT_EQ(extractFromString("foo"), "foo"); + ASSERT_EQ(extractFromString("foo bar"), "foo bar"); + +// While spec says that >> operator should set the value to std::numeric_limits::max() +// and set the failbit of the stream (and so detect the error), the gcc implementation (?) +// set the value to (std::numeric_limits::max()-55) and doesn't set the failbit. +// ASSERT_THROW(extractFromString("-55"), std::invalid_argument); + + ASSERT_THROW(extractFromString("-55.0"), std::invalid_argument); + ASSERT_THROW(extractFromString("55 foo"), std::invalid_argument); + ASSERT_THROW(extractFromString("3.14.5"), std::invalid_argument); +} + };