Merge pull request #15396 from jagannatharjun/downloadrequest

Fix invalid RSS feed icons

src/base/net/downloadhandlerimpl.cpp

@@ -42,6 +42,16 @@ namespace
 {
     bool saveToFile(const QByteArray &replyData, QString &filePath)
     {
+        if (!filePath.isEmpty())
+        {
+            QFile file {filePath};
+            if (!file.open(QIODevice::WriteOnly))
+                return false;
+
+            file.write(replyData);
+            return true;
+        }
+
         QTemporaryFile tmpfile {Utils::Fs::tempPath() + "XXXXXX"};
         tmpfile.setAutoRemove(false);
 
@@ -129,7 +139,7 @@ void DownloadHandlerImpl::processFinishedDownload()
 
     if (m_downloadRequest.saveToFile())
     {
-        QString filePath;
+        QString filePath {m_downloadRequest.destFileName()};
         if (saveToFile(m_result.data, filePath))
             m_result.filePath = filePath;
         else

src/base/net/downloadmanager.cpp

@@ -348,6 +348,17 @@ Net::DownloadRequest &Net::DownloadRequest::saveToFile(const bool value)
     return *this;
 }
 
+QString Net::DownloadRequest::destFileName() const
+{
+    return m_destFileName;
+}
+
+Net::DownloadRequest &Net::DownloadRequest::destFileName(const QString &value)
+{
+    m_destFileName = value;
+    return *this;
+}
+
 Net::ServiceID Net::ServiceID::fromURL(const QUrl &url)
 {
     return {url.host(), url.port(80)};

src/base/net/downloadmanager.h

@@ -78,11 +78,18 @@ namespace Net
         bool saveToFile() const;
         DownloadRequest &saveToFile(bool value);
 
+        // if saveToFile is set, the file is saved in destFileName
+        // (deprecated) if destFileName is not provided, the file will be saved
+        // in a temporary file, the name of file is set in DownloadResult::filePath
+        QString destFileName() const;
+        DownloadRequest &destFileName(const QString &value);
+
     private:
         QString m_url;
         QString m_userAgent;
         qint64 m_limit = 0;
         bool m_saveToFile = false;
+        QString m_destFileName;
     };
 
     struct DownloadResult

src/base/rss/rss_feed.cpp

@@ -66,7 +66,8 @@ Feed::Feed(const QUuid &uid, const QString &url, const QString &path, Session *s
     , m_uid(uid)
     , m_url(url)
 {
-    m_dataFileName = QString::fromLatin1(m_uid.toRfc4122().toHex()) + QLatin1String(".json");
+    const auto uidHex = QString::fromLatin1(m_uid.toRfc4122().toHex());
+    m_dataFileName = uidHex + QLatin1String(".json");
 
     // Move to new file naming scheme (since v4.1.2)
     const QString legacyFilename
@@ -76,6 +77,8 @@ Feed::Feed(const QUuid &uid, const QString &url, const QString &path, Session *s
     if (!QFile::exists(storageDir.absoluteFilePath(m_dataFileName)))
         QFile::rename(storageDir.absoluteFilePath(legacyFilename), storageDir.absoluteFilePath(m_dataFileName));
 
+    m_iconPath = Utils::Fs::toUniformPath(storageDir.absoluteFilePath(uidHex + QLatin1String(".ico")));
+
     m_parser = new Private::Parser(m_lastBuildDate);
     m_parser->moveToThread(m_session->workingThread());
     connect(this, &Feed::destroyed, m_parser, &Private::Parser::deleteLater);
@@ -96,7 +99,6 @@ Feed::Feed(const QUuid &uid, const QString &url, const QString &path, Session *s
 Feed::~Feed()
 {
     emit aboutToBeDestroyed(this);
-    Utils::Fs::forceRemove(m_iconPath);
 }
 
 QList<Article *> Feed::articles() const
@@ -136,6 +138,9 @@ void Feed::refresh()
     m_downloadHandler = Net::DownloadManager::instance()->download(m_url);
     connect(m_downloadHandler, &Net::DownloadHandler::finished, this, &Feed::handleDownloadFinished);
 
+    if (!QFile::exists(m_iconPath))
+        downloadIcon();
+
     m_isLoading = true;
     emit stateChanged(this);
 }
@@ -186,7 +191,6 @@ void Feed::handleIconDownloadFinished(const Net::DownloadResult &result)
 {
     if (result.status == Net::DownloadStatus::Success)
     {
-        m_iconPath = Utils::Fs::toUniformPath(result.filePath);
         emit iconLoaded(this);
     }
 }
@@ -423,7 +427,7 @@ void Feed::downloadIcon()
     const QUrl url(m_url);
     const auto iconUrl = QString::fromLatin1("%1://%2/favicon.ico").arg(url.scheme(), url.host());
     Net::DownloadManager::instance()->download(
-            Net::DownloadRequest(iconUrl).saveToFile(true)
+            Net::DownloadRequest(iconUrl).saveToFile(true).destFileName(m_iconPath)
                 , this, &Feed::handleIconDownloadFinished);
 }
 
@@ -545,6 +549,7 @@ void Feed::handleArticleRead(Article *article)
 void Feed::cleanup()
 {
     Utils::Fs::forceRemove(m_session->dataFileStorage()->storageDir().absoluteFilePath(m_dataFileName));
+    Utils::Fs::forceRemove(m_iconPath);
 }
 
 void Feed::timerEvent(QTimerEvent *event)

src/gui/rss/feedlistwidget.cpp

@@ -66,6 +66,25 @@ namespace
             return ((order == Qt::AscendingOrder) ? lhsSticky : rhsSticky);
         }
     };
+
+    QIcon loadIcon(const QString &path, const QString &fallbackId)
+    {
+        const QPixmap pixmap {path};
+        if (!pixmap.isNull())
+            return {pixmap};
+
+        return UIThemeManager::instance()->getIcon(fallbackId);
+    }
+
+    QIcon rssFeedIcon(const RSS::Feed *feed)
+    {
+        if (feed->isLoading())
+            return UIThemeManager::instance()->getIcon(QLatin1String("loading"));
+        if (feed->hasError())
+            return UIThemeManager::instance()->getIcon(QLatin1String("unavailable"));
+
+        return loadIcon(feed->iconPath(), QLatin1String("application-rss+xml"));
+    }
 }
 
 FeedListWidget::FeedListWidget(QWidget *parent)
@@ -113,16 +132,7 @@ void FeedListWidget::handleFeedStateChanged(RSS::Feed *feed)
     QTreeWidgetItem *item = m_rssToTreeItemMapping.value(feed);
     Q_ASSERT(item);
 
-    QIcon icon;
-    if (feed->isLoading())
-        icon = UIThemeManager::instance()->getIcon(QLatin1String("loading"));
-    else if (feed->hasError())
-        icon = UIThemeManager::instance()->getIcon(QLatin1String("unavailable"));
-    else if (!feed->iconPath().isEmpty())
-        icon = QIcon(feed->iconPath());
-    else
-        icon = UIThemeManager::instance()->getIcon(QLatin1String("application-rss+xml"));
-    item->setData(0, Qt::DecorationRole, icon);
+    item->setData(0, Qt::DecorationRole, rssFeedIcon(feed));
 }
 
 void FeedListWidget::handleFeedIconLoaded(RSS::Feed *feed)
@@ -132,7 +142,7 @@ void FeedListWidget::handleFeedIconLoaded(RSS::Feed *feed)
         QTreeWidgetItem *item = m_rssToTreeItemMapping.value(feed);
         Q_ASSERT(item);
 
-        item->setData(0, Qt::DecorationRole, QIcon(feed->iconPath()));
+        item->setData(0, Qt::DecorationRole, rssFeedIcon(feed));
     }
 }
 
@@ -270,20 +280,9 @@ QTreeWidgetItem *FeedListWidget::createItem(RSS::Item *rssItem, QTreeWidgetItem
 
     QIcon icon;
     if (auto feed = qobject_cast<RSS::Feed *>(rssItem))
-    {
-        if (feed->isLoading())
-            icon = UIThemeManager::instance()->getIcon(QLatin1String("loading"));
-        else if (feed->hasError())
-            icon = UIThemeManager::instance()->getIcon(QLatin1String("unavailable"));
-        else if (!feed->iconPath().isEmpty())
-            icon = QIcon(feed->iconPath());
+        icon = rssFeedIcon(feed);
     else
-            icon = UIThemeManager::instance()->getIcon(QLatin1String("application-rss+xml"));
-    }
-    else
-    {
         icon = UIThemeManager::instance()->getIcon(QLatin1String("inode-directory"));
-    }
     item->setData(0, Qt::DecorationRole, icon);
 
     connect(rssItem, &RSS::Item::unreadCountChanged, this, &FeedListWidget::handleItemUnreadCountChanged);