From 5e4449247bed9b6c0e820984ad12bb6416688242 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Fri, 12 Jun 2026 14:16:24 +0200 Subject: [PATCH] fix(downloads): inject visibility-aware client resolver --- lib/main.dart | 12 ++++------ lib/providers/multi_server_provider.dart | 14 ++++++----- lib/services/download_manager_service.dart | 17 +++++++------ lib/services/multi_server_manager.dart | 24 +++++++++++++++++++ test/providers/download_provider_test.dart | 19 +++++++++++---- .../offline_watch_provider_test.dart | 2 +- .../downloads_screen_focus_test.dart | 2 +- .../downloads/sync_rules_screen_test.dart | 2 +- test/screens/media_detail_screen_test.dart | 2 +- test/screens/playlist_detail_screen_test.dart | 2 +- .../download_manager_service_test.dart | 23 ++++++++++++++---- 11 files changed, 82 insertions(+), 37 deletions(-) diff --git a/lib/main.dart b/lib/main.dart index cacab831..c31f163d 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -480,13 +480,11 @@ class _MainAppState extends State with WidgetsBindingObserver { PlexApiCache.initialize(_appDatabase); JellyfinApiCache.initialize(_appDatabase); - _downloadManager = DownloadManagerService(database: _appDatabase, storageService: DownloadStorageService.instance); - _downloadManager.setClientResolver((serverId, {clientScopeId}) { - if (clientScopeId != null && clientScopeId.isNotEmpty) { - return _serverManager.getJellyfinClientByCompoundId(clientScopeId) ?? _serverManager.getClient(serverId); - } - return _serverManager.getClient(serverId); - }); + _downloadManager = DownloadManagerService( + database: _appDatabase, + storageService: DownloadStorageService.instance, + clientResolver: _serverManager.resolveDownloadClient, + ); _downloadManager.recoveryFuture = _downloadManager.recoverInterruptedDownloads(); _offlineWatchSyncService = OfflineWatchSyncService(database: _appDatabase, serverManager: _serverManager); diff --git a/lib/providers/multi_server_provider.dart b/lib/providers/multi_server_provider.dart index 3a4d86f7..924ab88e 100644 --- a/lib/providers/multi_server_provider.dart +++ b/lib/providers/multi_server_provider.dart @@ -52,7 +52,9 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi /// Visibility filter applied by the active app profile. `null` means /// "all servers visible" (no profile restriction); otherwise only server /// ids in the set surface through [serverIds] / [onlineServerIds]. - Set? _visibleServerIds; + /// State lives on [MultiServerManager] so the download client resolver + /// applies the same filter; this provider owns mutation + notification. + Set? get _visibleServerIds => _serverManager.visibleServerIds; /// True once the active profile has explicitly resolved visibility. An empty /// set is meaningful: the profile has servers, but none are currently visible. @@ -75,7 +77,7 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi _visibleServerIds!.containsAll(ids)) { return; } - _visibleServerIds = ids; + _serverManager.setVisibleServerIds(ids); _pruneLiveTvServersForVisibility(); safeNotifyListeners(); _refreshLiveTvAvailabilitySoon(); @@ -102,14 +104,14 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi void addToVisibleServerIds(ServerId serverId) { final current = _visibleServerIds; if (current == null) { - _visibleServerIds = {serverId}; + _serverManager.setVisibleServerIds({serverId}); _expectedVisibleServerIds = {...?_expectedVisibleServerIds, serverId}; safeNotifyListeners(); _refreshLiveTvAvailabilitySoon(); return; } if (current.contains(serverId)) return; - _visibleServerIds = {...current, serverId}; + _serverManager.setVisibleServerIds({...current, serverId}); _expectedVisibleServerIds = {...?_expectedVisibleServerIds, serverId}; safeNotifyListeners(); _refreshLiveTvAvailabilitySoon(); @@ -167,7 +169,7 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi final onlineExpected = _serverManager.onlineServerIds.where(expected.contains).where((id) => !visible.contains(id)); if (onlineExpected.isEmpty) return; - _visibleServerIds = {...visible, ...onlineExpected}; + _serverManager.setVisibleServerIds({...visible, ...onlineExpected}); } /// Get the multi-server manager @@ -257,7 +259,7 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi /// Clear all server connections void clearAllConnections() { _serverManager.disconnectAll(); - _visibleServerIds = null; + _serverManager.setVisibleServerIds(null); _expectedVisibleServerIds = null; appLogger.d('MultiServerProvider: All connections cleared'); safeNotifyListeners(); diff --git a/lib/services/download_manager_service.dart b/lib/services/download_manager_service.dart index 763ee534..b0f81648 100644 --- a/lib/services/download_manager_service.dart +++ b/lib/services/download_manager_service.dart @@ -78,9 +78,10 @@ class DownloadManagerService { // Items recovered with video complete but supplementary downloads missing final Set _pendingSupplementaryDownloads = {}; - // Resolve the correct MediaServerClient for a given serverId/scope (set via setClientResolver). - // Falls back to _fallbackClient only when no serverId or resolver is available. - MediaClientResolver? _clientResolver; + // Resolve the correct MediaServerClient for a given serverId/scope + // (constructor-injected). Falls back to _fallbackClient when no serverId + // is available. + final MediaClientResolver _clientResolver; MediaServerClient? _fallbackClient; OfflineModeSource? _offlineSource; @@ -158,10 +159,12 @@ class DownloadManagerService { DownloadManagerService({ required AppDatabase database, required DownloadStorageService storageService, + required MediaClientResolver clientResolver, MediaServerHttpClient? http, @visibleForTesting this._downloadsSupportedOverride, }) : _database = database, _storageService = storageService, + _clientResolver = clientResolver, _http = http ?? httpClient, _artworkService = DownloadArtworkService(storageService: storageService, http: http ?? httpClient); @@ -178,10 +181,6 @@ class DownloadManagerService { return true; } - void setClientResolver(MediaClientResolver resolver) { - _clientResolver = resolver; - } - /// Inject the offline-mode source. When `isOffline`, queue/resume paths skip /// network work and defer until connectivity returns. void setOfflineSource(OfflineModeSource? source) { @@ -193,8 +192,8 @@ class DownloadManagerService { /// Look up the correct client for [serverId]. /// Returns null if the server is offline — callers should skip/defer the work. MediaServerClient? _getClient(ServerId? serverId, {String? clientScopeId}) { - if (serverId != null && _clientResolver != null) { - return _clientResolver!(serverId, clientScopeId: clientScopeId); + if (serverId != null) { + return _clientResolver(serverId, clientScopeId: clientScopeId); } return _fallbackClient; } diff --git a/lib/services/multi_server_manager.dart b/lib/services/multi_server_manager.dart index 304b52ec..e63d5b10 100644 --- a/lib/services/multi_server_manager.dart +++ b/lib/services/multi_server_manager.dart @@ -112,6 +112,30 @@ class MultiServerManager { /// Get client for specific server. MediaServerClient? getClient(ServerId serverId) => _clients[serverId]; + /// Server ids visible to the active profile; `null` means no restriction. + /// Owned here rather than on `MultiServerProvider` so non-UI consumers + /// (the download client resolver) apply the same filter the UI does — + /// the provider delegates its filter state to this field. + Set? _visibleServerIds; + + Set? get visibleServerIds => _visibleServerIds; + + void setVisibleServerIds(Set? ids) => _visibleServerIds = ids; + + bool isServerVisible(ServerId serverId) => _visibleServerIds?.contains(serverId) ?? true; + + /// Resolve the client for a queued download: scope-aware (Jellyfin compound + /// connection ids) and restricted to servers visible to the active profile, + /// so background downloads never run against another profile's server + /// during or after a profile switch. + MediaServerClient? resolveDownloadClient(ServerId serverId, {String? clientScopeId}) { + if (!isServerVisible(serverId)) return null; + if (clientScopeId != null && clientScopeId.isNotEmpty) { + return getJellyfinClientByCompoundId(clientScopeId) ?? getClient(serverId); + } + return getClient(serverId); + } + /// Get the [PlexClient] for a server, or `null` if the server is Jellyfin /// (or not registered). Use for Plex-only flows (Live TV, server prefs, /// endpoint optimization) that don't yet have a backend-neutral diff --git a/test/providers/download_provider_test.dart b/test/providers/download_provider_test.dart index b2e350cd..11086408 100644 --- a/test/providers/download_provider_test.dart +++ b/test/providers/download_provider_test.dart @@ -54,6 +54,8 @@ void main() { late AppDatabase db; late DownloadManagerService downloadManager; + // Swappable per-test resolver behind the constructor-injected closure. + MediaClientResolver? testClientResolver; setUp(() { db = AppDatabase.forTesting(NativeDatabase.memory()); @@ -61,7 +63,12 @@ void main() { // constructor; reinitialize per test so each test sees the fresh in-memory DB. PlexApiCache.initialize(db); JellyfinApiCache.initialize(db); - downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + testClientResolver = null; + downloadManager = DownloadManagerService( + database: db, + storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => testClientResolver?.call(serverId, clientScopeId: clientScopeId), + ); // recoveryFuture is `late final` and would otherwise be unset; we never // exercise the recovery path in these tests but the field must be safe // to await. Set to a completed future. @@ -83,6 +90,7 @@ void main() { final unsupportedManager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, ); @@ -379,6 +387,7 @@ void main() { final unsupportedManager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, )..recoveryFuture = Future.value(); final p = DownloadProvider.forTesting(downloadManager: unsupportedManager, database: db); @@ -676,12 +685,12 @@ void main() { status: DownloadStatus.completed.index, ); await db.addDownloadOwner(profileId: 'test-profile', globalKey: 'jf-machine:ep-1'); - downloadManager.setClientResolver((serverId, {clientScopeId}) { + testClientResolver = (serverId, {clientScopeId}) { if (serverId == 'jf-machine') { return _ScopedTestClient(serverId: ServerId('jf-machine'), scopedServerId: 'jf-machine/user-b'); } return null; - }); + }; final p = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await p.ensureInitialized(); @@ -727,12 +736,12 @@ void main() { ratingKey: 'ep-1', actionType: 'watched', ); - downloadManager.setClientResolver((serverId, {clientScopeId}) { + testClientResolver = (serverId, {clientScopeId}) { if (serverId == 'jf-machine') { return _ScopedTestClient(serverId: ServerId('jf-machine'), scopedServerId: 'jf-machine/user-b'); } return null; - }); + }; final p = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await p.ensureInitialized(); diff --git a/test/providers/offline_watch_provider_test.dart b/test/providers/offline_watch_provider_test.dart index 228cb34e..8d62767a 100644 --- a/test/providers/offline_watch_provider_test.dart +++ b/test/providers/offline_watch_provider_test.dart @@ -25,7 +25,7 @@ void main() { serverManager = MultiServerManager(); syncService = OfflineWatchSyncService(database: db, serverManager: serverManager); - downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance, clientResolver: (serverId, {clientScopeId}) => null); downloadManager.recoveryFuture = Future.value(); downloadProvider = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await downloadProvider.ensureInitialized(); diff --git a/test/screens/downloads/downloads_screen_focus_test.dart b/test/screens/downloads/downloads_screen_focus_test.dart index 27a19d7c..546a52e2 100644 --- a/test/screens/downloads/downloads_screen_focus_test.dart +++ b/test/screens/downloads/downloads_screen_focus_test.dart @@ -44,7 +44,7 @@ void main() { PlexApiCache.initialize(db); JellyfinApiCache.initialize(db); - final downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + final downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance, clientResolver: (serverId, {clientScopeId}) => null); downloadProvider = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await downloadProvider.ensureInitialized(); diff --git a/test/screens/downloads/sync_rules_screen_test.dart b/test/screens/downloads/sync_rules_screen_test.dart index 227e26c0..b9d06acc 100644 --- a/test/screens/downloads/sync_rules_screen_test.dart +++ b/test/screens/downloads/sync_rules_screen_test.dart @@ -103,7 +103,7 @@ void main() { db = AppDatabase.forTesting(NativeDatabase.memory()); PlexApiCache.initialize(db); JellyfinApiCache.initialize(db); - downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance, clientResolver: (serverId, {clientScopeId}) => null); downloadProvider = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await downloadProvider.ensureInitialized(); serverManager = MultiServerManager(); diff --git a/test/screens/media_detail_screen_test.dart b/test/screens/media_detail_screen_test.dart index c6dbfe01..87f06ccf 100644 --- a/test/screens/media_detail_screen_test.dart +++ b/test/screens/media_detail_screen_test.dart @@ -584,7 +584,7 @@ void main() { final db = AppDatabase.forTesting(NativeDatabase.memory()); PlexApiCache.initialize(db); JellyfinApiCache.initialize(db); - final downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + final downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance, clientResolver: (serverId, {clientScopeId}) => null); downloadManager.recoveryFuture = Future.value(); final downloadProvider = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await downloadProvider.ensureInitialized(); diff --git a/test/screens/playlist_detail_screen_test.dart b/test/screens/playlist_detail_screen_test.dart index 320e6b4c..db59fc47 100644 --- a/test/screens/playlist_detail_screen_test.dart +++ b/test/screens/playlist_detail_screen_test.dart @@ -159,7 +159,7 @@ Future<_PlaylistHarness> _createHarness(List items) async { PlexApiCache.initialize(db); JellyfinApiCache.initialize(db); - final downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + final downloadManager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance, clientResolver: (serverId, {clientScopeId}) => null); downloadManager.recoveryFuture = Future.value(); final downloadProvider = DownloadProvider.forTesting(downloadManager: downloadManager, database: db); await downloadProvider.ensureInitialized(); diff --git a/test/services/download_manager_service_test.dart b/test/services/download_manager_service_test.dart index e2ad3e58..c259449c 100644 --- a/test/services/download_manager_service_test.dart +++ b/test/services/download_manager_service_test.dart @@ -102,13 +102,16 @@ void main() { ), ); - final manager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance) - ..setClientResolver((serverId, {clientScopeId}) { + final manager = DownloadManagerService( + database: db, + storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) { return _ScopedJellyfinClient( serverId: ServerId(serverId), scopedServerId: clientScopeId ?? 'jf-machine/user-b', ); - }); + }, + ); final item = await manager.lookupMetadata(ServerId('jf-machine'), 'item-1', preferActiveScope: true); @@ -130,7 +133,11 @@ void main() { }, }); - final manager = DownloadManagerService(database: db, storageService: DownloadStorageService.instance); + final manager = DownloadManagerService( + database: db, + storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, + ); final year = await manager.debugResolveSafRecoveryShowYear( MediaItem( id: 'ep-1', @@ -253,8 +260,9 @@ void main() { final manager = DownloadManagerService( database: db, storageService: storage, + clientResolver: (serverId, {clientScopeId}) => client, http: MediaServerHttpClient(client: _FakeHttpClient(200, utf8.encode('image bytes'))), - )..setClientResolver((serverId, {clientScopeId}) => client); + ); await manager.repairMissingArtworkForDownloads(); @@ -285,6 +293,7 @@ void main() { final manager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, ); addTearDown(manager.dispose); @@ -320,6 +329,7 @@ void main() { final manager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, ); addTearDown(manager.dispose); @@ -348,6 +358,7 @@ void main() { final manager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, ); addTearDown(manager.dispose); @@ -380,6 +391,7 @@ void main() { final manager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, ); addTearDown(manager.dispose); @@ -413,6 +425,7 @@ void main() { final manager = DownloadManagerService( database: db, storageService: DownloadStorageService.instance, + clientResolver: (serverId, {clientScopeId}) => null, downloadsSupportedOverride: false, ); addTearDown(manager.dispose);