Merge pull request #1149 from kiwix/directory_monitoring_improvements

Directory monitoring improvements

src/contentmanager.cpp

@@ -849,3 +849,62 @@ void ContentManager::setSortBy(const QString& sortBy, const bool sortOrderAsc)
     m_sortOrderAsc = sortOrderAsc;
     emit(booksChanged());
 }
+
+void ContentManager::setMonitorDirZims(QString monitorDir, Library::QStringSet zimList)
+{
+    m_knownZimsInDir[monitorDir] = zimList;
+}
+
+void ContentManager::asyncUpdateLibraryFromDir(QString dir)
+{
+    (void) QtConcurrent::run([=]() {
+        updateLibraryFromDir(dir);
+    });
+}
+
+void ContentManager::updateLibraryFromDir(QString monitorDir)
+{
+    typedef Library::QStringSet QStringSet;
+
+    QMutexLocker locker(&m_updateFromDirMutex);
+    const QDir dir(monitorDir);
+    const QStringSet oldDirEntries = m_knownZimsInDir[monitorDir];
+    QStringSet newDirEntries;
+    for (const auto &file : dir.entryList({"*.zim"})) {
+        newDirEntries.insert(QDir::toNativeSeparators(monitorDir + "/" + file));
+    }
+    const QStringSet addedZims = newDirEntries - oldDirEntries;
+    const QStringSet removedZims = oldDirEntries - newDirEntries;
+    const auto kiwixLib = mp_library->getKiwixLibrary();
+    kiwix::Manager manager(kiwixLib);
+    bool needsRefresh = !removedZims.empty();
+    for (auto bookPath : removedZims) {
+        try {
+            // qDebug() << "DBG: ContentManager::updateLibraryFromDir(): "
+            //          << "file disappeared: " << bookPath;
+            const auto book = kiwixLib->getBookByPath(bookPath.toStdString());
+            handleDisappearedZimFile(QString::fromStdString(book.getId()));
+        } catch (...) {}
+    }
+    for (auto bookPath : addedZims) {
+        if ( mp_library->isBeingDownloadedByUs(bookPath) ) {
+            // qDebug() << "DBG: ContentManager::updateLibraryFromDir(): "
+            //          << bookPath
+            //          << " ignored since it is being downloaded by us.";
+        } else {
+            // qDebug() << "DBG: ContentManager::updateLibraryFromDir(): "
+            //          << "file appeared: " << bookPath;
+            needsRefresh |= manager.addBookFromPath(bookPath.toStdString());
+        }
+    }
+    if (needsRefresh) {
+        mp_library->save();
+        emit(booksChanged());
+        setMonitorDirZims(monitorDir, newDirEntries);
+    }
+}
+
+void ContentManager::handleDisappearedZimFile(QString bookId)
+{
+    mp_library->removeBookFromLibraryById(bookId);
+}

src/contentmanager.h

@@ -69,6 +69,9 @@ public: // functions
     QStringList getCategories() const { return m_categories; }
     LanguageList getLanguages() const { return m_languages; }
 
+    void setMonitorDirZims(QString monitorDir, Library::QStringSet zimList);
+    void asyncUpdateLibraryFromDir(QString dir);
+
 signals:
     void filterParamsChanged();
     void booksChanged();
@@ -113,6 +116,8 @@ private: // functions
     void updateModel();
     void setCategories();
     void setLanguages();
+    void updateLibraryFromDir(QString dir);
+    void handleDisappearedZimFile(QString bookId);
 
     // Get the book with the specified id from
     // the remote or local library (in that order).
@@ -141,6 +146,9 @@ private: // data
 
     ContentManagerModel *managerModel;
     QMutex remoteLibraryLocker;
+
+    QMutex m_updateFromDirMutex;
+    QMap<QString, Library::QStringSet> m_knownZimsInDir;
 };
 
 #endif // CONTENTMANAGER_H

src/kiwixapp.cpp

@@ -103,21 +103,28 @@ void KiwixApp::init()
         }
     });
     connect(&m_watcher, &QFileSystemWatcher::directoryChanged, this, [=](QString monitorDir) {
-        m_library.asyncUpdateFromDir(monitorDir);
+        mp_manager->asyncUpdateLibraryFromDir(monitorDir);
     });
+
+    setupDirectoryMonitoring();
+
+    restoreTabs();
+    restoreWindowState();
+}
+
+void KiwixApp::setupDirectoryMonitoring()
+{
     QString monitorDir = m_settingsManager.getMonitorDir();
     QString downloadDir = m_settingsManager.getDownloadDir();
     auto dirList = QSet<QString>({monitorDir, downloadDir});
     for (auto dir : dirList) {
         if (dir != "") {
-            m_library.setMonitorDirZims(dir, m_library.getLibraryZimsFromDir(dir));
+            const auto zimsInDir = m_library.getLibraryZimsFromDir(dir);
+            mp_manager->setMonitorDirZims(dir, zimsInDir);
             m_watcher.addPath(dir);
-            m_library.asyncUpdateFromDir(dir);
+            mp_manager->asyncUpdateLibraryFromDir(dir);
         }
     }
-
-    restoreTabs();
-    restoreWindowState();
 }
 
 KiwixApp::~KiwixApp()
@@ -336,15 +343,10 @@ bool KiwixApp::isCurrentArticleBookmarked()
 
 void KiwixApp::setMonitorDir(const QString &dir) {
     m_settingsManager.setMonitorDir(dir);
-    m_library.setMonitorDirZims(dir, QStringList());
     for (auto path : m_watcher.directories()) {
         m_watcher.removePath(path);
     }
-    if (dir != "") {
+    setupDirectoryMonitoring();
-        m_watcher.addPath(dir);
-        m_watcher.addPath(m_settingsManager.getDownloadDir());
-        m_library.asyncUpdateFromDir(dir);
-    }
 }
 
 #define CREATE_ACTION(ID, TEXT) \

src/kiwixapp.h

@@ -123,6 +123,7 @@ private:
 
     QAction*     mpa_actions[MAX_ACTION];
 
+    void setupDirectoryMonitoring();
     QString findLibraryDirectory();
     void restoreTabs();
     void loadAndInstallTranslations(QTranslator& translator, const QString& filename, const QString& directory);

src/library.cpp

@@ -194,74 +194,21 @@ void Library::save()
     mp_library->writeBookmarksToFile(kiwix::appendToDirectory(m_libraryDirectory.toStdString(), "library.bookmarks.xml"));
 }
 
-void Library::setMonitorDirZims(QString monitorDir, QStringList zimList)
+Library::QStringSet Library::getLibraryZimsFromDir(QString dir) const
 {
-    m_knownZimsInDir[monitorDir] = zimList;
+    QStringSet zimsInDir;
-}
-
-QStringList Library::getLibraryZimsFromDir(QString dir) const
-{
-    QStringList zimsInDir;
     for (auto str : getBookIds()) {
         auto filePath = QString::fromStdString(getBookById(str).getPath());
         if ( filePath.endsWith(BEINGDOWNLOADEDSUFFIX) )
                 continue;
         QDir absoluteDir = QFileInfo(filePath).absoluteDir();
         if (absoluteDir == dir) {
-            zimsInDir.push_back(filePath);
+            zimsInDir.insert(filePath);
         }
     }
     return zimsInDir;
 }
 
-void Library::updateFromDir(QString monitorDir)
-{
-    QMutexLocker locker(&m_updateFromDirMutex);
-    const QDir dir(monitorDir);
-    QStringList newDirEntries = dir.entryList({"*.zim"});
-    QStringList oldDirEntries = m_knownZimsInDir[monitorDir];
-    for (auto &str : newDirEntries) {
-        str = QDir::toNativeSeparators(monitorDir + "/" + str);
-    }
-    QSet<QString> newDir, oldDir;
-#if QT_VERSION < QT_VERSION_CHECK(5, 15, 0)
-    newDir = QSet<QString>::fromList(newDirEntries);
-    oldDir = QSet<QString>::fromList(oldDirEntries);
-#else
-    newDir = QSet<QString>(newDirEntries.begin(), newDirEntries.end());
-    oldDir = QSet<QString>(oldDirEntries.begin(), oldDirEntries.end());
-#endif
-    QStringList addedZims = (newDir - oldDir).values();
-    QStringList removedZims = (oldDir - newDir).values();
-    auto manager = kiwix::Manager(LibraryManipulator(this));
-    bool needsRefresh = !removedZims.empty();
-    for (auto bookPath : addedZims) {
-        if ( isBeingDownloadedByUs(bookPath) ) {
-            // qDebug() << "DBG: Library::updateFromDir(): "
-            //          << bookPath
-            //          << " ignored since it is being downloaded by us.";
-        } else {
-            needsRefresh |= manager.addBookFromPath(bookPath.toStdString());
-        }
-    }
-    for (auto bookPath : removedZims) {
-        try {
-            removeBookFromLibraryById(QString::fromStdString(mp_library->getBookByPath(bookPath.toStdString()).getId()));
-        } catch (...) {}
-    }
-    if (needsRefresh) {
-        emit(booksChanged());
-        setMonitorDirZims(monitorDir, newDir.values());
-    }
-}
-
-void Library::asyncUpdateFromDir(QString dir)
-{
-    (void) QtConcurrent::run([=]() {
-        updateFromDir(dir);
-    });
-}
-
 const kiwix::Book &Library::getBookById(QString id) const
 {
     return mp_library->getBookById(id.toStdString());

src/library.h

@@ -26,6 +26,8 @@ class Library : public QObject
     Q_OBJECT
     Q_PROPERTY(QStringList bookIds READ getBookIds NOTIFY booksChanged)
 public:
+    typedef QSet<QString> QStringSet;
+
     Library(const QString& libraryDirectory);
     virtual ~Library();
     QString openBookFromPath(const QString& zimPath);
@@ -34,8 +36,7 @@ public:
     QStringList getBookIds() const;
     QStringList listBookIds(const kiwix::Filter& filter, kiwix::supportedListSortBy sortBy, bool ascending) const;
     const std::vector<kiwix::Bookmark> getBookmarks(bool onlyValidBookmarks = false) const { return mp_library->getBookmarks(onlyValidBookmarks); }
-    QStringList getLibraryZimsFromDir(QString dir) const;
+    QStringSet getLibraryZimsFromDir(QString dir) const;
-    void setMonitorDirZims(QString monitorDir, QStringList zimList);
     void addBookToLibrary(kiwix::Book& book);
     void addBookBeingDownloaded(const kiwix::Book& book, QString downloadDir);
     bool isBeingDownloadedByUs(QString path) const;
@@ -45,8 +46,6 @@ public:
     void addBookmark(kiwix::Bookmark& bookmark);
     void removeBookmark(const QString& zimId, const QString& url);
     void save();
-    void updateFromDir(QString dir);
-    void asyncUpdateFromDir(QString dir);
     kiwix::LibraryPtr getKiwixLibrary() { return mp_library; }
 public slots:
     const kiwix::Book& getBookById(QString id) const;
@@ -56,10 +55,8 @@ signals:
     void bookmarksChanged();
 
 private:
-    QMutex m_updateFromDirMutex;
     kiwix::LibraryPtr mp_library;
     QString m_libraryDirectory;
-    QMap<QString, QStringList> m_knownZimsInDir;
 friend class LibraryManipulator;
 };