From f1d5f5c34ebf8acf51a4963557e3d84cd4b9372c Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Mon, 29 Jun 2026 21:30:42 +0200 Subject: [PATCH] =?UTF-8?q?feat(modules):=20group-collection=20registry=20?= =?UTF-8?q?+=20group-KV=20storage=20schema=20(=C2=A711.6=20C1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Data layer for shared cross-app group collections (KV-only MVP), nothing wired yet. Two migrations + two repos, modeled on the extension_points (0051) marker and the kv_entries (0007) store: - 0052_group_collections.sql: owner-polymorphic marker table declaring a collection name group-shared, with a `kind` discriminator (CHECK 'kv' for now; generalizes to docs/files/topics/queue later) and per-owner partial-unique (owner, LOWER(name), kind) indexes. CASCADE — a marker is config, not code. - 0053_group_kv_entries.sql: the shared store, keyed by (group_id, collection, key) — NO app_id (a shared row belongs to the group). CASCADE on group delete (data dies with its group, like vars/secrets config; an app delete leaves it). - group_collection_repo: list_for_owner, insert/delete_collection_tx, and the load-bearing resolve_owning_group — walks the reading app's chain (CHAIN_LEVELS_CTE) for the nearest ancestor group declaring the name (nearest-wins via ORDER BY depth LIMIT 1). That walk IS the isolation boundary; the join is on group_owner only. - group_kv_repo: a near-clone of kv_repo keyed by group_id. Schema snapshot re-blessed (53 migrations). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../migrations/0052_group_collections.sql | 47 ++++ .../migrations/0053_group_kv_entries.sql | 30 +++ .../manager-core/src/group_collection_repo.rs | 151 ++++++++++++ crates/manager-core/src/group_kv_repo.rs | 222 ++++++++++++++++++ crates/manager-core/src/lib.rs | 3 + crates/manager-core/tests/expected_schema.txt | 41 ++++ 6 files changed, 494 insertions(+) create mode 100644 crates/manager-core/migrations/0052_group_collections.sql create mode 100644 crates/manager-core/migrations/0053_group_kv_entries.sql create mode 100644 crates/manager-core/src/group_collection_repo.rs create mode 100644 crates/manager-core/src/group_kv_repo.rs diff --git a/crates/manager-core/migrations/0052_group_collections.sql b/crates/manager-core/migrations/0052_group_collections.sql new file mode 100644 index 0000000..d6164ca --- /dev/null +++ b/crates/manager-core/migrations/0052_group_collections.sql @@ -0,0 +1,47 @@ +-- §11.6 (v1.2 Hierarchies): group-collection registry — shared cross-app data. +-- +-- A row marks a collection NAME at a node as group-SHARED: every app in the +-- owning group's subtree may read it (and, with an authenticated editor+, +-- write it) via the explicit `kv::shared("name")` SDK handle. Resolution is +-- nearest-ancestor-group-wins, walking the reading app's chain — that ancestry +-- walk is the isolation boundary (a foreign app's chain never contains the +-- owning group, so the name simply does not resolve). See docs §11.6. +-- +-- This table holds only the MARKER (owner, name, kind) — the data itself lives +-- in a per-kind storage table (`group_kv_entries`, 0053, for kind='kv'). It is +-- pure declaration, structurally identical to an `extension_points` marker +-- (0051). A marker is config, not code → ON DELETE CASCADE. +-- +-- Ownership is polymorphic (mirrors vars/secrets/scripts/extension_points): +-- exactly one of (app_id, group_id) is set. MVP authoring is restricted to +-- GROUP owners at the manifest layer (an app-declared shared collection is +-- degenerate — only that app would read it); the polymorphic shape keeps the +-- owner-generic reconcile path uniform with the other inheritable kinds. +-- +-- `kind` is the storage discriminator. Only 'kv' ships in the MVP; the column +-- + CHECK exist now so docs/files/topics/queue slot in later without a +-- migration, and so a future `docs` collection of the same name is a distinct +-- row (the unique index covers `kind`). + +CREATE TABLE group_collections ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + group_id UUID REFERENCES groups(id) ON DELETE CASCADE, + app_id UUID REFERENCES apps(id) ON DELETE CASCADE, + CONSTRAINT group_collections_owner_exactly_one + CHECK ((group_id IS NULL) <> (app_id IS NULL)), + name TEXT NOT NULL, + kind TEXT NOT NULL DEFAULT 'kv' CHECK (kind IN ('kv')), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW() +); + +-- One marker per (owner, name, kind), case-insensitive. Partial because the +-- owner is split across two nullable columns. +CREATE UNIQUE INDEX group_collections_group_uidx + ON group_collections (group_id, LOWER(name), kind) WHERE group_id IS NOT NULL; +CREATE UNIQUE INDEX group_collections_app_uidx + ON group_collections (app_id, LOWER(name), kind) WHERE app_id IS NOT NULL; + +-- Lookup indexes for the resolver's chain join + list-by-owner. +CREATE INDEX group_collections_group_id_idx ON group_collections (group_id) WHERE group_id IS NOT NULL; +CREATE INDEX group_collections_app_id_idx ON group_collections (app_id) WHERE app_id IS NOT NULL; diff --git a/crates/manager-core/migrations/0053_group_kv_entries.sql b/crates/manager-core/migrations/0053_group_kv_entries.sql new file mode 100644 index 0000000..576f691 --- /dev/null +++ b/crates/manager-core/migrations/0053_group_kv_entries.sql @@ -0,0 +1,30 @@ +-- §11.6 (v1.2 Hierarchies): group-KV storage — the shared read/write data for +-- a collection declared group-shared in `group_collections` (0052, kind='kv'). +-- +-- Identity tuple `(group_id, collection, key)`. Unlike per-app `kv_entries` +-- (0007) this is keyed by the OWNING GROUP, not by app — the whole point is +-- that many apps in the group's subtree share one store. There is no `app_id` +-- column: a shared row belongs to the group, not to whichever app wrote it. +-- +-- `value` is JSONB, mirroring `kv_entries`. The reading/writing app is resolved +-- to its owning group at the service layer (walking the app's ancestor chain); +-- this table only ever sees a concrete `group_id`. +-- +-- ON DELETE CASCADE on group_id: the shared store is DATA, so it dies with its +-- owning group — like `vars`/`secrets` config CASCADE, deliberately UNLIKE the +-- `scripts` (0050) RESTRICT (code is not data). Deleting an app does NOT touch +-- this table (the data is the group's, and survives the app's departure). + +CREATE TABLE group_kv_entries ( + group_id UUID NOT NULL REFERENCES groups(id) ON DELETE CASCADE, + collection TEXT NOT NULL, + key TEXT NOT NULL, + value JSONB NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + PRIMARY KEY (group_id, collection, key) +); + +-- Supports list-by-collection (keyset pagination); the PK already covers +-- (group_id, collection) as a prefix but the explicit index makes intent clear. +CREATE INDEX idx_group_kv_entries_group_collection ON group_kv_entries (group_id, collection); diff --git a/crates/manager-core/src/group_collection_repo.rs b/crates/manager-core/src/group_collection_repo.rs new file mode 100644 index 0000000..6b0d463 --- /dev/null +++ b/crates/manager-core/src/group_collection_repo.rs @@ -0,0 +1,151 @@ +//! Group-collection markers (§11.6) — the `group_collections` table (0052). +//! +//! A marker `(owner, name, kind)` declares that a collection name is +//! group-shared (the data lives in a per-kind store; `group_kv_entries` for +//! `kind='kv'`). This module holds the read + transactional-write helpers and +//! the runtime resolver; it mirrors the var/secret/extension-point tx-function +//! style (free functions over a `&PgPool` / `&mut Transaction`), keyed by the +//! shared [`ScriptOwner`]. +//! +//! The load-bearing function is [`resolve_owning_group`]: it walks the reading +//! app's ancestor chain ([`CHAIN_LEVELS_CTE`]) for the nearest group declaring +//! the collection. That walk **is** the isolation boundary — a foreign app's +//! chain never contains the owning group, so the name does not resolve. + +use picloud_shared::{AppId, GroupId, ScriptOwner}; +use sqlx::{PgPool, Postgres, Transaction}; + +use crate::config_resolver::CHAIN_LEVELS_CTE; + +/// The only storage kind that ships in the MVP. The `kind` column generalizes +/// to docs/files/topics/queue later; every query pins this value for now. +const KIND_KV: &str = "kv"; + +/// List the collection names declared **directly at** `owner` (not inherited), +/// case-insensitively sorted. Used by `load_current` (apply diff) and the +/// read-only `collections ls`. MVP is `kind='kv'` only. +pub async fn list_for_owner(pool: &PgPool, owner: ScriptOwner) -> Result, sqlx::Error> { + let rows: Vec<(String,)> = match owner { + ScriptOwner::App(a) => { + sqlx::query_as( + "SELECT name FROM group_collections \ + WHERE app_id = $1 AND kind = $2 ORDER BY LOWER(name)", + ) + .bind(a.into_inner()) + .bind(KIND_KV) + .fetch_all(pool) + .await? + } + ScriptOwner::Group(g) => { + sqlx::query_as( + "SELECT name FROM group_collections \ + WHERE group_id = $1 AND kind = $2 ORDER BY LOWER(name)", + ) + .bind(g.into_inner()) + .bind(KIND_KV) + .fetch_all(pool) + .await? + } + }; + Ok(rows.into_iter().map(|(n,)| n).collect()) +} + +/// Resolve the group that OWNS the shared collection `name` for a reading app: +/// the nearest ancestor group on the app's chain that declares it (`kind='kv'`). +/// Returns `None` when no group on the chain shares that name — the structural +/// "not shared with you" boundary. Nearest-wins (CoW shadowing) is enforced by +/// `ORDER BY depth ASC LIMIT 1` and is security-relevant. +/// +/// The join is on `group_owner` only: an app-declared marker (the degenerate +/// case) never makes a collection visible to *other* apps — sharing is a group +/// property. +pub async fn resolve_owning_group( + pool: &PgPool, + app_id: AppId, + name: &str, +) -> Result, sqlx::Error> { + let row: Option<(uuid::Uuid,)> = sqlx::query_as(&format!( + "{CHAIN_LEVELS_CTE} \ + SELECT gc.group_id FROM group_collections gc \ + JOIN chain c ON gc.group_id = c.group_owner \ + WHERE LOWER(gc.name) = LOWER($2) AND gc.kind = $3 \ + ORDER BY c.depth ASC LIMIT 1", + )) + .bind(app_id.into_inner()) + .bind(name) + .bind(KIND_KV) + .fetch_optional(pool) + .await?; + Ok(row.map(|(id,)| GroupId::from(id))) +} + +/// Insert a collection marker at `owner`, in the apply transaction. Idempotent: +/// a re-apply of an already-declared name is a no-op (`ON CONFLICT DO NOTHING`), +/// so the marker survives without a spurious version bump. +pub async fn insert_collection_tx( + tx: &mut Transaction<'_, Postgres>, + owner: ScriptOwner, + name: &str, +) -> Result<(), sqlx::Error> { + match owner { + ScriptOwner::App(a) => { + sqlx::query( + "INSERT INTO group_collections (app_id, name, kind) VALUES ($1, $2, $3) \ + ON CONFLICT (app_id, LOWER(name), kind) WHERE app_id IS NOT NULL DO NOTHING", + ) + .bind(a.into_inner()) + .bind(name) + .bind(KIND_KV) + .execute(&mut **tx) + .await?; + } + ScriptOwner::Group(g) => { + sqlx::query( + "INSERT INTO group_collections (group_id, name, kind) VALUES ($1, $2, $3) \ + ON CONFLICT (group_id, LOWER(name), kind) WHERE group_id IS NOT NULL DO NOTHING", + ) + .bind(g.into_inner()) + .bind(name) + .bind(KIND_KV) + .execute(&mut **tx) + .await?; + } + } + Ok(()) +} + +/// Delete a collection marker at `owner` (case-insensitive), in the apply +/// transaction. Used by `--prune` when the manifest stops declaring a name. +/// The `group_kv_entries` data is NOT dropped here — pruning a marker hides the +/// store but leaves the data until the owning group is deleted. +pub async fn delete_collection_tx( + tx: &mut Transaction<'_, Postgres>, + owner: ScriptOwner, + name: &str, +) -> Result<(), sqlx::Error> { + match owner { + ScriptOwner::App(a) => { + sqlx::query( + "DELETE FROM group_collections \ + WHERE app_id = $1 AND LOWER(name) = LOWER($2) AND kind = $3", + ) + .bind(a.into_inner()) + .bind(name) + .bind(KIND_KV) + .execute(&mut **tx) + .await?; + } + ScriptOwner::Group(g) => { + sqlx::query( + "DELETE FROM group_collections \ + WHERE group_id = $1 AND LOWER(name) = LOWER($2) AND kind = $3", + ) + .bind(g.into_inner()) + .bind(name) + .bind(KIND_KV) + .execute(&mut **tx) + .await?; + } + } + Ok(()) +} diff --git a/crates/manager-core/src/group_kv_repo.rs b/crates/manager-core/src/group_kv_repo.rs new file mode 100644 index 0000000..0968190 --- /dev/null +++ b/crates/manager-core/src/group_kv_repo.rs @@ -0,0 +1,222 @@ +//! Low-level Postgres CRUD over `group_kv_entries` (§11.6). A near-clone of +//! [`crate::kv_repo`] keyed by the owning `group_id` instead of `app_id` — +//! authorization, group resolution, event policy, and empty-collection +//! validation live one layer up in `GroupKvServiceImpl`. + +use async_trait::async_trait; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use base64::Engine as _; +use picloud_shared::{GroupId, KvListPage}; +use sqlx::PgPool; + +#[derive(Debug, thiserror::Error)] +pub enum GroupKvRepoError { + #[error("database error: {0}")] + Db(#[from] sqlx::Error), + + #[error("invalid pagination cursor")] + InvalidCursor, +} + +/// Repo surface. The trait is exposed so tests can substitute an in-memory +/// backing without spinning up Postgres. +#[async_trait] +pub trait GroupKvRepo: Send + Sync { + async fn get( + &self, + group_id: GroupId, + collection: &str, + key: &str, + ) -> Result, GroupKvRepoError>; + + /// Upserts the row. Returns the previous value (if any) so callers can + /// determine whether this was an `insert` or an `update`. + async fn set( + &self, + group_id: GroupId, + collection: &str, + key: &str, + value: serde_json::Value, + ) -> Result, GroupKvRepoError>; + + /// Returns the deleted value if present, `None` if the row didn't exist. + async fn delete( + &self, + group_id: GroupId, + collection: &str, + key: &str, + ) -> Result, GroupKvRepoError>; + + async fn has( + &self, + group_id: GroupId, + collection: &str, + key: &str, + ) -> Result; + + async fn list( + &self, + group_id: GroupId, + collection: &str, + cursor: Option<&str>, + limit: u32, + ) -> Result; +} + +pub struct PostgresGroupKvRepo { + pool: PgPool, +} + +impl PostgresGroupKvRepo { + #[must_use] + pub fn new(pool: PgPool) -> Self { + Self { pool } + } +} + +/// Hard ceiling on `list` page size (mirrors `kv_repo`). +const KV_LIST_MAX_LIMIT: u32 = 1_000; +const KV_LIST_DEFAULT_LIMIT: u32 = 100; + +#[async_trait] +impl GroupKvRepo for PostgresGroupKvRepo { + async fn get( + &self, + group_id: GroupId, + collection: &str, + key: &str, + ) -> Result, GroupKvRepoError> { + let row: Option<(serde_json::Value,)> = sqlx::query_as( + "SELECT value FROM group_kv_entries \ + WHERE group_id = $1 AND collection = $2 AND key = $3", + ) + .bind(group_id.into_inner()) + .bind(collection) + .bind(key) + .fetch_optional(&self.pool) + .await?; + Ok(row.map(|(v,)| v)) + } + + async fn set( + &self, + group_id: GroupId, + collection: &str, + key: &str, + value: serde_json::Value, + ) -> Result, GroupKvRepoError> { + let row: Option<(Option,)> = sqlx::query_as( + "WITH prev AS (\ + SELECT value FROM group_kv_entries \ + WHERE group_id = $1 AND collection = $2 AND key = $3\ + ), \ + upserted AS (\ + INSERT INTO group_kv_entries (group_id, collection, key, value) \ + VALUES ($1, $2, $3, $4) \ + ON CONFLICT (group_id, collection, key) DO UPDATE \ + SET value = EXCLUDED.value, updated_at = NOW() \ + RETURNING 1\ + ) \ + SELECT (SELECT value FROM prev) FROM upserted", + ) + .bind(group_id.into_inner()) + .bind(collection) + .bind(key) + .bind(value) + .fetch_optional(&self.pool) + .await?; + Ok(row.and_then(|(v,)| v)) + } + + async fn delete( + &self, + group_id: GroupId, + collection: &str, + key: &str, + ) -> Result, GroupKvRepoError> { + let row: Option<(serde_json::Value,)> = sqlx::query_as( + "DELETE FROM group_kv_entries \ + WHERE group_id = $1 AND collection = $2 AND key = $3 \ + RETURNING value", + ) + .bind(group_id.into_inner()) + .bind(collection) + .bind(key) + .fetch_optional(&self.pool) + .await?; + Ok(row.map(|(v,)| v)) + } + + async fn has( + &self, + group_id: GroupId, + collection: &str, + key: &str, + ) -> Result { + let row: Option<(i64,)> = sqlx::query_as( + "SELECT 1 FROM group_kv_entries \ + WHERE group_id = $1 AND collection = $2 AND key = $3", + ) + .bind(group_id.into_inner()) + .bind(collection) + .bind(key) + .fetch_optional(&self.pool) + .await?; + Ok(row.is_some()) + } + + async fn list( + &self, + group_id: GroupId, + collection: &str, + cursor: Option<&str>, + limit: u32, + ) -> Result { + let limit = if limit == 0 { + KV_LIST_DEFAULT_LIMIT + } else { + limit.min(KV_LIST_MAX_LIMIT) + }; + + let last_key = match cursor { + Some(c) => Some(decode_cursor(c)?), + None => None, + }; + + let take = i64::from(limit) + 1; + let rows: Vec<(String,)> = sqlx::query_as( + "SELECT key FROM group_kv_entries \ + WHERE group_id = $1 AND collection = $2 \ + AND ($3::text IS NULL OR key > $3) \ + ORDER BY key ASC \ + LIMIT $4", + ) + .bind(group_id.into_inner()) + .bind(collection) + .bind(last_key.as_deref()) + .bind(take) + .fetch_all(&self.pool) + .await?; + + let mut keys: Vec = rows.into_iter().map(|(k,)| k).collect(); + let next_cursor = if keys.len() > limit as usize { + keys.truncate(limit as usize); + keys.last().map(|k| encode_cursor(k)) + } else { + None + }; + + Ok(KvListPage { keys, next_cursor }) + } +} + +fn encode_cursor(last_key: &str) -> String { + URL_SAFE_NO_PAD.encode(last_key.as_bytes()) +} + +fn decode_cursor(cursor: &str) -> Result { + let bytes = URL_SAFE_NO_PAD + .decode(cursor) + .map_err(|_| GroupKvRepoError::InvalidCursor)?; + String::from_utf8(bytes).map_err(|_| GroupKvRepoError::InvalidCursor) +} diff --git a/crates/manager-core/src/lib.rs b/crates/manager-core/src/lib.rs index 26341a0..464283d 100644 --- a/crates/manager-core/src/lib.rs +++ b/crates/manager-core/src/lib.rs @@ -50,6 +50,8 @@ pub mod files_repo; pub mod files_service; pub mod files_sweep; pub mod gc; +pub mod group_collection_repo; +pub mod group_kv_repo; pub mod group_members_repo; pub mod group_repo; pub mod group_scripts_api; @@ -188,6 +190,7 @@ pub use group_scripts_api::{group_scripts_router, GroupScriptsApiError, GroupScr pub use groups_api::{groups_router, GroupsApiError, GroupsState}; pub use http_service::{HttpConfig, HttpServiceImpl}; pub use kv_api::{kv_admin_router, KvAdminState}; +pub use group_kv_repo::{GroupKvRepo, GroupKvRepoError, PostgresGroupKvRepo}; pub use kv_repo::{KvRepo, KvRepoError, PostgresKvRepo}; pub use kv_service::KvServiceImpl; pub use log_sink::PostgresExecutionLogSink; diff --git a/crates/manager-core/tests/expected_schema.txt b/crates/manager-core/tests/expected_schema.txt index 03e2392..7fa6c90 100644 --- a/crates/manager-core/tests/expected_schema.txt +++ b/crates/manager-core/tests/expected_schema.txt @@ -222,6 +222,23 @@ table: files_trigger_details collection_glob: text NOT NULL ops: ARRAY NOT NULL +table: group_collections + id: uuid NOT NULL default=gen_random_uuid() + group_id: uuid NULL + app_id: uuid NULL + name: text NOT NULL + kind: text NOT NULL default='kv'::text + created_at: timestamp with time zone NOT NULL default=now() + updated_at: timestamp with time zone NOT NULL default=now() + +table: group_kv_entries + group_id: uuid NOT NULL + collection: text NOT NULL + key: text NOT NULL + value: jsonb NOT NULL + created_at: timestamp with time zone NOT NULL default=now() + updated_at: timestamp with time zone NOT NULL default=now() + table: group_members group_id: uuid NOT NULL user_id: uuid NOT NULL @@ -485,6 +502,17 @@ indexes on files: indexes on files_trigger_details: files_trigger_details_pkey: public.files_trigger_details USING btree (trigger_id) +indexes on group_collections: + group_collections_app_id_idx: public.group_collections USING btree (app_id) WHERE (app_id IS NOT NULL) + group_collections_app_uidx: public.group_collections USING btree (app_id, lower(name), kind) WHERE (app_id IS NOT NULL) + group_collections_group_id_idx: public.group_collections USING btree (group_id) WHERE (group_id IS NOT NULL) + group_collections_group_uidx: public.group_collections USING btree (group_id, lower(name), kind) WHERE (group_id IS NOT NULL) + group_collections_pkey: public.group_collections USING btree (id) + +indexes on group_kv_entries: + group_kv_entries_pkey: public.group_kv_entries USING btree (group_id, collection, key) + idx_group_kv_entries_group_collection: public.group_kv_entries USING btree (group_id, collection) + indexes on group_members: group_members_pkey: public.group_members USING btree (group_id, user_id) group_members_user_id_idx: public.group_members USING btree (user_id) @@ -685,6 +713,17 @@ constraints on files_trigger_details: [FOREIGN KEY] files_trigger_details_trigger_id_fkey: FOREIGN KEY (trigger_id) REFERENCES triggers(id) ON DELETE CASCADE [PRIMARY KEY] files_trigger_details_pkey: PRIMARY KEY (trigger_id) +constraints on group_collections: + [CHECK] group_collections_kind_check: CHECK ((kind = 'kv'::text)) + [CHECK] group_collections_owner_exactly_one: CHECK (((group_id IS NULL) <> (app_id IS NULL))) + [FOREIGN KEY] group_collections_app_id_fkey: FOREIGN KEY (app_id) REFERENCES apps(id) ON DELETE CASCADE + [FOREIGN KEY] group_collections_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE + [PRIMARY KEY] group_collections_pkey: PRIMARY KEY (id) + +constraints on group_kv_entries: + [FOREIGN KEY] group_kv_entries_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE + [PRIMARY KEY] group_kv_entries_pkey: PRIMARY KEY (group_id, collection, key) + constraints on group_members: [CHECK] group_members_role_check: CHECK ((role = ANY (ARRAY['app_admin'::text, 'editor'::text, 'viewer'::text]))) [FOREIGN KEY] group_members_group_id_fkey: FOREIGN KEY (group_id) REFERENCES groups(id) ON DELETE CASCADE @@ -822,3 +861,5 @@ constraints on vars: 0049: group secrets 0050: group scripts 0051: extension points + 0052: group collections + 0053: group kv entries