fix(libraries): reload when a server connects after initial load

This commit is contained in:
edde746
2026-05-30 14:01:01 +02:00
parent 00fce3a997
commit 6111c381af
7 changed files with 420 additions and 19 deletions
+11 -1
View File
@@ -854,7 +854,17 @@ class _MainAppState extends State<MainApp> with WidgetsBindingObserver {
ChangeNotifierProvider(create: (context) => TraktAccountProvider()),
ChangeNotifierProvider(create: (context) => TrackersProvider()),
ChangeNotifierProvider(create: (context) => HiddenLibrariesProvider(), lazy: true),
ChangeNotifierProvider(create: (context) => LibrariesProvider()),
ChangeNotifierProvider(
create: (context) {
final provider = LibrariesProvider();
// Reload libraries when a new server comes online. Servers bind in
// waves on sign-in / profile switch and slow ones reconnect after
// the initial load; without this they stay missing from the sidebar
// until a profile re-switch or restart.
context.read<MultiServerProvider>().onOnlineServersChanged = provider.syncToOnlineServers;
return provider;
},
),
ChangeNotifierProvider(create: (context) => PlaybackStateProvider()),
ChangeNotifierProvider(create: (context) => WatchTogetherProvider()),
ChangeNotifierProvider(create: (context) => CompanionRemoteProvider()),
+86 -11
View File
@@ -23,6 +23,20 @@ class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixi
/// see the same in-flight result instead of racing two separate fetches.
Future<void>? _inFlightLoad;
/// Server ids whose library fetch *succeeded* in the current [_libraries], as
/// reported by [DataAggregationService.getMediaLibrariesFromAllServers].
/// Keyed on fetch success (not on which servers returned libraries) so a
/// server that genuinely has zero libraries still counts as loaded, while a
/// server whose fetch failed does not — the latter is retried on the next
/// status emission instead of being cached as "loaded" forever. Drives
/// [syncToOnlineServers].
Set<String> _loadedServerIds = {};
/// Set when a (re)load is requested while one is already in flight, so the
/// loop runs another pass: a server that comes online *during* a load would
/// otherwise be lost to the coalesced [_inFlightLoad].
bool _hasPendingLoad = false;
/// Unmodifiable list of all libraries (filtered for supported types, ordered)
List<MediaLibrary> get libraries => List.unmodifiable(_libraries);
@@ -47,30 +61,77 @@ class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixi
_aggregationService = service;
}
/// Load libraries from all connected servers.
/// Filters out music libraries and applies saved ordering.
Future<void> loadLibraries() {
return _inFlightLoad ??= _loadLibrariesInternal().whenComplete(() => _inFlightLoad = null);
/// Reload libraries when the set of online servers has grown since the last
/// load. Servers connect in waves — the owner Plex account, then each
/// borrowed/shared connection, then Jellyfin, plus slow servers that
/// reconnect after timing out — and each wave must surface in the sidebar
/// without a profile re-switch or app restart.
///
/// No-op when uninitialized, when [onlineServerIds] is empty, or when every
/// id is already represented in the current load. That last guard keeps the
/// many unrelated reasons the server-status stream fires (visibility churn,
/// auth errors, Live TV probes, a server going offline) from causing reload
/// storms.
Future<void> syncToOnlineServers(Set<String> onlineServerIds) {
if (_aggregationService == null || onlineServerIds.isEmpty) return Future<void>.value();
if (_loadState == LibrariesLoadState.loaded && _loadedServerIds.containsAll(onlineServerIds)) {
return Future<void>.value();
}
return _load();
}
Future<void> _loadLibrariesInternal() async {
/// Load libraries from all connected servers, unconditionally. Used by
/// pull-to-refresh, inline connection-add, and library reordering.
/// Filters out music libraries and applies saved ordering.
Future<void> loadLibraries() => _load();
/// Single entry point for every (re)load. Concurrent callers coalesce onto
/// one in-flight pass; a request that arrives mid-pass is replayed by
/// [_runLoadLoop] so it isn't masked by that coalescing. Each pass fetches
/// whatever is online at fetch time, so no caller needs to specify a target.
Future<void> _load() {
_hasPendingLoad = true;
return _inFlightLoad ??= _runLoadLoop().whenComplete(() => _inFlightLoad = null);
}
Future<void> _runLoadLoop() async {
while (_hasPendingLoad) {
_hasPendingLoad = false;
final succeeded = await _loadLibrariesInternal();
// Stop on failure so a persistently failing fetch can't hot-loop; the
// next server-status emission re-drives the sync.
if (!succeeded) break;
}
}
/// Returns `true` on a successful load, `false` on error.
Future<bool> _loadLibrariesInternal() async {
if (_aggregationService == null) {
appLogger.w('LibrariesProvider: Cannot load libraries - not initialized');
return;
return false;
}
_loadState = LibrariesLoadState.loading;
_errorMessage = null;
safeNotifyListeners();
// Reloading over an already-loaded list (a reactive server-connect sync, an
// inline connection add, a reorder) must not flip the UI back to a loading
// state: screens such as LibrariesScreen replace their whole body with a
// spinner whenever `isLoading` is true. Keep the current list visible and
// swap in the fuller one when the fetch completes; only the first load (or
// a reload after clear()/error) surfaces the spinner.
final reloadInPlace = _loadState == LibrariesLoadState.loaded;
if (!reloadInPlace) {
_loadState = LibrariesLoadState.loading;
_errorMessage = null;
safeNotifyListeners();
}
try {
// Fetch libraries from every connected backend (Plex + Jellyfin).
// The aggregation service converts Plex-typed responses to MediaLibrary
// internally; Jellyfin clients return MediaLibrary natively.
final allLibraries = await _aggregationService!.getMediaLibrariesFromAllServers();
final result = await _aggregationService!.getMediaLibrariesFromAllServers();
// Filter out music libraries (not supported)
final filteredLibraries = allLibraries.where((lib) => !ContentTypeHelper.isMusicLibrary(lib)).toList();
final filteredLibraries = result.libraries.where((lib) => !ContentTypeHelper.isMusicLibrary(lib)).toList();
// Apply saved library order
final storage = await StorageService.getInstance();
@@ -78,16 +139,28 @@ class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixi
final orderedLibraries = _applyLibraryOrder(filteredLibraries, savedOrder);
_libraries = orderedLibraries;
// Track which servers actually responded so [syncToOnlineServers] can tell
// a genuinely new server from one already covered. Keyed on fetch success
// (not on which servers returned libraries) so a zero-library server still
// counts as loaded, while a server whose fetch failed is left out and
// retried on the next status emission.
_loadedServerIds = result.succeededServerIds;
_loadState = LibrariesLoadState.loaded;
_errorMessage = null;
appLogger.i('LibrariesProvider: Loaded ${_libraries.length} libraries');
safeNotifyListeners();
return true;
} catch (e, stackTrace) {
appLogger.e('LibrariesProvider: Failed to load libraries', error: e, stackTrace: stackTrace);
// A refresh that fails over an existing list keeps the last good data and
// `loaded` state instead of blanking to an error screen; the next status
// emission re-drives the sync.
if (reloadInPlace) return false;
_loadState = LibrariesLoadState.error;
_errorMessage = e.toString();
safeNotifyListeners();
return false;
}
}
@@ -118,6 +191,8 @@ class LibrariesProvider extends ChangeNotifier with DisposableChangeNotifierMixi
_libraries = [];
_loadState = LibrariesLoadState.initial;
_errorMessage = null;
_loadedServerIds = {};
_hasPendingLoad = false;
safeNotifyListeners();
appLogger.d('LibrariesProvider: Cleared library data');
}
+14
View File
@@ -40,6 +40,14 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi
/// Previously-seen set of online server IDs, used to detect new servers
Set<String> _previousOnlineServerIds = {};
/// Invoked with the current visibility-filtered online server ids whenever
/// the manager's status stream fires (a server connects, reconnects, drops,
/// or its auth state changes). Lets `LibrariesProvider` reload when the
/// online set grows — servers bind in waves and slow ones reconnect after
/// the initial load — without coupling the two providers by type. Wired once
/// in `main.dart`.
void Function(Set<String> onlineServerIds)? onOnlineServersChanged;
/// 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].
@@ -136,6 +144,12 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi
safeNotifyListeners();
// Reload libraries when the online set changes. LibrariesProvider owns
// the "is anything actually new to me?" decision (its loaded set can
// differ from _previousOnlineServerIds after a load error or a profile
// switch that cleared it), so notify unconditionally and let it decide.
onOnlineServersChanged?.call(currentOnline);
// Only re-check live TV when a new server came online
if (hasNewServer) {
checkLiveTvAvailability();
+19 -6
View File
@@ -23,23 +23,34 @@ class DataAggregationService {
DataAggregationService(this._serverManager);
/// Fetch libraries from all online clients regardless of backend, returning
/// neutral [MediaLibrary]s.
Future<List<MediaLibrary>> getMediaLibrariesFromAllServers() async {
/// the merged neutral [MediaLibrary]s alongside the ids of the servers whose
/// fetch actually succeeded.
///
/// A per-server `fetchLibraries()` failure is swallowed (that server simply
/// contributes no libraries) so one unreachable server doesn't sink the whole
/// list. [succeededServerIds] lets callers tell a *failed* fetch apart from a
/// server that genuinely has no libraries — both contribute nothing, so
/// conflating them would let a transient failure be cached as "loaded" and
/// never retried.
Future<({List<MediaLibrary> libraries, Set<String> succeededServerIds})> getMediaLibrariesFromAllServers() async {
final clients = _serverManager.onlineClients;
if (clients.isEmpty) {
appLogger.w('No online servers available for fetching libraries (neutral)');
return [];
return (libraries: const <MediaLibrary>[], succeededServerIds: const <String>{});
}
final succeededServerIds = <String>{};
final futures = clients.entries.map((entry) async {
try {
return await entry.value.fetchLibraries();
final libraries = await entry.value.fetchLibraries();
succeededServerIds.add(entry.key);
return libraries;
} catch (e, stackTrace) {
appLogger.e('Failed neutral library fetch from ${entry.key}', error: e, stackTrace: stackTrace);
return <MediaLibrary>[];
}
});
final results = await Future.wait(futures);
return [for (final list in results) ...list];
return (libraries: [for (final list in results) ...list], succeededServerIds: succeededServerIds);
}
/// Fetch "On Deck" (Continue Watching) from all servers and merge by recency.
@@ -243,7 +254,9 @@ class DataAggregationService {
// Only fallback clients need a library prefetch when home layout is on;
// rich-hub backends return the intended home rows directly.
final needsLibraryPrefetch = useGlobalHubs && clients.values.any((client) => !client.capabilities.richHubs);
final libraries = needsLibraryPrefetch ? _groupLibrariesByServer(await getMediaLibrariesFromAllServers()) : null;
final libraries = needsLibraryPrefetch
? _groupLibrariesByServer((await getMediaLibrariesFromAllServers()).libraries)
: null;
final futures = clients.entries.map((entry) async {
final serverId = entry.key;
+267
View File
@@ -1,8 +1,13 @@
import 'dart:async';
import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/media/media_backend.dart';
import 'package:plezy/media/media_kind.dart';
import 'package:plezy/media/media_library.dart';
import 'package:plezy/media/media_server_client.dart';
import 'package:plezy/providers/libraries_provider.dart';
import 'package:plezy/services/data_aggregation_service.dart';
import 'package:plezy/services/multi_server_manager.dart';
import 'package:plezy/services/storage_service.dart';
import '../test_helpers/prefs.dart';
@@ -15,6 +20,46 @@ MediaLibrary _lib(String key, {String type = 'movie', String? serverId, String t
serverId: serverId,
);
MediaLibrary _serverLib(String serverId, String id, String title) =>
MediaLibrary(id: id, backend: MediaBackend.plex, title: title, kind: MediaKind.movie, serverId: serverId);
/// Minimal [MediaServerClient] returning canned libraries; only the surface the
/// aggregation service touches is implemented. An optional [gate] lets a test
/// hold `fetchLibraries` open to exercise the mid-load race; setting [error]
/// makes `fetchLibraries` throw, simulating a (possibly transient) failure.
class _FakeClient implements MediaServerClient {
_FakeClient({required this.serverId, this.libraries = const [], this.gate});
@override
final String serverId;
@override
final String serverName = 'Server';
final List<MediaLibrary> libraries;
final Future<void>? gate;
/// When non-null, [fetchLibraries] throws this instead of returning. Mutable
/// so a test can fail a fetch once and then let it recover.
Object? error;
int fetchLibrariesCalls = 0;
@override
Future<List<MediaLibrary>> fetchLibraries() async {
fetchLibrariesCalls++;
final pending = gate;
if (pending != null) await pending;
if (error != null) throw error!;
return libraries;
}
@override
void close() {}
@override
dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation);
}
void main() {
setUp(resetSharedPreferencesForTest);
@@ -115,4 +160,226 @@ void main() {
await p.updateLibraryOrder([_lib('1', serverId: 'srv')]);
});
});
group('LibrariesProvider.syncToOnlineServers', () {
test('loads when a server first comes online', () async {
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A'});
expect(p.hasLoaded, isTrue);
expect(p.libraries.map((l) => l.title), ['Movies A']);
expect(clientA.fetchLibrariesCalls, 1);
p.dispose();
manager.dispose();
});
test('does not reload when the online set is unchanged', () async {
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A'});
await p.syncToOnlineServers({'A'}); // already covered → no-op
expect(clientA.fetchLibrariesCalls, 1);
p.dispose();
manager.dispose();
});
test('reloads and surfaces a server that connects after the first load', () async {
// The bug: a server binding in a later wave (borrowed connection, or a
// slow server reconnecting after timing out) was never picked up because
// the load was one-shot.
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A'});
expect(p.libraries.map((l) => l.title), ['Movies A']);
final clientB = _FakeClient(serverId: 'B', libraries: [_serverLib('B', '1', 'Shows B')]);
manager.debugRegisterClientForTesting(clientB);
await p.syncToOnlineServers({'A', 'B'});
expect(p.libraries.map((l) => l.title), containsAll(<String>['Movies A', 'Shows B']));
expect(clientA.fetchLibrariesCalls, 2);
expect(clientB.fetchLibrariesCalls, 1);
p.dispose();
manager.dispose();
});
test('a background reload over existing data never surfaces a loading state', () async {
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A'});
expect(p.hasLoaded, isTrue);
// A server connecting later must not flip the provider back to a loading
// state — screens render `isLoading` as a full-screen spinner, so a
// background reload would blank content the user is already viewing.
final sawLoading = <bool>[];
p.addListener(() => sawLoading.add(p.isLoading));
final clientB = _FakeClient(serverId: 'B', libraries: [_serverLib('B', '1', 'Shows B')]);
manager.debugRegisterClientForTesting(clientB);
await p.syncToOnlineServers({'A', 'B'});
expect(sawLoading, isNot(contains(true)));
expect(p.libraries.map((l) => l.title), containsAll(<String>['Movies A', 'Shows B']));
p.dispose();
manager.dispose();
});
test('a server whose fetch fails is retried on the next sync, not cached as loaded', () async {
// Regression: getMediaLibrariesFromAllServers swallows a per-server fetch
// failure and returns no libraries for it — identical to a genuinely empty
// server. Keying loaded-state on fetch *success* keeps a transiently
// failed server out of _loadedServerIds so it reloads instead of staying
// missing until a profile re-switch/restart.
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
final clientB = _FakeClient(serverId: 'B', libraries: [_serverLib('B', '1', 'Shows B')])
..error = Exception('transient');
manager.debugRegisterClientForTesting(clientA);
manager.debugRegisterClientForTesting(clientB);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A', 'B'});
// A loaded; B's fetch failed, so it is absent and must not be recorded.
expect(p.libraries.map((l) => l.title), ['Movies A']);
// B recovers. The same online set must now reload it rather than treating
// B as already covered.
clientB.error = null;
await p.syncToOnlineServers({'A', 'B'});
expect(p.libraries.map((l) => l.title), containsAll(<String>['Movies A', 'Shows B']));
p.dispose();
manager.dispose();
});
test('does not reload when the online set shrinks', () async {
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
final clientB = _FakeClient(serverId: 'B', libraries: [_serverLib('B', '1', 'Shows B')]);
manager.debugRegisterClientForTesting(clientA);
manager.debugRegisterClientForTesting(clientB);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A', 'B'});
expect(clientA.fetchLibrariesCalls, 1);
// A drops; the visible online set is now a subset of what we loaded.
await p.syncToOnlineServers({'B'});
expect(clientA.fetchLibrariesCalls, 1);
expect(clientB.fetchLibrariesCalls, 1);
p.dispose();
manager.dispose();
});
test('a zero-library server is marked loaded and does not retrigger', () async {
final manager = MultiServerManager();
final clientC = _FakeClient(serverId: 'C', libraries: const []);
manager.debugRegisterClientForTesting(clientC);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'C'});
expect(p.hasLoaded, isTrue);
expect(p.libraries, isEmpty);
expect(clientC.fetchLibrariesCalls, 1);
// Tracking the requested set (not deriving from loaded libraries) is what
// stops a zero-library server from looking "never loaded" and reloading
// on every status emission.
await p.syncToOnlineServers({'C'});
expect(clientC.fetchLibrariesCalls, 1);
p.dispose();
manager.dispose();
});
test('a server appearing mid-load is still picked up', () async {
final manager = MultiServerManager();
final gate = Completer<void>();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')], gate: gate.future);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
// First load starts and suspends on A's gated fetch.
final inFlight = p.syncToOnlineServers({'A'});
// B comes online before the first load completes.
final clientB = _FakeClient(serverId: 'B', libraries: [_serverLib('B', '1', 'Shows B')]);
manager.debugRegisterClientForTesting(clientB);
unawaited(p.syncToOnlineServers({'A', 'B'})); // queued behind the in-flight pass
gate.complete();
await inFlight; // resolves after the replayed pass covering {A, B}
expect(p.libraries.map((l) => l.title), containsAll(<String>['Movies A', 'Shows B']));
expect(clientA.fetchLibrariesCalls, 2, reason: 'a second pass runs for the larger set');
p.dispose();
manager.dispose();
});
test('clear() resets tracking so the next sync reloads', () async {
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers({'A'});
expect(clientA.fetchLibrariesCalls, 1);
p.clear();
expect(p.hasLoaded, isFalse);
await p.syncToOnlineServers({'A'});
expect(clientA.fetchLibrariesCalls, 2);
p.dispose();
manager.dispose();
});
test('is a no-op for an empty set or before initialize', () async {
final manager = MultiServerManager();
final clientA = _FakeClient(serverId: 'A', libraries: [_serverLib('A', '1', 'Movies A')]);
manager.debugRegisterClientForTesting(clientA);
// Empty set on an initialized provider.
final p = LibrariesProvider()..initialize(DataAggregationService(manager));
await p.syncToOnlineServers(<String>{});
expect(p.loadState, LibrariesLoadState.initial);
expect(clientA.fetchLibrariesCalls, 0);
p.dispose();
// Non-empty set on an uninitialized provider.
final p2 = LibrariesProvider();
var notified = 0;
p2.addListener(() => notified++);
await p2.syncToOnlineServers({'A'});
expect(p2.loadState, LibrariesLoadState.initial);
expect(notified, 0);
p2.dispose();
manager.dispose();
});
});
}
@@ -82,6 +82,26 @@ void main() {
p.dispose();
});
test('invokes onOnlineServersChanged with the visibility-filtered online set', () async {
final p = MultiServerProvider(manager, aggregation);
final calls = <Set<String>>[];
p.onOnlineServersChanged = calls.add;
manager.updateServerStatus('srv-1', true);
await Future<void>.delayed(Duration.zero);
expect(calls, isNotEmpty);
expect(calls.last, {'srv-1'});
// A server that is online in the manager but outside the active profile's
// visibility filter must not appear in the payload.
p.setVisibleServerIds({'srv-1'});
manager.updateServerStatus('srv-2', true);
await Future<void>.delayed(Duration.zero);
expect(calls.last, {'srv-1'}, reason: 'srv-2 is online but filtered out');
p.dispose();
});
test('checkServerHealth with no clients completes without error', () async {
final p = MultiServerProvider(manager, aggregation);
// Empty clients map → no work, but the call must complete.
@@ -51,7 +51,9 @@ void main() {
group('DataAggregationService cross-server aggregation', () {
test('getMediaLibrariesFromAllServers returns empty when no clients connected', () async {
expect(await service.getMediaLibrariesFromAllServers(), isEmpty);
final result = await service.getMediaLibrariesFromAllServers();
expect(result.libraries, isEmpty);
expect(result.succeededServerIds, isEmpty);
});
test('searchAcrossServers and getOnDeckFromAllServers return empty when no clients', () async {