fix(api): create title move destination

crates/arr-api/src/movies.rs

@@ -1704,6 +1704,62 @@ mod tests {
         );
     }
 
+    /// Reported in production: moving a title into a root whose directory has
+    /// never been written to failed with
+    /// `could not move '...The Batman...': No such file or directory`, naming
+    /// a folder that was sitting exactly where the operator left it. A root is
+    /// a database row and nothing makes its directory exist; `rename` reports
+    /// a missing destination parent as the same `NotFound` as a missing
+    /// source. Every earlier test pointed the destination at a `tempdir`,
+    /// which is why the assumption was never exercised.
+    #[tokio::test]
+    async fn a_root_whose_directory_does_not_exist_yet_still_receives_the_title() {
+        let (_dir, state, base) = application().await;
+        let movie = add_movie(&base, 693_134, 1).await;
+        let id = movie["id"].as_i64().expect("id");
+        let source = tempfile::tempdir().expect("source root");
+        let parent = tempfile::tempdir().expect("media tree");
+        let folder = library_on_disk(&state, id, source.path()).await;
+
+        // Configured, but never written to: the row exists, the folder does not.
+        let destination = parent.path().join("movies").join("kids");
+        assert!(!destination.exists());
+        point_root_at(&state, 2, &destination).await;
+
+        let response = reqwest::Client::new()
+            .patch(format!("{base}/api/movies/{id}"))
+            .json(&serde_json::json!({"root_id": 2}))
+            .send()
+            .await
+            .expect("move root");
+        assert_eq!(
+            response.status(),
+            StatusCode::OK,
+            "a root that has no folder yet is not a reason to refuse the move"
+        );
+
+        assert!(!folder.exists(), "the folder left the old root");
+        assert!(
+            destination
+                .join("Dune Part Two (2024) [tmdbid-693134]")
+                .join("Dune Part Two (2024) [tmdbid-693134] - [2160p].mkv")
+                .exists(),
+            "the feature arrived in a root that had to be created for it"
+        );
+
+        let path: String = sqlx::query_scalar(
+            "SELECT path FROM media_files WHERE owner_kind = 'movie' AND owner_id = ?",
+        )
+        .bind(id)
+        .fetch_one(state.database().expect("database").pool())
+        .await
+        .expect("media file row");
+        assert!(
+            std::path::Path::new(&path).starts_with(&destination),
+            "the row follows the file: {path}"
+        );
+    }
+
     /// Issue #228: a title with nothing on disk changes root with no
     /// filesystem work at all — the seeded root paths do not even exist.
     #[tokio::test]
@@ -1780,22 +1836,21 @@ mod tests {
 
     /// Issue #228: if the rename fails, the row must not change — the
     /// operator sees the title where its files actually are and can retry.
+    #[cfg(unix)]
     #[tokio::test]
     async fn a_failed_rename_leaves_the_row_alone() {
+        use std::os::unix::fs::PermissionsExt;
+
         let (_dir, state, base) = application().await;
         let movie = add_movie(&base, 693_134, 1).await;
         let id = movie["id"].as_i64().expect("id");
         let source = tempfile::tempdir().expect("source root");
         let destination = tempfile::tempdir().expect("destination root");
         let folder = library_on_disk(&state, id, source.path()).await;
-        // A destination whose parent does not exist makes the rename itself
-        // fail while the collision pre-check still passes.
-        point_root_at(
-            &state,
-            2,
-            &destination.path().join("missing").join("library"),
-        )
-        .await;
+        point_root_at(&state, 2, destination.path()).await;
+        tokio::fs::set_permissions(&folder, std::fs::Permissions::from_mode(0o555))
+            .await
+            .expect("freeze the title folder");
 
         let response = reqwest::Client::new()
             .patch(format!("{base}/api/movies/{id}"))
@@ -1804,7 +1859,14 @@ mod tests {
             .await
             .expect("move root");
         assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR);
+        let body: serde_json::Value = response.json().await.expect("error body");
+        let error = body["error"].as_str().expect("error text");
+        assert!(error.contains(folder.to_str().expect("utf-8 source")));
+        assert!(error.contains(destination.path().to_str().expect("utf-8 destination")));
 
+        tokio::fs::set_permissions(&folder, std::fs::Permissions::from_mode(0o755))
+            .await
+            .expect("thaw the title folder");
         assert!(folder.exists(), "the folder never left the old root");
         let (root_id, path): (i64, String) = sqlx::query_as(
             "SELECT m.root_id, f.path FROM movies m

crates/arr-api/src/relocate.rs

@@ -60,15 +60,6 @@ pub(crate) struct Relocation {
     created_dirs: Vec<PathBuf>,
 }
 
-/// Whether the destination root is a directory that must already be there.
-/// A title moves into another configured root, which exists; a root moving to
-/// a new path is moving somewhere that need not exist yet.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-enum Destination {
-    Existing,
-    Create,
-}
-
 /// Rename the title's folders into the new root. Called before the row is
 /// written, and only when the root actually changes.
 ///
@@ -91,7 +82,7 @@ pub(crate) async fn relocate_title(
     let old_root = root_path(state, old_root_id).await?;
     let new_root = root_path(state, new_root_id).await?;
     let files = title_files(state, kind, title_id).await?;
-    relocate_files(&files, &old_root, &new_root, Destination::Existing).await
+    relocate_files(&files, &old_root, &new_root).await
 }
 
 /// Rename every title folder under a root into the root's new path, for a
@@ -114,7 +105,7 @@ pub(crate) async fn relocate_root(
     new_path: &str,
 ) -> Result<Relocation, ApiError> {
     let files = root_files(state, root_id).await?;
-    relocate_files(&files, old_path, new_path, Destination::Create).await
+    relocate_files(&files, old_path, new_path).await
 }
 
 /// The one mover both callers share: plan every rename, refuse every
@@ -124,7 +115,6 @@ async fn relocate_files(
     files: &[(i64, String)],
     old_root: &str,
     new_root: &str,
-    destination: Destination,
 ) -> Result<Relocation, ApiError> {
     let mut renames: Vec<PlannedRename> = Vec::new();
     let mut rewrites: Vec<(i64, String)> = Vec::new();
@@ -187,7 +177,10 @@ async fn relocate_files(
     // that already existed on disk. Recorded before `create_dir_all`, since
     // afterwards there is no way to tell which levels it made.
     let mut created_dirs: Vec<PathBuf> = Vec::new();
-    if destination == Destination::Create && !renames.is_empty() {
+    // A configured root need not exist on disk. Without creating it here,
+    // `rename` reports its missing parent as the same `NotFound` as a missing
+    // source and sends the operator looking at the wrong end of the move.
+    if !renames.is_empty() {
         created_dirs = missing_levels(std::path::Path::new(new_root)).await;
         if let Err(error) = tokio::fs::create_dir_all(new_root).await {
             return Err(ApiError::Filesystem(format!(
@@ -265,9 +258,13 @@ async fn failed(
     }
     .undo()
     .await;
+    // Name both ends. `rename` returns NotFound for a missing destination
+    // parent as readily as for a missing source, and naming only the source
+    // sent an operator looking for a file that was sitting where they left it.
     ApiError::Filesystem(format!(
-        "could not move '{}': {error}",
-        rename.source.display()
+        "could not move '{}' to '{}': {error}",
+        rename.source.display(),
+        rename.destination.display()
     ))
 }