fix: discard malformed video entries without failing pages
This commit is contained in:
@@ -73,6 +73,8 @@
|
||||
//! Feed includes saved `[channels]` entries, even untagged, plus direct tag entries, without duplicates.
|
||||
//! Untagging is not unsubscribing: remove mappings and tag references from the file to unsubscribe.
|
||||
//! Search pages contain 20 results; Feed/tag pages request 20 uploads per channel's /videos tab.
|
||||
//! Invalid individual video entries are discarded; raw entry counts still determine pagination.
|
||||
//! Invalid JSON, playlist-level metadata, and extractor failures remain page errors.
|
||||
//! In normal mode, approaching the loaded end fetches the next page without replacing rows/selection.
|
||||
//! Loading result/preview panes animate a spinner after their title, including additional pages.
|
||||
//! Info spinners cover image download/decode/resize and description fetching; errors stop them.
|
||||
|
||||
+101
-40
@@ -30,8 +30,10 @@ pub struct Video {
|
||||
}
|
||||
|
||||
#[derive(Deserialize)]
|
||||
struct SearchResults {
|
||||
entries: Vec<Option<Video>>,
|
||||
struct SearchResults<'a> {
|
||||
// Borrow raw rows so one schema error cannot abort siblings or copy unused metadata.
|
||||
#[serde(borrow)]
|
||||
entries: Vec<&'a serde_json::value::RawValue>,
|
||||
channel: Option<String>,
|
||||
channel_id: Option<String>,
|
||||
uploader_id: Option<String>,
|
||||
@@ -41,10 +43,15 @@ pub fn search_results(output: &str) -> Result<Page, String> {
|
||||
let results: SearchResults = serde_json::from_str(output)
|
||||
.map_err(|error| format!("Invalid search response: {error}"))?;
|
||||
let has_more = results.entries.len() >= PAGE_SIZE;
|
||||
let mut videos: Vec<Video> = results.entries.into_iter().flatten().collect();
|
||||
for video in &mut videos {
|
||||
validate_video(video).map_err(|error| format!("Search returned {error}"))?;
|
||||
}
|
||||
let videos = results
|
||||
.entries
|
||||
.into_iter()
|
||||
.filter_map(|entry| {
|
||||
let mut video = serde_json::from_str(entry.get()).ok()?;
|
||||
validate_video(&mut video).ok()?;
|
||||
Some(video)
|
||||
})
|
||||
.collect();
|
||||
Ok(Page {
|
||||
videos,
|
||||
has_more,
|
||||
@@ -128,19 +135,12 @@ pub fn feed_results(output: &str) -> Result<Page, String> {
|
||||
return Err("Feed returned an invalid YouTube channel ID".into());
|
||||
}
|
||||
// Raw rows include unavailable videos; filtering them must not end pagination early.
|
||||
if playlist.entries.len() >= PAGE_SIZE {
|
||||
has_more = true;
|
||||
let oldest = playlist
|
||||
.entries
|
||||
.iter()
|
||||
.flatten()
|
||||
.filter_map(|video| video.timestamp)
|
||||
.min();
|
||||
// A full undated/unavailable page may hide newer uploads on its next page.
|
||||
unknown_frontier |= oldest.is_none();
|
||||
cutoff = cutoff.max(oldest);
|
||||
}
|
||||
for mut video in playlist.entries.into_iter().flatten() {
|
||||
let full_page = playlist.entries.len() >= PAGE_SIZE;
|
||||
let first_video = videos.len();
|
||||
for entry in playlist.entries {
|
||||
let Ok(mut video) = serde_json::from_str::<Video>(entry.get()) else {
|
||||
continue;
|
||||
};
|
||||
if video.channel_id.is_none() {
|
||||
video.channel_id = playlist.channel_id.clone();
|
||||
}
|
||||
@@ -150,8 +150,19 @@ pub fn feed_results(output: &str) -> Result<Page, String> {
|
||||
if video.channel.is_none() {
|
||||
video.channel = playlist.channel.clone();
|
||||
}
|
||||
validate_video(&mut video).map_err(|error| format!("Feed returned {error}"))?;
|
||||
videos.push(video);
|
||||
if validate_video(&mut video).is_ok() {
|
||||
videos.push(video);
|
||||
}
|
||||
}
|
||||
if full_page {
|
||||
has_more = true;
|
||||
let oldest = videos[first_video..]
|
||||
.iter()
|
||||
.filter_map(|video| video.timestamp)
|
||||
.min();
|
||||
// A full undated/unavailable page may hide newer uploads on its next page.
|
||||
unknown_frontier |= oldest.is_none();
|
||||
cutoff = cutoff.max(oldest);
|
||||
}
|
||||
}
|
||||
if playlists == 0 {
|
||||
@@ -561,7 +572,66 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_handles_unavailable_entries_and_rejects_invalid_ids() {
|
||||
fn bad_video_entries_do_not_discard_valid_results() {
|
||||
for parse in [search_results, feed_results] {
|
||||
for bad in [
|
||||
r#"{"id":"UC4QobU6STFB0P71PMvOGN5A","title":"Channel result"}"#,
|
||||
r#"{"id":"--bad","title":"Unsafe ID"}"#,
|
||||
r#"{"id":"jNQXAC9IVRw","title":"Bad channel","channel_id":"malformed"}"#,
|
||||
r#"{"id":"jNQXAC9IVRw","title":"Bad handle","uploader_id":"@jawed\u001b"}"#,
|
||||
r#"{"id":42,"title":"Wrong ID type"}"#,
|
||||
r#"{"id":"jNQXAC9IVRw","title":null}"#,
|
||||
r#"{"id":"jNQXAC9IVRw","title":"Bad date type","timestamp":"yesterday"}"#,
|
||||
r#"{"id":"jNQXAC9IVRw","title":"Bad channel type","channel":[]}"#,
|
||||
r#"{"id":"jNQXAC9IVRw"}"#,
|
||||
"null",
|
||||
"42",
|
||||
"[]",
|
||||
"{}",
|
||||
] {
|
||||
let output = format!(
|
||||
r#"{{"entries":[{{"id":"jNQXAC9IVRw","title":"Before"}},{bad},{{"id":"rQ_J9WH6CGk","title":"After"}}]}}"#,
|
||||
);
|
||||
let page = parse(&output).unwrap_or_else(|error| {
|
||||
panic!("bad entry {bad} discarded the entire page: {error}")
|
||||
});
|
||||
assert_eq!(
|
||||
page.videos
|
||||
.iter()
|
||||
.map(|video| video.title.as_str())
|
||||
.collect::<Vec<_>>(),
|
||||
["Before", "After"]
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn discarded_entries_still_count_for_pagination_but_not_feed_frontiers() {
|
||||
let bad = serde_json::json!({"id":"invalid", "title":"Bad", "timestamp":-500});
|
||||
let mut entries = vec![bad; PAGE_SIZE];
|
||||
for parse in [search_results, feed_results] {
|
||||
let page = parse(&serde_json::json!({"entries":entries}).to_string()).unwrap();
|
||||
assert!(page.videos.is_empty());
|
||||
assert!(
|
||||
page.has_more,
|
||||
"an all-invalid full page must not stop pagination"
|
||||
);
|
||||
assert_eq!(page.cutoff, None);
|
||||
}
|
||||
entries[0] = serde_json::json!({"id":"jNQXAC9IVRw","title":"Valid","timestamp":100});
|
||||
let page = feed_results(&serde_json::json!({"entries":entries}).to_string()).unwrap();
|
||||
assert!(page.has_more);
|
||||
assert_eq!(
|
||||
page.cutoff,
|
||||
Some(100),
|
||||
"invalid dates must not release old feed rows"
|
||||
);
|
||||
assert_eq!(page.videos[0].id, "jNQXAC9IVRw");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn search_sanitizes_display_metadata_and_rejects_invalid_envelopes() {
|
||||
let videos = search_results(
|
||||
r#"{"entries":[null,{"id":"rQ_J9WH6CGk","title":"Rust\u001b","channel":null,"uploader_id":"@RustVideos"}]}"#,
|
||||
)
|
||||
@@ -571,17 +641,15 @@ mod tests {
|
||||
assert_eq!(videos[0].channel, None);
|
||||
assert_eq!(videos[0].channel_id, None);
|
||||
assert_eq!(videos[0].uploader_id.as_deref(), Some("@RustVideos"));
|
||||
assert!(
|
||||
search_results(
|
||||
r#"{"entries":[{"id":"rQ_J9WH6CGk","title":"Bad","channel_id":"not-a-channel"}]}"#
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
assert!(search_results(
|
||||
r#"{"entries":[{"id":"jNQXAC9IVRw","title":"Unsafe","uploader_id":"@jawed\u001b"}]}"#
|
||||
).is_err());
|
||||
assert!(search_results(r#"{"entries":[{"id":"--bad","title":"Bad"}]}"#).is_err());
|
||||
assert!(search_results("not JSON").is_err());
|
||||
for output in [
|
||||
"not JSON",
|
||||
"{}",
|
||||
r#"{"entries":null}"#,
|
||||
r#"{"entries":{}}"#,
|
||||
r#"{"entries":[{"id":} ]}"#,
|
||||
] {
|
||||
assert!(search_results(output).is_err());
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -620,13 +688,6 @@ mod tests {
|
||||
.videos;
|
||||
assert_eq!(unavailable[0].channel_id, None);
|
||||
assert!(feed_results(r#"{"channel_id":"malformed","entries":[]}"#).is_err());
|
||||
assert!(
|
||||
feed_results(
|
||||
r#"{"entries":[{"id":"jNQXAC9IVRw","title":"Bad","channel_id":"malformed"}]}"#
|
||||
)
|
||||
.is_err()
|
||||
);
|
||||
assert!(feed_results(r#"{"entries":[{"id":"bad","title":"Bad"}]}"#).is_err());
|
||||
assert!(feed_results("").is_err());
|
||||
assert!(feed_results("{}").is_err());
|
||||
assert!(feed_results("{not JSON}").is_err());
|
||||
|
||||
Reference in New Issue
Block a user