Merge branch 'saved-queries': saved queries in a file of their own

Items 23 and 82. Saved queries move out of the [queries] INI section into
~/.config/qtmaildir/queries.json, gaining the three things the INI could not
express: an order, a pinned flag, and a per-query account scope. They can now be
created, edited, reordered, unpinned and deleted from the UI rather than only by
hand-editing a config file.

The INI could not carry order at all: QSettings reads a section through
childKeys(), which sorts alphabetically and never follows the file, so the
buttons could not be arranged. Migration reads the old section once, marks every
entry pinned so nothing moves on the first launch, and leaves the config file
byte-identical, since rewriting it with QSettings would drop the user's comments
and key order.

Sent stops being a hardcoded button and becomes an ordinary entry carrying
"generated": "sent". Its query is still composed from the accounts every time
it runs, so correcting a folder name still needs no edit here, but the row now
follows one rule instead of having one member the user could not move, rename or
remove.

Item 82 was found by hand-testing item 23 and fixed on the same branch: saving
worked and nothing else did, so a saved query could be created and never
changed. It also uncovered a defect that predated it, where rebuilding the row
with deleteLater() left the stale row answering findChild(), which was already
reachable from the save path.

Hand-tested throughout: the migration against the real config, saving, unpinning,
deleting, reordering, the read-only query field on a generated entry, and an
unpin surviving a restart.

1 files changed, 244 insertions, 11 deletions

diff --git a/src/config.cpp b/src/config.cpp
index b1f730c..ae26555 100644
--- a/src/config.cpp
+++ b/src/config.cpp
@@ -24,8 +24,14 @@
 #include "messageview.h"
 
 #include <QDateTime>
+#include <QDir>
+#include <QFile>
 #include <QFileInfo>
+#include <QJsonArray>
+#include <QJsonDocument>
+#include <QJsonObject>
 #include <QLocale>
+#include <QSaveFile>
 #include <QSettings>
 #include <QStandardPaths>
 
@@ -37,6 +43,22 @@ namespace {
 constexpr int kMinToolbarIconSize = 16;
 constexpr int kMaxToolbarIconSize = 64;
 
+/// queries.json format version. Bump only for a BREAKING change: an optional
+/// field needs no bump, because an older build preserves what it does not
+/// understand rather than dropping it.
+///
+/// Unlike rules.json this file has ONE implementation, so a bump here is not a
+/// two-repo change and no hook stops tagging if it is half-deployed.
+constexpr int kQueriesFormatVersion = 1;
+
+/// Generators a saved query may name in its `generated` field.
+///
+/// A closed set, checked on load so a typo is reported rather than producing a
+/// button that silently finds nothing. Adding one here needs no format bump:
+/// an older build keeps the row and reports it, which is why an unknown
+/// generator is a problem rather than a reason to drop the entry.
+const QStringList kQueryGenerators = { QStringLiteral("sent") };
+
 } // namespace
 
 QString Account::scopedQuery(const QString &query) const
@@ -397,17 +419,10 @@ void Config::load(const QString &path)
         m_accounts.append(account);
     }
 
-    settings.beginGroup(QStringLiteral("queries"));
-    // QSettings::childKeys() returns keys in alphabetical order, not file
-    // order, so the saved-query button order in the UI is alphabetical too.
-    // A hand-rolled parser would be needed to preserve file order; not
-    // needed in v1.
-    for (const QString &name : settings.childKeys())
-        m_savedQueries.append({ name, settings.value(name).toString() });
-    settings.endGroup();
-
-    // Checked here rather than where startup_query is read: [queries] is not
-    // parsed until now. Only a name the user actually wrote is worth a
+    loadSavedQueries(path, settings);
+
+    // Checked here rather than where startup_query is read: the saved queries
+    // are not parsed until now. Only a name the user actually wrote is worth a
     // problem; the built-in default naming a query they never created is not
     // something they got wrong.
     if (m_startupQueryWasSet && !m_savedQueries.isEmpty()
@@ -419,6 +434,224 @@ void Config::load(const QString &path)
     }
 }
 
+QString Config::queriesPath(const QString &configPath)
+{
+    return QFileInfo(configPath).absolutePath()
+           + QStringLiteral("/queries.json");
+}
+
+void Config::loadSavedQueries(const QString &configPath, QSettings &settings)
+{
+    m_queriesPath = queriesPath(configPath);
+
+    QFile file(m_queriesPath);
+    if (!file.exists()) {
+        // Migration. Read [queries] once, write the JSON, and leave the INI
+        // section alone: stripping it would mean rewriting a hand-edited file
+        // with QSettings, which drops comments and key order across the WHOLE
+        // file. A few stale lines the user can delete by hand is the cheaper
+        // loss, and it keeps a downgrade working.
+        settings.beginGroup(QStringLiteral("queries"));
+        const QStringList names = settings.childKeys();
+        for (const QString &name : names) {
+            SavedQuery query;
+            query.name = name;
+            query.query = settings.value(name).toString();
+            // Pinned, because these are buttons today. A migration that left
+            // them unpinned would empty the query row on the first launch
+            // after an upgrade, which reads as data loss.
+            query.pinned = true;
+            m_savedQueries.append(query);
+        }
+        settings.endGroup();
+
+        // Sent was a hardcoded button beside the saved queries and becomes an
+        // ordinary row here, so it can be reordered, renamed, unpinned or
+        // removed like any other. It stays GENERATED, so it still follows the
+        // accounts. Appended last, where the button already sat.
+        //
+        // Only when an account actually configures a sent folder: the button
+        // was hidden entirely otherwise, and migrating a row that always finds
+        // nothing would be worse than what it replaces.
+        if (!allSentQuery().isEmpty()) {
+            SavedQuery sent;
+            sent.name = QStringLiteral("Sent");
+            sent.generated = QStringLiteral("sent");
+            sent.pinned = true;
+            sent.flat = true;
+            m_savedQueries.append(sent);
+        }
+
+        // Order is alphabetical here because childKeys() is genuinely all the
+        // INI knows. The user reorders once and it sticks from then on.
+        if (!m_savedQueries.isEmpty() && !saveSavedQueries()) {
+            addProblem(QStringLiteral("Could not write saved queries to %1.")
+                           .arg(m_queriesPath));
+        }
+        return;
+    }
+
+    if (!file.open(QIODevice::ReadOnly)) {
+        addProblem(QStringLiteral("Could not read %1: %2.")
+                       .arg(m_queriesPath, file.errorString()));
+        m_queriesRefused = true;
+        return;
+    }
+
+    QJsonParseError error;
+    const QJsonDocument document =
+        QJsonDocument::fromJson(file.readAll(), &error);
+    file.close();
+
+    if (error.error != QJsonParseError::NoError || !document.isObject()) {
+        addProblem(QStringLiteral("%1 is not valid JSON: %2.")
+                       .arg(m_queriesPath, error.errorString()));
+        m_queriesRefused = true;
+        return;
+    }
+
+    const QJsonObject root = document.object();
+    const int version =
+        root.value(QStringLiteral("version")).toInt(kQueriesFormatVersion);
+    if (version != kQueriesFormatVersion) {
+        // Refused rather than guessed at, and the refusal blocks the save:
+        // rewriting a newer document with this build's reading of it would
+        // destroy whatever the newer build stored.
+        addProblem(QStringLiteral("%1 has format version %2; this build "
+                                  "understands %3. Saved queries were not "
+                                  "loaded.")
+                       .arg(m_queriesPath)
+                       .arg(version)
+                       .arg(kQueriesFormatVersion));
+        m_queriesRefused = true;
+        return;
+    }
+
+    for (auto it = root.begin(); it != root.end(); ++it) {
+        if (it.key() != QStringLiteral("version")
+            && it.key() != QStringLiteral("queries"))
+            m_queriesUnknown.insert(it.key(), it.value());
+    }
+
+    const QJsonArray array = root.value(QStringLiteral("queries")).toArray();
+    for (const QJsonValue &value : array) {
+        const QJsonObject object = value.toObject();
+        SavedQuery query;
+        query.name = object.value(QStringLiteral("name")).toString();
+        query.query = object.value(QStringLiteral("query")).toString();
+        query.pinned = object.value(QStringLiteral("pinned")).toBool(false);
+        query.account = object.value(QStringLiteral("account")).toString();
+        query.generated = object.value(QStringLiteral("generated")).toString();
+        // A generator carries its own view mode, so "sent" is flat whether or
+        // not the file says so. Storing it as a plain field would let a
+        // hand-edited or migrated-from-elsewhere row produce a THREADED sent
+        // view, which folds every reply back into the conversation the user
+        // sent one message into. The file may still set it for an ordinary
+        // query.
+        query.flat = object.value(QStringLiteral("flat")).toBool(false)
+                     || query.generated == QStringLiteral("sent");
+
+        if (query.isGenerated()
+            && !kQueryGenerators.contains(query.generated)) {
+            // Reported but KEPT. A later build may know this generator, and
+            // dropping the row here would delete it from the file on the next
+            // save, which is the same data loss the unknown-field handling
+            // exists to prevent.
+            addProblem(QStringLiteral("Saved query '%1' uses an unknown "
+                                      "generator '%2' and will find nothing.")
+                           .arg(query.name, query.generated));
+        }
+
+        if (query.name.isEmpty()) {
+            addProblem(QStringLiteral("A saved query in %1 has no name and was "
+                                      "skipped.").arg(m_queriesPath));
+            continue;
+        }
+
+        for (auto it = object.begin(); it != object.end(); ++it) {
+            static const QStringList known = {
+                QStringLiteral("name"), QStringLiteral("query"),
+                QStringLiteral("pinned"), QStringLiteral("account"),
+                QStringLiteral("generated"), QStringLiteral("flat")
+            };
+            if (!known.contains(it.key()))
+                query.unknown.insert(it.key(), it.value());
+        }
+
+        m_savedQueries.append(query);
+    }
+}
+
+bool Config::saveSavedQueries() const
+{
+    if (m_queriesPath.isEmpty() || m_queriesRefused)
+        return false;
+
+    QJsonArray array;
+    for (const SavedQuery &query : m_savedQueries) {
+        // Only what carries information. The file is hand-editable, so a key
+        // that always holds the same value, or one the generator already
+        // implies, is just something the reader has to skip past. Same reason
+        // `pinned` and `account` are written only when set.
+        QJsonObject object;
+        object.insert(QStringLiteral("name"), query.name);
+        if (query.isGenerated()) {
+            object.insert(QStringLiteral("generated"), query.generated);
+        } else {
+            object.insert(QStringLiteral("query"), query.query);
+        }
+        if (query.pinned)
+            object.insert(QStringLiteral("pinned"), true);
+        if (!query.account.isEmpty())
+            object.insert(QStringLiteral("account"), query.account);
+        // Skipped when the generator already implies it, which loadSavedQueries
+        // reapplies on the way back in.
+        if (query.flat && query.generated != QStringLiteral("sent"))
+            object.insert(QStringLiteral("flat"), true);
+        for (auto it = query.unknown.begin(); it != query.unknown.end(); ++it)
+            object.insert(it.key(), it.value());
+        array.append(object);
+    }
+
+    QJsonObject root = m_queriesUnknown;
+    root.insert(QStringLiteral("version"), kQueriesFormatVersion);
+    root.insert(QStringLiteral("queries"), array);
+
+    QDir().mkpath(QFileInfo(m_queriesPath).absolutePath());
+
+    // QSaveFile writes a temporary and renames on commit, so an interrupted
+    // write cannot leave a half-written file where the queries used to be.
+    QSaveFile file(m_queriesPath);
+    if (!file.open(QIODevice::WriteOnly))
+        return false;
+    file.write(QJsonDocument(root).toJson(QJsonDocument::Indented));
+    return file.commit();
+}
+
+QString Config::resolvedQuery(const SavedQuery &query) const
+{
+    // Composed from the accounts every time it is asked for, which is the
+    // point: the answer follows the config rather than a copy of it taken when
+    // the entry was written.
+    if (query.isGenerated()) {
+        if (query.generated == QStringLiteral("sent"))
+            return allSentQuery();
+        // An unknown generator was reported on load. Empty rather than the
+        // bare stored query, which for a generated entry is empty anyway and
+        // would otherwise run as "match everything".
+        return QString();
+    }
+
+    if (query.account.isEmpty())
+        return query.query;
+
+    const Account scope = account(query.account);
+    if (!scope.isValid())
+        return query.query;
+
+    return scope.scopedQuery(query.query);
+}
+
 SavedQuery Config::startupSavedQuery() const
 {
     if (m_savedQueries.isEmpty())