Comments, errors, etc.

Signed-off-by: itowlson <[email protected]>

Author: itowlson

crates/factor-blobstore/src/host.rs

@@ -17,18 +17,36 @@ mod outgoing_value;
 
 pub(crate) use outgoing_value::OutgoingValue;
 
+use crate::DelegatingContainerManager;
+
 // TODO: I feel like the notions of "container" and "container manager" are muddled.
 // This was kinda modelled on the KV StoreManager but I am not sure it has worked.
 // A "container manager" actually manages only one container, making the `get` and
 // `is_defined` functions seemingly redundant. More clarity and better definition
 // is needed here, although the existing code does work!
-
+//
+// Part of the trouble is, I think, that the WIT has operations for "create container"
+// etc. which implies a level above "container" but whose semantics are very poorly
+// defined (the implication in the WIT is that a `blobstore` implementation backs
+// onto exactly one provider, and if you need to deal with multiple providers then
+// you need to do some double-import trickery, which does not seem right). Clarification
+// sought via https://github.com/WebAssembly/wasi-blobstore/issues/27, so we may need
+// to do some rework once the authors define it more fully.
+
+/// Allows obtaining a container. The only interesting implementation is
+/// [DelegatingContainerManager] (which is what [BlobStoreDispatch] uses);
+/// other implementations currently manage only one container. (See comments.)
 #[async_trait]
 pub trait ContainerManager: Sync + Send {
     async fn get(&self, name: &str) -> Result<Arc<dyn Container>, Error>;
     fn is_defined(&self, container_name: &str) -> bool;
 }
 
+/// A container. This represents the system or network resource defined by
+/// a label mapping in the runtime config, e.g. a file system directory,
+/// Azure blob storage account, or S3 bucket. This trait is implemented
+/// by providers; it is the interface through which the [BlobStoreDispatch]
+/// WASI host talks to the different implementations.
 #[async_trait]
 pub trait Container: Sync + Send {
     async fn exists(&self) -> anyhow::Result<bool>;
@@ -54,22 +72,27 @@ pub trait Container: Sync + Send {
     async fn list_objects(&self) -> anyhow::Result<Box<dyn ObjectNames>>;
 }
 
+/// An interface implemented by providers when listing objects.
 #[async_trait]
 pub trait ObjectNames: Send + Sync {
     async fn read(&mut self, len: u64) -> anyhow::Result<(Vec<String>, bool)>;
     async fn skip(&mut self, num: u64) -> anyhow::Result<(u64, bool)>;
 }
 
+/// The content of a blob being read from a container. Called by the host to
+/// handle WIT incoming-value methods, and implemented by providers.
+/// providers
 #[async_trait]
 pub trait IncomingData: Send + Sync {
     async fn consume_sync(&mut self) -> anyhow::Result<Vec<u8>>;
     fn consume_async(&mut self) -> wasmtime_wasi::pipe::AsyncReadStream;
     async fn size(&mut self) -> anyhow::Result<u64>;
 }
 
+/// Implements all the WIT host interfaces for wasi-blobstore.
 pub struct BlobStoreDispatch<'a> {
     allowed_containers: &'a HashSet<String>,
-    manager: &'a dyn ContainerManager,
+    manager: &'a DelegatingContainerManager,
     wasi_resources: &'a mut ResourceTable,
     containers: &'a RwLock<Table<Arc<dyn Container>>>,
     incoming_values: &'a RwLock<Table<Box<dyn IncomingData>>>,
@@ -80,7 +103,7 @@ pub struct BlobStoreDispatch<'a> {
 impl<'a> BlobStoreDispatch<'a> {
     pub(crate) fn new(
         allowed_containers: &'a HashSet<String>,
-        manager: &'a dyn ContainerManager,
+        manager: &'a DelegatingContainerManager,
         wasi_resources: &'a mut ResourceTable,
         containers: &'a RwLock<Table<Arc<dyn Container>>>,
         incoming_values: &'a RwLock<Table<Box<dyn IncomingData>>>,
@@ -131,7 +154,7 @@ impl bs::blobstore::Host for BlobStoreDispatch<'_> {
             let rep = self.containers.write().await.push(container).unwrap();
             Ok(Resource::new_own(rep))
         } else {
-            Err("forbidden container".to_owned())
+            Err(format!("Container {name:?} not defined or access denied"))
         }
     }