From d53d3bd89db3e641b67c656586682011c282c917 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Thu, 16 Jan 2025 15:58:47 +0100 Subject: [PATCH 1/3] Fix caching of Youtube API pagination + filter duplicate videoIds. --- app/lib/service/youtube/backend.dart | 92 ++++++++++++---------- app/lib/shared/redis_cache.dart | 21 ++--- app/test/service/youtube/backend_test.dart | 4 +- 3 files changed, 64 insertions(+), 53 deletions(-) diff --git a/app/lib/service/youtube/backend.dart b/app/lib/service/youtube/backend.dart index d6624a36c2..48f62601e0 100644 --- a/app/lib/service/youtube/backend.dart +++ b/app/lib/service/youtube/backend.dart @@ -121,54 +121,64 @@ class _PkgOfWeekVideoFetcher { final youtube = YouTubeApi(apiClient); try { + final pageTokensVisited = {}; + String nextPageToken = ''; + final videos = []; - String? nextPageToken; - for (var check = true; check && videos.length < 50;) { - final rs = await cache.youtubePlaylistItems().get( + final videoIds = {}; + while (videos.length < 50) { + // check visited status + if (pageTokensVisited.contains(nextPageToken)) { + break; + } + pageTokensVisited.add(nextPageToken); + + // get page from cache or from Youtube API + final rs = await cache.youtubePlaylistItems(nextPageToken).get( () async => await youtube.playlistItems.list( ['snippet', 'contentDetails'], playlistId: powPlaylistId, - pageToken: nextPageToken, + pageToken: nextPageToken.isEmpty ? null : nextPageToken, ), ); - videos.addAll(rs!.items!.map( - (i) { - try { - final videoId = i.contentDetails?.videoId; - if (videoId == null) { - return null; - } - final thumbnails = i.snippet?.thumbnails; - if (thumbnails == null) { - return null; - } - final thumbnail = thumbnails.high ?? - thumbnails.default_ ?? - thumbnails.maxres ?? - thumbnails.standard ?? - thumbnails.medium; - final thumbnailUrl = thumbnail?.url; - if (thumbnailUrl == null || thumbnailUrl.isEmpty) { - return null; - } - return PkgOfWeekVideo( - videoId: videoId, - title: i.snippet?.title ?? '', - description: - (i.snippet?.description ?? '').trim().split('\n').first, - thumbnailUrl: thumbnailUrl, - ); - } catch (e, st) { - // this item will be skipped, the rest of the list may be displayed - _logger.pubNoticeShout( - 'youtube', 'Processing Youtube PlaylistItem failed.', e, st); + + // process playlist items + for (final i in rs!.items!) { + try { + final videoId = i.contentDetails?.videoId; + if (videoId == null || videoIds.contains(videoId)) { + continue; + } + final thumbnails = i.snippet?.thumbnails; + if (thumbnails == null) { + continue; + } + final thumbnail = thumbnails.high ?? + thumbnails.default_ ?? + thumbnails.maxres ?? + thumbnails.standard ?? + thumbnails.medium; + final thumbnailUrl = thumbnail?.url; + if (thumbnailUrl == null || thumbnailUrl.isEmpty) { + continue; } - return null; - }, - ).nonNulls); - // next page - nextPageToken = rs.nextPageToken; - check = nextPageToken != null && nextPageToken.isNotEmpty; + videoIds.add(videoId); + videos.add(PkgOfWeekVideo( + videoId: videoId, + title: i.snippet?.title ?? '', + description: + (i.snippet?.description ?? '').trim().split('\n').first, + thumbnailUrl: thumbnailUrl, + )); + } catch (e, st) { + // this item will be skipped, the rest of the list may be displayed + _logger.pubNoticeShout( + 'youtube', 'Processing Youtube PlaylistItem failed.', e, st); + } + } + + // advance to next page token + nextPageToken = rs.nextPageToken ?? ''; } return videos; } finally { diff --git a/app/lib/shared/redis_cache.dart b/app/lib/shared/redis_cache.dart index f57f9f2aa9..ba4f0437d7 100644 --- a/app/lib/shared/redis_cache.dart +++ b/app/lib/shared/redis_cache.dart @@ -457,16 +457,17 @@ class CachePatterns { ResolvedDocUrlVersion.fromJson(v as Map), ))['$package-$version']; - Entry youtubePlaylistItems() => _cache - .withPrefix('youtube/playlist-item-list-response/') - .withTTL(Duration(hours: 6)) - .withCodec(utf8) - .withCodec(json) - .withCodec(wrapAsCodec( - encode: (PlaylistItemListResponse v) => v.toJson(), - decode: (v) => - PlaylistItemListResponse.fromJson(v as Map), - ))['']; + Entry youtubePlaylistItems(String pageToken) => + _cache + .withPrefix('youtube/playlist-item-list-response/') + .withTTL(Duration(hours: 6)) + .withCodec(utf8) + .withCodec(json) + .withCodec(wrapAsCodec( + encode: (PlaylistItemListResponse v) => v.toJson(), + decode: (v) => + PlaylistItemListResponse.fromJson(v as Map), + ))[pageToken]; } /// The active cache. diff --git a/app/test/service/youtube/backend_test.dart b/app/test/service/youtube/backend_test.dart index dbad6e3071..4e3e218834 100644 --- a/app/test/service/youtube/backend_test.dart +++ b/app/test/service/youtube/backend_test.dart @@ -40,12 +40,12 @@ void main() { }); test('selectRandomVideos', () { - final random = Random(123); final items = [0, 1, 2, 3, 4, 5, 6, 7, 9, 10]; for (var i = 0; i < 1000; i++) { - final selected = selectRandomVideos(random, items, 4); + final selected = selectRandomVideos(Random(i), items, 4); + expect(selected, hasLength(4)); expect(selected.first, 0); expect(selected[1], greaterThan(0)); expect(selected[1], lessThan(4)); From f80a0fbd0203b453d50e432215e548d89de15b08 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Thu, 16 Jan 2025 16:08:40 +0100 Subject: [PATCH 2/3] use nulls and different break logic --- app/lib/service/youtube/backend.dart | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/lib/service/youtube/backend.dart b/app/lib/service/youtube/backend.dart index 48f62601e0..829cf39258 100644 --- a/app/lib/service/youtube/backend.dart +++ b/app/lib/service/youtube/backend.dart @@ -122,23 +122,17 @@ class _PkgOfWeekVideoFetcher { try { final pageTokensVisited = {}; - String nextPageToken = ''; + String? nextPageToken; final videos = []; final videoIds = {}; while (videos.length < 50) { - // check visited status - if (pageTokensVisited.contains(nextPageToken)) { - break; - } - pageTokensVisited.add(nextPageToken); - // get page from cache or from Youtube API - final rs = await cache.youtubePlaylistItems(nextPageToken).get( + final rs = await cache.youtubePlaylistItems(nextPageToken ?? '').get( () async => await youtube.playlistItems.list( ['snippet', 'contentDetails'], playlistId: powPlaylistId, - pageToken: nextPageToken.isEmpty ? null : nextPageToken, + pageToken: nextPageToken, ), ); @@ -177,8 +171,13 @@ class _PkgOfWeekVideoFetcher { } } + pageTokensVisited.add(nextPageToken ?? ''); // advance to next page token - nextPageToken = rs.nextPageToken ?? ''; + nextPageToken = rs.nextPageToken; + if (nextPageToken == null || + pageTokensVisited.contains(nextPageToken)) { + break; + } } return videos; } finally { From 644eeb0fd494f5a29be7b0188ecb39889b33e6b5 Mon Sep 17 00:00:00 2001 From: Istvan Soos Date: Thu, 16 Jan 2025 17:30:26 +0100 Subject: [PATCH 3/3] remove nextPageToken repetition check --- app/lib/service/youtube/backend.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/lib/service/youtube/backend.dart b/app/lib/service/youtube/backend.dart index 829cf39258..bcf0df0f99 100644 --- a/app/lib/service/youtube/backend.dart +++ b/app/lib/service/youtube/backend.dart @@ -121,7 +121,6 @@ class _PkgOfWeekVideoFetcher { final youtube = YouTubeApi(apiClient); try { - final pageTokensVisited = {}; String? nextPageToken; final videos = []; @@ -171,11 +170,9 @@ class _PkgOfWeekVideoFetcher { } } - pageTokensVisited.add(nextPageToken ?? ''); // advance to next page token nextPageToken = rs.nextPageToken; - if (nextPageToken == null || - pageTokensVisited.contains(nextPageToken)) { + if (nextPageToken == null) { break; } }