From aecdc57cd44d2f8dcde1912746caf4267e12e9e7 Mon Sep 17 00:00:00 2001 From: Christophe Dumez Date: Sat, 18 Aug 2012 22:25:21 +0300 Subject: [PATCH] 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 | 22 +++++++++++------ src/rss/rss_imp.h | 1 + src/rss/rssarticle.cpp | 5 ++++ src/rss/rssfeed.cpp | 54 ++++++++++++++++++++++++++++++------------ src/rss/rssfeed.h | 3 ++- src/rss/rssfile.h | 3 +++ src/rss/rssmanager.cpp | 5 ++++ src/rss/rssmanager.h | 2 ++ 8 files changed, 72 insertions(+), 23 deletions(-) diff --git a/src/rss/rss_imp.cpp b/src/rss/rss_imp.cpp index efb2a52ca..d769cce9f 100644 --- a/src/rss/rss_imp.cpp +++ b/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(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())); diff --git a/src/rss/rss_imp.h b/src/rss/rss_imp.h index 29e342f31..f084c14ce 100644 --- a/src/rss/rss_imp.h +++ b/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 &items); void updateItemInfos(QTreeWidgetItem *item); void openNewsUrl(); diff --git a/src/rss/rssarticle.cpp b/src/rss/rssarticle.cpp index 14242e060..6cd7732d6 100644 --- a/src/rss/rssarticle.cpp +++ b/src/rss/rssarticle.cpp @@ -33,6 +33,7 @@ #include #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 diff --git a/src/rss/rssfeed.cpp b/src/rss/rssfeed.cpp index 14f006342..360292502 100644 --- a/src/rss/rssfeed.cpp +++ b/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; iguid()); + 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; +} diff --git a/src/rss/rssfeed.h b/src/rss/rssfeed.h index 332bf8135..db71f8e7a 100644 --- a/src/rss/rssfeed.h +++ b/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; diff --git a/src/rss/rssfile.h b/src/rss/rssfile.h index b2390e458..8d9684531 100644 --- a/src/rss/rssfile.h +++ b/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 diff --git a/src/rss/rssmanager.cpp b/src/rss/rssmanager.cpp index d1c76f6a6..f8a62c8c8 100644 --- a/src/rss/rssmanager.cpp +++ b/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); } diff --git a/src/rss/rssmanager.h b/src/rss/rssmanager.h index 1de71f578..545e293fa 100644 --- a/src/rss/rssmanager.h +++ b/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);