From 292617f8e85b8c18adafc88da3c8b5aa9392ee00 Mon Sep 17 00:00:00 2001 From: Edison Jwa Date: Thu, 11 Jun 2026 10:05:06 +0900 Subject: [PATCH] refactor(core,storage): deduplicate code patterns (TODO-014) Extract PttCapability event construction helper in events.rs. Extract ensure_dir() helper in chanora_storage. Add TODO(refactor) annotations for patterns requiring shared crate or API changes. --- core/chanora_core/src/events.rs | 11 +++++++++++ core/chanora_core/src/lib.rs | 27 ++++----------------------- crates/chanora_bridge/src/lib.rs | 4 ++++ crates/chanora_cache/src/lib.rs | 3 +++ crates/chanora_prefetch/src/lib.rs | 3 +++ crates/chanora_state/src/lib.rs | 4 ++++ crates/chanora_storage/src/lib.rs | 8 ++++++-- 7 files changed, 35 insertions(+), 25 deletions(-) diff --git a/core/chanora_core/src/events.rs b/core/chanora_core/src/events.rs index ca3c71f..e32168e 100644 --- a/core/chanora_core/src/events.rs +++ b/core/chanora_core/src/events.rs @@ -22,6 +22,17 @@ impl From for PttDescriptorSnapshot { } } +impl SessionEvent { + /// Construct a `PttCapability` event from an audio backend descriptor. + pub fn ptt_capability_from_descriptor(desc: &PttBackendDescriptor) -> Self { + Self::PttCapability { + level: desc.level.as_str().to_string(), + backend_id: desc.backend_id.to_string(), + bound_input_class: desc.bound_input_class.unwrap_or("").to_string(), + } + } +} + /// Persisted PTT binding state exposed to callers. #[derive(Debug, Clone, PartialEq, Eq)] pub struct PersistedPttBinding { diff --git a/core/chanora_core/src/lib.rs b/core/chanora_core/src/lib.rs index 088367b..8f121ec 100644 --- a/core/chanora_core/src/lib.rs +++ b/core/chanora_core/src/lib.rs @@ -924,21 +924,13 @@ impl ChanoraSession { // dialog) re-publish through the controller's // descriptor-watch. let initial_desc = controller.descriptor().await; - let _ = self.events_tx.send(SessionEvent::PttCapability { - level: initial_desc.level.as_str().to_string(), - backend_id: initial_desc.backend_id.to_string(), - bound_input_class: initial_desc.bound_input_class.unwrap_or("").to_string(), - }); + let _ = self.events_tx.send(SessionEvent::ptt_capability_from_descriptor(&initial_desc)); let mut watch_rx = controller.descriptor_watch(); let events_tx = self.events_tx.clone(); tokio::spawn(async move { while watch_rx.changed().await.is_ok() { let d = watch_rx.borrow_and_update().clone(); - let _ = events_tx.send(SessionEvent::PttCapability { - level: d.level.as_str().to_string(), - backend_id: d.backend_id.to_string(), - bound_input_class: d.bound_input_class.unwrap_or("").to_string(), - }); + let _ = events_tx.send(SessionEvent::ptt_capability_from_descriptor(&d)); } }); Ok(()) @@ -1000,11 +992,7 @@ impl ChanoraSession { if let Some(state) = guard.as_ref() { if let Some(controller) = state.ptt_controller.as_ref() { let desc = controller.set_binding(binding).await?; - let _ = self.events_tx.send(SessionEvent::PttCapability { - level: desc.level.as_str().to_string(), - backend_id: desc.backend_id.to_string(), - bound_input_class: desc.bound_input_class.unwrap_or("").to_string(), - }); + let _ = self.events_tx.send(SessionEvent::ptt_capability_from_descriptor(&desc)); } } Ok(()) @@ -2269,14 +2257,7 @@ async fn supervisor_loop(ctx: SupervisorContext) { // capability (SRS-196 / SDD-091). let d = controller.descriptor().await; let _ = - events_tx.send(SessionEvent::PttCapability { - level: d.level.as_str().to_string(), - backend_id: d.backend_id.to_string(), - bound_input_class: d - .bound_input_class - .unwrap_or("") - .to_string(), - }); + events_tx.send(SessionEvent::ptt_capability_from_descriptor(&d)); } Err(e) => { warn!( diff --git a/crates/chanora_bridge/src/lib.rs b/crates/chanora_bridge/src/lib.rs index e01c8af..f9e0059 100644 --- a/crates/chanora_bridge/src/lib.rs +++ b/crates/chanora_bridge/src/lib.rs @@ -56,6 +56,10 @@ pub enum BridgeError { /// The caller submitted a malformed command DTO. #[error("invalid command: {0}")] InvalidCommand(String), + // TODO(refactor): DnsFailed and ServerRejected mirror ProtocolError variants + // in chanora_protocol. These cannot be unified without changing the public FFI + // API (flutter_rust_bridge generates Dart types from these). Revisit only if + // the bridge error types are being reworked. /// Hostname resolution failed. Distinct from `Connection` so the /// UI can show a meaningful "Server not found" message. #[error("dns: could not resolve '{host}': {reason}")] diff --git a/crates/chanora_cache/src/lib.rs b/crates/chanora_cache/src/lib.rs index 15327b5..68f79c9 100644 --- a/crates/chanora_cache/src/lib.rs +++ b/crates/chanora_cache/src/lib.rs @@ -16,6 +16,9 @@ use std::path::{Path, PathBuf}; /// Errors raised by the blob cache. +// TODO(refactor): Io(String) variant is duplicated across chanora_cache, +// chanora_storage, and chanora_diagnostics. Could use a shared error type +// or derive From instead of manually wrapping. #[derive(Debug, thiserror::Error)] pub enum BlobCacheError { /// Filesystem I/O error. diff --git a/crates/chanora_prefetch/src/lib.rs b/crates/chanora_prefetch/src/lib.rs index 17befaf..52bd4c1 100644 --- a/crates/chanora_prefetch/src/lib.rs +++ b/crates/chanora_prefetch/src/lib.rs @@ -194,6 +194,9 @@ impl ServerPrefetcher { } } +// TODO(refactor): `normalize_host` duplicates `chanora_resolver::normalize_args` +// host trimming. Both do `host.trim().to_lowercase()`. Could move to a shared +// utility in chanora_protocol or a tiny chanora_common crate if more crates need it. fn normalize_host(host: &str) -> String { host.trim().to_lowercase() } diff --git a/crates/chanora_state/src/lib.rs b/crates/chanora_state/src/lib.rs index 394933d..0c7e876 100644 --- a/crates/chanora_state/src/lib.rs +++ b/crates/chanora_state/src/lib.rs @@ -187,6 +187,10 @@ fn normalize_snapshot(snapshot: ServerSnapshot) -> ServerSnapshot { } } +// TODO(refactor): StateEvent and Delta have mirrored variants (e.g. +// StateEvent::ChannelChanged/ChannelDeleted vs Delta::ChannelUpserted/ChannelRemoved). +// A proc-macro or macro_rules could generate the Delta-from-StateEvent mapping, but +// the manual match is currently clear and the types serve different roles (input vs output). /// A change to the server state that the bridge should publish to /// Flutter. Deltas are cheap to construct and carry only the /// information that changed. diff --git a/crates/chanora_storage/src/lib.rs b/crates/chanora_storage/src/lib.rs index 052941b..1a31609 100644 --- a/crates/chanora_storage/src/lib.rs +++ b/crates/chanora_storage/src/lib.rs @@ -79,6 +79,10 @@ pub enum StorageError { Crypto(String), } +fn ensure_dir(dir: &Path) -> Result<(), StorageError> { + fs::create_dir_all(dir).map_err(|e| StorageError::Io(format!("mkdir {dir:?}: {e}"))) +} + /// Audio-related per-identity settings persisted alongside the /// identity file as a small JSON blob (SDD-095 / SDD-096). These /// are *not* secrets; they sit beside the encrypted identity in @@ -193,7 +197,7 @@ impl IdentityFileStore { /// the DEK on first use; subsequent uses reuse the existing DEK. pub fn new(dir: impl AsRef) -> Result { let dir = dir.as_ref(); - fs::create_dir_all(dir).map_err(|e| StorageError::Io(format!("mkdir {dir:?}: {e}")))?; + ensure_dir(dir)?; let canonical = fs::canonicalize(dir) .map(|p| p.to_string_lossy().into_owned()) .unwrap_or_else(|_| dir.to_string_lossy().into_owned()); @@ -814,7 +818,7 @@ impl BookmarkRepository { } fn open(dir: &Path, crypto: Option>) -> Result { - fs::create_dir_all(dir).map_err(|e| StorageError::Io(format!("mkdir {dir:?}: {e}")))?; + ensure_dir(dir)?; let path = dir.join("chanora.db"); let conn = Connection::open(&path) .map_err(|e| StorageError::Sqlite(format!("open {path:?}: {e}")))?;