fix(arr): clear subtitle attempts on language drop
Removing a language from wanted_languages left its subtitle_attempts rows behind, resurrecting stale backoff on re-add (#224).
This commit is contained in:
+26
@@ -0,0 +1,26 @@
|
||||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "SELECT wanted_languages FROM subtitle_settings WHERE id = 1",
|
||||
"describe": {
|
||||
"columns": [
|
||||
{
|
||||
"name": "wanted_languages",
|
||||
"ordinal": 0,
|
||||
"type_info": "Text",
|
||||
"origin": {
|
||||
"Table": {
|
||||
"table": "subtitle_settings",
|
||||
"name": "wanted_languages"
|
||||
}
|
||||
}
|
||||
}
|
||||
],
|
||||
"parameters": {
|
||||
"Right": 0
|
||||
},
|
||||
"nullable": [
|
||||
false
|
||||
]
|
||||
},
|
||||
"hash": "0f94b0f839cfcbe4a70ee5be81f452c88dc610d05042d4af811963c0cbc6113e"
|
||||
}
|
||||
+12
@@ -0,0 +1,12 @@
|
||||
{
|
||||
"db_name": "SQLite",
|
||||
"query": "DELETE FROM subtitle_attempts WHERE language = ?",
|
||||
"describe": {
|
||||
"columns": [],
|
||||
"parameters": {
|
||||
"Right": 1
|
||||
},
|
||||
"nullable": []
|
||||
},
|
||||
"hash": "74178ac3ec41aa8a5ce164103a8d8301cb555eea2fa5d655a7365fd02c557cce"
|
||||
}
|
||||
@@ -196,8 +196,20 @@ pub async fn update(
|
||||
) -> Result<Json<SubtitleSettings>, ApiError> {
|
||||
let input = parsed(body)?;
|
||||
input.validate().map_err(ApiError::Invalid)?;
|
||||
let new_wanted = input.wanted_languages.clone();
|
||||
let columns = input.into_columns()?;
|
||||
let timeout_seconds = columns.remote_command_timeout_seconds;
|
||||
|
||||
let previous_wanted: String =
|
||||
sqlx::query_scalar!("SELECT wanted_languages FROM subtitle_settings WHERE id = 1")
|
||||
.fetch_one(pool(&state)?)
|
||||
.await?;
|
||||
let dropped: Vec<String> = column::<Vec<String>>("wanted_languages", &previous_wanted)?
|
||||
.into_iter()
|
||||
.filter(|language| !new_wanted.contains(language))
|
||||
.collect();
|
||||
|
||||
let mut transaction = pool(&state)?.begin().await?;
|
||||
sqlx::query!(
|
||||
r#"UPDATE subtitle_settings SET
|
||||
wanted_languages = ?, providers_enabled = ?, translation_engine = ?,
|
||||
@@ -212,8 +224,20 @@ pub async fn update(
|
||||
columns.translator_daily_budgets,
|
||||
columns.remote_command_timeout_seconds,
|
||||
)
|
||||
.execute(pool(&state)?)
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
// #224: a dropped language's attempt bookkeeping (backoff counter,
|
||||
// `last_failure`) must not resurrect if the language is re-added later.
|
||||
// Subtitle files stay — only the attempt rows are wanted-set-scoped.
|
||||
for language in &dropped {
|
||||
sqlx::query!(
|
||||
"DELETE FROM subtitle_attempts WHERE language = ?",
|
||||
language
|
||||
)
|
||||
.execute(&mut *transaction)
|
||||
.await?;
|
||||
}
|
||||
transaction.commit().await?;
|
||||
// Issue #219: the row alone never reaches the running backend. Push it
|
||||
// into the cell the remote-command translator re-reads per batch; the
|
||||
// cell counts milliseconds, the row counts seconds.
|
||||
@@ -270,6 +294,30 @@ mod tests {
|
||||
(dir, format!("http://{address}"), timeout)
|
||||
}
|
||||
|
||||
/// The same app, with the underlying pool exposed so a test can seed or
|
||||
/// inspect rows the API surface does not read back directly.
|
||||
async fn application_with_pool() -> (tempfile::TempDir, String, sqlx::SqlitePool) {
|
||||
let dir = tempfile::tempdir().expect("tempdir");
|
||||
let database = arr_db::Db::connect(dir.path().join("arr.db"))
|
||||
.await
|
||||
.expect("connect database");
|
||||
database.migrate().await.expect("migrate database");
|
||||
let pool = database.pool().clone();
|
||||
let state = AppState::new(Upstreams::new(
|
||||
"http://127.0.0.1:1".into(),
|
||||
"http://127.0.0.1:1".into(),
|
||||
))
|
||||
.expect("state")
|
||||
.with_database(database);
|
||||
let listener = tokio::net::TcpListener::bind("127.0.0.1:0")
|
||||
.await
|
||||
.expect("bind");
|
||||
let address = listener.local_addr().expect("address");
|
||||
let app = router(state);
|
||||
tokio::spawn(async move { axum::serve(listener, app).await.expect("serve") });
|
||||
(dir, format!("http://{address}"), pool)
|
||||
}
|
||||
|
||||
fn valid_input() -> serde_json::Value {
|
||||
serde_json::json!({
|
||||
"wanted_languages": ["pt-PT", "en"],
|
||||
@@ -334,6 +382,53 @@ mod tests {
|
||||
assert_eq!(refetched, updated);
|
||||
}
|
||||
|
||||
/// #224: dropping a language from `wanted_languages` must clear its
|
||||
/// `subtitle_attempts` rows — otherwise re-adding it later resurrects a
|
||||
/// stale backoff counter as though the attempts had just happened.
|
||||
#[tokio::test]
|
||||
async fn dropping_a_language_clears_its_attempt_rows() {
|
||||
let (_dir, base, pool) = application_with_pool().await;
|
||||
sqlx::query(
|
||||
"INSERT INTO media_files (id, owner_kind, owner_id, path, size)
|
||||
VALUES (1, 'movie', 1, 'x.mkv', 1)",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("media file");
|
||||
sqlx::query(
|
||||
"INSERT INTO subtitle_attempts
|
||||
(media_file_id, language, state, attempts, last_attempt_at, last_failure)
|
||||
VALUES (1, 'en', 'failed', 3, '2024-01-01T00:00:00Z', 'no provider match')",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("dropped-language attempt");
|
||||
sqlx::query(
|
||||
"INSERT INTO subtitle_attempts (media_file_id, language, state) VALUES (1, 'pt-PT', 'wanted')",
|
||||
)
|
||||
.execute(&pool)
|
||||
.await
|
||||
.expect("kept-language attempt");
|
||||
|
||||
let mut payload = valid_input();
|
||||
payload["wanted_languages"] = serde_json::json!(["pt-PT"]);
|
||||
reqwest::Client::new()
|
||||
.put(format!("{base}/api/settings/subtitles"))
|
||||
.json(&payload)
|
||||
.send()
|
||||
.await
|
||||
.expect("put settings")
|
||||
.error_for_status()
|
||||
.expect("valid input accepted");
|
||||
|
||||
let remaining: Vec<String> =
|
||||
sqlx::query_scalar("SELECT language FROM subtitle_attempts ORDER BY language")
|
||||
.fetch_all(&pool)
|
||||
.await
|
||||
.expect("attempts");
|
||||
assert_eq!(remaining, vec!["pt-PT".to_string()]);
|
||||
}
|
||||
|
||||
/// Issue #219: the row alone never reaches the running backend, so a PUT
|
||||
/// must push its value into the cell the command translator re-reads.
|
||||
#[tokio::test]
|
||||
|
||||
Reference in New Issue
Block a user