Merge pull request #636 from kiwix/library_reloading

include/library.h

@@ -24,6 +24,7 @@
 #include <vector>
 #include <map>
 #include <memory>
+#include <mutex>
 #include <zim/archive.h>
 
 #include "book.h"
@@ -139,20 +140,51 @@ private: // functions
     bool accept(const Book& book) const;
 };
 
-
 /**
- * A Library store several books.
+ * 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 Library
+class LibraryBase
 {
-  std::map<std::string, kiwix::Book> m_books;
+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
+{
+  // all data fields must be added in LibraryBase
+  mutable std::mutex m_mutex;
+
  public:
+  typedef LibraryRevision Revision;
   typedef std::vector<std::string> BookIdCollection;
   typedef std::map<std::string, int> AttributeCounts;
 
@@ -180,6 +212,11 @@ class Library
    */
   bool addBook(const Book& book);
 
+  /**
+   * A self-explanatory alias for addBook()
+   */
+  bool addOrUpdateBook(const Book& book) { return addBook(book); }
+
   /**
    * Add a bookmark to the library.
    *
@@ -196,10 +233,13 @@ class Library
    */
   bool removeBookmark(const std::string& zimId, const std::string& url);
 
+  // XXX: This is a non-thread-safe operation
   const Book& getBookById(const std::string& id) const;
-  Book& getBookById(const std::string& id);
+  // XXX: This is a non-thread-safe operation
   const Book& getBookByPath(const std::string& path) const;
-  Book& getBookByPath(const std::string& path);
+
+  Book getBookByIdThreadSafe(const std::string& id) const;
+
   std::shared_ptr<Reader> getReaderById(const std::string& id);
   std::shared_ptr<zim::Archive> getArchiveById(const std::string& id);
 
@@ -346,6 +386,24 @@ class Library
     const std::vector<std::string>& tags = {},
     size_t maxSize = 0) const;
 
+  /**
+   * Return the current revision of the library.
+   *
+   * The revision of the library is updated (incremented by one) only by
+   * the addBook() operation.
+   *
+   * @return Current revision of the library.
+   */
+  LibraryRevision getRevision() const;
+
+  /**
+   * Remove books that have not been updated since the specified revision.
+   *
+   * @param  rev the library revision to use
+   * @return Count of books that were removed by this operation.
+   */
+  uint32_t removeBooksNotUpdatedSince(LibraryRevision rev);
+
   friend class OPDSDumper;
   friend class libXMLDumper;
 
@@ -357,6 +415,7 @@ private: // functions
   std::vector<std::string> getBookPropValueSet(BookStrPropMemFn p) const;
   BookIdCollection filterViaBookDB(const Filter& filter) const;
   void updateBookDB(const Book& book);
+  void dropReader(const std::string& bookId);
 };
 
 }

include/manager.h

@@ -26,6 +26,7 @@
 
 #include <string>
 #include <vector>
+#include <memory>
 
 namespace pugi {
 class xml_document;
@@ -34,26 +35,25 @@ class xml_document;
 namespace kiwix
 {
 
-class LibraryManipulator {
+class LibraryManipulator
- public:
+{
-  virtual ~LibraryManipulator() {}
+ public: // functions
-  virtual bool addBookToLibrary(Book book) = 0;
+  explicit LibraryManipulator(Library* library);
-  virtual void addBookmarkToLibrary(Bookmark bookmark) = 0;
+  virtual ~LibraryManipulator();
-};
 
-class DefaultLibraryManipulator : public LibraryManipulator {
+  Library& getLibrary() const { return library; }
- public:
+
-  DefaultLibraryManipulator(Library* library) :
+  bool addBookToLibrary(const Book& book);
-    library(library) {}
+  void addBookmarkToLibrary(const Bookmark& bookmark);
-  virtual ~DefaultLibraryManipulator() {}
+  uint32_t removeBooksNotUpdatedSince(Library::Revision rev);
-  bool addBookToLibrary(Book book) {
+
-    return library->addBook(book);
+ protected: // overrides
-  }
+  virtual void bookWasAddedToLibrary(const Book& book);
-  void addBookmarkToLibrary(Bookmark bookmark) {
+  virtual void bookmarkWasAddedToLibrary(const Bookmark& bookmark);
-    library->addBookmark(bookmark);
+  virtual void booksWereRemovedFromLibrary();
-  }
+
- private:
+ private: // data
-   kiwix::Library* library;
+  kiwix::Library& library;
 };
 
 /**
@@ -61,10 +61,12 @@ class DefaultLibraryManipulator : public LibraryManipulator {
  */
 class Manager
 {
- public:
+ public: // types
-  Manager(LibraryManipulator* manipulator);
+  typedef std::vector<std::string> Paths;
-  Manager(Library* library);
+
-  ~Manager();
+ public: // functions
+  explicit Manager(LibraryManipulator* manipulator);
+  explicit Manager(Library* library);
 
   /**
    * Read a `library.xml` and add book in the file to the library.
@@ -72,10 +74,22 @@ class Manager
    * @param path The (utf8) path to the `library.xml`.
    * @param readOnly Set if the libray path could be overwritten latter with
    *                 updated content.
+   * @param trustLibrary use book metadata coming from XML.
    * @return True if file has been properly parsed.
    */
   bool readFile(const std::string& path, bool readOnly = true, bool trustLibrary = true);
 
+  /**
+   * Sync the contents of the library with one or more `library.xml` files.
+   *
+   * The metadata of the library files is trusted unconditionally.
+   * Any books not present in the input library.xml files are removed
+   * from the library.
+   *
+   * @param paths The (utf8) paths to the `library.xml` files.
+   */
+  void reload(const Paths& paths);
+
   /**
    * Load a library content store in the string.
    *
@@ -150,8 +164,7 @@ class Manager
   uint64_t m_itemsPerPage = 0;
 
  protected:
-  kiwix::LibraryManipulator* manipulator;
+  std::shared_ptr<kiwix::LibraryManipulator> manipulator;
-  bool mustDeleteManipulator;
 
   bool readBookFromPath(const std::string& path, Book* book);
   bool parseXmlDom(const pugi::xml_document& doc,

include/name_mapper.h

@@ -22,6 +22,8 @@
 
 #include <string>
 #include <map>
+#include <memory>
+#include <mutex>
 
 namespace kiwix
 {
@@ -31,15 +33,15 @@ class Library;
 class NameMapper {
   public:
     virtual ~NameMapper() = default;
-    virtual std::string getNameForId(const std::string& id) = 0;
+    virtual std::string getNameForId(const std::string& id) const = 0;
-    virtual std::string getIdForName(const std::string& name) = 0;
+    virtual std::string getIdForName(const std::string& name) const = 0;
 };
 
 
 class IdNameMapper : public NameMapper {
   public:
-    virtual std::string getNameForId(const std::string& id) { return id; };
+    virtual std::string getNameForId(const std::string& id) const { return id; };
-    virtual std::string getIdForName(const std::string& name) { return name; };
+    virtual std::string getIdForName(const std::string& name) const { return name; };
 };
 
 class HumanReadableNameMapper : public NameMapper {
@@ -50,11 +52,29 @@ class HumanReadableNameMapper : public NameMapper {
   public:
     HumanReadableNameMapper(kiwix::Library& library, bool withAlias);
     virtual ~HumanReadableNameMapper() = default;
-    virtual std::string getNameForId(const std::string& id);
+    virtual std::string getNameForId(const std::string& id) const;
-    virtual std::string getIdForName(const std::string& name);
+    virtual std::string getIdForName(const std::string& name) const;
 };
 
+class UpdatableNameMapper : public NameMapper {
+    typedef std::shared_ptr<NameMapper> NameMapperHandle;
+  public:
+    UpdatableNameMapper(Library& library, bool withAlias);
 
+    virtual std::string getNameForId(const std::string& id) const;
+    virtual std::string getIdForName(const std::string& name) const;
+
+    void update();
+
+  private:
+    NameMapperHandle currentNameMapper() const;
+
+  private:
+    mutable std::mutex mutex;
+    Library& library;
+    NameMapperHandle nameMapper;
+    const bool withAlias;
+};
 
 }
 

src/book.cpp

@@ -190,7 +190,7 @@ std::string Book::getHumanReadableIdFromPath() const
 {
   std::string id = m_path;
   if (!id.empty()) {
-    kiwix::removeAccents(id);
+    id = kiwix::removeAccents(id);
 
 #ifdef _WIN32
     id = replaceRegex(id, "", "^.*\\\\");

src/library.cpp

@@ -49,23 +49,48 @@ std::string normalizeText(const std::string& text)
   return removeAccents(text);
 }
 
+bool booksReferToTheSameArchive(const Book& book1, const Book& book2)
+{
+  return book1.isPathValid()
+      && book2.isPathValid()
+      && book1.getPath() == book2.getPath();
+}
+
 } // unnamed namespace
 
-class Library::BookDB : public Xapian::WritableDatabase
+class LibraryBase::BookDB : public Xapian::WritableDatabase
 {
 public:
   BookDB() : Xapian::WritableDatabase("", Xapian::DB_BACKEND_INMEMORY) {}
 };
 
-/* Constructor */
+LibraryBase::LibraryBase()
-Library::Library()
   : m_bookDB(new BookDB)
 {
 }
 
-Library::Library(Library&& ) = default;
+LibraryBase::~LibraryBase()
+{
+}
 
-Library& Library::operator=(Library&& ) = default;
+LibraryBase::LibraryBase(LibraryBase&& ) = default;
+LibraryBase& LibraryBase::operator=(LibraryBase&& ) = default;
+
+/* Constructor */
+Library::Library()
+{
+}
+
+Library::Library(Library&& other)
+  : LibraryBase(std::move(other))
+{
+}
+
+Library& Library::operator=(Library&& other)
+{
+  LibraryBase::operator=(std::move(other));
+  return *this;
+}
 
 /* Destructor */
 Library::~Library()
@@ -75,25 +100,37 @@ Library::~Library()
 
 bool Library::addBook(const Book& book)
 {
+  std::lock_guard<std::mutex> lock(m_mutex);
+  ++m_revision;
   /* Try to find it */
   updateBookDB(book);
   try {
     auto& oldbook = m_books.at(book.getId());
-    oldbook.update(book);
+    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;
     return false;
   } catch (std::out_of_range&) {
-    m_books[book.getId()] = book;
+    Entry& newEntry = m_books[book.getId()];
+    static_cast<Book&>(newEntry) = book;
+    newEntry.lastUpdatedRevision = m_revision;
     return true;
   }
 }
 
 void Library::addBookmark(const Bookmark& bookmark)
 {
+  std::lock_guard<std::mutex> lock(m_mutex);
   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++) {
     if (it->getBookId() == zimId && it->getUrl() == url) {
       m_bookmarks.erase(it);
@@ -104,28 +141,63 @@ 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);
+}
 
 bool Library::removeBookById(const std::string& id)
 {
+  std::lock_guard<std::mutex> lock(m_mutex);
   m_bookDB->delete_document("Q" + id);
-  m_readers.erase(id);
+  dropReader(id);
-  m_archives.erase(id);
   return m_books.erase(id) == 1;
 }
 
+Library::Revision Library::getRevision() const
+{
+  std::lock_guard<std::mutex> lock(m_mutex);
+  return m_revision;
+}
+
+uint32_t Library::removeBooksNotUpdatedSince(LibraryRevision libraryRevision)
+{
+  BookIdCollection booksToRemove;
+  {
+    std::lock_guard<std::mutex> lock(m_mutex);
+    for ( const auto& entry : m_books) {
+      if ( entry.second.lastUpdatedRevision <= libraryRevision ) {
+        booksToRemove.push_back(entry.first);
+      }
+    }
+  }
+
+  uint32_t countOfRemovedBooks = 0;
+  for ( const auto& id : booksToRemove ) {
+    if ( removeBookById(id) )
+      ++countOfRemovedBooks;
+  }
+  return countOfRemovedBooks;
+}
+
 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);
 }
 
-Book& Library::getBookById(const std::string& id)
+Book Library::getBookByIdThreadSafe(const std::string& id) const
 {
-  const Library& const_self = *this;
+  std::lock_guard<std::mutex> lock(m_mutex);
-  return const_cast<Book&>(const_self.getBookById(id));
+  return getBookById(id);
 }
 
 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) {
     auto& book = it.second;
     if (book.getPath() == path)
@@ -136,37 +208,26 @@ const Book& Library::getBookByPath(const std::string& path) const
   throw std::out_of_range(ss.str());
 }
 
-Book& Library::getBookByPath(const std::string& path)
-{
-  const Library& const_self = *this;
-  return const_cast<Book&>(const_self.getBookByPath(path));
-}
-
 std::shared_ptr<Reader> Library::getReaderById(const std::string& id)
 {
   try {
+    std::lock_guard<std::mutex> lock(m_mutex);
     return m_readers.at(id);
   } catch (std::out_of_range& e) {}
 
-  try {
+  const auto archive = getArchiveById(id);
-    auto reader = make_shared<Reader>(m_archives.at(id));
+  if ( !archive )
-    m_readers[id] = reader;
-    return reader;
-  } catch (std::out_of_range& e) {}
-
-  auto book = getBookById(id);
-  if (!book.isPathValid())
     return nullptr;
 
-  auto archive = make_shared<zim::Archive>(book.getPath());
+  const auto reader = make_shared<Reader>(archive);
-  m_archives[id] = archive;
+  std::lock_guard<std::mutex> lock(m_mutex);
-  auto reader = make_shared<Reader>(archive);
   m_readers[id] = reader;
   return reader;
 }
 
 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);
   } catch (std::out_of_range& e) {}
@@ -183,6 +244,7 @@ std::shared_ptr<zim::Archive> Library::getArchiveById(const std::string& id)
 unsigned int Library::getBookCount(const bool localBooks,
                                    const bool remoteBooks) const
 {
+  std::lock_guard<std::mutex> lock(m_mutex);
   unsigned int result = 0;
   for (auto& pair: m_books) {
     auto& book = pair.second;
@@ -196,20 +258,33 @@ unsigned int Library::getBookCount(const bool localBooks,
 
 bool Library::writeToFile(const std::string& path) const
 {
+  const auto allBookIds = getBooksIds();
+
   auto baseDir = removeLastPathElement(path);
   LibXMLDumper dumper(this);
   dumper.setBaseDir(baseDir);
-  return writeTextFile(path, dumper.dumpLibXMLContent(getBooksIds()));
+  std::string xml;
+  {
+    std::lock_guard<std::mutex> lock(m_mutex);
+    xml = dumper.dumpLibXMLContent(allBookIds);
+  };
+  return writeTextFile(path, xml);
 }
 
 bool Library::writeBookmarksToFile(const std::string& path) const
 {
   LibXMLDumper dumper(this);
-  return writeTextFile(path, dumper.dumpLibXMLBookmark());
+  std::string xml;
+  {
+    std::lock_guard<std::mutex> lock(m_mutex);
+    xml = dumper.dumpLibXMLBookmark();
+  };
+  return writeTextFile(path, xml);
 }
 
 Library::AttributeCounts Library::getBookAttributeCounts(BookStrPropMemFn p) const
 {
+  std::lock_guard<std::mutex> lock(m_mutex);
   AttributeCounts propValueCounts;
 
   for (const auto& pair: m_books) {
@@ -242,6 +317,7 @@ Library::AttributeCounts Library::getBooksLanguagesWithCounts() const
 
 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) {
@@ -272,6 +348,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);
   for(auto& bookmark:m_bookmarks) {
     if (std::find(booksId.begin(), booksId.end(), bookmark.getBookId()) != booksId.end()) {
       validBookmarks.push_back(bookmark);
@@ -282,6 +359,7 @@ const std::vector<kiwix::Bookmark> Library::getBookmarks(bool onlyValidBookmarks
 
 Library::BookIdCollection Library::getBooksIds() const
 {
+  std::lock_guard<std::mutex> lock(m_mutex);
   BookIdCollection bookIds;
 
   for (auto& pair: m_books) {
@@ -471,6 +549,7 @@ Library::BookIdCollection Library::filterViaBookDB(const Filter& filter) const
 
   BookIdCollection bookIds;
 
+  std::lock_guard<std::mutex> lock(m_mutex);
   Xapian::Enquire enquire(*m_bookDB);
   enquire.set_query(query);
   const auto results = enquire.get_mset(0, m_books.size());
@@ -484,7 +563,9 @@ Library::BookIdCollection Library::filterViaBookDB(const Filter& filter) const
 Library::BookIdCollection Library::filter(const Filter& filter) const
 {
   BookIdCollection result;
-  for(auto id : filterViaBookDB(filter)) {
+  const auto preliminaryResult = filterViaBookDB(filter);
+  std::lock_guard<std::mutex> lock(m_mutex);
+  for(auto id : preliminaryResult) {
     if(filter.accept(m_books.at(id))) {
       result.push_back(id);
     }
@@ -553,6 +634,11 @@ std::string Comparator<PUBLISHER>::get_key(const std::string& id)
 
 void Library::sort(BookIdCollection& bookIds, supportedListSortBy sort, bool ascending) const
 {
+  // NOTE: Can reimplement this method in a way that doesn't require locking
+  // 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);
   switch(sort) {
     case TITLE:
       std::sort(bookIds.begin(), bookIds.end(), Comparator<TITLE>(this, ascending));

src/manager.cpp

@@ -26,28 +26,81 @@
 
 namespace kiwix
 {
+
+namespace
+{
+
+struct NoDelete
+{
+  template<class T> void operator()(T*) {}
+};
+
+} // unnamed namespace
+
+////////////////////////////////////////////////////////////////////////////////
+// LibraryManipulator
+////////////////////////////////////////////////////////////////////////////////
+
+LibraryManipulator::LibraryManipulator(Library* library)
+  : library(*library)
+{}
+
+LibraryManipulator::~LibraryManipulator()
+{}
+
+bool LibraryManipulator::addBookToLibrary(const Book& book)
+{
+  const auto ret = library.addBook(book);
+  if ( ret ) {
+    bookWasAddedToLibrary(book);
+  }
+  return ret;
+}
+
+void LibraryManipulator::addBookmarkToLibrary(const Bookmark& bookmark)
+{
+  library.addBookmark(bookmark);
+  bookmarkWasAddedToLibrary(bookmark);
+}
+
+uint32_t LibraryManipulator::removeBooksNotUpdatedSince(Library::Revision rev)
+{
+  const auto n = library.removeBooksNotUpdatedSince(rev);
+  if ( n != 0 ) {
+    booksWereRemovedFromLibrary();
+  }
+  return n;
+}
+
+void LibraryManipulator::bookWasAddedToLibrary(const Book& book)
+{
+}
+
+void LibraryManipulator::bookmarkWasAddedToLibrary(const Bookmark& bookmark)
+{
+}
+
+void LibraryManipulator::booksWereRemovedFromLibrary()
+{
+}
+
+////////////////////////////////////////////////////////////////////////////////
+// Manager
+////////////////////////////////////////////////////////////////////////////////
+
 /* Constructor */
 Manager::Manager(LibraryManipulator* manipulator):
   writableLibraryPath(""),
-  manipulator(manipulator),
+  manipulator(manipulator, NoDelete())
-  mustDeleteManipulator(false)
 {
 }
 
 Manager::Manager(Library* library) :
   writableLibraryPath(""),
-  manipulator(new DefaultLibraryManipulator(library)),
+  manipulator(new LibraryManipulator(library))
-  mustDeleteManipulator(true)
 {
 }
 
-/* Destructor */
-Manager::~Manager()
-{
-  if (mustDeleteManipulator) {
-    delete manipulator;
-  }
-}
 bool Manager::parseXmlDom(const pugi::xml_document& doc,
                           bool readOnly,
                           const std::string& libraryPath,
@@ -249,4 +302,21 @@ bool Manager::readBookmarkFile(const std::string& path)
   return true;
 }
 
+void Manager::reload(const Paths& paths)
+{
+  const auto libRevision = manipulator->getLibrary().getRevision();
+  for (std::string path : paths) {
+    if (!path.empty()) {
+      if ( kiwix::isRelativePath(path) )
+        path = kiwix::computeAbsolutePath(kiwix::getCurrentDirectory(), path);
+
+      if (!readFile(path, false, true)) {
+        throw std::runtime_error("Failed to load the XML library file '" + path + "'.");
+      }
+    }
+  }
+
+  manipulator->removeBooksNotUpdatedSince(libRevision);
+}
+
 }

src/name_mapper.cpp

@@ -51,12 +51,54 @@ HumanReadableNameMapper::HumanReadableNameMapper(kiwix::Library& library, bool w
   }
 }
 
-std::string HumanReadableNameMapper::getNameForId(const std::string& id) {
+std::string HumanReadableNameMapper::getNameForId(const std::string& id) const {
   return m_idToName.at(id);
 }
 
-std::string HumanReadableNameMapper::getIdForName(const std::string& name) {
+std::string HumanReadableNameMapper::getIdForName(const std::string& name) const {
   return m_nameToId.at(name);
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// UpdatableNameMapper
+////////////////////////////////////////////////////////////////////////////////
+
+UpdatableNameMapper::UpdatableNameMapper(Library& lib, bool withAlias)
+  : library(lib)
+  , withAlias(withAlias)
+{
+  update();
+}
+
+void UpdatableNameMapper::update()
+{
+  const auto newNameMapper = new HumanReadableNameMapper(library, withAlias);
+  std::lock_guard<std::mutex> lock(mutex);
+  nameMapper.reset(newNameMapper);
+}
+
+UpdatableNameMapper::NameMapperHandle
+UpdatableNameMapper::currentNameMapper() const
+{
+  // Return a copy of the handle to the current NameMapper object. It will
+  // ensure that the object survives any call to UpdatableNameMapper::update()
+  // made before the completion of any pending operation on that object.
+  std::lock_guard<std::mutex> lock(mutex);
+  return nameMapper;
+}
+
+std::string UpdatableNameMapper::getNameForId(const std::string& id) const
+{
+  // Ensure that the current nameMapper object survives a concurrent call
+  // to UpdatableNameMapper::update()
+  return currentNameMapper()->getNameForId(id);
+}
+
+std::string UpdatableNameMapper::getIdForName(const std::string& name) const
+{
+  // Ensure that the current nameMapper object survives a concurrent call
+  // to UpdatableNameMapper::update()
+  return currentNameMapper()->getIdForName(name);
+}
+
 }

src/opds_dumper.cpp

@@ -108,10 +108,15 @@ BooksData getBooksData(const Library* library, const std::vector<std::string>& b
 {
   BooksData booksData;
   for ( const auto& bookId : bookIds ) {
-    const Book& book = library->getBookById(bookId);
+    try {
-    booksData.push_back(kainjow::mustache::object{
+      const Book book = library->getBookByIdThreadSafe(bookId);
-        {"entry", getSingleBookEntryXML(book, false, endpointRoot, partial)}
+      booksData.push_back(kainjow::mustache::object{
-    });
+          {"entry", getSingleBookEntryXML(book, false, endpointRoot, partial)}
+      });
+    } catch ( const std::out_of_range& ) {
+      // the book was removed from the library since its id was obtained
+      // ignore it
+    }
   }
 
   return booksData;

test/book.cpp

@@ -178,3 +178,32 @@ TEST(BookTest, updateTest)
     EXPECT_EQ(newBook.getFavicon(), book.getFavicon());
     EXPECT_EQ(newBook.getFaviconMimeType(), book.getFaviconMimeType());
 }
+
+namespace
+{
+
+std::string path2HumanReadableId(const std::string& path)
+{
+    const XMLDoc xml("<book id=\"xyz\" path=\"" + path + "\"></book>");
+
+    kiwix::Book book;
+    book.updateFromXml(xml.child("book"), "/data/zim");
+    return book.getHumanReadableIdFromPath();
+}
+
+} // unnamed namespace
+
+TEST(BookTest, getHumanReadableIdFromPath)
+{
+  EXPECT_EQ("abc",     path2HumanReadableId("abc.zim"));
+  EXPECT_EQ("abc",     path2HumanReadableId("ABC.zim"));
+  EXPECT_EQ("abc",     path2HumanReadableId("âbç.zim"));
+  EXPECT_EQ("ancient", path2HumanReadableId("ancient.zimbabwe"));
+  EXPECT_EQ("ab_cd",   path2HumanReadableId("ab cd.zim"));
+#ifdef _WIN32
+  EXPECT_EQ("abc",     path2HumanReadableId("C:\\Data\\ZIM\\abc.zim"));
+#else
+  EXPECT_EQ("abc",     path2HumanReadableId("/Data/ZIM/abc.zim"));
+#endif
+  EXPECT_EQ("3plus2",  path2HumanReadableId("3+2.zim"));
+}

test/library.cpp

@@ -232,7 +232,7 @@ class LibraryTest : public ::testing::Test {
   void SetUp() override {
      kiwix::Manager manager(&lib);
      manager.readOpds(sampleOpdsStream, "foo.urlHost");
-     manager.readXml(sampleLibraryXML, true, "./test/library.xml", true);
+     manager.readXml(sampleLibraryXML, false, "./test/library.xml", true);
   }
 
     kiwix::Bookmark createBookmark(const std::string &id) {
@@ -660,13 +660,14 @@ TEST_F(LibraryTest, filterByMultipleCriteria)
 
 TEST_F(LibraryTest, getBookByPath)
 {
-  auto& 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);
 }
@@ -706,4 +707,35 @@ TEST_F(LibraryTest, removeBookByIdUpdatesTheSearchDB)
   EXPECT_THROW(lib.getBookById("raycharles"), std::out_of_range);
 };
 
+TEST_F(LibraryTest, removeBooksNotUpdatedSince)
+{
+  EXPECT_FILTER_RESULTS(kiwix::Filter(),
+    "An example ZIM archive",
+    "Encyclopédie de la Tunisie",
+    "Granblue Fantasy Wiki",
+    "Géographie par Wikipédia",
+    "Islam Stack Exchange",
+    "Mathématiques",
+    "Movies & TV Stack Exchange",
+    "Mythology & Folklore Stack Exchange",
+    "Ray Charles",
+    "TED talks - Business",
+    "Tania Louis",
+    "Wikiquote"
+  );
+
+  const uint64_t rev = lib.getRevision();
+  for ( const auto& id : lib.filter(kiwix::Filter().query("exchange")) ) {
+    lib.addBook(lib.getBookByIdThreadSafe(id));
+  }
+
+  EXPECT_EQ(9u, lib.removeBooksNotUpdatedSince(rev));
+
+  EXPECT_FILTER_RESULTS(kiwix::Filter(),
+    "Islam Stack Exchange",
+    "Movies & TV Stack Exchange",
+    "Mythology & Folklore Stack Exchange",
+  );
+};
+
 };

test/manager.cpp

@@ -67,3 +67,29 @@ TEST(ManagerTest, readXml)
     EXPECT_EQ(45U, book.getMediaCount());
     EXPECT_EQ(678U*1024, book.getSize());
 }
+
+TEST(Manager, reload)
+{
+  kiwix::Library lib;
+  kiwix::Manager manager(&lib);
+
+  manager.reload({ "./test/library.xml" });
+  EXPECT_EQ(lib.getBooksIds(), (kiwix::Library::BookIdCollection{
+        "charlesray",
+        "raycharles",
+        "raycharles_uncategorized"
+  }));
+
+  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({
+        "charlesray",
+        "raycharles",
+        "raycharles_uncategorized"
+  }));
+}

test/meson.build

@@ -8,6 +8,7 @@ tests = [
     'kiwixserve',
     'book',
     'manager',
+    'name_mapper',
     'opds_catalog',
     'reader',
     'searcher'

test/name_mapper.cpp

@@ -0,0 +1,147 @@
+#include "../include/name_mapper.h"
+
+#include "../include/library.h"
+#include "../include/manager.h"
+#include "gtest/gtest.h"
+
+namespace
+{
+
+const char libraryXML[] = R"(
+<library version="1.0">
+  <book id="01"         path="/data/zero_one.zim">         </book>
+  <book id="02"         path="/data/zero two.zim">         </book>
+  <book id="03"         path="/data/ZERO thrêë.zim">       </book>
+  <book id="04-2021-10" path="/data/zero_four_2021-10.zim"></book>
+  <book id="04-2021-11" path="/data/zero_four_2021-11.zim"></book>
+</library>
+)";
+
+class NameMapperTest : public ::testing::Test {
+ protected:
+  void SetUp() override {
+     kiwix::Manager manager(&lib);
+     manager.readXml(libraryXML, false, "./library.xml", true);
+     for ( const std::string& id : lib.getBooksIds() ) {
+       kiwix::Book bookCopy = lib.getBookById(id);
+       bookCopy.setPathValid(true);
+       lib.addBook(bookCopy);
+     }
+  }
+
+  kiwix::Library lib;
+};
+
+class CapturedStderr
+{
+  std::ostringstream buffer;
+  std::streambuf* const sbuf;
+public:
+  CapturedStderr()
+    : sbuf(std::cerr.rdbuf())
+  {
+    std::cerr.rdbuf(buffer.rdbuf());
+  }
+
+  CapturedStderr(const CapturedStderr&) = delete;
+
+  ~CapturedStderr()
+  {
+    std::cerr.rdbuf(sbuf);
+  }
+
+  operator std::string() const { return buffer.str(); }
+};
+
+} // unnamed namespace
+
+void checkUnaliasedEntriesInNameMapper(const kiwix::NameMapper& nm)
+{
+  EXPECT_EQ("zero_one",           nm.getNameForId("01"));
+  EXPECT_EQ("zero_two",           nm.getNameForId("02"));
+  EXPECT_EQ("zero_three",         nm.getNameForId("03"));
+  EXPECT_EQ("zero_four_2021-10",  nm.getNameForId("04-2021-10"));
+  EXPECT_EQ("zero_four_2021-11",  nm.getNameForId("04-2021-11"));
+
+  EXPECT_EQ("01",         nm.getIdForName("zero_one"));
+  EXPECT_EQ("02",         nm.getIdForName("zero_two"));
+  EXPECT_EQ("03",         nm.getIdForName("zero_three"));
+  EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four_2021-10"));
+  EXPECT_EQ("04-2021-11", nm.getIdForName("zero_four_2021-11"));
+}
+
+TEST_F(NameMapperTest, HumanReadableNameMapperWithoutAliases)
+{
+  CapturedStderr stderror;
+  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");
+  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);
+}
+
+TEST_F(NameMapperTest, HumanReadableNameMapperWithAliases)
+{
+  CapturedStderr stderror;
+  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'."
+      " Therefore, only /data/zero_four_2021-10.zim will be served.\n"
+      , std::string(stderror)
+  );
+
+  checkUnaliasedEntriesInNameMapper(nm);
+  EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four"));
+
+  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"));
+}
+
+TEST_F(NameMapperTest, UpdatableNameMapperWithoutAliases)
+{
+  CapturedStderr stderror;
+  kiwix::UpdatableNameMapper 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");
+  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);
+  EXPECT_THROW(nm.getIdForName("zero_four"), std::out_of_range);
+}
+
+TEST_F(NameMapperTest, UpdatableNameMapperWithAliases)
+{
+  CapturedStderr stderror;
+  kiwix::UpdatableNameMapper 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'."
+      " Therefore, only /data/zero_four_2021-10.zim will be served.\n"
+      , std::string(stderror)
+  );
+
+  checkUnaliasedEntriesInNameMapper(nm);
+  EXPECT_EQ("04-2021-10", nm.getIdForName("zero_four"));
+
+  {
+    CapturedStderr nmUpdateStderror;
+    lib.removeBookById("04-2021-10");
+    nm.update();
+    EXPECT_EQ("", std::string(nmUpdateStderror));
+  }
+  EXPECT_EQ("04-2021-11", nm.getIdForName("zero_four"));
+  EXPECT_THROW(nm.getNameForId("04-2021-10"), std::out_of_range);
+  EXPECT_THROW(nm.getIdForName("zero_four_2021-10"), std::out_of_range);
+}