Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 30 additions & 19 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/schema-forge-acton/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ serde = { version = "1", features = ["derive"] }
serde_json = "1"
chrono = { version = "0.4", features = ["serde"] }
tokio = { version = "1", features = ["sync"] }
acton-service = { version = "0.34.1", default-features = false, features = ["http", "observability", "otel-metrics", "journald", "governor", "resilience", "audit", "openapi", "auth", "crypto-aws-lc-rs", "grpc", "tls"] }
acton-service = { version = "0.35.0", default-features = false, features = ["http", "observability", "otel-metrics", "journald", "governor", "resilience", "audit", "openapi", "auth", "crypto-aws-lc-rs", "grpc", "tls"] }
schema-forge-dsl = { path = "../schema-forge-dsl" }
schema-forge-surrealdb = { path = "../schema-forge-surrealdb", optional = true }
schema-forge-postgres = { path = "../schema-forge-postgres", optional = true }
Expand Down
67 changes: 67 additions & 0 deletions crates/schema-forge-acton/src/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,30 @@ impl ActorExtension for ForgeActor {
configure_registry_mutations(actor);
configure_backend_operations(actor);
}

/// Do not restart this actor.
///
/// `configure` registers handlers but sets no state: the registry,
/// backend, tenant config, policy store, storage registry and hook
/// dispatcher all arrive in the single [`InitForge`] message `serve`
/// sends at boot. A restart rebuilds from `configure`, so the
/// replacement would come back with `Default` state — an empty registry
/// and no backend — and nothing would ever send it a second `InitForge`.
///
/// The result would be a process that stays up and answers `404 schema
/// not found` on every entity route. That is fail-closed, but it points
/// an operator at a data problem when the actual fault is an actor that
/// died. Refusing the restart keeps the failure legible: the handle
/// resolves to `None` and routes answer `500` naming the missing actor.
///
/// This also preserves the behaviour of acton-service <= 0.34.1, where
/// the declared policy was never read and no extension could restart at
/// all. Making `ForgeActor` genuinely restartable means giving it a way
/// to re-initialise itself from the backend; until then, `Permanent`
/// would be a promise the actor cannot keep.
fn restart_policy() -> RestartPolicy {
RestartPolicy::Temporary
}
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -544,3 +568,46 @@ fn configure_backend_operations(actor: &mut ManagedActor<Idle, ForgeActor>) {
})
});
}

#[cfg(test)]
mod tests {
use super::*;

/// Pins the reasoning behind [`ForgeActor::restart_policy`].
///
/// acton-service 0.35.0 made the declared restart policy effective for
/// the first time, so this stopped being a dormant declaration and became
/// live supervision behaviour. `Temporary` is only the right answer for
/// as long as a rebuilt-from-`Default` `ForgeActor` is unusable — the
/// assertions below are that condition, not the policy value. If someone
/// later teaches the actor to re-initialise itself from the backend, this
/// test fails and the policy should be revisited rather than the test
/// relaxed.
#[test]
fn a_restarted_forge_actor_would_be_unusable() {
let rebuilt = ForgeActor::default();
assert!(
rebuilt.backend.is_none(),
"a restart rebuilds from Default, so the backend would be lost"
);
assert!(
rebuilt.registry.is_empty(),
"a restart rebuilds from Default, so every schema would be lost"
);
assert!(
rebuilt.policy_store.is_none(),
"a restart rebuilds from Default, so authz would have no store"
);
assert_eq!(ForgeActor::restart_policy(), RestartPolicy::Temporary);
}

/// The contrasting case: `HookDispatchActor` carries no state, so the
/// default `Permanent` policy is genuinely correct for it.
#[test]
fn hook_dispatch_actor_is_restartable() {
assert_eq!(
crate::hooks::HookDispatchActor::restart_policy(),
RestartPolicy::Permanent
);
}
}
6 changes: 6 additions & 0 deletions crates/schema-forge-acton/src/hooks/dispatch_actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ use super::{run_after_hook, HookDispatcher, HookInvocation, HooksConfig};
#[derive(Default, Debug)]
pub struct HookDispatchActor;

// Restart policy is left at the default (`Permanent`), which acton-service
// 0.35.0 made effective for the first time. That is the right policy here
// precisely because this actor is stateless: every input travels with the
// message, so a rebuilt-from-`Default` replacement is indistinguishable from
// the original. Contrast `ForgeActor`, whose whole state arrives in one
// `InitForge` at boot and which therefore opts out.
impl ActorExtension for HookDispatchActor {
fn configure(actor: &mut ManagedActor<Idle, Self>) {
actor.act_on::<DispatchHook>(|_actor, ctx| {
Expand Down
28 changes: 14 additions & 14 deletions crates/schema-forge-acton/src/routes/entities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ async fn execute_entity_query(
// relation-display machinery treats derived fields identically to
// stored RefArrays.
populate_derived_collections(
forge,
&forge,
schema_def,
&mut visible_entities,
claims,
Expand All @@ -1190,7 +1190,7 @@ async fn execute_entity_query(
// relation field is still scrubbed from the envelope alongside its
// `__display` sibling below.
let display_map = if resolve_relations && !visible_entities.is_empty() {
resolve_relation_displays(forge, schema_def, &visible_entities, claims, &tenant_config)
resolve_relation_displays(&forge, schema_def, &visible_entities, claims, &tenant_config)
.await?
} else {
HashMap::new()
Expand Down Expand Up @@ -2019,7 +2019,7 @@ pub async fn create_entity(
// `related.<F>.<col>` is dereferenced to its tenant-scoped related row and
// injected as a CEL binding before the pure evaluator runs.
check_requires_with_related(
forge,
&forge,
&schema_def,
&fields,
claims.as_ref(),
Expand All @@ -2035,7 +2035,7 @@ pub async fn create_entity(
// (possibly mutated) fields.
let hooks_config = state.config().custom.schema_forge.hooks.clone();
let hook_dispatcher = if hooks_config.enabled && schema_def.has_hooks() {
fetch_hook_dispatcher(forge).await
fetch_hook_dispatcher(&forge).await
} else {
None
};
Expand Down Expand Up @@ -2183,7 +2183,7 @@ pub async fn list_entities(
// before_read hook gate (no entity_id, no fields — list scope).
let hooks_config = state.config().custom.schema_forge.hooks.clone();
if hooks_config.enabled && schema_def.hook_for(HookEvent::BeforeRead).is_some() {
if let Some(dispatcher) = fetch_hook_dispatcher(forge).await {
if let Some(dispatcher) = fetch_hook_dispatcher(&forge).await {
let mut empty = BTreeMap::new();
apply_read_hook(
BeforeHookCtx {
Expand Down Expand Up @@ -2328,7 +2328,7 @@ pub async fn query_entities(
// before_read hook gate (no entity_id, no fields — query scope).
let hooks_config = state.config().custom.schema_forge.hooks.clone();
if hooks_config.enabled && schema_def.hook_for(HookEvent::BeforeRead).is_some() {
if let Some(dispatcher) = fetch_hook_dispatcher(forge).await {
if let Some(dispatcher) = fetch_hook_dispatcher(&forge).await {
let mut empty = BTreeMap::new();
apply_read_hook(
BeforeHookCtx {
Expand Down Expand Up @@ -2475,7 +2475,7 @@ pub async fn get_entity(
&& (schema_def.hook_for(HookEvent::BeforeRead).is_some()
|| schema_def.hook_for(HookEvent::AfterRead).is_some())
{
fetch_hook_dispatcher(forge).await
fetch_hook_dispatcher(&forge).await
} else {
None
};
Expand Down Expand Up @@ -2577,7 +2577,7 @@ pub async fn get_entity(
// helper handles one or many entities the same way.
let mut single = [entity];
populate_derived_collections(
forge,
&forge,
&schema_def,
&mut single,
claims.as_ref(),
Expand All @@ -2596,7 +2596,7 @@ pub async fn get_entity(
if parse_truthy_flag(&params, "resolve") {
let entities_slice = std::slice::from_ref(&entity);
let display_map = resolve_relation_displays(
forge,
&forge,
&schema_def,
entities_slice,
claims.as_ref(),
Expand Down Expand Up @@ -2738,7 +2738,7 @@ pub async fn update_entity(
// CEL @require validation rules (#92) — fail-closed, in-transaction,
// pre-persistence. Cross-entity reads (#95) resolved before evaluation.
check_requires_with_related(
forge,
&forge,
&schema_def,
&fields,
claims.as_ref(),
Expand All @@ -2753,7 +2753,7 @@ pub async fn update_entity(
// validation, then `before_change` runs on the (possibly mutated) fields.
let hooks_config = state.config().custom.schema_forge.hooks.clone();
let hook_dispatcher = if hooks_config.enabled && schema_def.has_hooks() {
fetch_hook_dispatcher(forge).await
fetch_hook_dispatcher(&forge).await
} else {
None
};
Expand Down Expand Up @@ -3015,7 +3015,7 @@ pub async fn patch_entity(
// predicates that reference unpatched fields still see their current
// values. Cross-entity reads (#95) resolved before evaluation.
check_requires_with_related(
forge,
&forge,
&schema_def,
&merged,
claims.as_ref(),
Expand All @@ -3029,7 +3029,7 @@ pub async fn patch_entity(
// finalized post-patch state.
let hooks_config = state.config().custom.schema_forge.hooks.clone();
let hook_dispatcher = if hooks_config.enabled && schema_def.has_hooks() {
fetch_hook_dispatcher(forge).await
fetch_hook_dispatcher(&forge).await
} else {
None
};
Expand Down Expand Up @@ -3251,7 +3251,7 @@ pub async fn delete_entity(
// hook is configured so the dispatcher sees the fields being deleted.
let hooks_config = state.config().custom.schema_forge.hooks.clone();
let hook_dispatcher = if hooks_config.enabled && schema_def.has_hooks() {
fetch_hook_dispatcher(forge).await
fetch_hook_dispatcher(&forge).await
} else {
None
};
Expand Down
8 changes: 4 additions & 4 deletions crates/schema-forge-acton/src/routes/schemas.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,14 +533,14 @@ pub async fn create_schema(
// 4a. Run the inverse-relation pairing pass across the full registry so
// any `-> X[]` field paired with an FK from an existing schema is marked
// as derived before the migration plan is generated.
pair_with_registry(forge, &mut definition).await?;
pair_with_registry(&forge, &mut definition).await?;

// 4b. Pre-validate the proposed Cedar bundle BEFORE running any DB
// migration. The actor will recompile and atomically swap on InsertSchema
// anyway, but doing the dry-run here means a malformed schema is rejected
// with a 400 instead of leaving the database in a state the running
// policy bundle can't reason about.
precheck_policy_bundle(&state, forge, &definition, false).await?;
precheck_policy_bundle(&state, &forge, &definition, false).await?;

// 5. Generate migration plan
let plan = DiffEngine::create_new(&definition);
Expand Down Expand Up @@ -770,11 +770,11 @@ pub async fn update_schema(
// 4a. Run the inverse-relation pairing pass before diffing, so newly
// added `-> X[]` fields are classified as derived (and therefore
// produce no AddRelation step for a physical column).
pair_with_registry(forge, &mut new_definition).await?;
pair_with_registry(&forge, &mut new_definition).await?;

// 4b. Dry-run the Cedar bundle for the proposed registry state so an
// invalid schema fails fast — before any DB migration.
precheck_policy_bundle(&state, forge, &new_definition, false).await?;
precheck_policy_bundle(&state, &forge, &new_definition, false).await?;

// 5. Compute diff and generate migration plan
let plan = DiffEngine::diff(&old_schema, &new_definition);
Expand Down
Loading
Loading