diff --git a/crates/didbot-pds/src/layout.rs b/crates/didbot-pds/src/layout.rs index e8e1ffaf..d98f02d7 100644 --- a/crates/didbot-pds/src/layout.rs +++ b/crates/didbot-pds/src/layout.rs @@ -59,16 +59,14 @@ //! `every_entry_variant_matches_its_declared_shape` constructs one sample of //! *every* variant, serializes each, and checks the real JSON object keys //! against `ENTRY_SHAPE`'s row for it — catching a field that is missing, an -//! extra one, or one misspelled relative to what actually reaches the wire -//! (struct-variant fields are **not** camelCased by this enum's -//! `rename_all`, only variant names are — `token_hash`, `expires_at` and -//! `mime_type` stay snake_case on disk, which an earlier version of this -//! table got wrong for the first two and a narrower version of this test -//! did not catch, because it only sampled the variants an integration test -//! happened to exercise through the server). An `Entry` change that forgets -//! to update `ENTRY_SHAPE` fails one of these two tests; one that updates it -//! changes [`SHAPE`] automatically. There is no step for a person to -//! remember. +//! extra one, or one misspelled relative to what actually reaches the wire. +//! Beside it, `every_entry_variant_writes_camel_case_at_every_depth` walks +//! the same samples and asserts every key at every depth is camelCase, so a +//! struct added to an entry's payload without `rename_all` is a failing test +//! rather than the one file in the deployment writing snake_case. An `Entry` +//! change that forgets to update `ENTRY_SHAPE` fails one of these tests; one +//! that updates it changes [`SHAPE`] automatically. There is no step for a +//! person to remember. //! //! # The other half: what the files are called //! @@ -149,7 +147,7 @@ pub const ENTRY_SHAPE: &[(&str, &[&str])] = &[ ("accountStateChanged", &["did", "op", "state"]), ( "accountTokenIssued", - &["did", "expires_at", "op", "token_hash"], + &["did", "expiresAt", "op", "tokenHash"], ), ("accountTokenRevoked", &["did", "op"]), ("accountUnlocked", &["did", "lock", "op", "party"]), @@ -157,7 +155,7 @@ pub const ENTRY_SHAPE: &[(&str, &[&str])] = &[ ("blobReferenced", &["cid", "did", "op", "refs"]), ( "blobUploaded", - &["at", "cid", "did", "mime_type", "op", "size"], + &["at", "cid", "did", "mimeType", "op", "size"], ), ("checkpointSealed", &["offset", "op", "segment"]), ("counterAdvanced", &["next", "op"]), @@ -168,15 +166,15 @@ pub const ENTRY_SHAPE: &[(&str, &[&str])] = &[ ( "oauthGrantIssued", &[ - "access_expires_at", - "access_hash", - "client_id", + "accessExpiresAt", + "accessHash", + "clientId", "did", - "dpop_thumbprint", + "dpopThumbprint", "family", "op", - "refresh_expires_at", - "refresh_hash", + "refreshExpiresAt", + "refreshHash", "scope", ], ), @@ -184,12 +182,12 @@ pub const ENTRY_SHAPE: &[(&str, &[&str])] = &[ ( "oauthGrantRotated", &[ - "access_expires_at", - "access_hash", + "accessExpiresAt", + "accessHash", "family", "op", - "refresh_expires_at", - "refresh_hash", + "refreshExpiresAt", + "refreshHash", ], ), ("recordRemoved", &["collection", "did", "op", "rkey"]), diff --git a/crates/didbot-pds/src/wal/mod.rs b/crates/didbot-pds/src/wal/mod.rs index 080d416c..4385e114 100644 --- a/crates/didbot-pds/src/wal/mod.rs +++ b/crates/didbot-pds/src/wal/mod.rs @@ -342,7 +342,11 @@ mod key_hex { /// nothing has to be merged. The `Debug` is derived and stays safe because /// [`SigningKey`]'s own `Debug` renders its public half; there is a test. #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -#[serde(tag = "op", rename_all = "camelCase")] +// `rename_all` renames the variants and `rename_all_fields` renames the +// fields inside them. Both are needed: without the second, `tokenHash` and +// `mimeType` would go out as `token_hash` and `mime_type`, and every JSON +// this server emits is camelCase. +#[serde(tag = "op", rename_all = "camelCase", rename_all_fields = "camelCase")] pub enum Entry { /// An account was provisioned, with the key it signs as. AccountInserted { @@ -2853,18 +2857,106 @@ mod tests { /// with no row, is caught the same way a field mismatch is. #[test] fn every_entry_variant_matches_its_declared_shape() { + let samples = one_of_every_variant(); + let mut seen: BTreeMap> = BTreeMap::new(); + for entry in &samples { + let value = serde_json::to_value(entry).expect("an entry serializes"); + let object = value.as_object().expect("an entry is an object"); + let op = object["op"] + .as_str() + .expect("every entry is tagged") + .to_owned(); + let mut keys: Vec = object.keys().cloned().collect(); + keys.sort(); + assert!( + seen.insert(op.clone(), keys).is_none(), + "two samples both serialized as `{op}`" + ); + } + + let declared: BTreeMap<&str, Vec> = crate::layout::ENTRY_SHAPE + .iter() + .map(|(op, fields)| { + ( + *op, + fields.iter().map(|field| (*field).to_owned()).collect(), + ) + }) + .collect(); + + let seen_ops: Vec<&str> = seen.keys().map(String::as_str).collect(); + let declared_ops: Vec<&str> = declared.keys().copied().collect(); + assert_eq!( + seen_ops, declared_ops, + "`layout::ENTRY_SHAPE` and the samples above must name exactly the same \ + variants — add a row for a variant with no sample, or a sample for a row \ + with none" + ); + + for (op, keys) in &seen { + assert_eq!( + keys, + &declared[op.as_str()], + "`{op}`'s real fields do not match its row in `layout::ENTRY_SHAPE`" + ); + } + } + + /// Every key the log writes, at every depth, is camelCase. + /// + /// The enum's own `rename_all_fields` covers an entry's own fields. What + /// it does not cover is a struct inside one — an account, a ledger entry, + /// a retention — and each of those carries its own `rename_all`. A struct + /// added without one would make the log the single file in the deployment + /// writing snake_case, and the log is the one file whose keys are + /// permanent. + #[test] + fn every_entry_variant_writes_camel_case_at_every_depth() { + fn walk(value: &serde_json::Value, at: &str, op: &str) { + match value { + serde_json::Value::Object(fields) => { + for (key, held) in fields { + assert!( + key.chars().next().is_some_and(char::is_lowercase) + && key.chars().all(|c| c.is_ascii_alphanumeric()), + "`{op}` writes `{at}{key}`, which is not camelCase" + ); + walk(held, &format!("{at}{key}."), op); + } + } + serde_json::Value::Array(held) => { + for one in held { + walk(one, at, op); + } + } + _ => {} + } + } + + for entry in one_of_every_variant() { + let value = serde_json::to_value(&entry).expect("an entry serializes"); + let op = value["op"] + .as_str() + .expect("every entry is tagged") + .to_owned(); + walk(&value, "", &op); + } + } + + /// One instance of every `Entry` variant. + /// + /// Each is the cheapest value that satisfies its field types — none of + /// this has to be a value the rest of the server would accept, because + /// what the tests above check is the wire shape rather than any business + /// rule. + fn one_of_every_variant() -> Vec { let now = OffsetDateTime::now_utc(); let did = crate::hosted::HostedDid::replayed( didbot_identity::AccountDid::parse("did:web:shape.example").expect("a valid did"), ); let account = HostedAccount::server(did.clone(), now); let key = SigningKey::generate(); - - // One instance of every variant. Each is the cheapest value that - // satisfies its field types — none of this has to be a value the - // rest of the server would accept, because what is being checked is - // the wire shape, not any business rule. - let samples: Vec = vec![ + vec![ Entry::AccountInserted { account: Box::new(account), key, @@ -3004,49 +3096,6 @@ mod tests { segment: 1, offset: 1, }, - ]; - - let mut seen: BTreeMap> = BTreeMap::new(); - for entry in &samples { - let value = serde_json::to_value(entry).expect("an entry serializes"); - let object = value.as_object().expect("an entry is an object"); - let op = object["op"] - .as_str() - .expect("every entry is tagged") - .to_owned(); - let mut keys: Vec = object.keys().cloned().collect(); - keys.sort(); - assert!( - seen.insert(op.clone(), keys).is_none(), - "two samples both serialized as `{op}`" - ); - } - - let declared: BTreeMap<&str, Vec> = crate::layout::ENTRY_SHAPE - .iter() - .map(|(op, fields)| { - ( - *op, - fields.iter().map(|field| (*field).to_owned()).collect(), - ) - }) - .collect(); - - let seen_ops: Vec<&str> = seen.keys().map(String::as_str).collect(); - let declared_ops: Vec<&str> = declared.keys().copied().collect(); - assert_eq!( - seen_ops, declared_ops, - "`layout::ENTRY_SHAPE` and the samples above must name exactly the same \ - variants — add a row for a variant with no sample, or a sample for a row \ - with none" - ); - - for (op, keys) in &seen { - assert_eq!( - keys, - &declared[op.as_str()], - "`{op}`'s real fields do not match its row in `layout::ENTRY_SHAPE`" - ); - } + ] } } diff --git a/crates/didbot-pds/tests/durability.rs b/crates/didbot-pds/tests/durability.rs index aecf77ab..4aa0aefe 100644 --- a/crates/didbot-pds/tests/durability.rs +++ b/crates/didbot-pds/tests/durability.rs @@ -2782,8 +2782,8 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ ), ( "accountTokenIssued", - r#"{"op":"accountTokenIssued","did":"did:web:a.agents.localhost","token_hash":"ab01", - "expires_at":"2026-06-01T00:00:00Z"}"#, + r#"{"op":"accountTokenIssued","did":"did:web:a.agents.localhost","tokenHash":"ab01", + "expiresAt":"2026-06-01T00:00:00Z"}"#, ), ( "accountTokenRevoked", @@ -2798,7 +2798,7 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ "blobUploaded", r#"{"op":"blobUploaded","did":"did:web:a.agents.localhost", "cid":"bafkreih2fxeo453tle5v67nikccodt7v2ta3xpjz7rcdb2ksr64cgzfn2q", - "mime_type":"text/plain","size":10,"at":"2026-01-01T00:00:00Z"}"#, + "mimeType":"text/plain","size":10,"at":"2026-01-01T00:00:00Z"}"#, ), ("counterAdvanced", r#"{"op":"counterAdvanced","next":41}"#), ( @@ -2820,10 +2820,10 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ ( "oauthGrantIssued", r#"{"op":"oauthGrantIssued","family":7,"did":"did:web:a.agents.localhost", - "client_id":"https://client.example/id.json","scope":"atproto", - "dpop_thumbprint":"quernstone","access_hash":"ab01","refresh_hash":"cd02", - "access_expires_at":"2026-01-01T00:05:00Z", - "refresh_expires_at":"2026-01-15T00:00:00Z"}"#, + "clientId":"https://client.example/id.json","scope":"atproto", + "dpopThumbprint":"quernstone","accessHash":"ab01","refreshHash":"cd02", + "accessExpiresAt":"2026-01-01T00:05:00Z", + "refreshExpiresAt":"2026-01-15T00:00:00Z"}"#, ), ( "oauthGrantRevoked", @@ -2831,9 +2831,9 @@ const WRITTEN_ENTRIES: &[(&str, &str)] = &[ ), ( "oauthGrantRotated", - r#"{"op":"oauthGrantRotated","family":7,"access_hash":"ef03","refresh_hash":"0a04", - "access_expires_at":"2026-01-01T00:10:00Z", - "refresh_expires_at":"2026-01-15T00:05:00Z"}"#, + r#"{"op":"oauthGrantRotated","family":7,"accessHash":"ef03","refreshHash":"0a04", + "accessExpiresAt":"2026-01-01T00:10:00Z", + "refreshExpiresAt":"2026-01-15T00:05:00Z"}"#, ), ( "recordRemoved",