mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-12 01:45:14 +02:00
mr-668: derive any_per_graph_policy on RegistrySnapshot; simplify dup check
`AppState::requires_bearer_auth` walked the entire registry per
request (cloning Arcs into a `Vec`, then `.iter().any(|h| h.policy
.is_some())`) to decide whether the auth middleware should challenge.
The walk is unnecessary — the answer only changes when the registry
mutates, which is exactly the moment a new snapshot is constructed.
Move the flag onto the snapshot itself:
* `RegistrySnapshot { graphs, any_per_graph_policy: bool }`.
* `RegistrySnapshot::new(graphs)` is the only construction path —
it derives the flag from `graphs.values().any(|h| h.policy
.is_some())` so the cached value can't drift from the source data.
* `Default` delegates to `new(HashMap::new())`.
* `GraphRegistry::from_handles` and `insert` build snapshots via
`RegistrySnapshot::new(...)`.
* `GraphRegistry::snapshot_ref()` exposes the current snapshot
through an `arc_swap::Guard`; callers that need cached derived
state go through this accessor (callers that only want `graphs`
still use `list` / `get`).
`requires_bearer_auth` becomes one `ArcSwap::load` + bool read.
Also (drive-by, same file, same hunk): replace the dead
`if let Some(other) = seen_uris.get(...)` + `let _ = other;` pattern
in `from_handles` with a plain `seen_uris.contains_key(...)`.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
fc95eb8ff3
commit
ca474e23b1
2 changed files with 45 additions and 7 deletions
|
|
@ -483,8 +483,10 @@ impl AppState {
|
|||
return true;
|
||||
}
|
||||
// Any per-graph policy also requires auth — otherwise the
|
||||
// policy gate would receive unauthenticated requests.
|
||||
self.registry.list().iter().any(|h| h.policy.is_some())
|
||||
// policy gate would receive unauthenticated requests. The flag
|
||||
// is derived at snapshot construction (`RegistrySnapshot::new`)
|
||||
// so this is an O(1) ArcSwap load — not a per-request walk.
|
||||
self.registry.snapshot_ref().any_per_graph_policy
|
||||
}
|
||||
|
||||
fn authenticate_bearer_token(&self, provided_token: &str) -> Option<ResolvedActor> {
|
||||
|
|
|
|||
|
|
@ -49,9 +49,38 @@ pub struct GraphHandle {
|
|||
|
||||
/// Immutable snapshot of the registry's current state. Replaced atomically
|
||||
/// via `ArcSwap`; readers see a consistent view of all graphs without locking.
|
||||
#[derive(Default)]
|
||||
///
|
||||
/// Derived state (`any_per_graph_policy`) is computed at snapshot
|
||||
/// construction so request-time middleware doesn't have to walk the
|
||||
/// graph map every call. Construct only via [`RegistrySnapshot::new`]
|
||||
/// (or `Default`) so the field stays in sync with `graphs`.
|
||||
pub struct RegistrySnapshot {
|
||||
pub graphs: HashMap<GraphKey, Arc<GraphHandle>>,
|
||||
/// `true` iff any registered graph has a per-graph policy installed.
|
||||
/// Used by `AppState::requires_bearer_auth` to decide whether the
|
||||
/// auth middleware should challenge a request — a per-graph policy
|
||||
/// implies bearer auth is required even when no server-level tokens
|
||||
/// or policy are configured.
|
||||
pub any_per_graph_policy: bool,
|
||||
}
|
||||
|
||||
impl RegistrySnapshot {
|
||||
/// Build a snapshot from a graph map, deriving cached fields.
|
||||
/// The only construction path — direct struct-literal use elsewhere
|
||||
/// would let derived state drift from `graphs`.
|
||||
pub fn new(graphs: HashMap<GraphKey, Arc<GraphHandle>>) -> Self {
|
||||
let any_per_graph_policy = graphs.values().any(|h| h.policy.is_some());
|
||||
Self {
|
||||
graphs,
|
||||
any_per_graph_policy,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for RegistrySnapshot {
|
||||
fn default() -> Self {
|
||||
Self::new(HashMap::new())
|
||||
}
|
||||
}
|
||||
|
||||
/// Result of a registry lookup. Two-valued — `Tombstoned` deferred with DELETE.
|
||||
|
|
@ -99,19 +128,26 @@ impl GraphRegistry {
|
|||
if graphs.contains_key(&handle.key) {
|
||||
return Err(InsertError::DuplicateKey(handle.key.clone()));
|
||||
}
|
||||
if let Some(other) = seen_uris.get(&handle.uri) {
|
||||
let _ = other; // existing key shown in the error message via uri
|
||||
if seen_uris.contains_key(&handle.uri) {
|
||||
return Err(InsertError::DuplicateUri(handle.uri.clone()));
|
||||
}
|
||||
seen_uris.insert(handle.uri.clone(), handle.key.clone());
|
||||
graphs.insert(handle.key.clone(), handle);
|
||||
}
|
||||
Ok(Self {
|
||||
snapshot: ArcSwap::from_pointee(RegistrySnapshot { graphs }),
|
||||
snapshot: ArcSwap::from_pointee(RegistrySnapshot::new(graphs)),
|
||||
mutate: Mutex::new(()),
|
||||
})
|
||||
}
|
||||
|
||||
/// Lock-free snapshot read. Callers that need derived state cached
|
||||
/// on the snapshot (e.g. `any_per_graph_policy`) go through here;
|
||||
/// callers that only need values of `graphs` should use [`list`]
|
||||
/// or [`get`].
|
||||
pub fn snapshot_ref(&self) -> arc_swap::Guard<Arc<RegistrySnapshot>> {
|
||||
self.snapshot.load()
|
||||
}
|
||||
|
||||
/// Lock-free read. Returns `Ready` if the graph is in the current snapshot,
|
||||
/// `Gone` otherwise.
|
||||
pub fn get(&self, key: &GraphKey) -> RegistryLookup {
|
||||
|
|
@ -164,7 +200,7 @@ impl GraphRegistry {
|
|||
let mut new_graphs = current.graphs.clone();
|
||||
new_graphs.insert(handle.key.clone(), handle);
|
||||
self.snapshot
|
||||
.store(Arc::new(RegistrySnapshot { graphs: new_graphs }));
|
||||
.store(Arc::new(RegistrySnapshot::new(new_graphs)));
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue