From af30552364e8c5f8403085192523beef95d983de Mon Sep 17 00:00:00 2001 From: Martin Rotter Date: Sun, 25 Jul 2021 09:08:43 +0200 Subject: [PATCH] very experimental solution for #451 --- src/librssguard/database/databasequeries.cpp | 74 +++++++++++-------- src/librssguard/database/databasequeries.h | 4 +- src/librssguard/services/abstract/feed.cpp | 4 +- .../services/standard/atomparser.cpp | 1 + .../services/standard/rssparser.cpp | 1 + 5 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/librssguard/database/databasequeries.cpp b/src/librssguard/database/databasequeries.cpp index 54885c51d..b0ca20289 100755 --- a/src/librssguard/database/databasequeries.cpp +++ b/src/librssguard/database/databasequeries.cpp @@ -79,7 +79,7 @@ bool DatabaseQueries::deassignLabelFromMessage(const QSqlDatabase& db, Label* la q.setForwardOnly(true); q.prepare("DELETE FROM LabelsInMessages WHERE label = :label AND message = :message AND account_id = :account_id;"); q.bindValue(QSL(":label"), label->customId()); - q.bindValue(QSL(":message"), msg.m_customId); + q.bindValue(QSL(":message"), msg.m_customId.isEmpty() ? QString::number(msg.m_id) : msg.m_customId); q.bindValue(QSL(":account_id"), label->getParentServiceRoot()->accountId()); return q.exec(); @@ -91,7 +91,7 @@ bool DatabaseQueries::assignLabelToMessage(const QSqlDatabase& db, Label* label, q.setForwardOnly(true); q.prepare("DELETE FROM LabelsInMessages WHERE label = :label AND message = :message AND account_id = :account_id;"); q.bindValue(QSL(":label"), label->customId()); - q.bindValue(QSL(":message"), msg.m_customId); + q.bindValue(QSL(":message"), msg.m_customId.isEmpty() ? QString::number(msg.m_id) : msg.m_customId); q.bindValue(QSL(":account_id"), label->getParentServiceRoot()->accountId()); auto succ = q.exec(); @@ -99,7 +99,7 @@ bool DatabaseQueries::assignLabelToMessage(const QSqlDatabase& db, Label* label, if (succ) { q.prepare("INSERT INTO LabelsInMessages (label, message, account_id) VALUES (:label, :message, :account_id);"); q.bindValue(QSL(":label"), label->customId()); - q.bindValue(QSL(":message"), msg.m_customId); + q.bindValue(QSL(":message"), msg.m_customId.isEmpty() ? QString::number(msg.m_id) : msg.m_customId); q.bindValue(QSL(":account_id"), label->getParentServiceRoot()->accountId()); succ = q.exec(); @@ -170,7 +170,7 @@ QList DatabaseQueries::getLabelsForMessage(const QSqlDatabase& db, q.prepare("SELECT DISTINCT label FROM LabelsInMessages WHERE message = :message AND account_id = :account_id;"); q.bindValue(QSL(":account_id"), msg.m_accountId); - q.bindValue(QSL(":message"), msg.m_customId); + q.bindValue(QSL(":message"), msg.m_customId.isEmpty() ? QString::number(msg.m_id) : msg.m_customId); if (q.exec()) { auto iter = boolinq::from(installed_labels); @@ -1024,8 +1024,7 @@ QHash DatabaseQueries::bagsOfMessages(const QSqlDatabase& QPair DatabaseQueries::updateMessages(QSqlDatabase db, const QList& messages, - const QString& feed_custom_id, - int account_id, + Feed* feed, const QString& url, bool force_update, bool* ok) { @@ -1036,6 +1035,8 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, bool use_transactions = qApp->settings()->value(GROUP(Database), SETTING(Database::UseTransactions)).toBool(); QPair updated_messages = { 0, 0 }; + int account_id = feed->getParentServiceRoot()->accountId(); + auto feed_custom_id = feed->customId(); // Prepare queries. QSqlQuery query_select_with_url(db); @@ -1058,14 +1059,14 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, // When we have custom ID of the message, we can check directly for existence // of that particular message. query_select_with_custom_id.setForwardOnly(true); - query_select_with_custom_id.prepare("SELECT id, date_created, is_read, is_important, contents, feed FROM Messages " + query_select_with_custom_id.prepare("SELECT id, date_created, is_read, is_important, contents, feed, title FROM Messages " "WHERE custom_id = :custom_id AND account_id = :account_id;"); // In some case, messages are already stored in the DB and they all have primary DB ID. // This is particularly the case when user runs some message filter manually on existing messages // of some feed. query_select_with_id.setForwardOnly(true); - query_select_with_id.prepare("SELECT date_created, is_read, is_important, contents, feed FROM Messages " + query_select_with_id.prepare("SELECT date_created, is_read, is_important, contents, feed, title FROM Messages " "WHERE id = :id AND account_id = :account_id;"); // Used to insert new messages. @@ -1090,6 +1091,7 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, for (Message message : messages) { // Check if messages contain relative URLs and if they do, then replace them. if (message.m_url.startsWith(QL1S("//"))) { + // TODO: This probably should be replace with HTTPS or taken scheme from feed's URL? message.m_url = QString(URI_SCHEME_HTTP) + message.m_url.mid(2); } else if (message.m_url.startsWith(QL1S("/"))) { @@ -1109,6 +1111,7 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, bool is_important_existing_message = false; QString contents_existing_message; QString feed_id_existing_message; + QString title_existing_message; if (message.m_id > 0) { // We recognize directly existing message. @@ -1128,6 +1131,7 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, is_important_existing_message = query_select_with_id.value(2).toBool(); contents_existing_message = query_select_with_id.value(3).toString(); feed_id_existing_message = query_select_with_id.value(4).toString(); + title_existing_message = query_select_with_id.value(5).toString(); qDebugNN << LOGSEC_DB << "Message with these attributes is already present in DB and has DB ID '" @@ -1144,8 +1148,9 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, query_select_with_id.finish(); } else if (message.m_customId.isEmpty()) { - // We need to recognize existing messages according URL & AUTHOR & TITLE. - // NOTE: This particularly concerns messages from standard account. + // We need to recognize existing messages according to URL & AUTHOR & TITLE. + // NOTE: This concerns articles from RSS/ATOM/JSON which do not + // provide unique ID/GUID. query_select_with_url.bindValue(QSL(":feed"), unnulifyString(feed_custom_id)); query_select_with_url.bindValue(QSL(":title"), unnulifyString(message.m_title)); query_select_with_url.bindValue(QSL(":url"), unnulifyString(message.m_url)); @@ -1153,13 +1158,13 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, query_select_with_url.bindValue(QSL(":account_id"), account_id); qDebugNN << LOGSEC_DB - << "Checking if message with title '" - << message.m_title - << "', url '" - << message.m_url - << "' and author '" - << message.m_author - << "' is present in DB."; + << "Checking if message with title " + << QUOTE_NO_SPACE(message.m_title) + << ", url " + << QUOTE_NO_SPACE(message.m_url) + << "' and author " + << QUOTE_NO_SPACE(message.m_author) + << " is present in DB."; if (query_select_with_url.exec() && query_select_with_url.next()) { id_existing_message = query_select_with_url.value(0).toInt(); @@ -1168,6 +1173,7 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, is_important_existing_message = query_select_with_url.value(3).toBool(); contents_existing_message = query_select_with_url.value(4).toString(); feed_id_existing_message = query_select_with_url.value(5).toString(); + title_existing_message = unnulifyString(message.m_title); qDebugNN << LOGSEC_DB << "Message with these attributes is already present in DB and has DB ID '" @@ -1201,6 +1207,7 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, is_important_existing_message = query_select_with_custom_id.value(3).toBool(); contents_existing_message = query_select_with_custom_id.value(4).toString(); feed_id_existing_message = query_select_with_custom_id.value(5).toString(); + title_existing_message = query_select_with_custom_id.value(6).toString(); qDebugNN << LOGSEC_DB << "Message with custom ID" @@ -1224,23 +1231,32 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, // Message is already in the DB. // // Now, we update it if at least one of next conditions is true: - // 1) Message has custom ID AND (its date OR read status OR starred status are changed or message - // was moved from one feed to another - this can particularly happen in Gmail feeds). + // 1) FOR SYNCHRONIZED SERVICES: Message has custom ID AND (its date OR read status OR starred status are changed + // or message was moved from one feed to another - this can particularly happen in Gmail feeds). // - // 2) Message has its date fetched from feed AND its date is different from date in DB and contents is changed. + // 2) FOR NON-SYNCHRONIZED SERVICES (RSS/ATOM/JSON): Message has custom ID/GUID and its title or contents are changed. // - // 3) Message update is force, we want to overwrite message as some arbitrary atribute was changed, + // 3) FOR ALL SERVICES: Message has its date fetched from feed AND its date is different + // from date in DB and contents is changed. + // + // 4) FOR ALL SERVICES: Message update is forced, we want to overwrite message as some arbitrary atribute was changed, // this particularly happens when manual message filter execution happens. - if (/* 1 */ (!message.m_customId.isEmpty() && (message.m_created.toMSecsSinceEpoch() != date_existing_message || - message.m_isRead != is_read_existing_message || - message.m_isImportant != is_important_existing_message || - message.m_feedId != feed_id_existing_message || - message.m_contents != contents_existing_message)) || + if (/* 1 */ (!message.m_customId.isEmpty() && feed->getParentServiceRoot()->isSyncable() && + (message.m_created.toMSecsSinceEpoch() != date_existing_message || + message.m_isRead != is_read_existing_message || + message.m_isImportant != is_important_existing_message || + message.m_feedId != feed_id_existing_message || + message.m_title != title_existing_message || + message.m_contents != contents_existing_message)) || - /* 2 */ (message.m_createdFromFeed && message.m_created.toMSecsSinceEpoch() != date_existing_message && + /* 2 */ (!message.m_customId.isEmpty() && !feed->getParentServiceRoot()->isSyncable() && + (message.m_title != title_existing_message || + message.m_contents != contents_existing_message)) || + + /* 3 */ (message.m_createdFromFeed && message.m_created.toMSecsSinceEpoch() != date_existing_message && message.m_contents != contents_existing_message) || - /* 3 */ force_update) { + /* 4 */ force_update) { // Message exists, it is changed, update it. query_update.bindValue(QSL(":title"), unnulifyString(message.m_title)); query_update.bindValue(QSL(":is_read"), int(message.m_isRead)); @@ -1251,7 +1267,7 @@ QPair DatabaseQueries::updateMessages(QSqlDatabase db, query_update.bindValue(QSL(":date_created"), message.m_created.toMSecsSinceEpoch()); query_update.bindValue(QSL(":contents"), unnulifyString(message.m_contents)); query_update.bindValue(QSL(":enclosures"), Enclosures::encodeEnclosuresToString(message.m_enclosures)); - query_update.bindValue(QSL(":feed"), unnulifyString(feed_id_existing_message)); + query_update.bindValue(QSL(":feed"), unnulifyString(message.m_feedId)); query_update.bindValue(QSL(":score"), message.m_score); query_update.bindValue(QSL(":id"), id_existing_message); diff --git a/src/librssguard/database/databasequeries.h b/src/librssguard/database/databasequeries.h index ea2b4efce..0a286d22d 100644 --- a/src/librssguard/database/databasequeries.h +++ b/src/librssguard/database/databasequeries.h @@ -113,8 +113,8 @@ class DatabaseQueries { static void createOverwriteAccount(const QSqlDatabase& db, ServiceRoot* account); // Returns counts of updated messages . - static QPair updateMessages(QSqlDatabase db, const QList& messages, const QString& feed_custom_id, - int account_id, const QString& url, bool force_update, bool* ok = nullptr); + static QPair updateMessages(QSqlDatabase db, const QList& messages, Feed* feed, + const QString& url, bool force_update, bool* ok = nullptr); static bool deleteAccount(const QSqlDatabase& db, int account_id); static bool deleteAccountData(const QSqlDatabase& db, int account_id, bool delete_messages_too); static bool cleanLabelledMessages(const QSqlDatabase& db, bool clean_read_only, Label* label); diff --git a/src/librssguard/services/abstract/feed.cpp b/src/librssguard/services/abstract/feed.cpp index 8c4b393ce..dfab22c66 100755 --- a/src/librssguard/services/abstract/feed.cpp +++ b/src/librssguard/services/abstract/feed.cpp @@ -205,14 +205,12 @@ QPair Feed::updateMessages(const QList& messages, bool force_ << QUOTE_W_SPACE_DOT(is_main_thread); bool ok = false; - QString custom_id = customId(); - int account_id = getParentServiceRoot()->accountId(); QSqlDatabase database = is_main_thread ? qApp->database()->driver()->connection(metaObject()->className()) : qApp->database()->driver()->connection(QSL("feed_upd")); updated_messages = DatabaseQueries::updateMessages(database, messages, - custom_id, account_id, + this, source(), force_update, &ok); diff --git a/src/librssguard/services/standard/atomparser.cpp b/src/librssguard/services/standard/atomparser.cpp index 20bb05049..acab10013 100755 --- a/src/librssguard/services/standard/atomparser.cpp +++ b/src/librssguard/services/standard/atomparser.cpp @@ -68,6 +68,7 @@ Message AtomParser::extractMessage(const QDomElement& msg_element, QDateTime cur new_message.m_title = qApp->web()->unescapeHtml(qApp->web()->stripTags(title)); new_message.m_contents = summary; new_message.m_author = qApp->web()->unescapeHtml(messageAuthor(msg_element)); + new_message.m_customId = msg_element.elementsByTagNameNS(m_atomNamespace, QSL("id")).at(0).toElement().text(); QString raw_contents; QTextStream str(&raw_contents); diff --git a/src/librssguard/services/standard/rssparser.cpp b/src/librssguard/services/standard/rssparser.cpp index 504e8b329..f7089f5e0 100644 --- a/src/librssguard/services/standard/rssparser.cpp +++ b/src/librssguard/services/standard/rssparser.cpp @@ -34,6 +34,7 @@ Message RssParser::extractMessage(const QDomElement& msg_element, QDateTime curr QString elem_enclosure = msg_element.namedItem(QSL("enclosure")).toElement().attribute(QSL("url")); QString elem_enclosure_type = msg_element.namedItem(QSL("enclosure")).toElement().attribute(QSL("type")); + new_message.m_customId = msg_element.namedItem(QSL("guid")).toElement().text(); new_message.m_url = msg_element.namedItem(QSL("link")).toElement().text(); if (new_message.m_url.isEmpty() && !new_message.m_enclosures.isEmpty()) {