diff --git a/dev-docs/specs/2026-07-22-obspec-store-pyclass-design.md b/dev-docs/specs/2026-07-22-obspec-store-pyclass-design.md new file mode 100644 index 0000000..410eb15 --- /dev/null +++ b/dev-docs/specs/2026-07-22-obspec-store-pyclass-design.md @@ -0,0 +1,196 @@ +# Obspec store as an explicit Python class + +**Date:** 2026-07-22 +**Status:** Approved, not yet implemented + +## Problem + +`ObspecStore` implements the `zarrs` sync storage traits on top of a Python object +following the obspec protocol. It reaches Python today only through a duck-typed +fallback in `PySyncStorage::extract`: any object carrying `head`, `get`, and +`get_ranges` is silently adopted as a store. + +That has three costs: + +- A user cannot signal intent. Passing an object that happens to have those three + attributes works by accident; passing one that is missing an attribute produces a + confusing "expected a FilesystemStore, MemoryStore, or ObspecStore" error rather than + naming what is wrong. +- The protocol check lives in a `FromPyObject` impl rather than in a constructor, so it + is a second parse point alongside the built-in stores' `cast::()` path. +- Nothing verifies the read path works. `ObspecStore` is the only store whose trait + impl is code in this repo rather than upstream `zarrs`, and no test exercises it. + +## Goals + +1. Expose `ObspecStore` as a real `#[pyclass]`, constructed explicitly. +2. Remove the duck-typed fallback so there is exactly one way in. +3. Override `ReadableStorageTraits::get` so whole-key reads use obspec's `get`. +4. Export the class and give it a smoke test. + +## Non-goals + +- **Implementing write and list.** The eight `todo!()` bodies on + `WritableStorageTraits` and `ListableStorageTraits` stay as they are. They panic if + reached; converting them to `StorageError::ReadOnly` / `Unsupported` and then + implementing them against `obstore.put` / `delete` / `list` / + `list_with_delimiter` is a separate piece of work. +- **The async counterpart.** An `AsyncObspecStore` driving `get_async` / + `get_ranges_async` is wanted eventually. Nothing here should block it — see + "Forward compatibility". +- **Any broader store read API.** Stores stay opaque handles, exposing only a + constructor and `__repr__`, as `FilesystemStore` and `MemoryStore` do. + +## Design + +### 1. `PyObspecStore` becomes a pyclass + +The existing two-type split is already the right shape and does not change: +`ObspecStore` carries the trait impls, `PyObspecStore` is the `Arc`-holding wrapper. +The wrapper simply becomes a pyclass, following `PyFilesystemStore` and +`PyMemoryStore` exactly: + +```rust +#[pyclass(module = "zarrista", frozen, name = "ObspecStore", skip_from_py_object)] +#[derive(Clone)] +pub struct PyObspecStore(pub(super) Arc); + +#[pymethods] +impl PyObspecStore { + /// Wrap a Python object implementing the obspec protocol. + #[new] + fn new(obj: Py) -> PyResult { ... } + + fn __repr__(&self) -> String { ... } +} +``` + +Rust reads `ObspecStore` (the store) against `PyObspecStore` (the Python wrapper); +Python sees only `ObspecStore`. This is the `Py`-prefix convention in CLAUDE.md. + +The `hasattr("head" | "get" | "get_ranges")` check moves from `FromPyObject::extract` +into `#[new]`, where a failure can name the missing attribute. The hand-written +`FromPyObject` and `IntoPyObject` impls are deleted: `skip_from_py_object` plus +`obj.cast::()` in `PySyncStorage::extract` is how the other two stores +already work. + +`__repr__` should include the wrapped object's own repr, since the wrapper is +otherwise indistinguishable between backends. + +**Behaviour change on the way out.** `Array.store` and `Group.store` return +`PySyncStorage` to Python. The hand-written `IntoPyObject` currently hands back the +*original* obspec object, so `Array.open(obstore_store).store is obstore_store` holds +today. + +This is forced by the explicit-only decision rather than incidental. Keeping a custom +`IntoPyObject` that returns the raw object would break the round-trip: + +```python +arr = Array.open(ObspecStore(s)) +Array.open(arr.store) # TypeError: raw obstore objects are no longer accepted +``` + +`Array.open(arr.store)` and `Group.open(arr.store, path)` are the natural way to reach +a sibling node from an existing handle, and they work today. Letting the derived +conversion return an `ObspecStore` wrapper keeps them working, and matches +`FilesystemStore` / `MemoryStore`, which already return the zarrista wrapper. + +The cost is that `arr.store` is no longer the obstore object, so passing it to obstore +functions (`obstore.get(arr.store, ...)`) stops working. + +**`ObspecStore.obj` restores that access.** A read-only getter returns the wrapped +Python object, so the underlying store stays reachable without making `arr.store` +itself the raw object: + +```python +arr = Array.open(ObspecStore(s)) +arr.store # ObspecStore — accepted by Array.open / Group.open +arr.store.obj is s # True — usable with obstore functions directly +``` + +This keeps the two roles separate: `arr.store` is what zarrista accepts back, `.obj` is +the escape hatch to the backend. + +**Required stub updates.** `python/zarrista/_array.pyi` and `python/zarrista/_group.pyi` +type the `store` getter as `FilesystemStore | MemoryStore`; both need `ObspecStore` +added. + +### 2. Override `get` + +`zarrs` provides a default: + +```rust +fn get(&self, key: &StoreKey) -> Result { + self.get_partial(key, ByteRange::FromStart(0, None)) +} +``` + +`FromStart(0, None)` lands in the `OpenEndedRanges` branch, so **every** whole-object +read — every `zarr.json`, every unsharded chunk — becomes +`get(path, options={"range": {"offset": 0}})`, sending `Range: bytes=0-` and taking a +206 response, instead of a plain `get(path)`. + +`ObspecStore` overrides `get` to call obspec's `get` with no `options`. + +The override's body is nearly identical to `execute_get`, so both collapse onto one +helper: + +```rust +/// Fetch a whole object, or one open-ended range of it, via obspec `get`. +fn call_get( + store: &Bound<'_, PyAny>, + key: &StoreKey, + range: Option<(&str, u64)>, // None -> no `options` kwarg at all +) -> Result, StorageError> +``` + +`get` passes `None`; `OpenEndedRanges::execute` passes `Some(("offset", offset))` or +`Some(("suffix", length))`. This removes a duplicated body rather than adding one. + +### 3. Exports + +- `python/zarrista/store.py`: import `ObspecStore` from `._zarrista`, add to `__all__`. +- `python/zarrista/_store.pyi`: the class stub. Per CLAUDE.md the `.pyi` is the single + source of truth for user-facing docs, so the prose lives here and the Rust `///` + comment stays a one-line summary. Cover: which obspec methods are required + (`head`, `get`, `get_ranges`), that the store is currently read-only and write or + list operations are unsupported, and an obstore example. + +### 4. Tests + +`tests/test_store_input.py` is currently failing: `test_open_rejects_non_store` +asserts on `"FilesystemStore or MemoryStore"`, but the message is now +`"expected a FilesystemStore, MemoryStore, or ObspecStore"`. Update the regex. + +Add one smoke test to the same file: open a zarr array through +`ObspecStore(obstore.store.MemoryStore())` and assert chunk contents, reusing the +existing `array_path` fixture pattern. This is deliberately minimal — no recording +fake, no assertions on which obspec calls were issued, no new API surface. Its purpose +is to establish that the read path executes at all, which nothing does today. + +Also add a test that a non-obspec object passed to `ObspecStore(...)` raises +`TypeError`, since the protocol check has moved into the constructor. + +**Known risk:** with the `todo!()` bodies retained, any test that reaches a list +operation aborts rather than failing cleanly. If the smoke test panics, that is a +finding — it means the array read path touches `ListableStorageTraits`, and the stubs +need addressing sooner than planned. + +## Forward compatibility + +`ObspecStore` holds a plain `Py`, not an obstore-specific type, so an +`AsyncObspecStore` can wrap the same Python object and call `get_async` / +`get_ranges_async` without restructuring. The range-partitioning types (`ReadRanges`, +`BoundedRanges`, `OpenEndedRanges`) are pure and contain no sync-specific logic, so an +async implementation reuses them directly; only the three `execute` methods need async +counterparts. + +## Testing gap accepted + +The ordering invariant — that a single `get_partial_many` mixing bounded, offset, and +suffix ranges returns bytes in the caller's order — is covered only by the existing +Rust unit tests on `ReadRanges` partitioning, not end-to-end. Driving mixed shapes +through `Array.open` deterministically is not currently possible, because store methods +are not exposed to Python and `cargo test` embeds the Homebrew interpreter rather than +the venv, so Rust tests cannot import `obstore` without a machine-specific +`PYTHONPATH`. Accepted for now; revisit if the read path proves fragile. diff --git a/src/storage/sync.rs b/src/storage/sync.rs deleted file mode 100644 index 2f0eb00..0000000 --- a/src/storage/sync.rs +++ /dev/null @@ -1,188 +0,0 @@ -use crate::error::ZarristaResult; -use pyo3::exceptions::PyTypeError; -use pyo3::prelude::*; -use std::path::PathBuf; -use std::sync::Arc; -use zarrs::filesystem::FilesystemStore; -use zarrs::storage::byte_range::{ByteRange, ByteRangeIterator}; -use zarrs::storage::store::MemoryStore; -use zarrs::storage::{ - Bytes, ListableStorageTraits, MaybeBytes, MaybeBytesIterator, OffsetBytesIterator, - ReadableListableStorageTraits, ReadableStorageTraits, ReadableWritableListableStorageTraits, - StorageError, StoreKey, StoreKeys, StoreKeysPrefixes, StorePrefix, WritableStorageTraits, -}; - -/// A zarrista sync store object adapted to the maximal `zarrs` storage trait. -#[derive(Clone, IntoPyObject)] -pub enum PySyncStorage { - Filesystem(PyFilesystemStore), - MemoryStore(PyMemoryStore), -} - -impl PySyncStorage { - pub fn inner(&self) -> Arc { - match self { - Self::Filesystem(store) => store.storage.clone(), - Self::MemoryStore(store) => store.0.clone(), - } - } -} - -impl FromPyObject<'_, '_> for PySyncStorage { - type Error = PyErr; - - fn extract(obj: Borrowed<'_, '_, PyAny>) -> Result { - if let Ok(s) = obj.cast::() { - return Ok(Self::Filesystem(s.get().clone())); - } - if let Ok(s) = obj.cast::() { - return Ok(Self::MemoryStore(s.get().clone())); - } - Err(PyTypeError::new_err( - "expected a FilesystemStore or MemoryStore", - )) - } -} - -impl From for Arc { - fn from(s: PySyncStorage) -> Self { - match s { - PySyncStorage::Filesystem(store) => store.storage, - PySyncStorage::MemoryStore(store) => store.0, - } - } -} - -/// A store backed by a local directory. -#[pyclass( - module = "zarrista", - frozen, - name = "FilesystemStore", - skip_from_py_object -)] -#[derive(Clone)] -pub struct PyFilesystemStore { - pub(crate) storage: Arc, - path: PathBuf, -} - -crate::wasm_send_sync!(PyFilesystemStore); - -#[pymethods] -impl PyFilesystemStore { - /// Open a filesystem store rooted at `path`. - #[new] - fn new(path: PathBuf) -> ZarristaResult { - let store = FilesystemStore::new(&path)?; - Ok(Self { - storage: Arc::new(store), - path, - }) - } - - fn __repr__(&self) -> String { - format!("FilesystemStore({})", self.path.display()) - } -} - -/// An in-memory store, primarily useful for testing. -#[pyclass(module = "zarrista", frozen, name = "MemoryStore", skip_from_py_object)] -#[derive(Clone)] -pub struct PyMemoryStore(Arc); - -crate::wasm_send_sync!(PyMemoryStore); - -#[pymethods] -impl PyMemoryStore { - #[new] - fn new() -> Self { - Self(Arc::new(MemoryStore::new())) - } - - fn __repr__(&self) -> String { - "MemoryStore()".to_string() - } -} - -/// A storage adapter that reads and lists transparently but rejects all writes at runtime. -pub struct ReadOnlyStorageAdapter(Arc); - -impl ReadOnlyStorageAdapter { - pub fn new(inner: Arc) -> Self { - Self(inner) - } -} - -impl ReadableStorageTraits for ReadOnlyStorageAdapter { - fn get(&self, key: &StoreKey) -> Result { - self.0.get(key) - } - - fn get_partial_many<'a>( - &'a self, - key: &StoreKey, - byte_ranges: ByteRangeIterator<'a>, - ) -> Result, StorageError> { - self.0.get_partial_many(key, byte_ranges) - } - - fn get_partial( - &self, - key: &StoreKey, - byte_range: ByteRange, - ) -> Result { - self.0.get_partial(key, byte_range) - } - - fn size_key(&self, key: &StoreKey) -> Result, StorageError> { - self.0.size_key(key) - } - - fn supports_get_partial(&self) -> bool { - self.0.supports_get_partial() - } -} - -impl ListableStorageTraits for ReadOnlyStorageAdapter { - fn list(&self) -> Result { - self.0.list() - } - - fn list_prefix(&self, prefix: &StorePrefix) -> Result { - self.0.list_prefix(prefix) - } - - fn list_dir(&self, prefix: &StorePrefix) -> Result { - self.0.list_dir(prefix) - } - - fn size_prefix(&self, prefix: &StorePrefix) -> Result { - self.0.size_prefix(prefix) - } -} - -impl WritableStorageTraits for ReadOnlyStorageAdapter { - fn set(&self, _key: &StoreKey, _value: Bytes) -> Result<(), StorageError> { - Err(StorageError::ReadOnly) - } - - fn set_partial_many( - &self, - _key: &StoreKey, - _offset_values: OffsetBytesIterator, - ) -> Result<(), StorageError> { - Err(StorageError::ReadOnly) - } - - fn erase(&self, _key: &StoreKey) -> Result<(), StorageError> { - Err(StorageError::ReadOnly) - } - - fn erase_prefix(&self, _prefix: &StorePrefix) -> Result<(), StorageError> { - Err(StorageError::ReadOnly) - } - - fn supports_set_partial(&self) -> bool { - false - } -} diff --git a/src/storage/sync/filesystem.rs b/src/storage/sync/filesystem.rs new file mode 100644 index 0000000..cb51367 --- /dev/null +++ b/src/storage/sync/filesystem.rs @@ -0,0 +1,37 @@ +use crate::error::ZarristaResult; +use pyo3::prelude::*; +use std::path::PathBuf; +use std::sync::Arc; +use zarrs::filesystem::FilesystemStore; + +/// A store backed by a local directory. +#[pyclass( + module = "zarrista", + frozen, + name = "FilesystemStore", + skip_from_py_object +)] +#[derive(Clone)] +pub struct PyFilesystemStore { + pub(crate) storage: Arc, + path: PathBuf, +} + +crate::wasm_send_sync!(PyFilesystemStore); + +#[pymethods] +impl PyFilesystemStore { + /// Open a filesystem store rooted at `path`. + #[new] + fn new(path: PathBuf) -> ZarristaResult { + let store = FilesystemStore::new(&path)?; + Ok(Self { + storage: Arc::new(store), + path, + }) + } + + fn __repr__(&self) -> String { + format!("FilesystemStore({})", self.path.display()) + } +} diff --git a/src/storage/sync/memory.rs b/src/storage/sync/memory.rs new file mode 100644 index 0000000..dcb5625 --- /dev/null +++ b/src/storage/sync/memory.rs @@ -0,0 +1,22 @@ +use pyo3::prelude::*; +use std::sync::Arc; +use zarrs::storage::store::MemoryStore; + +/// An in-memory store, primarily useful for testing. +#[pyclass(module = "zarrista", frozen, name = "MemoryStore", skip_from_py_object)] +#[derive(Clone)] +pub struct PyMemoryStore(pub(super) Arc); + +crate::wasm_send_sync!(PyMemoryStore); + +#[pymethods] +impl PyMemoryStore { + #[new] + fn new() -> Self { + Self(Arc::new(MemoryStore::new())) + } + + fn __repr__(&self) -> String { + "MemoryStore()".to_string() + } +} diff --git a/src/storage/sync/mod.rs b/src/storage/sync/mod.rs new file mode 100644 index 0000000..0755248 --- /dev/null +++ b/src/storage/sync/mod.rs @@ -0,0 +1,61 @@ +mod filesystem; +mod memory; +mod obspec; +mod read_only; + +pub use filesystem::PyFilesystemStore; +pub use memory::PyMemoryStore; +pub use obspec::PyObspecStore; +pub use read_only::ReadOnlyStorageAdapter; + +use pyo3::exceptions::PyTypeError; +use pyo3::prelude::*; +use std::sync::Arc; +use zarrs::storage::ReadableWritableListableStorageTraits; + +/// A zarrista sync store object adapted to the maximal `zarrs` storage trait. +#[derive(Clone, IntoPyObject)] +pub enum PySyncStorage { + Filesystem(PyFilesystemStore), + MemoryStore(PyMemoryStore), + Obspec(PyObspecStore), +} + +impl PySyncStorage { + pub fn inner(&self) -> Arc { + match self { + Self::Filesystem(store) => store.storage.clone(), + Self::MemoryStore(store) => store.0.clone(), + Self::Obspec(store) => store.0.clone(), + } + } +} + +impl FromPyObject<'_, '_> for PySyncStorage { + type Error = PyErr; + + fn extract(obj: Borrowed<'_, '_, PyAny>) -> Result { + if let Ok(s) = obj.cast::() { + return Ok(Self::Filesystem(s.get().clone())); + } + if let Ok(s) = obj.cast::() { + return Ok(Self::MemoryStore(s.get().clone())); + } + if let Ok(s) = obj.extract::() { + return Ok(Self::Obspec(s)); + } + Err(PyTypeError::new_err( + "expected a FilesystemStore, MemoryStore, or ObspecStore", + )) + } +} + +impl From for Arc { + fn from(s: PySyncStorage) -> Self { + match s { + PySyncStorage::Filesystem(store) => store.storage, + PySyncStorage::MemoryStore(store) => store.0, + PySyncStorage::Obspec(store) => store.0, + } + } +} diff --git a/src/storage/sync/obspec.rs b/src/storage/sync/obspec.rs new file mode 100644 index 0000000..a0b20c1 --- /dev/null +++ b/src/storage/sync/obspec.rs @@ -0,0 +1,403 @@ +use std::sync::Arc; + +use pyo3::exceptions::{PyFileNotFoundError, PyTypeError}; +use pyo3::prelude::*; +use pyo3::types::IntoPyDict; +use pyo3_bytes::PyBytes; +use zarrs::storage::byte_range::{ByteRange, ByteRangeIterator}; +use zarrs::storage::{ + Bytes, ListableStorageTraits, MaybeBytesIterator, OffsetBytesIterator, ReadableStorageTraits, + StorageError, StoreKey, StoreKeys, StoreKeysPrefixes, StorePrefix, WritableStorageTraits, +}; + +/// A `zarrs` store backed by a Python obspec object. +#[derive(Debug)] +pub(super) struct ObspecStore(Py); + +crate::wasm_send_sync!(ObspecStore); + +impl ObspecStore { + /// The wrapped Python object. + fn as_py(&self) -> &Py { + &self.0 + } +} + +/// An arc-ed wrapper of an [ObspecStore] +/// +/// It's useful to have two separate types because zarrs requires an `Arc`. +#[derive(Debug, Clone)] +pub struct PyObspecStore(pub(super) Arc); + +impl FromPyObject<'_, '_> for PyObspecStore { + type Error = PyErr; + + fn extract(obj: Borrowed<'_, '_, PyAny>) -> Result { + if !obj.hasattr("head")? || !obj.hasattr("get")? || !obj.hasattr("get_ranges")? { + return Err(PyTypeError::new_err( + "expected an object implementing the obspec protocol", + )); + } + + Ok(Self(Arc::new(ObspecStore(obj.into())))) + } +} + +impl<'py> IntoPyObject<'py> for PyObspecStore { + type Error = PyErr; + type Target = PyAny; + type Output = Bound<'py, Self::Target>; + + fn into_pyobject(self, py: Python<'py>) -> Result { + Ok(self.0.as_py().bind(py).clone()) + } +} + +/// Convert a Python exception into a [`StorageError`]. +fn map_py_err(err: PyErr) -> StorageError { + StorageError::Other(err.to_string()) +} + +/// Translate the result of an obspec call, mapping a missing object to `None`. +/// +/// obspec implementations signal that a key is absent by raising `FileNotFoundError`, +/// which for `zarrs` is the `Ok(None)` case rather than an error. +fn missing_as_none(py: Python<'_>, result: PyResult) -> Result, StorageError> { + match result { + Ok(value) => Ok(Some(value)), + Err(err) if err.is_instance_of::(py) => Ok(None), + Err(err) => Err(map_py_err(err)), + } +} + +/// A byte range with a known start and exclusive end. +struct BoundedRange { + /// Position of this range in the original request. + index: usize, + start: u64, + end: u64, +} + +/// Byte ranges that `get_ranges` can serve, because both ends are known. +#[derive(Default)] +struct BoundedRanges(Vec); + +impl BoundedRanges { + /// Fetch every bounded range in a single `get_ranges` call. + fn execute( + &self, + store: &Bound<'_, PyAny>, + key: &StoreKey, + ) -> Result>, StorageError> { + if self.0.is_empty() { + return Ok(Some(Vec::new())); + } + + let py = store.py(); + let starts = self.0.iter().map(|range| range.start).collect::>(); + let ends = self.0.iter().map(|range| range.end).collect::>(); + let kwargs = [("starts", starts), ("ends", ends)] + .into_py_dict(py) + .map_err(map_py_err)?; + + let result = store.call_method("get_ranges", (key.as_str(),), Some(&kwargs)); + let Some(buffers) = missing_as_none(py, result)? else { + return Ok(None); + }; + + let buffers = buffers.extract::>().map_err(map_py_err)?; + if buffers.len() != self.0.len() { + return Err(StorageError::Other(format!( + "obspec get_ranges returned {} buffers for {} requested byte ranges", + buffers.len(), + self.0.len() + ))); + } + + Ok(Some( + self.0 + .iter() + .zip(buffers) + .map(|(range, buffer)| (range.index, buffer.into_inner())) + .collect(), + )) + } +} + +/// A byte range that runs to the end of the object. +struct OpenEndedRange { + /// Position of this range in the original request. + index: usize, + /// The offset for an `OffsetRange`, or the length for a `SuffixRange`. + value: u64, +} + +/// Byte ranges that only the range option of `get` can express, one request each. +/// +/// [`ByteRange::FromStart`] with no length and [`ByteRange::Suffix`] differ only in the +/// obspec range option they map to, so they share this representation and the caller +/// supplies the option name at execution time. +#[derive(Default)] +struct OpenEndedRanges(Vec); + +impl OpenEndedRanges { + /// Fetch each range with its own `get` call. + /// + /// `option` is `"offset"` or `"suffix"`, matching obspec's `OffsetRange` and `SuffixRange`. + fn execute( + &self, + store: &Bound<'_, PyAny>, + key: &StoreKey, + option: &str, + ) -> Result>, StorageError> { + let mut buffers = Vec::with_capacity(self.0.len()); + for range in &self.0 { + let Some(buffer) = execute_get(store, key, option, range.value)? else { + return Ok(None); + }; + buffers.push((range.index, buffer)); + } + Ok(Some(buffers)) + } +} + +/// Fetch one open-ended range via the range option of `get`. +fn execute_get( + store: &Bound<'_, PyAny>, + key: &StoreKey, + option: &str, + value: u64, +) -> Result, StorageError> { + let py = store.py(); + let range = [(option, value)].into_py_dict(py).map_err(map_py_err)?; + let options = [("range", range)].into_py_dict(py).map_err(map_py_err)?; + let kwargs = [("options", options)] + .into_py_dict(py) + .map_err(map_py_err)?; + + let result = store.call_method("get", (key.as_str(),), Some(&kwargs)); + let Some(get_result) = missing_as_none(py, result)? else { + return Ok(None); + }; + + // `buffer` is the obspec spelling; obstore also exposes it as `bytes`. + let buffer = get_result + .call_method0("buffer") + .and_then(|buffer| buffer.extract::()) + .map_err(map_py_err)?; + + Ok(Some(buffer.into_inner())) +} + +/// The obspec requests needed to serve a set of requested byte ranges. +/// +/// `get_ranges` is the only bulk obspec read, and it requires a known start and +/// exclusive end for every range. +/// +/// The two open-ended [`ByteRange`] shapes — [`ByteRange::FromStart`] with `(_, None)` and +/// [`ByteRange::Suffix`] — can only be expressed through the range option of `get`, so each costs +/// its own request. +/// +/// Partitioning them out keeps every bounded range in the single `get_ranges` call, which is what +/// coalesces neighbouring reads. +/// +/// Every entry carries its index into the original byte range sequence so that the +/// responses can be restored to the caller's order. +#[derive(Default)] +struct ReadRanges { + bounded: BoundedRanges, + offset: OpenEndedRanges, + suffix: OpenEndedRanges, +} + +impl ReadRanges { + /// Construct a plan for the given byte ranges. + fn new(byte_ranges: ByteRangeIterator<'_>) -> Self { + let mut plan = Self::default(); + for (index, byte_range) in byte_ranges.enumerate() { + match byte_range { + ByteRange::FromStart(start, Some(length)) => plan.bounded.0.push(BoundedRange { + index, + start, + end: start + length, + }), + ByteRange::FromStart(start, None) => plan.offset.0.push(OpenEndedRange { + index, + value: start, + }), + ByteRange::Suffix(length) => plan.suffix.0.push(OpenEndedRange { + index, + value: length, + }), + } + } + plan + } + + /// The number of byte ranges the plan covers. + fn len(&self) -> usize { + self.bounded.0.len() + self.offset.0.len() + self.suffix.0.len() + } + + /// Issue the planned requests and collect the responses in the caller's order. + /// + /// Returns `Ok(None)` if `key` is absent from the store. + fn execute( + &self, + store: &Bound<'_, PyAny>, + key: &StoreKey, + ) -> Result>, StorageError> { + let Some(bounded) = self.bounded.execute(store, key)? else { + return Ok(None); + }; + let Some(offset) = self.offset.execute(store, key, "offset")? else { + return Ok(None); + }; + let Some(suffix) = self.suffix.execute(store, key, "suffix")? else { + return Ok(None); + }; + + // The three collections partition `0..len`, so every slot is assigned exactly once + // below and no placeholder survives. + let mut bytes = vec![Bytes::new(); self.len()]; + for (index, buffer) in bounded.into_iter().chain(offset).chain(suffix) { + bytes[index] = buffer; + } + + Ok(Some(bytes)) + } +} + +impl ReadableStorageTraits for ObspecStore { + fn get_partial_many<'a>( + &'a self, + key: &StoreKey, + byte_ranges: ByteRangeIterator<'a>, + ) -> Result, StorageError> { + let plan = ReadRanges::new(byte_ranges); + let bytes = Python::attach(|py| plan.execute(self.0.bind(py), key))?; + Ok(bytes.map(|bytes| Box::new(bytes.into_iter().map(Ok)) as _)) + } + + fn size_key(&self, key: &StoreKey) -> Result, StorageError> { + Python::attach(|py| { + let store = self.0.bind(py); + let result = store.call_method1("head", (key.as_str(),)); + let Some(object_meta) = missing_as_none(py, result)? else { + return Ok(None); + }; + let size = object_meta + .getattr("size") + .and_then(|size| size.extract()) + .map_err(map_py_err)?; + Ok(Some(size)) + }) + } + + fn supports_get_partial(&self) -> bool { + true + } +} + +impl ListableStorageTraits for ObspecStore { + fn list(&self) -> Result { + todo!() + } + + fn list_prefix(&self, prefix: &StorePrefix) -> Result { + todo!() + } + + fn list_dir(&self, prefix: &StorePrefix) -> Result { + todo!() + } + + fn size_prefix(&self, prefix: &StorePrefix) -> Result { + todo!() + } +} + +impl WritableStorageTraits for ObspecStore { + fn erase(&self, key: &StoreKey) -> Result<(), StorageError> { + todo!() + } + + fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> { + todo!() + } + + fn set(&self, key: &StoreKey, bytes: Bytes) -> Result<(), StorageError> { + todo!() + } + + fn set_partial_many( + &self, + key: &StoreKey, + offset_values: OffsetBytesIterator, + ) -> Result<(), StorageError> { + todo!() + } + + fn supports_set_partial(&self) -> bool { + false + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn plan(byte_ranges: Vec) -> ReadRanges { + ReadRanges::new(Box::new(byte_ranges.into_iter())) + } + + fn bounded(ranges: &BoundedRanges) -> Vec<(usize, u64, u64)> { + ranges + .0 + .iter() + .map(|range| (range.index, range.start, range.end)) + .collect() + } + + fn open_ended(ranges: &OpenEndedRanges) -> Vec<(usize, u64)> { + ranges + .0 + .iter() + .map(|range| (range.index, range.value)) + .collect() + } + + #[test] + fn partitions_each_byte_range_shape() { + let plan = plan(vec![ + ByteRange::FromStart(10, Some(5)), + ByteRange::Suffix(8), + ByteRange::FromStart(20, None), + ByteRange::FromStart(0, Some(4)), + ]); + + assert_eq!(bounded(&plan.bounded), vec![(0, 10, 15), (3, 0, 4)]); + assert_eq!(open_ended(&plan.offset), vec![(2, 20)]); + assert_eq!(open_ended(&plan.suffix), vec![(1, 8)]); + } + + #[test] + fn indices_partition_the_requested_ranges() { + let plan = plan(vec![ + ByteRange::Suffix(8), + ByteRange::FromStart(20, None), + ByteRange::FromStart(10, Some(5)), + ]); + + let mut indices = bounded(&plan.bounded) + .into_iter() + .map(|(index, ..)| index) + .chain(open_ended(&plan.offset).into_iter().map(|(index, _)| index)) + .chain(open_ended(&plan.suffix).into_iter().map(|(index, _)| index)) + .collect::>(); + indices.sort_unstable(); + + assert_eq!(plan.len(), 3); + assert_eq!(indices, vec![0, 1, 2]); + } +} diff --git a/src/storage/sync/read_only.rs b/src/storage/sync/read_only.rs new file mode 100644 index 0000000..ca05cbc --- /dev/null +++ b/src/storage/sync/read_only.rs @@ -0,0 +1,90 @@ +use std::sync::Arc; +use zarrs::storage::byte_range::{ByteRange, ByteRangeIterator}; +use zarrs::storage::{ + Bytes, ListableStorageTraits, MaybeBytes, MaybeBytesIterator, OffsetBytesIterator, + ReadableListableStorageTraits, ReadableStorageTraits, StorageError, StoreKey, StoreKeys, + StoreKeysPrefixes, StorePrefix, WritableStorageTraits, +}; + +/// A storage adapter that reads and lists transparently but rejects all writes at runtime. +pub struct ReadOnlyStorageAdapter(Arc); + +impl ReadOnlyStorageAdapter { + pub fn new(inner: Arc) -> Self { + Self(inner) + } +} + +impl ReadableStorageTraits for ReadOnlyStorageAdapter { + fn get(&self, key: &StoreKey) -> Result { + self.0.get(key) + } + + fn get_partial_many<'a>( + &'a self, + key: &StoreKey, + byte_ranges: ByteRangeIterator<'a>, + ) -> Result, StorageError> { + self.0.get_partial_many(key, byte_ranges) + } + + fn get_partial( + &self, + key: &StoreKey, + byte_range: ByteRange, + ) -> Result { + self.0.get_partial(key, byte_range) + } + + fn size_key(&self, key: &StoreKey) -> Result, StorageError> { + self.0.size_key(key) + } + + fn supports_get_partial(&self) -> bool { + self.0.supports_get_partial() + } +} + +impl ListableStorageTraits for ReadOnlyStorageAdapter { + fn list(&self) -> Result { + self.0.list() + } + + fn list_prefix(&self, prefix: &StorePrefix) -> Result { + self.0.list_prefix(prefix) + } + + fn list_dir(&self, prefix: &StorePrefix) -> Result { + self.0.list_dir(prefix) + } + + fn size_prefix(&self, prefix: &StorePrefix) -> Result { + self.0.size_prefix(prefix) + } +} + +impl WritableStorageTraits for ReadOnlyStorageAdapter { + fn set(&self, _key: &StoreKey, _value: Bytes) -> Result<(), StorageError> { + Err(StorageError::ReadOnly) + } + + fn set_partial_many( + &self, + _key: &StoreKey, + _offset_values: OffsetBytesIterator, + ) -> Result<(), StorageError> { + Err(StorageError::ReadOnly) + } + + fn erase(&self, _key: &StoreKey) -> Result<(), StorageError> { + Err(StorageError::ReadOnly) + } + + fn erase_prefix(&self, _prefix: &StorePrefix) -> Result<(), StorageError> { + Err(StorageError::ReadOnly) + } + + fn supports_set_partial(&self) -> bool { + false + } +}