Merge pull request #8976 from glassez/feed-uid

Don't use RSS feed URLs as base for file names. Closes #8399

src/base/rss/rss_feed.cpp

@@ -41,6 +41,7 @@
 #include <QUrl>
 
 #include "../asyncfilestorage.h"
+#include "../global.h"
 #include "../logger.h"
 #include "../net/downloadhandler.h"
 #include "../net/downloadmanager.h"
@@ -50,21 +51,30 @@
 #include "rss_article.h"
 #include "rss_session.h"
 
-const QString Str_Url(QStringLiteral("url"));
+const QString KEY_UID(QStringLiteral("uid"));
-const QString Str_Title(QStringLiteral("title"));
+const QString KEY_URL(QStringLiteral("url"));
-const QString Str_LastBuildDate(QStringLiteral("lastBuildDate"));
+const QString KEY_TITLE(QStringLiteral("title"));
-const QString Str_IsLoading(QStringLiteral("isLoading"));
+const QString KEY_LASTBUILDDATE(QStringLiteral("lastBuildDate"));
-const QString Str_HasError(QStringLiteral("hasError"));
+const QString KEY_ISLOADING(QStringLiteral("isLoading"));
-const QString Str_Articles(QStringLiteral("articles"));
+const QString KEY_HASERROR(QStringLiteral("hasError"));
+const QString KEY_ARTICLES(QStringLiteral("articles"));
 
 using namespace RSS;
 
-Feed::Feed(const QString &url, const QString &path, Session *session)
+Feed::Feed(const QUuid &uid, const QString &url, const QString &path, Session *session)
     : Item(path)
     , m_session(session)
+    , m_uid(uid)
     , m_url(url)
 {
-    m_dataFileName = QString("%1.json").arg(Utils::Fs::toValidFileSystemName(m_url, false, QLatin1String("_")));
+    m_dataFileName = QString::fromLatin1(m_uid.toRfc4122().toHex()) + QLatin1String(".json");
+
+    // Move to new file naming scheme (since v4.1.2)
+    const QString legacyFilename {Utils::Fs::toValidFileSystemName(m_url, false, QLatin1String("_"))
+                + QLatin1String(".json")};
+    const QDir storageDir {m_session->dataFileStorage()->storageDir()};
+    if (!QFile::exists(storageDir.absoluteFilePath(m_dataFileName)))
+        QFile::rename(storageDir.absoluteFilePath(legacyFilename), storageDir.absoluteFilePath(m_dataFileName));
 
     m_parser = new Private::Parser(m_lastBuildDate);
     m_parser->moveToThread(m_session->workingThread());
@@ -127,6 +137,11 @@ void Feed::refresh()
     emit stateChanged(this);
 }
 
+QUuid Feed::uid() const
+{
+    return m_uid;
+}
+
 QString Feed::url() const
 {
     return m_url;
@@ -414,25 +429,21 @@ QString Feed::iconPath() const
 
 QJsonValue Feed::toJsonValue(bool withData) const
 {
-    if (!withData) {
-        // if feed alias is empty we create "reduced" JSON
-        // value for it since its name is equal to its URL
-        return (name() == url() ? "" : url());
-        // if we'll need storing some more properties we should check
-        // for its default values and produce JSON object instead of (if it's required)
-    }
-
-    QJsonArray jsonArr;
-    foreach (Article *article, m_articles)
-        jsonArr << article->toJsonObject();
-
     QJsonObject jsonObj;
-    jsonObj.insert(Str_Url, url());
+    jsonObj.insert(KEY_UID, uid().toString());
-    jsonObj.insert(Str_Title, title());
+    jsonObj.insert(KEY_URL, url());
-    jsonObj.insert(Str_LastBuildDate, lastBuildDate());
+
-    jsonObj.insert(Str_IsLoading, isLoading());
+    if (withData) {
-    jsonObj.insert(Str_HasError, hasError());
+        jsonObj.insert(KEY_TITLE, title());
-    jsonObj.insert(Str_Articles, jsonArr);
+        jsonObj.insert(KEY_LASTBUILDDATE, lastBuildDate());
+        jsonObj.insert(KEY_ISLOADING, isLoading());
+        jsonObj.insert(KEY_HASERROR, hasError());
+
+        QJsonArray jsonArr;
+        for (Article *article : qAsConst(m_articles))
+            jsonArr << article->toJsonObject();
+        jsonObj.insert(KEY_ARTICLES, jsonArr);
+    }
 
     return jsonObj;
 }

src/base/rss/rss_feed.h

@@ -33,6 +33,7 @@
 #include <QBasicTimer>
 #include <QHash>
 #include <QList>
+#include <QUuid>
 
 #include "rss_item.h"
 
@@ -56,7 +57,7 @@ namespace RSS
 
         friend class Session;
 
-        Feed(const QString &url, const QString &path, Session *session);
+        Feed(const QUuid &uid, const QString &url, const QString &path, Session *session);
         ~Feed() override;
 
     public:
@@ -65,6 +66,7 @@ namespace RSS
         void markAsRead() override;
         void refresh() override;
 
+        QUuid uid() const;
         QString url() const;
         QString title() const;
         QString lastBuildDate() const;
@@ -105,6 +107,7 @@ namespace RSS
 
         Session *m_session;
         Private::Parser *m_parser;
+        const QUuid m_uid;
         const QString m_url;
         QString m_title;
         QString m_lastBuildDate;

src/base/rss/rss_session.cpp

@@ -166,7 +166,7 @@ bool Session::addFeed(const QString &url, const QString &path, QString *error)
     if (!destFolder)
         return false;
 
-    addItem(new Feed(url, path, this), destFolder);
+    addItem(new Feed(generateUID(), url, path, this), destFolder);
     store();
     if (m_processingEnabled)
         feedByURL(url)->refresh();
@@ -282,36 +282,61 @@ void Session::load()
 
 void Session::loadFolder(const QJsonObject &jsonObj, Folder *folder)
 {
+    bool updated = false;
     foreach (const QString &key, jsonObj.keys()) {
-        QJsonValue val = jsonObj[key];
+        const QJsonValue val {jsonObj[key]};
         if (val.isString()) {
+            // previous format (reduced form) doesn't contain UID
             QString url = val.toString();
             if (url.isEmpty())
                 url = key;
-            addFeedToFolder(url, key, folder);
+            addFeedToFolder(generateUID(), url, key, folder);
+            updated = true;
         }
-        else if (!val.isObject()) {
+        else if (val.isObject()) {
-            Logger::instance()->addMessage(
+            const QJsonObject valObj {val.toObject()};
-                        QString("Couldn't load RSS Item '%1'. Invalid data format.")
-                        .arg(QString("%1\\%2").arg(folder->path(), key)), Log::WARNING);
-        }
-        else {
-            QJsonObject valObj = val.toObject();
             if (valObj.contains("url")) {
                 if (!valObj["url"].isString()) {
-                    Logger::instance()->addMessage(
+                    LogMsg(QString("Couldn't load RSS Feed '%1'. URL is required.")
-                                QString("Couldn't load RSS Feed '%1'. URL is required.")
+                           .arg(QString("%1\\%2").arg(folder->path(), key)), Log::WARNING);
-                                .arg(QString("%1\\%2").arg(folder->path(), key)), Log::WARNING);
                     continue;
                 }
 
-                addFeedToFolder(valObj["url"].toString(), key, folder);
+                QUuid uid;
+                if (valObj.contains("uid")) {
+                    uid = QUuid {valObj["uid"].toString()};
+                    if (uid.isNull()) {
+                        LogMsg(QString("Couldn't load RSS Feed '%1'. UID is invalid.")
+                               .arg(QString("%1\\%2").arg(folder->path(), key)), Log::WARNING);
+                        continue;
+                    }
+
+                    if (m_feedsByUID.contains(uid)) {
+                        LogMsg(QString("Duplicate RSS Feed UID: %1. Configuration seems to be corrupted.")
+                               .arg(uid.toString()), Log::WARNING);
+                        continue;
+                    }
+                }
+                else {
+                    // previous format doesn't contain UID
+                    uid = generateUID();
+                    updated = true;
+                }
+
+                addFeedToFolder(uid, valObj["url"].toString(), key, folder);
             }
             else {
                 loadFolder(valObj, addSubfolder(key, folder));
             }
         }
+        else {
+            LogMsg(QString("Couldn't load RSS Item '%1'. Invalid data format.")
+                   .arg(QString("%1\\%2").arg(folder->path(), key)), Log::WARNING);
+        }
     }
+
+    if (updated)
+        store(); // convert to updated format
 }
 
 void Session::loadLegacy()
@@ -324,7 +349,7 @@ void Session::loadLegacy()
     }
 
     uint i = 0;
-    foreach (QString legacyPath, legacyFeedPaths) {
+    for (QString legacyPath : legacyFeedPaths) {
         if (Item::PathSeparator == QString(legacyPath[0]))
             legacyPath.remove(0, 1);
         const QString parentFolderPath = Item::parentPath(legacyPath);
@@ -380,9 +405,9 @@ Folder *Session::addSubfolder(const QString &name, Folder *parentFolder)
     return folder;
 }
 
-Feed *Session::addFeedToFolder(const QString &url, const QString &name, Folder *parentFolder)
+Feed *Session::addFeedToFolder(const QUuid &uid, const QString &url, const QString &name, Folder *parentFolder)
 {
-    auto feed = new Feed(url, Item::joinPath(parentFolder->path(), name), this);
+    auto feed = new Feed(uid, url, Item::joinPath(parentFolder->path(), name), this);
     addItem(feed, parentFolder);
     return feed;
 }
@@ -393,6 +418,7 @@ void Session::addItem(Item *item, Folder *destFolder)
         connect(feed, &Feed::titleChanged, this, &Session::handleFeedTitleChanged);
         connect(feed, &Feed::iconLoaded, this, &Session::feedIconLoaded);
         connect(feed, &Feed::stateChanged, this, &Session::feedStateChanged);
+        m_feedsByUID[feed->uid()] = feed;
         m_feedsByURL[feed->url()] = feed;
     }
 
@@ -473,8 +499,10 @@ void Session::handleItemAboutToBeDestroyed(Item *item)
 {
     m_itemsByPath.remove(item->path());
     auto feed = qobject_cast<Feed *>(item);
-    if (feed)
+    if (feed) {
+        m_feedsByUID.remove(feed->uid());
         m_feedsByURL.remove(feed->url());
+    }
 }
 
 void Session::handleFeedTitleChanged(Feed *feed)
@@ -485,6 +513,15 @@ void Session::handleFeedTitleChanged(Feed *feed)
         moveItem(feed, Item::joinPath(Item::parentPath(feed->path()), feed->title()));
 }
 
+QUuid Session::generateUID() const
+{
+    QUuid uid = QUuid::createUuid();
+    while (m_feedsByUID.contains(uid))
+        uid = QUuid::createUuid();
+
+    return uid;
+}
+
 int Session::maxArticlesPerFeed() const
 {
     return m_maxArticlesPerFeed;

src/base/rss/rss_session.h

@@ -37,13 +37,19 @@
  * {
  *     "folder1": {
  *         "subfolder1": {
- *             "Feed name (Alias)": "http://some-feed-url1",
+ *             "Feed name 1 (Alias)": {
- *             "http://some-feed-url2": ""
+ *                 "uid": "feed unique identifier",
+ *                 "url": "http://some-feed-url1"
+ *             }
+ *             "Feed name 2 (Alias)": {
+ *                 "uid": "feed unique identifier",
+ *                 "url": "http://some-feed-url2"
+ *             }
  *         },
  *         "subfolder2": {},
- *         "http://some-feed-url3": "",
+ *         "Feed name 3 (Alias)": {
- *         "Feed name (Alias)": {
+ *             "uid": "feed unique identifier",
- *             "url": "http://some-feed-url4",
+ *             "url": "http://some-feed-url3"
  *         }
  *     },
  *     "folder2": {},
@@ -53,8 +59,7 @@
  *
  * 1.   Document is JSON object (the same as Folder)
  * 2.   Folder is JSON object (keys are Item names, values are Items)
- * 3.1. Feed is JSON object (keys are property names, values are property values; 'url' is required)
+ * 3.   Feed is JSON object (keys are property names, values are property values; 'uid' and 'url' are required)
- * 3.2. (Reduced format) Feed is JSON string (string is URL unless it's empty, otherwise we take Feed URL from name)
  */
 
 #include <QHash>
@@ -130,13 +135,14 @@ namespace RSS
         void handleFeedTitleChanged(Feed *feed);
 
     private:
+        QUuid generateUID() const;
         void load();
         void loadFolder(const QJsonObject &jsonObj, Folder *folder);
         void loadLegacy();
         void store();
         Folder *prepareItemDest(const QString &path, QString *error);
         Folder *addSubfolder(const QString &name, Folder *parentFolder);
-        Feed *addFeedToFolder(const QString &url, const QString &name, Folder *parentFolder);
+        Feed *addFeedToFolder(const QUuid &uid, const QString &url, const QString &name, Folder *parentFolder);
         void addItem(Item *item, Folder *destFolder);
 
         static QPointer<Session> m_instance;
@@ -149,6 +155,7 @@ namespace RSS
         uint m_refreshInterval;
         int m_maxArticlesPerFeed;
         QHash<QString, Item *> m_itemsByPath;
+        QHash<QUuid, Feed *> m_feedsByUID;
         QHash<QString, Feed *> m_feedsByURL;
     };
 }