From 64b699cf0bda87a3f2c3ed3d7c3eaee04b154b13 Mon Sep 17 00:00:00 2001 From: Chris Pardy Date: Fri, 1 May 2026 11:22:11 -0400 Subject: [PATCH] refactor(scripts): bundle record-event runner args into RecordEventPayload MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_record_event_script` and `run_record_event_once` each took the record-event context as 6 positional arguments (nsid, action, uri, did, rkey, record). Six `&str`s in a row that the type system can't help you order — easy to swap `did` and `uri` and have it compile. Introduce a `RecordEventPayload<'a>` struct so the runner reads as run_record_event_script(state, payload).await run_record_event_once(state, &resolved, payload).await Both call sites (record_handler::handle_record_event and admin::dead_letters::retry_single) updated to construct the struct literal — names instead of positions. No behavior change. Signed-off-by: Chris Pardy --- src/admin/dead_letters.rs | 16 ++++--- src/lua/mod.rs | 7 +-- src/lua/scripts.rs | 90 ++++++++++++++++++++++----------------- src/record_handler.rs | 28 ++++++------ 4 files changed, 79 insertions(+), 62 deletions(-) diff --git a/src/admin/dead_letters.rs b/src/admin/dead_letters.rs index c5a1018..42c1d33 100644 --- a/src/admin/dead_letters.rs +++ b/src/admin/dead_letters.rs @@ -10,7 +10,7 @@ use super::permissions::Permission; use crate::AppState; use crate::db::{adapt_sql, now_rfc3339, parse_dt}; use crate::error::AppError; -use crate::lua::{resolve_record_event, run_record_event_once}; +use crate::lua::{RecordEventPayload, resolve_record_event, run_record_event_once}; use crate::record_handler::RecordEvent; // --------------------------------------------------------------------------- @@ -513,12 +513,14 @@ async fn retry_single(state: &AppState, id: &str) -> Result<(), AppError> { match run_record_event_once( state, &resolved, - &dl.action, - &dl.uri, - &dl.did, - &dl.collection, - &dl.rkey, - record.as_ref(), + RecordEventPayload { + nsid: &dl.collection, + action: &dl.action, + uri: &dl.uri, + did: &dl.did, + rkey: &dl.rkey, + record: record.as_ref(), + }, ) .await { diff --git a/src/lua/mod.rs b/src/lua/mod.rs index 7177414..bed07b3 100644 --- a/src/lua/mod.rs +++ b/src/lua/mod.rs @@ -14,7 +14,8 @@ pub(crate) use context::SpaceContext; pub(crate) use execute::{execute_procedure_script, execute_query_script}; pub(crate) use sandbox::validate_script; pub use scripts::{ - LabelAppliedEvent, LabelHookOutcome, ParsedTrigger, ResolvedScript, ScriptLanguage, ScriptRow, - TriggerKind, resolve, resolve_record_event, run_label_applied_script, run_record_event_once, - run_record_event_script, trigger_for_label_uri, + LabelAppliedEvent, LabelHookOutcome, ParsedTrigger, RecordEventPayload, ResolvedScript, + ScriptLanguage, ScriptRow, TriggerKind, resolve, resolve_record_event, + run_label_applied_script, run_record_event_once, run_record_event_script, + trigger_for_label_uri, }; diff --git a/src/lua/scripts.rs b/src/lua/scripts.rs index 870d373..f3f8ef6 100644 --- a/src/lua/scripts.rs +++ b/src/lua/scripts.rs @@ -278,37 +278,45 @@ pub async fn resolve_record_event( // Record-event runner (fail-open, retry + dead-letter) // --------------------------------------------------------------------------- +/// All the contextual fields a record-event script needs at execution +/// time. Bundled into a struct so the runner doesn't take 6+ `&str` +/// positional arguments — easy to swap `did` and `uri` and have the +/// type checker shrug. +#[derive(Clone, Copy, Debug)] +pub struct RecordEventPayload<'a> { + pub nsid: &'a str, + pub action: &'a str, + pub uri: &'a str, + pub did: &'a str, + pub rkey: &'a str, + pub record: Option<&'a Value>, +} + /// Run the record-event script (if any) for a given event. Returns the /// record body the indexer should store: `Some(record)` to proceed, /// `None` to skip indexing. /// /// Failure mode is fail-open: a script that exhausts its retry budget is /// dead-lettered and the indexer proceeds with the original record. -#[allow(clippy::too_many_arguments)] pub async fn run_record_event_script( state: &AppState, - nsid: &str, - action: &str, - uri: &str, - did: &str, - rkey: &str, - record: Option<&Value>, + payload: RecordEventPayload<'_>, ) -> Option { - let resolved = match resolve_record_event(state, nsid, action).await { + let resolved = match resolve_record_event(state, payload.nsid, payload.action).await { Some(s) => s, // No script for this trigger → indexer keeps the original record. - None => return record.cloned(), + None => return payload.record.cloned(), }; - let host_id = format!("{nsid}:{action}"); - let payload = serde_json::json!({ + let host_id = format!("{}:{}", payload.nsid, payload.action); + let event_payload = serde_json::json!({ "trigger": resolved.id, - "action": action, - "uri": uri, - "did": did, - "collection": nsid, - "rkey": rkey, - "record": record, + "action": payload.action, + "uri": payload.uri, + "did": payload.did, + "collection": payload.nsid, + "rkey": payload.rkey, + "record": payload.record, }); let mut last_error = String::new(); @@ -317,7 +325,7 @@ pub async fn run_record_event_script( let delay = std::time::Duration::from_secs(1 << (attempt - 1)); tokio::time::sleep(delay).await; } - match run_record_event_once(state, &resolved, action, uri, did, nsid, rkey, record).await { + match run_record_event_once(state, &resolved, payload).await { Ok(outcome) => { log_event( &state.db, @@ -325,7 +333,7 @@ pub async fn run_record_event_script( event_type: "script.executed".to_string(), severity: Severity::Info, actor_did: None, - subject: Some(uri.to_string()), + subject: Some(payload.uri.to_string()), detail: serde_json::json!({ "host_kind": "record", "host_id": host_id, @@ -341,7 +349,7 @@ pub async fn run_record_event_script( Err(e) => { last_error = e; tracing::warn!( - %uri, + uri = %payload.uri, trigger = %resolved.id, attempt = attempt + 1, "record script attempt failed: {last_error}" @@ -355,7 +363,7 @@ pub async fn run_record_event_script( &resolved, "record", &host_id, - &payload, + &event_payload, &last_error, MAX_ATTEMPTS, ) @@ -366,7 +374,7 @@ pub async fn run_record_event_script( event_type: "script.dead_lettered".to_string(), severity: Severity::Error, actor_did: None, - subject: Some(uri.to_string()), + subject: Some(payload.uri.to_string()), detail: serde_json::json!({ "host_kind": "record", "host_id": host_id, @@ -379,7 +387,7 @@ pub async fn run_record_event_script( .await; // Fail-open: indexer proceeds with the original record. - record.cloned() + payload.record.cloned() } /// Single attempt at the record-event Lua script. Used internally by the @@ -388,16 +396,10 @@ pub async fn run_record_event_script( /// Returns `Ok(Some(value))` to continue indexing with `value`, /// `Ok(None)` when the script returned `nil` (skip), or `Err(msg)` on /// any execution failure. -#[allow(clippy::too_many_arguments)] pub async fn run_record_event_once( state: &AppState, script: &ResolvedScript, - action: &str, - uri: &str, - did: &str, - collection: &str, - rkey: &str, - record: Option<&Value>, + payload: RecordEventPayload<'_>, ) -> Result, String> { if script.language != ScriptLanguage::Lua { return Err(format!( @@ -407,25 +409,33 @@ pub async fn run_record_event_once( } let lua = sandbox::create_sandbox().map_err(|e| format!("create sandbox: {e}"))?; let state_arc = Arc::new(state.clone()); - register_default_apis(&lua, &state_arc, &script.id, Some(did))?; + register_default_apis(&lua, &state_arc, &script.id, Some(payload.did))?; // Legacy globals (action, uri, did, collection, rkey, record) for // back-compat with scripts written against the old `index_hook` // surface. - context::set_hook_context(&lua, action, uri, did, collection, rkey, record) - .map_err(|e| format!("set hook context: {e}"))?; + context::set_hook_context( + &lua, + payload.action, + payload.uri, + payload.did, + payload.nsid, + payload.rkey, + payload.record, + ) + .map_err(|e| format!("set hook context: {e}"))?; // Also expose an `event` table — same fields, different idiom. New // scripts can read `event.action` / `event.record.title` instead of // the bare globals; both styles work. use mlua::LuaSerdeExt; let event_value = serde_json::json!({ - "action": action, - "uri": uri, - "did": did, - "collection": collection, - "rkey": rkey, - "record": record, + "action": payload.action, + "uri": payload.uri, + "did": payload.did, + "collection": payload.nsid, + "rkey": payload.rkey, + "record": payload.record, }); lua.globals() .set( @@ -459,7 +469,7 @@ pub async fn run_record_event_once( Ok(Some(v)) } // Non-nil, non-table return — pass-through: keep the original record. - _ => Ok(record.cloned()), + _ => Ok(payload.record.cloned()), } } diff --git a/src/record_handler.rs b/src/record_handler.rs index e3c5173..c35dd27 100644 --- a/src/record_handler.rs +++ b/src/record_handler.rs @@ -65,12 +65,14 @@ pub async fn handle_record_event(state: &AppState, record: &RecordEvent) { // `record.index:`; failures are dead-lettered fail-open. let hook_result = crate::lua::run_record_event_script( state, - &record.collection, - &record.action, - &uri, - &record.did, - &record.rkey, - Some(rec), + crate::lua::RecordEventPayload { + nsid: &record.collection, + action: &record.action, + uri: &uri, + did: &record.did, + rkey: &record.rkey, + record: Some(rec), + }, ) .await; let rec_to_store = match hook_result { @@ -183,12 +185,14 @@ pub async fn handle_record_event(state: &AppState, record: &RecordEvent) { // return aborts the delete; any other return continues. let hook_result = crate::lua::run_record_event_script( state, - &record.collection, - "delete", - &uri, - &record.did, - &record.rkey, - None, + crate::lua::RecordEventPayload { + nsid: &record.collection, + action: "delete", + uri: &uri, + did: &record.did, + rkey: &record.rkey, + record: None, + }, ) .await; if hook_result.is_none() { -- 2.51.2