dandri/simplex-chat
1
0
Fork 0
mirror of https://github.com/simplex-chat/simplex-chat.git synced 2026-07-14 11:29:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

wip

Browse Source
This commit is contained in:
spaced4ndy
2026-07-13 17:11:13 +04:00
parent 3f1f5373f5
commit 9df4bd122c
1 changed files with 17 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plans/2026-07-10-preferences-forward-compat.md
+17 -106
View File
@@ -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 = <input object> \\ 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).