From 2ff5cccabed97fc5626f7709849caf68e7b49166 Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Tue, 28 Apr 2026 17:33:42 +0200 Subject: [PATCH 1/2] feat(account): query handlers for account list/get/traffic (task 22) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds three CQRS query handlers wired through `QueryBus.with_account_repo`: `list_accounts`, `get_account`, `get_account_traffic`. New read models `AccountViewDto` and `AccountTrafficDto` (camelCase serde) carry every persisted column except passwords — no credential field exists on the type, so JSON output cannot leak one even by accident. `AccountFilter` AND-combines `service_name`, `account_type`, `enabled`. Service name is delegated to the repo's `list_by_service` for SQL-level pruning; type and enabled filter in memory afterwards. `get_account_traffic` returns persisted counters only — the upstream refresh path is the existing `account_validate` command (task 21), so queries stay side-effect free per the project CQRS rule. 21 new unit tests against an `InMemoryAccountRepoForQueries` fixture cover filter combinations, missing-id 404s, missing-repo validation, camelCase shape, and explicit "no password field" assertions on `serde_json::to_value` output. Coverage: account_view 100%, list_accounts 99.23%, get_account 98.29%, get_account_traffic 98.43%. Unblocks task 23 (Vue Accounts). --- CHANGELOG.md | 1 + .../src/application/queries/get_account.rs | 109 +++++++++ .../queries/get_account_traffic.rs | 124 +++++++++++ .../src/application/queries/list_accounts.rs | 210 ++++++++++++++++++ src-tauri/src/application/queries/mod.rs | 37 +++ src-tauri/src/application/query_bus.rs | 18 +- .../application/read_models/account_view.rs | 190 ++++++++++++++++ src-tauri/src/application/read_models/mod.rs | 1 + src-tauri/src/application/test_support.rs | 90 +++++++- 9 files changed, 775 insertions(+), 5 deletions(-) create mode 100644 src-tauri/src/application/queries/get_account.rs create mode 100644 src-tauri/src/application/queries/get_account_traffic.rs create mode 100644 src-tauri/src/application/queries/list_accounts.rs create mode 100644 src-tauri/src/application/read_models/account_view.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 806b02c7..69d64b8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Accounts queries** (PRD §6.4, PRD-v2 §P1.3, task 22): three CQRS query handlers (`list_accounts`, `get_account`, `get_account_traffic`) wired through the `QueryBus` builder via a new `with_account_repo` setter. New read models `AccountViewDto` and `AccountTrafficDto` (`#[serde(rename_all = "camelCase")]`) expose every persisted field — `id`, `service_name`, `username`, `account_type`, `enabled`, `traffic_left`, `traffic_total`, `valid_until`, `last_validated`, `created_at`, `credential_ref` — and never carry a password or raw credential field, by construction. `AccountFilter { service_name?, account_type?, enabled? }` AND-combines filters: `service_name` is delegated to the repo's `list_by_service` for SQL-level pruning, while `account_type` and `enabled` filter in memory. `get_account_traffic` returns the persisted counters; the upstream-refresh path is the existing `account_validate` command (task 21), keeping queries side-effect free per the project CQRS rule. 21 new unit tests against an `InMemoryAccountRepoForQueries` fixture cover filter combinations, missing-id 404s, missing-repo validation errors, camelCase serialization shape, and explicit "no password field" assertions on `serde_json::to_value` output. Unblocks task 23 (Vue Accounts). - **Accounts commands** (PRD §6.4, PRD-v2 §P1.2, task 21): six application-layer command handlers (`add_account`, `update_account`, `delete_account`, `validate_account`, `export_accounts`, `import_accounts`) wired through the `CommandBus` builder. New driven ports `AccountCredentialStore`, `AccountValidator` (with `ValidationOutcome`) and `PassphraseCodec` keep handlers free of plugin / crypto dependencies. `KeyringAccountStore` adapter persists per-account passwords under `vortex-account-{id}` keyring entries; `AesGcmPbkdf2Codec` adapter implements the import / export bundle format using AES-256-GCM with a PBKDF2-HMAC-SHA256 200 000-iteration KDF, fresh per-call salt + nonce, header bound as AAD, and a `VORTACC` magic + version byte so tampered or downgraded bundles fail authentication. Domain events `AccountAdded`, `AccountUpdated`, `AccountDeleted`, `AccountValidated`, `AccountValidationFailed`, `AccountsImported`, `AccountsExported` published via `EventBus` and forwarded by the Tauri bridge as `account-*` browser events. Add rolls back the SQLite row when the keyring write fails so credentials never end up orphaned; import validates every entry up-front and skips `(service_name, username)` pairs already present without inserting partial state. Unblocks task 23 (Vue Accounts). - **Accounts persistence** (PRD §6.4, PRD-v2 §P1.1, task 20): SQLite `accounts` table (migration `m20260428_000006`) with `id` / `service_name` / `username` / `account_type` / `enabled` / `traffic_left` / `traffic_total` / `valid_until` / `last_validated` / `created_at` columns and a UNIQUE `(service_name, username)` index. New `AccountRepository` driven port (`save` / `find_by_id` / `list` / `list_by_service` / `delete`) and `SqliteAccountRepo` adapter with sea-orm entity + `from_domain` / `into_domain` converters. UNIQUE violations surface as `DomainError::AlreadyExists` instead of leaking storage errors. Domain `Account` aggregate gained `traffic_total`, `last_validated`, `created_at` fields and switched its identifier to `AccountId(String)` so generated account ids match the spec's `TEXT PRIMARY KEY`. `Account::credential_ref()` returns a `keyring://{service}/{username}` URI exposing a logical reference suitable for diagnostics; passwords themselves are never persisted to SQLite — they live in the OS keychain via the `AccountCredentialStore` adapter (added in task 21, keyed by `AccountId`). Unblocks tasks 21-25, 38, 51-56, 75-76. diff --git a/src-tauri/src/application/queries/get_account.rs b/src-tauri/src/application/queries/get_account.rs new file mode 100644 index 00000000..297533da --- /dev/null +++ b/src-tauri/src/application/queries/get_account.rs @@ -0,0 +1,109 @@ +//! Handler for [`GetAccountQuery`]. +//! +//! Returns a single account as an [`AccountViewDto`]. Returns +//! `AppError::NotFound` when the id does not match any persisted row. + +use crate::application::error::AppError; +use crate::application::query_bus::QueryBus; +use crate::application::read_models::account_view::AccountViewDto; + +impl QueryBus { + pub async fn handle_get_account( + &self, + query: super::GetAccountQuery, + ) -> Result { + let repo = self + .account_repo() + .ok_or_else(|| AppError::Validation("account repository not configured".into()))?; + + let account = repo + .find_by_id(&query.id)? + .ok_or_else(|| AppError::NotFound(format!("account {}", query.id.as_str())))?; + + Ok(account.into()) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use crate::application::error::AppError; + use crate::application::queries::GetAccountQuery; + use crate::application::test_support::{ + InMemoryAccountRepoForQueries, query_bus_with_accounts, + }; + use crate::domain::model::account::{Account, AccountId, AccountType}; + use crate::domain::ports::driven::AccountRepository; + + fn populate_repo() -> Arc { + let repo = Arc::new(InMemoryAccountRepoForQueries::new()); + repo.save(&Account::new( + AccountId::new("acc-1"), + "real-debrid".to_string(), + "alice".to_string(), + AccountType::Premium, + 1_700_000_000_000, + )) + .unwrap(); + repo + } + + #[tokio::test] + async fn test_get_account_returns_dto_when_found() { + let repo = populate_repo(); + let bus = query_bus_with_accounts(repo); + let dto = bus + .handle_get_account(GetAccountQuery { + id: AccountId::new("acc-1"), + }) + .await + .unwrap(); + assert_eq!(dto.id, "acc-1"); + assert_eq!(dto.service_name, "real-debrid"); + assert_eq!(dto.username, "alice"); + assert_eq!(dto.account_type, "premium"); + } + + #[tokio::test] + async fn test_get_account_returns_not_found_when_missing() { + let repo = populate_repo(); + let bus = query_bus_with_accounts(repo); + let err = bus + .handle_get_account(GetAccountQuery { + id: AccountId::new("ghost"), + }) + .await + .expect_err("ghost id"); + assert!(matches!(err, AppError::NotFound(msg) if msg.contains("ghost"))); + } + + #[tokio::test] + async fn test_get_account_dto_omits_password_field() { + let repo = populate_repo(); + let bus = query_bus_with_accounts(repo); + let dto = bus + .handle_get_account(GetAccountQuery { + id: AccountId::new("acc-1"), + }) + .await + .unwrap(); + let value = serde_json::to_value(&dto).unwrap(); + let object = value.as_object().unwrap(); + assert!(!object.contains_key("password")); + } + + #[tokio::test] + async fn test_get_account_returns_validation_error_when_repo_missing() { + let bus = crate::application::test_support::make_history_query_bus(Arc::new( + crate::application::test_support::NoopHistoryRepo, + )); + let err = bus + .handle_get_account(GetAccountQuery { + id: AccountId::new("acc-1"), + }) + .await + .expect_err("missing repo"); + assert!(matches!(err, AppError::Validation(_))); + } +} diff --git a/src-tauri/src/application/queries/get_account_traffic.rs b/src-tauri/src/application/queries/get_account_traffic.rs new file mode 100644 index 00000000..44906b98 --- /dev/null +++ b/src-tauri/src/application/queries/get_account_traffic.rs @@ -0,0 +1,124 @@ +//! Handler for [`GetAccountTrafficQuery`]. +//! +//! Reads the persisted traffic counters for one account. The "refresh +//! from upstream" step is deliberately not in this handler — that +//! mutates state and lives in the [`ValidateAccountCommand`]( +//! crate::application::commands::ValidateAccountCommand) handler. +//! Splitting them this way keeps queries side-effect free, in line with +//! the project-wide CQRS rule. + +use crate::application::error::AppError; +use crate::application::query_bus::QueryBus; +use crate::application::read_models::account_view::AccountTrafficDto; + +impl QueryBus { + pub async fn handle_get_account_traffic( + &self, + query: super::GetAccountTrafficQuery, + ) -> Result { + let repo = self + .account_repo() + .ok_or_else(|| AppError::Validation("account repository not configured".into()))?; + + let account = repo + .find_by_id(&query.id)? + .ok_or_else(|| AppError::NotFound(format!("account {}", query.id.as_str())))?; + + Ok(account.into()) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use crate::application::error::AppError; + use crate::application::queries::GetAccountTrafficQuery; + use crate::application::test_support::{ + InMemoryAccountRepoForQueries, query_bus_with_accounts, + }; + use crate::domain::model::account::{Account, AccountId, AccountType}; + use crate::domain::ports::driven::AccountRepository; + + #[tokio::test] + async fn test_get_account_traffic_returns_persisted_counters() { + let repo = Arc::new(InMemoryAccountRepoForQueries::new()); + let mut acc = Account::new( + AccountId::new("acc-1"), + "real-debrid".to_string(), + "alice".to_string(), + AccountType::Premium, + 1_700_000_000_000, + ); + acc.set_traffic_left(50_000); + acc.set_traffic_total(100_000); + acc.set_valid_until(2_500_000_000_000); + acc.set_last_validated(1_900_000_000_000); + repo.save(&acc).unwrap(); + + let bus = query_bus_with_accounts(repo); + let dto = bus + .handle_get_account_traffic(GetAccountTrafficQuery { + id: AccountId::new("acc-1"), + }) + .await + .unwrap(); + assert_eq!(dto.id, "acc-1"); + assert_eq!(dto.traffic_left, Some(50_000)); + assert_eq!(dto.traffic_total, Some(100_000)); + assert_eq!(dto.valid_until, Some(2_500_000_000_000)); + assert_eq!(dto.last_validated, Some(1_900_000_000_000)); + } + + #[tokio::test] + async fn test_get_account_traffic_returns_none_counters_when_unset() { + let repo = Arc::new(InMemoryAccountRepoForQueries::new()); + repo.save(&Account::new( + AccountId::new("acc-2"), + "service".to_string(), + "u".to_string(), + AccountType::Free, + 0, + )) + .unwrap(); + + let bus = query_bus_with_accounts(repo); + let dto = bus + .handle_get_account_traffic(GetAccountTrafficQuery { + id: AccountId::new("acc-2"), + }) + .await + .unwrap(); + assert_eq!(dto.traffic_left, None); + assert_eq!(dto.traffic_total, None); + assert_eq!(dto.valid_until, None); + assert_eq!(dto.last_validated, None); + } + + #[tokio::test] + async fn test_get_account_traffic_returns_not_found_when_missing() { + let repo = Arc::new(InMemoryAccountRepoForQueries::new()); + let bus = query_bus_with_accounts(repo); + let err = bus + .handle_get_account_traffic(GetAccountTrafficQuery { + id: AccountId::new("ghost"), + }) + .await + .expect_err("ghost id"); + assert!(matches!(err, AppError::NotFound(msg) if msg.contains("ghost"))); + } + + #[tokio::test] + async fn test_get_account_traffic_returns_validation_error_when_repo_missing() { + let bus = crate::application::test_support::make_history_query_bus(Arc::new( + crate::application::test_support::NoopHistoryRepo, + )); + let err = bus + .handle_get_account_traffic(GetAccountTrafficQuery { + id: AccountId::new("acc-1"), + }) + .await + .expect_err("missing repo"); + assert!(matches!(err, AppError::Validation(_))); + } +} diff --git a/src-tauri/src/application/queries/list_accounts.rs b/src-tauri/src/application/queries/list_accounts.rs new file mode 100644 index 00000000..0138f804 --- /dev/null +++ b/src-tauri/src/application/queries/list_accounts.rs @@ -0,0 +1,210 @@ +//! Handler for [`ListAccountsQuery`]. +//! +//! Returns persisted accounts as [`AccountViewDto`] read models. +//! Filters AND together — service + type + enabled all match. The DTO +//! has no password field, so no credential can leak through this path +//! by construction. + +use crate::application::error::AppError; +use crate::application::query_bus::QueryBus; +use crate::application::read_models::account_view::AccountViewDto; +use crate::domain::model::account::Account; + +impl QueryBus { + pub async fn handle_list_accounts( + &self, + query: super::ListAccountsQuery, + ) -> Result, AppError> { + let repo = self + .account_repo() + .ok_or_else(|| AppError::Validation("account repository not configured".into()))?; + + let accounts = match query.filter.as_ref().and_then(|f| f.service_name.as_ref()) { + Some(service) => repo.list_by_service(service)?, + None => repo.list()?, + }; + + let filtered: Vec = accounts + .into_iter() + .filter(|a| match &query.filter { + None => true, + Some(f) => { + f.account_type.is_none_or(|t| a.account_type() == t) + && f.enabled.is_none_or(|e| a.is_enabled() == e) + } + }) + .collect(); + + Ok(filtered.into_iter().map(AccountViewDto::from).collect()) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use crate::application::queries::{AccountFilter, ListAccountsQuery}; + use crate::application::test_support::{ + InMemoryAccountRepoForQueries, query_bus_with_accounts, + }; + use crate::domain::model::account::{Account, AccountId, AccountType}; + use crate::domain::ports::driven::AccountRepository; + + fn account(id: &str, service: &str, user: &str, ty: AccountType, created: u64) -> Account { + Account::new( + AccountId::new(id), + service.to_string(), + user.to_string(), + ty, + created, + ) + } + + fn populate_default_repo() -> Arc { + let repo = Arc::new(InMemoryAccountRepoForQueries::new()); + repo.save(&account( + "rd-1", + "real-debrid", + "alice", + AccountType::Premium, + 1, + )) + .unwrap(); + repo.save(&account("rd-2", "real-debrid", "bob", AccountType::Free, 2)) + .unwrap(); + let mut disabled = account("ad-1", "alldebrid", "carol", AccountType::Debrid, 3); + disabled.disable(); + repo.save(&disabled).unwrap(); + repo + } + + #[tokio::test] + async fn test_list_accounts_no_filter_returns_all_ordered_by_created_at() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery::default()) + .await + .unwrap(); + assert_eq!(result.len(), 3); + assert_eq!(result[0].id, "rd-1"); + assert_eq!(result[1].id, "rd-2"); + assert_eq!(result[2].id, "ad-1"); + } + + #[tokio::test] + async fn test_list_accounts_filter_by_service_only() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery { + filter: Some(AccountFilter { + service_name: Some("real-debrid".into()), + ..Default::default() + }), + }) + .await + .unwrap(); + assert_eq!(result.len(), 2); + assert!(result.iter().all(|a| a.service_name == "real-debrid")); + } + + #[tokio::test] + async fn test_list_accounts_filter_combines_service_and_type() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery { + filter: Some(AccountFilter { + service_name: Some("real-debrid".into()), + account_type: Some(AccountType::Premium), + enabled: None, + }), + }) + .await + .unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].id, "rd-1"); + } + + #[tokio::test] + async fn test_list_accounts_filter_combines_type_and_enabled() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery { + filter: Some(AccountFilter { + service_name: None, + account_type: Some(AccountType::Debrid), + enabled: Some(false), + }), + }) + .await + .unwrap(); + assert_eq!(result.len(), 1); + assert_eq!(result[0].id, "ad-1"); + } + + #[tokio::test] + async fn test_list_accounts_filter_enabled_only_excludes_disabled() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery { + filter: Some(AccountFilter { + enabled: Some(true), + ..Default::default() + }), + }) + .await + .unwrap(); + assert_eq!(result.len(), 2); + assert!(result.iter().all(|a| a.enabled)); + } + + #[tokio::test] + async fn test_list_accounts_filter_unknown_service_returns_empty() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery { + filter: Some(AccountFilter { + service_name: Some("ghost".into()), + ..Default::default() + }), + }) + .await + .unwrap(); + assert!(result.is_empty()); + } + + #[tokio::test] + async fn test_list_accounts_returns_validation_error_when_repo_missing() { + let bus = crate::application::test_support::make_history_query_bus(Arc::new( + crate::application::test_support::NoopHistoryRepo, + )); + let err = bus + .handle_list_accounts(ListAccountsQuery::default()) + .await + .expect_err("missing repo"); + assert!( + matches!(err, crate::application::error::AppError::Validation(msg) if msg.contains("account")) + ); + } + + #[tokio::test] + async fn test_list_accounts_dto_omits_password_field() { + let repo = populate_default_repo(); + let bus = query_bus_with_accounts(repo); + let result = bus + .handle_list_accounts(ListAccountsQuery::default()) + .await + .unwrap(); + for dto in &result { + let value = serde_json::to_value(dto).unwrap(); + let object = value.as_object().unwrap(); + assert!(!object.contains_key("password")); + assert!(!object.contains_key("credential")); + } + } +} diff --git a/src-tauri/src/application/queries/mod.rs b/src-tauri/src/application/queries/mod.rs index 1316df40..29d54a0b 100644 --- a/src-tauri/src/application/queries/mod.rs +++ b/src-tauri/src/application/queries/mod.rs @@ -4,18 +4,22 @@ //! Handler implementations live in submodules and add methods to `QueryBus`. mod count_by_state; +mod get_account; +mod get_account_traffic; mod get_download_detail; mod get_downloads; mod get_history_entry; mod get_plugin_config; mod get_plugin_store; mod get_stats; +mod list_accounts; mod list_archive_contents; mod list_history; mod list_plugins; mod search_history; mod top_modules; +use crate::domain::model::account::{AccountId, AccountType}; use crate::domain::model::download::DownloadId; use crate::domain::model::views::{ DownloadFilter, HistoryFilter, HistorySort, SortOrder, StatsPeriod, @@ -99,3 +103,36 @@ pub struct GetPluginConfigQuery { pub plugin_name: String, } impl Query for GetPluginConfigQuery {} + +/// Filter combinable on the `ListAccountsQuery`. Each field is +/// optional; missing fields don't constrain the result. Multiple fields +/// AND together (service `"real-debrid"` AND type `Premium` AND +/// `enabled = true`). +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct AccountFilter { + pub service_name: Option, + pub account_type: Option, + pub enabled: Option, +} + +/// List accounts, optionally filtered. Results are ordered by +/// `created_at` ascending — same convention as the underlying repo. +#[derive(Debug, Default)] +pub struct ListAccountsQuery { + pub filter: Option, +} +impl Query for ListAccountsQuery {} + +/// Fetch a single account by id. +#[derive(Debug)] +pub struct GetAccountQuery { + pub id: AccountId, +} +impl Query for GetAccountQuery {} + +/// Fetch the persisted traffic counters for one account. +#[derive(Debug)] +pub struct GetAccountTrafficQuery { + pub id: AccountId, +} +impl Query for GetAccountTrafficQuery {} diff --git a/src-tauri/src/application/query_bus.rs b/src-tauri/src/application/query_bus.rs index 756a5dd9..05382f60 100644 --- a/src-tauri/src/application/query_bus.rs +++ b/src-tauri/src/application/query_bus.rs @@ -6,8 +6,8 @@ use std::sync::Arc; use crate::domain::ports::driven::{ - ArchiveExtractor, DownloadReadRepository, HistoryRepository, PluginConfigStore, PluginLoader, - PluginReadRepository, StatsRepository, + AccountRepository, ArchiveExtractor, DownloadReadRepository, HistoryRepository, + PluginConfigStore, PluginLoader, PluginReadRepository, StatsRepository, }; /// Central dispatcher for CQRS queries. @@ -22,6 +22,7 @@ pub struct QueryBus { archive_extractor: Arc, plugin_loader: Option>, plugin_config_store: Option>, + account_repo: Option>, } impl QueryBus { @@ -40,6 +41,7 @@ impl QueryBus { archive_extractor, plugin_loader: None, plugin_config_store: None, + account_repo: None, } } @@ -57,6 +59,18 @@ impl QueryBus { self } + /// Builder-style setter for the account repository. Optional so + /// existing fixtures that never query accounts don't have to + /// provide a mock. + pub fn with_account_repo(mut self, repo: Arc) -> Self { + self.account_repo = Some(repo); + self + } + + pub fn account_repo(&self) -> Option<&dyn AccountRepository> { + self.account_repo.as_deref() + } + pub fn download_read_repo(&self) -> &dyn DownloadReadRepository { self.download_read_repo.as_ref() } diff --git a/src-tauri/src/application/read_models/account_view.rs b/src-tauri/src/application/read_models/account_view.rs new file mode 100644 index 00000000..66c845d6 --- /dev/null +++ b/src-tauri/src/application/read_models/account_view.rs @@ -0,0 +1,190 @@ +//! Serializable account view DTOs for the frontend. +//! +//! These read models intentionally omit credentials. Passwords / tokens +//! never appear on this type — by construction, not by `#[serde(skip)]`. +//! Code that needs a password must read it from the keyring via the +//! [`AccountCredentialStore`](crate::domain::ports::driven::AccountCredentialStore) +//! port. + +use serde::Serialize; + +use crate::domain::model::account::Account; + +/// Read model for the Accounts list and detail panels. +/// +/// Mirrors the persisted columns of the `accounts` table minus any +/// credential reference. The frontend uses [`Self::credential_ref`] only +/// to look up the keyring entry for the "test connection" surface — the +/// actual password is fetched server-side. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AccountViewDto { + pub id: String, + pub service_name: String, + pub username: String, + pub account_type: String, + pub enabled: bool, + pub traffic_left: Option, + pub traffic_total: Option, + pub valid_until: Option, + pub last_validated: Option, + pub created_at: u64, + /// Opaque keyring URI (`keyring://service/user`) — never the + /// password itself. Lets the frontend correlate two `AccountView` + /// rows that share the same stored credential. + pub credential_ref: String, +} + +impl From for AccountViewDto { + fn from(account: Account) -> Self { + Self { + id: account.id().as_str().to_string(), + service_name: account.service_name().to_string(), + username: account.username().to_string(), + account_type: account.account_type().to_string(), + enabled: account.is_enabled(), + traffic_left: account.traffic_left(), + traffic_total: account.traffic_total(), + valid_until: account.valid_until(), + last_validated: account.last_validated(), + created_at: account.created_at(), + credential_ref: account.credential_ref(), + } + } +} + +/// Lightweight DTO returned by the `account_get_traffic` query. +/// +/// Reports the persisted traffic counters. The "refresh from upstream" +/// step is performed by the `account_validate` command (task 21); +/// callers that want fresh numbers run validate first, then this query. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct AccountTrafficDto { + pub id: String, + pub traffic_left: Option, + pub traffic_total: Option, + pub valid_until: Option, + pub last_validated: Option, +} + +impl From for AccountTrafficDto { + fn from(account: Account) -> Self { + Self { + id: account.id().as_str().to_string(), + traffic_left: account.traffic_left(), + traffic_total: account.traffic_total(), + valid_until: account.valid_until(), + last_validated: account.last_validated(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::model::account::{AccountId, AccountType}; + + fn make_account() -> Account { + Account::reconstruct( + AccountId::new("acc-42"), + "real-debrid".to_string(), + "alice".to_string(), + AccountType::Premium, + true, + Some(1_000), + Some(5_000), + Some(2_500_000_000_000), + Some(1_900_000_000_000), + 1_700_000_000_000, + ) + } + + #[test] + fn test_account_view_dto_from_domain_copies_metadata() { + let dto: AccountViewDto = make_account().into(); + assert_eq!(dto.id, "acc-42"); + assert_eq!(dto.service_name, "real-debrid"); + assert_eq!(dto.username, "alice"); + assert_eq!(dto.account_type, "premium"); + assert!(dto.enabled); + assert_eq!(dto.traffic_left, Some(1_000)); + assert_eq!(dto.traffic_total, Some(5_000)); + assert_eq!(dto.valid_until, Some(2_500_000_000_000)); + assert_eq!(dto.last_validated, Some(1_900_000_000_000)); + assert_eq!(dto.created_at, 1_700_000_000_000); + assert_eq!(dto.credential_ref, "keyring://real-debrid/alice"); + } + + #[test] + fn test_account_view_dto_does_not_serialize_password_field() { + // The DTO has no password member — round-tripping through JSON + // must never reveal one. + let dto: AccountViewDto = make_account().into(); + let value = serde_json::to_value(&dto).unwrap(); + let object = value.as_object().expect("dto serializes as object"); + assert!( + !object.contains_key("password"), + "AccountViewDto must never expose a password field" + ); + assert!( + !object.contains_key("credential"), + "AccountViewDto must never expose a raw credential field" + ); + } + + #[test] + fn test_account_view_dto_serializes_to_camel_case() { + let dto: AccountViewDto = make_account().into(); + let value = serde_json::to_value(&dto).unwrap(); + let object = value.as_object().unwrap(); + for camel_field in [ + "id", + "serviceName", + "username", + "accountType", + "enabled", + "trafficLeft", + "trafficTotal", + "validUntil", + "lastValidated", + "createdAt", + "credentialRef", + ] { + assert!( + object.contains_key(camel_field), + "camelCase field `{camel_field}` missing" + ); + } + } + + #[test] + fn test_account_traffic_dto_from_domain_copies_counters() { + let dto: AccountTrafficDto = make_account().into(); + assert_eq!(dto.id, "acc-42"); + assert_eq!(dto.traffic_left, Some(1_000)); + assert_eq!(dto.traffic_total, Some(5_000)); + assert_eq!(dto.valid_until, Some(2_500_000_000_000)); + assert_eq!(dto.last_validated, Some(1_900_000_000_000)); + } + + #[test] + fn test_account_traffic_dto_serializes_to_camel_case() { + let dto: AccountTrafficDto = make_account().into(); + let value = serde_json::to_value(&dto).unwrap(); + let object = value.as_object().unwrap(); + for camel_field in [ + "id", + "trafficLeft", + "trafficTotal", + "validUntil", + "lastValidated", + ] { + assert!( + object.contains_key(camel_field), + "camelCase field `{camel_field}` missing" + ); + } + assert!(!object.contains_key("password")); + } +} diff --git a/src-tauri/src/application/read_models/mod.rs b/src-tauri/src/application/read_models/mod.rs index 845c73c8..8f728117 100644 --- a/src-tauri/src/application/read_models/mod.rs +++ b/src-tauri/src/application/read_models/mod.rs @@ -1,5 +1,6 @@ //! Application-layer read model DTOs with serde serialization. +pub mod account_view; pub mod download_detail_view; pub mod download_view; pub mod history_view; diff --git a/src-tauri/src/application/test_support.rs b/src-tauri/src/application/test_support.rs index 91caf083..e99ef817 100644 --- a/src-tauri/src/application/test_support.rs +++ b/src-tauri/src/application/test_support.rs @@ -13,6 +13,7 @@ use crate::application::command_bus::CommandBus; use crate::application::query_bus::QueryBus; use crate::domain::error::DomainError; use crate::domain::event::DomainEvent; +use crate::domain::model::account::{Account, AccountId}; use crate::domain::model::archive::{ArchiveEntry, ArchiveFormat, ExtractSummary}; use crate::domain::model::config::{AppConfig, ConfigPatch}; use crate::domain::model::credential::Credential; @@ -28,9 +29,9 @@ use crate::domain::ports::driven::history_repository::{ MAX_HISTORY_PAGE_SIZE, MAX_HISTORY_SEARCH_RESULTS, }; use crate::domain::ports::driven::{ - ArchiveExtractor, ClipboardObserver, ConfigStore, CredentialStore, DownloadEngine, - DownloadReadRepository, DownloadRepository, EventBus, FileStorage, HistoryRepository, - HttpClient, PluginLoader, PluginReadRepository, StatsRepository, + AccountRepository, ArchiveExtractor, ClipboardObserver, ConfigStore, CredentialStore, + DownloadEngine, DownloadReadRepository, DownloadRepository, EventBus, FileStorage, + HistoryRepository, HttpClient, PluginLoader, PluginReadRepository, StatsRepository, }; fn host_component(url: &str) -> Option<&str> { @@ -560,3 +561,86 @@ pub(crate) fn query_bus_with_stats(stats: Arc) -> QueryBus Arc::new(StubQueryArchiveExtractor), ) } + +/// In-memory account repository used by the query-handler tests. +/// +/// Mirrors the production semantics: stable order by `created_at` +/// ascending then by id, and the `(service_name, username)` UNIQUE +/// constraint surfaced by the SQLite adapter. +pub(crate) struct InMemoryAccountRepoForQueries { + store: Mutex>, +} + +impl InMemoryAccountRepoForQueries { + pub(crate) fn new() -> Self { + Self { + store: Mutex::new(HashMap::new()), + } + } + + fn snapshot(&self) -> Vec { + let mut accounts: Vec = self.store.lock().unwrap().values().cloned().collect(); + accounts.sort_by(|a, b| { + a.created_at() + .cmp(&b.created_at()) + .then_with(|| a.id().as_str().cmp(b.id().as_str())) + }); + accounts + } +} + +impl AccountRepository for InMemoryAccountRepoForQueries { + fn find_by_id(&self, id: &AccountId) -> Result, DomainError> { + Ok(self.store.lock().unwrap().get(id).cloned()) + } + + fn save(&self, account: &Account) -> Result<(), DomainError> { + let mut guard = self.store.lock().unwrap(); + for (id, existing) in guard.iter() { + if id != account.id() + && existing.service_name() == account.service_name() + && existing.username() == account.username() + { + return Err(DomainError::AlreadyExists(format!( + "{}::{}", + account.service_name(), + account.username() + ))); + } + } + guard.insert(account.id().clone(), account.clone()); + Ok(()) + } + + fn list(&self) -> Result, DomainError> { + Ok(self.snapshot()) + } + + fn list_by_service(&self, service_name: &str) -> Result, DomainError> { + Ok(self + .snapshot() + .into_iter() + .filter(|a| a.service_name() == service_name) + .collect()) + } + + fn delete(&self, id: &AccountId) -> Result<(), DomainError> { + self.store.lock().unwrap().remove(id); + Ok(()) + } +} + +/// Build a [`QueryBus`] wired with the given account repository. +/// +/// Other read ports return empty/default data — suitable for tests +/// that only exercise account queries. +pub(crate) fn query_bus_with_accounts(repo: Arc) -> QueryBus { + QueryBus::new( + Arc::new(StubDownloadReadRepo), + Arc::new(NoopHistoryRepo), + Arc::new(StubStatsRepo), + Arc::new(StubPluginReadRepo), + Arc::new(StubQueryArchiveExtractor), + ) + .with_account_repo(repo) +} From 991d18fdf5a306efb2c31622377c028700703e9a Mon Sep 17 00:00:00 2001 From: Mathieu Piton <27002047+mpiton@users.noreply.github.com> Date: Tue, 28 Apr 2026 18:06:08 +0200 Subject: [PATCH 2/2] fix: address PR review comments - Reword list_accounts.rs doc to make explicit the secret-leak guarantee is about password/raw secret material, not non-secret credential references like usernames or opaque keyring URIs. - Fix AccountViewDto doc that contradicted the struct: it does carry a non-secret credential_ref (opaque keyring URI), while passwords remain server-side via the keyring. - Add unit test wiring with_account_repo() through account_repo() to guard the builder/accessor pair against silent regressions. --- .../src/application/queries/list_accounts.rs | 6 +- src-tauri/src/application/query_bus.rs | 75 +++++++++++++++++-- .../application/read_models/account_view.rs | 10 ++- 3 files changed, 79 insertions(+), 12 deletions(-) diff --git a/src-tauri/src/application/queries/list_accounts.rs b/src-tauri/src/application/queries/list_accounts.rs index 0138f804..d9d374ec 100644 --- a/src-tauri/src/application/queries/list_accounts.rs +++ b/src-tauri/src/application/queries/list_accounts.rs @@ -2,8 +2,10 @@ //! //! Returns persisted accounts as [`AccountViewDto`] read models. //! Filters AND together — service + type + enabled all match. The DTO -//! has no password field, so no credential can leak through this path -//! by construction. +//! carries no password or raw secret material, so no plaintext secret +//! can leak through this read path. Non-secret identifiers (username, +//! opaque `credential_ref`) are present and intentional — only the +//! credential itself is fetched server-side via the keyring. use crate::application::error::AppError; use crate::application::query_bus::QueryBus; diff --git a/src-tauri/src/application/query_bus.rs b/src-tauri/src/application/query_bus.rs index 05382f60..fde1cad9 100644 --- a/src-tauri/src/application/query_bus.rs +++ b/src-tauri/src/application/query_bus.rs @@ -107,6 +107,7 @@ mod tests { use super::QueryBus; use crate::domain::error::DomainError; + use crate::domain::model::account::{Account, AccountId}; use crate::domain::model::archive::ArchiveFormat; use crate::domain::model::download::DownloadId; use crate::domain::model::plugin::PluginInfo; @@ -115,8 +116,8 @@ mod tests { ModuleStats, SortOrder, StateCountMap, StatsPeriod, StatsView, }; use crate::domain::ports::driven::{ - ArchiveExtractor, DownloadReadRepository, HistoryRepository, PluginReadRepository, - StatsRepository, + AccountRepository, ArchiveExtractor, DownloadReadRepository, HistoryRepository, + PluginReadRepository, StatsRepository, }; struct FakeArchiveExtractor; @@ -258,15 +259,51 @@ mod tests { } } - #[test] - fn test_query_bus_new_compiles() { - let _bus = QueryBus::new( + /// Mock repo whose `list()` returns a sentinel account; lets the + /// wiring test prove `account_repo()` returns the *same* instance + /// passed to `with_account_repo`, not a stub or a copy. + struct SentinelAccountRepo; + impl AccountRepository for SentinelAccountRepo { + fn find_by_id(&self, _id: &AccountId) -> Result, DomainError> { + Ok(None) + } + + fn save(&self, _account: &Account) -> Result<(), DomainError> { + Ok(()) + } + + fn list(&self) -> Result, DomainError> { + Ok(vec![Account::new( + AccountId::new("sentinel"), + "svc".to_string(), + "user".to_string(), + crate::domain::model::account::AccountType::Free, + 42, + )]) + } + + fn list_by_service(&self, _service_name: &str) -> Result, DomainError> { + Ok(vec![]) + } + + fn delete(&self, _id: &AccountId) -> Result<(), DomainError> { + Ok(()) + } + } + + fn make_bus() -> QueryBus { + QueryBus::new( Arc::new(MockDownloadReadRepo), Arc::new(MockHistoryRepo), Arc::new(MockStatsRepo), Arc::new(MockPluginReadRepo), Arc::new(FakeArchiveExtractor), - ); + ) + } + + #[test] + fn test_query_bus_new_compiles() { + let _bus = make_bus(); } #[test] @@ -274,4 +311,30 @@ mod tests { fn assert_send_sync() {} assert_send_sync::(); } + + #[test] + fn test_account_repo_is_none_by_default() { + let bus = make_bus(); + assert!(bus.account_repo().is_none()); + } + + #[test] + fn test_with_account_repo_exposes_same_instance_through_accessor() { + let repo: Arc = Arc::new(SentinelAccountRepo); + let strong_before = Arc::strong_count(&repo); + + let bus = make_bus().with_account_repo(Arc::clone(&repo)); + + assert_eq!( + Arc::strong_count(&repo), + strong_before + 1, + "bus must hold its own strong reference" + ); + let stored = bus + .account_repo() + .expect("with_account_repo wires the accessor"); + let listed = stored.list().expect("sentinel list"); + assert_eq!(listed.len(), 1); + assert_eq!(listed[0].id().as_str(), "sentinel"); + } } diff --git a/src-tauri/src/application/read_models/account_view.rs b/src-tauri/src/application/read_models/account_view.rs index 66c845d6..7d58289f 100644 --- a/src-tauri/src/application/read_models/account_view.rs +++ b/src-tauri/src/application/read_models/account_view.rs @@ -12,10 +12,12 @@ use crate::domain::model::account::Account; /// Read model for the Accounts list and detail panels. /// -/// Mirrors the persisted columns of the `accounts` table minus any -/// credential reference. The frontend uses [`Self::credential_ref`] only -/// to look up the keyring entry for the "test connection" surface — the -/// actual password is fetched server-side. +/// Mirrors the persisted columns of the `accounts` table, including the +/// non-secret [`Self::credential_ref`] (an opaque keyring URI such as +/// `keyring://service/user`). The reference itself is never a password +/// or token — the actual secret is fetched server-side from the OS +/// keyring when the "test connection" surface needs it. Passwords and +/// raw credential material never appear on this DTO. #[derive(Debug, Clone, PartialEq, Eq, Serialize)] #[serde(rename_all = "camelCase")] pub struct AccountViewDto {