Move the `Library` mutex in `Library::Impl`.

The why of this mutex is in `Library` is a bit complex.
It has been introduced in c2927ce when there was only `Library` and no
`std::unique_ptr<Impl>`.
As introducing the mutex imply implementing the move constructor, we have
split all data in `LibraryBase` (and keep a default move constructor here)
and add the mutex in `Library` (and implement a simple move constructor).

Later, in 090c2fd, we have move the `LibraryBase` to `Library::Impl`
(which should have been `Library::Data`).

So at the end, `Library::Impl` is never moved. We can move the `mutex` in
it and still simply implement move constructor for `Library`.

include/library.h

@@ -178,9 +178,6 @@ class ZimSearcher : public zim::Searcher
  */
 class Library: public std::enable_shared_from_this<Library>
 {
-  // all data fields must be added in LibraryBase
-  mutable std::mutex m_mutex;
-
  public:
   typedef uint64_t Revision;
   typedef std::vector<std::string> BookIdCollection;

src/library.cpp

@@ -88,6 +88,7 @@ struct Library::Impl
     Library::Revision lastUpdatedRevision = 0;
   };
 
+  mutable std::mutex m_mutex;
   Library::Revision m_revision;
   std::map<std::string, Entry> m_books;
   using ArchiveCache = ConcurrentCache<std::string, std::shared_ptr<zim::Archive>>;
@@ -102,8 +103,8 @@ struct Library::Impl
   Impl();
   ~Impl();
 
-  Impl(Impl&& );
+  Impl(Impl&& ) = delete;
-  Impl& operator=(Impl&& );
+  Impl& operator=(Impl&& ) = delete;
 };
 
 Library::Impl::Impl()
@@ -117,9 +118,6 @@ 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
 {
@@ -156,7 +154,7 @@ Library::~Library() = default;
 
 bool Library::addBook(const Book& book)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   ++mp_impl->m_revision;
   /* Try to find it */
   updateBookDB(book);
@@ -187,13 +185,13 @@ bool Library::addBook(const Book& book)
 
 void Library::addBookmark(const Bookmark& bookmark)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   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);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   for(auto it=mp_impl->m_bookmarks.begin(); it!=mp_impl->m_bookmarks.end(); it++) {
     if (it->getBookId() == zimId && it->getUrl() == url) {
       mp_impl->m_bookmarks.erase(it);
@@ -212,7 +210,7 @@ void Library::dropCache(const std::string& id)
 
 bool Library::removeBookById(const std::string& id)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   mp_impl->m_bookDB.delete_document("Q" + id);
   dropCache(id);
   // We do not change the cache size here
@@ -230,7 +228,7 @@ bool Library::removeBookById(const std::string& id)
 
 Library::Revision Library::getRevision() const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   return mp_impl->m_revision;
 }
 
@@ -238,7 +236,7 @@ uint32_t Library::removeBooksNotUpdatedSince(Revision libraryRevision)
 {
   BookIdCollection booksToRemove;
   {
-    std::lock_guard<std::mutex> lock(m_mutex);
+    std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
     for ( const auto& entry : mp_impl->m_books) {
       if ( entry.second.lastUpdatedRevision <= libraryRevision ) {
         booksToRemove.push_back(entry.first);
@@ -263,7 +261,7 @@ const Book& Library::getBookById(const std::string& id) const
 
 Book Library::getBookByIdThreadSafe(const std::string& id) const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   return getBookById(id);
 }
 
@@ -321,7 +319,7 @@ std::shared_ptr<ZimSearcher> Library::getSearcherByIds(const BookIdSet& ids)
 unsigned int Library::getBookCount(const bool localBooks,
                                    const bool remoteBooks) const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   return mp_impl->getBookCount(localBooks, remoteBooks);
 }
 
@@ -334,7 +332,7 @@ bool Library::writeToFile(const std::string& path) const
   dumper.setBaseDir(baseDir);
   std::string xml;
   {
-    std::lock_guard<std::mutex> lock(m_mutex);
+    std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
     xml = dumper.dumpLibXMLContent(allBookIds);
   };
   return writeTextFile(path, xml);
@@ -350,7 +348,7 @@ bool Library::writeBookmarksToFile(const std::string& path) const
 
 Library::AttributeCounts Library::getBookAttributeCounts(BookStrPropMemFn p) const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   AttributeCounts propValueCounts;
 
   for (const auto& pair: mp_impl->m_books) {
@@ -382,7 +380,7 @@ std::vector<std::string> Library::getBooksLanguages() const
 
 Library::AttributeCounts Library::getBooksLanguagesWithCounts() const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   AttributeCounts langsWithCounts;
 
   for (const auto& pair: mp_impl->m_books) {
@@ -398,7 +396,7 @@ Library::AttributeCounts Library::getBooksLanguagesWithCounts() const
 
 std::vector<std::string> Library::getBooksCategories() const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   std::set<std::string> categories;
 
   for (const auto& pair: mp_impl->m_books) {
@@ -429,7 +427,7 @@ const std::vector<kiwix::Bookmark> Library::getBookmarks(bool onlyValidBookmarks
   }
   std::vector<kiwix::Bookmark> validBookmarks;
   auto booksId = getBooksIds();
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   for(auto& bookmark:mp_impl->m_bookmarks) {
     if (std::find(booksId.begin(), booksId.end(), bookmark.getBookId()) != booksId.end()) {
       validBookmarks.push_back(bookmark);
@@ -440,7 +438,7 @@ const std::vector<kiwix::Bookmark> Library::getBookmarks(bool onlyValidBookmarks
 
 Library::BookIdCollection Library::getBooksIds() const
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   BookIdCollection bookIds;
 
   for (auto& pair: mp_impl->m_books) {
@@ -646,7 +644,7 @@ Library::BookIdCollection Library::filterViaBookDB(const Filter& filter) const
 
   BookIdCollection bookIds;
 
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   Xapian::Enquire enquire(mp_impl->m_bookDB);
   enquire.set_query(query);
   const auto results = enquire.get_mset(0, mp_impl->m_books.size());
@@ -661,7 +659,7 @@ Library::BookIdCollection Library::filter(const Filter& filter) const
 {
   BookIdCollection result;
   const auto preliminaryResult = filterViaBookDB(filter);
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   for(auto id : preliminaryResult) {
     if(filter.accept(mp_impl->m_books.at(id))) {
       result.push_back(id);
@@ -735,7 +733,7 @@ void Library::sort(BookIdCollection& bookIds, supportedListSortBy sort, bool asc
   // NOTE: for the entire duration of the sort. Will need to obtain (under a
   // NOTE: lock) the required atributes from the books once, and then the
   // NOTE: sorting will run on a copy of data without locking.
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::mutex> lock(mp_impl->m_mutex);
   switch(sort) {
     case TITLE:
       std::sort(bookIds.begin(), bookIds.end(), Comparator<TITLE>(this, ascending));