From 2c292404d2305528e8ab7448ccec9278e346fb73 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 13:05:48 -0500 Subject: [PATCH] format: read attributes over Storage AttributeMessage::parse_in_storage, extract_attributes_full_in and extract_attributes_tolerant_in take the file as &dyn Storage: shared datatypes, dataspaces and attributes are resolved through the Storage shared-message path, and dense attributes' fractal heap through FractalHeapHeader::parse_in / read_managed_object_in. The dense-storage name index is a v2 B-tree, which is not read over Storage yet: over a backend without the whole file in memory it is a clean ContiguousStorageRequired error, never a partial list. The &[u8] functions are wrappers. New test: every object in five h5py-written fixtures (compact, shared and dense attributes) reads identically through a slice as Storage, and through a read_at-only CountingStorage except the dense ones, which give the clean error. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/attribute.rs | 144 +++++++++++++++++++++--- 1 file changed, 127 insertions(+), 17 deletions(-) diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index 6c4e9ae..815719e 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -15,6 +15,7 @@ use crate::fractal_heap::FractalHeapHeader; use crate::message_type::MessageType; use crate::object_header::ObjectHeader; use crate::shared_message; +use crate::storage::{Storage, require_contiguous}; use crate::vl_data; /// A parsed HDF5 attribute message. @@ -65,13 +66,24 @@ impl AttributeMessage { offset_size: u8, length_size: u8, ) -> Result { - Self::parse_impl(data, length_size, Some((file_data, offset_size))) + Self::parse_in_storage(data, &file_data, offset_size, length_size) + } + + /// [`AttributeMessage::parse_in_file`] with the file behind any + /// [`Storage`]. + pub fn parse_in_storage( + data: &[u8], + file: &dyn Storage, + offset_size: u8, + length_size: u8, + ) -> Result { + Self::parse_impl(data, length_size, Some((file, offset_size))) } fn parse_impl( data: &[u8], length_size: u8, - file: Option<(&[u8], u8)>, + file: Option<(&dyn Storage, u8)>, ) -> Result { ensure_len(data, 0, 2)?; let version = data[0]; @@ -91,14 +103,14 @@ impl AttributeMessage { shared: bool, msg_type: MessageType, length_size: u8, - file: Option<(&[u8], u8)>, + file: Option<(&dyn Storage, u8)>, ) -> Result, FormatError> { if !shared { return Ok(Cow::Borrowed(bytes)); } let (file_data, offset_size) = file.ok_or(FormatError::UnresolvedSharedMessage)?; let shared_ref = shared_message::parse_shared_ref_sized(bytes, offset_size, length_size)?; - shared_message::resolve_shared_message( + shared_message::resolve_shared_message_in( file_data, &shared_ref, msg_type, @@ -146,7 +158,7 @@ impl AttributeMessage { fn parse_v2( data: &[u8], length_size: u8, - file: Option<(&[u8], u8)>, + file: Option<(&dyn Storage, u8)>, ) -> Result { // Flags: bit 0 = datatype is shared, bit 1 = dataspace is shared. let flags = data.get(1).copied().unwrap_or(0); @@ -200,7 +212,7 @@ impl AttributeMessage { fn parse_v3( data: &[u8], length_size: u8, - file: Option<(&[u8], u8)>, + file: Option<(&dyn Storage, u8)>, ) -> Result { // Flags: bit 0 = datatype is shared, bit 1 = dataspace is shared. let flags = data.get(1).copied().unwrap_or(0); @@ -415,7 +427,20 @@ pub fn extract_attributes_full( offset_size: u8, length_size: u8, ) -> Result, FormatError> { - extract_attributes_with(file_data, header, offset_size, length_size, &mut Err) + extract_attributes_full_in(&file_data, header, offset_size, length_size) +} + +/// [`extract_attributes_full`] over any [`Storage`]. Dense attribute +/// storage is indexed by a v2 B-tree, which is not read over [`Storage`] +/// yet: on a backend without the whole file in memory an object with dense +/// attributes is [`FormatError::ContiguousStorageRequired`]. +pub fn extract_attributes_full_in( + file: &dyn Storage, + header: &ObjectHeader, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + extract_attributes_with(file, header, offset_size, length_size, &mut Err) } /// Like [`extract_attributes_full`], but an attribute that cannot be read @@ -431,6 +456,17 @@ pub fn extract_attributes_tolerant( header: &ObjectHeader, offset_size: u8, length_size: u8, +) -> Result<(Vec, Vec), FormatError> { + extract_attributes_tolerant_in(&file_data, header, offset_size, length_size) +} + +/// [`extract_attributes_tolerant`] over any [`Storage`] (see +/// [`extract_attributes_full_in`] for dense storage). +pub fn extract_attributes_tolerant_in( + file_data: &dyn Storage, + header: &ObjectHeader, + offset_size: u8, + length_size: u8, ) -> Result<(Vec, Vec), FormatError> { let mut errors = Vec::new(); let attrs = extract_attributes_with(file_data, header, offset_size, length_size, &mut |e| { @@ -443,7 +479,7 @@ pub fn extract_attributes_tolerant( /// Read every attribute; each one that fails goes to `on_error`, which /// either stops the read (returns the error) or skips that attribute. fn extract_attributes_with( - file_data: &[u8], + file_data: &dyn Storage, header: &ObjectHeader, offset_size: u8, length_size: u8, @@ -460,7 +496,7 @@ fn extract_attributes_with( // Shared attribute: resolve the reference to get actual attribute data shared_message::parse_shared_ref_sized(&msg.data, offset_size, length_size) .and_then(|shared_ref| { - shared_message::resolve_shared_message( + shared_message::resolve_shared_message_in( file_data, &shared_ref, MessageType::Attribute, @@ -469,7 +505,7 @@ fn extract_attributes_with( ) }) .and_then(|resolved| { - AttributeMessage::parse_in_file( + AttributeMessage::parse_in_storage( &resolved, file_data, offset_size, @@ -477,7 +513,7 @@ fn extract_attributes_with( ) }) } else { - AttributeMessage::parse_in_file(&msg.data, file_data, offset_size, length_size) + AttributeMessage::parse_in_storage(&msg.data, file_data, offset_size, length_size) }; let attr = attr.and_then(|a| check_in_header(a, header)); match attr { @@ -537,7 +573,7 @@ fn find_attribute_info( /// each one's creation order into `orders`. #[allow(clippy::too_many_arguments)] fn extract_dense_attributes( - file_data: &[u8], + file_data: &dyn Storage, attr_info: &AttributeInfoMessage, fh_addr: u64, offset_size: u8, @@ -547,7 +583,7 @@ fn extract_dense_attributes( on_error: &mut dyn FnMut(FormatError) -> Result<(), FormatError>, ) -> Result<(), FormatError> { // Parse fractal heap - let fh = FractalHeapHeader::parse(file_data, fh_addr as usize, offset_size, length_size)?; + let fh = FractalHeapHeader::parse_in(file_data, fh_addr, offset_size, length_size)?; // Parse B-tree v2 for name index (type 8) let btree_addr = attr_info @@ -556,8 +592,10 @@ fn extract_dense_attributes( expected: 1, available: 0, })?; - let btree_hdr = BTreeV2Header::parse(file_data, btree_addr as usize, offset_size, length_size)?; - let records = collect_btree_v2_records(file_data, &btree_hdr, offset_size, length_size)?; + let contiguous = require_contiguous(file_data, "dense attribute storage (a v2 B-tree)")?; + let btree_hdr = + BTreeV2Header::parse(contiguous, btree_addr as usize, offset_size, length_size)?; + let records = collect_btree_v2_records(contiguous, &btree_hdr, offset_size, length_size)?; for record in &records { // Per HDF5 spec, both type 8 and type 9 records start with heap_id: @@ -574,9 +612,9 @@ fn extract_dense_attributes( // The data in the heap is a complete attribute message let attr = fh - .read_managed_object(file_data, id_bytes, offset_size) + .read_managed_object_in(file_data, id_bytes, offset_size) .and_then(|attr_data| { - AttributeMessage::parse_in_file(&attr_data, file_data, offset_size, length_size) + AttributeMessage::parse_in_storage(&attr_data, file_data, offset_size, length_size) }); match attr { Ok(attr) => { @@ -986,4 +1024,76 @@ mod tests { let strs = attr.read_as_strings().unwrap(); assert_eq!(strs, vec!["abcd", "EFGH"]); } + + /// Every object's attributes in h5py-written files read identically + /// through a read_at-only CountingStorage — compact ones, shared ones + /// and those behind an Attribute Info message — except dense storage, + /// whose v2 B-tree index is not read over Storage yet: that is the clean + /// ContiguousStorageRequired error, never a partial list. Through a + /// slice as Storage every object matches. + #[test] + fn storage_reads_match_slice_reads() { + use crate::storage::CountingStorage; + let files: [(&str, &[u8]); 5] = [ + ("attrs", include_bytes!("../tests/fixtures/attrs.h5")), + ( + "mixed_attrs", + include_bytes!("../tests/fixtures/mixed_attrs.h5"), + ), + ( + "dense_attrs", + include_bytes!("../tests/fixtures/dense_attrs.h5"), + ), + ( + "dense_attrs_root", + include_bytes!("../tests/fixtures/dense_attrs_root.h5"), + ), + ( + "shared_fill_value", + include_bytes!("../tests/fixtures/shared_fill_value.h5"), + ), + ]; + let (mut same, mut dense, mut attrs) = (0, 0, 0); + for (name, file) in files { + let sb = crate::superblock::Superblock::parse(file, 0).unwrap(); + let (os, ls) = (sb.offset_size, sb.length_size); + let mut addrs = vec![sb.root_group_address]; + addrs.extend( + crate::group_v2::resolve_group_children(file, &sb, sb.root_group_address) + .unwrap() + .iter() + .map(|e| e.object_header_address), + ); + let storage = CountingStorage::new(file.to_vec()); + for addr in addrs { + let header = ObjectHeader::parse(file, addr as usize, os, ls).unwrap(); + let want = extract_attributes_full(file, &header, os, ls); + let slice_storage = extract_attributes_full_in(&file, &header, os, ls); + assert_eq!(format!("{slice_storage:?}"), format!("{want:?}")); + let got = extract_attributes_full_in(&storage, &header, os, ls); + let got_t = extract_attributes_tolerant_in(&storage, &header, os, ls); + let is_dense = find_attribute_info(&header, os) + .unwrap() + .is_some_and(|i| i.fractal_heap_address.is_some()); + if is_dense { + let e = FormatError::ContiguousStorageRequired( + "dense attribute storage (a v2 B-tree)", + ); + assert_eq!(got.unwrap_err(), e, "{name}"); + assert_eq!(got_t.unwrap_err(), e, "{name}"); + dense += 1; + } else { + attrs += want.as_ref().map_or(0, Vec::len); + assert_eq!(format!("{got:?}"), format!("{want:?}"), "{name}"); + let want_t = extract_attributes_tolerant(file, &header, os, ls); + assert_eq!(format!("{got_t:?}"), format!("{want_t:?}"), "{name}"); + same += 1; + } + } + } + assert!( + same >= 5 && dense >= 2 && attrs >= 5, + "{same} {dense} {attrs}" + ); + } }