[Storage] contiguous::Journal::append_many returns error on empty argument (#3562)

Author: danlaine

storage/src/journal/contiguous/fixed.rs

@@ -655,10 +655,10 @@ impl<E: Context, A: CodecFixedShared> Journal<E, A> {
     /// Append items to the journal, returning the position of the last item appended.
     ///
     /// Acquires the write lock once for all items instead of per-item.
-    /// No-ops if items is empty, returning the current size (next append position).
+    /// Returns [Error::EmptyAppend] if items is empty.
     pub async fn append_many<'a>(&'a self, items: Many<'a, A>) -> Result<u64, Error> {
         if items.is_empty() {
-            return Ok(self.inner.read().await.size);
+            return Err(Error::EmptyAppend);
         }
 
         // Encode all items into a single contiguous buffer before taking the write guard.

storage/src/journal/contiguous/mod.rs

@@ -114,7 +114,7 @@ pub trait Mutable: Contiguous + Send + Sync {
     /// The default implementation calls [Self::append] in a loop. Concrete implementations
     /// may override this to acquire the write lock once for all items.
     ///
-    /// No-ops if items is empty, returning the current size (next append position).
+    /// Returns [Error::EmptyAppend] if items is empty.
     fn append_many<'a>(
         &'a mut self,
         items: Many<'a, Self::Item>,
@@ -123,6 +123,9 @@ pub trait Mutable: Contiguous + Send + Sync {
         Self::Item: Sync,
     {
         async move {
+            if items.is_empty() {
+                return Err(Error::EmptyAppend);
+            }
             let mut last_pos = self.size().await;
             match items {
                 Many::Flat(items) => {

storage/src/journal/contiguous/tests.rs

@@ -1110,7 +1110,7 @@ where
     }
 }
 
-/// Test append_many with empty slice is a no-op.
+/// Test append_many with empty slice returns an error.
 async fn test_append_many_empty<F, J>(factory: &F)
 where
     F: Fn(String) -> BoxFuture<'static, Result<J, Error>>,
@@ -1122,9 +1122,11 @@ where
     journal.append(&10).await.unwrap();
     journal.append(&20).await.unwrap();
 
-    // append_many with empty slice should no-op and return current size.
-    let pos = journal.append_many(Many::Flat(&[])).await.unwrap();
-    assert_eq!(pos, 2);
+    // append_many with empty slice should return an error.
+    assert!(matches!(
+        journal.append_many(Many::Flat(&[])).await,
+        Err(Error::EmptyAppend)
+    ));
     assert_eq!(get_bounds(&journal).await.end, 2);
 
     journal.destroy().await.unwrap();

storage/src/journal/contiguous/variable.rs

@@ -521,10 +521,10 @@ impl<E: Context, V: CodecShared> Journal<E, V> {
     /// Append items to the journal, returning the position of the last item appended.
     ///
     /// Acquires the write lock once for all items instead of per-item.
-    /// No-ops if items is empty, returning the current size (next append position).
+    /// Returns [Error::EmptyAppend] if items is empty.
     pub async fn append_many<'a>(&'a self, items: Many<'a, V>) -> Result<u64, Error> {
         if items.is_empty() {
-            return Ok(self.inner.read().await.size);
+            return Err(Error::EmptyAppend);
         }
 
         // Encode before grabbing write guard.

storage/src/journal/mod.rs

@@ -63,4 +63,6 @@ pub enum Error {
     InvalidConfiguration(String),
     #[error("checksum mismatch: expected={0}, found={1}")]
     ChecksumMismatch(u32, u32),
+    #[error("empty append")]
+    EmptyAppend,
 }