From a89e9b47999232a854f9e240d161cf321494fa6e Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 31 Jan 2024 16:09:51 +0400 Subject: [PATCH 01/17] Extracted downloadStatus2String() --- src/contentmanager.cpp | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 337838206..3cfe2f81c 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -335,6 +335,24 @@ void ContentManager::openBook(const QString &id) } } +namespace +{ + +const char* downloadStatus2String(kiwix::Download::StatusResult status) +{ + switch(status){ + case kiwix::Download::K_ACTIVE: return "active"; + case kiwix::Download::K_WAITING: return "waiting"; + case kiwix::Download::K_PAUSED: return "paused"; + case kiwix::Download::K_ERROR: return "error"; + case kiwix::Download::K_COMPLETE: return "completed"; + case kiwix::Download::K_REMOVED: return "removed"; + default: return "unknown"; + } +} + +} // unnamed namespace + #define ADD_V(KEY, METH) {if(key==KEY) {values.insert(key, QString::fromStdString((d->METH()))); continue;}} QMap ContentManager::updateDownloadInfos(QString id, const QStringList &keys) { @@ -380,28 +398,7 @@ QMap ContentManager::updateDownloadInfos(QString id, const QS for(auto& key: keys){ ADD_V("id", getDid); if(key == "status") { - switch(d->getStatus()){ - case kiwix::Download::K_ACTIVE: - values.insert(key, "active"); - break; - case kiwix::Download::K_WAITING: - values.insert(key, "waiting"); - break; - case kiwix::Download::K_PAUSED: - values.insert(key, "paused"); - break; - case kiwix::Download::K_ERROR: - values.insert(key, "error"); - break; - case kiwix::Download::K_COMPLETE: - values.insert(key, "completed"); - break; - case kiwix::Download::K_REMOVED: - values.insert(key, "removed"); - break; - default: - values.insert(key, "unknown"); - } + values.insert(key, downloadStatus2String(d->getStatus())); continue; } ADD_V("followedBy", getFollowedBy); From da1f06f9c8602cbb865d50ec6d476dc3e8caafd0 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 31 Jan 2024 16:32:22 +0400 Subject: [PATCH 02/17] Extracted getDownloadInfo() --- src/contentmanager.cpp | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 3cfe2f81c..e09a3a6cd 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -338,7 +338,7 @@ void ContentManager::openBook(const QString &id) namespace { -const char* downloadStatus2String(kiwix::Download::StatusResult status) +QString downloadStatus2QString(kiwix::Download::StatusResult status) { switch(status){ case kiwix::Download::K_ACTIVE: return "active"; @@ -351,9 +351,21 @@ const char* downloadStatus2String(kiwix::Download::StatusResult status) } } +QString getDownloadInfo(const kiwix::Download& d, const QString& k) +{ + if (k == "id") return QString::fromStdString(d.getDid()); + if (k == "path") return QString::fromStdString(d.getPath()); + if (k == "status") return downloadStatus2QString(d.getStatus()); + if (k == "followedBy") return QString::fromStdString(d.getFollowedBy()); + if (k == "totalLength") return QString::number(d.getTotalLength()); + if (k == "downloadSpeed") return QString::number(d.getDownloadSpeed()); + if (k == "verifiedLength") return QString::number(d.getVerifiedLength()); + if (k == "completedLength") return QString::number(d.getCompletedLength()); + return ""; +} + } // unnamed namespace -#define ADD_V(KEY, METH) {if(key==KEY) {values.insert(key, QString::fromStdString((d->METH()))); continue;}} QMap ContentManager::updateDownloadInfos(QString id, const QStringList &keys) { QMap values; @@ -396,29 +408,10 @@ QMap ContentManager::updateDownloadInfos(QString id, const QS } } for(auto& key: keys){ - ADD_V("id", getDid); - if(key == "status") { - values.insert(key, downloadStatus2String(d->getStatus())); - continue; - } - ADD_V("followedBy", getFollowedBy); - ADD_V("path", getPath); - if(key == "totalLength") { - values.insert(key, QString::number(d->getTotalLength())); - } - if(key == "completedLength") { - values.insert(key, QString::number(d->getCompletedLength())); - } - if(key == "downloadSpeed") { - values.insert(key, QString::number(d->getDownloadSpeed())); - } - if(key == "verifiedLength") { - values.insert(key, QString::number(d->getVerifiedLength())); - } + values.insert(key, getDownloadInfo(*d, key)); } return values; } -#undef ADD_V QString ContentManager::downloadBook(const QString &id, QModelIndex index) { From fecf83a06143a8f498d4ac9ae600173558813da1 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Wed, 7 Feb 2024 18:57:45 +0400 Subject: [PATCH 03/17] Extracted ContentManager::downloadCompleted() --- src/contentmanager.cpp | 34 +++++++++++++++++++--------------- src/contentmanager.h | 1 + 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index e09a3a6cd..5821bc951 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -366,6 +366,24 @@ QString getDownloadInfo(const kiwix::Download& d, const QString& k) } // unnamed namespace +void ContentManager::downloadCompleted(const kiwix::Book& b, QString path) +{ + kiwix::Book bCopy(b); + bCopy.setPath(QDir::toNativeSeparators(path).toStdString()); + bCopy.setDownloadId(""); + bCopy.setPathValid(true); + // removing book url so that download link in kiwix-serve is not displayed. + bCopy.setUrl(""); + mp_library->getKiwixLibrary()->addOrUpdateBook(bCopy); + mp_library->save(); + mp_library->bookmarksChanged(); + if (!m_local) { + emit(oneBookChanged(QString::fromStdString(b.getId()))); + } else { + emit(mp_library->booksChanged()); + } +} + QMap ContentManager::updateDownloadInfos(QString id, const QStringList &keys) { QMap values; @@ -391,21 +409,7 @@ QMap ContentManager::updateDownloadInfos(QString id, const QS d->updateStatus(true); if (d->getStatus() == kiwix::Download::K_COMPLETE) { - QString tmp(QString::fromStdString(d->getPath())); - kiwix::Book bCopy(b); - bCopy.setPath(QDir::toNativeSeparators(tmp).toStdString()); - bCopy.setDownloadId(""); - bCopy.setPathValid(true); - // removing book url so that download link in kiwix-serve is not displayed. - bCopy.setUrl(""); - mp_library->getKiwixLibrary()->addOrUpdateBook(bCopy); - mp_library->save(); - mp_library->bookmarksChanged(); - if (!m_local) { - emit(oneBookChanged(id)); - } else { - emit(mp_library->booksChanged()); - } + downloadCompleted(b, QString::fromStdString(d->getPath())); } for(auto& key: keys){ values.insert(key, getDownloadInfo(*d, key)); diff --git a/src/contentmanager.h b/src/contentmanager.h index bf673aea7..585954ce2 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -62,6 +62,7 @@ class ContentManager : public QObject QMutex remoteLibraryLocker; void setCategories(); void setLanguages(); + void downloadCompleted(const kiwix::Book& book, QString path); signals: void filterParamsChanged(); From 4f4c8117113182701ecf5d77ded3fb797b733bf7 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 8 Feb 2024 17:41:45 +0400 Subject: [PATCH 04/17] Extracted ContentManager::downloadCancelled() --- src/contentmanager.cpp | 16 ++++++++++------ src/contentmanager.h | 1 + 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 5821bc951..a88899caf 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -366,6 +366,15 @@ QString getDownloadInfo(const kiwix::Download& d, const QString& k) } // unnamed namespace +void ContentManager::downloadCancelled(const kiwix::Book& b) +{ + kiwix::Book bCopy(b); + bCopy.setDownloadId(""); + mp_library->getKiwixLibrary()->addOrUpdateBook(bCopy); + mp_library->save(); + emit(mp_library->booksChanged()); +} + void ContentManager::downloadCompleted(const kiwix::Book& b, QString path) { kiwix::Book bCopy(b); @@ -389,7 +398,6 @@ QMap ContentManager::updateDownloadInfos(QString id, const QS QMap values; if (!mp_downloader) { for(auto& key: keys) { - (void) key; values.insert(key, ""); } return values; @@ -399,11 +407,7 @@ QMap ContentManager::updateDownloadInfos(QString id, const QS try { d = mp_downloader->getDownload(b.getDownloadId()); } catch(...) { - kiwix::Book bCopy(b); - bCopy.setDownloadId(""); - mp_library->getKiwixLibrary()->addOrUpdateBook(bCopy); - mp_library->save(); - emit(mp_library->booksChanged()); + downloadCancelled(b); return values; } diff --git a/src/contentmanager.h b/src/contentmanager.h index 585954ce2..c6ce486e2 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -62,6 +62,7 @@ class ContentManager : public QObject QMutex remoteLibraryLocker; void setCategories(); void setLanguages(); + void downloadCancelled(const kiwix::Book& b); void downloadCompleted(const kiwix::Book& book, QString path); signals: From dd69a9d2bd44a85ebfeea87465a1e796d8fdc435 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 8 Feb 2024 18:15:42 +0400 Subject: [PATCH 05/17] ContentManager::DownloadInfo typedef --- src/contentmanager.cpp | 4 ++-- src/contentmanager.h | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index a88899caf..4eb90346e 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -393,9 +393,9 @@ void ContentManager::downloadCompleted(const kiwix::Book& b, QString path) } } -QMap ContentManager::updateDownloadInfos(QString id, const QStringList &keys) +ContentManager::DownloadInfo ContentManager::updateDownloadInfos(QString id, const QStringList &keys) { - QMap values; + DownloadInfo values; if (!mp_downloader) { for(auto& key: keys) { values.insert(key, ""); diff --git a/src/contentmanager.h b/src/contentmanager.h index c6ce486e2..90f7f1970 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -23,6 +23,9 @@ class ContentManager : public QObject typedef ContentManagerModel::BookInfo BookInfo; typedef ContentManagerModel::BookInfoList BookInfoList; + // XXX: potentional source of confusion with ::DownloadInfo from rownode.h + typedef QMap DownloadInfo; + public: // functions explicit ContentManager(Library* library, kiwix::Downloader *downloader, QObject *parent = nullptr); virtual ~ContentManager() {} @@ -81,7 +84,7 @@ public slots: QStringList getTranslations(const QStringList &keys); BookInfo getBookInfos(QString id, const QStringList &keys); void openBook(const QString& id); - QMap updateDownloadInfos(QString id, const QStringList& keys); + DownloadInfo updateDownloadInfos(QString id, const QStringList& keys); QString downloadBook(const QString& id); QString downloadBook(const QString& id, QModelIndex index); void updateLibrary(); From 2fbfe40c11941cc3f3157464a77575c1dd0ac435 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 8 Feb 2024 18:55:58 +0400 Subject: [PATCH 06/17] Extracted ContentManager::getDownloadInfo() --- src/contentmanager.cpp | 39 +++++++++++++++++++++++++++------------ src/contentmanager.h | 7 ++++--- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 4eb90346e..b065b326e 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -366,18 +366,18 @@ QString getDownloadInfo(const kiwix::Download& d, const QString& k) } // unnamed namespace -void ContentManager::downloadCancelled(const kiwix::Book& b) +void ContentManager::downloadCancelled(QString bookId) { - kiwix::Book bCopy(b); + kiwix::Book bCopy(mp_library->getBookById(bookId)); bCopy.setDownloadId(""); mp_library->getKiwixLibrary()->addOrUpdateBook(bCopy); mp_library->save(); emit(mp_library->booksChanged()); } -void ContentManager::downloadCompleted(const kiwix::Book& b, QString path) +void ContentManager::downloadCompleted(QString bookId, QString path) { - kiwix::Book bCopy(b); + kiwix::Book bCopy(mp_library->getBookById(bookId)); bCopy.setPath(QDir::toNativeSeparators(path).toStdString()); bCopy.setDownloadId(""); bCopy.setPathValid(true); @@ -387,13 +387,13 @@ void ContentManager::downloadCompleted(const kiwix::Book& b, QString path) mp_library->save(); mp_library->bookmarksChanged(); if (!m_local) { - emit(oneBookChanged(QString::fromStdString(b.getId()))); + emit(oneBookChanged(bookId)); } else { emit(mp_library->booksChanged()); } } -ContentManager::DownloadInfo ContentManager::updateDownloadInfos(QString id, const QStringList &keys) +ContentManager::DownloadInfo ContentManager::getDownloadInfo(QString bookId, const QStringList &keys) const { DownloadInfo values; if (!mp_downloader) { @@ -402,25 +402,40 @@ ContentManager::DownloadInfo ContentManager::updateDownloadInfos(QString id, con } return values; } - auto& b = mp_library->getBookById(id); + + auto& b = mp_library->getBookById(bookId); std::shared_ptr d; try { d = mp_downloader->getDownload(b.getDownloadId()); } catch(...) { - downloadCancelled(b); return values; } d->updateStatus(true); - if (d->getStatus() == kiwix::Download::K_COMPLETE) { - downloadCompleted(b, QString::fromStdString(d->getPath())); - } + for(auto& key: keys){ - values.insert(key, getDownloadInfo(*d, key)); + values.insert(key, ::getDownloadInfo(*d, key)); } + return values; } +ContentManager::DownloadInfo ContentManager::updateDownloadInfos(QString bookId, QStringList keys) +{ + if ( !keys.contains("status") ) keys.append("status"); + if ( !keys.contains("path") ) keys.append("path"); + + const DownloadInfo result = getDownloadInfo(bookId, keys); + + if ( result.isEmpty() ) { + downloadCancelled(bookId); + } else if ( result["status"] == "completed" ) { + downloadCompleted(bookId, result["path"].toString()); + } + + return result; +} + QString ContentManager::downloadBook(const QString &id, QModelIndex index) { QString downloadStatus = downloadBook(id); diff --git a/src/contentmanager.h b/src/contentmanager.h index 90f7f1970..7b66103d9 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -65,8 +65,9 @@ class ContentManager : public QObject QMutex remoteLibraryLocker; void setCategories(); void setLanguages(); - void downloadCancelled(const kiwix::Book& b); - void downloadCompleted(const kiwix::Book& book, QString path); + void downloadCancelled(QString bookId); + void downloadCompleted(QString bookId, QString path); + DownloadInfo getDownloadInfo(QString bookId, const QStringList& keys) const; signals: void filterParamsChanged(); @@ -84,7 +85,7 @@ public slots: QStringList getTranslations(const QStringList &keys); BookInfo getBookInfos(QString id, const QStringList &keys); void openBook(const QString& id); - DownloadInfo updateDownloadInfos(QString id, const QStringList& keys); + DownloadInfo updateDownloadInfos(QString bookId, QStringList keys); QString downloadBook(const QString& id); QString downloadBook(const QString& id, QModelIndex index); void updateLibrary(); From b0b592dfd8e53273c38272b45f0979ecf4253031 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Thu, 8 Feb 2024 20:40:55 +0400 Subject: [PATCH 07/17] Extracted ContentManager::downloadStarted() --- src/contentmanager.cpp | 15 ++++++++++----- src/contentmanager.h | 2 ++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index b065b326e..9372087b8 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -366,6 +366,15 @@ QString getDownloadInfo(const kiwix::Download& d, const QString& k) } // unnamed namespace +void ContentManager::downloadStarted(const kiwix::Book& book, const std::string& downloadId) +{ + kiwix::Book bookCopy(book); + bookCopy.setDownloadId(downloadId); + mp_library->addBookToLibrary(bookCopy); + mp_library->save(); + emit(oneBookChanged(QString::fromStdString(book.getId()))); +} + void ContentManager::downloadCancelled(QString bookId) { kiwix::Book bCopy(mp_library->getBookById(bookId)); @@ -485,11 +494,7 @@ QString ContentManager::downloadBook(const QString &id) } catch (std::exception& e) { return ""; } - kiwix::Book bookCopy(book); - bookCopy.setDownloadId(download->getDid()); - mp_library->addBookToLibrary(bookCopy); - mp_library->save(); - emit(oneBookChanged(id)); + downloadStarted(book, download->getDid()); return QString::fromStdString(download->getDid()); } diff --git a/src/contentmanager.h b/src/contentmanager.h index 7b66103d9..c43f76a1d 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -65,6 +65,8 @@ class ContentManager : public QObject QMutex remoteLibraryLocker; void setCategories(); void setLanguages(); + + void downloadStarted(const kiwix::Book& book, const std::string& downloadId); void downloadCancelled(QString bookId); void downloadCompleted(QString bookId, QString path); DownloadInfo getDownloadInfo(QString bookId, const QStringList& keys) const; From a1c60d8dbe2d66024fcaaf85006384c58cef7bcf Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 15:54:53 +0400 Subject: [PATCH 08/17] Moved ownership of download state to ContentManager ContentManagerModel depends on the current view settings (filters) much more than ContentManager does. Since the download state (the set of active and/or paused downloads and their progress info) is independent of the view settings it is more natural for ContentManager to own it. --- src/contentmanager.cpp | 2 +- src/contentmanager.h | 3 +++ src/contentmanagermodel.cpp | 3 ++- src/contentmanagermodel.h | 7 +++++-- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 9372087b8..5a3904ff7 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -53,7 +53,7 @@ ContentManager::ContentManager(Library* library, kiwix::Downloader* downloader, // mp_view will be passed to the tab who will take ownership, // so, we don't need to delete it. mp_view = new ContentManagerView(); - managerModel = new ContentManagerModel(this); + managerModel = new ContentManagerModel(&m_downloads, this); const auto booksList = getBooksList(); managerModel->setBooksData(booksList); auto treeView = mp_view->getView(); diff --git a/src/contentmanager.h b/src/contentmanager.h index c43f76a1d..762ee0149 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -44,6 +44,7 @@ class ContentManager : public QObject Library* mp_library; kiwix::LibraryPtr mp_remoteLibrary; kiwix::Downloader* mp_downloader; + ContentManagerModel::Downloads m_downloads; OpdsRequestManager m_remoteLibraryManager; ContentManagerView* mp_view; bool m_local = true; @@ -61,8 +62,10 @@ class ContentManager : public QObject void reallyEraseBook(const QString& id, bool moveToTrash); void eraseBookFilesFromComputer(const QString dirPath, const QString filename, const bool moveToTrash); BookInfoList getBooksList(); + ContentManagerModel *managerModel; QMutex remoteLibraryLocker; + void setCategories(); void setLanguages(); diff --git a/src/contentmanagermodel.cpp b/src/contentmanagermodel.cpp index 9466369dc..3aaa81c5c 100644 --- a/src/contentmanagermodel.cpp +++ b/src/contentmanagermodel.cpp @@ -7,8 +7,9 @@ #include "kiwixapp.h" #include -ContentManagerModel::ContentManagerModel(QObject *parent) +ContentManagerModel::ContentManagerModel(Downloads* downloads, QObject *parent) : QAbstractItemModel(parent) + , m_downloads(*downloads) { connect(&td, &ThumbnailDownloader::oneThumbnailDownloaded, this, &ContentManagerModel::updateImage); } diff --git a/src/contentmanagermodel.h b/src/contentmanagermodel.h index 59b6bd0f0..b9196af74 100644 --- a/src/contentmanagermodel.h +++ b/src/contentmanagermodel.h @@ -21,8 +21,11 @@ class ContentManagerModel : public QAbstractItemModel typedef QMap BookInfo; typedef QList BookInfoList; + // BookId -> DownloadState map + typedef QMap> Downloads; + public: // functions - explicit ContentManagerModel(QObject *parent = nullptr); + ContentManagerModel(Downloads* downloads, QObject *parent = nullptr); ~ContentManagerModel(); QVariant data(const QModelIndex &index, int role) const override; @@ -63,7 +66,7 @@ public slots: ThumbnailDownloader td; QMap bookIdToRowMap; QMap iconMap; - QMap> m_downloads; + Downloads& m_downloads; }; #endif // CONTENTMANAGERMODEL_H From 7db27fd576969632634fed3094e845e2b040b78b Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 16:07:27 +0400 Subject: [PATCH 09/17] Better grouped declaration of ContentManager --- src/contentmanager.h | 68 ++++++++++++++++++++++---------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/src/contentmanager.h b/src/contentmanager.h index 762ee0149..4e13871bf 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -40,40 +40,6 @@ class ContentManager : public QObject QStringList getCategories() const { return m_categories; } LanguageList getLanguages() const { return m_languages; } -private: - Library* mp_library; - kiwix::LibraryPtr mp_remoteLibrary; - kiwix::Downloader* mp_downloader; - ContentManagerModel::Downloads m_downloads; - OpdsRequestManager m_remoteLibraryManager; - ContentManagerView* mp_view; - bool m_local = true; - QString m_currentLanguage; - QString m_searchQuery; - QString m_categoryFilter = "all"; - QStringList m_contentTypeFilters; - kiwix::supportedListSortBy m_sortBy = kiwix::UNSORTED; - bool m_sortOrderAsc = true; - LanguageList m_languages; - QStringList m_categories; - - QStringList getBookIds(); - // reallyEraseBook() doesn't ask for confirmation (unlike eraseBook()) - void reallyEraseBook(const QString& id, bool moveToTrash); - void eraseBookFilesFromComputer(const QString dirPath, const QString filename, const bool moveToTrash); - BookInfoList getBooksList(); - - ContentManagerModel *managerModel; - QMutex remoteLibraryLocker; - - void setCategories(); - void setLanguages(); - - void downloadStarted(const kiwix::Book& book, const std::string& downloadId); - void downloadCancelled(QString bookId); - void downloadCompleted(QString bookId, QString path); - DownloadInfo getDownloadInfo(QString bookId, const QStringList& keys) const; - signals: void filterParamsChanged(); void booksChanged(); @@ -109,6 +75,40 @@ public slots: void cancelBook(const QString& id, QModelIndex index); void onCustomContextMenu(const QPoint &point); void openBookWithIndex(const QModelIndex& index); + +private: // functions + QStringList getBookIds(); + // reallyEraseBook() doesn't ask for confirmation (unlike eraseBook()) + void reallyEraseBook(const QString& id, bool moveToTrash); + void eraseBookFilesFromComputer(const QString dirPath, const QString filename, const bool moveToTrash); + BookInfoList getBooksList(); + void setCategories(); + void setLanguages(); + + void downloadStarted(const kiwix::Book& book, const std::string& downloadId); + void downloadCancelled(QString bookId); + void downloadCompleted(QString bookId, QString path); + DownloadInfo getDownloadInfo(QString bookId, const QStringList& keys) const; + +private: // data + Library* mp_library; + kiwix::LibraryPtr mp_remoteLibrary; + kiwix::Downloader* mp_downloader; + ContentManagerModel::Downloads m_downloads; + OpdsRequestManager m_remoteLibraryManager; + ContentManagerView* mp_view; + bool m_local = true; + QString m_currentLanguage; + QString m_searchQuery; + QString m_categoryFilter = "all"; + QStringList m_contentTypeFilters; + kiwix::supportedListSortBy m_sortBy = kiwix::UNSORTED; + bool m_sortOrderAsc = true; + LanguageList m_languages; + QStringList m_categories; + + ContentManagerModel *managerModel; + QMutex remoteLibraryLocker; }; #endif // CONTENTMANAGER_H From bd54da2cb2df3fafdeed9b209381d7fb137c4124 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 16:14:57 +0400 Subject: [PATCH 10/17] Deleted obsolete/unused stuff from ContentManager --- src/contentmanager.cpp | 12 ------------ src/contentmanager.h | 4 ---- 2 files changed, 16 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 5a3904ff7..59b63f490 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -624,18 +624,6 @@ void ContentManager::cancelBook(const QString& id) emit(oneBookChanged(id)); } -QStringList ContentManager::getDownloadIds() -{ - QStringList list; - if (!mp_downloader) - return list; - for(auto& id: mp_downloader->getDownloadIds()) { - qInfo() << QString::fromStdString(id); - list.append(QString::fromStdString(id)); - } - return list; -} - void ContentManager::setCurrentLanguage(FilterList langPairList) { QStringList languageList; diff --git a/src/contentmanager.h b/src/contentmanager.h index 4e13871bf..b5f5a2444 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -2,7 +2,6 @@ #define CONTENTMANAGER_H #include -#include #include "library.h" #include "contentmanagerview.h" #include @@ -13,8 +12,6 @@ class ContentManager : public QObject { Q_OBJECT - Q_PROPERTY(QStringList bookIds READ getBookIds NOTIFY booksChanged) - Q_PROPERTY(QStringList downloadIds READ getDownloadIds NOTIFY downloadsChanged) Q_PROPERTY(bool isLocal MEMBER m_local READ isLocal WRITE setLocal NOTIFY localChanged) public: // types @@ -32,7 +29,6 @@ class ContentManager : public QObject ContentManagerView* getView() { return mp_view; } void setLocal(bool local); - QStringList getDownloadIds(); void setCurrentLanguage(FilterList languageList); void setCurrentCategoryFilter(FilterList category); void setCurrentContentTypeFilter(FilterList contentTypeFilter); From c70ab0ed0d48d483e78454b77fa4e53b9566c954 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 16:32:30 +0400 Subject: [PATCH 11/17] Unsplit download pause/resume operations Download pause/resume operations are no longer split across ContentManager and ContentManagerModel. --- src/contentmanager.cpp | 8 ++++++-- src/contentmanagermodel.cpp | 4 ---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 59b63f490..85575f4fc 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -574,8 +574,10 @@ void ContentManager::pauseBook(const QString& id) } auto& b = mp_library->getBookById(id); auto download = mp_downloader->getDownload(b.getDownloadId()); - if (download->getStatus() == kiwix::Download::K_ACTIVE) + if (download->getStatus() == kiwix::Download::K_ACTIVE) { download->pauseDownload(); + m_downloads[id]->pause(); + } } void ContentManager::resumeBook(const QString& id, QModelIndex index) @@ -591,8 +593,10 @@ void ContentManager::resumeBook(const QString& id) } auto& b = mp_library->getBookById(id); auto download = mp_downloader->getDownload(b.getDownloadId()); - if (download->getStatus() == kiwix::Download::K_PAUSED) + if (download->getStatus() == kiwix::Download::K_PAUSED) { download->resumeDownload(); + m_downloads[id]->resume(); + } } void ContentManager::cancelBook(const QString& id, QModelIndex index) diff --git a/src/contentmanagermodel.cpp b/src/contentmanagermodel.cpp index 3aaa81c5c..15f655d34 100644 --- a/src/contentmanagermodel.cpp +++ b/src/contentmanagermodel.cpp @@ -298,15 +298,11 @@ void ContentManagerModel::updateDownload(QString bookId) void ContentManagerModel::pauseDownload(QModelIndex index) { - auto node = static_cast(index.internalPointer()); - node->getDownloadState()->pause(); emit dataChanged(index, index); } void ContentManagerModel::resumeDownload(QModelIndex index) { - auto node = static_cast(index.internalPointer()); - node->getDownloadState()->resume(); emit dataChanged(index, index); } From a31aec56f8db734472288f0318abadf8250feb43 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 17:06:12 +0400 Subject: [PATCH 12/17] Extracted ContentManager::getRemoteOrLocalBook() ... mainly for readability. --- src/contentmanager.cpp | 24 +++++++++++++++--------- src/contentmanager.h | 4 ++++ 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 85575f4fc..c67987415 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -463,29 +463,35 @@ QString ContentManager::downloadBook(const QString &id, QModelIndex index) return downloadStatus; } +const kiwix::Book& ContentManager::getRemoteOrLocalBook(const QString &id) +{ + try { + QMutexLocker locker(&remoteLibraryLocker); + return mp_remoteLibrary->getBookById(id.toStdString()); + } catch (...) { + return mp_library->getBookById(id); + } +} QString ContentManager::downloadBook(const QString &id) { if (!mp_downloader) return ""; - const auto& book = [&]()->const kiwix::Book& { - try { - QMutexLocker locker(&remoteLibraryLocker); - return mp_remoteLibrary->getBookById(id.toStdString()); - } catch (...) { - return mp_library->getBookById(id); - } - }(); + + const auto& book = getRemoteOrLocalBook(id); auto downloadPath = KiwixApp::instance()->getSettingsManager()->getDownloadDir(); QStorageInfo storage(downloadPath); auto bytesAvailable = storage.bytesAvailable(); if (bytesAvailable == -1 || book.getSize() > (unsigned long long) bytesAvailable) { return "storage_error"; } + auto booksList = mp_library->getBookIds(); - for (auto b : booksList) + for (auto b : booksList) { if (b.toStdString() == book.getId()) return ""; + } + std::shared_ptr download; try { std::pair downloadDir("dir", downloadPath.toStdString()); diff --git a/src/contentmanager.h b/src/contentmanager.h index b5f5a2444..e804754bf 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -81,6 +81,10 @@ public slots: void setCategories(); void setLanguages(); + // Get the book with the specified id from + // the remote or local library (in that order). + const kiwix::Book& getRemoteOrLocalBook(const QString &id); + void downloadStarted(const kiwix::Book& book, const std::string& downloadId); void downloadCancelled(QString bookId); void downloadCompleted(QString bookId, QString path); From 142e6fed58d646520e27878c6e8c7665cd804875 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 17:17:00 +0400 Subject: [PATCH 13/17] Dropped unused return value --- src/contentmanager.cpp | 5 ++--- src/contentmanager.h | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index c67987415..187245681 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -445,7 +445,7 @@ ContentManager::DownloadInfo ContentManager::updateDownloadInfos(QString bookId, return result; } -QString ContentManager::downloadBook(const QString &id, QModelIndex index) +void ContentManager::downloadBook(const QString &id, QModelIndex index) { QString downloadStatus = downloadBook(id); QString dialogHeader, dialogText; @@ -457,10 +457,9 @@ QString ContentManager::downloadBook(const QString &id, QModelIndex index) dialogText = gt("download-storage-error-text"); } else { emit managerModel->startDownload(index); - return downloadStatus; } + showInfoBox(dialogHeader, dialogText, mp_view); - return downloadStatus; } const kiwix::Book& ContentManager::getRemoteOrLocalBook(const QString &id) diff --git a/src/contentmanager.h b/src/contentmanager.h index e804754bf..877728f07 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -54,7 +54,7 @@ public slots: void openBook(const QString& id); DownloadInfo updateDownloadInfos(QString bookId, QStringList keys); QString downloadBook(const QString& id); - QString downloadBook(const QString& id, QModelIndex index); + void downloadBook(const QString& id, QModelIndex index); void updateLibrary(); void setSearch(const QString& search); void setSortBy(const QString& sortBy, const bool sortOrderAsc); From 7ec85ac2fb45a2eed3e43e85252c7ba692ca06b0 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 17:35:43 +0400 Subject: [PATCH 14/17] Introduced ContentManagerError --- src/contentmanager.cpp | 56 +++++++++++++++++++++++++++++------------- src/contentmanager.h | 2 +- 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index 187245681..d9f788d17 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -26,6 +26,33 @@ namespace { +class ContentManagerError : public std::runtime_error +{ +public: + ContentManagerError(const QString& summary, const QString& details) + : std::runtime_error(summary.toStdString()) + , m_details(details) + {} + + QString summary() const { return QString::fromStdString(what()); } + QString details() const { return m_details; } + +private: + QString m_details; +}; + +void throwDownloadUnavailableError() +{ + throw ContentManagerError(gt("download-unavailable"), + gt("download-unavailable-text")); +} + +void throwStorageError() +{ + throw ContentManagerError(gt("download-storage-error"), + gt("download-storage-error-text")); +} + // Opens the directory containing the input file path. // parent is the widget serving as the parent for the error dialog in case of // failure. @@ -447,19 +474,15 @@ ContentManager::DownloadInfo ContentManager::updateDownloadInfos(QString bookId, void ContentManager::downloadBook(const QString &id, QModelIndex index) { - QString downloadStatus = downloadBook(id); - QString dialogHeader, dialogText; - if (downloadStatus.size() == 0) { - dialogHeader = gt("download-unavailable"); - dialogText = gt("download-unavailable-text"); - } else if (downloadStatus == "storage_error") { - dialogHeader = gt("download-storage-error"); - dialogText = gt("download-storage-error-text"); - } else { + try + { + downloadBook(id); emit managerModel->startDownload(index); } - - showInfoBox(dialogHeader, dialogText, mp_view); + catch ( const ContentManagerError& err ) + { + showInfoBox(err.summary(), err.details(), mp_view); + } } const kiwix::Book& ContentManager::getRemoteOrLocalBook(const QString &id) @@ -472,23 +495,23 @@ const kiwix::Book& ContentManager::getRemoteOrLocalBook(const QString &id) } } -QString ContentManager::downloadBook(const QString &id) +void ContentManager::downloadBook(const QString &id) { if (!mp_downloader) - return ""; + throwDownloadUnavailableError(); const auto& book = getRemoteOrLocalBook(id); auto downloadPath = KiwixApp::instance()->getSettingsManager()->getDownloadDir(); QStorageInfo storage(downloadPath); auto bytesAvailable = storage.bytesAvailable(); if (bytesAvailable == -1 || book.getSize() > (unsigned long long) bytesAvailable) { - return "storage_error"; + throwStorageError(); } auto booksList = mp_library->getBookIds(); for (auto b : booksList) { if (b.toStdString() == book.getId()) - return ""; + throwDownloadUnavailableError(); // but why??? } std::shared_ptr download; @@ -497,10 +520,9 @@ QString ContentManager::downloadBook(const QString &id) const std::vector> options = { downloadDir }; download = mp_downloader->startDownload(book.getUrl(), options); } catch (std::exception& e) { - return ""; + throwDownloadUnavailableError(); } downloadStarted(book, download->getDid()); - return QString::fromStdString(download->getDid()); } void ContentManager::eraseBookFilesFromComputer(const QString dirPath, const QString fileName, const bool moveToTrash) diff --git a/src/contentmanager.h b/src/contentmanager.h index 877728f07..4f581c32d 100644 --- a/src/contentmanager.h +++ b/src/contentmanager.h @@ -53,7 +53,7 @@ public slots: BookInfo getBookInfos(QString id, const QStringList &keys); void openBook(const QString& id); DownloadInfo updateDownloadInfos(QString bookId, QStringList keys); - QString downloadBook(const QString& id); + void downloadBook(const QString& id); void downloadBook(const QString& id, QModelIndex index); void updateLibrary(); void setSearch(const QString& search); From 852fa7cdb7efd4db64e96815a1577ad4d4014ea0 Mon Sep 17 00:00:00 2001 From: Veloman Yunkan Date: Fri, 9 Feb 2024 17:40:35 +0400 Subject: [PATCH 15/17] Extracted checkEnoughStorageAvailable() ... mainly for readability. --- src/contentmanager.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/contentmanager.cpp b/src/contentmanager.cpp index d9f788d17..d5fb8f757 100644 --- a/src/contentmanager.cpp +++ b/src/contentmanager.cpp @@ -47,10 +47,14 @@ void throwDownloadUnavailableError() gt("download-unavailable-text")); } -void throwStorageError() +void checkEnoughStorageAvailable(const kiwix::Book& book, QString targetDir) { - throw ContentManagerError(gt("download-storage-error"), - gt("download-storage-error-text")); + QStorageInfo storage(targetDir); + auto bytesAvailable = storage.bytesAvailable(); + if (bytesAvailable == -1 || book.getSize() > (unsigned long long) bytesAvailable) { + throw ContentManagerError(gt("download-storage-error"), + gt("download-storage-error-text")); + } } // Opens the directory containing the input file path. @@ -502,11 +506,7 @@ void ContentManager::downloadBook(const QString &id) const auto& book = getRemoteOrLocalBook(id); auto downloadPath = KiwixApp::instance()->getSettingsManager()->getDownloadDir(); - QStorageInfo storage(downloadPath); - auto bytesAvailable = storage.bytesAvailable(); - if (bytesAvailable == -1 || book.getSize() > (unsigned long long) bytesAvailable) { - throwStorageError(); - } + checkEnoughStorageAvailable(book, downloadPath); auto booksList = mp_library->getBookIds(); for (auto b : booksList) { From ae3284357419edd0f307ede8334b56f44c50e87c Mon Sep 17 00:00:00 2001 From: Emmanuel Engelhart Date: Sun, 25 Feb 2024 16:05:52 +0100 Subject: [PATCH 16/17] New libzim/libkiwix version reqs. in kiwix-desktop.pro --- kiwix-desktop.pro | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/kiwix-desktop.pro b/kiwix-desktop.pro index 1f162b3e8..b1e02197b 100644 --- a/kiwix-desktop.pro +++ b/kiwix-desktop.pro @@ -183,12 +183,12 @@ unix { INSTALLS += mime_file } -PKGCONFIG_CFLAGS = $$system(pkg-config --cflags $$PKGCONFIG_OPTION \"kiwix >= 13.0.0 libzim >= 8.0.0\") +PKGCONFIG_CFLAGS = $$system(pkg-config --cflags $$PKGCONFIG_OPTION \"kiwix >= 13.0.0 kiwix < 14.0.0 libzim >= 9.0.0 libzim < 10.0.0\") QMAKE_CXXFLAGS += $$PKGCONFIG_CFLAGS QMAKE_CFLAGS += $$PKGCONFIG_CFLAGS -LIBS += $$system(pkg-config --libs $$PKGCONFIG_OPTION \"kiwix >= 13.0.0 libzim >= 8.0.0\") +LIBS += $$system(pkg-config --libs $$PKGCONFIG_OPTION \"kiwix >= 13.0.0 kiwix < 14.0.0 libzim >= 9.0.0 libzim < 10.0.0\") RESOURCES += \ resources/kiwix.qrc \ From 1cc9dd1b887dfd46ba44ac83b8f4522bf81ed783 Mon Sep 17 00:00:00 2001 From: Emmanuel Engelhart Date: Sun, 25 Feb 2024 16:17:40 +0100 Subject: [PATCH 17/17] Update dependences in deb 'control' --- debian/control | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/debian/control b/debian/control index 303525b15..dcf74edbe 100644 --- a/debian/control +++ b/debian/control @@ -6,7 +6,8 @@ Build-Depends: debhelper-compat (= 13), pkg-config, qtbase5-dev, qtwebengine5-dev, - libkiwix-dev (>= 11.0.0~) + libkiwix-dev (>= 13.0.0), libkiwix-dev (<< 14.0.0), + libzim-dev (>= 9.0.0), libzim-dev (<< 10.0.0), Standards-Version: 4.5.0 Homepage: https://www.kiwix.org/ Rules-Requires-Root: no @@ -16,6 +17,8 @@ Architecture: any Depends: ${shlibs:Depends}, ${misc:Depends} Description: offline Wikipedia reader + Kiwix is an offline reader and manager for online content + like Wikipedia, Project Gutenberg, or TED Talks. Kiwix allows you to read and search through offline content as they were online. Similar to a browser, Kiwix works with the highly compressed ZIM file format.