-
-
Notifications
You must be signed in to change notification settings - Fork 504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GetVideosAsync fix #821
GetVideosAsync fix #821
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Added additional variable, which will not contain videos, that were already added.
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -189,4 +189,16 @@ public async Task I_can_get_a_subset_of_videos_included_in_a_playlist() | |||||||
// Assert | ||||||||
videos.Should().HaveCount(10); | ||||||||
} | ||||||||
|
||||||||
[Fact] | ||||||||
public async Task I_can_get_videos_included_in_a_buggy_playlist() | ||||||||
{ | ||||||||
var youtube = new YoutubeClient(); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
|
||||||||
// Act | ||||||||
var videos = await youtube.Playlists.GetVideosAsync(PlaylistIds.EnormousDuplicates); | ||||||||
|
||||||||
// Assert | ||||||||
videos.Should().HaveCountGreaterOrEqualTo(3_900); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe also worth asserting that there are no duplicates |
||||||||
} | ||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,10 @@ | |
using System.Runtime.CompilerServices; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using YoutubeExplode.Bridge; | ||
using YoutubeExplode.Common; | ||
using YoutubeExplode.Exceptions; | ||
using YoutubeExplode.Utils.Extensions; | ||
using YoutubeExplode.Videos; | ||
|
||
namespace YoutubeExplode.Playlists; | ||
|
@@ -92,9 +94,15 @@ public async IAsyncEnumerable<Batch<PlaylistVideo>> GetVideoBatchesAsync( | |
); | ||
|
||
var videos = new List<PlaylistVideo>(); | ||
var originalVideos = response.Videos.Where(v => | ||
{ | ||
return !encounteredIds.Any(e => string.Equals(e.Value, v.Id)); | ||
}); | ||
|
||
foreach (var videoData in response.Videos) | ||
foreach (var videoData in originalVideos) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused what fundamentally changed between the old and the current implementation. Can you explain in more detail? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Sorry for late answer. To be honest it was hard for me to detect it (and even harder to explain it), but I noticed that sometimes our duplicates can be as a last list element and while they are not added to It's probably make sense to remove if condition in foreach and just add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. BTW, I realised that duplicates might be in the beginning and then my suggestion doesn't cover it, so I will improve solution. |
||
{ | ||
PlaylistVideoData? lastVideoData = null ?? response.Videos.Last(); | ||
|
||
var videoId = | ||
videoData.Id | ||
?? throw new YoutubeExplodeException("Failed to extract the video ID."); | ||
|
@@ -105,9 +113,14 @@ public async IAsyncEnumerable<Batch<PlaylistVideo>> GetVideoBatchesAsync( | |
videoData.Index | ||
?? throw new YoutubeExplodeException("Failed to extract the video index."); | ||
|
||
// Don't yield the same video twice | ||
if (!encounteredIds.Add(videoId)) | ||
{ | ||
/*if (videoData.Id != lastVideoData.Id) | ||
{ | ||
continue; | ||
}*/ | ||
continue; | ||
} | ||
|
||
var videoTitle = | ||
videoData.Title | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it a better description?