feat(suppress): owner-polymorphic suppression repo + reconcile (M1.2)
suppression_repo takes a ScriptOwner (app or group), binding the matching owner column. apply_service load_current / create / prune / suppression_report all thread owner.as_script_owner(); the validate_bundle_for group-suppress rejection and the manifest parse guard are dropped — a [group] may now declare [suppress] to decline a template it inherits from a higher ancestor. No runtime consumption effect yet (the chain filters land in M1.3/M1.4). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -767,20 +767,11 @@ impl ApplyService {
|
|||||||
));
|
));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// §11 tail: only an APP opts out of inherited templates. A group that
|
// §11 tail M1: both an app and a group may declare suppressions — an
|
||||||
// doesn't want a template simply doesn't declare it (the CLI already
|
// app declines an inherited template for itself, a group for its whole
|
||||||
// rejects `[suppress]` on a `[group]`; this guards a hand-rolled wire
|
// subtree. Both are inheritance-only (the dispatch filters gate to
|
||||||
// Bundle — the reconcile's `owner.app_id().expect(...)` would otherwise
|
// group-owned templates on the suppressing owner's chain), so no
|
||||||
// panic on a group).
|
// owner-kind guard here.
|
||||||
if matches!(owner, ApplyOwner::Group(_))
|
|
||||||
&& (!bundle.suppress_triggers.is_empty() || !bundle.suppress_routes.is_empty())
|
|
||||||
{
|
|
||||||
return Err(ApplyError::Invalid(
|
|
||||||
"a group cannot declare suppressions — only an app opts out of \
|
|
||||||
inherited templates"
|
|
||||||
.into(),
|
|
||||||
));
|
|
||||||
}
|
|
||||||
self.validate_bundle(bundle, inherited_endpoints)
|
self.validate_bundle(bundle, inherited_endpoints)
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1065,17 +1056,22 @@ impl ApplyService {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// 3e. Per-app suppression markers (§11 tail) — insert each Create
|
// 3e. Per-app suppression markers (§11 tail) — insert each Create
|
||||||
// (idempotent). App-only; deletes happen in prune → the template
|
// (idempotent). Owner-polymorphic (§11 tail M1): an app declines for
|
||||||
// re-inherits. Key is `"{target_kind}:{reference}"`; split on the FIRST
|
// itself, a group for its whole subtree. Deletes happen in prune → the
|
||||||
// `:` so a route reference containing `:` (a param path `/users/:id`)
|
// template re-inherits. Key is `"{target_kind}:{reference}"`; split on
|
||||||
// survives.
|
// the FIRST `:` so a route reference containing `:` (a param path
|
||||||
|
// `/users/:id`) survives.
|
||||||
for ch in &plan.suppressions {
|
for ch in &plan.suppressions {
|
||||||
if ch.op == Op::Create {
|
if ch.op == Op::Create {
|
||||||
let app_id = owner.app_id().expect("suppressions are app-only");
|
|
||||||
let (kind, reference) = ch.key.split_once(':').expect("suppression key has ':'");
|
let (kind, reference) = ch.key.split_once(':').expect("suppression key has ':'");
|
||||||
crate::suppression_repo::insert_suppression_tx(&mut *tx, app_id, kind, reference)
|
crate::suppression_repo::insert_suppression_tx(
|
||||||
.await
|
&mut *tx,
|
||||||
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
owner.as_script_owner(),
|
||||||
|
kind,
|
||||||
|
reference,
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
||||||
report.suppressions_created += 1;
|
report.suppressions_created += 1;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1188,15 +1184,17 @@ impl ApplyService {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Per-app suppression markers are prunable config too (§11 tail).
|
// Suppression markers are prunable config too (§11 tail).
|
||||||
// Removing a marker re-inherits the template.
|
// Removing a marker re-inherits the template.
|
||||||
for ch in &plan.suppressions {
|
for ch in &plan.suppressions {
|
||||||
if ch.op == Op::Delete {
|
if ch.op == Op::Delete {
|
||||||
let app_id = owner.app_id().expect("suppressions are app-only");
|
|
||||||
let (kind, reference) =
|
let (kind, reference) =
|
||||||
ch.key.split_once(':').expect("suppression key has ':'");
|
ch.key.split_once(':').expect("suppression key has ':'");
|
||||||
crate::suppression_repo::delete_suppression_tx(
|
crate::suppression_repo::delete_suppression_tx(
|
||||||
&mut *tx, app_id, kind, reference,
|
&mut *tx,
|
||||||
|
owner.as_script_owner(),
|
||||||
|
kind,
|
||||||
|
reference,
|
||||||
)
|
)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
||||||
@@ -2221,17 +2219,14 @@ impl ApplyService {
|
|||||||
Ok(out)
|
Ok(out)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Read-only §11 tail report: an app's own suppression markers (the
|
/// Read-only §11 tail report: an owner's own suppression markers (the
|
||||||
/// inherited templates it declines). Group owner → empty (groups don't
|
/// inherited templates it declines). An app declines for itself, a group
|
||||||
/// suppress). Backs `pic suppress ls --app`.
|
/// for its subtree. Backs `pic suppress ls --app` / `--group`.
|
||||||
pub async fn suppression_report(
|
pub async fn suppression_report(
|
||||||
&self,
|
&self,
|
||||||
owner: ApplyOwner,
|
owner: ApplyOwner,
|
||||||
) -> Result<Vec<SuppressionInfo>, ApplyError> {
|
) -> Result<Vec<SuppressionInfo>, ApplyError> {
|
||||||
let ApplyOwner::App(app_id) = owner else {
|
let rows = crate::suppression_repo::list_for_owner(&self.pool, owner.as_script_owner())
|
||||||
return Ok(Vec::new());
|
|
||||||
};
|
|
||||||
let rows = crate::suppression_repo::list_for_app(&self.pool, app_id)
|
|
||||||
.await
|
.await
|
||||||
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
||||||
Ok(rows
|
Ok(rows
|
||||||
@@ -2529,14 +2524,13 @@ impl ApplyService {
|
|||||||
crate::group_collection_repo::list_all_for_owner(&self.pool, owner.as_script_owner())
|
crate::group_collection_repo::list_all_for_owner(&self.pool, owner.as_script_owner())
|
||||||
.await
|
.await
|
||||||
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
||||||
// §11 tail per-app suppression markers — app-only (a group never
|
// §11 tail suppression markers declared directly at this node
|
||||||
// suppresses; it just wouldn't declare the template).
|
// (§11 tail M1: owner-polymorphic — an app declines for itself, a group
|
||||||
let suppressions = match owner {
|
// for its subtree).
|
||||||
ApplyOwner::App(app_id) => crate::suppression_repo::list_for_app(&self.pool, app_id)
|
let suppressions =
|
||||||
|
crate::suppression_repo::list_for_owner(&self.pool, owner.as_script_owner())
|
||||||
.await
|
.await
|
||||||
.map_err(|e| ApplyError::Backend(e.to_string()))?,
|
.map_err(|e| ApplyError::Backend(e.to_string()))?;
|
||||||
ApplyOwner::Group(_) => Vec::new(),
|
|
||||||
};
|
|
||||||
Ok(CurrentState {
|
Ok(CurrentState {
|
||||||
scripts,
|
scripts,
|
||||||
routes,
|
routes,
|
||||||
|
|||||||
@@ -1,52 +1,62 @@
|
|||||||
//! Template-suppression markers (§11 tail) — the `template_suppressions` table
|
//! Template-suppression markers (§11 tail) — the `template_suppressions` table
|
||||||
//! (0058). A marker `(app_id, target_kind, reference)` records that an app opts
|
//! (0058, made owner-polymorphic in 0060). A marker `(owner, target_kind,
|
||||||
//! OUT of an inherited group template: a handler script name it declines
|
//! reference)` records that an OWNER opts OUT of an inherited group template: a
|
||||||
//! (`target_kind='trigger'`) or a path it declines (`target_kind='route'`).
|
//! handler script name it declines (`target_kind='trigger'`) or a path it
|
||||||
|
//! declines (`target_kind='route'`).
|
||||||
//!
|
//!
|
||||||
//! App-only (a group would just not declare the template), so — unlike the
|
//! Owner-polymorphic (like `extension_points`): an **app** declines for itself;
|
||||||
//! owner-polymorphic `extension_points` — these free functions take a plain
|
//! a **group** declines for its whole subtree. The functions take a
|
||||||
//! `AppId`. Same tx-function style: read over `&PgPool`, write over a
|
//! `ScriptOwner` and bind the matching owner column. Same tx-function style:
|
||||||
//! `&mut Transaction`.
|
//! read over `&PgPool`, write over a `&mut Transaction`.
|
||||||
|
|
||||||
use picloud_shared::AppId;
|
use picloud_shared::ScriptOwner;
|
||||||
use sqlx::{PgPool, Postgres, Transaction};
|
use sqlx::{PgPool, Postgres, Transaction};
|
||||||
|
|
||||||
/// The two suppressible template kinds, as stored in `target_kind`.
|
/// The two suppressible template kinds, as stored in `target_kind`.
|
||||||
pub const TARGET_TRIGGER: &str = "trigger";
|
pub const TARGET_TRIGGER: &str = "trigger";
|
||||||
pub const TARGET_ROUTE: &str = "route";
|
pub const TARGET_ROUTE: &str = "route";
|
||||||
|
|
||||||
/// List the suppression markers declared at `app_id`, as `(target_kind,
|
/// List the suppression markers declared at `owner`, as `(target_kind,
|
||||||
/// reference)` pairs, stably ordered. Backs `load_current` (the apply diff) and
|
/// reference)` pairs, stably ordered. Backs `load_current` (the apply diff) and
|
||||||
/// the read-only `suppress ls`.
|
/// the read-only `suppress ls`.
|
||||||
pub async fn list_for_app(
|
pub async fn list_for_owner(
|
||||||
pool: &PgPool,
|
pool: &PgPool,
|
||||||
app_id: AppId,
|
owner: ScriptOwner,
|
||||||
) -> Result<Vec<(String, String)>, sqlx::Error> {
|
) -> Result<Vec<(String, String)>, sqlx::Error> {
|
||||||
let rows: Vec<(String, String)> = sqlx::query_as(
|
let sql = match owner {
|
||||||
"SELECT target_kind, reference FROM template_suppressions \
|
ScriptOwner::App(_) => {
|
||||||
WHERE app_id = $1 ORDER BY target_kind, LOWER(reference)",
|
"SELECT target_kind, reference FROM template_suppressions \
|
||||||
)
|
WHERE app_id = $1 ORDER BY target_kind, LOWER(reference)"
|
||||||
.bind(app_id.into_inner())
|
}
|
||||||
.fetch_all(pool)
|
ScriptOwner::Group(_) => {
|
||||||
.await?;
|
"SELECT target_kind, reference FROM template_suppressions \
|
||||||
Ok(rows)
|
WHERE group_id = $1 ORDER BY target_kind, LOWER(reference)"
|
||||||
|
}
|
||||||
|
};
|
||||||
|
sqlx::query_as(sql)
|
||||||
|
.bind(owner_uuid(owner))
|
||||||
|
.fetch_all(pool)
|
||||||
|
.await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Insert a suppression marker in the apply transaction. Idempotent: re-apply
|
/// Insert a suppression marker in the apply transaction. Idempotent: re-apply
|
||||||
/// of an already-declared `(kind, reference)` is a no-op (`ON CONFLICT DO
|
/// of an already-declared `(kind, reference)` is a no-op (`ON CONFLICT DO
|
||||||
/// NOTHING`), so the marker survives without a spurious version bump.
|
/// NOTHING` against the per-owner partial unique index), so the marker survives
|
||||||
|
/// without a spurious version bump.
|
||||||
pub async fn insert_suppression_tx(
|
pub async fn insert_suppression_tx(
|
||||||
tx: &mut Transaction<'_, Postgres>,
|
tx: &mut Transaction<'_, Postgres>,
|
||||||
app_id: AppId,
|
owner: ScriptOwner,
|
||||||
target_kind: &str,
|
target_kind: &str,
|
||||||
reference: &str,
|
reference: &str,
|
||||||
) -> Result<(), sqlx::Error> {
|
) -> Result<(), sqlx::Error> {
|
||||||
|
let (app_id, group_id) = owner_columns(owner);
|
||||||
sqlx::query(
|
sqlx::query(
|
||||||
"INSERT INTO template_suppressions (app_id, target_kind, reference) \
|
"INSERT INTO template_suppressions (app_id, group_id, target_kind, reference) \
|
||||||
VALUES ($1, $2, $3) \
|
VALUES ($1, $2, $3, $4) \
|
||||||
ON CONFLICT (app_id, target_kind, reference) DO NOTHING",
|
ON CONFLICT DO NOTHING",
|
||||||
)
|
)
|
||||||
.bind(app_id.into_inner())
|
.bind(app_id)
|
||||||
|
.bind(group_id)
|
||||||
.bind(target_kind)
|
.bind(target_kind)
|
||||||
.bind(reference)
|
.bind(reference)
|
||||||
.execute(&mut **tx)
|
.execute(&mut **tx)
|
||||||
@@ -58,18 +68,41 @@ pub async fn insert_suppression_tx(
|
|||||||
/// the manifest stops declaring the reference → the template re-inherits.
|
/// the manifest stops declaring the reference → the template re-inherits.
|
||||||
pub async fn delete_suppression_tx(
|
pub async fn delete_suppression_tx(
|
||||||
tx: &mut Transaction<'_, Postgres>,
|
tx: &mut Transaction<'_, Postgres>,
|
||||||
app_id: AppId,
|
owner: ScriptOwner,
|
||||||
target_kind: &str,
|
target_kind: &str,
|
||||||
reference: &str,
|
reference: &str,
|
||||||
) -> Result<(), sqlx::Error> {
|
) -> Result<(), sqlx::Error> {
|
||||||
sqlx::query(
|
let sql = match owner {
|
||||||
"DELETE FROM template_suppressions \
|
ScriptOwner::App(_) => {
|
||||||
WHERE app_id = $1 AND target_kind = $2 AND reference = $3",
|
"DELETE FROM template_suppressions \
|
||||||
)
|
WHERE app_id = $1 AND target_kind = $2 AND reference = $3"
|
||||||
.bind(app_id.into_inner())
|
}
|
||||||
.bind(target_kind)
|
ScriptOwner::Group(_) => {
|
||||||
.bind(reference)
|
"DELETE FROM template_suppressions \
|
||||||
.execute(&mut **tx)
|
WHERE group_id = $1 AND target_kind = $2 AND reference = $3"
|
||||||
.await?;
|
}
|
||||||
|
};
|
||||||
|
sqlx::query(sql)
|
||||||
|
.bind(owner_uuid(owner))
|
||||||
|
.bind(target_kind)
|
||||||
|
.bind(reference)
|
||||||
|
.execute(&mut **tx)
|
||||||
|
.await?;
|
||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// `(app_id, group_id)` bind pair for the polymorphic owner — exactly one set.
|
||||||
|
fn owner_columns(owner: ScriptOwner) -> (Option<uuid::Uuid>, Option<uuid::Uuid>) {
|
||||||
|
match owner {
|
||||||
|
ScriptOwner::App(a) => (Some(a.into_inner()), None),
|
||||||
|
ScriptOwner::Group(g) => (None, Some(g.into_inner())),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The single owner UUID (app or group), for the WHERE-on-one-column queries.
|
||||||
|
fn owner_uuid(owner: ScriptOwner) -> uuid::Uuid {
|
||||||
|
match owner {
|
||||||
|
ScriptOwner::App(a) => a.into_inner(),
|
||||||
|
ScriptOwner::Group(g) => g.into_inner(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -73,16 +73,12 @@ impl Manifest {
|
|||||||
}
|
}
|
||||||
// A group node owns scripts + vars (+ secret names) and, as of §11 tail,
|
// A group node owns scripts + vars (+ secret names) and, as of §11 tail,
|
||||||
// ROUTE TEMPLATES + EVENT trigger TEMPLATES (kv/docs/files/pubsub) that
|
// ROUTE TEMPLATES + EVENT trigger TEMPLATES (kv/docs/files/pubsub) that
|
||||||
// fan out live to descendant apps. The stateful trigger kinds
|
// fan out live to descendant apps. As of §11 tail M1 a group may also
|
||||||
|
// declare `[suppress]` to decline a template it inherits from a higher
|
||||||
|
// ancestor, for its whole subtree. The stateful trigger kinds
|
||||||
// (cron/queue/email) stay app-only — they need per-app state/secrets →
|
// (cron/queue/email) stay app-only — they need per-app state/secrets →
|
||||||
// materialization.
|
// materialization.
|
||||||
if m.group.is_some() {
|
if m.group.is_some() {
|
||||||
if !m.suppress.is_empty() {
|
|
||||||
anyhow::bail!(
|
|
||||||
"a [group] cannot declare [suppress] — only an app opts out of \
|
|
||||||
inherited templates; a group simply doesn't declare the template"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
let t = &m.triggers;
|
let t = &m.triggers;
|
||||||
if !t.cron.is_empty() || !t.queue.is_empty() || !t.email.is_empty() {
|
if !t.cron.is_empty() || !t.queue.is_empty() || !t.email.is_empty() {
|
||||||
anyhow::bail!(
|
anyhow::bail!(
|
||||||
@@ -808,7 +804,7 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn app_suppress_parses_group_suppress_rejected() {
|
fn app_and_group_suppress_parse() {
|
||||||
// §11 tail: an [app] declares [suppress] to opt out of inherited
|
// §11 tail: an [app] declares [suppress] to opt out of inherited
|
||||||
// templates — script names (triggers) + paths (routes).
|
// templates — script names (triggers) + paths (routes).
|
||||||
let m = Manifest::parse(
|
let m = Manifest::parse(
|
||||||
@@ -819,13 +815,14 @@ mod tests {
|
|||||||
assert_eq!(m.suppress.triggers, ["audit"]);
|
assert_eq!(m.suppress.triggers, ["audit"]);
|
||||||
assert_eq!(m.suppress.routes, ["/hello"]);
|
assert_eq!(m.suppress.routes, ["/hello"]);
|
||||||
|
|
||||||
// A [group] cannot suppress — it simply wouldn't declare the template.
|
// §11 tail M1: a [group] MAY suppress — it declines a template it
|
||||||
let err = Manifest::parse(
|
// inherits from a higher ancestor, for its whole subtree.
|
||||||
|
let g = Manifest::parse(
|
||||||
"[group]\nslug = \"acme\"\nname = \"ACME\"\n\n\
|
"[group]\nslug = \"acme\"\nname = \"ACME\"\n\n\
|
||||||
[suppress]\ntriggers = [\"audit\"]\n",
|
[suppress]\ntriggers = [\"audit\"]\n",
|
||||||
)
|
)
|
||||||
.expect_err("group [suppress] is rejected");
|
.expect("group [suppress] must parse");
|
||||||
assert!(err.to_string().contains("suppress"), "got: {err}");
|
assert_eq!(g.suppress.triggers, ["audit"]);
|
||||||
|
|
||||||
// An unknown key inside [suppress] is a hard error (deny_unknown_fields).
|
// An unknown key inside [suppress] is a hard error (deny_unknown_fields).
|
||||||
let typo = "[app]\nslug = \"x\"\nname = \"X\"\n\n[suppress]\ntrigger = [\"a\"]\n";
|
let typo = "[app]\nslug = \"x\"\nname = \"X\"\n\n[suppress]\ntrigger = [\"a\"]\n";
|
||||||
|
|||||||
Reference in New Issue
Block a user