Merge remote-tracking branch 'origin/main' into issue/114-read-score

crates/arr-daemon/src/grab.rs

@@ -129,8 +129,9 @@ impl GrabAction {
         for movie in gaps {
             let (movie_id, title) = (movie.id, movie.title.clone());
             let movie = match self.refresh_metadata(database, movie).await {
-                Ok(Some(movie)) => movie,
-                Ok(None) => continue,
+                Ok((movie, true)) => movie,
+                // Not released yet: no targeted search (§6.2).
+                Ok((_, false)) => continue,
                 // A title's metadata is not the rest of the tick's problem,
                 // same as a grab failure below.
                 Err(error) => {
@@ -162,17 +163,22 @@ impl GrabAction {
         Ok(outcomes)
     }
 
+    /// Refresh one title from TMDB, returning it alongside whether it is
+    /// digitally released (§6.2). Targeted search is gated on that flag; the
+    /// manual deck refresh of an already-available movie is not, since the
+    /// file on disk is proof enough and TMDB does not know a digital date for
+    /// every title (issue #115).
     async fn refresh_metadata(
         &self,
         database: &Db,
         movie: PendingMovie,
-    ) -> Result<Option<PendingMovie>, GrabError> {
+    ) -> Result<(PendingMovie, bool), GrabError> {
         let Some(tmdb) = &self.tmdb else {
-            return Ok(Some(movie));
+            return Ok((movie, true));
         };
         if !metadata_refresh_due(movie.metadata_refreshed_at.as_deref()) {
             let released = is_digitally_released(movie.digital_release.as_deref());
-            return Ok(released.then_some(movie));
+            return Ok((movie, released));
         }
         let tmdb_id =
             u32::try_from(movie.tmdb_id).map_err(|_| GrabError::InvalidTmdbId(movie.id))?;
@@ -232,28 +238,31 @@ impl GrabAction {
             .execute(database.pool())
             .await?;
         }
-        if !metadata.is_digitally_released(chrono::Utc::now().date_naive()) {
+        let released = metadata.is_digitally_released(chrono::Utc::now().date_naive());
+        if !released {
             tracing::debug!(
                 movie_id = movie.id,
                 "digital release has not happened; skipping targeted search"
             );
-            return Ok(None);
         }
-        Ok(Some(PendingMovie {
-            id: movie.id,
-            tmdb_id: movie.tmdb_id,
-            title,
-            year,
-            original_language,
-            search_attempts: if changed { 0 } else { movie.search_attempts },
-            last_searched_at: if changed {
-                None
-            } else {
-                movie.last_searched_at
+        Ok((
+            PendingMovie {
+                id: movie.id,
+                tmdb_id: movie.tmdb_id,
+                title,
+                year,
+                original_language,
+                search_attempts: if changed { 0 } else { movie.search_attempts },
+                last_searched_at: if changed {
+                    None
+                } else {
+                    movie.last_searched_at
+                },
+                digital_release,
+                metadata_refreshed_at: None,
             },
-            digital_release,
-            metadata_refreshed_at: None,
-        }))
+            released,
+        ))
     }
 
     /// Search every indexer for one title, cache each candidate with its
@@ -374,28 +383,91 @@ impl GrabAction {
             .await
     }
 
-    /// The manual trigger (§6.2, §9.3, issue #107): reset the backoff so
-    /// `search_due` cannot skip the title, then run the same search-and-grab
-    /// path as a tick, scoped to this one movie instead of the tick's
+    /// Sweep every searchable indexer for one title and upsert the results
+    /// into its release deck (§9.3) without choosing a winner.
+    ///
+    /// Issue #115: a manual search on a movie that is already satisfied is an
+    /// upgrade view being refreshed, not a gap being closed. It must re-score
+    /// and re-cache candidates — the same writes as the grab lane — and stop
+    /// there. Only a wanted movie with nothing on disk auto-grabs.
+    async fn refresh_deck(
+        &self,
+        database: &Db,
+        movie: &PendingMovie,
+        indexers: &[i64],
+    ) -> Result<(), GrabError> {
+        let Some(loaded) = database.movie_policy(movie.id).await? else {
+            return Ok(());
+        };
+        // §5.2: no original language, nothing to evaluate the language rule
+        // against — same refusal as the grab lane.
+        let Some(original_language) = movie.original_language.as_deref() else {
+            tracing::warn!(
+                movie_id = movie.id,
+                title = movie.title,
+                "no original language yet; not searching"
+            );
+            return Ok(());
+        };
+        let original_language = arr_db::policy::language(original_language);
+        let blacklist = Blacklist::load(database.pool()).await?;
+        let eligible = self
+            .search(
+                database,
+                movie,
+                indexers,
+                &loaded,
+                &original_language,
+                &blacklist,
+            )
+            .await?;
+        record_search(database, movie.id).await?;
+        tracing::info!(
+            movie_id = movie.id,
+            title = movie.title,
+            eligible = eligible.len(),
+            "release deck refreshed; not grabbing an already-satisfied movie"
+        );
+        Ok(())
+    }
+
+    /// The manual trigger (§6.2, §9.3, issues #107 and #115): reset the
+    /// backoff so `search_due` cannot skip the title, then sweep the
+    /// indexers, scoped to this one movie instead of the tick's
     /// `MOVIES_PER_TICK` batch.
+    ///
+    /// What happens to the results depends on the movie, not on the command:
+    /// a gap — wanted, unblocked, nothing on disk, no grab in flight — gets
+    /// search and grab; anything else already satisfied gets its deck
+    /// refreshed and no grab; a blocked movie is refused outright (§6.3).
     pub(crate) async fn search_now(
         &self,
         database: &Db,
         movie_id: i64,
     ) -> Result<Option<Outcome>, GrabError> {
+        let Some((movie, lane)) = manual_movie(database, movie_id).await? else {
+            tracing::info!(movie_id, "manual search refused: unknown or blocked movie");
+            return Ok(None);
+        };
         reset_search_backoff(database, movie_id).await?;
-        let Some(movie) = pending_movie(database, movie_id).await? else {
+        let (movie, released) = self.refresh_metadata(database, movie).await?;
+        // The release-date gate belongs to targeted search. A movie that is
+        // already on disk is released whatever TMDB says.
+        if !released && lane == ManualSearch::SearchAndGrab {
             return Ok(None);
-        };
-        let Some(movie) = self.refresh_metadata(database, movie).await? else {
-            return Ok(None);
-        };
+        }
         let searchable = self.indexers.searchable().await?;
         if searchable.is_empty() {
             tracing::warn!("no indexer advertises a text search; nothing can be grabbed");
             return Ok(None);
         }
-        self.grab_one(database, &movie, &searchable).await
+        match lane {
+            ManualSearch::SearchAndGrab => self.grab_one(database, &movie, &searchable).await,
+            ManualSearch::RefreshDeck => {
+                self.refresh_deck(database, &movie, &searchable).await?;
+                Ok(None)
+            }
+        }
     }
 
     /// The manual one-click grab (§9.3, issue #107): the release is already
@@ -901,9 +973,24 @@ fn search_due(movie: &PendingMovie) -> bool {
     backoff_elapsed(movie.search_attempts, movie.last_searched_at.as_deref())
 }
 
-/// Same shape as [`pending_movies`], scoped to one id and without the tick's
-/// batch limit — the manual trigger already named which title to search.
-async fn pending_movie(database: &Db, movie_id: i64) -> Result<Option<PendingMovie>, GrabError> {
+/// What a manual search on one movie is allowed to do (issue #115).
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum ManualSearch {
+    /// The movie is a gap: sweep the indexers and grab the winner.
+    SearchAndGrab,
+    /// The movie is already satisfied: sweep the indexers, refresh the deck
+    /// (§9.3), grab nothing.
+    RefreshDeck,
+}
+
+/// Same columns as [`pending_movies`], scoped to one id, without the tick's
+/// batch limit — the manual trigger already named which title to search — and
+/// without the gap filter, which decides the lane here instead of hiding the
+/// movie. `blocked` still refuses (§6.3).
+async fn manual_movie(
+    database: &Db,
+    movie_id: i64,
+) -> Result<Option<(PendingMovie, ManualSearch)>, GrabError> {
     let row = sqlx::query!(
         r#"
         SELECT   m.id    AS "id!: i64",
@@ -914,36 +1001,48 @@ async fn pending_movie(database: &Db, movie_id: i64) -> Result<Option<PendingMov
                  m.search_attempts AS "search_attempts!: i64",
                  m.last_searched_at,
                  m.digital_release,
-                 m.metadata_refreshed_at
+                 m.metadata_refreshed_at,
+                 (
+                     m.wanted = 1
+                     AND NOT EXISTS (
+                             SELECT 1 FROM media_files f
+                             WHERE  f.owner_kind = 'movie' AND f.owner_id = m.id
+                         )
+                     AND NOT EXISTS (
+                             SELECT 1 FROM grabs g
+                             WHERE  g.target_kind = 'movie' AND g.target_id = m.id
+                               AND  g.state IN ('sent', 'downloaded', 'imported')
+                         )
+                 ) AS "is_gap!: i64"
         FROM     movies m
         WHERE    m.id = ?
-          AND    m.wanted = 1
           AND    m.blocked = 0
-          AND    NOT EXISTS (
-                     SELECT 1 FROM media_files f
-                     WHERE  f.owner_kind = 'movie' AND f.owner_id = m.id
-                 )
-          AND    NOT EXISTS (
-                     SELECT 1 FROM grabs g
-                     WHERE  g.target_kind = 'movie' AND g.target_id = m.id
-                       AND  g.state IN ('sent', 'downloaded', 'imported')
-                 )
         "#,
         movie_id
     )
     .fetch_optional(database.pool())
     .await?;
 
-    Ok(row.map(|row| PendingMovie {
-        id: row.id,
-        tmdb_id: row.tmdb_id,
-        title: row.title,
-        year: row.year,
-        original_language: row.original_language,
-        search_attempts: row.search_attempts,
-        last_searched_at: row.last_searched_at,
-        digital_release: row.digital_release,
-        metadata_refreshed_at: row.metadata_refreshed_at,
+    Ok(row.map(|row| {
+        let lane = if row.is_gap == 1 {
+            ManualSearch::SearchAndGrab
+        } else {
+            ManualSearch::RefreshDeck
+        };
+        (
+            PendingMovie {
+                id: row.id,
+                tmdb_id: row.tmdb_id,
+                title: row.title,
+                year: row.year,
+                original_language: row.original_language,
+                search_attempts: row.search_attempts,
+                last_searched_at: row.last_searched_at,
+                digital_release: row.digital_release,
+                metadata_refreshed_at: row.metadata_refreshed_at,
+            },
+            lane,
+        )
     }))
 }
 
@@ -2055,6 +2154,131 @@ mod tests {
         assert_eq!(rule.as_deref(), Some("blacklisted"));
     }
 
+    /// A movie that is already on disk. `wanted` stays set: §9.3's upgrade
+    /// flow is exactly a satisfied movie still looking for something better.
+    async fn available_movie() -> (tempfile::TempDir, Db) {
+        let (dir, database) = wanted_movie().await;
+        sqlx::query(
+            "INSERT INTO media_files (owner_kind, owner_id, path, size)
+             VALUES ('movie', 1, '/mnt/media/movies/Dune Part Two (2024).mkv', 1)",
+        )
+        .execute(database.pool())
+        .await
+        .unwrap();
+        sqlx::query("UPDATE movies SET state = 'available' WHERE id = 1")
+            .execute(database.pool())
+            .await
+            .unwrap();
+        (dir, database)
+    }
+
+    async fn deck(database: &Db) -> Vec<(String, String)> {
+        sqlx::query_as::<_, (String, String)>(
+            "SELECT r.name, r.verdict
+             FROM releases r JOIN movie_releases mr ON mr.release_id = r.id
+             WHERE mr.movie_id = 1 ORDER BY r.guid",
+        )
+        .fetch_all(database.pool())
+        .await
+        .unwrap()
+    }
+
+    /// Issue #115: a manual search on an available movie sweeps the indexers
+    /// and refreshes the deck §9.3 reads, and grabs nothing. The old code
+    /// loaded the movie through the gap filter, so this was a silent no-op.
+    #[tokio::test]
+    async fn a_manual_search_on_an_available_movie_refreshes_the_deck() {
+        let (_dir, database) = available_movie().await;
+        let indexer = prowlarr().await;
+        let (downloader, fake) = transmission().await;
+
+        let outcome = action(&indexer, &downloader)
+            .search_now(&database, 1)
+            .await
+            .unwrap();
+
+        assert!(outcome.is_none(), "an available movie must not auto-grab");
+        assert_eq!(targeted_searches(&indexer).await, 1);
+        assert_eq!(deck(&database).await.len(), 3, "every candidate is cached");
+        assert!(fake.torrents().is_empty());
+        assert!(grabs(&database).await.is_empty());
+    }
+
+    /// The digital-release gate belongs to targeted search (§6.2). A movie
+    /// with a file on disk is released whatever TMDB knows, so it must not
+    /// silence the manual deck refresh.
+    #[tokio::test]
+    async fn an_available_movie_refreshes_its_deck_before_the_digital_release() {
+        let (_dir, database) = available_movie().await;
+        let indexer = prowlarr().await;
+        let metadata = tmdb(UNRELEASED_METADATA).await;
+        let (downloader, _fake) = transmission().await;
+
+        action_with_tmdb(&indexer, &downloader, &metadata)
+            .search_now(&database, 1)
+            .await
+            .unwrap();
+
+        assert_eq!(deck(&database).await.len(), 3);
+    }
+
+    /// A grab already in flight is not a gap either: refresh the deck, do not
+    /// send a second torrent.
+    #[tokio::test]
+    async fn a_manual_search_with_a_grab_in_flight_does_not_grab_again() {
+        let (_dir, database) = wanted_movie().await;
+        let indexer = prowlarr().await;
+        let (downloader, fake) = transmission().await;
+        let action = action(&indexer, &downloader);
+        action.tick(&database).await.unwrap();
+        assert_eq!(grabs(&database).await.len(), 1);
+
+        action.search_now(&database, 1).await.unwrap();
+
+        assert_eq!(fake.torrents().len(), 1);
+        assert_eq!(grabs(&database).await.len(), 1);
+    }
+
+    /// The gap lane is unchanged: a wanted movie with nothing on disk still
+    /// searches and grabs from the manual trigger.
+    #[tokio::test]
+    async fn a_manual_search_on_a_wanted_movie_still_grabs() {
+        let (_dir, database) = wanted_movie().await;
+        let indexer = prowlarr().await;
+        let (downloader, fake) = transmission().await;
+
+        let outcome = action(&indexer, &downloader)
+            .search_now(&database, 1)
+            .await
+            .unwrap();
+
+        assert!(outcome.is_some());
+        assert_eq!(fake.torrents().len(), 1);
+        assert_eq!(grabs(&database).await.len(), 1);
+    }
+
+    /// §6.3: blocked refuses the manual trigger too, deck refresh included.
+    #[tokio::test]
+    async fn a_manual_search_on_a_blocked_movie_calls_no_indexer() {
+        let (_dir, database) = available_movie().await;
+        sqlx::query("UPDATE movies SET blocked = 1")
+            .execute(database.pool())
+            .await
+            .unwrap();
+        let indexer = prowlarr().await;
+        let (downloader, fake) = transmission().await;
+
+        let outcome = action(&indexer, &downloader)
+            .search_now(&database, 1)
+            .await
+            .unwrap();
+
+        assert!(outcome.is_none());
+        assert_eq!(targeted_searches(&indexer).await, 0);
+        assert!(deck(&database).await.is_empty());
+        assert!(fake.torrents().is_empty());
+    }
+
     /// §6.3: `blocked` stops targeted search for a title.
     #[tokio::test]
     async fn a_blocked_title_is_not_searched() {