RSS: Cache number of unread articles in a feed instead of computing it every time.

Optimization to address issue #34.

src/rss/rss_imp.cpp

@@ -605,18 +605,25 @@ void RSSImp::updateFeedIcon(const QString &url, const QString &icon_path) {
   item->setData(0,Qt::DecorationRole, QVariant(QIcon(icon_path)));
 }
 
-void RSSImp::updateFeedInfos(const QString &url, const QString &display_name, uint nbUnread) {
+void RSSImp::updateFeedInfos(const QString& url, const QString& display_name, uint nbUnread)
+{
   qDebug() << Q_FUNC_INFO << display_name;
   QTreeWidgetItem *item = m_feedList->getTreeItemFromUrl(url);
   RssFeedPtr stream = qSharedPointerCast<RssFeed>(m_feedList->getRSSItem(item));
-  item->setText(0, display_name + QString::fromUtf8("  (") + QString::number(nbUnread, 10)+ QString(")"));
+  item->setText(0, display_name + QString::fromUtf8("  (") + QString::number(nbUnread)+ QString(")"));
   if (!stream->isLoading())
-    item->setData(0,Qt::DecorationRole, QVariant(QIcon(stream->icon())));
+    item->setData(0, Qt::DecorationRole, QVariant(QIcon(stream->icon())));
   // Update parent
   if (item->parent())
     updateItemInfos(item->parent());
   // Update Unread item
   updateItemInfos(m_feedList->stickyUnreadItem());
+}
+
+void RSSImp::onFeedContentChanged(const QString& url)
+{
+  qDebug() << Q_FUNC_INFO << url;
+  QTreeWidgetItem *item = m_feedList->getTreeItemFromUrl(url);
   // If the feed is selected, update the displayed news
   if (m_feedList->currentItem() == item ) {
     refreshArticleList(item);
@@ -662,11 +669,12 @@ RSSImp::RSSImp(QWidget *parent) : QWidget(parent), m_rssManager(new RssManager)
   refreshArticleList(m_feedList->currentItem());
 
   loadFoldersOpenState();
-  connect(m_rssManager.data(), SIGNAL(feedInfosChanged(QString, QString, unsigned int)), this, SLOT(updateFeedInfos(QString, QString, unsigned int)));
-  connect(m_rssManager.data(), SIGNAL(feedIconChanged(QString, QString)), this, SLOT(updateFeedIcon(QString, QString)));
+  connect(m_rssManager.data(), SIGNAL(feedInfosChanged(QString, QString, unsigned int)), SLOT(updateFeedInfos(QString, QString, unsigned int)));
+  connect(m_rssManager.data(), SIGNAL(feedContentChanged(QString)), SLOT(onFeedContentChanged(QString)));
+  connect(m_rssManager.data(), SIGNAL(feedIconChanged(QString, QString)), SLOT(updateFeedIcon(QString, QString)));
 
-  connect(m_feedList, SIGNAL(customContextMenuRequested(const QPoint&)), this, SLOT(displayRSSListMenu(const QPoint&)));
-  connect(listArticles, SIGNAL(customContextMenuRequested(const QPoint&)), this, SLOT(displayItemsListMenu(const QPoint&)));
+  connect(m_feedList, SIGNAL(customContextMenuRequested(const QPoint&)), SLOT(displayRSSListMenu(const QPoint&)));
+  connect(listArticles, SIGNAL(customContextMenuRequested(const QPoint&)), SLOT(displayItemsListMenu(const QPoint&)));
 
   // Feeds list actions
   connect(actionDelete, SIGNAL(triggered()), this, SLOT(deleteSelectedItems()));

src/rss/rss_imp.h

@@ -68,6 +68,7 @@ private slots:
   void refreshTextBrowser();
   void updateFeedIcon(const QString &url, const QString &icon_path);
   void updateFeedInfos(const QString &url, const QString &display_name, uint nbUnread);
+  void onFeedContentChanged(const QString& url);
   void updateItemsInfos(const QList<QTreeWidgetItem*> &items);
   void updateItemInfos(QTreeWidgetItem *item);
   void openNewsUrl();

src/rss/rssarticle.cpp

@@ -33,6 +33,7 @@
 #include <iostream>
 
 #include "rssarticle.h"
+#include "rssfeed.h"
 
 // public constructor
 RssArticle::RssArticle(RssFeed* parent, const QString &guid):
@@ -102,7 +103,11 @@ bool RssArticle::isRead() const {
 }
 
 void RssArticle::markAsRead() {
+  if (m_read)
+    return;
+
   m_read = true;
+  m_parent->decrementUnreadCount();
 }
 
 const QString& RssArticle::guid() const

src/rss/rssfeed.cpp

@@ -43,7 +43,7 @@
 
 RssFeed::RssFeed(RssManager* manager, RssFolder* parent, const QString &url):
   m_manager(manager), m_parent(parent), m_icon(":/Icons/oxygen/application-rss+xml.png"),
-  m_dirty(false), m_inErrorState(false), m_loading(false) {
+  m_unreadCount(0), m_dirty(false), m_inErrorState(false), m_loading(false) {
   qDebug() << Q_FUNC_INFO << url;
   m_url = QUrl::fromEncoded(url.toUtf8()).toString();
   // Listen for new RSS downloads
@@ -97,6 +97,8 @@ void RssFeed::loadItemsFromDisk() {
     RssArticlePtr rss_item = hashToRssArticle(this, item);
     if (rss_item) {
       m_articles.insert(rss_item->guid(), rss_item);
+      if (!rss_item->isRead())
+        ++m_unreadCount;
     }
   }
 }
@@ -190,19 +192,13 @@ void RssFeed::markAsRead() {
   for ( ; it != itend; ++it) {
     it.value()->markAsRead();
   }
+  m_unreadCount = 0;
   m_manager->forwardFeedInfosChanged(m_url, displayName(), 0);
 }
 
-uint RssFeed::unreadCount() const {
-  uint nbUnread = 0;
-
-  RssArticleHash::ConstIterator it=m_articles.begin();
-  RssArticleHash::ConstIterator itend=m_articles.end();
-  for ( ; it != itend; ++it) {
-    if (!it.value()->isRead())
-      ++nbUnread;
-  }
-  return nbUnread;
+uint RssFeed::unreadCount() const
+{
+  return m_unreadCount;
 }
 
 RssArticleList RssFeed::articleList() const {
@@ -235,7 +231,10 @@ void RssFeed::removeOldArticles() {
     RssManager::sortArticleListByDateDesc(listItems);
     const int excess = nb_articles - max_articles;
     for (uint i=nb_articles-excess; i<nb_articles; ++i) {
-      m_articles.remove(listItems.at(i)->guid());
+      RssArticlePtr article = m_articles.take(listItems.at(i)->guid());
+      // Update unreadCount
+      if (!article->isRead())
+        --m_unreadCount;
     }
   }
 }
@@ -258,7 +257,7 @@ void RssFeed::handleDownloadFailure(const QString &url, const QString& error) {
   if (url != m_url) return;
   m_inErrorState = true;
   m_loading = false;
-  m_manager->forwardFeedInfosChanged(m_url, displayName(), unreadCount()); // XXX: Ugly
+  m_manager->forwardFeedInfosChanged(m_url, displayName(), m_unreadCount); // XXX: Ugly
   qWarning() << "Failed to download RSS feed at" << url;
   qWarning() << "Reason:" << error;
 }
@@ -268,7 +267,14 @@ void RssFeed::handleFeedTitle(const QString& feedUrl, const QString& title)
   if (feedUrl != m_url)
     return;
 
+  if (m_title == title)
+    return;
+
   m_title = title;
+
+  // Notify that we now have something better than a URL to display
+  if (m_alias.isEmpty())
+    m_manager->forwardFeedInfosChanged(feedUrl, title, m_unreadCount);
 }
 
 void RssFeed::handleNewArticle(const QString& feedUrl, const QVariantHash& articleData)
@@ -301,9 +307,14 @@ void RssFeed::handleNewArticle(const QString& feedUrl, const QVariantHash& artic
         QBtSession::instance()->downloadUrlAndSkipDialog(torrent_url, matching_rule->savePath(), matching_rule->label());
     }
   }
+  // Update unreadCount if necessary
+  if (!article->isRead())
+    ++m_unreadCount;
+
+  m_manager->forwardFeedInfosChanged(m_url, displayName(), m_unreadCount);
   // FIXME: We should forward the information here but this would seriously decrease
   // performance with current design.
-  //m_manager->forwardFeedInfosChanged(m_url, displayName(), unreadCount()); // XXX: Ugly
+  //m_manager->forwardFeedContentChanged(m_url);
 }
 
 void RssFeed::handleFeedParsingFinished(const QString& feedUrl, const QString& error)
@@ -311,12 +322,25 @@ void RssFeed::handleFeedParsingFinished(const QString& feedUrl, const QString& e
   if (feedUrl != m_url)
     return;
 
+  if (!error.isEmpty()) {
+    qWarning() << "Failed to parse RSS feed at" << feedUrl;
+    qWarning() << "Reason:" << error;
+  }
+
   // Make sure we limit the number of articles
   removeOldArticles();
 
   m_loading = false;
   m_inErrorState = !error.isEmpty();
-  m_manager->forwardFeedInfosChanged(m_url, displayName(), unreadCount()); // XXX: Ugly
+
+  m_manager->forwardFeedInfosChanged(m_url, displayName(), m_unreadCount);
+  // XXX: Would not be needed if we did this in handleNewArticle() instead
+  m_manager->forwardFeedContentChanged(m_url);
 
   saveItemsToDisk();
 }
+
+void RssFeed::decrementUnreadCount()
+{
+  --m_unreadCount;
+}

src/rss/rssfeed.h

@@ -73,6 +73,7 @@ public:
   virtual RssArticleList articleList() const;
   const RssArticleHash& articleHash() const { return m_articles; }
   virtual RssArticleList unreadArticleList() const;
+  void decrementUnreadCount();
 
 private slots:
   void handleFinishedDownload(const QString& url, const QString &file_path);
@@ -95,7 +96,7 @@ private:
   QString m_alias;
   QString m_icon;
   QString m_iconUrl;
-  bool m_read;
+  uint m_unreadCount;
   bool m_dirty;
   bool m_inErrorState;
   bool m_loading;

src/rss/rssfile.h

@@ -64,6 +64,9 @@ public:
   virtual void removeAllSettings() = 0;
   virtual void saveItemsToDisk() = 0;
   QStringList pathHierarchy() const;
+
+protected:
+  uint m_unreadCount;
 };
 
 #endif // RSSFILE_H

src/rss/rssmanager.cpp

@@ -108,6 +108,11 @@ void RssManager::loadStreamList() {
   qDebug("NB RSS streams loaded: %d", streamsUrl.size());
 }
 
+void RssManager::forwardFeedContentChanged(const QString& url)
+{
+  emit feedContentChanged(url);
+}
+
 void RssManager::forwardFeedInfosChanged(const QString &url, const QString &display_name, uint nbUnread) {
   emit feedInfosChanged(url, display_name, nbUnread);
 }

src/rss/rssmanager.h

@@ -59,12 +59,14 @@ public:
 public slots:
   void loadStreamList();
   void saveStreamList() const;
+  void forwardFeedContentChanged(const QString& url);
   void forwardFeedInfosChanged(const QString &url, const QString &aliasOrUrl, uint nbUnread);
   void forwardFeedIconChanged(const QString &url, const QString &icon_path);
   void moveFile(const RssFilePtr& file, const RssFolderPtr& dest_folder);
   void updateRefreshInterval(uint val);
 
 signals:
+  void feedContentChanged(const QString& url);
   void feedInfosChanged(const QString &url, const QString &display_name, uint nbUnread);
   void feedIconChanged(const QString &url, const QString &icon_path);