Skip to content

Commit d803f65

Browse files
committed
refactor(engine): replace long argument lists with params structs
Convert trigger firing (before/after/instead/common/batch/dml-hook) and event-plane alert evaluation to take dedicated params structs instead of long positional argument lists, updating all call sites accordingly.
1 parent dbc9d30 commit d803f65

13 files changed

Lines changed: 523 additions & 170 deletions

File tree

nodedb/src/control/server/pgwire/handler/routing/execute.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -399,12 +399,14 @@ impl NodeDbPgHandler {
399399
// --- AFTER triggers ---
400400
if let Some(ref info) = dml_info {
401401
crate::control::trigger::dml_hook_fire::fire_post_dispatch_triggers(
402-
&self.state,
403-
identity,
404-
tenant_id,
405-
info,
406-
&old_row,
407-
0,
402+
crate::control::trigger::dml_hook_fire::DispatchTriggerParams {
403+
state: &self.state,
404+
identity,
405+
tenant_id,
406+
info,
407+
old_row: &old_row,
408+
cascade_depth: 0,
409+
},
408410
)
409411
.await
410412
.map_err(|e| {

nodedb/src/control/server/pgwire/handler/routing/execute_dml_hooks.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -165,12 +165,14 @@ impl NodeDbPgHandler {
165165
if let Some(ref info) = dml_info {
166166
use crate::control::trigger::dml_hook_fire::PreDispatchResult;
167167
match crate::control::trigger::dml_hook_fire::fire_pre_dispatch_triggers(
168-
&self.state,
169-
identity,
170-
tenant_id,
171-
info,
172-
&old_row,
173-
0,
168+
crate::control::trigger::dml_hook_fire::DispatchTriggerParams {
169+
state: &self.state,
170+
identity,
171+
tenant_id,
172+
info,
173+
old_row: &old_row,
174+
cascade_depth: 0,
175+
},
174176
)
175177
.await
176178
.map_err(|e| {

nodedb/src/control/server/shared/ddl/neutral/collection/dml/triggers.rs

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,16 @@ pub(super) async fn fire_sync_after_update_triggers(
5151
) -> Option<Result<Vec<DdlResult>, DdlError>> {
5252
use crate::control::security::catalog::trigger_types::TriggerExecutionMode;
5353
if let Err(e) = crate::control::trigger::fire_after::fire_after_update(
54-
state,
55-
identity,
56-
tenant_id,
57-
coll_name,
58-
old_fields,
59-
new_fields,
60-
0,
61-
Some(TriggerExecutionMode::Sync),
54+
crate::control::trigger::fire_after::FireAfterUpdateParams {
55+
state,
56+
identity,
57+
tenant_id,
58+
collection: coll_name,
59+
old_fields,
60+
new_fields,
61+
cascade_depth: 0,
62+
mode_filter: Some(TriggerExecutionMode::Sync),
63+
},
6264
)
6365
.await
6466
{

nodedb/src/control/trigger/batch/before.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,24 @@ pub struct BeforeBatchResult {
3232
pub errors: ErrorAccumulator,
3333
}
3434

35+
/// Parameters for [`execute_before_batch`].
36+
pub struct BeforeBatchParams<'a> {
37+
/// Shared server state (trigger registry, block cache).
38+
pub state: &'a SharedState,
39+
/// Caller identity (used unless a trigger is SECURITY DEFINER).
40+
pub identity: &'a AuthenticatedIdentity,
41+
/// Tenant scope for trigger lookup and execution.
42+
pub tenant_id: TenantId,
43+
/// Target collection name.
44+
pub collection: &'a str,
45+
/// DML event kind (INSERT/UPDATE/DELETE) the batch represents.
46+
pub event: DmlEvent,
47+
/// Rows to process; possibly mutated in place by BEFORE trigger ASSIGN statements.
48+
pub rows: Vec<TriggerBatchRow>,
49+
/// Current cascade depth, for infinite-loop protection.
50+
pub cascade_depth: u32,
51+
}
52+
3553
/// Execute BEFORE triggers over a batch of rows.
3654
///
3755
/// For each BEFORE trigger on the collection:
@@ -41,16 +59,19 @@ pub struct BeforeBatchResult {
4159
/// 4. Capture per-row errors from RAISE EXCEPTION
4260
///
4361
/// Returns the mutated batch + error accumulator.
44-
#[allow(clippy::too_many_arguments)]
4562
pub async fn execute_before_batch(
46-
state: &SharedState,
47-
identity: &AuthenticatedIdentity,
48-
tenant_id: TenantId,
49-
collection: &str,
50-
event: DmlEvent,
51-
mut rows: Vec<TriggerBatchRow>,
52-
cascade_depth: u32,
63+
params: BeforeBatchParams<'_>,
5364
) -> crate::Result<BeforeBatchResult> {
65+
let BeforeBatchParams {
66+
state,
67+
identity,
68+
tenant_id,
69+
collection,
70+
event,
71+
mut rows,
72+
cascade_depth,
73+
} = params;
74+
5475
let triggers = state
5576
.trigger_registry
5677
.get_matching(tenant_id.as_u64(), collection, event);

nodedb/src/control/trigger/dml_hook_fire.rs

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,22 @@ pub enum PreDispatchResult {
2727
},
2828
}
2929

30+
/// Parameters shared by [`fire_pre_dispatch_triggers`] and [`fire_post_dispatch_triggers`].
31+
pub struct DispatchTriggerParams<'a> {
32+
/// Shared server state (trigger registry, block cache).
33+
pub state: &'a SharedState,
34+
/// Caller identity (used unless a trigger is SECURITY DEFINER).
35+
pub identity: &'a AuthenticatedIdentity,
36+
/// Tenant scope for trigger lookup and execution.
37+
pub tenant_id: TenantId,
38+
/// The DML write being dispatched.
39+
pub info: &'a DmlWriteInfo,
40+
/// The row's prior state, for UPDATE/DELETE (`None` for INSERT).
41+
pub old_row: &'a Option<HashMap<String, nodedb_types::Value>>,
42+
/// Current cascade depth, for infinite-loop protection.
43+
pub cascade_depth: u32,
44+
}
45+
3046
/// Fire BEFORE + INSTEAD OF triggers for a point write.
3147
///
3248
/// Returns `PreDispatchResult::Proceed` if the caller should dispatch normally.
@@ -37,15 +53,18 @@ pub enum PreDispatchResult {
3753
///
3854
/// On BEFORE trigger error (RAISE EXCEPTION), the error propagates and
3955
/// the caller should abort the write.
40-
#[allow(clippy::too_many_arguments)]
4156
pub async fn fire_pre_dispatch_triggers(
42-
state: &SharedState,
43-
identity: &AuthenticatedIdentity,
44-
tenant_id: TenantId,
45-
info: &DmlWriteInfo,
46-
old_row: &Option<HashMap<String, nodedb_types::Value>>,
47-
cascade_depth: u32,
57+
params: DispatchTriggerParams<'_>,
4858
) -> crate::Result<PreDispatchResult> {
59+
let DispatchTriggerParams {
60+
state,
61+
identity,
62+
tenant_id,
63+
info,
64+
old_row,
65+
cascade_depth,
66+
} = params;
67+
4968
// Check INSTEAD OF first — if it handles the write, skip everything else.
5069
match info.event {
5170
DmlEvent::Insert => {
@@ -70,13 +89,15 @@ pub async fn fire_pre_dispatch_triggers(
7089
let old_fields = old_row.as_ref().unwrap_or(&empty);
7190
let new_fields = info.new_fields.as_ref().unwrap_or(&empty);
7291
match super::fire_instead::fire_instead_of_update(
73-
state,
74-
identity,
75-
tenant_id,
76-
&info.collection,
77-
old_fields,
78-
new_fields,
79-
cascade_depth,
92+
super::fire_instead::InsteadOfUpdateParams {
93+
state,
94+
identity,
95+
tenant_id,
96+
collection: &info.collection,
97+
old_fields,
98+
new_fields,
99+
cascade_depth,
100+
},
80101
)
81102
.await?
82103
{
@@ -168,15 +189,16 @@ pub async fn fire_pre_dispatch_triggers(
168189
///
169190
/// Called after the Data Plane has committed the write. Only fires triggers
170191
/// with `execution_mode = Sync`. ASYNC triggers are handled by the Event Plane.
171-
#[allow(clippy::too_many_arguments)]
172-
pub async fn fire_post_dispatch_triggers(
173-
state: &SharedState,
174-
identity: &AuthenticatedIdentity,
175-
tenant_id: TenantId,
176-
info: &DmlWriteInfo,
177-
old_row: &Option<HashMap<String, nodedb_types::Value>>,
178-
cascade_depth: u32,
179-
) -> crate::Result<()> {
192+
pub async fn fire_post_dispatch_triggers(params: DispatchTriggerParams<'_>) -> crate::Result<()> {
193+
let DispatchTriggerParams {
194+
state,
195+
identity,
196+
tenant_id,
197+
info,
198+
old_row,
199+
cascade_depth,
200+
} = params;
201+
180202
let empty = HashMap::new();
181203

182204
// Fire SYNC AFTER ROW triggers.
@@ -198,16 +220,16 @@ pub async fn fire_post_dispatch_triggers(
198220
DmlEvent::Update => {
199221
let old_fields = old_row.as_ref().unwrap_or(&empty);
200222
let new_fields = info.new_fields.as_ref().unwrap_or(&empty);
201-
fire_after::fire_after_update(
223+
fire_after::fire_after_update(fire_after::FireAfterUpdateParams {
202224
state,
203225
identity,
204226
tenant_id,
205-
&info.collection,
227+
collection: &info.collection,
206228
old_fields,
207229
new_fields,
208230
cascade_depth,
209-
Some(TriggerExecutionMode::Sync),
210-
)
231+
mode_filter: Some(TriggerExecutionMode::Sync),
232+
})
211233
.await?;
212234
}
213235
DmlEvent::Delete => {

nodedb/src/control/trigger/fire.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
//! This file exists so that existing call sites (`control::trigger::fire::fire_after_insert`)
66
//! continue to work. New code should import from the specific sub-modules.
77
8-
pub use super::fire_after::{fire_after_delete, fire_after_insert, fire_after_update, fire_sql};
8+
pub use super::fire_after::{
9+
FireAfterUpdateParams, fire_after_delete, fire_after_insert, fire_after_update, fire_sql,
10+
};
911
pub use super::fire_before::{fire_before_delete, fire_before_insert, fire_before_update};
1012
pub use super::fire_instead::{
11-
InsteadOfResult, fire_instead_of_delete, fire_instead_of_insert, fire_instead_of_update,
13+
InsteadOfResult, InsteadOfUpdateParams, fire_instead_of_delete, fire_instead_of_insert,
14+
fire_instead_of_update,
1215
};

nodedb/src/control/trigger/fire_after.rs

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -73,21 +73,42 @@ pub async fn fire_after_insert(
7373
.await
7474
}
7575

76+
/// Parameters for [`fire_after_update`].
77+
pub struct FireAfterUpdateParams<'a> {
78+
/// Shared server state (trigger registry, block cache).
79+
pub state: &'a SharedState,
80+
/// Caller identity (used unless a trigger is SECURITY DEFINER).
81+
pub identity: &'a AuthenticatedIdentity,
82+
/// Tenant scope for trigger lookup and execution.
83+
pub tenant_id: TenantId,
84+
/// Target collection name.
85+
pub collection: &'a str,
86+
/// Row fields before the update (bound as `OLD.*`).
87+
pub old_fields: &'a HashMap<String, nodedb_types::Value>,
88+
/// Row fields after the update (bound as `NEW.*`).
89+
pub new_fields: &'a HashMap<String, nodedb_types::Value>,
90+
/// Current cascade depth, for infinite-loop protection.
91+
pub cascade_depth: u32,
92+
/// Restricts firing to a single execution mode; `None` fires all modes.
93+
pub mode_filter: Option<TriggerExecutionMode>,
94+
}
95+
7696
/// Fire AFTER ROW triggers for an UPDATE operation.
7797
///
7898
/// `old_fields` is the row before the update, `new_fields` is after.
7999
/// Both are available as OLD.field and NEW.field in the trigger body.
80-
#[allow(clippy::too_many_arguments)]
81-
pub async fn fire_after_update(
82-
state: &SharedState,
83-
identity: &AuthenticatedIdentity,
84-
tenant_id: TenantId,
85-
collection: &str,
86-
old_fields: &HashMap<String, nodedb_types::Value>,
87-
new_fields: &HashMap<String, nodedb_types::Value>,
88-
cascade_depth: u32,
89-
mode_filter: Option<TriggerExecutionMode>,
90-
) -> crate::Result<()> {
100+
pub async fn fire_after_update(params: FireAfterUpdateParams<'_>) -> crate::Result<()> {
101+
let FireAfterUpdateParams {
102+
state,
103+
identity,
104+
tenant_id,
105+
collection,
106+
old_fields,
107+
new_fields,
108+
cascade_depth,
109+
mode_filter,
110+
} = params;
111+
91112
let triggers =
92113
state
93114
.trigger_registry

nodedb/src/control/trigger/fire_before.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,10 @@ use crate::control::security::identity::AuthenticatedIdentity;
1818
use crate::control::state::SharedState;
1919
use crate::types::TenantId;
2020

21-
use super::fire_common::{check_cascade_depth, fire_before_triggers_with_mutation, fire_triggers};
21+
use super::fire_common::{
22+
BeforeTriggersMutationParams, check_cascade_depth, fire_before_triggers_with_mutation,
23+
fire_triggers,
24+
};
2225
use super::registry::DmlEvent;
2326

2427
/// Fire BEFORE ROW triggers for an INSERT operation.
@@ -54,16 +57,16 @@ pub async fn fire_before_insert(
5457

5558
let bindings = RowBindings::before_insert(collection, new_fields.clone());
5659

57-
let result = fire_before_triggers_with_mutation(
60+
let result = fire_before_triggers_with_mutation(BeforeTriggersMutationParams {
5861
state,
5962
identity,
6063
tenant_id,
6164
collection,
62-
&before_triggers,
63-
&bindings,
65+
triggers: &before_triggers,
66+
bindings: &bindings,
6467
cascade_depth,
65-
Some(new_fields.clone()),
66-
)
68+
new_fields: Some(new_fields.clone()),
69+
})
6770
.await?;
6871

6972
// Return the (possibly mutated) NEW fields. If None somehow, return original.
@@ -103,16 +106,16 @@ pub async fn fire_before_update(
103106

104107
let bindings = RowBindings::before_update(collection, old_fields.clone(), new_fields.clone());
105108

106-
let result = fire_before_triggers_with_mutation(
109+
let result = fire_before_triggers_with_mutation(BeforeTriggersMutationParams {
107110
state,
108111
identity,
109112
tenant_id,
110113
collection,
111-
&before_triggers,
112-
&bindings,
114+
triggers: &before_triggers,
115+
bindings: &bindings,
113116
cascade_depth,
114-
Some(new_fields.clone()),
115-
)
117+
new_fields: Some(new_fields.clone()),
118+
})
116119
.await?;
117120

118121
Ok(result.unwrap_or_else(|| new_fields.clone()))

0 commit comments

Comments
 (0)