From 1d9ffb74270e194407790ff10c9990fb92e05bca Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 2 Aug 2026 05:24:38 +0200 Subject: [PATCH] fix(search): exclude hidden libraries from global search results searchAcrossServers was the only aggregation entry point without a hiddenLibraryKeys parameter, so libraries hidden from home hubs, Continue Watching and the library rail still surfaced their contents in the Search tab. Thread the profile's hidden keys from SearchScreen through to the aggregation, and drop matching items between the fan-out and the ranking pass so hidden hits cannot spend the result limit and shrink what is shown. Items the backend cannot attribute to a library, such as Plex shared and external media, are kept. The screen re-runs the visible query when a library is hidden or unhidden while results are on screen. Its listener is attached only after the provider has hydrated, so the initial load notification cannot race the first query into running twice. Plex search rows now go through the library-aware tagger, so a response that names its section only via librarySectionKey or targetLibrarySectionID is still attributable, and therefore filterable. Jellyfin search results carry no library id at all: the mapper's ParentLibraryId is not a Jellyfin field, and ParentId resolves to a season or physical folder rather than a CollectionFolder. Filtering there needs server-side ParentId scoping and is left for a follow-up. close #1770 --- lib/screens/search_screen.dart | 51 +++++++++- lib/services/data_aggregation_service.dart | 41 +++++--- lib/services/plex_client.dart | 6 +- test/screens/search_screen_test.dart | 94 ++++++++++++++++++- .../data_aggregation_bridge_test.dart | 64 +++++++++++++ test/services/plex_search_test.dart | 44 +++++++++ 6 files changed, 283 insertions(+), 17 deletions(-) diff --git a/lib/screens/search_screen.dart b/lib/screens/search_screen.dart index 31d79524..c923b8f7 100644 --- a/lib/screens/search_screen.dart +++ b/lib/screens/search_screen.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:flutter/material.dart'; import 'package:material_symbols_icons/symbols.dart'; import 'package:provider/provider.dart'; @@ -10,6 +12,7 @@ import '../media/media_item.dart'; import '../mixins/debounced_media_search.dart'; import '../mixins/mounted_set_state_mixin.dart'; import '../mixins/refreshable.dart'; +import '../providers/hidden_libraries_provider.dart'; import '../providers/multi_server_provider.dart'; import '../services/data_aggregation_service.dart'; import '../utils/app_logger.dart'; @@ -38,10 +41,45 @@ class _SearchScreenState extends State AbortController? _activeSearchAbort; ({String query, SearchAggregationResult result})? _pendingSearchOutcome; + HiddenLibrariesProvider? _hiddenLibraries; + Set _lastSeenHiddenKeys = const {}; + @override void initState() { super.initState(); FocusUtils.requestFocusAfterBuild(this, searchFocusNode); + unawaited(_bindHiddenLibraries()); + } + + @override + void dispose() { + _hiddenLibraries?.removeListener(_onHiddenLibrariesChanged); + super.dispose(); + } + + /// Re-run the visible query when a library is hidden or unhidden while + /// results are on screen. The listener is attached only after the provider + /// has hydrated, so its initial load notification cannot race the first + /// query — which awaits that same hydration — into running twice. + Future _bindHiddenLibraries() async { + final hiddenLibraries = context.read(); + await hiddenLibraries.ensureInitialized(); + if (!mounted) return; + _lastSeenHiddenKeys = Set.of(hiddenLibraries.hiddenLibraryKeys); + _hiddenLibraries = hiddenLibraries..addListener(_onHiddenLibrariesChanged); + } + + void _onHiddenLibrariesChanged() { + final hiddenLibraries = _hiddenLibraries; + if (hiddenLibraries == null || !mounted) return; + final currentKeys = hiddenLibraries.hiddenLibraryKeys; + if (currentKeys.length == _lastSeenHiddenKeys.length && currentKeys.containsAll(_lastSeenHiddenKeys)) { + return; + } + _lastSeenHiddenKeys = Set.of(currentKeys); + final query = searchController.text.trim(); + if (query.isEmpty || !hasSearched) return; + unawaited(runSearch(query)); } @override @@ -54,10 +92,21 @@ class _SearchScreenState extends State throw const _SearchUnavailableException(); } + // Hidden-library keys must be hydrated before the first query, or a cold + // start would filter nothing. Read before any await: the profile subtree + // can be torn down mid-search. + final hiddenLibraries = context.read(); + await hiddenLibraries.ensureInitialized(); + if (!mounted) return const []; + final abort = AbortController(); _activeSearchAbort = abort; try { - final result = await multiServerProvider.aggregationService.searchAcrossServers(query, abort: abort); + final result = await multiServerProvider.aggregationService.searchAcrossServers( + query, + hiddenLibraryKeys: hiddenLibraries.hiddenLibraryKeys, + abort: abort, + ); abort.throwIfAborted(); if (result.succeededServerIds.isEmpty && result.failedServerIds.isNotEmpty) { throw const _SearchUnavailableException(); diff --git a/lib/services/data_aggregation_service.dart b/lib/services/data_aggregation_service.dart index 09af55b7..8f418939 100644 --- a/lib/services/data_aggregation_service.dart +++ b/lib/services/data_aggregation_service.dart @@ -54,6 +54,20 @@ Map _searchKindCounts(Iterable items) { return counts; } +/// Drop items belonging to a hidden library. +/// +/// Items the backend could not attribute to a library ([MediaItem.libraryGlobalKey] +/// is null) are kept: Plex search and `/library/shared/all` return shared and +/// external rows that have no local section, and those are not something the +/// user hid. +List _withoutHiddenLibraries(List items, Set? hiddenLibraryKeys) { + if (hiddenLibraryKeys == null || hiddenLibraryKeys.isEmpty) return items; + return items.where((item) { + final libraryKey = item.libraryGlobalKey; + return libraryKey == null || !hiddenLibraryKeys.contains(libraryKey); + }).toList(); +} + /// Cross-server aggregation: fans calls out to every online client and /// merges the results. Single-server operations now go through the /// [MediaServerClient] interface directly (resolved via @@ -172,17 +186,8 @@ class DataAggregationService { failureMessage: (serverId) => 'Failed on-deck fetch from $serverId', fetch: (_, client) => client.fetchContinueWatching(count: limit), ); - final allOnDeck = fetched.items; - // Filter out items from hidden libraries - List filteredOnDeck = allOnDeck; - if (hiddenLibraryKeys != null && hiddenLibraryKeys.isNotEmpty) { - filteredOnDeck = allOnDeck.where((item) { - if (item.libraryId == null || item.serverId == null) return true; - final globalKey = buildGlobalKey(ServerId(item.serverId!), item.libraryId!); - return !hiddenLibraryKeys.contains(globalKey); - }).toList(); - } + var filteredOnDeck = _withoutHiddenLibraries(fetched.items, hiddenLibraryKeys); // Sort by most recently viewed, falling back to addedAt for unwatched items. // Same key as JellyfinClient's continue-watching merge (MediaItem.recencySortKey) @@ -538,7 +543,16 @@ class DataAggregationService { /// Search across all online servers (Plex + Jellyfin). Per-server outcomes /// distinguish authoritative empty results from failed or cancelled legs. - Future searchAcrossServers(String query, {int? limit, AbortController? abort}) async { + /// + /// [hiddenLibraryKeys] excludes results the user has hidden, matching every + /// other aggregated surface. Backends whose search rows carry no library id + /// cannot be filtered here; they must scope the search server-side instead. + Future searchAcrossServers( + String query, { + int? limit, + Set? hiddenLibraryKeys, + AbortController? abort, + }) async { if (query.trim().isEmpty) { return ( items: const [], @@ -576,7 +590,10 @@ class DataAggregationService { }, ); abort?.throwIfAborted(); - final items = rankMediaSearchResults(fetched.items, query, limit: resultLimit); + // Before ranking, so hidden results cannot spend the `resultLimit` budget + // and silently shrink what the user sees. + final visible = _withoutHiddenLibraries(fetched.items, hiddenLibraryKeys); + final items = rankMediaSearchResults(visible, query, limit: resultLimit); appLogger.i( 'Search aggregation completed: ${items.length} results ' diff --git a/lib/services/plex_client.dart b/lib/services/plex_client.dart index d8339778..3cf3bd3c 100644 --- a/lib/services/plex_client.dart +++ b/lib/services/plex_client.dart @@ -1538,7 +1538,11 @@ class PlexClient const allowedTypes = {'movie', 'show', 'artist', 'album', 'track'}; if (!allowedTypes.contains(type)) continue; - results.add(_createTaggedMetadata(metadata)); + // Library-aware: search rows normally carry `librarySectionID`, but the + // tolerant resolver also accepts the `librarySectionKey` / + // `targetLibrarySectionID` forms. Without a section id the item cannot + // be matched against the user's hidden libraries. + results.add(_createTaggedMetadataWithLibrary(metadata)); } catch (e) { appLogger.w('Failed to parse search result', error: e); } diff --git a/test/screens/search_screen_test.dart b/test/screens/search_screen_test.dart index 96b8b244..e12ec949 100644 --- a/test/screens/search_screen_test.dart +++ b/test/screens/search_screen_test.dart @@ -16,9 +16,11 @@ import 'package:plezy/media/media_kind.dart'; import 'package:plezy/media/media_server_client.dart'; import 'package:plezy/media/server_capabilities.dart'; import 'package:plezy/mixins/refreshable.dart'; +import 'package:plezy/providers/hidden_libraries_provider.dart'; import 'package:plezy/providers/multi_server_provider.dart'; import 'package:plezy/screens/search_screen.dart'; import 'package:plezy/services/multi_server_manager.dart'; +import 'package:plezy/services/storage_service.dart'; import 'package:plezy/services/settings_service.dart'; import 'package:plezy/theme/mono_theme.dart'; import 'package:plezy/utils/platform_detector.dart'; @@ -42,6 +44,10 @@ void main() { _resetGlobalTestState(); resetSharedPreferencesForTest(); await SettingsService.getInstance(); + // HiddenLibrariesProvider resolves this lazily; the real SharedPreferences + // round trip never completes inside a testWidgets fake-async zone, so warm + // the singleton here. + await StorageService.getInstance(); }); tearDown(_resetGlobalTestState); @@ -57,9 +63,14 @@ void main() { serverName: 'Server', ); + final hiddenLibraries = HiddenLibrariesProvider(); + addTearDown(hiddenLibraries.dispose); await tester.pumpWidget( TranslationProvider( - child: MaterialApp(home: SearchScreen(key: key)), + child: ChangeNotifierProvider.value( + value: hiddenLibraries, + child: MaterialApp(home: SearchScreen(key: key)), + ), ), ); @@ -220,6 +231,53 @@ void main() { expect(find.text(t.messages.searchPartialResults), findsOneWidget); }); + testWidgets('results from a hidden library never reach the list', (tester) async { + final hiddenLibraries = HiddenLibrariesProvider(); + addTearDown(hiddenLibraries.dispose); + await hiddenLibraries.ensureInitialized(); + await hiddenLibraries.hideLibrary('server_1:2'); + + final (client, key) = await _pumpTvSearchScreen( + tester, + hiddenLibraries: hiddenLibraries, + items: _twoLibraryMovies(), + ); + await tester.pumpAndSettle(); + + (key.currentState! as SearchInputFocusable).submitSearchQuery('movie'); + await tester.pumpAndSettle(); + + // One request per user action: hydrating the hidden keys must not race the + // first query into running twice. + expect(client.queries, ['movie']); + expect(find.text('Movie 1'), findsOneWidget); + expect(find.text('Movie 2'), findsNothing); + }); + + testWidgets('hiding a library while results are shown re-runs the query', (tester) async { + final hiddenLibraries = HiddenLibrariesProvider(); + addTearDown(hiddenLibraries.dispose); + + final (client, key) = await _pumpTvSearchScreen( + tester, + hiddenLibraries: hiddenLibraries, + items: _twoLibraryMovies(), + ); + await tester.pumpAndSettle(); + + (key.currentState! as SearchInputFocusable).submitSearchQuery('movie'); + await tester.pumpAndSettle(); + expect(client.queries, ['movie']); + expect(find.text('Movie 2'), findsOneWidget); + + await hiddenLibraries.hideLibrary('server_1:2'); + await tester.pumpAndSettle(); + + expect(client.queries, ['movie', 'movie']); + expect(find.text('Movie 1'), findsOneWidget); + expect(find.text('Movie 2'), findsNothing); + }); + testWidgets('all server failures render the failed state instead of empty results', (tester) async { final (client, key) = await _pumpTvSearchScreen( tester, @@ -373,6 +431,7 @@ Future<(_FakeMediaServerClient, GlobalKey>)> _pumpTvSearchSc bool registerClient = true, Object? searchError, List<_FakeMediaServerClient> additionalClients = const [], + HiddenLibrariesProvider? hiddenLibraries, }) async { TvDetectionService.debugSetAppleTVOverride(true); tester.view.devicePixelRatio = 1.0; @@ -405,11 +464,17 @@ Future<(_FakeMediaServerClient, GlobalKey>)> _pumpTvSearchSc final provider = testMultiServerProvider(manager); addTearDown(provider.dispose); + final hidden = hiddenLibraries ?? HiddenLibrariesProvider(); + if (hiddenLibraries == null) addTearDown(hidden.dispose); + final key = GlobalKey>(); await tester.pumpWidget( TranslationProvider( - child: ChangeNotifierProvider.value( - value: provider, + child: MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: provider), + ChangeNotifierProvider.value(value: hidden), + ], child: MaterialApp( theme: monoTheme(dark: true), home: SearchScreen(key: key), @@ -426,6 +491,29 @@ Future<(_FakeMediaServerClient, GlobalKey>)> _pumpTvSearchSc return (client, key); } +/// Two movies on the same server in different libraries, so a test can hide +/// one library and assert only the other survives. +List _twoLibraryMovies() => [ + testMediaItem( + id: 'movie_1', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Movie 1', + serverId: 'server_1', + serverName: 'Server', + libraryId: '1', + ), + testMediaItem( + id: 'movie_2', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Movie 2', + serverId: 'server_1', + serverName: 'Server', + libraryId: '2', + ), +]; + TextEditingController _searchController(WidgetTester tester) { return tester.widget(find.byType(FocusableTextField)).controller; } diff --git a/test/services/data_aggregation_bridge_test.dart b/test/services/data_aggregation_bridge_test.dart index acdd814b..988c5ee3 100644 --- a/test/services/data_aggregation_bridge_test.dart +++ b/test/services/data_aggregation_bridge_test.dart @@ -178,6 +178,70 @@ void main() { expect(result.failedServerIds, {'failed'}); }); + test('searchAcrossServers drops hidden-library results and keeps unattributed ones', () async { + final visible = testMediaItem( + id: 'visible-1', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Target', + serverId: 'plex', + libraryId: '1', + ); + final hidden = testMediaItem( + id: 'hidden-1', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Target', + serverId: 'plex', + libraryId: '2', + ); + // Plex search asks for external media, and shared rows carry no local + // section. The user cannot have hidden a library that isn't theirs, so + // an unattributed row must survive the filter. + final unattributed = testMediaItem( + id: 'shared-1', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Target', + serverId: 'plex', + ); + manager.debugRegisterClientForTesting( + _LibrariesClient(ServerId('plex'), searchResults: [visible, hidden, unattributed]), + ); + + final result = await service.searchAcrossServers('Target', hiddenLibraryKeys: {'plex:2'}); + + expect(result.items.map((item) => item.id), unorderedEquals(['visible-1', 'shared-1'])); + expect(result.succeededServerIds, {'plex'}); + }); + + test('hidden-library results are dropped before the ranking limit is spent', () async { + // The hidden row is the exact-title match, so it outranks the visible + // one. Filtering after ranking would let it consume the single slot and + // return nothing at all. + final hidden = testMediaItem( + id: 'hidden-1', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Target', + serverId: 'plex', + libraryId: '2', + ); + final visible = testMediaItem( + id: 'visible-1', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Target Sequel', + serverId: 'plex', + libraryId: '1', + ); + manager.debugRegisterClientForTesting(_LibrariesClient(ServerId('plex'), searchResults: [hidden, visible])); + + final result = await service.searchAcrossServers('Target', limit: 1, hiddenLibraryKeys: {'plex:2'}); + + expect(result.items.map((item) => item.id), ['visible-1']); + }); + test('searchAcrossServers overfetches and ranks before trimming across backends', () async { final plexRequests = []; final jellyfinRequests = []; diff --git a/test/services/plex_search_test.dart b/test/services/plex_search_test.dart index 9ae77cb1..c87dd14a 100644 --- a/test/services/plex_search_test.dart +++ b/test/services/plex_search_test.dart @@ -57,6 +57,50 @@ void main() { expect(captured.single.queryParameters['searchTypes'], 'movies,tv,music'); }); + test('search rows carry their library so hidden libraries can be filtered', () async { + final client = makeClient((request) async { + if (request.url.path != '/library/search') return http.Response('unexpected request', 500); + return _json({ + 'MediaContainer': { + 'SearchResult': [ + { + 'score': 90, + 'Metadata': { + 'ratingKey': 'movie-1', + 'type': 'movie', + 'title': 'The Movie', + 'librarySectionID': 2, + 'librarySectionTitle': 'Movies', + }, + }, + // Older/edge responses name the section only by key. + { + 'score': 80, + 'Metadata': { + 'ratingKey': 'show-1', + 'type': 'show', + 'title': 'The Show', + 'librarySectionKey': '/library/sections/7', + }, + }, + // Shared/external media has no local section at all. + { + 'score': 70, + 'Metadata': {'ratingKey': 'shared-1', 'type': 'movie', 'title': 'The Shared Movie'}, + }, + ], + }, + }); + }); + addTearDown(client.close); + + final results = await client.searchItems('the'); + + expect(results.map((item) => item.id), ['movie-1', 'show-1', 'shared-1']); + expect(results.map((item) => item.libraryId), ['2', '7', null]); + expect(results.map((item) => item.libraryGlobalKey), ['plex-1:2', 'plex-1:7', null]); + }); + test('saturated mixed search supplements omitted media categories and deduplicates results', () async { final captured = []; final primaryResults = >[