From 9df4bd122c73f40945e7c066526767c3da3a1df9 Mon Sep 17 00:00:00 2001 From: spaced4ndy <8711996+spaced4ndy@users.noreply.github.com> Date: Mon, 13 Jul 2026 17:11:13 +0400 Subject: [PATCH] wip --- .../2026-07-10-preferences-forward-compat.md | 123 +++--------------- 1 file changed, 17 insertions(+), 106 deletions(-) diff --git a/plans/2026-07-10-preferences-forward-compat.md b/plans/2026-07-10-preferences-forward-compat.md index 38dbd8e073..5b72c4a4d3 100644 --- a/plans/2026-07-10-preferences-forward-compat.md +++ b/plans/2026-07-10-preferences-forward-compat.md @@ -2,121 +2,32 @@ ## Problem -Adding a new preference — a new field in `Preferences` (direct contact preferences) or `GroupPreferences` (group/channel preferences) — currently requires a **two-stage release**: - -1. First ship a version that can *parse and retain* the new preference field (dormant, not settable in the UI). -2. Only in a later release enable setting it in the UI. - -Without this staging, when an owner on a new client sets the new preference, every client still on the previous version silently drops the unknown field on parse and, on its next re-store or re-broadcast, wipes the value for everyone. The preference is then "lost forever" — the only recovery is for the owner to toggle it again after the whole membership has upgraded, which is impossible to coordinate for a group or channel owner. - -We want to eliminate the per-preference two-stage tax: a client on version *N* should transparently preserve preference fields introduced in version *N+1* that it does not yet understand. +Adding a preference to `Preferences` (direct) or `GroupPreferences` (group) needs a two-stage release: first ship a version that parses+keeps the field (dormant), then a later version that lets the UI set it. Otherwise, when a new client sets the pref, every client on the previous version drops the unknown key on parse and wipes it for everyone on its next store/re-send. Recovery requires re-toggling after the whole membership upgrades — impossible for a group/channel owner to coordinate. ## Root cause -`Preferences` and `GroupPreferences` are serialized with aeson `deriveJSON defaultJSON`: +`Preferences` (`Types/Preferences.hs:153`) and `GroupPreferences` (`:331`) use `deriveJSON defaultJSON` (`:1144`, `:1186`), which **silently drops unknown JSON keys on parse**. Both storage (the `preferences` TEXT columns, via `ToField`/`FromField = encodeJSON`/`decodeJSON`) and the wire (`Profile`/`GroupProfile` JSON delegate to these instances) reconstruct the value from the parsed record, so any future key is lost in the DB and on every re-send. -- `Preferences` — `src/Simplex/Chat/Types/Preferences.hs:153`, JSON at `:1144`. -- `GroupPreferences` — `src/Simplex/Chat/Types/Preferences.hs:331`, JSON at `:1186`. -- `defaultJSON = J.defaultOptions {J.omitNothingFields = True}` — `simplexmq/src/Simplex/Messaging/Parsers.hs:153`. `rejectUnknownFields` is not set (defaults to `False`). +## Fix -With these options, unknown object keys are **silently dropped on parse** (not rejected, not preserved), and `omitNothingFields` drops `Nothing` fields on encode. Because the value is always reconstructed from the parsed record, any future key is gone. +Give each type an `unknownFields :: J.Object` that captures unrecognized keys on parse and re-emits them on encode — the pattern already used by `CustomData`, `MCUnknown{json}`, `CInfoInvalidJSON`. Because storage and wire share these instances, this one change fixes both. **No schema change, no protocol version bump** (purely additive/transparent). It's a one-time cost; afterwards every new pref is single-stage. -Crucially, both persistence and the wire go through the same instances: +## What a reviewer needs to know -- **Storage.** `contact_profiles.preferences` and `group_profiles.preferences` are TEXT columns holding the JSON of `Preferences` / `GroupPreferences` via `ToField = encodeJSON` and `FromField = decodeJSON` (`Types/Preferences.hs:1146-1150`, `:1188-1192`). Written at `Store/Direct.hs:744`, `Store/Groups.hs:2732`, `Store/Groups.hs:2762`; read via `toContact` (`Store/Shared.hs:497`) and `toGroupInfo` (`Store/Shared.hs:694`). -- **Wire.** `XInfo Profile`, `XGrpInfo GroupProfile`, `XGrpPrefs GroupPreferences`. The derived `Profile` / `GroupProfile` JSON delegate to the `Preferences` / `GroupPreferences` instances. +- **`Eq` must include `unknownFields`.** Receive handlers store only when the value changed (`xGrpInfo` `unless (p == p')` `Subscriber.hs:3689`; `updateGroupPrefs_` `:3714`; `processContactProfileUpdate` via `sameProfileContent` `:2683`). If Eq ignored unknown keys, a pure-unknown-pref change would look like "no change" and never get stored. No spurious UI items result: the "group/profile updated" items are gated on separate functions that ignore prefs (`sameGroupProfileInfo` nulls prefs; direct item gated on visible fields), and feature-change items only iterate known features. +- **Modification paths.** Direct `setPreference_` (`:141`) is a record update — preserves the new field for free. Group `setGroupPreference*` (`:362`–`:399`) funnel through `mergeGroupPreferences`/`toGroupPreferences`, which rebuild the record; re-attach `maybe mempty unknownFields prefs_` at the four `setGroupPreference*` entry points (leaves `FullGroupPreferences` untouched). +- **Lossy mobile UI.** The group-prefs screen rebuilds the whole `groupPreferences` from its Kotlin model and re-sends the full profile via `apiUpdateGroupProfile` on every save (`GroupPreferences.kt:48`). So an owner on the previous version strips unknown keys on their *first* toggle. Core-only preservation can't see what the UI dropped — see decision 1. -So a value that retains unknown keys is retained in the DB *and* re-sent on the wire, with no changes to storage or protocol code. +## Change surface (small) -## Guarantee - -Make a previous client version a transparent pass-through for preference keys it does not understand. Two sub-guarantees: - -- **(a) Dormant survival.** A received-but-unknown preference is retained in local storage, so after the client upgrades it becomes active with no re-toggle. -- **(b) Loss-free re-broadcast.** A client that re-encodes or re-broadcasts profile/preferences state does not strip unknown keys. - -Both reduce to a single mechanism: **preserve unknown JSON keys inside the `Preferences` / `GroupPreferences` value**, because storage and wire both flow through the value's JSON instances. - -This is a **one-time** infrastructure cost that must ship before the first single-stage preference. It needs **no protocol version bump** (purely additive and transparent) and **no DB schema change** (same columns). The codebase already uses this `J.Object` pattern elsewhere: `CustomData` (`Types.hs:262`), `MCUnknown {json :: J.Object}`, `MRUnknown`, `LCUnknown`, and `CInfoInvalidJSON` (`Messages.hs:175`, per PR #6105 in `docs/CONTRIBUTING.md`). - -## Data-flow facts (verified) - -Receive and store-gating: - -- `processContactProfileUpdate` (`Library/Subscriber.hs:2665`) stores the received profile when `contentChanged = not (sameProfileContent p p')` (`:2683`). `sameProfileContent` (`Types.hs:746`) compares profiles including `preferences`. -- `xGrpInfo` (`Library/Subscriber.hs:3681`) stores when `unless (p == p')` (`:3689`); `xGrpPrefs` / `updateGroupPrefs_` (`:3707`, `:3712`) store when `unless (groupPreferences p == Just ps')` (`:3714`). -- **Consequence:** `unknownFields` must be part of derived `Eq`. Otherwise a change whose only difference is a new unknown key reads as "no change" and is never stored, breaking guarantee (a). - -No spurious UI items result from including unknown fields in `Eq`: - -- "Group updated" text item is gated on `sameGroupProfileInfo` (`Library/Internal.hs:2988`), which nulls `groupPreferences` before comparing — unaffected by unknown pref keys. -- "Profile updated" direct item is gated on visible display fields only (`visibleProfileUpdated`, `Library/Subscriber.hs:2703`). -- Feature-change items iterate known features only (`createGroupFeatureChangedItems`, `createRcvFeatureItems`), so an unknown pref produces none. - -Modification paths: - -- **Direct.** `setPreference` / `setPreference'` funnel through `setPreference_` (`Types/Preferences.hs:141`), a record update on `Preferences` — an added field is preserved automatically. -- **Group.** `setGroupPreference` (`:362`), `setGroupPreferenceRole` (`:369`), `setGroupPreference'` (`:376`), `setGroupTimedMessagesPreference` (`:397`) all funnel through `mergeGroupPreferences` (`Maybe GroupPreferences -> FullGroupPreferences`, `:1008`) and `toGroupPreferences` (`FullGroupPreferences -> GroupPreferences`, `:1029`). `toGroupPreferences` is called only from this set family, and every entry point takes `Maybe GroupPreferences` and returns `GroupPreferences`. So unknown keys can be re-attached at the four entry points without adding a field to `FullGroupPreferences`. - -Send paths: - -- Group toggles via `SetGroupFeature` / `SetGroupFeatureRole` / `SetGroupTimedMessages` (`Library/Commands.hs:3434-3459`) go through `updateGroupProfileByName` (`:3994`), which reads the **stored** `GroupProfile` and applies the modifier to `groupPreferences p` — a core-side path that preserves unknowns once storage and the set family preserve them. -- General group profile edit via `APIUpdateGroupProfile groupId p'` (`Library/Commands.hs:3132`) stores and broadcasts the caller-supplied `p'` wholesale through `runUpdateGroupProfile` (`:3909`, store at `:3913`). - -Lossy UI path (important): - -- The mobile group-preferences screen rebuilds the entire `groupPreferences` from its Kotlin model on every save and sends the whole `GroupProfile` via `apiUpdateGroupProfile`: `apps/multiplatform/common/src/commonMain/kotlin/chat/simplex/common/views/chat/group/GroupPreferences.kt:48` — `val gp = gInfo.groupProfile.copy(groupPreferences = preferences.toGroupPreferences())`. The Swift UI does the equivalent. -- So an owner on the previous version strips unknown pref keys on their *first* preference toggle — the normal save path, not a rare edge case. The core cannot see what the UI dropped, so full coverage of guarantee (b) for the mobile path requires either a core-side re-injection of stored unknown keys in `APIUpdateGroupProfile`, or UI-level preservation. - -Encoding detail: - -- `encodeJSON` uses `J.encode`, which uses `toEncoding`. The aeson fork keeps the default `toEncoding = E.value . toJSON` (`dist-newstyle/src/aeson-*/src/Data/Aeson/Types/ToJSON.hs:313`). So defining only a merging `toJSON` makes both DB and wire preserve unknown keys; no separate `toEncoding` needed. - -## Design - -### 1. Core representation — `Types/Preferences.hs` - -- Add `unknownFields :: J.Object` as the last field of `Preferences` (`:153`) and `GroupPreferences` (`:331`). Keep `deriving (Eq, Show)` — `Eq` must include it (store-gating). -- Replace the two `deriveJSON` splices with hand-written instances: - - `parseJSON = withObject ...` parses the known fields exactly as today (`.:?` per field — all fields are `Maybe`, matching the derived Maybe semantics) and sets `unknownFields = \\ knownKeys`. - - `toJSON x = Object (knownPairs <> unknownFields x)` with known keys winning the left-biased merge, so a stale duplicate can never shadow a real field. Rely on the default `toEncoding`. - - One `knownKeys` list per type drives the difference (single source of truth; a round-trip test asserts no known field ever lands in `unknownFields`). - - `ToField` / `FromField` are unchanged — they now carry unknowns automatically. -- Set `unknownFields = mempty` at the literal construction sites: `emptyChatPrefs` (`:494`), `emptyGroupPrefs` (`:515`), `defaultBusinessGroupPrefs` (`:534`), `toGroupPreferences` (`:1029`), the `Preferences {..}` and `GroupPreferences {..}` record literals. `businessGroupPrefs` (`:517`) is a record update over `defaultBusinessGroupPrefs` and inherits `mempty`. -- Group modify: in `setGroupPreference`, `setGroupPreferenceRole`, `setGroupPreference'`, `setGroupTimedMessagesPreference`, re-attach `maybe mempty unknownFields prefs_` onto the result. The direct set path needs no change (record update preserves). - -### 2. Core-side defence against lossy UIs — `Library/Commands.hs` - -- In `APIUpdateGroupProfile groupId p'`, before `runUpdateGroupProfile`, re-inject unknown preference keys from the stored `groupProfile gInfo` into `p'`. A small helper `restoreUnknownGroupPrefs stored new` sets the result's `groupPreferences.unknownFields` to `storedUnknown <> newUnknown` (falling back to `emptyGroupPrefs` when `new`'s `groupPreferences` is `Nothing`). This makes the normal mobile group-prefs save path loss-free without touching Kotlin or Swift. The semantics — a client cannot delete a preference key it does not understand — are correct. -- Symmetric option (low value): the same for `APIUpdateProfile` / `updateProfile` (the user's own `Preferences`). This only matters on downgrade; propagation-critical contact preferences are store-only and already covered by section 1. - -### 3. Tests — `tests/ProtocolTests.hs` (and `tests/JSONTests.hs` if useful) - -- Round-trip: an object with an extra unknown key decodes then encodes with the key present and known fields intact. -- Guard: all known fields present ⇒ `unknownFields == mempty` (catches a forgotten `knownKeys` entry). -- Modify preserves: `setGroupPreference` on a `GroupPreferences` carrying an unknown key ⇒ the encoded result still contains the key. -- Eq: two values differing only by an unknown key are `/=` (store-gating relies on this). -- Per project rules: core JSON/round-trip tests only; no UI unit tests. - -## Change surface - -- `src/Simplex/Chat/Types/Preferences.hs` — sections 1 (representation, JSON, constructors, group set family). The bulk of the change. -- `src/Simplex/Chat/Library/Commands.hs` — section 2 (small helper + one call site; optional symmetric direct case). -- `tests/ProtocolTests.hs` (+ possibly `tests/JSONTests.hs`) — section 3. -- No schema, no migration, no protocol-version change. +- `Types/Preferences.hs`: add the field to the 2 types; hand-write their `FromJSON`/`ToJSON` (parse known fields as today + capture the rest; encode known + merge unknowns, known wins); set `unknownFields = mempty` at the literal constructors (`emptyChatPrefs`, `emptyGroupPrefs`, `defaultBusinessGroupPrefs`, `toGroupPreferences`, record literals); re-attach unknowns in the 4 group `set*` helpers. +- `Library/Commands.hs` (decision 1): in `APIUpdateGroupProfile` (`:3132`), re-inject stored unknown pref keys into the caller's profile before `runUpdateGroupProfile`. +- `tests/ProtocolTests.hs`: round-trip keeps an unknown key; all-known ⇒ `unknownFields == mempty`; `setGroupPreference` keeps an unknown key; values differing only by an unknown key are `/=`. ## Open decisions -1. **Robustness against the lossy mobile-UI re-broadcast path.** - - (A, recommended) Core JSON preservation **plus** the `APIUpdateGroupProfile` merge (section 2). Makes the normal mobile save path loss-free with zero UI changes. - - (B) Core JSON preservation only. Guarantees dormant-survival and loss-free re-broadcast via core toggle commands, but an older-version owner editing group prefs through the mobile UI still strips unknown keys. - - (C) A, plus add unknown-pref preservation to the Kotlin and Swift preference models (belt-and-suspenders; unnecessary for correctness once A is in place; touches two UI codebases; verified by review only). - -2. **Depth.** - - (Recommended) Top-level features only — the described problem (a whole new preference like `comments` / `sessions`). Leaf preference types keep using `omittedField` for removed fields. - - Also leaf sub-fields — apply the same `J.Object` capture to each leaf preference type so future sub-fields within an existing preference also survive. More code across the leaf types; rarely needed. - -## Out of scope - -- Unknown sub-fields inside a leaf preference type (unless decision 2 selects it). -- Kotlin/Swift UI-level unknown-pref preservation (unless decision 1 selects C). +1. **Robustness vs. the lossy mobile save path.** + (A, recommended) core JSON + re-inject unknown keys in `APIUpdateGroupProfile` — makes the normal mobile save loss-free, zero UI changes. + (B) core JSON only — dormant survival guaranteed; an old-version owner editing prefs via mobile still strips. + (C) A + also preserve unknown prefs in the Kotlin/Swift models — belt-and-suspenders, unnecessary once A is in, touches two UI codebases. +2. **Depth.** (recommended) new top-level features only; or also unknown sub-fields inside a leaf pref type (same mechanism, more types).