Merge pull request #1036 from kiwix/more_reliable_thumbnail_loading

More reliable loading of remote thumbnails

src/contentmanager.cpp

@@ -679,12 +679,26 @@ void ContentManager::updateLibrary() {
     } catch (std::runtime_error&) {}
 }
 
+namespace
+{
+
+QString makeHttpUrl(QString host, int port)
+{
+    return port == 443
+         ? "https://" + host
+         : "http://" + host + ":" + QString::number(port);
+}
+
+} // unnamed namespace
+
 void ContentManager::updateRemoteLibrary(const QString& content) {
     QtConcurrent::run([=]() {
         QMutexLocker locker(&remoteLibraryLocker);
         mp_remoteLibrary = kiwix::Library::create();
         kiwix::Manager manager(mp_remoteLibrary);
-        const auto catalogUrl = m_remoteLibraryManager.getCatalogHost();
+        const auto catalogHost = m_remoteLibraryManager.getCatalogHost();
+        const auto catalogPort = m_remoteLibraryManager.getCatalogPort();
+        const auto catalogUrl = makeHttpUrl(catalogHost, catalogPort);
         manager.readOpds(content.toStdString(), catalogUrl.toStdString());
         emit(this->booksChanged());
         emit(this->pendingRequest(false));

src/contentmanagermodel.cpp

@@ -112,7 +112,7 @@ void ContentManagerModel::setBooksData(const BookInfoList& data)
 
 std::shared_ptr<RowNode> ContentManagerModel::createNode(BookInfo bookItem, QMap<QString, QByteArray> iconMap) const
 {
-    auto faviconUrl = "https://" + bookItem["faviconUrl"].toString();
+    const auto faviconUrl = bookItem["faviconUrl"].toString();
     QString id = bookItem["id"].toString();
     QByteArray bookIcon;
     try {
@@ -168,14 +168,14 @@ void ContentManagerModel::refreshIcons()
     for (auto i = 0; i < rowCount() && i < m_data.size(); i++) {
         auto bookItem = m_data[i];
         auto id = bookItem["id"].toString();
-        auto faviconUrl = "https://" + bookItem["faviconUrl"].toString();
+        const auto faviconUrl = bookItem["faviconUrl"].toString();
         auto app = KiwixApp::instance();
         try {
             auto book = app->getLibrary()->getBookById(id);
             auto item = book.getIllustration(48);
         } catch (...) {
             if (faviconUrl != "" && !iconMap.contains(faviconUrl)) {
-                td.addDownload(faviconUrl, index(i, 0));
+                td.addDownload(faviconUrl, id);
             }
         }
     }
@@ -230,15 +230,17 @@ void ContentManagerModel::sort(int column, Qt::SortOrder order)
     KiwixApp::instance()->getContentManager()->setSortBy(sortBy, order == Qt::AscendingOrder);
 }
 
-void ContentManagerModel::updateImage(QModelIndex index, QString url, QByteArray imageData)
+void ContentManagerModel::updateImage(QString bookId, QString url, QByteArray imageData)
 {
-    if (!index.isValid())
-        return;
-    auto item = static_cast<RowNode*>(index.internalPointer());
-    if (!rootNode->isChild(item))
+    const auto it = bookIdToRowMap.constFind(bookId);
+    if ( it == bookIdToRowMap.constEnd() )
         return;
+
+    const size_t row = it.value();
+    const auto item = static_cast<RowNode*>(rootNode->child(row).get());
     item->setIconData(imageData);
     iconMap[url] = imageData;
+    const QModelIndex index = this->index(row, 0);
     emit dataChanged(index, index);
 }
 

src/contentmanagermodel.h

@@ -43,7 +43,7 @@ public: // functions
     std::shared_ptr<RowNode> createNode(BookInfo bookItem, QMap<QString, QByteArray> iconMap) const;
 
 public slots:
-    void updateImage(QModelIndex index, QString url, QByteArray imageData);
+    void updateImage(QString bookId, QString url, QByteArray imageData);
     void startDownload(QModelIndex index);
     void pauseDownload(QModelIndex index);
     void resumeDownload(QModelIndex index);

src/thumbnaildownloader.cpp

@@ -4,49 +4,45 @@
 #include <QPixmap>
 #include <QIcon>
 
-ThumbnailDownloader::ThumbnailDownloader(QObject *parent)
+ThumbnailDownloader::ThumbnailDownloader()
 {
-    connect(this, &ThumbnailDownloader::oneThumbnailDownloaded, [=]() {
-        if (m_urlPairList.size() != 0)
-            downloadOnePair(m_urlPairList.takeFirst());
-        else
-            m_isDownloading = false;
-    });
+    connect(this, &ThumbnailDownloader::oneThumbnailDownloaded,
+            this, &ThumbnailDownloader::startNextDownload);
 }
 
 ThumbnailDownloader::~ThumbnailDownloader()
 {
 }
 
-void ThumbnailDownloader::addDownload(QString url, QModelIndex index)
+void ThumbnailDownloader::addDownload(QString url, ThumbnailId index)
 {
-    m_urlPairList.append({index, url});
+    m_downloadQueue.append({index, url});
     if (!m_isDownloading)
-        startDownload();
+        startNextDownload();
 }
 
-void ThumbnailDownloader::startDownload()
+void ThumbnailDownloader::startNextDownload()
 {
-    if (m_urlPairList.size() == 0) {
+    if (m_downloadQueue.size() == 0) {
         m_isDownloading = false;
         return;
     }
     m_isDownloading = true;
-    downloadOnePair(m_urlPairList.takeFirst());
+    downloadThumbnail(m_downloadQueue.takeFirst());
 }
 
-void ThumbnailDownloader::downloadOnePair(QPair<QModelIndex, QString> urlPair)
+void ThumbnailDownloader::downloadThumbnail(ThumbnailInfo thumbnailInfo)
 {
-    QNetworkRequest req(urlPair.second);
+    QNetworkRequest req(thumbnailInfo.second);
     auto reply = manager.get(req);
     connect(reply, &QNetworkReply::finished, this, [=](){
-        fileDownloaded(reply, urlPair);
+        fileDownloaded(reply, thumbnailInfo);
     });
 }
 
-void ThumbnailDownloader::fileDownloaded(QNetworkReply *pReply, QPair<QModelIndex, QString> urlPair)
+void ThumbnailDownloader::fileDownloaded(QNetworkReply *pReply, ThumbnailInfo thumbnailInfo)
 {
     auto downloadedData = pReply->readAll();
-    emit oneThumbnailDownloaded(urlPair.first, urlPair.second, downloadedData);
+    emit oneThumbnailDownloaded(thumbnailInfo.first, thumbnailInfo.second, downloadedData);
     pReply->deleteLater();
 }

src/thumbnaildownloader.h

@@ -4,33 +4,37 @@
 #include <QObject>
 #include <QQueue>
 #include <QNetworkAccessManager>
-#include <QIcon>
 #include <QNetworkReply>
-#include <QModelIndex>
 
 class ThumbnailDownloader : public QObject
 {
     Q_OBJECT
 
 public:
-    ThumbnailDownloader(QObject *parent = 0);
+    typedef QString ThumbnailId;
+    typedef QPair<ThumbnailId, QString> ThumbnailInfo;
+
+public:
+    ThumbnailDownloader();
     ~ThumbnailDownloader();
 
-    void addDownload(QString url, QModelIndex index);
-    void startDownload();
-    void downloadOnePair(QPair<QModelIndex, QString> urlPair);
-    void clearQueue() { m_urlPairList.clear(); }
-
-signals:
-    void oneThumbnailDownloaded(QModelIndex, QString, QByteArray);
+    void addDownload(QString url, ThumbnailId index);
+    void startNextDownload();
+    void clearQueue() { m_downloadQueue.clear(); }
 
 private:
-    QQueue<QPair<QModelIndex, QString>> m_urlPairList;
+    void downloadThumbnail(ThumbnailInfo thumbnailInfo);
+
+signals:
+    void oneThumbnailDownloaded(ThumbnailId, QString, QByteArray);
+
+private:
+    QQueue<ThumbnailInfo> m_downloadQueue;
     QNetworkAccessManager manager;
     bool m_isDownloading = false;
 
 private slots:
-    void fileDownloaded(QNetworkReply *pReply, QPair<QModelIndex, QString> urlPair);
+    void fileDownloaded(QNetworkReply *pReply, ThumbnailInfo thumbnailInfo);
 
 };