diff --git a/lib/navigation/profile_session_screen.dart b/lib/navigation/profile_session_screen.dart index 14a3bc74..d7754462 100644 --- a/lib/navigation/profile_session_screen.dart +++ b/lib/navigation/profile_session_screen.dart @@ -17,6 +17,7 @@ import '../providers/trakt_account_provider.dart'; import '../providers/trackers_provider.dart'; import '../providers/watch_state_store.dart'; import '../screens/main_screen.dart'; +import '../services/api_cache.dart'; import '../services/storage_service.dart'; import '../utils/app_logger.dart'; import '../watch_together/providers/watch_together_provider.dart'; @@ -60,6 +61,9 @@ class _ProfileSessionScreenState extends State { // callback rather than during build to avoid mutating state mid-build. bool _hasBuiltSession = false; + bool _seenFirstActiveId = false; + String? _lastSessionActiveId; + @override void initState() { super.initState(); @@ -68,11 +72,30 @@ class _ProfileSessionScreenState extends State { }); } + /// The keyed remount below recreates every session-scoped provider on a + /// profile switch, but [ApiCache] is app-global and its Plex rows are + /// keyed by server only — one home user's cached responses would serve + /// the next user's session. Clear the volatile rows at the seam itself; + /// doing it from inside MainScreen can't work, the remount unmounts it + /// before any settle-await completes. + void _onSessionProfileChanged(String? activeId) { + if (!_seenFirstActiveId) { + _seenFirstActiveId = true; + _lastSessionActiveId = activeId; + return; + } + if (_lastSessionActiveId == activeId) return; + _lastSessionActiveId = activeId; + final cache = ApiCache.maybeInstance; + if (cache != null) unawaited(cache.clearVolatile()); + } + @override Widget build(BuildContext context) { return Consumer( builder: (context, activeProfile, _) { final activeId = activeProfile.activeId; + _onSessionProfileChanged(activeId); final initialPromptHandled = widget.initialPromptHandled || _hasBuiltSession; return KeyedSubtree( key: ValueKey('profile-session:$activeId'), diff --git a/lib/profiles/active_profile_binder.dart b/lib/profiles/active_profile_binder.dart index 7014f89f..7a7fb2e2 100644 --- a/lib/profiles/active_profile_binder.dart +++ b/lib/profiles/active_profile_binder.dart @@ -75,6 +75,13 @@ class ActiveProfileBinder { bool _isSwitching = false; String? _lastBoundProfileId; String? _bindingProfileId; + + /// Profile whose most recent bind failed (PIN cancel, offline, error). + /// Passive provider notifications must not retry it — mid-session retries + /// bypass the token cache, so a protected Plex Home profile would pop a + /// PIN dialog with no user action. Explicit paths ([rebindActive], a + /// user-initiated activation, a pre-verified switch) clear the marker. + String? _lastFailedProfileId; bool _pendingRebind = false; // Set when something asks for a rebind of the *currently-active* profile // while a rebind is already in flight. The normal `_pendingRebind` path @@ -103,10 +110,12 @@ class ActiveProfileBinder { void markPlexHomePreVerified(String profileId) { _plexHomePreVerified.add(profileId); + if (_lastFailedProfileId == profileId) _lastFailedProfileId = null; } void markUserInitiatedActivation(String profileId) { _userInitiatedActivations.add(profileId); + if (_lastFailedProfileId == profileId) _lastFailedProfileId = null; } @visibleForTesting @@ -163,6 +172,9 @@ class ActiveProfileBinder { return; } if (id == _lastBoundProfileId) return; + // Don't retry a failed profile from a passive notification — see + // [_lastFailedProfileId]. A different profile id still rebinds. + if (id != null && id == _lastFailedProfileId) return; unawaited(_rebind()); } @@ -175,6 +187,7 @@ class ActiveProfileBinder { /// Safe to call while a rebind is in flight; the request is queued and /// the loop runs an extra pass when the current one settles. Future rebindActive() async { + _lastFailedProfileId = null; if (_isSwitching) { _pendingSameIdRebind = true; return; @@ -193,11 +206,18 @@ class ActiveProfileBinder { Future _rebind() async { if (_isSwitching) return; _isSwitching = true; + // Binding is marked per CYCLE, not per pass: `awaitBindingSettle` + // waiters must observe the FINAL outcome. Settling between passes hands + // a caller who activated profile B mid-pass the outcome of profile A's + // pass — reporting a switch as succeeded/failed before B's bind ran. + _bindingProfileId = activeProfile.activeId; + activeProfile.markBindingStarted(); + var success = false; try { do { _pendingRebind = false; _pendingSameIdRebind = false; - await _runRebindOnce(); + success = await _runRebindOnce(); // Loop only when the active id has drifted to something we haven't // bound yet, OR when an explicit same-id rebind was queued (borrow // / connection-list mutation while a rebind was in flight). Bare @@ -206,13 +226,17 @@ class ActiveProfileBinder { // re-asserts). } while (_pendingSameIdRebind || (_pendingRebind && activeProfile.activeId != _lastBoundProfileId)); } finally { + // Notify while `_isSwitching`/`_bindingProfileId` still attribute the + // notification to this cycle — otherwise the binder's own listener + // would treat it as an external change and immediately re-rebind. + activeProfile.markBindingFinished(success: success); + _bindingProfileId = null; _isSwitching = false; } } - Future _runRebindOnce() async { + Future _runRebindOnce() async { _bindingProfileId = activeProfile.activeId; - activeProfile.markBindingStarted(); final stopwatch = Stopwatch()..start(); var success = false; String? attemptedProfileId; @@ -226,7 +250,7 @@ class ActiveProfileBinder { // profile cannot leak into the no-selection state. _clearBoundServers(); success = true; - return; + return success; } attemptedProfileId = profile.id; @@ -236,27 +260,43 @@ class ActiveProfileBinder { _clearBoundServers(); attemptedProfileId = null; success = true; - return; + return success; } appLogger.i('ActiveProfileBinder: rebinding for ${profile.displayName} (${profile.id})'); - final expectedServerIds = await _expectedServerIdsForProfile(profile); + // One snapshot of the join rows + connections per pass — every + // downstream helper reads from these instead of re-querying (each + // registry read pays per-row CredentialVault reveals). + final joinRows = await profileConnections.listForProfile(profile.id); + final connectionsById = {for (final c in await connections.list()) c.id: c}; + + // PIN prompts may only surface from a user-initiated bind or the + // session's initial bind (cold-start resume). Passive rebinds — an + // hourly Plex Home refresh, an unrelated table write — must never pop + // a modal PIN dialog over whatever the user is doing. + final allowPinPrompt = userInitiated || !_hasBoundOnce; + + final expectedServerIds = _expectedServerIdsForProfile( + profile, + joinRows: joinRows, + connectionsById: connectionsById, + ); multiServerProvider.setExpectedVisibleServerIds(expectedServerIds); - final localProfileHasJoinRows = - profile.isLocal && (await profileConnections.listForProfile(profile.id)).isNotEmpty; + final localProfileHasJoinRows = profile.isLocal && joinRows.isNotEmpty; // Bind the implicit Plex Home parent and borrowed/extra join rows in // parallel. A slow/offline Plex parent should not add its timeout budget // on top of an otherwise reachable Jellyfin or borrowed-server bind. final results = await Future.wait([ - if (profile.isPlexHome) _bindPlexHome(profile), + if (profile.isPlexHome) + _bindPlexHome(profile, joinRows: joinRows, connectionsById: connectionsById, allowPinPrompt: allowPinPrompt), // Both kinds also bind borrowed/extra connections via the join table. // For plex_home this handles a Jellyfin server (or extra Plex account) // that was attached to the profile via the borrow flow — the parent // account is bound by `_bindPlexHome` above and isn't represented in // the join table. - _bindJoinRows(profile), + _bindJoinRows(profile, joinRows: joinRows, connectionsById: connectionsById, allowPinPrompt: allowPinPrompt), ]); final visibleServerIds = {}; for (final result in results) { @@ -287,35 +327,37 @@ class ActiveProfileBinder { } finally { if (success) { _lastBoundProfileId = attemptedProfileId; - } else if (_lastBoundProfileId == attemptedProfileId) { - _lastBoundProfileId = null; + _lastFailedProfileId = null; + } else { + if (_lastBoundProfileId == attemptedProfileId) { + _lastBoundProfileId = null; + } + _lastFailedProfileId = attemptedProfileId; } appLogger.i( 'ActiveProfileBinder: rebind settled', error: {'profileId': attemptedProfileId, 'success': success, 'elapsedMs': stopwatch.elapsedMilliseconds}, ); - activeProfile.markBindingFinished(success: success); - _bindingProfileId = null; } + return success; } - Future> _expectedServerIdsForProfile(Profile profile) async { + Set _expectedServerIdsForProfile( + Profile profile, { + required List joinRows, + required Map connectionsById, + }) { final expected = {}; final parentId = profile.parentConnectionId; if (profile.isPlexHome && parentId != null) { - final account = await connections.getPlexAccount(parentId); - if (account != null) { - expected.addAll(account.servers.map((server) => server.clientIdentifier)); + if (connectionsById[parentId] case PlexAccountConnection(:final servers)) { + expected.addAll(servers.map((server) => server.clientIdentifier)); } } - final pcs = await profileConnections.listForProfile(profile.id); - if (pcs.isEmpty) return expected; - final all = await connections.list(); - final byId = {for (final c in all) c.id: c}; - for (final pc in pcs) { + for (final pc in joinRows) { if (parentId != null && pc.connectionId == parentId) continue; - switch (byId[pc.connectionId]) { + switch (connectionsById[pc.connectionId]) { case PlexAccountConnection(:final servers): expected.addAll(servers.map((server) => server.clientIdentifier)); case JellyfinConnection(:final serverMachineId): @@ -327,14 +369,22 @@ class ActiveProfileBinder { return expected; } - Future<_ProfileBindResult> _bindPlexHome(Profile profile) async { + Future<_ProfileBindResult> _bindPlexHome( + Profile profile, { + required List joinRows, + required Map connectionsById, + required bool allowPinPrompt, + }) async { final parentId = profile.parentConnectionId; final homeUuid = profile.plexHomeUserUuid; if (parentId == null || homeUuid == null) { appLogger.w('ActiveProfileBinder: ${profile.displayName} missing parent/uuid metadata'); return const _ProfileBindResult.empty(); } - final account = await connections.getPlexAccount(parentId); + final account = switch (connectionsById[parentId]) { + final PlexAccountConnection a => a, + _ => null, + }; if (account == null) { appLogger.w('ActiveProfileBinder: parent connection $parentId for ${profile.displayName} not found'); return const _ProfileBindResult.empty(); @@ -348,10 +398,17 @@ class ActiveProfileBinder { // needed. A just-preverified activation also uses the fresh cache once to // avoid a redundant second prompt. final preVerified = consumePlexHomePreVerified(profile.id); + final allowPin = allowPinPrompt || preVerified; final useCache = shouldUsePlexHomeTokenCache(preVerified: preVerified, hasBoundOnce: _hasBoundOnce); String? cachedToken; if (useCache) { - final pc = await profileConnections.get(profile.id, parentId); + ProfileConnection? pc; + for (final row in joinRows) { + if (row.connectionId == parentId) { + pc = row; + break; + } + } cachedToken = pc?.hasToken == true ? pc!.userToken : null; } appLogger.d( @@ -415,13 +472,21 @@ class ActiveProfileBinder { } } + if (!allowPin && profile.plexProtected) { + appLogger.i('ActiveProfileBinder: suppressing PIN-gated /switch for passive rebind of ${profile.displayName}'); + return const _ProfileBindResult.empty(); + } appLogger.i('ActiveProfileBinder: minting fresh user-token via /switch for ${profile.displayName}'); final result = await switchPlexHomeUserWithPin( auth: auth, accountToken: account.accountToken, homeUserUuid: homeUuid, requiresPin: profile.plexProtected, - promptForPin: ({String? errorMessage}) => pinPrompt(profile, errorMessage: errorMessage), + // Plex can demand a PIN (error 1041) even when we didn't expect one; + // a passive rebind answers that demand with a cancel, not a dialog. + promptForPin: allowPin + ? ({String? errorMessage}) => pinPrompt(profile, errorMessage: errorMessage) + : ({String? errorMessage}) async => null, logLabel: profile.displayName, ); if (!result.succeeded) return const _ProfileBindResult.empty(); @@ -454,24 +519,26 @@ class ActiveProfileBinder { /// join table). Skips Plex rows whose `connectionId` matches the parent /// (defensive guard — sync code shouldn't insert one, but treating it as /// a borrow would re-mint a redundant token). - Future<_ProfileBindResult> _bindJoinRows(Profile profile) async { - final pcs = await profileConnections.listForProfile(profile.id); - if (pcs.isEmpty) { + Future<_ProfileBindResult> _bindJoinRows( + Profile profile, { + required List joinRows, + required Map connectionsById, + required bool allowPinPrompt, + }) async { + if (joinRows.isEmpty) { if (profile.isLocal) { appLogger.w('ActiveProfileBinder: ${profile.displayName} has no connections'); } return const _ProfileBindResult.empty(); } - final all = await connections.list(); - final byId = {for (final c in all) c.id: c}; final parentId = profile.parentConnectionId; final visible = {}; final expected = {}; final futures = >[]; - for (final pc in pcs) { + for (final pc in joinRows) { if (parentId != null && pc.connectionId == parentId) continue; - final conn = byId[pc.connectionId]; + final conn = connectionsById[pc.connectionId]; if (conn == null) { appLogger.w('ActiveProfileBinder: missing connection ${pc.connectionId} for ${profile.displayName}'); continue; @@ -479,7 +546,7 @@ class ActiveProfileBinder { switch (conn) { case PlexAccountConnection(): expected.addAll(conn.servers.map((server) => server.clientIdentifier)); - futures.add(_bindLocalPlexConnection(profile: profile, conn: conn, pc: pc)); + futures.add(_bindLocalPlexConnection(profile: profile, conn: conn, pc: pc, allowPinPrompt: allowPinPrompt)); case JellyfinConnection(): expected.add(conn.serverMachineId); futures.add(_bindJellyfin(conn)); @@ -497,6 +564,7 @@ class ActiveProfileBinder { required Profile profile, required PlexAccountConnection conn, required ProfileConnection pc, + required bool allowPinPrompt, }) async { final auth = await _ensureAuth(); String? userToken = pc.userToken; @@ -550,7 +618,13 @@ class ActiveProfileBinder { appLogger.w('ActiveProfileBinder: ${profile.displayName} has no Plex Home user identifier'); return const _ProfileBindResult.empty(); } - final minted = await _mintLocalPlexToken(auth: auth, profile: profile, conn: conn, pc: pc); + final minted = await _mintLocalPlexToken( + auth: auth, + profile: profile, + conn: conn, + pc: pc, + allowPinPrompt: allowPinPrompt, + ); if (minted == null) return const _ProfileBindResult.empty(); userToken = minted; try { @@ -587,15 +661,19 @@ class ActiveProfileBinder { required Profile profile, required PlexAccountConnection conn, required ProfileConnection pc, + required bool allowPinPrompt, }) async { final result = await switchPlexHomeUserWithPin( auth: auth, accountToken: conn.accountToken, homeUserUuid: pc.userIdentifier, // Local profiles don't carry the protected flag; the loop will - // re-prompt if Plex disagrees. + // re-prompt if Plex disagrees — unless this is a passive rebind, which + // answers the demand with a cancel instead of an unsolicited dialog. requiresPin: false, - promptForPin: ({String? errorMessage}) => pinPrompt(profile, errorMessage: errorMessage), + promptForPin: allowPinPrompt + ? ({String? errorMessage}) => pinPrompt(profile, errorMessage: errorMessage) + : ({String? errorMessage}) async => null, logLabel: profile.displayName, ); if (!result.succeeded) return null; @@ -891,6 +969,7 @@ class ActiveProfileBinder { activeProfile.removeListener(_onActiveProfileChanged); _plexHomePreVerified.clear(); _userInitiatedActivations.clear(); + _lastFailedProfileId = null; _plexAuth?.dispose(); _plexAuth = null; _started = false; diff --git a/lib/profiles/active_profile_provider.dart b/lib/profiles/active_profile_provider.dart index 665567a7..802ea092 100644 --- a/lib/profiles/active_profile_provider.dart +++ b/lib/profiles/active_profile_provider.dart @@ -1,4 +1,5 @@ import 'dart:async'; +import 'dart:convert'; import 'package:flutter/foundation.dart'; @@ -136,14 +137,21 @@ class ActiveProfileProvider extends ChangeNotifier with DisposableChangeNotifier Future _initialize() async { await _reloadSnapshot(); + // Every listener diffs its snapshot first: drift re-emits on any table + // write (including no-op upserts like the binder's server refresh), and + // each unchecked pass rebuilds every Profile and notifies the whole + // listener tree (binder, MainScreen, pickers). _localSub = _registry.watchProfiles().listen((list) { + if (listEquals(list, _localProfiles)) return; _localProfiles = list; _recomputeProfiles(); _resolveActive(); safeNotifyListeners(); }); _connSub = _connections.watchConnections().listen((list) { - _connectionsById = {for (final c in list) c.id: c}; + final byId = {for (final c in list) c.id: c}; + if (_sameConnections(byId, _connectionsById)) return; + _connectionsById = byId; _recomputeProfiles(); _resolveActive(); safeNotifyListeners(); @@ -173,6 +181,19 @@ class ActiveProfileProvider extends ChangeNotifier with DisposableChangeNotifier _resolveActive(); } + /// [Connection] has no value equality; its persisted config is the + /// cheapest faithful comparison key for the handful of rows involved. + static bool _sameConnections(Map a, Map b) { + if (a.length != b.length) return false; + for (final entry in a.entries) { + final other = b[entry.key]; + if (other == null) return false; + if (identical(entry.value, other)) continue; + if (jsonEncode(entry.value.toConfigJson()) != jsonEncode(other.toConfigJson())) return false; + } + return true; + } + void _recomputeProfiles() { _profiles = mergeLocalWithPlexHome( locals: _localProfiles, @@ -202,15 +223,13 @@ class ActiveProfileProvider extends ChangeNotifier with DisposableChangeNotifier return; } } - // Saved id no longer matches anything (e.g. Plex admin removed the - // home user). Clear it so storage-scoped settings do not keep reading - // and writing under the removed profile's user scope, and let the UI - // force an explicit profile choice. + // Saved id no longer matches anything. Keep the persisted id: this + // resolver runs on every stream emission, and early/partial snapshots + // (boot before migration, a Plex Home cache that hasn't hydrated yet) + // must not irreversibly wipe the user's selection. Genuinely + // unresolvable states clear the id at their decision points — the boot + // guard and the post-removal settle flow. _active = null; - final storage = _storage; - if (storage != null) { - unawaited(storage.clearActiveProfileId()); - } } /// Activate [profile]. PIN-protected local profiles must supply a matching diff --git a/lib/profiles/plex_home_service.dart b/lib/profiles/plex_home_service.dart index fda56315..15dcd8bd 100644 --- a/lib/profiles/plex_home_service.dart +++ b/lib/profiles/plex_home_service.dart @@ -179,8 +179,23 @@ class PlexHomeService { _storage = storage; try { final users = await _fetchHomeUsers(conn.accountToken); + // The account may have been removed while the fetch was in flight — + // caching now would resurrect its home users (and virtual profiles) + // as ghosts until the next removal event. + if (await _connections.get(conn.id) == null) { + appLogger.d('PlexHomeService: dropping fetch result for removed account ${conn.accountLabel}'); + return; + } + final encoded = users.map((u) => u.toJson()).toList(); + // Unchanged fetches (the hourly ticker, mostly) must not emit: every + // emission fans out through ActiveProfileProvider into a full + // recompute/notify cascade across the app. + if (_byConnection.containsKey(conn.id) && storage.getPlexHomeUsersCacheJson(conn.id) == jsonEncode(encoded)) { + appLogger.d('PlexHomeService: home users unchanged for ${conn.accountLabel}'); + return; + } _byConnection[conn.id] = users; - await storage.savePlexHomeUsersCache(conn.id, users.map((u) => u.toJson()).toList()); + await storage.savePlexHomeUsersCache(conn.id, encoded); _emit(); appLogger.d('PlexHomeService: cached ${users.length} home users for ${conn.accountLabel}'); } catch (e, st) { diff --git a/lib/providers/multi_server_provider.dart b/lib/providers/multi_server_provider.dart index b68ca642..ed48a288 100644 --- a/lib/providers/multi_server_provider.dart +++ b/lib/providers/multi_server_provider.dart @@ -106,7 +106,10 @@ class MultiServerProvider extends ChangeNotifier with DisposableChangeNotifierMi _expectedVisibleServerIds!.containsAll(ids)) { return; } - _expectedVisibleServerIds = ids; + // Defensive copy: callers (the binder) keep mutating their set after + // handing it over, which would silently edit provider state and defeat + // the idempotence check above. + _expectedVisibleServerIds = ids == null ? null : Set.of(ids); safeNotifyListeners(); } diff --git a/lib/providers/user_profile_provider.dart b/lib/providers/user_profile_provider.dart index bb5db7cb..6328d203 100644 --- a/lib/providers/user_profile_provider.dart +++ b/lib/providers/user_profile_provider.dart @@ -5,7 +5,6 @@ import 'package:flutter/foundation.dart'; import '../connection/connection.dart'; import '../connection/connection_registry.dart'; -import '../i18n/strings.g.dart'; import '../media/media_server_user_profile.dart'; import '../mixins/disposable_change_notifier_mixin.dart'; import '../profiles/active_profile_provider.dart'; @@ -37,13 +36,9 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi UserProfileProvider({StorageService? storageService}) : _storageService = storageService; MediaServerUserProfile? _profileSettings; - bool _isLoading = false; - String? _error; bool _isInitialized = false; MediaServerUserProfile? get profileSettings => _profileSettings; - bool get isLoading => _isLoading; - String? get error => _error; PlexAuthService? _authService; StorageService? _storageService; @@ -55,6 +50,7 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi StreamSubscription>? _profileConnectionSubscription; String? _watchedProfileConnectionProfileId; ProfileConnectionRegistry? _watchedProfileConnectionRegistry; + String? _watchedProfileConnectionFingerprint; /// Wire the dependencies needed to resolve the active user's token / client. /// May be called multiple times (proxy provider re-builds) — only the @@ -95,6 +91,11 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi final id = ap.activeId; if (id == _lastSeenActiveId) return; _lastSeenActiveId = id; + // The previous profile's settings must not bleed into the new profile + // (playback defaults, parental restrictions) while the fetch runs — or + // permanently, when the fetch fails/is unavailable. + _profileSettings = null; + safeNotifyListeners(); _watchActiveProfileConnections(ap.active); if (_isInitialized) unawaited(refreshProfileSettings()); } @@ -110,9 +111,23 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi _profileConnectionSubscription = null; _watchedProfileConnectionRegistry = registry; _watchedProfileConnectionProfileId = profileId; + _watchedProfileConnectionFingerprint = null; if (registry == null || profileId == null) return; - _profileConnectionSubscription = registry.watchForProfile(profileId).listen((_) { + _profileConnectionSubscription = registry.watchForProfile(profileId).listen((rows) { + // Refresh only when something settings-relevant changed. The binder + // bumps lastUsedAt on every bind (markUsed), and drift re-emits on + // each of those writes — refetching plex.tv settings for them is + // wasted round-trips that also wake every awaitBindingSettle path. + final fingerprint = [ + for (final row in rows) '${row.connectionId}|${row.userToken ?? ''}|${row.isDefault}', + ].join(';'); + if (fingerprint == _watchedProfileConnectionFingerprint) return; + final first = _watchedProfileConnectionFingerprint == null; + _watchedProfileConnectionFingerprint = fingerprint; + // The initial emission mirrors the subscribe-time state; the profile + // change that created this subscription already refreshes. + if (first) return; if (_isInitialized) unawaited(refreshProfileSettings()); }); } @@ -134,7 +149,6 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi _isInitialized = true; } catch (e) { appLogger.e('UserProfileProvider: critical initialization failure', error: e); - _setError(t.profiles.initializeServicesFailed); _authService = null; _storageService = null; _isInitialized = false; @@ -150,6 +164,11 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi // read the freshly-minted user-token rather than racing the cache. await _activeProfile?.awaitBindingSettle(); + // A late-landing fetch must not clobber another profile's settings — + // discard the result when the active profile changed mid-flight. + final requestedId = _activeProfile?.activeId; + bool stale() => _activeProfile?.activeId != requestedId; + final settingsConnection = await _resolveActiveSettingsConnection(); final connection = settingsConnection?.connection; if (connection is JellyfinConnection) { @@ -159,7 +178,7 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi return; } final profile = await jellyfinClient.fetchUserProfile(); - if (profile != null) { + if (profile != null && !stale()) { _profileSettings = profile; safeNotifyListeners(); } @@ -175,6 +194,7 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi try { _authService ??= await PlexAuthService.create(); final profile = await _authService!.getUserProfile(userToken); + if (stale()) return; _profileSettings = profile; safeNotifyListeners(); } catch (e) { @@ -285,8 +305,6 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi /// screen "sign out" action; the rest of the teardown (clearing /// connections, profiles, etc.) happens in the screen's logout flow. Future logout() async { - _isLoading = true; - safeNotifyListeners(); try { _storageService ??= await StorageService.getInstance(); await _storageService!.clearUserData(); @@ -294,25 +312,14 @@ class UserProfileProvider extends ChangeNotifier with DisposableChangeNotifierMi _authService = null; _storageService = null; _isInitialized = false; - _clearError(); appLogger.i('UserProfileProvider: logged out'); } catch (e) { appLogger.e('UserProfileProvider: logout error', error: e); } finally { - _isLoading = false; safeNotifyListeners(); } } - void _setError(String error) { - _error = error; - safeNotifyListeners(); - } - - void _clearError() { - _error = null; - } - @override void dispose() { _activeProfile?.removeListener(_onActiveProfileChanged); diff --git a/lib/screens/main_screen.dart b/lib/screens/main_screen.dart index 03448520..6eaeeaaf 100644 --- a/lib/screens/main_screen.dart +++ b/lib/screens/main_screen.dart @@ -133,7 +133,7 @@ bool shouldPassTvosMenuToSystem({ } @visibleForTesting -enum ProfileInvalidationAction { none, waitForProfileSwitch, invalidateNow } +enum ProfileInvalidationAction { none, invalidateNow } @visibleForTesting ProfileInvalidationAction profileInvalidationAction({ @@ -141,13 +141,13 @@ ProfileInvalidationAction profileInvalidationAction({ required String? currentProfileId, required bool wasBindingPreviously, required bool isBindingNow, - required bool hasPendingProfileSwitchInvalidation, - required String? pendingProfileSwitchInvalidationId, }) { + // An active-id change remounts the whole session subtree + // ([ProfileSessionScreen] keys on the id), recreating this screen and + // every session-scoped provider — nothing to invalidate from here. The + // app-global pieces (ApiCache volatile rows) are cleared at that remount + // seam, where the unmount can't outrun the work. if (currentProfileId != previousProfileId) { - return ProfileInvalidationAction.waitForProfileSwitch; - } - if (hasPendingProfileSwitchInvalidation && pendingProfileSwitchInvalidationId == currentProfileId) { return ProfileInvalidationAction.none; } if (wasBindingPreviously && !isBindingNow) { @@ -242,8 +242,6 @@ 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 @@ -508,53 +506,17 @@ class _MainScreenState extends State currentProfileId: id, wasBindingPreviously: _wasBindingPrev, isBindingNow: isBindingNow, - hasPendingProfileSwitchInvalidation: _hasPendingProfileSwitchInvalidation, - pendingProfileSwitchInvalidationId: _pendingProfileSwitchInvalidationId, ); - - 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 - // here would resolve immediately. Hop to a microtask so the binder's - // 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; - try { - if (_hasPendingProfileSwitchInvalidation && - _pendingProfileSwitchInvalidationId == scheduledProfileId && - activeProfile.activeId == scheduledProfileId) { - await _invalidateAllScreens(); - } - } finally { - if (_hasPendingProfileSwitchInvalidation && _pendingProfileSwitchInvalidationId == scheduledProfileId) { - _hasPendingProfileSwitchInvalidation = false; - _pendingProfileSwitchInvalidationId = null; - } - } - }), - ); - return; - } + _lastSeenProfileId = id; + _wasBindingPrev = isBindingNow; // Same active id, but a rebind cycle for that profile just settled // (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 (action == ProfileInvalidationAction.invalidateNow) { - _wasBindingPrev = isBindingNow; unawaited(_invalidateAllScreens()); - return; } - _wasBindingPrev = isBindingNow; } Future _promptForInitialProfileSelection() async { diff --git a/lib/screens/profile/profile_teardown.dart b/lib/screens/profile/profile_teardown.dart index 109d79c4..efc4b012 100644 --- a/lib/screens/profile/profile_teardown.dart +++ b/lib/screens/profile/profile_teardown.dart @@ -105,6 +105,10 @@ Future settleSessionAfterRemoval(SessionTeardownScope scope, {bool rebindI break; } } + // The removal was a user action: mark the hand-off activation as + // user-initiated so the binder binds it (bypassing the initial-bind + // defer and any suppressed failed-bind marker). + if (next != null) scope.binder.markUserInitiatedActivation(next.id); final activated = next != null && await scope.active.activate(next); if (!activated) { await scope.active.clearActiveProfile(); diff --git a/lib/services/api_cache.dart b/lib/services/api_cache.dart index e2565354..91e55637 100644 --- a/lib/services/api_cache.dart +++ b/lib/services/api_cache.dart @@ -34,6 +34,10 @@ abstract class ApiCache { return _instance!; } + /// Like [instance], but `null` before any backend cache registered — + /// for best-effort callers (nothing cached yet means nothing to clear). + static ApiCache? get maybeInstance => _instance; + static final Map _byBackend = {}; /// Subclasses call this from their own `initialize` to register themselves diff --git a/test/profiles/active_profile_binder_test.dart b/test/profiles/active_profile_binder_test.dart index 4328ff00..63b517c5 100644 --- a/test/profiles/active_profile_binder_test.dart +++ b/test/profiles/active_profile_binder_test.dart @@ -511,6 +511,155 @@ void main() { expect(prepared.manager.refreshCalls, 3); }); }); + + group('rebind cycle semantics', () { + test('queued same-id rebind settles once, after the last pass', () async { + binder.dispose(); + multiServerProvider.dispose(); + + final gated = _GatedJellyfinManager(); + manager = gated; + multiServerProvider = MultiServerProvider(manager, DataAggregationService(manager)); + binder = ActiveProfileBinder( + activeProfile: activeProfile, + connections: connections, + profileConnections: profileConnections, + serverManager: manager, + multiServerProvider: multiServerProvider, + pinPrompt: (_, {String? errorMessage}) async => null, + shouldDeferInitialBind: (_) async => false, + ); + + final profile = await createActiveLocalProfile('local-queued'); + final jellyfin = _jellyfinConnection(); + await connections.upsert(jellyfin); + await profileConnections.upsert( + ProfileConnection(profileId: profile.id, connectionId: jellyfin.id, userIdentifier: jellyfin.userId), + ); + + final cycle = binder.rebindActive(); + await pumpUntil(() async => gated.calls == 1); + + int? callsAtSettle; + unawaited(activeProfile.awaitBindingSettle().then((_) => callsAtSettle = gated.calls)); + unawaited(binder.rebindActive()); // queues a same-id follow-up pass + gated.gate.complete(); + await cycle; + await Future.delayed(Duration.zero); + + expect(gated.calls, 2); + // Waiters must observe the whole cycle, not the first pass's outcome. + expect(callsAtSettle, 2); + expect(activeProfile.isBinding, isFalse); + }); + + test('passive notifications do not retry a failed profile; explicit rebind does', () async { + binder.dispose(); + multiServerProvider.dispose(); + + final failing = _CountingFailingJellyfinManager(); + manager = failing; + multiServerProvider = MultiServerProvider(manager, DataAggregationService(manager)); + binder = ActiveProfileBinder( + activeProfile: activeProfile, + connections: connections, + profileConnections: profileConnections, + serverManager: manager, + multiServerProvider: multiServerProvider, + pinPrompt: (_, {String? errorMessage}) async => null, + shouldDeferInitialBind: (_) async => false, + ); + + final profile = await createActiveLocalProfile('local-failing'); + final jellyfin = _jellyfinConnection(); + await connections.upsert(jellyfin); + await profileConnections.upsert( + ProfileConnection(profileId: profile.id, connectionId: jellyfin.id, userIdentifier: jellyfin.userId), + ); + + binder.start(); + await pumpUntil(() async => failing.calls == 1 && !activeProfile.isBinding); + expect(activeProfile.lastBindingSucceeded, isFalse); + + // A passive data change (an unrelated connection appearing) must not + // re-run the failed bind — mid-session retries can pop PIN prompts. + await connections.upsert(_jellyfinConnection2()); + await Future.delayed(const Duration(milliseconds: 50)); + expect(failing.calls, 1); + + // An explicit rebind clears the marker and retries. + await binder.rebindActive(); + expect(failing.calls, greaterThan(1)); + }); + + test('passive rebind of a protected Plex Home profile never prompts for a PIN', () async { + binder.dispose(); + multiServerProvider.dispose(); + + var pinPrompts = 0; + manager = _CapturingMultiServerManager(); + multiServerProvider = MultiServerProvider(manager, DataAggregationService(manager)); + binder = ActiveProfileBinder( + activeProfile: activeProfile, + connections: connections, + profileConnections: profileConnections, + serverManager: manager, + multiServerProvider: multiServerProvider, + pinPrompt: (_, {String? errorMessage}) async { + pinPrompts++; + return null; + }, + shouldDeferInitialBind: (_) async => false, + plexAuth: PlexAuthService.forTesting( + http: MediaServerHttpClient(client: MockClient((_) async => http.Response('{}', 500))), + ), + ); + + final account = PlexAccountConnection( + id: 'plex.account', + accountToken: 'account-token', + clientIdentifier: 'client-id', + accountLabel: 'Owner', + servers: [_server(accessToken: 'account-server-token')], + createdAt: DateTime(2026, 1, 1), + ); + await connections.upsert(account); + final homeUser = PlexHomeUser( + id: 1, + uuid: 'protected-uuid', + title: 'Protected', + thumb: '', + hasPassword: true, + restricted: false, + updatedAt: null, + admin: false, + guest: false, + protected: true, + ); + fetchedHomeUsers = [homeUser]; + await storage.savePlexHomeUsersCache(account.id, [homeUser.toJson()]); + + // Bind a harmless local profile first so the session crosses the + // cold-start boundary (_hasBoundOnce) — the state in which passive + // rebinds would otherwise PIN-prompt via /switch. + await createActiveLocalProfile('local-first'); + binder.start(); + await pumpUntil(() async => !activeProfile.isBinding && binder.debugLastBoundProfileId == 'local-first'); + + // Activation WITHOUT the user-initiated mark: the binder must fail the + // bind silently instead of popping a PIN dialog. + final protectedProfile = Profile.virtualPlexHome(connectionId: account.id, homeUser: homeUser); + await activeProfile.activate(protectedProfile); + await pumpUntil(() async => !activeProfile.isBinding); + expect(pinPrompts, 0); + expect(activeProfile.lastBindingSucceeded, isFalse); + + // The same switch marked user-initiated prompts (and the spy cancels). + binder.markUserInitiatedActivation(protectedProfile.id); + await binder.rebindActive(); + expect(pinPrompts, 1); + }); + }); } PlexServer _server({required String accessToken}) { @@ -567,6 +716,44 @@ JellyfinConnection _jellyfinConnection() { ); } +JellyfinConnection _jellyfinConnection2() { + return JellyfinConnection( + id: 'jf-other/user-b', + baseUrl: 'https://other.example', + serverName: 'Other', + serverMachineId: 'jf-other', + userId: 'user-b', + userName: 'User B', + accessToken: 'token-b', + deviceId: 'device', + createdAt: DateTime(2026, 1, 2), + ); +} + +class _GatedJellyfinManager extends MultiServerManager { + final Completer gate = Completer(); + int calls = 0; + + @override + Future addJellyfinConnection(JellyfinConnection connection) async { + calls++; + if (calls == 1) await gate.future; + updateServerStatus(ServerId(connection.serverMachineId), true); + return true; + } +} + +class _CountingFailingJellyfinManager extends MultiServerManager { + int calls = 0; + + @override + Future addJellyfinConnection(JellyfinConnection connection) async { + calls++; + updateServerStatus(ServerId(connection.serverMachineId), false); + return false; + } +} + class _CapturingMultiServerManager extends MultiServerManager { int refreshCalls = 0; PlexAccountConnection? lastConnection; diff --git a/test/profiles/active_profile_provider_test.dart b/test/profiles/active_profile_provider_test.dart index c865aa9a..cdfd7ca4 100644 --- a/test/profiles/active_profile_provider_test.dart +++ b/test/profiles/active_profile_provider_test.dart @@ -122,15 +122,35 @@ void main() { expect(provider.active?.displayName, 'Migrated User'); }); - test('initialize clears storage when stored id is stale', () async { - // A previously-active profile that was deleted should not keep - // storage-scoped settings under the removed profile id. + test('initialize keeps a stored id it cannot resolve (transient snapshots must not wipe it)', () async { + // Early snapshots can legitimately miss state (boot before migration, + // Plex Home cache not hydrated yet) — resolution goes inactive but the + // persisted selection survives for a later snapshot to resolve. + // Genuinely unresolvable ids are cleared by the boot guard and the + // post-removal settle flow, not here. await registry.upsert(Profile.local(id: 'p1', displayName: 'Owner', createdAt: DateTime(2026, 1, 1))); await storage.setActiveProfileId('ghost-id-no-longer-exists'); await provider.initialize(); await Future.delayed(Duration.zero); expect(provider.activeId, isNull); - expect(storage.getActiveProfileId(), isNull); + expect(storage.getActiveProfileId(), 'ghost-id-no-longer-exists'); + }); + + test('re-upserting an identical connection does not notify listeners', () async { + await registry.upsert(Profile.local(id: 'p1', displayName: 'Owner', createdAt: DateTime(2026, 1, 1))); + final account = _account('plex.acct'); + await connections.upsert(account); + await provider.initialize(); + await Future.delayed(const Duration(milliseconds: 20)); + + var notifications = 0; + provider.addListener(() => notifications++); + // Same row content — the binder does this on every successful bind + // (persisting refreshed-but-identical server metadata). + await connections.upsert(account); + await Future.delayed(const Duration(milliseconds: 20)); + + expect(notifications, 0); }); test('initialize resolves the stored active profile id', () async { diff --git a/test/providers/user_profile_provider_test.dart b/test/providers/user_profile_provider_test.dart index e00fd305..125235e7 100644 --- a/test/providers/user_profile_provider_test.dart +++ b/test/providers/user_profile_provider_test.dart @@ -20,11 +20,9 @@ void main() { setUp(resetSharedPreferencesForTest); group('UserProfileProvider (settings-only)', () { - test('starts with all-null state and no error', () { + test('starts with null settings', () { final p = UserProfileProvider(); expect(p.profileSettings, isNull); - expect(p.isLoading, isFalse); - expect(p.error, isNull); p.dispose(); }); @@ -43,7 +41,6 @@ void main() { final p = UserProfileProvider(); await p.logout(); expect(p.profileSettings, isNull); - expect(p.error, isNull); p.dispose(); }); diff --git a/test/screens/main_screen_layout_test.dart b/test/screens/main_screen_layout_test.dart index 3bb1dbbd..812a4222 100644 --- a/test/screens/main_screen_layout_test.dart +++ b/test/screens/main_screen_layout_test.dart @@ -70,27 +70,13 @@ void main() { expect(shouldPass(isAppleTV: false), isFalse); }); - test('profile switch waits for one post-bind invalidation', () { + test('profile switch invalidates nothing here — the keyed session remount owns it', () { 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, ); @@ -103,8 +89,6 @@ void main() { currentProfileId: 'owner', wasBindingPreviously: true, isBindingNow: false, - hasPendingProfileSwitchInvalidation: false, - pendingProfileSwitchInvalidationId: null, ), ProfileInvalidationAction.invalidateNow, ); @@ -115,8 +99,6 @@ void main() { currentProfileId: 'owner', wasBindingPreviously: false, isBindingNow: false, - hasPendingProfileSwitchInvalidation: false, - pendingProfileSwitchInvalidationId: null, ), ProfileInvalidationAction.none, );