From 84c255b91725343bf5e95374da7a86493ebdce72 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Sun, 21 Dec 2025 13:19:21 +0100 Subject: [PATCH] refactor: deduplicate logic --- lib/providers/offline_watch_provider.dart | 38 ++++---- lib/screens/settings/mpv_config_screen.dart | 30 +++--- lib/services/data_aggregation_service.dart | 96 ++++++------------- lib/services/track_selection_service.dart | 12 +-- .../providers/watch_together_provider.dart | 18 ++-- .../services/watch_together_peer_service.dart | 77 +++++++-------- lib/widgets/download_tree_view.dart | 25 +++-- .../horizontal_scroll_with_arrows.dart | 19 +--- 8 files changed, 131 insertions(+), 184 deletions(-) diff --git a/lib/providers/offline_watch_provider.dart b/lib/providers/offline_watch_provider.dart index 8d25fe62..26762f9e 100644 --- a/lib/providers/offline_watch_provider.dart +++ b/lib/providers/offline_watch_provider.dart @@ -106,6 +106,22 @@ class OfflineWatchProvider extends ChangeNotifier { return episodes; } + /// Batch resolve watch statuses for a list of episodes. + /// + /// Returns a map of globalKey -> isWatched for each episode. + Future> _resolveEpisodeWatchStatuses(List episodes) async { + if (episodes.isEmpty) return {}; + + final globalKeys = episodes.map((e) => e.globalKey).toSet(); + final localStatuses = await _syncService.getLocalWatchStatusesBatched(globalKeys); + + return { + for (final episode in episodes) + episode.globalKey: + localStatuses[episode.globalKey] ?? _downloadProvider.getMetadata(episode.globalKey)?.isWatched ?? false, + }; + } + /// Find the next unwatched downloaded episode for a show. /// /// This is the "offline OnDeck" calculation - finds the first @@ -118,15 +134,11 @@ class OfflineWatchProvider extends ChangeNotifier { final episodes = _getSortedEpisodes(showRatingKey); if (episodes.isEmpty) return null; - // Batch fetch all watch statuses in a single query - final globalKeys = episodes.map((e) => e.globalKey).toSet(); - final localStatuses = await _syncService.getLocalWatchStatusesBatched(globalKeys); + final watchStatuses = await _resolveEpisodeWatchStatuses(episodes); // Find first unwatched episode for (final episode in episodes) { - final localStatus = localStatuses[episode.globalKey]; - final watched = localStatus ?? _downloadProvider.getMetadata(episode.globalKey)?.isWatched ?? false; - if (!watched) { + if (!watchStatuses[episode.globalKey]!) { return episode; } } @@ -176,21 +188,11 @@ class OfflineWatchProvider extends ChangeNotifier { /// Uses batched database query for efficiency. Future> getEpisodesWithWatchStatus(String showRatingKey) async { final episodes = _downloadProvider.getDownloadedEpisodesForShow(showRatingKey); - if (episodes.isEmpty) return []; - // Batch fetch all watch statuses in a single query - final globalKeys = episodes.map((e) => e.globalKey).toSet(); - final localStatuses = await _syncService.getLocalWatchStatusesBatched(globalKeys); + final watchStatuses = await _resolveEpisodeWatchStatuses(episodes); - final results = <(PlexMetadata, bool)>[]; - for (final episode in episodes) { - final localStatus = localStatuses[episode.globalKey]; - final watched = localStatus ?? _downloadProvider.getMetadata(episode.globalKey)?.isWatched ?? false; - results.add((episode, watched)); - } - - return results; + return [for (final episode in episodes) (episode, watchStatuses[episode.globalKey]!)]; } /// Trigger a manual sync of pending items. diff --git a/lib/screens/settings/mpv_config_screen.dart b/lib/screens/settings/mpv_config_screen.dart index 19cadf12..b760941d 100644 --- a/lib/screens/settings/mpv_config_screen.dart +++ b/lib/screens/settings/mpv_config_screen.dart @@ -129,20 +129,27 @@ class _MpvConfigScreenState extends State { valueController.dispose(); } - Future _showDeleteEntryDialog(int index) async { + Future _showConfirmDeleteDialog({required String title, required String content}) async { final result = await showDialog( context: context, builder: (context) => AlertDialog( - title: Text(t.mpvConfig.deleteProperty), - content: Text(t.mpvConfig.confirmDeleteProperty), + title: Text(title), + content: Text(content), actions: [ TextButton(onPressed: () => Navigator.pop(context, false), child: Text(t.common.cancel)), TextButton(onPressed: () => Navigator.pop(context, true), child: Text(t.common.delete)), ], ), ); + return result == true; + } - if (result == true) { + Future _showDeleteEntryDialog(int index) async { + final confirmed = await _showConfirmDeleteDialog( + title: t.mpvConfig.deleteProperty, + content: t.mpvConfig.confirmDeleteProperty, + ); + if (confirmed) { _deleteEntry(index); } } @@ -201,19 +208,12 @@ class _MpvConfigScreenState extends State { } Future _deletePreset(MpvPreset preset) async { - final result = await showDialog( - context: context, - builder: (context) => AlertDialog( - title: Text(t.mpvConfig.deletePreset), - content: Text(t.mpvConfig.confirmDeletePreset), - actions: [ - TextButton(onPressed: () => Navigator.pop(context, false), child: Text(t.common.cancel)), - TextButton(onPressed: () => Navigator.pop(context, true), child: Text(t.common.delete)), - ], - ), + final confirmed = await _showConfirmDeleteDialog( + title: t.mpvConfig.deletePreset, + content: t.mpvConfig.confirmDeletePreset, ); - if (result == true) { + if (confirmed) { await _settingsService.deleteMpvPreset(preset.name); setState(() { _presets = _settingsService.getMpvPresets(); diff --git a/lib/services/data_aggregation_service.dart b/lib/services/data_aggregation_service.dart index a6360be1..2ef24851 100644 --- a/lib/services/data_aggregation_service.dart +++ b/lib/services/data_aggregation_service.dart @@ -247,15 +247,11 @@ class DataAggregationService { // Private helper methods - /// Higher-order helper for per-server fan-out operations + /// Base helper for per-server fan-out operations /// - /// Iterates over all online clients, executes the operation for each server, - /// handles errors, updates server status, and aggregates results. - /// - /// Type parameter `T` is the item type returned by the operation - /// [operationName] is used for logging (e.g., "fetching libraries") - /// [operation] is the async function to run per server, returning `List` - Future> _perServer({ + /// Returns raw results as (serverId, result) tuples. + /// Used by [_perServer] and [_perServerGrouped] for different aggregation strategies. + Future result)>> _perServerRaw({ required String operationName, required Future> Function(String serverId, PlexClient client, PlexServer? server) operation, }) async { @@ -268,61 +264,6 @@ class DataAggregationService { appLogger.d('$operationName from ${clients.length} servers'); - final allResults = []; - - // Execute operation on all servers in parallel - final Iterable>> futures = clients.entries.map((entry) async { - final serverId = entry.key; - final client = entry.value; - final server = _serverManager.getServer(serverId); - final sw = Stopwatch()..start(); - - try { - final result = await operation(serverId, client, server); - appLogger.d( - '$operationName for server $serverId completed in ${sw.elapsedMilliseconds}ms with ${result.length} items', - ); - return result; - } catch (e, stackTrace) { - appLogger.e('Failed $operationName from server $serverId', error: e, stackTrace: stackTrace); - _serverManager.updateServerStatus(serverId, false); - appLogger.d('$operationName for server $serverId failed after ${sw.elapsedMilliseconds}ms'); - return []; - } - }); - - final List> results = await Future.wait>(futures); - - // Flatten results - for (final items in results) { - allResults.addAll(items); - } - - return allResults; - } - - /// Higher-order helper for per-server fan-out operations that groups results by server - /// - /// Similar to [_perServer] but returns a Map with results grouped by serverId - /// instead of flattening into a single list. - /// - /// Type parameter `T` is the item type returned by the operation - /// [operationName] is used for logging (e.g., "fetching libraries") - /// [operation] is the async function to run per server, returning `List` - Future>> _perServerGrouped({ - required String operationName, - required Future> Function(String serverId, PlexClient client, PlexServer? server) operation, - }) async { - final clients = _serverManager.onlineClients; - - if (clients.isEmpty) { - appLogger.w('No online servers available for $operationName'); - return {}; - } - - appLogger.d('$operationName from ${clients.length} servers'); - - // Execute operation on all servers in parallel final futures = clients.entries.map((entry) async { final serverId = entry.key; final client = entry.value; @@ -334,17 +275,38 @@ class DataAggregationService { appLogger.d( '$operationName for server $serverId completed in ${sw.elapsedMilliseconds}ms with ${result.length} items', ); - return MapEntry(serverId, result); + return (serverId, result); } catch (e, stackTrace) { appLogger.e('Failed $operationName from server $serverId', error: e, stackTrace: stackTrace); _serverManager.updateServerStatus(serverId, false); appLogger.d('$operationName for server $serverId failed after ${sw.elapsedMilliseconds}ms'); - return MapEntry(serverId, []); + return (serverId, []); } }); - final results = await Future.wait(futures); + return await Future.wait(futures); + } - return Map.fromEntries(results); + /// Higher-order helper for per-server fan-out operations + /// + /// Iterates over all online clients, executes the operation for each server, + /// handles errors, updates server status, and flattens results into a single list. + Future> _perServer({ + required String operationName, + required Future> Function(String serverId, PlexClient client, PlexServer? server) operation, + }) async { + final results = await _perServerRaw(operationName: operationName, operation: operation); + return [for (final (_, items) in results) ...items]; + } + + /// Higher-order helper for per-server fan-out operations that groups results by server + /// + /// Similar to [_perServer] but returns a Map with results grouped by serverId. + Future>> _perServerGrouped({ + required String operationName, + required Future> Function(String serverId, PlexClient client, PlexServer? server) operation, + }) async { + final results = await _perServerRaw(operationName: operationName, operation: operation); + return {for (final (id, items) in results) id: items}; } } diff --git a/lib/services/track_selection_service.dart b/lib/services/track_selection_service.dart index 2be6e733..1746d153 100644 --- a/lib/services/track_selection_service.dart +++ b/lib/services/track_selection_service.dart @@ -32,12 +32,7 @@ class TrackSelectionService { final PlexMetadata metadata; final PlexMediaInfo? plexMediaInfo; - TrackSelectionService({ - required this.player, - this.profileSettings, - required this.metadata, - this.plexMediaInfo, - }); + TrackSelectionService({required this.player, this.profileSettings, required this.metadata, this.plexMediaInfo}); /// Build list of preferred languages from a user profile List _buildPreferredLanguages(PlexUserProfile profile, {required bool isAudio}) { @@ -324,7 +319,10 @@ class TrackSelectionService { /// Priority 3: Per-media language preference /// Priority 4: User profile preferences /// Priority 5: Default or first track - TrackSelectionResult? selectAudioTrack(List availableTracks, AudioTrack? preferredAudioTrack) { + TrackSelectionResult? selectAudioTrack( + List availableTracks, + AudioTrack? preferredAudioTrack, + ) { if (availableTracks.isEmpty) return null; AudioTrack? trackToSelect; diff --git a/lib/watch_together/providers/watch_together_provider.dart b/lib/watch_together/providers/watch_together_provider.dart index 3c76bd71..088112cb 100644 --- a/lib/watch_together/providers/watch_together_provider.dart +++ b/lib/watch_together/providers/watch_together_provider.dart @@ -76,6 +76,14 @@ class WatchTogetherProvider with ChangeNotifier { _displayName = name; } + /// Wire up sync manager's state change callback to update provider state + void _wireSyncStateChanges() { + _syncManager!.onSyncStateChanged = (isSyncing) { + _isSyncing = isSyncing; + notifyListeners(); + }; + } + /// Create a new watch together session as host Future createSession({ required ControlMode controlMode, @@ -113,10 +121,7 @@ class WatchTogetherProvider with ChangeNotifier { displayName: _displayName, ); - _syncManager!.onSyncStateChanged = (isSyncing) { - _isSyncing = isSyncing; - notifyListeners(); - }; + _wireSyncStateChanges(); notifyListeners(); appLogger.d('WatchTogether: Session created: $sessionId'); @@ -164,10 +169,7 @@ class WatchTogetherProvider with ChangeNotifier { notifyListeners(); }; - _syncManager!.onSyncStateChanged = (isSyncing) { - _isSyncing = isSyncing; - notifyListeners(); - }; + _wireSyncStateChanges(); // Add self to participants _participants.add(Participant(peerId: _peerService!.myPeerId!, displayName: _displayName, isHost: false)); diff --git a/lib/watch_together/services/watch_together_peer_service.dart b/lib/watch_together/services/watch_together_peer_service.dart index c20e3f78..3d7eb03f 100644 --- a/lib/watch_together/services/watch_together_peer_service.dart +++ b/lib/watch_together/services/watch_together_peer_service.dart @@ -83,6 +83,31 @@ class WatchTogetherPeerService { return const Uuid().v4().substring(0, 8).toUpperCase(); } + /// Attach common peer event listeners for disconnected/close/error events + void _attachCommonPeerListeners({ + required Completer completer, + required PeerErrorType errorType, + required String errorMessage, + }) { + _peer!.on('disconnected').listen((_) { + appLogger.w('WatchTogether: Peer disconnected from server'); + _handleDisconnectedFromServer(); + }); + + _peer!.on('close').listen((_) { + appLogger.d('WatchTogether: Peer closed'); + _connectionStateController.add(false); + }); + + _peer!.on('error').listen((error) { + appLogger.e('WatchTogether: Peer error', error: error); + _errorController.add(PeerError(type: errorType, message: '$errorMessage: $error', originalError: error)); + if (!completer.isCompleted) { + completer.completeError(error); + } + }); + } + /// Create a new session as host /// /// Returns the session ID that others can use to join @@ -115,25 +140,11 @@ class WatchTogetherPeerService { _handleNewConnection(dataConn); }); - _peer!.on('error').listen((error) { - appLogger.e('WatchTogether: Peer error', error: error); - _errorController.add( - PeerError(type: PeerErrorType.serverError, message: error.toString(), originalError: error), - ); - if (!completer.isCompleted) { - completer.completeError(error); - } - }); - - _peer!.on('disconnected').listen((_) { - appLogger.w('WatchTogether: Peer disconnected from server'); - _handleDisconnectedFromServer(); - }); - - _peer!.on('close').listen((_) { - appLogger.d('WatchTogether: Peer closed'); - _connectionStateController.add(false); - }); + _attachCommonPeerListeners( + completer: completer, + errorType: PeerErrorType.serverError, + errorMessage: 'Server error', + ); } catch (e) { appLogger.e('WatchTogether: Failed to create peer', error: e); if (!completer.isCompleted) { @@ -178,29 +189,11 @@ class WatchTogetherPeerService { _handleNewConnection(conn, isOutgoing: true, completer: completer); }); - _peer!.on('error').listen((error) { - appLogger.e('WatchTogether: Peer error', error: error); - _errorController.add( - PeerError( - type: PeerErrorType.connectionFailed, - message: 'Failed to connect to session: $error', - originalError: error, - ), - ); - if (!completer.isCompleted) { - completer.completeError(error); - } - }); - - _peer!.on('disconnected').listen((_) { - appLogger.w('WatchTogether: Peer disconnected from server'); - _handleDisconnectedFromServer(); - }); - - _peer!.on('close').listen((_) { - appLogger.d('WatchTogether: Peer closed'); - _connectionStateController.add(false); - }); + _attachCommonPeerListeners( + completer: completer, + errorType: PeerErrorType.connectionFailed, + errorMessage: 'Failed to connect to session', + ); } catch (e) { appLogger.e('WatchTogether: Failed to create peer for joining', error: e); if (!completer.isCompleted) { diff --git a/lib/widgets/download_tree_view.dart b/lib/widgets/download_tree_view.dart index 4e66b194..2c123812 100644 --- a/lib/widgets/download_tree_view.dart +++ b/lib/widgets/download_tree_view.dart @@ -241,19 +241,9 @@ class _DownloadTreeViewState extends State { ); } - // Sort shows by status and title - shows.sort((a, b) { - final statusCompare = _compareByStatus(a.status, b.status); - if (statusCompare != 0) return statusCompare; - return a.title.compareTo(b.title); - }); - - // Sort movies by status and title - movies.sort((a, b) { - final statusCompare = _compareByStatus(a.status, b.status); - if (statusCompare != 0) return statusCompare; - return a.title.compareTo(b.title); - }); + // Sort shows and movies by status and title + _sortNodesByStatusAndTitle(shows); + _sortNodesByStatusAndTitle(movies); // Combine movies and shows return [...movies, ...shows]; @@ -292,6 +282,15 @@ class _DownloadTreeViewState extends State { return (statusOrder[a] ?? 99).compareTo(statusOrder[b] ?? 99); } + /// Sort nodes by status (downloading first) then by title + void _sortNodesByStatusAndTitle(List nodes) { + nodes.sort((a, b) { + final statusCompare = _compareByStatus(a.status, b.status); + if (statusCompare != 0) return statusCompare; + return a.title.compareTo(b.title); + }); + } + /// Flatten the tree into a list of visible nodes with their depths List<_FlatNode> _flattenTree(List nodes, [int depth = 0]) { final List<_FlatNode> result = []; diff --git a/lib/widgets/horizontal_scroll_with_arrows.dart b/lib/widgets/horizontal_scroll_with_arrows.dart index 577d0000..af79368c 100644 --- a/lib/widgets/horizontal_scroll_with_arrows.dart +++ b/lib/widgets/horizontal_scroll_with_arrows.dart @@ -61,25 +61,16 @@ class _HorizontalScrollWithArrowsState extends State }); } - void _scrollLeft() { + void _animateScroll(double direction) { final position = _scrollController.position; - final targetScroll = (position.pixels - (position.viewportDimension * widget.scrollAmount)).clamp( - 0.0, - position.maxScrollExtent, - ); - + final delta = direction * position.viewportDimension * widget.scrollAmount; + final targetScroll = (position.pixels + delta).clamp(0.0, position.maxScrollExtent); _scrollController.animateTo(targetScroll, duration: tokens(context).slow, curve: Curves.easeInOut); } - void _scrollRight() { - final position = _scrollController.position; - final targetScroll = (position.pixels + (position.viewportDimension * widget.scrollAmount)).clamp( - 0.0, - position.maxScrollExtent, - ); + void _scrollLeft() => _animateScroll(-1); - _scrollController.animateTo(targetScroll, duration: tokens(context).slow, curve: Curves.easeInOut); - } + void _scrollRight() => _animateScroll(1); Widget _buildArrowButton({ required double position,