diff --git a/include/library.h b/include/library.h index 602cfb9fd..f1a58f6c5 100644 --- a/include/library.h +++ b/include/library.h @@ -34,6 +34,10 @@ #define KIWIX_LIBRARY_VERSION "20110515" +namespace Xapian { +class WritableDatabase; +}; + namespace kiwix { @@ -173,31 +177,42 @@ class ZimSearcher : public zim::Searcher std::mutex m_mutex; }; +template +class ConcurrentCache; + +template +class MultiKeyCache; + +using LibraryPtr = std::shared_ptr; +using ConstLibraryPtr = std::shared_ptr; + /** * A Library store several books. */ -class Library +class Library: public std::enable_shared_from_this { - // all data fields must be added in LibraryBase - mutable std::mutex m_mutex; - public: typedef uint64_t Revision; typedef std::vector BookIdCollection; typedef std::map AttributeCounts; typedef std::set BookIdSet; - public: + private: Library(); + + public: + [[nodiscard]] static LibraryPtr create() { + return LibraryPtr(new Library()); + } ~Library(); /** * Library is not a copiable object. However it can be moved. */ Library(const Library& ) = delete; - Library(Library&& ); + Library(Library&& ) = delete; void operator=(const Library& ) = delete; - Library& operator=(Library&& ); + Library& operator=(Library&& ) = delete; /** * Add a book to the library. @@ -368,19 +383,32 @@ class Library private: // types typedef const std::string& (Book::*BookStrPropMemFn)() const; - struct Impl; + struct Entry : Book + { + Library::Revision lastUpdatedRevision = 0; + }; private: // functions AttributeCounts getBookAttributeCounts(BookStrPropMemFn p) const; std::vector getBookPropValueSet(BookStrPropMemFn p) const; BookIdCollection filterViaBookDB(const Filter& filter) const; + unsigned int getBookCount_not_protected(const bool localBooks, const bool remoteBooks) const; void updateBookDB(const Book& book); void dropCache(const std::string& bookId); private: //data - std::unique_ptr mp_impl; + mutable std::mutex m_mutex; + Library::Revision m_revision; + std::map m_books; + using ArchiveCache = ConcurrentCache>; + std::unique_ptr mp_archiveCache; + using SearcherCache = MultiKeyCache>; + std::unique_ptr mp_searcherCache; + std::vector m_bookmarks; + std::unique_ptr m_bookDB; }; + } #endif diff --git a/include/manager.h b/include/manager.h index d0addad70..415db6224 100644 --- a/include/manager.h +++ b/include/manager.h @@ -37,10 +37,10 @@ namespace kiwix class LibraryManipulator { public: // functions - explicit LibraryManipulator(Library* library); + explicit LibraryManipulator(LibraryPtr library); virtual ~LibraryManipulator(); - Library& getLibrary() const { return library; } + LibraryPtr getLibrary() const { return library; } bool addBookToLibrary(const Book& book); void addBookmarkToLibrary(const Bookmark& bookmark); @@ -52,7 +52,7 @@ class LibraryManipulator virtual void booksWereRemovedFromLibrary(); private: // data - kiwix::Library& library; + LibraryPtr library; }; /** @@ -64,8 +64,8 @@ class Manager typedef std::vector Paths; public: // functions - explicit Manager(LibraryManipulator* manipulator); - explicit Manager(Library* library); + explicit Manager(LibraryManipulator manipulator); + explicit Manager(LibraryPtr library); /** * Read a `library.xml` and add book in the file to the library. @@ -163,7 +163,7 @@ class Manager uint64_t m_itemsPerPage = 0; protected: - std::shared_ptr manipulator; + kiwix::LibraryManipulator manipulator; bool readBookFromPath(const std::string& path, Book* book); bool parseXmlDom(const pugi::xml_document& doc, diff --git a/include/meson.build b/include/meson.build index 405297a34..03e6f0465 100644 --- a/include/meson.build +++ b/include/meson.build @@ -4,10 +4,6 @@ headers = [ 'common.h', 'library.h', 'manager.h', - 'libxml_dumper.h', - 'opds_dumper.h', - 'library_dumper.h', - 'html_dumper.h', 'downloader.h', 'search_renderer.h', 'server.h', diff --git a/include/name_mapper.h b/include/name_mapper.h index 4247c5c3c..63333515d 100644 --- a/include/name_mapper.h +++ b/include/name_mapper.h @@ -59,7 +59,7 @@ class HumanReadableNameMapper : public NameMapper { class UpdatableNameMapper : public NameMapper { typedef std::shared_ptr NameMapperHandle; public: - UpdatableNameMapper(Library& library, bool withAlias); + UpdatableNameMapper(std::shared_ptr library, bool withAlias); virtual std::string getNameForId(const std::string& id) const; virtual std::string getIdForName(const std::string& name) const; @@ -71,7 +71,7 @@ class UpdatableNameMapper : public NameMapper { private: mutable std::mutex mutex; - Library& library; + std::shared_ptr library; NameMapperHandle nameMapper; const bool withAlias; }; diff --git a/include/search_renderer.h b/include/search_renderer.h index 0190ee5f0..2669b84cf 100644 --- a/include/search_renderer.h +++ b/include/search_renderer.h @@ -37,29 +37,11 @@ class SearchRenderer /** * Construct a SearchRenderer from a SearchResultSet. * - * The constructed version of the SearchRenderer will not introduce - * the book name for each result. It is better to use the other constructor - * with a Library pointer to have a better html page. - * * @param srs The `SearchResultSet` to render. - * @param mapper The `NameMapper` to use to do the rendering. * @param start The start offset used for the srs. * @param estimatedResultCount The estimatedResultCount of the whole search */ - SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, - unsigned int start, unsigned int estimatedResultCount); - - /** - * Construct a SearchRenderer from a SearchResultSet. - * - * @param srs The `SearchResultSet` to render. - * @param mapper The `NameMapper` to use to do the rendering. - * @param library The `Library` to use to look up book details for search results. - * @param start The start offset used for the srs. - * @param estimatedResultCount The estimatedResultCount of the whole search - */ - SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, Library* library, - unsigned int start, unsigned int estimatedResultCount); + SearchRenderer(zim::SearchResultSet srs, unsigned int start, unsigned int estimatedResultCount); ~SearchRenderer(); @@ -90,24 +72,32 @@ class SearchRenderer this->pageLength = pageLength; } - std::string renderTemplate(const std::string& tmpl_str); - /** * Generate the html page with the resutls of the search. + * + * @param mapper The `NameMapper` to use to do the rendering. + * @param library The `Library` to use to look up book details for search results. + May be nullptr. In this case, bookName is not set in the rendered string. + * @return The html string */ - std::string getHtml(); + std::string getHtml(const NameMapper& mapper, const Library* library); - /** + /** * Generate the xml page with the resutls of the search. + * + * @param mapper The `NameMapper` to use to do the rendering. + * @param library The `Library` to use to look up book details for search results. + May be nullptr. In this case, bookName is not set in the rendered string. + * @return The xml string */ - std::string getXml(); + std::string getXml(const NameMapper& mapper, const Library* library); + protected: // function + std::string renderTemplate(const std::string& tmpl_str, const NameMapper& mapper, const Library *library); protected: std::string beautifyInteger(const unsigned int number); zim::SearchResultSet m_srs; - NameMapper* mp_nameMapper; - Library* mp_library; std::string searchBookQuery; std::string searchPattern; std::string protocolPrefix; diff --git a/include/server.h b/include/server.h index 0cd579c57..d1981c8c2 100644 --- a/include/server.h +++ b/include/server.h @@ -36,7 +36,7 @@ namespace kiwix * * @param library The library to serve. */ - Server(Library* library, NameMapper* nameMapper=nullptr); + Server(std::shared_ptr library, std::shared_ptr nameMapper=nullptr); virtual ~Server(); @@ -66,8 +66,8 @@ namespace kiwix std::string getAddress(); protected: - Library* mp_library; - NameMapper* mp_nameMapper; + std::shared_ptr mp_library; + std::shared_ptr mp_nameMapper; std::string m_root = ""; std::string m_addr = ""; std::string m_indexTemplateString = ""; diff --git a/include/html_dumper.h b/src/html_dumper.h similarity index 100% rename from include/html_dumper.h rename to src/html_dumper.h diff --git a/src/library.cpp b/src/library.cpp index 6ee51602f..702bf8bdd 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -39,6 +39,8 @@ namespace kiwix { + + namespace { @@ -58,6 +60,8 @@ bool booksReferToTheSameArchive(const Book& book1, const Book& book2) && book1.getPath() == book2.getPath(); } +} // unnamed namespace + template class MultiKeyCache: public ConcurrentCache, Value> { @@ -79,49 +83,8 @@ class MultiKeyCache: public ConcurrentCache, Value> } }; -} // unnamed namespace - -struct Library::Impl -{ - struct Entry : Book - { - Library::Revision lastUpdatedRevision = 0; - }; - - Library::Revision m_revision; - std::map m_books; - using ArchiveCache = ConcurrentCache>; - std::unique_ptr mp_archiveCache; - using SearcherCache = MultiKeyCache>; - std::unique_ptr mp_searcherCache; - 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&& ); -}; - -Library::Impl::Impl() - : mp_archiveCache(new ArchiveCache(std::max(getEnvVar("KIWIX_ARCHIVE_CACHE_SIZE", 1), 1))), - mp_searcherCache(new SearcherCache(std::max(getEnvVar("KIWIX_SEARCHER_CACHE_SIZE", 1), 1))), - m_bookDB("", Xapian::DB_BACKEND_INMEMORY) -{ -} - -Library::Impl::~Impl() -{ -} - -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 +Library::getBookCount_not_protected(const bool localBooks, const bool remoteBooks) const { unsigned int result = 0; for (auto& pair: m_books) { @@ -136,50 +99,41 @@ Library::Impl::getBookCount(const bool localBooks, const bool remoteBooks) const /* Constructor */ Library::Library() - : mp_impl(new Library::Impl) + : mp_archiveCache(new ArchiveCache(std::max(getEnvVar("KIWIX_ARCHIVE_CACHE_SIZE", 1), 1))), + mp_searcherCache(new SearcherCache(std::max(getEnvVar("KIWIX_SEARCHER_CACHE_SIZE", 1), 1))), + m_bookDB(new Xapian::WritableDatabase("", Xapian::DB_BACKEND_INMEMORY)) { } -Library::Library(Library&& other) - : mp_impl(std::move(other.mp_impl)) -{ -} - -Library& Library::operator=(Library&& other) -{ - mp_impl = std::move(other.mp_impl); - return *this; -} - /* Destructor */ Library::~Library() = default; bool Library::addBook(const Book& book) { std::lock_guard lock(m_mutex); - ++mp_impl->m_revision; + ++m_revision; /* Try to find it */ updateBookDB(book); try { - auto& oldbook = mp_impl->m_books.at(book.getId()); + auto& oldbook = m_books.at(book.getId()); if ( ! booksReferToTheSameArchive(oldbook, book) ) { dropCache(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 = mp_impl->m_revision; + oldbook.lastUpdatedRevision = m_revision; return false; } catch (std::out_of_range&) { - auto& newEntry = mp_impl->m_books[book.getId()]; + auto& newEntry = m_books[book.getId()]; static_cast(newEntry) = book; - newEntry.lastUpdatedRevision = mp_impl->m_revision; - size_t new_cache_size = static_cast(std::ceil(mp_impl->getBookCount(true, true)*0.1)); + newEntry.lastUpdatedRevision = m_revision; + size_t new_cache_size = static_cast(std::ceil(getBookCount_not_protected(true, true)*0.1)); if (getEnvVar("KIWIX_ARCHIVE_CACHE_SIZE", -1) <= 0) { - mp_impl->mp_archiveCache->setMaxSize(new_cache_size); + mp_archiveCache->setMaxSize(new_cache_size); } if (getEnvVar("KIWIX_SEARCHER_CACHE_SIZE", -1) <= 0) { - mp_impl->mp_searcherCache->setMaxSize(new_cache_size); + mp_searcherCache->setMaxSize(new_cache_size); } return true; } @@ -188,15 +142,15 @@ bool Library::addBook(const Book& book) void Library::addBookmark(const Bookmark& bookmark) { std::lock_guard lock(m_mutex); - mp_impl->m_bookmarks.push_back(bookmark); + 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=mp_impl->m_bookmarks.begin(); it!=mp_impl->m_bookmarks.end(); it++) { + for(auto it=m_bookmarks.begin(); it!=m_bookmarks.end(); it++) { if (it->getBookId() == zimId && it->getUrl() == url) { - mp_impl->m_bookmarks.erase(it); + m_bookmarks.erase(it); return true; } } @@ -206,14 +160,14 @@ bool Library::removeBookmark(const std::string& zimId, const std::string& url) void Library::dropCache(const std::string& id) { - mp_impl->mp_archiveCache->drop(id); - mp_impl->mp_searcherCache->drop(id); + mp_archiveCache->drop(id); + mp_searcherCache->drop(id); } bool Library::removeBookById(const std::string& id) { std::lock_guard lock(m_mutex); - mp_impl->m_bookDB.delete_document("Q" + id); + m_bookDB->delete_document("Q" + id); dropCache(id); // We do not change the cache size here // Most of the time, the book is remove in case of library refresh, it is @@ -221,9 +175,9 @@ bool Library::removeBookById(const std::string& id) // Having a too big cache is not a problem here (or it would have been before) // (And setMaxSize doesn't actually reduce the cache size, extra cached items // will be removed in put or getOrPut). - const bool bookWasRemoved = mp_impl->m_books.erase(id) == 1; + const bool bookWasRemoved = m_books.erase(id) == 1; if ( bookWasRemoved ) { - ++mp_impl->m_revision; + ++m_revision; } return bookWasRemoved; } @@ -231,7 +185,7 @@ bool Library::removeBookById(const std::string& id) Library::Revision Library::getRevision() const { std::lock_guard lock(m_mutex); - return mp_impl->m_revision; + return m_revision; } uint32_t Library::removeBooksNotUpdatedSince(Revision libraryRevision) @@ -239,7 +193,7 @@ uint32_t Library::removeBooksNotUpdatedSince(Revision libraryRevision) BookIdCollection booksToRemove; { std::lock_guard lock(m_mutex); - for ( const auto& entry : mp_impl->m_books) { + for ( const auto& entry : m_books) { if ( entry.second.lastUpdatedRevision <= libraryRevision ) { booksToRemove.push_back(entry.first); } @@ -258,7 +212,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 mp_impl->m_books.at(id); + return m_books.at(id); } Book Library::getBookByIdThreadSafe(const std::string& id) const @@ -271,7 +225,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: mp_impl->m_books) { + for(auto& it: m_books) { auto& book = it.second; if (book.getPath() == path) return book; @@ -284,7 +238,7 @@ const Book& Library::getBookByPath(const std::string& path) const std::shared_ptr Library::getArchiveById(const std::string& id) { try { - return mp_impl->mp_archiveCache->getOrPut(id, + return mp_archiveCache->getOrPut(id, [&](){ auto book = getBookById(id); if (!book.isPathValid()) { @@ -301,7 +255,7 @@ std::shared_ptr Library::getSearcherByIds(const BookIdSet& ids) { assert(!ids.empty()); try { - return mp_impl->mp_searcherCache->getOrPut(ids, + return mp_searcherCache->getOrPut(ids, [&](){ std::vector archives; for(auto& id:ids) { @@ -322,7 +276,7 @@ unsigned int Library::getBookCount(const bool localBooks, const bool remoteBooks) const { std::lock_guard lock(m_mutex); - return mp_impl->getBookCount(localBooks, remoteBooks); + return getBookCount_not_protected(localBooks, remoteBooks); } bool Library::writeToFile(const std::string& path) const @@ -353,7 +307,7 @@ Library::AttributeCounts Library::getBookAttributeCounts(BookStrPropMemFn p) con std::lock_guard lock(m_mutex); AttributeCounts propValueCounts; - for (const auto& pair: mp_impl->m_books) { + for (const auto& pair: m_books) { const auto& book = pair.second; if (book.getOrigId().empty()) { propValueCounts[(book.*p)()] += 1; @@ -385,7 +339,7 @@ Library::AttributeCounts Library::getBooksLanguagesWithCounts() const std::lock_guard lock(m_mutex); AttributeCounts langsWithCounts; - for (const auto& pair: mp_impl->m_books) { + for (const auto& pair: m_books) { const auto& book = pair.second; if (book.getOrigId().empty()) { for ( const auto& lang : book.getLanguages() ) { @@ -401,7 +355,7 @@ std::vector Library::getBooksCategories() const std::lock_guard lock(m_mutex); std::set categories; - for (const auto& pair: mp_impl->m_books) { + for (const auto& pair: m_books) { const auto& book = pair.second; const auto& c = book.getCategory(); if ( !c.empty() ) { @@ -425,12 +379,12 @@ std::vector Library::getBooksPublishers() const const std::vector Library::getBookmarks(bool onlyValidBookmarks) const { if (!onlyValidBookmarks) { - return mp_impl->m_bookmarks; + return m_bookmarks; } std::vector validBookmarks; auto booksId = getBooksIds(); std::lock_guard lock(m_mutex); - for(auto& bookmark:mp_impl->m_bookmarks) { + for(auto& bookmark:m_bookmarks) { if (std::find(booksId.begin(), booksId.end(), bookmark.getBookId()) != booksId.end()) { validBookmarks.push_back(bookmark); } @@ -443,7 +397,7 @@ Library::BookIdCollection Library::getBooksIds() const std::lock_guard lock(m_mutex); BookIdCollection bookIds; - for (auto& pair: mp_impl->m_books) { + for (auto& pair: m_books) { bookIds.push_back(pair.first); } @@ -498,7 +452,7 @@ void Library::updateBookDB(const Book& book) doc.set_data(book.getId()); - mp_impl->m_bookDB.replace_document(idterm, doc); + m_bookDB->replace_document(idterm, doc); } namespace @@ -647,9 +601,9 @@ Library::BookIdCollection Library::filterViaBookDB(const Filter& filter) const BookIdCollection bookIds; std::lock_guard lock(m_mutex); - Xapian::Enquire enquire(mp_impl->m_bookDB); + Xapian::Enquire enquire(*m_bookDB); enquire.set_query(query); - const auto results = enquire.get_mset(0, mp_impl->m_books.size()); + const auto results = enquire.get_mset(0, m_books.size()); for ( auto it = results.begin(); it != results.end(); ++it ) { bookIds.push_back(it.get_document().get_data()); } @@ -663,7 +617,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(mp_impl->m_books.at(id))) { + if(filter.accept(m_books.at(id))) { result.push_back(id); } } diff --git a/include/library_dumper.h b/src/library_dumper.h similarity index 100% rename from include/library_dumper.h rename to src/library_dumper.h diff --git a/include/libxml_dumper.h b/src/libxml_dumper.h similarity index 100% rename from include/libxml_dumper.h rename to src/libxml_dumper.h diff --git a/src/manager.cpp b/src/manager.cpp index 55d9eff70..ba4d0c75f 100644 --- a/src/manager.cpp +++ b/src/manager.cpp @@ -27,22 +27,12 @@ namespace kiwix { -namespace -{ - -struct NoDelete -{ - template void operator()(T*) {} -}; - -} // unnamed namespace - //////////////////////////////////////////////////////////////////////////////// // LibraryManipulator //////////////////////////////////////////////////////////////////////////////// -LibraryManipulator::LibraryManipulator(Library* library) - : library(*library) +LibraryManipulator::LibraryManipulator(LibraryPtr library) + : library(library) {} LibraryManipulator::~LibraryManipulator() @@ -50,7 +40,7 @@ LibraryManipulator::~LibraryManipulator() bool LibraryManipulator::addBookToLibrary(const Book& book) { - const auto ret = library.addBook(book); + const auto ret = library->addBook(book); if ( ret ) { bookWasAddedToLibrary(book); } @@ -59,13 +49,13 @@ bool LibraryManipulator::addBookToLibrary(const Book& book) void LibraryManipulator::addBookmarkToLibrary(const Bookmark& bookmark) { - library.addBookmark(bookmark); + library->addBookmark(bookmark); bookmarkWasAddedToLibrary(bookmark); } uint32_t LibraryManipulator::removeBooksNotUpdatedSince(Library::Revision rev) { - const auto n = library.removeBooksNotUpdatedSince(rev); + const auto n = library->removeBooksNotUpdatedSince(rev); if ( n != 0 ) { booksWereRemovedFromLibrary(); } @@ -89,15 +79,15 @@ void LibraryManipulator::booksWereRemovedFromLibrary() //////////////////////////////////////////////////////////////////////////////// /* Constructor */ -Manager::Manager(LibraryManipulator* manipulator): +Manager::Manager(LibraryManipulator manipulator): writableLibraryPath(""), - manipulator(manipulator, NoDelete()) + manipulator(manipulator) { } -Manager::Manager(Library* library) : +Manager::Manager(LibraryPtr library) : writableLibraryPath(""), - manipulator(new LibraryManipulator(library)) + manipulator(LibraryManipulator(library)) { } @@ -121,7 +111,7 @@ bool Manager::parseXmlDom(const pugi::xml_document& doc, if (!trustLibrary && !book.getPath().empty()) { this->readBookFromPath(book.getPath(), &book); } - manipulator->addBookToLibrary(book); + manipulator.addBookToLibrary(book); } return true; @@ -166,7 +156,7 @@ bool Manager::parseOpdsDom(const pugi::xml_document& doc, const std::string& url book.updateFromOpds(entryNode, urlHost); /* Update the book properties with the new importer */ - manipulator->addBookToLibrary(book); + manipulator.addBookToLibrary(book); } return true; @@ -241,7 +231,7 @@ std::string Manager::addBookFromPathAndGetId(const std::string& pathToOpen, || (!book.getTitle().empty() && !book.getLanguages().empty() && !book.getDate().empty())) { book.setUrl(url); - manipulator->addBookToLibrary(book); + manipulator.addBookToLibrary(book); return book.getId(); } } @@ -296,7 +286,7 @@ bool Manager::readBookmarkFile(const std::string& path) bookmark.updateFromXml(node); - manipulator->addBookmarkToLibrary(bookmark); + manipulator.addBookmarkToLibrary(bookmark); } return true; @@ -304,7 +294,7 @@ bool Manager::readBookmarkFile(const std::string& path) void Manager::reload(const Paths& paths) { - const auto libRevision = manipulator->getLibrary().getRevision(); + const auto libRevision = manipulator.getLibrary()->getRevision(); for (std::string path : paths) { if (!path.empty()) { if ( kiwix::isRelativePath(path) ) @@ -316,7 +306,7 @@ void Manager::reload(const Paths& paths) } } - manipulator->removeBooksNotUpdatedSince(libRevision); + manipulator.removeBooksNotUpdatedSince(libRevision); } } diff --git a/src/name_mapper.cpp b/src/name_mapper.cpp index dccf40c9b..bf2a2e4dd 100644 --- a/src/name_mapper.cpp +++ b/src/name_mapper.cpp @@ -63,7 +63,7 @@ std::string HumanReadableNameMapper::getIdForName(const std::string& name) const // UpdatableNameMapper //////////////////////////////////////////////////////////////////////////////// -UpdatableNameMapper::UpdatableNameMapper(Library& lib, bool withAlias) +UpdatableNameMapper::UpdatableNameMapper(LibraryPtr lib, bool withAlias) : library(lib) , withAlias(withAlias) { @@ -72,7 +72,7 @@ UpdatableNameMapper::UpdatableNameMapper(Library& lib, bool withAlias) void UpdatableNameMapper::update() { - const auto newNameMapper = new HumanReadableNameMapper(library, withAlias); + const auto newNameMapper = new HumanReadableNameMapper(*library, withAlias); std::lock_guard lock(mutex); nameMapper.reset(newNameMapper); } diff --git a/include/opds_dumper.h b/src/opds_dumper.h similarity index 100% rename from include/opds_dumper.h rename to src/opds_dumper.h diff --git a/src/search_renderer.cpp b/src/search_renderer.cpp index c47b93368..3b2ae161b 100644 --- a/src/search_renderer.cpp +++ b/src/search_renderer.cpp @@ -36,16 +36,9 @@ namespace kiwix { /* Constructor */ -SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, - unsigned int start, unsigned int estimatedResultCount) - : SearchRenderer(srs, mapper, nullptr, start, estimatedResultCount) -{} - -SearchRenderer::SearchRenderer(zim::SearchResultSet srs, NameMapper* mapper, Library* library, +SearchRenderer::SearchRenderer(zim::SearchResultSet srs, unsigned int start, unsigned int estimatedResultCount) : m_srs(srs), - mp_nameMapper(mapper), - mp_library(library), protocolPrefix("zim://"), searchProtocolPrefix("search://"), estimatedResultCount(estimatedResultCount), @@ -164,7 +157,7 @@ kainjow::mustache::data buildPagination( return pagination; } -std::string SearchRenderer::renderTemplate(const std::string& tmpl_str) +std::string SearchRenderer::renderTemplate(const std::string& tmpl_str, const NameMapper& nameMapper, const Library* library) { const std::string absPathPrefix = protocolPrefix; // Build the results list @@ -172,12 +165,12 @@ std::string SearchRenderer::renderTemplate(const std::string& tmpl_str) for (auto it = m_srs.begin(); it != m_srs.end(); it++) { kainjow::mustache::data result; const std::string zim_id(it.getZimId()); - const auto path = mp_nameMapper->getNameForId(zim_id) + "/" + it.getPath(); + const auto path = nameMapper.getNameForId(zim_id) + "/" + it.getPath(); result.set("title", it.getTitle()); result.set("absolutePath", absPathPrefix + urlEncode(path)); result.set("snippet", it.getSnippet()); - if (mp_library) { - result.set("bookTitle", mp_library->getBookById(zim_id).getTitle()); + if (library) { + result.set("bookTitle", library->getBookById(zim_id).getTitle()); } if (it.getWordCount() >= 0) { result.set("wordCount", kiwix::beautifyInteger(it.getWordCount())); @@ -222,14 +215,14 @@ std::string SearchRenderer::renderTemplate(const std::string& tmpl_str) return ss.str(); } -std::string SearchRenderer::getHtml() +std::string SearchRenderer::getHtml(const NameMapper& mapper, const Library* library) { - return renderTemplate(RESOURCE::templates::search_result_html); + return renderTemplate(RESOURCE::templates::search_result_html, mapper, library); } -std::string SearchRenderer::getXml() +std::string SearchRenderer::getXml(const NameMapper& mapper, const Library* library) { - return renderTemplate(RESOURCE::templates::search_result_xml); + return renderTemplate(RESOURCE::templates::search_result_xml, mapper, library); } diff --git a/src/server.cpp b/src/server.cpp index e9373417c..1a4ecfa9e 100644 --- a/src/server.cpp +++ b/src/server.cpp @@ -29,7 +29,7 @@ namespace kiwix { -Server::Server(Library* library, NameMapper* nameMapper) : +Server::Server(LibraryPtr library, std::shared_ptr nameMapper) : mp_library(library), mp_nameMapper(nameMapper), mp_server(nullptr) diff --git a/src/server/internalServer.cpp b/src/server/internalServer.cpp index 544f7e31f..c8446cb17 100644 --- a/src/server/internalServer.cpp +++ b/src/server/internalServer.cpp @@ -254,6 +254,11 @@ get_matching_if_none_match_etag(const RequestContext& r, const std::string& etag } } +struct NoDelete +{ + template void operator()(T*) {} +}; + } // unnamed namespace std::pair InternalServer::selectBooks(const RequestContext& request) const @@ -406,8 +411,8 @@ public: }; -InternalServer::InternalServer(Library* library, - NameMapper* nameMapper, +InternalServer::InternalServer(LibraryPtr library, + std::shared_ptr nameMapper, std::string addr, int port, std::string root, @@ -433,7 +438,7 @@ InternalServer::InternalServer(Library* library, m_ipConnectionLimit(ipConnectionLimit), mp_daemon(nullptr), mp_library(library), - mp_nameMapper(nameMapper ? nameMapper : &defaultNameMapper), + mp_nameMapper(nameMapper ? nameMapper : std::shared_ptr(&defaultNameMapper, NoDelete())), searchCache(getEnvVar("KIWIX_SEARCH_CACHE_SIZE", DEFAULT_CACHE_SIZE)), suggestionSearcherCache(getEnvVar("KIWIX_SUGGESTION_SEARCHER_CACHE_SIZE", std::max((unsigned int) (mp_library->getBookCount(true, true)*0.1), 1U))), m_customizedResources(new CustomizedResources) @@ -787,7 +792,7 @@ std::unique_ptr InternalServer::handle_no_js(const RequestContext& req { const auto url = request.get_url(); const auto urlParts = kiwix::split(url, "/", true, false); - HTMLDumper htmlDumper(mp_library, mp_nameMapper); + HTMLDumper htmlDumper(mp_library.get(), mp_nameMapper.get()); htmlDumper.setRootLocation(m_root); htmlDumper.setLibraryId(getLibraryId()); auto userLang = request.get_user_language(); @@ -958,7 +963,7 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon const auto pageLength = getSearchPageSize(request); /* Get the results */ - SearchRenderer renderer(search->getResults(start-1, pageLength), mp_nameMapper, mp_library, start, + SearchRenderer renderer(search->getResults(start-1, pageLength), start, search->getEstimatedMatches()); renderer.setSearchPattern(searchInfo.pattern); renderer.setSearchBookQuery(searchInfo.bookFilterQuery); @@ -966,9 +971,17 @@ std::unique_ptr InternalServer::handle_search_request(const RequestCon renderer.setSearchProtocolPrefix(m_root + "/search"); renderer.setPageLength(pageLength); if (request.get_requested_format() == "xml") { - return ContentResponse::build(*this, renderer.getXml(), "application/rss+xml; charset=utf-8"); + return ContentResponse::build( + *this, + renderer.getXml(*mp_nameMapper, mp_library.get()), + "application/rss+xml; charset=utf-8" + ); } - auto response = ContentResponse::build(*this, renderer.getHtml(), "text/html; charset=utf-8"); + auto response = ContentResponse::build( + *this, + renderer.getHtml(*mp_nameMapper, mp_library.get()), + "text/html; charset=utf-8" + ); // XXX: Now this has to be handled by the iframe-based viewer which // XXX: has to resolve if the book selection resulted in a single book. /* diff --git a/src/server/internalServer.h b/src/server/internalServer.h index 867b324e0..cdd7cebe2 100644 --- a/src/server/internalServer.h +++ b/src/server/internalServer.h @@ -92,8 +92,8 @@ class OPDSDumper; class InternalServer { public: - InternalServer(Library* library, - NameMapper* nameMapper, + InternalServer(LibraryPtr library, + std::shared_ptr nameMapper, std::string addr, int port, std::string root, @@ -178,8 +178,8 @@ class InternalServer { int m_ipConnectionLimit; struct MHD_Daemon* mp_daemon; - Library* mp_library; - NameMapper* mp_nameMapper; + LibraryPtr mp_library; + std::shared_ptr mp_nameMapper; SearchCache searchCache; SuggestionSearcherCache suggestionSearcherCache; diff --git a/src/server/internalServer_catalog.cpp b/src/server/internalServer_catalog.cpp index a43d968e9..fa50450a9 100644 --- a/src/server/internalServer_catalog.cpp +++ b/src/server/internalServer_catalog.cpp @@ -82,7 +82,7 @@ std::unique_ptr InternalServer::handle_catalog(const RequestContext& r } zim::Uuid uuid; - kiwix::OPDSDumper opdsDumper(mp_library, mp_nameMapper); + kiwix::OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); std::vector bookIdsToDump; @@ -164,7 +164,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_root(const RequestCo std::unique_ptr InternalServer::handle_catalog_v2_entries(const RequestContext& request, bool partial) { - OPDSDumper opdsDumper(mp_library, mp_nameMapper); + OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); const auto bookIds = search_catalog(request, opdsDumper); @@ -185,7 +185,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const + urlNotFoundMsg; } - OPDSDumper opdsDumper(mp_library, mp_nameMapper); + OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); const auto opdsFeed = opdsDumper.dumpOPDSCompleteEntry(entryId); @@ -198,7 +198,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_complete_entry(const std::unique_ptr InternalServer::handle_catalog_v2_categories(const RequestContext& request) { - OPDSDumper opdsDumper(mp_library, mp_nameMapper); + OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( @@ -210,7 +210,7 @@ std::unique_ptr InternalServer::handle_catalog_v2_categories(const Req std::unique_ptr InternalServer::handle_catalog_v2_languages(const RequestContext& request) { - OPDSDumper opdsDumper(mp_library, mp_nameMapper); + OPDSDumper opdsDumper(mp_library.get(), mp_nameMapper.get()); opdsDumper.setRootLocation(m_root); opdsDumper.setLibraryId(getLibraryId()); return ContentResponse::build( diff --git a/test/library.cpp b/test/library.cpp index 4fb4b7dd1..30a77fcff 100644 --- a/test/library.cpp +++ b/test/library.cpp @@ -238,14 +238,14 @@ typedef std::vector Langs; TEST(LibraryOpdsImportTest, allInOne) { - kiwix::Library lib; - kiwix::Manager manager(&lib); + auto lib = kiwix::Library::create(); + kiwix::Manager manager(lib); manager.readOpds(sampleOpdsStream, "library-opds-import.unittests.dev"); - EXPECT_EQ(10U, lib.getBookCount(true, true)); + EXPECT_EQ(10U, lib->getBookCount(true, true)); { - const kiwix::Book& book1 = lib.getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd"); + const kiwix::Book& book1 = lib->getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd"); EXPECT_EQ(book1.getTitle(), "Encyclopédie de la Tunisie"); EXPECT_EQ(book1.getName(), "wikipedia_fr_tunisie_novid_2018-10"); @@ -271,7 +271,7 @@ TEST(LibraryOpdsImportTest, allInOne) } { - const kiwix::Book& book2 = lib.getBookById("0189d9be-2fd0-b4b6-7300-20fab0b5cdc8"); + const kiwix::Book& book2 = lib->getBookById("0189d9be-2fd0-b4b6-7300-20fab0b5cdc8"); EXPECT_EQ(book2.getTitle(), "TED talks - Business"); EXPECT_EQ(book2.getName(), ""); EXPECT_EQ(book2.getFlavour(), ""); @@ -301,8 +301,10 @@ class LibraryTest : public ::testing::Test { typedef kiwix::Library::BookIdCollection BookIdCollection; typedef std::vector TitleCollection; + LibraryTest(): lib(kiwix::Library::create()) {} + void SetUp() override { - kiwix::Manager manager(&lib); + kiwix::Manager manager(lib); manager.readOpds(sampleOpdsStream, "foo.urlHost"); manager.readXml(sampleLibraryXML, false, "./test/library.xml", true); } @@ -316,25 +318,25 @@ class LibraryTest : public ::testing::Test { TitleCollection ids2Titles(const BookIdCollection& ids) { TitleCollection titles; for ( const auto& bookId : ids ) { - titles.push_back(lib.getBookById(bookId).getTitle()); + titles.push_back(lib->getBookById(bookId).getTitle()); } std::sort(titles.begin(), titles.end()); return titles; } - kiwix::Library lib; + std::shared_ptr lib; }; TEST_F(LibraryTest, getBookMarksTest) { - auto bookId1 = lib.getBooksIds()[0]; - auto bookId2 = lib.getBooksIds()[1]; + auto bookId1 = lib->getBooksIds()[0]; + auto bookId2 = lib->getBooksIds()[1]; - lib.addBookmark(createBookmark(bookId1)); - lib.addBookmark(createBookmark("invalid-bookmark-id")); - lib.addBookmark(createBookmark(bookId2)); - auto onlyValidBookmarks = lib.getBookmarks(); - auto allBookmarks = lib.getBookmarks(false); + lib->addBookmark(createBookmark(bookId1)); + lib->addBookmark(createBookmark("invalid-bookmark-id")); + lib->addBookmark(createBookmark(bookId2)); + auto onlyValidBookmarks = lib->getBookmarks(); + auto allBookmarks = lib->getBookmarks(false); EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1); EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2); @@ -346,11 +348,11 @@ TEST_F(LibraryTest, getBookMarksTest) TEST_F(LibraryTest, sanityCheck) { - EXPECT_EQ(lib.getBookCount(true, true), 12U); - EXPECT_EQ(lib.getBooksLanguages(), + EXPECT_EQ(lib->getBookCount(true, true), 12U); + EXPECT_EQ(lib->getBooksLanguages(), std::vector({"deu", "eng", "fra", "ita", "spa"}) ); - EXPECT_EQ(lib.getBooksCreators(), std::vector({ + EXPECT_EQ(lib->getBooksCreators(), std::vector({ "Islam Stack Exchange", "Movies & TV Stack Exchange", "Mythology & Folklore Stack Exchange", @@ -361,7 +363,7 @@ TEST_F(LibraryTest, sanityCheck) "Wikipedia", "Wikiquote" })); - EXPECT_EQ(lib.getBooksPublishers(), std::vector({ + EXPECT_EQ(lib->getBooksPublishers(), std::vector({ "", "Kiwix", "Kiwix & Some Enthusiasts", @@ -371,22 +373,22 @@ TEST_F(LibraryTest, sanityCheck) TEST_F(LibraryTest, categoryHandling) { - EXPECT_EQ("", lib.getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd").getCategory()); - EXPECT_EQ("category_defined_via_tags_only", lib.getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2").getCategory()); - EXPECT_EQ("category_defined_via_category_element_only", lib.getBookById("0ea1cde6-441d-6c58-f2c7-21c2838e659f").getCategory()); - EXPECT_EQ("category_element_overrides_tags", lib.getBookById("1123e574-6eef-6d54-28fc-13e4caeae474").getCategory()); - EXPECT_EQ("category_element_overrides_tags", lib.getBookById("14829621-c490-c376-0792-9de558b57efa").getCategory()); + EXPECT_EQ("", lib->getBookById("0c45160e-f917-760a-9159-dfe3c53cdcdd").getCategory()); + EXPECT_EQ("category_defined_via_tags_only", lib->getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2").getCategory()); + EXPECT_EQ("category_defined_via_category_element_only", lib->getBookById("0ea1cde6-441d-6c58-f2c7-21c2838e659f").getCategory()); + EXPECT_EQ("category_element_overrides_tags", lib->getBookById("1123e574-6eef-6d54-28fc-13e4caeae474").getCategory()); + EXPECT_EQ("category_element_overrides_tags", lib->getBookById("14829621-c490-c376-0792-9de558b57efa").getCategory()); } TEST_F(LibraryTest, emptyFilter) { - const auto bookIds = lib.filter(kiwix::Filter()); - EXPECT_EQ(bookIds, lib.getBooksIds()); + const auto bookIds = lib->filter(kiwix::Filter()); + EXPECT_EQ(bookIds, lib->getBooksIds()); } #define EXPECT_FILTER_RESULTS(f, ...) \ EXPECT_EQ( \ - ids2Titles(lib.filter(f)), \ + ids2Titles(lib->filter(f)), \ TitleCollection({ __VA_ARGS__ }) \ ) @@ -736,33 +738,33 @@ TEST_F(LibraryTest, filterByMultipleCriteria) TEST_F(LibraryTest, getBookByPath) { - kiwix::Book book = lib.getBookById(lib.getBooksIds()[0]); + kiwix::Book book = lib->getBookById(lib->getBooksIds()[0]); #ifdef _WIN32 auto path = "C:\\some\\abs\\path.zim"; #else auto path = "/some/abs/path.zim"; #endif book.setPath(path); - lib.addBook(book); - EXPECT_EQ(lib.getBookByPath(path).getId(), book.getId()); - EXPECT_THROW(lib.getBookByPath("non/existant/path.zim"), std::out_of_range); + lib->addBook(book); + EXPECT_EQ(lib->getBookByPath(path).getId(), book.getId()); + EXPECT_THROW(lib->getBookByPath("non/existant/path.zim"), std::out_of_range); } TEST_F(LibraryTest, removeBookByIdRemovesTheBook) { - const auto initialBookCount = lib.getBookCount(true, true); + const auto initialBookCount = lib->getBookCount(true, true); ASSERT_GT(initialBookCount, 0U); - EXPECT_NO_THROW(lib.getBookById("raycharles")); - lib.removeBookById("raycharles"); - EXPECT_EQ(initialBookCount - 1, lib.getBookCount(true, true)); - EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range); + EXPECT_NO_THROW(lib->getBookById("raycharles")); + lib->removeBookById("raycharles"); + EXPECT_EQ(initialBookCount - 1, lib->getBookCount(true, true)); + EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range); }; TEST_F(LibraryTest, removeBookByIdDropsTheReader) { - EXPECT_NE(nullptr, lib.getArchiveById("raycharles")); - lib.removeBookById("raycharles"); - EXPECT_THROW(lib.getArchiveById("raycharles"), std::out_of_range); + EXPECT_NE(nullptr, lib->getArchiveById("raycharles")); + lib->removeBookById("raycharles"); + EXPECT_THROW(lib->getArchiveById("raycharles"), std::out_of_range); }; TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB) @@ -770,17 +772,17 @@ TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB) kiwix::Filter f; f.local(true).valid(true).query(R"(title:"ray charles")", false); - EXPECT_NO_THROW(lib.getBookById("raycharles")); - EXPECT_EQ(1U, lib.filter(f).size()); + EXPECT_NO_THROW(lib->getBookById("raycharles")); + EXPECT_EQ(1U, lib->filter(f).size()); - lib.removeBookById("raycharles"); + lib->removeBookById("raycharles"); - EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range); - EXPECT_EQ(0U, lib.filter(f).size()); + EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range); + EXPECT_EQ(0U, lib->filter(f).size()); // make sure that Library::filter() doesn't add an empty book with // an id surviving in the search DB - EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range); + EXPECT_THROW(lib->getBookById("raycharles"), std::out_of_range); }; TEST_F(LibraryTest, removeBooksNotUpdatedSince) @@ -800,18 +802,18 @@ TEST_F(LibraryTest, removeBooksNotUpdatedSince) "Wikiquote" ); - const uint64_t rev = lib.getRevision(); - for ( const auto& id : lib.filter(kiwix::Filter().query("exchange")) ) { - lib.addBook(lib.getBookByIdThreadSafe(id)); + const uint64_t rev = lib->getRevision(); + for ( const auto& id : lib->filter(kiwix::Filter().query("exchange")) ) { + lib->addBook(lib->getBookByIdThreadSafe(id)); } - EXPECT_GT(lib.getRevision(), rev); + EXPECT_GT(lib->getRevision(), rev); - const uint64_t rev2 = lib.getRevision(); + const uint64_t rev2 = lib->getRevision(); - EXPECT_EQ(9u, lib.removeBooksNotUpdatedSince(rev)); + EXPECT_EQ(9u, lib->removeBooksNotUpdatedSince(rev)); - EXPECT_GT(lib.getRevision(), rev2); + EXPECT_GT(lib->getRevision(), rev2); EXPECT_FILTER_RESULTS(kiwix::Filter(), "Islam Stack Exchange", diff --git a/test/manager.cpp b/test/manager.cpp index e2b600644..47f1edf86 100644 --- a/test/manager.cpp +++ b/test/manager.cpp @@ -8,18 +8,18 @@ TEST(ManagerTest, addBookFromPathAndGetIdTest) { - kiwix::Library lib; - kiwix::Manager manager = kiwix::Manager(&lib); + auto lib = kiwix::Library::create(); + kiwix::Manager manager = kiwix::Manager(lib); auto bookId = manager.addBookFromPathAndGetId("./test/example.zim"); ASSERT_NE(bookId, ""); - kiwix::Book book = lib.getBookById(bookId); + kiwix::Book book = lib->getBookById(bookId); EXPECT_EQ(book.getPath(), kiwix::computeAbsolutePath("", "./test/example.zim")); const std::string pathToSave = "./pathToSave"; const std::string url = "url"; bookId = manager.addBookFromPathAndGetId("./test/example.zim", pathToSave, url, true); - book = lib.getBookById(bookId); + book = lib->getBookById(bookId); auto savedPath = kiwix::computeAbsolutePath(kiwix::removeLastPathElement(manager.writableLibraryPath), pathToSave); EXPECT_EQ(book.getPath(), savedPath); EXPECT_EQ(book.getUrl(), url); @@ -48,11 +48,11 @@ const char sampleLibraryXML[] = R"( TEST(ManagerTest, readXml) { - kiwix::Library lib; - kiwix::Manager manager = kiwix::Manager(&lib); + auto lib = kiwix::Library::create(); + kiwix::Manager manager = kiwix::Manager(lib); EXPECT_EQ(true, manager.readXml(sampleLibraryXML, true, "/data/lib.xml", true)); - kiwix::Book book = lib.getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2"); + kiwix::Book book = lib->getBookById("0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2"); EXPECT_EQ("/data/zimfiles/unittest.zim", book.getPath()); EXPECT_EQ("https://example.com/zimfiles/unittest.zim", book.getUrl()); EXPECT_EQ("Unit Test", book.getTitle()); @@ -70,24 +70,24 @@ TEST(ManagerTest, readXml) TEST(Manager, reload) { - kiwix::Library lib; - kiwix::Manager manager(&lib); + auto lib = kiwix::Library::create(); + kiwix::Manager manager(lib); manager.reload({ "./test/library.xml" }); - EXPECT_EQ(lib.getBooksIds(), (kiwix::Library::BookIdCollection{ + EXPECT_EQ(lib->getBooksIds(), (kiwix::Library::BookIdCollection{ "charlesray", "raycharles", "raycharles_uncategorized" })); - lib.removeBookById("raycharles"); - EXPECT_EQ(lib.getBooksIds(), (kiwix::Library::BookIdCollection{ + lib->removeBookById("raycharles"); + EXPECT_EQ(lib->getBooksIds(), (kiwix::Library::BookIdCollection{ "charlesray", "raycharles_uncategorized" })); manager.reload({ "./test/library.xml" }); - EXPECT_EQ(lib.getBooksIds(), kiwix::Library::BookIdCollection({ + EXPECT_EQ(lib->getBooksIds(), kiwix::Library::BookIdCollection({ "charlesray", "raycharles", "raycharles_uncategorized" diff --git a/test/name_mapper.cpp b/test/name_mapper.cpp index 0bc60254a..4396356e8 100644 --- a/test/name_mapper.cpp +++ b/test/name_mapper.cpp @@ -18,18 +18,20 @@ const char libraryXML[] = R"( )"; class NameMapperTest : public ::testing::Test { + public: + NameMapperTest(): lib(kiwix::Library::create()) {} protected: void SetUp() override { - kiwix::Manager manager(&lib); + kiwix::Manager manager(lib); manager.readXml(libraryXML, false, "./library.xml", true); - for ( const std::string& id : lib.getBooksIds() ) { - kiwix::Book bookCopy = lib.getBookById(id); + for ( const std::string& id : lib->getBooksIds() ) { + kiwix::Book bookCopy = lib->getBookById(id); bookCopy.setPathValid(true); - lib.addBook(bookCopy); + lib->addBook(bookCopy); } } - kiwix::Library lib; + std::shared_ptr lib; }; class CapturedStderr @@ -73,13 +75,13 @@ void checkUnaliasedEntriesInNameMapper(const kiwix::NameMapper& nm) TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases) { CapturedStderr stderror; - kiwix::HumanReadableNameMapper nm(lib, false); + kiwix::HumanReadableNameMapper nm(*lib, false); EXPECT_EQ("", std::string(stderror)); checkUnaliasedEntriesInNameMapper(nm); EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range); - lib.removeBookById("04-2021-10"); + lib->removeBookById("04-2021-10"); EXPECT_EQ("zero_four_2021-10", nm.getNameForId("04-2021-10")); EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four_2021-10")); EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range); @@ -88,7 +90,7 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases) TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases) { CapturedStderr stderror; - kiwix::HumanReadableNameMapper nm(lib, true); + kiwix::HumanReadableNameMapper nm(*lib, true); EXPECT_EQ( "Path collision: /data/zero_four_2021-10.zim and" " /data/zero_four_2021-11.zim can't share the same URL path 'zero_four'." @@ -99,7 +101,7 @@ TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases) checkUnaliasedEntriesInNameMapper(nm); EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four")); - lib.removeBookById("04-2021-10"); + lib->removeBookById("04-2021-10"); EXPECT_EQ("zero_four_2021-10", nm.getNameForId("04-2021-10")); EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four_2021-10")); EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four")); @@ -114,7 +116,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases) checkUnaliasedEntriesInNameMapper(nm); EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range); - lib.removeBookById("04-2021-10"); + lib->removeBookById("04-2021-10"); nm.update(); EXPECT_THROW(nm.getNameForId("04-2021-10"), std::out_of_range); EXPECT_THROW(nm.getIdForName("zero_four_2021-10"), std::out_of_range); @@ -137,7 +139,7 @@ TEST_F(NameMapperTest, UpdatableNameMapperWithAliases) { CapturedStderr nmUpdateStderror; - lib.removeBookById("04-2021-10"); + lib->removeBookById("04-2021-10"); nm.update(); EXPECT_EQ("", std::string(nmUpdateStderror)); } diff --git a/test/server_testing_tools.h b/test/server_testing_tools.h index 777bb48e8..3ea10ab57 100644 --- a/test/server_testing_tools.h +++ b/test/server_testing_tools.h @@ -98,16 +98,17 @@ private: void run(int serverPort, std::string indexTemplateString = ""); private: // data - kiwix::Library library; + std::shared_ptr library; kiwix::Manager manager; - std::unique_ptr nameMapper; + std::shared_ptr nameMapper; std::unique_ptr server; std::unique_ptr client; const Cfg cfg; }; ZimFileServer::ZimFileServer(int serverPort, Cfg _cfg, std::string libraryFilePath) -: manager(&this->library) +: library(kiwix::Library::create()) +, manager(this->library) , cfg(_cfg) { if ( kiwix::isRelativePath(libraryFilePath) ) @@ -120,7 +121,8 @@ ZimFileServer::ZimFileServer(int serverPort, Cfg _cfg, const FilePathCollection& zimpaths, std::string indexTemplateString) -: manager(&this->library) +: library(kiwix::Library::create()) +, manager(this->library) , cfg(_cfg) { for ( const auto& zimpath : zimpaths ) { @@ -136,9 +138,9 @@ void ZimFileServer::run(int serverPort, std::string indexTemplateString) if (cfg.options & NO_NAME_MAPPER) { nameMapper.reset(new kiwix::IdNameMapper()); } else { - nameMapper.reset(new kiwix::HumanReadableNameMapper(library, false)); + nameMapper.reset(new kiwix::HumanReadableNameMapper(*library, false)); } - server.reset(new kiwix::Server(&library, nameMapper.get())); + server.reset(new kiwix::Server(library, nameMapper)); server->setRoot(cfg.root); server->setAddress(address); server->setPort(serverPort);