fix(search): guard stale refresh callbacks

This commit is contained in:
edde746
2026-05-24 07:37:02 +02:00
parent f1a02ade17
commit eeaa831cc2
7 changed files with 102 additions and 34 deletions
+2
View File
@@ -21,6 +21,8 @@ mixin ItemUpdatable<T extends StatefulWidget> on State<T> {
/// If the fetch fails, the error is silently caught and the item will
/// be updated on the next full refresh.
Future<void> updateItem(String itemId) async {
if (!mounted) return;
try {
final serverId = itemServerId;
if (serverId == null) return;
+2
View File
@@ -94,6 +94,8 @@ class _DiscoverScreenState extends State<DiscoverScreen>
/// mixin's single-server [itemServerId] hook.
@override
Future<void> updateItem(String itemId) async {
if (!mounted) return;
try {
final serverId = _serverIdForItem(itemId);
if (serverId == null) return;
+25 -7
View File
@@ -8,6 +8,7 @@ import '../focus/focusable_text_field.dart';
import '../i18n/strings.g.dart';
import '../media/media_item.dart';
import '../mixins/controller_disposer_mixin.dart';
import '../mixins/mounted_set_state_mixin.dart';
import '../mixins/refreshable.dart';
import '../providers/multi_server_provider.dart';
import '../utils/app_logger.dart';
@@ -28,7 +29,13 @@ class SearchScreen extends StatefulWidget {
}
class _SearchScreenState extends State<SearchScreen>
with Refreshable, FullRefreshable, SearchInputFocusable, FocusableTab, ControllerDisposerMixin {
with
Refreshable,
FullRefreshable,
SearchInputFocusable,
FocusableTab,
ControllerDisposerMixin,
MountedSetStateMixin {
late final _searchController = createTextEditingController();
final _searchFocusNode = FocusNode(debugLabel: 'SearchInput');
final _firstResultFocusNode = FocusNode(debugLabel: 'SearchFirstResult');
@@ -56,11 +63,13 @@ class _SearchScreenState extends State<SearchScreen>
}
void _onSearchChanged() {
if (!mounted) return;
final query = _searchController.text;
if (query.trim().isEmpty) {
_searchDebounce.cancel();
setState(() {
setStateIfMounted(() {
_searchResults = [];
_hasSearched = false;
_isSearching = false;
@@ -78,20 +87,23 @@ class _SearchScreenState extends State<SearchScreen>
}
Future<void> _performSearch(String query) async {
if (!mounted) return;
if (query.trim().isEmpty) {
setState(() {
setStateIfMounted(() {
_searchResults = [];
_hasSearched = false;
});
return;
}
setState(() {
setStateIfMounted(() {
_isSearching = true;
_hasSearched = true;
});
try {
if (!mounted) return;
final multiServerProvider = Provider.of<MultiServerProvider>(context, listen: false);
if (!multiServerProvider.hasConnectedServers) {
@@ -100,7 +112,7 @@ class _SearchScreenState extends State<SearchScreen>
final neutral = await multiServerProvider.aggregationService.searchAcrossServers(query);
if (mounted) {
setState(() {
setStateIfMounted(() {
_searchResults = neutral;
_isSearching = false;
_lastSearchedQuery = query.trim();
@@ -108,7 +120,7 @@ class _SearchScreenState extends State<SearchScreen>
}
} catch (e) {
if (mounted) {
setState(() {
setStateIfMounted(() {
_isSearching = false;
});
showErrorSnackBar(context, t.errors.searchFailed(error: e));
@@ -118,6 +130,7 @@ class _SearchScreenState extends State<SearchScreen>
@override
void refresh() {
if (!mounted) return;
if (_searchController.text.isNotEmpty) {
_performSearch(_searchController.text);
}
@@ -126,27 +139,31 @@ class _SearchScreenState extends State<SearchScreen>
/// Focus the search input field
@override
void focusSearchInput() {
if (!mounted) return;
_searchFocusNode.requestFocus();
}
@override
void focusActiveTabIfReady() {
if (!mounted) return;
_searchFocusNode.requestFocus();
}
/// Set the search query externally (e.g. from companion remote)
@override
void setSearchQuery(String query) {
if (!mounted) return;
_searchController.text = query;
}
// Public method to fully reload all content (for profile switches)
@override
void fullRefresh() {
if (!mounted) return;
appLogger.d('SearchScreen.fullRefresh() called - clearing search and reloading');
// Clear search results and search text for new profile
_searchController.clear();
setState(() {
setStateIfMounted(() {
_searchResults.clear();
_isSearching = false;
_hasSearched = false;
@@ -155,6 +172,7 @@ class _SearchScreenState extends State<SearchScreen>
}
void updateItem(String _) {
if (!mounted) return;
// Trigger a refresh of the search to get updated metadata
if (_searchController.text.isNotEmpty) {
_performSearch(_searchController.text);
+4 -4
View File
@@ -100,7 +100,7 @@ Future<MediaNavigationResult> navigateToMediaItem(
case MediaKind.clip:
case MediaKind.episode:
final result = await navigateToVideoPlayer(context, metadata: mi, isOffline: isOffline);
if (result == true) {
if (result == true && context.mounted) {
onRefresh?.call(mi.id);
}
return MediaNavigationResult.navigated;
@@ -108,7 +108,7 @@ Future<MediaNavigationResult> navigateToMediaItem(
case MediaKind.movie:
if (playDirectly) {
final result = await navigateToVideoPlayer(context, metadata: mi, isOffline: isOffline);
if (result == true) {
if (result == true && context.mounted) {
onRefresh?.call(mi.id);
}
return MediaNavigationResult.navigated;
@@ -133,7 +133,7 @@ Future<MediaNavigationResult> navigateToMediaItem(
context,
mediaDetailRoute(metadata: showStub, isOffline: isOffline, initialSeasonIndex: mi.index),
);
if (result == true) {
if (result == true && context.mounted) {
onRefresh?.call(mi.id);
}
return MediaNavigationResult.navigated;
@@ -152,7 +152,7 @@ Future<MediaNavigationResult> _showDetail(
void Function(String)? onRefresh,
) async {
final result = await Navigator.push<bool>(context, mediaDetailRoute(metadata: mi, isOffline: isOffline));
if (result == true) {
if (result == true && context.mounted) {
onRefresh?.call(mi.id);
}
return MediaNavigationResult.navigated;
+1 -1
View File
@@ -174,7 +174,7 @@ Future<bool?> navigateToVideoPlayerWithRefresh(
appLogger.d('Returned from playback, refreshing metadata');
if (!isOffline && onRefresh != null) {
if (!isOffline && onRefresh != null && context.mounted) {
onRefresh();
}
+29 -22
View File
@@ -121,6 +121,16 @@ class MediaContextMenuState extends State<MediaContextMenu> {
bool get isContextMenuOpen => _isContextMenuOpen;
void _notifyRefresh(String itemId) {
if (!mounted) return;
widget.onRefresh?.call(itemId);
}
void _notifyListRefresh() {
if (!mounted) return;
widget.onListRefresh?.call();
}
/// The widget's [item] cast as a [MediaItem]. Returns `null` for playlists.
MediaItem? get _mediaItem => widget.item is MediaItem ? widget.item as MediaItem : null;
@@ -545,7 +555,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
await offlineWatch.markAsWatched(serverId: mediaItem!.serverId!, itemId: mediaItem.id);
if (context.mounted) {
showAppSnackBar(context, t.messages.markedAsWatchedOffline);
widget.onRefresh?.call(mediaItem.id);
_notifyRefresh(mediaItem.id);
}
} else {
// Resolve the right backend client — Plex hits scrobble, Jellyfin
@@ -570,7 +580,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
await offlineWatch.markAsUnwatched(serverId: mediaItem!.serverId!, itemId: mediaItem.id);
if (context.mounted) {
showAppSnackBar(context, t.messages.markedAsUnwatchedOffline);
widget.onRefresh?.call(mediaItem.id);
_notifyRefresh(mediaItem.id);
}
} else {
await _executeAction(context, () async {
@@ -596,7 +606,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
if (widget.onRemoveFromContinueWatching != null) {
widget.onRemoveFromContinueWatching!();
} else {
widget.onRefresh?.call(mediaItem.id);
_notifyRefresh(mediaItem.id);
}
}
} catch (e) {
@@ -622,22 +632,21 @@ class MediaContextMenuState extends State<MediaContextMenu> {
case 'edit_metadata':
didNavigate = true;
if (context.mounted) {
final item = mediaItem!;
await Navigator.push(
context,
MaterialPageRoute(builder: (context) => PlexMetadataEditScreen(metadata: mediaItem!)),
MaterialPageRoute(builder: (context) => PlexMetadataEditScreen(metadata: item)),
);
widget.onRefresh?.call(mediaItem!.id);
_notifyRefresh(item.id);
}
break;
case 'match':
didNavigate = true;
if (context.mounted) {
await Navigator.push(
context,
MaterialPageRoute(builder: (context) => PlexMatchScreen(metadata: mediaItem!)),
);
widget.onRefresh?.call(mediaItem!.id);
final item = mediaItem!;
await Navigator.push(context, MaterialPageRoute(builder: (context) => PlexMatchScreen(metadata: item)));
_notifyRefresh(item.id);
}
break;
@@ -753,7 +762,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
await action();
if (context.mounted) {
showSuccessSnackBar(context, successMessage);
widget.onRefresh?.call(_itemId());
_notifyRefresh(_itemId());
}
} catch (e) {
if (context.mounted) {
@@ -785,7 +794,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
if (!context.mounted) return;
if (success) {
showSuccessSnackBar(context, t.matchScreen.unmatchSuccess);
widget.onRefresh?.call(item.id);
_notifyRefresh(item.id);
} else {
showErrorSnackBar(context, t.matchScreen.unmatchFailed);
}
@@ -806,12 +815,13 @@ class MediaContextMenuState extends State<MediaContextMenu> {
if (id == null) return;
final client = _getMediaClientForItem();
final refreshItemId = _itemId();
try {
final metadata = await client.fetchItem(id);
if (metadata != null && context.mounted) {
await Navigator.push(context, routeBuilder(metadata));
widget.onRefresh?.call(_itemId());
_notifyRefresh(refreshItemId);
}
} catch (e) {
if (context.mounted) {
@@ -1123,11 +1133,8 @@ class MediaContextMenuState extends State<MediaContextMenu> {
await OverlaySheetController.showAdaptive(
context,
showDragHandle: true,
builder: (context) => RatingBottomSheet(
item: item,
serverClient: client,
onServerRatingChanged: (_) => widget.onRefresh?.call(item.id),
),
builder: (context) =>
RatingBottomSheet(item: item, serverClient: client, onServerRatingChanged: (_) => _notifyRefresh(item.id)),
);
}
@@ -1158,7 +1165,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
// Trigger refresh of collections tab
LibraryRefreshNotifier().notifyCollectionsChanged();
// Trigger list refresh to remove the item from the view
widget.onListRefresh?.call();
_notifyListRefresh();
} else {
showErrorSnackBar(context, t.collections.removeFromCollectionFailed);
}
@@ -1223,7 +1230,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
if (success) {
showSuccessSnackBar(context, isCollection ? t.collections.deleted : t.playlists.deleted);
// Trigger list refresh
widget.onListRefresh?.call();
_notifyListRefresh();
} else {
showErrorSnackBar(context, isCollection ? t.collections.deleteFailed : t.playlists.errorDeleting);
}
@@ -1395,7 +1402,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
// DownloadProvider.deleteDownload now broadcasts the DeletionEvent,
// so DeletionAware screens (e.g. offline season detail) update without
// a duplicate notification here.
widget.onRefresh?.call(item.id);
_notifyRefresh(item.id);
}
} catch (e) {
appLogger.e('Failed to delete download', error: e);
@@ -1494,7 +1501,7 @@ class MediaContextMenuState extends State<MediaContextMenu> {
// Broadcast deletion event for cross-screen propagation
DeletionNotifier().notifyDeletedItem(item: item);
// Backward-compatible list refresh for screens that are not DeletionAware yet
widget.onListRefresh?.call();
_notifyListRefresh();
} else {
showErrorSnackBar(context, t.mediaMenu.mediaFailedToDelete);
}
+39
View File
@@ -0,0 +1,39 @@
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:plezy/i18n/strings.g.dart';
import 'package:plezy/mixins/refreshable.dart';
import 'package:plezy/screens/search_screen.dart';
void main() {
TestWidgetsFlutterBinding.ensureInitialized();
setUp(() {
LocaleSettings.setLocaleSync(AppLocale.en);
});
testWidgets('stale callbacks are no-ops after SearchScreen is disposed', (tester) async {
final key = GlobalKey<State<SearchScreen>>();
await tester.pumpWidget(
TranslationProvider(
child: MaterialApp(home: SearchScreen(key: key)),
),
);
final state = key.currentState!;
final searchInput = state as SearchInputFocusable;
searchInput.setSearchQuery('movie');
await tester.pump();
await tester.pumpWidget(const SizedBox.shrink());
await tester.pump();
expect(tester.takeException(), isNull);
expect(() => (state as Refreshable).refresh(), returnsNormally);
expect(() => (state as dynamic).updateItem('movie_1'), returnsNormally);
expect(() => (state as FullRefreshable).fullRefresh(), returnsNormally);
expect(() => searchInput.setSearchQuery('new movie'), returnsNormally);
expect(() => (state as FocusableTab).focusActiveTabIfReady(), returnsNormally);
expect(tester.takeException(), isNull);
});
}