mirror of
https://github.com/TheFunny/TelegramTwitterMediaBot.git
synced 2026-09-25 23:52:04 +00:00
fix: bound the inline state map, test the 300s sweep, add a bot-wide send budget
Three gaps the last audit list named, all in the "resource growth, background timers and limits nobody watches" class. **Idle inline-query entries are pruned.** `DebounceStates` had no eviction at all: one entry per user who ever used inline mode, forever, while the rate limiter's buckets and the chat store both prune in the 300s sweep. Entries now carry a `last_seen` stamp and `prune_idle_states()` drops the ones idle past 300s — the window Telegram caches an inline answer for (`cache_time(300)`), after which a repeat reaches the bot again and has to be answered fresh, so the entry would only suppress a fetch the user is waiting for. The boundary is tested through `prune_idle_at(now, idle_for)` so it does not depend on ageing a monotonic clock. **The 300s sweep is a function, and tested.** It was an inline `tokio::spawn` block: the expiry edit (the only part that talks to Telegram) had no test at all. It is now `periodic_sweep(sender, chat_store, link_cache, task_queue, config, stop)`, which also prunes the inline entries, driven in a test with `start_paused` — the loop's own timer fires the tick, exactly one expired prompt is rewritten in place, a live one keeps its record and buttons. The interval is pinned as a constant because no assertion on the edits can see it (a shorter one produces the same single edit; the paused clock can jump past the boundary while a tick's DB work is in flight). To make the edit reachable at all, `edit_message_text` joined the `MediaSender` trait (Bot impl + mock recording), which is also what keeps `main.rs`'s remaining `Bot` calls unambiguous. `main.rs` leaves the "untested modules" list except for startup/shutdown and the dispatcher tree. **The bot-wide send budget exists.** Telegram throttles a bot in total (~30 msg/s) as well as per chat; only the per-chat bucket existed, so a batch forward fanned out over many chats was unguarded and earned 429s the queue then retried. `acquire_global` charges the same spend against a single shared bucket at the three paced sites (`send_media_group`, `send_animation`, `copy_messages`). The unpaced ones (`send_message`, the edits, the toasts) stay unpaced on purpose: they are one call per action, far below the ceiling, and pacing a user-visible reply would delay it. Not covered: that the send paths call it (they need a real `Bot`), which is the same structural gap as the dispatcher tree. Also: the startup token-exchange decision is now `startup_validation(result)` instead of living inside the `Site::validate` future, so "a 5xx while the container comes up must not disable pixiv" is asserted as a decision — the message the admin gets plus `enabled()` unchanged. The rejected-credential half is deliberately not exercised: it calls `disable()`, a process-wide flag with no reset, and a test touching it would order-couple every other pixiv test. Verified: `cargo fmt`, `cargo clippy --workspace --all-targets --locked -- -D warnings`, `cargo test --workspace --locked` (184 passed, 14 ignored) — plus mutations, each confirmed to fail the relevant test: the sweep not being driven on its timer, the interval shortened to 60s, and (earlier) the queue sweep's missing wake-up. Dropped an empty leftover `crates/x-media/tests/` directory while there (never tracked by git).
This commit is contained in:
@@ -20,6 +20,12 @@ use x_media::media::Media;
|
||||
/// post id. Only answer once the query has been stable for this long.
|
||||
const INLINE_DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(800);
|
||||
|
||||
/// How long a debounce entry is worth keeping: the window Telegram caches an
|
||||
/// inline answer for (`answer_inline_query` asks for `cache_time(300)`). Past
|
||||
/// it a repeat is sent to the bot again and has to be answered fresh, so the
|
||||
/// entry would only suppress a fetch the user is waiting for.
|
||||
const INLINE_STATE_TTL: std::time::Duration = std::time::Duration::from_secs(300);
|
||||
|
||||
/// Last seen inline query per user and whether it was already answered.
|
||||
/// Guards the debounce timer: a repeat of an answered query is served by
|
||||
/// Telegram's inline cache (see `cache_time`), not by another fetch. Keyed by
|
||||
@@ -28,6 +34,10 @@ const INLINE_DEBOUNCE: std::time::Duration = std::time::Duration::from_millis(80
|
||||
struct InlineDebounceState {
|
||||
query: String,
|
||||
answered: bool,
|
||||
/// When a query last touched this entry, so the periodic sweep can drop
|
||||
/// one per user who ever used inline mode (the map had no eviction at all,
|
||||
/// unlike the rate limiter's buckets and the chat store).
|
||||
last_seen: std::time::Instant,
|
||||
}
|
||||
|
||||
#[derive(Default)]
|
||||
@@ -49,11 +59,21 @@ impl DebounceStates {
|
||||
InlineDebounceState {
|
||||
query: query.to_string(),
|
||||
answered: false,
|
||||
last_seen: std::time::Instant::now(),
|
||||
},
|
||||
);
|
||||
true
|
||||
}
|
||||
|
||||
/// Drops entries no query has touched for `idle_for`. Split from the clock
|
||||
/// so the boundary is testable without ageing a monotonic instant.
|
||||
fn prune_idle_at(&mut self, now: std::time::Instant, idle_for: std::time::Duration) -> usize {
|
||||
let before = self.0.len();
|
||||
self.0
|
||||
.retain(|_, state| now.saturating_duration_since(state.last_seen) < idle_for);
|
||||
before - self.0.len()
|
||||
}
|
||||
|
||||
/// Claims the answer for the user's newest query; false when a newer query
|
||||
/// superseded it or the answer was already claimed.
|
||||
fn claim(&mut self, user_id: u64, query: &str) -> bool {
|
||||
@@ -64,6 +84,7 @@ impl DebounceStates {
|
||||
return false;
|
||||
}
|
||||
state.answered = true;
|
||||
state.last_seen = std::time::Instant::now();
|
||||
true
|
||||
}
|
||||
|
||||
@@ -73,10 +94,19 @@ impl DebounceStates {
|
||||
&& state.query == query
|
||||
{
|
||||
state.answered = false;
|
||||
state.last_seen = std::time::Instant::now();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Drops debounce entries idle for [`INLINE_STATE_TTL`]; the 300 s sweep calls
|
||||
/// this next to the rate limiter's prune. Returns how many were dropped.
|
||||
pub(crate) fn prune_idle_states() -> usize {
|
||||
INLINE_DEBOUNCE_STATE
|
||||
.lock()
|
||||
.prune_idle_at(std::time::Instant::now(), INLINE_STATE_TTL)
|
||||
}
|
||||
|
||||
static INLINE_DEBOUNCE_STATE: LazyLock<parking_lot::Mutex<DebounceStates>> =
|
||||
LazyLock::new(|| parking_lot::Mutex::new(DebounceStates::default()));
|
||||
|
||||
@@ -212,7 +242,7 @@ async fn answer_inline_query(bot: Bot, query: InlineQuery) -> Result<bool, Reque
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::DebounceStates;
|
||||
use super::{DebounceStates, INLINE_STATE_TTL};
|
||||
|
||||
const URL_A: &str = "https://x.com/a/status/1";
|
||||
const URL_B: &str = "https://x.com/b/status/2";
|
||||
@@ -241,6 +271,29 @@ mod tests {
|
||||
assert!(states.claim(2, URL_A));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn idle_states_are_pruned_and_live_ones_kept() {
|
||||
let mut states = DebounceStates::default();
|
||||
assert!(states.note(1, URL_A));
|
||||
let first = states.0[&1].last_seen;
|
||||
// Entry 2 is strictly newer, so one timestamp can sit exactly on the
|
||||
// window's edge for one and comfortably inside it for the other.
|
||||
std::thread::sleep(std::time::Duration::from_millis(2));
|
||||
assert!(states.note(2, URL_B));
|
||||
|
||||
assert_eq!(
|
||||
states.prune_idle_at(first + INLINE_STATE_TTL, INLINE_STATE_TTL),
|
||||
1
|
||||
);
|
||||
assert!(
|
||||
!states.0.contains_key(&1),
|
||||
"the entry past the window must go"
|
||||
);
|
||||
assert!(states.0.contains_key(&2), "the live entry must stay");
|
||||
// A pruned user's repeat is answered fresh instead of suppressed.
|
||||
assert!(states.note(1, URL_A));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn newer_query_supersedes_and_failed_answer_is_released() {
|
||||
let mut states = DebounceStates::default();
|
||||
|
||||
@@ -15,6 +15,7 @@ mod urls;
|
||||
pub use callback::callback_query_handler;
|
||||
pub use commands::register_commands;
|
||||
pub use inline::inline_query_handler;
|
||||
pub(crate) use inline::prune_idle_states;
|
||||
/// The resolved `$DATA_DIR/task_queue.db` path, for the startup config line.
|
||||
pub(crate) use statics::db_path;
|
||||
pub use statics::{CHAT_STORE, CONFIG, LINK_CACHE, TASK_QUEUE};
|
||||
|
||||
Reference in New Issue
Block a user