From cd1978ee1c8e66543601d07122d7293191c86fdf Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Tue, 23 Jun 2026 00:13:00 +0200 Subject: [PATCH] fix: scope hidden libraries to profiles close #1388 --- lib/navigation/profile_session_screen.dart | 3 +- lib/providers/hidden_libraries_provider.dart | 32 +++++++--- lib/screens/main_screen.dart | 58 +++++++++++++++++-- lib/services/storage_service.dart | 25 ++++++++ .../profile_session_screen_test.dart | 33 ++++++++++- .../hidden_libraries_provider_test.dart | 41 +++++++++++++ test/screens/main_screen_layout_test.dart | 52 +++++++++++++++++ .../data_aggregation_bridge_test.dart | 55 ++++++++++++++++++ test/services/storage_service_test.dart | 13 +++++ 9 files changed, 297 insertions(+), 15 deletions(-) diff --git a/lib/navigation/profile_session_screen.dart b/lib/navigation/profile_session_screen.dart index f8e8d3b0..14a3bc74 100644 --- a/lib/navigation/profile_session_screen.dart +++ b/lib/navigation/profile_session_screen.dart @@ -113,7 +113,8 @@ class _ProfileSessionScreenState extends State { }, ), ChangeNotifierProvider( - create: (context) => HiddenLibrariesProvider(storageService: context.read()), + create: (context) => + HiddenLibrariesProvider(storageService: context.read(), profileId: activeId), lazy: true, ), ChangeNotifierProvider( diff --git a/lib/providers/hidden_libraries_provider.dart b/lib/providers/hidden_libraries_provider.dart index fbde16e8..1cd2d352 100644 --- a/lib/providers/hidden_libraries_provider.dart +++ b/lib/providers/hidden_libraries_provider.dart @@ -7,11 +7,12 @@ import '../services/storage_service.dart'; /// all other screens are automatically updated. class HiddenLibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixin { StorageService? _storageService; + final String? profileId; Set _hiddenLibraryKeys = {}; bool _isInitialized = false; Future? _initFuture; - HiddenLibrariesProvider({StorageService? storageService}) : _storageService = storageService { + HiddenLibrariesProvider({StorageService? storageService, this.profileId}) : _storageService = storageService { // Start initialization eagerly to reduce race conditions _initFuture = _initialize(); } @@ -29,19 +30,36 @@ class HiddenLibrariesProvider extends ChangeNotifier with DisposableChangeNotifi /// Initialize the provider by loading hidden libraries from storage Future _initialize() async { if (_isInitialized) return; - final storage = _storageService ??= await StorageService.getInstance(); - _hiddenLibraryKeys = storage.getHiddenLibraries(); + await _loadFromStorage(); _isInitialized = true; safeNotifyListeners(); } + Future _loadFromStorage() async { + final storage = _storageService ??= await StorageService.getInstance(); + final scopedProfileId = profileId; + _hiddenLibraryKeys = scopedProfileId == null + ? storage.getHiddenLibraries() + : storage.getHiddenLibrariesForProfile(scopedProfileId); + } + + Future _saveToStorage() async { + final storage = _storageService ??= await StorageService.getInstance(); + final scopedProfileId = profileId; + if (scopedProfileId == null) { + await storage.saveHiddenLibraries(_hiddenLibraryKeys); + } else { + await storage.saveHiddenLibrariesForProfile(scopedProfileId, _hiddenLibraryKeys); + } + } + /// Hide a library by its key /// Updates both in-memory state and persistent storage Future hideLibrary(String libraryKey) async { if (!_isInitialized) await _initialize(); if (!_hiddenLibraryKeys.contains(libraryKey)) { _hiddenLibraryKeys = Set.from(_hiddenLibraryKeys)..add(libraryKey); - await _storageService!.saveHiddenLibraries(_hiddenLibraryKeys); + await _saveToStorage(); safeNotifyListeners(); } } @@ -52,7 +70,7 @@ class HiddenLibrariesProvider extends ChangeNotifier with DisposableChangeNotifi if (!_isInitialized) await _initialize(); if (_hiddenLibraryKeys.contains(libraryKey)) { _hiddenLibraryKeys = Set.from(_hiddenLibraryKeys)..remove(libraryKey); - await _storageService!.saveHiddenLibraries(_hiddenLibraryKeys); + await _saveToStorage(); safeNotifyListeners(); } } @@ -63,8 +81,8 @@ class HiddenLibrariesProvider extends ChangeNotifier with DisposableChangeNotifi /// Refresh hidden libraries from storage /// Useful if storage was modified outside the provider Future refresh() async { - final storage = _storageService ??= await StorageService.getInstance(); - _hiddenLibraryKeys = storage.getHiddenLibraries(); + await _loadFromStorage(); + _isInitialized = true; safeNotifyListeners(); } } diff --git a/lib/screens/main_screen.dart b/lib/screens/main_screen.dart index 61f7ab2f..0bc8493c 100644 --- a/lib/screens/main_screen.dart +++ b/lib/screens/main_screen.dart @@ -118,6 +118,30 @@ bool shouldPassTvosMenuToSystem({ isCurrentTabRoot; } +@visibleForTesting +enum ProfileInvalidationAction { none, waitForProfileSwitch, invalidateNow } + +@visibleForTesting +ProfileInvalidationAction profileInvalidationAction({ + required String? previousProfileId, + required String? currentProfileId, + required bool wasBindingPreviously, + required bool isBindingNow, + required bool hasPendingProfileSwitchInvalidation, + required String? pendingProfileSwitchInvalidationId, +}) { + if (currentProfileId != previousProfileId) { + return ProfileInvalidationAction.waitForProfileSwitch; + } + if (hasPendingProfileSwitchInvalidation && pendingProfileSwitchInvalidationId == currentProfileId) { + return ProfileInvalidationAction.none; + } + if (wasBindingPreviously && !isBindingNow) { + return ProfileInvalidationAction.invalidateNow; + } + return ProfileInvalidationAction.none; +} + class MainScreen extends StatefulWidget { final bool isOfflineMode; @@ -204,6 +228,8 @@ class _MainScreenState extends State // we only invalidate on id change and the libraries sidebar keeps // stale entries until the user switches profiles. bool _wasBindingPrev = false; + bool _hasPendingProfileSwitchInvalidation = false; + String? _pendingProfileSwitchInvalidationId; /// Subscription to MultiServerManager status changes. Used to resume any /// queued downloads as soon as a Plex client comes online for the first @@ -463,10 +489,20 @@ class _MainScreenState extends State if (activeProfile == null) return; final id = activeProfile.activeId; final isBindingNow = activeProfile.isBinding; + final action = profileInvalidationAction( + previousProfileId: _lastSeenProfileId, + currentProfileId: id, + wasBindingPreviously: _wasBindingPrev, + isBindingNow: isBindingNow, + hasPendingProfileSwitchInvalidation: _hasPendingProfileSwitchInvalidation, + pendingProfileSwitchInvalidationId: _pendingProfileSwitchInvalidationId, + ); - if (id != _lastSeenProfileId) { + if (action == ProfileInvalidationAction.waitForProfileSwitch) { _lastSeenProfileId = id; _wasBindingPrev = isBindingNow; + _hasPendingProfileSwitchInvalidation = true; + _pendingProfileSwitchInvalidationId = id; // We're called inside the synchronous notify cascade *before* the // binder's listener has fired (registration order). At this exact // instant `_isBinding` is still false, so calling awaitBindingSettle @@ -474,10 +510,22 @@ class _MainScreenState extends State // listener gets to flip the flag first, then wait properly. unawaited( Future.microtask(() async { + final scheduledProfileId = id; if (!mounted) return; await activeProfile.awaitBindingSettle(); if (!mounted) return; - await _invalidateAllScreens(); + try { + if (_hasPendingProfileSwitchInvalidation && + _pendingProfileSwitchInvalidationId == scheduledProfileId && + activeProfile.activeId == scheduledProfileId) { + await _invalidateAllScreens(); + } + } finally { + if (_hasPendingProfileSwitchInvalidation && _pendingProfileSwitchInvalidationId == scheduledProfileId) { + _hasPendingProfileSwitchInvalidation = false; + _pendingProfileSwitchInvalidationId = null; + } + } }), ); return; @@ -487,7 +535,7 @@ class _MainScreenState extends State // (true → false transition). Fires after borrow / connection-removal // flows trigger ActiveProfileBinder.rebindIfActive, so the libraries // sidebar reflects the new server set without an app restart. - if (_wasBindingPrev && !isBindingNow) { + if (action == ProfileInvalidationAction.invalidateNow) { _wasBindingPrev = isBindingNow; unawaited(_invalidateAllScreens()); return; @@ -1274,6 +1322,9 @@ class _MainScreenState extends State appLogger.w('Failed to clear ApiCache on profile switch', error: e, stackTrace: st); } + await hiddenLibrariesProvider.refresh(); + if (!mounted) return; + librariesProvider.clear(); if (multiServerProvider.serverManager.serverIds.isNotEmpty) { @@ -1286,7 +1337,6 @@ class _MainScreenState extends State await librariesProvider.refresh(); } - unawaited(hiddenLibrariesProvider.refresh()); playbackStateProvider.clearShuffle(); if (_discoverKey.currentState case final FullRefreshable refreshable) { diff --git a/lib/services/storage_service.dart b/lib/services/storage_service.dart index 600efa60..cde69d8f 100644 --- a/lib/services/storage_service.dart +++ b/lib/services/storage_service.dart @@ -224,8 +224,33 @@ class StorageService extends BaseSharedPreferencesService { await _setStringList('$_userPrefix$_keyHiddenLibraries', libraryKeys.toList()); } + Future saveHiddenLibrariesForProfile(String profileId, Set libraryKeys) async { + await _setStringList('${_userPrefixForProfileId(profileId)}$_keyHiddenLibraries', libraryKeys.toList()); + } + Set getHiddenLibraries() { final jsonString = _getScopedString(_keyHiddenLibraries); + return _decodeStringSet(jsonString); + } + + Set getHiddenLibrariesForProfile(String profileId) { + final scopedKey = '${_userPrefixForProfileId(profileId)}$_keyHiddenLibraries'; + var jsonString = prefs.getString(scopedKey); + if (jsonString == null && getActiveProfileId() == profileId) { + // One-time migration from the legacy unscoped key, but only for the + // currently active profile. Otherwise merely opening another profile's + // scoped provider could steal legacy preferences into the wrong scope. + final legacy = prefs.getString(_keyHiddenLibraries); + if (legacy != null) { + prefs.setString(scopedKey, legacy); + prefs.remove(_keyHiddenLibraries); + jsonString = legacy; + } + } + return _decodeStringSet(jsonString); + } + + Set _decodeStringSet(String? jsonString) { if (jsonString == null) return {}; try { diff --git a/test/navigation/profile_session_screen_test.dart b/test/navigation/profile_session_screen_test.dart index 4ba98f3f..921215a8 100644 --- a/test/navigation/profile_session_screen_test.dart +++ b/test/navigation/profile_session_screen_test.dart @@ -12,6 +12,7 @@ import 'package:plezy/profiles/profile.dart'; import 'package:plezy/profiles/profile_connection_registry.dart'; import 'package:plezy/profiles/profile_registry.dart'; import 'package:plezy/providers/discover_provider.dart'; +import 'package:plezy/providers/hidden_libraries_provider.dart'; import 'package:plezy/providers/multi_server_provider.dart'; import 'package:plezy/services/data_aggregation_service.dart'; import 'package:plezy/services/multi_server_manager.dart'; @@ -47,6 +48,7 @@ void main() { final serverManager = MultiServerManager(); final multiServer = MultiServerProvider(serverManager, DataAggregationService(serverManager)); final discoverProviders = []; + final hiddenProviders = []; final disposedActiveIds = []; addTearDown(() async { @@ -64,6 +66,8 @@ void main() { final kids = Profile.local(id: 'local-kids', displayName: 'Kids', createdAt: DateTime(2026, 1, 2)); await profileRegistry.upsert(owner); await profileRegistry.upsert(kids); + await storage.saveHiddenLibrariesForProfile(owner.id, {'srv:owner'}); + await storage.saveHiddenLibrariesForProfile(kids.id, {'srv:kids'}); await storage.setActiveProfileId(owner.id); await activeProfile.initialize(); @@ -77,8 +81,11 @@ void main() { child: MaterialApp( home: ProfileSessionScreen.forTesting( initialPromptHandled: true, - profileShellBuilder: (context) => - _ProfileProbeShell(discoverProviders: discoverProviders, disposedActiveIds: disposedActiveIds), + profileShellBuilder: (context) => _ProfileProbeShell( + discoverProviders: discoverProviders, + hiddenProviders: hiddenProviders, + disposedActiveIds: disposedActiveIds, + ), ), ), ), @@ -87,8 +94,13 @@ void main() { expect(find.text('active:local-owner'), findsOneWidget); expect(discoverProviders, hasLength(1)); + expect(hiddenProviders, hasLength(1)); final ownerNavigator = profileNavigationRegistry.navigator; final ownerDiscover = discoverProviders.single; + final ownerHidden = hiddenProviders.single; + await ownerHidden.ensureInitialized(); + expect(ownerHidden.profileId, owner.id); + expect(ownerHidden.hiddenLibraryKeys, {'srv:owner'}); await tester.tap(find.byKey(const ValueKey('push-profile-route'))); await tester.pumpAndSettle(); @@ -102,14 +114,24 @@ void main() { expect(disposedActiveIds, contains('local-owner')); expect(discoverProviders, hasLength(2)); expect(discoverProviders.last, isNot(same(ownerDiscover))); + expect(hiddenProviders, hasLength(2)); + expect(hiddenProviders.last, isNot(same(ownerHidden))); + await hiddenProviders.last.ensureInitialized(); + expect(hiddenProviders.last.profileId, kids.id); + expect(hiddenProviders.last.hiddenLibraryKeys, {'srv:kids'}); expect(profileNavigationRegistry.navigator, isNot(same(ownerNavigator))); }); } class _ProfileProbeShell extends StatefulWidget { - const _ProfileProbeShell({required this.discoverProviders, required this.disposedActiveIds}); + const _ProfileProbeShell({ + required this.discoverProviders, + required this.hiddenProviders, + required this.disposedActiveIds, + }); final List discoverProviders; + final List hiddenProviders; final List disposedActiveIds; @override @@ -118,16 +140,21 @@ class _ProfileProbeShell extends StatefulWidget { class _ProfileProbeShellState extends State<_ProfileProbeShell> { DiscoverProvider? _discoverProvider; + HiddenLibrariesProvider? _hiddenProvider; String _activeId = 'none'; @override void didChangeDependencies() { super.didChangeDependencies(); _discoverProvider = context.read(); + _hiddenProvider = context.read(); _activeId = context.read().activeId ?? 'none'; if (widget.discoverProviders.isEmpty || !identical(widget.discoverProviders.last, _discoverProvider)) { widget.discoverProviders.add(_discoverProvider!); } + if (widget.hiddenProviders.isEmpty || !identical(widget.hiddenProviders.last, _hiddenProvider)) { + widget.hiddenProviders.add(_hiddenProvider!); + } } @override diff --git a/test/providers/hidden_libraries_provider_test.dart b/test/providers/hidden_libraries_provider_test.dart index fecab30b..b0c5e7fa 100644 --- a/test/providers/hidden_libraries_provider_test.dart +++ b/test/providers/hidden_libraries_provider_test.dart @@ -88,6 +88,47 @@ void main() { p.dispose(); }); + test('explicit profile id stays isolated when active profile changes', () async { + final storage = await StorageService.getInstance(); + await storage.saveHiddenLibrariesForProfile('owner', {'srv:movies'}); + await storage.saveHiddenLibrariesForProfile('kids', {'srv:kids'}); + await storage.setActiveProfileId('owner'); + + final ownerProvider = HiddenLibrariesProvider(storageService: storage, profileId: 'owner'); + final kidsProvider = HiddenLibrariesProvider(storageService: storage, profileId: 'kids'); + + await storage.setActiveProfileId('kids'); + await ownerProvider.refresh(); + await kidsProvider.refresh(); + + expect(ownerProvider.hiddenLibraryKeys, {'srv:movies'}); + expect(kidsProvider.hiddenLibraryKeys, {'srv:kids'}); + + await ownerProvider.hideLibrary('srv:documentaries'); + + expect(storage.getHiddenLibrariesForProfile('owner'), {'srv:movies', 'srv:documentaries'}); + expect(storage.getHiddenLibrariesForProfile('kids'), {'srv:kids'}); + expect(storage.getHiddenLibraries(), {'srv:kids'}); + + ownerProvider.dispose(); + kidsProvider.dispose(); + }); + + test('refresh marks provider initialized after deterministic reload', () async { + final storage = await StorageService.getInstance(); + await storage.saveHiddenLibrariesForProfile('owner', {'srv:movies'}); + + final p = HiddenLibrariesProvider(storageService: storage, profileId: 'owner'); + expect(p.isInitialized, isFalse); + + await p.refresh(); + + expect(p.isInitialized, isTrue); + expect(p.hiddenLibraryKeys, {'srv:movies'}); + + p.dispose(); + }); + test('hiddenLibraryKeys returns an unmodifiable view', () async { final p = HiddenLibrariesProvider(); await p.ensureInitialized(); diff --git a/test/screens/main_screen_layout_test.dart b/test/screens/main_screen_layout_test.dart index 9241f229..3bb1dbbd 100644 --- a/test/screens/main_screen_layout_test.dart +++ b/test/screens/main_screen_layout_test.dart @@ -70,6 +70,58 @@ void main() { expect(shouldPass(isAppleTV: false), isFalse); }); + test('profile switch waits for one post-bind invalidation', () { + expect( + profileInvalidationAction( + previousProfileId: 'owner', + currentProfileId: 'kids', + wasBindingPreviously: false, + isBindingNow: false, + hasPendingProfileSwitchInvalidation: false, + pendingProfileSwitchInvalidationId: null, + ), + ProfileInvalidationAction.waitForProfileSwitch, + ); + + expect( + profileInvalidationAction( + previousProfileId: 'kids', + currentProfileId: 'kids', + wasBindingPreviously: true, + isBindingNow: false, + hasPendingProfileSwitchInvalidation: true, + pendingProfileSwitchInvalidationId: 'kids', + ), + ProfileInvalidationAction.none, + ); + }); + + test('same-profile rebind invalidates once when binding settles', () { + expect( + profileInvalidationAction( + previousProfileId: 'owner', + currentProfileId: 'owner', + wasBindingPreviously: true, + isBindingNow: false, + hasPendingProfileSwitchInvalidation: false, + pendingProfileSwitchInvalidationId: null, + ), + ProfileInvalidationAction.invalidateNow, + ); + + expect( + profileInvalidationAction( + previousProfileId: 'owner', + currentProfileId: 'owner', + wasBindingPreviously: false, + isBindingNow: false, + hasPendingProfileSwitchInvalidation: false, + pendingProfileSwitchInvalidationId: null, + ), + ProfileInvalidationAction.none, + ); + }); + testWidgets('side navigation bleed animates from the previous value', (tester) async { Widget build(double targetBleed) { return Directionality( diff --git a/test/services/data_aggregation_bridge_test.dart b/test/services/data_aggregation_bridge_test.dart index 3b692af4..be825799 100644 --- a/test/services/data_aggregation_bridge_test.dart +++ b/test/services/data_aggregation_bridge_test.dart @@ -171,6 +171,61 @@ void main() { expect(captured.single.queryParameters['count'], '21'); }); + test('getOnDeckFromAllServers filters hidden Plex continue-watching libraries', () async { + final client = PlexClient.forTesting( + config: PlexConfig( + baseUrl: 'https://plex.example.com', + token: 'token', + clientIdentifier: 'client-id', + product: 'Plezy', + version: 'test', + ), + serverId: ServerId('plex-1'), + serverName: 'Plex', + httpClient: MockClient((req) async { + if (req.url.path == '/hubs') { + return _json({ + 'MediaContainer': { + 'Hub': [ + { + 'key': '/hubs/home/continueWatching', + 'title': 'Continue Watching', + 'type': 'mixed', + 'hubIdentifier': 'home.continue', + 'size': 2, + 'Metadata': [ + { + 'ratingKey': 'movie-visible', + 'type': 'movie', + 'title': 'Visible Movie', + 'lastViewedAt': 100, + 'librarySectionID': 1, + }, + { + 'ratingKey': 'movie-hidden', + 'type': 'movie', + 'title': 'Hidden Movie', + 'lastViewedAt': 200, + 'librarySectionID': 2, + }, + ], + }, + ], + }, + }); + } + return http.Response('unexpected request', 500); + }), + ); + addTearDown(client.close); + manager.debugRegisterClientForTesting(client); + + final result = await service.getOnDeckFromAllServers(limit: 10, hiddenLibraryKeys: {'plex-1:2'}); + + expect(result.items.map((item) => item.id), ['movie-visible']); + expect(result.succeededServerIds, {'plex-1'}); + }); + test('getOnDeckFromAllServers hides duplicate show entries by stable show ids', () async { final client = PlexClient.forTesting( config: PlexConfig( diff --git a/test/services/storage_service_test.dart b/test/services/storage_service_test.dart index 6d58863b..b729fe2b 100644 --- a/test/services/storage_service_test.dart +++ b/test/services/storage_service_test.dart @@ -153,6 +153,19 @@ void main() { expect(s.getHiddenLibraries(), equals({'lib-a', 'lib-b'})); }); + test('explicit profile helpers isolate hidden libraries from active profile changes', () async { + final s = await StorageService.getInstance(); + + await s.setActiveProfileId('owner'); + await s.saveHiddenLibrariesForProfile('owner', {'srv:movies'}); + await s.setActiveProfileId('kids'); + await s.saveHiddenLibrariesForProfile('kids', {'srv:kids'}); + + expect(s.getHiddenLibrariesForProfile('owner'), {'srv:movies'}); + expect(s.getHiddenLibrariesForProfile('kids'), {'srv:kids'}); + expect(s.getHiddenLibraries(), {'srv:kids'}); + }); + test('overwrite replaces previous set', () async { final s = await StorageService.getInstance(); await s.saveHiddenLibraries({'lib-a', 'lib-b'});