Merge pull request #1043 from kiwix/bookmarks_migrations

Migrate bookmarks between books

include/bookmark.h

@@ -29,19 +29,33 @@ class xml_node;
 namespace kiwix
 {
 
+class Book;
 /**
  * A class to store information about a bookmark (an article in a book)
  */
 class Bookmark
 {
  public:
+  /**
+   *  Create an empty bookmark.
+   *
+   * Bookmark must be populated with `set*` methods
+   */
   Bookmark();
+
+  /**
+   * Create a bookmark given a Book, a path and a title.
+   */
+  Bookmark(const Book& book, const std::string& path, const std::string& title);
+
   ~Bookmark();
 
   void updateFromXml(const pugi::xml_node& node);
 
   const std::string& getBookId() const { return m_bookId; }
   const std::string& getBookTitle()  const { return m_bookTitle; }
+  const std::string& getBookName()  const { return m_bookName; }
+  const std::string& getBookFlavour()  const { return m_bookFlavour; }
   const std::string& getUrl() const { return m_url; }
   const std::string& getTitle() const { return m_title; }
   const std::string& getLanguage() const { return m_language; }
@@ -49,6 +63,8 @@ class Bookmark
 
   void setBookId(const std::string& bookId) { m_bookId = bookId; }
   void setBookTitle(const std::string& bookTitle) { m_bookTitle = bookTitle; }
+  void setBookName(const std::string& bookName) { m_bookName = bookName; }
+  void setBookFlavour(const std::string& bookFlavour) { m_bookFlavour = bookFlavour; }
   void setUrl(const std::string& url) { m_url = url; }
   void setTitle(const std::string& title) { m_title = title; }
   void setLanguage(const std::string& language) { m_language = language; }
@@ -57,6 +73,8 @@ class Bookmark
  protected:
   std::string m_bookId;
   std::string m_bookTitle;
+  std::string m_bookName;
+  std::string m_bookFlavour;
   std::string m_url;
   std::string m_title;
   std::string m_language;

include/library.h

@@ -55,6 +55,22 @@ enum supportedListMode {
   NOVALID = 1 << 5
 };
 
+enum MigrationMode {
+  /** When migrating bookmarks, do not allow to migrate to an older book than the currently pointed one
+   * (or date stored in the bookmark if book is invalid)
+   *
+   * If no newer books are found, no upgrade is made.
+   */
+  UPGRADE_ONLY = 0,
+
+  /** Try hard to do a migration. This mostly does:
+   * - Try to find a newer book.
+   * - If book is invalid: find a best book, potentially older.
+   * Older book will never be returned if current book is a valid one.
+   */
+  ALLOW_DOWNGRADE = 1,
+};
+
 class Filter {
   public: // types
     using Tags = std::vector<std::string>;
@@ -71,6 +87,7 @@ class Filter {
     std::string _query;
     bool _queryIsPartial;
     std::string _name;
+    std::string _flavour;
 
   public: // functions
     Filter();
@@ -130,6 +147,7 @@ class Filter {
     Filter& maxSize(size_t size);
     Filter& query(std::string query, bool partial=true);
     Filter& name(std::string name);
+    Filter& flavour(std::string flavour);
     Filter& clearLang();
     Filter& clearCategory();
 
@@ -152,6 +170,9 @@ class Filter {
     bool hasCreator() const;
     const std::string& getCreator() const { return _creator; }
 
+    bool hasFlavour() const;
+    const std::string& getFlavour() const { return _flavour; }
+
     const Tags& getAcceptTags() const { return _acceptTags; }
     const Tags& getRejectTags() const { return _rejectTags; }
 
@@ -250,7 +271,7 @@ class Library: public std::enable_shared_from_this<Library>
   void addBookmark(const Bookmark& bookmark);
 
   /**
-   * Remove a bookmarkk
+   * Remove a bookmark
    *
    * @param zimId The zimId of the bookmark.
    * @param url The url of the bookmark.
@@ -258,6 +279,66 @@ class Library: public std::enable_shared_from_this<Library>
    */
   bool removeBookmark(const std::string& zimId, const std::string& url);
 
+  /**
+   * Migrate all invalid bookmarks.
+   *
+   * All invalid bookmarks (ie pointing to unknown books, no check is made on bookmark pointing to
+   * invalid articles of valid book) will be migrated (if possible) to a better book.
+   * "Better book", will be determined using method `getBestTargetBookId`.
+   *
+   * @return A tuple<int, int>: <The number of bookmarks updated>, <Number of invalid bookmarks before migration was performed>.
+   */
+  std::tuple<int, int> migrateBookmarks(MigrationMode migrationMode = ALLOW_DOWNGRADE);
+
+  /**
+   * Migrate all bookmarks associated to a specific book.
+   *
+   * All bookmarks associated to `sourceBookId` book will be migrated to a better book.
+   * "Better book", will be determined using method `getBestTargetBookId`.
+   *
+   * @param sourceBookId the source bookId of the bookmarks to migrate.
+   * @param migrationMode how we will find the best book.
+   * @return The number of bookmarks updated.
+   */
+  int migrateBookmarks(const std::string& sourceBookId, MigrationMode migrationMode = UPGRADE_ONLY);
+
+  /**
+   * Migrate bookmarks
+   *
+   * Migrate all bookmarks pointing to `source` to `destination`.
+   *
+   * @param sourceBookId the source bookId of the bookmarks to migrate.
+   * @param targetBookId the destination bookId to migrate the bookmarks to.
+   * @return The number of bookmarks updated.
+   */
+  int migrateBookmarks(const std::string& sourceBookId, const std::string& targetBookId);
+ 
+  /**
+   * Get the best available bookId for a bookmark.
+   *
+   * Given a bookmark, return the best available bookId.
+   * "best available bookId" is determined using heuristitcs based on book name, flavour and date.
+   *
+   * @param bookmark The bookmark to search the bookId for.
+   * @param migrationMode The migration mode to use.
+   * @return A bookId. Potentially empty string if no suitable book found.
+   */
+  std::string getBestTargetBookId(const Bookmark& bookmark, MigrationMode migrationMode) const;
+
+  /**
+   * Get the best bookId for a combination of book's name, flavour and date.
+   *
+   * Given a bookName (mandatory), try to find the best book.
+   * If preferedFlavour is given, will try to find a book with the same flavour. If not found, return a book with a different flavour.
+   * If minDate is given, return a book newer than minDate. If not found, return a empty bookId.
+   *
+   * @param bookName The name of the book
+   * @param preferedFlavour The prefered flavour.
+   * @param minDate the minimal book date acceptable. Must be a string in the format "YYYY-MM-DD".
+   * @return A bookId corresponding to the query, or empty string if not found.
+   */
+  std::string getBestTargetBookId(const std::string& bookName, const std::string& preferedFlavour="", const std::string& minDate="") const;
+
   // XXX: This is a non-thread-safe operation
   const Book& getBookById(const std::string& id) const;
   // XXX: This is a non-thread-safe operation
@@ -403,12 +484,13 @@ private: // functions
   AttributeCounts getBookAttributeCounts(BookStrPropMemFn p) const;
   std::vector<std::string> getBookPropValueSet(BookStrPropMemFn p) const;
   BookIdCollection filterViaBookDB(const Filter& filter) const;
+  std::string getBestFromBookCollection(BookIdCollection books, const Bookmark& bookmark, MigrationMode migrationMode) 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
-  mutable std::mutex m_mutex;
+  mutable std::recursive_mutex m_mutex;
   Library::Revision m_revision;
   std::map<std::string, Entry> m_books;
   using ArchiveCache = ConcurrentCache<std::string, std::shared_ptr<zim::Archive>>;

src/bookmark.cpp

@@ -18,6 +18,7 @@
  */
 
 #include "bookmark.h"
+#include "book.h"
 
 #include <pugixml.hpp>
 
@@ -28,6 +29,17 @@ Bookmark::Bookmark()
 {
 }
 
+Bookmark::Bookmark(const Book& book, const std::string& path, const std::string& title):
+  m_bookId(book.getId()),
+  m_bookTitle(book.getTitle()),
+  m_bookName(book.getName()),
+  m_bookFlavour(book.getFlavour()),
+  m_url(path),
+  m_title(title),
+  m_language(book.getCommaSeparatedLanguages()),
+  m_date(book.getDate())
+{}
+
 /* Destructor */
 Bookmark::~Bookmark()
 {
@@ -38,6 +50,8 @@ void Bookmark::updateFromXml(const pugi::xml_node& node)
   auto bookNode = node.child("book");
   m_bookId = bookNode.child("id").child_value();
   m_bookTitle = bookNode.child("title").child_value();
+  m_bookName = bookNode.child("name").child_value();
+  m_bookFlavour = bookNode.child("flavour").child_value();
   m_language = bookNode.child("language").child_value();
   m_date = bookNode.child("date").child_value();
   m_title = node.child("title").child_value();

src/library.cpp

@@ -110,7 +110,7 @@ Library::~Library() = default;
 
 bool Library::addBook(const Book& book)
 {
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   ++m_revision;
   /* Try to find it */
   updateBookDB(book);
@@ -141,13 +141,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::recursive_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);
+  std::lock_guard<std::recursive_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);
@@ -157,6 +157,159 @@ bool Library::removeBookmark(const std::string& zimId, const std::string& url)
   return false;
 }
 
+std::tuple<int, int> Library::migrateBookmarks(MigrationMode migrationMode) {
+  std::set<std::string> sourceBooks;
+  int invalidBookmarks = 0;
+  {
+    std::lock_guard<std::recursive_mutex> lock(m_mutex);
+    for(auto& bookmark:m_bookmarks) {
+      if (m_books.find(bookmark.getBookId()) == m_books.end()) {
+        invalidBookmarks += 1;
+        sourceBooks.insert(bookmark.getBookId());
+      }
+    }
+  }
+  int changed = 0;
+  for(auto& sourceBook:sourceBooks) {
+    changed += migrateBookmarks(sourceBook, migrationMode);
+  }
+  return std::make_tuple(changed, invalidBookmarks);
+}
+
+std::string Library::getBestFromBookCollection(BookIdCollection books, const Bookmark& bookmark, MigrationMode migrationMode) const {
+  // This function try to get the best book for a bookmark from a book collection.
+  // It assumes that all books in the collection are "acceptable".
+  // (this definiton is not clear but for now it is book's name is equal to bookmark's bookName)
+  //
+  // The algorithm first sort the colletion by "flavour equality" and date.
+  // "flavour equality" is if book's flavour is same that bookmark's flavour (let's say "flavourA" here)
+  // So we have the sorted collection:
+  // - flavourA, date 5
+  // - flavourA, date 4
+  // - flavourB, date 6
+  // - flavourC, date 5
+  // - flavourB, date 3
+  //
+  // Then, depending of migrationMode:
+  // - If ALLOW_DOWNGRADE => take the first one
+  // - If UPGRADE_ONLY => loop on books until we find a book newer than bookmark.
+  //   So if bookmark date is 5 => flavourB, date 6
+  //      if bookmark date is 4 => flavourA, date 5
+  //      if bookmark date is 7 => No book
+
+  if (books.empty()) {
+    return "";
+  }
+
+  sort(books, DATE, false);
+  stable_sort(books.begin(), books.end(), [&](const std::string& bookId1, const std::string& bookId2) {
+    const auto& book1 = getBookById(bookId1);
+    const auto& book2 = getBookById(bookId2);
+    bool same_flavour1 = book1.getFlavour() == bookmark.getBookFlavour();
+    bool same_flavour2 = book2.getFlavour() == bookmark.getBookFlavour();
+    // return True if bookId1 is before bookId2, ie if same_flavour1 and not same_flavour2
+    return same_flavour1 > same_flavour2;
+  });
+
+  if (migrationMode == ALLOW_DOWNGRADE) {
+    return books[0];
+  } else {
+    for (const auto& bookId: books) {
+      const auto& book = getBookById(bookId);
+      if (book.getDate() >= bookmark.getDate()) {
+        return bookId;
+      }
+    }
+  }
+
+  return "";
+}
+
+std::string remove_quote(std::string input) {
+  std::replace(input.begin(), input.end(), '"', ' ');
+  return input;
+}
+
+std::string Library::getBestTargetBookId(const std::string& bookName, const std::string& preferedFlavour, const std::string& minDate) const {
+  // Let's reuse our algorithm based on bookmark.
+  MigrationMode migrationMode = UPGRADE_ONLY;
+  auto bookmark = Bookmark();
+  bookmark.setBookName(bookName);
+  bookmark.setBookFlavour(preferedFlavour);
+
+  if (minDate.empty()) {
+    migrationMode = ALLOW_DOWNGRADE;
+  } else {
+    bookmark.setDate(minDate);
+  }
+
+  return getBestTargetBookId(bookmark, migrationMode);
+}
+
+std::string Library::getBestTargetBookId(const Bookmark& bookmark, MigrationMode migrationMode) const {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+  // Search for a existing book with the same name
+  auto book_filter = Filter();
+  if (!bookmark.getBookName().empty()) {
+    book_filter.name(bookmark.getBookName());
+  } else {
+    // We don't have a name stored (older bookmarks)
+    // Fallback on title (All bookmarks should have one, but let's be safe against wrongly filled bookmark)
+    if (bookmark.getBookTitle().empty()) {
+        // No bookName nor bookTitle, no way to find target book.
+        return "";
+    }
+    book_filter.query("title:\"" + remove_quote(bookmark.getBookTitle()) + "\"");
+  }
+  auto targetBooks = filter(book_filter);
+  auto bestBook = getBestFromBookCollection(targetBooks, bookmark, migrationMode);
+  if (bestBook.empty()) {
+    try {
+      getBookById(bookmark.getBookId());
+      return bookmark.getBookId();
+    } catch (std::out_of_range&) {}
+  }
+  return bestBook;
+}
+
+int Library::migrateBookmarks(const std::string& sourceBookId, MigrationMode migrationMode) {
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
+
+  Bookmark firstBookmarkToChange;
+  for(auto& bookmark:m_bookmarks) {
+    if (bookmark.getBookId() == sourceBookId) {
+      firstBookmarkToChange = bookmark;
+      break;
+    }
+  }
+
+  if (firstBookmarkToChange.getBookId().empty()) {
+    return 0;
+  }
+
+  std::string betterBook = getBestTargetBookId(firstBookmarkToChange, migrationMode);
+
+  if (betterBook.empty()) {
+    return 0;
+  }
+
+  return migrateBookmarks(sourceBookId, betterBook);
+}
+
+int Library::migrateBookmarks(const std::string& sourceBookId, const std::string& targetBookId) {
+  if (sourceBookId == targetBookId) {
+    return 0;
+  }
+  int changed = 0;
+  for (auto& bookmark:m_bookmarks) {
+    if (bookmark.getBookId() == sourceBookId) {
+      bookmark.setBookId(targetBookId);
+      changed +=1;
+    }
+  }
+  return changed;
+}
+
 
 void Library::dropCache(const std::string& id)
 {
@@ -166,7 +319,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::recursive_mutex> lock(m_mutex);
   m_bookDB->delete_document("Q" + id);
   dropCache(id);
   // We do not change the cache size here
@@ -184,7 +337,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::recursive_mutex> lock(m_mutex);
   return m_revision;
 }
 
@@ -192,7 +345,7 @@ uint32_t Library::removeBooksNotUpdatedSince(Revision libraryRevision)
 {
   BookIdCollection booksToRemove;
   {
-    std::lock_guard<std::mutex> lock(m_mutex);
+    std::lock_guard<std::recursive_mutex> lock(m_mutex);
     for ( const auto& entry : m_books) {
       if ( entry.second.lastUpdatedRevision <= libraryRevision ) {
         booksToRemove.push_back(entry.first);
@@ -217,7 +370,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::recursive_mutex> lock(m_mutex);
   return getBookById(id);
 }
 
@@ -275,7 +428,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::recursive_mutex> lock(m_mutex);
   return getBookCount_not_protected(localBooks, remoteBooks);
 }
 
@@ -288,7 +441,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::recursive_mutex> lock(m_mutex);
     xml = dumper.dumpLibXMLContent(allBookIds);
   };
   return writeTextFile(path, xml);
@@ -304,7 +457,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::recursive_mutex> lock(m_mutex);
   AttributeCounts propValueCounts;
 
   for (const auto& pair: m_books) {
@@ -336,7 +489,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::recursive_mutex> lock(m_mutex);
   AttributeCounts langsWithCounts;
 
   for (const auto& pair: m_books) {
@@ -352,7 +505,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::recursive_mutex> lock(m_mutex);
   std::set<std::string> categories;
 
   for (const auto& pair: m_books) {
@@ -383,7 +536,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::recursive_mutex> lock(m_mutex);
   for(auto& bookmark:m_bookmarks) {
     if (std::find(booksId.begin(), booksId.end(), bookmark.getBookId()) != booksId.end()) {
       validBookmarks.push_back(bookmark);
@@ -394,7 +547,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::recursive_mutex> lock(m_mutex);
   BookIdCollection bookIds;
 
   for (auto& pair: m_books) {
@@ -437,6 +590,7 @@ void Library::updateBookDB(const Book& book)
   indexer.index_text(normalizeText(book.getCreator()),   1, "A");
   indexer.index_text(normalizeText(book.getPublisher()), 1, "XP");
   doc.add_term("XN"+normalizeText(book.getName()));
+  indexer.index_text(normalizeText(book.getFlavour()),  1, "XF");
   indexer.index_text(normalizeText(book.getCategory()),  1, "XC");
 
   for ( const auto& tag : split(normalizeText(book.getTags()), ";") ) {
@@ -477,6 +631,7 @@ Xapian::Query buildXapianQueryFromFilterQuery(const Filter& filter)
   queryParser.add_prefix("title", "S");
   queryParser.add_prefix("description", "XD");
   queryParser.add_prefix("name", "XN");
+  queryParser.add_prefix("flavour", "XF");
   queryParser.add_prefix("category", "XC");
   queryParser.add_prefix("lang", "L");
   queryParser.add_prefix("publisher", "XP");
@@ -503,6 +658,11 @@ Xapian::Query nameQuery(const std::string& name)
   return Xapian::Query("XN" + normalizeText(name));
 }
 
+Xapian::Query flavourQuery(const std::string& name)
+{
+  return Xapian::Query("XF" + normalizeText(name));
+}
+
 Xapian::Query multipleParamQuery(const std::string& commaSeparatedList, const std::string& prefix)
 {
   Xapian::Query q;
@@ -570,6 +730,9 @@ Xapian::Query buildXapianQuery(const Filter& filter)
   if ( filter.hasName() ) {
     q = Xapian::Query(Xapian::Query::OP_AND, q, nameQuery(filter.getName()));
   }
+  if ( filter.hasFlavour() )  {
+    q = Xapian::Query(Xapian::Query::OP_AND, q, flavourQuery(filter.getFlavour()));     
+  }
   if ( filter.hasCategory() ) {
     q = Xapian::Query(Xapian::Query::OP_AND, q, categoryQuery(filter.getCategory()));
   }
@@ -600,7 +763,7 @@ Library::BookIdCollection Library::filterViaBookDB(const Filter& filter) const
 
   BookIdCollection bookIds;
 
-  std::lock_guard<std::mutex> lock(m_mutex);
+  std::lock_guard<std::recursive_mutex> lock(m_mutex);
   Xapian::Enquire enquire(*m_bookDB);
   enquire.set_query(query);
   const auto results = enquire.get_mset(0, m_books.size());
@@ -615,7 +778,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::recursive_mutex> lock(m_mutex);
   for(auto id : preliminaryResult) {
     if(filter.accept(m_books.at(id))) {
       result.push_back(id);
@@ -689,7 +852,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::recursive_mutex> lock(m_mutex);
   switch(sort) {
     case TITLE:
       std::sort(bookIds.begin(), bookIds.end(), Comparator<TITLE>(this, ascending));
@@ -735,6 +898,7 @@ enum filterTypes {
   QUERY = FLAG(12),
   NAME = FLAG(13),
   CATEGORY = FLAG(14),
+  FLAVOUR = FLAG(15),
 };
 
 Filter& Filter::local(bool accept)
@@ -837,6 +1001,13 @@ Filter& Filter::name(std::string name)
   return *this;
 }
   
+Filter& Filter::flavour(std::string flavour)
+{
+  _flavour = flavour;
+  activeFilters |= FLAVOUR;
+  return *this;
+}
+
 Filter& Filter::clearLang()
 {
   activeFilters &= ~LANG;
@@ -881,6 +1052,12 @@ bool Filter::hasCreator() const
   return ACTIVE(_CREATOR);
 }
 
+bool Filter::hasFlavour() const
+{
+  return ACTIVE(FLAVOUR);
+}
+
+
 bool Filter::accept(const Book& book) const
 {
   auto local = !book.getPath().empty();

src/libxml_dumper.cpp

@@ -97,11 +97,15 @@ void LibXMLDumper::handleBookmark(Bookmark bookmark, pugi::xml_node root_node) {
     auto book = library->getBookByIdThreadSafe(bookmark.getBookId());
     ADD_TEXT_ENTRY(book_node, "id", book.getId());
     ADD_TEXT_ENTRY(book_node, "title", book.getTitle());
+    ADD_TEXT_ENTRY(book_node, "name", book.getName());
+    ADD_TEXT_ENTRY(book_node, "flavour", book.getFlavour());
     ADD_TEXT_ENTRY(book_node, "language", book.getCommaSeparatedLanguages());
     ADD_TEXT_ENTRY(book_node, "date", book.getDate());
   } catch (...) {
     ADD_TEXT_ENTRY(book_node, "id", bookmark.getBookId());
     ADD_TEXT_ENTRY(book_node, "title", bookmark.getBookTitle());
+    ADD_TEXT_ENTRY(book_node, "name", bookmark.getBookName());
+    ADD_TEXT_ENTRY(book_node, "flavour", bookmark.getBookFlavour());
     ADD_TEXT_ENTRY(book_node, "language", bookmark.getLanguage());
     ADD_TEXT_ENTRY(book_node, "date", bookmark.getDate());
   }
@@ -135,7 +139,7 @@ std::string LibXMLDumper::dumpLibXMLBookmark()
   pugi::xml_node bookmarksNode = doc.append_child("bookmarks");
 
   if (library) {
-    for (auto& bookmark: library->getBookmarks()) {
+    for (auto& bookmark: library->getBookmarks(false)) {
       handleBookmark(bookmark, bookmarksNode);
     }
   }

test/library.cpp

@@ -20,7 +20,6 @@
 #include "gtest/gtest.h"
 #include <string>
 
-
 const char * sampleOpdsStream = R"(
 <feed xmlns="http://www.w3.org/2005/Atom"
       xmlns:dc="http://purl.org/dc/terms/"
@@ -28,12 +27,12 @@ const char * sampleOpdsStream = R"(
   <id>00000000-0000-0000-0000-000000000000</id>
   <entry>
     <title>Encyclopédie de la Tunisie</title>
-    <name>wikipedia_fr_tunisie_novid_2018-10</name>
+    <name>wikipedia_fr_tunisie</name>
-    <flavour>unforgettable</flavour>
+    <flavour>novid</flavour>
     <id>urn:uuid:0c45160e-f917-760a-9159-dfe3c53cdcdd</id>
     <icon>/meta?name=favicon&amp;content=wikipedia_fr_tunisie_novid_2018-10</icon>
     <updated>2018-10-08T00:00::00:Z</updated>
-    <dc:issued>8 Oct 2018</dc:issued>
+    <dc:issued>2018-10-08T00:00::00:Z</dc:issued>
     <language>fra</language>
     <summary>Le meilleur de Wikipédia sur la Tunisie</summary>
     <tags>wikipedia;novid;_ftindex</tags>
@@ -49,9 +48,53 @@ const char * sampleOpdsStream = R"(
     <mediaCount>1100</mediaCount>
     <articleCount>172</articleCount>
   </entry>
+  <entry>
+    <title>Encyclopédie de la Tunisie</title>
+    <name>wikipedia_fr_tunisie</name>
+    <flavour>novid</flavour>
+    <id>urn:uuid:0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater</id>
+    <updated>2019-10-08T00:00::00:Z</updated>
+    <dc:issued>2019-10-08T00:00::00:Z</dc:issued>
+    <language>fra</language>
+    <summary>Le meilleur de Wikipédia sur la Tunisie. Updated in 2019</summary>
+    <author>
+      <name>Wikipedia</name>
+    </author>
+    <link rel="http://opds-spec.org/acquisition/open-access" type="application/x-zim" href="http://download.kiwix.org/zim/wikipedia/wikipedia_fr_tunisie_novid_2018-10.zim.meta4" length="90030080" />
+  </entry>
+  <entry>
+    <title>Encyclopédie de la Tunisie</title>
+    <name>wikipedia_fr_tunisie</name>
+    <flavour>other_flavour</flavour>
+    <id>urn:uuid:0c45160e-f917-760a-9159-dfe3c53cdcdd_flavour</id>
+    <updated>2018-10-08T00:00::00:Z</updated>
+    <dc:issued>2018-10-08T00:00::00:Z</dc:issued>
+    <language>fra</language>
+    <summary>Le meilleur de Wikipédia sur la Tunisie. With another flavour</summary>
+    <author>
+      <name>Wikipedia</name>
+    </author>
+    <link rel="http://opds-spec.org/acquisition/open-access" type="application/x-zim" href="http://download.kiwix.org/zim/wikipedia/wikipedia_fr_tunisie_novid_2018-10.zim.meta4" length="90030080" />
+  </entry>
+  <entry>
+    <title>Encyclopédie de la Tunisie</title>
+    <name>wikipedia_fr_tunisie</name>
+    <flavour>other_flavour</flavour>
+    <id>urn:uuid:0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater_flavour</id>
+    <updated>2019-10-08T00:00::00:Z</updated>
+    <dc:issued>2019-10-08T00:00::00:Z</dc:issued>
+    <language>fra</language>
+    <summary>Le meilleur de Wikipédia sur la Tunisie. Updated in 2019, and other flavour</summary>
+    <author>
+      <name>Wikipedia</name>
+    </author>
+    <link rel="http://opds-spec.org/acquisition/open-access" type="application/x-zim" href="http://download.kiwix.org/zim/wikipedia/wikipedia_fr_tunisie_novid_2018-10.zim.meta4" length="90030080" />
+  </entry>
   <entry>
     <title>Tania Louis</title>
     <id>urn:uuid:0d0bcd57-d3f6-cb22-44cc-a723ccb4e1b2</id>
+    <name>biologie-tout-compris_fr_all</name>
+    <flavour>full</flavour>
     <icon>/meta?name=favicon&amp;content=biologie-tout-compris_fr_all_2018-06</icon>
     <updated>2018-06-23T00:00::00:Z</updated>
     <language>fra</language>
@@ -67,6 +110,8 @@ const char * sampleOpdsStream = R"(
   <entry>
     <title>Wikiquote</title>
     <id>urn:uuid:0ea1cde6-441d-6c58-f2c7-21c2838e659f</id>
+    <name>wikiquote_fr_all</name>
+    <flavour>full</flavour>
     <icon>/meta?name=favicon&amp;content=wikiquote_fr_all_nopic_2019-06</icon>
     <updated>2019-06-05T00:00::00:Z</updated>
     <language>fra,ita</language>
@@ -83,6 +128,8 @@ const char * sampleOpdsStream = R"(
   <entry>
     <title>Géographie par Wikipédia</title>
     <id>urn:uuid:1123e574-6eef-6d54-28fc-13e4caeae474</id>
+    <name>wikipedia_fr_geography</name>
+    <flavour>full</flavour>
     <icon>/meta?name=favicon&amp;content=wikipedia_fr_geography_nopic_2019-06</icon>
     <updated>2019-06-02T00:00::00:Z</updated>
     <summary>Une sélection d'articles de Wikipédia sur la géographie</summary>
@@ -99,6 +146,8 @@ const char * sampleOpdsStream = R"(
   <entry>
     <title>Mathématiques</title>
     <id>urn:uuid:14829621-c490-c376-0792-9de558b57efa</id>
+    <name>wikipedia_fr_mathematics</name>
+    <flavour>novid</flavour>
     <icon>/meta?name=favicon&amp;content=wikipedia_fr_mathematics_nopic_2019-05</icon>
     <updated>2019-05-13T00:00::00:Z</updated>
     <language>fra</language>
@@ -115,6 +164,8 @@ const char * sampleOpdsStream = R"(
 <entry>
     <title>Granblue Fantasy Wiki</title>
     <id>urn:uuid:006cbd1b-16d8-b00d-a584-c1ae110a94ed</id>
+    <name>grandbluefantasy_en_all</name>
+    <flavour>novid</flavour>
     <icon>/meta?name=favicon&amp;content=granbluefantasy_en_all_all_nopic_2018-10</icon>
     <updated>2018-10-14T00:00::00:Z</updated>
     <language>eng</language>
@@ -130,6 +181,8 @@ const char * sampleOpdsStream = R"(
   <entry>
     <title>Movies &amp; TV Stack Exchange</title>
     <id>urn:uuid:00f37b00-f4da-0675-995a-770f9c72903e</id>
+    <name>movies.stackexchange.com_en_all</name>
+    <flavour>novid</flavour>
     <icon>/meta?name=favicon&amp;content=movies.stackexchange.com_en_all_2019-02</icon>
     <updated>2019-02-03T00:00::00:Z</updated>
     <language>eng</language>
@@ -143,8 +196,10 @@ const char * sampleOpdsStream = R"(
     <link rel="http://opds-spec.org/image/thumbnail" type="image/png" href="/meta?name=favicon&amp;content=movies.stackexchange.com_en_all_2019-02" />
   </entry>
   <entry>
-    <title>TED talks - Business</title>
+    <title>TED"talks" - Business</title>
     <id>urn:uuid:0189d9be-2fd0-b4b6-7300-20fab0b5cdc8</id>
+    <name>ted_en_business</name>
+    <flavour>nodet</flavour>
     <icon>/meta?name=favicon&amp;content=ted_en_business_2018-07</icon>
     <updated>2018-07-23T00:00::00:Z</updated>
     <language>eng</language>
@@ -157,9 +212,28 @@ const char * sampleOpdsStream = R"(
     <link rel="http://opds-spec.org/acquisition/open-access" type="application/x-zim" href="http://download.kiwix.org/zim/ted/ted_en_business_2018-07.zim.meta4" length="8855827456" />
     <link rel="http://opds-spec.org/image/thumbnail" type="image/png" href="/meta?name=favicon&amp;content=ted_en_business_2018-07" />
   </entry>
+  <entry>
+    <title>Business talks about TED</title>
+    <id>Dummy id </id>
+    <name>speak_business</name>
+    <flavour>nodet</flavour>
+    <icon>/meta?name=favicon&amp;content=ted_en_business_2018-07</icon>
+    <updated>2018-08-23T00:00::00:Z</updated>
+    <language>eng</language>
+    <summary>Ideas worth spreading</summary>
+    <tags></tags>
+    <link type="text/html" href="/ted_en_business_2018-07" />
+    <author>
+      <name>TED</name>
+    </author>
+    <link rel="http://opds-spec.org/acquisition/open-access" type="application/x-zim" href="http://download.kiwix.org/zim/ted/ted_en_business_2018-07.zim.meta4" length="8855827456" />
+    <link rel="http://opds-spec.org/image/thumbnail" type="image/png" href="/meta?name=favicon&amp;content=ted_en_business_2018-07" />
+  </entry>
   <entry>
     <title>Mythology &amp; Folklore Stack Exchange</title>
     <id>urn:uuid:028055ac-4acc-1d54-65e0-a96de45e1b22</id>
+    <name>mythology.stackexchange.com_en_all</name>
+    <flavour>novid</flavour>
     <icon>/meta?name=favicon&amp;content=mythology.stackexchange.com_en_all_2019-02</icon>
     <updated>2019-02-03T00:00::00:Z</updated>
     <language>eng</language>
@@ -175,6 +249,8 @@ const char * sampleOpdsStream = R"(
   <entry>
     <title>Islam Stack Exchange</title>
     <id>urn:uuid:02e9c7ff-36fc-9c6e-6ac7-cd7085989029</id>
+    <name>islam.stackexchange.com_en_all</name>
+    <flavour>novid</flavour>
     <icon>/meta?name=favicon&amp;content=islam.stackexchange.com_en_all_2019-01</icon>
     <updated>2019-01-31T00:00::00:Z</updated>
     <language>eng</language>
@@ -229,6 +305,7 @@ const char sampleLibraryXML[] = R"(
 
 #include "../include/library.h"
 #include "../include/manager.h"
+#include "../include/book.h"
 #include "../include/bookmark.h"
 
 namespace
@@ -242,17 +319,17 @@ TEST(LibraryOpdsImportTest, allInOne)
   kiwix::Manager manager(lib);
   manager.readOpds(sampleOpdsStream, "library-opds-import.unittests.dev");
 
-  EXPECT_EQ(10U, lib->getBookCount(true, true));
+  EXPECT_EQ(14U, lib->getBookCount(true, true));
 
   {
   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");
+  EXPECT_EQ(book1.getName(), "wikipedia_fr_tunisie");
-  EXPECT_EQ(book1.getFlavour(), "unforgettable");
+  EXPECT_EQ(book1.getFlavour(), "novid");
   EXPECT_EQ(book1.getLanguages(), Langs{ "fra" });
   EXPECT_EQ(book1.getCommaSeparatedLanguages(), "fra");
-  EXPECT_EQ(book1.getDate(), "8 Oct 2018");
+  EXPECT_EQ(book1.getDate(), "2018-10-08");
   EXPECT_EQ(book1.getDescription(), "Le meilleur de Wikipédia sur la Tunisie");
   EXPECT_EQ(book1.getCreator(), "Wikipedia");
   EXPECT_EQ(book1.getPublisher(), "Wikipedia Publishing House");
@@ -272,9 +349,9 @@ TEST(LibraryOpdsImportTest, allInOne)
 
   {
   const kiwix::Book& book2 = lib->getBookById("0189d9be-2fd0-b4b6-7300-20fab0b5cdc8");
-  EXPECT_EQ(book2.getTitle(), "TED talks - Business");
+  EXPECT_EQ(book2.getTitle(), "TED\"talks\" - Business");
-  EXPECT_EQ(book2.getName(), "");
+  EXPECT_EQ(book2.getName(), "ted_en_business");
-  EXPECT_EQ(book2.getFlavour(), "");
+  EXPECT_EQ(book2.getFlavour(), "nodet");
   EXPECT_EQ(book2.getLanguages(), Langs{ "eng" });
   EXPECT_EQ(book2.getCommaSeparatedLanguages(), "eng");
   EXPECT_EQ(book2.getDate(), "2018-07-23");
@@ -309,9 +386,16 @@ class LibraryTest : public ::testing::Test {
      manager.readXml(sampleLibraryXML, false, "./test/library.xml", true);
   }
 
-    kiwix::Bookmark createBookmark(const std::string &id) {
+  kiwix::Bookmark createBookmark(const std::string &id, const std::string& url="", const std::string& title="") {
     kiwix::Bookmark bookmark;
     bookmark.setBookId(id);
+    bookmark.setUrl(url);
+    bookmark.setTitle(title);
+    return bookmark;
+  };
+
+  kiwix::Bookmark createBookmark(const kiwix::Book& book, const std::string& url="", const std::string& title="") {
+    kiwix::Bookmark bookmark(book, url, title);
     return bookmark;
   };
 
@@ -327,14 +411,35 @@ class LibraryTest : public ::testing::Test {
   std::shared_ptr<kiwix::Library> lib;
 };
 
+TEST_F(LibraryTest, createBookMark)
+{
+  auto bookId = "0c45160e-f917-760a-9159-dfe3c53cdcdd";
+  auto book = lib->getBookById(bookId);
+
+  auto bookmark = createBookmark(book, "/a/url", "A title");
+
+  EXPECT_EQ(bookmark.getUrl(), "/a/url");
+  EXPECT_EQ(bookmark.getTitle(), "A title");
+  EXPECT_EQ(bookmark.getBookId(), bookId);
+  EXPECT_EQ(bookmark.getBookName(), book.getName());
+  EXPECT_EQ(bookmark.getBookName(), "wikipedia_fr_tunisie");
+  EXPECT_EQ(bookmark.getBookTitle(), book.getTitle());
+  EXPECT_EQ(bookmark.getDate(), book.getDate());
+  EXPECT_EQ(bookmark.getBookFlavour(), book.getFlavour());
+  EXPECT_EQ(bookmark.getLanguage(), book.getCommaSeparatedLanguages());
+}
+
 TEST_F(LibraryTest, getBookMarksTest)
 {
-    auto bookId1 = lib->getBooksIds()[0];
+    auto bookId1 = "0c45160e-f917-760a-9159-dfe3c53cdcdd";
-    auto bookId2 = lib->getBooksIds()[1];
+    auto bookId2 = "0189d9be-2fd0-b4b6-7300-20fab0b5cdc8";
 
-    lib->addBookmark(createBookmark(bookId1));
+    auto book1 = lib->getBookById(bookId1);
-    lib->addBookmark(createBookmark("invalid-bookmark-id"));
+    auto book2 = lib->getBookById(bookId2);
-    lib->addBookmark(createBookmark(bookId2));
+
+    lib->addBookmark(createBookmark(book1));
+    lib->addBookmark(createBookmark("invalid-book-id"));
+    lib->addBookmark(createBookmark(book2));
     auto onlyValidBookmarks = lib->getBookmarks();
     auto allBookmarks = lib->getBookmarks(false);
 
@@ -342,13 +447,284 @@ TEST_F(LibraryTest, getBookMarksTest)
     EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
 
     EXPECT_EQ(allBookmarks[0].getBookId(), bookId1);
-    EXPECT_EQ(allBookmarks[1].getBookId(), "invalid-bookmark-id");
+    EXPECT_EQ(allBookmarks[1].getBookId(), "invalid-book-id");
     EXPECT_EQ(allBookmarks[2].getBookId(), bookId2);
 }
 
+TEST_F(LibraryTest, bookmarksSerializationTest)
+{
+    auto bookId1 = lib->getBooksIds()[0];
+    auto bookId2 = lib->getBooksIds()[1];
+
+    auto book1 = lib->getBookById(bookId1);
+    auto book2 = lib->getBookById(bookId2);
+
+    // Create bookmarks using three different ways.
+    lib->addBookmark(createBookmark(bookId1, "a/url", "Article title1"));
+    lib->addBookmark(createBookmark("invalid-book-id", "another/url", "Unknown title"));
+    lib->addBookmark(createBookmark(book2, "a/url/2", "Article title2"));
+
+    lib->writeBookmarksToFile("__test__bookmarks.xml");
+
+    // Build a new library
+    auto new_lib = kiwix::Library::create();
+    {
+      kiwix::Manager manager(new_lib);
+      manager.readOpds(sampleOpdsStream, "foo.urlHost");
+      manager.readXml(sampleLibraryXML, false, "./test/library.xml", true);
+      manager.readBookmarkFile("__test__bookmarks.xml");
+    }
+    std::remove("__test__bookmarks.xml");
+
+    auto onlyValidBookmarks = new_lib->getBookmarks();
+    auto allBookmarks = new_lib->getBookmarks(false);
+
+    ASSERT_EQ(onlyValidBookmarks.size(), 2);
+    EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1);
+    EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
+
+    ASSERT_EQ(allBookmarks.size(), 3);
+    auto bookmark1 = allBookmarks[0];
+    EXPECT_EQ(bookmark1.getBookId(), bookId1);
+    EXPECT_EQ(bookmark1.getBookTitle(), book1.getTitle());
+    EXPECT_EQ(bookmark1.getBookName(), book1.getName());
+    EXPECT_EQ(bookmark1.getBookFlavour(), book1.getFlavour());
+    EXPECT_EQ(bookmark1.getUrl(), "a/url");
+    EXPECT_EQ(bookmark1.getTitle(), "Article title1");
+    EXPECT_EQ(bookmark1.getLanguage(), book1.getCommaSeparatedLanguages());
+    EXPECT_EQ(bookmark1.getDate(), book1.getDate());
+
+    auto bookmark2 = allBookmarks[1];
+    EXPECT_EQ(bookmark2.getBookId(), "invalid-book-id");
+    EXPECT_EQ(bookmark2.getBookTitle(), "");
+    EXPECT_EQ(bookmark2.getBookName(), "");
+    EXPECT_EQ(bookmark2.getBookFlavour(), "");
+    EXPECT_EQ(bookmark2.getUrl(), "another/url");
+    EXPECT_EQ(bookmark2.getTitle(), "Unknown title");
+    EXPECT_EQ(bookmark2.getLanguage(), "");
+    EXPECT_EQ(bookmark2.getDate(), "");
+
+    auto bookmark3 = allBookmarks[2];
+    EXPECT_EQ(bookmark3.getBookId(), bookId2);
+    EXPECT_EQ(bookmark3.getBookTitle(), book2.getTitle());
+    EXPECT_EQ(bookmark3.getBookName(), book2.getName());
+    EXPECT_EQ(bookmark3.getBookFlavour(), book2.getFlavour());
+    EXPECT_EQ(bookmark3.getUrl(), "a/url/2");
+    EXPECT_EQ(bookmark3.getTitle(), "Article title2");
+    EXPECT_EQ(bookmark3.getLanguage(), book2.getCommaSeparatedLanguages());
+    EXPECT_EQ(bookmark3.getDate(), book2.getDate());
+}
+
+TEST_F(LibraryTest, MigrateBookmark)
+{
+    std::string bookId1 = "0c45160e-f917-760a-9159-dfe3c53cdcdd";
+    std::string bookId2 = "0189d9be-2fd0-b4b6-7300-20fab0b5cdc8";
+
+    auto book1 = lib->getBookById(bookId1);
+    auto book1Flavour = lib->getBookById(bookId1+"_flavour");
+    auto book2 = lib->getBookById(bookId2);
+
+    lib->addBookmark(createBookmark(book1));
+    lib->addBookmark(createBookmark("invalid-book-id"));
+    lib->addBookmark(createBookmark(book2));
+
+    auto wrongIdBookmark = createBookmark(book1);
+    wrongIdBookmark.setBookId("wrong-book-id");
+    lib->addBookmark(wrongIdBookmark);
+
+    auto wrongIdBookmarkNoName = createBookmark(book2);
+    wrongIdBookmarkNoName.setBookId("wrong-book-id-noname");
+    wrongIdBookmarkNoName.setBookName("");
+    lib->addBookmark(wrongIdBookmarkNoName);
+
+    auto wrongIdFlavourBookmark = createBookmark(book1Flavour);
+    wrongIdFlavourBookmark.setBookId("wrong-book-flavour-id");
+    lib->addBookmark(wrongIdFlavourBookmark);
+
+    auto onlyValidBookmarks = lib->getBookmarks();
+    auto allBookmarks = lib->getBookmarks(false);
+
+    ASSERT_EQ(onlyValidBookmarks.size(), 2);
+    EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1);
+    EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
+
+    ASSERT_EQ(allBookmarks.size(), 6);
+    EXPECT_EQ(allBookmarks[0].getBookId(), bookId1);
+    EXPECT_EQ(allBookmarks[1].getBookId(), "invalid-book-id");
+    EXPECT_EQ(allBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[3].getBookId(), "wrong-book-id");
+    EXPECT_EQ(allBookmarks[4].getBookId(), "wrong-book-id-noname");
+    EXPECT_EQ(allBookmarks[5].getBookId(), "wrong-book-flavour-id");
+
+    ASSERT_EQ(lib->migrateBookmarks("no-existant-book"), 0);
+
+    ASSERT_EQ(lib->migrateBookmarks(), std::make_tuple(3, 4));
+
+    onlyValidBookmarks = lib->getBookmarks();
+    allBookmarks = lib->getBookmarks(false);
+
+    ASSERT_EQ(onlyValidBookmarks.size(), 5);
+    EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId1);
+    EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[2].getBookId(), bookId1+"_updated1yearlater");
+    EXPECT_EQ(onlyValidBookmarks[3].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[4].getBookId(), bookId1+"_updated1yearlater_flavour");
+
+    ASSERT_EQ(allBookmarks.size(), 6);
+    EXPECT_EQ(allBookmarks[0].getBookId(), bookId1);
+    EXPECT_EQ(allBookmarks[1].getBookId(), "invalid-book-id");
+    EXPECT_EQ(allBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[3].getBookId(), bookId1+"_updated1yearlater");
+    EXPECT_EQ(allBookmarks[4].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[5].getBookId(), bookId1+"_updated1yearlater_flavour");
+
+    ASSERT_EQ(lib->migrateBookmarks(), std::make_tuple(0, 1));
+
+    ASSERT_EQ(lib->migrateBookmarks(bookId1), 1);
+    allBookmarks = lib->getBookmarks(false);
+    ASSERT_EQ(allBookmarks.size(), 6);
+    EXPECT_EQ(allBookmarks[0].getBookId(), bookId1+"_updated1yearlater");
+    EXPECT_EQ(allBookmarks[1].getBookId(), "invalid-book-id");
+    EXPECT_EQ(allBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[3].getBookId(), bookId1+"_updated1yearlater");
+    EXPECT_EQ(allBookmarks[4].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[5].getBookId(), bookId1+"_updated1yearlater_flavour");
+
+    ASSERT_EQ(lib->migrateBookmarks(bookId1, bookId2), 0); // No more bookId1 bookmark
+
+    ASSERT_EQ(lib->migrateBookmarks(bookId1+"_updated1yearlater", bookId2), 2);
+    onlyValidBookmarks = lib->getBookmarks();
+    allBookmarks = lib->getBookmarks(false);
+
+    ASSERT_EQ(onlyValidBookmarks.size(), 5);
+    EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[3].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[4].getBookId(), bookId1+"_updated1yearlater_flavour");
+
+    ASSERT_EQ(allBookmarks.size(), 6);
+    EXPECT_EQ(allBookmarks[0].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[1].getBookId(), "invalid-book-id");
+    EXPECT_EQ(allBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[3].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[4].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[5].getBookId(), bookId1+"_updated1yearlater_flavour");
+
+    ASSERT_EQ(lib->migrateBookmarks("invalid-book-id", bookId1), 1);
+
+    onlyValidBookmarks = lib->getBookmarks();
+    allBookmarks = lib->getBookmarks(false);
+
+    ASSERT_EQ(onlyValidBookmarks.size(), 6);
+    EXPECT_EQ(onlyValidBookmarks[0].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[1].getBookId(), bookId1);
+    EXPECT_EQ(onlyValidBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[3].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[4].getBookId(), bookId2);
+    EXPECT_EQ(onlyValidBookmarks[5].getBookId(), bookId1+"_updated1yearlater_flavour");
+
+
+    ASSERT_EQ(allBookmarks.size(), 6);
+    EXPECT_EQ(allBookmarks[0].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[1].getBookId(), bookId1);
+    EXPECT_EQ(allBookmarks[2].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[3].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[4].getBookId(), bookId2);
+    EXPECT_EQ(allBookmarks[5].getBookId(), bookId1+"_updated1yearlater_flavour");
+}
+
+TEST_F(LibraryTest, GetBestTargetBookIdOlder)
+{
+    auto bookId = std::string("0c45160e-f917-760a-9159-dfe3c53cdcdd");
+
+    auto book = lib->getBookById(bookId);
+
+    auto validBookmark = createBookmark(book);
+    lib->addBookmark(validBookmark);
+
+    ASSERT_EQ(lib->getBestTargetBookId(validBookmark, kiwix::UPGRADE_ONLY), bookId+"_updated1yearlater");
+    ASSERT_EQ(lib->getBestTargetBookId(validBookmark, kiwix::ALLOW_DOWNGRADE), bookId+"_updated1yearlater");
+}
+
+TEST_F(LibraryTest, GetBestTargetBookIdNewer)
+{
+    auto bookId = std::string("0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater");
+
+    auto book = lib->getBookById(bookId);
+    EXPECT_EQ(book.getDate(), "2019-10-08");
+
+    auto validBookmark = createBookmark(book);
+    // Make the bookmark more recent than any books in the library.
+    // (But still pointing to existing book)
+    validBookmark.setDate("2020-10-08");
+    lib->addBookmark(validBookmark);
+
+    //  The best book for the bookmark is bookId...
+    ASSERT_EQ(lib->getBestTargetBookId(validBookmark, kiwix::UPGRADE_ONLY), bookId);
+    // but there is not migration to do as the bookmark already point to it.
+    ASSERT_EQ(lib->migrateBookmarks(bookId, kiwix::UPGRADE_ONLY), 0);
+
+    ASSERT_EQ(lib->getBestTargetBookId(validBookmark, kiwix::ALLOW_DOWNGRADE), bookId);
+}
+
+TEST_F(LibraryTest, GetBestTargetBookIdInvalidOlder)
+{
+    auto bookId = std::string("0c45160e-f917-760a-9159-dfe3c53cdcdd");
+
+    auto book = lib->getBookById(bookId);
+
+    auto invalidBookmark = createBookmark(book);
+    invalidBookmark.setBookId("invalid-book-id");
+    lib->addBookmark(invalidBookmark);
+
+    ASSERT_EQ(lib->getBestTargetBookId(invalidBookmark, kiwix::UPGRADE_ONLY), bookId+"_updated1yearlater");
+    ASSERT_EQ(lib->getBestTargetBookId(invalidBookmark, kiwix::ALLOW_DOWNGRADE), bookId+"_updated1yearlater");
+}
+
+TEST_F(LibraryTest, GetBestTargetBookIdInvalidNewer)
+{
+    auto bookId = std::string("0c45160e-f917-760a-9159-dfe3c53cdcdd");
+
+    auto book = lib->getBookById(bookId);
+    EXPECT_EQ(book.getDate(), "2018-10-08");
+
+    auto invalidBookmark = createBookmark(book);
+    invalidBookmark.setBookId("invalid-book-id");
+    invalidBookmark.setDate("2020-10-08");
+    lib->addBookmark(invalidBookmark);
+
+    ASSERT_EQ(lib->getBestTargetBookId(invalidBookmark, kiwix::UPGRADE_ONLY), "");
+    ASSERT_EQ(lib->getBestTargetBookId(invalidBookmark, kiwix::ALLOW_DOWNGRADE), bookId+"_updated1yearlater");
+}
+
+TEST_F(LibraryTest, GetBestTargetBookIdFlavour)
+{
+    auto bookId = std::string("0c45160e-f917-760a-9159-dfe3c53cdcdd_flavour");
+
+    auto book = lib->getBookById(bookId);
+    EXPECT_EQ(book.getDate(), "2018-10-08");
+
+    auto invalidBookmark = createBookmark(book);
+    invalidBookmark.setBookId("invalid-book-id");
+    invalidBookmark.setDate("2020-10-08");
+    lib->addBookmark(invalidBookmark);
+
+    ASSERT_EQ(lib->getBestTargetBookId(invalidBookmark, kiwix::UPGRADE_ONLY), "");
+    ASSERT_EQ(lib->getBestTargetBookId(invalidBookmark, kiwix::ALLOW_DOWNGRADE), "0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater_flavour");
+}
+
+TEST_F(LibraryTest, GetBestTargetBookIdName)
+{
+    ASSERT_EQ(lib->getBestTargetBookId("wikipedia_fr_tunisie"), "0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater");
+    ASSERT_EQ(lib->getBestTargetBookId("wikipedia_fr_tunisie", "novid"), "0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater");
+    ASSERT_EQ(lib->getBestTargetBookId("wikipedia_fr_tunisie", "other_flavour"), "0c45160e-f917-760a-9159-dfe3c53cdcdd_updated1yearlater_flavour");
+    ASSERT_EQ(lib->getBestTargetBookId("wikipedia_fr_tunisie", "other_flavour", "2020-12-12"), "");
+}
+
 TEST_F(LibraryTest, sanityCheck)
 {
-  EXPECT_EQ(lib->getBookCount(true, true), 12U);
+  EXPECT_EQ(lib->getBookCount(true, true), 16U);
   EXPECT_EQ(lib->getBooksLanguages(),
             std::vector<std::string>({"deu", "eng", "fra", "ita", "spa"})
   );
@@ -400,6 +776,10 @@ TEST_F(LibraryTest, filterLocal)
   );
 
   EXPECT_FILTER_RESULTS(kiwix::Filter().local(false),
+    "Business talks about TED",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Granblue Fantasy Wiki",
     "Géographie par Wikipédia",
@@ -407,7 +787,7 @@ TEST_F(LibraryTest, filterLocal)
     "Mathématiques",
     "Movies & TV Stack Exchange",
     "Mythology & Folklore Stack Exchange",
-    "TED talks - Business",
+    "TED\"talks\" - Business",
     "Tania Louis",
     "Wikiquote"
   );
@@ -416,6 +796,10 @@ TEST_F(LibraryTest, filterLocal)
 TEST_F(LibraryTest, filterRemote)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter().remote(true),
+    "Business talks about TED",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Granblue Fantasy Wiki",
     "Géographie par Wikipédia",
@@ -424,7 +808,7 @@ TEST_F(LibraryTest, filterRemote)
     "Movies & TV Stack Exchange",
     "Mythology & Folklore Stack Exchange",
     "Ray Charles",
-    "TED talks - Business",
+    "TED\"talks\" - Business",
     "Tania Louis",
     "Wikiquote"
   );
@@ -437,21 +821,23 @@ TEST_F(LibraryTest, filterRemote)
 TEST_F(LibraryTest, filterByLanguage)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter().lang("eng"),
+    "Business talks about TED",
     "Granblue Fantasy Wiki",
     "Islam Stack Exchange",
     "Movies & TV Stack Exchange",
     "Mythology & Folklore Stack Exchange",
     "Ray Charles",
-    "TED talks - Business"
+    "TED\"talks\" - Business"
   );
 
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("lang:eng"),
+    "Business talks about TED",
     "Granblue Fantasy Wiki",
     "Islam Stack Exchange",
     "Movies & TV Stack Exchange",
     "Mythology & Folklore Stack Exchange",
     "Ray Charles",
-    "TED talks - Business"
+    "TED\"talks\" - Business"
   );
 
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("eng"),
@@ -459,6 +845,25 @@ TEST_F(LibraryTest, filterByLanguage)
   );
 }
 
+TEST_F(LibraryTest, filterByFlavour)
+{
+  EXPECT_FILTER_RESULTS(kiwix::Filter().flavour("full"),
+    "Géographie par Wikipédia",
+    "Tania Louis",
+    "Wikiquote"
+  );
+
+  EXPECT_FILTER_RESULTS(kiwix::Filter().query("flavour:full"),
+    "Géographie par Wikipédia",
+    "Tania Louis",
+    "Wikiquote"
+  );
+
+  EXPECT_FILTER_RESULTS(kiwix::Filter().query("full"),
+    /* no results */
+  );
+}
+
 TEST_F(LibraryTest, filterByTags)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter().acceptTags({"stackexchange"}),
@@ -558,6 +963,9 @@ TEST_F(LibraryTest, filterByQuery)
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("Wiki"),
     "An example ZIM archive", // due to the "wikibooks" tag
     "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Granblue Fantasy Wiki",
     "Géographie par Wikipédia",
     "Mathématiques", // due to the "wikipedia" tag
@@ -576,6 +984,10 @@ TEST_F(LibraryTest, filteringByEmptyQueryReturnsAllEntries)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter().query(""),
     "An example ZIM archive",
+    "Business talks about TED",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Granblue Fantasy Wiki",
     "Géographie par Wikipédia",
@@ -584,7 +996,7 @@ TEST_F(LibraryTest, filteringByEmptyQueryReturnsAllEntries)
     "Movies & TV Stack Exchange",
     "Mythology & Folklore Stack Exchange",
     "Ray Charles",
-    "TED talks - Business",
+    "TED\"talks\" - Business",
     "Tania Louis",
     "Wikiquote"
   );
@@ -593,6 +1005,9 @@ TEST_F(LibraryTest, filteringByEmptyQueryReturnsAllEntries)
 TEST_F(LibraryTest, filterByCreator)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter().creator("Wikipedia"),
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Géographie par Wikipédia",
     "Mathématiques",
@@ -634,6 +1049,9 @@ TEST_F(LibraryTest, filterByCreator)
   );
 
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("creator:Wikipedia"),
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Géographie par Wikipédia",
     "Mathématiques",
@@ -741,6 +1159,9 @@ TEST_F(LibraryTest, filterByMaxSize)
 TEST_F(LibraryTest, filterByMultipleCriteria)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("Wiki").creator("Wikipedia"),
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Géographie par Wikipédia",
     "Mathématiques", // due to the "wikipedia" tag
@@ -748,11 +1169,17 @@ TEST_F(LibraryTest, filterByMultipleCriteria)
   );
 
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("Wiki").creator("Wikipedia").maxSize(100000000UL),
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Ray Charles"
   );
 
   EXPECT_FILTER_RESULTS(kiwix::Filter().query("Wiki").creator("Wikipedia").maxSize(100000000UL).local(false),
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie"
   );
 }
@@ -810,6 +1237,10 @@ TEST_F(LibraryTest, removeBooksNotUpdatedSince)
 {
   EXPECT_FILTER_RESULTS(kiwix::Filter(),
     "An example ZIM archive",
+    "Business talks about TED",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
+    "Encyclopédie de la Tunisie",
     "Encyclopédie de la Tunisie",
     "Granblue Fantasy Wiki",
     "Géographie par Wikipédia",
@@ -818,7 +1249,7 @@ TEST_F(LibraryTest, removeBooksNotUpdatedSince)
     "Movies & TV Stack Exchange",
     "Mythology & Folklore Stack Exchange",
     "Ray Charles",
-    "TED talks - Business",
+    "TED\"talks\" - Business",
     "Tania Louis",
     "Wikiquote"
   );
@@ -832,7 +1263,7 @@ TEST_F(LibraryTest, removeBooksNotUpdatedSince)
 
   const uint64_t rev2 = lib->getRevision();
 
-  EXPECT_EQ(9u, lib->removeBooksNotUpdatedSince(rev));
+  EXPECT_EQ(13u, lib->removeBooksNotUpdatedSince(rev));
 
   EXPECT_GT(lib->getRevision(), rev2);