Fix caching of Youtube API pagination + filter duplicate videoIds. (d…

…art-lang#8472)
isoos · Jan 16, 2025 · 2b0b5c4 · 2b0b5c4
1 parent 7065813
commit 2b0b5c4
Show file tree

Hide file tree

Showing 3 changed files with 58 additions and 51 deletions.
diff --git a/app/lib/service/youtube/backend.dart b/app/lib/service/youtube/backend.dart
@@ -121,54 +121,60 @@ class _PkgOfWeekVideoFetcher {
     final youtube = YouTubeApi(apiClient);
 
     try {
-      final videos = <PkgOfWeekVideo>[];
       String? nextPageToken;
-      for (var check = true; check && videos.length < 50;) {
-        final rs = await cache.youtubePlaylistItems().get(
+
+      final videos = <PkgOfWeekVideo>[];
+      final videoIds = <String>{};
+      while (videos.length < 50) {
+        // 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,
               ),
             );
-        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
+            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;
-        check = nextPageToken != null && nextPageToken.isNotEmpty;
+        if (nextPageToken == null) {
+          break;
+        }
       }
       return videos;
     } finally {

diff --git 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<String, dynamic>),
           ))['$package-$version'];
 
-  Entry<PlaylistItemListResponse> 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<String, dynamic>),
-      ))[''];
+  Entry<PlaylistItemListResponse> 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<String, dynamic>),
+          ))[pageToken];
 }
 
 /// The active cache.

diff --git 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 = <int>[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));