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/src/library.cpp b/src/library.cpp index bf62c4183..a1b66668a 100644 --- a/src/library.cpp +++ b/src/library.cpp @@ -58,66 +58,92 @@ bool booksReferToTheSameArchive(const Book& book1, const Book& book2) } // unnamed namespace -class LibraryBase::BookDB : public Xapian::WritableDatabase +struct Library::Impl +{ + 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; + class BookDB; + std::unique_ptr m_bookDB; + + Impl(); + ~Impl(); + + Impl(Impl&& ); + Impl& operator=(Impl&& ); +}; + + +class Library::Impl::BookDB : public Xapian::WritableDatabase { public: BookDB() : Xapian::WritableDatabase("", Xapian::DB_BACKEND_INMEMORY) {} }; -LibraryBase::LibraryBase() +Library::Impl::Impl() : m_bookDB(new BookDB) { } -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; + + /* 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 +151,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 +169,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 +211,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 +224,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 +238,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 +247,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 +255,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 +263,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; } @@ -246,7 +272,7 @@ unsigned int Library::getBookCount(const bool localBooks, { std::lock_guard lock(m_mutex); unsigned int result = 0; - for (auto& pair: m_books) { + for (auto& pair: mp_impl->m_books) { auto& book = pair.second; if ((!book.getPath().empty() && localBooks) || (book.getPath().empty() && remoteBooks)) { @@ -284,7 +310,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 +343,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 +367,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 +385,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 +431,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 +564,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 +580,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); } }