From 45d928248cf104f202f3af348088e97d84e78443 Mon Sep 17 00:00:00 2001 From: Lewis Date: Mon, 21 Sep 2026 12:45:42 +0300 Subject: [PATCH] knot-types: rkey can be a TID *or* a nice slug Lewis: May this revision serve well! --- knot2/crates/knot-types/src/hex.rs | 10 +-- knot2/crates/knot-types/src/ids.rs | 99 ++++++++++++++++++++++----- knot2/crates/knot-types/src/lib.rs | 10 +-- knot2/crates/knot-types/src/record.rs | 48 +++++++++---- 4 files changed, 128 insertions(+), 39 deletions(-) diff --git a/knot2/crates/knot-types/src/hex.rs b/knot2/crates/knot-types/src/hex.rs index 1aaeaaf73..08c48bdb9 100644 --- a/knot2/crates/knot-types/src/hex.rs +++ b/knot2/crates/knot-types/src/hex.rs @@ -17,7 +17,9 @@ pub fn decode_hex(text: impl AsRef<[u8]>) -> Option> { let text = text.as_ref(); match text.len() % 2 { 0 => text - .chunks_exact(2) + .as_chunks::<2>() + .0 + .iter() .map(|pair| { let hi = (pair[0] as char).to_digit(16)?; let lo = (pair[1] as char).to_digit(16)?; @@ -51,13 +53,13 @@ mod tests { assert_eq!( decode_hex("abc"), None, - "an odd digit count can't form whole bytes" + "An odd digit count can't form whole bytes" ); - assert_eq!(decode_hex("zz"), None, "a non-hex digit decodes to nothing"); + assert_eq!(decode_hex("zz"), None, "A non-hex digit decodes to nothing"); assert_eq!( decode_hex("AB").unwrap(), [0xab], - "uppercase input still decodes even though the encoder emits lowercase" + "Uppercase input still decodes even though the encoder emits lowercase" ); } } diff --git a/knot2/crates/knot-types/src/ids.rs b/knot2/crates/knot-types/src/ids.rs index 04316be6b..5721efbc2 100644 --- a/knot2/crates/knot-types/src/ids.rs +++ b/knot2/crates/knot-types/src/ids.rs @@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)] pub enum ParseError { - #[error("invalid {kind}: {value:?}")] + #[error("Invalid {kind}: {value:?}")] Invalid { kind: &'static str, value: String }, } @@ -29,8 +29,12 @@ impl Did { } } +#[macro_export] macro_rules! string_id { (@traits $name:ident) => { + $crate::string_id!(@traits_for $name, ParseError); + }; + (@traits_for $name:ident, $err:ty) => { impl PartialEq for $name { fn eq(&self, other: &Self) -> bool { self.as_str() == other.as_str() @@ -78,7 +82,7 @@ macro_rules! string_id { } impl FromStr for $name { - type Err = ParseError; + type Err = $err; fn from_str(s: &str) -> Result { Self::new(s) @@ -122,7 +126,7 @@ macro_rules! string_id { } } - string_id!(@traits $name); + $crate::string_id!(@traits $name); }; ($name:ident, $label:literal, $parse:path) => { #[derive(Clone)] @@ -139,7 +143,23 @@ macro_rules! string_id { } } - string_id!(@traits $name); + $crate::string_id!(@traits $name); + }; + ($name:ident, try $new:path, err $err:ty) => { + #[derive(Clone)] + pub struct $name(String); + + impl $name { + pub fn new(value: impl Into) -> Result { + $new(value) + } + + pub fn as_str(&self) -> &str { + &self.0 + } + } + + $crate::string_id!(@traits_for $name, $err); }; } @@ -244,6 +264,17 @@ fn parse_tid(value: &str) -> Option { Tid::new(value).ok() } +fn parse_label_key(value: &str) -> Option { + let slug = !value.is_empty() + && value.len() <= MAX_LABEL_KEY_BYTES + && parse_tid(value).is_none() + && value.bytes().all(|byte| { + matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'.' | b'-' | b'_' | b'~') + }); + slug.then(|| LabelKey(value.to_string())) +} +const MAX_LABEL_KEY_BYTES: usize = 64; + fn parse_record_collection(value: &str) -> Option> { let nsid = parse_nsid(value)?; @@ -385,7 +416,7 @@ string_id!(UriRkey, "at-uri record key", via parse_rkey => Rkey); string_id!(RefName, "ref name", parse_ref_name); string_id!(TypeName, "COB type name", via parse_nsid => Nsid); string_id!(RecordCollection, "record collection", via parse_record_collection => Nsid); -string_id!(RecordRkey, "record key", via parse_tid => Tid); +string_id!(LabelKey, "label def key", parse_label_key); string_id!(ActorId, "actor public key", parse_multikey); string_id!(KnotHostname, "knot hostname", parse_knot_hostname); string_id!(BranchName, "branch name", parse_branch_name); @@ -404,9 +435,45 @@ impl From<&DidRkey> for RepoDid { impl RecordRkey { pub fn minted(tid: Tid) -> Self { - Self(tid) + Self::Minted(tid) + } + + pub fn composed(key: LabelKey) -> Self { + Self::Composed(key) + } + + pub fn as_label_key(&self) -> Option<&LabelKey> { + match self { + Self::Composed(key) => Some(key), + Self::Minted(_) => None, + } } } + +#[derive(Clone)] +pub enum RecordRkey { + Minted(Tid), + Composed(LabelKey), +} + +impl RecordRkey { + pub fn new(value: impl Into) -> Result { + let value = value.into(); + match parse_tid(&value) { + Some(tid) => Ok(Self::Minted(tid)), + None => LabelKey::new(value).map(Self::Composed), + } + } + + pub fn as_str(&self) -> &str { + match self { + Self::Minted(tid) => tid.as_str(), + Self::Composed(key) => key.as_str(), + } + } +} + +string_id!(@traits RecordRkey); #[derive(Debug, Clone, Default, PartialEq, Eq, serde::Serialize)] #[serde(transparent)] pub struct PushOptions(Vec); @@ -504,21 +571,21 @@ impl KnotServiceUrl { impl KnotHostname { pub fn knot_did(&self) -> KnotId { - KnotId::new(format!("did:web:{}", self.0)).expect("validated knot hostname forms did:web") + KnotId::new(format!("did:web:{}", self.0)).expect("Validated knot hostname forms did:web") } } impl BranchName { pub fn head_ref(&self) -> RefName { RefName::new(format!("refs/heads/{}", self.0)) - .expect("validated branch name forms refs/heads ref") + .expect("Validated branch name forms refs/heads ref") } } impl TagName { pub fn tag_ref(&self) -> RefName { RefName::new(format!("refs/tags/{}", self.0)) - .expect("validated tag name forms refs/tags ref") + .expect("Validated tag name forms refs/tags ref") } } @@ -1046,7 +1113,7 @@ mod tests { fn uri_rkeys_accept_record_key_alphabet_and_reject_dot_and_dotdot() { assert!( UriRkey::new("refs~1").is_ok(), - "an escaped git ref should be a rkey" + "An escaped git ref should be an rkey" ); assert!(UriRkey::new("3lubrptx57d22").is_ok()); assert!(UriRkey::new(".").is_err()); @@ -1146,7 +1213,7 @@ mod tests { assert_eq!( plus.names().cloned().collect::>(), vec![RepoName::new("c++").unwrap()], - "a repo name accepts the wider charset, so the segment resolves by name" + "A repo name accepts the wider charset, so the segment resolves by name" ); let long = "x".repeat(200); @@ -1230,7 +1297,7 @@ mod tests { #[test] fn oid_roundtrips_through_hex() { let hex = "0123456789abcdef0123456789abcdef01234567"; - let oid = Oid::from_hex(hex).expect("valid sha1 hex"); + let oid = Oid::from_hex(hex).expect("Valid SHA-1 hex"); assert_eq!(oid.to_hex(), hex); assert_eq!(oid, Oid::from(oid.object_id())); assert!(Oid::from_hex("zz").is_err()); @@ -1487,7 +1554,7 @@ mod prop_tests { if let Ok(value) = <$ty>::new(raw.clone()) { prop_assert_eq!(value.as_str(), raw.as_str()); let reparsed = <$ty>::new(value.to_string()) - .expect("display output reparses"); + .expect("Display output reparses"); prop_assert_eq!(reparsed, value); } } @@ -1528,9 +1595,9 @@ mod prop_tests { proptest! { #[test] fn oid_hex_identity(hex in "[0-9a-f]{40}") { - let oid = Oid::from_hex(&hex).expect("forty lowercase hex chars are valid sha1"); + let oid = Oid::from_hex(&hex).expect("Forty lowercase hex chars are valid SHA-1"); prop_assert_eq!(oid.to_hex(), hex.clone()); - prop_assert_eq!(Oid::from_hex(&oid.to_hex()).expect("reparse"), oid); + prop_assert_eq!(Oid::from_hex(&oid.to_hex()).expect("Reparse"), oid); } #[test] @@ -1538,7 +1605,7 @@ mod prop_tests { let sec1: Vec = std::iter::once(tag).chain(body).collect(); let actor = ActorId::from_secp256k1(&sec1); let encoded = actor.as_str().to_string(); - let reparsed = ActorId::new(encoded.clone()).expect("multikey output reparses"); + let reparsed = ActorId::new(encoded.clone()).expect("Multikey output reparses"); prop_assert_eq!(reparsed.as_str(), encoded.as_str()); prop_assert_eq!(reparsed, actor); } diff --git a/knot2/crates/knot-types/src/lib.rs b/knot2/crates/knot-types/src/lib.rs index b74f6ce91..3104e3af8 100644 --- a/knot2/crates/knot-types/src/lib.rs +++ b/knot2/crates/knot-types/src/lib.rs @@ -7,11 +7,11 @@ pub use changes::{ChangedFiles, ChangedFilesBudget, Listing}; mod ids; pub use ids::{ AccountDid, ActorId, AppviewEndpoint, AuthorName, BranchName, ChangeId, CiLogsAddr, ClonePath, - CobId, DidRkey, Email, HttpStatus, KnotHostname, KnotId, KnotServiceUrl, LanguageBytes, - LanguageName, LogsHost, LogsPort, ObjectCount, ObjectFormat, OfferedKey, Oid, OriginUrl, - OwnerDid, OwnerRef, ParseError, PushOption, PushOptions, RecordCollection, RecordRkey, RefName, - RefTransition, RepoDid, RepoName, RepoPath, RepoRkey, ServiceDid, TagName, TypeName, - UnixMicros, UnixSeconds, UriRkey, + CobId, DidRkey, Email, HttpStatus, KnotHostname, KnotId, KnotServiceUrl, LabelKey, + LanguageBytes, LanguageName, LogsHost, LogsPort, ObjectCount, ObjectFormat, OfferedKey, Oid, + OriginUrl, OwnerDid, OwnerRef, ParseError, PushOption, PushOptions, RecordCollection, + RecordRkey, RefName, RefTransition, RepoDid, RepoName, RepoPath, RepoRkey, ServiceDid, TagName, + TypeName, UnixMicros, UnixSeconds, UriRkey, }; mod record; diff --git a/knot2/crates/knot-types/src/record.rs b/knot2/crates/knot-types/src/record.rs index 1918c3a1d..ecfe39f5f 100644 --- a/knot2/crates/knot-types/src/record.rs +++ b/knot2/crates/knot-types/src/record.rs @@ -20,11 +20,11 @@ pub const MAX_RECORD_BYTES: usize = 1_000_000; const SHA2_256_BYTES: usize = 32; #[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] pub enum AddressParseError { - #[error("uri authority has to be a repository DID")] + #[error("URI authority has to be a repository DID")] Authority, - #[error("collection is missing from the uri or isn't a record collection")] + #[error("Collection is missing from the URI or isn't a record collection")] Collection, - #[error("the uri's record key is missing, or it isn't a TID")] + #[error("The URI's record key is missing, or rkey isn't a TID or label key")] RecordKey, } @@ -57,7 +57,7 @@ impl RecordAddress { pub fn at_uri(&self, repo: &RepoDid) -> AtUri { AtUri::from_parts_owned(repo.as_str(), self.collection.as_str(), self.rkey.as_str()) - .expect("Repo DID, collection nsid and TID compose at-uri") + .expect("Repo DID, collection NSID and record key compose at-uri") } pub fn parse_at_uri(uri: &AtUri) -> Result<(RepoDid, Self), AddressParseError> { @@ -82,7 +82,7 @@ impl fmt::Display for RecordAddress { } #[derive(Debug, thiserror::Error)] -#[error("Mst key of {0} bytes is over {MST_KEY_MAX_BYTES} byte limit")] +#[error("MST key of {0} bytes is over {MST_KEY_MAX_BYTES} byte limit")] pub struct MstKeyTooLong(pub usize); #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] @@ -196,7 +196,7 @@ pub enum RecordBodyError { #[error("Record body is dag-cbor and these bytes aren't: {0}")] NotDagCbor(String), #[error( - "record body is canonical dag-cbor, and these {0} bytes aren't canonical encoding of their value" + "Record body is canonical dag-cbor, and these {0} bytes aren't canonical encoding of their value" )] NotCanonical(usize), #[error("Record body of {0} bytes is over {MAX_RECORD_BYTES} byte limit")] @@ -275,7 +275,7 @@ mod tests { assert_eq!( RecordAddress::parse_at_uri(&crate::AtUri::new_owned("at://did:plc:squid").unwrap()), Err(super::AddressParseError::Collection), - "an at-uri without a collection shouldn't parse as a record address" + "An at-uri without a collection shouldn't parse as a record address" ); } @@ -294,13 +294,33 @@ mod tests { } #[test] - fn a_record_key_parses_only_a_tid() { + fn record_key_parses_as_a_tid_or_a_composed_slug() { assert!( - RecordRkey::new("self").is_err(), - "Knot-minted record key is TID, and `self` is the shape DidRkey takes" + RecordRkey::new("3lubrptx57d22").is_ok_and(|rkey| rkey.as_label_key().is_none()), + "a knot-minted record key is a TID" + ); + assert!( + RecordRkey::new("wont-fix.2").is_ok_and(|rkey| rkey.as_label_key().is_some()), + "A label def's record key is the slug the owner composed" + ); + assert!( + RecordRkey::new("self").is_ok_and(|rkey| rkey.as_label_key().is_some()), + "`self` is a slug here, and a DID can't spell a slug: the colon isn't in the slug \ + charset" ); assert!(RecordRkey::new("did:plc:squid").is_err()); - assert!(RecordRkey::new("3lubrptx57d2").is_err()); + assert!(RecordRkey::new("").is_err()); + assert!( + crate::LabelKey::new("3lubrptx57d22").is_err(), + "a key that spells a TID parses as a TID, so a TID-shaped slug never reaches the \ + composed arm" + ); + let composed = RecordRkey::composed(crate::LabelKey::new("wont-fix.2").unwrap()); + assert!( + RecordRkey::new(composed.as_str()) + .is_ok_and(|reparsed| reparsed.as_label_key().is_some()), + "A composed key re-parses as composed, whatever string boundary the round trip crossed" + ); } #[test] @@ -316,7 +336,7 @@ mod tests { ); assert!( serde_json::from_str::("0").is_err(), - "number off wire goes through the same door, as one built in process" + "Number off wire goes through the same door, as one built in process" ); assert_eq!( serde_json::from_str::("7").unwrap(), @@ -371,7 +391,7 @@ mod tests { ); assert!( RecordBody::new([body.as_bytes(), b"trailing"].concat()).is_err(), - "trailing bytes don't decode, dag-cbor is one complete value" + "Trailing bytes don't decode; dag-cbor is one complete value" ); } @@ -415,7 +435,7 @@ mod tests { assert_eq!( door_out_of(&[0x05]), "accepted", - "canonical spelling, of the same integer, is the one that stands" + "Canonical spelling of the same integer is the one that stands" ); } -- 2.51.2