Fixes for synchronization cache, fixes for cache in TT-RSS, more to come.

.github/ISSUE_TEMPLATE/bug_report.md

@@ -1,8 +1,8 @@
 ---
 name: Bug report
 about: Create a report to help us improve
-title: ''
+title: '[BUG]: '
-labels: ''
+labels: Type-Defect
 assignees: martinrotter
 ---
 <!---

.github/ISSUE_TEMPLATE/feature_request.md

@@ -1,7 +1,7 @@
 ---
 name: Feature request
 about: Suggest an idea for this project
-title: ''
+title: '[FR]: '
 labels: Type-Enhancement
 assignees: martinrotter
 ---

src/librssguard/services/abstract/cacheforserviceroot.cpp

@@ -10,35 +10,43 @@
 #include <QDir>
 #include <QSet>
 
-CacheForServiceRoot::CacheForServiceRoot() : m_cacheSaveMutex(new QMutex()) {}
+CacheForServiceRoot::CacheForServiceRoot() : m_uniqueId(NO_PARENT_CATEGORY), m_cacheSaveMutex(new QMutex()) {}
-
-void CacheForServiceRoot::addLabelsAssignmentsToCache(const QList<Message>& ids_of_messages, Label* lbl, bool assign) {
-  auto custom_ids = lbl->getParentServiceRoot()->customIDsOfMessages(ids_of_messages);
 
+void CacheForServiceRoot::addLabelsAssignmentsToCache(const QStringList& ids_of_messages,
+                                                      const QString& lbl_custom_id,
+                                                      bool assign) {
   if (assign) {
-    for (const QString& custom_id : custom_ids) {
+    for (const QString& custom_id : ids_of_messages) {
-      if (m_cachedLabelDeassignments[lbl->customId()].contains(custom_id)) {
+      if (m_cachedLabelDeassignments[lbl_custom_id].contains(custom_id)) {
         // We want to assign this ID but it was marked for deassignment, remove from deassignment.
-        m_cachedLabelDeassignments[lbl->customId()].removeAll(custom_id);
+        m_cachedLabelDeassignments[lbl_custom_id].removeAll(custom_id);
       }
       else {
-        m_cachedLabelAssignments[lbl->customId()].append(custom_id);
+        m_cachedLabelAssignments[lbl_custom_id].append(custom_id);
-        m_cachedLabelAssignments[lbl->customId()].removeDuplicates();
+        m_cachedLabelAssignments[lbl_custom_id].removeDuplicates();
       }
     }
   }
   else {
-    for (const QString& custom_id : custom_ids) {
+    for (const QString& custom_id : ids_of_messages) {
-      if (m_cachedLabelAssignments[lbl->customId()].contains(custom_id)) {
+      if (m_cachedLabelAssignments[lbl_custom_id].contains(custom_id)) {
         // We want to deassign this ID but it was marked for assignment, remove from assignment.
-        m_cachedLabelAssignments[lbl->customId()].removeAll(custom_id);
+        m_cachedLabelAssignments[lbl_custom_id].removeAll(custom_id);
       }
       else {
-        m_cachedLabelDeassignments[lbl->customId()].append(custom_id);
+        m_cachedLabelDeassignments[lbl_custom_id].append(custom_id);
-        m_cachedLabelDeassignments[lbl->customId()].removeDuplicates();
+        m_cachedLabelDeassignments[lbl_custom_id].removeDuplicates();
       }
     }
   }
+
+  saveCacheToFile();
+}
+
+void CacheForServiceRoot::addLabelsAssignmentsToCache(const QList<Message>& ids_of_messages, Label* lbl, bool assign) {
+  auto custom_ids = lbl->getParentServiceRoot()->customIDsOfMessages(ids_of_messages);
+
+  addLabelsAssignmentsToCache(custom_ids, lbl->customId(), assign);
 }
 
 void CacheForServiceRoot::addMessageStatesToCache(const QList<Message>& ids_of_messages, RootItem::Importance importance) {
@@ -65,6 +73,8 @@ void CacheForServiceRoot::addMessageStatesToCache(const QList<Message>& ids_of_m
   list_act.append(set_act.values());
   list_other.clear();
   list_other.append(set_other.values());
+
+  saveCacheToFile();
 }
 
 void CacheForServiceRoot::addMessageStatesToCache(const QStringList& ids_of_messages, RootItem::ReadStatus read) {
@@ -91,13 +101,13 @@ void CacheForServiceRoot::addMessageStatesToCache(const QStringList& ids_of_mess
   list_act.append(set_act.values());
   list_other.clear();
   list_other.append(set_other.values());
+
+  saveCacheToFile();
 }
 
-void CacheForServiceRoot::saveCacheToFile(int acc_id) {
+void CacheForServiceRoot::saveCacheToFile() {
-  QMutexLocker lck(m_cacheSaveMutex.data());
-
   // Save to file.
-  const QString file_cache = qApp->userDataFolder() + QDir::separator() + QString::number(acc_id) + "-cached-msgs.dat";
+  const QString file_cache = qApp->userDataFolder() + QDir::separator() + QString::number(m_uniqueId) + "-cached-msgs.dat";
 
   if (isEmpty()) {
     QFile::remove(file_cache);
@@ -112,8 +122,6 @@ void CacheForServiceRoot::saveCacheToFile(int acc_id) {
       file.flush();
       file.close();
     }
-
-    clearCache();
   }
 }
 
@@ -124,28 +132,29 @@ void CacheForServiceRoot::clearCache() {
   m_cachedLabelDeassignments.clear();
 }
 
-void CacheForServiceRoot::loadCacheFromFile(int acc_id) {
+void CacheForServiceRoot::loadCacheFromFile() {
   QMutexLocker lck(m_cacheSaveMutex.data());
 
   clearCache();
 
   // Load from file.
-  const QString file_cache = qApp->userDataFolder() + QDir::separator() + QString::number(acc_id) + "-cached-msgs.dat";
+  const QString file_cache = qApp->userDataFolder() + QDir::separator() + QString::number(m_uniqueId) + "-cached-msgs.dat";
   QFile file(file_cache);
 
   if (file.exists()) {
-    if (file.open(QIODevice::ReadOnly)) {
+    if (file.open(QIODevice::OpenModeFlag::ReadOnly)) {
       QDataStream stream(&file);
 
       stream >> m_cachedStatesImportant >> m_cachedStatesRead >> m_cachedLabelAssignments >> m_cachedLabelDeassignments;
-      file.flush();
       file.close();
     }
-
-    file.remove();
   }
 }
 
+void CacheForServiceRoot::setUniqueId(int unique_id) {
+  m_uniqueId = unique_id;
+}
+
 CacheSnapshot CacheForServiceRoot::takeMessageCache() {
   QMutexLocker lck(m_cacheSaveMutex.data());
 
@@ -165,6 +174,7 @@ CacheSnapshot CacheForServiceRoot::takeMessageCache() {
   cached_deass_lbl.detach();
 
   clearCache();
+  saveCacheToFile();
 
   CacheSnapshot c;
 

src/librssguard/services/abstract/cacheforserviceroot.h

@@ -22,20 +22,29 @@ class CacheForServiceRoot {
   public:
     explicit CacheForServiceRoot();
 
+    virtual void saveAllCachedData(bool async = true) = 0;
+
+    void addLabelsAssignmentsToCache(const QStringList& ids_of_messages, const QString& lbl_custom_id, bool assign);
     void addLabelsAssignmentsToCache(const QList<Message>& ids_of_messages, Label* lbl, bool assign);
     void addMessageStatesToCache(const QList<Message>& ids_of_messages, RootItem::Importance importance);
     void addMessageStatesToCache(const QStringList& ids_of_messages, RootItem::ReadStatus read);
 
-    // Persistently saves/loads cached changes to/from file.
+    void loadCacheFromFile();
-    // NOTE: The whole cache is cleared after save is done and before load is done.
+    void setUniqueId(int unique_id);
-    void saveCacheToFile(int acc_id);
-    void loadCacheFromFile(int acc_id);
-
-    virtual void saveAllCachedData(bool async = true) = 0;
 
   protected:
+
+    // Returns all cached data and clears the cache.
+    // NOTE: If returned data are not successfuly passed back to
+    // server then caller needs to re-add the data back to cache.
     CacheSnapshot takeMessageCache();
 
+  private:
+    bool isEmpty() const;
+    void clearCache();
+    void saveCacheToFile();
+
+    int m_uniqueId;
     QScopedPointer<QMutex> m_cacheSaveMutex;
 
     // Map where key is label's custom ID and value is list of message custom IDs
@@ -51,10 +60,6 @@ class CacheForServiceRoot {
 
     // Map of cached important/unimportant changes.
     QMap<RootItem::Importance, QList<Message>> m_cachedStatesImportant;
-
-  private:
-    bool isEmpty() const;
-    void clearCache();
 };
 
 #endif // CACHEFORSERVICEROOT_H

src/librssguard/services/abstract/serviceroot.cpp

@@ -499,6 +499,12 @@ int ServiceRoot::accountId() const {
 
 void ServiceRoot::setAccountId(int account_id) {
   m_accountId = account_id;
+
+  auto* cache = dynamic_cast<CacheForServiceRoot*>(this);
+
+  if (cache != nullptr) {
+    cache->setUniqueId(account_id);
+  }
 }
 
 bool ServiceRoot::loadMessagesForItem(RootItem* item, MessagesModel* model) {

src/librssguard/services/gmail/gmailserviceroot.cpp

@@ -184,17 +184,14 @@ void GmailServiceRoot::start(bool freshly_activated) {
   Q_UNUSED(freshly_activated)
 
   loadFromDatabase();
-  loadCacheFromFile(accountId());
+  loadCacheFromFile();
 
   if (childCount() <= 3) {
     syncIn();
   }
-
+  else {
-  m_network->oauth()->login();
+    m_network->oauth()->login();
-}
+  }
-
-void GmailServiceRoot::stop() {
-  saveCacheToFile(accountId());
 }
 
 QString GmailServiceRoot::code() const {

src/librssguard/services/gmail/gmailserviceroot.h

@@ -31,7 +31,6 @@ class GmailServiceRoot : public ServiceRoot, public CacheForServiceRoot {
     virtual bool supportsFeedAdding() const;
     virtual bool supportsCategoryAdding() const;
     virtual void start(bool freshly_activated);
-    virtual void stop();
     virtual QString code() const;
     virtual QString additionalTooltip() const;
     virtual void saveAllCachedData(bool async = true);

src/librssguard/services/inoreader/inoreaderserviceroot.cpp

@@ -108,7 +108,7 @@ void InoreaderServiceRoot::start(bool freshly_activated) {
   Q_UNUSED(freshly_activated)
 
   loadFromDatabase();
-  loadCacheFromFile(accountId());
+  loadCacheFromFile();
 
   if (childCount() <= 3) {
     syncIn();
@@ -118,10 +118,6 @@ void InoreaderServiceRoot::start(bool freshly_activated) {
   }
 }
 
-void InoreaderServiceRoot::stop() {
-  saveCacheToFile(accountId());
-}
-
 QString InoreaderServiceRoot::code() const {
   return InoreaderEntryPoint().code();
 }

src/librssguard/services/inoreader/inoreaderserviceroot.h

@@ -29,7 +29,6 @@ class InoreaderServiceRoot : public ServiceRoot, public CacheForServiceRoot {
     virtual bool supportsFeedAdding() const;
     virtual bool supportsCategoryAdding() const;
     virtual void start(bool freshly_activated);
-    virtual void stop();
     virtual QString code() const;
     virtual QString additionalTooltip() const;
     virtual void saveAllCachedData(bool async = true);

src/librssguard/services/owncloud/owncloudserviceroot.cpp

@@ -65,17 +65,13 @@ bool OwnCloudServiceRoot::supportsCategoryAdding() const {
 void OwnCloudServiceRoot::start(bool freshly_activated) {
   Q_UNUSED(freshly_activated)
   loadFromDatabase();
-  loadCacheFromFile(accountId());
+  loadCacheFromFile();
 
   if (childCount() <= 3) {
     syncIn();
   }
 }
 
-void OwnCloudServiceRoot::stop() {
-  saveCacheToFile(accountId());
-}
-
 QString OwnCloudServiceRoot::code() const {
   return OwnCloudServiceEntryPoint().code();
 }

src/librssguard/services/owncloud/owncloudserviceroot.h

@@ -18,24 +18,22 @@ class OwnCloudServiceRoot : public ServiceRoot, public CacheForServiceRoot {
     explicit OwnCloudServiceRoot(RootItem* parent = nullptr);
     virtual ~OwnCloudServiceRoot();
 
-    bool isSyncable() const;
+    virtual bool isSyncable() const;
-    bool canBeEdited() const;
+    virtual bool canBeEdited() const;
-    bool canBeDeleted() const;
+    virtual bool canBeDeleted() const;
-    bool editViaGui();
+    virtual bool editViaGui();
-    bool deleteViaGui();
+    virtual bool deleteViaGui();
-    bool supportsFeedAdding() const;
+    virtual bool supportsFeedAdding() const;
-    bool supportsCategoryAdding() const;
+    virtual bool supportsCategoryAdding() const;
+    virtual void start(bool freshly_activated);
+    virtual QString code() const;
+    virtual void saveAllCachedData(bool async = true);
 
-    void start(bool freshly_activated);
-    void stop();
-    QString code() const;
     OwnCloudNetworkFactory* network() const;
 
     void updateTitle();
     void saveAccountDataToDatabase();
 
-    void saveAllCachedData(bool async = true);
-
   protected:
     virtual RootItem* obtainNewTreeForSyncIn() const;
 

src/librssguard/services/tt-rss/network/ttrssnetworkfactory.cpp

@@ -332,7 +332,8 @@ TtRssResponse TtRssNetworkFactory::setArticleLabel(const QStringList& article_id
 
 TtRssUpdateArticleResponse TtRssNetworkFactory::updateArticles(const QStringList& ids,
                                                                UpdateArticle::OperatingField field,
-                                                               UpdateArticle::Mode mode, bool async) {
+                                                               UpdateArticle::Mode mode,
+                                                               bool async) {
   Q_UNUSED(async)
 
   QJsonObject json;
@@ -515,6 +516,7 @@ TtRssResponse::TtRssResponse(const QString& raw_content) {
 }
 
 TtRssResponse::~TtRssResponse() = default;
+
 bool TtRssResponse::isLoaded() const {
   return !m_rawContent.isEmpty();
 }

src/librssguard/services/tt-rss/ttrssserviceroot.cpp

@@ -39,7 +39,7 @@ ServiceRoot::LabelOperation TtRssServiceRoot::supportedLabelOperations() const {
 void TtRssServiceRoot::start(bool freshly_activated) {
   Q_UNUSED(freshly_activated)
   loadFromDatabase();
-  loadCacheFromFile(accountId());
+  loadCacheFromFile();
 
   if (childCount() <= 3) {
     syncIn();
@@ -47,8 +47,6 @@ void TtRssServiceRoot::start(bool freshly_activated) {
 }
 
 void TtRssServiceRoot::stop() {
-  saveCacheToFile(accountId());
-
   m_network->logout();
   qDebugNN << LOGSEC_TTRSS
            << "Stopping Tiny Tiny RSS account, logging out with result"
@@ -130,12 +128,16 @@ void TtRssServiceRoot::saveAllCachedData(bool async) {
     QStringList ids = i.value();
 
     if (!ids.isEmpty()) {
-      network()->updateArticles(ids,
+      auto res = network()->updateArticles(ids,
-                                UpdateArticle::OperatingField::Unread,
+                                           UpdateArticle::OperatingField::Unread,
-                                key == RootItem::ReadStatus::Unread
+                                           key == RootItem::ReadStatus::Unread
-                                ? UpdateArticle::Mode::SetToTrue
+                                           ? UpdateArticle::Mode::SetToTrue
-                                : UpdateArticle::Mode::SetToFalse,
+                                           : UpdateArticle::Mode::SetToFalse,
-                                async);
+                                           async);
+
+      if (network()->lastError() != QNetworkReply::NetworkError::NoError || res.hasError()) {
+        addMessageStatesToCache(ids, key);
+      }
     }
   }
 
@@ -149,13 +151,16 @@ void TtRssServiceRoot::saveAllCachedData(bool async) {
 
     if (!messages.isEmpty()) {
       QStringList ids = customIDsOfMessages(messages);
+      auto res = network()->updateArticles(ids,
+                                           UpdateArticle::OperatingField::Starred,
+                                           key == RootItem::Importance::Important
+                                           ? UpdateArticle::Mode::SetToTrue
+                                           : UpdateArticle::Mode::SetToFalse,
+                                           async);
 
-      network()->updateArticles(ids,
+      if (network()->lastError() != QNetworkReply::NetworkError::NoError || res.hasError()) {
-                                UpdateArticle::OperatingField::Starred,
+        addMessageStatesToCache(messages, key);
-                                key == RootItem::Importance::Important
+      }
-                                ? UpdateArticle::Mode::SetToTrue
-                                : UpdateArticle::Mode::SetToFalse,
-                                async);
     }
   }
 
@@ -168,7 +173,11 @@ void TtRssServiceRoot::saveAllCachedData(bool async) {
     QStringList messages = k.value();
 
     if (!messages.isEmpty()) {
-      network()->setArticleLabel(messages, label_custom_id, true);
+      auto res = network()->setArticleLabel(messages, label_custom_id, true);
+
+      if (network()->lastError() != QNetworkReply::NetworkError::NoError || res.hasError()) {
+        addLabelsAssignmentsToCache(messages, label_custom_id, true);
+      }
     }
   }
 
@@ -181,7 +190,11 @@ void TtRssServiceRoot::saveAllCachedData(bool async) {
     QStringList messages = l.value();
 
     if (!messages.isEmpty()) {
-      network()->setArticleLabel(messages, label_custom_id, false);
+      auto res = network()->setArticleLabel(messages, label_custom_id, false);
+
+      if (network()->lastError() != QNetworkReply::NetworkError::NoError || res.hasError()) {
+        addLabelsAssignmentsToCache(messages, label_custom_id, false);
+      }
     }
   }
 }