From 1911691420313dc799978162c4920b656c181d58 Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Sun, 7 Jun 2026 21:10:46 +0200 Subject: [PATCH] fix(manager-core): F-Q-011 blanket Arc impl; delete PostgresScriptRepoHandle MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A 70-line newtype lived in picloud/src/lib.rs wrapping Arc and re-implementing every ScriptRepository method as `self.0.method(...).await`. Hand-delegation invited silent skew when the trait gained a method, and forced 8 call sites to know about the wrapper. Add a blanket impl ScriptRepository for Arc in manager-core::repo. The implementations forward via (**self).method(...) so any owner of an Arc (or Arc) can be passed where the trait is expected. Migrate the 7 picloud-binary call sites: Arc::new(PostgresScriptRepoHandle(script_repo.clone())) โ†’ script_repo.clone() Delete the newtype + its hand-delegated impl. Leaves a 6-line comment documenting why the boilerplate is gone. AUDIT.md anchor: F-Q-011. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/manager-core/src/repo.rs | 62 +++++++++++++++++++ crates/picloud/src/lib.rs | 102 +++++--------------------------- 2 files changed, 76 insertions(+), 88 deletions(-) diff --git a/crates/manager-core/src/repo.rs b/crates/manager-core/src/repo.rs index c9a4bb2..25255bf 100644 --- a/crates/manager-core/src/repo.rs +++ b/crates/manager-core/src/repo.rs @@ -76,6 +76,68 @@ pub trait ScriptRepository: Send + Sync { -> Result, ScriptRepositoryError>; } +/// F-Q-011: blanket impl for `Arc`. Lets callers +/// pass a shared `Arc` directly anywhere a +/// `dyn ScriptRepository` is wanted โ€” replacing the hand-delegated +/// `PostgresScriptRepoHandle` newtype that previously had to be +/// rewritten in lockstep every time the trait gained a method. +#[async_trait::async_trait] +impl ScriptRepository for std::sync::Arc { + async fn get(&self, id: ScriptId) -> Result, ScriptRepositoryError> { + (**self).get(id).await + } + async fn get_by_name( + &self, + app_id: AppId, + name: &str, + ) -> Result, ScriptRepositoryError> { + (**self).get_by_name(app_id, name).await + } + async fn list(&self) -> Result, ScriptRepositoryError> { + (**self).list().await + } + async fn list_for_app(&self, app_id: AppId) -> Result, ScriptRepositoryError> { + (**self).list_for_app(app_id).await + } + async fn list_for_user( + &self, + user_id: AdminUserId, + ) -> Result, ScriptRepositoryError> { + (**self).list_for_user(user_id).await + } + async fn create(&self, input: NewScript) -> Result { + (**self).create(input).await + } + async fn update( + &self, + id: ScriptId, + patch: ScriptPatch, + ) -> Result { + (**self).update(id, patch).await + } + async fn delete(&self, id: ScriptId) -> Result<(), ScriptRepositoryError> { + (**self).delete(id).await + } + async fn count_routes_for_script( + &self, + script_id: ScriptId, + ) -> Result { + (**self).count_routes_for_script(script_id).await + } + async fn count_triggers_for_script( + &self, + script_id: ScriptId, + ) -> Result { + (**self).count_triggers_for_script(script_id).await + } + async fn list_imports( + &self, + script_id: ScriptId, + ) -> Result, ScriptRepositoryError> { + (**self).list_imports(script_id).await + } +} + /// Inbound shape for create. Defaults match the migration's CHECK /// constraints; the repo enforces them in the DB regardless. #[derive(Debug, Clone)] diff --git a/crates/picloud/src/lib.rs b/crates/picloud/src/lib.rs index 65de655..04b3680 100644 --- a/crates/picloud/src/lib.rs +++ b/crates/picloud/src/lib.rs @@ -299,7 +299,7 @@ pub async fn build_app( // that the dispatcher fires through the standard executor path. let invoke: Arc = Arc::new( picloud_manager_core::invoke_service::InvokeServiceImpl::new( - Arc::new(PostgresScriptRepoHandle(script_repo.clone())), + script_repo.clone(), route_table.clone(), outbox_repo.clone(), ), @@ -348,9 +348,7 @@ pub async fn build_app( .collect(); app_domain_table.replace(compiled_domains); - let resolver = Arc::new(RepoResolver::new(PostgresScriptRepoHandle( - script_repo.clone(), - ))); + let resolver = Arc::new(RepoResolver::new(script_repo.clone())); // Single global gate โ€” overflow is rejected with 503 + Retry-After. // See `ExecutionGate` docs and `PICLOUD_MAX_CONCURRENT_EXECUTIONS`. let gate = Arc::new(ExecutionGate::from_env()); @@ -360,7 +358,7 @@ pub async fn build_app( // due rows to the executor. Shares the `ExecutionGate` with sync // HTTP per design notes ยง2 (one cap for everything). let dispatcher_script_repo: Arc = - Arc::new(PostgresScriptRepoHandle(script_repo.clone())); + script_repo.clone(); let principals: Arc = Arc::new(AdminPrincipalResolver::new(auth.users.clone())); // The InboxRegistry is constructed once and shared between the @@ -385,7 +383,7 @@ pub async fn build_app( .spawn(); let admin = AdminState { - repo: Arc::new(PostgresScriptRepoHandle(script_repo.clone())), + repo: script_repo.clone(), logs: log_repo, apps: apps_repo.clone(), authz: authz.clone(), @@ -394,7 +392,7 @@ pub async fn build_app( }; let route_admin = RouteAdminState { routes: route_repo.clone(), - scripts: Arc::new(PostgresScriptRepoHandle(script_repo.clone())), + scripts: script_repo.clone(), domains: domains_repo.clone(), table: route_table.clone(), authz: authz.clone(), @@ -439,7 +437,7 @@ pub async fn build_app( triggers: trigger_repo.clone(), apps: apps_repo.clone(), authz: authz.clone(), - scripts: Arc::new(PostgresScriptRepoHandle(script_repo.clone())), + scripts: script_repo.clone(), config: trigger_config, master_key: master_key.clone(), }; @@ -533,7 +531,7 @@ pub async fn build_app( triggers: trigger_repo_for_queues.clone(), apps: apps_repo.clone(), authz: authz.clone(), - scripts: Arc::new(PostgresScriptRepoHandle(script_repo.clone())), + scripts: script_repo.clone(), }, )) .merge(files_admin_router(files_admin_state)) @@ -638,82 +636,10 @@ async fn version() -> Json { })) } -// ---------------------------------------------------------------------------- -// Bridge: a single `PostgresScriptRepository` Arc is shared between the -// admin router (writes) and the resolver (reads). The resolver wants -// owned `impl ScriptRepository`, so we wrap the Arc in a delegating -// handle here rather than instantiating two repos against the same pool. -// ---------------------------------------------------------------------------- - -struct PostgresScriptRepoHandle(Arc); - -#[async_trait::async_trait] -impl picloud_manager_core::ScriptRepository for PostgresScriptRepoHandle { - async fn get( - &self, - id: picloud_shared::ScriptId, - ) -> Result, picloud_manager_core::ScriptRepositoryError> { - self.0.get(id).await - } - async fn get_by_name( - &self, - app_id: picloud_shared::AppId, - name: &str, - ) -> Result, picloud_manager_core::ScriptRepositoryError> { - self.0.get_by_name(app_id, name).await - } - async fn list( - &self, - ) -> Result, picloud_manager_core::ScriptRepositoryError> { - self.0.list().await - } - async fn list_for_app( - &self, - app_id: picloud_shared::AppId, - ) -> Result, picloud_manager_core::ScriptRepositoryError> { - self.0.list_for_app(app_id).await - } - async fn list_for_user( - &self, - user_id: picloud_shared::AdminUserId, - ) -> Result, picloud_manager_core::ScriptRepositoryError> { - self.0.list_for_user(user_id).await - } - async fn create( - &self, - input: picloud_manager_core::NewScript, - ) -> Result { - self.0.create(input).await - } - async fn update( - &self, - id: picloud_shared::ScriptId, - patch: picloud_manager_core::ScriptPatch, - ) -> Result { - self.0.update(id, patch).await - } - async fn delete( - &self, - id: picloud_shared::ScriptId, - ) -> Result<(), picloud_manager_core::ScriptRepositoryError> { - self.0.delete(id).await - } - async fn count_routes_for_script( - &self, - script_id: picloud_shared::ScriptId, - ) -> Result { - self.0.count_routes_for_script(script_id).await - } - async fn count_triggers_for_script( - &self, - script_id: picloud_shared::ScriptId, - ) -> Result { - self.0.count_triggers_for_script(script_id).await - } - async fn list_imports( - &self, - script_id: picloud_shared::ScriptId, - ) -> Result, picloud_manager_core::ScriptRepositoryError> { - self.0.list_imports(script_id).await - } -} +// F-Q-011: the PostgresScriptRepoHandle newtype lived here so the +// resolver could take an owned `impl ScriptRepository` from the shared +// Arc. The blanket +// `impl ScriptRepository for Arc` in +// manager-core::repo lets us pass `script_repo.clone()` directly, +// eliminating 70 lines of hand-delegated boilerplate that had to be +// rewritten every time the trait gained a method.