jobq-wire: name every outcome, keep the enum spellings, document the schema
Three things, all from review: Accepted outcomes were built from a hand-listed [Done, Failed, Cancelled, Skipped] array. Exhaustive today, silently short the day someone adds a variant — the new outcome would vanish from every edge that accepts it. BitFlags::ALL asks the type instead. TerminalState carried rename_all = "snake_case" while its sibling State did not, so one enum shipped "done" and the other "Done". A rename is a second spelling of a name that then has to be kept in agreement by hand; both now serialise their variant names verbatim. Nothing else reads TerminalState off a wire, so no consumer moves. GraphDep's tag values likewise. The endpoint documented its body as serde_json::Value, which tells a spec reader nothing. hive-jobq-wire now derives ToSchema. State and TerminalState are foreign types here and utoipa stays out of the scheduler crate, so the schema points at local mirror enums. A mirror that drifts is worse than none: the conversions are exhaustive (a new upstream variant fails the build) and a test asserts each documented name equals the serialised one, since an exhaustive match still compiles when only the spellings diverge.
This commit is contained in:
parent
7966d5eb66
commit
fe0906c043
5 changed files with 148 additions and 18 deletions
|
|
@ -9,6 +9,10 @@ workspace = true
|
|||
|
||||
[dependencies]
|
||||
chrono = { workspace = true }
|
||||
enumflags2 = { workspace = true }
|
||||
hive-jobq = { workspace = true }
|
||||
serde = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
# `chrono` for the lifecycle timestamps' schema; the workspace default carries
|
||||
# only `axum_extras`, and features are additive so this affects nothing else.
|
||||
utoipa = { workspace = true, features = ["chrono"] }
|
||||
|
|
|
|||
|
|
@ -28,8 +28,74 @@
|
|||
//! [`Node`]: hive_jobq::Node
|
||||
|
||||
use chrono::{DateTime, Utc};
|
||||
use enumflags2::BitFlags;
|
||||
use hive_jobq::{Dep, Graph, NodeId, State, TerminalState};
|
||||
use serde::{Deserialize, Serialize};
|
||||
use utoipa::ToSchema;
|
||||
|
||||
/// `OpenAPI` mirror of [`hive_jobq::State`].
|
||||
///
|
||||
/// Exists **only** so the generated spec can enumerate the states: `State` is
|
||||
/// a foreign type, so neither `ToSchema` nor a newtype around it can be
|
||||
/// implemented here, and `utoipa` is deliberately not a dependency of the
|
||||
/// scheduler crate. Referenced via `#[schema(value_type = …)]`; nothing is ever
|
||||
/// serialised through it, so it cannot change the wire.
|
||||
///
|
||||
/// [`StateSchema::of`] is what keeps it honest — an exhaustive match, so adding
|
||||
/// a variant upstream **fails to compile here** instead of quietly leaving the
|
||||
/// documented enum short.
|
||||
#[derive(Debug, Clone, Copy, ToSchema)]
|
||||
pub enum StateSchema {
|
||||
Pending,
|
||||
Running,
|
||||
Finishing,
|
||||
Done,
|
||||
Failed,
|
||||
Cancelled,
|
||||
Skipped,
|
||||
}
|
||||
|
||||
impl StateSchema {
|
||||
/// The mirror variant for `state`. Public because the mirror type is: a
|
||||
/// consumer that renders the documented enum needs the same mapping, and a
|
||||
/// constructor nobody outside can call is a schema nobody can check.
|
||||
#[must_use]
|
||||
pub fn of(state: State) -> Self {
|
||||
match state {
|
||||
State::Pending => Self::Pending,
|
||||
State::Running => Self::Running,
|
||||
State::Finishing => Self::Finishing,
|
||||
State::Done => Self::Done,
|
||||
State::Failed => Self::Failed,
|
||||
State::Cancelled => Self::Cancelled,
|
||||
State::Skipped => Self::Skipped,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `OpenAPI` mirror of [`hive_jobq::TerminalState`] — same rationale and same
|
||||
/// exhaustive-match guard as [`StateSchema`].
|
||||
#[derive(Debug, Clone, Copy, ToSchema)]
|
||||
pub enum TerminalStateSchema {
|
||||
Done,
|
||||
Failed,
|
||||
Cancelled,
|
||||
Skipped,
|
||||
}
|
||||
|
||||
impl TerminalStateSchema {
|
||||
/// The mirror variant for `outcome`. Public for the same reason as
|
||||
/// [`StateSchema::of`].
|
||||
#[must_use]
|
||||
pub fn of(outcome: TerminalState) -> Self {
|
||||
match outcome {
|
||||
TerminalState::Done => Self::Done,
|
||||
TerminalState::Failed => Self::Failed,
|
||||
TerminalState::Cancelled => Self::Cancelled,
|
||||
TerminalState::Skipped => Self::Skipped,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Node id, carried verbatim from [`hive_jobq::NodeId`]: globally unique across
|
||||
/// the whole graph, not per group. Opaque to consumers — they group by
|
||||
|
|
@ -67,8 +133,9 @@ pub trait WireResource {
|
|||
}
|
||||
|
||||
/// One node, serialised near-raw from [`hive_jobq::Node`].
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
pub struct GraphNode {
|
||||
/// Unique across the whole graph, not per group.
|
||||
pub id: WireId,
|
||||
/// Structural parent, or `None` for a group root.
|
||||
///
|
||||
|
|
@ -77,7 +144,12 @@ pub struct GraphNode {
|
|||
/// `state` answers "how is this whole group doing".
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub parent: Option<WireId>,
|
||||
/// Lifecycle state. On a group root this is also the subtree's answer:
|
||||
/// `Finishing` = own logic done, children still running; a terminal value
|
||||
/// is the rolled-up outcome.
|
||||
#[schema(value_type = StateSchema)]
|
||||
pub state: State,
|
||||
/// What must hold before this node runs. Omitted when empty.
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub deps: Vec<GraphDep>,
|
||||
/// When the node entered `Running`. `None` until it starts; a node that
|
||||
|
|
@ -95,7 +167,7 @@ pub struct GraphNode {
|
|||
}
|
||||
|
||||
/// What a node *is*, in terms the graph layer does not interpret.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
pub struct NodePayload {
|
||||
/// From [`WireNode::label`].
|
||||
pub label: String,
|
||||
|
|
@ -107,8 +179,12 @@ pub struct NodePayload {
|
|||
}
|
||||
|
||||
/// What must hold before a node runs.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
#[serde(rename_all = "snake_case", tag = "kind")]
|
||||
///
|
||||
/// Externally tagged on `kind`, whose values are the **variant names verbatim**
|
||||
/// (`"Node"`, `"Resource"`) — no `rename_all`. A rename is a second spelling of
|
||||
/// the same name that has to be kept in agreement with the Rust one by hand.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
#[serde(tag = "kind")]
|
||||
pub enum GraphDep {
|
||||
/// Depend on another node finishing acceptably.
|
||||
Node {
|
||||
|
|
@ -122,6 +198,10 @@ pub enum GraphDep {
|
|||
/// node, distinguished only by which outcomes each accepts (one for
|
||||
/// `Done`, one for `Failed`/`Cancelled`, …). Collapsing this to a
|
||||
/// boolean renders those as identical nodes.
|
||||
///
|
||||
/// Every accepted outcome is **named**, so a client never shifts bits
|
||||
/// to read an edge.
|
||||
#[schema(value_type = Vec<TerminalStateSchema>)]
|
||||
accepts: Vec<TerminalState>,
|
||||
},
|
||||
/// Need `count` units of a named resource, per [`WireResource::name`].
|
||||
|
|
@ -188,15 +268,14 @@ fn wire_dep<R: WireResource>(dep: &Dep<R>) -> GraphDep {
|
|||
match dep {
|
||||
Dep::Node { id, when } => GraphDep::Node {
|
||||
id: id.get(),
|
||||
accepts: [
|
||||
TerminalState::Done,
|
||||
TerminalState::Failed,
|
||||
TerminalState::Cancelled,
|
||||
TerminalState::Skipped,
|
||||
]
|
||||
.into_iter()
|
||||
.filter(|outcome| when.accepts(*outcome))
|
||||
.collect(),
|
||||
// Every outcome the type knows about, not a hand-listed set: a
|
||||
// variant added to `TerminalState` is then named on the wire
|
||||
// automatically, where a literal array would have silently
|
||||
// dropped it from every edge that accepts it.
|
||||
accepts: BitFlags::<TerminalState>::ALL
|
||||
.iter()
|
||||
.filter(|outcome| when.accepts(*outcome))
|
||||
.collect(),
|
||||
},
|
||||
Dep::Resource { name, count } => GraphDep::Resource {
|
||||
name: name.name(),
|
||||
|
|
@ -212,9 +291,51 @@ mod tests {
|
|||
use hive_jobq::scheduler::Scheduler;
|
||||
|
||||
use super::{
|
||||
GraphDep, GraphNode, GraphWire, NodePayload, State, TerminalState, WireId, WireNode,
|
||||
BitFlags, GraphDep, GraphNode, GraphWire, NodePayload, State, StateSchema, TerminalState,
|
||||
TerminalStateSchema, WireId, WireNode,
|
||||
};
|
||||
|
||||
/// The mirrors document what the wire actually says — a mirror that
|
||||
/// disagreed would be worse than none, since the spec is all a consumer
|
||||
/// has.
|
||||
///
|
||||
/// `StateSchema::of` is the *compile-time* half (exhaustive, so a new
|
||||
/// upstream variant breaks the build). This is the *value* half: the
|
||||
/// documented variant name has to equal the serialised one. Both are
|
||||
/// needed — an exhaustive match still compiles if the names diverge, which
|
||||
/// is exactly what a stray `rename_all` upstream would do.
|
||||
#[test]
|
||||
fn the_openapi_mirrors_name_states_exactly_as_the_wire_does() {
|
||||
let states = [
|
||||
State::Pending,
|
||||
State::Running,
|
||||
State::Finishing,
|
||||
State::Done,
|
||||
State::Failed,
|
||||
State::Cancelled,
|
||||
State::Skipped,
|
||||
];
|
||||
for state in states {
|
||||
let on_the_wire = serde_json::to_string(&state).expect("serialises");
|
||||
let documented = format!("{:?}", StateSchema::of(state));
|
||||
assert_eq!(
|
||||
on_the_wire,
|
||||
format!("\"{documented}\""),
|
||||
"{state:?} is documented as {documented} but serialises as {on_the_wire}"
|
||||
);
|
||||
}
|
||||
|
||||
for outcome in BitFlags::<TerminalState>::ALL {
|
||||
let on_the_wire = serde_json::to_string(&outcome).expect("serialises");
|
||||
let documented = format!("{:?}", TerminalStateSchema::of(outcome));
|
||||
assert_eq!(
|
||||
on_the_wire,
|
||||
format!("\"{documented}\""),
|
||||
"{outcome:?} is documented as {documented} but serialises as {on_the_wire}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// A scheduler over the test payloads, built through the same public API a
|
||||
/// host uses.
|
||||
///
|
||||
|
|
|
|||
Loading…
Reference in a new issue