From 76f95f4e7f7458a7ceb27e0fa702a66005104686 Mon Sep 17 00:00:00 2001 From: Kingkor Roy Tirtho Date: Fri, 8 Sep 2023 12:30:52 +0600 Subject: [PATCH] fix: Windows memory leak due refetchOnStale user-liked-tracks (#705) * chore: refactor CLI stuff to separate service folder * chore: trying to fix memory leak * chore: fix fl_Query_devtools in wrong place * chore: upgrade deps * fix: user liked tracks memory leak due to isStale & updateQueryFn --- lib/components/playlist/playlist_card.dart | 4 +- lib/components/shared/heart_button.dart | 79 ++++++++++++------- .../use_disable_battery_optimizations.dart | 2 +- lib/main.dart | 44 +---------- lib/pages/playlist/playlist.dart | 7 +- lib/services/cli/cli.dart | 46 +++++++++++ lib/services/queries/playlist.dart | 67 ++++++++++------ lib/services/queries/user.dart | 8 ++ pubspec.lock | 58 ++++++++++---- pubspec.yaml | 3 +- 10 files changed, 206 insertions(+), 112 deletions(-) create mode 100644 lib/services/cli/cli.dart diff --git a/lib/components/playlist/playlist_card.dart b/lib/components/playlist/playlist_card.dart index be7abfb9e..edb374c8a 100644 --- a/lib/components/playlist/playlist_card.dart +++ b/lib/components/playlist/playlist_card.dart @@ -62,7 +62,7 @@ class PlaylistCard extends HookConsumerWidget { List fetchedTracks = await queryBowl.fetchQuery( "playlist-tracks/${playlist.id}", - () => useQueries.playlist.tracksOf(playlist.id!, spotify), + () => useQueries.playlist.tracksOf(playlist.id!, spotify, ref), ) ?? []; @@ -83,7 +83,7 @@ class PlaylistCard extends HookConsumerWidget { if (isPlaylistPlaying) return; List fetchedTracks = await queryBowl.fetchQuery( "playlist-tracks/${playlist.id}", - () => useQueries.playlist.tracksOf(playlist.id!, spotify), + () => useQueries.playlist.tracksOf(playlist.id!, spotify, ref), ) ?? []; diff --git a/lib/components/shared/heart_button.dart b/lib/components/shared/heart_button.dart index 2b877ecf5..c18ca2e4b 100644 --- a/lib/components/shared/heart_button.dart +++ b/lib/components/shared/heart_button.dart @@ -29,7 +29,7 @@ class HeartButton extends HookConsumerWidget { Widget build(BuildContext context, ref) { final auth = ref.watch(AuthenticationNotifier.provider); - if (auth == null) return Container(); + if (auth == null) return const SizedBox.shrink(); return IconButton( tooltip: tooltip, @@ -57,18 +57,21 @@ class HeartButton extends HookConsumerWidget { } } -({ +typedef UseTrackToggleLike = ({ bool isLiked, Mutation toggleTrackLike, Query me, -}) useTrackToggleLike(Track track, WidgetRef ref) { +}); + +UseTrackToggleLike useTrackToggleLike(Track track, WidgetRef ref) { final me = useQueries.user.me(ref); - final savedTracks = - useQueries.playlist.tracksOfQuery(ref, "user-liked-tracks"); + final savedTracks = useQueries.playlist.likedTracksQuery(ref); - final isLiked = - savedTracks.data?.any((element) => element.id == track.id) ?? false; + final isLiked = useMemoized( + () => savedTracks.data?.any((element) => element.id == track.id) ?? false, + [savedTracks.data, track.id], + ); final mounted = useIsMounted(); @@ -76,28 +79,48 @@ class HeartButton extends HookConsumerWidget { ref, track.id!, onMutate: (isLiked) { - savedTracks.setData( - [ - if (isLiked == true) - ...?savedTracks.data?.where((element) => element.id != track.id) - else - ...?savedTracks.data?..add(track) - ], - ); + print("Toggle Like onMutate: $isLiked"); + + if (isLiked) { + savedTracks.setData( + savedTracks.data + ?.where((element) => element.id != track.id) + .toList() ?? + [], + ); + } else { + savedTracks.setData( + [ + ...?savedTracks.data, + track, + ], + ); + } return isLiked; }, onData: (data, recoveryData) async { + print("Toggle Like onData: $data"); await savedTracks.refresh(); }, onError: (payload, isLiked) { + print("Toggle Like onError: $payload"); if (!mounted()) return; - savedTracks.setData([ - if (isLiked != true) - ...?savedTracks.data?.where((element) => element.id != track.id) - else - ...?savedTracks.data?..add(track), - ]); + if (isLiked != true) { + savedTracks.setData( + savedTracks.data + ?.where((element) => element.id != track.id) + .toList() ?? + [], + ); + } else { + savedTracks.setData( + [ + ...?savedTracks.data, + track, + ], + ); + } }, ); @@ -113,21 +136,21 @@ class TrackHeartButton extends HookConsumerWidget { @override Widget build(BuildContext context, ref) { - final savedTracks = - useQueries.playlist.tracksOfQuery(ref, "user-liked-tracks"); - final toggler = useTrackToggleLike(track, ref); - if (toggler.me.isLoading || !toggler.me.hasData) { + final savedTracks = useQueries.playlist.likedTracksQuery(ref); + final (:me, :isLiked, :toggleTrackLike) = useTrackToggleLike(track, ref); + + if (me.isLoading || !me.hasData) { return const CircularProgressIndicator(); } return HeartButton( - tooltip: toggler.isLiked + tooltip: isLiked ? context.l10n.remove_from_favorites : context.l10n.save_as_favorite, - isLiked: toggler.isLiked, + isLiked: isLiked, onPressed: savedTracks.hasData ? () { - toggler.toggleTrackLike.mutate(toggler.isLiked); + toggleTrackLike.mutate(isLiked); } : null, ); diff --git a/lib/hooks/use_disable_battery_optimizations.dart b/lib/hooks/use_disable_battery_optimizations.dart index cf1ad0c14..267655b62 100644 --- a/lib/hooks/use_disable_battery_optimizations.dart +++ b/lib/hooks/use_disable_battery_optimizations.dart @@ -4,7 +4,7 @@ import 'package:shared_preferences/shared_preferences.dart'; import 'package:spotube/hooks/use_async_effect.dart'; bool _asked = false; -void useDisableBatterOptimizations() { +void useDisableBatteryOptimizations() { useAsyncEffect(() async { if (!DesktopTools.platform.isAndroid || _asked) return; final localStorage = await SharedPreferences.getInstance(); diff --git a/lib/main.dart b/lib/main.dart index a7f37bad9..ee6fdf17b 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -1,9 +1,7 @@ -import 'dart:io'; - -import 'package:args/args.dart'; import 'package:catcher/catcher.dart'; import 'package:device_preview/device_preview.dart'; import 'package:fl_query/fl_query.dart'; +import 'package:fl_query_devtools/fl_query_devtools.dart'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:flutter/services.dart'; @@ -14,7 +12,6 @@ import 'package:hive/hive.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:media_kit/media_kit.dart'; import 'package:metadata_god/metadata_god.dart'; -import 'package:package_info_plus/package_info_plus.dart'; import 'package:shared_preferences/shared_preferences.dart'; import 'package:spotube/collections/routes.dart'; import 'package:spotube/collections/intents.dart'; @@ -26,6 +23,7 @@ import 'package:spotube/models/skip_segment.dart'; import 'package:spotube/provider/palette_provider.dart'; import 'package:spotube/provider/user_preferences_provider.dart'; import 'package:spotube/services/audio_player/audio_player.dart'; +import 'package:spotube/services/cli/cli.dart'; import 'package:spotube/services/connectivity_adapter.dart'; import 'package:spotube/themes/theme.dart'; import 'package:spotube/utils/persisted_state_notifier.dart'; @@ -37,41 +35,7 @@ import 'package:flutter_native_splash/flutter_native_splash.dart'; import 'package:flutter_displaymode/flutter_displaymode.dart'; Future main(List rawArgs) async { - final parser = ArgParser(); - - parser.addFlag( - 'verbose', - abbr: 'v', - help: 'Verbose mode', - defaultsTo: !kReleaseMode, - callback: (verbose) { - if (verbose) { - logEnv['VERBOSE'] = 'true'; - logEnv['DEBUG'] = 'true'; - logEnv['ERROR'] = 'true'; - } - }, - ); - parser.addFlag( - "version", - help: "Print version and exit", - negatable: false, - ); - - parser.addFlag("help", abbr: "h", negatable: false); - - final arguments = parser.parse(rawArgs); - - if (arguments["help"] == true) { - print(parser.usage); - exit(0); - } - - if (arguments["version"] == true) { - final package = await PackageInfo.fromPlatform(); - print("Spotube v${package.version}"); - exit(0); - } + final arguments = await startCLI(rawArgs); final widgetsBinding = WidgetsFlutterBinding.ensureInitialized(); @@ -215,7 +179,7 @@ class SpotubeState extends ConsumerState { }; }, []); - useDisableBatterOptimizations(); + useDisableBatteryOptimizations(); final lightTheme = useMemoized( () => theme(paletteColor ?? accentMaterialColor, Brightness.light), diff --git a/lib/pages/playlist/playlist.dart b/lib/pages/playlist/playlist.dart index baee06698..722fcb6d9 100644 --- a/lib/pages/playlist/playlist.dart +++ b/lib/pages/playlist/playlist.dart @@ -55,7 +55,12 @@ class PlaylistView extends HookConsumerWidget { final mediaQuery = MediaQuery.of(context); final meSnapshot = useQueries.user.me(ref); - final tracksSnapshot = useQueries.playlist.tracksOfQuery(ref, playlist.id!); + final playlistTrackSnapshot = + useQueries.playlist.tracksOfQuery(ref, playlist.id!); + final likedTracksSnapshot = useQueries.playlist.likedTracksQuery(ref); + final tracksSnapshot = playlist.id! == "user-liked-tracks" + ? likedTracksSnapshot + : playlistTrackSnapshot; final isPlaylistPlaying = useMemoized( () => proxyPlaylist.collections.contains(playlist.id!), diff --git a/lib/services/cli/cli.dart b/lib/services/cli/cli.dart new file mode 100644 index 000000000..61af710ed --- /dev/null +++ b/lib/services/cli/cli.dart @@ -0,0 +1,46 @@ +import 'dart:io'; + +import 'package:args/args.dart'; +import 'package:flutter/foundation.dart'; +import 'package:package_info_plus/package_info_plus.dart'; +import 'package:spotube/models/logger.dart'; + +Future startCLI(List args) async { + final parser = ArgParser(); + + parser.addFlag( + 'verbose', + abbr: 'v', + help: 'Verbose mode', + defaultsTo: !kReleaseMode, + callback: (verbose) { + if (verbose) { + logEnv['VERBOSE'] = 'true'; + logEnv['DEBUG'] = 'true'; + logEnv['ERROR'] = 'true'; + } + }, + ); + parser.addFlag( + "version", + help: "Print version and exit", + negatable: false, + ); + + parser.addFlag("help", abbr: "h", negatable: false); + + final arguments = parser.parse(args); + + if (arguments["help"] == true) { + print(parser.usage); + exit(0); + } + + if (arguments["version"] == true) { + final package = await PackageInfo.fromPlatform(); + print("Spotube v${package.version}"); + exit(0); + } + + return arguments; +} diff --git a/lib/services/queries/playlist.dart b/lib/services/queries/playlist.dart index 70b9ebe7e..f3a00eac4 100644 --- a/lib/services/queries/playlist.dart +++ b/lib/services/queries/playlist.dart @@ -1,3 +1,6 @@ +import 'dart:io'; +import 'dart:math'; + import 'package:catcher/catcher.dart'; import 'package:fl_query/fl_query.dart'; import 'package:fl_query_hooks/fl_query_hooks.dart'; @@ -10,6 +13,7 @@ import 'package:spotube/extensions/track.dart'; import 'package:spotube/hooks/use_spotify_infinite_query.dart'; import 'package:spotube/hooks/use_spotify_query.dart'; import 'package:spotube/pages/library/playlist_generate/playlist_generate.dart'; +import 'package:spotube/provider/authentication_provider.dart'; import 'package:spotube/provider/custom_spotify_endpoint_provider.dart'; import 'package:spotube/provider/user_preferences_provider.dart'; @@ -138,20 +142,51 @@ class PlaylistQueries { (lastPageData.items?.length ?? 0) < 10 || lastPageData.isLast ? null : lastPage + 1, - retryConfig: RetryConfig.withConstantDefaults( - maxRetries: 1, - retryDelay: const Duration(seconds: 5), + ref: ref, + ); + } + + Future> likedTracks( + SpotifyApi spotify, + WidgetRef ref, + ) async { + final tracks = await spotify.tracks.me.saved.all(); + + return tracks.map((e) => e.track!).toList(); + } + + Query, dynamic> likedTracksQuery(WidgetRef ref) { + final query = useCallback((spotify) => likedTracks(spotify, ref), []); + final context = useContext(); + + return useSpotifyQuery, dynamic>( + "user-liked-tracks", + query, + jsonConfig: JsonConfig( + toJson: (tracks) => { + 'tracks': tracks.map((e) => e.toJson()).toList(), + }, + fromJson: (json) => (json['tracks'] as List) + .map( + (e) => Track.fromJson((e as Map).castKeyDeep()), + ) + .toList(), + ), + refreshConfig: RefreshConfig.withDefaults( + context, + // will never make it stale + staleDuration: const Duration(days: 60), ), ref: ref, ); } - Future> tracksOf(String playlistId, SpotifyApi spotify) { - if (playlistId == "user-liked-tracks") { - return spotify.tracks.me.saved.all().then( - (tracks) => tracks.map((e) => e.track!).toList(), - ); - } + Future> tracksOf( + String playlistId, + SpotifyApi spotify, + WidgetRef ref, + ) async { + if (playlistId == "user-liked-tracks") return []; return spotify.playlists.getTracksByPlaylistId(playlistId).all().then( (value) => value.toList(), ); @@ -163,19 +198,7 @@ class PlaylistQueries { ) { return useSpotifyQuery, dynamic>( "playlist-tracks/$playlistId", - (spotify) => tracksOf(playlistId, spotify), - jsonConfig: playlistId == "user-liked-tracks" - ? JsonConfig( - toJson: (tracks) => { - 'tracks': tracks.map((e) => e.toJson()).toList() - }, - fromJson: (json) => (json['tracks'] as List) - .map((e) => Track.fromJson( - (e as Map).castKeyDeep(), - )) - .toList(), - ) - : null, + (spotify) => tracksOf(playlistId, spotify, ref), ref: ref, ); } diff --git a/lib/services/queries/user.dart b/lib/services/queries/user.dart index 63d58afde..897925929 100644 --- a/lib/services/queries/user.dart +++ b/lib/services/queries/user.dart @@ -1,4 +1,5 @@ import 'package:fl_query/fl_query.dart'; +import 'package:flutter_hooks/flutter_hooks.dart'; import 'package:hooks_riverpod/hooks_riverpod.dart'; import 'package:spotify/spotify.dart'; import 'package:spotube/hooks/use_spotify_query.dart'; @@ -8,6 +9,8 @@ import 'package:spotube/utils/type_conversion_utils.dart'; class UserQueries { const UserQueries(); Query me(WidgetRef ref) { + final context = useContext(); + return useSpotifyQuery( "current-user", (spotify) async { @@ -26,6 +29,11 @@ class UserQueries { } return me; }, + refreshConfig: RefreshConfig.withDefaults( + context, + // will never make it stale + staleDuration: const Duration(days: 60), + ), ref: ref, ); } diff --git a/pubspec.lock b/pubspec.lock index 21c864b69..e596944e5 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -318,10 +318,10 @@ packages: dependency: "direct main" description: name: collection - sha256: "4a07be6cb69c84d677a6c3096fcf960cc3285a8330b4603e0d463d15d9bd934c" + sha256: f092b211a4319e98e5ff58223576de6c2803db36221657b46c82574721240687 url: "https://pub.dev" source: hosted - version: "1.17.1" + version: "1.17.2" color: dependency: transitive description: @@ -522,6 +522,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.0.0-alpha.3" + fl_query_devtools: + dependency: "direct main" + description: + name: fl_query_devtools + sha256: f46148364d7fc49fb02ab2d3b2c280e6652edd3984e9fdf14c1b49d4d8473907 + url: "https://pub.dev" + source: hosted + version: "0.1.0-alpha.1" fl_query_hooks: dependency: "direct main" description: @@ -954,10 +962,10 @@ packages: dependency: "direct main" description: name: intl - sha256: a3715e3bc90294e971cb7dc063fbf3cd9ee0ebf8604ffeafabd9e6f16abbdbe6 + sha256: "3bc132a9dbce73a7e4a21a17d06e1878839ffbf975568bc875c60537824b0c4d" url: "https://pub.dev" source: hosted - version: "0.18.0" + version: "0.18.1" introduction_screen: dependency: "direct main" description: @@ -998,6 +1006,14 @@ packages: url: "https://pub.dev" source: hosted version: "6.6.2" + json_view: + dependency: transitive + description: + name: json_view + sha256: "905c69f9e69d1eab5406b87ab6c10c3706c04c70c6a4959621bd2b43c2d27374" + url: "https://pub.dev" + source: hosted + version: "0.4.2" jwt_decode: dependency: transitive description: @@ -1050,18 +1066,18 @@ packages: dependency: transitive description: name: matcher - sha256: "6501fbd55da300384b768785b83e5ce66991266cec21af89ab9ae7f5ce1c4cbb" + sha256: "1803e76e6653768d64ed8ff2e1e67bea3ad4b923eb5c56a295c3e634bad5960e" url: "https://pub.dev" source: hosted - version: "0.12.15" + version: "0.12.16" material_color_utilities: dependency: transitive description: name: material_color_utilities - sha256: d92141dc6fe1dad30722f9aa826c7fbc896d021d792f80678280601aff8cf724 + sha256: "9528f2f296073ff54cb9fee677df673ace1218163c3bc7628093e7eed5203d41" url: "https://pub.dev" source: hosted - version: "0.2.0" + version: "0.5.0" media_kit: dependency: "direct main" description: @@ -1610,10 +1626,10 @@ packages: dependency: "direct main" description: name: skeleton_text - sha256: "6e088723b97ddcccfcce45312ce5e385ed1e5139a57afdf574f753d51eaa77f1" + sha256: bacd536bf0664efe1cae53bcbd78c3d4040a120f300f69dc85d83f358471cc6c url: "https://pub.dev" source: hosted - version: "3.0.0" + version: "3.0.1" sky_engine: dependency: transitive description: flutter @@ -1647,10 +1663,10 @@ packages: dependency: transitive description: name: source_span - sha256: dd904f795d4b4f3b870833847c461801f6750a9fa8e61ea5ac53f9422b31f250 + sha256: "53e943d4206a5e30df338fd4c6e7a077e02254531b138a15aec3bd143c1a8b3c" url: "https://pub.dev" source: hosted - version: "1.9.1" + version: "1.10.0" spotify: dependency: "direct main" description: @@ -1791,10 +1807,10 @@ packages: dependency: transitive description: name: test_api - sha256: eb6ac1540b26de412b3403a163d919ba86f6a973fe6cc50ae3541b80092fdcfb + sha256: "75760ffd7786fffdfb9597c35c5b27eaeec82be8edfb6d71d32651128ed7aab8" url: "https://pub.dev" source: hosted - version: "0.5.1" + version: "0.6.0" time: dependency: transitive description: @@ -1967,10 +1983,10 @@ packages: dependency: transitive description: name: vm_service - sha256: f6deed8ed625c52864792459709183da231ebf66ff0cf09e69b573227c377efe + sha256: c620a6f783fa22436da68e42db7ebbf18b8c44b9a46ab911f666ff09ffd9153f url: "https://pub.dev" source: hosted - version: "11.3.0" + version: "11.7.1" watcher: dependency: transitive description: @@ -1979,6 +1995,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.0.2" + web: + dependency: transitive + description: + name: web + sha256: dc8ccd225a2005c1be616fe02951e2e342092edf968cf0844220383757ef8f10 + url: "https://pub.dev" + source: hosted + version: "0.1.4-beta" web_socket_channel: dependency: transitive description: @@ -2069,5 +2093,5 @@ packages: source: hosted version: "2.0.1" sdks: - dart: ">=3.0.0 <4.0.0" + dart: ">=3.1.0-185.0.dev <4.0.0" flutter: ">=3.10.0" diff --git a/pubspec.yaml b/pubspec.yaml index 1055f4c1a..78fc28293 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -36,6 +36,7 @@ dependencies: file_picker: ^5.2.2 fl_query: ^1.0.0-alpha.3 fl_query_hooks: ^1.0.0-alpha.3 + fl_query_devtools: ^0.1.0-alpha.1 fluentui_system_icons: ^1.1.189 flutter: sdk: flutter @@ -81,7 +82,7 @@ dependencies: scroll_to_index: ^3.0.1 shared_preferences: ^2.0.11 sidebarx: ^0.15.0 - skeleton_text: ^3.0.0 + skeleton_text: ^3.0.1 smtc_windows: ^0.1.0 spotify: ^0.11.0 supabase: ^1.9.9