diff --git a/AGENTS.md b/AGENTS.md index 3ddc70e..de4e31a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,7 +44,7 @@ The `x-media` library: `site::fetch(url)` dispatches through the `SITES` registr | `crates/xmedia-bot/src/handlers/` | Handler modules: `mod.rs` (message entry point, `reply`, `log_key`, the group-only `GROUP_LINK_HINT` for a supported link posted outside a private chat), `commands.rs` (teloxide `BotCommands` enum + command executor, incl. `/test ` (send-only) / `/debug ` (parse-only) and the admin-only `/bot_dict` state dump; `/set_format` rejects unknown `{…}` placeholders and resets with `-`), `urls.rs` (URL extraction + bounded job channel (256) drained by `URL_WORKERS = 8` workers (`start_url_workers`) — backpressure instead of unbounded spawns; teloxide's per-chat workers are sequential — batch-forwards need concurrency), `inline.rs`/`callback.rs` (inline queries / edit-before-forward buttons, incl. `skip`; a forward that fails retryably is both queued *and* settles the prompt — the queued row carries the message ids itself, and a prompt left live let a second Confirm copy the same messages twice and let Skip answer "nothing was forwarded" while the row still delivered), `statics.rs` (global statics) | | `crates/xmedia-bot/src/state.rs` | `ChatStore`: parking_lot `Mutex` cache + SQLite write-through (`chat_state` table) | | `crates/xmedia-bot/src/link_cache.rs` | `LinkCache`: SQLite-backed cache (`link_cache` table) of successfully sent posts — raw caption fields + Telegram `file_id`s; repeat links re-send locally (no fetch/upload), TTL + prune, invalidated on permanent send failure | -| `crates/xmedia-bot/src/queue.rs` | `PersistentTaskQueue`: SQLite-backed queue (`tasks` table), `QUEUE_WORKERS = 4` concurrent workers (lease via `BEGIN IMMEDIATE` + `locked_until` TTL), retry→dead-letter, a `lease_token` fence: `lease_next` stamps a random token and every write-back (heartbeat, `delete`, `reschedule`, `mark_done`) is guarded by it, so a lease that expired and was re-leased cannot be written by its former holder — a lost lease stops the attempt instead; a finished row's `DELETE`/reschedule retried and a failed delete falling back to a `done` tombstone (the lease query and the sweep only look at `pending`/`in_progress`, so a task that already ran cannot be resurrected and re-run), `notify_one` worker wakeup plus a separate `Notify` for the 30 s lease-expiry sweep (a shared one let the sweep steal the workers' wakeup permit), `busy_timeout` on all connections | +| `crates/xmedia-bot/src/queue.rs` | `PersistentTaskQueue`: SQLite-backed queue (`tasks` table), `QUEUE_WORKERS = 4` concurrent workers (lease via `BEGIN IMMEDIATE` + `locked_until` TTL), retry→dead-letter, a `lease_token` fence: `lease_next` stamps a random token and every write-back (heartbeat, `delete`, `reschedule`, `mark_done`) is guarded by it, so a lease that expired and was re-leased cannot be written by its former holder — a lost lease stops the attempt instead; a finished row's `DELETE`/reschedule retried and a failed delete falling back to a `done` tombstone (the lease query and the sweep only look at `pending`/`in_progress`, so a task that already ran cannot be resurrected and re-run), `notify_one` worker wakeup plus a separate `Notify` for the 30 s lease-expiry sweep (a shared one let the sweep steal the workers' wakeup permit; the sweep does notify the workers after it actually recovered a row, since a recovered task is due immediately while every worker may be parked on `notify` with no pending row to sleep on), `busy_timeout` on all connections | | `crates/xmedia-bot/src/ctx.rs` | `AppContext`: the injected collaborators (`sender` + `ChatStore`/`PersistentTaskQueue`/`LinkCache`/`Config`), `from_statics` for production and the `CONTEXT` static the worker closures hold. `test_support::TestStores` backs handler tests with a tempdir store set | | `crates/xmedia-bot/src/send/` | `send/mod.rs`: `Task`/`MediaItemPayload` payloads, `SendError`/`Classification`, `send_media_sequence`/`send_animation`/`forward_messages`; `send/input_media.rs`: payload → `InputFile`/`InputMedia` + `build_media_group` (caption on the first item only); `send/upload.rs`: the download-and-reupload fallback (`prepare_upload_item`/`send_batch_via_upload`, photo downscale handoff); `send/post_send.rs`: link-cache write, `KEEP_ALIVE` registry, `settle_task`, `post_send_actions`, `handle_task`/`dead_letter_notify` | | `crates/xmedia-bot/src/media_sender.rs` | `MediaSender` trait: the user-flow surface (`send_media_group`/`send_animation`/`copy_messages`/`send_message`/`answer_callback_query`/`edit_message_caption`/`delete_message`/`send_chat_action`) implemented by teloxide `Bot` (per-chat rate-limited) and by a recording `MockSender` in tests. Admin/setup APIs (`get_chat`, `set_my_commands`, …) stay on the concrete `Bot` | @@ -105,9 +105,9 @@ Docker: `docker build -t tgxmb .` then `docker run --rm -d --name tgxmb --env-fi ## Testing & QA -- **~200 tests, all inline `#[cfg(test)] mod tests`** — no `tests/` integration directories. Framework: built-in Rust test + `#[tokio::test]` (dev-deps only in `x-media`: tokio macros/rt-multi-thread, dotenv). +- **~180 tests, all inline `#[cfg(test)] mod tests`** — no `tests/` integration directories. Framework: built-in Rust test + `#[tokio::test]` (dev-deps only in `x-media`: tokio macros/rt-multi-thread, dotenv). - No mocking framework anywhere (no mockito/wiremock/mockall). Conventions: pure-function units (regex parsing, serde round-trips, chunking, retry math) tested synchronously; async tests use real dependencies — file-backed SQLite via `tempfile` (`queue.rs::new_queue()` helper), live network fetches. -- Live-network tests exist in `site/twitter/interface.rs` (5), `site/bsky/interface.rs` (2), `site/misskey/interface.rs` (1), `site/bilibili/interface.rs` (4), `site/pixiv/api.rs` (1); `photo.rs` adds one `#[ignore = "heavy: …"]` test. `site/mod.rs` also has a **token-gated but not `#[ignore]`d** pixiv download test (`download_media_pixiv_original_with_referer`): it hits `i.pximg.net` whenever `PIXIV_REFRESH_TOKEN` is set, so a local `cargo test --workspace` is not fully offline and can flake on a pixiv CDN body timeout. `disabled_site_is_reported_not_ignored` (same file) is gated the other way round: it asserts `fetch` answers `FetchError::Disabled { site: "pixiv" }` for a pixiv link and early-returns when `PIXIV_REFRESH_TOKEN` **is** set (the site is then enabled). Test gating convention (enforced by `.github/workflows/ci.yml`): pure unit tests always run; live-network tests carry `#[ignore = "live network: ..."]` (run via `cargo test --workspace -- --ignored live`); token-gated pixiv tests early-return when `PIXIV_REFRESH_TOKEN` is absent **or empty** (an unset GitHub secret arrives as `""` — `is_err()` alone would run them tokenless and fail), and the bilibili live tests early-return when the API answers risk control (`-352`, which bilibili applies per IP by request volume). Run the full offline suite with `cargo test --workspace`. +- Live-network tests exist in `site/twitter/interface.rs` (5), `site/bsky/interface.rs` (1), `site/misskey/interface.rs` (1), `site/bilibili/interface.rs` (5), `site/pixiv/api.rs` (1); `photo.rs` adds one `#[ignore = "heavy: …"]` test. `site/mod.rs` also has a **token-gated but not `#[ignore]`d** pixiv download test (`download_media_pixiv_original_with_referer`): it hits `i.pximg.net` whenever `PIXIV_REFRESH_TOKEN` is set, so a local `cargo test --workspace` is not fully offline and can flake on a pixiv CDN body timeout. `disabled_site_is_reported_not_ignored` (same file) is gated the other way round: it asserts `fetch` answers `FetchError::Disabled { site: "pixiv" }` for a pixiv link and early-returns when `PIXIV_REFRESH_TOKEN` **is** set (the site is then enabled). Test gating convention (enforced by `.github/workflows/ci.yml`): pure unit tests always run; live-network tests carry `#[ignore = "live network: ..."]` (run via `cargo test --workspace -- --ignored live`); token-gated pixiv tests early-return when `PIXIV_REFRESH_TOKEN` is absent **or empty** (an unset GitHub secret arrives as `""` — `is_err()` alone would run them tokenless and fail), and the bilibili live tests early-return when the API answers risk control (`-352`, which bilibili applies per IP by request volume). Run the full offline suite with `cargo test --workspace`. - Fixtures are inline `serde_json::json!` builder fns (`fixture()`, `thread_json()`, `illust_json()`), not files. The shared `CLIENT` sets `pool_max_idle_per_host(0)` under `#[cfg(test)]` to avoid cross-runtime `DispatchGone`. - **CI** — `.github/workflows/ci.yml` (actions pinned to commit SHAs, `--locked` on every cargo invocation, `concurrency` cancels superseded runs, `RUST_BACKTRACE=1`) runs `cargo fmt --check` + `cargo clippy --workspace --all-targets --locked -- -D warnings` + `cargo test --workspace --locked` + a release-profile `cargo build --release --locked` + an `actions-rust-lang/audit` dependency-vulnerability gate (offline, no secrets, on every push/PR) and a `live` job (schedule/manual/tag only, `-p x-media` since every network/secret-gated test lives there, `continue-on-error`) for the `#[ignore]`d live + token tests. `.github/workflows/docker.yml` builds and pushes the image on master/tag and runs a **build-only check on pull requests touching the build inputs** (`Dockerfile`, entrypoint, manifests, `.dockerignore`); a release tag must match both crate versions or the build stops, and `FFMPEG_URL`/`FFMPEG_SHA256` are taken from repository variables when set (a release can pin an exact ffmpeg build). `.github/dependabot.yml` keeps crates, the pinned actions and the Docker base images current. - Untested and hard to test without a mock seam: `main.rs`, `config.rs`, `db.rs`, `handlers/statics.rs`, `media_sender.rs` (holds the `MockSender` itself); in `x-media`: `media.rs`, `lib.rs`, all `model.rs`. The `commands.rs` *executor* needs a real `Bot` (only its pure report builder is tested). Everything else — `handlers/{mod,callback,inline,urls}.rs`, `send/*`, `ctx.rs`, `state.rs`, `queue.rs`, `link_cache.rs`, `rate_limit.rs` — is driven through `TestStores`/`ctx::test_support` and the scripted `MockSender`. diff --git a/crates/x-media/src/site/bilibili/interface.rs b/crates/x-media/src/site/bilibili/interface.rs index efa88ab..74a7a99 100644 --- a/crates/x-media/src/site/bilibili/interface.rs +++ b/crates/x-media/src/site/bilibili/interface.rs @@ -563,7 +563,8 @@ mod tests { /// bot keeps ignoring them instead of answering with a failure. #[test] fn pattern_ignores_short_links() { - assert!(!PATTERN.is_match("https://b23.tv/abc123")); + // Short links usually point at videos, so they stay unmatched: no cache + // key, and the fetch dispatcher answers `Ok(None)` (silence). assert_eq!(cache_key("https://b23.tv/abc123"), None); } @@ -583,6 +584,8 @@ mod tests { } } + /// The legacy `major.draw` shape stays supported alongside the + /// `itemOpusStyle` serialization that moves pictures to `major.opus.pics`. #[test] fn from_item_maps_draw_images_and_topic() { let fetched = parse(item_json( @@ -746,24 +749,6 @@ mod tests { } } - /// The legacy shape stays supported: bilibili's `itemOpusStyle` flag is - /// what moves the pictures to `major.opus.pics`, but `major.draw` items - /// and a text-only `desc` must keep working if it is retired. - #[test] - fn from_item_legacy_draw_shape_still_parses() { - let fetched = parse(item_json( - draw_item("http://i0.hdslb.com/bfs/new_dyn/l.jpg"), - "legacy 正文", - )); - assert_eq!(fetched.title, ""); - assert_eq!(fetched.content, "legacy 正文"); - assert_eq!(fetched.media.len(), 1); - assert_eq!( - fetched.media[0].url(), - "https://i0.hdslb.com/bfs/new_dyn/l.jpg" - ); - } - /// The video stream is out of scope; an AV dynamic still yields its cover. #[test] fn from_item_maps_archive_cover() { diff --git a/crates/x-media/src/site/bsky/interface.rs b/crates/x-media/src/site/bsky/interface.rs index bd16a08..8d60943 100644 --- a/crates/x-media/src/site/bsky/interface.rs +++ b/crates/x-media/src/site/bsky/interface.rs @@ -490,31 +490,18 @@ mod tests { )); } + /// The one live bsky check: a labelled post with photos — source URL, + /// caption, media and the sensitive label all survive the parse. This + /// replaced a second byte-identical live test whose URL is a *text-only* + /// post, so neither copy pinned any media. #[tokio::test] #[ignore = "live network: requires outbound HTTPS to public.api.bsky.app"] async fn live_fetch_with_photos() { - let fetched = - fetch_from_url("https://bsky.app/profile/asagi0398.bsky.social/post/3mqkhrq5w6k2m") - .await - .unwrap(); - assert_eq!( - fetched.source_url, - "https://bsky.app/profile/asagi0398.bsky.social/post/3mqkhrq5w6k2m" - ); - assert!(!fetched.caption.is_empty()); - } - - #[tokio::test] - #[ignore = "live network: requires outbound HTTPS to public.api.bsky.app"] - async fn live_fetch_smoke() { - let fetched = - fetch_from_url("https://bsky.app/profile/fu-futa.bsky.social/post/3laoveufjv224") - .await - .unwrap(); - assert_eq!( - fetched.source_url, - "https://bsky.app/profile/fu-futa.bsky.social/post/3laoveufjv224" - ); + let url = "https://bsky.app/profile/fu-futa.bsky.social/post/3laoveufjv224"; + let fetched = fetch_from_url(url).await.unwrap(); + assert_eq!(fetched.source_url, url); assert!(!fetched.caption.is_empty()); + assert!(!fetched.media.is_empty(), "expected photos in {url}"); + assert!(fetched.sensitive, "expected a label on {url}"); } } diff --git a/crates/x-media/src/site/mod.rs b/crates/x-media/src/site/mod.rs index 2608f64..093bde2 100644 --- a/crates/x-media/src/site/mod.rs +++ b/crates/x-media/src/site/mod.rs @@ -778,13 +778,25 @@ mod tests { #[test] fn truncate_caption_does_not_split_an_html_entity() { - // An entity crossing the cut must not be left half-open (& without ;). - let mut long = "a".repeat(MAX_CAPTION_CHARS - 4); - long.push_str("&bbbb"); - let out = truncate_caption(&long); - assert!(out.chars().count() <= MAX_CAPTION_CHARS); - assert!(!out.contains("&"), "half entity left: {out:?}"); - assert!(!out.ends_with('&')); + // The exact output is what pins the guard: a cut that keeps `&am` (no + // `;`) leaves a half-open entity that `!contains("&")` cannot see, + // so the old assertions stayed green with the guard deleted. Both + // directions matter — an entity the cut falls inside is dropped whole, + // one the cut falls after is kept whole. + for (long, expected) in [ + ( + "a".repeat(MAX_CAPTION_CHARS - 4) + "&bbbb", + "a".repeat(MAX_CAPTION_CHARS - 4) + "…", + ), + ( + "a".repeat(MAX_CAPTION_CHARS - 6) + "&bbbb", + "a".repeat(MAX_CAPTION_CHARS - 6) + "&…", + ), + ] { + let out = truncate_caption(&long); + assert_eq!(out, expected); + assert!(out.chars().count() <= MAX_CAPTION_CHARS, "{out:?}"); + } } #[test] @@ -796,15 +808,15 @@ mod tests { } #[tokio::test] - async fn unsupported_url_returns_none() { - let result = fetch("https://example.com/some/article").await; - assert!(matches!(result, Ok(None)), "got {result:?}"); - } - - #[tokio::test] - async fn unknown_scheme_returns_none() { - let result = fetch("not a url at all").await; - assert!(matches!(result, Ok(None)), "got {result:?}"); + async fn unsupported_urls_return_none() { + // Neither a URL no site pattern matches nor a string that is no URL at + // all is an error: both answer `Ok(None)`, which is what keeps the bot + // silent on links it cannot handle (only a registered-but-disabled site + // gets a reply). + for url in ["https://example.com/some/article", "not a url at all"] { + let result = fetch(url).await; + assert!(matches!(result, Ok(None)), "{url}: got {result:?}"); + } } #[test] diff --git a/crates/x-media/src/site/pixiv/api.rs b/crates/x-media/src/site/pixiv/api.rs index db5bd69..28f3ae5 100644 --- a/crates/x-media/src/site/pixiv/api.rs +++ b/crates/x-media/src/site/pixiv/api.rs @@ -398,35 +398,20 @@ mod tests { use super::*; use dotenv::dotenv; - /// Skips when `PIXIV_REFRESH_TOKEN` is absent or empty (CI without the - /// secret must stay green; GitHub Actions exposes an unset secret as an - /// empty string, so `is_err()` alone is not enough). - fn require_pixiv_token() -> bool { - std::env::var("PIXIV_REFRESH_TOKEN") - .ok() - .filter(|s| !s.is_empty()) - .is_some() - } - - #[tokio::test] - async fn test_fetch() { - dotenv().ok(); - if !require_pixiv_token() { - eprintln!("skipping: no PIXIV_REFRESH_TOKEN"); - return; - } - let result = fetch(126839080).await; - assert!(result.is_ok()); - println!("{:#?}", result); - } - #[tokio::test] #[ignore = "live network: requires outbound HTTPS to oauth.secure.pixiv.net"] async fn live_validate_with_bogus_token_fails() { dotenv().ok(); - // A bogus token must surface as Api error (invalid_grant), not panic. + // A rejected credential must surface as a permanent status, not a panic + // and not a retryable class: the exchange answers 4xx and the status is + // checked before the body is read (api.rs, `get_access_token`). This + // used to assert `Api`, which that check made unreachable — `Api` is + // only reached from a 2xx body without an `access_token`. let client = PixivAPI::new("bogus_token_for_testing".to_string()); let result = client.get_access_token().await; - assert!(matches!(result, Err(PixivError::Api(_))), "got {result:?}"); + assert!( + matches!(result, Err(PixivError::Status(code)) if (400..500).contains(&code)), + "got {result:?}" + ); } } diff --git a/crates/x-media/src/site/pixiv/interface.rs b/crates/x-media/src/site/pixiv/interface.rs index 1d926ab..9896ddc 100644 --- a/crates/x-media/src/site/pixiv/interface.rs +++ b/crates/x-media/src/site/pixiv/interface.rs @@ -422,16 +422,6 @@ mod tests { } } - #[test] - fn startup_validation_only_disables_on_a_definitive_failure() { - // A bad moment: the site must stay enabled for later links. - assert!(pixiv_error_is_retryable(&PixivError::Status(503))); - assert!(pixiv_error_is_retryable(&PixivError::Status(429))); - // A rejected credential is what `disable()` is for. - assert!(!pixiv_error_is_retryable(&PixivError::Status(403))); - assert!(!pixiv_error_is_retryable(&PixivError::NoAuth)); - } - #[test] fn is_retryable_classifies_transient_and_permanent() { // Transient: network errors, explicit transient, pixiv 429/5xx. diff --git a/crates/x-media/src/site/twitter/interface.rs b/crates/x-media/src/site/twitter/interface.rs index 67e19b1..87e5852 100644 --- a/crates/x-media/src/site/twitter/interface.rs +++ b/crates/x-media/src/site/twitter/interface.rs @@ -524,14 +524,6 @@ mod tests { ); } - #[test] - fn syndication_text_only_has_no_media() { - let raw = fixture(serde_json::json!([])); - let tweet = Tweet::from_syndication_json(&raw.to_string()).unwrap(); - let fetched: Fetched = tweet.into(); - assert!(fetched.media.is_empty()); - } - #[test] fn syndication_gif_maps_to_animated() { let raw = fixture(serde_json::json!([ @@ -550,54 +542,30 @@ mod tests { #[test] fn syndication_text_strips_trailing_media_short_link() { // Real syndication shape: the appended media short link sits after the - // visible text; the unmapped t.co link is stripped by content. - let raw = serde_json::json!({ - "__typename": "Tweet", - "id_str": "1", - "text": "hello world https://t.co/abc123", - "user": { "name": "N", "screen_name": "h" }, - "mediaDetails": [] - }); - let tweet = Tweet::from_syndication_json(&raw.to_string()).unwrap(); - assert_eq!(tweet.text, "hello world"); - assert!(!tweet.caption().contains("t.co")); - } - - #[test] - fn syndication_text_strips_trailing_link_regardless_of_index_units() { - // Real tweet 2084567054481571919: the visible text is 30 code points - // but 41 UTF-16 units, and the two endpoints historically reported - // display_text_range in different units (UTF-16 on syndication, code - // points on GraphQL). The FxEmbed-style content-based strip ignores - // the range entirely, so the appended media link is removed for any - // response shape. - let text = "妄想𝑨𝒅𝒅𝒊𝒄𝒕𝒊𝒐𝒏…🩷💚❤️\n#ゼンゼロ #zzzero https://t.co/XnIi83EkEB"; - let visible = "妄想𝑨𝒅𝒅𝒊𝒄𝒕𝒊𝒐𝒏…🩷💚❤️\n#ゼンゼロ #zzzero"; - let raw = serde_json::json!({ - "__typename": "Tweet", - "id_str": "2084567054481571919", - "text": text, - "user": { "name": "N", "screen_name": "h" }, - "mediaDetails": [] - }); - let tweet = Tweet::from_syndication_json(&raw.to_string()).unwrap(); - assert_eq!(tweet.text, visible, "left a partial link"); - assert!(!tweet.caption().contains("t.co")); - } - - #[test] - fn syndication_text_strips_trailing_short_link_without_entities() { - // No URL entities at all: the leftover t.co link is stripped by the - // content regex. - let raw = serde_json::json!({ - "__typename": "Tweet", - "id_str": "1", - "text": "hello https://t.co/abc123", - "user": { "name": "N", "screen_name": "h" }, - "mediaDetails": [] - }); - let tweet = Tweet::from_syndication_json(&raw.to_string()).unwrap(); - assert_eq!(tweet.text, "hello"); + // visible text and there are no URL entities, so the unmapped t.co link + // is stripped by content alone. The second row is real tweet + // 2084567054481571919 (30 code points but 41 UTF-16 units, and the two + // endpoints historically reported `display_text_range` in different + // units): a content-based strip cannot leave a partial link behind for + // either unit system. + for (text, visible) in [ + ("hello world https://t.co/abc123", "hello world"), + ( + "妄想𝑨𝒅𝒅𝒊𝒄𝒕𝒊𝒐𝒏…🩷💚❤️\n#ゼンゼロ #zzzero https://t.co/XnIi83EkEB", + "妄想𝑨𝒅𝒅𝒊𝒄𝒕𝒊𝒐𝒏…🩷💚❤️\n#ゼンゼロ #zzzero", + ), + ] { + let raw = serde_json::json!({ + "__typename": "Tweet", + "id_str": "1", + "text": text, + "user": { "name": "N", "screen_name": "h" }, + "mediaDetails": [] + }); + let tweet = Tweet::from_syndication_json(&raw.to_string()).unwrap(); + assert_eq!(tweet.text, visible, "left a partial link in {text:?}"); + assert!(!tweet.caption().contains("t.co"), "{text:?}"); + } } #[test] @@ -666,23 +634,6 @@ mod tests { assert!(!tweet.caption().contains("t.co")); } - #[test] - fn syndication_text_keeps_multibyte_text() { - // Text-only tweet: no short links, the multibyte text is untouched. - let text = "コミティア落ちたので、明日は行きません。🙏ごめんなさい"; - let units: Vec = text.encode_utf16().collect(); - assert_eq!(units.len(), 28); - let raw = serde_json::json!({ - "__typename": "Tweet", - "id_str": "1", - "text": text, - "user": { "name": "N", "screen_name": "h" }, - "mediaDetails": [] - }); - let tweet = Tweet::from_syndication_json(&raw.to_string()).unwrap(); - assert_eq!(tweet.text, text, "full text kept intact"); - } - #[test] fn original_twimg_url_rewrites_photo_urls() { assert_eq!( @@ -706,9 +657,14 @@ mod tests { #[test] fn syndication_token_matches_js_formula() { - // JS: ((861627479294746624 / 1e15) * PI).toString(36) == "236.vrsocvda" - let token = syndication_token(861627479294746624); - assert!(token.starts_with("236.v"), "got {token}"); + // JS: ((861627479294746624 / 1e15) * PI).toString(36) == "236.vrsocvda". + // This loop truncates ten base-36 fraction digits instead of rendering + // the shortest round-tripping one, so it agrees with JS on the stem and + // diverges in the tail (`…d9ui` vs `…da`). Pinned exactly, because the + // token is a fixed function of the id: a stub or a wrong constant must + // not pass. The endpoint currently serves public tweets regardless of + // the token, which is why the tail is left as is. + assert_eq!(syndication_token(861627479294746624), "236.vrsocvd9ui"); } #[test] diff --git a/crates/xmedia-bot/src/handlers/commands.rs b/crates/xmedia-bot/src/handlers/commands.rs index 99d9d3c..acb9dee 100644 --- a/crates/xmedia-bot/src/handlers/commands.rs +++ b/crates/xmedia-bot/src/handlers/commands.rs @@ -846,11 +846,8 @@ mod tests { ); assert!(report.contains("site: twitter"), "{report}"); assert!(report.contains("key: twitter:1"), "{report}"); - assert!(report.contains("title: My title"), "{report}"); assert!(report.contains("content: My content"), "{report}"); - assert!(report.contains("author: Author"), "{report}"); assert!(report.contains("author_url: https://x.com/u"), "{report}"); - assert!(report.contains("tags: tag1 tag2"), "{report}"); assert!(report.contains("sensitive: false"), "{report}"); assert!(report.contains("media (2):"), "{report}"); assert!( @@ -864,11 +861,12 @@ mod tests { } #[test] - fn debug_report_without_render_data_and_no_media() { + fn debug_report_without_render_data_has_no_author_line() { let report = debug_report("u", "pixiv", "s", "t", "c", None, true, "p", &[]); + // The `None` branch above is the point: with no render fields there is + // no author line to print. The `sensitive`/`media` lines are the same + // format sites the escaping test already pins with values. assert!(!report.contains("author:"), "{report}"); - assert!(report.contains("sensitive: true"), "{report}"); - assert!(report.contains("media (0):"), "{report}"); } #[test] @@ -1016,20 +1014,6 @@ mod tests { command.command ); } - - // A command with a `String` argument must parse with its whole - // argument: without `parse_with`, teloxide's default parser rejects - // `/remove_template x` and the command silently falls through to the - // URL flow. - assert!(matches!( - Command::parse("/settings", ""), - Ok(Command::Settings) - )); - match Command::parse("/remove_template tpl", "") { - Ok(Command::RemoveTemplate(name)) => assert_eq!(name, "tpl"), - Ok(_) => panic!("/remove_template parsed as another command"), - Err(e) => panic!("parse error: {e}"), - } } #[test] diff --git a/crates/xmedia-bot/src/handlers/urls.rs b/crates/xmedia-bot/src/handlers/urls.rs index 73ec930..9658670 100644 --- a/crates/xmedia-bot/src/handlers/urls.rs +++ b/crates/xmedia-bot/src/handlers/urls.rs @@ -682,29 +682,6 @@ mod tests { ); } - #[tokio::test] - async fn cache_hit_success_keeps_the_cache_entry() { - let stores = TestStores::new(); - let sender = MockSender::scripted(vec![Outcome::GroupOk], permanent_error); - let ctx = stores.ctx(&sender); - stores - .link_cache() - .put("twitter:1", &cached_photo_entry()) - .await; - - url_media(&ctx, 1, 2, "https://x.com/u/status/1", PostSend::FromChat).await; - - assert_eq!(sender.calls(), vec!["send_chat_action", "send_media_group"]); - // Success must not evict the entry. - assert!( - stores - .link_cache() - .get("twitter:1", Duration::from_secs(3600)) - .await - .is_some() - ); - } - /// The caption-quote threshold matches the post's text inside the caption, /// so a long-text cache hit is quoted and a short-text one is not. #[tokio::test] @@ -803,7 +780,8 @@ mod tests { url_media(&ctx, 1, 2, "https://x.com/u/status/1", PostSend::Suppressed).await; assert_eq!(sender.calls(), vec!["send_chat_action", "send_media_group"]); - // The send is otherwise ordinary: the post stays cached. + // The send is otherwise ordinary: the post stays cached (this is also + // the retention control for the eviction case above). assert!( stores .link_cache() @@ -881,13 +859,6 @@ mod tests { assert!(sensitive.contains("TWITTER_AUTH_TOKEN"), "{sensitive}"); let blocked = fetch_error_message(&FetchError::Blocked); assert!(blocked.contains("refused"), "{blocked}"); - - // Each class that has something to say must differ from the generic - // fallback — one generic sentence for everything is what this fixes. - let generic = fetch_error_message(&FetchError::TooLarge); - for text in [disabled, sensitive, blocked] { - assert_ne!(text, generic); - } } #[test] diff --git a/crates/xmedia-bot/src/photo.rs b/crates/xmedia-bot/src/photo.rs index b228613..2d69f20 100644 --- a/crates/xmedia-bot/src/photo.rs +++ b/crates/xmedia-bot/src/photo.rs @@ -460,7 +460,10 @@ mod tests { #[test] fn pipeline_resizes_oversized_jpeg() { - // Build a small over-dimension JPEG with jpeg-encoder. + // Build a small over-dimension JPEG with jpeg-encoder: 9999x2 sums to + // one over the cap. The output's own headers are what must show the + // resize — a copy-through is a perfectly valid JPEG, so magic bytes + // and a non-empty buffer used to pass for nothing. let (w, h) = (9999u16, 2u16); let rgb = vec![90u8; (w as usize) * (h as usize) * 3]; let mut bytes = Vec::new(); @@ -476,8 +479,15 @@ mod tests { PhotoPrep::Upload(file) => { let out = std::fs::read(file.path()).unwrap(); assert!(out.starts_with(&[0xFF, 0xD8]), "output must stay jpeg"); - // 9999x2 downscaled: the buffer length tells the new dims. - assert!(out.len() > 100); + let mut decoder = zune_jpeg::JpegDecoder::new(std::io::Cursor::new(out.as_slice())); + decoder.decode_headers().unwrap(); + let info = decoder.info().unwrap(); + let (nw, nh) = (info.width as u32, info.height as u32); + assert!( + nw + nh <= PHOTO_MAX_DIMENSION_SUM, + "still over the cap: {nw}x{nh}" + ); + assert_ne!((nw, nh), (w as u32, h as u32), "output was not resized"); } PhotoPrep::UseFallback => panic!("over-dimension JPEG should have been resized"), } diff --git a/crates/xmedia-bot/src/queue.rs b/crates/xmedia-bot/src/queue.rs index 9c8a41d..a213e79 100644 --- a/crates/xmedia-bot/src/queue.rs +++ b/crates/xmedia-bot/src/queue.rs @@ -92,13 +92,30 @@ struct QueueWorker { } /// Resets rows left `in_progress` with an expired lock TTL back to `pending` -/// so they can be leased again (crash/panic recovery). -fn recover_update(conn: &rusqlite::Connection) -> rusqlite::Result<()> { +/// so they can be leased again (crash/panic recovery). Returns how many rows +/// came back, which is what decides whether a worker needs waking. +fn recover_update(conn: &rusqlite::Connection) -> rusqlite::Result { conn.execute( "UPDATE tasks SET status='pending', locked_until=0 WHERE status='in_progress' AND locked_until < ?1", params![now_f64()], - )?; - Ok(()) + ) +} + +/// Runs [`recover_update`] and wakes a worker when something actually came +/// back. A recovered row is due immediately, but every worker may be parked on +/// `notify` — with no pending row there is no `earliest_run_after` to sleep on +/// — so without this the recovered task waits for the next unrelated enqueue. +/// Same permit semantics as `enqueue`: `notify_one` stores a permit when no +/// worker is registered. +async fn recover_expired(pool: &std::sync::Arc, notify: &Notify) { + match pool.with_conn(move |conn| recover_update(conn)).await { + Ok(recovered) if recovered > 0 => { + log::warn!("queue: recovered {recovered} row(s) from an expired lease"); + notify.notify_one(); + } + Ok(_) => {} + Err(e) => log::error!("queue recovery failed: {e}"), + } } /// Base delay × 2^attempts (attempts = retries already done), capped at 300s. @@ -136,7 +153,7 @@ impl PersistentTaskQueue { let handler: Arc = Arc::new(move |payload| Box::pin(handler(payload))); let dead_letter: Arc = Arc::new(move |payload, message| Box::pin(dead_letter(payload, message))); - self.recover_stale().await; + recover_expired(&self.pool, &self.notify).await; let mut handles = Vec::with_capacity(QUEUE_WORKERS + 1); for _ in 0..QUEUE_WORKERS { let worker = QueueWorker { @@ -156,6 +173,7 @@ impl PersistentTaskQueue { let sweep_pool = std::sync::Arc::clone(&self.pool); let sweep_notify = Arc::clone(&self.sweep_notify); let sweep_stop = Arc::clone(&self.stop); + let sweep_workers = Arc::clone(&self.notify); handles.push(tokio::spawn(async move { let mut interval = tokio::time::interval(Duration::from_secs(30)); loop { @@ -168,10 +186,7 @@ impl PersistentTaskQueue { if sweep_stop.load(Ordering::Relaxed) { break; } - let result = sweep_pool.with_conn(move |conn| recover_update(conn)).await; - if let Err(e) = result { - log::error!("queue sweep failed: {e}"); - } + recover_expired(&sweep_pool, &sweep_workers).await; } })); *self.worker.lock() = handles; @@ -247,17 +262,6 @@ impl PersistentTaskQueue { } } } - - async fn recover_stale(&self) { - self.recover_sweep().await; - } - - async fn recover_sweep(&self) { - let result = self.pool.with_conn(move |conn| recover_update(conn)).await; - if let Err(e) = result { - log::error!("queue recovery failed: {e}"); - } - } } /// Last-resort terminal state for a row whose `DELETE` would not go through: @@ -960,7 +964,12 @@ mod tests { queue.stop().await; } - #[tokio::test] + /// A row that goes stale *after* startup is picked up by the periodic + /// sweep — the spawned task, its 30 s interval included — and the worker + /// that sweeps it gets woken. The paused clock is what makes this the real + /// test: this used to call the recovery by hand, which proved the SQL but + /// left the wiring free to be deleted. + #[tokio::test(start_paused = true)] async fn runtime_sweep_recovers_expired_lease() { let (queue, _dir) = new_queue().await; let calls = Arc::new(AtomicUsize::new(0)); @@ -975,8 +984,12 @@ mod tests { |_payload, _message| async {}, ) .await; - // Insert a stale leased row AFTER startup: without a runtime sweep it - // would stay `in_progress` forever (only start() used to recover). + // Let every worker park and the sweep consume its immediate first tick, + // so only a later tick can see the row. The clock is paused: yields do + // not advance it, and the sleep below does. + for _ in 0..16 { + tokio::task::yield_now().await; + } { let conn = rusqlite::Connection::open(queue.pool.path()).unwrap(); conn.execute( @@ -986,12 +999,18 @@ mod tests { ) .unwrap(); } - queue.recover_sweep().await; - tokio::time::sleep(Duration::from_millis(300)).await; + assert_eq!( + calls.load(AtomicOrdering::SeqCst), + 0, + "the row is stale but no sweep has run since it appeared" + ); + + tokio::time::sleep(Duration::from_secs(31)).await; + assert_eq!( calls.load(AtomicOrdering::SeqCst), 1, - "expired lease must be recovered and processed exactly once" + "the periodic sweep must recover the row and wake a worker" ); queue.stop().await; } diff --git a/crates/xmedia-bot/src/send/mod.rs b/crates/xmedia-bot/src/send/mod.rs index de85bfd..bb14905 100644 --- a/crates/xmedia-bot/src/send/mod.rs +++ b/crates/xmedia-bot/src/send/mod.rs @@ -760,11 +760,10 @@ mod tests { #[test] fn oversized_photo_boundary() { - // The empirical Telegram limit: sum 10000 passes, 10001 fails. + // The empirical Telegram limit: sum 10000 passes, 10001 fails. Pinned + // cross-crate because `photo.rs` and `upload.rs` both branch on it. // Const-block asserts so clippy's assertions_on_constants stays quiet. const { assert!(crate::photo::PHOTO_MAX_DIMENSION_SUM == 10000) }; - const { assert!(6100 + 3900 <= crate::photo::PHOTO_MAX_DIMENSION_SUM) }; - const { assert!(6300 + 3730 > crate::photo::PHOTO_MAX_DIMENSION_SUM) }; } #[test] @@ -1364,32 +1363,6 @@ mod tests { assert_eq!(sender.calls(), vec!["send_animation", "send_animation"]); } - #[tokio::test] - async fn media_group_success_and_forward_ok() { - // GroupOk: the group send succeeds (empty message list → no file ids - // collected, the batch counts as sent). CopyOk: the forward succeeds. - let dir = tempfile::tempdir().unwrap(); - let file = dir.path().join("media.jpg"); - std::fs::write(&file, b"not-a-real-jpeg").unwrap(); - let sender = MockSender::scripted(vec![Outcome::GroupOk], media_fetch_error); - let stores = TestStores::new(); - let ctx = stores.ctx(&sender); - let task = sequence_task(file.to_str().unwrap()); - let result = send_media_sequence(&ctx, &task).await; - assert!(result.is_ok(), "got {result:?}"); - - let sender = MockSender::scripted(vec![Outcome::CopyOk], media_fetch_error); - let ctx = stores.ctx(&sender); - let task = Task::ForwardMessages { - from_chat_id: 1, - to_chat_id: 2, - message_ids: vec![3], - notify_chat_id: None, - notify_message_id: None, - }; - assert!(forward_messages(&ctx, &task).await.is_ok()); - } - #[tokio::test] async fn forward_classifies_retry_after_and_permanent() { use teloxide::types::Seconds;