From e2813392ea3ed36525d94b0acbfaa20d20cc955c Mon Sep 17 00:00:00 2001 From: Michel de Graaf Date: Mon, 27 Jul 2026 09:48:46 +0200 Subject: [PATCH] refactor(api): put a SessionApi seam between the commands, the TUI, and the client Commands and the TUI now hold an Arc instead of an Arc. The trait is implemented on Client itself, so the local path is the same client behind one vtable and no wrapper type exists to keep in sync. Daemon mode plugs a remote implementation into the same handle. --- soulseek-rs/src/api.rs | 295 ++++++++++++++++++++++++++ soulseek-rs/src/commands/mod.rs | 12 +- soulseek-rs/src/commands/peer.rs | 6 +- soulseek-rs/src/commands/social.rs | 13 +- soulseek-rs/src/commands/transfer.rs | 15 +- soulseek-rs/src/commands/wish.rs | 12 +- soulseek-rs/src/lib.rs | 1 + soulseek-rs/src/main.rs | 1 + soulseek-rs/src/ui/main_tui/mod.rs | 8 +- soulseek-rs/src/ui/main_tui/render.rs | 2 +- 10 files changed, 336 insertions(+), 29 deletions(-) create mode 100644 soulseek-rs/src/api.rs diff --git a/soulseek-rs/src/api.rs b/soulseek-rs/src/api.rs new file mode 100644 index 0000000..585375a --- /dev/null +++ b/soulseek-rs/src/api.rs @@ -0,0 +1,295 @@ +//! The seam between "what this program does" and "where the session lives". +//! +//! Commands and the TUI hold an `Arc` rather than an +//! `Arc`. Locally that *is* the client — the trait is implemented on +//! `Client` itself, so an `Arc` coerces straight to the trait object +//! and the local path keeps calling the same methods it always did. In daemon +//! mode the same handle is a [`crate::remote::RemoteSession`] that answers over +//! a socket. +//! +//! The signatures deliberately mirror `Client`'s, because every one of them is +//! already load-bearing somewhere in the UI or the command layer. The one +//! change is [`SessionApi::username`], which returns an owned `String`: a +//! remote session has no borrowed client to hand a `&str` out of. + +use soulseek_rs::types::{Download, DownloadMetadata}; +use soulseek_rs::{ + Client, DownloadStatus, Result, RoomEvent, SearchResult, SessionLoss, + SharedDirectory, UploadInfo, UserInfo, UserMessage, +}; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::mpsc::Receiver; +use std::time::Duration; + +/// Everything the command layer and the TUI ask of a logged-in session. +/// +/// Two rules keep the remote implementation honest: +/// +/// * Non-destructive getters (`uploads`, `room_members`, `user_info`, …) are +/// plain request/response — a caller may poll them as often as it likes. +/// * The `take_*` drains are destructive, so with more than one client +/// attached they cannot be served by asking the daemon to drain on demand: +/// whoever asked first would steal the events. The daemon drains once and +/// fans the events out, and a remote session's `take_*` empties its own +/// local mirror. +pub trait SessionApi: Send + Sync { + fn username(&self) -> String; + fn listen_port(&self) -> Option; + fn session_loss(&self) -> Option; + + fn search( + &self, + query: &str, + timeout: Duration, + ) -> Result>; + fn search_with_cancel( + &self, + query: &str, + timeout: Duration, + cancel: Option>, + ) -> Result>; + fn get_search_results(&self, key: &str) -> Vec; + fn get_search_results_count(&self, key: &str) -> usize; + fn try_get_search_results(&self, key: &str) -> Option>; + fn start_wishlist_search(&self, query: &str) -> Result<()>; + fn wishlist_interval(&self) -> Duration; + + fn download( + &self, + filename: String, + username: String, + size: u64, + download_directory: String, + ) -> Result<(Download, Receiver)>; + fn download_with_metadata( + &self, + filename: String, + username: String, + size: u64, + download_directory: String, + metadata: DownloadMetadata, + ) -> Result<(Download, Receiver)>; + fn get_all_downloads(&self) -> Vec; + fn pause_download(&self, username: &str, filename: &str) -> bool; + fn resume_download(&self, username: &str, filename: &str) -> bool; + fn remove_queued_download(&self, username: &str, filename: &str) -> bool; + fn remove_download(&self, username: &str, filename: &str) -> bool; + + fn uploads(&self) -> Vec; + fn take_upload_events(&self) -> Vec; + fn cancel_upload(&self, username: &str, filename: &str) -> bool; + fn set_upload_slots(&self, slots: usize); + fn check_privileges(&self) -> Result<()>; + fn own_privilege_seconds(&self) -> Option; + + fn request_room_list(&self) -> Result<()>; + fn join_room(&self, room: &str) -> Result<()>; + fn leave_room(&self, room: &str) -> Result<()>; + fn say_in_room(&self, room: &str, message: &str) -> Result<()>; + fn room_members(&self, room: &str) -> Vec; + fn take_room_events(&self) -> Vec; + + fn send_private_message(&self, username: &str, message: &str) -> Result<()>; + fn take_private_messages(&self) -> Vec; + + fn browse_user(&self, username: &str) -> Result<()>; + fn take_browse_result( + &self, + username: &str, + ) -> Option>; + fn request_user_info(&self, username: &str) -> Result<()>; + fn user_info(&self, username: &str) -> Option; + + fn shared_counts(&self) -> (u32, u32); + fn shared_directories(&self) -> Vec; + fn set_shared_directories(&self, directories: Vec) -> Result<()>; +} + +/// The local session: the library client itself, so `Arc` is already an +/// `Arc` and the in-process path costs nothing but a vtable. +impl SessionApi for Client { + fn username(&self) -> String { + Self::username(self).to_string() + } + + fn listen_port(&self) -> Option { + Self::listen_port(self) + } + + fn session_loss(&self) -> Option { + Self::session_loss(self) + } + + fn search( + &self, + query: &str, + timeout: Duration, + ) -> Result> { + Self::search(self, query, timeout) + } + + fn search_with_cancel( + &self, + query: &str, + timeout: Duration, + cancel: Option>, + ) -> Result> { + Self::search_with_cancel(self, query, timeout, cancel) + } + + fn get_search_results(&self, key: &str) -> Vec { + Self::get_search_results(self, key) + } + + fn get_search_results_count(&self, key: &str) -> usize { + Self::get_search_results_count(self, key) + } + + fn try_get_search_results(&self, key: &str) -> Option> { + Self::try_get_search_results(self, key) + } + + fn start_wishlist_search(&self, query: &str) -> Result<()> { + Self::start_wishlist_search(self, query) + } + + fn wishlist_interval(&self) -> Duration { + Self::wishlist_interval(self) + } + + fn download( + &self, + filename: String, + username: String, + size: u64, + download_directory: String, + ) -> Result<(Download, Receiver)> { + Self::download(self, filename, username, size, download_directory) + } + + fn download_with_metadata( + &self, + filename: String, + username: String, + size: u64, + download_directory: String, + metadata: DownloadMetadata, + ) -> Result<(Download, Receiver)> { + Self::download_with_metadata( + self, + filename, + username, + size, + download_directory, + metadata, + ) + } + + fn get_all_downloads(&self) -> Vec { + Self::get_all_downloads(self) + } + + fn pause_download(&self, username: &str, filename: &str) -> bool { + Self::pause_download(self, username, filename) + } + + fn resume_download(&self, username: &str, filename: &str) -> bool { + Self::resume_download(self, username, filename) + } + + fn remove_queued_download(&self, username: &str, filename: &str) -> bool { + Self::remove_queued_download(self, username, filename) + } + + fn remove_download(&self, username: &str, filename: &str) -> bool { + Self::remove_download(self, username, filename) + } + + fn uploads(&self) -> Vec { + Self::uploads(self) + } + + fn take_upload_events(&self) -> Vec { + Self::take_upload_events(self) + } + + fn cancel_upload(&self, username: &str, filename: &str) -> bool { + Self::cancel_upload(self, username, filename) + } + + fn set_upload_slots(&self, slots: usize) { + Self::set_upload_slots(self, slots); + } + + fn check_privileges(&self) -> Result<()> { + Self::check_privileges(self) + } + + fn own_privilege_seconds(&self) -> Option { + Self::own_privilege_seconds(self) + } + + fn request_room_list(&self) -> Result<()> { + Self::request_room_list(self) + } + + fn join_room(&self, room: &str) -> Result<()> { + Self::join_room(self, room) + } + + fn leave_room(&self, room: &str) -> Result<()> { + Self::leave_room(self, room) + } + + fn say_in_room(&self, room: &str, message: &str) -> Result<()> { + Self::say_in_room(self, room, message) + } + + fn room_members(&self, room: &str) -> Vec { + Self::room_members(self, room) + } + + fn take_room_events(&self) -> Vec { + Self::take_room_events(self) + } + + fn send_private_message(&self, username: &str, message: &str) -> Result<()> { + Self::send_private_message(self, username, message) + } + + fn take_private_messages(&self) -> Vec { + Self::take_private_messages(self) + } + + fn browse_user(&self, username: &str) -> Result<()> { + Self::browse_user(self, username) + } + + fn take_browse_result( + &self, + username: &str, + ) -> Option> { + Self::take_browse_result(self, username) + } + + fn request_user_info(&self, username: &str) -> Result<()> { + Self::request_user_info(self, username) + } + + fn user_info(&self, username: &str) -> Option { + Self::user_info(self, username) + } + + fn shared_counts(&self) -> (u32, u32) { + Self::shared_counts(self) + } + + fn shared_directories(&self) -> Vec { + Self::shared_directories(self) + } + + fn set_shared_directories(&self, directories: Vec) -> Result<()> { + Self::set_shared_directories(self, directories) + } +} diff --git a/soulseek-rs/src/commands/mod.rs b/soulseek-rs/src/commands/mod.rs index a28240f..f0ffa83 100644 --- a/soulseek-rs/src/commands/mod.rs +++ b/soulseek-rs/src/commands/mod.rs @@ -18,6 +18,7 @@ use crate::output::{CliError, CliResult, Exit, Out, PortmapRecord}; use crate::port_mapping::{self, PortMapper}; use soulseek_rs::types::{RoomEvent, RoomInfo}; use soulseek_rs::{Client, ClientSettings, SessionLoss}; +use crate::api::SessionApi; use std::net::{SocketAddr, TcpStream, ToSocketAddrs}; use std::sync::{Arc, OnceLock}; use std::time::{Duration, Instant}; @@ -31,7 +32,7 @@ pub struct Ctx { pub search_timeout: Duration, /// The client of the session this command opened, once it has one, so the /// verdict can be checked against the session that produced it. - pub session: OnceLock>, + pub session: OnceLock>, } impl Ctx { @@ -75,7 +76,7 @@ fn session_lost(loss: SessionLoss) -> CliError { /// A logged-in client plus, while the listener is enabled, the port mapping /// that makes us reachable. Both live until the command ends. pub struct Session { - pub client: Arc, + pub client: Arc, _mapper: Option, } @@ -90,7 +91,8 @@ impl Session { ctx.out.status(&format!("connecting to {address}…")); reachable(&address, REACH_TIMEOUT)?; - let client = Arc::new(login(ctx.settings.clone(), &address)?); + let client: Arc = + Arc::new(login(ctx.settings.clone(), &address)?); ctx.out .status(&format!("logged in as {}", ctx.settings.username)); let _ = ctx.session.set(Arc::clone(&client)); @@ -184,7 +186,7 @@ fn login(settings: ClientSettings, address: &str) -> CliResult { /// Events already queued are discarded first: servers push a room list at /// login, and counting that one would answer a question we had not asked yet. pub fn await_room_list( - client: &Client, + client: &dyn SessionApi, timeout: Duration, poll: Duration, ) -> Option> { @@ -215,7 +217,7 @@ pub fn await_room_list( /// message reached the socket. The room list is the cheapest such request and /// every server answers it, which lets `message send` and `room say` report /// delivery honestly instead of sleeping and hoping. -pub fn confirm_flush(client: &Client, timeout: Duration) -> bool { +pub fn confirm_flush(client: &dyn SessionApi, timeout: Duration) -> bool { await_room_list(client, timeout, Duration::from_millis(50)).is_some() } diff --git a/soulseek-rs/src/commands/peer.rs b/soulseek-rs/src/commands/peer.rs index f9b19ad..dd06f56 100644 --- a/soulseek-rs/src/commands/peer.rs +++ b/soulseek-rs/src/commands/peer.rs @@ -31,7 +31,7 @@ pub fn whoami(ctx: &Ctx) -> CliResult { shared_folders: folders, shared_files: files, download_dir: ctx.download_dir.clone(), - privilege_seconds: own_privileges(&session.client), + privilege_seconds: own_privileges(session.client.as_ref()), }); Ok(()) } @@ -40,7 +40,7 @@ pub fn whoami(ctx: &Ctx) -> CliResult { /// /// A server that does not answer leaves the field null rather than reporting a /// zero, because "no privileges" and "did not say" are different facts. -fn own_privileges(client: &soulseek_rs::Client) -> Option { +fn own_privileges(client: &dyn crate::api::SessionApi) -> Option { if client.check_privileges().is_err() { return None; } @@ -159,7 +159,7 @@ pub fn serve( } } if let Some(sweeper) = sweeper.as_mut() { - sweeper.tick(ctx, &session.client); + sweeper.tick(ctx, session.client.as_ref()); } std::thread::sleep(POLL); } diff --git a/soulseek-rs/src/commands/social.rs b/soulseek-rs/src/commands/social.rs index 435a137..613581a 100644 --- a/soulseek-rs/src/commands/social.rs +++ b/soulseek-rs/src/commands/social.rs @@ -8,7 +8,8 @@ use crate::output::{ RoomEventRecord, RoomMemberRecord, RoomRecord, }; use soulseek_rs::types::RoomEvent; -use soulseek_rs::{Client, SharedDirectory}; +use crate::api::SessionApi; +use soulseek_rs::SharedDirectory; use std::time::{Duration, Instant}; /// How long to wait for the server to acknowledge something we sent. @@ -30,7 +31,7 @@ pub fn full_path(directory: &str, basename: &str) -> String { /// Ask a peer for its share listing and wait for the answer. pub fn fetch_listing( - client: &Client, + client: &dyn SessionApi, user: &str, timeout: Duration, ) -> CliResult> { @@ -57,7 +58,7 @@ pub fn browse(ctx: &Ctx, args: &BrowseArgs) -> CliResult { .status(&format!("requesting {}'s shared files…", args.user)); let directories = fetch_listing( - &session.client, + session.client.as_ref(), &args.user, Duration::from_secs(args.timeout), )?; @@ -89,7 +90,7 @@ pub fn room_list(ctx: &Ctx, timeout: Duration) -> CliResult { ctx.out.status("fetching the room list…"); let Some(mut rooms) = - super::await_room_list(&session.client, timeout, POLL) + super::await_room_list(session.client.as_ref(), timeout, POLL) else { return Err(CliError::timeout(format!( "the server sent no room list within {}s", @@ -123,7 +124,7 @@ pub fn room_say(ctx: &Ctx, room: &str, message: &str) -> CliResult { CliError::connection(format!("cannot post to {room}: {e}")) })?; - if confirm_flush(&session.client, FLUSH_TIMEOUT) { + if confirm_flush(session.client.as_ref(), FLUSH_TIMEOUT) { ctx.out.status(&format!("posted to {room}")); return Ok(()); } @@ -248,7 +249,7 @@ pub fn message_send(ctx: &Ctx, user: &str, message: &str) -> CliResult { CliError::connection(format!("cannot send to {user}: {e}")) })?; - if confirm_flush(&session.client, FLUSH_TIMEOUT) { + if confirm_flush(session.client.as_ref(), FLUSH_TIMEOUT) { ctx.out.status(&format!("sent to {user}")); return Ok(()); } diff --git a/soulseek-rs/src/commands/transfer.rs b/soulseek-rs/src/commands/transfer.rs index 6200b5b..67d9b36 100644 --- a/soulseek-rs/src/commands/transfer.rs +++ b/soulseek-rs/src/commands/transfer.rs @@ -8,7 +8,8 @@ use crate::output::{ CliError, CliResult, DownloadRecord, Exit, Out, SearchRecord, }; use soulseek_rs::utils::path::expand_tilde; -use soulseek_rs::{Client, DownloadStatus, SearchResult}; +use crate::api::SessionApi; +use soulseek_rs::{DownloadStatus, SearchResult}; use std::collections::{HashMap, VecDeque}; use std::io::BufRead; use std::path::PathBuf; @@ -241,7 +242,7 @@ pub fn download(ctx: &Ctx, args: &DownloadArgs) -> CliResult { ensure_download_dir(&ctx.download_dir)?; let session = Session::open(ctx)?; - let requests = resolve_sizes(ctx, &session.client, requests)?; + let requests = resolve_sizes(ctx, session.client.as_ref(), requests)?; download_all( ctx, &session.client, @@ -283,7 +284,7 @@ pub fn get(ctx: &Ctx, args: &GetArgs) -> CliResult { /// late responders are often the better sources. fn collect( ctx: &Ctx, - client: &Arc, + client: &Arc, query: &str, stop_early: bool, ) -> CliResult> { @@ -365,7 +366,7 @@ fn dedupe( /// and saved empty, so look it up in the peer's share listing first. fn resolve_sizes( ctx: &Ctx, - client: &Client, + client: &dyn SessionApi, requests: Vec, ) -> CliResult> { if requests.iter().all(|request| request.size > 0) { @@ -434,7 +435,7 @@ fn ensure_download_dir(download_dir: &str) -> CliResult { /// each result as it lands. Any failure fails the command. fn download_all( ctx: &Ctx, - client: &Arc, + client: &Arc, requests: Vec, timeout: Duration, ) -> CliResult { @@ -452,7 +453,7 @@ fn download_all( queue.lock().ok().and_then(|mut q| q.pop_front()) { let outcome = download_one( - client, + client.as_ref(), &ctx.out, &request, &ctx.download_dir, @@ -499,7 +500,7 @@ fn download_all( /// on the status channel rather than through the call that starts the /// transfer, so the channel is the only thing worth believing. fn download_one( - client: &Client, + client: &dyn SessionApi, out: &Out, request: &Request, download_dir: &str, diff --git a/soulseek-rs/src/commands/wish.rs b/soulseek-rs/src/commands/wish.rs index a1a7eaf..82b65b6 100644 --- a/soulseek-rs/src/commands/wish.rs +++ b/soulseek-rs/src/commands/wish.rs @@ -9,6 +9,7 @@ use super::{Ctx, Session, settings::Store, transfer}; use crate::cli::{SortKey, WishRunArgs}; use crate::output::{CliError, CliResult, Out, WishRecord}; use crate::persist::config::FileConfig; +use crate::api::SessionApi; use soulseek_rs::Client; use std::time::Instant; @@ -56,7 +57,7 @@ pub fn run(ctx: &Ctx, store: &Store, args: &WishRunArgs) -> CliResult { let wishes = selected(&store.config, args.only.as_deref())?; let session = Session::open(ctx)?; - if !sweep(ctx, &session.client, &wishes, &Sweep::from(args)) { + if !sweep(ctx, session.client.as_ref(), &wishes, &Sweep::from(args)) { return Err(CliError::no_results( "no wish matched anything on the network", )); @@ -121,7 +122,7 @@ impl Sweeper { /// unreported and `--duration` overshoots by that much. Move the sweep to its /// own thread with a channel of records if that ever matters; what mattered /// was that it used to be one window *per wish*. - pub fn tick(&mut self, ctx: &Ctx, client: &Client) { + pub fn tick(&mut self, ctx: &Ctx, client: &dyn SessionApi) { if Instant::now() < self.next { return; } @@ -139,7 +140,12 @@ impl Sweeper { /// All the searches go out before anything is waited for: peers answer whenever /// they get round to it, so one shared window covers every wish, where waiting /// per wish would cost a full window each. -fn sweep(ctx: &Ctx, client: &Client, wishes: &[String], how: &Sweep) -> bool { +fn sweep( + ctx: &Ctx, + client: &dyn SessionApi, + wishes: &[String], + how: &Sweep, +) -> bool { let mut sent = Vec::new(); for wish in wishes { match client.start_wishlist_search(wish) { diff --git a/soulseek-rs/src/lib.rs b/soulseek-rs/src/lib.rs index 0a2714b..1ad8b30 100644 --- a/soulseek-rs/src/lib.rs +++ b/soulseek-rs/src/lib.rs @@ -1,6 +1,7 @@ //! Library surface of the TUI binary, exposed for integration tests. //! The binary itself compiles these modules directly (see `main.rs`). +pub mod api; pub mod cli; pub mod models; pub mod persist; diff --git a/soulseek-rs/src/main.rs b/soulseek-rs/src/main.rs index 213cabe..2a008d4 100644 --- a/soulseek-rs/src/main.rs +++ b/soulseek-rs/src/main.rs @@ -1,3 +1,4 @@ +mod api; mod cli; mod commands; mod directories; diff --git a/soulseek-rs/src/ui/main_tui/mod.rs b/soulseek-rs/src/ui/main_tui/mod.rs index 864e116..7c132d5 100644 --- a/soulseek-rs/src/ui/main_tui/mod.rs +++ b/soulseek-rs/src/ui/main_tui/mod.rs @@ -6,6 +6,7 @@ mod rooms; mod search; mod settings; +use crate::api::SessionApi; use crate::models::AppState; use crate::persist::{ snapshot::{Snapshot, restore_messages, restore_searches}, @@ -16,11 +17,10 @@ use ratatui::{ DefaultTerminal, crossterm::event::{self, Event, KeyEventKind, poll}, }; -use soulseek_rs::Client; use std::{sync::Arc, time::Duration}; pub struct MainTui { - client: Arc, + client: Arc, state: AppState, download_dir: String, #[allow(dead_code)] @@ -34,7 +34,7 @@ pub struct MainTui { impl MainTui { pub fn new( - client: Arc, + client: Arc, download_dir: String, max_concurrent_downloads: usize, search_timeout: Duration, @@ -227,7 +227,7 @@ impl MainTui { pub fn launch_main_tui( terminal: DefaultTerminal, - client: Arc, + client: Arc, download_dir: String, max_concurrent_downloads: usize, search_timeout: Duration, diff --git a/soulseek-rs/src/ui/main_tui/render.rs b/soulseek-rs/src/ui/main_tui/render.rs index dc3c632..f0f7779 100644 --- a/soulseek-rs/src/ui/main_tui/render.rs +++ b/soulseek-rs/src/ui/main_tui/render.rs @@ -264,7 +264,7 @@ impl MainTui { // it renders the messages and the compose line itself. let area = centered_rect(70, 60, frame.area()); frame.render_widget(ratatui::widgets::Clear, area); - render_chat_pane(frame, area, &self.state, self.client.username()); + render_chat_pane(frame, area, &self.state, &self.client.username()); } /// Context shortcuts for the chat-rooms popup.