Storage and array erase methods now return () instead of bool

CHANGELOG.md

@@ -17,6 +17,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
  - Separate `Array` methods into separate files for each storage trait
  - **Breaking**: Remove `_opt` and `par_` variants of `async_retrieve_array_subset` and `async_store_array_subset` (including `_elements` and `_ndarray` variants)
  - Revise `array_write_read` and `async_array_write_read` examples
+ - **Breaking**: Storage `erase`/`erase_values`/`erase_prefix` methods and `Array::erase_chunk` now return `()` instead of `bool` and succeed irrespective of the whether the key/prefix exists
 
 ## [0.8.0] - 2023-12-26
 

src/array/array_async_writable.rs

@@ -231,11 +231,11 @@ impl<TStorage: ?Sized + AsyncWritableStorageTraits> Array<TStorage> {
 
     /// Erase the chunk at `chunk_indices`.
     ///
-    /// Returns true if the chunk was erased, or false if it did not exist.
+    /// Succeeds if the key does not exist.
     ///
     /// # Errors
     /// Returns a [`StorageError`] if there is an underlying store error.
-    pub async fn async_erase_chunk(&self, chunk_indices: &[u64]) -> Result<bool, StorageError> {
+    pub async fn async_erase_chunk(&self, chunk_indices: &[u64]) -> Result<(), StorageError> {
         let storage_handle = Arc::new(StorageHandle::new(&*self.storage));
         let storage_transformer = self
             .storage_transformers()

src/array/array_sync_writable.rs

@@ -313,11 +313,11 @@ impl<TStorage: ?Sized + WritableStorageTraits> Array<TStorage> {
 
     /// Erase the chunk at `chunk_indices`.
     ///
-    /// Returns true if the chunk was erased, or false if it did not exist.
+    /// Succeeds if the chunk does not exist.
     ///
     /// # Errors
     /// Returns a [`StorageError`] if there is an underlying store error.
-    pub fn erase_chunk(&self, chunk_indices: &[u64]) -> Result<bool, StorageError> {
+    pub fn erase_chunk(&self, chunk_indices: &[u64]) -> Result<(), StorageError> {
         let storage_handle = Arc::new(StorageHandle::new(&*self.storage));
         let storage_transformer = self
             .storage_transformers()

src/storage/storage_async.rs

@@ -234,35 +234,30 @@ pub trait AsyncWritableStorageTraits: Send + Sync {
 
     /// Erase a [`StoreKey`].
     ///
-    /// Returns true if the key exists and was erased, or false if the key does not exist.
+    /// Succeeds if the key does not exist.
     ///
     /// # Errors
     /// Returns a [`StorageError`] if there is an underlying storage error.
-    async fn erase(&self, key: &StoreKey) -> Result<bool, StorageError>;
+    async fn erase(&self, key: &StoreKey) -> Result<(), StorageError>;
 
     /// Erase a list of [`StoreKey`].
     ///
-    /// Returns true if all keys existed and were erased, or false if any key does not exist.
-    ///
     /// # Errors
     /// Returns a [`StorageError`] if there is an underlying storage error.
-    async fn erase_values(&self, keys: &[StoreKey]) -> Result<bool, StorageError> {
+    async fn erase_values(&self, keys: &[StoreKey]) -> Result<(), StorageError> {
         let futures_erase = keys.iter().map(|key| self.erase(key));
-        let result = futures::future::join_all(futures_erase)
+        futures::future::join_all(futures_erase)
             .await
             .into_iter()
             .collect::<Result<Vec<_>, _>>()?;
-        let all_deleted = result.iter().all(|b| *b);
-        Ok(all_deleted)
+        Ok(())
     }
 
     /// Erase all [`StoreKey`] under [`StorePrefix`].
     ///
-    /// Returns true if the prefix and all its children were removed.
-    ///
     /// # Errors
-    /// Returns a [`StorageError`] is the prefix is not in the store, or the erase otherwise fails.
-    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError>;
+    /// Returns a [`StorageError`] if there is an underlying storage error.
+    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError>;
 }
 
 /// A supertrait of [`AsyncReadableStorageTraits`] and [`AsyncWritableStorageTraits`].
@@ -396,7 +391,7 @@ pub async fn async_erase_chunk(
     array_path: &NodePath,
     chunk_grid_indices: &[u64],
     chunk_key_encoding: &ChunkKeyEncoding,
-) -> Result<bool, StorageError> {
+) -> Result<(), StorageError> {
     storage
         .erase(&data_key(
             array_path,
@@ -469,7 +464,7 @@ pub async fn async_discover_nodes(
 pub async fn async_erase_node(
     storage: &dyn AsyncWritableStorageTraits,
     path: &NodePath,
-) -> Result<bool, StorageError> {
+) -> Result<(), StorageError> {
     let prefix = path.try_into()?;
     storage.erase_prefix(&prefix).await
 }

src/storage/storage_handle.rs

@@ -95,15 +95,15 @@ impl<TStorage: ?Sized + WritableStorageTraits> WritableStorageTraits
         self.0.set_partial_values(key_start_values)
     }
 
-    fn erase(&self, key: &super::StoreKey) -> Result<bool, super::StorageError> {
+    fn erase(&self, key: &super::StoreKey) -> Result<(), super::StorageError> {
         self.0.erase(key)
     }
 
-    fn erase_values(&self, keys: &[super::StoreKey]) -> Result<bool, super::StorageError> {
+    fn erase_values(&self, keys: &[super::StoreKey]) -> Result<(), super::StorageError> {
         self.0.erase_values(keys)
     }
 
-    fn erase_prefix(&self, prefix: &super::StorePrefix) -> Result<bool, super::StorageError> {
+    fn erase_prefix(&self, prefix: &super::StorePrefix) -> Result<(), super::StorageError> {
         self.0.erase_prefix(prefix)
     }
 }
@@ -193,15 +193,15 @@ impl<TStorage: ?Sized + AsyncWritableStorageTraits> AsyncWritableStorageTraits
         self.0.set_partial_values(key_start_values).await
     }
 
-    async fn erase(&self, key: &super::StoreKey) -> Result<bool, super::StorageError> {
+    async fn erase(&self, key: &super::StoreKey) -> Result<(), super::StorageError> {
         self.0.erase(key).await
     }
 
-    async fn erase_values(&self, keys: &[super::StoreKey]) -> Result<bool, super::StorageError> {
+    async fn erase_values(&self, keys: &[super::StoreKey]) -> Result<(), super::StorageError> {
         self.0.erase_values(keys).await
     }
 
-    async fn erase_prefix(&self, prefix: &super::StorePrefix) -> Result<bool, super::StorageError> {
+    async fn erase_prefix(&self, prefix: &super::StorePrefix) -> Result<(), super::StorageError> {
         self.0.erase_prefix(prefix).await
     }
 }

src/storage/storage_sync.rs

@@ -226,7 +226,7 @@ pub trait WritableStorageTraits: Send + Sync {
     /// # Errors
     ///
     /// Returns a [`StorageError`] if there is an underlying storage error.
-    fn erase(&self, key: &StoreKey) -> Result<bool, StorageError>;
+    fn erase(&self, key: &StoreKey) -> Result<(), StorageError>;
 
     /// Erase a list of [`StoreKey`].
     ///
@@ -235,12 +235,9 @@ pub trait WritableStorageTraits: Send + Sync {
     /// # Errors
     ///
     /// Returns a [`StorageError`] if there is an underlying storage error.
-    fn erase_values(&self, keys: &[StoreKey]) -> Result<bool, StorageError> {
-        let mut all_deleted = true;
-        for key in keys {
-            all_deleted = all_deleted && self.erase(key)?;
-        }
-        Ok(all_deleted)
+    fn erase_values(&self, keys: &[StoreKey]) -> Result<(), StorageError> {
+        keys.iter().try_for_each(|key| self.erase(key))?;
+        Ok(())
     }
 
     /// Erase all [`StoreKey`] under [`StorePrefix`].
@@ -249,7 +246,7 @@ pub trait WritableStorageTraits: Send + Sync {
     ///
     /// # Errors
     /// Returns a [`StorageError`] is the prefix is not in the store, or the erase otherwise fails.
-    fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError>;
+    fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError>;
 }
 
 /// A supertrait of [`ReadableStorageTraits`] and [`WritableStorageTraits`].
@@ -359,14 +356,16 @@ pub fn retrieve_chunk(
 
 /// Erase a chunk.
 ///
+/// Succeeds if the chunk does not exist.
+///
 /// # Errors
 /// Returns a [`StorageError`] if there is an underlying error with the store.
 pub fn erase_chunk(
     storage: &dyn WritableStorageTraits,
     array_path: &NodePath,
     chunk_grid_indices: &[u64],
     chunk_key_encoding: &ChunkKeyEncoding,
-) -> Result<bool, StorageError> {
+) -> Result<(), StorageError> {
     storage.erase(&data_key(
         array_path,
         chunk_grid_indices,
@@ -425,14 +424,14 @@ pub fn discover_nodes(storage: &dyn ListableStorageTraits) -> Result<StoreKeys,
 
 /// Erase a node (group or array) and all of its children.
 ///
-/// Returns true if the node existed and was removed.
+/// Succeeds if the node does not exist.
 ///
 /// # Errors
 /// Returns a [`StorageError`] if there is an underlying error with the store.
 pub fn erase_node(
     storage: &dyn WritableStorageTraits,
     path: &NodePath,
-) -> Result<bool, StorageError> {
+) -> Result<(), StorageError> {
     let prefix = path.try_into()?;
     storage.erase_prefix(&prefix)
 }

src/storage/storage_transformer/performance_metrics.rs

@@ -269,15 +269,15 @@ impl<TStorage: ?Sized + WritableStorageTraits> WritableStorageTraits
         self.storage.set_partial_values(key_start_values)
     }
 
-    fn erase(&self, key: &StoreKey) -> Result<bool, StorageError> {
+    fn erase(&self, key: &StoreKey) -> Result<(), StorageError> {
         self.storage.erase(key)
     }
 
-    fn erase_values(&self, keys: &[StoreKey]) -> Result<bool, StorageError> {
+    fn erase_values(&self, keys: &[StoreKey]) -> Result<(), StorageError> {
         self.storage.erase_values(keys)
     }
 
-    fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError> {
+    fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> {
         self.storage.erase_prefix(prefix)
     }
 }
@@ -408,15 +408,15 @@ impl<TStorage: ?Sized + AsyncWritableStorageTraits> AsyncWritableStorageTraits
         self.storage.set_partial_values(key_start_values).await
     }
 
-    async fn erase(&self, key: &StoreKey) -> Result<bool, StorageError> {
+    async fn erase(&self, key: &StoreKey) -> Result<(), StorageError> {
         self.storage.erase(key).await
     }
 
-    async fn erase_values(&self, keys: &[StoreKey]) -> Result<bool, StorageError> {
+    async fn erase_values(&self, keys: &[StoreKey]) -> Result<(), StorageError> {
         self.storage.erase_values(keys).await
     }
 
-    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError> {
+    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> {
         self.storage.erase_prefix(prefix).await
     }
 }

src/storage/storage_transformer/usage_log.rs

@@ -261,7 +261,7 @@ impl<TStorage: ?Sized + WritableStorageTraits> WritableStorageTraits
         self.storage.set_partial_values(key_start_values)
     }
 
-    fn erase(&self, key: &StoreKey) -> Result<bool, StorageError> {
+    fn erase(&self, key: &StoreKey) -> Result<(), StorageError> {
         writeln!(
             self.handle.lock().unwrap(),
             "{}erase({key:?}",
@@ -270,7 +270,7 @@ impl<TStorage: ?Sized + WritableStorageTraits> WritableStorageTraits
         self.storage.erase(key)
     }
 
-    fn erase_values(&self, keys: &[StoreKey]) -> Result<bool, StorageError> {
+    fn erase_values(&self, keys: &[StoreKey]) -> Result<(), StorageError> {
         writeln!(
             self.handle.lock().unwrap(),
             "{}erase_values({keys:?}",
@@ -279,7 +279,7 @@ impl<TStorage: ?Sized + WritableStorageTraits> WritableStorageTraits
         self.storage.erase_values(keys)
     }
 
-    fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError> {
+    fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> {
         writeln!(
             self.handle.lock().unwrap(),
             "{}erase_prefix({prefix:?}",
@@ -436,7 +436,7 @@ impl<TStorage: ?Sized + AsyncWritableStorageTraits> AsyncWritableStorageTraits
         self.storage.set_partial_values(key_start_values).await
     }
 
-    async fn erase(&self, key: &StoreKey) -> Result<bool, StorageError> {
+    async fn erase(&self, key: &StoreKey) -> Result<(), StorageError> {
         writeln!(
             self.handle.lock().unwrap(),
             "{}erase({key:?}",
@@ -445,7 +445,7 @@ impl<TStorage: ?Sized + AsyncWritableStorageTraits> AsyncWritableStorageTraits
         self.storage.erase(key).await
     }
 
-    async fn erase_values(&self, keys: &[StoreKey]) -> Result<bool, StorageError> {
+    async fn erase_values(&self, keys: &[StoreKey]) -> Result<(), StorageError> {
         writeln!(
             self.handle.lock().unwrap(),
             "{}erase_values({keys:?}",
@@ -454,7 +454,7 @@ impl<TStorage: ?Sized + AsyncWritableStorageTraits> AsyncWritableStorageTraits
         self.storage.erase_values(keys).await
     }
 
-    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError> {
+    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> {
         writeln!(
             self.handle.lock().unwrap(),
             "{}erase_prefix({prefix:?}",

src/storage/store/store_async.rs

@@ -46,6 +46,7 @@ mod test_util {
 
         store.set(&"erase".try_into()?, &[]).await?;
         store.erase(&"erase".try_into()?).await?;
+        store.erase(&"erase".try_into()?).await?; // succeeds
 
         store.set(&"erase_values_0".try_into()?, &[]).await?;
         store.set(&"erase_values_1".try_into()?, &[]).await?;

src/storage/store/store_async/object_store.rs

@@ -156,11 +156,12 @@ impl<T: object_store::ObjectStore> AsyncWritableStorageTraits for AsyncObjectSto
         crate::storage::async_store_set_partial_values(self, key_start_values).await
     }
 
-    async fn erase(&self, key: &StoreKey) -> Result<bool, StorageError> {
-        Ok(handle_result(self.object_store.delete(&key_to_path(key)).await)?.is_some())
+    async fn erase(&self, key: &StoreKey) -> Result<(), StorageError> {
+        handle_result(self.object_store.delete(&key_to_path(key)).await)?;
+        Ok(())
     }
 
-    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError> {
+    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> {
         let prefix: object_store::path::Path = prefix.as_str().into();
         let locations = self
             .object_store
@@ -171,7 +172,7 @@ impl<T: object_store::ObjectStore> AsyncWritableStorageTraits for AsyncObjectSto
             .delete_stream(locations)
             .try_collect::<Vec<Path>>()
             .await?;
-        Ok(true)
+        Ok(())
     }
 }
 

src/storage/store/store_async/opendal.rs

@@ -152,16 +152,14 @@ impl AsyncWritableStorageTraits for AsyncOpendalStore {
         crate::storage::async_store_set_partial_values(self, key_start_values).await
     }
 
-    async fn erase(&self, key: &StoreKey) -> Result<bool, StorageError> {
-        // FIXME: Make erase return ()?
+    async fn erase(&self, key: &StoreKey) -> Result<(), StorageError> {
         self.operator.remove(vec![key.to_string()]).await?;
-        Ok(true)
+        Ok(())
     }
 
-    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<bool, StorageError> {
-        // FIXME: Make erase_prefix return ()?
+    async fn erase_prefix(&self, prefix: &StorePrefix) -> Result<(), StorageError> {
         self.operator.remove_all(prefix.as_str()).await?;
-        Ok(true)
+        Ok(())
     }
 }
 

src/storage/store/store_sync.rs

@@ -45,6 +45,7 @@ mod test_util {
 
         store.set(&"erase".try_into()?, &[])?;
         store.erase(&"erase".try_into()?)?;
+        store.erase(&"erase".try_into()?)?; // succeeds
 
         store.set(&"erase_values_0".try_into()?, &[])?;
         store.set(&"erase_values_1".try_into()?, &[])?;