From 9fd183362e4ead3adc899c284ce0bfea4dc89c87 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 10:39:53 -0300 Subject: [PATCH 1/8] refactor(lore): add LoreAPI actor message protocol This commit adds Tokio as the async runtime dependency and introduces LoreApiMessage as the message protocol for the upcoming LoreAPI actor. The protocol mirrors the current LoreServiceApi surface without wiring App or main to the actor yet. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- Cargo.lock | 24 ++++++++++- Cargo.toml | 1 + src/lore/application/messages.rs | 71 ++++++++++++++++++++++++++++++++ src/lore/application/mod.rs | 1 + 4 files changed, 96 insertions(+), 1 deletion(-) create mode 100644 src/lore/application/messages.rs diff --git a/Cargo.lock b/Cargo.lock index 75ea973..38f9236 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -840,6 +840,7 @@ dependencies = [ "serde-xml-rs", "serde_json", "thiserror 2.0.16", + "tokio", "tracing", "tracing-appender", "tracing-subscriber", @@ -1342,6 +1343,27 @@ dependencies = [ "time-core", ] +[[package]] +name = "tokio" +version = "1.52.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" +dependencies = [ + "pin-project-lite", + "tokio-macros", +] + +[[package]] +name = "tokio-macros" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "385a6cb71ab9ab790c5fe8d67f1645e6c450a7ce006a33de03daa956cf70a496" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "tracing" version = "0.1.41" diff --git a/Cargo.toml b/Cargo.toml index 8a1193a..b6c51b4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -27,6 +27,7 @@ chrono = "0.4.41" ansi-to-tui = "7.0.0" which = "8.0.0" ureq = { version = "3.0.12", features = ["rustls"] } +tokio = { version = "1.52.3", features = ["rt-multi-thread", "macros", "sync"] } [dev-dependencies] ctor = "0.2" diff --git a/src/lore/application/messages.rs b/src/lore/application/messages.rs new file mode 100644 index 0000000..7465622 --- /dev/null +++ b/src/lore/application/messages.rs @@ -0,0 +1,71 @@ +#![allow(dead_code)] // Phase 6 wires this protocol in follow-up commits. + +use std::{ + collections::{HashMap, HashSet}, + path::PathBuf, +}; + +use tokio::sync::oneshot; + +use crate::{ + infrastructure::shell::ShellCommand, + lore::{ + application::{ + cache::{BootstrapLoreData, CacheMode}, + dto::PatchsetDetails, + errors::LoreError, + }, + domain::{mailing_list::MailingList, patch::Patch}, + }, +}; + +pub type LoreApiResult = Result; + +pub enum LoreApiMessage { + GetBootstrapData { + reply: oneshot::Sender>, + }, + FetchAvailableLists { + cache_mode: CacheMode, + reply: oneshot::Sender>>, + }, + FetchFeedPage { + target_list: String, + page_size: usize, + page_number: usize, + cache_mode: CacheMode, + reply: oneshot::Sender>>, + }, + FetchPatchsetDetails { + representative_patch: Patch, + cache_mode: CacheMode, + reply: oneshot::Sender>, + }, + LoadBookmarks { + reply: oneshot::Sender>>, + }, + SaveBookmarks { + bookmarks: Vec, + reply: oneshot::Sender>, + }, + LoadReviewed { + reply: oneshot::Sender>>>, + }, + SaveReviewed { + reviewed: HashMap>, + reply: oneshot::Sender>, + }, + GetGitSignature { + git_repo_path: String, + reply: oneshot::Sender<(String, String)>, + }, + PrepareReplyCommands { + tmp_dir: PathBuf, + target_list: String, + patches: Vec, + patches_to_reply: Vec, + git_signature: String, + git_send_email_options: String, + reply: oneshot::Sender>>, + }, +} diff --git a/src/lore/application/mod.rs b/src/lore/application/mod.rs index 55f4fb4..3003a92 100644 --- a/src/lore/application/mod.rs +++ b/src/lore/application/mod.rs @@ -2,6 +2,7 @@ pub mod api; pub mod cache; pub mod dto; pub mod errors; +pub mod messages; pub mod service; #[cfg(test)] From f62a219bfcf07bf7bbadde99a6b2890ce663e9b2 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 10:45:18 -0300 Subject: [PATCH 2/8] refactor(lore): add LoreAPI actor and async handle This commit introduces LoreApiHandle and LoreApiActor around the existing LoreService core. The actor owns LoreService state, receives LoreApiMessage requests over mpsc, replies over oneshot, and runs service operations through spawn_blocking so blocking Lore work stays behind the actor boundary. It also adds actor-level tests for bootstrap data loading and preserving Lore cache state across sequential requests. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/lore/application/actor.rs | 256 +++++++++++++++++++++++++++++++ src/lore/application/errors.rs | 3 + src/lore/application/handle.rs | 156 +++++++++++++++++++ src/lore/application/messages.rs | 2 +- src/lore/application/mod.rs | 2 + 5 files changed, 418 insertions(+), 1 deletion(-) create mode 100644 src/lore/application/actor.rs create mode 100644 src/lore/application/handle.rs diff --git a/src/lore/application/actor.rs b/src/lore/application/actor.rs new file mode 100644 index 0000000..b4abe49 --- /dev/null +++ b/src/lore/application/actor.rs @@ -0,0 +1,256 @@ +#![allow(dead_code)] // Follow-up commits wire the actor into main/App. + +use tokio::{sync::mpsc, task}; + +use crate::lore::application::{ + api::LoreServiceApi, errors::LoreError, handle::LoreApiHandle, messages::LoreApiMessage, + service::LoreService, +}; + +pub const DEFAULT_LORE_API_CHANNEL_SIZE: usize = 32; + +pub struct LoreApiActor { + core: Option, + rx: mpsc::Receiver, +} + +impl LoreApiActor { + pub fn new(core: LoreService, rx: mpsc::Receiver) -> Self { + Self { + core: Some(core), + rx, + } + } + + pub fn spawn(core: LoreService) -> LoreApiHandle { + let (tx, rx) = mpsc::channel(DEFAULT_LORE_API_CHANNEL_SIZE); + tokio::spawn(Self::new(core, rx).run()); + LoreApiHandle::new(tx) + } + + pub async fn run(mut self) { + while let Some(message) = self.rx.recv().await { + self.handle_message(message).await; + } + } + + async fn handle_message(&mut self, message: LoreApiMessage) { + match message { + LoreApiMessage::GetBootstrapData { reply } => { + let result = self + .with_core(|core| core.warm_bootstrap_cache()) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::FetchAvailableLists { cache_mode, reply } => { + let result = self + .with_core(move |core| core.fetch_available_lists(cache_mode)) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::FetchFeedPage { + target_list, + page_size, + page_number, + cache_mode, + reply, + } => { + let result = self + .with_core(move |core| { + core.fetch_next_patch_page( + &target_list, + page_size, + page_number, + cache_mode, + ) + }) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::FetchPatchsetDetails { + representative_patch, + cache_mode, + reply, + } => { + let result = self + .with_core(move |core| { + core.fetch_patchset_details(&representative_patch, cache_mode) + }) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::LoadBookmarks { reply } => { + let result = self + .with_core(|core| core.load_bookmarked_patchsets()) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::SaveBookmarks { bookmarks, reply } => { + let result = self + .with_core(move |core| core.save_bookmarked_patchsets(&bookmarks)) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::LoadReviewed { reply } => { + let result = self + .with_core(|core| core.load_reviewed_patchsets()) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::SaveReviewed { reviewed, reply } => { + let result = self + .with_core(move |core| core.save_reviewed_patchsets(&reviewed)) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + LoreApiMessage::GetGitSignature { + git_repo_path, + reply, + } => { + let result = self + .with_core(move |core| core.get_git_signature(&git_repo_path)) + .await; + let _ = reply.send(result); + } + LoreApiMessage::PrepareReplyCommands { + tmp_dir, + target_list, + patches, + patches_to_reply, + git_signature, + git_send_email_options, + reply, + } => { + let result = self + .with_core(move |core| { + core.prepare_reply_commands( + &tmp_dir, + &target_list, + &patches, + &patches_to_reply, + &git_signature, + &git_send_email_options, + ) + }) + .await + .and_then(|result| result); + let _ = reply.send(result); + } + } + } + + async fn with_core(&mut self, operation: F) -> Result + where + T: Send + 'static, + F: FnOnce(&mut LoreService) -> T + Send + 'static, + { + let mut core = self + .core + .take() + .ok_or_else(|| LoreError::ActorUnavailable("core unavailable".to_string()))?; + let (core, result) = task::spawn_blocking(move || { + let result = operation(&mut core); + (core, result) + }) + .await + .map_err(|e| LoreError::ActorUnavailable(e.to_string()))?; + self.core = Some(core); + Ok(result) + } +} + +#[cfg(test)] +mod tests { + use std::{collections::HashMap, sync::Arc}; + + use crate::{ + infrastructure::{file_system::MockFileSystemTrait, shell::MockShellTrait}, + lore::{ + application::{cache::{CacheMode, CacheTtl}, handle::LoreApiHandle}, + domain::mailing_list::MailingList, + infrastructure::{ + http_lore_client::{MockFeedGateway, MockListsGateway, MockPatchHtmlGateway}, + patchset_fetcher::MockPatchsetFetcher, + patchset_parser::MockPatchsetParser, + persistence::{MockMailingListsCacheStore, MockUserLoreStateStore}, + }, + }, + }; + + use super::*; + + fn make_service( + lists_store: MockMailingListsCacheStore, + user_state: MockUserLoreStateStore, + ) -> LoreService { + LoreService::new( + Arc::new(MockListsGateway::new()), + Arc::new(MockFeedGateway::new()), + Arc::new(MockPatchHtmlGateway::new()), + Arc::new(lists_store), + Arc::new(user_state), + Arc::new(MockPatchsetFetcher::new()), + Arc::new(MockPatchsetParser::new()), + Arc::new(MockFileSystemTrait::new()), + Arc::new(MockShellTrait::new()), + CacheTtl::default(), + ) + } + + fn spawn_test_actor(core: LoreService) -> LoreApiHandle { + LoreApiActor::spawn(core) + } + + #[tokio::test] + async fn handle_returns_bootstrap_data_from_actor() { + let mut lists_store = MockMailingListsCacheStore::new(); + lists_store + .expect_load_available_lists() + .times(1) + .returning(|| Ok(vec![MailingList::new("linux-mm", "")])); + + let mut user_state = MockUserLoreStateStore::new(); + user_state + .expect_load_bookmarked_patchsets() + .times(1) + .returning(|| Ok(vec![])); + user_state + .expect_load_reviewed_patchsets() + .times(1) + .returning(|| Ok(HashMap::new())); + + let handle = spawn_test_actor(make_service(lists_store, user_state)); + + let data = handle.get_bootstrap_data().await.unwrap(); + + assert_eq!(1, data.mailing_lists.len()); + assert_eq!("linux-mm", data.mailing_lists[0].name()); + assert!(data.bookmarks.is_empty()); + assert!(data.reviewed.is_empty()); + } + + #[tokio::test] + async fn actor_preserves_cache_across_messages() { + let mut lists_store = MockMailingListsCacheStore::new(); + lists_store + .expect_load_available_lists() + .times(1) + .returning(|| Ok(vec![MailingList::new("cached-list", "")])); + + let handle = spawn_test_actor(make_service(lists_store, MockUserLoreStateStore::new())); + + let first = handle.fetch_available_lists(CacheMode::UseCache).await.unwrap(); + let second = handle.fetch_available_lists(CacheMode::UseCache).await.unwrap(); + + assert_eq!("cached-list", first[0].name()); + assert_eq!("cached-list", second[0].name()); + } +} diff --git a/src/lore/application/errors.rs b/src/lore/application/errors.rs index 496b4ee..1a33a36 100644 --- a/src/lore/application/errors.rs +++ b/src/lore/application/errors.rs @@ -21,4 +21,7 @@ pub enum LoreError { #[error("feed ended")] EndOfFeed, + + #[error("lore api actor unavailable: {0}")] + ActorUnavailable(String), } diff --git a/src/lore/application/handle.rs b/src/lore/application/handle.rs new file mode 100644 index 0000000..00dab0c --- /dev/null +++ b/src/lore/application/handle.rs @@ -0,0 +1,156 @@ +#![allow(dead_code)] // Follow-up commits replace App's synchronous LoreServiceApi usage. + +use std::{ + collections::{HashMap, HashSet}, + path::PathBuf, +}; + +use tokio::sync::{mpsc, oneshot}; + +use crate::{ + infrastructure::shell::ShellCommand, + lore::{ + application::{ + cache::{BootstrapLoreData, CacheMode}, + dto::PatchsetDetails, + errors::LoreError, + messages::{LoreApiMessage, LoreApiResult}, + }, + domain::{mailing_list::MailingList, patch::Patch}, + }, +}; + +#[derive(Clone)] +pub struct LoreApiHandle { + tx: mpsc::Sender, +} + +impl LoreApiHandle { + pub fn new(tx: mpsc::Sender) -> Self { + Self { tx } + } + + pub async fn get_bootstrap_data(&self) -> LoreApiResult { + self.request_result(|reply| LoreApiMessage::GetBootstrapData { reply }) + .await + } + + pub async fn fetch_available_lists( + &self, + cache_mode: CacheMode, + ) -> LoreApiResult> { + self.request_result(|reply| LoreApiMessage::FetchAvailableLists { cache_mode, reply }) + .await + } + + pub async fn fetch_feed_page( + &self, + target_list: String, + page_size: usize, + page_number: usize, + cache_mode: CacheMode, + ) -> LoreApiResult> { + self.request_result(|reply| LoreApiMessage::FetchFeedPage { + target_list, + page_size, + page_number, + cache_mode, + reply, + }) + .await + } + + pub async fn fetch_patchset_details( + &self, + representative_patch: Patch, + cache_mode: CacheMode, + ) -> LoreApiResult { + self.request_result(|reply| LoreApiMessage::FetchPatchsetDetails { + representative_patch, + cache_mode, + reply, + }) + .await + } + + pub async fn load_bookmarks(&self) -> LoreApiResult> { + self.request_result(|reply| LoreApiMessage::LoadBookmarks { reply }) + .await + } + + pub async fn save_bookmarks(&self, bookmarks: Vec) -> LoreApiResult<()> { + self.request_result(|reply| LoreApiMessage::SaveBookmarks { bookmarks, reply }) + .await + } + + pub async fn load_reviewed(&self) -> LoreApiResult>> { + self.request_result(|reply| LoreApiMessage::LoadReviewed { reply }) + .await + } + + pub async fn save_reviewed( + &self, + reviewed: HashMap>, + ) -> LoreApiResult<()> { + self.request_result(|reply| LoreApiMessage::SaveReviewed { reviewed, reply }) + .await + } + + pub async fn get_git_signature( + &self, + git_repo_path: String, + ) -> Result<(String, String), LoreError> { + self.request_result(|reply| LoreApiMessage::GetGitSignature { + git_repo_path, + reply, + }) + .await + } + + pub async fn prepare_reply_commands( + &self, + tmp_dir: PathBuf, + target_list: String, + patches: Vec, + patches_to_reply: Vec, + git_signature: String, + git_send_email_options: String, + ) -> LoreApiResult> { + self.request_result(|reply| LoreApiMessage::PrepareReplyCommands { + tmp_dir, + target_list, + patches, + patches_to_reply, + git_signature, + git_send_email_options, + reply, + }) + .await + } + + async fn request_result( + &self, + build_message: impl FnOnce(oneshot::Sender>) -> LoreApiMessage, + ) -> LoreApiResult + where + T: Send + 'static, + { + self.request(build_message).await? + } + + async fn request( + &self, + build_message: impl FnOnce(oneshot::Sender) -> LoreApiMessage, + ) -> Result + where + T: Send + 'static, + { + let (reply, rx) = oneshot::channel(); + self.tx + .send(build_message(reply)) + .await + .map_err(|_| LoreError::ActorUnavailable("request channel closed".to_string()))?; + rx.await + .map_err(|_| LoreError::ActorUnavailable("reply channel closed".to_string())) + } +} diff --git a/src/lore/application/messages.rs b/src/lore/application/messages.rs index 7465622..3ee1806 100644 --- a/src/lore/application/messages.rs +++ b/src/lore/application/messages.rs @@ -57,7 +57,7 @@ pub enum LoreApiMessage { }, GetGitSignature { git_repo_path: String, - reply: oneshot::Sender<(String, String)>, + reply: oneshot::Sender>, }, PrepareReplyCommands { tmp_dir: PathBuf, diff --git a/src/lore/application/mod.rs b/src/lore/application/mod.rs index 3003a92..a0cfc37 100644 --- a/src/lore/application/mod.rs +++ b/src/lore/application/mod.rs @@ -1,7 +1,9 @@ +pub mod actor; pub mod api; pub mod cache; pub mod dto; pub mod errors; +pub mod handle; pub mod messages; pub mod service; From 371240939eb33034ae168feb24ecf1f3cb4063b8 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 10:53:11 -0300 Subject: [PATCH 3/8] refactor(lore): load app bootstrap through LoreAPI actor This commit starts the LoreAPI actor during application startup and loads BootstrapLoreData through LoreApiHandle before constructing App. App::new now receives the already-resolved Lore bootstrap data instead of warming Lore cache itself. The remaining App Lore flows still use the existing LoreServiceApi path and will be migrated in follow-up commits. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/app/mod.rs | 10 +++++++--- src/lore/application/actor.rs | 2 -- src/lore/application/handle.rs | 2 +- src/lore/application/messages.rs | 2 +- src/main.rs | 22 ++++++++++++++++++++-- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index d082999..89e973c 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -26,7 +26,11 @@ use crate::{ shell::{ShellCommand, ShellTrait}, }, lore::{ - application::{api::LoreServiceApi, cache::CacheMode, errors::LoreError}, + application::{ + api::LoreServiceApi, + cache::{BootstrapLoreData, CacheMode}, + errors::LoreError, + }, domain::patch::{Author, Patch}, }, ui::popup::info_popup::InfoPopUp, @@ -73,13 +77,13 @@ impl App { /// `App` instance with loading configurations and app data. pub fn new( config_service: Box, + bootstrap: BootstrapLoreData, fs: Box, shell: Box, env: Box, - mut lore_service: Box, + lore_service: Box, render: Box, ) -> color_eyre::Result { - let bootstrap = lore_service.warm_bootstrap_cache().unwrap_or_default(); let config = config_service.snapshot(); event!(Level::INFO, "patch-hub started"); diff --git a/src/lore/application/actor.rs b/src/lore/application/actor.rs index b4abe49..79acd3f 100644 --- a/src/lore/application/actor.rs +++ b/src/lore/application/actor.rs @@ -1,5 +1,3 @@ -#![allow(dead_code)] // Follow-up commits wire the actor into main/App. - use tokio::{sync::mpsc, task}; use crate::lore::application::{ diff --git a/src/lore/application/handle.rs b/src/lore/application/handle.rs index 00dab0c..96fadbc 100644 --- a/src/lore/application/handle.rs +++ b/src/lore/application/handle.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] // Follow-up commits replace App's synchronous LoreServiceApi usage. +#![allow(dead_code)] // Follow-up commits replace remaining synchronous LoreServiceApi usage. use std::{ collections::{HashMap, HashSet}, diff --git a/src/lore/application/messages.rs b/src/lore/application/messages.rs index 3ee1806..4b5cf6d 100644 --- a/src/lore/application/messages.rs +++ b/src/lore/application/messages.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] // Phase 6 wires this protocol in follow-up commits. +#![allow(dead_code)] // Follow-up commits wire the remaining message variants. use std::{ collections::{HashMap, HashSet}, diff --git a/src/main.rs b/src/main.rs index 0d3b004..5add7e3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,7 +24,9 @@ use infrastructure::{ terminal::{init, restore}, }; use lore::{ - application::{api::LoreServiceApi, cache::CacheTtl, service::LoreService}, + application::{ + actor::LoreApiActor, api::LoreServiceApi, cache::CacheTtl, service::LoreService, + }, infrastructure::{ http_lore_client::HttpLoreGateway, patchset_fetcher::B4PatchsetFetcher, @@ -84,7 +86,8 @@ fn check_external_deps(env: &dyn EnvTrait, config: &ConfigSnapshot) -> bool { app_can_run } -fn main() -> color_eyre::Result<()> { +#[tokio::main] +async fn main() -> color_eyre::Result<()> { // file writer guards should be propagated to main() so the logging thread lives enough let InitMonitoringProduct { logging_guards_by_file_name, @@ -136,6 +139,20 @@ fn main() -> color_eyre::Result<()> { let render: Box = Box::new(ShellRenderService::new(shell_arc.clone())); + let lore_api = LoreApiActor::spawn(LoreService::new( + gateway.clone(), + gateway.clone(), + gateway.clone(), + persistence.clone() as Arc, + persistence.clone() as Arc, + fetcher.clone(), + parser.clone(), + fs_arc.clone(), + shell_arc.clone(), + CacheTtl::default(), + )); + let bootstrap = lore_api.get_bootstrap_data().await.unwrap_or_default(); + let lore_service: Box = Box::new(LoreService::new( gateway.clone(), gateway.clone(), @@ -151,6 +168,7 @@ fn main() -> color_eyre::Result<()> { let app = App::new( config_service, + bootstrap, Box::new(OsFileSystem), Box::new(OsShell), Box::new(env), From d623af4b784efa50eeca14ca835d52139e0c0220 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 10:57:34 -0300 Subject: [PATCH 4/8] refactor(lore): route feed and list flows through LoreAPI This commit wires App with LoreApiHandle for mailing-list refresh and latest feed pagination. The affected App methods and handler paths become async so these flows request Lore data through the LoreAPI actor instead of calling the synchronous LoreServiceApi path. The legacy LoreServiceApi service remains temporarily for patchset details and review actions, which will be migrated in follow-up commits. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/app/mod.rs | 17 +++-- src/app/screens/latest.rs | 122 +++++++++++++++++++++++++--------- src/app/screens/mail_list.rs | 8 +-- src/handler/latest.rs | 8 +-- src/handler/mail_list.rs | 6 +- src/handler/mod.rs | 21 +++--- src/lore/application/actor.rs | 22 +++--- src/main.rs | 3 +- 8 files changed, 140 insertions(+), 67 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 89e973c..7242224 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -30,6 +30,7 @@ use crate::{ api::LoreServiceApi, cache::{BootstrapLoreData, CacheMode}, errors::LoreError, + handle::LoreApiHandle, }, domain::patch::{Author, Patch}, }, @@ -48,6 +49,7 @@ pub use view_model::AppViewModel; /// Injected capabilities used by `App` orchestration (not screen state). pub struct AppServices { + pub lore_api: LoreApiHandle, pub lore: Box, pub render: Box, pub shell: Box, @@ -81,6 +83,7 @@ impl App { fs: Box, shell: Box, env: Box, + lore_api: LoreApiHandle, lore_service: Box, render: Box, ) -> color_eyre::Result { @@ -116,6 +119,7 @@ impl App { popup: None, }, services: AppServices { + lore_api, lore: lore_service, render, shell, @@ -153,22 +157,25 @@ impl App { } /// Fetches (or re-fetches) the current page of latest patchsets from Lore. - pub fn fetch_latest_current_page(&mut self) -> color_eyre::Result<()> { - let lore = self.services.lore.as_mut(); + pub async fn fetch_latest_current_page(&mut self) -> color_eyre::Result<()> { + let lore_api = &self.services.lore_api; let latest_patchsets = &mut self.state.lore.latest_patchsets; if let Some(patchsets) = latest_patchsets.as_mut() { - patchsets.fetch_current_page(lore, CacheMode::UseCache) + patchsets + .fetch_current_page(lore_api, CacheMode::UseCache) + .await } else { Ok(()) } } /// Refreshes available mailing lists and updates [`LoreUiState::mailing_list_selection`]. - pub fn refresh_mailing_lists(&mut self) -> color_eyre::Result<()> { + pub async fn refresh_mailing_lists(&mut self) -> color_eyre::Result<()> { self.state .lore .mailing_list_selection - .refresh_available_mailing_lists(self.services.lore.as_mut(), CacheMode::Refresh) + .refresh_available_mailing_lists(&self.services.lore_api, CacheMode::Refresh) + .await } /// Loads patchset details into [`LoreUiState::details`]. diff --git a/src/app/screens/latest.rs b/src/app/screens/latest.rs index b443ade..4a5140d 100644 --- a/src/app/screens/latest.rs +++ b/src/app/screens/latest.rs @@ -1,7 +1,7 @@ use color_eyre::eyre::bail; use crate::lore::{ - application::{api::LoreServiceApi, cache::CacheMode, errors::LoreError}, + application::{cache::CacheMode, errors::LoreError, handle::LoreApiHandle}, domain::patch::Patch, }; @@ -43,17 +43,20 @@ impl LatestPatchsetsState { self.page_size } - pub fn fetch_current_page( + pub async fn fetch_current_page( &mut self, - lore_service: &mut dyn LoreServiceApi, + lore_api: &LoreApiHandle, mode: CacheMode, ) -> color_eyre::Result<()> { - match lore_service.fetch_next_patch_page( - &self.target_list, - self.page_size, - self.page_number, - mode, - ) { + match lore_api + .fetch_feed_page( + self.target_list.clone(), + self.page_size, + self.page_number, + mode, + ) + .await + { Ok(patches) => { self.current_page = patches; } @@ -109,7 +112,22 @@ impl LatestPatchsetsState { #[cfg(test)] mod tests { use super::*; - use crate::lore::application::MockLoreServiceApi; + use std::sync::Arc; + + use crate::{ + infrastructure::{file_system::MockFileSystemTrait, shell::MockShellTrait}, + lore::{ + application::{actor::LoreApiActor, cache::CacheTtl, service::LoreService}, + infrastructure::{ + http_lore_client::{ + LoreHttpError, MockFeedGateway, MockListsGateway, MockPatchHtmlGateway, + }, + patchset_fetcher::MockPatchsetFetcher, + patchset_parser::MockPatchsetParser, + persistence::{MockMailingListsCacheStore, MockUserLoreStateStore}, + }, + }, + }; fn make_patch(msg_id: &str) -> Patch { serde_json::from_value(serde_json::json!({ @@ -121,51 +139,91 @@ mod tests { .unwrap() } - #[test] - fn test_fetch_current_page_success() { - let mut mock = MockLoreServiceApi::new(); - mock.expect_fetch_next_patch_page() + fn make_handle(feed_gateway: MockFeedGateway) -> LoreApiHandle { + let service = LoreService::new( + Arc::new(MockListsGateway::new()), + Arc::new(feed_gateway), + Arc::new(MockPatchHtmlGateway::new()), + Arc::new(MockMailingListsCacheStore::new()), + Arc::new(MockUserLoreStateStore::new()), + Arc::new(MockPatchsetFetcher::new()), + Arc::new(MockPatchsetParser::new()), + Arc::new(MockFileSystemTrait::new()), + Arc::new(MockShellTrait::new()), + CacheTtl::default(), + ); + LoreApiActor::spawn(service) + } + + fn patch_feed_response() -> String { + r#" + + + test patch + Testtest@test.com + + 2023-01-01 + + + test patch 2 + Testtest@test.com + + 2023-01-01 + +"# + .to_string() + } + + #[tokio::test] + async fn test_fetch_current_page_success() { + let mut feed_gateway = MockFeedGateway::new(); + feed_gateway + .expect_fetch_patch_feed_page() .times(1) - .returning(|_, _, _, _| Ok(vec![make_patch("id-1"), make_patch("id-2")])); + .returning(|_, _| Ok(patch_feed_response())); - let mut lp = LatestPatchsetsState::new("some-list".to_string(), 5); - let result = lp.fetch_current_page(&mut mock, CacheMode::UseCache); + let handle = make_handle(feed_gateway); + let mut lp = LatestPatchsetsState::new("some-list".to_string(), 2); + let result = lp.fetch_current_page(&handle, CacheMode::UseCache).await; assert!(result.is_ok()); assert_eq!(lp.processed_patchsets_count(), 2); } - #[test] - fn test_fetch_current_page_end_of_feed() { - let mut mock = MockLoreServiceApi::new(); - mock.expect_fetch_next_patch_page() + #[tokio::test] + async fn test_fetch_current_page_end_of_feed() { + let mut feed_gateway = MockFeedGateway::new(); + feed_gateway + .expect_fetch_patch_feed_page() .times(1) - .returning(|_, _, _, _| Err(LoreError::EndOfFeed)); + .returning(|_, _| Err(LoreHttpError::EndOfFeed)); + let handle = make_handle(feed_gateway); let mut lp = LatestPatchsetsState::new("some-list".to_string(), 5); - let result = lp.fetch_current_page(&mut mock, CacheMode::UseCache); + let result = lp.fetch_current_page(&handle, CacheMode::UseCache).await; assert!(result.is_ok()); assert_eq!(lp.processed_patchsets_count(), 0); } - #[test] - fn test_fetch_current_page_error() { + #[tokio::test] + async fn test_fetch_current_page_error() { use crate::infrastructure::net::NetError; - use crate::lore::infrastructure::http_lore_client::LoreHttpError; - let mut mock = MockLoreServiceApi::new(); - mock.expect_fetch_next_patch_page() + let mut feed_gateway = MockFeedGateway::new(); + feed_gateway + .expect_fetch_patch_feed_page() .times(1) - .returning(|_, _, _, _| { - Err(LoreError::Http(LoreHttpError::Net(NetError::HttpStatus { + .returning(|_, _| { + Err(LoreHttpError::Net(NetError::HttpStatus { code: 500, message: "Internal Server Error".to_string(), - }))) + })) }); + let handle = make_handle(feed_gateway); let mut lp = LatestPatchsetsState::new("some-list".to_string(), 5); - let result = lp.fetch_current_page(&mut mock, CacheMode::UseCache); + let result = lp.fetch_current_page(&handle, CacheMode::UseCache).await; assert!(result.is_err()); } diff --git a/src/app/screens/mail_list.rs b/src/app/screens/mail_list.rs index c6db609..0074d71 100644 --- a/src/app/screens/mail_list.rs +++ b/src/app/screens/mail_list.rs @@ -1,7 +1,7 @@ use color_eyre::eyre::bail; use crate::lore::{ - application::{api::LoreServiceApi, cache::CacheMode}, + application::{cache::CacheMode, handle::LoreApiHandle}, domain::mailing_list::MailingList, }; @@ -13,12 +13,12 @@ pub struct MailingListSelectionState { } impl MailingListSelectionState { - pub fn refresh_available_mailing_lists( + pub async fn refresh_available_mailing_lists( &mut self, - lore_service: &mut dyn LoreServiceApi, + lore_api: &LoreApiHandle, mode: CacheMode, ) -> color_eyre::Result<()> { - match lore_service.fetch_available_lists(mode) { + match lore_api.fetch_available_lists(mode).await { Ok(available_mailing_lists) => { self.mailing_lists = available_mailing_lists; } diff --git a/src/handler/latest.rs b/src/handler/latest.rs index 999effe..4169727 100644 --- a/src/handler/latest.rs +++ b/src/handler/latest.rs @@ -12,7 +12,7 @@ use crate::{ ui::popup::{help::HelpPopUpBuilder, info_popup::InfoPopUp, PopUp}, }; -pub fn handle_latest_patchsets( +pub async fn handle_latest_patchsets( app: &mut App, key: KeyEvent, mut terminal: Terminal, @@ -63,7 +63,7 @@ where .as_mut() .unwrap() .increment_page(); - app.fetch_latest_current_page() + app.fetch_latest_current_page().await } }; } @@ -74,8 +74,8 @@ where .as_mut() .unwrap() .decrement_page(); - // Reload from cache (no network call since LoreService caches all pages) - app.fetch_latest_current_page()?; + // Reload from cache (no network call since LoreAPI caches all pages) + app.fetch_latest_current_page().await?; } KeyCode::Enter => { terminal = loading_screen! { diff --git a/src/handler/mail_list.rs b/src/handler/mail_list.rs index a40f895..97fd188 100644 --- a/src/handler/mail_list.rs +++ b/src/handler/mail_list.rs @@ -12,7 +12,7 @@ use crate::{ ui::popup::{help::HelpPopUpBuilder, PopUp}, }; -pub fn handle_mailing_list_selection( +pub async fn handle_mailing_list_selection( app: &mut App, key: KeyEvent, mut terminal: Terminal, @@ -45,7 +45,7 @@ where terminal = loading_screen! { terminal, format!("Fetching patchsets from {}", list_name) => { - let result = app.fetch_latest_current_page(); + let result = app.fetch_latest_current_page().await; if result.is_ok() { app.state.lore.mailing_list_selection.clear_target_list(); app.set_current_screen(CurrentScreen::LatestPatchsets); @@ -59,7 +59,7 @@ where terminal = loading_screen! { terminal, "Refreshing lists" => { - app.refresh_mailing_lists() + app.refresh_mailing_lists().await } }; } diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 45a83fc..411d7d0 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -27,7 +27,7 @@ use edit_config::handle_edit_config; use latest::handle_latest_patchsets; use mail_list::handle_mailing_list_selection; -fn key_handling( +async fn key_handling( mut terminal: Terminal, app: &mut App, key: KeyEvent, @@ -44,7 +44,7 @@ where } else { match app.state.navigation.current_screen { CurrentScreen::MailingListSelection => { - return handle_mailing_list_selection(app, key, terminal); + return handle_mailing_list_selection(app, key, terminal).await; } CurrentScreen::BookmarkedPatchsets => { return handle_bookmarked_patchsets(app, key, terminal); @@ -56,14 +56,17 @@ where handle_edit_config(app, key)?; } CurrentScreen::LatestPatchsets => { - return handle_latest_patchsets(app, key, terminal); + return handle_latest_patchsets(app, key, terminal).await; } } } Ok(ControlFlow::Continue(terminal)) } -fn logic_handling(mut terminal: Terminal, app: &mut App) -> color_eyre::Result> +async fn logic_handling( + mut terminal: Terminal, + app: &mut App, +) -> color_eyre::Result> where B: Backend + Send + 'static, { @@ -78,7 +81,7 @@ where { terminal = loading_screen! { terminal, "Fetching mailing lists" => { - app.refresh_mailing_lists() + app.refresh_mailing_lists().await } }; } @@ -91,7 +94,7 @@ where terminal = loading_screen! { terminal, format!("Fetching patchsets from {}", target_list) => { - app.fetch_latest_current_page() + app.fetch_latest_current_page().await } }; @@ -115,12 +118,12 @@ where Ok(terminal) } -pub fn run_app(mut terminal: Terminal, mut app: App) -> color_eyre::Result<()> +pub async fn run_app(mut terminal: Terminal, mut app: App) -> color_eyre::Result<()> where B: Backend + Send + 'static, { loop { - terminal = logic_handling(terminal, &mut app)?; + terminal = logic_handling(terminal, &mut app).await?; terminal.draw(|f| draw_ui(f, &app.to_view_model()))?; @@ -133,7 +136,7 @@ where if key.kind == KeyEventKind::Release { continue; } - match key_handling(terminal, &mut app, key)? { + match key_handling(terminal, &mut app, key).await? { ControlFlow::Continue(t) => terminal = t, ControlFlow::Break(_) => return Ok(()), } diff --git a/src/lore/application/actor.rs b/src/lore/application/actor.rs index 79acd3f..f63ae6a 100644 --- a/src/lore/application/actor.rs +++ b/src/lore/application/actor.rs @@ -57,12 +57,7 @@ impl LoreApiActor { } => { let result = self .with_core(move |core| { - core.fetch_next_patch_page( - &target_list, - page_size, - page_number, - cache_mode, - ) + core.fetch_next_patch_page(&target_list, page_size, page_number, cache_mode) }) .await .and_then(|result| result); @@ -172,7 +167,10 @@ mod tests { use crate::{ infrastructure::{file_system::MockFileSystemTrait, shell::MockShellTrait}, lore::{ - application::{cache::{CacheMode, CacheTtl}, handle::LoreApiHandle}, + application::{ + cache::{CacheMode, CacheTtl}, + handle::LoreApiHandle, + }, domain::mailing_list::MailingList, infrastructure::{ http_lore_client::{MockFeedGateway, MockListsGateway, MockPatchHtmlGateway}, @@ -245,8 +243,14 @@ mod tests { let handle = spawn_test_actor(make_service(lists_store, MockUserLoreStateStore::new())); - let first = handle.fetch_available_lists(CacheMode::UseCache).await.unwrap(); - let second = handle.fetch_available_lists(CacheMode::UseCache).await.unwrap(); + let first = handle + .fetch_available_lists(CacheMode::UseCache) + .await + .unwrap(); + let second = handle + .fetch_available_lists(CacheMode::UseCache) + .await + .unwrap(); assert_eq!("cached-list", first[0].name()); assert_eq!("cached-list", second[0].name()); diff --git a/src/main.rs b/src/main.rs index 5add7e3..a32ee5d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -172,6 +172,7 @@ async fn main() -> color_eyre::Result<()> { Box::new(OsFileSystem), Box::new(OsShell), Box::new(env), + lore_api, lore_service, render, )?; @@ -183,7 +184,7 @@ async fn main() -> color_eyre::Result<()> { bail!("patch-hub cannot be executed because some dependencies are missing, check logs for more information"); } - run_app(terminal, app)?; + run_app(terminal, app).await?; restore()?; event!(Level::INFO, "patch-hub finished"); From 176cb6bba486dd4c35ce58a052ab51ae12c58223 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 11:00:15 -0300 Subject: [PATCH 5/8] refactor(lore): load patchset details through LoreAPI This commit migrates patchset detail retrieval from the synchronous LoreServiceApi path to LoreApiHandle. App::open_patchset_details is now async, and the latest/bookmarked handlers await it while preserving the existing preview rendering flow. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/app/mod.rs | 7 ++++--- src/handler/bookmarked.rs | 4 ++-- src/handler/latest.rs | 2 +- src/handler/mod.rs | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 7242224..336717d 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -179,7 +179,7 @@ impl App { } /// Loads patchset details into [`LoreUiState::details`]. - pub fn open_patchset_details(&mut self) -> color_eyre::Result { + pub async fn open_patchset_details(&mut self) -> color_eyre::Result { let representative_patch: Patch; let mut is_patchset_bookmarked = true; @@ -214,8 +214,9 @@ impl App { let details = match self .services - .lore - .fetch_patchset_details(&representative_patch, CacheMode::UseCache) + .lore_api + .fetch_patchset_details(representative_patch.clone(), CacheMode::UseCache) + .await { Ok(d) => d, Err(LoreError::PatchNotFound(err)) => return Ok(B4Result::PatchNotFound(err)), diff --git a/src/handler/bookmarked.rs b/src/handler/bookmarked.rs index 2f3eac0..b370e06 100644 --- a/src/handler/bookmarked.rs +++ b/src/handler/bookmarked.rs @@ -12,7 +12,7 @@ use crate::{ ui::popup::{help::HelpPopUpBuilder, info_popup::InfoPopUp, PopUp}, }; -pub fn handle_bookmarked_patchsets( +pub async fn handle_bookmarked_patchsets( app: &mut App, key: KeyEvent, mut terminal: Terminal, @@ -45,7 +45,7 @@ where terminal = loading_screen! { terminal, "Loading patchset" => { - let result = app.open_patchset_details(); + let result = app.open_patchset_details().await; if result.is_ok() { // If a patchset has been bookmarked UI, this means that // b4 was successful in fetching it, so it shouldn't be diff --git a/src/handler/latest.rs b/src/handler/latest.rs index 4169727..f83c066 100644 --- a/src/handler/latest.rs +++ b/src/handler/latest.rs @@ -81,7 +81,7 @@ where terminal = loading_screen! { terminal, "Loading patchset" => { - let result = app.open_patchset_details(); + let result = app.open_patchset_details().await; if result.is_ok() { match result.unwrap() { B4Result::PatchFound => { diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 411d7d0..45550dd 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -47,7 +47,7 @@ where return handle_mailing_list_selection(app, key, terminal).await; } CurrentScreen::BookmarkedPatchsets => { - return handle_bookmarked_patchsets(app, key, terminal); + return handle_bookmarked_patchsets(app, key, terminal).await; } CurrentScreen::PatchsetDetails => { handle_patchset_details(app, key, &mut terminal)?; From a2c822e22521bd907a68d24244ae49ba35891867 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 11:03:38 -0300 Subject: [PATCH 6/8] refactor(lore): route review actions through LoreAPI This commit migrates bookmark persistence, reviewed-state persistence, git signature lookup, and reply command preparation to LoreApiHandle. Patchset action consolidation and the details handler are now async so Lore-owned review data flows through the LoreAPI actor. It also removes the remaining production AppServices.lore injection because App no longer needs to call LoreServiceApi directly. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/app/mod.rs | 54 ++++++++++++++++++---------------- src/handler/details_actions.rs | 6 ++-- src/handler/mod.rs | 2 +- src/main.rs | 18 +----------- 4 files changed, 34 insertions(+), 46 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 336717d..a53f662 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -13,7 +13,7 @@ use tracing::{event, Level}; use std::{ collections::{HashMap, HashSet}, - path::Path, + path::PathBuf, }; use crate::{ @@ -27,7 +27,6 @@ use crate::{ }, lore::{ application::{ - api::LoreServiceApi, cache::{BootstrapLoreData, CacheMode}, errors::LoreError, handle::LoreApiHandle, @@ -50,7 +49,6 @@ pub use view_model::AppViewModel; /// Injected capabilities used by `App` orchestration (not screen state). pub struct AppServices { pub lore_api: LoreApiHandle, - pub lore: Box, pub render: Box, pub shell: Box, pub fs: Box, @@ -84,7 +82,6 @@ impl App { shell: Box, env: Box, lore_api: LoreApiHandle, - lore_service: Box, render: Box, ) -> color_eyre::Result { let config = config_service.snapshot(); @@ -120,7 +117,6 @@ impl App { }, services: AppServices { lore_api, - lore: lore_service, render, shell, fs, @@ -283,14 +279,14 @@ impl App { /// # Panics /// /// Panics if [`LoreUiState::details`] is `None`. - pub fn consolidate_patchset_actions(&mut self) -> color_eyre::Result<()> { - self.sync_patchset_bookmark()?; - self.execute_reviewed_reply()?; + pub async fn consolidate_patchset_actions(&mut self) -> color_eyre::Result<()> { + self.sync_patchset_bookmark().await?; + self.execute_reviewed_reply().await?; self.execute_apply_patchset(); Ok(()) } - fn sync_patchset_bookmark(&mut self) -> color_eyre::Result<()> { + async fn sync_patchset_bookmark(&mut self) -> color_eyre::Result<()> { let details = self.state.lore.details.as_ref().unwrap(); let representative_patch = &details.representative_patch; let patchset_actions = &details.patchset_actions; @@ -308,19 +304,20 @@ impl App { } self.services - .lore - .save_bookmarked_patchsets( - &self - .state + .lore_api + .save_bookmarks( + self.state .user_state .bookmarked_patchsets - .bookmarked_patchsets, + .bookmarked_patchsets + .clone(), ) + .await .map_err(|e| eyre!("{e:#?}"))?; Ok(()) } - fn execute_reviewed_reply(&mut self) -> color_eyre::Result<()> { + async fn execute_reviewed_reply(&mut self) -> color_eyre::Result<()> { let details = self.state.lore.details.as_ref().unwrap(); let representative_patch = details.representative_patch.clone(); let patchset_actions = &details.patchset_actions; @@ -335,7 +332,12 @@ impl App { .remove(&representative_patch.message_id().href) .unwrap_or_default(); - let (git_user_name, git_user_email) = self.services.lore.get_git_signature(""); + let (git_user_name, git_user_email) = self + .services + .lore_api + .get_git_signature(String::new()) + .await + .map_err(|e| eyre!("{e:#?}"))?; if git_user_name.is_empty() || git_user_email.is_empty() { println!("`git config user.name` or `git config user.email` not set\nAborting..."); @@ -350,20 +352,21 @@ impl App { .map_err(|e| eyre!("invalid utf-8 in temp dir path: {}", e))? .trim() .to_string(); - let tmp_dir = Path::new(&tmp_dir_str); + let tmp_dir = PathBuf::from(tmp_dir_str); let git_signature = format!("{git_user_name} <{git_user_email}>"); let git_reply_commands = self .services - .lore + .lore_api .prepare_reply_commands( tmp_dir, - "all", - &raw_patches, - &patches_to_reply, - &git_signature, - self.state.config.git_send_email_options(), + "all".to_string(), + raw_patches, + patches_to_reply.clone(), + git_signature, + self.state.config.git_send_email_options().to_string(), ) + .await .map_err(|e| eyre!("{e:#?}"))?; let reply_indexes: Vec = patches_to_reply @@ -389,8 +392,9 @@ impl App { ); self.services - .lore - .save_reviewed_patchsets(&self.state.user_state.reviewed_patchsets) + .lore_api + .save_reviewed(self.state.user_state.reviewed_patchsets.clone()) + .await .map_err(|e| eyre!("{e:#?}"))?; self.state diff --git a/src/handler/details_actions.rs b/src/handler/details_actions.rs index 0d3fdaf..4d55c11 100644 --- a/src/handler/details_actions.rs +++ b/src/handler/details_actions.rs @@ -14,7 +14,7 @@ use crate::{ use super::wait_key_press; -pub fn handle_patchset_details( +pub async fn handle_patchset_details( app: &mut App, key: KeyEvent, terminal: &mut Terminal, @@ -109,7 +109,7 @@ pub fn handle_patchset_details( KeyCode::Enter => { if patchset_details_and_actions.actions_require_user_io() { setup_user_io(terminal)?; - app.consolidate_patchset_actions()?; + app.consolidate_patchset_actions().await?; println!("\nPress ENTER continue..."); loop { if let Event::Key(key) = event::read()? { @@ -120,7 +120,7 @@ pub fn handle_patchset_details( } teardown_user_io(terminal)?; } else { - app.consolidate_patchset_actions()?; + app.consolidate_patchset_actions().await?; } app.set_current_screen(CurrentScreen::PatchsetDetails); } diff --git a/src/handler/mod.rs b/src/handler/mod.rs index 45550dd..3d00151 100644 --- a/src/handler/mod.rs +++ b/src/handler/mod.rs @@ -50,7 +50,7 @@ where return handle_bookmarked_patchsets(app, key, terminal).await; } CurrentScreen::PatchsetDetails => { - handle_patchset_details(app, key, &mut terminal)?; + handle_patchset_details(app, key, &mut terminal).await?; } CurrentScreen::EditConfig => { handle_edit_config(app, key)?; diff --git a/src/main.rs b/src/main.rs index a32ee5d..b52a2dc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -24,9 +24,7 @@ use infrastructure::{ terminal::{init, restore}, }; use lore::{ - application::{ - actor::LoreApiActor, api::LoreServiceApi, cache::CacheTtl, service::LoreService, - }, + application::{actor::LoreApiActor, cache::CacheTtl, service::LoreService}, infrastructure::{ http_lore_client::HttpLoreGateway, patchset_fetcher::B4PatchsetFetcher, @@ -153,19 +151,6 @@ async fn main() -> color_eyre::Result<()> { )); let bootstrap = lore_api.get_bootstrap_data().await.unwrap_or_default(); - let lore_service: Box = Box::new(LoreService::new( - gateway.clone(), - gateway.clone(), - gateway.clone(), - persistence.clone() as Arc, - persistence.clone() as Arc, - fetcher, - parser, - fs_arc, - shell_arc, - CacheTtl::default(), - )); - let app = App::new( config_service, bootstrap, @@ -173,7 +158,6 @@ async fn main() -> color_eyre::Result<()> { Box::new(OsShell), Box::new(env), lore_api, - lore_service, render, )?; if !check_external_deps(&*app.services.env, &app.state.config) { From 7cf36f0aa8409fdbbbb68c4e8d703a2a98aef216 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 11:15:24 -0300 Subject: [PATCH 7/8] refactor(lore): remove legacy LoreServiceApi boundary This commit removes the old LoreServiceApi trait and MockLoreServiceApi export now that App no longer calls LoreService directly. LoreService becomes the internal stateful core used by LoreApiActor, with the application-facing Lore boundary provided by LoreApiHandle. This commit is part of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/lore/application/actor.rs | 3 +- src/lore/application/api.rs | 113 -------------------------------- src/lore/application/cache.rs | 2 +- src/lore/application/handle.rs | 2 +- src/lore/application/mod.rs | 5 -- src/lore/application/service.rs | 26 ++++---- 6 files changed, 17 insertions(+), 134 deletions(-) delete mode 100644 src/lore/application/api.rs diff --git a/src/lore/application/actor.rs b/src/lore/application/actor.rs index f63ae6a..0e00991 100644 --- a/src/lore/application/actor.rs +++ b/src/lore/application/actor.rs @@ -1,8 +1,7 @@ use tokio::{sync::mpsc, task}; use crate::lore::application::{ - api::LoreServiceApi, errors::LoreError, handle::LoreApiHandle, messages::LoreApiMessage, - service::LoreService, + errors::LoreError, handle::LoreApiHandle, messages::LoreApiMessage, service::LoreService, }; pub const DEFAULT_LORE_API_CHANNEL_SIZE: usize = 32; diff --git a/src/lore/application/api.rs b/src/lore/application/api.rs deleted file mode 100644 index 7c51ea0..0000000 --- a/src/lore/application/api.rs +++ /dev/null @@ -1,113 +0,0 @@ -use mockall::automock; - -use std::{ - collections::{HashMap, HashSet}, - path::Path, -}; - -use crate::{ - infrastructure::shell::ShellCommand, - lore::{ - application::{ - cache::{BootstrapLoreData, CacheMode}, - dto::PatchsetDetails, - errors::LoreError, - }, - domain::{mailing_list::MailingList, patch::Patch}, - }, -}; - -/// Single entry-point for all Lore-domain operations consumed by the UI. -/// -/// Implemented synchronously in [`super::service::LoreService`]. The trait -/// boundary makes it trivial to swap in a mock for tests or an async actor -/// implementation in a later phase. -#[automock] -pub trait LoreServiceApi { - // ── Mailing lists ───────────────────────────────────────────────────────── - - /// Return available mailing lists according to `mode`: - /// - /// * `UseCache` — in-memory hit → disk fallback → error (no network) - /// * `Refresh` — unconditionally fetch from the network, persist, update cache - /// * `Bypass` — fetch from the network without reading or writing cache - fn fetch_available_lists(&mut self, mode: CacheMode) -> Result, LoreError>; - - // ── User state ──────────────────────────────────────────────────────────── - - fn load_bookmarked_patchsets(&self) -> Result, LoreError>; - fn save_bookmarked_patchsets(&self, patchsets: &[Patch]) -> Result<(), LoreError>; - - fn load_reviewed_patchsets(&self) -> Result>, LoreError>; - fn save_reviewed_patchsets( - &self, - reviewed: &HashMap>, - ) -> Result<(), LoreError>; - - // ── Feed pagination ─────────────────────────────────────────────────────── - - /// Return the patches for `page_number` (1-based) of `target_list`. - /// - /// Internally fetches more feed pages from the network as needed. - /// Returns [`LoreError::EndOfFeed`] when the list is exhausted. - /// - /// * `UseCache` — return from the in-memory index if not stale and already - /// large enough; otherwise extend the index from the network. - /// * `Refresh` — evict the cached index first, then fetch from the network. - /// * `Bypass` — fetch from the network; the result is still accumulated - /// in the in-memory index for subsequent pagination requests. - fn fetch_next_patch_page( - &mut self, - target_list: &str, - page_size: usize, - page_number: usize, - mode: CacheMode, - ) -> Result, LoreError>; - - // ── Patchset details ────────────────────────────────────────────────────── - - /// Download and parse `representative_patch`, returning the full patchset - /// data needed to populate the details screen. - /// - /// * `UseCache` — return from in-memory cache if present and not stale. - /// * `Refresh` — evict the cached entry first, then re-download. - /// * `Bypass` — download and return without reading or writing cache. - fn fetch_patchset_details( - &mut self, - representative_patch: &Patch, - mode: CacheMode, - ) -> Result; - - // ── Reply commands ──────────────────────────────────────────────────────── - - /// Build the `git send-email` commands required to reply to the selected - /// patches with a `Reviewed-by` trailer. - /// - /// Files are written under `tmp_dir`; the caller is responsible for - /// spawning the returned commands interactively. - fn prepare_reply_commands( - &self, - tmp_dir: &Path, - target_list: &str, - patches: &[String], - patches_to_reply: &[bool], - git_signature: &str, - git_send_email_options: &str, - ) -> Result, LoreError>; - - // ── Git helpers ─────────────────────────────────────────────────────────── - - /// Return `(user.name, user.email)` from `git config` for the given repo - /// path. Pass an empty string to use the global git config. - fn get_git_signature(&self, git_repo_path: &str) -> (String, String); - - // ── Bootstrap ───────────────────────────────────────────────────────────── - - /// Warm the bootstrap cache and return all data needed to initialise `App`. - /// - /// Internally calls `fetch_available_lists(UseCache)`, - /// `load_bookmarked_patchsets`, and `load_reviewed_patchsets`. Each - /// failure is logged and replaced with an empty default so that the caller - /// can treat this method as infallible in practice. - fn warm_bootstrap_cache(&mut self) -> Result; -} diff --git a/src/lore/application/cache.rs b/src/lore/application/cache.rs index 373d1b9..f1f40c2 100644 --- a/src/lore/application/cache.rs +++ b/src/lore/application/cache.rs @@ -182,7 +182,7 @@ impl Default for LoreCache { // ── Bootstrap result ────────────────────────────────────────────────────────── -/// Data returned by `LoreServiceApi::warm_bootstrap_cache`, used to initialise +/// Data returned by `LoreService::warm_bootstrap_cache`, used to initialise /// `App` without it knowing about persistence paths or network policy. #[derive(Default)] pub struct BootstrapLoreData { diff --git a/src/lore/application/handle.rs b/src/lore/application/handle.rs index 96fadbc..727cfc1 100644 --- a/src/lore/application/handle.rs +++ b/src/lore/application/handle.rs @@ -1,4 +1,4 @@ -#![allow(dead_code)] // Follow-up commits replace remaining synchronous LoreServiceApi usage. +#![allow(dead_code)] // Some protocol methods are reserved for future App flows. use std::{ collections::{HashMap, HashSet}, diff --git a/src/lore/application/mod.rs b/src/lore/application/mod.rs index a0cfc37..4b0a1ac 100644 --- a/src/lore/application/mod.rs +++ b/src/lore/application/mod.rs @@ -1,12 +1,7 @@ pub mod actor; -pub mod api; pub mod cache; pub mod dto; pub mod errors; pub mod handle; pub mod messages; pub mod service; - -#[cfg(test)] -#[allow(unused_imports)] -pub use api::MockLoreServiceApi; diff --git a/src/lore/application/service.rs b/src/lore/application/service.rs index 2f858ac..bf51832 100644 --- a/src/lore/application/service.rs +++ b/src/lore/application/service.rs @@ -13,7 +13,6 @@ use crate::{ }, lore::{ application::{ - api::LoreServiceApi, cache::{ BootstrapLoreData, CacheMode, CacheTtl, FeedCacheEntry, LoreCache, MailingListsCacheEntry, PatchsetCacheEntry, PatchsetCacheKey, @@ -79,8 +78,11 @@ impl LoreService { } } -impl LoreServiceApi for LoreService { - fn fetch_available_lists(&mut self, mode: CacheMode) -> Result, LoreError> { +impl LoreService { + pub fn fetch_available_lists( + &mut self, + mode: CacheMode, + ) -> Result, LoreError> { const LORE_PAGE_SIZE: usize = 200; match mode { @@ -146,26 +148,26 @@ impl LoreServiceApi for LoreService { Ok(all_lists) } - fn load_bookmarked_patchsets(&self) -> Result, LoreError> { + pub fn load_bookmarked_patchsets(&self) -> Result, LoreError> { Ok(self.user_state.load_bookmarked_patchsets()?) } - fn save_bookmarked_patchsets(&self, patchsets: &[Patch]) -> Result<(), LoreError> { + pub fn save_bookmarked_patchsets(&self, patchsets: &[Patch]) -> Result<(), LoreError> { Ok(self.user_state.save_bookmarked_patchsets(patchsets)?) } - fn load_reviewed_patchsets(&self) -> Result>, LoreError> { + pub fn load_reviewed_patchsets(&self) -> Result>, LoreError> { Ok(self.user_state.load_reviewed_patchsets()?) } - fn save_reviewed_patchsets( + pub fn save_reviewed_patchsets( &self, reviewed: &HashMap>, ) -> Result<(), LoreError> { Ok(self.user_state.save_reviewed_patchsets(reviewed)?) } - fn fetch_next_patch_page( + pub fn fetch_next_patch_page( &mut self, target_list: &str, page_size: usize, @@ -246,7 +248,7 @@ impl LoreServiceApi for LoreService { } } - fn fetch_patchset_details( + pub fn fetch_patchset_details( &mut self, representative_patch: &Patch, mode: CacheMode, @@ -312,7 +314,7 @@ impl LoreServiceApi for LoreService { }) } - fn prepare_reply_commands( + pub fn prepare_reply_commands( &self, tmp_dir: &Path, target_list: &str, @@ -360,7 +362,7 @@ impl LoreServiceApi for LoreService { Ok(commands) } - fn warm_bootstrap_cache(&mut self) -> Result { + pub fn warm_bootstrap_cache(&mut self) -> Result { let mailing_lists = self .fetch_available_lists(CacheMode::UseCache) .unwrap_or_else(|e| { @@ -382,7 +384,7 @@ impl LoreServiceApi for LoreService { }) } - fn get_git_signature(&self, git_repo_path: &str) -> (String, String) { + pub fn get_git_signature(&self, git_repo_path: &str) -> (String, String) { let mut name_args = vec!["config".to_string(), "user.name".to_string()]; let mut email_args = vec!["config".to_string(), "user.email".to_string()]; From 333509dedd7142187fe7dd892b4552685174cc55 Mon Sep 17 00:00:00 2001 From: lorenzoberts Date: Sun, 31 May 2026 11:31:52 -0300 Subject: [PATCH 8/8] refactor(lore): instrument LoreAPI actor requests This commit adds structured tracing around the LoreAPI actor lifecycle and message handling. Each LoreApiMessage now has a stable request name used in logs, and actor replies log Lore failures or dropped response receivers. This commit is the last one of the architecture's refactoring phase 6. Signed-off-by: lorenzoberts --- src/lore/application/actor.rs | 80 +++++++++++++++++++++++++++----- src/lore/application/messages.rs | 17 +++++++ 2 files changed, 86 insertions(+), 11 deletions(-) diff --git a/src/lore/application/actor.rs b/src/lore/application/actor.rs index 0e00991..aa4c8ab 100644 --- a/src/lore/application/actor.rs +++ b/src/lore/application/actor.rs @@ -1,4 +1,7 @@ -use tokio::{sync::mpsc, task}; +use tokio::{ + sync::{mpsc, oneshot}, + task, +}; use crate::lore::application::{ errors::LoreError, handle::LoreApiHandle, messages::LoreApiMessage, service::LoreService, @@ -21,31 +24,42 @@ impl LoreApiActor { pub fn spawn(core: LoreService) -> LoreApiHandle { let (tx, rx) = mpsc::channel(DEFAULT_LORE_API_CHANNEL_SIZE); + tracing::debug!( + channel_size = DEFAULT_LORE_API_CHANNEL_SIZE, + "spawning lore api actor" + ); tokio::spawn(Self::new(core, rx).run()); LoreApiHandle::new(tx) } pub async fn run(mut self) { + tracing::info!("lore api actor started"); while let Some(message) = self.rx.recv().await { self.handle_message(message).await; } + tracing::info!("lore api actor stopped"); } async fn handle_message(&mut self, message: LoreApiMessage) { + let message_name = message.name(); + tracing::debug!(message = message_name, "lore api request received"); + match message { LoreApiMessage::GetBootstrapData { reply } => { + tracing::debug!("loading lore bootstrap data"); let result = self .with_core(|core| core.warm_bootstrap_cache()) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::FetchAvailableLists { cache_mode, reply } => { + tracing::debug!(?cache_mode, "fetching available mailing lists"); let result = self .with_core(move |core| core.fetch_available_lists(cache_mode)) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::FetchFeedPage { target_list, @@ -54,63 +68,80 @@ impl LoreApiActor { cache_mode, reply, } => { + tracing::debug!( + list = %target_list, + page_size, + page_number, + ?cache_mode, + "fetching lore feed page" + ); let result = self .with_core(move |core| { core.fetch_next_patch_page(&target_list, page_size, page_number, cache_mode) }) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::FetchPatchsetDetails { representative_patch, cache_mode, reply, } => { + tracing::debug!( + message_id = %representative_patch.message_id().href, + ?cache_mode, + "fetching patchset details" + ); let result = self .with_core(move |core| { core.fetch_patchset_details(&representative_patch, cache_mode) }) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::LoadBookmarks { reply } => { + tracing::debug!("loading bookmarked patchsets"); let result = self .with_core(|core| core.load_bookmarked_patchsets()) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::SaveBookmarks { bookmarks, reply } => { + tracing::debug!(count = bookmarks.len(), "saving bookmarked patchsets"); let result = self .with_core(move |core| core.save_bookmarked_patchsets(&bookmarks)) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::LoadReviewed { reply } => { + tracing::debug!("loading reviewed patchsets"); let result = self .with_core(|core| core.load_reviewed_patchsets()) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::SaveReviewed { reviewed, reply } => { + tracing::debug!(patchsets = reviewed.len(), "saving reviewed patchsets"); let result = self .with_core(move |core| core.save_reviewed_patchsets(&reviewed)) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::GetGitSignature { git_repo_path, reply, } => { + tracing::debug!(repo = %git_repo_path, "loading git signature"); let result = self .with_core(move |core| core.get_git_signature(&git_repo_path)) .await; - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } LoreApiMessage::PrepareReplyCommands { tmp_dir, @@ -121,6 +152,12 @@ impl LoreApiActor { git_send_email_options, reply, } => { + tracing::debug!( + target_list = %target_list, + patch_count = patches.len(), + selected_count = patches_to_reply.iter().filter(|selected| **selected).count(), + "preparing reply commands" + ); let result = self .with_core(move |core| { core.prepare_reply_commands( @@ -134,7 +171,7 @@ impl LoreApiActor { }) .await .and_then(|result| result); - let _ = reply.send(result); + send_lore_reply(message_name, reply, result); } } } @@ -159,6 +196,27 @@ impl LoreApiActor { } } +fn send_lore_reply( + message_name: &'static str, + reply: oneshot::Sender>, + result: Result, +) { + if let Err(error) = &result { + tracing::warn!( + message = message_name, + error = %error, + "lore api request failed" + ); + } + + if reply.send(result).is_err() { + tracing::warn!( + message = message_name, + "lore api reply receiver dropped before response" + ); + } +} + #[cfg(test)] mod tests { use std::{collections::HashMap, sync::Arc}; diff --git a/src/lore/application/messages.rs b/src/lore/application/messages.rs index 4b5cf6d..6926c43 100644 --- a/src/lore/application/messages.rs +++ b/src/lore/application/messages.rs @@ -69,3 +69,20 @@ pub enum LoreApiMessage { reply: oneshot::Sender>>, }, } + +impl LoreApiMessage { + pub fn name(&self) -> &'static str { + match self { + LoreApiMessage::GetBootstrapData { .. } => "GetBootstrapData", + LoreApiMessage::FetchAvailableLists { .. } => "FetchAvailableLists", + LoreApiMessage::FetchFeedPage { .. } => "FetchFeedPage", + LoreApiMessage::FetchPatchsetDetails { .. } => "FetchPatchsetDetails", + LoreApiMessage::LoadBookmarks { .. } => "LoadBookmarks", + LoreApiMessage::SaveBookmarks { .. } => "SaveBookmarks", + LoreApiMessage::LoadReviewed { .. } => "LoadReviewed", + LoreApiMessage::SaveReviewed { .. } => "SaveReviewed", + LoreApiMessage::GetGitSignature { .. } => "GetGitSignature", + LoreApiMessage::PrepareReplyCommands { .. } => "PrepareReplyCommands", + } + } +}