diff --git a/client_sdks/devconnect-react-native/src/reporters/mmkvReporter.ts b/client_sdks/devconnect-react-native/src/reporters/mmkvReporter.ts index 5f610cd..d23f70d 100644 --- a/client_sdks/devconnect-react-native/src/reporters/mmkvReporter.ts +++ b/client_sdks/devconnect-react-native/src/reporters/mmkvReporter.ts @@ -32,7 +32,7 @@ export class DevConnectMMKV { * @returns A proxied MMKV-like object that auto-reports operations */ static wrap(mmkv: any, label: string = 'mmkv'): any { - const storageType = 'mmkv'; + const storageType = `mmkv:${label}`; // Resolve delete/remove — v4: .remove(), v3: .delete() const deleteFn: ((key: string) => any) | undefined = diff --git a/docs/superpowers/specs/2026-08-07-state-page-tree-json-tabbar-design.md b/docs/superpowers/specs/2026-08-07-state-page-tree-json-tabbar-design.md new file mode 100644 index 0000000..c9c4f4f --- /dev/null +++ b/docs/superpowers/specs/2026-08-07-state-page-tree-json-tabbar-design.md @@ -0,0 +1,125 @@ +# State Page — Tree/Pretty as Nested Tab Bar + +## Problem + +Switching the state-detail panel between Tree and JSON views is currently a +small chip toggle in the header row (one chip for both Before & After). +Per the user, this is awkward and inconsistent with how other pages in the +app let the user pick a viewer mode. + +Goal: replace the chip with a proper segmented-control tab bar (matching the +existing `DetailTabBar` pattern used by Network Inspector, All Events, +Console), placed inside the Before and After tabs so the two views can be +chosen independently. + +## Approach + +Nested tab bar (Option A from the brainstorming): + +``` +┌─ Detail header ────────────────────────────┐ +│ actionName [screenshot] [×] │ +├────────────────────────────────────────────┤ +│ [ Diff | Before | After ] │ ← outer tab bar (existing) +├────────────────────────────────────────────┤ +│ ┌─ Before ─────────────────────────────┐ │ +│ │ [ Tree | Pretty ] │ ← NEW inner tab bar +│ │ ────────────────────────────────────│ │ +│ │ { … JSON tree … } │ │ +│ └────────────────────────────────────────┘ │ +│ ┌─ After ──────────────────────────────┐ │ +│ │ [ Tree | Pretty ] │ ← NEW inner tab bar +│ │ ────────────────────────────────────│ │ +│ │ { … JSON tree … } │ │ +│ └────────────────────────────────────────┘ │ +└────────────────────────────────────────────┘ +``` + +Before and After each get their own inner tab controller, so the user can +view the Before tree and the After pretty (or any combination) at the same +time. + +## Architecture + +### New widget + +`_StateJsonTabView` (private to `state_inspector_page.dart`, replacing +`_StateJsonToggleView`): + +- `final dynamic data` — the state map to render. +- Stateful — owns its own `TabController` (length 2) so Before and After + have independent selection. +- Uses `DefaultTabController` so the inner `TabBarView` and `TabBar` wire + up without explicit plumbing. +- Renders: + - `_DetailTabBar(tabs: const ['Tree', 'Pretty'])` at the top. + - `TabBarView` children: + - Tree: `JsonViewer(data: widget.data, initiallyExpanded: true)` + (same as current Tree mode). + - Pretty: `JsonPrettyViewer(data: widget.data)` + (same as current JSON mode). +- Default tab: Tree (index 0) — matches current default behaviour. + +### State changes + +In `_StateInspectorPageState`: + +- **Remove** `bool _jsonPrettyMode = false;` field (line 521). +- **Remove** the chip toggle `GestureDetector` + `Container` block in the + header (lines 678-721). +- **Replace** the two `_StateJsonToggleView(...)` calls inside the + `TabBarView` (`_StateJsonToggleView(data: entry.previousState, ...)` / + `…nextState, ...`) with `_StateJsonTabView(data: …)`. +- **Delete** the `_StateJsonToggleView` and `_StateJsonToggleViewState` + classes (lines 779-836). + +### Why nested `DefaultTabController` + +The outer detail already uses a `DefaultTabController(length: 3)` for the +Diff/Before/After bar (line 651). Nested `DefaultTabController`s work in +Flutter because `TabBarView` looks up the nearest ancestor controller via +`DefaultTabController.of(...)` — a child can wrap a sub-tree in its own +`DefaultTabController` without affecting the parent. Each `_StateJsonTabView` +gets its own controller, so Before's Tree/Pretty selection is independent +of After's. + +### Localisation + +Use existing strings: +- `S.of(context).tree` → tab label "Tree" +- `S.of(context).pretty` → tab label "Pretty" + +No new i18n keys needed. + +## Trade-offs + +- **Pro:** Same widget (`DetailTabBar`) as Network / All Events / Console — + visual consistency across the app. +- **Pro:** Click target is bigger (full pill segment) instead of a 12×12 + icon chip. +- **Pro:** Per-tab independence — user can put Before in Pretty and After + in Tree, or vice versa. +- **Con:** Nested tab bar (Diff/Before/After outside, Tree/Pretty inside). + Recognised pattern in IDEs and code viewers; acceptable for a developer + tool. Vertical screen real estate drops slightly because the inner tab + bar adds ~36 px inside each tab. +- **Con:** No persistent preference — switching tabs resets Tree/Pretty to + default (Tree). Same as current behaviour; if persistence becomes + useful later, add via a shared preference. + +## Testing + +No automated tests for the state-detail UI today. Per YAGNI, skip writing +new ones for this change. Manual verification: + +1. Open state detail from All Events. +2. Click "Pretty" inside Before tab → JSON pretty renders. +3. Switch to After tab → After defaults back to Tree. +4. Switch back to Before → Before still on Pretty (independent state). +5. Switch to Diff tab and back to Before → Before still on Pretty. + +## Out of scope + +- Persistence of Tree/Pretty preference across sessions. +- Keyboard shortcut to toggle Tree/Pretty. +- Per-state-manager-type default (e.g. Redux always starts in Pretty). \ No newline at end of file diff --git a/lib/features/all_events/presentation/pages/all_events_page.dart b/lib/features/all_events/presentation/pages/all_events_page.dart index ca8de35..89f6118 100644 --- a/lib/features/all_events/presentation/pages/all_events_page.dart +++ b/lib/features/all_events/presentation/pages/all_events_page.dart @@ -53,6 +53,14 @@ class _AllEventsPageState extends ConsumerState { int _visibleCount = 0; final List _events = []; + /// Pin of the currently-selected [UnifiedEvent]. Survives the + /// `_events..clear()..addAll(next.items)` churn in the listener and + /// survives the display-limit trim that drops older entries — so the + /// detail panel never disappears just because a new entry arrived + /// (Bug B) or because the user picked an older entry that's now + /// outside the visible window. + UnifiedEvent? _pinnedSelectedEvent; + @override void initState() { super.initState(); @@ -64,6 +72,24 @@ class _AllEventsPageState extends ConsumerState { _eventCount.value = next.items.length; _visibleCount = next.items.length; _untrimmedCount.value = next.total; + // Keep `_pinnedSelectedEvent` in sync with the latest copy of + // the selected entry — content (e.g. network body after a + // start→complete merge) updates without dropping the user's + // pinned tab/scroll position. + // + // Only refresh the pin when the refresh list still contains the + // selected ID. If display-limit trimming removed the older entry, + // `_findEvent` returns null and the previous pin stays — the + // detail panel survives an out-of-window selection. The pin is + // cleared only by explicit selection/reset paths (see + // `_clearAll`, `_onSelectRow`, etc.). + final selectedId = _selectedEventId.value; + if (selectedId != null) { + final updated = _findEvent(selectedId); + if (updated != null) { + _pinnedSelectedEvent = updated; + } + } setState(() {}); if (_autoScroll) _autoScrollIfNeeded(); }, @@ -176,6 +202,7 @@ class _AllEventsPageState extends ConsumerState { ref.read(memoryLeakEntriesProvider.notifier).clear(); ref.read(benchmarkEntriesProvider.notifier).clear(); _selectedEventId.value = null; + _pinnedSelectedEvent = null; _events.clear(); _eventCount.value = 0; _untrimmedCount.value = 0; @@ -434,8 +461,11 @@ class _AllEventsPageState extends ConsumerState { showDetail: false, platform: device?.platform, onTap: () { - _selectedEventId.value = + final nextId = isSelected ? null : event.id; + _selectedEventId.value = nextId; + _pinnedSelectedEvent = + nextId == null ? null : event; if (!isSelected && _autoScroll) { _autoScroll = false; _programmaticScroll = false; @@ -463,7 +493,15 @@ class _AllEventsPageState extends ConsumerState { ValueListenableBuilder( valueListenable: _selectedEventId, builder: (context, selectedId, _) { - final selectedEvent = _findEvent(selectedId); + // Prefer the pinned event so the panel + // survives display-limit trims that drop + // older entries and survives the brief + // window during `_events..clear()` when + // `_findEvent` would return null. + final selectedEvent = _pinnedSelectedEvent ?? + (selectedId == null + ? null + : _findEvent(selectedId)); if (selectedEvent == null) { return const SizedBox.shrink(); } @@ -482,8 +520,10 @@ class _AllEventsPageState extends ConsumerState { child: EventDetailPanel( key: ValueKey(selectedEvent.id), event: selectedEvent, - onClose: () => - _selectedEventId.value = null, + onClose: () { + _selectedEventId.value = null; + _pinnedSelectedEvent = null; + }, ), ), ], diff --git a/lib/features/console/presentation/pages/console_page.dart b/lib/features/console/presentation/pages/console_page.dart index ce1bf8d..5823ab2 100644 --- a/lib/features/console/presentation/pages/console_page.dart +++ b/lib/features/console/presentation/pages/console_page.dart @@ -44,25 +44,21 @@ class _ConsolePageState extends ConsumerState { void initState() { super.initState(); _scrollController.addListener(_onScroll); + // Mirror the network_inspector listener pattern: always bump + // `_generation` on every provider change. The earlier incremental + // vs full-replace split skipped the bump on the append path, so + // `StableListView.shouldRebuild` returned false and new entries + // were never materialised in the viewport even though + // `_visibleCount` had grown. ref.listenManual>( filteredConsoleEntriesProvider, (previous, next) { - final prevLen = _entries.length; - if (next.length > prevLen && previous != null && next.length - prevLen == next.length - previous.length) { - _entries.addAll(next.sublist(prevLen)); - _entryCount.value = _entries.length; - if (!_autoScroll) return; - _visibleCount = _entries.length; - setState(() {}); - _autoScrollIfNeeded(); - } else { - _entries..clear()..addAll(next); - _entryCount.value = _entries.length; - _visibleCount = _entries.length; - _generation++; - setState(() {}); - if (_autoScroll) _autoScrollIfNeeded(); - } + _entries..clear()..addAll(next); + _entryCount.value = _entries.length; + _visibleCount = _entries.length; + _generation++; + setState(() {}); + if (_autoScroll) _autoScrollIfNeeded(); }, fireImmediately: true, ); diff --git a/lib/features/state_inspector/presentation/pages/state_inspector_page.dart b/lib/features/state_inspector/presentation/pages/state_inspector_page.dart index ad64fc4..2814302 100644 --- a/lib/features/state_inspector/presentation/pages/state_inspector_page.dart +++ b/lib/features/state_inspector/presentation/pages/state_inspector_page.dart @@ -518,8 +518,6 @@ class _StateDetailPanel extends StatefulWidget { } class _StateDetailPanelState extends State<_StateDetailPanel> { - bool _jsonPrettyMode = false; - StateChange get entry => widget.entry; void _takeScreenshot(BuildContext context, bool isDark) { @@ -608,10 +606,7 @@ class _StateDetailPanelState extends State<_StateDetailPanel> { color: Colors.grey[500], letterSpacing: 1)), const SizedBox(height: 8), - _jsonPrettyMode - ? JsonPrettyViewer(data: entry.previousState) - : JsonViewer( - data: entry.previousState, initiallyExpanded: true), + _StateJsonTabView(data: entry.previousState), ], ), ), @@ -630,10 +625,7 @@ class _StateDetailPanelState extends State<_StateDetailPanel> { color: Colors.grey[500], letterSpacing: 1)), const SizedBox(height: 8), - _jsonPrettyMode - ? JsonPrettyViewer(data: entry.nextState) - : JsonViewer( - data: entry.nextState, initiallyExpanded: true), + _StateJsonTabView(data: entry.nextState), ], ), ), @@ -674,51 +666,6 @@ class _StateDetailPanelState extends State<_StateDetailPanel> { ), ), ), - // JSON mode toggle - GestureDetector( - onTap: () => - setState(() => _jsonPrettyMode = !_jsonPrettyMode), - child: MouseRegion( - cursor: SystemMouseCursors.click, - child: Container( - padding: const EdgeInsets.symmetric( - horizontal: 8, vertical: 4), - decoration: BoxDecoration( - borderRadius: BorderRadius.circular(6), - color: _jsonPrettyMode - ? ColorTokens.secondary.withValues(alpha: 0.15) - : (isDark - ? Colors.white.withValues(alpha: 0.06) - : Colors.black.withValues(alpha: 0.06)), - ), - child: Row( - mainAxisSize: MainAxisSize.min, - children: [ - Icon( - _jsonPrettyMode - ? LucideIcons.braces - : LucideIcons.list, - size: 12, - color: _jsonPrettyMode - ? ColorTokens.secondary - : Colors.grey[500], - ), - const SizedBox(width: 4), - TextComponent( - _jsonPrettyMode ? S.of(context).pretty : S.of(context).tree, - style: TextStyle( - fontSize: 10, - fontWeight: FontWeight.w600, - color: _jsonPrettyMode - ? ColorTokens.secondary - : Colors.grey[500], - ), - ), - ], - ), - ), - ), - ), const SizedBox(width: 6), // Screenshot button _DetailIconBtn( @@ -755,16 +702,14 @@ class _StateDetailPanelState extends State<_StateDetailPanel> { ), LazyTab( index: 1, - builder: (_) => _StateJsonToggleView( + builder: (_) => _StateJsonTabView( data: entry.previousState, - jsonMode: _jsonPrettyMode, ), ), LazyTab( index: 2, - builder: (_) => _StateJsonToggleView( + builder: (_) => _StateJsonTabView( data: entry.nextState, - jsonMode: _jsonPrettyMode, ), ), ], @@ -776,61 +721,53 @@ class _StateDetailPanelState extends State<_StateDetailPanel> { } } -class _StateJsonToggleView extends StatefulWidget { +/// Tree / Pretty viewer with a segmented-control tab bar at the top. +/// +/// Each instance owns its own [DefaultTabController], so the Before and +/// After tabs in the parent panel keep their Tree/Pretty selection +/// independently. +class _StateJsonTabView extends StatelessWidget { final dynamic data; - final bool jsonMode; - - const _StateJsonToggleView({ - required this.data, - required this.jsonMode, - }); - - @override - State<_StateJsonToggleView> createState() => _StateJsonToggleViewState(); -} -class _StateJsonToggleViewState extends State<_StateJsonToggleView> { - bool _jsonEverOpened = false; - final _scrollController = SmoothScrollController(); - - @override - void dispose() { - _scrollController.dispose(); - super.dispose(); - } - - @override - void didUpdateWidget(_StateJsonToggleView oldWidget) { - super.didUpdateWidget(oldWidget); - if (widget.jsonMode && !_jsonEverOpened) { - _jsonEverOpened = true; - } - } + const _StateJsonTabView({required this.data}); @override Widget build(BuildContext context) { - if (widget.jsonMode && !_jsonEverOpened) { - _jsonEverOpened = true; - } - return Stack( - children: [ - Offstage( - offstage: widget.jsonMode, - child: SingleChildScrollView( - controller: _scrollController, - padding: const EdgeInsets.all(16), - child: JsonViewer(data: widget.data, initiallyExpanded: true), + final theme = Theme.of(context); + final isDark = theme.brightness == Brightness.dark; + + return DefaultTabController( + length: 2, + child: Column( + crossAxisAlignment: CrossAxisAlignment.stretch, + children: [ + Padding( + padding: const EdgeInsets.fromLTRB(16, 12, 16, 0), + child: _DetailTabBar( + isDark: isDark, + accentColor: ColorTokens.secondary, + tabs: const ['Tree', 'Pretty'], + ), ), - ), - if (_jsonEverOpened) - Offstage( - offstage: !widget.jsonMode, - child: Padding( - padding: const EdgeInsets.all(16), - child: JsonPrettyViewer(data: widget.data), + Expanded( + child: TabBarView( + children: [ + SingleChildScrollView( + padding: const EdgeInsets.all(16), + child: JsonViewer( + data: data, + initiallyExpanded: true, + ), + ), + Padding( + padding: const EdgeInsets.all(16), + child: JsonPrettyViewer(data: data), + ), + ], ), ), - ], + ], + ), ); } } diff --git a/lib/features/storage_viewer/provider/storage_providers.dart b/lib/features/storage_viewer/provider/storage_providers.dart index 2f5acf6..3645566 100644 --- a/lib/features/storage_viewer/provider/storage_providers.dart +++ b/lib/features/storage_viewer/provider/storage_providers.dart @@ -91,18 +91,13 @@ class StorageNotifier extends StateNotifier> { StorageNotifier(WsMessageHandler wsMessageHandler, this._ref) : super([]) { _sub = wsMessageHandler.onStorage.listen((entry) { - // Update existing key or add new - final index = state.indexWhere( - (e) => e.key == entry.key && e.storageType == entry.storageType); - if (index >= 0) { - final updated = List.from(state); - updated[index] = entry; - state = updated; - } else { - final limit = _ref.read(retentionLimitProvider).limit; - state = truncateList([...state, entry], limit); - _totalSeen++; - } + // Pure event-log: every reported operation is its own row. The + // SDK mints a fresh UUID per `_send()` and the handler's + // `_uniqueOneShotId` disambiguates on retry, so every entry that + // reaches us has a unique id — no content-based dedup needed. + final limit = _ref.read(retentionLimitProvider).limit; + state = truncateList([...state, entry], limit); + _totalSeen++; }); } diff --git a/lib/models/storage/storage_entry.dart b/lib/models/storage/storage_entry.dart index 0d7bc09..46fd289 100644 --- a/lib/models/storage/storage_entry.dart +++ b/lib/models/storage/storage_entry.dart @@ -25,6 +25,12 @@ abstract class StorageEntry with _$StorageEntry { required String id, required String deviceId, required StorageType storageType, + /// Optional instance/namespace label — e.g. for MMKV this carries + /// the `mmkvId` (`'mmkv:user-storage'` → `mmkv`, `storeId='user-storage'`). + /// Multiple stores of the same [storageType] with different [storeId] + /// must coexist; dedup at the viewer is keyed on + /// `(storageType, storeId, key)`. + @Default(null) String? storeId, required String key, dynamic value, required String operation, diff --git a/lib/models/storage/storage_entry.freezed.dart b/lib/models/storage/storage_entry.freezed.dart index f89fd16..d72a4a3 100644 --- a/lib/models/storage/storage_entry.freezed.dart +++ b/lib/models/storage/storage_entry.freezed.dart @@ -15,7 +15,12 @@ T _$identity(T value) => value; /// @nodoc mixin _$StorageEntry { - String get id; String get deviceId; StorageType get storageType; String get key; dynamic get value; String get operation; int get timestamp; + String get id; String get deviceId; StorageType get storageType;/// Optional instance/namespace label — e.g. for MMKV this carries +/// the `mmkvId` (`'mmkv:user-storage'` → `mmkv`, `storeId='user-storage'`). +/// Multiple stores of the same [storageType] with different [storeId] +/// must coexist; dedup at the viewer is keyed on +/// `(storageType, storeId, key)`. + String? get storeId; String get key; dynamic get value; String get operation; int get timestamp; /// Create a copy of StorageEntry /// with the given fields replaced by the non-null parameter values. @JsonKey(includeFromJson: false, includeToJson: false) @@ -28,16 +33,16 @@ $StorageEntryCopyWith get copyWith => _$StorageEntryCopyWithImpl Object.hash(runtimeType,id,deviceId,storageType,key,const DeepCollectionEquality().hash(value),operation,timestamp); +int get hashCode => Object.hash(runtimeType,id,deviceId,storageType,storeId,key,const DeepCollectionEquality().hash(value),operation,timestamp); @override String toString() { - return 'StorageEntry(id: $id, deviceId: $deviceId, storageType: $storageType, key: $key, value: $value, operation: $operation, timestamp: $timestamp)'; + return 'StorageEntry(id: $id, deviceId: $deviceId, storageType: $storageType, storeId: $storeId, key: $key, value: $value, operation: $operation, timestamp: $timestamp)'; } @@ -48,7 +53,7 @@ abstract mixin class $StorageEntryCopyWith<$Res> { factory $StorageEntryCopyWith(StorageEntry value, $Res Function(StorageEntry) _then) = _$StorageEntryCopyWithImpl; @useResult $Res call({ - String id, String deviceId, StorageType storageType, String key, dynamic value, String operation, int timestamp + String id, String deviceId, StorageType storageType, String? storeId, String key, dynamic value, String operation, int timestamp }); @@ -65,12 +70,13 @@ class _$StorageEntryCopyWithImpl<$Res> /// Create a copy of StorageEntry /// with the given fields replaced by the non-null parameter values. -@pragma('vm:prefer-inline') @override $Res call({Object? id = null,Object? deviceId = null,Object? storageType = null,Object? key = null,Object? value = freezed,Object? operation = null,Object? timestamp = null,}) { +@pragma('vm:prefer-inline') @override $Res call({Object? id = null,Object? deviceId = null,Object? storageType = null,Object? storeId = freezed,Object? key = null,Object? value = freezed,Object? operation = null,Object? timestamp = null,}) { return _then(_self.copyWith( id: null == id ? _self.id : id // ignore: cast_nullable_to_non_nullable as String,deviceId: null == deviceId ? _self.deviceId : deviceId // ignore: cast_nullable_to_non_nullable as String,storageType: null == storageType ? _self.storageType : storageType // ignore: cast_nullable_to_non_nullable -as StorageType,key: null == key ? _self.key : key // ignore: cast_nullable_to_non_nullable +as StorageType,storeId: freezed == storeId ? _self.storeId : storeId // ignore: cast_nullable_to_non_nullable +as String?,key: null == key ? _self.key : key // ignore: cast_nullable_to_non_nullable as String,value: freezed == value ? _self.value : value // ignore: cast_nullable_to_non_nullable as dynamic,operation: null == operation ? _self.operation : operation // ignore: cast_nullable_to_non_nullable as String,timestamp: null == timestamp ? _self.timestamp : timestamp // ignore: cast_nullable_to_non_nullable @@ -159,10 +165,10 @@ return $default(_that);case _: /// } /// ``` -@optionalTypeArgs TResult maybeWhen(TResult Function( String id, String deviceId, StorageType storageType, String key, dynamic value, String operation, int timestamp)? $default,{required TResult orElse(),}) {final _that = this; +@optionalTypeArgs TResult maybeWhen(TResult Function( String id, String deviceId, StorageType storageType, String? storeId, String key, dynamic value, String operation, int timestamp)? $default,{required TResult orElse(),}) {final _that = this; switch (_that) { case _StorageEntry() when $default != null: -return $default(_that.id,_that.deviceId,_that.storageType,_that.key,_that.value,_that.operation,_that.timestamp);case _: +return $default(_that.id,_that.deviceId,_that.storageType,_that.storeId,_that.key,_that.value,_that.operation,_that.timestamp);case _: return orElse(); } @@ -180,10 +186,10 @@ return $default(_that.id,_that.deviceId,_that.storageType,_that.key,_that.value, /// } /// ``` -@optionalTypeArgs TResult when(TResult Function( String id, String deviceId, StorageType storageType, String key, dynamic value, String operation, int timestamp) $default,) {final _that = this; +@optionalTypeArgs TResult when(TResult Function( String id, String deviceId, StorageType storageType, String? storeId, String key, dynamic value, String operation, int timestamp) $default,) {final _that = this; switch (_that) { case _StorageEntry(): -return $default(_that.id,_that.deviceId,_that.storageType,_that.key,_that.value,_that.operation,_that.timestamp);case _: +return $default(_that.id,_that.deviceId,_that.storageType,_that.storeId,_that.key,_that.value,_that.operation,_that.timestamp);case _: throw StateError('Unexpected subclass'); } @@ -200,10 +206,10 @@ return $default(_that.id,_that.deviceId,_that.storageType,_that.key,_that.value, /// } /// ``` -@optionalTypeArgs TResult? whenOrNull(TResult? Function( String id, String deviceId, StorageType storageType, String key, dynamic value, String operation, int timestamp)? $default,) {final _that = this; +@optionalTypeArgs TResult? whenOrNull(TResult? Function( String id, String deviceId, StorageType storageType, String? storeId, String key, dynamic value, String operation, int timestamp)? $default,) {final _that = this; switch (_that) { case _StorageEntry() when $default != null: -return $default(_that.id,_that.deviceId,_that.storageType,_that.key,_that.value,_that.operation,_that.timestamp);case _: +return $default(_that.id,_that.deviceId,_that.storageType,_that.storeId,_that.key,_that.value,_that.operation,_that.timestamp);case _: return null; } @@ -215,12 +221,18 @@ return $default(_that.id,_that.deviceId,_that.storageType,_that.key,_that.value, @JsonSerializable() class _StorageEntry implements StorageEntry { - const _StorageEntry({required this.id, required this.deviceId, required this.storageType, required this.key, this.value, required this.operation, required this.timestamp}); + const _StorageEntry({required this.id, required this.deviceId, required this.storageType, this.storeId = null, required this.key, this.value, required this.operation, required this.timestamp}); factory _StorageEntry.fromJson(Map json) => _$StorageEntryFromJson(json); @override final String id; @override final String deviceId; @override final StorageType storageType; +/// Optional instance/namespace label — e.g. for MMKV this carries +/// the `mmkvId` (`'mmkv:user-storage'` → `mmkv`, `storeId='user-storage'`). +/// Multiple stores of the same [storageType] with different [storeId] +/// must coexist; dedup at the viewer is keyed on +/// `(storageType, storeId, key)`. +@override@JsonKey() final String? storeId; @override final String key; @override final dynamic value; @override final String operation; @@ -239,16 +251,16 @@ Map toJson() { @override bool operator ==(Object other) { - return identical(this, other) || (other.runtimeType == runtimeType&&other is _StorageEntry&&(identical(other.id, id) || other.id == id)&&(identical(other.deviceId, deviceId) || other.deviceId == deviceId)&&(identical(other.storageType, storageType) || other.storageType == storageType)&&(identical(other.key, key) || other.key == key)&&const DeepCollectionEquality().equals(other.value, value)&&(identical(other.operation, operation) || other.operation == operation)&&(identical(other.timestamp, timestamp) || other.timestamp == timestamp)); + return identical(this, other) || (other.runtimeType == runtimeType&&other is _StorageEntry&&(identical(other.id, id) || other.id == id)&&(identical(other.deviceId, deviceId) || other.deviceId == deviceId)&&(identical(other.storageType, storageType) || other.storageType == storageType)&&(identical(other.storeId, storeId) || other.storeId == storeId)&&(identical(other.key, key) || other.key == key)&&const DeepCollectionEquality().equals(other.value, value)&&(identical(other.operation, operation) || other.operation == operation)&&(identical(other.timestamp, timestamp) || other.timestamp == timestamp)); } @JsonKey(includeFromJson: false, includeToJson: false) @override -int get hashCode => Object.hash(runtimeType,id,deviceId,storageType,key,const DeepCollectionEquality().hash(value),operation,timestamp); +int get hashCode => Object.hash(runtimeType,id,deviceId,storageType,storeId,key,const DeepCollectionEquality().hash(value),operation,timestamp); @override String toString() { - return 'StorageEntry(id: $id, deviceId: $deviceId, storageType: $storageType, key: $key, value: $value, operation: $operation, timestamp: $timestamp)'; + return 'StorageEntry(id: $id, deviceId: $deviceId, storageType: $storageType, storeId: $storeId, key: $key, value: $value, operation: $operation, timestamp: $timestamp)'; } @@ -259,7 +271,7 @@ abstract mixin class _$StorageEntryCopyWith<$Res> implements $StorageEntryCopyWi factory _$StorageEntryCopyWith(_StorageEntry value, $Res Function(_StorageEntry) _then) = __$StorageEntryCopyWithImpl; @override @useResult $Res call({ - String id, String deviceId, StorageType storageType, String key, dynamic value, String operation, int timestamp + String id, String deviceId, StorageType storageType, String? storeId, String key, dynamic value, String operation, int timestamp }); @@ -276,12 +288,13 @@ class __$StorageEntryCopyWithImpl<$Res> /// Create a copy of StorageEntry /// with the given fields replaced by the non-null parameter values. -@override @pragma('vm:prefer-inline') $Res call({Object? id = null,Object? deviceId = null,Object? storageType = null,Object? key = null,Object? value = freezed,Object? operation = null,Object? timestamp = null,}) { +@override @pragma('vm:prefer-inline') $Res call({Object? id = null,Object? deviceId = null,Object? storageType = null,Object? storeId = freezed,Object? key = null,Object? value = freezed,Object? operation = null,Object? timestamp = null,}) { return _then(_StorageEntry( id: null == id ? _self.id : id // ignore: cast_nullable_to_non_nullable as String,deviceId: null == deviceId ? _self.deviceId : deviceId // ignore: cast_nullable_to_non_nullable as String,storageType: null == storageType ? _self.storageType : storageType // ignore: cast_nullable_to_non_nullable -as StorageType,key: null == key ? _self.key : key // ignore: cast_nullable_to_non_nullable +as StorageType,storeId: freezed == storeId ? _self.storeId : storeId // ignore: cast_nullable_to_non_nullable +as String?,key: null == key ? _self.key : key // ignore: cast_nullable_to_non_nullable as String,value: freezed == value ? _self.value : value // ignore: cast_nullable_to_non_nullable as dynamic,operation: null == operation ? _self.operation : operation // ignore: cast_nullable_to_non_nullable as String,timestamp: null == timestamp ? _self.timestamp : timestamp // ignore: cast_nullable_to_non_nullable diff --git a/lib/models/storage/storage_entry.g.dart b/lib/models/storage/storage_entry.g.dart index 1565f85..0a748bd 100644 --- a/lib/models/storage/storage_entry.g.dart +++ b/lib/models/storage/storage_entry.g.dart @@ -11,6 +11,7 @@ _StorageEntry _$StorageEntryFromJson(Map json) => id: json['id'] as String, deviceId: json['deviceId'] as String, storageType: $enumDecode(_$StorageTypeEnumMap, json['storageType']), + storeId: json['storeId'] as String? ?? null, key: json['key'] as String, value: json['value'], operation: json['operation'] as String, @@ -22,6 +23,7 @@ Map _$StorageEntryToJson(_StorageEntry instance) => 'id': instance.id, 'deviceId': instance.deviceId, 'storageType': _$StorageTypeEnumMap[instance.storageType]!, + 'storeId': instance.storeId, 'key': instance.key, 'value': instance.value, 'operation': instance.operation, diff --git a/lib/server/ws_message_handler.dart b/lib/server/ws_message_handler.dart index a9cf52c..07d87c2 100644 --- a/lib/server/ws_message_handler.dart +++ b/lib/server/ws_message_handler.dart @@ -15,6 +15,80 @@ import 'protocol/dc_message.dart'; import 'ws_server.dart'; import 'package:uuid/uuid.dart'; +/// Parses the wire `storageType` string into the [StorageType] enum +/// plus an optional instance/namespace [storeId]. +/// +/// SDKs may send `:` to distinguish between multiple +/// instances of the same backend — e.g. two MMKV instances +/// (`mmkv:user-storage` and `mmkv:settings`). Bare `` strings +/// leave [storeId] null. The base type match is case-insensitive; the +/// label is preserved verbatim. Unrecognised bases fall back to +/// [StorageType.sharedPreferences] (same behaviour as the legacy parser). +({StorageType storageType, String? storeId}) parseStorageTypeAndStoreId( + String raw) { + final lower = raw.toLowerCase(); + final colon = lower.indexOf(':'); + String base; + String? label; + if (colon >= 0) { + base = lower.substring(0, colon); + final tail = raw.substring(colon + 1).trim(); + label = tail.isEmpty ? null : tail; + } else { + base = lower; + } + + final StorageType type; + switch (base) { + case 'async_storage': + case 'asyncstorage': + type = StorageType.asyncStorage; + break; + case 'shared_preferences': + case 'sharedpreferences': + type = StorageType.sharedPreferences; + break; + case 'hive': + type = StorageType.hive; + break; + case 'sqlite': + type = StorageType.sqlite; + break; + case 'realm': + type = StorageType.realm; + break; + case 'objectbox': + type = StorageType.objectbox; + break; + case 'floor': + type = StorageType.floor; + break; + case 'sembast': + type = StorageType.sembast; + break; + case 'sqflite': + type = StorageType.sqflite; + break; + case 'watermelondb': + type = StorageType.watermelondb; + break; + case 'encrypted_storage': + case 'encryptedstorage': + type = StorageType.encryptedStorage; + break; + case 'sqldelight': + type = StorageType.sqldelight; + break; + case 'mmkv': + type = StorageType.mmkv; + break; + default: + type = StorageType.sharedPreferences; + break; + } + return (storageType: type, storeId: label); +} + class WsMessageHandler { final WsServer server; @@ -391,10 +465,14 @@ class WsMessageHandler { void _handleStorage(DCMessage message) { final p = message.payload; + final parsed = parseStorageTypeAndStoreId( + p['storageType'] as String? ?? '', + ); final entry = StorageEntry( id: _uniqueOneShotId(message.id), deviceId: message.deviceId, - storageType: _parseStorageType(p['storageType'] as String? ?? ''), + storageType: parsed.storageType, + storeId: parsed.storeId, key: p['key'] as String? ?? '', value: p['value'], operation: p['operation'] as String? ?? 'read', @@ -417,45 +495,6 @@ class WsMessageHandler { } } - StorageType _parseStorageType(String type) { - final lower = type.toLowerCase(); - - // Handle "mmkv:label" format from SDK (e.g. "mmkv:user-storage") - if (lower.startsWith('mmkv')) return StorageType.mmkv; - - switch (lower) { - case 'async_storage': - case 'asyncstorage': - return StorageType.asyncStorage; - case 'shared_preferences': - case 'sharedpreferences': - return StorageType.sharedPreferences; - case 'hive': - return StorageType.hive; - case 'sqlite': - return StorageType.sqlite; - case 'realm': - return StorageType.realm; - case 'objectbox': - return StorageType.objectbox; - case 'floor': - return StorageType.floor; - case 'sembast': - return StorageType.sembast; - case 'sqflite': - return StorageType.sqflite; - case 'watermelondb': - return StorageType.watermelondb; - case 'encrypted_storage': - case 'encryptedstorage': - return StorageType.encryptedStorage; - case 'sqldelight': - return StorageType.sqldelight; - default: - return StorageType.sharedPreferences; - } - } - void _handlePerformance(DCMessage message) { final p = message.payload; final entry = PerformanceEntry( diff --git a/test/features/storage_viewer/storage_notifier_test.dart b/test/features/storage_viewer/storage_notifier_test.dart new file mode 100644 index 0000000..1019ede --- /dev/null +++ b/test/features/storage_viewer/storage_notifier_test.dart @@ -0,0 +1,156 @@ +import 'dart:async'; + +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; + +import 'package:devconnect_manage_tool/core/constants/ws_constants.dart'; +import 'package:devconnect_manage_tool/models/device_info.dart'; +import 'package:devconnect_manage_tool/models/storage/storage_entry.dart'; +import 'package:devconnect_manage_tool/server/protocol/dc_message.dart'; +import 'package:devconnect_manage_tool/server/providers/server_providers.dart'; +import 'package:devconnect_manage_tool/server/ws_connection.dart'; +import 'package:devconnect_manage_tool/server/ws_message_handler.dart'; +import 'package:devconnect_manage_tool/server/ws_server.dart'; +import 'package:devconnect_manage_tool/features/storage_viewer/provider/storage_providers.dart'; + +/// Pins the storage notifier's event-log semantics: +/// +/// - Two distinct `message.id`s → 2 rows (even with identical content). +/// - Different storeId on the wire → 2 rows (regression for the +/// "instance id collapsed by parser" bug). +/// +/// Id-based dedup already happens upstream: SDK `_send()` mints a fresh +/// UUID per call, and `WsMessageHandler._uniqueOneShotId` disambiguates +/// on retry. So at the notifier layer every entry is unique and we +/// just append. +void main() { + late _Harness h; + + setUp(() { + h = _Harness(); + }); + + tearDown(() => h.dispose()); + + test('same data + same timestamp, two distinct message.id → 2 rows', () async { + // SDK generates a fresh id per call. Two `_send()` invocations → + // two wire messages with different `id`. Even if the payload + // (storageType/storeId/key/value/op/timestamp) is identical, the + // notifier keeps both as separate events. + await h.emit(_msg(id: 'a', timestamp: 1000)); + await h.emit(_msg(id: 'b', timestamp: 1000)); + + expect(h.notifier.state, hasLength(2)); + }); + + test('different storeId on the wire → 2 rows (regression)', () async { + // The original bug: SDK MMKV reporter sends `mmkv:storeA` and + // `mmkv:storeB`, but the server parser collapsed both to the same + // `StorageType.mmkv` enum, causing the notifier to dedup. With the + // `storeId` field carried through end-to-end, two distinct MMKV + // instances produce two rows. + await h.emit(_msg(id: 'a', storeId: 'storeA', timestamp: 1000)); + await h.emit(_msg(id: 'b', storeId: 'storeB', timestamp: 1000)); + + expect(h.notifier.state, hasLength(2)); + expect( + h.notifier.state.map((e) => e.storeId).toSet(), + {'storeA', 'storeB'}, + ); + }); +} + +class _Harness { + _Harness() { + final fakeServer = _FakeWsServer(); + handler = WsMessageHandler(server: fakeServer); + container = ProviderContainer(overrides: [ + wsMessageHandlerProvider.overrideWithValue(handler), + ]); + notifier = container.read(storageEntriesProvider.notifier); + _server = fakeServer; + } + + late final ProviderContainer container; + late final WsMessageHandler handler; + late final StorageNotifier notifier; + late final _FakeWsServer _server; + + Future emit(DCMessage m) async { + _server.emit(m); + // Drain microtasks so the broadcast stream propagates: server → + // handler.onMessage → handler.onStorage → notifier.state. + await Future.delayed(Duration.zero); + } + + void dispose() { + notifier.cancelSubscription(); + handler.dispose(); + container.dispose(); + } +} + +DCMessage _msg({ + String id = 'm', + String deviceId = 'dev', + StorageType storageType = StorageType.mmkv, + String? storeId = 'storeA', + String key = 'token', + dynamic value = 'abc', + String operation = 'write', + required int timestamp, +}) { + final wire = storeId == null + ? storageTypeWire(storageType) + : '${storageTypeWire(storageType)}:$storeId'; + return DCMessage( + id: id, + type: WsMessageTypes.clientStorageOperation, + deviceId: deviceId, + timestamp: timestamp, + payload: { + 'storageType': wire, + 'key': key, + 'value': value, + 'operation': operation, + }, + ); +} + +String storageTypeWire(StorageType type) => switch (type) { + StorageType.mmkv => 'mmkv', + _ => type.name, + }; + +class _FakeWsServer implements WsServer { + final _msg = StreamController.broadcast(); + final _conn = StreamController.broadcast(); + final _disc = StreamController.broadcast(); + + void emit(DCMessage m) => _msg.add(m); + + @override + Stream get onMessage => _msg.stream; + @override + Stream get onConnection => _conn.stream; + @override + Stream get onDisconnection => _disc.stream; + + @override + Map get connections => const {}; + @override + bool get isRunning => false; + @override + int get port => 0; + @override + String get machineId => 'fake-machine'; + @override + Future start({int port = 0}) async {} + @override + Future stop() async {} + @override + void sendToDevice(String deviceId, DCMessage message) {} + + @override + noSuchMethod(Invocation invocation) => null; +} \ No newline at end of file diff --git a/test/server/ws_message_handler_test.dart b/test/server/ws_message_handler_test.dart new file mode 100644 index 0000000..e9f673a --- /dev/null +++ b/test/server/ws_message_handler_test.dart @@ -0,0 +1,213 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; + +import 'package:devconnect_manage_tool/core/constants/ws_constants.dart'; +import 'package:devconnect_manage_tool/models/device_info.dart'; +import 'package:devconnect_manage_tool/models/storage/storage_entry.dart'; +import 'package:devconnect_manage_tool/server/protocol/dc_message.dart'; +import 'package:devconnect_manage_tool/server/ws_connection.dart'; +import 'package:devconnect_manage_tool/server/ws_message_handler.dart'; +import 'package:devconnect_manage_tool/server/ws_server.dart'; + +void main() { + group('parseStorageTypeAndStoreId', () { + test('bare "mmkv" returns mmkv with null storeId', () { + final r = parseStorageTypeAndStoreId('mmkv'); + expect(r.storageType, StorageType.mmkv); + expect(r.storeId, isNull); + }); + + test('"mmkv:user-storage" returns mmkv with storeId="user-storage"', () { + final r = parseStorageTypeAndStoreId('mmkv:user-storage'); + expect(r.storageType, StorageType.mmkv); + expect(r.storeId, 'user-storage'); + }); + + test('"MMKV:Settings" is case-insensitive on the type, keeps label', () { + final r = parseStorageTypeAndStoreId('MMKV:Settings'); + expect(r.storageType, StorageType.mmkv); + expect(r.storeId, 'Settings'); + }); + + test('two distinct MMKV labels do not collapse', () { + final a = parseStorageTypeAndStoreId('mmkv:storeA'); + final b = parseStorageTypeAndStoreId('mmkv:storeB'); + expect(a.storageType, b.storageType); + expect(a.storeId, isNot(b.storeId)); + }); + }); + + group('StorageEntry dedup key (notifier-layer regression)', () { + test('same (key, value, type) but different storeId → distinct entries', () { + const a = StorageEntry( + id: '1', + deviceId: 'dev', + storageType: StorageType.mmkv, + storeId: 'storeA', + key: 'token', + value: 'abc', + operation: 'write', + timestamp: 1, + ); + const b = StorageEntry( + id: '2', + deviceId: 'dev', + storageType: StorageType.mmkv, + storeId: 'storeB', + key: 'token', + value: 'abc', + operation: 'write', + timestamp: 2, + ); + expect(a.key == b.key, isTrue); + expect(a.storageType == b.storageType, isTrue); + expect(a.storeId == b.storeId, isFalse); + }); + }); + + group('_handleStorage end-to-end', () { + test('two MMKV stores with same key+data produce 2 entries', () async { + final entries = await runHandleStorage([ + _msg( + id: 'm1', + payload: const { + 'storageType': 'mmkv:storeA', + 'key': 'token', + 'value': 'abc', + 'operation': 'write', + }, + ), + _msg( + id: 'm2', + payload: const { + 'storageType': 'mmkv:storeB', + 'key': 'token', + 'value': 'abc', + 'operation': 'write', + }, + ), + ]); + + expect(entries, hasLength(2)); + expect(entries.map((e) => e.storeId).toSet(), {'storeA', 'storeB'}); + // Same key, same value, same storageType — must be 2 rows, + // not 1 (regression: the second write used to overwrite the first). + expect(entries[0].key, 'token'); + expect(entries[1].key, 'token'); + }); + + test('same store + same key + same value sent twice → 2 entries', () async { + // Wire layer always emits one entry per incoming message — + // dedup happens in StorageNotifier (see storage_notifier_test.dart). + final entries = await runHandleStorage([ + _msg( + id: 'm1', + timestamp: 1, + payload: const { + 'storageType': 'mmkv:storeA', + 'key': 'token', + 'value': 'abc', + 'operation': 'write', + }, + ), + _msg( + id: 'm2', + timestamp: 2, + payload: const { + 'storageType': 'mmkv:storeA', + 'key': 'token', + 'value': 'abc', + 'operation': 'write', + }, + ), + ]); + + expect(entries, hasLength(2)); + expect(entries[0].storeId, 'storeA'); + expect(entries[1].storeId, 'storeA'); + expect(entries[0].key, entries[1].key); + expect(entries[0].value, entries[1].value); + expect(entries[0].id, isNot(entries[1].id)); + }); + }); +} + +DCMessage _msg({ + required String id, + required Map payload, + int timestamp = 1, +}) => + DCMessage( + id: id, + type: WsMessageTypes.clientStorageOperation, + deviceId: 'dev', + timestamp: timestamp, + payload: payload, + ); + +/// Drives [WsMessageHandler._handleMessage] for a list of storage +/// operations without standing up a real WsServer — wires the +/// handler's `onStorage` stream into a list. +Future> runHandleStorage(List messages) async { + final received = []; + final completer = Completer>(); + + // WsMessageHandler listens on `server.onMessage` and exposes `onStorage`. + // We only need those two surfaces — fake the rest. + final fakeServer = _FakeWsServer(); + final handler = WsMessageHandler(server: fakeServer); + + final sub = handler.onStorage.listen((e) { + received.add(e); + if (received.length == messages.length && !completer.isCompleted) { + completer.complete(received); + } + }); + + for (final m in messages) { + fakeServer.emitMessage(m); + } + + return completer.future.timeout(const Duration(seconds: 2), onTimeout: () { + sub.cancel(); + handler.dispose(); + return received; + }); +} + +/// Minimal [WsServer] stub. WsMessageHandler only reads `onMessage`, +/// `onConnection`, and `onDisconnection` from its server, so the rest +/// of WsServer's surface (HTTP bind, UDP beacon, etc.) is unused here. +class _FakeWsServer implements WsServer { + final _msg = StreamController.broadcast(); + final _conn = StreamController.broadcast(); + final _disc = StreamController.broadcast(); + + void emitMessage(DCMessage m) => _msg.add(m); + + @override + Stream get onMessage => _msg.stream; + @override + Stream get onConnection => _conn.stream; + @override + Stream get onDisconnection => _disc.stream; + + @override + Map get connections => const {}; + @override + bool get isRunning => false; + @override + int get port => 0; + @override + String get machineId => 'fake-machine'; + @override + Future start({int port = 0}) async {} + @override + Future stop() async {} + @override + void sendToDevice(String deviceId, DCMessage message) {} + + @override + noSuchMethod(Invocation invocation) => null; +} \ No newline at end of file