Resolve review #003 nits N1, N3, N4
- N1: number_from_f64 now returns AlkTypeError::Access for non-finite floats instead of silently materializing Value::Null. A NaN/Inf in the buffer surfaces as a clear access error rather than a misleading 'expected a number' validation error. - N3: drop the dead Value::String arm from check_bytes; the materializer only ever emits bytes as an array of u8. Update the validation-model docs to match. - N4: fix stale ADR references in doc comments (ADR-096 -> ADR-002, ADR-097 -> ADR-003, ADR-098 -> ADR-004, ADR-101 -> ADR-007). N2 (BastType::alk_kind returning Struct for ) left as documented; every current caller resolves the ref first, so forcing Option through the call sites is churn without benefit. Verification: cargo test --release (411 pass), cargo clippy --all-targets -- -D warnings (clean), cargo doc --no-deps (clean).
This commit is contained in:
1 parent
230345a867
commit
62ed009281
10 files changed
+56
-53
No files matched your search
+24
-40
@@ -211,48 +211,32 @@ fn check_string(value: &Value, path: &str, max_length: Option<usize>) -> Result<
|
||||
}
|
||||
|
||||
fn check_bytes(value: &Value, path: &str, max_length: Option<usize>) -> Result<(), AlkTypeError> {
|
||||
match value {
|
||||
Value::String(s) => {
|
||||
if let Some(max) = max_length {
|
||||
if s.len() > max {
|
||||
return Err(validation_err(
|
||||
path,
|
||||
format!("bytes length {} exceeds maxLength {max}", s.len()),
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
let arr = value
|
||||
.as_array()
|
||||
.ok_or_else(|| validation_err(path, "expected an array of u8 for bytes"))?;
|
||||
if let Some(max) = max_length {
|
||||
if arr.len() > max {
|
||||
return Err(validation_err(
|
||||
path,
|
||||
format!("bytes array length {} exceeds maxLength {max}", arr.len()),
|
||||
));
|
||||
}
|
||||
Value::Array(arr) => {
|
||||
if let Some(max) = max_length {
|
||||
if arr.len() > max {
|
||||
return Err(validation_err(
|
||||
path,
|
||||
format!("bytes array length {} exceeds maxLength {max}", arr.len()),
|
||||
));
|
||||
}
|
||||
}
|
||||
for (i, entry) in arr.iter().enumerate() {
|
||||
let n = entry.as_u64().ok_or_else(|| {
|
||||
validation_err(
|
||||
path,
|
||||
format!("bytes array entry {i} is not a non-negative integer"),
|
||||
)
|
||||
})?;
|
||||
if n > 255 {
|
||||
return Err(validation_err(
|
||||
path,
|
||||
format!("bytes array entry {i} = {n} is not a u8 (0..=255)"),
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
_ => Err(validation_err(
|
||||
path,
|
||||
"expected a string or array of u8 for bytes",
|
||||
)),
|
||||
}
|
||||
for (i, entry) in arr.iter().enumerate() {
|
||||
let n = entry.as_u64().ok_or_else(|| {
|
||||
validation_err(
|
||||
path,
|
||||
format!("bytes array entry {i} is not a non-negative integer"),
|
||||
)
|
||||
})?;
|
||||
if n > 255 {
|
||||
return Err(validation_err(
|
||||
path,
|
||||
format!("bytes array entry {i} = {n} is not a u8 (0..=255)"),
|
||||
));
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn validate_timestamp(value: &Value, path: &str) -> Result<(), AlkTypeError> {
|
||||
|
||||
+2
-2
@@ -48,7 +48,7 @@ pub enum LayoutMode {
|
||||
enum Layout {
|
||||
/// Packed sequential layout. The write-side is [`LayoutBuilder`]; the
|
||||
/// read-side is a fresh [`SequentialReader`] constructed on demand
|
||||
/// (ADR-101 — the reader has mutable cursor state that the consumer
|
||||
/// (ADR-007 — the reader has mutable cursor state that the consumer
|
||||
/// owns, so the engine is a factory, not a holder).
|
||||
Packed {
|
||||
builder: LayoutBuilder,
|
||||
@@ -194,7 +194,7 @@ impl AlkTypeEngine {
|
||||
}
|
||||
|
||||
/// Construct a fresh [`SequentialReader`] for packed-mode reads
|
||||
/// (ADR-101). Each call returns a new reader with the cursor at
|
||||
/// (ADR-007). Each call returns a new reader with the cursor at
|
||||
/// position 0. The consumer owns the reader and calls
|
||||
/// `read_next`/`read_field`/`reset` on it directly.
|
||||
///
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
//! Error types for the alktype engine.
|
||||
//!
|
||||
//! Decided in ADR-098: a single `AlkTypeError` enum covers all error
|
||||
//! Decided in ADR-004: a single `AlkTypeError` enum covers all error
|
||||
//! conditions across the engine's three phases (schema parsing, offset
|
||||
//! computation, read/write) plus validation.
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Packed sequential `LayoutBuilder` — Mode 1 write-side (ADR-096).
|
||||
//! Packed sequential `LayoutBuilder` — Mode 1 write-side (ADR-002).
|
||||
//!
|
||||
//! Fields are packed with no alignment padding. Variable-length fields
|
||||
//! shift all subsequent fields. The consumer provides actual data sizes
|
||||
|
||||
+23
-4
@@ -172,12 +172,12 @@ fn materialize_typeref_packed(
|
||||
BastType::Primitive(AlkTypeKind::Float32) => {
|
||||
let v = data_access::read_f32(buffer, *offset, field_path, endian)?;
|
||||
*offset += 4;
|
||||
Ok(number_from_f64(v as f64))
|
||||
Ok(number_from_f64(v as f64, field_path)?)
|
||||
}
|
||||
BastType::Primitive(AlkTypeKind::Float64) => {
|
||||
let v = data_access::read_f64(buffer, *offset, field_path, endian)?;
|
||||
*offset += 8;
|
||||
Ok(number_from_f64(v))
|
||||
Ok(number_from_f64(v, field_path)?)
|
||||
}
|
||||
BastType::Primitive(AlkTypeKind::Boolean) => {
|
||||
let v = data_access::read_bool(buffer, *offset, field_path)?;
|
||||
@@ -444,10 +444,13 @@ fn union_discriminator_key(value: &Value, field_path: &str) -> Result<String, Al
|
||||
}
|
||||
}
|
||||
|
||||
fn number_from_f64(f: f64) -> Value {
|
||||
fn number_from_f64(f: f64, field_path: &str) -> Result<Value, AlkTypeError> {
|
||||
serde_json::Number::from_f64(f)
|
||||
.map(Value::Number)
|
||||
.unwrap_or(Value::Null)
|
||||
.ok_or_else(|| AlkTypeError::Access {
|
||||
field_path: field_path.to_string(),
|
||||
reason: format!("non-finite float {f} cannot be represented as JSON"),
|
||||
})
|
||||
}
|
||||
|
||||
/// Aligned-mode materialization: walk the BAST typed tree, but read
|
||||
@@ -1193,4 +1196,20 @@ mod tests {
|
||||
let v = materialize_aligned_strict(&doc, &buf).expect("materialize");
|
||||
assert_eq!(v["crc"], json!(0x01020304u32));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn materialize_packed_rejects_non_finite_float() {
|
||||
let root = json!({
|
||||
"$defs": {
|
||||
"S": {
|
||||
"kind": "struct",
|
||||
"fields": [ { "name": "f", "kind": "float32" } ]
|
||||
}
|
||||
}
|
||||
});
|
||||
let doc = doc_from(&root, "S");
|
||||
let buf = f32::NAN.to_le_bytes();
|
||||
let err = materialize_packed_strict(&doc, &buf).unwrap_err();
|
||||
assert!(matches!(err, AlkTypeError::Access { .. }), "got {err:?}");
|
||||
}
|
||||
}
|
||||
+1
-1
@@ -1,4 +1,4 @@
|
||||
//! Aligned static `OffsetMap` — Mode 2 of the two layout modes (ADR-096).
|
||||
//! Aligned static `OffsetMap` — Mode 2 of the two layout modes (ADR-002).
|
||||
//!
|
||||
//! Fields have fixed positions with natural alignment padding.
|
||||
//! Variable-length fields get a 4-byte length prefix at a known offset;
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
//! Packed sequential `SequentialReader` — Mode 1 read-side (ADR-096).
|
||||
//! Packed sequential `SequentialReader` — Mode 1 read-side (ADR-002).
|
||||
//!
|
||||
//! Walks a buffer field-by-field according to the BAST schema, reading
|
||||
//! length prefixes to determine variable-length data positions. Used at
|
||||
|
||||
+1
-1
@@ -1,4 +1,4 @@
|
||||
//! TUnion discriminator dispatch (ADR-097 §4).
|
||||
//! TUnion discriminator dispatch (ADR-003 §4).
|
||||
//!
|
||||
//! TUnion supports two discriminator kinds: byte-offset (protocol
|
||||
//! dispatch, e.g., SFTP type bytes) and field-name (typedef.ts string
|
||||
|
||||
Reference in new issue
Block a user