[pim/akonadi] /: ETMViewStateSaver: add stable remote-path key format

Git commit 3b4023ceb3e6b421cd259a7e77a489779f3a2184 by David Faure.
Committed on 03/08/2026 at 11:22.
Pushed by carlschwan into branch 'master'.

ETMViewStateSaver: add stable remote-path key format

The selection (e.g. korganizer's checked calendars) was keyed by numeric
collection id. That id changes when a resource re-lists its collections from
scratch (seen with a DAV resource after network trouble), so the selection was
lost even though the same remote calendars came back.

Add an opt-in RemotePathKeys format that keys collections by resource id plus
the chain of remoteIds, which survives id renumbering. Reading understands both
formats, so an existing config migrates to the new one on the next save.

In this format saveState() also keeps stored path keys whose collections are
currently absent from the model, so saving on shutdown (before the asynchronous
restore has populated the model) no longer wipes the selection.

M  +1    -0    autotests/libs/CMakeLists.txt
A  +286  -0    autotests/libs/etmviewstatesavertest.cpp     [License: LGPL(v2.0+)]
M  +116  -0    src/widgets/etmviewstatesaver.cpp
M  +42   -0    src/widgets/etmviewstatesaver.h

https://invent.kde.org/pim/akonadi/-/commit/3b4023ceb3e6b421cd259a7e77a489779f3a2184

diff --git a/autotests/libs/CMakeLists.txt b/autotests/libs/CMakeLists.txt
index 297b51c56..b1a288749 100644
--- a/autotests/libs/CMakeLists.txt
+++ b/autotests/libs/CMakeLists.txt
@@ -67,6 +67,7 @@ target_sources(
 )
 target_link_libraries(actionstatemanagertest KF6::XmlGui)
 add_akonadi_test_widgets(conflictresolvedialogtest.cpp conflictresolvedialogtest.h)
+add_akonadi_test_widgets(etmviewstatesavertest.cpp)
 add_akonadi_test(tagmodeltest.cpp)
 add_akonadi_test(statisticsproxymodeltest.cpp)
 
diff --git a/autotests/libs/etmviewstatesavertest.cpp b/autotests/libs/etmviewstatesavertest.cpp
new file mode 100644
index 000000000..fb0f059e2
--- /dev/null
+++ b/autotests/libs/etmviewstatesavertest.cpp
@@ -0,0 +1,286 @@
+/*
+    SPDX-FileCopyrightText: 2026 David Faure <[email protected]>
+
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include "etmviewstatesaver.h"
+#include "entitytreemodel.h"
+
+#include <KConfig>
+#include <KConfigGroup>
+
+#include <QItemSelectionModel>
+#include <QStandardItemModel>
+#include <QTemporaryDir>
+#include <QTest>
+
+using namespace Akonadi;
+
+// Exposes the protected serialization methods for testing.
+class TestStateSaver : public ETMViewStateSaver
+{
+public:
+    using ETMViewStateSaver::indexFromConfigString;
+    using ETMViewStateSaver::indexToConfigString;
+};
+
+class ETMViewStateSaverTest : public QObject
+{
+    Q_OBJECT
+
+private:
+    // Builds a QStandardItem carrying a Collection in EntityTreeModel::CollectionRole.
+    static QStandardItem *collectionItem(Collection::Id id, const QString &resource, const QString &remoteId)
+    {
+        Collection col(id);
+        col.setResource(resource);
+        col.setRemoteId(remoteId);
+        auto *item = new QStandardItem(remoteId.isEmpty() ? resource : remoteId);
+        item->setData(QVariant::fromValue(col), EntityTreeModel::CollectionRole);
+        return item;
+    }
+
+private Q_SLOTS:
+    void shouldWriteStableRemotePathKey()
+    {
+        QStandardItemModel model;
+        // res1 root -> calendar
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *calendar = collectionItem(2, QStringLiteral("res1"), QStringLiteral("https://s/cal/work/"));
+        root->appendRow(calendar);
+        model.appendRow(root);
+
+        TestStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+
+        // The '/' inside the remoteId is escaped; the resource-root remoteId is not part of the key.
+        QCOMPARE(saver.indexToConfigString(calendar->index()), QStringLiteral("rres1/https:%2F%2Fs%2Fcal%2Fwork%2F"));
+    }
+
+    void shouldRoundTripStableRemotePathKey()
+    {
+        QStandardItemModel model;
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *work = collectionItem(2, QStringLiteral("res1"), QStringLiteral("work"));
+        auto *home = collectionItem(3, QStringLiteral("res1"), QStringLiteral("home"));
+        root->appendRow(work);
+        root->appendRow(home);
+        model.appendRow(root);
+
+        TestStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+
+        const QString key = saver.indexToConfigString(home->index());
+        QCOMPARE(key, QStringLiteral("rres1/home"));
+        QCOMPARE(saver.indexFromConfigString(&model, key), home->index());
+    }
+
+    void shouldKeySingleCollectionResourceByResourceAlone()
+    {
+        // A resource whose only collection is the root itself (e.g. a single-file iCal calendar).
+        QStandardItemModel model;
+        auto *root = collectionItem(5, QStringLiteral("res2"), QStringLiteral("res2root"));
+        model.appendRow(root);
+
+        TestStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+
+        const QString key = saver.indexToConfigString(root->index());
+        QCOMPARE(key, QStringLiteral("rres2/"));
+        QCOMPARE(saver.indexFromConfigString(&model, key), root->index());
+    }
+
+    void shouldFallBackToIdKeyWhenRemoteIdMissing()
+    {
+        // A pathless collection (empty remoteId) has no stable path, so the id key is used.
+        QStandardItemModel model;
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *pathless = collectionItem(7, QStringLiteral("res1"), QString());
+        root->appendRow(pathless);
+        model.appendRow(root);
+
+        TestStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+
+        QCOMPARE(saver.indexToConfigString(pathless->index()), QStringLiteral("c7"));
+    }
+
+    void shouldNotResolveUnknownStableKey()
+    {
+        QStandardItemModel model;
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        model.appendRow(root);
+
+        TestStateSaver saver;
+        QVERIFY(!saver.indexFromConfigString(&model, QStringLiteral("rres1/gone")).isValid());
+    }
+
+    void shouldSaveSelectionAsRemotePathKeys()
+    {
+        // Drives the real public API: a checked calendar must land in the config as an "r" key.
+        QStandardItemModel model;
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *work = collectionItem(2, QStringLiteral("res1"), QStringLiteral("https://s/cal/work/"));
+        root->appendRow(work);
+        model.appendRow(root);
+
+        QItemSelectionModel selectionModel(&model);
+        selectionModel.select(work->index(), QItemSelectionModel::Select);
+
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        KConfig config(dir.filePath(QStringLiteral("testrc")));
+        KConfigGroup group = config.group(QStringLiteral("GlobalCollectionSelection"));
+
+        ETMViewStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+        saver.setSelectionModel(&selectionModel);
+        saver.saveState(group);
+
+        QCOMPARE(group.readEntry("Selection", QStringList()), QStringList{QStringLiteral("rres1/https:%2F%2Fs%2Fcal%2Fwork%2F")});
+    }
+
+    void shouldRestoreSelectionFromRemotePathKeys()
+    {
+        // The other half of the migration: an already-migrated config must check the same calendar again,
+        // even though the collection ids are completely different from the ones saved earlier.
+        QStandardItemModel model;
+        auto *root = collectionItem(101, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *work = collectionItem(102, QStringLiteral("res1"), QStringLiteral("https://s/cal/work/"));
+        auto *home = collectionItem(103, QStringLiteral("res1"), QStringLiteral("https://s/cal/home/"));
+        root->appendRow(work);
+        root->appendRow(home);
+        model.appendRow(root);
+
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        KConfig config(dir.filePath(QStringLiteral("testrc")));
+        KConfigGroup group = config.group(QStringLiteral("GlobalCollectionSelection"));
+        group.writeEntry("Selection", QStringList{QStringLiteral("rres1/https:%2F%2Fs%2Fcal%2Fwork%2F")});
+
+        QItemSelectionModel selectionModel(&model);
+        ETMViewStateSaver saver;
+        saver.setSelectionModel(&selectionModel);
+        saver.restoreState(group);
+
+        QCOMPARE(selectionModel.selectedIndexes(), QModelIndexList{work->index()});
+    }
+
+    // Reproduces the shutdown race: KOrganizer's queryClose() saves unconditionally, but the
+    // restore is asynchronous. If the collections are not in the model yet, the selection model
+    // is still empty and saving overwrites the stored keys with nothing.
+    void shouldNotLoseUnresolvedKeysOnSave()
+    {
+        QStandardItemModel model; // deliberately empty: the resource has not populated yet
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        KConfig config(dir.filePath(QStringLiteral("testrc")));
+        KConfigGroup group = config.group(QStringLiteral("GlobalCollectionSelection"));
+        const QStringList stored{QStringLiteral("rres1/https:%2F%2Fs%2Fcal%2Fwork%2F")};
+        group.writeEntry("Selection", stored);
+
+        QItemSelectionModel selectionModel(&model);
+        ETMViewStateSaver restorer;
+        restorer.setSelectionModel(&selectionModel);
+        restorer.restoreState(group); // key stays pending, nothing gets selected
+        QVERIFY(selectionModel.selectedIndexes().isEmpty());
+
+        // Now the user quits before the model populated.
+        ETMViewStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+        saver.setSelectionModel(&selectionModel);
+        saver.saveState(group);
+
+        QCOMPARE(group.readEntry("Selection", QStringList()), stored);
+    }
+
+    void shouldStillDropDeselectedKeys()
+    {
+        // The safety net must not resurrect a calendar the user deliberately unchecked:
+        // its collection *is* in the model, it is just no longer selected.
+        QStandardItemModel model;
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *work = collectionItem(2, QStringLiteral("res1"), QStringLiteral("work"));
+        root->appendRow(work);
+        model.appendRow(root);
+
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        KConfig config(dir.filePath(QStringLiteral("testrc")));
+        KConfigGroup group = config.group(QStringLiteral("GlobalCollectionSelection"));
+        group.writeEntry("Selection", QStringList{QStringLiteral("rres1/work")});
+
+        QItemSelectionModel selectionModel(&model); // nothing selected: the user unchecked it
+        ETMViewStateSaver saver;
+        saver.setKeyFormat(ETMViewStateSaver::RemotePathKeys);
+        saver.setSelectionModel(&selectionModel);
+        saver.saveState(group);
+
+        QVERIFY(group.readEntry("Selection", QStringList()).isEmpty());
+    }
+
+    void shouldNotKeepMissingCollectionsForIdKeys()
+    {
+        // In the default id format, saveState() must behave exactly like the base class: a stored
+        // key whose collection is missing is dropped, because a stale id can resolve to a different
+        // collection later.
+        QStandardItemModel model; // empty: the stored collection is not present
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        KConfig config(dir.filePath(QStringLiteral("testrc")));
+        KConfigGroup group = config.group(QStringLiteral("GlobalCollectionSelection"));
+        group.writeEntry("Selection", QStringList{QStringLiteral("c5")});
+
+        QItemSelectionModel selectionModel(&model);
+        ETMViewStateSaver saver; // default IdKeys
+        saver.setSelectionModel(&selectionModel);
+        saver.saveState(group);
+
+        QVERIFY(group.readEntry("Selection", QStringList()).isEmpty());
+    }
+
+    void shouldSurviveKConfigRoundTrip()
+    {
+        // KConfigViewStateSaver stores the keys as a QStringList under "Selection".
+        // The stable keys contain '/', '\' and can contain ',', so make sure KConfig's
+        // escaping gives them back unchanged.
+        const QStringList keys{
+            QStringLiteral("rres1/https:%2F%2Fs%2Fcal%2Fwork%2F"),
+            QStringLiteral("rres1/with,comma"),
+            QStringLiteral("rres2/"),
+            QStringLiteral("c7"), // pathless fallback entries live alongside
+        };
+
+        QTemporaryDir dir;
+        QVERIFY(dir.isValid());
+        const QString path = dir.filePath(QStringLiteral("testrc"));
+        {
+            KConfig config(path);
+            KConfigGroup group = config.group(QStringLiteral("GlobalCollectionSelection"));
+            group.writeEntry("Selection", keys);
+            config.sync();
+        }
+
+        KConfig reread(path);
+        const KConfigGroup group = reread.group(QStringLiteral("GlobalCollectionSelection"));
+        QCOMPARE(group.readEntry("Selection", QStringList()), keys);
+    }
+
+    void shouldWriteIdKeyByDefault()
+    {
+        // Default format is unchanged, so other consumers keep the legacy behavior.
+        QStandardItemModel model;
+        auto *root = collectionItem(1, QStringLiteral("res1"), QStringLiteral("res1root"));
+        auto *calendar = collectionItem(2, QStringLiteral("res1"), QStringLiteral("work"));
+        root->appendRow(calendar);
+        model.appendRow(root);
+
+        TestStateSaver saver;
+        QCOMPARE(saver.indexToConfigString(calendar->index()), QStringLiteral("c2"));
+    }
+};
+
+QTEST_MAIN(ETMViewStateSaverTest)
+
+#include "etmviewstatesavertest.moc"
diff --git a/src/widgets/etmviewstatesaver.cpp b/src/widgets/etmviewstatesaver.cpp
index aa5813638..66deb9c55 100644
--- a/src/widgets/etmviewstatesaver.cpp
+++ b/src/widgets/etmviewstatesaver.cpp
@@ -8,6 +8,9 @@
 
 #include "etmviewstatesaver.h"
 
+#include <KConfigGroup>
+
+#include <QItemSelectionModel>
 #include <QModelIndex>
 
 #include "entitytreemodel.h"
@@ -19,8 +22,77 @@ ETMViewStateSaver::ETMViewStateSaver(QObject *parent)
 {
 }
 
+void ETMViewStateSaver::setKeyFormat(KeyFormat format)
+{
+    mKeyFormat = format;
+}
+
+ETMViewStateSaver::KeyFormat ETMViewStateSaver::keyFormat() const
+{
+    return mKeyFormat;
+}
+
+// The remoteId chain is joined with '/', so percent-encode any literal '/' inside a remoteId
+// (and '%' itself, to keep the encoding reversible). Backslash escaping would be more in line
+// with Akonadi::CollectionPathResolver, but KConfig escapes backslashes twice over, which turns
+// a DAV url into an unreadable wall of '\\\\/'.
+static QString escapeSegment(const QString &segment)
+{
+    QString escaped = segment;
+    escaped.replace(u'%', QStringLiteral("%25"));
+    escaped.replace(u'/', QStringLiteral("%2F"));
+    return escaped;
+}
+
+QString ETMViewStateSaver::stableKeyForIndex(const QModelIndex &index) const
+{
+    const auto collection = index.data(EntityTreeModel::CollectionRole).value<Collection>();
+    if (!collection.isValid()) {
+        return QString(); // an item, not a collection
+    }
+    const QString resource = collection.resource();
+    if (resource.isEmpty()) {
+        return QString(); // can't anchor to a resource
+    }
+
+    // Collect remoteIds from the collection up to, but not including, the
+    // resource-root collection (the top-level row, whose parent index is invalid).
+    // resource() already identifies that root, so its own remoteId is redundant.
+    QStringList segments;
+    for (QModelIndex idx = index; idx.parent().isValid(); idx = idx.parent()) {
+        const auto col = idx.data(EntityTreeModel::CollectionRole).value<Collection>();
+        if (col.remoteId().isEmpty()) {
+            return QString(); // pathless node -> caller falls back to the id key
+        }
+        segments.prepend(escapeSegment(col.remoteId()));
+    }
+    // A resource with a single collection (the root itself) yields an empty path,
+    // which still uniquely denotes that root via the resource anchor.
+    return QStringLiteral("r%1/%2").arg(escapeSegment(resource), segments.join(u'/'));
+}
+
+QModelIndex ETMViewStateSaver::indexForStableKey(const QAbstractItemModel *model, const QString &key, const QModelIndex &parent) const
+{
+    const int rows = model->rowCount(parent);
+    for (int row = 0; row < rows; ++row) {
+        const QModelIndex idx = model->index(row, 0, parent);
+        if (stableKeyForIndex(idx) == key) {
+            return idx;
+        }
+        const QModelIndex child = indexForStableKey(model, key, idx);
+        if (child.isValid()) {
+            return child;
+        }
+    }
+    return QModelIndex();
+}
+
 QModelIndex ETMViewStateSaver::indexFromConfigString(const QAbstractItemModel *model, const QString &key) const
 {
+    if (key.startsWith(u'r')) {
+        return indexForStableKey(model, key, QModelIndex());
+    }
+
     if (key.startsWith(u'x')) {
         return QModelIndex();
     }
@@ -53,6 +125,13 @@ QString ETMViewStateSaver::indexToConfigString(const QModelIndex &index) const
     }
     const auto c = index.data(EntityTreeModel::CollectionRole).value<Collection>();
     if (c.isValid()) {
+        if (mKeyFormat == RemotePathKeys) {
+            const QString key = stableKeyForIndex(index);
+            if (!key.isEmpty()) {
+                return key;
+            }
+            // No usable remote path (e.g. a search/virtual collection): fall back to the id key.
+        }
         return QStringLiteral("c%1").arg(c.id());
     }
     auto id = index.data(EntityTreeModel::ItemIdRole).value<Item::Id>();
@@ -62,6 +141,43 @@ QString ETMViewStateSaver::indexToConfigString(const QModelIndex &index) const
     return QString();
 }
 
+void ETMViewStateSaver::saveState(KConfigGroup &configGroup)
+{
+    // Same key KConfigViewStateSaver stores the selection under.
+    static const char selectionKey[] = "Selection";
+
+    // Only path keys benefit from (and are safe for) keeping missing collections, so id-based
+    // users get the base behavior unchanged.
+    if (mKeyFormat != RemotePathKeys || !selectionModel()) {
+        KConfigViewStateSaver::saveState(configGroup);
+        return;
+    }
+
+    const QStringList previous = configGroup.readEntry(selectionKey, QStringList());
+    KConfigViewStateSaver::saveState(configGroup);
+    if (previous.isEmpty()) {
+        return;
+    }
+
+    // Keep stable path keys whose collections are currently missing from the model: restoring is
+    // asynchronous, so saving on shutdown before the model is populated would otherwise drop them.
+    const QAbstractItemModel *model = selectionModel()->model();
+    QStringList keys = configGroup.readEntry(selectionKey, QStringList());
+    for (const QString &key : previous) {
+        if (!key.startsWith(u'r')) {
+            continue;
+        }
+        if (keys.contains(key)) {
+            continue;
+        }
+        if (indexFromConfigString(model, key).isValid()) {
+            continue; // the collection is there, so the user really unchecked it
+        }
+        keys.append(key);
+    }
+    configGroup.writeEntry(selectionKey, keys);
+}
+
 void ETMViewStateSaver::selectCollections(const Akonadi::Collection::List &list)
 {
     QStringList colStrings;
diff --git a/src/widgets/etmviewstatesaver.h b/src/widgets/etmviewstatesaver.h
index 305799f86..32e3a3263 100644
--- a/src/widgets/etmviewstatesaver.h
+++ b/src/widgets/etmviewstatesaver.h
@@ -33,6 +33,39 @@ public:
      */
     explicit ETMViewStateSaver(QObject *parent = nullptr);
 
+    /*!
+     * How checked/selected collections are keyed in the config.
+     */
+    enum KeyFormat {
+        IdKeys, ///< By numeric collection id ("c<id>"). Default. Not stable if a resource re-lists collections with new ids.
+        RemotePathKeys, ///< By resource identifier + chain of remoteIds ("r<resource>/<rid>/..."). Stable across id renumbering.
+    };
+
+    /*!
+     * Sets how checked/selected collections are written to the config.
+     *
+     * Only affects saving; reading always understands both formats, so a config
+     * written with \c IdKeys migrates automatically to \c RemotePathKeys on the
+     * next save. \a format The key format to write.
+     */
+    void setKeyFormat(KeyFormat format);
+    /*!
+     * Returns the key format used when saving.
+     */
+    [[nodiscard]] KeyFormat keyFormat() const;
+
+    /*!
+     * Reimplemented to keep stable (\c RemotePathKeys) entries of the existing configuration
+     * whose collections are currently missing from the model.
+     *
+     * Restoring is asynchronous, so an application that saves on shutdown would otherwise
+     * overwrite the stored selection with an empty one when it quits before the model is
+     * populated. Kept entries are limited to path keys; a missing id key is still dropped,
+     * because a stale id can later resolve to a different collection. \a configGroup The group
+     * to save to.
+     */
+    void saveState(KConfigGroup &configGroup);
+
     /*!
      * Selects the given collections in the view.
      * \a list The list of collections to select.
@@ -69,6 +102,15 @@ protected:
     /* reimp */
     QModelIndex indexFromConfigString(const QAbstractItemModel *model, const QString &key) const override;
     QString indexToConfigString(const QModelIndex &index) const override;
+
+private:
+    /// Builds the stable "r<resource>/<rid>/..." key for a collection index, or an empty string
+    /// if it has no usable remote path (item, or a collection with an empty remoteId anywhere in the chain).
+    [[nodiscard]] QString stableKeyForIndex(const QModelIndex &index) const;
+    /// Depth-first search for the collection index whose stableKeyForIndex() equals \a key.
+    [[nodiscard]] QModelIndex indexForStableKey(const QAbstractItemModel *model, const QString &key, const QModelIndex &parent) const;
+
+    KeyFormat mKeyFormat = IdKeys;
 };
 
 }