feat(api,web): remove a movie, files optional
`DELETE /api/movies/{id}` took the row and left the files, and no
surface exposed it. It now accepts `delete_files`, which unlinks the
title's §7.4 folder — atomic, so sidecars go with the feature. Targets
come from `media_files`, never from re-deriving the folder name, and a
path outside its root is never touched. The torrent is untouched (§7.3):
it keeps seeding and the reaper owns it.
The release deck grows a quiet REMOVE control opening one inline
confirmation: file count, size and folder first, then a delete-files
toggle that starts off, then what the choice costs.
Closes #104
This commit is contained in:
@@ -23,7 +23,8 @@ use utoipa_scalar::{Scalar, Servable};
|
||||
|
||||
pub use health::{Check, Health, HealthReport, Status};
|
||||
pub use movies::{
|
||||
Accepted, AttentionQueues, CreateMovie, ErrorBody, Movie, MovieFile, Release, UpdateMovie,
|
||||
Accepted, AttentionQueues, CreateMovie, DeleteMovieQuery, ErrorBody, Movie, MovieFile, Release,
|
||||
UpdateMovie,
|
||||
};
|
||||
pub use owners::{CreateOwner, Owner, UpdateOwner};
|
||||
pub use roots::Root;
|
||||
|
||||
@@ -113,6 +113,10 @@ pub enum ApiError {
|
||||
Invalid(String),
|
||||
Unavailable,
|
||||
Database(String),
|
||||
/// A library delete that could not touch the disk. Named separately from
|
||||
/// [`Self::Database`] because the row is still there and a retry is the
|
||||
/// right next move.
|
||||
Filesystem(String),
|
||||
}
|
||||
|
||||
impl IntoResponse for ApiError {
|
||||
@@ -133,6 +137,13 @@ impl IntoResponse for ApiError {
|
||||
tracing::error!(%error, "API database error");
|
||||
(StatusCode::INTERNAL_SERVER_ERROR, "database error".into())
|
||||
}
|
||||
Self::Filesystem(error) => {
|
||||
tracing::error!(%error, "API filesystem error");
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("files not removed: {error}"),
|
||||
)
|
||||
}
|
||||
};
|
||||
(status, Json(ErrorBody { error })).into_response()
|
||||
}
|
||||
@@ -332,9 +343,18 @@ pub async fn update(
|
||||
Ok(Json(load_movie(&state, id).await?))
|
||||
}
|
||||
|
||||
#[derive(Debug, Deserialize, IntoParams)]
|
||||
pub struct DeleteMovieQuery {
|
||||
/// Also remove the title's folder under its root (§7.4). Off unless the
|
||||
/// caller says otherwise: dropping the row is reversible, unlinking
|
||||
/// 40 GB is not.
|
||||
#[serde(default)]
|
||||
pub delete_files: bool,
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
delete, path = "/api/movies/{movie_id}", tag = "movies",
|
||||
params(("movie_id" = i64, Path, description = "Movie row id")),
|
||||
params(("movie_id" = i64, Path, description = "Movie row id"), DeleteMovieQuery),
|
||||
responses(
|
||||
(status = 204),
|
||||
(status = 404, body = ErrorBody),
|
||||
@@ -346,7 +366,29 @@ pub async fn update(
|
||||
pub async fn delete(
|
||||
State(state): State<AppState>,
|
||||
Path(id): Path<i64>,
|
||||
Query(query): Query<DeleteMovieQuery>,
|
||||
) -> Result<StatusCode, ApiError> {
|
||||
// The row is loaded first so a missing movie is 404 before anything
|
||||
// touches the disk.
|
||||
load_movie(&state, id).await?;
|
||||
if query.delete_files {
|
||||
remove_library_files(&state, id).await?;
|
||||
}
|
||||
// `media_files.path` is UNIQUE and the owner is polymorphic, so nothing
|
||||
// cascades: leaving the rows behind would block re-importing the same
|
||||
// path after a re-add. Owner tags go with the title they tagged.
|
||||
sqlx::query!(
|
||||
"DELETE FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?",
|
||||
id
|
||||
)
|
||||
.execute(pool(&state)?)
|
||||
.await?;
|
||||
sqlx::query!(
|
||||
"DELETE FROM title_owners WHERE title_kind = 'movie' AND title_id = ?",
|
||||
id
|
||||
)
|
||||
.execute(pool(&state)?)
|
||||
.await?;
|
||||
let result = sqlx::query!("DELETE FROM movies WHERE id = ?", id)
|
||||
.execute(pool(&state)?)
|
||||
.await?;
|
||||
@@ -356,6 +398,83 @@ pub async fn delete(
|
||||
Ok(StatusCode::NO_CONTENT)
|
||||
}
|
||||
|
||||
/// Unlink everything this title put under its root.
|
||||
///
|
||||
/// The service knows only what it wrote (§2), so the targets come from
|
||||
/// `media_files`, never from a scan and never from re-deriving the §7.4 name
|
||||
/// — a title renamed after import would derive a folder that does not exist
|
||||
/// while the real one stayed. Deleting the folder rather than the file is
|
||||
/// what makes the delete atomic (§7.4): sidecar subtitles and artwork go
|
||||
/// with it.
|
||||
///
|
||||
/// The torrent is untouched (§7.3). It keeps seeding under its own rule and
|
||||
/// the reaper deletes it; a hardlinked file loses only its library name.
|
||||
///
|
||||
/// Failure leaves the database alone, so the operator sees the title still
|
||||
/// there and can retry rather than losing the record of what is on disk.
|
||||
async fn remove_library_files(state: &AppState, id: i64) -> Result<(), ApiError> {
|
||||
let root = sqlx::query_scalar!(
|
||||
r#"SELECT r.path AS "path!: String" FROM roots r JOIN movies m ON m.root_id = r.id WHERE m.id = ?"#,
|
||||
id
|
||||
)
|
||||
.fetch_one(pool(state)?)
|
||||
.await?;
|
||||
let paths = sqlx::query_scalar!(
|
||||
r#"SELECT path AS "path!: String" FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?"#,
|
||||
id
|
||||
)
|
||||
.fetch_all(pool(state)?)
|
||||
.await?;
|
||||
|
||||
let mut targets: Vec<std::path::PathBuf> = Vec::new();
|
||||
for path in &paths {
|
||||
let Some(target) = title_target(&root, path) else {
|
||||
// Outside its own root: not ours to delete. The row still goes,
|
||||
// so the operator sees the title leave and the file stay.
|
||||
tracing::warn!(%path, %root, "media file is outside its root, not deleted");
|
||||
continue;
|
||||
};
|
||||
if !targets.contains(&target) {
|
||||
targets.push(target);
|
||||
}
|
||||
}
|
||||
|
||||
for target in targets {
|
||||
let metadata = match tokio::fs::symlink_metadata(&target).await {
|
||||
Ok(metadata) => metadata,
|
||||
// Already gone is the state we wanted.
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue,
|
||||
Err(error) => return Err(ApiError::Filesystem(error.to_string())),
|
||||
};
|
||||
let removed = if metadata.is_dir() {
|
||||
tokio::fs::remove_dir_all(&target).await
|
||||
} else {
|
||||
tokio::fs::remove_file(&target).await
|
||||
};
|
||||
match removed {
|
||||
Ok(()) => tracing::info!(target = %target.display(), "removed library files"),
|
||||
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {}
|
||||
Err(error) => return Err(ApiError::Filesystem(error.to_string())),
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// What to unlink for one library file: the title folder directly under the
|
||||
/// root, or the file itself when it sits in the root with no folder of its
|
||||
/// own. `None` when the file is not under the root at all, which is the
|
||||
/// guard that keeps a delete inside the library it belongs to.
|
||||
fn title_target(root: &str, file: &str) -> Option<std::path::PathBuf> {
|
||||
let root = std::path::Path::new(root);
|
||||
let relative = std::path::Path::new(file).strip_prefix(root).ok()?;
|
||||
let first = relative.components().next()?;
|
||||
let std::path::Component::Normal(name) = first else {
|
||||
// `..` or a root component would climb out of the library.
|
||||
return None;
|
||||
};
|
||||
Some(root.join(name))
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
post, path = "/api/movies/{movie_id}/search", tag = "movies",
|
||||
params(("movie_id" = i64, Path, description = "Movie row id")),
|
||||
@@ -681,6 +800,160 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A movie's root is a real directory for the duration of one test, with
|
||||
/// the §7.4 folder already in it: one feature and one sidecar subtitle.
|
||||
async fn library_on_disk(
|
||||
state: &AppState,
|
||||
movie_id: i64,
|
||||
root: &std::path::Path,
|
||||
) -> std::path::PathBuf {
|
||||
let folder = root.join("Dune Part Two (2024) [tmdbid-693134]");
|
||||
tokio::fs::create_dir_all(&folder)
|
||||
.await
|
||||
.expect("create title folder");
|
||||
let feature = folder.join("Dune Part Two (2024) [tmdbid-693134] - [2160p].mkv");
|
||||
tokio::fs::write(&feature, b"feature").await.expect("write");
|
||||
tokio::fs::write(folder.join("dune.pt.srt"), b"subs")
|
||||
.await
|
||||
.expect("write sidecar");
|
||||
|
||||
let pool = state.database().expect("database").pool();
|
||||
let root_path = root.to_str().expect("utf-8 root");
|
||||
sqlx::query("UPDATE roots SET path = ? WHERE id = 1")
|
||||
.bind(root_path)
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("point the root at the tempdir");
|
||||
sqlx::query(
|
||||
"INSERT INTO media_files (owner_kind, owner_id, path, size) VALUES ('movie', ?, ?, 7)",
|
||||
)
|
||||
.bind(movie_id)
|
||||
.bind(feature.to_str().expect("utf-8 path"))
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("media file");
|
||||
folder
|
||||
}
|
||||
|
||||
/// The §7.4 folder is the unit of deletion, so the sidecar goes with the
|
||||
/// feature — and the root itself is never touched.
|
||||
#[tokio::test]
|
||||
async fn delete_files_removes_the_whole_title_folder() {
|
||||
let (_dir, state, base) = application().await;
|
||||
let movie = add_movie(&base, 693_134, 1).await;
|
||||
let id = movie["id"].as_i64().expect("id");
|
||||
let root = tempfile::tempdir().expect("root");
|
||||
let folder = library_on_disk(&state, id, root.path()).await;
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.delete(format!("{base}/api/movies/{id}?delete_files=true"))
|
||||
.send()
|
||||
.await
|
||||
.expect("delete");
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
assert!(
|
||||
!folder.exists(),
|
||||
"the title folder and its sidecars are gone"
|
||||
);
|
||||
assert!(root.path().exists(), "the root survives its titles");
|
||||
|
||||
let orphans: i64 = sqlx::query_scalar(
|
||||
"SELECT count(*) FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?",
|
||||
)
|
||||
.bind(id)
|
||||
.fetch_one(state.database().expect("database").pool())
|
||||
.await
|
||||
.expect("count files");
|
||||
assert_eq!(orphans, 0, "the file rows go with the files");
|
||||
}
|
||||
|
||||
/// Default off (§7.4 in the issue): removing a title from the library is
|
||||
/// not the same decision as unlinking 40 GB.
|
||||
#[tokio::test]
|
||||
async fn delete_without_the_flag_leaves_the_files_on_disk() {
|
||||
let (_dir, state, base) = application().await;
|
||||
let movie = add_movie(&base, 693_134, 1).await;
|
||||
let id = movie["id"].as_i64().expect("id");
|
||||
let root = tempfile::tempdir().expect("root");
|
||||
let folder = library_on_disk(&state, id, root.path()).await;
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.delete(format!("{base}/api/movies/{id}"))
|
||||
.send()
|
||||
.await
|
||||
.expect("delete");
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
assert!(folder.exists(), "the files stay until asked for");
|
||||
assert_eq!(
|
||||
reqwest::get(format!("{base}/api/movies/{id}"))
|
||||
.await
|
||||
.expect("get deleted")
|
||||
.status(),
|
||||
StatusCode::NOT_FOUND
|
||||
);
|
||||
}
|
||||
|
||||
/// The guard that keeps a delete inside the library: a path that is not
|
||||
/// under the title's root is left alone, whatever the row says.
|
||||
#[tokio::test]
|
||||
async fn a_file_outside_its_root_is_never_unlinked() {
|
||||
let (_dir, state, base) = application().await;
|
||||
let movie = add_movie(&base, 693_134, 1).await;
|
||||
let id = movie["id"].as_i64().expect("id");
|
||||
let root = tempfile::tempdir().expect("root");
|
||||
let elsewhere = tempfile::tempdir().expect("elsewhere");
|
||||
let stray = elsewhere.path().join("not-ours.mkv");
|
||||
tokio::fs::write(&stray, b"stray").await.expect("write");
|
||||
|
||||
let pool = state.database().expect("database").pool();
|
||||
sqlx::query("UPDATE roots SET path = ? WHERE id = 1")
|
||||
.bind(root.path().to_str().expect("utf-8 root"))
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("point the root at the tempdir");
|
||||
sqlx::query(
|
||||
"INSERT INTO media_files (owner_kind, owner_id, path, size) VALUES ('movie', ?, ?, 5)",
|
||||
)
|
||||
.bind(id)
|
||||
.bind(stray.to_str().expect("utf-8 path"))
|
||||
.execute(pool)
|
||||
.await
|
||||
.expect("media file");
|
||||
|
||||
let response = reqwest::Client::new()
|
||||
.delete(format!("{base}/api/movies/{id}?delete_files=true"))
|
||||
.send()
|
||||
.await
|
||||
.expect("delete");
|
||||
assert_eq!(response.status(), StatusCode::NO_CONTENT);
|
||||
assert!(
|
||||
stray.exists(),
|
||||
"a path outside the root is not ours to delete"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_title_target_is_the_folder_directly_under_the_root() {
|
||||
let root = "/mnt/media/movies/main";
|
||||
assert_eq!(
|
||||
title_target(
|
||||
root,
|
||||
"/mnt/media/movies/main/Dune (2021) [tmdbid-1]/Dune.mkv"
|
||||
),
|
||||
Some(std::path::PathBuf::from(
|
||||
"/mnt/media/movies/main/Dune (2021) [tmdbid-1]"
|
||||
))
|
||||
);
|
||||
// A file sitting straight in the root is its own target: deleting the
|
||||
// root because a file was misplaced would take the whole library.
|
||||
assert_eq!(
|
||||
title_target(root, "/mnt/media/movies/main/loose.mkv"),
|
||||
Some(std::path::PathBuf::from("/mnt/media/movies/main/loose.mkv"))
|
||||
);
|
||||
assert_eq!(title_target(root, "/mnt/media/movies/kids/other.mkv"), None);
|
||||
assert_eq!(title_target(root, root), None);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn release_actions_are_scoped_to_the_movie() {
|
||||
let (_dir, state, base) = application().await;
|
||||
|
||||
Reference in New Issue
Block a user