fix: add DmScope field to SessionConfig to persist dm_scope setting - #3067
Conversation
The frontend sends dm_scope as part of the session config, but the backend SessionConfig struct lacked the corresponding field. Go's encoding/json silently discards unknown fields, so the value was lost on every PATCH request. Additionally, MarshalJSON only emitted the session block when Dimensions or IdentityLinks were set, so even a stored dm_scope would not appear in GET responses. - Add DmScope string field with json tag 'dm_scope' to SessionConfig - Update MarshalJSON condition to include session when DmScope is set
The dm_scope field was stored in config but never translated into the dimensions array that the routing layer actually consumes. This meant changing the session isolation scope in the UI had no effect at runtime. Add ApplyDmScope() to SessionConfig which maps the user-facing dm_scope values (per-channel-peer, per-channel, per-peer, global) to the corresponding dimension arrays. Call it in LoadConfig post-processing and in both the PATCH and PUT API handlers. Includes table-driven tests covering all dm_scope values and the precedence rule (explicit dimensions > derived from dm_scope).
Update: dm_scope is now wired into runtime behaviorThe initial commit only fixed the storage problem ( What was missingThe routing layer reads What this commit adds
|
|
Hi @SiYue-ZO, thank you for the PR, PATCH /api/config leaves session.dimensions stale Inconsistent scope mapping for legacy/fresh configs in UI |
The reviewer identified two bugs in the original PR:
1. PATCH /api/config leaves session.dimensions stale: LoadConfig()
derives dimensions from the old dm_scope, and the merge carries
those stale dimensions forward. ApplyDmScope() then exits early
because dimensions is already populated, causing a mismatch between
dm_scope (new) and dimensions (old).
2. Legacy/default configs omit dm_scope in GET response: configs with
explicit dimensions but no dm_scope (including DefaultConfig) return
no dm_scope field, causing the frontend to fall back to its default
('per-channel-peer'), which may not match the actual dimensions.
Fix:
- Add DeriveDmScope() to reverse-map known dimensions arrays to
dm_scope when dm_scope is empty.
- Call it in LoadConfig(), PUT handler, PATCH handler, and
ResetToDefaults() for consistent normalization.
- In PATCH handler, clear stale dimensions from the merge result when
the patch contains session.dm_scope but not session.dimensions,
allowing ApplyDmScope() to re-derive from the new scope.
- Add comprehensive unit tests for DeriveDmScope() and scope
transition scenarios.
67fe569 to
ef002d9
Compare
maybe you can review again?thanks! |
2 participants
Problem
The "Session Scope"（运行时会话隔离范围）setting on the config page can be modified in the UI but cannot be saved. After saving and reloading the page, it always reverts to the default
per-channel-peer.Root Cause
The frontend sends
dm_scopeas part of thesessionconfig, but the backendSessionConfigstruct lacked the correspondingDmScopefield. Go'sencoding/jsonsilently discards unknown fields during unmarshal, so the value was lost on every PATCH request.Additionally, the custom
MarshalJSONonly emits thesessionblock whenDimensionsorIdentityLinksare non-empty, so even ifdm_scopewere stored, it would not appear in GET responses.Fix
DmScope stringfield with json tagdm_scopetoSessionConfigstructMarshalJSONcondition to include session whenDmScopeis setOnly one file changed:
pkg/config/config.go(2 insertions, 1 deletion).