Skip to content

Commit

Permalink
fix: Windows memory leak due refetchOnStale user-liked-tracks (KRTirt…
Browse files Browse the repository at this point in the history 
…ho#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
  • Loading branch information
@KRTirtho @meenbeese
KRTirtho authored and meenbeese committed Sep 20, 2023
1 parent b920c10 commit 76f95f4
Show file tree
Hide file tree
Showing 10 changed files with 206 additions and 112 deletions.
4 changes: 2 additions & 2 deletions lib/components/playlist/playlist_card.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ class PlaylistCard extends HookConsumerWidget {

List<Track> fetchedTracks = await queryBowl.fetchQuery(
"playlist-tracks/${playlist.id}",
() => useQueries.playlist.tracksOf(playlist.id!, spotify),
() => useQueries.playlist.tracksOf(playlist.id!, spotify, ref),
) ??
[];

Expand All @@ -83,7 +83,7 @@ class PlaylistCard extends HookConsumerWidget {
if (isPlaylistPlaying) return;
List<Track> fetchedTracks = await queryBowl.fetchQuery(
"playlist-tracks/${playlist.id}",
() => useQueries.playlist.tracksOf(playlist.id!, spotify),
() => useQueries.playlist.tracksOf(playlist.id!, spotify, ref),
) ??
[];

Expand Down
79 changes: 51 additions & 28 deletions lib/components/shared/heart_button.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -57,47 +57,70 @@ class HeartButton extends HookConsumerWidget {
}
}

({
typedef UseTrackToggleLike = ({
bool isLiked,
Mutation<bool, dynamic, bool> toggleTrackLike,
Query<User?, dynamic> 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();

final toggleTrackLike = useMutations.track.toggleFavorite(
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,
],
);
}
},
);

Expand All @@ -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,
);
Expand Down
2 changes: 1 addition & 1 deletion lib/hooks/use_disable_battery_optimizations.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
44 changes: 4 additions & 40 deletions lib/main.dart
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -37,41 +35,7 @@ import 'package:flutter_native_splash/flutter_native_splash.dart';
import 'package:flutter_displaymode/flutter_displaymode.dart';

Future<void> main(List<String> 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();

Expand Down Expand Up @@ -215,7 +179,7 @@ class SpotubeState extends ConsumerState<Spotube> {
};
}, []);

useDisableBatterOptimizations();
useDisableBatteryOptimizations();

final lightTheme = useMemoized(
() => theme(paletteColor ?? accentMaterialColor, Brightness.light),
Expand Down
7 changes: 6 additions & 1 deletion lib/pages/playlist/playlist.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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!),
Expand Down
46 changes: 46 additions & 0 deletions lib/services/cli/cli.dart
View file
Original file line number Diff line number Diff line change
@@ -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<ArgResults> startCLI(List<String> 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;
}
67 changes: 45 additions & 22 deletions lib/services/queries/playlist.dart
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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';

Expand Down Expand Up @@ -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<List<Track>> likedTracks(
SpotifyApi spotify,
WidgetRef ref,
) async {
final tracks = await spotify.tracks.me.saved.all();

return tracks.map((e) => e.track!).toList();
}

Query<List<Track>, dynamic> likedTracksQuery(WidgetRef ref) {
final query = useCallback((spotify) => likedTracks(spotify, ref), []);
final context = useContext();

return useSpotifyQuery<List<Track>, dynamic>(
"user-liked-tracks",
query,
jsonConfig: JsonConfig(
toJson: (tracks) => <String, dynamic>{
'tracks': tracks.map((e) => e.toJson()).toList(),
},
fromJson: (json) => (json['tracks'] as List)
.map(
(e) => Track.fromJson((e as Map).castKeyDeep<String>()),
)
.toList(),
),
refreshConfig: RefreshConfig.withDefaults(
context,
// will never make it stale
staleDuration: const Duration(days: 60),
),
ref: ref,
);
}

Future<List<Track>> 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<List<Track>> tracksOf(
String playlistId,
SpotifyApi spotify,
WidgetRef ref,
) async {
if (playlistId == "user-liked-tracks") return <Track>[];
return spotify.playlists.getTracksByPlaylistId(playlistId).all().then(
(value) => value.toList(),
);
Expand All @@ -163,19 +198,7 @@ class PlaylistQueries {
) {
return useSpotifyQuery<List<Track>, dynamic>(
"playlist-tracks/$playlistId",
(spotify) => tracksOf(playlistId, spotify),
jsonConfig: playlistId == "user-liked-tracks"
? JsonConfig(
toJson: (tracks) => <String, dynamic>{
'tracks': tracks.map((e) => e.toJson()).toList()
},
fromJson: (json) => (json['tracks'] as List)
.map((e) => Track.fromJson(
(e as Map).castKeyDeep<String>(),
))
.toList(),
)
: null,
(spotify) => tracksOf(playlistId, spotify, ref),
ref: ref,
);
}
Expand Down

0 comments on commit 76f95f4

Please sign in to comment.