Move LibraryBase out of public API.

We use composition instead of inheritance to implement Library.
This commit is contained in:
Matthieu Gautier 2022-03-21 17:25:58 +01:00
parent ff2c7b1fb2
commit 090c2fd31a
2 changed files with 76 additions and 81 deletions

View File

@ -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<std::string, Entry> m_books;
std::map<std::string, std::shared_ptr<Reader>> m_readers;
std::map<std::string, std::shared_ptr<zim::Archive>> m_archives;
std::vector<kiwix::Bookmark> m_bookmarks;
class BookDB;
std::unique_ptr<BookDB> 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<std::string> BookIdCollection;
typedef std::map<std::string, int> 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<Impl> mp_impl;
};
}

View File

@ -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<std::string, Entry> m_books;
std::map<std::string, std::shared_ptr<Reader>> m_readers;
std::map<std::string, std::shared_ptr<zim::Archive>> m_archives;
std::vector<kiwix::Bookmark> m_bookmarks;
class BookDB;
std::unique_ptr<BookDB> 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<std::mutex> 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<Book&>(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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<Reader> Library::getReaderById(const std::string& id)
{
try {
std::lock_guard<std::mutex> 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<Reader> Library::getReaderById(const std::string& id)
const shared_ptr<Reader> reader(new Reader(archive, true));
std::lock_guard<std::mutex> lock(m_mutex);
m_readers[id] = reader;
mp_impl->m_readers[id] = reader;
return reader;
}
@ -229,7 +255,7 @@ std::shared_ptr<zim::Archive> Library::getArchiveById(const std::string& id)
{
std::lock_guard<std::mutex> 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<zim::Archive> Library::getArchiveById(const std::string& id)
return nullptr;
auto sptr = make_shared<zim::Archive>(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<std::mutex> 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<std::mutex> 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<std::string> Library::getBooksCategories() const
std::lock_guard<std::mutex> lock(m_mutex);
std::set<std::string> 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<std::string> Library::getBooksPublishers() const
const std::vector<kiwix::Bookmark> Library::getBookmarks(bool onlyValidBookmarks) const
{
if (!onlyValidBookmarks) {
return m_bookmarks;
return mp_impl->m_bookmarks;
}
std::vector<kiwix::Bookmark> validBookmarks;
auto booksId = getBooksIds();
std::lock_guard<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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);
}
}