diff options
author | Benedikt Peetz <benedikt.peetz@b-peetz.de> | 2025-07-24 16:20:34 +0200 |
---|---|---|
committer | Benedikt Peetz <benedikt.peetz@b-peetz.de> | 2025-07-24 16:20:34 +0200 |
commit | a9fddbeebf428eb57c60afab96fbbd38629a636e (patch) | |
tree | 480d9c1ae6f43598bfff7e4a29a378691b0a7508 /crates | |
parent | fix(crates/yt/{commands/playlist,videos/format_video}): Correctly calculate w... (diff) | |
download | yt-a9fddbeebf428eb57c60afab96fbbd38629a636e.zip |
fix(treewide): Use `json_try_get!` instead of `json.get(..).map(|| ..)`
`json.get` will return `Some(Value::Null)` if the json key exists but has been set to `null`. This is obviously not what we want, and as such we also need to check that the value is not null, before calling map. The `json_try_get!` macro does exactly that.
Diffstat (limited to 'crates')
-rw-r--r-- | crates/yt/src/commands/select/implm/standalone/add.rs | 17 | ||||
-rw-r--r-- | crates/yt/src/commands/subscriptions/implm.rs | 16 | ||||
-rw-r--r-- | crates/yt/src/commands/update/implm/updater.rs | 7 | ||||
-rw-r--r-- | crates/yt/src/commands/videos/implm.rs | 4 | ||||
-rw-r--r-- | crates/yt/src/storage/db/extractor_hash.rs | 14 | ||||
-rw-r--r-- | crates/yt/src/storage/db/get/video/mod.rs | 15 | ||||
-rw-r--r-- | crates/yt/src/storage/db/subscription.rs | 4 | ||||
-rw-r--r-- | crates/yt/src/yt_dlp/mod.rs | 54 | ||||
-rw-r--r-- | crates/yt/tests/watch/mod.rs | 6 | ||||
-rw-r--r-- | crates/yt_dlp/src/lib.rs | 44 |
10 files changed, 91 insertions, 90 deletions
diff --git a/crates/yt/src/commands/select/implm/standalone/add.rs b/crates/yt/src/commands/select/implm/standalone/add.rs index ec32039..dd11cb4 100644 --- a/crates/yt/src/commands/select/implm/standalone/add.rs +++ b/crates/yt/src/commands/select/implm/standalone/add.rs @@ -17,7 +17,7 @@ use crate::{ use anyhow::{Context, Result, bail}; use log::{error, warn}; use url::Url; -use yt_dlp::{YoutubeDL, info_json::InfoJson, json_cast, json_get}; +use yt_dlp::{YoutubeDL, info_json::InfoJson, json_cast, json_get, json_try_get}; #[allow(clippy::too_many_lines)] pub(crate) async fn add( @@ -55,14 +55,10 @@ pub(crate) async fn add( .await .with_context(|| format!( "Failed to format hash of video '{}' as short hash", - entry - .get("url") - .map_or("<Unknown video Url>".to_owned(), ToString::to_string) + json_try_get!(entry, "url", as_str).unwrap_or("<Unknown video Url>") ))?, - entry.get("title").map_or(String::new(), |title| format!( - " (\"{}\")", - json_cast!(title, as_str) - )) + json_try_get!(entry, "title", as_str) + .map_or(String::new(), |title| format!(" (\"{title}\")")) ); return Ok(()); } @@ -82,7 +78,7 @@ pub(crate) async fn add( .extract_info(&url, false, true) .with_context(|| format!("Failed to fetch entry for url: '{url}'"))?; - match entry.get("_type").map(|val| json_cast!(val, as_str)) { + match json_try_get!(entry, "_type", as_str) { Some("video") => { add_entry(app, entry).await?; if start.is_some() || stop.is_some() { @@ -92,8 +88,7 @@ pub(crate) async fn add( } } Some("playlist") => { - if let Some(entries) = entry.get("entries") { - let entries = json_cast!(entries, as_array); + if let Some(entries) = json_try_get!(entry, "entries", as_array) { let start = start.unwrap_or(0); let stop = stop.unwrap_or(entries.len() - 1); diff --git a/crates/yt/src/commands/subscriptions/implm.rs b/crates/yt/src/commands/subscriptions/implm.rs index 3051522..311ae2b 100644 --- a/crates/yt/src/commands/subscriptions/implm.rs +++ b/crates/yt/src/commands/subscriptions/implm.rs @@ -16,7 +16,7 @@ use tokio::{ io::{AsyncBufRead, AsyncBufReadExt, BufReader, stdin}, }; use url::Url; -use yt_dlp::{json_cast, json_get, options::YoutubeDLOptions}; +use yt_dlp::{json_cast, json_get, json_try_get, options::YoutubeDLOptions}; impl SubscriptionCommand { pub(crate) async fn implm(self, app: &App) -> Result<()> { @@ -27,10 +27,10 @@ impl SubscriptionCommand { no_check, } => { let mut ops = Operations::new("main: subscribe"); - subscribe(&app, name, url, no_check, &mut ops) + subscribe(app, name, url, no_check, &mut ops) .await .context("Failed to add a subscription")?; - ops.commit(&app).await?; + ops.commit(app).await?; } SubscriptionCommand::Remove { name } => { let mut present_subscriptions = Subscriptions::get(app).await?; @@ -46,14 +46,14 @@ impl SubscriptionCommand { .with_context(|| format!("Failed to unsubscribe from {name:?}"))?; } SubscriptionCommand::List {} => { - let all_subs = Subscriptions::get(&app).await?; + let all_subs = Subscriptions::get(app).await?; for (key, val) in all_subs.0 { println!("{}: '{}'", key, val.url); } } SubscriptionCommand::Export {} => { - let all_subs = Subscriptions::get(&app).await?; + let all_subs = Subscriptions::get(app).await?; for val in all_subs.0.values() { println!("{}", val.url); } @@ -66,9 +66,9 @@ impl SubscriptionCommand { if let Some(file) = file { let f = File::open(file).await?; - import(&app, BufReader::new(f), force, no_check).await?; + import(app, BufReader::new(f), force, no_check).await?; } else { - import(&app, BufReader::new(stdin()), force, no_check).await?; + import(app, BufReader::new(stdin()), force, no_check).await?; } } } @@ -215,7 +215,7 @@ async fn actual_subscribe( let info = yt_dlp.extract_info(&url, false, false)?; - if info.get("_type").map(|v| json_cast!(v, as_str)) == Some("playlist") { + if json_try_get!(info, "_type", as_str) == Some("playlist") { json_get!(info, "title", as_str).to_owned() } else { bail!("The url ('{}') does not represent a playlist!", &url) diff --git a/crates/yt/src/commands/update/implm/updater.rs b/crates/yt/src/commands/update/implm/updater.rs index 5969d54..3e4bc85 100644 --- a/crates/yt/src/commands/update/implm/updater.rs +++ b/crates/yt/src/commands/update/implm/updater.rs @@ -6,8 +6,7 @@ use log::{Level, debug, error, log_enabled}; use tokio::io::{AsyncWriteExt, stderr}; use tokio_util::task::LocalPoolHandle; use yt_dlp::{ - info_json::InfoJson, json_cast, options::YoutubeDLOptions, process_ie_result, - python_error::PythonError, + info_json::InfoJson, json_cast, json_try_get, options::YoutubeDLOptions, process_ie_result, python_error::PythonError }; use crate::{ @@ -93,9 +92,7 @@ impl Updater { .with_context(|| format!("Failed to get playlist '{}'.", sub.name))?; let empty = vec![]; - let entries = info - .get("entries") - .map_or(&empty, |val| json_cast!(val, as_array)); + let entries = json_try_get!(info, "entries", as_array).unwrap_or(&empty); let valid_entries: Vec<(Subscription, InfoJson)> = entries .iter() diff --git a/crates/yt/src/commands/videos/implm.rs b/crates/yt/src/commands/videos/implm.rs index 7d13ceb..7c2ec8a 100644 --- a/crates/yt/src/commands/videos/implm.rs +++ b/crates/yt/src/commands/videos/implm.rs @@ -42,12 +42,12 @@ impl VideosCommand { } } VideosCommand::Info { hash, format } => { - let video = hash.realize(&app, None).await?.get_with_app(&app).await?; + let video = hash.realize(app, None).await?.get_with_app(app).await?; print!( "{}", &video - .to_info_display(&app, format) + .to_info_display(app, format) .await .context("Failed to format video")? ); diff --git a/crates/yt/src/storage/db/extractor_hash.rs b/crates/yt/src/storage/db/extractor_hash.rs index 15b57e7..3ad8273 100644 --- a/crates/yt/src/storage/db/extractor_hash.rs +++ b/crates/yt/src/storage/db/extractor_hash.rs @@ -16,7 +16,7 @@ use blake3::Hash; use log::debug; use serde::{Deserialize, Serialize}; use tokio::sync::OnceCell; -use yt_dlp::{info_json::InfoJson, json_cast, json_get}; +use yt_dlp::{info_json::InfoJson, json_cast, json_get, json_try_get}; use crate::app::App; @@ -118,10 +118,10 @@ impl ExtractorHash { // NOTE(@bpeetz): `yt-dlp` apparently uses these two different names for the same thing <2025-07-04> let ie_key = { - if let Some(val) = entry.get("ie_key") { - json_cast!(val, as_str) - } else if let Some(val) = entry.get("extractor_key") { - json_cast!(val, as_str) + if let Some(ie_key) = json_try_get!(entry, "ie_key", as_str) { + ie_key + } else if let Some(extractor_key) = json_try_get!(entry, "extractor_key", as_str) { + extractor_key } else { unreachable!( "Either `ie_key` or `extractor_key` \ @@ -136,8 +136,8 @@ impl ExtractorHash { // one (when it is still available!). The new one is called `display_id`. // Therefore, we simply check if the new one is explicitly returned, and otherwise use the // normal `id` value, as these are cases where the old one is no longer available. <2025-07-04> - let id = if let Some(val) = entry.get("display_id") { - json_cast!(val, as_str).as_bytes() + let id = if let Some(display_id) = json_try_get!(entry, "display_id", as_str) { + display_id.as_bytes() } else { json_get!(entry, "id", as_str).as_bytes() }; diff --git a/crates/yt/src/storage/db/get/video/mod.rs b/crates/yt/src/storage/db/get/video/mod.rs index 5f6700e..1f847cb 100644 --- a/crates/yt/src/storage/db/get/video/mod.rs +++ b/crates/yt/src/storage/db/get/video/mod.rs @@ -3,7 +3,7 @@ use std::{fs::File, path::PathBuf}; use anyhow::{Context, Result, bail}; use log::debug; use sqlx::query; -use yt_dlp::{info_json::InfoJson, json_cast}; +use yt_dlp::{info_json::InfoJson, json_cast, json_try_get}; use crate::{ app::App, @@ -64,9 +64,8 @@ impl Video { And thus the info.json should be available.", ); - let description = info_json - .get("description") - .map_or("<No description>", |val| json_cast!(val, as_str)) + let description = json_try_get!(info_json, "description", as_str) + .unwrap_or("<No description>") .to_owned(); Ok(description) @@ -90,8 +89,8 @@ impl Video { And thus the info.json should be available.", ); - let raw_comments = if let Some(comments) = info_json.get("comments") { - json_cast!(comments, as_array) + let raw_comments = if let Some(comments) = json_try_get!(info_json, "comments", as_array) { + comments .iter() .cloned() .map(serde_json::from_value) @@ -101,9 +100,7 @@ impl Video { bail!( "The video ('{}') does not have comments!", - info_json - .get("title") - .map_or("<No Title>", |val| json_cast!(val, as_str)) + json_try_get!(info_json, "title", as_str).unwrap_or("<No Title>") ) }; diff --git a/crates/yt/src/storage/db/subscription.rs b/crates/yt/src/storage/db/subscription.rs index 0d9e160..eb03e47 100644 --- a/crates/yt/src/storage/db/subscription.rs +++ b/crates/yt/src/storage/db/subscription.rs @@ -4,7 +4,7 @@ use anyhow::Result; use log::debug; use serde::{Deserialize, Serialize}; use url::Url; -use yt_dlp::{json_cast, options::YoutubeDLOptions}; +use yt_dlp::{json_cast, json_try_get, options::YoutubeDLOptions}; #[derive(Clone, Debug, Serialize, Deserialize)] pub(crate) struct Subscription { @@ -38,5 +38,5 @@ pub(crate) async fn check_url(url: Url) -> Result<bool> { debug!("{info:#?}"); - Ok(info.get("_type").map(|v| json_cast!(v, as_str)) == Some("playlist")) + Ok(json_try_get!(info, "_type", as_str) == Some("playlist")) } diff --git a/crates/yt/src/yt_dlp/mod.rs b/crates/yt/src/yt_dlp/mod.rs index edf27e8..307ecdc 100644 --- a/crates/yt/src/yt_dlp/mod.rs +++ b/crates/yt/src/yt_dlp/mod.rs @@ -7,7 +7,9 @@ use log::{error, warn}; use serde_json::json; use tokio::{fs, io}; use url::Url; -use yt_dlp::{YoutubeDL, info_json::InfoJson, json_cast, json_get, options::YoutubeDLOptions}; +use yt_dlp::{ + YoutubeDL, info_json::InfoJson, json_cast, json_get, json_try_get, options::YoutubeDLOptions, +}; use crate::{ app::App, @@ -53,20 +55,19 @@ impl Video { .extract_info(&self.url, false, true) .with_context(|| format!("Failed to extract video information: '{}'", self.title))?; - let size = if let Some(val) = result.get("filesize") { - json_cast!(val, as_u64) - } else if let Some(serde_json::Value::Number(num)) = result.get("filesize_approx") { - // NOTE(@bpeetz): yt_dlp sets this value to `Null`, instead of omitting it when it - // can't calculate the approximate filesize. - // Thus, we have to check, that it is actually non-null, before we cast it. <2025-06-15> - json_cast!(num, as_u64) - } else if result.get("duration").is_some() && result.get("tbr").is_some() { + let size = if let Some(filesize) = json_try_get!(result, "filesize", as_u64) { + filesize + } else if let Some(num) = json_try_get!(result, "filesize_approx", as_u64) { + num + } else if let Some(duration) = json_try_get!(result, "duration", as_f64) + && let Some(tbr) = json_try_get!(result, "tbr", as_f64) + { #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] - let duration = json_get!(result, "duration", as_f64).ceil() as u64; + let duration = duration.ceil() as u64; // TODO: yt_dlp gets this from the format #[allow(clippy::cast_sign_loss, clippy::cast_possible_truncation)] - let tbr = json_get!(result, "tbr", as_f64).ceil() as u64; + let tbr = tbr.ceil() as u64; duration * tbr * (1000 / 8) } else { @@ -97,9 +98,7 @@ impl Video { } } - let publish_date = if let Some(date) = &entry.get("upload_date") { - let date = json_cast!(date, as_str); - + let publish_date = if let Some(date) = json_try_get!(entry, "upload_date", as_str) { let year: u32 = date .chars() .take(4) @@ -135,13 +134,16 @@ impl Video { None }; - let thumbnail_url = match (&entry.get("thumbnails"), &entry.get("thumbnail")) { + let thumbnail_url = match ( + &json_try_get!(entry, "thumbnails", as_array), + &json_try_get!(entry, "thumbnail", as_str), + ) { (None, None) => None, - (None, Some(thumbnail)) => Some(Url::from_str(json_cast!(thumbnail, as_str))?), + (None, Some(thumbnail)) => Some(Url::from_str(thumbnail)?), // TODO: The algorithm is not exactly the best <2024-05-28> (Some(thumbnails), None) => { - if let Some(thumbnail) = json_cast!(thumbnails, as_array).first() { + if let Some(thumbnail) = thumbnails.first() { Some(Url::from_str(json_get!( json_cast!(thumbnail, as_object), "url", @@ -151,7 +153,7 @@ impl Video { None } } - (Some(_), Some(thumnail)) => Some(Url::from_str(json_cast!(thumnail, as_str))?), + (Some(_), Some(thumnail)) => Some(Url::from_str(thumnail)?), }; let url = { @@ -171,12 +173,8 @@ impl Video { let subscription_name = if let Some(sub) = sub { Some(sub.name.clone()) - } else if let Some(uploader) = entry.get("uploader").map(|val| json_cast!(val, as_str)) { - if entry - .get("webpage_url_domain") - .map(|val| json_cast!(val, as_str)) - == Some("youtube.com") - { + } else if let Some(uploader) = json_try_get!(entry, "uploader", as_str) { + if json_try_get!(entry, "webpage_url_domain", as_str) == Some("youtube.com") { Some(format!("{uploader} - Videos")) } else { Some(uploader.to_owned()) @@ -186,12 +184,8 @@ impl Video { }; let video = Video { - description: entry - .get("description") - .map(|val| json_cast!(val, as_str).to_owned()), - duration: MaybeDuration::from_maybe_secs_f64( - entry.get("duration").map(|val| json_cast!(val, as_f64)), - ), + description: json_try_get!(entry, "description", as_str).map(ToOwned::to_owned), + duration: MaybeDuration::from_maybe_secs_f64(json_try_get!(entry, "duration", as_f64)), extractor_hash, last_status_change: TimeStamp::from_now(), parent_subscription_name: subscription_name, diff --git a/crates/yt/tests/watch/mod.rs b/crates/yt/tests/watch/mod.rs index 7d6f264..deea597 100644 --- a/crates/yt/tests/watch/mod.rs +++ b/crates/yt/tests/watch/mod.rs @@ -7,7 +7,7 @@ use std::{ use colors::{Colorize, IntoCanvas}; use serde_json::json; -use yt_dlp::{json_cast, json_get}; +use yt_dlp::{json_cast, json_get, json_try_get}; use crate::_testenv::TestEnv; @@ -94,8 +94,8 @@ impl MpvControl { serde_json::from_str(&buf).expect("Mpv only returns json") }; - if let Some(rid) = response.get("request_id") { - if json_cast!(rid, as_u64) == tl_rid { + if let Some(rid) = json_try_get!(response, "request_id", as_u64) { + if rid == tl_rid { let error = json_get!(response, "error", as_str); if error == "success" { diff --git a/crates/yt_dlp/src/lib.rs b/crates/yt_dlp/src/lib.rs index dc602db..6be5e87 100644 --- a/crates/yt_dlp/src/lib.rs +++ b/crates/yt_dlp/src/lib.rs @@ -50,21 +50,36 @@ macro_rules! json_get { } #[macro_export] +macro_rules! json_try_get { + ($value:expr, $name:literal, $into:ident) => {{ + if let Some(val) = $value.get($name) { + if val.is_null() { + None + } else { + Some(json_cast!(@log_key $name, val, $into)) + } + } else { + None + } + }}; +} + +#[macro_export] macro_rules! json_cast { ($value:expr, $into:ident) => {{ - json_cast!(@log_key "<unknown>", $value, $into) + let value_name = stringify!($value); + json_cast!(@log_key value_name, $value, $into) }}; - (@log_key $name:literal, $value:expr, $into:ident) => {{ + (@log_key $name:expr, $value:expr, $into:ident) => {{ match $value.$into() { Some(result) => result, None => panic!( concat!( - "Expected to be able to cast '", - $name, - "' value ({:#?}) ", + "Expected to be able to cast '{}' value (which is '{:?}') ", stringify!($into) ), + $name, $value ), } @@ -130,8 +145,9 @@ impl YoutubeDL { let info_json = self.extract_info(url, true, true)?; // Try to work around yt-dlp type weirdness - let result_string = if let Some(filename) = info_json.get("filename") { - PathBuf::from(json_cast!(filename, as_str)) + let result_string = if let Some(filename) = json_try_get!(info_json, "filename", as_str) + { + PathBuf::from(filename) } else { PathBuf::from(json_get!( json_cast!( @@ -195,9 +211,10 @@ impl YoutubeDL { // already resolved. Do nothing } else if let Ok(generator) = generator.downcast::<PyIterator>() { // A python generator object. - let max_backlog = self.options.get("playlistend").map_or(10000, |value| { - usize::try_from(json_cast!(value, as_u64)).expect("Should work") - }); + let max_backlog = json_try_get!(self.options, "playlistend", as_u64) + .map_or(10000, |playlistend| { + usize::try_from(playlistend).expect("Should work") + }); let mut out = vec![]; for output in generator { @@ -211,9 +228,10 @@ impl YoutubeDL { result.set_item(intern!(py, "entries"), out).wrap_exc(py)?; } else { // Probably some sort of paged list (`OnDemand` or otherwise) - let max_backlog = self.options.get("playlistend").map_or(10000, |value| { - usize::try_from(json_cast!(value, as_u64)).expect("Should work") - }); + let max_backlog = json_try_get!(self.options, "playlistend", as_u64) + .map_or(10000, |playlistend| { + usize::try_from(playlistend).expect("Should work") + }); let next = generator.getattr(intern!(py, "getslice")).wrap_exc(py)?; |